[PATCH] twl4030-madc: Request processed values in twl4030_get_madc_conversion

[PATCH] twl4030-madc: Request processed values in twl4030_get_madc_conversion

[Paul Kocialkowski]

Not setting the raw parameter in the request causes it to be randomly
initialized to a value that might be different from zero or zero. This leads to
values that are randomly either raw or processed, making it very difficult to
make reliable use of the values.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 drivers/iio/adc/twl4030-madc.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/twl4030-madc.c b/drivers/iio/adc/twl4030-madc.c
index 7de1c4c..eb86786 100644
--- a/drivers/iio/adc/twl4030-madc.c
+++ b/drivers/iio/adc/twl4030-madc.c
@@ -645,6 +645,7 @@ int twl4030_get_madc_conversion(int channel_no)
 	req.channels = (1 << channel_no);
 	req.method = TWL4030_MADC_SW2;
 	req.active = 0;
+	req.raw = 0;
 	req.func_cb = NULL;
 	ret = twl4030_madc_conversion(&req);
 	if (ret < 0)
-- 
1.7.9.5

Re: [PATCH] twl4030-madc: Request processed values in twl4030_get_madc_conversion

[Jonathan Cameron]

On 24/05/14 13:09, Paul Kocialkowski wrote:
> Not setting the raw parameter in the request causes it to be randomly
> initialized to a value that might be different from zero or zero. This leads to
> values that are randomly either raw or processed, making it very difficult to
> make reliable use of the values.
>
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
This one seems obvious enough to me that I've applied it to the fixes-togreg
branch. Note you should cc the author or if the log makes it clear someone
else has been working with the driver recently, then cc them.
In this case Sebastian.

With quite a few of these mfd element drivers, the chance the relevant author
is still subscribed to the list is next to none (here goes for being proved
wrong ;)

Thanks for the patch!

Jonathan


> ---
>   drivers/iio/adc/twl4030-madc.c |    1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/adc/twl4030-madc.c b/drivers/iio/adc/twl4030-madc.c
> index 7de1c4c..eb86786 100644
> --- a/drivers/iio/adc/twl4030-madc.c
> +++ b/drivers/iio/adc/twl4030-madc.c
> @@ -645,6 +645,7 @@ int twl4030_get_madc_conversion(int channel_no)
>   	req.channels = (1 << channel_no);
>   	req.method = TWL4030_MADC_SW2;
>   	req.active = 0;
> +	req.raw = 0;
>   	req.func_cb = NULL;
>   	ret = twl4030_madc_conversion(&req);
>   	if (ret < 0)
>

Re: [PATCH] twl4030-madc: Request processed values in twl4030_get_madc_conversion

[Sebastian Reichel]

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

Hi,

On Mon, Jun 02, 2014 at 07:00:06PM +0100, Jonathan Cameron wrote:
> On 24/05/14 13:09, Paul Kocialkowski wrote:
> >Not setting the raw parameter in the request causes it to be randomly
> >initialized to a value that might be different from zero or zero. This leads to
> >values that are randomly either raw or processed, making it very difficult to
> >make reliable use of the values.
> >
> >Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
>
> This one seems obvious enough to me that I've applied it to the fixes-togreg
> branch. Note you should cc the author or if the log makes it clear someone
> else has been working with the driver recently, then cc them.
> In this case Sebastian.

The commit message looks a bit odd "[...] different from zero or
zero". In case this is still relevant:

Acked-By: Sebastian Reichel <sre@kernel.org>

> [...]

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] twl4030-madc: Request processed values in twl4030_get_madc_conversion

[Paul Kocialkowski]

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

