[PATCH 1/2] iio: Fix sign extension table in mcp3422 driver

[PATCH 1/2] iio: Fix sign extension table in mcp3422 driver

[Peter Meerwald]

the index argument to sign_extend32() gives the bit position (from 0)
to the sign bit

so e.g. if the measurement has 16-bit resolution, we need to pass 15;
a measurement of 0x8000 should be reported as -32768, not 32768

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Cc: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
 drivers/iio/adc/mcp3422.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/mcp3422.c b/drivers/iio/adc/mcp3422.c
index 1294832..c8c1baa 100644
--- a/drivers/iio/adc/mcp3422.c
+++ b/drivers/iio/adc/mcp3422.c
@@ -88,10 +88,10 @@ static const int mcp3422_sample_rates[4] = {
 
 /* sample rates to sign extension table */
 static const int mcp3422_sign_extend[4] = {
-	[MCP3422_SRATE_240] = 12,
-	[MCP3422_SRATE_60] = 14,
-	[MCP3422_SRATE_15] = 16,
-	[MCP3422_SRATE_3] = 18 };
+	[MCP3422_SRATE_240] = 11,
+	[MCP3422_SRATE_60] = 13,
+	[MCP3422_SRATE_15] = 15,
+	[MCP3422_SRATE_3] = 17 };
 
 /* Client data (each client gets its own) */
 struct mcp3422 {
-- 
1.8.4.2

[PATCH 2/2] iio: Drop scan_type from viperboard adc driver

[Peter Meerwald]

the driver does not support buffering, hence scan_type is not needed

Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
Cc: Lars Poeschel <poeschel@lemonage.de>
---
 drivers/iio/adc/viperboard_adc.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/iio/adc/viperboard_adc.c b/drivers/iio/adc/viperboard_adc.c
index 09727a7..94f404d 100644
--- a/drivers/iio/adc/viperboard_adc.c
+++ b/drivers/iio/adc/viperboard_adc.c
@@ -42,11 +42,6 @@ struct vprbrd_adc {
 	.indexed = 1,					\
 	.channel = _index,				\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
-	.scan_index = _index,				\
-	.scan_type = {					\
-		.sign = 'u',				\
-		.realbits = 8,				\
-		.storagebits = 8,			\
 	},						\
 }
 
@@ -73,7 +68,7 @@ static int vprbrd_iio_read_raw(struct iio_dev *iio_dev,
 		mutex_lock(&vb->lock);
 
 		admsg->cmd = VPRBRD_ADC_CMD_GET;
-		admsg->chan = chan->scan_index;
+		admsg->chan = chan->channel;
 		admsg->val = 0x00;
 
 		ret = usb_control_msg(vb->usb_dev,
-- 
1.8.4.2

Re: [PATCH 1/2] iio: Fix sign extension table in mcp3422 driver

[Angelo Compagnucci]

Hi Peter,

You are totally right, index is 0 based and this is a bug!

Thank your for fixing!

2013/11/2 Peter Meerwald <pmeerw@pmeerw.net>:
> the index argument to sign_extend32() gives the bit position (from 0)
> to the sign bit
>
> so e.g. if the measurement has 16-bit resolution, we need to pass 15;
> a measurement of 0x8000 should be reported as -32768, not 32768
>
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> Cc: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> ---
>  drivers/iio/adc/mcp3422.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/mcp3422.c b/drivers/iio/adc/mcp3422.c
> index 1294832..c8c1baa 100644
> --- a/drivers/iio/adc/mcp3422.c
> +++ b/drivers/iio/adc/mcp3422.c
> @@ -88,10 +88,10 @@ static const int mcp3422_sample_rates[4] = {
>
>  /* sample rates to sign extension table */
>  static const int mcp3422_sign_extend[4] = {
> -       [MCP3422_SRATE_240] = 12,
> -       [MCP3422_SRATE_60] = 14,
> -       [MCP3422_SRATE_15] = 16,
> -       [MCP3422_SRATE_3] = 18 };
> +       [MCP3422_SRATE_240] = 11,
> +       [MCP3422_SRATE_60] = 13,
> +       [MCP3422_SRATE_15] = 15,
> +       [MCP3422_SRATE_3] = 17 };
>
>  /* Client data (each client gets its own) */
>  struct mcp3422 {
> --
> 1.8.4.2
>



-- 
Profile: http://it.linkedin.com/in/compagnucciangelo

Re: [PATCH 1/2] iio: Fix sign extension table in mcp3422 driver

[Jonathan Cameron]

On 11/02/13 22:09, Angelo Compagnucci wrote:
> Hi Peter,
> 
> You are totally right, index is 0 based and this is a bug!
> 
> Thank your for fixing!
If you are happy with a patch that effects your code, the convention is
to reply with

Acked-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>

to indicate this in the commit log.

Do you mind me adding that to this patch?
> 
> 2013/11/2 Peter Meerwald <pmeerw@pmeerw.net>:
>> the index argument to sign_extend32() gives the bit position (from 0)
>> to the sign bit
>>
>> so e.g. if the measurement has 16-bit resolution, we need to pass 15;
>> a measurement of 0x8000 should be reported as -32768, not 32768
>>
>> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
>> Cc: Angelo Compagnucci <angelo.compagnucci@gmail.com>
>> ---
>>  drivers/iio/adc/mcp3422.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/adc/mcp3422.c b/drivers/iio/adc/mcp3422.c
>> index 1294832..c8c1baa 100644
>> --- a/drivers/iio/adc/mcp3422.c
>> +++ b/drivers/iio/adc/mcp3422.c
>> @@ -88,10 +88,10 @@ static const int mcp3422_sample_rates[4] = {
>>
>>  /* sample rates to sign extension table */
>>  static const int mcp3422_sign_extend[4] = {
>> -       [MCP3422_SRATE_240] = 12,
>> -       [MCP3422_SRATE_60] = 14,
>> -       [MCP3422_SRATE_15] = 16,
>> -       [MCP3422_SRATE_3] = 18 };
>> +       [MCP3422_SRATE_240] = 11,
>> +       [MCP3422_SRATE_60] = 13,
>> +       [MCP3422_SRATE_15] = 15,
>> +       [MCP3422_SRATE_3] = 17 };
>>
>>  /* Client data (each client gets its own) */
>>  struct mcp3422 {
>> --
>> 1.8.4.2
>>
> 
> 
> 

Re: [PATCH 1/2] iio: Fix sign extension table in mcp3422 driver

[Angelo Compagnucci]

Hi jonathan,

Yes, this patch should be applied.

Acked-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>

2013/11/4 Jonathan Cameron <jic23@kernel.org>:
> On 11/02/13 22:09, Angelo Compagnucci wrote:
>> Hi Peter,
>>
>> You are totally right, index is 0 based and this is a bug!
>>
>> Thank your for fixing!
> If you are happy with a patch that effects your code, the convention is
> to reply with
>
> Acked-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
>
> to indicate this in the commit log.
>
> Do you mind me adding that to this patch?
>>
>> 2013/11/2 Peter Meerwald <pmeerw@pmeerw.net>:
>>> the index argument to sign_extend32() gives the bit position (from 0)
>>> to the sign bit
>>>
>>> so e.g. if the measurement has 16-bit resolution, we need to pass 15;
>>> a measurement of 0x8000 should be reported as -32768, not 32768
>>>
>>> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
>>> Cc: Angelo Compagnucci <angelo.compagnucci@gmail.com>
>>> ---
>>>  drivers/iio/adc/mcp3422.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/mcp3422.c b/drivers/iio/adc/mcp3422.c
>>> index 1294832..c8c1baa 100644
>>> --- a/drivers/iio/adc/mcp3422.c
>>> +++ b/drivers/iio/adc/mcp3422.c
>>> @@ -88,10 +88,10 @@ static const int mcp3422_sample_rates[4] = {
>>>
>>>  /* sample rates to sign extension table */
>>>  static const int mcp3422_sign_extend[4] = {
>>> -       [MCP3422_SRATE_240] = 12,
>>> -       [MCP3422_SRATE_60] = 14,
>>> -       [MCP3422_SRATE_15] = 16,
>>> -       [MCP3422_SRATE_3] = 18 };
>>> +       [MCP3422_SRATE_240] = 11,
>>> +       [MCP3422_SRATE_60] = 13,
>>> +       [MCP3422_SRATE_15] = 15,
>>> +       [MCP3422_SRATE_3] = 17 };
>>>
>>>  /* Client data (each client gets its own) */
>>>  struct mcp3422 {
>>> --
>>> 1.8.4.2
>>>
>>
>>
>>



-- 
Profile: http://it.linkedin.com/in/compagnucciangelo

Re: [PATCH 1/2] iio: Fix sign extension table in mcp3422 driver

[Jonathan Cameron]

Thanks. Given interesting merge window might be a few weeks before it hits mainline.

Angelo Compagnucci <angelo.compagnucci@gmail.com> wrote:
>Hi jonathan,
>
>Yes, this patch should be applied.
>
>Acked-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
>
>2013/11/4 Jonathan Cameron <jic23@kernel.org>:
>> On 11/02/13 22:09, Angelo Compagnucci wrote:
>>> Hi Peter,
>>>
>>> You are totally right, index is 0 based and this is a bug!
>>>
>>> Thank your for fixing!
>> If you are happy with a patch that effects your code, the convention
>is
>> to reply with
>>
>> Acked-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
>>
>> to indicate this in the commit log.
>>
>> Do you mind me adding that to this patch?
>>>
>>> 2013/11/2 Peter Meerwald <pmeerw@pmeerw.net>:
>>>> the index argument to sign_extend32() gives the bit position (from
>0)
>>>> to the sign bit
>>>>
>>>> so e.g. if the measurement has 16-bit resolution, we need to pass
>15;
>>>> a measurement of 0x8000 should be reported as -32768, not 32768
>>>>
>>>> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
>>>> Cc: Angelo Compagnucci <angelo.compagnucci@gmail.com>
>>>> ---
>>>>  drivers/iio/adc/mcp3422.c | 8 ++++----
>>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/adc/mcp3422.c b/drivers/iio/adc/mcp3422.c
>>>> index 1294832..c8c1baa 100644
>>>> --- a/drivers/iio/adc/mcp3422.c
>>>> +++ b/drivers/iio/adc/mcp3422.c
>>>> @@ -88,10 +88,10 @@ static const int mcp3422_sample_rates[4] = {
>>>>
>>>>  /* sample rates to sign extension table */
>>>>  static const int mcp3422_sign_extend[4] = {
>>>> -       [MCP3422_SRATE_240] = 12,
>>>> -       [MCP3422_SRATE_60] = 14,
>>>> -       [MCP3422_SRATE_15] = 16,
>>>> -       [MCP3422_SRATE_3] = 18 };
>>>> +       [MCP3422_SRATE_240] = 11,
>>>> +       [MCP3422_SRATE_60] = 13,
>>>> +       [MCP3422_SRATE_15] = 15,
>>>> +       [MCP3422_SRATE_3] = 17 };
>>>>
>>>>  /* Client data (each client gets its own) */
>>>>  struct mcp3422 {
>>>> --
>>>> 1.8.4.2
>>>>
>>>
>>>
>>>

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

Re: [PATCH 1/2] iio: Fix sign extension table in mcp3422 driver

[Jonathan Cameron]

On 11/05/13 13:28, Jonathan Cameron wrote:
> Thanks. Given interesting merge window might be a few weeks before it hits mainline.
> 
> Angelo Compagnucci <angelo.compagnucci@gmail.com> wrote:
>> Hi jonathan,
>>
>> Yes, this patch should be applied.
>>
>> Acked-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
Applied to the fixes-togreg branch of iio.git

Thanks,
>>
>> 2013/11/4 Jonathan Cameron <jic23@kernel.org>:
>>> On 11/02/13 22:09, Angelo Compagnucci wrote:
>>>> Hi Peter,
>>>>
>>>> You are totally right, index is 0 based and this is a bug!
>>>>
>>>> Thank your for fixing!
>>> If you are happy with a patch that effects your code, the convention
>> is
>>> to reply with
>>>
>>> Acked-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
>>>
>>> to indicate this in the commit log.
>>>
>>> Do you mind me adding that to this patch?
>>>>
>>>> 2013/11/2 Peter Meerwald <pmeerw@pmeerw.net>:
>>>>> the index argument to sign_extend32() gives the bit position (from
>> 0)
>>>>> to the sign bit
>>>>>
>>>>> so e.g. if the measurement has 16-bit resolution, we need to pass
>> 15;
>>>>> a measurement of 0x8000 should be reported as -32768, not 32768
>>>>>
>>>>> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
>>>>> Cc: Angelo Compagnucci <angelo.compagnucci@gmail.com>
>>>>> ---
>>>>>  drivers/iio/adc/mcp3422.c | 8 ++++----
>>>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/iio/adc/mcp3422.c b/drivers/iio/adc/mcp3422.c
>>>>> index 1294832..c8c1baa 100644
>>>>> --- a/drivers/iio/adc/mcp3422.c
>>>>> +++ b/drivers/iio/adc/mcp3422.c
>>>>> @@ -88,10 +88,10 @@ static const int mcp3422_sample_rates[4] = {
>>>>>
>>>>>  /* sample rates to sign extension table */
>>>>>  static const int mcp3422_sign_extend[4] = {
>>>>> -       [MCP3422_SRATE_240] = 12,
>>>>> -       [MCP3422_SRATE_60] = 14,
>>>>> -       [MCP3422_SRATE_15] = 16,
>>>>> -       [MCP3422_SRATE_3] = 18 };
>>>>> +       [MCP3422_SRATE_240] = 11,
>>>>> +       [MCP3422_SRATE_60] = 13,
>>>>> +       [MCP3422_SRATE_15] = 15,
>>>>> +       [MCP3422_SRATE_3] = 17 };
>>>>>
>>>>>  /* Client data (each client gets its own) */
>>>>>  struct mcp3422 {
>>>>> --
>>>>> 1.8.4.2
>>>>>
>>>>
>>>>
>>>>
> 

Re: [PATCH 2/2] iio: Drop scan_type from viperboard adc driver

[Jonathan Cameron]

On 11/02/13 19:44, Peter Meerwald wrote:
> the driver does not support buffering, hence scan_type is not needed
> 
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> Cc: Lars Poeschel <poeschel@lemonage.de>
We've left this sort of thing in in the past as it acts as documentation whilst
doing little harm.

Still in this driver there is nothing 'interesting' in the definition
so I've applied your patch to the togreg branch of iio.git

Thanks,

Jonathan
> ---
>  drivers/iio/adc/viperboard_adc.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/adc/viperboard_adc.c b/drivers/iio/adc/viperboard_adc.c
> index 09727a7..94f404d 100644
> --- a/drivers/iio/adc/viperboard_adc.c
> +++ b/drivers/iio/adc/viperboard_adc.c
> @@ -42,11 +42,6 @@ struct vprbrd_adc {
>  	.indexed = 1,					\
>  	.channel = _index,				\
>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
> -	.scan_index = _index,				\
> -	.scan_type = {					\
> -		.sign = 'u',				\
> -		.realbits = 8,				\
> -		.storagebits = 8,			\
>  	},						\
>  }
>  
> @@ -73,7 +68,7 @@ static int vprbrd_iio_read_raw(struct iio_dev *iio_dev,
>  		mutex_lock(&vb->lock);
>  
>  		admsg->cmd = VPRBRD_ADC_CMD_GET;
> -		admsg->chan = chan->scan_index;
> +		admsg->chan = chan->channel;
>  		admsg->val = 0x00;
>  
>  		ret = usb_control_msg(vb->usb_dev,
> 

Re: [PATCH 2/2] iio: Drop scan_type from viperboard adc driver

[Jonathan Cameron]

On 11/09/13 11:48, Jonathan Cameron wrote:
> On 11/02/13 19:44, Peter Meerwald wrote:
>> the driver does not support buffering, hence scan_type is not needed
>>
>> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
>> Cc: Lars Poeschel <poeschel@lemonage.de>
> We've left this sort of thing in in the past as it acts as documentation whilst
> doing little harm.
> 
> Still in this driver there is nothing 'interesting' in the definition
> so I've applied your patch to the togreg branch of iio.git
> 
> Thanks,
> 
> Jonathan
>> ---
>>  drivers/iio/adc/viperboard_adc.c | 7 +------
>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/drivers/iio/adc/viperboard_adc.c b/drivers/iio/adc/viperboard_adc.c
>> index 09727a7..94f404d 100644
>> --- a/drivers/iio/adc/viperboard_adc.c
>> +++ b/drivers/iio/adc/viperboard_adc.c
>> @@ -42,11 +42,6 @@ struct vprbrd_adc {
>>  	.indexed = 1,					\
>>  	.channel = _index,				\
>>  	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),	\
>> -	.scan_index = _index,				\
>> -	.scan_type = {					\
>> -		.sign = 'u',				\
>> -		.realbits = 8,				\
>> -		.storagebits = 8,			\
>>  	},						\
Too many brackets here - picked up by the autobuilders.

Now fixed up in my tree (note I now push to a testing branch specifically
to get build coverage) before I push out to the togreg branch.

thanks,

Jonathan
>>  }
>>  
>> @@ -73,7 +68,7 @@ static int vprbrd_iio_read_raw(struct iio_dev *iio_dev,
>>  		mutex_lock(&vb->lock);
>>  
>>  		admsg->cmd = VPRBRD_ADC_CMD_GET;
>> -		admsg->chan = chan->scan_index;
>> +		admsg->chan = chan->channel;
>>  		admsg->val = 0x00;
>>  
>>  		ret = usb_control_msg(vb->usb_dev,
>>
> --
> 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
>