[PATCH 0/2] iio: adc: exynos_adc: Add support for s3c64xx/s3c24xx ADC

[PATCH 0/2] iio: adc: exynos_adc: Add support for s3c64xx/s3c24xx ADC

[Chanwoo Choi]

This patch add support for s3c64xx/s3c24xx ADC. s3c64xx/s3c24xx is alomost same
as ADCv1. But, s3c64xx/s3c24xx has a little difference from ADCv1 as following:
- ADCMUX register address to select channel
- ADCDAT mask (10bit or 12bit ADC resolution according to SoC version)

This patchset is implemented based on exynos3250 ADC patchset[1].
 [1] http://www.spinics.net/lists/kernel/msg1791299.html

Arnd Bergmann (1):
  iio: adc: exynos_adc: add support for s3c64xx adc

Chanwoo Choi (1):
  iio: adc: exynos_adc: Add support for S3C24xx ADC

 .../devicetree/bindings/arm/samsung/exynos-adc.txt |  12 +-
 drivers/iio/adc/exynos_adc.c                       | 121 ++++++++++++++++++++-
 2 files changed, 129 insertions(+), 4 deletions(-)

-- 
1.8.0

[PATCH 1/2] iio: adc: exynos_adc: add support for s3c64xx adc

[Chanwoo Choi]

From: Arnd Bergmann <arnd@arndb.de>

The ADC in s3c64xx is almost the same as exynosv1, but
has a different 'select' method. Adding this here will be
helpful to move over the existing s3c64xx platform from the
legacy plat-samsung/adc driver to the new exynos-adc.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 .../devicetree/bindings/arm/samsung/exynos-adc.txt |  2 ++
 drivers/iio/adc/exynos_adc.c                       | 32 +++++++++++++++++++++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
index 6d34891..b6e3989 100644
--- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
@@ -16,6 +16,8 @@ Required properties:
 				future controllers.
 			Must be "samsung,exynos3250-adc" for
 				controllers compatible with ADC of Exynos3250.
+			Must be "samsung,s3c6410-adc" for
+				the ADC in s3c6410 and compatibles
 - reg:			Contains ADC register address range (base address and
 			length) and the address of the phy enable register.
 - interrupts: 		Contains the interrupt information for the timer. The
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index 87e0895..05bdd2f12 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -40,12 +40,16 @@
 #include <linux/iio/machine.h>
 #include <linux/iio/driver.h>
 
-/* EXYNOS4412/5250 ADC_V1 registers definitions */
+/* S3C/EXYNOS4412/5250 ADC_V1 registers definitions */
 #define ADC_V1_CON(x)		((x) + 0x00)
+#define ADC_V1_TSC(x)		((x) + 0x04)
 #define ADC_V1_DLY(x)		((x) + 0x08)
 #define ADC_V1_DATX(x)		((x) + 0x0C)
+#define ADC_V1_DATY(x)		((x) + 0x10)
+#define ADC_V1_UPDN(x)		((x) + 0x14)
 #define ADC_V1_INTCLR(x)	((x) + 0x18)
 #define ADC_V1_MUX(x)		((x) + 0x1c)
+#define ADC_V1_CLRINTPNDNUP(x)	((x) + 0x20)
 
 /* Future ADC_V2 registers definitions */
 #define ADC_V2_CON1(x)		((x) + 0x00)
@@ -61,6 +65,9 @@
 #define ADC_V1_CON_PRSCLV(x)	(((x) & 0xFF) << 6)
 #define ADC_V1_CON_STANDBY	(1u << 2)
 
+/* Bit definitions for S3C2410 ADC */
+#define ADC_S3C2410_CON_SELMUX(x) (((x) & 7) <<3)
+
 /* Bit definitions for ADC_V2 */
 #define ADC_V2_CON1_SOFT_RESET	(1u << 2)
 
@@ -217,6 +224,26 @@ static const struct exynos_adc_data const exynos_adc_v1_data = {
 	.start_conv	= exynos_adc_v1_start_conv,
 };
 
