[PATCH v2 0/8] berlin2-adc fixes and cleanup

[PATCH v2 0/8] berlin2-adc fixes and cleanup

[Hartmut Knaack]

There is some inconsistency in the register definition. Antoine already
confirmed some of them.
The rest are mainly cleanups and code restructuring.

Changes in V2:
  - fix also data mask for temperature data as reported by Antoine Tenart

Hartmut Knaack (8):
  iio:adc:berlin2-adc: Fix register definition
  iio:adc:berlin2-adc: pass up real error code
  iio:adc:berlin2-adc: use GENMASK and BIT for masks
  iio:adc:berlin2-adc: constify iio_chan_spec
  iio:adc:berlin2-adc: use short operator format
  iio:adc:berlin2-adc: use channel-array size directly
  iio:adc:berlin2-adc: enable interrupts with mutex locked
  iio:adc:berlin2-adc: coding style cleanup

 drivers/iio/adc/berlin2-adc.c | 115 ++++++++++++++++++++++--------------------
 1 file changed, 59 insertions(+), 56 deletions(-)

-- 
2.4.6

[PATCH v2 1/8] iio:adc:berlin2-adc: Fix register definition

[Hartmut Knaack]

Active channel number is stored in BERLIN2_SM_CTRL as value, instead of a
bit map.
The masks for channel interrupts and data ready are a 16 bits wide bit
map each, instead of just 4 bits.

Also correct the data mask for the temperature sensor, which was
Reported-by: Antoine Tenart <antoine.tenart@free-electrons.com>

Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/adc/berlin2-adc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
index aecc9ad995ad..26fa6e6cb624 100644
--- a/drivers/iio/adc/berlin2-adc.c
+++ b/drivers/iio/adc/berlin2-adc.c
@@ -26,7 +26,7 @@
 #define BERLIN2_SM_CTRL				0x14
 #define  BERLIN2_SM_CTRL_SM_SOC_INT		BIT(1)
 #define  BERLIN2_SM_CTRL_SOC_SM_INT		BIT(2)
-#define  BERLIN2_SM_CTRL_ADC_SEL(x)		(BIT(x) << 5)	/* 0-15 */
+#define  BERLIN2_SM_CTRL_ADC_SEL(x)		((x) << 5)	/* 0-15 */
 #define  BERLIN2_SM_CTRL_ADC_SEL_MASK		(0xf << 5)
 #define  BERLIN2_SM_CTRL_ADC_POWER		BIT(9)
 #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV2	(0x0 << 10)
@@ -53,14 +53,14 @@
 #define  BERLIN2_SM_ADC_MASK			0x3ff
 #define BERLIN2_SM_ADC_STATUS			0x1c
 #define  BERLIN2_SM_ADC_STATUS_DATA_RDY(x)	BIT(x)		/* 0-15 */
-#define  BERLIN2_SM_ADC_STATUS_DATA_RDY_MASK	0xf
+#define  BERLIN2_SM_ADC_STATUS_DATA_RDY_MASK	GENMASK(15, 0)
 #define  BERLIN2_SM_ADC_STATUS_INT_EN(x)	(BIT(x) << 16)	/* 0-15 */
-#define  BERLIN2_SM_ADC_STATUS_INT_EN_MASK	(0xf << 16)
+#define  BERLIN2_SM_ADC_STATUS_INT_EN_MASK	GENMASK(31, 16)
 #define BERLIN2_SM_TSEN_STATUS			0x24
 #define  BERLIN2_SM_TSEN_STATUS_DATA_RDY	BIT(0)
 #define  BERLIN2_SM_TSEN_STATUS_INT_EN		BIT(1)
 #define BERLIN2_SM_TSEN_DATA			0x28
-#define  BERLIN2_SM_TSEN_MASK			0xfff
+#define  BERLIN2_SM_TSEN_MASK			GENMASK(9, 0)
 #define BERLIN2_SM_TSEN_CTRL			0x74
 #define  BERLIN2_SM_TSEN_CTRL_START		BIT(8)
 #define  BERLIN2_SM_TSEN_CTRL_SETTLING_4	(0x0 << 21)	/* 4 us */
-- 
2.4.6

Re: [PATCH v2 1/8] iio:adc:berlin2-adc: Fix register definition

[Antoine Tenart]

On Tue, Jul 28, 2015 at 12:38:57AM +0200, Hartmut Knaack wrote:
> Active channel number is stored in BERLIN2_SM_CTRL as value, instead of a
> bit map.
> The masks for channel interrupts and data ready are a 16 bits wide bit
> map each, instead of just 4 bits.
> 
> Also correct the data mask for the temperature sensor, which was
> Reported-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> 
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>

Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>

> ---
>  drivers/iio/adc/berlin2-adc.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
> index aecc9ad995ad..26fa6e6cb624 100644
> --- a/drivers/iio/adc/berlin2-adc.c
> +++ b/drivers/iio/adc/berlin2-adc.c
> @@ -26,7 +26,7 @@
>  #define BERLIN2_SM_CTRL				0x14
>  #define  BERLIN2_SM_CTRL_SM_SOC_INT		BIT(1)
>  #define  BERLIN2_SM_CTRL_SOC_SM_INT		BIT(2)
> -#define  BERLIN2_SM_CTRL_ADC_SEL(x)		(BIT(x) << 5)	/* 0-15 */
> +#define  BERLIN2_SM_CTRL_ADC_SEL(x)		((x) << 5)	/* 0-15 */
>  #define  BERLIN2_SM_CTRL_ADC_SEL_MASK		(0xf << 5)
>  #define  BERLIN2_SM_CTRL_ADC_POWER		BIT(9)
>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV2	(0x0 << 10)
> @@ -53,14 +53,14 @@
>  #define  BERLIN2_SM_ADC_MASK			0x3ff
>  #define BERLIN2_SM_ADC_STATUS			0x1c
>  #define  BERLIN2_SM_ADC_STATUS_DATA_RDY(x)	BIT(x)		/* 0-15 */
> -#define  BERLIN2_SM_ADC_STATUS_DATA_RDY_MASK	0xf
> +#define  BERLIN2_SM_ADC_STATUS_DATA_RDY_MASK	GENMASK(15, 0)
>  #define  BERLIN2_SM_ADC_STATUS_INT_EN(x)	(BIT(x) << 16)	/* 0-15 */
> -#define  BERLIN2_SM_ADC_STATUS_INT_EN_MASK	(0xf << 16)
> +#define  BERLIN2_SM_ADC_STATUS_INT_EN_MASK	GENMASK(31, 16)
>  #define BERLIN2_SM_TSEN_STATUS			0x24
>  #define  BERLIN2_SM_TSEN_STATUS_DATA_RDY	BIT(0)
>  #define  BERLIN2_SM_TSEN_STATUS_INT_EN		BIT(1)
>  #define BERLIN2_SM_TSEN_DATA			0x28
> -#define  BERLIN2_SM_TSEN_MASK			0xfff
> +#define  BERLIN2_SM_TSEN_MASK			GENMASK(9, 0)
>  #define BERLIN2_SM_TSEN_CTRL			0x74
>  #define  BERLIN2_SM_TSEN_CTRL_START		BIT(8)
>  #define  BERLIN2_SM_TSEN_CTRL_SETTLING_4	(0x0 << 21)	/* 4 us */
> -- 
> 2.4.6
> 

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH v2 1/8] iio:adc:berlin2-adc: Fix register definition

[Jonathan Cameron]

On 30/07/15 13:47, Antoine Tenart wrote:
> On Tue, Jul 28, 2015 at 12:38:57AM +0200, Hartmut Knaack wrote:
>> Active channel number is stored in BERLIN2_SM_CTRL as value, instead of a
>> bit map.
>> The masks for channel interrupts and data ready are a 16 bits wide bit
>> map each, instead of just 4 bits.
>>
>> Also correct the data mask for the temperature sensor, which was
>> Reported-by: Antoine Tenart <antoine.tenart@free-electrons.com>
>>
>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> 
> Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Applied to the fixes-togreg branch of iio.git

