[U-Boot] [U-Boot,1/8] adc: Add driver for Rockchip Saradc

[Philipp Tomsich]

On Wed, 13 Sep 2017, David Wu wrote:

> The ADC can support some channels signal-ended some bits Successive Approximation
> Register (SAR) A/D Converter, like 6-channel and 10-bit. It converts the analog
> input signal into some bits binary digital codes.
>
> Signed-off-by: David Wu <david.wu at rock-chips.com>

Reviewed-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>

Please see below for requested changes.

> ---
> drivers/adc/Kconfig           |   9 ++
> drivers/adc/Makefile          |   1 +
> drivers/adc/rockchip-saradc.c | 188 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 198 insertions(+)
> create mode 100644 drivers/adc/rockchip-saradc.c
>
> diff --git a/drivers/adc/Kconfig b/drivers/adc/Kconfig
> index e5335f7..830fe0f 100644
> --- a/drivers/adc/Kconfig
> +++ b/drivers/adc/Kconfig
> @@ -20,6 +20,15 @@ config ADC_EXYNOS
> 	  - 12-bit resolution
> 	  - 600 KSPS of sample rate
>
> +config SARADC_ROCKCHIP
> +	bool "Enable Rockchip SARADC driver"
> +	help
> +	  This enables driver for Rockchip SARADC.
> +	  It provides:
> +	  - 2~6 analog input channels
> +	  - 1O-bit resolution
> +	  - 1MSPS of sample rate
> +
> config ADC_SANDBOX
> 	bool "Enable Sandbox ADC test driver"
> 	help
> diff --git a/drivers/adc/Makefile b/drivers/adc/Makefile
> index cebf26d..4b5aa69 100644
> --- a/drivers/adc/Makefile
> +++ b/drivers/adc/Makefile
> @@ -8,3 +8,4 @@
> obj-$(CONFIG_ADC) += adc-uclass.o
> obj-$(CONFIG_ADC_EXYNOS) += exynos-adc.o
> obj-$(CONFIG_ADC_SANDBOX) += sandbox.o
> +obj-$(CONFIG_SARADC_ROCKCHIP) += rockchip-saradc.o

Do you feel strongly about the "SARADC_ROCKCHIP" or would "ADC_ROCKCHIP" 
be correct as well?  I don't care either way, but this is the first entry 
here that does not start with CONFIG_ADC_, so I am wondering...

> diff --git a/drivers/adc/rockchip-saradc.c b/drivers/adc/rockchip-saradc.c
> new file mode 100644
> index 0000000..5c7c3d9
> --- /dev/null
> +++ b/drivers/adc/rockchip-saradc.c
> @@ -0,0 +1,188 @@
> +/*
> + * (C) Copyright 2017, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + *
> + * Rockchip Saradc driver for U-Boot

Should this be SARADC (all uppercase)?

> + */
> +
> +#include <asm/io.h>
> +#include <clk.h>
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <adc.h>

Please see https://www.denx.de/wiki/U-Boot/CodingStyle for the include 
order. Please revise.

@Simon: yes, I am quick study ;-)

> +
> +#define SARADC_DATA			0x00
> +
> +#define SARADC_STAS			0x04

Please see https://www.denx.de/wiki/U-Boot/CodingStyle for the guidelines 
on using structures for I/O accesses.  Please revise.

