[PATCH 1/4] iio: adc: mcp320x: add masking of unknown bits

[PATCH 1/4] iio: adc: mcp320x: add masking of unknown bits

[Andrea Galbusera]

According to the datasheet, the 2 MSBs for parts 3001 and 3201 are
unspecified and should be masked out

Signed-off-by: Andrea Galbusera <gizero@gmail.com>
---

This was already suggested but for some reason got lost during review of a
previous patch. See http://marc.info/?l=linux-iio&m=140748835509583&w=2
and following discussion

 drivers/iio/adc/mcp320x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
index 8d9c9b9..0673ee7 100644
--- a/drivers/iio/adc/mcp320x.c
+++ b/drivers/iio/adc/mcp320x.c
@@ -114,13 +114,13 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
 
 	switch (device_index) {
 	case mcp3001:
-		return (adc->rx_buf[0] << 5 | adc->rx_buf[1] >> 3);
+		return ((adc->rx_buf[0] & 0x1f) << 5 | adc->rx_buf[1] >> 3);
 	case mcp3002:
 	case mcp3004:
 	case mcp3008:
 		return (adc->rx_buf[0] << 2 | adc->rx_buf[1] >> 6);
 	case mcp3201:
-		return (adc->rx_buf[0] << 7 | adc->rx_buf[1] >> 1);
+		return ((adc->rx_buf[0] & 0x1f) << 7 | adc->rx_buf[1] >> 1);
 	case mcp3202:
 	case mcp3204:
 	case mcp3208:
-- 
1.9.1

[PATCH 2/4] iio: adc: mcp320x: Add support for mcp3301

[Andrea Galbusera]

This adds support for Microchip's 13 bit 1 channel AD converter MCP3301

Signed-off-by: Andrea Galbusera <gizero@gmail.com>
---
 drivers/iio/adc/mcp320x.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
index 0673ee7..4de77e4 100644
--- a/drivers/iio/adc/mcp320x.c
+++ b/drivers/iio/adc/mcp320x.c
@@ -25,6 +25,7 @@
  * http://ww1.microchip.com/downloads/en/DeviceDoc/21290D.pdf  mcp3201
  * http://ww1.microchip.com/downloads/en/DeviceDoc/21034D.pdf  mcp3202
  * http://ww1.microchip.com/downloads/en/DeviceDoc/21298c.pdf  mcp3204/08
+ * http://ww1.microchip.com/downloads/en/DeviceDoc/21700E.pdf  mcp3301
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -47,6 +48,7 @@ enum {
 	mcp3202,
 	mcp3204,
 	mcp3208,
+	mcp3301,
 };
 
 struct mcp320x_chip_info {
@@ -76,6 +78,7 @@ static int mcp320x_channel_to_tx_data(int device_index,
 	switch (device_index) {
 	case mcp3001:
 	case mcp3201:
+	case mcp3301:
 		return 0;
 	case mcp3002:
 	case mcp3202:
@@ -102,7 +105,7 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
 	adc->tx_buf = mcp320x_channel_to_tx_data(device_index,
 						channel, differential);
 
-	if (device_index != mcp3001 && device_index != mcp3201) {
+	if (device_index != mcp3001 && device_index != mcp3201 && device_index != mcp3301) {
 		ret = spi_sync(adc->spi, &adc->msg);
 		if (ret < 0)
 			return ret;
@@ -125,6 +128,8 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
 	case mcp3204:
 	case mcp3208:
 		return (adc->rx_buf[0] << 4 | adc->rx_buf[1] >> 4);
+	case mcp3301:
+		return sign_extend32(adc->rx_buf[0] & 0x1f << 8 | adc->rx_buf[1], 12);
 	default:
 		return -EINVAL;
 	}
@@ -274,6 +279,11 @@ static const struct mcp320x_chip_info mcp320x_chip_infos[] = {
 		.num_channels = ARRAY_SIZE(mcp3208_channels),
 		.resolution = 12
 	},
+	[mcp3301] = {
+		.channels = mcp3201_channels,
+		.num_channels = ARRAY_SIZE(mcp3201_channels),
+		.resolution = 13
+	},
 };
 
 static int mcp320x_probe(struct spi_device *spi)
@@ -367,6 +377,9 @@ static const struct of_device_id mcp320x_dt_ids[] = {
 		.compatible = "mcp3208",
 		.data = &mcp320x_chip_infos[mcp3208],
 	}, {
+		.compatible = "mcp3301",
+		.data = &mcp320x_chip_infos[mcp3301],
+	}, {
 	}
 };
 MODULE_DEVICE_TABLE(of, mcp320x_dt_ids);
@@ -381,6 +394,7 @@ static const struct spi_device_id mcp320x_id[] = {
 	{ "mcp3202", mcp3202 },
 	{ "mcp3204", mcp3204 },
 	{ "mcp3208", mcp3208 },
+	{ "mcp3301", mcp3301 },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, mcp320x_id);
-- 
1.9.1

[PATCH 3/4] iio: adc: mcp320x: Kconfig update description

[Andrea Galbusera]

Signed-off-by: Andrea Galbusera <gizero@gmail.com>
---
 drivers/iio/adc/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 7c55658..f074b6a 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -207,8 +207,8 @@ config MCP320X
 	depends on SPI
 	help
 	  Say yes here to build support for Microchip Technology's
-	  MCP3001, MCP3002, MCP3004, MCP3008, MCP3201, MCP3202, MCP3204 or
-	  MCP3208 analog to digital converter.
+	  MCP3001, MCP3002, MCP3004, MCP3008, MCP3201, MCP3202, MCP3204,
+	  MCP3208 or MCP3301 analog to digital converter.
 
 	  This driver can also be built as a module. If so, the module will be
 	  called mcp320x.
-- 
1.9.1

Re: [PATCH 3/4] iio: adc: mcp320x: Kconfig update description

[Jonathan Cameron]

On 14 July 2015 14:36:22 BST, Andrea Galbusera <gizero@gmail.com> wrote:
>Signed-off-by: Andrea Galbusera <gizero@gmail.com>
Doesn't need a separate patch.  If this is only issue I will merge it with patch 2 whilst applying it.
>---
> drivers/iio/adc/Kconfig | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>index 7c55658..f074b6a 100644
>--- a/drivers/iio/adc/Kconfig
>+++ b/drivers/iio/adc/Kconfig
>@@ -207,8 +207,8 @@ config MCP320X
> 	depends on SPI
> 	help
> 	  Say yes here to build support for Microchip Technology's
>-	  MCP3001, MCP3002, MCP3004, MCP3008, MCP3201, MCP3202, MCP3204 or
>-	  MCP3208 analog to digital converter.
>+	  MCP3001, MCP3002, MCP3004, MCP3008, MCP3201, MCP3202, MCP3204,
>+	  MCP3208 or MCP3301 analog to digital converter.
> 
>	  This driver can also be built as a module. If so, the module will be
> 	  called mcp320x.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[PATCH 4/4] iio: adc: mcp320x: update dt-bindings help

[Andrea Galbusera]

Signed-off-by: Andrea Galbusera <gizero@gmail.com>
---
 Documentation/devicetree/bindings/iio/adc/mcp320x.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
index b851843..2a1f3af 100644
--- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
+++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
@@ -18,6 +18,7 @@ Required properties:
 				"mcp3202"
 				"mcp3204"
 				"mcp3208"
+				"mcp3301"
 
 
 Examples:
-- 
1.9.1

Re: [PATCH 4/4] iio: adc: mcp320x: update dt-bindings help

[Jonathan Cameron]

On 14 July 2015 14:36:23 BST, Andrea Galbusera <gizero@gmail.com> wrote:
>Signed-off-by: Andrea Galbusera <gizero@gmail.com>
Again no need for separate patch. All 3 are adding the new part support and so should be 1 patch.

I will fix up when applying.
>---
> Documentation/devicetree/bindings/iio/adc/mcp320x.txt | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
>b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
>index b851843..2a1f3af 100644
>--- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
>+++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
>@@ -18,6 +18,7 @@ Required properties:
> 				"mcp3202"
> 				"mcp3204"
> 				"mcp3208"
>+				"mcp3301"
> 
> 
> Examples:

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH 4/4] iio: adc: mcp320x: update dt-bindings help

[Jonathan Cameron]

On 14/07/15 22:02, Jonathan Cameron wrote:
> 
> 
> On 14 July 2015 14:36:23 BST, Andrea Galbusera <gizero@gmail.com> wrote:
>> Signed-off-by: Andrea Galbusera <gizero@gmail.com>
> Again no need for separate patch. All 3 are adding the new part support and so should be 1 patch.
> 
> I will fix up when applying.
Applied patches 2-4 as a single one adding the new part.
>> ---
>> Documentation/devicetree/bindings/iio/adc/mcp320x.txt | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
>> b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
>> index b851843..2a1f3af 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
>> +++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt
>> @@ -18,6 +18,7 @@ Required properties:
>> 				"mcp3202"
>> 				"mcp3204"
>> 				"mcp3208"
>> +				"mcp3301"
>>
>>
>> Examples:
> 

Re: [PATCH 1/4] iio: adc: mcp320x: add masking of unknown bits

[Jonathan Cameron]

On 14 July 2015 14:36:20 BST, Andrea Galbusera <gizero@gmail.com> wrote:
>According to the datasheet, the 2 MSBs for parts 3001 and 3201 are
>unspecified and should be masked out
Theoretical issue or one with observed bad effects?

Changes the path and timing of this getting applied. Patch itself is fine either way!
>
>Signed-off-by: Andrea Galbusera <gizero@gmail.com>
>---
>
>This was already suggested but for some reason got lost during review
>of a
>previous patch. See http://marc.info/?l=linux-iio&m=140748835509583&w=2
>and following discussion
>
> drivers/iio/adc/mcp320x.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
>index 8d9c9b9..0673ee7 100644
>--- a/drivers/iio/adc/mcp320x.c
>+++ b/drivers/iio/adc/mcp320x.c
>@@ -114,13 +114,13 @@ static int mcp320x_adc_conversion(struct mcp320x
>*adc, u8 channel,
> 
> 	switch (device_index) {
> 	case mcp3001:
>-		return (adc->rx_buf[0] << 5 | adc->rx_buf[1] >> 3);
>+		return ((adc->rx_buf[0] & 0x1f) << 5 | adc->rx_buf[1] >> 3);
> 	case mcp3002:
> 	case mcp3004:
> 	case mcp3008:
> 		return (adc->rx_buf[0] << 2 | adc->rx_buf[1] >> 6);
> 	case mcp3201:
>-		return (adc->rx_buf[0] << 7 | adc->rx_buf[1] >> 1);
>+		return ((adc->rx_buf[0] & 0x1f) << 7 | adc->rx_buf[1] >> 1);
> 	case mcp3202:
> 	case mcp3204:
> 	case mcp3208:

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH 1/4] iio: adc: mcp320x: add masking of unknown bits

[Andrea Galbusera]

[-- Attachment #1: Type: text/plain, Size: 2263 bytes --]

On Tue, Jul 14, 2015 at 11:04 PM, Jonathan Cameron <jic23@kernel.org> wrote:

>
>
> On 14 July 2015 14:36:20 BST, Andrea Galbusera <gizero@gmail.com> wrote:
> >According to the datasheet, the 2 MSBs for parts 3001 and 3201 are
> >unspecified and should be masked out
> Theoretical issue or one with observed bad effects?
>

Hi! Thanks for reviewing. While in the business of adding support for 3301,
I couldn't figure out why no masking wasn't done for similar parts like
3001 and 3201. Further investigation of previous patches reviews [1], where
masking was suggested and initially implemented, convinced me the bit was
lost somewhere, with no explicit motivation. Nevertheless, 3301 is the only
chip of the family I have at hands: hence, yes, this specific patch in the
set is addressing a theoretical issue.

[1] http://marc.info/?l=linux-iio&m=140748835509583&w=2


> Changes the path and timing of this getting applied. Patch itself is fine
> either way!
> >
> >Signed-off-by: Andrea Galbusera <gizero@gmail.com>
> >---
> >
> >This was already suggested but for some reason got lost during review
> >of a
> >previous patch. See http://marc.info/?l=linux-iio&m=140748835509583&w=2
> >and following discussion
> >
> > drivers/iio/adc/mcp320x.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
> >index 8d9c9b9..0673ee7 100644
> >--- a/drivers/iio/adc/mcp320x.c
> >+++ b/drivers/iio/adc/mcp320x.c
> >@@ -114,13 +114,13 @@ static int mcp320x_adc_conversion(struct mcp320x
> >*adc, u8 channel,
> >
> >       switch (device_index) {
> >       case mcp3001:
> >-              return (adc->rx_buf[0] << 5 | adc->rx_buf[1] >> 3);
> >+              return ((adc->rx_buf[0] & 0x1f) << 5 | adc->rx_buf[1] >>
> 3);
> >       case mcp3002:
> >       case mcp3004:
> >       case mcp3008:
> >               return (adc->rx_buf[0] << 2 | adc->rx_buf[1] >> 6);
> >       case mcp3201:
> >-              return (adc->rx_buf[0] << 7 | adc->rx_buf[1] >> 1);
> >+              return ((adc->rx_buf[0] & 0x1f) << 7 | adc->rx_buf[1] >>
> 1);
> >       case mcp3202:
> >       case mcp3204:
> >       case mcp3208:
>
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.
>

[-- Attachment #2: Type: text/html, Size: 3790 bytes --]

Re: [PATCH 1/4] iio: adc: mcp320x: add masking of unknown bits

[Andrea Galbusera]

On Tue, Jul 14, 2015 at 11:04 PM, Jonathan Cameron <jic23@kernel.org> wrote:
>
>
> On 14 July 2015 14:36:20 BST, Andrea Galbusera <gizero@gmail.com> wrote:
>>According to the datasheet, the 2 MSBs for parts 3001 and 3201 are
>>unspecified and should be masked out
> Theoretical issue or one with observed bad effects?

Hi! Thanks for reviewing. While in the business of adding support for
3301, I couldn't figure out why no masking wasn't done for similar
parts like 3001 and 3201. Further investigation of previous patches
reviews [1], where masking was suggested and initially implemented,
convinced me the bit was lost somewhere, with no explicit motivation.
Nevertheless, 3301 is the only chip of the family I have at hands:
hence, yes, this specific patch in the set is addressing a theoretical
issue.

[1] http://marc.info/?l=linux-iio&m=140748835509583&w=2

> Changes the path and timing of this getting applied. Patch itself is fine either way!
>>
>>Signed-off-by: Andrea Galbusera <gizero@gmail.com>
>>---
>>
>>This was already suggested but for some reason got lost during review
>>of a
>>previous patch. See http://marc.info/?l=linux-iio&m=140748835509583&w=2
>>and following discussion
>>
>> drivers/iio/adc/mcp320x.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>>diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
>>index 8d9c9b9..0673ee7 100644
>>--- a/drivers/iio/adc/mcp320x.c
>>+++ b/drivers/iio/adc/mcp320x.c
>>@@ -114,13 +114,13 @@ static int mcp320x_adc_conversion(struct mcp320x
>>*adc, u8 channel,
>>
>>       switch (device_index) {
>>       case mcp3001:
>>-              return (adc->rx_buf[0] << 5 | adc->rx_buf[1] >> 3);
>>+              return ((adc->rx_buf[0] & 0x1f) << 5 | adc->rx_buf[1] >> 3);
>>       case mcp3002:
>>       case mcp3004:
>>       case mcp3008:
>>               return (adc->rx_buf[0] << 2 | adc->rx_buf[1] >> 6);
>>       case mcp3201:
>>-              return (adc->rx_buf[0] << 7 | adc->rx_buf[1] >> 1);
>>+              return ((adc->rx_buf[0] & 0x1f) << 7 | adc->rx_buf[1] >> 1);
>>       case mcp3202:
>>       case mcp3204:
>>       case mcp3208:
>
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH 1/4] iio: adc: mcp320x: add masking of unknown bits

[Hartmut Knaack]

Andrea Galbusera schrieb am 14.07.2015 um 15:36:
> According to the datasheet, the 2 MSBs for parts 3001 and 3201 are
> unspecified and should be masked out
> 

Hi,
since resolution is already set, better make it more generic by applying
the mask in _read_raw like this:

*val = ret & GENMASK(adc->chip_info->resolution - 1, 0);

Thanks,
Hartmut

> Signed-off-by: Andrea Galbusera <gizero@gmail.com>
> ---
> 
> This was already suggested but for some reason got lost during review of a
> previous patch. See http://marc.info/?l=linux-iio&m=140748835509583&w=2
> and following discussion
> 
>  drivers/iio/adc/mcp320x.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
> index 8d9c9b9..0673ee7 100644
> --- a/drivers/iio/adc/mcp320x.c
> +++ b/drivers/iio/adc/mcp320x.c
> @@ -114,13 +114,13 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
>  
>  	switch (device_index) {
>  	case mcp3001:
> -		return (adc->rx_buf[0] << 5 | adc->rx_buf[1] >> 3);
> +		return ((adc->rx_buf[0] & 0x1f) << 5 | adc->rx_buf[1] >> 3);
>  	case mcp3002:
>  	case mcp3004:
>  	case mcp3008:
>  		return (adc->rx_buf[0] << 2 | adc->rx_buf[1] >> 6);
>  	case mcp3201:
> -		return (adc->rx_buf[0] << 7 | adc->rx_buf[1] >> 1);
> +		return ((adc->rx_buf[0] & 0x1f) << 7 | adc->rx_buf[1] >> 1);
>  	case mcp3202:
>  	case mcp3204:
>  	case mcp3208:
> 

Re: [PATCH 1/4] iio: adc: mcp320x: add masking of unknown bits

[Jonathan Cameron]

On 15/07/15 19:12, Hartmut Knaack wrote:
> Andrea Galbusera schrieb am 14.07.2015 um 15:36:
>> According to the datasheet, the 2 MSBs for parts 3001 and 3201 are
>> unspecified and should be masked out
>>
> 
> Hi,
> since resolution is already set, better make it more generic by applying
> the mask in _read_raw like this:
> 
> *val = ret & GENMASK(adc->chip_info->resolution - 1, 0);
> 
> Thanks,
> Hartmut
Agreed, that would be cleaner.
> 
>> Signed-off-by: Andrea Galbusera <gizero@gmail.com>
>> ---
>>
>> This was already suggested but for some reason got lost during review of a
>> previous patch. See http://marc.info/?l=linux-iio&m=140748835509583&w=2
>> and following discussion
>>
>>  drivers/iio/adc/mcp320x.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
>> index 8d9c9b9..0673ee7 100644
>> --- a/drivers/iio/adc/mcp320x.c
>> +++ b/drivers/iio/adc/mcp320x.c
>> @@ -114,13 +114,13 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
>>  
>>  	switch (device_index) {
>>  	case mcp3001:
>> -		return (adc->rx_buf[0] << 5 | adc->rx_buf[1] >> 3);
>> +		return ((adc->rx_buf[0] & 0x1f) << 5 | adc->rx_buf[1] >> 3);
>>  	case mcp3002:
>>  	case mcp3004:
>>  	case mcp3008:
>>  		return (adc->rx_buf[0] << 2 | adc->rx_buf[1] >> 6);
>>  	case mcp3201:
>> -		return (adc->rx_buf[0] << 7 | adc->rx_buf[1] >> 1);
>> +		return ((adc->rx_buf[0] & 0x1f) << 7 | adc->rx_buf[1] >> 1);
>>  	case mcp3202:
>>  	case mcp3204:
>>  	case mcp3208:
>>
> 
> --
> 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 1/4] iio: adc: mcp320x: add masking of unknown bits

[Andrea Galbusera]

On Sun, Jul 19, 2015 at 2:33 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 15/07/15 19:12, Hartmut Knaack wrote:
>> Andrea Galbusera schrieb am 14.07.2015 um 15:36:
>>> According to the datasheet, the 2 MSBs for parts 3001 and 3201 are
>>> unspecified and should be masked out
>>>
>>
>> Hi,
>> since resolution is already set, better make it more generic by applying
>> the mask in _read_raw like this:
>>
>> *val = ret & GENMASK(adc->chip_info->resolution - 1, 0);
>>
>> Thanks,
>> Hartmut
> Agreed, that would be cleaner.

Fine for me! I will respin the patch and resend. My only concern is on
how this will get applied. If I understand correctly my other three
patches in the series will probably get applied before this one. The
2,3,4/4 will add the new part with masking applied in
mcp320x_adc_conversion. Then the new version of this one should move
masking in _read_raw. So, is it fine to rebase the new version of this
on top of the other three and resend as a standalone patch? Otherwise
I can respin the entire series, but this one will still affect other
parts... Please forgive my newbie questions, and let me know the best
approach to follow.

>>
>>> Signed-off-by: Andrea Galbusera <gizero@gmail.com>
>>> ---
>>>
>>> This was already suggested but for some reason got lost during review of a
>>> previous patch. See http://marc.info/?l=linux-iio&m=140748835509583&w=2
>>> and following discussion
>>>
>>>  drivers/iio/adc/mcp320x.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
>>> index 8d9c9b9..0673ee7 100644
>>> --- a/drivers/iio/adc/mcp320x.c
>>> +++ b/drivers/iio/adc/mcp320x.c
>>> @@ -114,13 +114,13 @@ static int mcp320x_adc_conversion(struct mcp320x *adc, u8 channel,
>>>
>>>      switch (device_index) {
>>>      case mcp3001:
>>> -            return (adc->rx_buf[0] << 5 | adc->rx_buf[1] >> 3);
>>> +            return ((adc->rx_buf[0] & 0x1f) << 5 | adc->rx_buf[1] >> 3);
>>>      case mcp3002:
>>>      case mcp3004:
>>>      case mcp3008:
>>>              return (adc->rx_buf[0] << 2 | adc->rx_buf[1] >> 6);
>>>      case mcp3201:
>>> -            return (adc->rx_buf[0] << 7 | adc->rx_buf[1] >> 1);
>>> +            return ((adc->rx_buf[0] & 0x1f) << 7 | adc->rx_buf[1] >> 1);
>>>      case mcp3202:
>>>      case mcp3204:
>>>      case mcp3208:
>>>
>>
>> --
>> 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 1/4] iio: adc: mcp320x: add masking of unknown bits

[Jonathan Cameron]

On 22 July 2015 06:15:27 BST, Andrea Galbusera <gizero@gmail.com> wrote:
>On Sun, Jul 19, 2015 at 2:33 PM, Jonathan Cameron <jic23@kernel.org>
>wrote:
>> On 15/07/15 19:12, Hartmut Knaack wrote:
>>> Andrea Galbusera schrieb am 14.07.2015 um 15:36:
>>>> According to the datasheet, the 2 MSBs for parts 3001 and 3201 are
>>>> unspecified and should be masked out
>>>>
>>>
>>> Hi,
>>> since resolution is already set, better make it more generic by
>applying
>>> the mask in _read_raw like this:
>>>
>>> *val = ret & GENMASK(adc->chip_info->resolution - 1, 0);
>>>
>>> Thanks,
>>> Hartmut
>> Agreed, that would be cleaner.
>
>Fine for me! I will respin the patch and resend. My only concern is on
>how this will get applied. If I understand correctly my other three
>patches in the series will probably get applied before this one. The
>2,3,4/4 will add the new part with masking applied in
>mcp320x_adc_conversion. Then the new version of this one should move
>masking in _read_raw. So, is it fine to rebase the new version of this
>on top of the other three and resend as a standalone patch? Otherwise
>I can respin the entire series, but this one will still affect other
>parts... Please forgive my newbie questions, and let me know the best
>approach to follow.
Spin it to go on top. Thanks

J
>
>>>
>>>> Signed-off-by: Andrea Galbusera <gizero@gmail.com>
>>>> ---
>>>>
>>>> This was already suggested but for some reason got lost during
>review of a
>>>> previous patch. See
>http://marc.info/?l=linux-iio&m=140748835509583&w=2
>>>> and following discussion
>>>>
>>>>  drivers/iio/adc/mcp320x.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
>>>> index 8d9c9b9..0673ee7 100644
>>>> --- a/drivers/iio/adc/mcp320x.c
>>>> +++ b/drivers/iio/adc/mcp320x.c
>>>> @@ -114,13 +114,13 @@ static int mcp320x_adc_conversion(struct
>mcp320x *adc, u8 channel,
>>>>
>>>>      switch (device_index) {
>>>>      case mcp3001:
>>>> -            return (adc->rx_buf[0] << 5 | adc->rx_buf[1] >> 3);
>>>> +            return ((adc->rx_buf[0] & 0x1f) << 5 | adc->rx_buf[1]
>>> 3);
>>>>      case mcp3002:
>>>>      case mcp3004:
>>>>      case mcp3008:
>>>>              return (adc->rx_buf[0] << 2 | adc->rx_buf[1] >> 6);
>>>>      case mcp3201:
>>>> -            return (adc->rx_buf[0] << 7 | adc->rx_buf[1] >> 1);
>>>> +            return ((adc->rx_buf[0] & 0x1f) << 7 | adc->rx_buf[1]
>>> 1);
>>>>      case mcp3202:
>>>>      case mcp3204:
>>>>      case mcp3208:
>>>>
>>>
>>> --
>>> 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

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.