Re: [U-Boot] [PATCH V3 08/14] dm: adc: add Exynos54xx compatible ADC driver

On 27 October 2015 at 06:08, 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 methods for ADC single channel operations:

• adc_start_channel()
• adc_channel_data()
• adc_stop()

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 Cc: Minkyu Kang mk7.kang@samsung.com Cc: Simon Glass sjg@chromium.org

---

Changes V2:

• new commit - move previous adc driver from SoC directory to drivers/adc

Changes V3:

• rework the driver to fit new API
• remove call to sdelay() from driver's code
• add active_channel to keep consistency between ADC request and result

---

arch/arm/mach-exynos/include/mach/adc.h | 44 ++++++++++ drivers/adc/Kconfig | 9 ++ drivers/adc/Makefile | 1 + drivers/adc/exynos-adc.c | 145 ++++++++++++++++++++++++++++++++ 4 files changed, 199 insertions(+) create mode 100644 drivers/adc/exynos-adc.c

Reviewed-by: Simon Glass sjg@chromium.org

Some things below.

diff --git a/arch/arm/mach-exynos/include/mach/adc.h b/arch/arm/mach-exynos/include/mach/adc.h index a0e26d7..9af51ab 100644 --- a/arch/arm/mach-exynos/include/mach/adc.h +++ b/arch/arm/mach-exynos/include/mach/adc.h @@ -9,6 +9,39 @@ #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

+#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_MASK 0xf +#define ADC_V2_CON2_CHAN_SEL(x) ((x) & ADC_V2_CON2_CHAN_SEL_MASK)

+#define ADC_V2_GET_STATUS_FLAG(x) (((x) >> 2) & 0x1)

Can you please avoid these sorts of macro accessors? Instead define:

#define ADC_V2_STATUS_SHIFT 2 #define ADC_V2_STATUS_MASK (1 << ADC_V2_STATUS_SHIFT)

and then you can put the access directly in the code.

+#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 +54,17 @@ 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;
  

+}; #endif

#endif /* __ASM_ARM_ARCH_ADC_H_ */ diff --git a/drivers/adc/Kconfig b/drivers/adc/Kconfig index b6e226a..223b65e 100644 --- a/drivers/adc/Kconfig +++ b/drivers/adc/Kconfig @@ -10,3 +10,12 @@ config ADC - methods for get Vdd/Vss reference Voltage values with polarity - support supply's phandle with auto-enable - supply polarity setting in fdt

+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
  

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..534e68d --- /dev/null +++ b/drivers/adc/exynos-adc.c @@ -0,0 +1,145 @@ +/*

• • 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>

That should go below common.h

+#include <asm/arch/adc.h>

+struct exynos_adc_priv {

•   int active_channel;
  

•   struct exynos_adc_v2 *regs;
  

+};

+int exynos_adc_channel_data(struct udevice *dev, int channel,

•                       unsigned int *data)
  

+{

•   struct exynos_adc_priv *priv = dev_get_priv(dev);
  

•   struct exynos_adc_v2 *regs = priv->regs;
  

•   if (channel != priv->active_channel) {
  

•           error("Requested channel is not active!");
  

•           return -EINVAL;
  

•   }
  

•   if (ADC_V2_GET_STATUS_FLAG(readl(&regs->status)) != FLAG_CONV_END)
  

•           return -EBUSY;
  

•   *data = readl(&regs->dat) & ADC_V2_DAT_MASK;
  

•   return 0;
  

+}

+int exynos_adc_start_channel(struct udevice *dev, int channel) +{

•   struct exynos_adc_priv *priv = dev_get_priv(dev);
  

•   struct exynos_adc_v2 *regs = priv->regs;
  

•   unsigned int cfg;
  

•   /* Choose channel */
  

•   cfg = readl(&regs->con2);
  

•   cfg &= ~ADC_V2_CON2_CHAN_SEL_MASK;
  

•   cfg |= ADC_V2_CON2_CHAN_SEL(channel);
  

•   writel(cfg, &regs->con2);
  

Can you use:

clrsetbits_le32(&regs->con2, ADC_V2_CON2_CHAN_SEL_MASK, ADC_V2_CON2_CHAN_SEL(channel))

•   /* Start conversion */
  

•   cfg = readl(&regs->con1);
  

•   writel(cfg | ADC_V2_CON1_STC_EN, &regs->con1);
  

setbits_le32

•   priv->active_channel = channel;
  

•   return 0;
  

+}

+int exynos_adc_stop(struct udevice *dev) +{

•   struct exynos_adc_priv *priv = dev_get_priv(dev);
  

•   struct exynos_adc_v2 *regs = priv->regs;
  

•   unsigned int cfg;
  

•   /* Stop conversion */
  

•   cfg = readl(&regs->con1);
  

•   cfg |= ~ADC_V2_CON1_STC_EN;
  

•   writel(cfg, &regs->con1);
  

clrbits_le32

etc.

•   priv->active_channel = -1;
  

•   return 0;
  

+}

+int exynos_adc_probe(struct udevice *dev) +{

•   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_ESEL(ESEL_ADC_EVAL_TIME_20CLK);
  

•   cfg |= ADC_V2_CON2_HIGHF(HIGHF_CONV_RATE_600KSPS);
  

•   writel(cfg, &regs->con2);
  

•   priv->active_channel = -1;
  

•   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);
  

•   if (priv->regs == (struct exynos_adc_v2 *)FDT_ADDR_T_NONE) {
  

•           error("Dev: %s - can't get address!", dev->name);
  

•           return -ENODATA;
  

•   }
  

•   uc_pdata->data_mask = ADC_V2_DAT_MASK;
  

•   uc_pdata->data_format = ADC_DATA_FORMAT_BIN;
  

•   uc_pdata->data_timeout_us = ADC_V2_CONV_TIMEOUT_US;
  

•   /* Mask available channel bits: [0:9] */
  

•   uc_pdata->channel_mask = (2 << ADC_V2_MAX_CHANNEL) - 1;
  

•   return 0;
  

+}

+static const struct adc_ops exynos_adc_ops = {

•   .start_channel = exynos_adc_start_channel,
  

•   .channel_data = exynos_adc_channel_data,
  

•   .stop = exynos_adc_stop,
  

+};

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

•   .probe          = exynos_adc_probe,
  

•   .ofdata_to_platdata = exynos_adc_ofdata_to_platdata,
  

•   .priv_auto_alloc_size = sizeof(struct exynos_adc_priv),
  

+};

1.9.1

Regards, Simon