[PATCH] wimax: attempt to address style issues.
From: Gabriele Modena
When running checkpatch.pl on op-msg.c, op-rfkill.c
and stack.c I noticed that they contained a few style issues
at warning level of severity. This patch is both an attempt to
address the warnings, as well as a way for me to familiarise
with the linux kernel contribution process, by following
tasks proposed by a popular online challenge.
Signed-off-by: Gabriele Modena
---
drivers/staging/wimax/op-msg.c| 8
drivers/staging/wimax/op-rfkill.c | 7 ---
drivers/staging/wimax/stack.c | 12 ++--
3 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/wimax/op-msg.c b/drivers/staging/wimax/op-msg.c
index e20ac7d84e82..11da48bf704f 100644
--- a/drivers/staging/wimax/op-msg.c
+++ b/drivers/staging/wimax/op-msg.c
@@ -142,8 +142,8 @@ struct sk_buff *wimax_msg_alloc(struct wimax_dev *wimax_dev,
}
result = nla_put(skb, WIMAX_GNL_MSG_DATA, size, msg);
if (result < 0) {
- dev_err(dev, "no memory to add payload (msg %p size %zu) in "
- "attribute: %d\n", msg, size, result);
+ dev_err(dev, "no memory to add payload (msg %p size %zu) in
attribute: %d\n",
+ msg, size, result);
goto error_nla_put;
}
genlmsg_end(skb, genl_msg);
@@ -260,6 +260,7 @@ int wimax_msg_send(struct wimax_dev *wimax_dev, struct
sk_buff *skb)
struct device *dev = wimax_dev_to_dev(wimax_dev);
void *msg = skb->data;
size_t size = skb->len;
+
might_sleep();
d_printf(1, dev, "CTX: wimax msg, %zu bytes\n", size);
@@ -340,8 +341,7 @@ int wimax_gnl_doit_msg_from_user(struct sk_buff *skb,
struct genl_info *info)
/* Unpack arguments */
result = -EINVAL;
if (info->attrs[WIMAX_GNL_MSG_DATA] == NULL) {
- dev_err(dev, "WIMAX_GNL_MSG_FROM_USER: can't find MSG_DATA "
- "attribute\n");
+ dev_err(dev, "WIMAX_GNL_MSG_FROM_USER: can't find MSG_DATA
attribute\n");
goto error_no_data;
}
msg_buf = nla_data(info->attrs[WIMAX_GNL_MSG_DATA]);
diff --git a/drivers/staging/wimax/op-rfkill.c
b/drivers/staging/wimax/op-rfkill.c
index 78b294481a59..52612ed09183 100644
--- a/drivers/staging/wimax/op-rfkill.c
+++ b/drivers/staging/wimax/op-rfkill.c
@@ -294,7 +294,8 @@ int wimax_rfkill(struct wimax_dev *wimax_dev, enum
wimax_rf_state state)
/* While initializing, < 1.4.3 wimax-tools versions use
* this call to check if the device is a valid WiMAX
* device; so we allow it to proceed always,
-* considering the radios are all off. */
+* considering the radios are all off.
+*/
if (result == -ENOMEDIUM && state == WIMAX_RF_QUERY)
result = WIMAX_RF_OFF << 1 | WIMAX_RF_OFF;
goto error_not_ready;
@@ -378,6 +379,7 @@ int wimax_rfkill_add(struct wimax_dev *wimax_dev)
void wimax_rfkill_rm(struct wimax_dev *wimax_dev)
{
struct device *dev = wimax_dev_to_dev(wimax_dev);
+
d_fnstart(3, dev, "(wimax_dev %p)\n", wimax_dev);
rfkill_unregister(wimax_dev->rfkill);
rfkill_destroy(wimax_dev->rfkill);
@@ -415,8 +417,7 @@ int wimax_gnl_doit_rfkill(struct sk_buff *skb, struct
genl_info *info)
dev = wimax_dev_to_dev(wimax_dev);
result = -EINVAL;
if (info->attrs[WIMAX_GNL_RFKILL_STATE] == NULL) {
- dev_err(dev, "WIMAX_GNL_RFKILL: can't find RFKILL_STATE "
- "attribute\n");
+ dev_err(dev, "WIMAX_GNL_RFKILL: can't find RFKILL_STATE
attribute\n");
goto error_no_pid;
}
new_state = nla_get_u32(info->attrs[WIMAX_GNL_RFKILL_STATE]);
diff --git a/drivers/staging/wimax/stack.c b/drivers/staging/wimax/stack.c
index ace24a6dfd2d..e44158334246 100644
--- a/drivers/staging/wimax/stack.c
+++ b/drivers/staging/wimax/stack.c
@@ -51,9 +51,7 @@ static char wimax_debug_params[128];
module_param_string(debug, wimax_debug_params, sizeof(wimax_debug_params),
0644);
MODULE_PARM_DESC(debug,
-"String of space-separated NAME:VALUE pairs, where NAMEs "
-"are the different debug submodules and VALUE are the "
-"initial debug value to set.");
+"String of space-separated NAME:VALUE pairs, where NAMEs are
the different debug submodules and VALUE are the initial debug value to set.");
/*
* Authoritative source for the RE_STATE_CHANGE attribute policy
@@ -62,7 +60,7 @@ MODULE_PARM_DESC(debug,
* close to where the data is generated.
*/
/*
-static const struct nla_policy wimax_gnl_re_status_change[WIMAX_GNL_ATTR_MAX +
1] = {
+ * static const struct nla_policy
wimax_gnl_re_status_change[WIMAX_GNL_ATTR_MAX + 1] = {
[WIMAX_GNL_STCH_STATE_OLD] = { .type = NLA_U8 },
[WIMAX_GNL_STCH_STATE_NEW
Re: [PATCH] wimax: attempt to address style issues.
On Tue, Mar 02, 2021 at 10:11:02AM +0100, gabriele.mod...@gmail.com wrote:
> From: Gabriele Modena
>
> When running checkpatch.pl on op-msg.c, op-rfkill.c
> and stack.c I noticed that they contained a few style issues
> at warning level of severity. This patch is both an attempt to
> address the warnings, as well as a way for me to familiarise
> with the linux kernel contribution process, by following
> tasks proposed by a popular online challenge.
>
> Signed-off-by: Gabriele Modena
> ---
> drivers/staging/wimax/op-msg.c| 8
> drivers/staging/wimax/op-rfkill.c | 7 ---
> drivers/staging/wimax/stack.c | 12 ++--
> 3 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/wimax/op-msg.c b/drivers/staging/wimax/op-msg.c
> index e20ac7d84e82..11da48bf704f 100644
> --- a/drivers/staging/wimax/op-msg.c
> +++ b/drivers/staging/wimax/op-msg.c
> @@ -142,8 +142,8 @@ struct sk_buff *wimax_msg_alloc(struct wimax_dev
> *wimax_dev,
> }
> result = nla_put(skb, WIMAX_GNL_MSG_DATA, size, msg);
> if (result < 0) {
> - dev_err(dev, "no memory to add payload (msg %p size %zu) in "
> - "attribute: %d\n", msg, size, result);
> + dev_err(dev, "no memory to add payload (msg %p size %zu) in
> attribute: %d\n",
> + msg, size, result);
> goto error_nla_put;
> }
> genlmsg_end(skb, genl_msg);
> @@ -260,6 +260,7 @@ int wimax_msg_send(struct wimax_dev *wimax_dev, struct
> sk_buff *skb)
> struct device *dev = wimax_dev_to_dev(wimax_dev);
> void *msg = skb->data;
> size_t size = skb->len;
> +
> might_sleep();
>
> d_printf(1, dev, "CTX: wimax msg, %zu bytes\n", size);
> @@ -340,8 +341,7 @@ int wimax_gnl_doit_msg_from_user(struct sk_buff *skb,
> struct genl_info *info)
> /* Unpack arguments */
> result = -EINVAL;
> if (info->attrs[WIMAX_GNL_MSG_DATA] == NULL) {
> - dev_err(dev, "WIMAX_GNL_MSG_FROM_USER: can't find MSG_DATA "
> - "attribute\n");
> + dev_err(dev, "WIMAX_GNL_MSG_FROM_USER: can't find MSG_DATA
> attribute\n");
> goto error_no_data;
> }
> msg_buf = nla_data(info->attrs[WIMAX_GNL_MSG_DATA]);
> diff --git a/drivers/staging/wimax/op-rfkill.c
> b/drivers/staging/wimax/op-rfkill.c
> index 78b294481a59..52612ed09183 100644
> --- a/drivers/staging/wimax/op-rfkill.c
> +++ b/drivers/staging/wimax/op-rfkill.c
> @@ -294,7 +294,8 @@ int wimax_rfkill(struct wimax_dev *wimax_dev, enum
> wimax_rf_state state)
> /* While initializing, < 1.4.3 wimax-tools versions use
>* this call to check if the device is a valid WiMAX
>* device; so we allow it to proceed always,
> - * considering the radios are all off. */
> + * considering the radios are all off.
> + */
> if (result == -ENOMEDIUM && state == WIMAX_RF_QUERY)
> result = WIMAX_RF_OFF << 1 | WIMAX_RF_OFF;
> goto error_not_ready;
> @@ -378,6 +379,7 @@ int wimax_rfkill_add(struct wimax_dev *wimax_dev)
> void wimax_rfkill_rm(struct wimax_dev *wimax_dev)
> {
> struct device *dev = wimax_dev_to_dev(wimax_dev);
> +
> d_fnstart(3, dev, "(wimax_dev %p)\n", wimax_dev);
> rfkill_unregister(wimax_dev->rfkill);
> rfkill_destroy(wimax_dev->rfkill);
> @@ -415,8 +417,7 @@ int wimax_gnl_doit_rfkill(struct sk_buff *skb, struct
> genl_info *info)
> dev = wimax_dev_to_dev(wimax_dev);
> result = -EINVAL;
> if (info->attrs[WIMAX_GNL_RFKILL_STATE] == NULL) {
> - dev_err(dev, "WIMAX_GNL_RFKILL: can't find RFKILL_STATE "
> - "attribute\n");
> + dev_err(dev, "WIMAX_GNL_RFKILL: can't find RFKILL_STATE
> attribute\n");
> goto error_no_pid;
> }
> new_state = nla_get_u32(info->attrs[WIMAX_GNL_RFKILL_STATE]);
> diff --git a/drivers/staging/wimax/stack.c b/drivers/staging/wimax/stack.c
> index ace24a6dfd2d..e44158334246 100644
> --- a/drivers/staging/wimax/stack.c
> +++ b/drivers/staging/wimax/stack.c
> @@ -51,9 +51,7 @@ static char wimax_debug_params[128];
> module_param_string(debug, wimax_debug_params, sizeof(wimax_debug_params),
> 0644);
> MODULE_PARM_DESC(debug,
> - "String of space-separated NAME:VALUE pairs, where NAMEs "
> - "are the different debug submodules and VALUE are the "
> - "initial debug value to set.");
> + "String of space-separated NAME:VALUE pairs, where NAMEs are
> the different debug submodules and VALUE are the initial debug value to
> set.");
>
> /*
> * Authoritative source for the RE_STATE_CHANGE attribute policy
> @@ -62,7 +60,7 @@ MODULE_PARM_DESC(debug,
> * close to where the data is generated.
> */
> /*
> -static const struct nla_policy wimax_gnl_re_status_change[WIMAX_GNL_A
[PATCH] phy: ralink: phy-mt7621-pci: fix XTAL bitmask
When this was rewriten to get mainlined and start to
use 'linux/bitfield.h' headers, XTAL_MASK was wrong.
It must mask three bits but only two were used. Hence
properly fix it to make things work.
Fixes: d87da32372a0 ("phy: ralink: Add PHY driver for MT7621 PCIe PHY")
Signed-off-by: Sergio Paracuellos
---
drivers/phy/ralink/phy-mt7621-pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/ralink/phy-mt7621-pci.c
b/drivers/phy/ralink/phy-mt7621-pci.c
index 9a610b414b1f..84ee2b5c2228 100644
--- a/drivers/phy/ralink/phy-mt7621-pci.c
+++ b/drivers/phy/ralink/phy-mt7621-pci.c
@@ -62,7 +62,7 @@
#define RG_PE1_FRC_MSTCKDIVBIT(5)
-#define XTAL_MASK GENMASK(7, 6)
+#define XTAL_MASK GENMASK(8, 6)
#define MAX_PHYS 2
--
2.25.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: ks7010: prevent buffer overflow in ks_wlan_set_scan()
The user can specify a "req->essid_len" of up to 255 but if it's
over IW_ESSID_MAX_SIZE (32) that can lead to memory corruption.
Fixes: 13a9930d15b4 ("staging: ks7010: add driver from Nanonote
extra-repository")
Signed-off-by: Dan Carpenter
---
drivers/staging/ks7010/ks_wlan_net.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/ks7010/ks_wlan_net.c
b/drivers/staging/ks7010/ks_wlan_net.c
index dc09cc6e1c47..09e7b4cd0138 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1120,6 +1120,7 @@ static int ks_wlan_set_scan(struct net_device *dev,
{
struct ks_wlan_private *priv = netdev_priv(dev);
struct iw_scan_req *req = NULL;
+ int len;
if (priv->sleep_mode == SLP_SLEEP)
return -EPERM;
@@ -1129,8 +1130,9 @@ static int ks_wlan_set_scan(struct net_device *dev,
if (wrqu->data.length == sizeof(struct iw_scan_req) &&
wrqu->data.flags & IW_SCAN_THIS_ESSID) {
req = (struct iw_scan_req *)extra;
- priv->scan_ssid_len = req->essid_len;
- memcpy(priv->scan_ssid, req->essid, priv->scan_ssid_len);
+ len = min_t(int, req->essid_len, IW_ESSID_MAX_SIZE);
+ priv->scan_ssid_len = len;
+ memcpy(priv->scan_ssid, req->essid, len);
} else {
priv->scan_ssid_len = 0;
}
--
2.30.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] phy: ralink: phy-mt7621-pci: fix XTAL bitmask
On 02-03-21, 11:54, Sergio Paracuellos wrote: > When this was rewriten to get mainlined and start to > use 'linux/bitfield.h' headers, XTAL_MASK was wrong. > It must mask three bits but only two were used. Hence > properly fix it to make things work. Applied, thanks -- ~Vinod ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] This patch fixes the check-patch errors
On Tue, Mar 02, 2021 at 06:12:54PM +0530, Vikas Kumar Sharma wrote: > Disclaimer:This message is intended only for the designated recipient(s). > It may contain confidential or proprietary information and may be subject > to other confidentiality protections. If you are not a designated > recipient, you may not review, copy or distribute this message. Please > notify the sender by e-mail and delete this message. GlobalEdge does not > accept any liability for virus infected mails. Email is now deleted, sorry, this does not belong on any emails sent to public mailing lists for kernel development efforts. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] This patch fixes the check-patch errors
ERROR: Macros with complex values should be enclosed in parentheses.
Signed-off-by: Vikas Kumar Sharma
---
drivers/staging/ks7010/ks_hostif.h | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/ks7010/ks_hostif.h
b/drivers/staging/ks7010/ks_hostif.h
index 39138191a556..c62a494ed6bb 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -498,20 +498,20 @@ struct hostif_mic_failure_request {
#define TX_RATE_FIXED 5
/* 11b rate */
-#define TX_RATE_1M (u8)(10 / 5)/* 11b 11g basic rate */
-#define TX_RATE_2M (u8)(20 / 5)/* 11b 11g basic rate */
-#define TX_RATE_5M (u8)(55 / 5)/* 11g basic rate */
-#define TX_RATE_11M(u8)(110 / 5) /* 11g basic rate */
+#define TX_RATE_1M ((u8)(10 / 5)) /* 11b 11g basic rate */
+#define TX_RATE_2M ((u8)(20 / 5)) /* 11b 11g basic rate */
+#define TX_RATE_5M ((u8)(55 / 5)) /* 11g basic rate */
+#define TX_RATE_11M((u8)(110 / 5)) /* 11g basic rate */
/* 11g rate */
-#define TX_RATE_6M (u8)(60 / 5)/* 11g basic rate */
-#define TX_RATE_12M(u8)(120 / 5) /* 11g basic rate */
-#define TX_RATE_24M(u8)(240 / 5) /* 11g basic rate */
-#define TX_RATE_9M (u8)(90 / 5)
-#define TX_RATE_18M(u8)(180 / 5)
-#define TX_RATE_36M(u8)(360 / 5)
-#define TX_RATE_48M(u8)(480 / 5)
-#define TX_RATE_54M(u8)(540 / 5)
+#define TX_RATE_6M ((u8)(60 / 5)) /* 11g basic rate */
+#define TX_RATE_12M((u8)(120 / 5)) /* 11g basic rate */
+#define TX_RATE_24M((u8)(240 / 5)) /* 11g basic rate */
+#define TX_RATE_9M ((u8)(90 / 5))
+#define TX_RATE_18M((u8)(180 / 5))
+#define TX_RATE_36M((u8)(360 / 5))
+#define TX_RATE_48M((u8)(480 / 5))
+#define TX_RATE_54M((u8)(540 / 5))
static inline bool is_11b_rate(u8 rate)
{
--
2.25.1
--
Disclaimer:This message is intended only for the designated recipient(s).
It may contain confidential or proprietary information and may be subject
to other confidentiality protections. If you are not a designated
recipient, you may not review, copy or distribute this message. Please
notify the sender by e-mail and delete this message. GlobalEdge does not
accept any liability for virus infected mails.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8192e: remove redundant variable shadowing
In function rtl92e_start_adapter() automatic variable 'i' referenced only
within certain loops, used as iteration counter. Control flow can't get
into such loop w/o 'i = 0' assignment.
It's redundant to shadow this variable by creating scope around loop.
This patch fixes the following sparse warning:
warning: symbol 'i' shadows an earlier one
Signed-off-by: Nikolay Kyx
---
drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c
b/drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c
index ff843d7ec606..8cd085ebea81 100644
--- a/drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c
+++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_dev.c
@@ -800,12 +800,10 @@ bool rtl92e_start_adapter(struct net_device *dev)
}
rtl92e_writew(dev, ATIMWND, 2);
rtl92e_writew(dev, BCN_INTERVAL, 100);
- {
- int i;
- for (i = 0; i < QOS_QUEUE_NUM; i++)
- rtl92e_writel(dev, WDCAPARA_ADD[i], 0x005e4332);
- }
+ for (i = 0; i < QOS_QUEUE_NUM; i++)
+ rtl92e_writel(dev, WDCAPARA_ADD[i], 0x005e4332);
+
rtl92e_writeb(dev, 0xbe, 0xc0);
rtl92e_config_mac(dev);
--
2.30.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192u avoid flex array of flex array
On Sat, Feb 27, 2021 at 07:06:14PM -0600, Darryl T. Agostinelli wrote: > Undo the flex array in struct ieee80211_info_element. It is used as the flex > array type in other structs (creating a flex array of flex arrays) making > sparse unhappy. This change maintains the intent of the code and satisfies > sparse. We have been trying to convert the kernel to use the [] style over time, please don't move backwards on this. There are loads of commits by Gustavo in the tree that show this work. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: rtl8192e: Change state information from u16 to u8
On Mon, Feb 22, 2021 at 10:53:30PM +0530, Atul Gopinathan wrote: > On Mon, Feb 22, 2021 at 04:26:33PM +0100, Greg KH wrote: > > On Sun, Feb 21, 2021 at 10:27:21PM +0530, Atul Gopinathan wrote: > > > On Sun, Feb 21, 2021 at 02:08:26PM +0100, Greg KH wrote: > > > > On Sat, Feb 20, 2021 at 11:51:55PM +0530, Atul Gopinathan wrote: > > > > > The "CcxRmState" field in struct "rtllib_network" is defined > > > > > as a u16 array of size 2 (so, 4 bytes in total). > > > > > > > > > > But the operations performed on this array throughout the code > > > > > base (in rtl8192e/) are all in byte size 2 indicating that this > > > > > array's type was defined wrongly. > > > > > > > > > > There are two situation were u16 type of this field could yield > > > > > incorrect behaviour: > > > > > > > > > > 1. In rtllib_rx.c:1970: > > > > > memcpy(network->CcxRmState, &info_element->data[4], 2); > > > > > > > > > > Here last 2 bytes (index 4 and 5) from the info_element->data[] > > > > > array are meant to be copied into CcxRmState[]. > > > > > Note that "data" array here is an array of type u8. > > > > > > > > > > 2. In function "update_network()" in staging/rtl8192e/rtllib_rx.c: > > > > > memcpy(dst->CcxRmState, src->CcxRmState, 2); > > > > > > > > > > Here again, only 2 bytes are copied from the source state to > > > > > destination state. > > > > > > > > > > There are no instances of "CcxRmState" requiring u16 data type. > > > > > Here is the output of "grep -IRn 'CcxRmState'" on the rtl8192e/ > > > > > directory for reviewing: > > > > > > > > > > rtllib_rx.c:1970: memcpy(network->CcxRmState, > > > > > &info_element->data[4], 2); > > > > > rtllib_rx.c:1971: if (network->CcxRmState[0] != 0) > > > > > rtllib_rx.c:1975: network->MBssidMask = > > > > > network->CcxRmState[1] & 0x07; > > > > > rtllib_rx.c:2520: memcpy(dst->CcxRmState, src->CcxRmState, 2); > > > > > rtllib.h:1108:u8 CcxRmState[2]; > > > > > > > > You just changed the logic in line 1975 in that file, right? Are you > > > > _SURE_ that is ok? Do you have a device to test this on? > > > > > > I'm sorry, I didn't quite get you. By line 1975 in rtllib_rx.c, did you > > > mean > > > the following line?: > > > > > > network->MBssidMask = network->CcxRmState[1] & 0x07; > > > > Yes. > > > > > network->CcxRmState is being fed with 2 bytes of u8 data, in line 1970 (as > > > seen above). I believe my patch doesn't change the logic of an "&" > > > operation > > > being performed on it with 0x07, right? > > > > It changes the location of the [1] operation to point to a different > > place in memory from what I can tell, as you changed the type of that > > array. > > Oh yes, earlier, the network->CcxRmState[] array had memory locations as: > [x, x+16]. With this patch, it's locations are [x, x+8]. > > And I strongly believe this is how it should be based on how the original > author is using the CcxRmState[] array throughout the codebase: Ok, but this has changed the way memory is addressed, which is what I was trying to point out when you said that nothing had changed. > Allow me to explain (Based on the output of "grep -IRn 'CcxRmState'" that > I sent previously): > 1. At line 1970: > > memcpy(network->CcxRmState, &info_element->data[4], 2); > > this is where the array CcxRmState[] is being fed with > data. And one can see the source is an array named "data" which itself > has type u8. The third argument is "2", meaning 2 bytes of data should > be written from "data" array to "CcxRmState". > > Also note that, the array CcxRmState has a size 2, as defined in > rtllib.h, in struct "rtllib_network": > > u16 CcxRmState[2]; > > Say if CcxRmState[] _was_ supposed to be u16 and not u8, then both elements > of the source "data" array will only be written into the first element of > "CcxRmState", i.e, "CcxRmState[0]". The 2nd element, "CcxRmState[1]" will > never be fed with any data. The resultant CcxRmState[] array would look > something like this: > > [(u8-data and u8-data squashed), 0]. > > The 2 u8-data here refers to info_element->data[4] and > info_element->data[5]. > > Instead, if "CcxRmState" was of type u8, then both elements of source > "data" array will be written into the 2 elements of "CcxRmState" > respectively: > > [u8 data, u8 data] > > This makes a lot more sense. > > 2. Line 1975: > network->MBssidMask = network->CcxRmState[1] & 0x07; > > With point 1 clear, it should now be easy to understand that > the the "&" operation in line 1975, will _always_ yield 0 if "CcxRmState" > is u16, simply because CcxRmState[1] is never fed with any data at > all. > > Oh and "network->MBssidMask" is also of type u8. > > 3. Line 2520: > memcpy(dst->CcxRmState, src->CcxRmState, 2); > > 2 bytes, and not 4, again.:D > The above line belongs to the following function: > > static inline void update_network(struct rtllib_device *ieee, >
[PATCH] staging: rtl8192e: remove unnecessary break
From: Perrin Smith removed unnecessary break at end of while loop Signed-off-by: Perrin Smith --- drivers/staging/rtl8192e/rtllib_rx.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/rtl8192e/rtllib_rx.c b/drivers/staging/rtl8192e/rtllib_rx.c index b8ab34250e6a..2de6330b7737 100644 --- a/drivers/staging/rtl8192e/rtllib_rx.c +++ b/drivers/staging/rtl8192e/rtllib_rx.c @@ -460,8 +460,6 @@ static bool AddReorderEntry(struct rx_ts_record *pTS, ((struct rx_reorder_entry *)list_entry(pList->next, struct rx_reorder_entry, List))->SeqNum)) return false; - else - break; } pReorderEntry->List.next = pList->next; pReorderEntry->List.next->prev = &pReorderEntry->List; -- 2.26.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192e: remove unnecessary break
On Tue, Mar 02, 2021 at 11:29:03PM +0800, Perrin Smith wrote: > From: Perrin Smith > > removed unnecessary break at end of while loop > > Signed-off-by: Perrin Smith > --- > drivers/staging/rtl8192e/rtllib_rx.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/staging/rtl8192e/rtllib_rx.c > b/drivers/staging/rtl8192e/rtllib_rx.c > index b8ab34250e6a..2de6330b7737 100644 > --- a/drivers/staging/rtl8192e/rtllib_rx.c > +++ b/drivers/staging/rtl8192e/rtllib_rx.c > @@ -460,8 +460,6 @@ static bool AddReorderEntry(struct rx_ts_record *pTS, > ((struct rx_reorder_entry *)list_entry(pList->next, > struct rx_reorder_entry, List))->SeqNum)) > return false; > - else > - break; No, this break is necessary. The patch introduces a bug. You need to be careful following checkpatch's advice because it's just a simple Perl script. > } regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: rtl8192e: Change state information from u16 to u8
On Tue, Mar 02, 2021 at 03:38:52PM +0100, Greg KH wrote: > On Mon, Feb 22, 2021 at 10:53:30PM +0530, Atul Gopinathan wrote: > > On Mon, Feb 22, 2021 at 04:26:33PM +0100, Greg KH wrote: > > > On Sun, Feb 21, 2021 at 10:27:21PM +0530, Atul Gopinathan wrote: > > > > On Sun, Feb 21, 2021 at 02:08:26PM +0100, Greg KH wrote: > > > > > On Sat, Feb 20, 2021 at 11:51:55PM +0530, Atul Gopinathan wrote: > > > > > > The "CcxRmState" field in struct "rtllib_network" is defined > > > > > > as a u16 array of size 2 (so, 4 bytes in total). > > > > > > > > > > > > But the operations performed on this array throughout the code > > > > > > base (in rtl8192e/) are all in byte size 2 indicating that this > > > > > > array's type was defined wrongly. > > > > > > > > > > > > There are two situation were u16 type of this field could yield > > > > > > incorrect behaviour: > > > > > > > > > > > > 1. In rtllib_rx.c:1970: > > > > > > memcpy(network->CcxRmState, &info_element->data[4], 2); > > > > > > > > > > > > Here last 2 bytes (index 4 and 5) from the info_element->data[] > > > > > > array are meant to be copied into CcxRmState[]. > > > > > > Note that "data" array here is an array of type u8. > > > > > > > > > > > > 2. In function "update_network()" in staging/rtl8192e/rtllib_rx.c: > > > > > > memcpy(dst->CcxRmState, src->CcxRmState, 2); > > > > > > > > > > > > Here again, only 2 bytes are copied from the source state to > > > > > > destination state. > > > > > > > > > > > > There are no instances of "CcxRmState" requiring u16 data type. > > > > > > Here is the output of "grep -IRn 'CcxRmState'" on the rtl8192e/ > > > > > > directory for reviewing: > > > > > > > > > > > > rtllib_rx.c:1970: memcpy(network->CcxRmState, > > > > > > &info_element->data[4], 2); > > > > > > rtllib_rx.c:1971: if (network->CcxRmState[0] != 0) > > > > > > rtllib_rx.c:1975: network->MBssidMask = > > > > > > network->CcxRmState[1] & 0x07; > > > > > > rtllib_rx.c:2520: memcpy(dst->CcxRmState, src->CcxRmState, 2); > > > > > > rtllib.h:1108: u8 CcxRmState[2]; > > > > > > > > > > You just changed the logic in line 1975 in that file, right? Are you > > > > > _SURE_ that is ok? Do you have a device to test this on? > > > > > > > > I'm sorry, I didn't quite get you. By line 1975 in rtllib_rx.c, did you > > > > mean > > > > the following line?: > > > > > > > > network->MBssidMask = network->CcxRmState[1] & 0x07; > > > > > > Yes. > > > > > > > network->CcxRmState is being fed with 2 bytes of u8 data, in line 1970 > > > > (as > > > > seen above). I believe my patch doesn't change the logic of an "&" > > > > operation > > > > being performed on it with 0x07, right? > > > > > > It changes the location of the [1] operation to point to a different > > > place in memory from what I can tell, as you changed the type of that > > > array. > > > > Oh yes, earlier, the network->CcxRmState[] array had memory locations as: > > [x, x+16]. With this patch, it's locations are [x, x+8]. > > > > And I strongly believe this is how it should be based on how the original > > author is using the CcxRmState[] array throughout the codebase: > > Ok, but this has changed the way memory is addressed, which is what I > was trying to point out when you said that nothing had changed. Ah sorry about that, It just wasn't clear to me what you meant and my mind was too fixated on the "&" operation. > > > Allow me to explain (Based on the output of "grep -IRn 'CcxRmState'" that > > I sent previously): > > 1. At line 1970: > > > > memcpy(network->CcxRmState, &info_element->data[4], 2); > > > > this is where the array CcxRmState[] is being fed with > > data. And one can see the source is an array named "data" which itself > > has type u8. The third argument is "2", meaning 2 bytes of data should > > be written from "data" array to "CcxRmState". > > > > Also note that, the array CcxRmState has a size 2, as defined in > > rtllib.h, in struct "rtllib_network": > > > > u16 CcxRmState[2]; > > > > Say if CcxRmState[] _was_ supposed to be u16 and not u8, then both elements > > of the source "data" array will only be written into the first element of > > "CcxRmState", i.e, "CcxRmState[0]". The 2nd element, "CcxRmState[1]" will > > never be fed with any data. The resultant CcxRmState[] array would look > > something like this: > > > > [(u8-data and u8-data squashed), 0]. > > > > The 2 u8-data here refers to info_element->data[4] and > > info_element->data[5]. > > > > Instead, if "CcxRmState" was of type u8, then both elements of source > > "data" array will be written into the 2 elements of "CcxRmState" > > respectively: > > > > [u8 data, u8 data] > > > > This makes a lot more sense. > > > > 2. Line 1975: > > network->MBssidMask = network->CcxRmState[1] & 0x07; > > > > With point 1 clear, it should now be easy to understand that > > the the "&" operation in line 1975, will _always_ yield
Re: [PATCH] staging: wfx: remove unused included header files
Hello Muhammad,
Sorry, I am a bit late for the review of this patch. Thank you for your
contribution.
On Thursday 11 February 2021 15:36:37 CET Muhammad Usama Anjum wrote:
>
> Many header files have been included, but never used. Those header
> files have been removed.
>
> Signed-off-by: Muhammad Usama Anjum
> ---
> drivers/staging/wfx/bh.c | 1 -
> drivers/staging/wfx/bh.h | 4
> drivers/staging/wfx/bus.h | 3 ---
> drivers/staging/wfx/bus_sdio.c| 6 --
> drivers/staging/wfx/bus_spi.c | 7 ---
> drivers/staging/wfx/data_rx.c | 5 -
> drivers/staging/wfx/data_tx.c | 5 -
> drivers/staging/wfx/data_tx.h | 3 ---
> drivers/staging/wfx/debug.c | 6 --
> drivers/staging/wfx/fwio.c| 2 --
> drivers/staging/wfx/hif_api_cmd.h | 4
> drivers/staging/wfx/hif_api_general.h | 9 -
> drivers/staging/wfx/hif_tx.c | 4
> drivers/staging/wfx/hif_tx_mib.c | 5 -
> drivers/staging/wfx/hwio.c| 3 ---
> drivers/staging/wfx/hwio.h| 2 --
> drivers/staging/wfx/key.c | 2 --
> drivers/staging/wfx/key.h | 2 --
> drivers/staging/wfx/main.c| 7 ---
> drivers/staging/wfx/main.h| 3 ---
> drivers/staging/wfx/queue.c | 4
> drivers/staging/wfx/queue.h | 3 ---
> drivers/staging/wfx/scan.h| 2 --
> drivers/staging/wfx/sta.c | 6 --
> drivers/staging/wfx/sta.h | 2 --
> drivers/staging/wfx/traces.h | 3 ---
> drivers/staging/wfx/wfx.h | 3 ---
> 27 files changed, 106 deletions(-)
>
> diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
> index ed53d0b45592..cd6bcfdfbe9a 100644
> --- a/drivers/staging/wfx/bh.c
> +++ b/drivers/staging/wfx/bh.c
> @@ -5,7 +5,6 @@
> * Copyright (c) 2017-2020, Silicon Laboratories, Inc.
> * Copyright (c) 2010, ST-Ericsson
> */
> -#include
> #include
Though bh.c refers to gpiod_set_value_cansleep()
> #include "bh.h"
> diff --git a/drivers/staging/wfx/bh.h b/drivers/staging/wfx/bh.h
> index 78c49329e22a..92ef3298d4ac 100644
> --- a/drivers/staging/wfx/bh.h
> +++ b/drivers/staging/wfx/bh.h
> @@ -8,10 +8,6 @@
> #ifndef WFX_BH_H
> #define WFX_BH_H
>
> -#include
> -#include
> -#include
> -
> struct wfx_dev;
>
> struct wfx_hif {
Ditto, bh.h refers to atomic_t, struct work_struct and struct
completion. If you try to compile bh.h alone (with something like
gcc -xc .../bh.h) it won't work.
Maybe it works now because we are lucky in the order the headers are
included, but I think it is not sufficient.
[... same problem repeats multiple times in the following ...]
--
Jérôme Pouiller
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[staging:staging-linus] BUILD SUCCESS d1a5bd3f875c3a507470ecce1b77e40406e34302
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git staging-linus branch HEAD: d1a5bd3f875c3a507470ecce1b77e40406e34302 staging: comedi: pcl726: Use 16-bit 0 for interrupt data elapsed time: 731m configs tested: 92 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig arm ep93xx_defconfig arm am200epdkit_defconfig powerpcwarp_defconfig m68kstmark2_defconfig riscvnommu_k210_defconfig m68k amiga_defconfig powerpc acadia_defconfig mips bigsur_defconfig armoxnas_v6_defconfig xtensa nommu_kc705_defconfig s390 zfcpdump_defconfig sh rts7751r2d1_defconfig sparc defconfig c6x alldefconfig sh kfr2r09-romimage_defconfig parisc defconfig arm h3600_defconfig armvexpress_defconfig xtensa iss_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig s390 allyesconfig s390 allmodconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig i386 tinyconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a005-20210302 i386 randconfig-a003-20210302 i386 randconfig-a002-20210302 i386 randconfig-a004-20210302 i386 randconfig-a006-20210302 i386 randconfig-a001-20210302 i386 randconfig-a016-20210302 i386 randconfig-a012-20210302 i386 randconfig-a014-20210302 i386 randconfig-a013-20210302 i386 randconfig-a011-20210302 i386 randconfig-a015-20210302 x86_64 randconfig-a006-20210302 x86_64 randconfig-a001-20210302 x86_64 randconfig-a004-20210302 x86_64 randconfig-a002-20210302 x86_64 randconfig-a005-20210302 x86_64 randconfig-a003-20210302 riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 rhel-8.3-kbuiltin x86_64 kexec clang tested configs: x86_64 randconfig-a013-20210302 x86_64 randconfig-a016-20210302 x86_64 randconfig-a015-20210302 x86_64 randconfig-a014-20210302 x86_64 randconfig-a012-20210302 x86_64 randconfig-a011-20210302 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[staging:staging-testing] BUILD SUCCESS 4e1c5d4c35d8d5a5f861019f1392ebaa0abb490b
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git staging-testing branch HEAD: 4e1c5d4c35d8d5a5f861019f1392ebaa0abb490b staging: wimax/i2400m: convert __le32 type to host byte-order elapsed time: 731m configs tested: 90 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig arm ep93xx_defconfig arm am200epdkit_defconfig powerpcwarp_defconfig m68kstmark2_defconfig mips bigsur_defconfig armoxnas_v6_defconfig xtensa nommu_kc705_defconfig s390 zfcpdump_defconfig sh rts7751r2d1_defconfig sparc defconfig c6x alldefconfig sh kfr2r09-romimage_defconfig parisc defconfig arm h3600_defconfig armvexpress_defconfig xtensa iss_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig s390 allyesconfig s390 allmodconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig i386 tinyconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a005-20210302 i386 randconfig-a003-20210302 i386 randconfig-a002-20210302 i386 randconfig-a004-20210302 i386 randconfig-a006-20210302 i386 randconfig-a001-20210302 i386 randconfig-a016-20210302 i386 randconfig-a012-20210302 i386 randconfig-a014-20210302 i386 randconfig-a013-20210302 i386 randconfig-a011-20210302 i386 randconfig-a015-20210302 x86_64 randconfig-a006-20210302 x86_64 randconfig-a001-20210302 x86_64 randconfig-a004-20210302 x86_64 randconfig-a002-20210302 x86_64 randconfig-a005-20210302 x86_64 randconfig-a003-20210302 riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 rhel-8.3-kbuiltin x86_64 kexec clang tested configs: x86_64 randconfig-a013-20210302 x86_64 randconfig-a016-20210302 x86_64 randconfig-a015-20210302 x86_64 randconfig-a014-20210302 x86_64 randconfig-a012-20210302 x86_64 randconfig-a011-20210302 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