> +#define SARADC_STAS_BUSY		BIT(0)
> +
> +#define SARADC_CTRL			0x08
> +#define SARADC_CTRL_POWER_CTRL		BIT(3)
> +#define SARADC_CTRL_CHN_MASK		0x7
> +#define SARADC_CTRL_IRQ_STATUS		BIT(6)
> +#define SARADC_CTRL_IRQ_ENABLE		BIT(5)
> +
> +#define SARADC_DLY_PU_SOC		0x0c
> +
> +#define SARADC_TIMEOUT			(100 * 1000)
> +
> +struct rockchip_saradc_data {
> +	int				num_bits;
> +	int				num_channels;
> +	unsigned long			clk_rate;
> +};
> +
> +struct rockchip_saradc_priv {
> +	fdt_addr_t				regs;
> +	int 					active_channel;
> +	const struct rockchip_saradc_data	*data;
> +};
> +
> +int rockchip_saradc_channel_data(struct udevice *dev, int channel,
> +			    unsigned int *data)
> +{
> +	struct rockchip_saradc_priv *priv = dev_get_priv(dev);
> +
> +	if (channel != priv->active_channel) {
> +		error("Requested channel is not active!");
> +		return -EINVAL;
> +	}
> +
> +	if ((readl(priv->regs + SARADC_CTRL) & SARADC_CTRL_IRQ_STATUS) != SARADC_CTRL_IRQ_STATUS)
> +		return -EBUSY;
> +
> +	/* Read value */
> +	*data = readl(priv->regs + SARADC_DATA);
> +	*data &= (1 << priv->data->num_bits) - 1;

This recomputes the data_mask (from below).
Can we just use the data_mask again?

> +
> +	/* Power down adc */
> +	writel(0, priv->regs + SARADC_CTRL);
> +
> +	return 0;
> +}
> +
> +int rockchip_saradc_start_channel(struct udevice *dev, int channel)
> +{
> +	struct rockchip_saradc_priv *priv = dev_get_priv(dev);
> +
> +	if (channel < 0 || channel >= priv->data->num_channels) {
> +		error("Requested channel is invalid!");
> +		return -EINVAL;
> +	}
> +
> +	/* 8 clock periods as delay between power up and start cmd */
> +	writel(8, priv->regs + SARADC_DLY_PU_SOC);
> +
> +	/* Select the channel to be used and trigger conversion */
> +	writel(SARADC_CTRL_POWER_CTRL
> +			| (channel & SARADC_CTRL_CHN_MASK) | SARADC_CTRL_IRQ_ENABLE,

This line does not match the style guideline: it is too wide and the 
operator should be before the line-break.

Did you run checkpatch or use patman?

> +		   priv->regs + SARADC_CTRL);
> +
> +	priv->active_channel = channel;
> +
> +	return 0;
> +}
> +
> +int rockchip_saradc_stop(struct udevice *dev)
> +{
> +	struct rockchip_saradc_priv *priv = dev_get_priv(dev);
> +
> +	/* Power down adc */
> +	writel(0, priv->regs + SARADC_CTRL);
> +
> +	priv->active_channel = -1;
> +
> +	return 0;
> +}
> +
> +int rockchip_saradc_probe(struct udevice *dev)
> +{
> +	struct rockchip_saradc_priv *priv = dev_get_priv(dev);
> +	struct clk clk;
> +	int ret;
> +
> +	ret = clk_get_by_index(dev, 0, &clk);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_set_rate(&clk, priv->data->clk_rate);
> +	if (IS_ERR_VALUE(ret))
> +		return ret;
> +
> +	priv->active_channel = -1;
> +
> +	return 0;
> +}
> +
> +int rockchip_saradc_ofdata_to_platdata(struct udevice *dev)
> +{
> +	struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
> +	struct rockchip_saradc_priv *priv = dev_get_priv(dev);
> +	struct rockchip_saradc_data *data =
> +					(struct rockchip_saradc_data *)dev_get_driver_data(dev);
> +
> +	priv->regs = devfdt_get_addr(dev);

Shouldn't this be dev_read_addr?

> +	if (priv->regs == FDT_ADDR_T_NONE) {
> +		error("Dev: %s - can't get address!", dev->name);
> +		return -ENODATA;
> +	}
> +
> +	priv->data = data;
> +	uc_pdata->data_mask = (1 << priv->data->num_bits) - 1;;
> +	uc_pdata->data_format = ADC_DATA_FORMAT_BIN;
> +	uc_pdata->data_timeout_us = SARADC_TIMEOUT / 5;
> +	uc_pdata->channel_mask = (1 << priv->data->num_channels) - 1;
> +
> +	return 0;
> +}
> +
> +static const struct adc_ops rockchip_saradc_ops = {
> +	.start_channel = rockchip_saradc_start_channel,
> +	.channel_data = rockchip_saradc_channel_data,
> +	.stop = rockchip_saradc_stop,
> +};
> +
> +static const struct rockchip_saradc_data saradc_data = {
> +	.num_bits = 10,
> +	.num_channels = 3,
> +	.clk_rate = 1000000,
> +};
> +
> +static const struct rockchip_saradc_data rk3066_tsadc_data = {
> +	.num_bits = 12,
> +	.num_channels = 2,
> +	.clk_rate = 50000,
> +};
> +
> +static const struct rockchip_saradc_data rk3399_saradc_data = {
> +	.num_bits = 10,
> +	.num_channels = 6,
> +	.clk_rate = 1000000,
> +};
> +
> +static const struct udevice_id rockchip_saradc_ids[] = {
> +	{
> +		.compatible = "rockchip,saradc",
> +		.data = (ulong)&saradc_data,
> +	},
> +	{
> +		.compatible = "rockchip,rk3066-tsadc",
> +		.data = (ulong)&rk3066_tsadc_data,
> +	}, {
> +		.compatible = "rockchip,rk3399-saradc",
> +		.data = (ulong)&rk3399_saradc_data,
> +	},
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(rockchip_saradc) = {
> +	.name		= "rockchip_saradc",
> +	.id		= UCLASS_ADC,
> +	.of_match	= rockchip_saradc_ids,
> +	.ops		= &rockchip_saradc_ops,
> +	.probe		= rockchip_saradc_probe,
> +	.ofdata_to_platdata = rockchip_saradc_ofdata_to_platdata,
> +	.priv_auto_alloc_size = sizeof(struct rockchip_saradc_priv),
> +};
>