[U-Boot] [PATCH V2 07/11] dm: adc: add Exynos54xx compatible ADC driver

[Przemyslaw Marczak <p.marczak@samsung.com>]

Hello Simon,

On 10/03/2015 04:28 PM, Simon Glass wrote:
> Hi Przemyslaw,
>
> On 21 September 2015 at 13:26, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
>> This commit adds driver for Exynos54xx ADC subsystem.
>>
>> The driver is implemented using driver model,
>> amd provides ADC uclass's operations:
>> - adc_init()
>> - adc_data()
>>
>> This driver uses sdelay() function on pooling, so it can
>> be used before the delay timer is inited.
>> The basic parameters of ADC conversion, are:
>> - sample rate: 600KSPS
>> - output the data as average of 8 time conversion
>>
>> ADC features:
>> - sample rate: 600KSPS
>> - resolution: 12-bit
>> - channels: 10 (analog multiplexer)
>>
>> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>> ---
>> Changes V2:
>> - new commit - move previous adc driver from SoC directory to drivers/adc
>> ---
>>   arch/arm/mach-exynos/include/mach/adc.h |  45 ++++++++++++++
>>   drivers/adc/Kconfig                     |   9 +++
>>   drivers/adc/Makefile                    |   1 +
>>   drivers/adc/exynos-adc.c                | 102 ++++++++++++++++++++++++++++++++
>>   4 files changed, 157 insertions(+)
>>   create mode 100644 drivers/adc/exynos-adc.c
>>
>> diff --git a/arch/arm/mach-exynos/include/mach/adc.h b/arch/arm/mach-exynos/include/mach/adc.h
>> index a0e26d7..228acf4 100644
>> --- a/arch/arm/mach-exynos/include/mach/adc.h
>> +++ b/arch/arm/mach-exynos/include/mach/adc.h
>> @@ -9,6 +9,38 @@
>>   #ifndef __ASM_ARM_ARCH_ADC_H_
>>   #define __ASM_ARM_ARCH_ADC_H_
>>
>> +#define ADC_V2_CON1_SOFT_RESET         (0x2 << 1)
>> +#define ADC_V2_CON1_STC_EN             (0x1)
>
> No need for brackets on these simple ones
>

Okay, I will remove them.

>> +
>> +#define ADC_V2_CON2_OSEL(x)            (((x) & 0x1) << 10)
>> +#define OSEL_2S                                (0x0)
>> +#define OSEL_BINARY                    (0x1)
>> +#define ADC_V2_CON2_ESEL(x)            (((x) & 0x1) << 9)
>> +#define ESEL_ADC_EVAL_TIME_40CLK       (0x0)
>> +#define ESEL_ADC_EVAL_TIME_20CLK       (0x1)
>> +#define ADC_V2_CON2_HIGHF(x)           (((x) & 0x1) << 8)
>> +#define HIGHF_CONV_RATE_30KSPS         (0x0)
>> +#define HIGHF_CONV_RATE_600KSPS                (0x1)
>> +#define ADC_V2_CON2_C_TIME(x)          (((x) & 0x7) << 4)
>> +#define ADC_V2_CON2_CHAN_SEL(x)                ((x) & 0xf)
>> +
>> +#define ADC_V2_GET_STATUS_FLAG(x)      (((x) >> 2) & 0x1)
>> +#define FLAG_CONV_END                  (0x1)
>> +
>> +#define ADC_V2_INT_DISABLE             (0x0)
>> +#define ADC_V2_INT_ENABLE              (0x1)
>> +#define INT_NOT_GENERATED              (0x0)
>> +#define INT_GENERATED                  (0x1)
>> +
>> +#define ADC_V2_VERSION                 (0x80000008)
>> +
>> +#define ADC_V2_MAX_CHANNEL             (9)
>> +
>> +/* For default 8 time convertion with sample rate 600 kSPS - 15us timeout */
>> +#define ADC_V2_CONV_TIMEOUT_US         (15)
>> +
>> +#define ADC_V2_DAT_MASK                        (0xfff)
>> +
>>   #ifndef __ASSEMBLY__
>>   struct s5p_adc {
>>          unsigned int adccon;
>> @@ -21,6 +53,19 @@ struct s5p_adc {
>>          unsigned int adcmux;
>>          unsigned int adcclrintpndnup;
>>   };
>> +
>> +struct exynos_adc_v2 {
>> +       unsigned int con1;
>> +       unsigned int con2;
>> +       unsigned int status;
>> +       unsigned int dat;
>> +       unsigned int int_en;
>> +       unsigned int int_status;
>> +       unsigned int reserved[2];
>> +       unsigned int version;
>> +};
>> +
>> +int exynos_adc_read_channel(int channel);
>>   #endif
>>
>>   #endif /* __ASM_ARM_ARCH_ADC_H_ */
>> diff --git a/drivers/adc/Kconfig b/drivers/adc/Kconfig
>> index 1cb1a8d..8ee6531 100644
>> --- a/drivers/adc/Kconfig
>> +++ b/drivers/adc/Kconfig
>> @@ -6,3 +6,12 @@ config ADC
>>            - device enable
>>            - conversion init
>>            - conversion start and return data with data mask
>> +
>> +config ADC_EXYNOS
>> +       bool "Enable Exynos 54xx ADC driver"
>> +       help
>> +         This enables basic driver for Exynos ADC compatible with Exynos54xx.
>> +         It provides:
>> +         - 10 analog input channels
>> +         - 12-bit resolution
>> +         - 600 KSPS of sample rate
>
> Great help!
>

Thanks:)