Thanks,

Jonathan
> 
>> ---
>>  drivers/iio/adc/berlin2-adc.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
>> index aecc9ad995ad..26fa6e6cb624 100644
>> --- a/drivers/iio/adc/berlin2-adc.c
>> +++ b/drivers/iio/adc/berlin2-adc.c
>> @@ -26,7 +26,7 @@
>>  #define BERLIN2_SM_CTRL				0x14
>>  #define  BERLIN2_SM_CTRL_SM_SOC_INT		BIT(1)
>>  #define  BERLIN2_SM_CTRL_SOC_SM_INT		BIT(2)
>> -#define  BERLIN2_SM_CTRL_ADC_SEL(x)		(BIT(x) << 5)	/* 0-15 */
>> +#define  BERLIN2_SM_CTRL_ADC_SEL(x)		((x) << 5)	/* 0-15 */
>>  #define  BERLIN2_SM_CTRL_ADC_SEL_MASK		(0xf << 5)
>>  #define  BERLIN2_SM_CTRL_ADC_POWER		BIT(9)
>>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV2	(0x0 << 10)
>> @@ -53,14 +53,14 @@
>>  #define  BERLIN2_SM_ADC_MASK			0x3ff
>>  #define BERLIN2_SM_ADC_STATUS			0x1c
>>  #define  BERLIN2_SM_ADC_STATUS_DATA_RDY(x)	BIT(x)		/* 0-15 */
>> -#define  BERLIN2_SM_ADC_STATUS_DATA_RDY_MASK	0xf
>> +#define  BERLIN2_SM_ADC_STATUS_DATA_RDY_MASK	GENMASK(15, 0)
>>  #define  BERLIN2_SM_ADC_STATUS_INT_EN(x)	(BIT(x) << 16)	/* 0-15 */
>> -#define  BERLIN2_SM_ADC_STATUS_INT_EN_MASK	(0xf << 16)
>> +#define  BERLIN2_SM_ADC_STATUS_INT_EN_MASK	GENMASK(31, 16)
>>  #define BERLIN2_SM_TSEN_STATUS			0x24
>>  #define  BERLIN2_SM_TSEN_STATUS_DATA_RDY	BIT(0)
>>  #define  BERLIN2_SM_TSEN_STATUS_INT_EN		BIT(1)
>>  #define BERLIN2_SM_TSEN_DATA			0x28
>> -#define  BERLIN2_SM_TSEN_MASK			0xfff
>> +#define  BERLIN2_SM_TSEN_MASK			GENMASK(9, 0)
>>  #define BERLIN2_SM_TSEN_CTRL			0x74
>>  #define  BERLIN2_SM_TSEN_CTRL_START		BIT(8)
>>  #define  BERLIN2_SM_TSEN_CTRL_SETTLING_4	(0x0 << 21)	/* 4 us */
>> -- 
>> 2.4.6
>>
> 

[PATCH v2 2/8] iio:adc:berlin2-adc: pass up real error code

[Hartmut Knaack]

Pass up the real error code returned by platform_get_irq_byname().

Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/iio/adc/berlin2-adc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
index 26fa6e6cb624..b31bcf4c7c90 100644
--- a/drivers/iio/adc/berlin2-adc.c
+++ b/drivers/iio/adc/berlin2-adc.c
@@ -301,11 +301,11 @@ static int berlin2_adc_probe(struct platform_device *pdev)
 
 	irq = platform_get_irq_byname(pdev, "adc");
 	if (irq < 0)
-		return -ENODEV;
+		return irq;
 
 	tsen_irq = platform_get_irq_byname(pdev, "tsen");
 	if (tsen_irq < 0)
-		return -ENODEV;
+		return tsen_irq;
 
 	ret = devm_request_irq(&pdev->dev, irq, berlin2_adc_irq, 0,
 			pdev->dev.driver->name, indio_dev);
-- 
2.4.6

Re: [PATCH v2 2/8] iio:adc:berlin2-adc: pass up real error code

[Jonathan Cameron]

On 27/07/15 23:38, Hartmut Knaack wrote:
> Pass up the real error code returned by platform_get_irq_byname().
> 
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Applied to the togreg branch of iio.git.  Initially pushed out
as testing for the autobuilders to play with it.

Jonathan
> ---
>  drivers/iio/adc/berlin2-adc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
> index 26fa6e6cb624..b31bcf4c7c90 100644
> --- a/drivers/iio/adc/berlin2-adc.c
> +++ b/drivers/iio/adc/berlin2-adc.c
> @@ -301,11 +301,11 @@ static int berlin2_adc_probe(struct platform_device *pdev)
>  
>  	irq = platform_get_irq_byname(pdev, "adc");
>  	if (irq < 0)
> -		return -ENODEV;
> +		return irq;
>  
>  	tsen_irq = platform_get_irq_byname(pdev, "tsen");
>  	if (tsen_irq < 0)
> -		return -ENODEV;
> +		return tsen_irq;
>  
>  	ret = devm_request_irq(&pdev->dev, irq, berlin2_adc_irq, 0,
>  			pdev->dev.driver->name, indio_dev);
> 

[PATCH v2 3/8] iio:adc:berlin2-adc: use GENMASK and BIT for masks

[Hartmut Knaack]

Make use of GENMASK for consecutive bitmasks and BIT for single bitmasks.

Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/adc/berlin2-adc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
index b31bcf4c7c90..ed31a0a8d78e 100644
--- a/drivers/iio/adc/berlin2-adc.c
+++ b/drivers/iio/adc/berlin2-adc.c
@@ -27,13 +27,13 @@
 #define  BERLIN2_SM_CTRL_SM_SOC_INT		BIT(1)
 #define  BERLIN2_SM_CTRL_SOC_SM_INT		BIT(2)
 #define  BERLIN2_SM_CTRL_ADC_SEL(x)		((x) << 5)	/* 0-15 */
-#define  BERLIN2_SM_CTRL_ADC_SEL_MASK		(0xf << 5)
+#define  BERLIN2_SM_CTRL_ADC_SEL_MASK		GENMASK(8, 5)
 #define  BERLIN2_SM_CTRL_ADC_POWER		BIT(9)
 #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV2	(0x0 << 10)
 #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV3	(0x1 << 10)
 #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV4	(0x2 << 10)
 #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV8	(0x3 << 10)
-#define  BERLIN2_SM_CTRL_ADC_CLKSEL_MASK	(0x3 << 10)
+#define  BERLIN2_SM_CTRL_ADC_CLKSEL_MASK	GENMASK(11, 10)
 #define  BERLIN2_SM_CTRL_ADC_START		BIT(12)
 #define  BERLIN2_SM_CTRL_ADC_RESET		BIT(13)
 #define  BERLIN2_SM_CTRL_ADC_BANDGAP_RDY	BIT(14)
@@ -50,7 +50,7 @@
 #define  BERLIN2_SM_CTRL_TSEN_MODE_10_50	(0x1 << 22)	/* 10-50 C */
 #define  BERLIN2_SM_CTRL_TSEN_RESET		BIT(29)
 #define BERLIN2_SM_ADC_DATA			0x20
-#define  BERLIN2_SM_ADC_MASK			0x3ff
+#define  BERLIN2_SM_ADC_MASK			GENMASK(9, 0)
 #define BERLIN2_SM_ADC_STATUS			0x1c
 #define  BERLIN2_SM_ADC_STATUS_DATA_RDY(x)	BIT(x)		/* 0-15 */
 #define  BERLIN2_SM_ADC_STATUS_DATA_RDY_MASK	GENMASK(15, 0)
