Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752576AbdCEEuu (ORCPT <rfc822;w@1wt.eu>);
        Sat, 4 Mar 2017 23:50:50 -0500
Received: from mail-ot0-f195.google.com ([74.125.82.195]:36840 "EHLO
        mail-ot0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751338AbdCEEut (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Sat, 4 Mar 2017 23:50:49 -0500
MIME-Version: 1.0
In-Reply-To: <CALrZqyP2BpnP=XAptdAZBcRP=s4ezpWcTrU9X2XvgZrikHM5Jw@mail.gmail.com>
References: <20170302130459.GA31856@singhal-Inspiron-5558> <fe8d6a85-3d4e-8019-937b-22389b942da3@tronnes.org>
 <20170305000632.GA5628@d830.WORKGROUP> <CALrZqyP2BpnP=XAptdAZBcRP=s4ezpWcTrU9X2XvgZrikHM5Jw@mail.gmail.com>
From: Vaishali Thakkar <vthakkar1994@gmail.com>
Date: Sun, 5 Mar 2017 10:20:47 +0530
Message-ID: <CAK-LDbKxL1q4n+igMJZtcL58z9YDCiVqc+yFSvT4EnEe2Z9DYQ@mail.gmail.com>
Subject: Re: [Outreachy kernel] Re: [PATCH v2 4/6] staging: fbtft: Fix sparse
 warnings of incorrect type in assignment
To: SIMRAN SINGHAL <singhalsimran0@gmail.com>
Cc: Alison Schofield <amsfield22@gmail.com>,
        =?UTF-8?Q?Noralf_Tr=C3=B8nnes?= <noralf@tronnes.org>,
        Greg KH <gregkh@linuxfoundation.org>, devel@driverdev.osuosl.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        sergio.paracuellos@gmail.com,
        Juliana Rodrigues <juliana.orod@gmail.com>,
        thomas.petazzoni@free-electrons.com,
        outreachy-kernel <outreachy-kernel@googlegroups.com>
Content-Type: text/plain; charset=UTF-8
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v254pRi6028844
Content-Length: 5421
Lines: 133

On Sun, Mar 5, 2017 at 10:10 AM, SIMRAN SINGHAL
<singhalsimran0@gmail.com> wrote:
> On Sun, Mar 5, 2017 at 5:36 AM, Alison Schofield <amsfield22@gmail.com> 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@gmail.com>
>>> >---
>>> >  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@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/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@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/CALrZqyP2BpnP%3DXAptdAZBcRP%3Ds4ezpWcTrU9X2XvgZrikHM5Jw%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.



-- 
Vaishali
http://vaishalithakkar.in/