staging: wilc1000: fix some endianness sparse warnings

PATCH In review Thibaut Robert 3 participants 8 messages May 29, 2018 – Jun 5, 2018 Staging

Share & tools

Thibaut Robert

author

2018-05-29 19:11 UTC #0 lore raw

PATCH 1/2 staging: wilc1000: fix some endianness sparse warnings

This commit fix a few sparse warnings. It mostly consists of fixing the type declarations and avoiding the use of variables with mixed endianness values.

Signed-off-by Thibaut Robert <[email protected]>

---
 drivers/staging/wilc1000/wilc_spi.c           | 15 ++++++-----
 .../staging/wilc1000/wilc_wfi_cfgoperations.c |  2 +-
 drivers/staging/wilc1000/wilc_wlan.c          | 26 +++++++++----------
 drivers/staging/wilc1000/wilc_wlan_cfg.c      |  8 +++---
 drivers/staging/wilc1000/wilc_wlan_cfg.h      |  4 +--
 5 files changed, 27 insertions(+), 28 deletions(-)

drivers/staging/wilc1000/wilc_spi.c +8 -7

index 647526387784..e51f0d91a376 100644
@@ -666,11 +666,11 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
 static int spi_internal_write(struct wilc *wilc, u32 adr, u32 dat)
 {
 	struct spi_device *spi = to_spi_device(wilc->dev);
+	__le32 le32dat = cpu_to_le32(dat);
 	int result;
 
-	dat = cpu_to_le32(dat);
-	result = spi_cmd_complete(wilc, CMD_INTERNAL_WRITE, adr, (u8 *)&dat, 4,
-				  0);
+	result = spi_cmd_complete(wilc, CMD_INTERNAL_WRITE, adr, (u8 *)&le32dat,
+				  4, 0);
 	if (result != N_OK)
 		dev_err(&spi->dev, "Failed internal write cmd...\n");

@@ -689,7 +689,7 @@ static int spi_internal_read(struct wilc *wilc, u32 adr, u32 *data)
 		return 0;
 	}
 
-	*data = cpu_to_le32(*data);
+	le32_to_cpus(data);
 
 	return 1;
 }

@@ -706,15 +706,16 @@ static int wilc_spi_write_reg(struct wilc *wilc, u32 addr, u32 data)
 	int result = N_OK;
 	u8 cmd = CMD_SINGLE_WRITE;
 	u8 clockless = 0;
+	__le32 le32data = cpu_to_le32(data);
 
-	data = cpu_to_le32(data);
 	if (addr < 0x30) {
 		/* Clockless register */
 		cmd = CMD_INTERNAL_WRITE;
 		clockless = 1;
 	}
 
-	result = spi_cmd_complete(wilc, cmd, addr, (u8 *)&data, 4, clockless);
+	result = spi_cmd_complete(wilc, cmd, addr, (u8 *)&le32data, 4,
+				  clockless);
 	if (result != N_OK)
 		dev_err(&spi->dev, "Failed cmd, write reg (%08x)...\n", addr);

@@ -769,7 +770,7 @@ static int wilc_spi_read_reg(struct wilc *wilc, u32 addr, u32 *data)
 		return 0;
 	}
 
-	*data = cpu_to_le32(*data);
+	le32_to_cpus(data);
 
 	return 1;
 }

drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +1 -1

index e248702ee519..745bf5ca2622 100644
@@ -1431,7 +1431,7 @@ void wilc_wfi_p2p_rx(struct net_device *dev, u8 *buff, u32 size)
 
 	freq = ieee80211_channel_to_frequency(curr_channel, NL80211_BAND_2GHZ);
 
-	if (!ieee80211_is_action(buff[FRAME_TYPE_ID])) {
+	if (!ieee80211_is_action(cpu_to_le16(buff[FRAME_TYPE_ID]))) {
 		cfg80211_rx_mgmt(priv->wdev, freq, 0, buff, size, 0);
 		return;
 	}

drivers/staging/wilc1000/wilc_wlan.c +12 ~~-14~~

index 28c93f3f846e..a5ac1d26590b 100644
@@ -560,7 +560,8 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)
 	int ret = 0;
 	int counter;
 	int timeout;
