2017-03-02 13:15:03

by Simran Singhal

[permalink] [raw]
Subject: [PATCH v2 4/6] staging: fbtft: Fix sparse warnings of incorrect type in assignment

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 <[email protected]>
---

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];

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++;
--
2.7.4


2017-03-02 13:31:42

by Noralf Trønnes

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] staging: fbtft: Fix sparse warnings of incorrect type in assignment


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 <[email protected]>
> ---
>
> 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.

> 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++;

2017-03-05 00:06:39

by Alison Schofield

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

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 <[email protected]>
> >---
> > 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.

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 anyway.

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 [email protected].
> To post to this group, send email to [email protected].
> 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.

2017-03-05 04:40:34

by Simran Singhal

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

On Sun, Mar 5, 2017 at 5:36 AM, Alison Schofield <[email protected]> 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 <[email protected]>
>> >---
>> > 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



> 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 [email protected].
>> To post to this group, send email to [email protected].
>> 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.

2017-03-05 04:50:50

by Vaishali Thakkar

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

On Sun, Mar 5, 2017 at 10:10 AM, SIMRAN SINGHAL
<[email protected]> wrote:
> On Sun, Mar 5, 2017 at 5:36 AM, Alison Schofield <[email protected]> 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 <[email protected]>
>>> >---
>>> > 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 [email protected].
>>> To post to this group, send email to [email protected].
>>> 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 [email protected].
> To post to this group, send email to [email protected].
> 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/