Re: [Outreachy kernel] Re: [PATCH v2 4/6] staging: fbtft: Fix sparse warnings of incorrect type in assignment

[Vaishali Thakkar]

On Sun, Mar 5, 2017 at 10:10 AM, SIMRAN SINGHAL
<singhalsimran0@xxxxxxxxx> wrote:
> On Sun, Mar 5, 2017 at 5:36 AM, Alison Schofield <amsfield22@xxxxxxxxx> wrote:
>> On Thu, Mar 02, 2017 at 02:26:37PM +0100, Noralf TrÃnnes wrote:
>>>
>>> Den 02.03.2017 14.04, skrev simran singhal:
>>> >This patch fixes the following sparse warnings:
>>> >
>>> >drivers/staging/fbtft/fbtft-bus.c:166:36: warning: incorrect type in assignment (different base types)
>>> >drivers/staging/fbtft/fbtft-bus.c:166:36:    expected unsigned short [unsigned] [short] [usertype] <noident>
>>> >drivers/staging/fbtft/fbtft-bus.c:166:36:    got restricted __be16 [usertype] <noident>
>>> >
>>> >drivers/staging/fbtft/fbtft-io.c:74:29: warning: incorrect type in assignment (different base types)
>>> >drivers/staging/fbtft/fbtft-io.c:74:29:    expected unsigned long long [unsigned] [long] [long long] [usertype] <noident>
>>> >drivers/staging/fbtft/fbtft-io.c:74:29:    got restricted __be64 [usertype] <noident>
>>> >
>>> >Signed-off-by: simran singhal <singhalsimran0@xxxxxxxxx>
>>> >---
>>> >  v2:
>>> >    -changed commit message
>>> >
>>> >  drivers/staging/fbtft/fbtft-bus.c | 2 +-
>>> >  drivers/staging/fbtft/fbtft-io.c  | 2 +-
>>> >  2 files changed, 2 insertions(+), 2 deletions(-)
>>> >
>>> >diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
>>> >index ec45043..df2223e 100644
>>> >--- a/drivers/staging/fbtft/fbtft-bus.c
>>> >+++ b/drivers/staging/fbtft/fbtft-bus.c
>>> >@@ -163,7 +163,7 @@ int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len)
>>> >                                             to_copy, remain - to_copy);
>>> >             for (i = 0; i < to_copy; i++)
>>> >-                    txbuf16[i] = cpu_to_be16(vmem16[i]);
>>> >+                    txbuf16[i] = vmem16[i];
>>>
>>> This change breaks functionality on little endian machines like
>>> the Raspberry Pi.
>>>
>>>
>>> Noralf.
>>>
>>
>> Hi Simran,
>>
>> It's probably good to get back to this one while we have Noralf's
>> attention.
>>
>> While the change above - in fbtft-bus.c is a problem, the change
>> below in fbtft-io.c looks ok.  So, compare what you did.  You can't
>> *not* do the endian conversion.
>>
>> This is a cleanup change, a cosmetic change.  There is no underlying
>> bug, so you just need to get the typing correct w/out affecting
>> behavior.
>> anyway.
>>
>> BTW: I think it's OK to pull this one out and send a v3 of this alone.
>> That would mean abandoning the patchset and doing them one at a time.
>> They are all in different drivers
>
> Hi alison,
>
> Thanks for the explaination.
>
> I will drop the changes I did in fbtft-bus.c and send a v3 of this alone.
>
> Can you please suggest me any source through which I can understand the
> concept of endianess. As right know I didn't understand it, I just
> tried to fix the
> warnings which I got through sparse and as you can see most of the fixes are
> wrong.
>
> Thanks!
> Simran
>

Hi Simran,

I would suggest to have a look at thie LWN article[1]. It is pretty old
article but explains very well on the concept of why these annotations
[__le16, __le32 etc] were introduced at first place and how Sparse
gives warnings based on these annotations.

In addition, you should look at the definitions of byte ordering macros.
For example, in this example we have 'cpu_to_be64' then you can go
and look at the definition of that macro see where they are used and
why they are used in the first place.

Thanks.

[1] https://lwn.net/Articles/205624/

>
>> alisons
>
>>
>>
>>
>>> >             vmem16 = vmem16 + to_copy;
>>> >             ret = par->fbtftops.write(par, par->txbuf.buf,
>>> >diff --git a/drivers/staging/fbtft/fbtft-io.c b/drivers/staging/fbtft/fbtft-io.c
>>> >index d868405..ffb9a3b 100644
>>> >--- a/drivers/staging/fbtft/fbtft-io.c
>>> >+++ b/drivers/staging/fbtft/fbtft-io.c
>>> >@@ -71,7 +71,7 @@ int fbtft_write_spi_emulate_9(struct fbtft_par *par, void *buf, size_t len)
>>> >                     src++;
>>> >             }
>>> >             tmp |= ((*src & 0x0100) ? 1 : 0);
>>> >-            *(u64 *)dst = cpu_to_be64(tmp);
>>> >+            *(__be64 *)dst = cpu_to_be64(tmp);
>>> >             dst += 8;
>>> >             *dst++ = (u8)(*src++ & 0x00FF);
>>> >             added++;
>>>
>>> --
>>> 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@xxxxxxxxxxxxxxxxx
>>> To post to this group, send email to outreachy-kernel@xxxxxxxxxxxxxxxxx
>>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/fe8d6a85-3d4e-8019-937b-22389b942da3%40tronnes.org.
>>> 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@xxxxxxxxxxxxxxxxx
> To post to this group, send email to outreachy-kernel@xxxxxxxxxxxxxxxxx
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CALrZqyP2BpnP%3DXAptdAZBcRP%3Ds4ezpWcTrU9X2XvgZrikHM5Jw%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.



-- 
Vaishali
http://vaishalithakkar.in/