Le lundi 02 juin 2014 à 20:10 +0200, Sebastian Reichel a écrit :
> Hi,
> 
> On Mon, Jun 02, 2014 at 07:00:06PM +0100, Jonathan Cameron wrote:
> > On 24/05/14 13:09, Paul Kocialkowski wrote:
> > >Not setting the raw parameter in the request causes it to be randomly
> > >initialized to a value that might be different from zero or zero. This leads to
> > >values that are randomly either raw or processed, making it very difficult to
> > >make reliable use of the values.
> > >
> > >Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> >
> > This one seems obvious enough to me that I've applied it to the fixes-togreg
> > branch. Note you should cc the author or if the log makes it clear someone
> > else has been working with the driver recently, then cc them.
> > In this case Sebastian.

Thanks for your feedback and sorry that I didn't check the previous
contributors to the driver!

> The commit message looks a bit odd "[...] different from zero or
> zero".

How about replacing that with: "that may or may not be zero" and
inversing "processed" and "raw" in the next sentence (if that's not too
late)? The commit message would be:

Not setting the raw parameter in the request causes it to be randomly
initialized to a value that may or may not be zero. This leads to
values that are randomly either processed or raw, making it very
difficult to make reliable use of the values.

> In case this is still relevant:
> 
> Acked-By: Sebastian Reichel <sre@kernel.org>

Paul

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] twl4030-madc: Request processed values in twl4030_get_madc_conversion

[Jonathan Cameron]

On 02/06/14 20:17, Paul Kocialkowski wrote:
> Le lundi 02 juin 2014 à 20:10 +0200, Sebastian Reichel a écrit :
>> Hi,
>>
>> On Mon, Jun 02, 2014 at 07:00:06PM +0100, Jonathan Cameron wrote:
>>> On 24/05/14 13:09, Paul Kocialkowski wrote:
>>>> Not setting the raw parameter in the request causes it to be randomly
>>>> initialized to a value that might be different from zero or zero. This leads to
>>>> values that are randomly either raw or processed, making it very difficult to
>>>> make reliable use of the values.
>>>>
>>>> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
>>>
>>> This one seems obvious enough to me that I've applied it to the fixes-togreg
>>> branch. Note you should cc the author or if the log makes it clear someone
>>> else has been working with the driver recently, then cc them.
>>> In this case Sebastian.
>
> Thanks for your feedback and sorry that I didn't check the previous
> contributors to the driver!
>
>> The commit message looks a bit odd "[...] different from zero or
>> zero".
>
> How about replacing that with: "that may or may not be zero" and
> inversing "processed" and "raw" in the next sentence (if that's not too
> late)? The commit message would be:
>
> Not setting the raw parameter in the request causes it to be randomly
> initialized to a value that may or may not be zero. This leads to
> values that are randomly either processed or raw, making it very
> difficult to make reliable use of the values.
Whilst original text was a little unwieldy it was clear enough so I've left
it bit.
>
>> In case this is still relevant:
>>
>> Acked-By: Sebastian Reichel <sre@kernel.org>
>
> Paul
>

Re: [PATCH] twl4030-madc: Request processed values in twl4030_get_madc_conversion

[Jonathan Cameron]

On 02/06/14 19:10, Sebastian Reichel wrote:
> Hi,
>
> On Mon, Jun 02, 2014 at 07:00:06PM +0100, Jonathan Cameron wrote:
>> On 24/05/14 13:09, Paul Kocialkowski wrote:
>>> Not setting the raw parameter in the request causes it to be randomly
>>> initialized to a value that might be different from zero or zero. This leads to
>>> values that are randomly either raw or processed, making it very difficult to
>>> make reliable use of the values.
>>>
>>> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
>>
>> This one seems obvious enough to me that I've applied it to the fixes-togreg
>> branch. Note you should cc the author or if the log makes it clear someone
>> else has been working with the driver recently, then cc them.
>> In this case Sebastian.
>
> The commit message looks a bit odd "[...] different from zero or
> zero". In case this is still relevant:
>
> Acked-By: Sebastian Reichel <sre@kernel.org>
Given I was a muppet and applied it to the wrong branch, I've added the ack
and now applied it to the fixes-togreg branch....

>
>> [...]
>
> -- Sebastian
>