+static void exynos_adc_s3c64xx_start_conv(struct exynos_adc *info,
+					  unsigned long addr)
+{
+	u32 con1;
+
+	con1 = readl(ADC_V1_CON(info->regs));
+	con1 &= ~ADC_S3C2410_CON_SELMUX(7);
+	con1 |= ADC_S3C2410_CON_SELMUX(addr);
+	writel(con1 | ADC_CON_EN_START, ADC_V1_CON(info->regs));
+}
+
+static struct exynos_adc_data const exynos_adc_s3c64xx_data = {
+	.num_channels	= MAX_ADC_V1_CHANNELS,
+
+	.init_hw	= exynos_adc_v1_init_hw,
+	.exit_hw	= exynos_adc_v1_exit_hw,
+	.clear_irq	= exynos_adc_v1_clear_irq,
+	.start_conv	= exynos_adc_s3c64xx_start_conv,
+};
+
 static void exynos_adc_v2_init_hw(struct exynos_adc *info)
 {
 	u32 con1, con2;
@@ -285,6 +312,9 @@ static const struct exynos_adc_data const exynos3250_adc_data = {
 
 static const struct of_device_id exynos_adc_match[] = {
 	{
+		.compatible = "samsung,s3c6410-adc",
+		.data = &exynos_adc_s3c64xx_data,
+	}, {
 		.compatible = "samsung,exynos-adc-v1",
 		.data = &exynos_adc_v1_data,
 	}, {
-- 
1.8.0

Re: [PATCH 1/2] iio: adc: exynos_adc: add support for s3c64xx adc

[Hartmut Knaack]

Chanwoo Choi schrieb:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The ADC in s3c64xx is almost the same as exynosv1, but
> has a different 'select' method. Adding this here will be
> helpful to move over the existing s3c64xx platform from the
> legacy plat-samsung/adc driver to the new exynos-adc.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  .../devicetree/bindings/arm/samsung/exynos-adc.txt |  2 ++
>  drivers/iio/adc/exynos_adc.c                       | 32 +++++++++++++++++++++-
>  2 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> index 6d34891..b6e3989 100644
> --- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> @@ -16,6 +16,8 @@ Required properties:
>  				future controllers.
>  			Must be "samsung,exynos3250-adc" for
>  				controllers compatible with ADC of Exynos3250.
> +			Must be "samsung,s3c6410-adc" for
> +				the ADC in s3c6410 and compatibles
>  - reg:			Contains ADC register address range (base address and
>  			length) and the address of the phy enable register.
>  - interrupts: 		Contains the interrupt information for the timer. The
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index 87e0895..05bdd2f12 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -40,12 +40,16 @@
>  #include <linux/iio/machine.h>
>  #include <linux/iio/driver.h>
>  
> -/* EXYNOS4412/5250 ADC_V1 registers definitions */
> +/* S3C/EXYNOS4412/5250 ADC_V1 registers definitions */
>  #define ADC_V1_CON(x)		((x) + 0x00)
> +#define ADC_V1_TSC(x)		((x) + 0x04)
>  #define ADC_V1_DLY(x)		((x) + 0x08)
>  #define ADC_V1_DATX(x)		((x) + 0x0C)
> +#define ADC_V1_DATY(x)		((x) + 0x10)
> +#define ADC_V1_UPDN(x)		((x) + 0x14)
>  #define ADC_V1_INTCLR(x)	((x) + 0x18)
>  #define ADC_V1_MUX(x)		((x) + 0x1c)
> +#define ADC_V1_CLRINTPNDNUP(x)	((x) + 0x20)
>  
>  /* Future ADC_V2 registers definitions */
>  #define ADC_V2_CON1(x)		((x) + 0x00)
> @@ -61,6 +65,9 @@
>  #define ADC_V1_CON_PRSCLV(x)	(((x) & 0xFF) << 6)
>  #define ADC_V1_CON_STANDBY	(1u << 2)
>  
> +/* Bit definitions for S3C2410 ADC */
> +#define ADC_S3C2410_CON_SELMUX(x) (((x) & 7) <<3)
There is a whitespace missing.
> +
>  /* Bit definitions for ADC_V2 */
>  #define ADC_V2_CON1_SOFT_RESET	(1u << 2)
>  
> @@ -217,6 +224,26 @@ static const struct exynos_adc_data const exynos_adc_v1_data = {
>  	.start_conv	= exynos_adc_v1_start_conv,
>  };
>  
> +static void exynos_adc_s3c64xx_start_conv(struct exynos_adc *info,
> +					  unsigned long addr)
> +{
> +	u32 con1;
> +
> +	con1 = readl(ADC_V1_CON(info->regs));
> +	con1 &= ~ADC_S3C2410_CON_SELMUX(7);
> +	con1 |= ADC_S3C2410_CON_SELMUX(addr);
> +	writel(con1 | ADC_CON_EN_START, ADC_V1_CON(info->regs));
> +}
> +
> +static struct exynos_adc_data const exynos_adc_s3c64xx_data = {
> +	.num_channels	= MAX_ADC_V1_CHANNELS,
> +
> +	.init_hw	= exynos_adc_v1_init_hw,
> +	.exit_hw	= exynos_adc_v1_exit_hw,
> +	.clear_irq	= exynos_adc_v1_clear_irq,
> +	.start_conv	= exynos_adc_s3c64xx_start_conv,
> +};
> +
>  static void exynos_adc_v2_init_hw(struct exynos_adc *info)
>  {
>  	u32 con1, con2;
> @@ -285,6 +312,9 @@ static const struct exynos_adc_data const exynos3250_adc_data = {
>  
>  static const struct of_device_id exynos_adc_match[] = {
>  	{
> +		.compatible = "samsung,s3c6410-adc",
> +		.data = &exynos_adc_s3c64xx_data,
> +	}, {
>  		.compatible = "samsung,exynos-adc-v1",
>  		.data = &exynos_adc_v1_data,
>  	}, {

Re: [PATCH 1/2] iio: adc: exynos_adc: add support for s3c64xx adc

[Chanwoo Choi]

On 07/28/2014 03:49 AM, Hartmut Knaack wrote:
> Chanwoo Choi schrieb:
>> From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>>
>> The ADC in s3c64xx is almost the same as exynosv1, but
>> has a different 'select' method. Adding this here will be
>> helpful to move over the existing s3c64xx platform from the
>> legacy plat-samsung/adc driver to the new exynos-adc.
>>
>> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>> Signed-off-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>>  .../devicetree/bindings/arm/samsung/exynos-adc.txt |  2 ++
>>  drivers/iio/adc/exynos_adc.c                       | 32 +++++++++++++++++++++-
>>  2 files changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
>> index 6d34891..b6e3989 100644
>> --- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
>> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
>> @@ -16,6 +16,8 @@ Required properties:
>>  				future controllers.
>>  			Must be "samsung,exynos3250-adc" for
>>  				controllers compatible with ADC of Exynos3250.
>> +			Must be "samsung,s3c6410-adc" for
>> +				the ADC in s3c6410 and compatibles
>>  - reg:			Contains ADC register address range (base address and
>>  			length) and the address of the phy enable register.
>>  - interrupts: 		Contains the interrupt information for the timer. The
>> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
>> index 87e0895..05bdd2f12 100644
>> --- a/drivers/iio/adc/exynos_adc.c
>> +++ b/drivers/iio/adc/exynos_adc.c
>> @@ -40,12 +40,16 @@
>>  #include <linux/iio/machine.h>
>>  #include <linux/iio/driver.h>
>>  
>> -/* EXYNOS4412/5250 ADC_V1 registers definitions */
>> +/* S3C/EXYNOS4412/5250 ADC_V1 registers definitions */
>>  #define ADC_V1_CON(x)		((x) + 0x00)
>> +#define ADC_V1_TSC(x)		((x) + 0x04)
>>  #define ADC_V1_DLY(x)		((x) + 0x08)
>>  #define ADC_V1_DATX(x)		((x) + 0x0C)
>> +#define ADC_V1_DATY(x)		((x) + 0x10)
>> +#define ADC_V1_UPDN(x)		((x) + 0x14)
>>  #define ADC_V1_INTCLR(x)	((x) + 0x18)
>>  #define ADC_V1_MUX(x)		((x) + 0x1c)
>> +#define ADC_V1_CLRINTPNDNUP(x)	((x) + 0x20)
>>  
>>  /* Future ADC_V2 registers definitions */
>>  #define ADC_V2_CON1(x)		((x) + 0x00)
>> @@ -61,6 +65,9 @@
>>  #define ADC_V1_CON_PRSCLV(x)	(((x) & 0xFF) << 6)
>>  #define ADC_V1_CON_STANDBY	(1u << 2)
>>  
>> +/* Bit definitions for S3C2410 ADC */
>> +#define ADC_S3C2410_CON_SELMUX(x) (((x) & 7) <<3)
> There is a whitespace missing.

OK, I'll fix it.

Best Regards,
Chanwoo Choi

[PATCH 2/2] iio: adc: exynos_adc: Add support for S3C24xx ADC

[Chanwoo Choi]

This patch add support for s3c2410/s3c2416/s3c2440/s3c2443 ADC. The s3c24xx
is alomost same as ADCv1. But, There are a little difference as following:
- ADCMUX register address to select channel
- ADCDAT mask (10bit or 12bit ADC resolution according to SoC version)

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 .../devicetree/bindings/arm/samsung/exynos-adc.txt | 10 ++-
 drivers/iio/adc/exynos_adc.c                       | 89 +++++++++++++++++++++-
 2 files changed, 96 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
index b6e3989..fe34c76 100644
--- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
@@ -11,11 +11,19 @@ New driver handles the following
 
 Required properties:
 - compatible:		Must be "samsung,exynos-adc-v1"
-				for exynos4412/5250 controllers.
+				for exynos4412/5250 and s5pv210 controllers.
 			Must be "samsung,exynos-adc-v2" for
 				future controllers.
 			Must be "samsung,exynos3250-adc" for
 				controllers compatible with ADC of Exynos3250.
+			Must be "samsung,s3c2410-adc" for
+				the ADC in s3c2410 and compatibles
+			Must be "samsung,s3c2416-adc" for
+				the ADC in s3c2416 and compatibles
+			Must be "samsung,s3c2440-adc" for
+				the ADC in s3c2440 and compatibles
+			Must be "samsung,s3c2443-adc" for
+				the ADC in s3c2443 and compatibles
 			Must be "samsung,s3c6410-adc" for
 				the ADC in s3c6410 and compatibles
 - reg:			Contains ADC register address range (base address and
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
index 05bdd2f12..7d28032 100644
--- a/drivers/iio/adc/exynos_adc.c
+++ b/drivers/iio/adc/exynos_adc.c
@@ -51,6 +51,9 @@
 #define ADC_V1_MUX(x)		((x) + 0x1c)
 #define ADC_V1_CLRINTPNDNUP(x)	((x) + 0x20)
 
+/* S3C2410 ADC registers definitions */
+#define ADC_S3C2410_MUX(x)	((x) + 0x18)
+
 /* Future ADC_V2 registers definitions */
 #define ADC_V2_CON1(x)		((x) + 0x00)
 #define ADC_V2_CON2(x)		((x) + 0x04)
@@ -67,6 +70,8 @@
 
 /* Bit definitions for S3C2410 ADC */
 #define ADC_S3C2410_CON_SELMUX(x) (((x) & 7) <<3)
+#define ADC_S3C2410_DATX_MASK	0x3FF
+#define ADC_S3C2416_CON_RES_SEL	(1 << 3)
 
 /* Bit definitions for ADC_V2 */
 #define ADC_V2_CON1_SOFT_RESET	(1u << 2)
@@ -84,6 +89,7 @@
 
 /* Bit definitions common for ADC_V1 and ADC_V2 */
 #define ADC_CON_EN_START	(1u << 0)
+#define ADC_CON_EN_START_MASK	(0x3 << 0)
 #define ADC_DATX_MASK		0xFFF
 
 #define EXYNOS_ADC_TIMEOUT	(msecs_to_jiffies(100))
@@ -101,12 +107,14 @@ struct exynos_adc {
 	struct completion	completion;
 
 	u32			value;
+	u32			value2;
 	unsigned int            version;
 };
 
 struct exynos_adc_data {
 	int num_channels;
 	bool needs_sclk;
+	u32 mask;
 
 	void (*init_hw)(struct exynos_adc *info);
 	void (*exit_hw)(struct exynos_adc *info);
@@ -217,6 +225,17 @@ static void exynos_adc_v1_start_conv(struct exynos_adc *info,
 
 static const struct exynos_adc_data const exynos_adc_v1_data = {
 	.num_channels	= MAX_ADC_V1_CHANNELS,
+	.mask		= ADC_DATX_MASK, /* 12bit ADC resolution */
+
+	.init_hw	= exynos_adc_v1_init_hw,
+	.exit_hw	= exynos_adc_v1_exit_hw,
+	.clear_irq	= exynos_adc_v1_clear_irq,
+	.start_conv	= exynos_adc_v1_start_conv,
+};
+
+static struct exynos_adc_data const exynos_adc_s3c24xx_data = {
+	.num_channels	= MAX_ADC_V1_CHANNELS,
+	.mask		= ADC_S3C2410_DATX_MASK, /* 10bit ADC resolution */
 
 	.init_hw	= exynos_adc_v1_init_hw,
 	.exit_hw	= exynos_adc_v1_exit_hw,
@@ -224,6 +243,55 @@ static const struct exynos_adc_data const exynos_adc_v1_data = {
 	.start_conv	= exynos_adc_v1_start_conv,
 };
 
+static void exynos_adc_s3c2416_start_conv(struct exynos_adc *info,
+					  unsigned long addr)
+{
+	u32 con1;
+
+	/* Enable 12bit ADC resolution */
+	con1 = readl(ADC_V1_CON(info->regs));
+	con1 |= ADC_S3C2416_CON_RES_SEL;
+	writel(con1, ADC_V1_CON(info->regs));
+
+	/* Select channel for S3C2416 */
+	writel(addr, ADC_S3C2410_MUX(info->regs));
+
+	con1 = readl(ADC_V1_CON(info->regs));
+	writel(con1 | ADC_CON_EN_START, ADC_V1_CON(info->regs));
+}
+
+static struct exynos_adc_data const exynos_adc_s3c2416_data = {
+	.num_channels	= MAX_ADC_V1_CHANNELS,
+	.mask		= ADC_DATX_MASK, /* 12bit ADC resolution */
+
+	.init_hw	= exynos_adc_v1_init_hw,
+	.exit_hw	= exynos_adc_v1_exit_hw,
+	.clear_irq	= exynos_adc_v1_clear_irq,
+	.start_conv	= exynos_adc_s3c2416_start_conv,
+};
+
+static void exynos_adc_s3c2443_start_conv(struct exynos_adc *info,
+					  unsigned long addr)
+{
+	u32 con1;
+
+	/* Select channel for S3C2433 */
+	writel(addr, ADC_S3C2410_MUX(info->regs));
+
+	con1 = readl(ADC_V1_CON(info->regs));
+	writel(con1 | ADC_CON_EN_START, ADC_V1_CON(info->regs));
+}
+
+static struct exynos_adc_data const exynos_adc_s3c2443_data = {
+	.num_channels	= MAX_ADC_V1_CHANNELS,
+	.mask		= ADC_S3C2410_DATX_MASK, /* 10bit ADC resolution */
+
+	.init_hw	= exynos_adc_v1_init_hw,
+	.exit_hw	= exynos_adc_v1_exit_hw,
+	.clear_irq	= exynos_adc_v1_clear_irq,
+	.start_conv	= exynos_adc_s3c2443_start_conv,
+};
+
 static void exynos_adc_s3c64xx_start_conv(struct exynos_adc *info,
 					  unsigned long addr)
 {
@@ -237,6 +305,7 @@ static void exynos_adc_s3c64xx_start_conv(struct exynos_adc *info,
 
 static struct exynos_adc_data const exynos_adc_s3c64xx_data = {
 	.num_channels	= MAX_ADC_V1_CHANNELS,
+	.mask		= ADC_DATX_MASK, /* 12bit ADC resolution */
 
 	.init_hw	= exynos_adc_v1_init_hw,
 	.exit_hw	= exynos_adc_v1_exit_hw,
@@ -293,6 +362,7 @@ static void exynos_adc_v2_start_conv(struct exynos_adc *info,
 
 static const struct exynos_adc_data const exynos_adc_v2_data = {
 	.num_channels	= MAX_ADC_V2_CHANNELS,
+	.mask		= ADC_DATX_MASK, /* 12bit ADC resolution */
 
 	.init_hw	= exynos_adc_v2_init_hw,
 	.exit_hw	= exynos_adc_v2_exit_hw,
@@ -302,6 +372,7 @@ static const struct exynos_adc_data const exynos_adc_v2_data = {
 
 static const struct exynos_adc_data const exynos3250_adc_data = {
 	.num_channels	= MAX_EXYNOS3250_ADC_CHANNELS,
+	.mask		= ADC_DATX_MASK, /* 12bit ADC resolution */
 	.needs_sclk	= true,
 
 	.init_hw	= exynos_adc_v2_init_hw,
@@ -312,6 +383,18 @@ static const struct exynos_adc_data const exynos3250_adc_data = {
 
 static const struct of_device_id exynos_adc_match[] = {
 	{
+		.compatible = "samsung,s3c2410-adc",
+		.data = &exynos_adc_s3c24xx_data,
+	}, {
+		.compatible = "samsung,s3c2416-adc",
+		.data = &exynos_adc_s3c2416_data,
+	}, {
+		.compatible = "samsung,s3c2440-adc",
+		.data = &exynos_adc_s3c24xx_data,
+	}, {
+		.compatible = "samsung,s3c2443-adc",
+		.data = &exynos_adc_s3c2443_data,
+	}, {
 		.compatible = "samsung,s3c6410-adc",
 		.data = &exynos_adc_s3c64xx_data,
 	}, {
@@ -365,7 +448,7 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
 		ret = -ETIMEDOUT;
 	} else {
 		*val = info->value;
-		*val2 = 0;
+		*val2 = info->value2;
 		ret = IIO_VAL_INT;
 	}
 
@@ -377,9 +460,11 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
 static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
 {
 	struct exynos_adc *info = (struct exynos_adc *)dev_id;
+	u32 mask = info->data->mask;
 
 	/* Read value */
-	info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
+	info->value = readl(ADC_V1_DATX(info->regs)) & mask;
+	info->value2 = readl(ADC_V1_DATY(info->regs)) & mask;
 
 	/* clear irq */
 	if (info->data->clear_irq)
-- 
1.8.0

Re: [PATCH 2/2] iio: adc: exynos_adc: Add support for S3C24xx ADC

[Arnd Bergmann]

On Tuesday 22 July 2014 11:11:14 Chanwoo Choi wrote:
> This patch add support for s3c2410/s3c2416/s3c2440/s3c2443 ADC. The s3c24xx
> is alomost same as ADCv1. But, There are a little difference as following:
> - ADCMUX register address to select channel
> - ADCDAT mask (10bit or 12bit ADC resolution according to SoC version)

Very good, thanks for doing this patch!

(adding Heiko to Cc, he's probably interested in seeing this as well.

One comment:
 
> @@ -101,12 +107,14 @@ struct exynos_adc {
>  	struct completion	completion;
>  
>  	u32			value;
> +	u32			value2;
>  	unsigned int            version;
>  };
> ...
> @@ -365,7 +448,7 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
>  		ret = -ETIMEDOUT;
>  	} else {
>  		*val = info->value;
> -		*val2 = 0;
> +		*val2 = info->value2;
>  		ret = IIO_VAL_INT;
>  	}
>  
> @@ -377,9 +460,11 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
>  static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
>  {
>  	struct exynos_adc *info = (struct exynos_adc *)dev_id;
> +	u32 mask = info->data->mask;
>  
>  	/* Read value */
> -	info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
> +	info->value = readl(ADC_V1_DATX(info->regs)) & mask;
> +	info->value2 = readl(ADC_V1_DATY(info->regs)) & mask;
>  
>  	/* clear irq */
>  	if (info->data->clear_irq)

If I understand it right, this would only be necessary if we want
to do the touchscreen driver as a separate iio client using the
in-kernel interfaces. As Jonathan Cameron commented, we probably
don't want to do that though. Even if we do, it should be a separate
patch and not mixed in with the s3c24xx support.

Aside from this:

Acked-by: Arnd Bergmann <arnd@arndb.de>

	Arnd

Re: [PATCH 2/2] iio: adc: exynos_adc: Add support for S3C24xx ADC

[Heiko Stübner]

Am Dienstag, 22. Juli 2014, 10:39:38 schrieb Arnd Bergmann:
> On Tuesday 22 July 2014 11:11:14 Chanwoo Choi wrote:
> > This patch add support for s3c2410/s3c2416/s3c2440/s3c2443 ADC. The
> > s3c24xx
> > is alomost same as ADCv1. But, There are a little difference as following:
> > - ADCMUX register address to select channel
> > - ADCDAT mask (10bit or 12bit ADC resolution according to SoC version)
> 
> Very good, thanks for doing this patch!
> 
> (adding Heiko to Cc, he's probably interested in seeing this as well.

indeed. Thanks for implementing this.

While trying to build a test setup for this, I noticed two points:

(1) I'm not sure what the second register (a "phy enable register" according
to the binding) is supposed to be.
According to binding and adc code it is mandatory, but I didn't find any
lone adc register in the s3c2416 manual.


(2) You might need something along the lines of:

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 11b048a..088c99a 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -129,7 +129,7 @@ config AT91_ADC
 
 config EXYNOS_ADC
        tristate "Exynos ADC driver support"
-       depends on ARCH_EXYNOS || (OF && COMPILE_TEST)
+       depends on ARCH_EXYNOS || ARCH_S3C24XX || ARCH_S3C64XX || (OF && COMPILE_TEST)
        help
          Core support for the ADC block found in the Samsung EXYNOS series
          of SoCs for drivers such as the touchscreen and hwmon to use to share


Thanks
Heiko

> 
> One comment:
> > @@ -101,12 +107,14 @@ struct exynos_adc {
> > 
> >  	struct completion	completion;
> >  	
> >  	u32			value;
> > 
> > +	u32			value2;
> > 
> >  	unsigned int            version;
> >  
> >  };
> > 
> > ...
> > @@ -365,7 +448,7 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
> > 
> >  		ret = -ETIMEDOUT;
> >  	
> >  	} else {
> >  	
> >  		*val = info->value;
> > 
> > -		*val2 = 0;
> > +		*val2 = info->value2;
> > 
> >  		ret = IIO_VAL_INT;
> >  	
> >  	}
> > 
> > @@ -377,9 +460,11 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
> > 
> >  static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
> >  {
> >  
> >  	struct exynos_adc *info = (struct exynos_adc *)dev_id;
> > 
> > +	u32 mask = info->data->mask;
> > 
> >  	/* Read value */
> > 
> > -	info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
> > +	info->value = readl(ADC_V1_DATX(info->regs)) & mask;
> > +	info->value2 = readl(ADC_V1_DATY(info->regs)) & mask;
> > 
> >  	/* clear irq */
> >  	if (info->data->clear_irq)
> 
> If I understand it right, this would only be necessary if we want
> to do the touchscreen driver as a separate iio client using the
> in-kernel interfaces. As Jonathan Cameron commented, we probably
> don't want to do that though. Even if we do, it should be a separate
> patch and not mixed in with the s3c24xx support.
> 
> Aside from this:
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> 	Arnd

Re: [PATCH 2/2] iio: adc: exynos_adc: Add support for S3C24xx ADC

[Chanwoo Choi]

On 07/22/2014 07:44 PM, Heiko Stübner wrote:
> Am Dienstag, 22. Juli 2014, 10:39:38 schrieb Arnd Bergmann:
>> On Tuesday 22 July 2014 11:11:14 Chanwoo Choi wrote:
>>> This patch add support for s3c2410/s3c2416/s3c2440/s3c2443 ADC. The
>>> s3c24xx
>>> is alomost same as ADCv1. But, There are a little difference as following:
>>> - ADCMUX register address to select channel
>>> - ADCDAT mask (10bit or 12bit ADC resolution according to SoC version)
>>
>> Very good, thanks for doing this patch!
>>
>> (adding Heiko to Cc, he's probably interested in seeing this as well.
> 
> indeed. Thanks for implementing this.
> 
> While trying to build a test setup for this, I noticed two points:
> 
> (1) I'm not sure what the second register (a "phy enable register" according
> to the binding) is supposed to be.
> According to binding and adc code it is mandatory, but I didn't find any
> lone adc register in the s3c2416 manual.

You're right. I don't find ADC_PHY_CONTROL register on s3c2410 datasheet.
So, if 'needs_adc_phy' field is true, exynos-adc would only get 'phy enable register'
from dt node.

-       mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-       info->enable_reg = devm_ioremap_resource(&pdev->dev, mem);
-       if (IS_ERR(info->enable_reg))
-               return PTR_ERR(info->enable_reg);
+
+       if (info->data->needs_adc_phy) {
+               mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+               info->enable_reg = devm_ioremap_resource(&pdev->dev, mem);
+               if (IS_ERR(info->enable_reg))
+                       return PTR_ERR(info->enable_reg);
+       }


> 
> 
> (2) You might need something along the lines of:
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 11b048a..088c99a 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -129,7 +129,7 @@ config AT91_ADC
>  
>  config EXYNOS_ADC
>         tristate "Exynos ADC driver support"
> -       depends on ARCH_EXYNOS || (OF && COMPILE_TEST)
> +       depends on ARCH_EXYNOS || ARCH_S3C24XX || ARCH_S3C64XX || (OF && COMPILE_TEST)
>         help
>           Core support for the ADC block found in the Samsung EXYNOS series
>           of SoCs for drivers such as the touchscreen and hwmon to use to share

OK, I'll modify it as your comment.

Best Regards,
Chanwoo Choi

Re: [PATCH 2/2] iio: adc: exynos_adc: Add support for S3C24xx ADC

[Chanwoo Choi]

On 07/22/2014 05:39 PM, Arnd Bergmann wrote:
> On Tuesday 22 July 2014 11:11:14 Chanwoo Choi wrote:
>> This patch add support for s3c2410/s3c2416/s3c2440/s3c2443 ADC. The s3c24xx
>> is alomost same as ADCv1. But, There are a little difference as following:
>> - ADCMUX register address to select channel
>> - ADCDAT mask (10bit or 12bit ADC resolution according to SoC version)
> 
> Very good, thanks for doing this patch!
> 
> (adding Heiko to Cc, he's probably interested in seeing this as well.
> 
> One comment:
>  
>> @@ -101,12 +107,14 @@ struct exynos_adc {
>>  	struct completion	completion;
>>  
>>  	u32			value;
>> +	u32			value2;
>>  	unsigned int            version;
>>  };
>> ...
>> @@ -365,7 +448,7 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
>>  		ret = -ETIMEDOUT;
>>  	} else {
>>  		*val = info->value;
>> -		*val2 = 0;
>> +		*val2 = info->value2;
>>  		ret = IIO_VAL_INT;
>>  	}
>>  
>> @@ -377,9 +460,11 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
>>  static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
>>  {
>>  	struct exynos_adc *info = (struct exynos_adc *)dev_id;
>> +	u32 mask = info->data->mask;
>>  
>>  	/* Read value */
>> -	info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
>> +	info->value = readl(ADC_V1_DATX(info->regs)) & mask;
>> +	info->value2 = readl(ADC_V1_DATY(info->regs)) & mask;
>>  
>>  	/* clear irq */
>>  	if (info->data->clear_irq)
> 
> If I understand it right, this would only be necessary if we want
> to do the touchscreen driver as a separate iio client using the
> in-kernel interfaces. As Jonathan Cameron commented, we probably
> don't want to do that though. Even if we do, it should be a separate
> patch and not mixed in with the s3c24xx support.

OK, I'll drop this sentence which reading DATY register.

Best Regards,
Chanwoo Choi

> 
> Aside from this:
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> 	Arnd
> 

Re: [PATCH 2/2] iio: adc: exynos_adc: Add support for S3C24xx ADC

[Arnd Bergmann]

On Tuesday 22 July 2014 11:11:14 Chanwoo Choi wrote:
> This patch add support for s3c2410/s3c2416/s3c2440/s3c2443 ADC. The s3c24xx
> is alomost same as ADCv1. But, There are a little difference as following:
> - ADCMUX register address to select channel
> - ADCDAT mask (10bit or 12bit ADC resolution according to SoC version)
> 
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 

While looking at the driver again to see if the touchscreen patch needs
an update for this, I noticed that the s3c24xx variants don't have the
ADC_V1_INTCLR and ADC_V1_CLRINTPNDNUP registers, so I assume your patch
will have to be updated not to acknowledge the interrupts.

It's possible that writing to the missing registers is harmless though and
that you don't need that change.

	Arnd

Re: [PATCH 2/2] iio: adc: exynos_adc: Add support for S3C24xx ADC

[Chanwoo Choi]

On 07/22/2014 09:59 PM, Arnd Bergmann wrote:
> On Tuesday 22 July 2014 11:11:14 Chanwoo Choi wrote:
>> This patch add support for s3c2410/s3c2416/s3c2440/s3c2443 ADC. The s3c24xx
>> is alomost same as ADCv1. But, There are a little difference as following:
>> - ADCMUX register address to select channel
>> - ADCDAT mask (10bit or 12bit ADC resolution according to SoC version)
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>>
> 
> While looking at the driver again to see if the touchscreen patch needs
> an update for this, I noticed that the s3c24xx variants don't have the
> ADC_V1_INTCLR and ADC_V1_CLRINTPNDNUP registers, so I assume your patch
> will have to be updated not to acknowledge the interrupts.
> 
> It's possible that writing to the missing registers is harmless though and
> that you don't need that change.

OK, I'll remove the function pointer of clear_irq for s3c24xx.

Best Regards,
Chanwoo Choi

Re: [PATCH 2/2] iio: adc: exynos_adc: Add support for S3C24xx ADC

[Hartmut Knaack]

Chanwoo Choi schrieb:
> This patch add support for s3c2410/s3c2416/s3c2440/s3c2443 ADC. The s3c24xx
> is alomost same as ADCv1. But, There are a little difference as following:
> - ADCMUX register address to select channel
> - ADCDAT mask (10bit or 12bit ADC resolution according to SoC version)
Hi, just some style issues: better separate things like 10bit with a space to 10 bit, there are some instances of this type in your code. Another issue inline.
>
> Signed-off-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> ---
>  .../devicetree/bindings/arm/samsung/exynos-adc.txt | 10 ++-
>  drivers/iio/adc/exynos_adc.c                       | 89 +++++++++++++++++++++-
>  2 files changed, 96 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> index b6e3989..fe34c76 100644
> --- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
> @@ -11,11 +11,19 @@ New driver handles the following
>  
>  Required properties:
>  - compatible:		Must be "samsung,exynos-adc-v1"
> -				for exynos4412/5250 controllers.
> +				for exynos4412/5250 and s5pv210 controllers.
>  			Must be "samsung,exynos-adc-v2" for
>  				future controllers.
>  			Must be "samsung,exynos3250-adc" for
>  				controllers compatible with ADC of Exynos3250.
> +			Must be "samsung,s3c2410-adc" for
> +				the ADC in s3c2410 and compatibles
> +			Must be "samsung,s3c2416-adc" for
> +				the ADC in s3c2416 and compatibles
> +			Must be "samsung,s3c2440-adc" for
> +				the ADC in s3c2440 and compatibles
> +			Must be "samsung,s3c2443-adc" for
> +				the ADC in s3c2443 and compatibles
>  			Must be "samsung,s3c6410-adc" for
>  				the ADC in s3c6410 and compatibles
>  - reg:			Contains ADC register address range (base address and
> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
> index 05bdd2f12..7d28032 100644
> --- a/drivers/iio/adc/exynos_adc.c
> +++ b/drivers/iio/adc/exynos_adc.c
> @@ -51,6 +51,9 @@
>  #define ADC_V1_MUX(x)		((x) + 0x1c)
>  #define ADC_V1_CLRINTPNDNUP(x)	((x) + 0x20)
>  
> +/* S3C2410 ADC registers definitions */
> +#define ADC_S3C2410_MUX(x)	((x) + 0x18)
> +
>  /* Future ADC_V2 registers definitions */
>  #define ADC_V2_CON1(x)		((x) + 0x00)
>  #define ADC_V2_CON2(x)		((x) + 0x04)
> @@ -67,6 +70,8 @@
>  
>  /* Bit definitions for S3C2410 ADC */
>  #define ADC_S3C2410_CON_SELMUX(x) (((x) & 7) <<3)
> +#define ADC_S3C2410_DATX_MASK	0x3FF
> +#define ADC_S3C2416_CON_RES_SEL	(1 << 3)
Since it is done this way in this driver, better use (1u << 3) here.
>  
>  /* Bit definitions for ADC_V2 */
>  #define ADC_V2_CON1_SOFT_RESET	(1u << 2)
> @@ -84,6 +89,7 @@
>  
>  /* Bit definitions common for ADC_V1 and ADC_V2 */
>  #define ADC_CON_EN_START	(1u << 0)
> +#define ADC_CON_EN_START_MASK	(0x3 << 0)
>  #define ADC_DATX_MASK		0xFFF
>  
>  #define EXYNOS_ADC_TIMEOUT	(msecs_to_jiffies(100))
> @@ -101,12 +107,14 @@ struct exynos_adc {
>  	struct completion	completion;
>  
>  	u32			value;
> +	u32			value2;
>  	unsigned int            version;
>  };
>  
>  struct exynos_adc_data {
>  	int num_channels;
>  	bool needs_sclk;
> +	u32 mask;
>  
>  	void (*init_hw)(struct exynos_adc *info);
>  	void (*exit_hw)(struct exynos_adc *info);
> @@ -217,6 +225,17 @@ static void exynos_adc_v1_start_conv(struct exynos_adc *info,
>  
>  static const struct exynos_adc_data const exynos_adc_v1_data = {
>  	.num_channels	= MAX_ADC_V1_CHANNELS,
> +	.mask		= ADC_DATX_MASK, /* 12bit ADC resolution */
> +
> +	.init_hw	= exynos_adc_v1_init_hw,
> +	.exit_hw	= exynos_adc_v1_exit_hw,
> +	.clear_irq	= exynos_adc_v1_clear_irq,
> +	.start_conv	= exynos_adc_v1_start_conv,
> +};
> +
> +static struct exynos_adc_data const exynos_adc_s3c24xx_data = {
> +	.num_channels	= MAX_ADC_V1_CHANNELS,
> +	.mask		= ADC_S3C2410_DATX_MASK, /* 10bit ADC resolution */
>  
>  	.init_hw	= exynos_adc_v1_init_hw,
>  	.exit_hw	= exynos_adc_v1_exit_hw,
> @@ -224,6 +243,55 @@ static const struct exynos_adc_data const exynos_adc_v1_data = {
>  	.start_conv	= exynos_adc_v1_start_conv,
>  };
>  
> +static void exynos_adc_s3c2416_start_conv(struct exynos_adc *info,
> +					  unsigned long addr)
> +{
> +	u32 con1;
> +
> +	/* Enable 12bit ADC resolution */
> +	con1 = readl(ADC_V1_CON(info->regs));
> +	con1 |= ADC_S3C2416_CON_RES_SEL;
> +	writel(con1, ADC_V1_CON(info->regs));
> +
> +	/* Select channel for S3C2416 */
> +	writel(addr, ADC_S3C2410_MUX(info->regs));
> +
> +	con1 = readl(ADC_V1_CON(info->regs));
> +	writel(con1 | ADC_CON_EN_START, ADC_V1_CON(info->regs));
> +}
> +
> +static struct exynos_adc_data const exynos_adc_s3c2416_data = {
> +	.num_channels	= MAX_ADC_V1_CHANNELS,
> +	.mask		= ADC_DATX_MASK, /* 12bit ADC resolution */
> +
> +	.init_hw	= exynos_adc_v1_init_hw,
> +	.exit_hw	= exynos_adc_v1_exit_hw,
> +	.clear_irq	= exynos_adc_v1_clear_irq,
> +	.start_conv	= exynos_adc_s3c2416_start_conv,
> +};
> +
> +static void exynos_adc_s3c2443_start_conv(struct exynos_adc *info,
> +					  unsigned long addr)
> +{
> +	u32 con1;
> +
> +	/* Select channel for S3C2433 */
> +	writel(addr, ADC_S3C2410_MUX(info->regs));
> +
> +	con1 = readl(ADC_V1_CON(info->regs));
> +	writel(con1 | ADC_CON_EN_START, ADC_V1_CON(info->regs));
> +}
> +
> +static struct exynos_adc_data const exynos_adc_s3c2443_data = {
> +	.num_channels	= MAX_ADC_V1_CHANNELS,
> +	.mask		= ADC_S3C2410_DATX_MASK, /* 10bit ADC resolution */
> +
> +	.init_hw	= exynos_adc_v1_init_hw,
> +	.exit_hw	= exynos_adc_v1_exit_hw,
> +	.clear_irq	= exynos_adc_v1_clear_irq,
> +	.start_conv	= exynos_adc_s3c2443_start_conv,
> +};
> +
>  static void exynos_adc_s3c64xx_start_conv(struct exynos_adc *info,
>  					  unsigned long addr)
>  {
> @@ -237,6 +305,7 @@ static void exynos_adc_s3c64xx_start_conv(struct exynos_adc *info,
>  
>  static struct exynos_adc_data const exynos_adc_s3c64xx_data = {
>  	.num_channels	= MAX_ADC_V1_CHANNELS,
> +	.mask		= ADC_DATX_MASK, /* 12bit ADC resolution */
>  
>  	.init_hw	= exynos_adc_v1_init_hw,
>  	.exit_hw	= exynos_adc_v1_exit_hw,
> @@ -293,6 +362,7 @@ static void exynos_adc_v2_start_conv(struct exynos_adc *info,
>  
>  static const struct exynos_adc_data const exynos_adc_v2_data = {
>  	.num_channels	= MAX_ADC_V2_CHANNELS,
> +	.mask		= ADC_DATX_MASK, /* 12bit ADC resolution */
>  
>  	.init_hw	= exynos_adc_v2_init_hw,
>  	.exit_hw	= exynos_adc_v2_exit_hw,
> @@ -302,6 +372,7 @@ static const struct exynos_adc_data const exynos_adc_v2_data = {
>  
>  static const struct exynos_adc_data const exynos3250_adc_data = {
>  	.num_channels	= MAX_EXYNOS3250_ADC_CHANNELS,
> +	.mask		= ADC_DATX_MASK, /* 12bit ADC resolution */
>  	.needs_sclk	= true,
>  
>  	.init_hw	= exynos_adc_v2_init_hw,
> @@ -312,6 +383,18 @@ static const struct exynos_adc_data const exynos3250_adc_data = {
>  
>  static const struct of_device_id exynos_adc_match[] = {
>  	{
> +		.compatible = "samsung,s3c2410-adc",
> +		.data = &exynos_adc_s3c24xx_data,
> +	}, {
> +		.compatible = "samsung,s3c2416-adc",
> +		.data = &exynos_adc_s3c2416_data,
> +	}, {
> +		.compatible = "samsung,s3c2440-adc",
> +		.data = &exynos_adc_s3c24xx_data,
> +	}, {
> +		.compatible = "samsung,s3c2443-adc",
> +		.data = &exynos_adc_s3c2443_data,
> +	}, {
>  		.compatible = "samsung,s3c6410-adc",
>  		.data = &exynos_adc_s3c64xx_data,
>  	}, {
> @@ -365,7 +448,7 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
>  		ret = -ETIMEDOUT;
>  	} else {
>  		*val = info->value;
> -		*val2 = 0;
> +		*val2 = info->value2;
>  		ret = IIO_VAL_INT;
>  	}
>  
> @@ -377,9 +460,11 @@ static int exynos_read_raw(struct iio_dev *indio_dev,
>  static irqreturn_t exynos_adc_isr(int irq, void *dev_id)
>  {
>  	struct exynos_adc *info = (struct exynos_adc *)dev_id;
> +	u32 mask = info->data->mask;
>  
>  	/* Read value */
> -	info->value = readl(ADC_V1_DATX(info->regs)) & ADC_DATX_MASK;
> +	info->value = readl(ADC_V1_DATX(info->regs)) & mask;
> +	info->value2 = readl(ADC_V1_DATY(info->regs)) & mask;
>  
>  	/* clear irq */
>  	if (info->data->clear_irq)

Re: [PATCH 2/2] iio: adc: exynos_adc: Add support for S3C24xx ADC

[Chanwoo Choi]

On 07/28/2014 05:35 AM, Hartmut Knaack wrote:
> Chanwoo Choi schrieb:
>> This patch add support for s3c2410/s3c2416/s3c2440/s3c2443 ADC. The s3c24xx
>> is alomost same as ADCv1. But, There are a little difference as following:
>> - ADCMUX register address to select channel
>> - ADCDAT mask (10bit or 12bit ADC resolution according to SoC version)
> Hi, just some style issues: better separate things like 10bit with a space to 10 bit, there are some instances of this type in your code. Another issue inline.

OK I'll fix it.

>>
>> Signed-off-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>> ---
>>  .../devicetree/bindings/arm/samsung/exynos-adc.txt | 10 ++-
>>  drivers/iio/adc/exynos_adc.c                       | 89 +++++++++++++++++++++-
>>  2 files changed, 96 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
>> index b6e3989..fe34c76 100644
>> --- a/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
>> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos-adc.txt
>> @@ -11,11 +11,19 @@ New driver handles the following
>>  
>>  Required properties:
>>  - compatible:		Must be "samsung,exynos-adc-v1"
>> -				for exynos4412/5250 controllers.
>> +				for exynos4412/5250 and s5pv210 controllers.
>>  			Must be "samsung,exynos-adc-v2" for
>>  				future controllers.
>>  			Must be "samsung,exynos3250-adc" for
>>  				controllers compatible with ADC of Exynos3250.
>> +			Must be "samsung,s3c2410-adc" for
>> +				the ADC in s3c2410 and compatibles
>> +			Must be "samsung,s3c2416-adc" for
>> +				the ADC in s3c2416 and compatibles
>> +			Must be "samsung,s3c2440-adc" for
>> +				the ADC in s3c2440 and compatibles
>> +			Must be "samsung,s3c2443-adc" for
>> +				the ADC in s3c2443 and compatibles
>>  			Must be "samsung,s3c6410-adc" for
>>  				the ADC in s3c6410 and compatibles
>>  - reg:			Contains ADC register address range (base address and
>> diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c
>> index 05bdd2f12..7d28032 100644
>> --- a/drivers/iio/adc/exynos_adc.c
>> +++ b/drivers/iio/adc/exynos_adc.c
>> @@ -51,6 +51,9 @@
>>  #define ADC_V1_MUX(x)		((x) + 0x1c)
>>  #define ADC_V1_CLRINTPNDNUP(x)	((x) + 0x20)
>>  
>> +/* S3C2410 ADC registers definitions */
>> +#define ADC_S3C2410_MUX(x)	((x) + 0x18)
>> +
>>  /* Future ADC_V2 registers definitions */
>>  #define ADC_V2_CON1(x)		((x) + 0x00)
>>  #define ADC_V2_CON2(x)		((x) + 0x04)
>> @@ -67,6 +70,8 @@
>>  
>>  /* Bit definitions for S3C2410 ADC */
>>  #define ADC_S3C2410_CON_SELMUX(x) (((x) & 7) <<3)
>> +#define ADC_S3C2410_DATX_MASK	0x3FF
>> +#define ADC_S3C2416_CON_RES_SEL	(1 << 3)
> Since it is done this way in this driver, better use (1u << 3) here.

OK, I'll fix it.

Best Regards,
Chanwoo Choi