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

Simon Glass
2015-10-19T02:20:51Z

Hi Przemyslaw,

On 13 October 2015 at 05:58, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
> 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?

Yes - so long as the loop is not in the driver it is good.

>
>
>>> +
>>> + *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

Regards,
Simon