From: Alison Schofield <amsfield22@gmail.com>
To: SIMRAN SINGHAL <singhalsimran0@gmail.com>
Cc: Julia Lawall <julia.lawall@lip6.fr>,
Greg KH <gregkh@linuxfoundation.org>,
devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
sergio.paracuellos@gmail.com,
Juliana Rodrigues <juliana.orod@gmail.com>,
thomas.petazzoni@free-electrons.com, noralf@tronnes.org,
outreachy-kernel@googlegroups.com
Subject: Re: [Outreachy kernel] [PATCH 1/6] staging: wlan-ng: Fix sparse warning of restricted __le16
Date: Thu, 2 Mar 2017 09:34:18 -0800 [thread overview]
Message-ID: <20170302173417.GA2934@d830.WORKGROUP> (raw)
In-Reply-To: <alpine.DEB.2.20.1703021213050.3414@hadrien>
On Thu, Mar 02, 2017 at 12:14:40PM +0100, Julia Lawall wrote:
>
>
> On Thu, 2 Mar 2017, SIMRAN SINGHAL wrote:
>
> > On Thu, Mar 2, 2017 at 3:20 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > >
> > >
> > > On Thu, 2 Mar 2017, Julia Lawall wrote:
> > >
> > >>
> > >>
> > >> On Thu, 2 Mar 2017, simran singhal wrote:
> > >>
> > >> > This patch fixes the following sparse warning:
> > >> > warning: cast to restricted __le16
> > >>
> > >> You commit message should not say just fix X. What have you done to carry
> > >> out the fix and why is this the correct approach?
> > >
> > > This comment applies to all of the patches in the series.
> > >
> > > julia
> > >
> > The changes and sparse warnings for them seems reasonable to me as
> > after doing the
> > change sparse was not showing error like cast to __le16 or something like this.
> > Also it compiled perfectly.
>
> This doesn't mean that it actually works. The function calls at least may
> have been put there for a reason. Maysbe you can find other patches that
> relate to this call and see what they do.
>
> julia
Hi Simran,
Just following on Julia's feedback and I see you've gotten
a message that this breaks little endian machines.
Good for you for trying to go deeper. I'd suggest taking this one
at a time and figure out what is going on with one of them. As Julia
says, with this kind of change, you will have to prove why it is
correct. (You might not be able to change any of these - I don't know,
but I know you'll learn something!)
Look here - and then pick one to trace through -
https://kernelnewbies.org/EndianIssues
Generally - probably best to test the waters with something of this
type, with a single patch. Get that nailed, then move onto a patchset,
if you have a pattern.
Heads down & good luck!
alisons
>
>
> > >>
> > >> julia
> > >>
> > >> >
> > >> > Signed-off-by: simran singhal <singhalsimran0@gmail.com>
> > >> > ---
> > >> > drivers/staging/wlan-ng/prism2sta.c | 51 ++++++++++++++++++-------------------
> > >> > 1 file changed, 25 insertions(+), 26 deletions(-)
> > >> >
> > >> > diff --git a/drivers/staging/wlan-ng/prism2sta.c b/drivers/staging/wlan-ng/prism2sta.c
> > >> > index 984804b..82d3a70 100644
> > >> > --- a/drivers/staging/wlan-ng/prism2sta.c
> > >> > +++ b/drivers/staging/wlan-ng/prism2sta.c
> > >> > @@ -372,10 +372,9 @@ static int prism2sta_mlmerequest(struct wlandevice *wlandev,
> > >> > qualmsg->noise.status =
> > >> > P80211ENUM_msgitem_status_data_ok;
> > >> >
> > >> > - qualmsg->link.data = le16_to_cpu(hw->qual.cq_curr_bss);
> > >> > - qualmsg->level.data =
> > >> > - le16_to_cpu(hw->qual.asl_curr_bss);
> > >> > - qualmsg->noise.data = le16_to_cpu(hw->qual.anl_curr_fc);
> > >> > + qualmsg->link.data = hw->qual.cq_curr_bss;
> > >> > + qualmsg->level.data = hw->qual.asl_curr_bss;
> > >> > + qualmsg->noise.data = hw->qual.anl_curr_fc;
> > >> > qualmsg->txrate.data = hw->txrate;
> > >> >
> > >> > break;
> > >> > @@ -603,10 +602,10 @@ static int prism2sta_getcardinfo(struct wlandevice *wlandev)
> > >> > }
> > >> >
> > >> > /* get all the nic id fields in host byte order */
> > >> > - hw->ident_nic.id = le16_to_cpu(hw->ident_nic.id);
> > >> > - hw->ident_nic.variant = le16_to_cpu(hw->ident_nic.variant);
> > >> > - hw->ident_nic.major = le16_to_cpu(hw->ident_nic.major);
> > >> > - hw->ident_nic.minor = le16_to_cpu(hw->ident_nic.minor);
> > >> > + hw->ident_nic.id = hw->ident_nic.id;
> > >> > + hw->ident_nic.variant = hw->ident_nic.variant;
> > >> > + hw->ident_nic.major = hw->ident_nic.major;
> > >> > + hw->ident_nic.minor = hw->ident_nic.minor;
> > >> >
> > >> > netdev_info(wlandev->netdev, "ident: nic h/w: id=0x%02x %d.%d.%d\n",
> > >> > hw->ident_nic.id, hw->ident_nic.major,
> > >> > @@ -622,10 +621,10 @@ static int prism2sta_getcardinfo(struct wlandevice *wlandev)
> > >> > }
> > >> >
> > >> > /* get all the private fw id fields in host byte order */
> > >> > - hw->ident_pri_fw.id = le16_to_cpu(hw->ident_pri_fw.id);
> > >> > - hw->ident_pri_fw.variant = le16_to_cpu(hw->ident_pri_fw.variant);
> > >> > - hw->ident_pri_fw.major = le16_to_cpu(hw->ident_pri_fw.major);
> > >> > - hw->ident_pri_fw.minor = le16_to_cpu(hw->ident_pri_fw.minor);
> > >> > + hw->ident_pri_fw.id = hw->ident_pri_fw.id;
> > >> > + hw->ident_pri_fw.variant = hw->ident_pri_fw.variant;
> > >> > + hw->ident_pri_fw.major = hw->ident_pri_fw.major;
> > >> > + hw->ident_pri_fw.minor = hw->ident_pri_fw.minor;
> > >> >
> > >> > netdev_info(wlandev->netdev, "ident: pri f/w: id=0x%02x %d.%d.%d\n",
> > >> > hw->ident_pri_fw.id, hw->ident_pri_fw.major,
> > >> > @@ -648,10 +647,10 @@ static int prism2sta_getcardinfo(struct wlandevice *wlandev)
> > >> > }
> > >> >
> > >> > /* get all the station fw id fields in host byte order */
> > >> > - hw->ident_sta_fw.id = le16_to_cpu(hw->ident_sta_fw.id);
> > >> > - hw->ident_sta_fw.variant = le16_to_cpu(hw->ident_sta_fw.variant);
> > >> > - hw->ident_sta_fw.major = le16_to_cpu(hw->ident_sta_fw.major);
> > >> > - hw->ident_sta_fw.minor = le16_to_cpu(hw->ident_sta_fw.minor);
> > >> > + hw->ident_sta_fw.id = hw->ident_sta_fw.id;
> > >> > + hw->ident_sta_fw.variant = hw->ident_sta_fw.variant;
> > >> > + hw->ident_sta_fw.major = hw->ident_sta_fw.major;
> > >> > + hw->ident_sta_fw.minor = hw->ident_sta_fw.minor;
> > >> >
> > >> > /* strip out the 'special' variant bits */
> > >> > hw->mm_mods = hw->ident_sta_fw.variant & GENMASK(15, 14);
> > >> > @@ -683,11 +682,11 @@ static int prism2sta_getcardinfo(struct wlandevice *wlandev)
> > >> > /* get all the Compatibility range, modem interface supplier
> > >> > * fields in byte order
> > >> > */
> > >> > - hw->cap_sup_mfi.role = le16_to_cpu(hw->cap_sup_mfi.role);
> > >> > - hw->cap_sup_mfi.id = le16_to_cpu(hw->cap_sup_mfi.id);
> > >> > - hw->cap_sup_mfi.variant = le16_to_cpu(hw->cap_sup_mfi.variant);
> > >> > - hw->cap_sup_mfi.bottom = le16_to_cpu(hw->cap_sup_mfi.bottom);
> > >> > - hw->cap_sup_mfi.top = le16_to_cpu(hw->cap_sup_mfi.top);
> > >> > + hw->cap_sup_mfi.role = hw->cap_sup_mfi.role;
> > >> > + hw->cap_sup_mfi.id = hw->cap_sup_mfi.id;
> > >> > + hw->cap_sup_mfi.variant = hw->cap_sup_mfi.variant;
> > >> > + hw->cap_sup_mfi.bottom = hw->cap_sup_mfi.bottom;
> > >> > + hw->cap_sup_mfi.top = hw->cap_sup_mfi.top;
> > >> >
> > >> > netdev_info(wlandev->netdev,
> > >> > "MFI:SUP:role=0x%02x:id=0x%02x:var=0x%02x:b/t=%d/%d\n",
> > >> > @@ -707,11 +706,11 @@ static int prism2sta_getcardinfo(struct wlandevice *wlandev)
> > >> > /* get all the Compatibility range, controller interface supplier
> > >> > * fields in byte order
> > >> > */
> > >> > - hw->cap_sup_cfi.role = le16_to_cpu(hw->cap_sup_cfi.role);
> > >> > - hw->cap_sup_cfi.id = le16_to_cpu(hw->cap_sup_cfi.id);
> > >> > - hw->cap_sup_cfi.variant = le16_to_cpu(hw->cap_sup_cfi.variant);
> > >> > - hw->cap_sup_cfi.bottom = le16_to_cpu(hw->cap_sup_cfi.bottom);
> > >> > - hw->cap_sup_cfi.top = le16_to_cpu(hw->cap_sup_cfi.top);
> > >> > + hw->cap_sup_cfi.role = hw->cap_sup_cfi.role;
> > >> > + hw->cap_sup_cfi.id = hw->cap_sup_cfi.id;
> > >> > + hw->cap_sup_cfi.variant = hw->cap_sup_cfi.variant;
> > >> > + hw->cap_sup_cfi.bottom = hw->cap_sup_cfi.bottom;
> > >> > + hw->cap_sup_cfi.top = hw->cap_sup_cfi.top;
> > >> >
> > >> > netdev_info(wlandev->netdev,
> > >> > "CFI:SUP:role=0x%02x:id=0x%02x:var=0x%02x:b/t=%d/%d\n",
> > >> > --
> > >> > 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/1488448146-29452-1-git-send-email-singhalsimran0%40gmail.com.
> > >> > 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/alpine.DEB.2.20.1703021047350.3414%40hadrien.
> > >> 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/CALrZqyNGZcP8c9oeS65r-PRTPpwcn664iB2WvWdx3ccD5KRRLg%40mail.gmail.com.
> > 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/alpine.DEB.2.20.1703021213050.3414%40hadrien.
> For more options, visit https://groups.google.com/d/optout.
next prev parent reply other threads:[~2017-03-02 17:34 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-02 9:49 [PATCH 1/6] staging: wlan-ng: Fix sparse warning of restricted __le16 simran singhal
2017-03-02 9:48 ` [Outreachy kernel] " Julia Lawall
2017-03-02 9:50 ` Julia Lawall
2017-03-02 11:11 ` SIMRAN SINGHAL
2017-03-02 11:14 ` Julia Lawall
2017-03-02 17:34 ` Alison Schofield [this message]
2017-03-02 9:49 ` [PATCH 2/6] staging: wlan-ng: Fix sparse warnings simran singhal
2017-03-02 9:49 ` [PATCH 3/6] staging: wlan-ng: Fix sparse warnings in hfa384x_usb.c simran singhal
2017-03-02 9:49 ` [PATCH 4/6] staging: fbtft: Fix sparse warnings of incorrect type in assignment simran singhal
2017-03-02 9:49 ` [PATCH 5/6] staging: rtl8192u: Fix Sparse warning of cast from restricted __le16 simran singhal
2017-03-02 9:49 ` [PATCH 6/6] staging: rtl8192u: Fix Sparse warning of cast to " simran singhal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170302173417.GA2934@d830.WORKGROUP \
--to=amsfield22@gmail.com \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=julia.lawall@lip6.fr \
--cc=juliana.orod@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=noralf@tronnes.org \
--cc=outreachy-kernel@googlegroups.com \
--cc=sergio.paracuellos@gmail.com \
--cc=singhalsimran0@gmail.com \
--cc=thomas.petazzoni@free-electrons.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).