@@ -65,9 +65,9 @@
 #define  BERLIN2_SM_TSEN_CTRL_START		BIT(8)
 #define  BERLIN2_SM_TSEN_CTRL_SETTLING_4	(0x0 << 21)	/* 4 us */
 #define  BERLIN2_SM_TSEN_CTRL_SETTLING_12	(0x1 << 21)	/* 12 us */
-#define  BERLIN2_SM_TSEN_CTRL_SETTLING_MASK	(0x1 << 21)
+#define  BERLIN2_SM_TSEN_CTRL_SETTLING_MASK	BIT(21)
 #define  BERLIN2_SM_TSEN_CTRL_TRIM(x)		((x) << 22)
-#define  BERLIN2_SM_TSEN_CTRL_TRIM_MASK		(0xf << 22)
+#define  BERLIN2_SM_TSEN_CTRL_TRIM_MASK		GENMASK(25, 22)
 
 struct berlin2_adc_priv {
 	struct regmap		*regmap;
-- 
2.4.6

Re: [PATCH v2 3/8] iio:adc:berlin2-adc: use GENMASK and BIT for masks

[Antoine Tenart]

On Tue, Jul 28, 2015 at 12:38:59AM +0200, Hartmut Knaack wrote:
> Make use of GENMASK for consecutive bitmasks and BIT for single bitmasks.
> 
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>

Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>

> ---
>  drivers/iio/adc/berlin2-adc.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
> index b31bcf4c7c90..ed31a0a8d78e 100644
> --- a/drivers/iio/adc/berlin2-adc.c
> +++ b/drivers/iio/adc/berlin2-adc.c
> @@ -27,13 +27,13 @@
>  #define  BERLIN2_SM_CTRL_SM_SOC_INT		BIT(1)
>  #define  BERLIN2_SM_CTRL_SOC_SM_INT		BIT(2)
>  #define  BERLIN2_SM_CTRL_ADC_SEL(x)		((x) << 5)	/* 0-15 */
> -#define  BERLIN2_SM_CTRL_ADC_SEL_MASK		(0xf << 5)
> +#define  BERLIN2_SM_CTRL_ADC_SEL_MASK		GENMASK(8, 5)
>  #define  BERLIN2_SM_CTRL_ADC_POWER		BIT(9)
>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV2	(0x0 << 10)
>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV3	(0x1 << 10)
>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV4	(0x2 << 10)
>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV8	(0x3 << 10)
> -#define  BERLIN2_SM_CTRL_ADC_CLKSEL_MASK	(0x3 << 10)
> +#define  BERLIN2_SM_CTRL_ADC_CLKSEL_MASK	GENMASK(11, 10)
>  #define  BERLIN2_SM_CTRL_ADC_START		BIT(12)
>  #define  BERLIN2_SM_CTRL_ADC_RESET		BIT(13)
>  #define  BERLIN2_SM_CTRL_ADC_BANDGAP_RDY	BIT(14)
> @@ -50,7 +50,7 @@
>  #define  BERLIN2_SM_CTRL_TSEN_MODE_10_50	(0x1 << 22)	/* 10-50 C */
>  #define  BERLIN2_SM_CTRL_TSEN_RESET		BIT(29)
>  #define BERLIN2_SM_ADC_DATA			0x20
> -#define  BERLIN2_SM_ADC_MASK			0x3ff
> +#define  BERLIN2_SM_ADC_MASK			GENMASK(9, 0)
>  #define BERLIN2_SM_ADC_STATUS			0x1c
>  #define  BERLIN2_SM_ADC_STATUS_DATA_RDY(x)	BIT(x)		/* 0-15 */
>  #define  BERLIN2_SM_ADC_STATUS_DATA_RDY_MASK	GENMASK(15, 0)
> @@ -65,9 +65,9 @@
>  #define  BERLIN2_SM_TSEN_CTRL_START		BIT(8)
>  #define  BERLIN2_SM_TSEN_CTRL_SETTLING_4	(0x0 << 21)	/* 4 us */
>  #define  BERLIN2_SM_TSEN_CTRL_SETTLING_12	(0x1 << 21)	/* 12 us */
> -#define  BERLIN2_SM_TSEN_CTRL_SETTLING_MASK	(0x1 << 21)
> +#define  BERLIN2_SM_TSEN_CTRL_SETTLING_MASK	BIT(21)
>  #define  BERLIN2_SM_TSEN_CTRL_TRIM(x)		((x) << 22)
> -#define  BERLIN2_SM_TSEN_CTRL_TRIM_MASK		(0xf << 22)
> +#define  BERLIN2_SM_TSEN_CTRL_TRIM_MASK		GENMASK(25, 22)
>  
>  struct berlin2_adc_priv {
>  	struct regmap		*regmap;
> -- 
> 2.4.6
> 

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH v2 3/8] iio:adc:berlin2-adc: use GENMASK and BIT for masks

[Jonathan Cameron]

On 30/07/15 13:47, Antoine Tenart wrote:
> On Tue, Jul 28, 2015 at 12:38:59AM +0200, Hartmut Knaack wrote:
>> Make use of GENMASK for consecutive bitmasks and BIT for single bitmasks.
>>
>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> 
> Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> 
As you would expect this clashes with the fix in patch 1.  Hence
I'll hold this for now.  As ever I will probably lose it down the
back of the sofa so do poke me if I forget to pick it up later.

Jonathan
>> ---
>>  drivers/iio/adc/berlin2-adc.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
>> index b31bcf4c7c90..ed31a0a8d78e 100644
>> --- a/drivers/iio/adc/berlin2-adc.c
>> +++ b/drivers/iio/adc/berlin2-adc.c
>> @@ -27,13 +27,13 @@
>>  #define  BERLIN2_SM_CTRL_SM_SOC_INT		BIT(1)
>>  #define  BERLIN2_SM_CTRL_SOC_SM_INT		BIT(2)
>>  #define  BERLIN2_SM_CTRL_ADC_SEL(x)		((x) << 5)	/* 0-15 */
>> -#define  BERLIN2_SM_CTRL_ADC_SEL_MASK		(0xf << 5)
>> +#define  BERLIN2_SM_CTRL_ADC_SEL_MASK		GENMASK(8, 5)
>>  #define  BERLIN2_SM_CTRL_ADC_POWER		BIT(9)
>>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV2	(0x0 << 10)
>>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV3	(0x1 << 10)
>>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV4	(0x2 << 10)
>>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV8	(0x3 << 10)
>> -#define  BERLIN2_SM_CTRL_ADC_CLKSEL_MASK	(0x3 << 10)
>> +#define  BERLIN2_SM_CTRL_ADC_CLKSEL_MASK	GENMASK(11, 10)
>>  #define  BERLIN2_SM_CTRL_ADC_START		BIT(12)
>>  #define  BERLIN2_SM_CTRL_ADC_RESET		BIT(13)
>>  #define  BERLIN2_SM_CTRL_ADC_BANDGAP_RDY	BIT(14)
>> @@ -50,7 +50,7 @@
>>  #define  BERLIN2_SM_CTRL_TSEN_MODE_10_50	(0x1 << 22)	/* 10-50 C */
>>  #define  BERLIN2_SM_CTRL_TSEN_RESET		BIT(29)
>>  #define BERLIN2_SM_ADC_DATA			0x20
>> -#define  BERLIN2_SM_ADC_MASK			0x3ff
>> +#define  BERLIN2_SM_ADC_MASK			GENMASK(9, 0)
>>  #define BERLIN2_SM_ADC_STATUS			0x1c
>>  #define  BERLIN2_SM_ADC_STATUS_DATA_RDY(x)	BIT(x)		/* 0-15 */
>>  #define  BERLIN2_SM_ADC_STATUS_DATA_RDY_MASK	GENMASK(15, 0)
>> @@ -65,9 +65,9 @@
>>  #define  BERLIN2_SM_TSEN_CTRL_START		BIT(8)
>>  #define  BERLIN2_SM_TSEN_CTRL_SETTLING_4	(0x0 << 21)	/* 4 us */
>>  #define  BERLIN2_SM_TSEN_CTRL_SETTLING_12	(0x1 << 21)	/* 12 us */
>> -#define  BERLIN2_SM_TSEN_CTRL_SETTLING_MASK	(0x1 << 21)
>> +#define  BERLIN2_SM_TSEN_CTRL_SETTLING_MASK	BIT(21)
>>  #define  BERLIN2_SM_TSEN_CTRL_TRIM(x)		((x) << 22)
>> -#define  BERLIN2_SM_TSEN_CTRL_TRIM_MASK		(0xf << 22)
>> +#define  BERLIN2_SM_TSEN_CTRL_TRIM_MASK		GENMASK(25, 22)
>>  
>>  struct berlin2_adc_priv {
>>  	struct regmap		*regmap;
>> -- 
>> 2.4.6
>>
> 

Re: [PATCH v2 3/8] iio:adc:berlin2-adc: use GENMASK and BIT for masks

[Jonathan Cameron]

On 30/07/15 13:47, Antoine Tenart wrote:
> On Tue, Jul 28, 2015 at 12:38:59AM +0200, Hartmut Knaack wrote:
>> Make use of GENMASK for consecutive bitmasks and BIT for single bitmasks.
>>
>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> 
> Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> 
Applied to the togreg branch of iio.git.

Thanks,

Jonathan
>> ---
>>  drivers/iio/adc/berlin2-adc.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
>> index b31bcf4c7c90..ed31a0a8d78e 100644
>> --- a/drivers/iio/adc/berlin2-adc.c
>> +++ b/drivers/iio/adc/berlin2-adc.c
>> @@ -27,13 +27,13 @@
>>  #define  BERLIN2_SM_CTRL_SM_SOC_INT		BIT(1)
>>  #define  BERLIN2_SM_CTRL_SOC_SM_INT		BIT(2)
>>  #define  BERLIN2_SM_CTRL_ADC_SEL(x)		((x) << 5)	/* 0-15 */
>> -#define  BERLIN2_SM_CTRL_ADC_SEL_MASK		(0xf << 5)
>> +#define  BERLIN2_SM_CTRL_ADC_SEL_MASK		GENMASK(8, 5)
>>  #define  BERLIN2_SM_CTRL_ADC_POWER		BIT(9)
>>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV2	(0x0 << 10)
>>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV3	(0x1 << 10)
>>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV4	(0x2 << 10)
>>  #define  BERLIN2_SM_CTRL_ADC_CLKSEL_DIV8	(0x3 << 10)
>> -#define  BERLIN2_SM_CTRL_ADC_CLKSEL_MASK	(0x3 << 10)
>> +#define  BERLIN2_SM_CTRL_ADC_CLKSEL_MASK	GENMASK(11, 10)
>>  #define  BERLIN2_SM_CTRL_ADC_START		BIT(12)
>>  #define  BERLIN2_SM_CTRL_ADC_RESET		BIT(13)
>>  #define  BERLIN2_SM_CTRL_ADC_BANDGAP_RDY	BIT(14)
>> @@ -50,7 +50,7 @@
>>  #define  BERLIN2_SM_CTRL_TSEN_MODE_10_50	(0x1 << 22)	/* 10-50 C */
>>  #define  BERLIN2_SM_CTRL_TSEN_RESET		BIT(29)
>>  #define BERLIN2_SM_ADC_DATA			0x20
>> -#define  BERLIN2_SM_ADC_MASK			0x3ff
>> +#define  BERLIN2_SM_ADC_MASK			GENMASK(9, 0)
>>  #define BERLIN2_SM_ADC_STATUS			0x1c
>>  #define  BERLIN2_SM_ADC_STATUS_DATA_RDY(x)	BIT(x)		/* 0-15 */
>>  #define  BERLIN2_SM_ADC_STATUS_DATA_RDY_MASK	GENMASK(15, 0)
>> @@ -65,9 +65,9 @@
>>  #define  BERLIN2_SM_TSEN_CTRL_START		BIT(8)
>>  #define  BERLIN2_SM_TSEN_CTRL_SETTLING_4	(0x0 << 21)	/* 4 us */
>>  #define  BERLIN2_SM_TSEN_CTRL_SETTLING_12	(0x1 << 21)	/* 12 us */
>> -#define  BERLIN2_SM_TSEN_CTRL_SETTLING_MASK	(0x1 << 21)
>> +#define  BERLIN2_SM_TSEN_CTRL_SETTLING_MASK	BIT(21)
>>  #define  BERLIN2_SM_TSEN_CTRL_TRIM(x)		((x) << 22)
>> -#define  BERLIN2_SM_TSEN_CTRL_TRIM_MASK		(0xf << 22)
>> +#define  BERLIN2_SM_TSEN_CTRL_TRIM_MASK		GENMASK(25, 22)
>>  
>>  struct berlin2_adc_priv {
>>  	struct regmap		*regmap;
>> -- 
>> 2.4.6
>>
> 

[PATCH v2 4/8] iio:adc:berlin2-adc: constify iio_chan_spec

[Hartmut Knaack]

Mark berlin2_adc_channels array as constant.

Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/iio/adc/berlin2-adc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
index ed31a0a8d78e..745061091bd3 100644
--- a/drivers/iio/adc/berlin2-adc.c
+++ b/drivers/iio/adc/berlin2-adc.c
@@ -86,7 +86,7 @@ struct berlin2_adc_priv {
 			.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
 		}
 
-static struct iio_chan_spec berlin2_adc_channels[] = {
+static const struct iio_chan_spec berlin2_adc_channels[] = {
 	BERLIN2_ADC_CHANNEL(0, IIO_VOLTAGE),	/* external input */
 	BERLIN2_ADC_CHANNEL(1, IIO_VOLTAGE),	/* external input */
 	BERLIN2_ADC_CHANNEL(2, IIO_VOLTAGE),	/* external input */
-- 
2.4.6

Re: [PATCH v2 4/8] iio:adc:berlin2-adc: constify iio_chan_spec

[Jonathan Cameron]

On 27/07/15 23:39, Hartmut Knaack wrote:
> Mark berlin2_adc_channels array as constant.
> 
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Applied
> ---
>  drivers/iio/adc/berlin2-adc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
> index ed31a0a8d78e..745061091bd3 100644
> --- a/drivers/iio/adc/berlin2-adc.c
> +++ b/drivers/iio/adc/berlin2-adc.c
> @@ -86,7 +86,7 @@ struct berlin2_adc_priv {
>  			.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
>  		}
>  
> -static struct iio_chan_spec berlin2_adc_channels[] = {
> +static const struct iio_chan_spec berlin2_adc_channels[] = {
>  	BERLIN2_ADC_CHANNEL(0, IIO_VOLTAGE),	/* external input */
>  	BERLIN2_ADC_CHANNEL(1, IIO_VOLTAGE),	/* external input */
>  	BERLIN2_ADC_CHANNEL(2, IIO_VOLTAGE),	/* external input */
> 

[PATCH v2 5/8] iio:adc:berlin2-adc: use short operator format

[Hartmut Knaack]

Use augmented assignment to subtract the offset for negative temperature
values.
Specify the amount of private data to be allocated through
devm_iio_device_alloc() with sizeof(*priv), as it is shorter and common
practice in IIO.

Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/iio/adc/berlin2-adc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
index 745061091bd3..1e1d618ce152 100644
--- a/drivers/iio/adc/berlin2-adc.c
+++ b/drivers/iio/adc/berlin2-adc.c
@@ -221,7 +221,7 @@ static int berlin2_adc_read_raw(struct iio_dev *indio_dev,
 			return temp;
 
 		if (temp > 2047)
-			temp = -(4096 - temp);
+			temp -= 4096;
 
 		/* Convert to milli Celsius */
 		*val = ((temp * 100000) / 264 - 270000);
@@ -286,8 +286,7 @@ static int berlin2_adc_probe(struct platform_device *pdev)
 	int irq, tsen_irq;
 	int ret;
 
-	indio_dev = devm_iio_device_alloc(&pdev->dev,
-					  sizeof(struct berlin2_adc_priv));
+	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
 	if (!indio_dev)
 		return -ENOMEM;
 
-- 
2.4.6

Re: [PATCH v2 5/8] iio:adc:berlin2-adc: use short operator format

[Jonathan Cameron]

On 27/07/15 23:39, Hartmut Knaack wrote:
> Use augmented assignment to subtract the offset for negative temperature
> values.
> Specify the amount of private data to be allocated through
> devm_iio_device_alloc() with sizeof(*priv), as it is shorter and common
> practice in IIO.
> 
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Applied.
> ---
>  drivers/iio/adc/berlin2-adc.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
> index 745061091bd3..1e1d618ce152 100644
> --- a/drivers/iio/adc/berlin2-adc.c
> +++ b/drivers/iio/adc/berlin2-adc.c
> @@ -221,7 +221,7 @@ static int berlin2_adc_read_raw(struct iio_dev *indio_dev,
>  			return temp;
>  
>  		if (temp > 2047)
> -			temp = -(4096 - temp);
> +			temp -= 4096;
>  
>  		/* Convert to milli Celsius */
>  		*val = ((temp * 100000) / 264 - 270000);
> @@ -286,8 +286,7 @@ static int berlin2_adc_probe(struct platform_device *pdev)
>  	int irq, tsen_irq;
>  	int ret;
>  
> -	indio_dev = devm_iio_device_alloc(&pdev->dev,
> -					  sizeof(struct berlin2_adc_priv));
> +	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
> 

[PATCH v2 6/8] iio:adc:berlin2-adc: use channel-array size directly

[Hartmut Knaack]

Drop the otherwise unused definition of the channel-array size and use it
directly in _probe - makes it a bit more obvious.

Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/iio/adc/berlin2-adc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
index 1e1d618ce152..6e20c856f479 100644
--- a/drivers/iio/adc/berlin2-adc.c
+++ b/drivers/iio/adc/berlin2-adc.c
@@ -103,7 +103,6 @@ static const struct iio_chan_spec berlin2_adc_channels[] = {
 	BERLIN2_ADC_CHANNEL(7, IIO_VOLTAGE),	/* reserved */
 	IIO_CHAN_SOFT_TIMESTAMP(8),		/* timestamp */
 };
-#define BERLIN2_N_CHANNELS	ARRAY_SIZE(berlin2_adc_channels)
 
 static int berlin2_adc_read(struct iio_dev *indio_dev, int channel)
 {
@@ -324,8 +323,8 @@ static int berlin2_adc_probe(struct platform_device *pdev)
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &berlin2_adc_info;
 
-	indio_dev->num_channels = BERLIN2_N_CHANNELS;
 	indio_dev->channels = berlin2_adc_channels;
+	indio_dev->num_channels = ARRAY_SIZE(berlin2_adc_channels);
 
 	/* Power up the ADC */
 	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
-- 
2.4.6

Re: [PATCH v2 6/8] iio:adc:berlin2-adc: use channel-array size directly

[Jonathan Cameron]

On 27/07/15 23:39, Hartmut Knaack wrote:
> Drop the otherwise unused definition of the channel-array size and use it
> directly in _probe - makes it a bit more obvious.
> 
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Applied.
> ---
>  drivers/iio/adc/berlin2-adc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
> index 1e1d618ce152..6e20c856f479 100644
> --- a/drivers/iio/adc/berlin2-adc.c
> +++ b/drivers/iio/adc/berlin2-adc.c
> @@ -103,7 +103,6 @@ static const struct iio_chan_spec berlin2_adc_channels[] = {
>  	BERLIN2_ADC_CHANNEL(7, IIO_VOLTAGE),	/* reserved */
>  	IIO_CHAN_SOFT_TIMESTAMP(8),		/* timestamp */
>  };
> -#define BERLIN2_N_CHANNELS	ARRAY_SIZE(berlin2_adc_channels)
>  
>  static int berlin2_adc_read(struct iio_dev *indio_dev, int channel)
>  {
> @@ -324,8 +323,8 @@ static int berlin2_adc_probe(struct platform_device *pdev)
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->info = &berlin2_adc_info;
>  
> -	indio_dev->num_channels = BERLIN2_N_CHANNELS;
>  	indio_dev->channels = berlin2_adc_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(berlin2_adc_channels);
>  
>  	/* Power up the ADC */
>  	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
> 

[PATCH v2 7/8] iio:adc:berlin2-adc: enable interrupts with mutex locked

[Hartmut Knaack]

Move the call to enable channel interrupts into its _read() function to
have it protected by a mutex. This ensures that only one channel is
sampled at a time.

Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
---
 drivers/iio/adc/berlin2-adc.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
index 6e20c856f479..13cfeb494e34 100644
--- a/drivers/iio/adc/berlin2-adc.c
+++ b/drivers/iio/adc/berlin2-adc.c
@@ -111,6 +111,10 @@ static int berlin2_adc_read(struct iio_dev *indio_dev, int channel)
 
 	mutex_lock(&priv->lock);
 
+	/* Enable the interrupts */
+	regmap_write(priv->regmap, BERLIN2_SM_ADC_STATUS,
+		     BERLIN2_SM_ADC_STATUS_INT_EN(channel));
+
 	/* Configure the ADC */
 	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
 			BERLIN2_SM_CTRL_ADC_RESET | BERLIN2_SM_CTRL_ADC_SEL_MASK
@@ -149,6 +153,10 @@ static int berlin2_adc_tsen_read(struct iio_dev *indio_dev)
 
 	mutex_lock(&priv->lock);
 
+	/* Enable interrupts */
+	regmap_write(priv->regmap, BERLIN2_SM_TSEN_STATUS,
+		     BERLIN2_SM_TSEN_STATUS_INT_EN);
+
 	/* Configure the ADC */
 	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
 			BERLIN2_SM_CTRL_TSEN_RESET | BERLIN2_SM_CTRL_ADC_ROTATE,
@@ -190,7 +198,6 @@ static int berlin2_adc_read_raw(struct iio_dev *indio_dev,
 		struct iio_chan_spec const *chan, int *val, int *val2,
 		long mask)
 {
-	struct berlin2_adc_priv *priv = iio_priv(indio_dev);
 	int temp;
 
 	switch (mask) {
@@ -198,10 +205,6 @@ static int berlin2_adc_read_raw(struct iio_dev *indio_dev,
 		if (chan->type != IIO_VOLTAGE)
 			return -EINVAL;
 
-		/* Enable the interrupts */
-		regmap_write(priv->regmap, BERLIN2_SM_ADC_STATUS,
-				BERLIN2_SM_ADC_STATUS_INT_EN(chan->channel));
-
 		*val = berlin2_adc_read(indio_dev, chan->channel);
 		if (*val < 0)
 			return *val;
@@ -211,10 +214,6 @@ static int berlin2_adc_read_raw(struct iio_dev *indio_dev,
 		if (chan->type != IIO_TEMP)
 			return -EINVAL;
 
-		/* Enable interrupts */
-		regmap_write(priv->regmap, BERLIN2_SM_TSEN_STATUS,
-				BERLIN2_SM_TSEN_STATUS_INT_EN);
-
 		temp = berlin2_adc_tsen_read(indio_dev);
 		if (temp < 0)
 			return temp;
-- 
2.4.6

Re: [PATCH v2 7/8] iio:adc:berlin2-adc: enable interrupts with mutex locked

[Hartmut Knaack]

Antoine, any opinion about this one?


Hartmut Knaack schrieb am 28.07.2015 um 00:39:
> Move the call to enable channel interrupts into its _read() function to
> have it protected by a mutex. This ensures that only one channel is
> sampled at a time.
> 
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> ---
>  drivers/iio/adc/berlin2-adc.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
> index 6e20c856f479..13cfeb494e34 100644
> --- a/drivers/iio/adc/berlin2-adc.c
> +++ b/drivers/iio/adc/berlin2-adc.c
> @@ -111,6 +111,10 @@ static int berlin2_adc_read(struct iio_dev *indio_dev, int channel)
>  
>  	mutex_lock(&priv->lock);
>  
> +	/* Enable the interrupts */
> +	regmap_write(priv->regmap, BERLIN2_SM_ADC_STATUS,
> +		     BERLIN2_SM_ADC_STATUS_INT_EN(channel));
> +
>  	/* Configure the ADC */
>  	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
>  			BERLIN2_SM_CTRL_ADC_RESET | BERLIN2_SM_CTRL_ADC_SEL_MASK
> @@ -149,6 +153,10 @@ static int berlin2_adc_tsen_read(struct iio_dev *indio_dev)
>  
>  	mutex_lock(&priv->lock);
>  
> +	/* Enable interrupts */
> +	regmap_write(priv->regmap, BERLIN2_SM_TSEN_STATUS,
> +		     BERLIN2_SM_TSEN_STATUS_INT_EN);
> +
>  	/* Configure the ADC */
>  	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
>  			BERLIN2_SM_CTRL_TSEN_RESET | BERLIN2_SM_CTRL_ADC_ROTATE,
> @@ -190,7 +198,6 @@ static int berlin2_adc_read_raw(struct iio_dev *indio_dev,
>  		struct iio_chan_spec const *chan, int *val, int *val2,
>  		long mask)
>  {
> -	struct berlin2_adc_priv *priv = iio_priv(indio_dev);
>  	int temp;
>  
>  	switch (mask) {
> @@ -198,10 +205,6 @@ static int berlin2_adc_read_raw(struct iio_dev *indio_dev,
>  		if (chan->type != IIO_VOLTAGE)
>  			return -EINVAL;
>  
> -		/* Enable the interrupts */
> -		regmap_write(priv->regmap, BERLIN2_SM_ADC_STATUS,
> -				BERLIN2_SM_ADC_STATUS_INT_EN(chan->channel));
> -
>  		*val = berlin2_adc_read(indio_dev, chan->channel);
>  		if (*val < 0)
>  			return *val;
> @@ -211,10 +214,6 @@ static int berlin2_adc_read_raw(struct iio_dev *indio_dev,
>  		if (chan->type != IIO_TEMP)
>  			return -EINVAL;
>  
> -		/* Enable interrupts */
> -		regmap_write(priv->regmap, BERLIN2_SM_TSEN_STATUS,
> -				BERLIN2_SM_TSEN_STATUS_INT_EN);
> -
>  		temp = berlin2_adc_tsen_read(indio_dev);
>  		if (temp < 0)
>  			return temp;
> 

Re: [PATCH v2 7/8] iio:adc:berlin2-adc: enable interrupts with mutex locked

[Jonathan Cameron]

On 01/08/15 00:54, Hartmut Knaack wrote:
> Antoine, any opinion about this one?
> 
> 
> Hartmut Knaack schrieb am 28.07.2015 um 00:39:
>> Move the call to enable channel interrupts into its _read() function to
>> have it protected by a mutex. This ensures that only one channel is
>> sampled at a time.
>>
>> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
Looks fine to me and Antoine has had a while to confirm.

Applied to the togreg branch of iio.git

Thanks,

Jonathan
>> ---
>>  drivers/iio/adc/berlin2-adc.c | 17 ++++++++---------
>>  1 file changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
>> index 6e20c856f479..13cfeb494e34 100644
>> --- a/drivers/iio/adc/berlin2-adc.c
>> +++ b/drivers/iio/adc/berlin2-adc.c
>> @@ -111,6 +111,10 @@ static int berlin2_adc_read(struct iio_dev *indio_dev, int channel)
>>  
>>  	mutex_lock(&priv->lock);
>>  
>> +	/* Enable the interrupts */
>> +	regmap_write(priv->regmap, BERLIN2_SM_ADC_STATUS,
>> +		     BERLIN2_SM_ADC_STATUS_INT_EN(channel));
>> +
>>  	/* Configure the ADC */
>>  	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
>>  			BERLIN2_SM_CTRL_ADC_RESET | BERLIN2_SM_CTRL_ADC_SEL_MASK
>> @@ -149,6 +153,10 @@ static int berlin2_adc_tsen_read(struct iio_dev *indio_dev)
>>  
>>  	mutex_lock(&priv->lock);
>>  
>> +	/* Enable interrupts */
>> +	regmap_write(priv->regmap, BERLIN2_SM_TSEN_STATUS,
>> +		     BERLIN2_SM_TSEN_STATUS_INT_EN);
>> +
>>  	/* Configure the ADC */
>>  	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
>>  			BERLIN2_SM_CTRL_TSEN_RESET | BERLIN2_SM_CTRL_ADC_ROTATE,
>> @@ -190,7 +198,6 @@ static int berlin2_adc_read_raw(struct iio_dev *indio_dev,
>>  		struct iio_chan_spec const *chan, int *val, int *val2,
>>  		long mask)
>>  {
>> -	struct berlin2_adc_priv *priv = iio_priv(indio_dev);
>>  	int temp;
>>  
>>  	switch (mask) {
>> @@ -198,10 +205,6 @@ static int berlin2_adc_read_raw(struct iio_dev *indio_dev,
>>  		if (chan->type != IIO_VOLTAGE)
>>  			return -EINVAL;
>>  
>> -		/* Enable the interrupts */
>> -		regmap_write(priv->regmap, BERLIN2_SM_ADC_STATUS,
>> -				BERLIN2_SM_ADC_STATUS_INT_EN(chan->channel));
>> -
>>  		*val = berlin2_adc_read(indio_dev, chan->channel);
>>  		if (*val < 0)
>>  			return *val;
>> @@ -211,10 +214,6 @@ static int berlin2_adc_read_raw(struct iio_dev *indio_dev,
>>  		if (chan->type != IIO_TEMP)
>>  			return -EINVAL;
>>  
>> -		/* Enable interrupts */
>> -		regmap_write(priv->regmap, BERLIN2_SM_TSEN_STATUS,
>> -				BERLIN2_SM_TSEN_STATUS_INT_EN);
>> -
>>  		temp = berlin2_adc_tsen_read(indio_dev);
>>  		if (temp < 0)
>>  			return temp;
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v2 7/8] iio:adc:berlin2-adc: enable interrupts with mutex locked

[Antoine Tenart]

On Wed, Aug 12, 2015 at 10:13:39PM +0100, Jonathan Cameron wrote:
> On 01/08/15 00:54, Hartmut Knaack wrote:
> > Antoine, any opinion about this one?

Yes, sorry for the long delay... This is perfectly fine.

> > Hartmut Knaack schrieb am 28.07.2015 um 00:39:
> >> Move the call to enable channel interrupts into its _read() function to
> >> have it protected by a mutex. This ensures that only one channel is
> >> sampled at a time.
> >>
> >> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> Looks fine to me and Antoine has had a while to confirm.
> 
> Applied to the togreg branch of iio.git

Thanks!

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH v2 8/8] iio:adc:berlin2-adc: coding style cleanup

[Hartmut Knaack]

Some adjustment of indentation to make checkpatch.pl happy in strict mode.

Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/iio/adc/berlin2-adc.c | 64 +++++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 29 deletions(-)

diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
index 13cfeb494e34..71c806ecc722 100644
--- a/drivers/iio/adc/berlin2-adc.c
+++ b/drivers/iio/adc/berlin2-adc.c
@@ -78,13 +78,13 @@ struct berlin2_adc_priv {
 };
 
 #define BERLIN2_ADC_CHANNEL(n, t)					\
-		{							\
-			.channel	= n,				\
-			.datasheet_name	= "channel"#n,			\
-			.type		= t,				\
-			.indexed	= 1,				\
-			.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
-		}
+	{								\
+		.channel		= n,				\
+		.datasheet_name		= "channel"#n,			\
+		.type			= t,				\
+		.indexed		= 1,				\
+		.info_mask_separate	= BIT(IIO_CHAN_INFO_RAW),	\
+	}
 
 static const struct iio_chan_spec berlin2_adc_channels[] = {
 	BERLIN2_ADC_CHANNEL(0, IIO_VOLTAGE),	/* external input */
@@ -117,16 +117,18 @@ static int berlin2_adc_read(struct iio_dev *indio_dev, int channel)
 
 	/* Configure the ADC */
 	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
-			BERLIN2_SM_CTRL_ADC_RESET | BERLIN2_SM_CTRL_ADC_SEL_MASK
-			| BERLIN2_SM_CTRL_ADC_START,
-			BERLIN2_SM_CTRL_ADC_SEL(channel) | BERLIN2_SM_CTRL_ADC_START);
+			   BERLIN2_SM_CTRL_ADC_RESET |
+			   BERLIN2_SM_CTRL_ADC_SEL_MASK |
+			   BERLIN2_SM_CTRL_ADC_START,
+			   BERLIN2_SM_CTRL_ADC_SEL(channel) |
+			   BERLIN2_SM_CTRL_ADC_START);
 
 	ret = wait_event_interruptible_timeout(priv->wq, priv->data_available,
-			msecs_to_jiffies(1000));
+					       msecs_to_jiffies(1000));
 
 	/* Disable the interrupts */
 	regmap_update_bits(priv->regmap, BERLIN2_SM_ADC_STATUS,
-			BERLIN2_SM_ADC_STATUS_INT_EN(channel), 0);
+			   BERLIN2_SM_ADC_STATUS_INT_EN(channel), 0);
 
 	if (ret == 0)
 		ret = -ETIMEDOUT;
@@ -136,7 +138,7 @@ static int berlin2_adc_read(struct iio_dev *indio_dev, int channel)
 	}
 
 	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
-			BERLIN2_SM_CTRL_ADC_START, 0);
+			   BERLIN2_SM_CTRL_ADC_START, 0);
 
 	data = priv->data;
 	priv->data_available = false;
@@ -159,22 +161,25 @@ static int berlin2_adc_tsen_read(struct iio_dev *indio_dev)
 
 	/* Configure the ADC */
 	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
-			BERLIN2_SM_CTRL_TSEN_RESET | BERLIN2_SM_CTRL_ADC_ROTATE,
-			BERLIN2_SM_CTRL_ADC_ROTATE);
+			   BERLIN2_SM_CTRL_TSEN_RESET |
+			   BERLIN2_SM_CTRL_ADC_ROTATE,
+			   BERLIN2_SM_CTRL_ADC_ROTATE);
 
 	/* Configure the temperature sensor */
 	regmap_update_bits(priv->regmap, BERLIN2_SM_TSEN_CTRL,
-			BERLIN2_SM_TSEN_CTRL_TRIM_MASK | BERLIN2_SM_TSEN_CTRL_SETTLING_MASK
-			| BERLIN2_SM_TSEN_CTRL_START,
-			BERLIN2_SM_TSEN_CTRL_TRIM(3) | BERLIN2_SM_TSEN_CTRL_SETTLING_12
-			| BERLIN2_SM_TSEN_CTRL_START);
+			   BERLIN2_SM_TSEN_CTRL_TRIM_MASK |
+			   BERLIN2_SM_TSEN_CTRL_SETTLING_MASK |
+			   BERLIN2_SM_TSEN_CTRL_START,
+			   BERLIN2_SM_TSEN_CTRL_TRIM(3) |
+			   BERLIN2_SM_TSEN_CTRL_SETTLING_12 |
+			   BERLIN2_SM_TSEN_CTRL_START);
 
 	ret = wait_event_interruptible_timeout(priv->wq, priv->data_available,
-			msecs_to_jiffies(1000));
+					       msecs_to_jiffies(1000));
 
 	/* Disable interrupts */
 	regmap_update_bits(priv->regmap, BERLIN2_SM_TSEN_STATUS,
-			BERLIN2_SM_TSEN_STATUS_INT_EN, 0);
+			   BERLIN2_SM_TSEN_STATUS_INT_EN, 0);
 
 	if (ret == 0)
 		ret = -ETIMEDOUT;
@@ -184,7 +189,7 @@ static int berlin2_adc_tsen_read(struct iio_dev *indio_dev)
 	}
 
 	regmap_update_bits(priv->regmap, BERLIN2_SM_TSEN_CTRL,
-			BERLIN2_SM_TSEN_CTRL_START, 0);
+			   BERLIN2_SM_TSEN_CTRL_START, 0);
 
 	data = priv->data;
 	priv->data_available = false;
@@ -195,8 +200,8 @@ static int berlin2_adc_tsen_read(struct iio_dev *indio_dev)
 }
 
 static int berlin2_adc_read_raw(struct iio_dev *indio_dev,
-		struct iio_chan_spec const *chan, int *val, int *val2,
-		long mask)
+				struct iio_chan_spec const *chan, int *val,
+				int *val2, long mask)
 {
 	int temp;
 
@@ -305,12 +310,12 @@ static int berlin2_adc_probe(struct platform_device *pdev)
 		return tsen_irq;
 
 	ret = devm_request_irq(&pdev->dev, irq, berlin2_adc_irq, 0,
-			pdev->dev.driver->name, indio_dev);
+			       pdev->dev.driver->name, indio_dev);
 	if (ret)
 		return ret;
 
 	ret = devm_request_irq(&pdev->dev, tsen_irq, berlin2_adc_tsen_irq,
-			0, pdev->dev.driver->name, indio_dev);
+			       0, pdev->dev.driver->name, indio_dev);
 	if (ret)
 		return ret;
 
@@ -327,13 +332,14 @@ static int berlin2_adc_probe(struct platform_device *pdev)
 
 	/* Power up the ADC */
 	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
-			BERLIN2_SM_CTRL_ADC_POWER, BERLIN2_SM_CTRL_ADC_POWER);
+			   BERLIN2_SM_CTRL_ADC_POWER,
+			   BERLIN2_SM_CTRL_ADC_POWER);
 
 	ret = iio_device_register(indio_dev);
 	if (ret) {
 		/* Power down the ADC */
 		regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
-				BERLIN2_SM_CTRL_ADC_POWER, 0);
+				   BERLIN2_SM_CTRL_ADC_POWER, 0);
 		return ret;
 	}
 
@@ -349,7 +355,7 @@ static int berlin2_adc_remove(struct platform_device *pdev)
 
 	/* Power down the ADC */
 	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
-			BERLIN2_SM_CTRL_ADC_POWER, 0);
+			   BERLIN2_SM_CTRL_ADC_POWER, 0);
 
 	return 0;
 }
