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

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

[Hartmut Knaack]

There is some inconsistency in the register definition. Unfortunately, I
could not find a data sheet, so anyone with the data sheet at hand
(Antoine?) needs to check against the first patch before applying that
fix.
The rest are mainly cleanups and code restructuring.

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.3

[PATCH 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.

Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
---
Driver analysis indicates that only one ADC channel can be sampled at a
time, and this channel number is stored in BERLIN2_SM_CTRL as a value,
instead of a bit map.
In register BERLIN2_SM_ADC_STATUS however, interrupts can be enabled for
each ADC channel, and data ready is indicated for each ADC channel,
too - in both cases as a bit map. Since there are 8 channels available,
both masks should be at least 8 bits wide. But as the comment indicates,
there are 16 bits for each mask.

 drivers/iio/adc/berlin2-adc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
index aecc9ad995ad..97863d1ba79a 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,9 +53,9 @@
 #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)
-- 
2.4.3

[PATCH 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>
---
 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 97863d1ba79a..b5a099c7bf34 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.3

[PATCH 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 | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
index b5a099c7bf34..42e214b3e93a 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)
@@ -60,14 +60,14 @@
 #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(11, 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 */
 #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.3

[PATCH 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>
---
 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 42e214b3e93a..b7df94a0b879 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.3

[PATCH 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>
---
 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 b7df94a0b879..1d14290ba6f2 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.3

[PATCH 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>
---
 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 1d14290ba6f2..c840ba8b2aa4 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.3

[PATCH 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 c840ba8b2aa4..3621ee9f5032 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.3

[PATCH 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>
---
 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 3621ee9f5032..73d625313388 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.3

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

[Antoine Tenart]

On Sun, Jul 26, 2015 at 12:43:19AM +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.

Thanks for fixing this!

> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>

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

> ---
> Driver analysis indicates that only one ADC channel can be sampled at a
> time, and this channel number is stored in BERLIN2_SM_CTRL as a value,
> instead of a bit map.
> In register BERLIN2_SM_ADC_STATUS however, interrupts can be enabled for
> each ADC channel, and data ready is indicated for each ADC channel,
> too - in both cases as a bit map. Since there are 8 channels available,
> both masks should be at least 8 bits wide. But as the comment indicates,
> there are 16 bits for each mask.
> 
>  drivers/iio/adc/berlin2-adc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
> index aecc9ad995ad..97863d1ba79a 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,9 +53,9 @@
>  #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)
> -- 
> 2.4.3
> 

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

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

[Antoine Tenart]

On Sun, Jul 26, 2015 at 12:43:20AM +0200, 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>

> ---
>  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 97863d1ba79a..b5a099c7bf34 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.3
> 

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

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

[Antoine Tenart]

Hi,

On Sun, Jul 26, 2015 at 12:43:21AM +0200, Hartmut Knaack wrote:
> Make use of GENMASK for consecutive bitmasks and BIT for single bitmasks.

Thanks for doing this clean-up.

> Signed-off-by: Hartmut Knaack <knaack.h@gmx.de>
> ---
> @@ -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)
> @@ -60,14 +60,14 @@
>  #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

There was actually an error here. It should have been 0x3ff as
BERLIN2_SM_ADC_MASK.

> +#define  BERLIN2_SM_TSEN_MASK			GENMASK(11, 0)

So, GENMASK(9, 0).

Antoine

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

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

[Antoine Tenart]

On Sun, Jul 26, 2015 at 12:43:22AM +0200, 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>

> ---
>  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 42e214b3e93a..b7df94a0b879 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.3
> 

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

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

[Antoine Tenart]

On Sun, Jul 26, 2015 at 12:43:23AM +0200, 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>

> ---
>  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 b7df94a0b879..1d14290ba6f2 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.3
> 

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

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

[Antoine Tenart]

On Sun, Jul 26, 2015 at 12:43:24AM +0200, 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>

> ---
>  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 1d14290ba6f2..c840ba8b2aa4 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.3
> 

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

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

[Antoine Tenart]

On Sun, Jul 26, 2015 at 12:43:26AM +0200, 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>

> ---
>  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 3621ee9f5032..73d625313388 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.3
> 

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