-	u32 vmm_table[WILC_VMM_TBL_SIZE];
+	__le32 vmm_table[WILC_VMM_TBL_SIZE];
+	u32 table_entry;
 	struct wilc_vif *vif;
 	struct wilc *wilc;
 	const struct wilc_hif_func *func;

@@ -598,10 +599,10 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)
 			if ((sum + vmm_sz) > LINUX_TX_SIZE)
 				break;
 
-			vmm_table[i] = vmm_sz / 4;
+			table_entry = vmm_sz / 4;
 			if (tqe->type == WILC_CFG_PKT)
-				vmm_table[i] |= BIT(10);
-			vmm_table[i] = cpu_to_le32(vmm_table[i]);
+				table_entry |= BIT(10);
+			vmm_table[i] = cpu_to_le32(table_entry);
 
 			i++;
 			sum += vmm_sz;

@@ -704,8 +705,7 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)
 		if (vmm_table[i] == 0)
 			break;
 
-		vmm_table[i] = cpu_to_le32(vmm_table[i]);
-		vmm_sz = (vmm_table[i] & 0x3ff);
+		vmm_sz = (le32_to_cpu(vmm_table[i]) & 0x3ff);
 		vmm_sz *= 4;
 		header = (tqe->type << 31) |
 			 (tqe->buffer_size << 15) |

@@ -715,8 +715,7 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count)
 		else
 			header &= ~BIT(30);
 