-- 
2.4.6

Re: [PATCH v2 8/8] iio:adc:berlin2-adc: coding style cleanup

[Jonathan Cameron]

On 27/07/15 23:39, Hartmut Knaack wrote:
> Some adjustment of indentation to make checkpatch.pl happy in strict mode.
> 
> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Applied.

Jonathan
> ---
>  drivers/iio/adc/berlin2-adc.c | 64 +++++++++++++++++++++++--------------------
>  1 file changed, 35 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
> index 13cfeb494e34..71c806ecc722 100644
> --- a/drivers/iio/adc/berlin2-adc.c
> +++ b/drivers/iio/adc/berlin2-adc.c
> @@ -78,13 +78,13 @@ struct berlin2_adc_priv {
>  };
>  
>  #define BERLIN2_ADC_CHANNEL(n, t)					\
> -		{							\
> -			.channel	= n,				\
> -			.datasheet_name	= "channel"#n,			\
> -			.type		= t,				\
> -			.indexed	= 1,				\
> -			.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
> -		}
> +	{								\
> +		.channel		= n,				\
> +		.datasheet_name		= "channel"#n,			\
> +		.type			= t,				\
> +		.indexed		= 1,				\
> +		.info_mask_separate	= BIT(IIO_CHAN_INFO_RAW),	\
> +	}
>  
>  static const struct iio_chan_spec berlin2_adc_channels[] = {
>  	BERLIN2_ADC_CHANNEL(0, IIO_VOLTAGE),	/* external input */
> @@ -117,16 +117,18 @@ static int berlin2_adc_read(struct iio_dev *indio_dev, int channel)
>  
>  	/* Configure the ADC */
>  	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
> -			BERLIN2_SM_CTRL_ADC_RESET | BERLIN2_SM_CTRL_ADC_SEL_MASK
> -			| BERLIN2_SM_CTRL_ADC_START,
> -			BERLIN2_SM_CTRL_ADC_SEL(channel) | BERLIN2_SM_CTRL_ADC_START);
> +			   BERLIN2_SM_CTRL_ADC_RESET |
> +			   BERLIN2_SM_CTRL_ADC_SEL_MASK |
> +			   BERLIN2_SM_CTRL_ADC_START,
> +			   BERLIN2_SM_CTRL_ADC_SEL(channel) |
> +			   BERLIN2_SM_CTRL_ADC_START);
>  
>  	ret = wait_event_interruptible_timeout(priv->wq, priv->data_available,
> -			msecs_to_jiffies(1000));
> +					       msecs_to_jiffies(1000));
>  
>  	/* Disable the interrupts */
>  	regmap_update_bits(priv->regmap, BERLIN2_SM_ADC_STATUS,
> -			BERLIN2_SM_ADC_STATUS_INT_EN(channel), 0);
> +			   BERLIN2_SM_ADC_STATUS_INT_EN(channel), 0);
>  
>  	if (ret == 0)
>  		ret = -ETIMEDOUT;
> @@ -136,7 +138,7 @@ static int berlin2_adc_read(struct iio_dev *indio_dev, int channel)
>  	}
>  
>  	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
> -			BERLIN2_SM_CTRL_ADC_START, 0);
> +			   BERLIN2_SM_CTRL_ADC_START, 0);
>  
>  	data = priv->data;
>  	priv->data_available = false;
> @@ -159,22 +161,25 @@ static int berlin2_adc_tsen_read(struct iio_dev *indio_dev)
>  
>  	/* Configure the ADC */
>  	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
> -			BERLIN2_SM_CTRL_TSEN_RESET | BERLIN2_SM_CTRL_ADC_ROTATE,
> -			BERLIN2_SM_CTRL_ADC_ROTATE);
> +			   BERLIN2_SM_CTRL_TSEN_RESET |
> +			   BERLIN2_SM_CTRL_ADC_ROTATE,
> +			   BERLIN2_SM_CTRL_ADC_ROTATE);
>  
>  	/* Configure the temperature sensor */
>  	regmap_update_bits(priv->regmap, BERLIN2_SM_TSEN_CTRL,
> -			BERLIN2_SM_TSEN_CTRL_TRIM_MASK | BERLIN2_SM_TSEN_CTRL_SETTLING_MASK
> -			| BERLIN2_SM_TSEN_CTRL_START,
> -			BERLIN2_SM_TSEN_CTRL_TRIM(3) | BERLIN2_SM_TSEN_CTRL_SETTLING_12
> -			| BERLIN2_SM_TSEN_CTRL_START);
> +			   BERLIN2_SM_TSEN_CTRL_TRIM_MASK |
> +			   BERLIN2_SM_TSEN_CTRL_SETTLING_MASK |
> +			   BERLIN2_SM_TSEN_CTRL_START,
> +			   BERLIN2_SM_TSEN_CTRL_TRIM(3) |
> +			   BERLIN2_SM_TSEN_CTRL_SETTLING_12 |
> +			   BERLIN2_SM_TSEN_CTRL_START);
>  
>  	ret = wait_event_interruptible_timeout(priv->wq, priv->data_available,
> -			msecs_to_jiffies(1000));
> +					       msecs_to_jiffies(1000));
>  
>  	/* Disable interrupts */
>  	regmap_update_bits(priv->regmap, BERLIN2_SM_TSEN_STATUS,
> -			BERLIN2_SM_TSEN_STATUS_INT_EN, 0);
> +			   BERLIN2_SM_TSEN_STATUS_INT_EN, 0);
>  
>  	if (ret == 0)
>  		ret = -ETIMEDOUT;
> @@ -184,7 +189,7 @@ static int berlin2_adc_tsen_read(struct iio_dev *indio_dev)
>  	}
>  
>  	regmap_update_bits(priv->regmap, BERLIN2_SM_TSEN_CTRL,
> -			BERLIN2_SM_TSEN_CTRL_START, 0);
> +			   BERLIN2_SM_TSEN_CTRL_START, 0);
>  
>  	data = priv->data;
>  	priv->data_available = false;
> @@ -195,8 +200,8 @@ static int berlin2_adc_tsen_read(struct iio_dev *indio_dev)
>  }
>  
>  static int berlin2_adc_read_raw(struct iio_dev *indio_dev,
> -		struct iio_chan_spec const *chan, int *val, int *val2,
> -		long mask)
> +				struct iio_chan_spec const *chan, int *val,
> +				int *val2, long mask)
>  {
>  	int temp;
>  
> @@ -305,12 +310,12 @@ static int berlin2_adc_probe(struct platform_device *pdev)
>  		return tsen_irq;
>  
>  	ret = devm_request_irq(&pdev->dev, irq, berlin2_adc_irq, 0,
> -			pdev->dev.driver->name, indio_dev);
> +			       pdev->dev.driver->name, indio_dev);
>  	if (ret)
>  		return ret;
>  
>  	ret = devm_request_irq(&pdev->dev, tsen_irq, berlin2_adc_tsen_irq,
> -			0, pdev->dev.driver->name, indio_dev);
> +			       0, pdev->dev.driver->name, indio_dev);
>  	if (ret)
>  		return ret;
>  
> @@ -327,13 +332,14 @@ static int berlin2_adc_probe(struct platform_device *pdev)
>  
>  	/* Power up the ADC */
>  	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
> -			BERLIN2_SM_CTRL_ADC_POWER, BERLIN2_SM_CTRL_ADC_POWER);
> +			   BERLIN2_SM_CTRL_ADC_POWER,
> +			   BERLIN2_SM_CTRL_ADC_POWER);
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret) {
>  		/* Power down the ADC */
>  		regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
> -				BERLIN2_SM_CTRL_ADC_POWER, 0);
> +				   BERLIN2_SM_CTRL_ADC_POWER, 0);
>  		return ret;
>  	}
>  
> @@ -349,7 +355,7 @@ static int berlin2_adc_remove(struct platform_device *pdev)
>  
>  	/* Power down the ADC */
>  	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
> -			BERLIN2_SM_CTRL_ADC_POWER, 0);
> +			   BERLIN2_SM_CTRL_ADC_POWER, 0);
>  
>  	return 0;
>  }
>