linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Convert __le16 to cpu before casting to u32
@ 2017-03-25 23:24 Guillaume Brogi
  2017-04-08 10:31 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 6+ messages in thread
From: Guillaume Brogi @ 2017-03-25 23:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Bhumika Goyal, devel, linux-kernel


This patch fixes the following sparse warnings:
drivers/staging/rtl8192u/r8192U_dm.c:2307:49: warning: cast from restricted __le16
drivers/staging/rtl8192u/r8192U_dm.c:2308:44: warning: cast from restricted __le16
drivers/staging/rtl8192u/r8192U_dm.c:2309:44: warning: cast from restricted __le16

Those three members of qos_parameters are indeed __le16 so they should
be converted to the cpu's endianness before being cast to u32.

The lines are over the 80 character limit. They already were, and, for
the sake of readability, I don't think they should be split.

Signed-off-by: Guillaume Brogi <gui-gui@netcourrier.com>
---
 drivers/staging/rtl8192u/r8192U_dm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8192u/r8192U_dm.c b/drivers/staging/rtl8192u/r8192U_dm.c
index 9209aad0515e..5946c1f8d37d 100644
--- a/drivers/staging/rtl8192u/r8192U_dm.c
+++ b/drivers/staging/rtl8192u/r8192U_dm.c
@@ -2304,9 +2304,9 @@ static void dm_check_edca_turbo(
 				/*  For Each time updating EDCA parameter, reset EDCA turbo mode status. */
 				dm_init_edca_turbo(dev);
 				u1bAIFS = qos_parameters->aifs[0] * ((mode&(IEEE_G|IEEE_N_24G)) ? 9 : 20) + aSifsTime;
-				u4bAcParam = (((u32)(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET)|
-					(((u32)(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET)|
-					(((u32)(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET)|
+				u4bAcParam = (((u32)le16_to_cpu(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET) |
+					(((u32)le16_to_cpu(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET) |
+					(((u32)le16_to_cpu(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET) |
 					((u32)u1bAIFS << AC_PARAM_AIFS_OFFSET);
 				/*write_nic_dword(dev, WDCAPARA_ADD[i], u4bAcParam);*/
 				write_nic_dword(dev, EDCAPARA_BE,  u4bAcParam);
-- 
2.12.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Convert __le16 to cpu before casting to u32
  2017-03-25 23:24 [PATCH] Convert __le16 to cpu before casting to u32 Guillaume Brogi
@ 2017-04-08 10:31 ` Greg Kroah-Hartman
  2017-04-08 18:32   ` [PATCH] staging: rtl8192u: " Guillaume Brogi
  0 siblings, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2017-04-08 10:31 UTC (permalink / raw)
  To: Guillaume Brogi; +Cc: Bhumika Goyal, devel, linux-kernel

On Sun, Mar 26, 2017 at 12:24:14AM +0100, Guillaume Brogi wrote:
> 
> This patch fixes the following sparse warnings:
> drivers/staging/rtl8192u/r8192U_dm.c:2307:49: warning: cast from restricted __le16
> drivers/staging/rtl8192u/r8192U_dm.c:2308:44: warning: cast from restricted __le16
> drivers/staging/rtl8192u/r8192U_dm.c:2309:44: warning: cast from restricted __le16
> 
> Those three members of qos_parameters are indeed __le16 so they should
> be converted to the cpu's endianness before being cast to u32.
> 
> The lines are over the 80 character limit. They already were, and, for
> the sake of readability, I don't think they should be split.

Please fix up your subject: line to match other patches for this driver,
I almost missed it :(

> 
> Signed-off-by: Guillaume Brogi <gui-gui@netcourrier.com>
> ---
>  drivers/staging/rtl8192u/r8192U_dm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192u/r8192U_dm.c b/drivers/staging/rtl8192u/r8192U_dm.c
> index 9209aad0515e..5946c1f8d37d 100644
> --- a/drivers/staging/rtl8192u/r8192U_dm.c
> +++ b/drivers/staging/rtl8192u/r8192U_dm.c
> @@ -2304,9 +2304,9 @@ static void dm_check_edca_turbo(
>  				/*  For Each time updating EDCA parameter, reset EDCA turbo mode status. */
>  				dm_init_edca_turbo(dev);
>  				u1bAIFS = qos_parameters->aifs[0] * ((mode&(IEEE_G|IEEE_N_24G)) ? 9 : 20) + aSifsTime;
> -				u4bAcParam = (((u32)(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET)|
> -					(((u32)(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET)|
> -					(((u32)(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET)|
> +				u4bAcParam = (((u32)le16_to_cpu(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET) |
> +					(((u32)le16_to_cpu(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET) |
> +					(((u32)le16_to_cpu(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET) |
>  					((u32)u1bAIFS << AC_PARAM_AIFS_OFFSET);
>  				/*write_nic_dword(dev, WDCAPARA_ADD[i], u4bAcParam);*/
>  				write_nic_dword(dev, EDCAPARA_BE,  u4bAcParam);

How are you sure that this change is correct?  How did you verify it?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging: rtl8192u: Convert __le16 to cpu before casting to u32
  2017-04-08 10:31 ` Greg Kroah-Hartman
@ 2017-04-08 18:32   ` Guillaume Brogi
  2017-05-07 12:56     ` Guillaume Brogi
  0 siblings, 1 reply; 6+ messages in thread
From: Guillaume Brogi @ 2017-04-08 18:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Bhumika Goyal, devel, linux-kernel

On Sat, Apr 08, 2017 at 12:31:25PM +0200, Greg Kroah-Hartman wrote:
> On Sun, Mar 26, 2017 at 12:24:14AM +0100, Guillaume Brogi wrote:
> > 
> > This patch fixes the following sparse warnings:
> > drivers/staging/rtl8192u/r8192U_dm.c:2307:49: warning: cast from restricted __le16
> > drivers/staging/rtl8192u/r8192U_dm.c:2308:44: warning: cast from restricted __le16
> > drivers/staging/rtl8192u/r8192U_dm.c:2309:44: warning: cast from restricted __le16
> > 
> > Those three members of qos_parameters are indeed __le16 so they should
> > be converted to the cpu's endianness before being cast to u32.
> > 
> > The lines are over the 80 character limit. They already were, and, for
> > the sake of readability, I don't think they should be split.
> 
> Please fix up your subject: line to match other patches for this driver,
> I almost missed it :(
> 
Sorry about that. I realised my mistake after sending the email.

> > 
> > Signed-off-by: Guillaume Brogi <gui-gui@netcourrier.com>
> > ---
> >  drivers/staging/rtl8192u/r8192U_dm.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/staging/rtl8192u/r8192U_dm.c b/drivers/staging/rtl8192u/r8192U_dm.c
> > index 9209aad0515e..5946c1f8d37d 100644
> > --- a/drivers/staging/rtl8192u/r8192U_dm.c
> > +++ b/drivers/staging/rtl8192u/r8192U_dm.c
> > @@ -2304,9 +2304,9 @@ static void dm_check_edca_turbo(
> >  				/*  For Each time updating EDCA parameter, reset EDCA turbo mode status. */
> >  				dm_init_edca_turbo(dev);
> >  				u1bAIFS = qos_parameters->aifs[0] * ((mode&(IEEE_G|IEEE_N_24G)) ? 9 : 20) + aSifsTime;
> > -				u4bAcParam = (((u32)(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET)|
> > -					(((u32)(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET)|
> > -					(((u32)(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET)|
> > +				u4bAcParam = (((u32)le16_to_cpu(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET) |
> > +					(((u32)le16_to_cpu(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET) |
> > +					(((u32)le16_to_cpu(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET) |
> >  					((u32)u1bAIFS << AC_PARAM_AIFS_OFFSET);
> >  				/*write_nic_dword(dev, WDCAPARA_ADD[i], u4bAcParam);*/
> >  				write_nic_dword(dev, EDCAPARA_BE,  u4bAcParam);
> 
> How are you sure that this change is correct?  How did you verify it?
> 
Sadly, I don't have the hardware to test that. I only checked the code
and compiled it.

> thanks,
> 
Cheers,

-- 
Guillaume Brogi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging: rtl8192u: Convert __le16 to cpu before casting to u32
  2017-04-08 18:32   ` [PATCH] staging: rtl8192u: " Guillaume Brogi
@ 2017-05-07 12:56     ` Guillaume Brogi
  2017-05-12  8:08       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 6+ messages in thread
From: Guillaume Brogi @ 2017-05-07 12:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Bhumika Goyal, devel, linux-kernel

On Sat, Apr 08, 2017 at 08:32:36PM +0200, Guillaume Brogi wrote:
> On Sat, Apr 08, 2017 at 12:31:25PM +0200, Greg Kroah-Hartman wrote:
> > On Sun, Mar 26, 2017 at 12:24:14AM +0100, Guillaume Brogi wrote:
> > > 
> > > This patch fixes the following sparse warnings:
> > > drivers/staging/rtl8192u/r8192U_dm.c:2307:49: warning: cast from restricted __le16
> > > drivers/staging/rtl8192u/r8192U_dm.c:2308:44: warning: cast from restricted __le16
> > > drivers/staging/rtl8192u/r8192U_dm.c:2309:44: warning: cast from restricted __le16
> > > 
> > > Those three members of qos_parameters are indeed __le16 so they should
> > > be converted to the cpu's endianness before being cast to u32.
> > > 
> > > The lines are over the 80 character limit. They already were, and, for
> > > the sake of readability, I don't think they should be split.
> > 
> > Please fix up your subject: line to match other patches for this driver,
> > I almost missed it :(
> > 
> Sorry about that. I realised my mistake after sending the email.
> 
> > > 
> > > Signed-off-by: Guillaume Brogi <gui-gui@netcourrier.com>
> > > ---
> > >  drivers/staging/rtl8192u/r8192U_dm.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/staging/rtl8192u/r8192U_dm.c b/drivers/staging/rtl8192u/r8192U_dm.c
> > > index 9209aad0515e..5946c1f8d37d 100644
> > > --- a/drivers/staging/rtl8192u/r8192U_dm.c
> > > +++ b/drivers/staging/rtl8192u/r8192U_dm.c
> > > @@ -2304,9 +2304,9 @@ static void dm_check_edca_turbo(
> > >  				/*  For Each time updating EDCA parameter, reset EDCA turbo mode status. */
> > >  				dm_init_edca_turbo(dev);
> > >  				u1bAIFS = qos_parameters->aifs[0] * ((mode&(IEEE_G|IEEE_N_24G)) ? 9 : 20) + aSifsTime;
> > > -				u4bAcParam = (((u32)(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET)|
> > > -					(((u32)(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET)|
> > > -					(((u32)(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET)|
> > > +				u4bAcParam = (((u32)le16_to_cpu(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET) |
> > > +					(((u32)le16_to_cpu(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET) |
> > > +					(((u32)le16_to_cpu(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET) |
> > >  					((u32)u1bAIFS << AC_PARAM_AIFS_OFFSET);
> > >  				/*write_nic_dword(dev, WDCAPARA_ADD[i], u4bAcParam);*/
> > >  				write_nic_dword(dev, EDCAPARA_BE,  u4bAcParam);
> > 
> > How are you sure that this change is correct?  How did you verify it?
> > 
> Sadly, I don't have the hardware to test that. I only checked the code
> and compiled it.
> 
> > thanks,
> > 
> Cheers,
> 
Hello again,

Since there was no news for a while, I figured I'd ask what's the status
of this patch. I hope that's OK. Has it simply been forgotten or is
there something blocking/unacceptable?

Cheers,

-- 
Guillaume

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging: rtl8192u: Convert __le16 to cpu before casting to u32
  2017-05-07 12:56     ` Guillaume Brogi
@ 2017-05-12  8:08       ` Greg Kroah-Hartman
  2017-05-12 20:09         ` Guillaume Brogi
  0 siblings, 1 reply; 6+ messages in thread
From: Greg Kroah-Hartman @ 2017-05-12  8:08 UTC (permalink / raw)
  To: Guillaume Brogi; +Cc: Bhumika Goyal, devel, linux-kernel

On Sun, May 07, 2017 at 02:56:36PM +0200, Guillaume Brogi wrote:
> On Sat, Apr 08, 2017 at 08:32:36PM +0200, Guillaume Brogi wrote:
> > On Sat, Apr 08, 2017 at 12:31:25PM +0200, Greg Kroah-Hartman wrote:
> > > On Sun, Mar 26, 2017 at 12:24:14AM +0100, Guillaume Brogi wrote:
> > > > 
> > > > This patch fixes the following sparse warnings:
> > > > drivers/staging/rtl8192u/r8192U_dm.c:2307:49: warning: cast from restricted __le16
> > > > drivers/staging/rtl8192u/r8192U_dm.c:2308:44: warning: cast from restricted __le16
> > > > drivers/staging/rtl8192u/r8192U_dm.c:2309:44: warning: cast from restricted __le16
> > > > 
> > > > Those three members of qos_parameters are indeed __le16 so they should
> > > > be converted to the cpu's endianness before being cast to u32.
> > > > 
> > > > The lines are over the 80 character limit. They already were, and, for
> > > > the sake of readability, I don't think they should be split.
> > > 
> > > Please fix up your subject: line to match other patches for this driver,
> > > I almost missed it :(
> > > 
> > Sorry about that. I realised my mistake after sending the email.
> > 
> > > > 
> > > > Signed-off-by: Guillaume Brogi <gui-gui@netcourrier.com>
> > > > ---
> > > >  drivers/staging/rtl8192u/r8192U_dm.c | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/staging/rtl8192u/r8192U_dm.c b/drivers/staging/rtl8192u/r8192U_dm.c
> > > > index 9209aad0515e..5946c1f8d37d 100644
> > > > --- a/drivers/staging/rtl8192u/r8192U_dm.c
> > > > +++ b/drivers/staging/rtl8192u/r8192U_dm.c
> > > > @@ -2304,9 +2304,9 @@ static void dm_check_edca_turbo(
> > > >  				/*  For Each time updating EDCA parameter, reset EDCA turbo mode status. */
> > > >  				dm_init_edca_turbo(dev);
> > > >  				u1bAIFS = qos_parameters->aifs[0] * ((mode&(IEEE_G|IEEE_N_24G)) ? 9 : 20) + aSifsTime;
> > > > -				u4bAcParam = (((u32)(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET)|
> > > > -					(((u32)(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET)|
> > > > -					(((u32)(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET)|
> > > > +				u4bAcParam = (((u32)le16_to_cpu(qos_parameters->tx_op_limit[0])) << AC_PARAM_TXOP_LIMIT_OFFSET) |
> > > > +					(((u32)le16_to_cpu(qos_parameters->cw_max[0])) << AC_PARAM_ECW_MAX_OFFSET) |
> > > > +					(((u32)le16_to_cpu(qos_parameters->cw_min[0])) << AC_PARAM_ECW_MIN_OFFSET) |
> > > >  					((u32)u1bAIFS << AC_PARAM_AIFS_OFFSET);
> > > >  				/*write_nic_dword(dev, WDCAPARA_ADD[i], u4bAcParam);*/
> > > >  				write_nic_dword(dev, EDCAPARA_BE,  u4bAcParam);
> > > 
> > > How are you sure that this change is correct?  How did you verify it?
> > > 
> > Sadly, I don't have the hardware to test that. I only checked the code
> > and compiled it.
> > 
> > > thanks,
> > > 
> > Cheers,
> > 
> Hello again,
> 
> Since there was no news for a while, I figured I'd ask what's the status
> of this patch. I hope that's OK. Has it simply been forgotten or is
> there something blocking/unacceptable?

I don't see it in my queue, sorry.  If you can't test this, or verify
that it is correct some other way, I don't want to take endian fixes,
sorry.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging: rtl8192u: Convert __le16 to cpu before casting to u32
  2017-05-12  8:08       ` Greg Kroah-Hartman
@ 2017-05-12 20:09         ` Guillaume Brogi
  0 siblings, 0 replies; 6+ messages in thread
From: Guillaume Brogi @ 2017-05-12 20:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Bhumika Goyal, devel, linux-kernel

> > Since there was no news for a while, I figured I'd ask what's the status
> > of this patch. I hope that's OK. Has it simply been forgotten or is
> > there something blocking/unacceptable?
> 
> I don't see it in my queue, sorry.  If you can't test this, or verify
> that it is correct some other way, I don't want to take endian fixes,
> sorry.
> 
No worries. I figured that was a possibility after your comments.
I figured running sparse and trying to fix errors in staging could be a
good starting point, but apparently it is not (I don't think I have any
of the exotic hardware in staging). Is there anywhere else I could get
started? Or are there other errors from sparse which are less likely to
be complex to check and hence require hardware to test the changes?

Cheers,

-- 
Guillaume

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-05-12 20:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-25 23:24 [PATCH] Convert __le16 to cpu before casting to u32 Guillaume Brogi
2017-04-08 10:31 ` Greg Kroah-Hartman
2017-04-08 18:32   ` [PATCH] staging: rtl8192u: " Guillaume Brogi
2017-05-07 12:56     ` Guillaume Brogi
2017-05-12  8:08       ` Greg Kroah-Hartman
2017-05-12 20:09         ` Guillaume Brogi

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