* [PATCH] staging: wfx: refactor to avoid duplication at hif_tx.c
@ 2020-08-05 8:56 Tomer Samara
2020-08-05 9:04 ` Greg KH
2020-08-05 11:24 ` kernel test robot
0 siblings, 2 replies; 4+ messages in thread
From: Tomer Samara @ 2020-08-05 8:56 UTC (permalink / raw)
To: jerome.pouiller; +Cc: gregkh, devel, linux-kernel
Add functions wfx_full_send(), wfx_full_send_no_reply_async(),
wfx_full_send_no_reply() and wfx_full_send_no_reply_free()
which works as follow:
wfx_full_send() - simple wrapper for both wfx_fill_header()
and wfx_cmd_send().
wfx_full_send_no_reply_async() - wrapper for both but with
NULL as reply and size zero.
wfx_full_send_no_reply() - same as wfx_full_send_no_reply_async()
but with false async value
wfx_full_send_no_reply_free() - same as wfx_full_send_no_reply()
but also free the struct hif_msg.
Signed-off-by: Tomer Samara <tomersamara98@gmail.com>
---
drivers/staging/wfx/hif_tx.c | 179 ++++++++++++++++-------------------
1 file changed, 79 insertions(+), 100 deletions(-)
diff --git a/drivers/staging/wfx/hif_tx.c b/drivers/staging/wfx/hif_tx.c
index 5110f9b93762..1ee84e5d47ef 100644
--- a/drivers/staging/wfx/hif_tx.c
+++ b/drivers/staging/wfx/hif_tx.c
@@ -40,7 +40,7 @@ static void wfx_fill_header(struct hif_msg *hif, int if_id,
static void *wfx_alloc_hif(size_t body_len, struct hif_msg **hif)
{
- *hif = kzalloc(sizeof(struct hif_msg) + body_len, GFP_KERNEL);
+ *hif = kzalloc(sizeof(*hif) + body_len, GFP_KERNEL);
if (*hif)
return (*hif)->body;
else
@@ -123,9 +123,38 @@ int wfx_cmd_send(struct wfx_dev *wdev, struct hif_msg *request,
return ret;
}
+int wfx_full_send(struct wfx_dev *wdev, struct hif_msg *hif, void *reply, size_t reply_len,
+ bool async, int if_id, unsigned int cmd, int size)
+{
+ wfx_fill_header(hif, if_id, cmd, size);
+ return wfx_cmd_send(wdev, hif, reply, reply_len, async);
+}
+
+int wfx_full_send_no_reply_async(struct wfx_dev *wdev, struct hif_msg *hif, int if_id,
+ unsigned int cmd, int size, bool async)
+{
+ return wfx_full_send(wdev, hif, NULL, 0, async, if_id, cmd, size);
+}
+
+int wfx_full_send_no_reply(struct wfx_dev *wdev, struct hif_msg *hif, int if_id,
+ unsigned int cmd, int size)
+{
+ return wfx_full_send_no_reply_async(wdev, hif, if_id, cmd, size, false);
+}
+
+int wfx_full_send_no_reply_free(struct wfx_dev *wdev, struct hif_msg *hif, int if_id,
+ unsigned int cmd, int size)
+{
+ int ret;
+
+ ret = wfx_full_send_no_reply(wdev, hif, if_id, cmd, size);
+ kfree(hif);
+ return ret;
+}
+
// This function is special. After HIF_REQ_ID_SHUT_DOWN, chip won't reply to any
// request anymore. We need to slightly hack struct wfx_hif_cmd for that job. Be
-// carefull to only call this funcion during device unregister.
+// careful to only call this function during device unregister.
int hif_shutdown(struct wfx_dev *wdev)
{
int ret;
@@ -136,8 +165,8 @@ int hif_shutdown(struct wfx_dev *wdev)
wfx_alloc_hif(0, &hif);
if (!hif)
return -ENOMEM;
- wfx_fill_header(hif, -1, HIF_REQ_ID_SHUT_DOWN, 0);
- ret = wfx_cmd_send(wdev, hif, NULL, 0, true);
+ ret = wfx_full_send_no_reply_async(wdev, hif, -1, HIF_REQ_ID_SHUT_DOWN,
+ 0, true);
// After this command, chip won't reply. Be sure to give enough time to
// bh to send buffer:
msleep(100);
@@ -154,7 +183,6 @@ int hif_shutdown(struct wfx_dev *wdev)
int hif_configuration(struct wfx_dev *wdev, const u8 *conf, size_t len)
{
- int ret;
size_t buf_len = sizeof(struct hif_req_configuration) + len;
struct hif_msg *hif;
struct hif_req_configuration *body = wfx_alloc_hif(buf_len, &hif);
@@ -163,25 +191,20 @@ int hif_configuration(struct wfx_dev *wdev, const u8 *conf, size_t len)
return -ENOMEM;
body->length = cpu_to_le16(len);
memcpy(body->pds_data, conf, len);
- wfx_fill_header(hif, -1, HIF_REQ_ID_CONFIGURATION, buf_len);
- ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wdev, hif, -1, HIF_REQ_ID_CONFIGURATION,
+ buf_len);
}
int hif_reset(struct wfx_vif *wvif, bool reset_stat)
{
- int ret;
struct hif_msg *hif;
struct hif_req_reset *body = wfx_alloc_hif(sizeof(*body), &hif);
if (!hif)
return -ENOMEM;
body->reset_flags.reset_stat = reset_stat;
- wfx_fill_header(hif, wvif->id, HIF_REQ_ID_RESET, sizeof(*body));
- ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wvif->wdev, hif, wvif->id,
+ HIF_REQ_ID_RESET, sizeof(*body));
}
int hif_read_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
@@ -198,9 +221,8 @@ int hif_read_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
goto out;
}
body->mib_id = cpu_to_le16(mib_id);
- wfx_fill_header(hif, vif_id, HIF_REQ_ID_READ_MIB, sizeof(*body));
- ret = wfx_cmd_send(wdev, hif, reply, buf_len, false);
-
+ ret = wfx_full_send(wdev, hif, reply, buf_len, false, vif_id,
+ HIF_REQ_ID_READ_MIB, sizeof(*body));
if (!ret && mib_id != le16_to_cpu(reply->mib_id)) {
dev_warn(wdev->dev, "%s: confirmation mismatch request\n",
__func__);
@@ -223,7 +245,6 @@ int hif_read_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
int hif_write_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
void *val, size_t val_len)
{
- int ret;
struct hif_msg *hif;
int buf_len = sizeof(struct hif_req_write_mib) + val_len;
struct hif_req_write_mib *body = wfx_alloc_hif(buf_len, &hif);
@@ -233,16 +254,14 @@ int hif_write_mib(struct wfx_dev *wdev, int vif_id, u16 mib_id,
body->mib_id = cpu_to_le16(mib_id);
body->length = cpu_to_le16(val_len);
memcpy(&body->mib_data, val, val_len);
- wfx_fill_header(hif, vif_id, HIF_REQ_ID_WRITE_MIB, buf_len);
- ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wdev, hif, vif_id, HIF_REQ_ID_WRITE_MIB,
+ buf_len);
}
int hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req,
int chan_start_idx, int chan_num, int *timeout)
{
- int ret, i;
+ int i;
struct hif_msg *hif;
size_t buf_len =
sizeof(struct hif_req_start_scan_alt) + chan_num * sizeof(u8);
@@ -292,31 +311,25 @@ int hif_scan(struct wfx_vif *wvif, struct cfg80211_scan_request *req,
if (timeout)
*timeout = usecs_to_jiffies(tmo);
- wfx_fill_header(hif, wvif->id, HIF_REQ_ID_START_SCAN, buf_len);
- ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply(wvif->wdev, hif, wvif->id,
+ HIF_REQ_ID_START_SCAN, buf_len);
}
int hif_stop_scan(struct wfx_vif *wvif)
{
- int ret;
struct hif_msg *hif;
// body associated to HIF_REQ_ID_STOP_SCAN is empty
wfx_alloc_hif(0, &hif);
if (!hif)
return -ENOMEM;
- wfx_fill_header(hif, wvif->id, HIF_REQ_ID_STOP_SCAN, 0);
- ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wvif->wdev, hif, wvif->id,
+ HIF_REQ_ID_STOP_SCAN, 0);
}
int hif_join(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
struct ieee80211_channel *channel, const u8 *ssid, int ssidlen)
{
- int ret;
struct hif_msg *hif;
struct hif_req_join *body = wfx_alloc_hif(sizeof(*body), &hif);
@@ -341,15 +354,12 @@ int hif_join(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
body->ssid_length = cpu_to_le32(ssidlen);
memcpy(body->ssid, ssid, ssidlen);
}
- wfx_fill_header(hif, wvif->id, HIF_REQ_ID_JOIN, sizeof(*body));
- ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wvif->wdev, hif, wvif->id,
+ HIF_REQ_ID_JOIN, sizeof(*body));
}
int hif_set_bss_params(struct wfx_vif *wvif, int aid, int beacon_lost_count)
{
- int ret;
struct hif_msg *hif;
struct hif_req_set_bss_params *body =
wfx_alloc_hif(sizeof(*body), &hif);
@@ -358,16 +368,13 @@ int hif_set_bss_params(struct wfx_vif *wvif, int aid, int beacon_lost_count)
return -ENOMEM;
body->aid = cpu_to_le16(aid);
body->beacon_lost_count = beacon_lost_count;
- wfx_fill_header(hif, wvif->id, HIF_REQ_ID_SET_BSS_PARAMS,
- sizeof(*body));
- ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wvif->wdev, hif, wvif->id,
+ HIF_REQ_ID_SET_BSS_PARAMS,
+ sizeof(*body));
}
int hif_add_key(struct wfx_dev *wdev, const struct hif_req_add_key *arg)
{
- int ret;
struct hif_msg *hif;
// FIXME: only send necessary bits
struct hif_req_add_key *body = wfx_alloc_hif(sizeof(*body), &hif);
@@ -379,34 +386,28 @@ int hif_add_key(struct wfx_dev *wdev, const struct hif_req_add_key *arg)
if (wfx_api_older_than(wdev, 1, 5))
// Legacy firmwares expect that add_key to be sent on right
// interface.
- wfx_fill_header(hif, arg->int_id, HIF_REQ_ID_ADD_KEY,
- sizeof(*body));
- else
- wfx_fill_header(hif, -1, HIF_REQ_ID_ADD_KEY, sizeof(*body));
- ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wdev, hif, arg->int_id,
+ HIF_REQ_ID_ADD_KEY,
+ sizeof(*body));
+ return wfx_full_send_no_reply_free(wdev, hif, -1, HIF_REQ_ID_ADD_KEY,
+ sizeof(*body));
}
int hif_remove_key(struct wfx_dev *wdev, int idx)
{
- int ret;
struct hif_msg *hif;
struct hif_req_remove_key *body = wfx_alloc_hif(sizeof(*body), &hif);
if (!hif)
return -ENOMEM;
body->entry_index = idx;
- wfx_fill_header(hif, -1, HIF_REQ_ID_REMOVE_KEY, sizeof(*body));
- ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wdev, hif, -1, HIF_REQ_ID_REMOVE_KEY,
+ sizeof(*body));
}
int hif_set_edca_queue_params(struct wfx_vif *wvif, u16 queue,
const struct ieee80211_tx_queue_params *arg)
{
- int ret;
struct hif_msg *hif;
struct hif_req_edca_queue_params *body = wfx_alloc_hif(sizeof(*body),
&hif);
@@ -427,16 +428,13 @@ int hif_set_edca_queue_params(struct wfx_vif *wvif, u16 queue,
body->queue_id = HIF_QUEUE_ID_BACKGROUND;
if (wfx_api_older_than(wvif->wdev, 2, 0) && queue == IEEE80211_AC_BK)
body->queue_id = HIF_QUEUE_ID_BESTEFFORT;
- wfx_fill_header(hif, wvif->id, HIF_REQ_ID_EDCA_QUEUE_PARAMS,
- sizeof(*body));
- ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wvif->wdev, hif, wvif->id,
+ HIF_REQ_ID_EDCA_QUEUE_PARAMS,
+ sizeof(*body));
}
int hif_set_pm(struct wfx_vif *wvif, bool ps, int dynamic_ps_timeout)
{
- int ret;
struct hif_msg *hif;
struct hif_req_set_pm_mode *body = wfx_alloc_hif(sizeof(*body), &hif);
@@ -452,16 +450,13 @@ int hif_set_pm(struct wfx_vif *wvif, bool ps, int dynamic_ps_timeout)
if (body->fast_psm_idle_period)
body->pm_mode.fast_psm = 1;
}
- wfx_fill_header(hif, wvif->id, HIF_REQ_ID_SET_PM_MODE, sizeof(*body));
- ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wvif->wdev, hif, wvif->id,
+ HIF_REQ_ID_SET_PM_MODE, sizeof(*body));
}
int hif_start(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
const struct ieee80211_channel *channel)
{
- int ret;
struct hif_msg *hif;
struct hif_req_start *body = wfx_alloc_hif(sizeof(*body), &hif);
@@ -476,15 +471,12 @@ int hif_start(struct wfx_vif *wvif, const struct ieee80211_bss_conf *conf,
cpu_to_le32(wfx_rate_mask_to_hw(wvif->wdev, conf->basic_rates));
body->ssid_length = conf->ssid_len;
memcpy(body->ssid, conf->ssid, conf->ssid_len);
- wfx_fill_header(hif, wvif->id, HIF_REQ_ID_START, sizeof(*body));
- ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wvif->wdev, hif, wvif->id,
+ HIF_REQ_ID_START, sizeof(*body));
}
int hif_beacon_transmit(struct wfx_vif *wvif, bool enable)
{
- int ret;
struct hif_msg *hif;
struct hif_req_beacon_transmit *body = wfx_alloc_hif(sizeof(*body),
&hif);
@@ -492,16 +484,13 @@ int hif_beacon_transmit(struct wfx_vif *wvif, bool enable)
if (!hif)
return -ENOMEM;
body->enable_beaconing = enable ? 1 : 0;
- wfx_fill_header(hif, wvif->id, HIF_REQ_ID_BEACON_TRANSMIT,
- sizeof(*body));
- ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wvif->wdev, hif, wvif->id,
+ HIF_REQ_ID_BEACON_TRANSMIT,
+ sizeof(*body));
}
int hif_map_link(struct wfx_vif *wvif, u8 *mac_addr, int flags, int sta_id)
{
- int ret;
struct hif_msg *hif;
struct hif_req_map_link *body = wfx_alloc_hif(sizeof(*body), &hif);
@@ -511,15 +500,12 @@ int hif_map_link(struct wfx_vif *wvif, u8 *mac_addr, int flags, int sta_id)
ether_addr_copy(body->mac_addr, mac_addr);
body->map_link_flags = *(struct hif_map_link_flags *)&flags;
body->peer_sta_id = sta_id;
- wfx_fill_header(hif, wvif->id, HIF_REQ_ID_MAP_LINK, sizeof(*body));
- ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wvif->wdev, hif, wvif->id,
+ HIF_REQ_ID_MAP_LINK, sizeof(*body));
}
int hif_update_ie_beacon(struct wfx_vif *wvif, const u8 *ies, size_t ies_len)
{
- int ret;
struct hif_msg *hif;
int buf_len = sizeof(struct hif_req_update_ie) + ies_len;
struct hif_req_update_ie *body = wfx_alloc_hif(buf_len, &hif);
@@ -529,10 +515,8 @@ int hif_update_ie_beacon(struct wfx_vif *wvif, const u8 *ies, size_t ies_len)
body->ie_flags.beacon = 1;
body->num_ies = cpu_to_le16(1);
memcpy(body->ie, ies, ies_len);
- wfx_fill_header(hif, wvif->id, HIF_REQ_ID_UPDATE_IE, buf_len);
- ret = wfx_cmd_send(wvif->wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wvif->wdev, hif, wvif->id,
+ HIF_REQ_ID_UPDATE_IE, buf_len);
}
int hif_sl_send_pub_keys(struct wfx_dev *wdev,
@@ -549,10 +533,9 @@ int hif_sl_send_pub_keys(struct wfx_dev *wdev,
memcpy(body->host_pub_key, pubkey, sizeof(body->host_pub_key));
memcpy(body->host_pub_key_mac, pubkey_hmac,
sizeof(body->host_pub_key_mac));
- wfx_fill_header(hif, -1, HIF_REQ_ID_SL_EXCHANGE_PUB_KEYS,
- sizeof(*body));
- ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
- kfree(hif);
+ ret = wfx_full_send_no_reply_free(wdev, hif, -1,
+ HIF_REQ_ID_SL_EXCHANGE_PUB_KEYS,
+ sizeof(*body));
// Compatibility with legacy secure link
if (ret == le32_to_cpu(HIF_STATUS_SLK_NEGO_SUCCESS))
ret = 0;
@@ -561,17 +544,14 @@ int hif_sl_send_pub_keys(struct wfx_dev *wdev,
int hif_sl_config(struct wfx_dev *wdev, const unsigned long *bitmap)
{
- int ret;
struct hif_msg *hif;
struct hif_req_sl_configure *body = wfx_alloc_hif(sizeof(*body), &hif);
if (!hif)
return -ENOMEM;
memcpy(body->encr_bmp, bitmap, sizeof(body->encr_bmp));
- wfx_fill_header(hif, -1, HIF_REQ_ID_SL_CONFIGURE, sizeof(*body));
- ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
- kfree(hif);
- return ret;
+ return wfx_full_send_no_reply_free(wdev, hif, -1, HIF_REQ_ID_SL_CONFIGURE,
+ sizeof(*body));
}
int hif_sl_set_mac_key(struct wfx_dev *wdev, const u8 *slk_key, int destination)
@@ -585,9 +565,8 @@ int hif_sl_set_mac_key(struct wfx_dev *wdev, const u8 *slk_key, int destination)
return -ENOMEM;
memcpy(body->key_value, slk_key, sizeof(body->key_value));
body->otp_or_ram = destination;
- wfx_fill_header(hif, -1, HIF_REQ_ID_SET_SL_MAC_KEY, sizeof(*body));
- ret = wfx_cmd_send(wdev, hif, NULL, 0, false);
- kfree(hif);
+ ret = wfx_full_send_no_reply_free(wdev, hif, -1, HIF_REQ_ID_SET_SL_MAC_KEY,
+ sizeof(*body));
// Compatibility with legacy secure link
if (ret == le32_to_cpu(HIF_STATUS_SLK_SET_KEY_SUCCESS))
ret = 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] staging: wfx: refactor to avoid duplication at hif_tx.c
2020-08-05 8:56 [PATCH] staging: wfx: refactor to avoid duplication at hif_tx.c Tomer Samara
@ 2020-08-05 9:04 ` Greg KH
2020-08-05 10:48 ` Tomer Samara
2020-08-05 11:24 ` kernel test robot
1 sibling, 1 reply; 4+ messages in thread
From: Greg KH @ 2020-08-05 9:04 UTC (permalink / raw)
To: Tomer Samara; +Cc: jerome.pouiller, devel, linux-kernel
On Wed, Aug 05, 2020 at 11:56:08AM +0300, Tomer Samara wrote:
> Add functions wfx_full_send(), wfx_full_send_no_reply_async(),
> wfx_full_send_no_reply() and wfx_full_send_no_reply_free()
> which works as follow:
> wfx_full_send() - simple wrapper for both wfx_fill_header()
> and wfx_cmd_send().
> wfx_full_send_no_reply_async() - wrapper for both but with
> NULL as reply and size zero.
> wfx_full_send_no_reply() - same as wfx_full_send_no_reply_async()
> but with false async value
> wfx_full_send_no_reply_free() - same as wfx_full_send_no_reply()
> but also free the struct hif_msg.
Please only do one-thing-per-patch. Why shouldn't this be a 4 patch
series?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: wfx: refactor to avoid duplication at hif_tx.c
2020-08-05 9:04 ` Greg KH
@ 2020-08-05 10:48 ` Tomer Samara
0 siblings, 0 replies; 4+ messages in thread
From: Tomer Samara @ 2020-08-05 10:48 UTC (permalink / raw)
To: Greg KH; +Cc: jerome.pouiller, devel, linux-kernel
On Wed, Aug 05, 2020 at 11:04:25AM +0200, Greg KH wrote:
> On Wed, Aug 05, 2020 at 11:56:08AM +0300, Tomer Samara wrote:
> > Add functions wfx_full_send(), wfx_full_send_no_reply_async(),
> > wfx_full_send_no_reply() and wfx_full_send_no_reply_free()
> > which works as follow:
> > wfx_full_send() - simple wrapper for both wfx_fill_header()
> > and wfx_cmd_send().
> > wfx_full_send_no_reply_async() - wrapper for both but with
> > NULL as reply and size zero.
> > wfx_full_send_no_reply() - same as wfx_full_send_no_reply_async()
> > but with false async value
> > wfx_full_send_no_reply_free() - same as wfx_full_send_no_reply()
> > but also free the struct hif_msg.
>
> Please only do one-thing-per-patch. Why shouldn't this be a 4 patch
> series?
>
> thanks,
>
> greg k-h
All of the 4 functions are wrappers for the same duplicate code when
every time there are different flags, so they are all connected, it is
feel to me more legit to patch them all together, should I split them
into 4 different patches?
Thanks,
Tomer Samara
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging: wfx: refactor to avoid duplication at hif_tx.c
2020-08-05 8:56 [PATCH] staging: wfx: refactor to avoid duplication at hif_tx.c Tomer Samara
2020-08-05 9:04 ` Greg KH
@ 2020-08-05 11:24 ` kernel test robot
1 sibling, 0 replies; 4+ messages in thread
From: kernel test robot @ 2020-08-05 11:24 UTC (permalink / raw)
To: Tomer Samara, jerome.pouiller; +Cc: kbuild-all, gregkh, devel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 7672 bytes --]
Hi Tomer,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on staging/staging-testing]
url: https://github.com/0day-ci/linux/commits/Tomer-Samara/staging-wfx-refactor-to-avoid-duplication-at-hif_tx-c/20200805-165649
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 5bbd90550da8f7bdac769b5825597e67183c9411
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from arch/m68k/include/asm/io_mm.h:25,
from arch/m68k/include/asm/io.h:8,
from include/linux/scatterlist.h:9,
from include/linux/dma-mapping.h:11,
from include/linux/skbuff.h:31,
from include/linux/if_ether.h:19,
from include/linux/etherdevice.h:20,
from drivers/staging/wfx/hif_tx.c:9:
arch/m68k/include/asm/raw_io.h: In function 'raw_rom_outsb':
arch/m68k/include/asm/raw_io.h:83:7: warning: variable '__w' set but not used [-Wunused-but-set-variable]
83 | ({u8 __w, __v = (b); u32 _addr = ((u32) (addr)); \
| ^~~
arch/m68k/include/asm/raw_io.h:430:3: note: in expansion of macro 'rom_out_8'
430 | rom_out_8(port, *buf++);
| ^~~~~~~~~
arch/m68k/include/asm/raw_io.h: In function 'raw_rom_outsw':
arch/m68k/include/asm/raw_io.h:86:8: warning: variable '__w' set but not used [-Wunused-but-set-variable]
86 | ({u16 __w, __v = (w); u32 _addr = ((u32) (addr)); \
| ^~~
arch/m68k/include/asm/raw_io.h:448:3: note: in expansion of macro 'rom_out_be16'
448 | rom_out_be16(port, *buf++);
| ^~~~~~~~~~~~
arch/m68k/include/asm/raw_io.h: In function 'raw_rom_outsw_swapw':
arch/m68k/include/asm/raw_io.h:90:8: warning: variable '__w' set but not used [-Wunused-but-set-variable]
90 | ({u16 __w, __v = (w); u32 _addr = ((u32) (addr)); \
| ^~~
arch/m68k/include/asm/raw_io.h:466:3: note: in expansion of macro 'rom_out_le16'
466 | rom_out_le16(port, *buf++);
| ^~~~~~~~~~~~
In file included from include/linux/kernel.h:11,
from include/linux/skbuff.h:13,
from include/linux/if_ether.h:19,
from include/linux/etherdevice.h:20,
from drivers/staging/wfx/hif_tx.c:9:
include/linux/scatterlist.h: In function 'sg_set_buf':
arch/m68k/include/asm/page_mm.h:169:49: warning: ordered comparison of pointer with null pointer [-Wextra]
169 | #define virt_addr_valid(kaddr) ((void *)(kaddr) >= (void *)PAGE_OFFSET && (void *)(kaddr) < high_memory)
| ^~
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON'
143 | BUG_ON(!virt_addr_valid(buf));
| ^~~~~~
include/linux/scatterlist.h:143:10: note: in expansion of macro 'virt_addr_valid'
143 | BUG_ON(!virt_addr_valid(buf));
| ^~~~~~~~~~~~~~~
In file included from arch/m68k/include/asm/bug.h:32,
from include/linux/bug.h:5,
from include/linux/thread_info.h:12,
from include/asm-generic/preempt.h:5,
from ./arch/m68k/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/skbuff.h:15,
from include/linux/if_ether.h:19,
from include/linux/etherdevice.h:20,
from drivers/staging/wfx/hif_tx.c:9:
include/linux/dma-mapping.h: In function 'dma_map_resource':
arch/m68k/include/asm/page_mm.h:169:49: warning: ordered comparison of pointer with null pointer [-Wextra]
169 | #define virt_addr_valid(kaddr) ((void *)(kaddr) >= (void *)PAGE_OFFSET && (void *)(kaddr) < high_memory)
| ^~
include/asm-generic/bug.h:144:27: note: in definition of macro 'WARN_ON_ONCE'
144 | int __ret_warn_once = !!(condition); \
| ^~~~~~~~~
arch/m68k/include/asm/page_mm.h:170:25: note: in expansion of macro 'virt_addr_valid'
170 | #define pfn_valid(pfn) virt_addr_valid(pfn_to_virt(pfn))
| ^~~~~~~~~~~~~~~
include/linux/dma-mapping.h:352:19: note: in expansion of macro 'pfn_valid'
352 | if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
| ^~~~~~~~~
drivers/staging/wfx/hif_tx.c: At top level:
>> drivers/staging/wfx/hif_tx.c:126:5: warning: no previous prototype for 'wfx_full_send' [-Wmissing-prototypes]
126 | int wfx_full_send(struct wfx_dev *wdev, struct hif_msg *hif, void *reply, size_t reply_len,
| ^~~~~~~~~~~~~
>> drivers/staging/wfx/hif_tx.c:133:5: warning: no previous prototype for 'wfx_full_send_no_reply_async' [-Wmissing-prototypes]
133 | int wfx_full_send_no_reply_async(struct wfx_dev *wdev, struct hif_msg *hif, int if_id,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/wfx/hif_tx.c:139:5: warning: no previous prototype for 'wfx_full_send_no_reply' [-Wmissing-prototypes]
139 | int wfx_full_send_no_reply(struct wfx_dev *wdev, struct hif_msg *hif, int if_id,
| ^~~~~~~~~~~~~~~~~~~~~~
>> drivers/staging/wfx/hif_tx.c:145:5: warning: no previous prototype for 'wfx_full_send_no_reply_free' [-Wmissing-prototypes]
145 | int wfx_full_send_no_reply_free(struct wfx_dev *wdev, struct hif_msg *hif, int if_id,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/wfx_full_send +126 drivers/staging/wfx/hif_tx.c
125
> 126 int wfx_full_send(struct wfx_dev *wdev, struct hif_msg *hif, void *reply, size_t reply_len,
127 bool async, int if_id, unsigned int cmd, int size)
128 {
129 wfx_fill_header(hif, if_id, cmd, size);
130 return wfx_cmd_send(wdev, hif, reply, reply_len, async);
131 }
132
> 133 int wfx_full_send_no_reply_async(struct wfx_dev *wdev, struct hif_msg *hif, int if_id,
134 unsigned int cmd, int size, bool async)
135 {
136 return wfx_full_send(wdev, hif, NULL, 0, async, if_id, cmd, size);
137 }
138
> 139 int wfx_full_send_no_reply(struct wfx_dev *wdev, struct hif_msg *hif, int if_id,
140 unsigned int cmd, int size)
141 {
142 return wfx_full_send_no_reply_async(wdev, hif, if_id, cmd, size, false);
143 }
144
> 145 int wfx_full_send_no_reply_free(struct wfx_dev *wdev, struct hif_msg *hif, int if_id,
146 unsigned int cmd, int size)
147 {
148 int ret;
149
150 ret = wfx_full_send_no_reply(wdev, hif, if_id, cmd, size);
151 kfree(hif);
152 return ret;
153 }
154
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 57295 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-05 20:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-05 8:56 [PATCH] staging: wfx: refactor to avoid duplication at hif_tx.c Tomer Samara
2020-08-05 9:04 ` Greg KH
2020-08-05 10:48 ` Tomer Samara
2020-08-05 11:24 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox