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.