Re: [PATCH] staging: rtl8192u: Convert __le16 to cpu before casting to u32

2017-05-07 Thread Guillaume Brogi
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 
> > > ---
> > >  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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: rtl8192u: Convert __le16 to cpu before casting to u32

2017-05-12 Thread Guillaume Brogi
> > 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


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Convert __le16 to cpu before casting to u32

2017-03-25 Thread Guillaume Brogi

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


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: rtl8192u: Convert __le16 to cpu before casting to u32

2017-04-08 Thread Guillaume Brogi
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 
> > ---
> >  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

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Checkpatch fix: lines longer than 80 columns

2015-04-26 Thread Guillaume Brogi
This patch fixes lines longer than 80 columns in mac.c.
5 lines longer than 80 columns remain for the sake of readability.

Signed-off-by: Guillaume Brogi 
---
 drivers/staging/vt6655/mac.c | 49 +---
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/vt6655/mac.c b/drivers/staging/vt6655/mac.c
index 8048b32..aed530f 100644
--- a/drivers/staging/vt6655/mac.c
+++ b/drivers/staging/vt6655/mac.c
@@ -70,7 +70,8 @@
  * Return Value: true if all test bits On; otherwise false
  *
  */
-bool MACbIsRegBitsOn(void __iomem *dwIoBase, unsigned char byRegOfs, unsigned 
char byTestBits)
+bool MACbIsRegBitsOn(void __iomem *dwIoBase, unsigned char byRegOfs,
+unsigned char byTestBits)
 {
unsigned char byData;
 
@@ -93,7 +94,8 @@ bool MACbIsRegBitsOn(void __iomem *dwIoBase, unsigned char 
byRegOfs, unsigned ch
  * Return Value: true if all test bits Off; otherwise false
  *
  */
-bool MACbIsRegBitsOff(void __iomem *dwIoBase, unsigned char byRegOfs, unsigned 
char byTestBits)
+bool MACbIsRegBitsOff(void __iomem *dwIoBase, unsigned char byRegOfs,
+ unsigned char byTestBits)
 {
unsigned char byData;
 
@@ -218,7 +220,8 @@ void MACvSaveContext(void __iomem *dwIoBase, unsigned char 
*pbyCxtBuf)
 
/* read page1 register */
for (ii = 0; ii < MAC_MAX_CONTEXT_SIZE_PAGE1; ii++)
-   VNSvInPortB((dwIoBase + ii), (pbyCxtBuf + 
MAC_MAX_CONTEXT_SIZE_PAGE0 + ii));
+   VNSvInPortB((dwIoBase + ii),
+   (pbyCxtBuf + MAC_MAX_CONTEXT_SIZE_PAGE0 + ii));
 
MACvSelectPage0(dwIoBase);
 }
@@ -244,7 +247,8 @@ void MACvRestoreContext(void __iomem *dwIoBase, unsigned 
char *pbyCxtBuf)
MACvSelectPage1(dwIoBase);
/* restore page1 */
for (ii = 0; ii < MAC_MAX_CONTEXT_SIZE_PAGE1; ii++)
-   VNSvOutPortB((dwIoBase + ii), *(pbyCxtBuf + 
MAC_MAX_CONTEXT_SIZE_PAGE0 + ii));
+   VNSvOutPortB((dwIoBase + ii),
+*(pbyCxtBuf + MAC_MAX_CONTEXT_SIZE_PAGE0 + ii));
 
MACvSelectPage0(dwIoBase);
 
@@ -263,13 +267,18 @@ void MACvRestoreContext(void __iomem *dwIoBase, unsigned 
char *pbyCxtBuf)
VNSvOutPortB(dwIoBase + ii, *(pbyCxtBuf + ii));
 
/* restore CURR_RX_DESC_ADDR, CURR_TX_DESC_ADDR */
-   VNSvOutPortD(dwIoBase + MAC_REG_TXDMAPTR0, *(unsigned long *)(pbyCxtBuf 
+ MAC_REG_TXDMAPTR0));
-   VNSvOutPortD(dwIoBase + MAC_REG_AC0DMAPTR, *(unsigned long *)(pbyCxtBuf 
+ MAC_REG_AC0DMAPTR));
-   VNSvOutPortD(dwIoBase + MAC_REG_BCNDMAPTR, *(unsigned long *)(pbyCxtBuf 
+ MAC_REG_BCNDMAPTR));
-
-   VNSvOutPortD(dwIoBase + MAC_REG_RXDMAPTR0, *(unsigned long *)(pbyCxtBuf 
+ MAC_REG_RXDMAPTR0));
-
-   VNSvOutPortD(dwIoBase + MAC_REG_RXDMAPTR1, *(unsigned long *)(pbyCxtBuf 
+ MAC_REG_RXDMAPTR1));
+   VNSvOutPortD(dwIoBase + MAC_REG_TXDMAPTR0,
+*(unsigned long *)(pbyCxtBuf + MAC_REG_TXDMAPTR0));
+   VNSvOutPortD(dwIoBase + MAC_REG_AC0DMAPTR,
+*(unsigned long *)(pbyCxtBuf + MAC_REG_AC0DMAPTR));
+   VNSvOutPortD(dwIoBase + MAC_REG_BCNDMAPTR,
+*(unsigned long *)(pbyCxtBuf + MAC_REG_BCNDMAPTR));
+
+   VNSvOutPortD(dwIoBase + MAC_REG_RXDMAPTR0,
+*(unsigned long *)(pbyCxtBuf + MAC_REG_RXDMAPTR0));
+
+   VNSvOutPortD(dwIoBase + MAC_REG_RXDMAPTR1,
+*(unsigned long *)(pbyCxtBuf + MAC_REG_RXDMAPTR1));
 }
 
 /*
@@ -641,7 +650,8 @@ void MACvSetCurrRx1DescAddr(void __iomem *dwIoBase, 
unsigned long dwCurrDescAddr
  * Return Value: none
  *
  */
-void MACvSetCurrTx0DescAddrEx(void __iomem *dwIoBase, unsigned long 
dwCurrDescAddr)
+void MACvSetCurrTx0DescAddrEx(void __iomem *dwIoBase,
+ unsigned long dwCurrDescAddr)
 {
unsigned short ww;
unsigned char byData;
@@ -679,7 +689,8 @@ void MACvSetCurrTx0DescAddrEx(void __iomem *dwIoBase, 
unsigned long dwCurrDescAd
  *
  */
 /* TxDMA1 = AC0DMA */
-void MACvSetCurrAC0DescAddrEx(void __iomem *dwIoBase, unsigned long 
dwCurrDescAddr)
+void MACvSetCurrAC0DescAddrEx(void __iomem *dwIoBase,
+ unsigned long dwCurrDescAddr)
 {
unsigned short ww;
unsigned char byData;
@@ -703,7 +714,8 @@ void MACvSetCurrAC0DescAddrEx(void __iomem *dwIoBase, 
unsigned long dwCurrDescAd
VNSvOutPortB(dwIoBase + MAC_REG_AC0DMACTL, DMACTL_RUN);
 }
 
-void MACvSetCurrTXDescAddr(int iTxType, void __iomem *dwIoBase, unsigned long 
dwCurrDescAddr)
+void MACvSetCurrTXDescAddr(int iTxType, void __iomem *dwIoBase,
+  unsigned long dwCurrDescAddr)
 {
if (iTxType == TYPE_AC0DMA)
MACvSetCurrAC0DescAddrEx(dwIoBase, dwCurrDescAddr);
@@ -767,7 +779,8 @@ void MACvOneShotTimer1MicroSec(void __iomem *dwIoBase, 
unsigned int uDelayTime)
VNSvOutPortB(dwIoBase + MAC_REG_TMCTL1, (TMCTL_TMD | TM