-		header = cpu_to_le32(header);
-		memcpy(&txb[offset], &header, 4);
+		*((__le32 *)&txb[offset]) = cpu_to_le32(header);
 		if (tqe->type == WILC_CFG_PKT) {
 			buffer_offset = ETH_CONFIG_PKT_HDR_OFFSET;
 		} else if (tqe->type == WILC_NET_PKT) {

@@ -770,8 +769,7 @@ static void wilc_wlan_handle_rx_buff(struct wilc *wilc, u8 *buffer, int size)
 
 	do {
 		buff_ptr = buffer + offset;
-		memcpy(&header, buff_ptr, 4);
-		header = cpu_to_le32(header);
+		header = le32_to_cpup((__le32 *)buff_ptr);
 
 		is_cfg_packet = (header >> 31) & 0x1;
 		pkt_offset = (header >> 22) & 0x1ff;

@@ -942,6 +940,7 @@ int wilc_wlan_firmware_download(struct wilc *wilc, const u8 *buffer,
 	u32 offset;
 	u32 addr, size, size2, blksz;
 	u8 *dma_buffer;
+	const __le32 *header;
 	int ret = 0;
 
 	blksz = BIT(12);

@@ -952,10 +951,9 @@ int wilc_wlan_firmware_download(struct wilc *wilc, const u8 *buffer,
 
 	offset = 0;
 	do {
-		memcpy(&addr, &buffer[offset], 4);
-		memcpy(&size, &buffer[offset + 4], 4);
-		addr = cpu_to_le32(addr);
-		size = cpu_to_le32(size);
+		header = (__le32 *)buffer + offset;
+		addr = le32_to_cpu(header[0]);
+		size = le32_to_cpu(header[1]);
 		acquire_bus(wilc, ACQUIRE_ONLY);
 		offset += 8;
 		while (((int)size) && (offset < buffer_size)) {

drivers/staging/wilc1000/wilc_wlan_cfg.c +4 -4

index c0b9b700f4d7..4a914d8572aa 100644
@@ -275,14 +275,14 @@ static int wilc_wlan_cfg_set_bin(u8 *frame, u32 offset, u16 id, u8 *b, u32 size)
 
 static void wilc_wlan_parse_response_frame(u8 *info, int size)
 {
-	u32 wid, len = 0, i = 0;
+	u32 wid;
+	int len = 0, i = 0;
 
 	while (size > 0) {
 		i = 0;
-		wid = info[0] | (info[1] << 8);
-		wid = cpu_to_le32(wid);
+		wid = le16_to_cpup((__le16 *)info);
 
-		switch ((wid >> 12) & 0x7) {
+		switch (info[1] >> 4) {
 		case WID_CHAR:
 			do {
 				if (g_cfg_byte[i].id == WID_NIL)

drivers/staging/wilc1000/wilc_wlan_cfg.h +2 -2

index 08092a551840..9e029338bcab 100644
@@ -18,12 +18,12 @@ struct wilc_cfg_byte {
 
 struct wilc_cfg_hword {
 	u16 id;
-	u16 val;
+	__le16 val;
 };
 
 struct wilc_cfg_word {
 	u32 id;
-	u32 val;
+	__le32 val;
 };
 
 struct wilc_cfg_str {

Signature · 1 line

2.17.0

Dan Carpenter

2018-06-05 07:36 UTC #3 lore raw

Quoted 16 lines · Thibaut Robert

Quoted 7 lines · Dan Carpenter
Quoted 6 lines · Thibaut Robert
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +0 -0
index e248702ee519..745bf5ca2622 100644
@@ -1431,7 +1431,7 @@ void wilc_wfi_p2p_rx(struct net_device *dev, u8 *buff, u32 size)
 
Quoted 8 lines
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c void wilc_wfi_p2p_rx(struct net_device *dev, u8 *buff, u32 size) · line 1434
Quoted 4 lines
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +1 -1
 	freq = ieee80211_channel_to_frequency(curr_channel, NL80211_BAND_2GHZ);
 
-	if (!ieee80211_is_action(buff[FRAME_TYPE_ID])) {
+	if (!ieee80211_is_action(cpu_to_le16(buff[FRAME_TYPE_ID]))) {
"buff" comes from the network, it's going to be little endian, not cpu endian. The rest of the function treats it as CPU endian but I'm pretty sure it's wrong...

Quoted 5 lines

buff comes from the network but we are looking at single byte here. ieee80211_is_action expects an le16, so we I added this to extend an u8 to an le16. Is this incorrect ?
Or maybe we the buff has the second byte ? but that I can't tell.

You raise a good point that I hadn't seen. The original code is clearly buggy. But your fix isn't correct either... The other thing to consider is that cpu_to_le16() is basically a cast to u16 on x86 so it's a no-op here.

Really the right thing is to not treat buff as an array of u8 but a struct. The code assumes that frame_type is 0-255 but probably it's supposed to go up to U16_MAX.

struct whatever {
	__le16 frame_type;
	...

There probably is already a struct like that, but I don't know what it is. I don't know this code at all, I'm just guessing.

regards,
dan carpenter

Thibaut Robert

author

2018-06-05 08:33 UTC #4 lore raw

Quoted 21 lines · Dan Carpenter

Quoted 20 lines · Thibaut Robert
Quoted 7 lines · Dan Carpenter
Quoted 6 lines · Thibaut Robert
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +0 -0
index e248702ee519..745bf5ca2622 100644
@@ -1431,7 +1431,7 @@ void wilc_wfi_p2p_rx(struct net_device *dev, u8 *buff, u32 size)
 
Quoted 8 lines
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c void wilc_wfi_p2p_rx(struct net_device *dev, u8 *buff, u32 size) · line 1434
Quoted 4 lines
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +1 -1
 	freq = ieee80211_channel_to_frequency(curr_channel, NL80211_BAND_2GHZ);
 
-	if (!ieee80211_is_action(buff[FRAME_TYPE_ID])) {
+	if (!ieee80211_is_action(cpu_to_le16(buff[FRAME_TYPE_ID]))) {
"buff" comes from the network, it's going to be little endian, not cpu endian. The rest of the function treats it as CPU endian but I'm pretty sure it's wrong...
buff comes from the network but we are looking at single byte here. ieee80211_is_action expects an le16, so we I added this to extend an u8 to an le16. Is this incorrect ?

Quoted 6 lines

Quoted 1 line
Or maybe we the buff has the second byte ? but that I can't tell.
You raise a good point that I hadn't seen. The original code is clearly buggy. But your fix isn't correct either... The other thing to consider is that cpu_to_le16() is basically a cast to u16 on x86 so it's a no-op here.

The sparse warning is clearly spotting a real issue. I tried to at least have big endian handle correctly the 0-255 case. I am willing to drop the change (since I agree it's not very satisfying and will mask an issue), but may I ask you to explain how it is wrong ? How would you correctly expand an u8 to __le16 ? I think in big endian we need to swap the bytes.

Quoted 5 lines

Really the right thing is to not treat buff as an array of u8 but a struct. The code assumes that frame_type is 0-255 but probably it's supposed to go up to U16_MAX.

Quoted 7 lines

struct whatever {
	__le16 frame_type;
	...
There probably is already a struct like that, but I don't know what it is. I don't know this code at all, I'm just guessing.

I was thinking the same. I don't know whether this buf contains a standard struct or something hw-specific. I'll try to dig further and submit a separate patch if I can. Maybe Aditya can give more information ?

Quoted 3 lines

regards,
dan carpenter

Regards,
Thibaut

Dan Carpenter

2018-06-05 08:53 UTC #5 lore raw

Quoted 28 lines · Thibaut Robert

Quoted 21 lines · Dan Carpenter
Quoted 20 lines · Thibaut Robert
Quoted 7 lines · Dan Carpenter
Quoted 6 lines · Thibaut Robert
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +0 -0
index e248702ee519..745bf5ca2622 100644
@@ -1431,7 +1431,7 @@ void wilc_wfi_p2p_rx(struct net_device *dev, u8 *buff, u32 size)
 
Quoted 8 lines
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c void wilc_wfi_p2p_rx(struct net_device *dev, u8 *buff, u32 size) · line 1434
Quoted 4 lines
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +1 -1
 	freq = ieee80211_channel_to_frequency(curr_channel, NL80211_BAND_2GHZ);
 
-	if (!ieee80211_is_action(buff[FRAME_TYPE_ID])) {
+	if (!ieee80211_is_action(cpu_to_le16(buff[FRAME_TYPE_ID]))) {
"buff" comes from the network, it's going to be little endian, not cpu endian. The rest of the function treats it as CPU endian but I'm pretty sure it's wrong...
buff comes from the network but we are looking at single byte here. ieee80211_is_action expects an le16, so we I added this to extend an u8 to an le16. Is this incorrect ?
Quoted 6 lines
Quoted 1 line
Or maybe we the buff has the second byte ? but that I can't tell.
You raise a good point that I hadn't seen. The original code is clearly buggy. But your fix isn't correct either... The other thing to consider is that cpu_to_le16() is basically a cast to u16 on x86 so it's a no-op here.

Quoted 5 lines

The sparse warning is clearly spotting a real issue. I tried to at least have big endian handle correctly the 0-255 case. I am willing to drop the change (since I agree it's not very satisfying and will mask an issue), but may I ask you to explain how it is wrong ? How would you correctly expand an u8 to __le16 ? I think in big endian we need to swap the bytes.

You're right that on big endian we would need to swap the bytes and cpu_to_le16() does that.

regards,
dan carpenter

Ajay Singh

2018-06-05 11:28 UTC #6 lore raw

In reply to Thibaut Robert

Quoted 34 lines · On Tue, 5 Jun 2018 10:33:25 +0200 Thibaut Robert <[email protected]>…

Quoted 26 lines · Dan Carpenter
Quoted 25 lines · Thibaut Robert
Le mercredi 30 mai 2018 à 14:17:25 (+0300), Dan Carpenter a écrit :
Quoted 11 lines
Quoted 9 lines · Thibaut Robert
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index e248702ee519..745bf5ca2622 100644 --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -1431,7 +1431,7 @@ void wilc_wfi_p2p_rx(struct net_device *dev, u8 *buff, u32 size) freq = ieee80211_channel_to_frequency(curr_channel,
Quoted 8 lines
Quoted 4 lines
NL80211_BAND_2GHZ);
(quoted diff) +1 -1
-	if (!ieee80211_is_action(buff[FRAME_TYPE_ID])) {
+	if
(!ieee80211_is_action(cpu_to_le16(buff[FRAME_TYPE_ID]))) {
"buff" comes from the network, it's going to be little endian, not cpu endian. The rest of the function treats it as CPU endian but I'm pretty sure it's wrong...
buff comes from the network but we are looking at single byte here. ieee80211_is_action expects an le16, so we I added this to extend an u8 to an le16. Is this incorrect ?
Quoted 7 lines
Quoted 2 lines
Or maybe we the buff has the second byte ? but that I can't tell.
You raise a good point that I hadn't seen. The original code is clearly buggy. But your fix isn't correct either... The other thing to consider is that cpu_to_le16() is basically a cast to u16 on x86 so it's a no-op here.

Quoted 7 lines

The sparse warning is clearly spotting a real issue. I tried to at least have big endian handle correctly the 0-255 case. I am willing to drop the change (since I agree it's not very satisfying and will mask an issue), but may I ask you to explain how it is wrong ? How would you correctly expand an u8 to __le16 ? I think in big endian we need to swap the bytes.

Yes, when sending the data to ieee80211_is_action() it should pass 16bit value instead of 8bit value. It should be corrected.

But the buff received from the wifi module is already in little endian format. As the data is already in LE endian format so in my opinion the conversion is not required while calling ieee80211_is_action().

Quoted 9 lines

Quoted 9 lines
Really the right thing is to not treat buff as an array of u8 but a struct. The code assumes that frame_type is 0-255 but probably it's supposed to go up to U16_MAX.
struct whatever {
	__le16 frame_type;
	...

Quoted 8 lines

Quoted 3 lines
There probably is already a struct like that, but I don't know what it is. I don't know this code at all, I'm just guessing.
I was thinking the same. I don't know whether this buf contains a standard struct or something hw-specific. I'll try to dig further and submit a separate patch if I can. Maybe Aditya can give more information ?

The buffer structure contains both some custom information along with standard info. As the frame_control is the first 2 byte in the in buff so casting it can also help to extract frame_control.

Please refer the code in mgmt_tx() which use like below

const struct ieee80211_mgmt *mgmt;
....
mgmt = (const struct ieee80211_mgmt *)buf;

ieee80211_is_action(mgmt->frame_control)

I think another approach can be by using proper type cast with first 2 bytes of buffer(to extract 16bit value) and pass it to ieee80211_is_action().

Regards,
Ajay

Ajay Singh

2018-05-30 14:50 UTC #7 lore raw

In reply to Thibaut Robert

Quoted 153 lines · On Tue, 29 May 2018 21:11:43 +0200 Thibaut Robert <[email protected]>…

This commit fix a few sparse warnings. It mostly consists of fixing the type declarations and avoiding the use of variables with mixed endianness values.

Signed-off-by Thibaut Robert <[email protected]>

---
 drivers/staging/wilc1000/wilc_spi.c           | 15 ++++++-----
 .../staging/wilc1000/wilc_wfi_cfgoperations.c |  2 +-
 drivers/staging/wilc1000/wilc_wlan.c          | 26
+++++++++---------- drivers/staging/wilc1000/wilc_wlan_cfg.c      |
8 +++--- drivers/staging/wilc1000/wilc_wlan_cfg.h      |  4 +--
 5 files changed, 27 insertions(+), 28 deletions(-)

(quoted diff) +0 -0

b/drivers/staging/wilc1000/wilc_spi.c index 647526387784..e51f0d91a376 100644 --- a/drivers/staging/wilc1000/wilc_spi.c +++ b/drivers/staging/wilc1000/wilc_spi.c @@ -666,11 +666,11 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz) static int spi_internal_write(struct wilc *wilc, u32 adr, u32 dat) {

drivers/staging/wilc1000/wilc_spi.c +1 -2

 	struct spi_device *spi = to_spi_device(wilc->dev);
+	__le32 le32dat = cpu_to_le32(dat);
 	int result;
 
-	dat = cpu_to_le32(dat);
-	result = spi_cmd_complete(wilc, CMD_INTERNAL_WRITE, adr, (u8

*)&dat, 4,

(quoted diff) +1 -1

-				  0);
+	result = spi_cmd_complete(wilc, CMD_INTERNAL_WRITE, adr, (u8

*)&le32dat,

drivers/staging/wilc1000/wilc_spi.c +5 -3

+				  4, 0);
 	if (result != N_OK)
 		dev_err(&spi->dev, "Failed internal write cmd...\n");

@@ -689,7 +689,7 @@ static int spi_internal_read(struct wilc *wilc,
 u32 adr, u32 *data) return 0;
 	}
 
-	*data = cpu_to_le32(*data);
+	le32_to_cpus(data);
 
 	return 1;
 }

@@ -706,15 +706,16 @@ static int wilc_spi_write_reg(struct wilc
 *wilc, u32 addr, u32 data) int result = N_OK;
 	u8 cmd = CMD_SINGLE_WRITE;
 	u8 clockless = 0;
+	__le32 le32data = cpu_to_le32(data);
 
-	data = cpu_to_le32(data);
 	if (addr < 0x30) {
 		/* Clockless register */
 		cmd = CMD_INTERNAL_WRITE;
 		clockless = 1;
 	}
 
-	result = spi_cmd_complete(wilc, cmd, addr, (u8 *)&data, 4,
 clockless);
+	result = spi_cmd_complete(wilc, cmd, addr, (u8 *)&le32data,
 4,
+				  clockless);
 	if (result != N_OK)

dev_err(&spi->dev, "Failed cmd, write reg
(%08x)...\n", addr);

drivers/staging/wilc1000/wilc_spi.c +1 -1

@@ -769,7 +770,7 @@ static int wilc_spi_read_reg(struct wilc *wilc,
 u32 addr, u32 *data) return 0;
 	}
 
-	*data = cpu_to_le32(*data);
+	le32_to_cpus(data);
 
 	return 1;
 }

(quoted diff) +0 -0

b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index e248702ee519..745bf5ca2622 100644 --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -1431,7 +1431,7 @@ void wilc_wfi_p2p_rx(struct net_device *dev, u8 *buff, u32 size) freq = ieee80211_channel_to_frequency(curr_channel, NL80211_BAND_2GHZ);

drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +1 -1

-	if (!ieee80211_is_action(buff[FRAME_TYPE_ID])) {
+	if (!ieee80211_is_action(cpu_to_le16(buff[FRAME_TYPE_ID]))) {
 		cfg80211_rx_mgmt(priv->wdev, freq, 0, buff, size, 0);
 		return;
 	}

(quoted diff) +0 -0

b/drivers/staging/wilc1000/wilc_wlan.c index 28c93f3f846e..a5ac1d26590b 100644 --- a/drivers/staging/wilc1000/wilc_wlan.c +++ b/drivers/staging/wilc1000/wilc_wlan.c @@ -560,7 +560,8 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 *txq_count) int ret = 0; int counter;

drivers/staging/wilc1000/wilc_wlan.c +6 -6

 	int timeout;
-	u32 vmm_table[WILC_VMM_TBL_SIZE];
+	__le32 vmm_table[WILC_VMM_TBL_SIZE];
+	u32 table_entry;
 	struct wilc_vif *vif;
 	struct wilc *wilc;
 	const struct wilc_hif_func *func;

@@ -598,10 +599,10 @@ int wilc_wlan_handle_txq(struct net_device
 *dev, u32 *txq_count) if ((sum + vmm_sz) > LINUX_TX_SIZE)
 				break;
 
-			vmm_table[i] = vmm_sz / 4;
+			table_entry = vmm_sz / 4;
 			if (tqe->type == WILC_CFG_PKT)
-				vmm_table[i] |= BIT(10);
-			vmm_table[i] = cpu_to_le32(vmm_table[i]);
+				table_entry |= BIT(10);
+			vmm_table[i] = cpu_to_le32(table_entry);
 
 			i++;
 			sum += vmm_sz;

@@ -704,8 +705,7 @@ int wilc_wlan_handle_txq(struct net_device *dev,
 u32 *txq_count) if (vmm_table[i] == 0)
 			break;
 
-		vmm_table[i] = cpu_to_le32(vmm_table[i]);
-		vmm_sz = (vmm_table[i] & 0x3ff);
+		vmm_sz = (le32_to_cpu(vmm_table[i]) & 0x3ff);
 		vmm_sz *= 4;
 		header = (tqe->type << 31) |
 			 (tqe->buffer_size << 15) |

@@ -715,8 +715,7 @@ int wilc_wlan_handle_txq(struct net_device *dev,

u32 *txq_count) else

drivers/staging/wilc1000/wilc_wlan.c +3 -4

 			header &= ~BIT(30);
 
-		header = cpu_to_le32(header);
-		memcpy(&txb[offset], &header, 4);
+		*((__le32 *)&txb[offset]) = cpu_to_le32(header);
 		if (tqe->type == WILC_CFG_PKT) {
 			buffer_offset = ETH_CONFIG_PKT_HDR_OFFSET;
 		} else if (tqe->type == WILC_NET_PKT) {

@@ -770,8 +769,7 @@ static void wilc_wlan_handle_rx_buff(struct wilc
 *wilc, u8 *buffer, int size) 
 	do {
 		buff_ptr = buffer + offset;
-		memcpy(&header, buff_ptr, 4);
-		header = cpu_to_le32(header);
+		header = le32_to_cpup((__le32 *)buff_ptr);
 
 		is_cfg_packet = (header >> 31) & 0x1;
 		pkt_offset = (header >> 22) & 0x1ff;

@@ -942,6 +940,7 @@ int wilc_wlan_firmware_download(struct wilc
 *wilc, const u8 *buffer, u32 offset;
 	u32 addr, size, size2, blksz;
 	u8 *dma_buffer;
+	const __le32 *header;
 	int ret = 0;
 
 	blksz = BIT(12);

drivers/staging/wilc1000/wilc_wlan.c int wilc_wlan_firmware_download(struct wilc · line 954

Quoted 8 lines

drivers/staging/wilc1000/wilc_wlan.c +1 -4

@@ -952,10 +951,9 @@ int wilc_wlan_firmware_download(struct wilc
 *wilc, const u8 *buffer, 
 	offset = 0;
 	do {
-		memcpy(&addr, &buffer[offset], 4);
-		memcpy(&size, &buffer[offset + 4], 4);
-		addr = cpu_to_le32(addr);
-		size = cpu_to_le32(size);
+		header = (__le32 *)buffer + offset;

In addition to Dan's comments, I have one more observation.

The above line has a crash(Oops), while downloading the firmware to WILC1000 module i.e "header = (__le32 *)buffer + offset;".

The crash can be fixes by putting extra bracket like header = (__le32 *)(buffer + offset);

But I suggest to avoid use of 'header' variable and only change the below 2 lines.

   addr = cpu_to_le32(addr);
   size = cpu_to_le32(size);

   le32_to_cpus(addr);
   le32_to_cpus(size);

Quoted 47 lines

drivers/staging/wilc1000/wilc_wlan.c +2 -0
+		addr = le32_to_cpu(header[0]);
+		size = le32_to_cpu(header[1]);
 		acquire_bus(wilc, ACQUIRE_ONLY);
 		offset += 8;
 		while (((int)size) && (offset < buffer_size)) {
(quoted diff) +0 -0
b/drivers/staging/wilc1000/wilc_wlan_cfg.c index c0b9b700f4d7..4a914d8572aa 100644 --- a/drivers/staging/wilc1000/wilc_wlan_cfg.c +++ b/drivers/staging/wilc1000/wilc_wlan_cfg.c @@ -275,14 +275,14 @@ static int wilc_wlan_cfg_set_bin(u8 *frame, u32 offset, u16 id, u8 *b, u32 size) static void wilc_wlan_parse_response_frame(u8 *info, int size) {
drivers/staging/wilc1000/wilc_wlan_cfg.c +4 -4
-	u32 wid, len = 0, i = 0;
+	u32 wid;
+	int len = 0, i = 0;
 
 	while (size > 0) {
 		i = 0;
-		wid = info[0] | (info[1] << 8);
-		wid = cpu_to_le32(wid);
+		wid = le16_to_cpup((__le16 *)info);
 
-		switch ((wid >> 12) & 0x7) {
+		switch (info[1] >> 4) {
 		case WID_CHAR:
 			do {
 				if (g_cfg_byte[i].id == WID_NIL)
(quoted diff) +0 -0
b/drivers/staging/wilc1000/wilc_wlan_cfg.h index 08092a551840..9e029338bcab 100644 --- a/drivers/staging/wilc1000/wilc_wlan_cfg.h +++ b/drivers/staging/wilc1000/wilc_wlan_cfg.h @@ -18,12 +18,12 @@ struct wilc_cfg_byte {
struct wilc_cfg_hword {
drivers/staging/wilc1000/wilc_wlan_cfg.h +2 -2
 	u16 id;
-	u16 val;
+	__le16 val;
 };
 
 struct wilc_cfg_word {
 	u32 id;
-	u32 val;
+	__le32 val;
 };
 
 struct wilc_cfg_str {