>> diff --git a/drivers/adc/Makefile b/drivers/adc/Makefile
>> index c4d9618..eb85b8b 100644
>> --- a/drivers/adc/Makefile
>> +++ b/drivers/adc/Makefile
>> @@ -6,3 +6,4 @@
>>   #
>>
>>   obj-$(CONFIG_ADC) += adc-uclass.o
>> +obj-$(CONFIG_ADC_EXYNOS) += exynos-adc.o
>> diff --git a/drivers/adc/exynos-adc.c b/drivers/adc/exynos-adc.c
>> new file mode 100644
>> index 0000000..fdffea0
>> --- /dev/null
>> +++ b/drivers/adc/exynos-adc.c
>> @@ -0,0 +1,102 @@
>> +/*
>> + * Copyright (C) 2015 Samsung Electronics
>> + * Przemyslaw Marczak <p.marczak@samsung.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +#include <common.h>
>> +#include <errno.h>
>> +#include <dm.h>
>> +#include <adc.h>
>> +#include <asm/arch/adc.h>
>> +
>> +struct exynos_adc_priv {
>> +       struct exynos_adc_v2 *regs;
>> +};
>> +
>> +extern void sdelay(unsigned long loops);
>
> Can you put this include system.h for exynos, or similar? Also, I
> think the problem is that timer_init() is called after initf_dm().
> There is a patch to add timer support to driver model so perhaps that
> will help?
>

Ok, will move this. And yes, the problem is with too late timer init.
I saw the work for timer uclass, however I would prefer just use this 
function. It can be moved in the future.

I'm trying add simple functionality, move everything to driver model is 
not related to this patch set.

We can add the timer driver later, and then fix each timer call.

>> +
>> +int exynos_adc_data(struct udevice *dev, unsigned int *data)
>> +{
>> +       struct exynos_adc_priv *priv = dev_get_priv(dev);
>> +       struct exynos_adc_v2 *regs = priv->regs;
>> +       unsigned int cfg, timeout_us = ADC_V2_CONV_TIMEOUT_US;
>> +
>> +       /* Start conversion */
>> +       cfg = readl(&regs->con1);
>> +       writel(cfg | ADC_V2_CON1_STC_EN, &regs->con1);
>> +
>> +       while (ADC_V2_GET_STATUS_FLAG(readl(&regs->status)) != FLAG_CONV_END) {
>> +               sdelay(4);
>> +               if (!timeout_us--) {
>> +                       error("ADC conversion timeout!");
>> +                       return -ETIME;
>> +               }
>> +       }
>
> Hmm, we don't want to put delays in drivers. I though that the init()
> method was used to start the conversion, but now I see that it is not.
> I think the uclass is going to have to know how long to wait for the
> conversion to be ready.

