Re: [PATCH] staging: rtl8192u: Convert __le16 to cpu before casting to u32
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
> > 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
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
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
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