* [PATCH 0/7] Fix various compilation issues with wfx driver
@ 2019-10-08 9:42 Jerome Pouiller
2019-10-08 9:42 ` [PATCH 1/7] staging: wfx: simplify memory allocation in wfx_update_filtering() Jerome Pouiller
` (7 more replies)
0 siblings, 8 replies; 14+ messages in thread
From: Jerome Pouiller @ 2019-10-08 9:42 UTC (permalink / raw)
To: devel; +Cc: linux-kernel, Greg Kroah-Hartman, Jerome Pouiller
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
Most of problems are related to big-endian architectures.
Jérôme Pouiller (7):
staging: wfx: simplify memory allocation in wfx_update_filtering()
staging: wfx: remove misused call to cpu_to_le16()
staging: wfx: le16_to_cpus() takes a reference as parameter
staging: wfx: correctly cast data on big-endian targets
staging: wfx: fix copy_{to,from}_user() usage
staging: wfx: drop calls to BUG_ON()
staging: wfx: avoid namespace contamination
drivers/staging/wfx/bh.c | 8 +++----
drivers/staging/wfx/bus_sdio.c | 4 ++--
drivers/staging/wfx/data_tx.c | 40 ++++++++++++++++----------------
drivers/staging/wfx/data_tx.h | 2 +-
drivers/staging/wfx/debug.c | 5 ++--
drivers/staging/wfx/hif_tx_mib.h | 23 ++++++++++++------
drivers/staging/wfx/key.c | 32 ++++++++++++-------------
drivers/staging/wfx/queue.c | 6 ++---
drivers/staging/wfx/scan.c | 2 +-
drivers/staging/wfx/sta.c | 21 +++++++----------
10 files changed, 74 insertions(+), 69 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/7] staging: wfx: simplify memory allocation in wfx_update_filtering()
2019-10-08 9:42 [PATCH 0/7] Fix various compilation issues with wfx driver Jerome Pouiller
@ 2019-10-08 9:42 ` Jerome Pouiller
2019-10-08 11:59 ` Dan Carpenter
2019-10-08 9:42 ` [PATCH 2/7] staging: wfx: remove misused call to cpu_to_le16() Jerome Pouiller
` (6 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Jerome Pouiller @ 2019-10-08 9:42 UTC (permalink / raw)
To: devel
Cc: linux-kernel, Greg Kroah-Hartman, Jerome Pouiller, kbuild test robot
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
Original code did not handle case where kmalloc failed. By the way, it
is more convenient to allocate and build HIF message in
hif_set_beacon_filter_table() instead of to ask to caller function to
build it.
Fixes: 40115bbc40e2 ("staging: wfx: implement the rest of mac80211 API")
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/hif_tx_mib.h | 19 ++++++++++++++-----
drivers/staging/wfx/sta.c | 17 +++++++----------
2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/wfx/hif_tx_mib.h b/drivers/staging/wfx/hif_tx_mib.h
index f6624a403016..167c5dec009f 100644
--- a/drivers/staging/wfx/hif_tx_mib.h
+++ b/drivers/staging/wfx/hif_tx_mib.h
@@ -86,13 +86,22 @@ static inline int hif_set_rx_filter(struct wfx_vif *wvif, bool filter_bssid,
}
static inline int hif_set_beacon_filter_table(struct wfx_vif *wvif,
- struct hif_mib_bcn_filter_table *ft)
+ int tbl_len,
+ struct hif_ie_table_entry *tbl)
{
- size_t buf_len = struct_size(ft, ie_table, ft->num_of_info_elmts);
+ int ret;
+ struct hif_mib_bcn_filter_table *val;
+ int buf_len = struct_size(val, ie_table, tbl_len);
- cpu_to_le32s(&ft->num_of_info_elmts);
- return hif_write_mib(wvif->wdev, wvif->id,
- HIF_MIB_ID_BEACON_FILTER_TABLE, ft, buf_len);
+ val = kzalloc(buf_len, GFP_KERNEL);
+ if (!val)
+ return -ENOMEM;
+ val->num_of_info_elmts = cpu_to_le32(tbl_len);
+ memcpy(val->ie_table, tbl, tbl_len * sizeof(*tbl));
+ ret = hif_write_mib(wvif->wdev, wvif->id,
+ HIF_MIB_ID_BEACON_FILTER_TABLE, val, buf_len);
+ kfree(val);
+ return ret;
}
static inline int hif_beacon_filter_control(struct wfx_vif *wvif,
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 2855d14a709c..12198b8f3685 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -217,14 +217,13 @@ void wfx_update_filtering(struct wfx_vif *wvif)
bool filter_bssid = wvif->filter_bssid;
bool fwd_probe_req = wvif->fwd_probe_req;
struct hif_mib_bcn_filter_enable bf_ctrl;
- struct hif_mib_bcn_filter_table *bf_tbl;
- struct hif_ie_table_entry ie_tbl[] = {
+ struct hif_ie_table_entry filter_ies[] = {
{
.ie_id = WLAN_EID_VENDOR_SPECIFIC,
.has_changed = 1,
.no_longer = 1,
.has_appeared = 1,
- .oui = { 0x50, 0x6F, 0x9A},
+ .oui = { 0x50, 0x6F, 0x9A },
}, {
.ie_id = WLAN_EID_HT_OPERATION,
.has_changed = 1,
@@ -237,36 +236,34 @@ void wfx_update_filtering(struct wfx_vif *wvif)
.has_appeared = 1,
}
};
+ int n_filter_ies;
if (wvif->state == WFX_STATE_PASSIVE)
return;
- bf_tbl = kmalloc(sizeof(struct hif_mib_bcn_filter_table) + sizeof(ie_tbl), GFP_KERNEL);
- memcpy(bf_tbl->ie_table, ie_tbl, sizeof(ie_tbl));
if (wvif->disable_beacon_filter) {
bf_ctrl.enable = 0;
bf_ctrl.bcn_count = 1;
- bf_tbl->num_of_info_elmts = 0;
+ n_filter_ies = 0;
} else if (!is_sta) {
bf_ctrl.enable = HIF_BEACON_FILTER_ENABLE | HIF_BEACON_FILTER_AUTO_ERP;
bf_ctrl.bcn_count = 0;
- bf_tbl->num_of_info_elmts = 2;
+ n_filter_ies = 2;
} else {
bf_ctrl.enable = HIF_BEACON_FILTER_ENABLE;
bf_ctrl.bcn_count = 0;
- bf_tbl->num_of_info_elmts = 3;
+ n_filter_ies = 3;
}
ret = hif_set_rx_filter(wvif, filter_bssid, fwd_probe_req);
if (!ret)
- ret = hif_set_beacon_filter_table(wvif, bf_tbl);
+ ret = hif_set_beacon_filter_table(wvif, n_filter_ies, filter_ies);
if (!ret)
ret = hif_beacon_filter_control(wvif, bf_ctrl.enable, bf_ctrl.bcn_count);
if (!ret)
ret = wfx_set_mcast_filter(wvif, &wvif->mcast_filter);
if (ret)
dev_err(wvif->wdev->dev, "update filtering failed: %d\n", ret);
- kfree(bf_tbl);
}
void wfx_update_filtering_work(struct work_struct *work)
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/7] staging: wfx: le16_to_cpus() takes a reference as parameter
2019-10-08 9:42 [PATCH 0/7] Fix various compilation issues with wfx driver Jerome Pouiller
2019-10-08 9:42 ` [PATCH 1/7] staging: wfx: simplify memory allocation in wfx_update_filtering() Jerome Pouiller
2019-10-08 9:42 ` [PATCH 2/7] staging: wfx: remove misused call to cpu_to_le16() Jerome Pouiller
@ 2019-10-08 9:42 ` Jerome Pouiller
2019-10-08 9:43 ` [PATCH 4/7] staging: wfx: correctly cast data on big-endian targets Jerome Pouiller
` (4 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2019-10-08 9:42 UTC (permalink / raw)
To: devel
Cc: linux-kernel, Greg Kroah-Hartman, Jerome Pouiller,
kbuild test robot, Stephen Rothwell
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
Original code caused an (100% reproducible) invalid memory access on
big-endian targets.
Fixes: b0998f0c040d "staging: wfx: add IRQ handling"
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/bh.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 6000c03bb658..3715bb18bd78 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -83,12 +83,12 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
// piggyback is probably correct.
return piggyback;
}
- le16_to_cpus(hif->len);
+ le16_to_cpus(&hif->len);
computed_len = round_up(hif->len - sizeof(hif->len), 16)
+ sizeof(struct hif_sl_msg)
+ sizeof(struct hif_sl_tag);
} else {
- le16_to_cpus(hif->len);
+ le16_to_cpus(&hif->len);
computed_len = round_up(hif->len, 2);
}
if (computed_len != read_len) {
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/7] staging: wfx: remove misused call to cpu_to_le16()
2019-10-08 9:42 [PATCH 0/7] Fix various compilation issues with wfx driver Jerome Pouiller
2019-10-08 9:42 ` [PATCH 1/7] staging: wfx: simplify memory allocation in wfx_update_filtering() Jerome Pouiller
@ 2019-10-08 9:42 ` Jerome Pouiller
2019-10-08 9:42 ` [PATCH 3/7] staging: wfx: le16_to_cpus() takes a reference as parameter Jerome Pouiller
` (5 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2019-10-08 9:42 UTC (permalink / raw)
To: devel
Cc: linux-kernel, Greg Kroah-Hartman, Jerome Pouiller,
kbuild test robot, Stephen Rothwell
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
Indeed, hif_msg->id is a uint8_t, so use of cpu_to_le16() is a madness.
Fixes: 9bca45f3d692 ("staging: wfx: allow to send 802.11 frames")
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/data_tx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 7f2799fbdafe..1891bcaaf9fc 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -620,7 +620,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta, struct
memset(skb->data, 0, wmsg_len);
hif_msg = (struct hif_msg *) skb->data;
hif_msg->len = cpu_to_le16(skb->len);
- hif_msg->id = cpu_to_le16(HIF_REQ_ID_TX);
+ hif_msg->id = HIF_REQ_ID_TX;
hif_msg->interface = wvif->id;
if (skb->len > wvif->wdev->hw_caps.size_inp_ch_buf) {
dev_warn(wvif->wdev->dev, "requested frame size (%d) is larger than maximum supported (%d)\n",
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/7] staging: wfx: fix copy_{to,from}_user() usage
2019-10-08 9:42 [PATCH 0/7] Fix various compilation issues with wfx driver Jerome Pouiller
` (3 preceding siblings ...)
2019-10-08 9:43 ` [PATCH 4/7] staging: wfx: correctly cast data on big-endian targets Jerome Pouiller
@ 2019-10-08 9:43 ` Jerome Pouiller
2019-10-08 9:43 ` [PATCH 7/7] staging: wfx: avoid namespace contamination Jerome Pouiller
` (2 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2019-10-08 9:43 UTC (permalink / raw)
To: devel
Cc: linux-kernel, Greg Kroah-Hartman, Jerome Pouiller,
kbuild test robot, Dan Carpenter
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
On error, copy_to_user() returns number of bytes remaining. Driver
should return -EFAULT.
Fixes: 4f8b7fabb15d ("staging: wfx: allow to send commands to chip")
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/debug.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/wfx/debug.c b/drivers/staging/wfx/debug.c
index 3261b267c385..8de16ad7c710 100644
--- a/drivers/staging/wfx/debug.c
+++ b/drivers/staging/wfx/debug.c
@@ -256,9 +256,8 @@ static ssize_t wfx_send_hif_msg_read(struct file *file, char __user *user_buf,
return context->ret;
// Be carefull, write() is waiting for a full message while read()
// only return a payload
- ret = copy_to_user(user_buf, context->reply, count);
- if (ret)
- return ret;
+ if (copy_to_user(user_buf, context->reply, count))
+ return -EFAULT;
return count;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/7] staging: wfx: correctly cast data on big-endian targets
2019-10-08 9:42 [PATCH 0/7] Fix various compilation issues with wfx driver Jerome Pouiller
` (2 preceding siblings ...)
2019-10-08 9:42 ` [PATCH 3/7] staging: wfx: le16_to_cpus() takes a reference as parameter Jerome Pouiller
@ 2019-10-08 9:43 ` Jerome Pouiller
2019-10-08 12:01 ` Dan Carpenter
2019-10-08 9:43 ` [PATCH 5/7] staging: wfx: fix copy_{to,from}_user() usage Jerome Pouiller
` (3 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Jerome Pouiller @ 2019-10-08 9:43 UTC (permalink / raw)
To: devel
Cc: linux-kernel, Greg Kroah-Hartman, Jerome Pouiller,
kbuild test robot, Stephen Rothwell
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
When built for a big-endian target, original code caused error:
include/uapi/linux/swab.h:242:29: note: expected '__u32 * {aka unsigned int *}' but argument is of type 'struct hif_mib_protected_mgmt_policy *'
Fixes: f95a29d40782 ("staging: wfx: add HIF commands helpers")
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/hif_tx_mib.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/wfx/hif_tx_mib.h b/drivers/staging/wfx/hif_tx_mib.h
index 167c5dec009f..4f132348f5fa 100644
--- a/drivers/staging/wfx/hif_tx_mib.h
+++ b/drivers/staging/wfx/hif_tx_mib.h
@@ -145,7 +145,7 @@ static inline int hif_set_mfp(struct wfx_vif *wvif, bool capable, bool required)
}
if (!required)
val.unpmf_allowed = 1;
- cpu_to_le32s(&val);
+ cpu_to_le32s((uint32_t *) &val);
return hif_write_mib(wvif->wdev, wvif->id,
HIF_MIB_ID_PROTECTED_MGMT_POLICY,
&val, sizeof(val));
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 7/7] staging: wfx: avoid namespace contamination
2019-10-08 9:42 [PATCH 0/7] Fix various compilation issues with wfx driver Jerome Pouiller
` (4 preceding siblings ...)
2019-10-08 9:43 ` [PATCH 5/7] staging: wfx: fix copy_{to,from}_user() usage Jerome Pouiller
@ 2019-10-08 9:43 ` Jerome Pouiller
2019-10-08 9:43 ` [PATCH 6/7] staging: wfx: drop calls to BUG_ON() Jerome Pouiller
2019-10-08 15:10 ` [PATCH 0/7] Fix various compilation issues with wfx driver Greg Kroah-Hartman
7 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2019-10-08 9:43 UTC (permalink / raw)
To: devel
Cc: linux-kernel, Greg Kroah-Hartman, Jerome Pouiller, kbuild test robot
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
tx_policy_init() was already defined in driver cw1200. So, compilation
failed when wfx and cw1200 were both built-in.
In order to keep a coherent naming scheme, this patch prefixes all
"tx_policy_*" functions with "wfx_".
Fixes: 9bca45f3d692 ("staging: wfx: allow to send 802.11 frames")
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/data_tx.c | 34 +++++++++++++++++-----------------
drivers/staging/wfx/data_tx.h | 2 +-
drivers/staging/wfx/sta.c | 2 +-
3 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index b2ca3986c6d0..6e4dd4ac5544 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -37,7 +37,7 @@ static int wfx_get_hw_rate(struct wfx_dev *wdev, const struct ieee80211_tx_rate
/* TX policy cache implementation */
-static void tx_policy_build(struct wfx_vif *wvif, struct tx_policy *policy,
+static void wfx_tx_policy_build(struct wfx_vif *wvif, struct tx_policy *policy,
struct ieee80211_tx_rate *rates)
{
int i;
@@ -124,7 +124,7 @@ static bool tx_policy_is_equal(const struct tx_policy *a, const struct tx_policy
return !memcmp(a->rates, b->rates, sizeof(a->rates));
}
-static int tx_policy_find(struct tx_policy_cache *cache, struct tx_policy *wanted)
+static int wfx_tx_policy_find(struct tx_policy_cache *cache, struct tx_policy *wanted)
{
struct tx_policy *it;
@@ -137,13 +137,13 @@ static int tx_policy_find(struct tx_policy_cache *cache, struct tx_policy *wante
return -1;
}
-static void tx_policy_use(struct tx_policy_cache *cache, struct tx_policy *entry)
+static void wfx_tx_policy_use(struct tx_policy_cache *cache, struct tx_policy *entry)
{
++entry->usage_count;
list_move(&entry->link, &cache->used);
}
-static int tx_policy_release(struct tx_policy_cache *cache, struct tx_policy *entry)
+static int wfx_tx_policy_release(struct tx_policy_cache *cache, struct tx_policy *entry)
{
int ret = --entry->usage_count;
@@ -152,21 +152,21 @@ static int tx_policy_release(struct tx_policy_cache *cache, struct tx_policy *en
return ret;
}
-static int tx_policy_get(struct wfx_vif *wvif, struct ieee80211_tx_rate *rates,
+static int wfx_tx_policy_get(struct wfx_vif *wvif, struct ieee80211_tx_rate *rates,
bool *renew)
{
int idx;
struct tx_policy_cache *cache = &wvif->tx_policy_cache;
struct tx_policy wanted;
- tx_policy_build(wvif, &wanted, rates);
+ wfx_tx_policy_build(wvif, &wanted, rates);
spin_lock_bh(&cache->lock);
if (WARN_ON(list_empty(&cache->free))) {
spin_unlock_bh(&cache->lock);
return WFX_INVALID_RATE_ID;
}
- idx = tx_policy_find(cache, &wanted);
+ idx = wfx_tx_policy_find(cache, &wanted);
if (idx >= 0) {
*renew = false;
} else {
@@ -181,7 +181,7 @@ static int tx_policy_get(struct wfx_vif *wvif, struct ieee80211_tx_rate *rates,
entry->usage_count = 0;
idx = entry - cache->cache;
}
- tx_policy_use(cache, &cache->cache[idx]);
+ wfx_tx_policy_use(cache, &cache->cache[idx]);
if (list_empty(&cache->free)) {
/* Lock TX queues. */
wfx_tx_queues_lock(wvif->wdev);
@@ -190,14 +190,14 @@ static int tx_policy_get(struct wfx_vif *wvif, struct ieee80211_tx_rate *rates,
return idx;
}
-static void tx_policy_put(struct wfx_vif *wvif, int idx)
+static void wfx_tx_policy_put(struct wfx_vif *wvif, int idx)
{
int usage, locked;
struct tx_policy_cache *cache = &wvif->tx_policy_cache;
spin_lock_bh(&cache->lock);
locked = list_empty(&cache->free);
- usage = tx_policy_release(cache, &cache->cache[idx]);
+ usage = wfx_tx_policy_release(cache, &cache->cache[idx]);
if (locked && !usage) {
/* Unlock TX queues. */
wfx_tx_queues_unlock(wvif->wdev);
@@ -205,7 +205,7 @@ static void tx_policy_put(struct wfx_vif *wvif, int idx)
spin_unlock_bh(&cache->lock);
}
-static int tx_policy_upload(struct wfx_vif *wvif)
+static int wfx_tx_policy_upload(struct wfx_vif *wvif)
{
int i;
struct tx_policy_cache *cache = &wvif->tx_policy_cache;
@@ -238,18 +238,18 @@ static int tx_policy_upload(struct wfx_vif *wvif)
return 0;
}
-static void tx_policy_upload_work(struct work_struct *work)
+static void wfx_tx_policy_upload_work(struct work_struct *work)
{
struct wfx_vif *wvif =
container_of(work, struct wfx_vif, tx_policy_upload_work);
- tx_policy_upload(wvif);
+ wfx_tx_policy_upload(wvif);
wfx_tx_unlock(wvif->wdev);
wfx_tx_queues_unlock(wvif->wdev);
}
-void tx_policy_init(struct wfx_vif *wvif)
+void wfx_tx_policy_init(struct wfx_vif *wvif)
{
struct tx_policy_cache *cache = &wvif->tx_policy_cache;
int i;
@@ -259,7 +259,7 @@ void tx_policy_init(struct wfx_vif *wvif)
spin_lock_init(&cache->lock);
INIT_LIST_HEAD(&cache->used);
INIT_LIST_HEAD(&cache->free);
- INIT_WORK(&wvif->tx_policy_upload_work, tx_policy_upload_work);
+ INIT_WORK(&wvif->tx_policy_upload_work, wfx_tx_policy_upload_work);
for (i = 0; i < HIF_MIB_NUM_TX_RATE_RETRY_POLICIES; ++i)
list_add(&cache->cache[i].link, &cache->free);
@@ -527,7 +527,7 @@ static uint8_t wfx_tx_get_rate_id(struct wfx_vif *wvif, struct ieee80211_tx_info
bool tx_policy_renew = false;
uint8_t rate_id;
- rate_id = tx_policy_get(wvif, tx_info->driver_rates, &tx_policy_renew);
+ rate_id = wfx_tx_policy_get(wvif, tx_info->driver_rates, &tx_policy_renew);
WARN(rate_id == WFX_INVALID_RATE_ID, "unable to get a valid Tx policy");
if (tx_policy_renew) {
@@ -794,6 +794,6 @@ void wfx_skb_dtor(struct wfx_dev *wdev, struct sk_buff *skb)
WARN_ON(!wvif);
skb_pull(skb, offset);
wfx_notify_buffered_tx(wvif, skb, req);
- tx_policy_put(wvif, req->tx_flags.retry_policy_index);
+ wfx_tx_policy_put(wvif, req->tx_flags.retry_policy_index);
ieee80211_tx_status_irqsafe(wdev->hw, skb);
}
diff --git a/drivers/staging/wfx/data_tx.h b/drivers/staging/wfx/data_tx.h
index f59a259bb744..0a19ef10a4ab 100644
--- a/drivers/staging/wfx/data_tx.h
+++ b/drivers/staging/wfx/data_tx.h
@@ -60,7 +60,7 @@ struct wfx_tx_priv {
uint8_t tid;
} __packed;
-void tx_policy_init(struct wfx_vif *wvif);
+void wfx_tx_policy_init(struct wfx_vif *wvif);
void wfx_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control *control,
struct sk_buff *skb);
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 733b93a8f830..3c715cc88ab2 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -1534,7 +1534,7 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
}
wfx_set_uapsd_param(wvif, &wvif->edca);
- tx_policy_init(wvif);
+ wfx_tx_policy_init(wvif);
wvif = NULL;
while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
// Combo mode does not support Block Acks. We can re-enable them
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 6/7] staging: wfx: drop calls to BUG_ON()
2019-10-08 9:42 [PATCH 0/7] Fix various compilation issues with wfx driver Jerome Pouiller
` (5 preceding siblings ...)
2019-10-08 9:43 ` [PATCH 7/7] staging: wfx: avoid namespace contamination Jerome Pouiller
@ 2019-10-08 9:43 ` Jerome Pouiller
2019-10-08 12:07 ` Dan Carpenter
2019-10-08 15:10 ` [PATCH 0/7] Fix various compilation issues with wfx driver Greg Kroah-Hartman
7 siblings, 1 reply; 14+ messages in thread
From: Jerome Pouiller @ 2019-10-08 9:43 UTC (permalink / raw)
To: devel; +Cc: linux-kernel, Greg Kroah-Hartman, Jerome Pouiller, Andrew Lunn
From: Jérôme Pouiller <jerome.pouiller@silabs.com>
Most of calls to BUG_ON() could replaced by WARN().
By the way, this patch also try to favor WARN() (that include a comment
about the problem) instead of WARN_ON().
Reported-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
drivers/staging/wfx/bh.c | 4 ++--
drivers/staging/wfx/bus_sdio.c | 4 ++--
drivers/staging/wfx/data_tx.c | 4 ++--
drivers/staging/wfx/hif_tx_mib.h | 2 +-
drivers/staging/wfx/key.c | 32 ++++++++++++++++----------------
drivers/staging/wfx/queue.c | 6 +++---
drivers/staging/wfx/scan.c | 2 +-
drivers/staging/wfx/sta.c | 2 +-
8 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 3715bb18bd78..3355183fc86c 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -56,7 +56,7 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
int release_count;
int piggyback = 0;
- WARN_ON(read_len < 4);
+ WARN(read_len < 4, "corrupted read");
WARN(read_len > round_down(0xFFF, 2) * sizeof(u16),
"%s: request exceed WFx capability", __func__);
@@ -173,7 +173,7 @@ static void tx_helper(struct wfx_dev *wdev, struct hif_msg *hif)
bool is_encrypted = false;
size_t len = le16_to_cpu(hif->len);
- BUG_ON(len < sizeof(*hif));
+ WARN(len < sizeof(*hif), "try to send corrupted data");
hif->seqnum = wdev->hif.tx_seqnum;
wdev->hif.tx_seqnum = (wdev->hif.tx_seqnum + 1) % (HIF_COUNTER_MAX + 1);
diff --git a/drivers/staging/wfx/bus_sdio.c b/drivers/staging/wfx/bus_sdio.c
index 05f02c278782..f97360513150 100644
--- a/drivers/staging/wfx/bus_sdio.c
+++ b/drivers/staging/wfx/bus_sdio.c
@@ -37,7 +37,7 @@ static int wfx_sdio_copy_from_io(void *priv, unsigned int reg_id,
unsigned int sdio_addr = reg_id << 2;
int ret;
- BUG_ON(reg_id > 7);
+ WARN(reg_id > 7, "chip only has 7 registers");
WARN(((uintptr_t) dst) & 3, "unaligned buffer size");
WARN(count & 3, "unaligned buffer address");
@@ -58,7 +58,7 @@ static int wfx_sdio_copy_to_io(void *priv, unsigned int reg_id,
unsigned int sdio_addr = reg_id << 2;
int ret;
- BUG_ON(reg_id > 7);
+ WARN(reg_id > 7, "chip only has 7 registers");
WARN(((uintptr_t) src) & 3, "unaligned buffer size");
WARN(count & 3, "unaligned buffer address");
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 1891bcaaf9fc..b2ca3986c6d0 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -44,7 +44,7 @@ static void tx_policy_build(struct wfx_vif *wvif, struct tx_policy *policy,
size_t count;
struct wfx_dev *wdev = wvif->wdev;
- BUG_ON(rates[0].idx < 0);
+ WARN(rates[0].idx < 0, "invalid rate policy");
memset(policy, 0, sizeof(*policy));
for (i = 1; i < IEEE80211_TX_MAX_RATES; i++)
if (rates[i].idx < 0)
@@ -162,7 +162,7 @@ static int tx_policy_get(struct wfx_vif *wvif, struct ieee80211_tx_rate *rates,
tx_policy_build(wvif, &wanted, rates);
spin_lock_bh(&cache->lock);
- if (WARN_ON_ONCE(list_empty(&cache->free))) {
+ if (WARN_ON(list_empty(&cache->free))) {
spin_unlock_bh(&cache->lock);
return WFX_INVALID_RATE_ID;
}
diff --git a/drivers/staging/wfx/hif_tx_mib.h b/drivers/staging/wfx/hif_tx_mib.h
index 4f132348f5fa..3339ad95f732 100644
--- a/drivers/staging/wfx/hif_tx_mib.h
+++ b/drivers/staging/wfx/hif_tx_mib.h
@@ -138,7 +138,7 @@ static inline int hif_set_mfp(struct wfx_vif *wvif, bool capable, bool required)
{
struct hif_mib_protected_mgmt_policy val = { };
- WARN_ON(required && !capable);
+ WARN(required && !capable, "incoherent arguments");
if (capable) {
val.pmf_enable = 1;
val.host_enc_auth_frames = 1;
diff --git a/drivers/staging/wfx/key.c b/drivers/staging/wfx/key.c
index 4e7d2b510a9c..6d03abec20e4 100644
--- a/drivers/staging/wfx/key.c
+++ b/drivers/staging/wfx/key.c
@@ -26,7 +26,7 @@ static int wfx_alloc_key(struct wfx_dev *wdev)
static void wfx_free_key(struct wfx_dev *wdev, int idx)
{
- BUG_ON(!(wdev->key_map & BIT(idx)));
+ WARN(!(wdev->key_map & BIT(idx)), "inconsistent key allocation");
memset(&wdev->keys[idx], 0, sizeof(wdev->keys[idx]));
wdev->key_map &= ~BIT(idx);
}
@@ -34,7 +34,7 @@ static void wfx_free_key(struct wfx_dev *wdev, int idx)
static uint8_t fill_wep_pair(struct hif_wep_pairwise_key *msg,
struct ieee80211_key_conf *key, u8 *peer_addr)
{
- WARN_ON(key->keylen > sizeof(msg->key_data));
+ WARN(key->keylen > sizeof(msg->key_data), "inconsistent data");
msg->key_length = key->keylen;
memcpy(msg->key_data, key->key, key->keylen);
ether_addr_copy(msg->peer_address, peer_addr);
@@ -44,7 +44,7 @@ static uint8_t fill_wep_pair(struct hif_wep_pairwise_key *msg,
static uint8_t fill_wep_group(struct hif_wep_group_key *msg,
struct ieee80211_key_conf *key)
{
- WARN_ON(key->keylen > sizeof(msg->key_data));
+ WARN(key->keylen > sizeof(msg->key_data), "inconsistent data");
msg->key_id = key->keyidx;
msg->key_length = key->keylen;
memcpy(msg->key_data, key->key, key->keylen);
@@ -56,9 +56,9 @@ static uint8_t fill_tkip_pair(struct hif_tkip_pairwise_key *msg,
{
uint8_t *keybuf = key->key;
- WARN_ON(key->keylen != sizeof(msg->tkip_key_data)
- + sizeof(msg->tx_mic_key)
- + sizeof(msg->rx_mic_key));
+ WARN(key->keylen != sizeof(msg->tkip_key_data)
+ + sizeof(msg->tx_mic_key)
+ + sizeof(msg->rx_mic_key), "inconsistent data");
memcpy(msg->tkip_key_data, keybuf, sizeof(msg->tkip_key_data));
keybuf += sizeof(msg->tkip_key_data);
memcpy(msg->tx_mic_key, keybuf, sizeof(msg->tx_mic_key));
@@ -75,8 +75,8 @@ static uint8_t fill_tkip_group(struct hif_tkip_group_key *msg,
{
uint8_t *keybuf = key->key;
- WARN_ON(key->keylen != sizeof(msg->tkip_key_data)
- + 2 * sizeof(msg->rx_mic_key));
+ WARN(key->keylen != sizeof(msg->tkip_key_data)
+ + 2 * sizeof(msg->rx_mic_key), "inconsistent data");
msg->key_id = key->keyidx;
memcpy(msg->rx_sequence_counter, &seq->tkip.iv16, sizeof(seq->tkip.iv16));
memcpy(msg->rx_sequence_counter + sizeof(uint16_t), &seq->tkip.iv32, sizeof(seq->tkip.iv32));
@@ -94,7 +94,7 @@ static uint8_t fill_tkip_group(struct hif_tkip_group_key *msg,
static uint8_t fill_ccmp_pair(struct hif_aes_pairwise_key *msg,
struct ieee80211_key_conf *key, u8 *peer_addr)
{
- WARN_ON(key->keylen != sizeof(msg->aes_key_data));
+ WARN(key->keylen != sizeof(msg->aes_key_data), "inconsistent data");
ether_addr_copy(msg->peer_address, peer_addr);
memcpy(msg->aes_key_data, key->key, key->keylen);
return HIF_KEY_TYPE_AES_PAIRWISE;
@@ -104,7 +104,7 @@ static uint8_t fill_ccmp_group(struct hif_aes_group_key *msg,
struct ieee80211_key_conf *key,
struct ieee80211_key_seq *seq)
{
- WARN_ON(key->keylen != sizeof(msg->aes_key_data));
+ WARN(key->keylen != sizeof(msg->aes_key_data), "inconsistent data");
memcpy(msg->aes_key_data, key->key, key->keylen);
memcpy(msg->rx_sequence_counter, seq->ccmp.pn, sizeof(seq->ccmp.pn));
memreverse(msg->rx_sequence_counter, sizeof(seq->ccmp.pn));
@@ -117,8 +117,8 @@ static uint8_t fill_sms4_pair(struct hif_wapi_pairwise_key *msg,
{
uint8_t *keybuf = key->key;
- WARN_ON(key->keylen != sizeof(msg->wapi_key_data)
- + sizeof(msg->mic_key_data));
+ WARN(key->keylen != sizeof(msg->wapi_key_data)
+ + sizeof(msg->mic_key_data), "inconsistent data");
ether_addr_copy(msg->peer_address, peer_addr);
memcpy(msg->wapi_key_data, keybuf, sizeof(msg->wapi_key_data));
keybuf += sizeof(msg->wapi_key_data);
@@ -132,8 +132,8 @@ static uint8_t fill_sms4_group(struct hif_wapi_group_key *msg,
{
uint8_t *keybuf = key->key;
- WARN_ON(key->keylen != sizeof(msg->wapi_key_data)
- + sizeof(msg->mic_key_data));
+ WARN(key->keylen != sizeof(msg->wapi_key_data)
+ + sizeof(msg->mic_key_data), "inconsistent data");
memcpy(msg->wapi_key_data, keybuf, sizeof(msg->wapi_key_data));
keybuf += sizeof(msg->wapi_key_data);
memcpy(msg->mic_key_data, keybuf, sizeof(msg->mic_key_data));
@@ -145,7 +145,7 @@ static uint8_t fill_aes_cmac_group(struct hif_igtk_group_key *msg,
struct ieee80211_key_conf *key,
struct ieee80211_key_seq *seq)
{
- WARN_ON(key->keylen != sizeof(msg->igtk_key_data));
+ WARN(key->keylen != sizeof(msg->igtk_key_data), "inconsistent data");
memcpy(msg->igtk_key_data, key->key, key->keylen);
memcpy(msg->ipn, seq->aes_cmac.pn, sizeof(seq->aes_cmac.pn));
memreverse(msg->ipn, sizeof(seq->aes_cmac.pn));
@@ -163,7 +163,7 @@ static int wfx_add_key(struct wfx_vif *wvif, struct ieee80211_sta *sta,
int idx = wfx_alloc_key(wvif->wdev);
bool pairwise = key->flags & IEEE80211_KEY_FLAG_PAIRWISE;
- WARN_ON(key->flags & IEEE80211_KEY_FLAG_PAIRWISE && !sta);
+ WARN(key->flags & IEEE80211_KEY_FLAG_PAIRWISE && !sta, "inconsistent data");
ieee80211_get_key_rx_seq(key, 0, &seq);
if (idx < 0)
return -EINVAL;
diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 6f1be4f6f463..ee9b2c3fde5a 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -78,7 +78,7 @@ void wfx_tx_queues_unlock(struct wfx_dev *wdev)
for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
queue = &wdev->tx_queue[i];
spin_lock_bh(&queue->queue.lock);
- BUG_ON(!queue->tx_locked_cnt);
+ WARN(!queue->tx_locked_cnt, "queue already unlocked");
if (--queue->tx_locked_cnt == 0)
ieee80211_wake_queue(wdev->hw, queue->queue_id);
spin_unlock_bh(&queue->queue.lock);
@@ -295,8 +295,8 @@ struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id)
return skb;
}
}
- WARN_ON(1);
spin_unlock_bh(&stats->pending.lock);
+ WARN(1, "cannot find packet in pending queue");
return NULL;
}
@@ -408,7 +408,7 @@ static bool hif_handle_tx_data(struct wfx_vif *wvif, struct sk_buff *skb,
switch (action) {
case do_drop:
- BUG_ON(wfx_pending_remove(wvif->wdev, skb));
+ wfx_pending_remove(wvif->wdev, skb);
handled = true;
break;
case do_wep:
diff --git a/drivers/staging/wfx/scan.c b/drivers/staging/wfx/scan.c
index ea5001c915f6..cba735c1e73c 100644
--- a/drivers/staging/wfx/scan.c
+++ b/drivers/staging/wfx/scan.c
@@ -107,7 +107,7 @@ int wfx_hw_scan(struct ieee80211_hw *hw,
wfx_tx_lock_flush(wdev);
- BUG_ON(wvif->scan.req);
+ WARN(wvif->scan.req, "unexpected concurrent scan");
wvif->scan.req = req;
wvif->scan.n_ssids = 0;
wvif->scan.status = 0;
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 12198b8f3685..733b93a8f830 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -1454,6 +1454,7 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
},
};
+ BUILD_BUG_ON(ARRAY_SIZE(default_edca_params) != ARRAY_SIZE(wvif->edca.params));
if (wfx_api_older_than(wdev, 2, 0)) {
default_edca_params[IEEE80211_AC_BE].queue_id = HIF_QUEUE_ID_BACKGROUND;
default_edca_params[IEEE80211_AC_BK].queue_id = HIF_QUEUE_ID_BESTEFFORT;
@@ -1526,7 +1527,6 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
mutex_unlock(&wdev->conf_mutex);
hif_set_macaddr(wvif, vif->addr);
- BUG_ON(ARRAY_SIZE(default_edca_params) != ARRAY_SIZE(wvif->edca.params));
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
memcpy(&wvif->edca.params[i], &default_edca_params[i], sizeof(default_edca_params[i]));
wvif->edca.uapsd_enable[i] = false;
--
2.20.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/7] staging: wfx: simplify memory allocation in wfx_update_filtering()
2019-10-08 9:42 ` [PATCH 1/7] staging: wfx: simplify memory allocation in wfx_update_filtering() Jerome Pouiller
@ 2019-10-08 11:59 ` Dan Carpenter
0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2019-10-08 11:59 UTC (permalink / raw)
To: Jerome Pouiller; +Cc: devel, Greg Kroah-Hartman, linux-kernel
These patches are good. I just have a few nits to point out for future
reference.
On Tue, Oct 08, 2019 at 09:42:58AM +0000, Jerome Pouiller wrote:
> static inline int hif_set_beacon_filter_table(struct wfx_vif *wvif,
> - struct hif_mib_bcn_filter_table *ft)
> + int tbl_len,
> + struct hif_ie_table_entry *tbl)
> {
> - size_t buf_len = struct_size(ft, ie_table, ft->num_of_info_elmts);
> + int ret;
> + struct hif_mib_bcn_filter_table *val;
> + int buf_len = struct_size(val, ie_table, tbl_len);
This is fine for now, but since this is networking code, in the future
could you write your declarations in reverse Christmas tree order?
long laskdfjasldfj asldfkj aslkdfj alskdfj askldfj;
mid asdfasdflkajdflasjdf lkasjdf;
short asdfasd;
shortest i;
>
> - cpu_to_le32s(&ft->num_of_info_elmts);
> - return hif_write_mib(wvif->wdev, wvif->id,
> - HIF_MIB_ID_BEACON_FILTER_TABLE, ft, buf_len);
[ snip ]
> diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
> index 2855d14a709c..12198b8f3685 100644
> --- a/drivers/staging/wfx/sta.c
> +++ b/drivers/staging/wfx/sta.c
> @@ -217,14 +217,13 @@ void wfx_update_filtering(struct wfx_vif *wvif)
> bool filter_bssid = wvif->filter_bssid;
> bool fwd_probe_req = wvif->fwd_probe_req;
> struct hif_mib_bcn_filter_enable bf_ctrl;
> - struct hif_mib_bcn_filter_table *bf_tbl;
> - struct hif_ie_table_entry ie_tbl[] = {
> + struct hif_ie_table_entry filter_ies[] = {
> {
> .ie_id = WLAN_EID_VENDOR_SPECIFIC,
> .has_changed = 1,
> .no_longer = 1,
> .has_appeared = 1,
> - .oui = { 0x50, 0x6F, 0x9A},
> + .oui = { 0x50, 0x6F, 0x9A },
Please don't do unrelated white space changes in their own patches.
> }, {
> .ie_id = WLAN_EID_HT_OPERATION,
> .has_changed = 1,
regards,
dan carpenter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/7] staging: wfx: correctly cast data on big-endian targets
2019-10-08 9:43 ` [PATCH 4/7] staging: wfx: correctly cast data on big-endian targets Jerome Pouiller
@ 2019-10-08 12:01 ` Dan Carpenter
0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2019-10-08 12:01 UTC (permalink / raw)
To: Jerome Pouiller; +Cc: devel, Greg Kroah-Hartman, Stephen Rothwell, linux-kernel
On Tue, Oct 08, 2019 at 09:43:00AM +0000, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller@silabs.com>
>
> When built for a big-endian target, original code caused error:
>
> include/uapi/linux/swab.h:242:29: note: expected '__u32 * {aka unsigned int *}' but argument is of type 'struct hif_mib_protected_mgmt_policy *'
>
> Fixes: f95a29d40782 ("staging: wfx: add HIF commands helpers")
> Reported-by: kbuild test robot <lkp@intel.com>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> ---
> drivers/staging/wfx/hif_tx_mib.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/wfx/hif_tx_mib.h b/drivers/staging/wfx/hif_tx_mib.h
> index 167c5dec009f..4f132348f5fa 100644
> --- a/drivers/staging/wfx/hif_tx_mib.h
> +++ b/drivers/staging/wfx/hif_tx_mib.h
> @@ -145,7 +145,7 @@ static inline int hif_set_mfp(struct wfx_vif *wvif, bool capable, bool required)
> }
> if (!required)
> val.unpmf_allowed = 1;
> - cpu_to_le32s(&val);
> + cpu_to_le32s((uint32_t *) &val);
Again, this is fine for now, but in the future there shouldn't be a
space after the cast. It's to mark that it's a high precedence
operation.
cpu_to_le32s((uint32_t *)&val);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 6/7] staging: wfx: drop calls to BUG_ON()
2019-10-08 9:43 ` [PATCH 6/7] staging: wfx: drop calls to BUG_ON() Jerome Pouiller
@ 2019-10-08 12:07 ` Dan Carpenter
0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2019-10-08 12:07 UTC (permalink / raw)
To: Jerome Pouiller; +Cc: devel, Greg Kroah-Hartman, Andrew Lunn, linux-kernel
On Tue, Oct 08, 2019 at 09:43:01AM +0000, Jerome Pouiller wrote:
> @@ -56,9 +56,9 @@ static uint8_t fill_tkip_pair(struct hif_tkip_pairwise_key *msg,
> {
> uint8_t *keybuf = key->key;
>
> - WARN_ON(key->keylen != sizeof(msg->tkip_key_data)
> - + sizeof(msg->tx_mic_key)
> - + sizeof(msg->rx_mic_key));
> + WARN(key->keylen != sizeof(msg->tkip_key_data)
> + + sizeof(msg->tx_mic_key)
> + + sizeof(msg->rx_mic_key), "inconsistent data");
This is not a comment on the patch since the code was like that
originally, but the " +" should go of the first line:
WARN(key->keylen != sizeof(msg->tkip_key_data) +
sizeof(msg->tx_mic_key) +
sizeof(msg->rx_mic_key),
"inconsistent data");
That doesn't look too good still... The error message is sort of
rubbish also. Anyway the operator goes on the first line.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/7] Fix various compilation issues with wfx driver
2019-10-08 9:42 [PATCH 0/7] Fix various compilation issues with wfx driver Jerome Pouiller
` (6 preceding siblings ...)
2019-10-08 9:43 ` [PATCH 6/7] staging: wfx: drop calls to BUG_ON() Jerome Pouiller
@ 2019-10-08 15:10 ` Greg Kroah-Hartman
2019-10-09 15:13 ` Jerome Pouiller
7 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2019-10-08 15:10 UTC (permalink / raw)
To: Jerome Pouiller; +Cc: devel, linux-kernel
On Tue, Oct 08, 2019 at 09:42:47AM +0000, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller@silabs.com>
>
> Most of problems are related to big-endian architectures.
kbuild still reports 2 errors with these patches applied:
Regressions in current branch:
drivers/staging/wfx/hif_tx.c:82:2-8: preceding lock on line 65
drivers/staging/wfx/main.c:188:14-21: ERROR: PTR_ERR applied after initialization to constant on line 183
Can you please fix those up as well?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/7] Fix various compilation issues with wfx driver
2019-10-08 15:10 ` [PATCH 0/7] Fix various compilation issues with wfx driver Greg Kroah-Hartman
@ 2019-10-09 15:13 ` Jerome Pouiller
2019-10-09 18:58 ` Dan Carpenter
0 siblings, 1 reply; 14+ messages in thread
From: Jerome Pouiller @ 2019-10-09 15:13 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: devel, linux-kernel
On Tuesday 8 October 2019 17:10:56 CEST Greg Kroah-Hartman wrote:
> On Tue, Oct 08, 2019 at 09:42:47AM +0000, Jerome Pouiller wrote:
> > From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> >
> > Most of problems are related to big-endian architectures.
>
> kbuild still reports 2 errors with these patches applied:
>
> Regressions in current branch:
>
> drivers/staging/wfx/hif_tx.c:82:2-8: preceding lock on line 65
As I replied to Julia, this behavior is intended.
> drivers/staging/wfx/main.c:188:14-21: ERROR: PTR_ERR applied after initialization to constant on line 183
This is a false positive, as confirmed by Dan.
You may also notice:
drivers/staging/wfx/scan.c:207 wfx_scan_work() warn: inconsistent returns 'sem:&wvif->scan.lock'
I also consider it as a false positive.
> Can you please fix those up as well?
Beside these ones, I will address the other reported problems.
--
Jérôme Pouiller
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/7] Fix various compilation issues with wfx driver
2019-10-09 15:13 ` Jerome Pouiller
@ 2019-10-09 18:58 ` Dan Carpenter
0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2019-10-09 18:58 UTC (permalink / raw)
To: Jerome Pouiller; +Cc: Greg Kroah-Hartman, devel, linux-kernel
On Wed, Oct 09, 2019 at 03:13:14PM +0000, Jerome Pouiller wrote:
> On Tuesday 8 October 2019 17:10:56 CEST Greg Kroah-Hartman wrote:
> > On Tue, Oct 08, 2019 at 09:42:47AM +0000, Jerome Pouiller wrote:
> > > From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> > >
> > > Most of problems are related to big-endian architectures.
> >
> > kbuild still reports 2 errors with these patches applied:
> >
> > Regressions in current branch:
> >
> > drivers/staging/wfx/hif_tx.c:82:2-8: preceding lock on line 65
>
> As I replied to Julia, this behavior is intended.
>
> > drivers/staging/wfx/main.c:188:14-21: ERROR: PTR_ERR applied after initialization to constant on line 183
>
> This is a false positive, as confirmed by Dan.
>
> You may also notice:
>
> drivers/staging/wfx/scan.c:207 wfx_scan_work() warn: inconsistent returns 'sem:&wvif->scan.lock'
>
> I also consider it as a false positive.
Yeah. I thought it might be. The beauty of 0day bot is that normally
the warnings come really quick after the original author wrote the code
so it's fresh in their heads. I suspected it might be a false positive
but I wasn't sure either way and I try not to spend a lot of time
reviewing those warnings.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-10-09 18:58 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-08 9:42 [PATCH 0/7] Fix various compilation issues with wfx driver Jerome Pouiller
2019-10-08 9:42 ` [PATCH 1/7] staging: wfx: simplify memory allocation in wfx_update_filtering() Jerome Pouiller
2019-10-08 11:59 ` Dan Carpenter
2019-10-08 9:42 ` [PATCH 2/7] staging: wfx: remove misused call to cpu_to_le16() Jerome Pouiller
2019-10-08 9:42 ` [PATCH 3/7] staging: wfx: le16_to_cpus() takes a reference as parameter Jerome Pouiller
2019-10-08 9:43 ` [PATCH 4/7] staging: wfx: correctly cast data on big-endian targets Jerome Pouiller
2019-10-08 12:01 ` Dan Carpenter
2019-10-08 9:43 ` [PATCH 5/7] staging: wfx: fix copy_{to,from}_user() usage Jerome Pouiller
2019-10-08 9:43 ` [PATCH 7/7] staging: wfx: avoid namespace contamination Jerome Pouiller
2019-10-08 9:43 ` [PATCH 6/7] staging: wfx: drop calls to BUG_ON() Jerome Pouiller
2019-10-08 12:07 ` Dan Carpenter
2019-10-08 15:10 ` [PATCH 0/7] Fix various compilation issues with wfx driver Greg Kroah-Hartman
2019-10-09 15:13 ` Jerome Pouiller
2019-10-09 18:58 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).