Hi Przemyslaw, On 13 October 2015 at 05:58, Przemyslaw Marczak <p.marc...@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.marc...@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.marc...@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.marc...@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(®s->con1); >>> + writel(cfg | ADC_V2_CON1_STC_EN, ®s->con1); >>> + >>> + while (ADC_V2_GET_STATUS_FLAG(readl(®s->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(®s->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(®s->version) != ADC_V2_VERSION) { >>> + error("This driver supports only ADC v2!"); >>> + return -ENXIO; >>> + } >>> + >>> + /* ADC Reset */ >>> + writel(ADC_V2_CON1_SOFT_RESET, ®s->con1); >>> + >>> + /* Disable INT - will read status only */ >>> + writel(0x0, ®s->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, ®s->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.marc...@samsung.com Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot