[PATCH] staging: ks7010: Change capability field to __le16

[PATCH] staging: ks7010: Change capability field to __le16

[Johan Svensson]

Change capability field to __le16 in struct ap_info_t,
struct link_ap_info_t, and struct local_ap_t.
This fixes a sparse warning.

Signed-off-by: Johan Svensson <johan.svensson692@gmail.com>
---
 drivers/staging/ks7010/ks_hostif.h | 4 ++--
 drivers/staging/ks7010/ks_wlan.h   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.h b/drivers/staging/ks7010/ks_hostif.h
index 30c49b6..e7f814f 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -284,7 +284,7 @@ struct ap_info_t {
 	uint8_t noise;	/* +08 */
 	uint8_t pad0;	/* +09 */
 	uint16_t beacon_period;	/* +10 */
-	uint16_t capability;	/* +12 */
+	__le16 capability;	/* +12 */
 #define BSS_CAP_ESS             (1<<0)
 #define BSS_CAP_IBSS            (1<<1)
 #define BSS_CAP_CF_POLABLE      (1<<2)
@@ -311,7 +311,7 @@ struct link_ap_info_t {
 	uint8_t noise;	/* +08 */
 	uint8_t pad0;	/* +09 */
 	uint16_t beacon_period;	/* +10 */
-	uint16_t capability;	/* +12 */
+	__le16 capability;	/* +12 */
 	struct rate_set8_t rate_set;	/* +14 */
 	struct FhParms_t fh_parameter;	/* +24 */
 	struct DsParms_t ds_parameter;	/* +29 */
diff --git a/drivers/staging/ks7010/ks_wlan.h b/drivers/staging/ks7010/ks_wlan.h
index 9ab80e1..a4655a0 100644
--- a/drivers/staging/ks7010/ks_wlan.h
+++ b/drivers/staging/ks7010/ks_wlan.h
@@ -243,7 +243,7 @@ struct local_ap_t {
 		u8 body[16];
 		u8 rate_pad;
 	} rate_set;
-	u16 capability;
+	__le16 capability;
 	u8 channel;
 	u8 noise;
 	struct rsn_ie_t wpa_ie;
-- 
2.7.4

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

Re: [PATCH] staging: ks7010: Change capability field to __le16

[Greg KH]

On Tue, Apr 18, 2017 at 08:24:01PM +0200, Johan Svensson wrote:
> Change capability field to __le16 in struct ap_info_t,
> struct link_ap_info_t, and struct local_ap_t.
> This fixes a sparse warning.

What warning is it fixing?  And are you sure this is the correct fix?
How did you test it?

thanks,

greg k-h

Re: [PATCH] staging: ks7010: Change capability field to __le16

[Johan Svensson]

On 2017-04-18 20:38, Greg KH wrote:
> On Tue, Apr 18, 2017 at 08:24:01PM +0200, Johan Svensson wrote:
>> Change capability field to __le16 in struct ap_info_t,
>> struct link_ap_info_t, and struct local_ap_t.
>> This fixes a sparse warning.
> What warning is it fixing?  And are you sure this is the correct fix?
> How did you test it?
>
> thanks,
>
> greg k-h
Without the patch, sparse reports:
drivers/staging/ks7010/ks_wlan_net.c:1459:24: warning: cast to restricted __le16

The capability field in the structs that are changed are already being treated as little endian i.e. the patch is correct if the le16_to_cpu conversion in ks_wlan_net.c:1459 is correct.

regards,
Johan Svensson

Re: [PATCH] staging: ks7010: Change capability field to __le16

[Tobin C. Harding]

On Thu, Apr 20, 2017 at 09:02:26PM +0200, Johan Svensson wrote:
> 
> On 2017-04-18 20:38, Greg KH wrote:
> > On Tue, Apr 18, 2017 at 08:24:01PM +0200, Johan Svensson wrote:
> >> Change capability field to __le16 in struct ap_info_t,
> >> struct link_ap_info_t, and struct local_ap_t.
> >> This fixes a sparse warning.
> > What warning is it fixing?  And are you sure this is the correct fix?
> > How did you test it?
> >
> > thanks,
> >
> > greg k-h
> Without the patch, sparse reports:
> drivers/staging/ks7010/ks_wlan_net.c:1459:24: warning: cast to restricted __le16
> 
> The capability field in the structs that are changed are already being treated as little endian i.e. the patch is correct if the le16_to_cpu conversion in ks_wlan_net.c:1459 is correct.

We have no guarantee that the above call to le16_to_cpu() is correct
(for example, this call would work correctly on little endian machines
regardless).

It may be safer to test endian code changes on hardware (at least with
this driver).

thanks,
Tobin.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ks7010: Change capability field to __le16

[Tobin C. Harding]

On Mon, Apr 24, 2017 at 08:44:38PM +1000, Tobin C. Harding wrote:
> On Thu, Apr 20, 2017 at 09:02:26PM +0200, Johan Svensson wrote:
> > 
> > On 2017-04-18 20:38, Greg KH wrote:
> > > On Tue, Apr 18, 2017 at 08:24:01PM +0200, Johan Svensson wrote:
> > >> Change capability field to __le16 in struct ap_info_t,
> > >> struct link_ap_info_t, and struct local_ap_t.
> > >> This fixes a sparse warning.
> > > What warning is it fixing?  And are you sure this is the correct fix?
> > > How did you test it?
> > >
> > > thanks,
> > >
> > > greg k-h
> > Without the patch, sparse reports:
> > drivers/staging/ks7010/ks_wlan_net.c:1459:24: warning: cast to restricted __le16
> > 
> > The capability field in the structs that are changed are already being treated as little endian i.e. the patch is correct if the le16_to_cpu conversion in ks_wlan_net.c:1459 is correct.
> 
> We have no guarantee that the above call to le16_to_cpu() is correct
> (for example, this call would work correctly on little endian machines
> regardless).

This statement is incorrect, my apologies.

> It may be safer to test endian code changes on hardware (at least with
> this driver).

I cannot get a proper handle on the endianness in ks7010 yet, perhaps
someone more experienced would not find it so difficult. It seems
there are some anomalies in the way this driver handles byte order.

I still maintain that hardware testing is needed to resolve these issues.

thanks,
Tobin.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ks7010: Change capability field to __le16

[Greg KH]

On Tue, Apr 18, 2017 at 08:24:01PM +0200, Johan Svensson wrote:
> Change capability field to __le16 in struct ap_info_t,
> struct link_ap_info_t, and struct local_ap_t.
> This fixes a sparse warning.
> 
> Signed-off-by: Johan Svensson <johan.svensson692@gmail.com>
> ---
>  drivers/staging/ks7010/ks_hostif.h | 4 ++--
>  drivers/staging/ks7010/ks_wlan.h   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

Based on the lack of testing of this, I'm going to drop it from my
queue.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel