* [PATCH v2] iio: temperature: maxim_thermocouple: Fix sparse endianness warnings
@ 2016-09-26 17:14 sayli karnik
2016-09-27 6:05 ` [Outreachy kernel] " Alison Schofield
0 siblings, 1 reply; 6+ messages in thread
From: sayli karnik @ 2016-09-26 17:14 UTC (permalink / raw)
To: outreachy-kernel
Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler
Fix following endianness warnings using sparse:
warning: cast to restricted __be16
warning: cast to restricted __be32
Use __be16 type for incoming read data when storage_bytes equals 2 and __be32
when storage_bytes equals 4. Since the variable allocated already has the
correct storage size the patch is classified as cosmetic.
Signed-off-by: sayli karnik <karniksayli1995@gmail.com>
---
v2:
Updated the subject line and changelog
Moved the spi_read function call inside respective case statements so that data
is read only once.
Moved the return statement after the switch case
drivers/iio/temperature/maxim_thermocouple.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/temperature/maxim_thermocouple.c b/drivers/iio/temperature/maxim_thermocouple.c
index 39dd202..2683b0f 100644
--- a/drivers/iio/temperature/maxim_thermocouple.c
+++ b/drivers/iio/temperature/maxim_thermocouple.c
@@ -123,22 +123,24 @@ static int maxim_thermocouple_read(struct maxim_thermocouple_data *data,
{
unsigned int storage_bytes = data->chip->read_size;
unsigned int shift = chan->scan_type.shift + (chan->address * 8);
- unsigned int buf;
+ __be16 buf1;
+ __be32 buf2;
int ret;
- ret = spi_read(data->spi, (void *) &buf, storage_bytes);
- if (ret)
- return ret;
-
switch (storage_bytes) {
case 2:
- *val = be16_to_cpu(buf);
+ ret = spi_read(data->spi, (void *) &buf1, storage_bytes);
+ *val = be16_to_cpu(buf1);
break;
case 4:
- *val = be32_to_cpu(buf);
+ ret = spi_read(data->spi, (void *) &buf2, storage_bytes);
+ *val = be32_to_cpu(buf2);
break;
}
+ if (ret)
+ return ret;
+
/* check to be sure this is a valid reading */
if (*val & data->chip->status_bit)
return -EINVAL;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] iio: temperature: maxim_thermocouple: Fix sparse endianness warnings
2016-09-26 17:14 [PATCH v2] iio: temperature: maxim_thermocouple: Fix sparse endianness warnings sayli karnik
@ 2016-09-27 6:05 ` Alison Schofield
2016-09-27 8:07 ` Lars-Peter Clausen
2016-09-27 15:05 ` sayli karnik
0 siblings, 2 replies; 6+ messages in thread
From: Alison Schofield @ 2016-09-27 6:05 UTC (permalink / raw)
To: sayli karnik
Cc: outreachy-kernel, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler
On Mon, Sep 26, 2016 at 10:44:04PM +0530, sayli karnik wrote:
> Fix following endianness warnings using sparse:
> warning: cast to restricted __be16
> warning: cast to restricted __be32
> Use __be16 type for incoming read data when storage_bytes equals 2 and __be32
> when storage_bytes equals 4. Since the variable allocated already has the
> correct storage size the patch is classified as cosmetic.
>
> Signed-off-by: sayli karnik <karniksayli1995@gmail.com>
Hi Sayli -
v2 code looks like it handles both cases.
I think you've made a bug fix!
When the storage_bytes == 2, it would work on LE systems but give
invalid data on BE systems. That unsigned int buf points to a location
where only 2 bytes of data are placed during spi_read. Those 2 bytes go
in the upper bytes. Then when the be16_to_cpu() happens that buf
gets cast to 16bits. We get the upper bytes on LE systems (good bits)
but the lower bytes on BE systems (bad bits)
Lars - Please check my logic and the code too. If this is a bug
let us know if Sayli should include a fix tag.
Pending Lars' review, you can update the changelog.
Take a further look at storage_bytes and you'll find when it is
set to 2 and when it is set to 4. That would be good information
to include in the changelog. You'll find that this is only a bug
in one of the two chips this driver supports.
Check your precommit checkpatch hooks. There are still CHECKs in
the spi_read lines of code.
And, back to the less exciting. It would be good for that commit
message to say what you did: replace unsigned int with __be16/32
I know you're short of space on that line. It's ok to drop the
subdir (temperature) and just state the driver as in:
iio: maxim_thermocouple: commit message.
Please 'CC linux-iio on v3 and the module author, from bottom of
source file.
Thanks Sayli,
alisons
> ---
> v2:
> Updated the subject line and changelog
> Moved the spi_read function call inside respective case statements so that data
> is read only once.
> Moved the return statement after the switch case
>
> drivers/iio/temperature/maxim_thermocouple.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/temperature/maxim_thermocouple.c b/drivers/iio/temperature/maxim_thermocouple.c
> index 39dd202..2683b0f 100644
> --- a/drivers/iio/temperature/maxim_thermocouple.c
> +++ b/drivers/iio/temperature/maxim_thermocouple.c
> @@ -123,22 +123,24 @@ static int maxim_thermocouple_read(struct maxim_thermocouple_data *data,
> {
> unsigned int storage_bytes = data->chip->read_size;
> unsigned int shift = chan->scan_type.shift + (chan->address * 8);
> - unsigned int buf;
> + __be16 buf1;
> + __be32 buf2;
> int ret;
>
> - ret = spi_read(data->spi, (void *) &buf, storage_bytes);
> - if (ret)
> - return ret;
> -
> switch (storage_bytes) {
> case 2:
> - *val = be16_to_cpu(buf);
> + ret = spi_read(data->spi, (void *) &buf1, storage_bytes);
> + *val = be16_to_cpu(buf1);
> break;
> case 4:
> - *val = be32_to_cpu(buf);
> + ret = spi_read(data->spi, (void *) &buf2, storage_bytes);
> + *val = be32_to_cpu(buf2);
> break;
> }
>
> + if (ret)
> + return ret;
> +
> /* check to be sure this is a valid reading */
> if (*val & data->chip->status_bit)
> return -EINVAL;
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160926171404.GA11773%40sayli-HP-15-Notebook-PC.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] iio: temperature: maxim_thermocouple: Fix sparse endianness warnings
2016-09-27 6:05 ` [Outreachy kernel] " Alison Schofield
@ 2016-09-27 8:07 ` Lars-Peter Clausen
2016-09-27 15:05 ` sayli karnik
1 sibling, 0 replies; 6+ messages in thread
From: Lars-Peter Clausen @ 2016-09-27 8:07 UTC (permalink / raw)
To: Alison Schofield, sayli karnik
Cc: outreachy-kernel, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald-Stadler
On 09/27/2016 08:05 AM, Alison Schofield wrote:
> On Mon, Sep 26, 2016 at 10:44:04PM +0530, sayli karnik wrote:
>> Fix following endianness warnings using sparse:
>> warning: cast to restricted __be16
>> warning: cast to restricted __be32
>> Use __be16 type for incoming read data when storage_bytes equals 2 and __be32
>> when storage_bytes equals 4. Since the variable allocated already has the
>> correct storage size the patch is classified as cosmetic.
>>
>> Signed-off-by: sayli karnik <karniksayli1995@gmail.com>
>
> Hi Sayli -
>
> v2 code looks like it handles both cases.
>
> I think you've made a bug fix!
>
> When the storage_bytes == 2, it would work on LE systems but give
> invalid data on BE systems. That unsigned int buf points to a location
> where only 2 bytes of data are placed during spi_read. Those 2 bytes go
> in the upper bytes. Then when the be16_to_cpu() happens that buf
> gets cast to 16bits. We get the upper bytes on LE systems (good bits)
> but the lower bytes on BE systems (bad bits)
>
> Lars - Please check my logic and the code too. If this is a bug
> let us know if Sayli should include a fix tag.
Yes, your analysis is correct.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] iio: temperature: maxim_thermocouple: Fix sparse endianness warnings
2016-09-27 6:05 ` [Outreachy kernel] " Alison Schofield
2016-09-27 8:07 ` Lars-Peter Clausen
@ 2016-09-27 15:05 ` sayli karnik
2016-09-27 16:20 ` Julia Lawall
2016-09-27 16:25 ` Alison Schofield
1 sibling, 2 replies; 6+ messages in thread
From: sayli karnik @ 2016-09-27 15:05 UTC (permalink / raw)
To: Alison Schofield; +Cc: outreachy-kernel
On Tue, Sep 27, 2016 at 11:35 AM, Alison Schofield <amsfield22@gmail.com> wrote:
> On Mon, Sep 26, 2016 at 10:44:04PM +0530, sayli karnik wrote:
>> Fix following endianness warnings using sparse:
>> warning: cast to restricted __be16
>> warning: cast to restricted __be32
>> Use __be16 type for incoming read data when storage_bytes equals 2 and __be32
>> when storage_bytes equals 4. Since the variable allocated already has the
>> correct storage size the patch is classified as cosmetic.
>>
>> Signed-off-by: sayli karnik <karniksayli1995@gmail.com>
>
> Hi Sayli -
>
> v2 code looks like it handles both cases.
>
> I think you've made a bug fix!
>
> When the storage_bytes == 2, it would work on LE systems but give
> invalid data on BE systems. That unsigned int buf points to a location
> where only 2 bytes of data are placed during spi_read. Those 2 bytes go
> in the upper bytes. Then when the be16_to_cpu() happens that buf
> gets cast to 16bits. We get the upper bytes on LE systems (good bits)
> but the lower bytes on BE systems (bad bits)
>
Yes I agree..so it's a bug fix for storage_bytes=2 and cosmetic for
storage_bytes=4.
> Lars - Please check my logic and the code too. If this is a bug
> let us know if Sayli should include a fix tag.
>
> Pending Lars' review, you can update the changelog.
>
> Take a further look at storage_bytes and you'll find when it is
> set to 2 and when it is set to 4. That would be good information
> to include in the changelog. You'll find that this is only a bug
> in one of the two chips this driver supports.
>
> Check your precommit checkpatch hooks. There are still CHECKs in
> the spi_read lines of code.
>
I did run checkpatch, but no CHECKs were caught.
This is strange, only errors and warnings were caught when I ran:
perl scripts/checkpatch.pl -f drivers/iio/temperature/* | less
I have run checkpatch many times on staging drivers until now.
I can only predict that the CHECK in my code must be that there
shouldn't be a space after the type cast.
> And, back to the less exciting. It would be good for that commit
> message to say what you did: replace unsigned int with __be16/32
> I know you're short of space on that line. It's ok to drop the
> subdir (temperature) and just state the driver as in:
> iio: maxim_thermocouple: commit message.
>
> Please 'CC linux-iio on v3 and the module author, from bottom of
> source file.
>
Sure!
> Thanks Sayli,
> alisons
>
>> ---
>> v2:
>> Updated the subject line and changelog
>> Moved the spi_read function call inside respective case statements so that data
>> is read only once.
>> Moved the return statement after the switch case
>>
>> drivers/iio/temperature/maxim_thermocouple.c | 16 +++++++++-------
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iio/temperature/maxim_thermocouple.c b/drivers/iio/temperature/maxim_thermocouple.c
>> index 39dd202..2683b0f 100644
>> --- a/drivers/iio/temperature/maxim_thermocouple.c
>> +++ b/drivers/iio/temperature/maxim_thermocouple.c
>> @@ -123,22 +123,24 @@ static int maxim_thermocouple_read(struct maxim_thermocouple_data *data,
>> {
>> unsigned int storage_bytes = data->chip->read_size;
>> unsigned int shift = chan->scan_type.shift + (chan->address * 8);
>> - unsigned int buf;
>> + __be16 buf1;
>> + __be32 buf2;
>> int ret;
>>
>> - ret = spi_read(data->spi, (void *) &buf, storage_bytes);
>> - if (ret)
>> - return ret;
>> -
>> switch (storage_bytes) {
>> case 2:
>> - *val = be16_to_cpu(buf);
>> + ret = spi_read(data->spi, (void *) &buf1, storage_bytes);
>> + *val = be16_to_cpu(buf1);
>> break;
>> case 4:
>> - *val = be32_to_cpu(buf);
>> + ret = spi_read(data->spi, (void *) &buf2, storage_bytes);
>> + *val = be32_to_cpu(buf2);
>> break;
>> }
>>
>> + if (ret)
>> + return ret;
>> +
>> /* check to be sure this is a valid reading */
>> if (*val & data->chip->status_bit)
>> return -EINVAL;
>> --
>> 2.7.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160926171404.GA11773%40sayli-HP-15-Notebook-PC.
>> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] iio: temperature: maxim_thermocouple: Fix sparse endianness warnings
2016-09-27 15:05 ` sayli karnik
@ 2016-09-27 16:20 ` Julia Lawall
2016-09-27 16:25 ` Alison Schofield
1 sibling, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2016-09-27 16:20 UTC (permalink / raw)
To: sayli karnik; +Cc: Alison Schofield, outreachy-kernel
On Tue, 27 Sep 2016, sayli karnik wrote:
> On Tue, Sep 27, 2016 at 11:35 AM, Alison Schofield <amsfield22@gmail.com> wrote:
> > On Mon, Sep 26, 2016 at 10:44:04PM +0530, sayli karnik wrote:
> >> Fix following endianness warnings using sparse:
> >> warning: cast to restricted __be16
> >> warning: cast to restricted __be32
> >> Use __be16 type for incoming read data when storage_bytes equals 2 and __be32
> >> when storage_bytes equals 4. Since the variable allocated already has the
> >> correct storage size the patch is classified as cosmetic.
> >>
> >> Signed-off-by: sayli karnik <karniksayli1995@gmail.com>
> >
> > Hi Sayli -
> >
> > v2 code looks like it handles both cases.
> >
> > I think you've made a bug fix!
> >
> > When the storage_bytes == 2, it would work on LE systems but give
> > invalid data on BE systems. That unsigned int buf points to a location
> > where only 2 bytes of data are placed during spi_read. Those 2 bytes go
> > in the upper bytes. Then when the be16_to_cpu() happens that buf
> > gets cast to 16bits. We get the upper bytes on LE systems (good bits)
> > but the lower bytes on BE systems (bad bits)
> >
> Yes I agree..so it's a bug fix for storage_bytes=2 and cosmetic for
> storage_bytes=4.
>
> > Lars - Please check my logic and the code too. If this is a bug
> > let us know if Sayli should include a fix tag.
> >
> > Pending Lars' review, you can update the changelog.
> >
> > Take a further look at storage_bytes and you'll find when it is
> > set to 2 and when it is set to 4. That would be good information
> > to include in the changelog. You'll find that this is only a bug
> > in one of the two chips this driver supports.
> >
> > Check your precommit checkpatch hooks. There are still CHECKs in
> > the spi_read lines of code.
> >
> I did run checkpatch, but no CHECKs were caught.
> This is strange, only errors and warnings were caught when I ran:
> perl scripts/checkpatch.pl -f drivers/iio/temperature/* | less
> I have run checkpatch many times on staging drivers until now.
> I can only predict that the CHECK in my code must be that there
> shouldn't be a space after the type cast.
checkpatch may be less verbose on non-staging files. Try running
checkpatch on your patch, rather than on the file.
julia
>
> > And, back to the less exciting. It would be good for that commit
> > message to say what you did: replace unsigned int with __be16/32
> > I know you're short of space on that line. It's ok to drop the
> > subdir (temperature) and just state the driver as in:
> > iio: maxim_thermocouple: commit message.
> >
> > Please 'CC linux-iio on v3 and the module author, from bottom of
> > source file.
> >
> Sure!
>
> > Thanks Sayli,
> > alisons
> >
> >> ---
> >> v2:
> >> Updated the subject line and changelog
> >> Moved the spi_read function call inside respective case statements so that data
> >> is read only once.
> >> Moved the return statement after the switch case
> >>
> >> drivers/iio/temperature/maxim_thermocouple.c | 16 +++++++++-------
> >> 1 file changed, 9 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/iio/temperature/maxim_thermocouple.c b/drivers/iio/temperature/maxim_thermocouple.c
> >> index 39dd202..2683b0f 100644
> >> --- a/drivers/iio/temperature/maxim_thermocouple.c
> >> +++ b/drivers/iio/temperature/maxim_thermocouple.c
> >> @@ -123,22 +123,24 @@ static int maxim_thermocouple_read(struct maxim_thermocouple_data *data,
> >> {
> >> unsigned int storage_bytes = data->chip->read_size;
> >> unsigned int shift = chan->scan_type.shift + (chan->address * 8);
> >> - unsigned int buf;
> >> + __be16 buf1;
> >> + __be32 buf2;
> >> int ret;
> >>
> >> - ret = spi_read(data->spi, (void *) &buf, storage_bytes);
> >> - if (ret)
> >> - return ret;
> >> -
> >> switch (storage_bytes) {
> >> case 2:
> >> - *val = be16_to_cpu(buf);
> >> + ret = spi_read(data->spi, (void *) &buf1, storage_bytes);
> >> + *val = be16_to_cpu(buf1);
> >> break;
> >> case 4:
> >> - *val = be32_to_cpu(buf);
> >> + ret = spi_read(data->spi, (void *) &buf2, storage_bytes);
> >> + *val = be32_to_cpu(buf2);
> >> break;
> >> }
> >>
> >> + if (ret)
> >> + return ret;
> >> +
> >> /* check to be sure this is a valid reading */
> >> if (*val & data->chip->status_bit)
> >> return -EINVAL;
> >> --
> >> 2.7.4
> >>
> >> --
> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160926171404.GA11773%40sayli-HP-15-Notebook-PC.
> >> For more options, visit https://groups.google.com/d/optout.
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAKG5xWhp5iZ8Ae6j90m7KVjeAz4KEUcarqJ_R6pu8pL2WaZ5Kg%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH v2] iio: temperature: maxim_thermocouple: Fix sparse endianness warnings
2016-09-27 15:05 ` sayli karnik
2016-09-27 16:20 ` Julia Lawall
@ 2016-09-27 16:25 ` Alison Schofield
1 sibling, 0 replies; 6+ messages in thread
From: Alison Schofield @ 2016-09-27 16:25 UTC (permalink / raw)
To: sayli karnik; +Cc: outreachy-kernel, jic23, knaack.h, pmeerw, lars
On Tue, Sep 27, 2016 at 08:35:27PM +0530, sayli karnik wrote:
> On Tue, Sep 27, 2016 at 11:35 AM, Alison Schofield <amsfield22@gmail.com> wrote:
> > On Mon, Sep 26, 2016 at 10:44:04PM +0530, sayli karnik wrote:
> >> Fix following endianness warnings using sparse:
> >> warning: cast to restricted __be16
> >> warning: cast to restricted __be32
> >> Use __be16 type for incoming read data when storage_bytes equals 2 and __be32
> >> when storage_bytes equals 4. Since the variable allocated already has the
> >> correct storage size the patch is classified as cosmetic.
> >>
> >> Signed-off-by: sayli karnik <karniksayli1995@gmail.com>
> >
> > Hi Sayli -
> >
> > v2 code looks like it handles both cases.
> >
> > I think you've made a bug fix!
> >
> > When the storage_bytes == 2, it would work on LE systems but give
> > invalid data on BE systems. That unsigned int buf points to a location
> > where only 2 bytes of data are placed during spi_read. Those 2 bytes go
> > in the upper bytes. Then when the be16_to_cpu() happens that buf
> > gets cast to 16bits. We get the upper bytes on LE systems (good bits)
> > but the lower bytes on BE systems (bad bits)
> >
> Yes I agree..so it's a bug fix for storage_bytes=2 and cosmetic for
> storage_bytes=4.
You got it. See more inline below...
>
> > Lars - Please check my logic and the code too. If this is a bug
> > let us know if Sayli should include a fix tag.
> >
> > Pending Lars' review, you can update the changelog.
> >
> > Take a further look at storage_bytes and you'll find when it is
> > set to 2 and when it is set to 4. That would be good information
> > to include in the changelog. You'll find that this is only a bug
> > in one of the two chips this driver supports.
You'll state the impact of the issue in your changelog. It's only
an issue on Big Endian systems with one of the supported chips (look
that up)
In regard to the Fix tag:
I probably didn't need to ask if we should put it a fix tag.
We should just put one in, explain the impact of of the bug, and then
Jonathan can make the decision from there. To make a fix tag,
we go back in the git log to find when this issue was introduced. This
one's easy...short history. Here's the tag. You will put it after
your Signed off line, but above the ---. You can see how it's done
in some other logs on in linus-iio email.
Fixes: 1f25ca11d84a ("iio: temperature: add support for Maxim thermocouple chips
> >
> > Check your precommit checkpatch hooks. There are still CHECKs in
> > the spi_read lines of code.
> >
> I did run checkpatch, but no CHECKs were caught.
> This is strange, only errors and warnings were caught when I ran:
> perl scripts/checkpatch.pl -f drivers/iio/temperature/* | less
> I have run checkpatch many times on staging drivers until now.
> I can only predict that the CHECK in my code must be that there
> shouldn't be a space after the type cast.
Your prediction is right. Found with checkpatch --strict option.
CHECK: No space is necessary after a cast
#132: FILE: maxim_thermocouple.c:132:
+ ret = spi_read(data->spi, (void *) &buf1, storage_bytes);
CHECK: No space is necessary after a cast
#136: FILE: maxim_thermocouple.c:136:
+ ret = spi_read(data->spi, (void *) &buf2, storage_bytes);
I think you're set. Create a new version and let's get it reviewed by
linux-iio further.
You dropped the 'cc list on this thread. Adding them back in.
Thanks Sayli,
alisons
>
> > And, back to the less exciting. It would be good for that commit
> > message to say what you did: replace unsigned int with __be16/32
> > I know you're short of space on that line. It's ok to drop the
> > subdir (temperature) and just state the driver as in:
> > iio: maxim_thermocouple: commit message.
> >
> > Please 'CC linux-iio on v3 and the module author, from bottom of
> > source file.
> >
> Sure!
>
> > Thanks Sayli,
> > alisons
> >
> >> ---
> >> v2:
> >> Updated the subject line and changelog
> >> Moved the spi_read function call inside respective case statements so that data
> >> is read only once.
> >> Moved the return statement after the switch case
> >>
> >> drivers/iio/temperature/maxim_thermocouple.c | 16 +++++++++-------
> >> 1 file changed, 9 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/iio/temperature/maxim_thermocouple.c b/drivers/iio/temperature/maxim_thermocouple.c
> >> index 39dd202..2683b0f 100644
> >> --- a/drivers/iio/temperature/maxim_thermocouple.c
> >> +++ b/drivers/iio/temperature/maxim_thermocouple.c
> >> @@ -123,22 +123,24 @@ static int maxim_thermocouple_read(struct maxim_thermocouple_data *data,
> >> {
> >> unsigned int storage_bytes = data->chip->read_size;
> >> unsigned int shift = chan->scan_type.shift + (chan->address * 8);
> >> - unsigned int buf;
> >> + __be16 buf1;
> >> + __be32 buf2;
> >> int ret;
> >>
> >> - ret = spi_read(data->spi, (void *) &buf, storage_bytes);
> >> - if (ret)
> >> - return ret;
> >> -
> >> switch (storage_bytes) {
> >> case 2:
> >> - *val = be16_to_cpu(buf);
> >> + ret = spi_read(data->spi, (void *) &buf1, storage_bytes);
> >> + *val = be16_to_cpu(buf1);
> >> break;
> >> case 4:
> >> - *val = be32_to_cpu(buf);
> >> + ret = spi_read(data->spi, (void *) &buf2, storage_bytes);
> >> + *val = be32_to_cpu(buf2);
> >> break;
> >> }
> >>
> >> + if (ret)
> >> + return ret;
> >> +
> >> /* check to be sure this is a valid reading */
> >> if (*val & data->chip->status_bit)
> >> return -EINVAL;
> >> --
> >> 2.7.4
> >>
> >> --
> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160926171404.GA11773%40sayli-HP-15-Notebook-PC.
> >> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-27 16:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-26 17:14 [PATCH v2] iio: temperature: maxim_thermocouple: Fix sparse endianness warnings sayli karnik
2016-09-27 6:05 ` [Outreachy kernel] " Alison Schofield
2016-09-27 8:07 ` Lars-Peter Clausen
2016-09-27 15:05 ` sayli karnik
2016-09-27 16:20 ` Julia Lawall
2016-09-27 16:25 ` Alison Schofield
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.