[PATCH] staging: rtl8723au: Fix sparse warning: cast to restricted __le16

[PATCH] staging: rtl8723au: Fix sparse warning: cast to restricted __le16

[Johannes Postma]

usPtr is used as __le16 *, but was defined as u16 *.
This was reported by sparse as:
drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c:1850:29: warning: cast to
restricted __le16

This patch fixes the type of usPtr.

Signed-off-by: Johannes Postma <jgmpostma@gmail.com>
---
 drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c b/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
index cb5076a..eb76ac4 100644
--- a/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
+++ b/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
@@ -1838,7 +1838,7 @@ Hal_EfuseParseThermalMeter_8723A(struct rtw_adapter *padapter,
 
 static void rtl8723a_cal_txdesc_chksum(struct tx_desc *ptxdesc)
 {
-	u16 *usPtr = (u16 *) ptxdesc;
+	__le16 *usPtr = (__le16 *)ptxdesc;
 	u32 count = 16;		/*  (32 bytes / 2 bytes per XOR) => 16 times */
 	u32 index;
 	u16 checksum = 0;
-- 
2.5.0

Re: [PATCH] staging: rtl8723au: Fix sparse warning: cast to restricted __le16

[Jes Sorensen]

Johannes Postma <jgmpostma@gmail.com> writes:
> usPtr is used as __le16 *, but was defined as u16 *.
> This was reported by sparse as:
> drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c:1850:29: warning: cast to
> restricted __le16
>
> This patch fixes the type of usPtr.
>
> Signed-off-by: Johannes Postma <jgmpostma@gmail.com>
> ---
>  drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Looks OK to me. Probably worth changing the *(usPtr + index) to be
usPtr[index] as well to make the code easier to read.

Jes

> diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c b/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
> index cb5076a..eb76ac4 100644
> --- a/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
> +++ b/drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
> @@ -1838,7 +1838,7 @@ Hal_EfuseParseThermalMeter_8723A(struct rtw_adapter *padapter,
>  
>  static void rtl8723a_cal_txdesc_chksum(struct tx_desc *ptxdesc)
>  {
> -	u16 *usPtr = (u16 *) ptxdesc;
> +	__le16 *usPtr = (__le16 *)ptxdesc;
>  	u32 count = 16;		/*  (32 bytes / 2 bytes per XOR) => 16 times */
>  	u32 index;
>  	u16 checksum = 0;

Re: [PATCH] staging: rtl8723au: Fix sparse warning: cast to restricted __le16

[Johannes Postma]

On 06/08/15 at 08:21am, Jes Sorensen wrote:
> 
> Looks OK to me. Probably worth changing the *(usPtr + index) to be
> usPtr[index] as well to make the code easier to read.
> 
> Jes
> 

Thank you for reviewing.  I will make a seperate patch for that.  I will
send it after this one is accepted.  Or should I combine them into a
patch serie?

	Johannes

Re: [PATCH] staging: rtl8723au: Fix sparse warning: cast to restricted __le16

[Jes Sorensen]

Johannes Postma <jgmpostma@gmail.com> writes:
> On 06/08/15 at 08:21am, Jes Sorensen wrote:
>> 
>> Looks OK to me. Probably worth changing the *(usPtr + index) to be
>> usPtr[index] as well to make the code easier to read.
>> 
>> Jes
>> 
>
> Thank you for reviewing.  I will make a seperate patch for that.  I will
> send it after this one is accepted.  Or should I combine them into a
> patch serie?

Either is fine with me.

Jes

Re: [PATCH] staging: rtl8723au: Fix sparse warning: cast to restricted __le16

[Johannes Postma]

On 07/08/15 at 08:26am, Jes Sorensen wrote:
> Johannes Postma <jgmpostma@gmail.com> writes:
> > On 06/08/15 at 08:21am, Jes Sorensen wrote:
> >> 
> >> Looks OK to me. Probably worth changing the *(usPtr + index) to be
> >> usPtr[index] as well to make the code easier to read.
> >> 
> >> Jes
> >> 
> >
> > Thank you for reviewing.  I will make a seperate patch for that.  I will
> > send it after this one is accepted.  Or should I combine them into a
> > patch serie?
> 
> Either is fine with me.
> 
> Jes

Ok, I have sent it as a separate patch.  Since it doesn't depend on this
patch and should apply cleanly I have already sent it in.

	Johannes

[PATCH] staging: rtl8723au: Fix sparse warning cast to restricted __le16

[Marcus Folkesson]

This patch fixes the following sparse warnings:

  CHECK   drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
  drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c:265:37: warning:
  cast to restricted __le16
  drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c:267:39: warning:
  cast to restricted __le16

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/staging/rtl8723au/include/rtl8723a_hal.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723au/include/rtl8723a_hal.h b/drivers/staging/rtl8723au/include/rtl8723a_hal.h
index e146336..f642b11 100644
--- a/drivers/staging/rtl8723au/include/rtl8723a_hal.h
+++ b/drivers/staging/rtl8723au/include/rtl8723a_hal.h
@@ -255,10 +255,10 @@ struct hal_data_8723a {
 	struct hal_version		VersionID;
 	enum rt_customer_id CustomerID;
 
-	u16	FirmwareVersion;
-	u16	FirmwareVersionRev;
-	u16	FirmwareSubVersion;
-	u16	FirmwareSignature;
+	__le16	FirmwareVersion;
+	__le16	FirmwareVersionRev;
+	__le16	FirmwareSubVersion;
+	__le16	FirmwareSignature;
 
 	/* current WIFI_PHY values */
 	u32	ReceiveConfig;
-- 
1.9.1

Re: [PATCH] staging: rtl8723au: Fix sparse warning cast to restricted __le16

[Jes Sorensen]

Marcus Folkesson <marcus.folkesson@gmail.com> writes:
> This patch fixes the following sparse warnings:
>
>   CHECK   drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c
>   drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c:265:37: warning:
>   cast to restricted __le16
>   drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c:267:39: warning:
>   cast to restricted __le16
>
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---
>  drivers/staging/rtl8723au/include/rtl8723a_hal.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/rtl8723au/include/rtl8723a_hal.h b/drivers/staging/rtl8723au/include/rtl8723a_hal.h
> index e146336..f642b11 100644
> --- a/drivers/staging/rtl8723au/include/rtl8723a_hal.h
> +++ b/drivers/staging/rtl8723au/include/rtl8723a_hal.h
> @@ -255,10 +255,10 @@ struct hal_data_8723a {
>  	struct hal_version		VersionID;
>  	enum rt_customer_id CustomerID;
>  
> -	u16	FirmwareVersion;
> -	u16	FirmwareVersionRev;
> -	u16	FirmwareSubVersion;
> -	u16	FirmwareSignature;
> +	__le16	FirmwareVersion;
> +	__le16	FirmwareVersionRev;
> +	__le16	FirmwareSubVersion;
> +	__le16	FirmwareSignature;

Ehm I am pretty sure it doesn't:

rtl8723au_hal_init.c:265
        pHalData->FirmwareVersion = le16_to_cpu(pFwHdr->Version);
	pHalData->FirmwareSubVersion = pFwHdr->Subversion;
	pHalData->FirmwareSignature = le16_to_cpu(pFwHdr->Signature);

If anything, the second assignment there should be changed to use
le16_to_cpu(), but your conversion is definitely wrong.

NACK

Jes

[PATCH] staging:rtl8723au: Fix sparse warning cast to restricted __le16

[Tobenna P. Igwe]

This patch fixes the following sparse warning:

drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c:343:36: warning:
cast to restricted __le16

by using the le16_to_cpus function.

Signed-off-by: Tobenna P. Igwe <ptigwe@gmail.com>
---
 drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c b/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c
index d80ea4e..78665ee 100644
--- a/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c
+++ b/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c
@@ -340,7 +340,7 @@ static u8 bthci_GetAssocInfo(struct rtw_adapter *padapter, u8 EntryNum)
 			tempBuf, TotalLen-BaseMemoryShift);
 
 		pAmpAsoc = (struct amp_assoc_structure *)tempBuf;
-		pAmpAsoc->Length = le16_to_cpu(pAmpAsoc->Length);
+		le16_to_cpus(&pAmpAsoc->Length);
 		BaseMemoryShift += 3 + pAmpAsoc->Length;
 
 		RTPRINT(FIOCTL, IOCTL_BT_HCICMD, ("TypeID = 0x%x, ", pAmpAsoc->TypeID));
-- 
1.7.10.4

Re: [PATCH] staging:rtl8723au: Fix sparse warning cast to restricted __le16

[Greg KH]

On Tue, Nov 18, 2014 at 01:45:41AM +0000, Tobenna P. Igwe wrote:
> This patch fixes the following sparse warning:
> 
> drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c:343:36: warning:
> cast to restricted __le16
> 
> by using the le16_to_cpus function.
> 
> Signed-off-by: Tobenna P. Igwe <ptigwe@gmail.com>
> ---
>  drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c b/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c
> index d80ea4e..78665ee 100644
> --- a/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c
> +++ b/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c
> @@ -340,7 +340,7 @@ static u8 bthci_GetAssocInfo(struct rtw_adapter *padapter, u8 EntryNum)
>  			tempBuf, TotalLen-BaseMemoryShift);
>  
>  		pAmpAsoc = (struct amp_assoc_structure *)tempBuf;
> -		pAmpAsoc->Length = le16_to_cpu(pAmpAsoc->Length);
> +		le16_to_cpus(&pAmpAsoc->Length);

This doesn't change the logic, and you should still have the same
warning.  How about fixing up Length to be the proper endian type?

thanks,

greg k-h

Re: [PATCH] staging:rtl8723au: Fix sparse warning cast to restricted __le16

[Tobenna Peter, Igwe]

On Mon, Nov 17, 2014 at 05:59:47PM -0800, Greg KH wrote:
> On Tue, Nov 18, 2014 at 01:45:41AM +0000, Tobenna P. Igwe wrote:
> > This patch fixes the following sparse warning:
> > 
> > drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c:343:36: warning:
> > cast to restricted __le16
> > 
> > by using the le16_to_cpus function.
> > 
> > Signed-off-by: Tobenna P. Igwe <ptigwe@gmail.com>
> > ---
> >  drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c b/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c
> > index d80ea4e..78665ee 100644
> > --- a/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c
> > +++ b/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c
> > @@ -340,7 +340,7 @@ static u8 bthci_GetAssocInfo(struct rtw_adapter *padapter, u8 EntryNum)
> >  			tempBuf, TotalLen-BaseMemoryShift);
> >  
> >  		pAmpAsoc = (struct amp_assoc_structure *)tempBuf;
> > -		pAmpAsoc->Length = le16_to_cpu(pAmpAsoc->Length);
> > +		le16_to_cpus(&pAmpAsoc->Length);
> 
> This doesn't change the logic, and you should still have the same
> warning.  How about fixing up Length to be the proper endian type?
> 
> thanks,
> 
> greg k-h

I've checked and sparse doesn't return the warning anymore. Also, I
backtracked through the file and Length seems to be set to its
correct endian type. The reason for the conversion is tempBuf being
an array of 'u8'.

Kind Regards,
Tobenna P. Igwe

Re: [PATCH] staging:rtl8723au: Fix sparse warning cast to restricted __le16

[Greg KH]

On Tue, Nov 18, 2014 at 02:51:45AM +0000, Tobenna Peter, Igwe wrote:
> On Mon, Nov 17, 2014 at 05:59:47PM -0800, Greg KH wrote:
> > On Tue, Nov 18, 2014 at 01:45:41AM +0000, Tobenna P. Igwe wrote:
> > > This patch fixes the following sparse warning:
> > > 
> > > drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c:343:36: warning:
> > > cast to restricted __le16
> > > 
> > > by using the le16_to_cpus function.
> > > 
> > > Signed-off-by: Tobenna P. Igwe <ptigwe@gmail.com>
> > > ---
> > >  drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c b/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c
> > > index d80ea4e..78665ee 100644
> > > --- a/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c
> > > +++ b/drivers/staging/rtl8723au/hal/rtl8723a_bt-coexist.c
> > > @@ -340,7 +340,7 @@ static u8 bthci_GetAssocInfo(struct rtw_adapter *padapter, u8 EntryNum)
> > >  			tempBuf, TotalLen-BaseMemoryShift);
> > >  
> > >  		pAmpAsoc = (struct amp_assoc_structure *)tempBuf;
> > > -		pAmpAsoc->Length = le16_to_cpu(pAmpAsoc->Length);
> > > +		le16_to_cpus(&pAmpAsoc->Length);
> > 
> > This doesn't change the logic, and you should still have the same
> > warning.  How about fixing up Length to be the proper endian type?
> > 
> > thanks,
> > 
> > greg k-h
> 
> I've checked and sparse doesn't return the warning anymore. Also, I
> backtracked through the file and Length seems to be set to its
> correct endian type. The reason for the conversion is tempBuf being
> an array of 'u8'.

Ah, that makes sense, ok, I'll queue this up in a few days when I get a
chance.

thanks,

greg k-h