Good point, I will move the delay to uclass driver.

>
> How about this for an interface:
>
> adc_start(dev, channel)
> adc_read(dev, channel)
>
> which waits until the conversion is ready on that channel. The driver
> can set up the time that the conversion will be ready (and store it in
> the uclass) when the conversion starts.

Ok, that sounds good.

>
> Can we support conversions on several channels at once?
>

Some hardware can do this, but the Exynos internal ADC is simple and 
allows only for a single channel conversion at a time.

I can add an uclass operation function call, e.g. adc_channels_data(), 
and if it's not implemented by the device driver, then it can get the 
data for each channel in a loop.

Is that good to you?

>> +
>> +       *data = readl(&regs->dat) & ADC_V2_DAT_MASK;
>> +
>> +       return 0;
>> +}
>> +
>> +int exynos_adc_init(struct udevice *dev, int channel)
>> +{
>> +       struct exynos_adc_priv *priv = dev_get_priv(dev);
>> +       struct exynos_adc_v2 *regs = priv->regs;
>> +       unsigned int cfg;
>> +
>> +       /* Check HW version */
>> +       if (readl(&regs->version) != ADC_V2_VERSION) {
>> +               error("This driver supports only ADC v2!");
>> +               return -ENXIO;
>> +       }
>> +
>> +       /* ADC Reset */
>> +       writel(ADC_V2_CON1_SOFT_RESET, &regs->con1);
>> +
>> +       /* Disable INT - will read status only */
>> +       writel(0x0, &regs->int_en);
>> +
>> +       /* CON2 - set conversion parameters */
>> +       cfg = ADC_V2_CON2_C_TIME(3); /* Conversion times: (1 << 3) = 8 */
>> +       cfg |= ADC_V2_CON2_OSEL(OSEL_BINARY);
>> +       cfg |= ADC_V2_CON2_CHAN_SEL(channel);
>> +       cfg |= ADC_V2_CON2_ESEL(ESEL_ADC_EVAL_TIME_20CLK);
>> +       cfg |= ADC_V2_CON2_HIGHF(HIGHF_CONV_RATE_600KSPS);
>> +       writel(cfg, &regs->con2);
>
> Can this function go in probe() instead? The only thing that can't is
> 'channel'. Is that setting which channel does the conversion? If so,
> perhaps that particular setting could move to the start function?
>

Yes, those settings should be done before each conversion.

>> +
>> +       return 0;
>> +}
>> +
>> +int exynos_adc_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +       struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
>> +       struct exynos_adc_priv *priv = dev_get_priv(dev);
>> +
>> +       priv->regs = (struct exynos_adc_v2 *)dev_get_addr(dev);
>> +
>> +       uc_pdata->data_mask = ADC_V2_DAT_MASK;
>> +       /* Channel count starts from 0 */
>> +       uc_pdata->channels_num = ADC_V2_MAX_CHANNEL + 1;
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct adc_ops exynos_adc_ops = {
>> +       .adc_init = exynos_adc_init,
>> +       .adc_data = exynos_adc_data,
>> +};
>> +
>> +static const struct udevice_id exynos_adc_ids[] = {
>> +       { .compatible = "samsung,exynos-adc-v2" },
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(exynos_adc) = {
>> +       .name           = "exynos-adc",
>> +       .id             = UCLASS_ADC,
>> +       .of_match       = exynos_adc_ids,
>> +       .ops            = &exynos_adc_ops,
>> +       .ofdata_to_platdata = exynos_adc_ofdata_to_platdata,
>> +       .priv_auto_alloc_size = sizeof(struct exynos_adc_priv),
>> +};
>> --
>> 1.9.1
>>
>
> Regards,
> Simon
>

Best regards
-- 
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com