Drop unused second argument of TX callback functions and use
'struct txq_entry_t *' as the only argument, thus removing
'struct wilc_p2p_mgmt_data', 'struct tx_complete_mon_data'
and 'struct tx_complete_data' (actually intended just to
pass callbacks parameters) as well. This also shrinks
'struct txq_entry_t' by 'priv' field and eliminates a few
'kmalloc()/kfree()' calls (at the cost of having dummy
stack-allocated 'struct txq_entry_t' instances).
Signed-off-by: Dmitry Antipov <[email protected]>
---
.../wireless/microchip/wilc1000/cfg80211.c | 33 +++---------
drivers/net/wireless/microchip/wilc1000/mon.c | 32 +++---------
.../net/wireless/microchip/wilc1000/netdev.c | 24 ++-------
.../net/wireless/microchip/wilc1000/wlan.c | 50 +++++++++----------
.../net/wireless/microchip/wilc1000/wlan.h | 22 +++-----
5 files changed, 50 insertions(+), 111 deletions(-)
diff --git a/drivers/net/wireless/microchip/wilc1000/cfg80211.c b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
index b545d93c6e37..4490713a963b 100644
--- a/drivers/net/wireless/microchip/wilc1000/cfg80211.c
+++ b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
@@ -54,11 +54,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
};
#endif
-struct wilc_p2p_mgmt_data {
- int size;
- u8 *buff;
-};
-
struct wilc_p2p_pub_act_frame {
u8 category;
u8 action;
@@ -1086,12 +1081,9 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff, u32 size)
cfg80211_rx_mgmt(&priv->wdev, freq, 0, buff, size, 0);
}
-static void wilc_wfi_mgmt_tx_complete(void *priv, int status)
+static void wilc_wfi_mgmt_tx_complete(struct txq_entry_t *tqe)
{
- struct wilc_p2p_mgmt_data *pv_data = priv;
-
- kfree(pv_data->buff);
- kfree(pv_data);
+ kfree(tqe->buffer);
}
static void wilc_wfi_remain_on_channel_expired(void *data, u64 cookie)
@@ -1172,7 +1164,6 @@ static int mgmt_tx(struct wiphy *wiphy,
const u8 *buf = params->buf;
size_t len = params->len;
const struct ieee80211_mgmt *mgmt;
- struct wilc_p2p_mgmt_data *mgmt_tx;
struct wilc_vif *vif = netdev_priv(wdev->netdev);
struct wilc_priv *priv = &vif->priv;
struct host_if_drv *wfi_drv = priv->hif_drv;
@@ -1181,6 +1172,7 @@ static int mgmt_tx(struct wiphy *wiphy,
int ie_offset = offsetof(struct ieee80211_mgmt, u) + sizeof(*d);
const u8 *vendor_ie;
int ret = 0;
+ u8 *copy;
*cookie = get_random_u32();
priv->tx_cookie = *cookie;
@@ -1189,21 +1181,12 @@ static int mgmt_tx(struct wiphy *wiphy,
if (!ieee80211_is_mgmt(mgmt->frame_control))
goto out;
- mgmt_tx = kmalloc(sizeof(*mgmt_tx), GFP_KERNEL);
- if (!mgmt_tx) {
- ret = -ENOMEM;
- goto out;
- }
-
- mgmt_tx->buff = kmemdup(buf, len, GFP_KERNEL);
- if (!mgmt_tx->buff) {
+ copy = kmemdup(buf, len, GFP_KERNEL);
+ if (!copy) {
ret = -ENOMEM;
- kfree(mgmt_tx);
goto out;
}
- mgmt_tx->size = len;
-
if (ieee80211_is_probe_resp(mgmt->frame_control)) {
wilc_set_mac_chnl_num(vif, chan->hw_value);
vif->wilc->op_ch = chan->hw_value;
@@ -1230,8 +1213,7 @@ static int mgmt_tx(struct wiphy *wiphy,
goto out_set_timeout;
vendor_ie = cfg80211_find_vendor_ie(WLAN_OUI_WFA, WLAN_OUI_TYPE_WFA_P2P,
- mgmt_tx->buff + ie_offset,
- len - ie_offset);
+ copy + ie_offset, len - ie_offset);
if (!vendor_ie)
goto out_set_timeout;
@@ -1243,8 +1225,7 @@ static int mgmt_tx(struct wiphy *wiphy,
out_txq_add_pkt:
- wilc_wlan_txq_add_mgmt_pkt(wdev->netdev, mgmt_tx,
- mgmt_tx->buff, mgmt_tx->size,
+ wilc_wlan_txq_add_mgmt_pkt(wdev->netdev, NULL, copy, len,
wilc_wfi_mgmt_tx_complete);
out:
diff --git a/drivers/net/wireless/microchip/wilc1000/mon.c b/drivers/net/wireless/microchip/wilc1000/mon.c
index 03b7229a0ff5..05e0af133dd3 100644
--- a/drivers/net/wireless/microchip/wilc1000/mon.c
+++ b/drivers/net/wireless/microchip/wilc1000/mon.c
@@ -95,45 +95,25 @@ void wilc_wfi_monitor_rx(struct net_device *mon_dev, u8 *buff, u32 size)
netif_rx(skb);
}
-struct tx_complete_mon_data {
- int size;
- void *buff;
-};
-
-static void mgmt_tx_complete(void *priv, int status)
+static void mgmt_tx_complete(struct txq_entry_t *tqe)
{
- struct tx_complete_mon_data *pv_data = priv;
- /*
- * in case of fully hosting mode, the freeing will be done
- * in response to the cfg packet
- */
- kfree(pv_data->buff);
-
- kfree(pv_data);
+ kfree(tqe->buffer);
}
static int mon_mgmt_tx(struct net_device *dev, const u8 *buf, size_t len)
{
- struct tx_complete_mon_data *mgmt_tx = NULL;
+ u8 *buff;
if (!dev)
return -EFAULT;
netif_stop_queue(dev);
- mgmt_tx = kmalloc(sizeof(*mgmt_tx), GFP_ATOMIC);
- if (!mgmt_tx)
- return -ENOMEM;
- mgmt_tx->buff = kmemdup(buf, len, GFP_ATOMIC);
- if (!mgmt_tx->buff) {
- kfree(mgmt_tx);
+ buff = kmemdup(buf, len, GFP_ATOMIC);
+ if (!buff)
return -ENOMEM;
- }
-
- mgmt_tx->size = len;
- wilc_wlan_txq_add_mgmt_pkt(dev, mgmt_tx, mgmt_tx->buff, mgmt_tx->size,
- mgmt_tx_complete);
+ wilc_wlan_txq_add_mgmt_pkt(dev, NULL, buff, len, mgmt_tx_complete);
netif_wake_queue(dev);
return 0;
diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
index e9f59de31b0b..74864505ea22 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.c
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
@@ -713,19 +713,15 @@ static void wilc_set_multicast_list(struct net_device *dev)
kfree(mc_list);
}
-static void wilc_tx_complete(void *priv, int status)
+static void wilc_tx_complete(struct txq_entry_t *tqe)
{
- struct tx_complete_data *pv_data = priv;
-
- dev_kfree_skb(pv_data->skb);
- kfree(pv_data);
+ dev_kfree_skb(tqe->skb);
}
netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *ndev)
{
struct wilc_vif *vif = netdev_priv(ndev);
struct wilc *wilc = vif->wilc;
- struct tx_complete_data *tx_data = NULL;
int queue_count;
if (skb->dev != ndev) {
@@ -734,21 +730,9 @@ netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *ndev)
return NETDEV_TX_OK;
}
- tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
- if (!tx_data) {
- dev_kfree_skb(skb);
- netif_wake_queue(ndev);
- return NETDEV_TX_OK;
- }
-
- tx_data->buff = skb->data;
- tx_data->size = skb->len;
- tx_data->skb = skb;
-
vif->netstats.tx_packets++;
- vif->netstats.tx_bytes += tx_data->size;
- queue_count = wilc_wlan_txq_add_net_pkt(ndev, tx_data,
- tx_data->buff, tx_data->size,
+ vif->netstats.tx_bytes += skb->len;
+ queue_count = wilc_wlan_txq_add_net_pkt(ndev, skb, skb->data, skb->len,
wilc_tx_complete);
if (queue_count > FLOW_CONTROL_UPPER_THRESHOLD) {
diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.c b/drivers/net/wireless/microchip/wilc1000/wlan.c
index 58bbf50081e4..19561a807137 100644
--- a/drivers/net/wireless/microchip/wilc1000/wlan.c
+++ b/drivers/net/wireless/microchip/wilc1000/wlan.c
@@ -221,8 +221,7 @@ static void wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev)
wilc_wlan_txq_remove(wilc, tqe->q_num, tqe);
tqe->status = 1;
if (tqe->tx_complete_func)
- tqe->tx_complete_func(tqe->priv,
- tqe->status);
+ tqe->tx_complete_func(tqe);
kfree(tqe);
dropped++;
}
@@ -270,10 +269,10 @@ static int wilc_wlan_txq_add_cfg_pkt(struct wilc_vif *vif, u8 *buffer,
}
tqe->type = WILC_CFG_PKT;
+ tqe->skb = NULL;
tqe->buffer = buffer;
tqe->buffer_size = buffer_size;
tqe->tx_complete_func = NULL;
- tqe->priv = NULL;
tqe->q_num = AC_VO_Q;
tqe->ack_idx = NOT_TCP_ACK;
tqe->vif = vif;
@@ -410,12 +409,12 @@ static inline u8 ac_change(struct wilc *wilc, u8 *ac)
return 1;
}
-int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
- struct tx_complete_data *tx_data, u8 *buffer,
- u32 buffer_size,
- void (*tx_complete_fn)(void *, int))
+int wilc_wlan_txq_add_net_pkt(struct net_device *dev, struct sk_buff *skb,
+ u8 *buffer, u32 buffer_size,
+ void (*tx_complete_fn)(struct txq_entry_t *))
{
- struct txq_entry_t *tqe;
+ struct txq_entry_t *tqe, dummy = { .skb = skb, .buffer = buffer,
+ .buffer_size = buffer_size };
struct wilc_vif *vif = netdev_priv(dev);
struct wilc *wilc;
u8 q_num;
@@ -423,32 +422,32 @@ int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
wilc = vif->wilc;
if (wilc->quit) {
- tx_complete_fn(tx_data, 0);
+ tx_complete_fn(&dummy);
return 0;
}
if (!wilc->initialized) {
- tx_complete_fn(tx_data, 0);
+ tx_complete_fn(&dummy);
return 0;
}
tqe = kmalloc(sizeof(*tqe), GFP_ATOMIC);
if (!tqe) {
- tx_complete_fn(tx_data, 0);
+ tx_complete_fn(&dummy);
return 0;
}
tqe->type = WILC_NET_PKT;
+ tqe->skb = skb;
tqe->buffer = buffer;
tqe->buffer_size = buffer_size;
tqe->tx_complete_func = tx_complete_fn;
- tqe->priv = tx_data;
tqe->vif = vif;
- q_num = ac_classify(wilc, tx_data->skb);
+ q_num = ac_classify(wilc, skb);
tqe->q_num = q_num;
if (ac_change(wilc, &q_num)) {
- tx_complete_fn(tx_data, 0);
+ tx_complete_fn(tqe);
kfree(tqe);
return 0;
}
@@ -459,43 +458,44 @@ int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
tcp_process(dev, tqe);
wilc_wlan_txq_add_to_tail(dev, q_num, tqe);
} else {
- tx_complete_fn(tx_data, 0);
+ tx_complete_fn(tqe);
kfree(tqe);
}
return wilc->txq_entries;
}
-int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, void *priv, u8 *buffer,
- u32 buffer_size,
- void (*tx_complete_fn)(void *, int))
+int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, struct sk_buff *skb,
+ u8 *buffer, u32 buffer_size,
+ void (*tx_complete_fn)(struct txq_entry_t *))
{
- struct txq_entry_t *tqe;
+ struct txq_entry_t *tqe, dummy = { .skb = skb, .buffer = buffer,
+ .buffer_size = buffer_size };
struct wilc_vif *vif = netdev_priv(dev);
struct wilc *wilc;
wilc = vif->wilc;
if (wilc->quit) {
- tx_complete_fn(priv, 0);
+ tx_complete_fn(&dummy);
return 0;
}
if (!wilc->initialized) {
- tx_complete_fn(priv, 0);
+ tx_complete_fn(&dummy);
return 0;
}
tqe = kmalloc(sizeof(*tqe), GFP_ATOMIC);
if (!tqe) {
- tx_complete_fn(priv, 0);
+ tx_complete_fn(&dummy);
return 0;
}
tqe->type = WILC_MGMT_PKT;
+ tqe->skb = skb;
tqe->buffer = buffer;
tqe->buffer_size = buffer_size;
tqe->tx_complete_func = tx_complete_fn;
- tqe->priv = priv;
tqe->q_num = AC_BE_Q;
tqe->ack_idx = NOT_TCP_ACK;
tqe->vif = vif;
@@ -918,7 +918,7 @@ int wilc_wlan_handle_txq(struct wilc *wilc, u32 *txq_count)
i++;
tqe->status = 1;
if (tqe->tx_complete_func)
- tqe->tx_complete_func(tqe->priv, tqe->status);
+ tqe->tx_complete_func(tqe);
if (tqe->ack_idx != NOT_TCP_ACK &&
tqe->ack_idx < MAX_PENDING_ACKS)
vif->ack_filter.pending_acks[tqe->ack_idx].txqe = NULL;
@@ -1244,7 +1244,7 @@ void wilc_wlan_cleanup(struct net_device *dev)
for (ac = 0; ac < NQUEUES; ac++) {
while ((tqe = wilc_wlan_txq_remove_from_head(wilc, ac))) {
if (tqe->tx_complete_func)
- tqe->tx_complete_func(tqe->priv, 0);
+ tqe->tx_complete_func(tqe);
kfree(tqe);
}
}
diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.h b/drivers/net/wireless/microchip/wilc1000/wlan.h
index a72cd5cac81d..ecccd43baaa3 100644
--- a/drivers/net/wireless/microchip/wilc1000/wlan.h
+++ b/drivers/net/wireless/microchip/wilc1000/wlan.h
@@ -328,10 +328,10 @@ struct txq_entry_t {
int ack_idx;
u8 *buffer;
int buffer_size;
- void *priv;
+ struct sk_buff *skb;
int status;
struct wilc_vif *vif;
- void (*tx_complete_func)(void *priv, int status);
+ void (*tx_complete_func)(struct txq_entry_t *tqe);
};
struct txq_fw_recv_queue_stat {
@@ -378,12 +378,6 @@ struct wilc_hif_func {
#define WILC_MAX_CFG_FRAME_SIZE 1468
-struct tx_complete_data {
- int size;
- void *buff;
- struct sk_buff *skb;
-};
-
struct wilc_cfg_cmd_hdr {
u8 cmd_type;
u8 seq_no;
@@ -407,10 +401,9 @@ int wilc_wlan_firmware_download(struct wilc *wilc, const u8 *buffer,
u32 buffer_size);
int wilc_wlan_start(struct wilc *wilc);
int wilc_wlan_stop(struct wilc *wilc, struct wilc_vif *vif);
-int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
- struct tx_complete_data *tx_data, u8 *buffer,
- u32 buffer_size,
- void (*tx_complete_fn)(void *, int));
+int wilc_wlan_txq_add_net_pkt(struct net_device *dev, struct sk_buff *skb,
+ u8 *buffer, u32 buffer_size,
+ void (*tx_complete_fn)(struct txq_entry_t *));
int wilc_wlan_handle_txq(struct wilc *wl, u32 *txq_count);
void wilc_handle_isr(struct wilc *wilc);
void wilc_wlan_cleanup(struct net_device *dev);
@@ -418,8 +411,9 @@ int wilc_wlan_cfg_set(struct wilc_vif *vif, int start, u16 wid, u8 *buffer,
u32 buffer_size, int commit, u32 drv_handler);
int wilc_wlan_cfg_get(struct wilc_vif *vif, int start, u16 wid, int commit,
u32 drv_handler);
-int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, void *priv, u8 *buffer,
- u32 buffer_size, void (*func)(void *, int));
+int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, struct sk_buff *skb,
+ u8 *buffer, u32 buffer_size,
+ void (*tx_complete_fn)(struct txq_entry_t *));
void wilc_enable_tcp_ack_filter(struct wilc_vif *vif, bool value);
int wilc_wlan_get_num_conn_ifcs(struct wilc *wilc);
netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *dev);
--
2.41.0
Hi Dmitry,
On 7/13/23 01:26, Dmitry Antipov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Drop unused second argument of TX callback functions and use
> 'struct txq_entry_t *' as the only argument, thus removing
> 'struct wilc_p2p_mgmt_data', 'struct tx_complete_mon_data'
> and 'struct tx_complete_data' (actually intended just to
> pass callbacks parameters) as well. This also shrinks
> 'struct txq_entry_t' by 'priv' field and eliminates a few
> 'kmalloc()/kfree()' calls (at the cost of having dummy
> stack-allocated 'struct txq_entry_t' instances).
I'm just curious to know if you have tested this patch with the real
hardware.
In my opinion, some of these patch changes would make the code bit
difficult to read because it is partially modifying the current design
that handles the different categories of Tx frames.
As you see, the 'txq_entry' struct is used to hold three different
types(WLAN, network and WID's config) of frames. The driver allocates
the appropriate type of frame and then transfer to the firmware over bus
interface. Once the frame is transferred to firmware, the driver needs
to free the appropriate buffer using tx_complete callback for that the
driver needs the correct buffer pointer.
Moreover, the buffer format is not same for each type of frame i.e in
case of network frame it's skb buffer whereas WID's config frames uses a
raw buffer which contains data as per WID format, so it needs to know
the entry type to call the specific free function.
Currently, the 'priv' element is used to store either 'skb' or
'tx_complete_mon/tx_complete' which is based on the type of the frame,
so changing 'priv' to 'skb' doesn't make sense for non data frames.
Maybe, if we store only one buffer pointer and let the complete callback
function take care of freeing it i.e using dev_kfree_skb() for data
frame or kfree() for the rest then it makes more sense.
Also, when some of the tx_complete structures, which are used to free-up
the buffers, are removed, some of the API parameters should be modified
accordingly.
>
> Signed-off-by: Dmitry Antipov <[email protected]>
> ---
> .../wireless/microchip/wilc1000/cfg80211.c | 33 +++---------
> drivers/net/wireless/microchip/wilc1000/mon.c | 32 +++---------
> .../net/wireless/microchip/wilc1000/netdev.c | 24 ++-------
> .../net/wireless/microchip/wilc1000/wlan.c | 50 +++++++++----------
> .../net/wireless/microchip/wilc1000/wlan.h | 22 +++-----
> 5 files changed, 50 insertions(+), 111 deletions(-)
>
> diff --git a/drivers/net/wireless/microchip/wilc1000/cfg80211.c b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
> index b545d93c6e37..4490713a963b 100644
> --- a/drivers/net/wireless/microchip/wilc1000/cfg80211.c
> +++ b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
> @@ -54,11 +54,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
> };
> #endif
>
> -struct wilc_p2p_mgmt_data {
> - int size;
> - u8 *buff;
> -};
> -
> struct wilc_p2p_pub_act_frame {
> u8 category;
> u8 action;
> @@ -1086,12 +1081,9 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff, u32 size)
> cfg80211_rx_mgmt(&priv->wdev, freq, 0, buff, size, 0);
> }
>
> -static void wilc_wfi_mgmt_tx_complete(void *priv, int status)
> +static void wilc_wfi_mgmt_tx_complete(struct txq_entry_t *tqe)
> {
> - struct wilc_p2p_mgmt_data *pv_data = priv;
> -
> - kfree(pv_data->buff);
> - kfree(pv_data);
> + kfree(tqe->buffer);
> }
>
> static void wilc_wfi_remain_on_channel_expired(void *data, u64 cookie)
> @@ -1172,7 +1164,6 @@ static int mgmt_tx(struct wiphy *wiphy,
> const u8 *buf = params->buf;
> size_t len = params->len;
> const struct ieee80211_mgmt *mgmt;
> - struct wilc_p2p_mgmt_data *mgmt_tx;
> struct wilc_vif *vif = netdev_priv(wdev->netdev);
> struct wilc_priv *priv = &vif->priv;
> struct host_if_drv *wfi_drv = priv->hif_drv;
> @@ -1181,6 +1172,7 @@ static int mgmt_tx(struct wiphy *wiphy,
> int ie_offset = offsetof(struct ieee80211_mgmt, u) + sizeof(*d);
> const u8 *vendor_ie;
> int ret = 0;
> + u8 *copy;
>
> *cookie = get_random_u32();
> priv->tx_cookie = *cookie;
> @@ -1189,21 +1181,12 @@ static int mgmt_tx(struct wiphy *wiphy,
> if (!ieee80211_is_mgmt(mgmt->frame_control))
> goto out;
>
> - mgmt_tx = kmalloc(sizeof(*mgmt_tx), GFP_KERNEL);
> - if (!mgmt_tx) {
> - ret = -ENOMEM;
> - goto out;
> - }
> -
> - mgmt_tx->buff = kmemdup(buf, len, GFP_KERNEL);
> - if (!mgmt_tx->buff) {
> + copy = kmemdup(buf, len, GFP_KERNEL);
> + if (!copy) {
> ret = -ENOMEM;
> - kfree(mgmt_tx);
> goto out;
> }
>
> - mgmt_tx->size = len;
> -
> if (ieee80211_is_probe_resp(mgmt->frame_control)) {
> wilc_set_mac_chnl_num(vif, chan->hw_value);
> vif->wilc->op_ch = chan->hw_value;
> @@ -1230,8 +1213,7 @@ static int mgmt_tx(struct wiphy *wiphy,
> goto out_set_timeout;
>
> vendor_ie = cfg80211_find_vendor_ie(WLAN_OUI_WFA, WLAN_OUI_TYPE_WFA_P2P,
> - mgmt_tx->buff + ie_offset,
> - len - ie_offset);
> + copy + ie_offset, len - ie_offset);
> if (!vendor_ie)
> goto out_set_timeout;
>
> @@ -1243,8 +1225,7 @@ static int mgmt_tx(struct wiphy *wiphy,
>
> out_txq_add_pkt:
>
> - wilc_wlan_txq_add_mgmt_pkt(wdev->netdev, mgmt_tx,
> - mgmt_tx->buff, mgmt_tx->size,
> + wilc_wlan_txq_add_mgmt_pkt(wdev->netdev, NULL, copy, len,
> wilc_wfi_mgmt_tx_complete);
No need to pass 'NULL'. The second paramter in
'wilc_wlan_txq_add_mgmt_pkt' can be removed.
>
> out:
> diff --git a/drivers/net/wireless/microchip/wilc1000/mon.c b/drivers/net/wireless/microchip/wilc1000/mon.c
> index 03b7229a0ff5..05e0af133dd3 100644
> --- a/drivers/net/wireless/microchip/wilc1000/mon.c
> +++ b/drivers/net/wireless/microchip/wilc1000/mon.c
> @@ -95,45 +95,25 @@ void wilc_wfi_monitor_rx(struct net_device *mon_dev, u8 *buff, u32 size)
> netif_rx(skb);
> }
>
> -struct tx_complete_mon_data {
> - int size;
> - void *buff;
> -};
> -
> -static void mgmt_tx_complete(void *priv, int status)
> +static void mgmt_tx_complete(struct txq_entry_t *tqe)
This function can be removed and instead 'wilc_wfi_mgmt_tx_complete' cb
can be used since both are doing the same.
> {
> - struct tx_complete_mon_data *pv_data = priv;
> - /*
> - * in case of fully hosting mode, the freeing will be done
> - * in response to the cfg packet
> - */
> - kfree(pv_data->buff);
> -
> - kfree(pv_data);
> + kfree(tqe->buffer);
> }
>
> static int mon_mgmt_tx(struct net_device *dev, const u8 *buf, size_t len)
> {
> - struct tx_complete_mon_data *mgmt_tx = NULL;
> + u8 *buff;
>
> if (!dev)
> return -EFAULT;
>
> netif_stop_queue(dev);
> - mgmt_tx = kmalloc(sizeof(*mgmt_tx), GFP_ATOMIC);
> - if (!mgmt_tx)
> - return -ENOMEM;
>
> - mgmt_tx->buff = kmemdup(buf, len, GFP_ATOMIC);
> - if (!mgmt_tx->buff) {
> - kfree(mgmt_tx);
> + buff = kmemdup(buf, len, GFP_ATOMIC);
> + if (!buff)
> return -ENOMEM;
> - }
> -
> - mgmt_tx->size = len;
>
> - wilc_wlan_txq_add_mgmt_pkt(dev, mgmt_tx, mgmt_tx->buff, mgmt_tx->size,
> - mgmt_tx_complete);
> + wilc_wlan_txq_add_mgmt_pkt(dev, NULL, buff, len, mgmt_tx_complete);
>
Same as above, no need to pass the 'NULL' value.
> netif_wake_queue(dev);
> return 0;
> diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
> index e9f59de31b0b..74864505ea22 100644
> --- a/drivers/net/wireless/microchip/wilc1000/netdev.c
> +++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
> @@ -713,19 +713,15 @@ static void wilc_set_multicast_list(struct net_device *dev)
> kfree(mc_list);
> }
>
> -static void wilc_tx_complete(void *priv, int status)
> +static void wilc_tx_complete(struct txq_entry_t *tqe)
> {
> - struct tx_complete_data *pv_data = priv;
> -
> - dev_kfree_skb(pv_data->skb);
> - kfree(pv_data);
> + dev_kfree_skb(tqe->skb);
> }
>
> netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *ndev)
> {
> struct wilc_vif *vif = netdev_priv(ndev);
> struct wilc *wilc = vif->wilc;
> - struct tx_complete_data *tx_data = NULL;
> int queue_count;
>
> if (skb->dev != ndev) {
> @@ -734,21 +730,9 @@ netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *ndev)
> return NETDEV_TX_OK;
> }
>
> - tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
> - if (!tx_data) {
> - dev_kfree_skb(skb);
> - netif_wake_queue(ndev);
> - return NETDEV_TX_OK;
> - }
> -
> - tx_data->buff = skb->data;
> - tx_data->size = skb->len;
> - tx_data->skb = skb;
> -
> vif->netstats.tx_packets++;
> - vif->netstats.tx_bytes += tx_data->size;
> - queue_count = wilc_wlan_txq_add_net_pkt(ndev, tx_data,
> - tx_data->buff, tx_data->size,
> + vif->netstats.tx_bytes += skb->len;
> + queue_count = wilc_wlan_txq_add_net_pkt(ndev, skb, skb->data, skb->len,
> wilc_tx_complete);
Passing only 'skb' should be enough and wilc_wlan_txq_add_net_pkt should
be modify to extract the buffer and length value internally.
>
> if (queue_count > FLOW_CONTROL_UPPER_THRESHOLD) {
> diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.c b/drivers/net/wireless/microchip/wilc1000/wlan.c
> index 58bbf50081e4..19561a807137 100644
> --- a/drivers/net/wireless/microchip/wilc1000/wlan.c
> +++ b/drivers/net/wireless/microchip/wilc1000/wlan.c
> @@ -221,8 +221,7 @@ static void wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev)
> wilc_wlan_txq_remove(wilc, tqe->q_num, tqe);
> tqe->status = 1;
> if (tqe->tx_complete_func)
> - tqe->tx_complete_func(tqe->priv,
> - tqe->status);
> + tqe->tx_complete_func(tqe);
> kfree(tqe);
> dropped++;
> }
> @@ -270,10 +269,10 @@ static int wilc_wlan_txq_add_cfg_pkt(struct wilc_vif *vif, u8 *buffer,
> }
>
> tqe->type = WILC_CFG_PKT;
> + tqe->skb = NULL;
> tqe->buffer = buffer;
> tqe->buffer_size = buffer_size;
> tqe->tx_complete_func = NULL;
> - tqe->priv = NULL;
> tqe->q_num = AC_VO_Q;
> tqe->ack_idx = NOT_TCP_ACK;
> tqe->vif = vif;
> @@ -410,12 +409,12 @@ static inline u8 ac_change(struct wilc *wilc, u8 *ac)
> return 1;
> }
>
> -int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
> - struct tx_complete_data *tx_data, u8 *buffer,
> - u32 buffer_size,
> - void (*tx_complete_fn)(void *, int))
> +int wilc_wlan_txq_add_net_pkt(struct net_device *dev, struct sk_buff *skb,
> + u8 *buffer, u32 buffer_size,
> + void (*tx_complete_fn)(struct txq_entry_t *))
> {
> - struct txq_entry_t *tqe;
> + struct txq_entry_t *tqe, dummy = { .skb = skb, .buffer = buffer,
> + .buffer_size = buffer_size };
No need to use a 'dummy' variable here.
> struct wilc_vif *vif = netdev_priv(dev);
> struct wilc *wilc;
> u8 q_num;
> @@ -423,32 +422,32 @@ int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
> wilc = vif->wilc;
>
> if (wilc->quit) {
> - tx_complete_fn(tx_data, 0);
> + tx_complete_fn(&dummy);
'skb' buffer can be free directly or 'tx_complete_fn' should be modify
to work using buffer pointer since 'tx_data' is deleted in this patch.
> return 0;
> }
>
> if (!wilc->initialized) {
> - tx_complete_fn(tx_data, 0);
> + tx_complete_fn(&dummy);
same as above comment.
> return 0;
> }
>
> tqe = kmalloc(sizeof(*tqe), GFP_ATOMIC);
>
> if (!tqe) {
> - tx_complete_fn(tx_data, 0);
> + tx_complete_fn(&dummy);
> return 0;
> }
> tqe->type = WILC_NET_PKT;
> + tqe->skb = skb;
> tqe->buffer = buffer;
> tqe->buffer_size = buffer_size;
The above can be modified to fill using skb buffer
e.g
tqe->buffer = skb->data;
tqe->buffer_size = skb->len;
> tqe->tx_complete_func = tx_complete_fn;
> - tqe->priv = tx_data;
> tqe->vif = vif;
>
> - q_num = ac_classify(wilc, tx_data->skb);
> + q_num = ac_classify(wilc, skb);
> tqe->q_num = q_num;
> if (ac_change(wilc, &q_num)) {
> - tx_complete_fn(tx_data, 0);
> + tx_complete_fn(tqe);
> kfree(tqe);
> return 0;
> }
> @@ -459,43 +458,44 @@ int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
> tcp_process(dev, tqe);
> wilc_wlan_txq_add_to_tail(dev, q_num, tqe);
> } else {
> - tx_complete_fn(tx_data, 0);
> + tx_complete_fn(tqe);
> kfree(tqe);
> }
>
> return wilc->txq_entries;
> }
>
> -int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, void *priv, u8 *buffer,
> - u32 buffer_size,
> - void (*tx_complete_fn)(void *, int))
> +int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, struct sk_buff *skb,
> + u8 *buffer, u32 buffer_size,
> + void (*tx_complete_fn)(struct txq_entry_t *))
> {
'skb' argument is not necessary in this function.
> - struct txq_entry_t *tqe;
> + struct txq_entry_t *tqe, dummy = { .skb = skb, .buffer = buffer,
> + .buffer_size = buffer_size };
No need to use a 'dummy' variable here.
> struct wilc_vif *vif = netdev_priv(dev);
> struct wilc *wilc;
>
> wilc = vif->wilc;
>
> if (wilc->quit) {
> - tx_complete_fn(priv, 0);
> + tx_complete_fn(&dummy);
same as earlier 'tx_complete_fn' call.
> return 0;
> }
>
> if (!wilc->initialized) {
> - tx_complete_fn(priv, 0);
> + tx_complete_fn(&dummy);
same as above.
> return 0;
> }
> tqe = kmalloc(sizeof(*tqe), GFP_ATOMIC);
>
> if (!tqe) {
> - tx_complete_fn(priv, 0);
> + tx_complete_fn(&dummy);
same as above.
> return 0;
> }
> tqe->type = WILC_MGMT_PKT;
> + tqe->skb = skb;
Setting of 'skb' buffer is not needed for WLAN frames.
> tqe->buffer = buffer;
> tqe->buffer_size = buffer_size;
> tqe->tx_complete_func = tx_complete_fn;
> - tqe->priv = priv;
> tqe->q_num = AC_BE_Q;
> tqe->ack_idx = NOT_TCP_ACK;
> tqe->vif = vif;
> @@ -918,7 +918,7 @@ int wilc_wlan_handle_txq(struct wilc *wilc, u32 *txq_count)
> i++;
> tqe->status = 1;
> if (tqe->tx_complete_func)
> - tqe->tx_complete_func(tqe->priv, tqe->status);
> + tqe->tx_complete_func(tqe);
> if (tqe->ack_idx != NOT_TCP_ACK &&
> tqe->ack_idx < MAX_PENDING_ACKS)
> vif->ack_filter.pending_acks[tqe->ack_idx].txqe = NULL;
> @@ -1244,7 +1244,7 @@ void wilc_wlan_cleanup(struct net_device *dev)
> for (ac = 0; ac < NQUEUES; ac++) {
> while ((tqe = wilc_wlan_txq_remove_from_head(wilc, ac))) {
> if (tqe->tx_complete_func)
> - tqe->tx_complete_func(tqe->priv, 0);
> + tqe->tx_complete_func(tqe);
> kfree(tqe);
> }
> }
> diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.h b/drivers/net/wireless/microchip/wilc1000/wlan.h
> index a72cd5cac81d..ecccd43baaa3 100644
> --- a/drivers/net/wireless/microchip/wilc1000/wlan.h
> +++ b/drivers/net/wireless/microchip/wilc1000/wlan.h
> @@ -328,10 +328,10 @@ struct txq_entry_t {
> int ack_idx;
> u8 *buffer;
> int buffer_size;
> - void *priv;
> + struct sk_buff *skb;
In my opinion, if a single buffer is used then it patch makes sense
otherwise the previous implementation was more readable.
> int status;
> struct wilc_vif *vif;
> - void (*tx_complete_func)(void *priv, int status);
> + void (*tx_complete_func)(struct txq_entry_t *tqe);
> };
>
> struct txq_fw_recv_queue_stat {
> @@ -378,12 +378,6 @@ struct wilc_hif_func {
>
> #define WILC_MAX_CFG_FRAME_SIZE 1468
>
> -struct tx_complete_data {
> - int size;
> - void *buff;
> - struct sk_buff *skb;
> -};
> -
> struct wilc_cfg_cmd_hdr {
> u8 cmd_type;
> u8 seq_no;
> @@ -407,10 +401,9 @@ int wilc_wlan_firmware_download(struct wilc *wilc, const u8 *buffer,
> u32 buffer_size);
> int wilc_wlan_start(struct wilc *wilc);
> int wilc_wlan_stop(struct wilc *wilc, struct wilc_vif *vif);
> -int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
> - struct tx_complete_data *tx_data, u8 *buffer,
> - u32 buffer_size,
> - void (*tx_complete_fn)(void *, int));
> +int wilc_wlan_txq_add_net_pkt(struct net_device *dev, struct sk_buff *skb,
> + u8 *buffer, u32 buffer_size,
> + void (*tx_complete_fn)(struct txq_entry_t *));
> int wilc_wlan_handle_txq(struct wilc *wl, u32 *txq_count);
> void wilc_handle_isr(struct wilc *wilc);
> void wilc_wlan_cleanup(struct net_device *dev);
> @@ -418,8 +411,9 @@ int wilc_wlan_cfg_set(struct wilc_vif *vif, int start, u16 wid, u8 *buffer,
> u32 buffer_size, int commit, u32 drv_handler);
> int wilc_wlan_cfg_get(struct wilc_vif *vif, int start, u16 wid, int commit,
> u32 drv_handler);
> -int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, void *priv, u8 *buffer,
> - u32 buffer_size, void (*func)(void *, int));
> +int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, struct sk_buff *skb,
> + u8 *buffer, u32 buffer_size,
> + void (*tx_complete_fn)(struct txq_entry_t *));
> void wilc_enable_tcp_ack_filter(struct wilc_vif *vif, bool value);
> int wilc_wlan_get_num_conn_ifcs(struct wilc *wilc);
> netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *dev);
> --
> 2.41.0
>
Not a formal description but some thoughts on Ajay's review:
1. An initial intention was to remove intermediate data structures
used only to pass parameters, and related 'kmalloc()/kfree()' calls
used to manage these structures.
2. Why not use the common cleanup function for all type of frames?
Since the type is always known (it's recorded in 'struct txq_entry_t'),
cleanup function may call 'dev_kfree_skb()' or 'kfree()' for plain
buffers. This also shrinks 'struct txq_entry_t' by one pointer field.
3. IMHO it makes sense to make 'struct txq_entry_t' as small as
possible. To avoid the redundancy (of storing 'skb' nearby to raw
buffer and its size) for WILC_NET_PKT frames, it's possible to use
a union, e.g.:
struct txq_entry_t {
int type;
...
union {
/* Used by WILC_NET_PKT entries */
struct sk_buff *skb;
/* Used by WILC_MGMT_PKT and WILC_CFG_PKT entries */
struct {
u8 *buffer;
u32 size;
} data;
} u;
...
};
but this will require some wrappers to access frame data and size,
e.g.:
static inline u8 *txq_entry_data(struct txq_entry_t *tqe)
{
return (tqe->type == WILC_NET_PKT
? tqe->u.skb->data : tqe->u.data.buffer);
}
static inline u32 txq_entry_size(struct txq_entry_t *tqe)
{
return (tqe->type == WILC_NET_PKT
? tqe->u.skb->len : tqe->u.data.size);
}
I'm not sure that this makes sense just to shrink 'struct txq_entry_t'
by one more pointer field.
Signed-off-by: Dmitry Antipov <[email protected]>
---
.../wireless/microchip/wilc1000/cfg80211.c | 37 ++-------
drivers/net/wireless/microchip/wilc1000/mon.c | 47 +++--------
.../net/wireless/microchip/wilc1000/netdev.c | 34 ++++----
.../net/wireless/microchip/wilc1000/netdev.h | 1 +
.../net/wireless/microchip/wilc1000/wlan.c | 80 +++++--------------
.../net/wireless/microchip/wilc1000/wlan.h | 25 ++----
6 files changed, 66 insertions(+), 158 deletions(-)
diff --git a/drivers/net/wireless/microchip/wilc1000/cfg80211.c b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
index b545d93c6e37..7d244c6c1a92 100644
--- a/drivers/net/wireless/microchip/wilc1000/cfg80211.c
+++ b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
@@ -54,11 +54,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
};
#endif
-struct wilc_p2p_mgmt_data {
- int size;
- u8 *buff;
-};
-
struct wilc_p2p_pub_act_frame {
u8 category;
u8 action;
@@ -1086,14 +1081,6 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff, u32 size)
cfg80211_rx_mgmt(&priv->wdev, freq, 0, buff, size, 0);
}
-static void wilc_wfi_mgmt_tx_complete(void *priv, int status)
-{
- struct wilc_p2p_mgmt_data *pv_data = priv;
-
- kfree(pv_data->buff);
- kfree(pv_data);
-}
-
static void wilc_wfi_remain_on_channel_expired(void *data, u64 cookie)
{
struct wilc_vif *vif = data;
@@ -1172,7 +1159,6 @@ static int mgmt_tx(struct wiphy *wiphy,
const u8 *buf = params->buf;
size_t len = params->len;
const struct ieee80211_mgmt *mgmt;
- struct wilc_p2p_mgmt_data *mgmt_tx;
struct wilc_vif *vif = netdev_priv(wdev->netdev);
struct wilc_priv *priv = &vif->priv;
struct host_if_drv *wfi_drv = priv->hif_drv;
@@ -1181,6 +1167,7 @@ static int mgmt_tx(struct wiphy *wiphy,
int ie_offset = offsetof(struct ieee80211_mgmt, u) + sizeof(*d);
const u8 *vendor_ie;
int ret = 0;
+ u8 *copy;
*cookie = get_random_u32();
priv->tx_cookie = *cookie;
@@ -1189,21 +1176,12 @@ static int mgmt_tx(struct wiphy *wiphy,
if (!ieee80211_is_mgmt(mgmt->frame_control))
goto out;
- mgmt_tx = kmalloc(sizeof(*mgmt_tx), GFP_KERNEL);
- if (!mgmt_tx) {
+ copy = kmemdup(buf, len, GFP_KERNEL);
+ if (!copy) {
ret = -ENOMEM;
goto out;
}
- mgmt_tx->buff = kmemdup(buf, len, GFP_KERNEL);
- if (!mgmt_tx->buff) {
- ret = -ENOMEM;
- kfree(mgmt_tx);
- goto out;
- }
-
- mgmt_tx->size = len;
-
if (ieee80211_is_probe_resp(mgmt->frame_control)) {
wilc_set_mac_chnl_num(vif, chan->hw_value);
vif->wilc->op_ch = chan->hw_value;
@@ -1230,8 +1208,7 @@ static int mgmt_tx(struct wiphy *wiphy,
goto out_set_timeout;
vendor_ie = cfg80211_find_vendor_ie(WLAN_OUI_WFA, WLAN_OUI_TYPE_WFA_P2P,
- mgmt_tx->buff + ie_offset,
- len - ie_offset);
+ copy + ie_offset, len - ie_offset);
if (!vendor_ie)
goto out_set_timeout;
@@ -1243,10 +1220,8 @@ static int mgmt_tx(struct wiphy *wiphy,
out_txq_add_pkt:
- wilc_wlan_txq_add_mgmt_pkt(wdev->netdev, mgmt_tx,
- mgmt_tx->buff, mgmt_tx->size,
- wilc_wfi_mgmt_tx_complete);
-
+ if (wilc_wlan_txq_add_mgmt_pkt(wdev->netdev, copy, len) < 0)
+ kfree(copy);
out:
return ret;
diff --git a/drivers/net/wireless/microchip/wilc1000/mon.c b/drivers/net/wireless/microchip/wilc1000/mon.c
index 03b7229a0ff5..28c642af943d 100644
--- a/drivers/net/wireless/microchip/wilc1000/mon.c
+++ b/drivers/net/wireless/microchip/wilc1000/mon.c
@@ -95,48 +95,25 @@ void wilc_wfi_monitor_rx(struct net_device *mon_dev, u8 *buff, u32 size)
netif_rx(skb);
}
-struct tx_complete_mon_data {
- int size;
- void *buff;
-};
-
-static void mgmt_tx_complete(void *priv, int status)
-{
- struct tx_complete_mon_data *pv_data = priv;
- /*
- * in case of fully hosting mode, the freeing will be done
- * in response to the cfg packet
- */
- kfree(pv_data->buff);
-
- kfree(pv_data);
-}
-
-static int mon_mgmt_tx(struct net_device *dev, const u8 *buf, size_t len)
+static int mon_mgmt_tx(struct net_device *dev, const u8 *buf, u32 len)
{
- struct tx_complete_mon_data *mgmt_tx = NULL;
+ u8 *copy;
+ int ret = -EFAULT;
if (!dev)
- return -EFAULT;
+ return ret;
netif_stop_queue(dev);
- mgmt_tx = kmalloc(sizeof(*mgmt_tx), GFP_ATOMIC);
- if (!mgmt_tx)
- return -ENOMEM;
-
- mgmt_tx->buff = kmemdup(buf, len, GFP_ATOMIC);
- if (!mgmt_tx->buff) {
- kfree(mgmt_tx);
- return -ENOMEM;
+ copy = kmemdup(buf, len, GFP_ATOMIC);
+ if (!copy) {
+ ret = -ENOMEM;
+ } else {
+ if (wilc_wlan_txq_add_mgmt_pkt(dev, copy, len) < 0)
+ kfree(copy);
+ ret = 0;
}
-
- mgmt_tx->size = len;
-
- wilc_wlan_txq_add_mgmt_pkt(dev, mgmt_tx, mgmt_tx->buff, mgmt_tx->size,
- mgmt_tx_complete);
-
netif_wake_queue(dev);
- return 0;
+ return ret;
}
static netdev_tx_t wilc_wfi_mon_xmit(struct sk_buff *skb,
diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
index e9f59de31b0b..82143d4d16e1 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.c
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
@@ -713,19 +713,28 @@ static void wilc_set_multicast_list(struct net_device *dev)
kfree(mc_list);
}
-static void wilc_tx_complete(void *priv, int status)
+void wilc_tx_complete(struct txq_entry_t *tqe)
{
- struct tx_complete_data *pv_data = priv;
-
- dev_kfree_skb(pv_data->skb);
- kfree(pv_data);
+ switch (tqe->type) {
+ case WILC_NET_PKT:
+ dev_kfree_skb(tqe->skb);
+ break;
+ case WILC_MGMT_PKT:
+ kfree(tqe->buffer);
+ break;
+ case WILC_CFG_PKT:
+ /* nothing */
+ break;
+ default:
+ netdev_err(tqe->vif->ndev, "bad packet type %d\n", tqe->type);
+ break;
+ }
}
netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *ndev)
{
struct wilc_vif *vif = netdev_priv(ndev);
struct wilc *wilc = vif->wilc;
- struct tx_complete_data *tx_data = NULL;
int queue_count;
if (skb->dev != ndev) {
@@ -734,22 +743,15 @@ netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *ndev)
return NETDEV_TX_OK;
}
- tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
- if (!tx_data) {
+ queue_count = wilc_wlan_txq_add_net_pkt(ndev, skb);
+ if (queue_count < 0) {
dev_kfree_skb(skb);
netif_wake_queue(ndev);
return NETDEV_TX_OK;
}
- tx_data->buff = skb->data;
- tx_data->size = skb->len;
- tx_data->skb = skb;
-
vif->netstats.tx_packets++;
- vif->netstats.tx_bytes += tx_data->size;
- queue_count = wilc_wlan_txq_add_net_pkt(ndev, tx_data,
- tx_data->buff, tx_data->size,
- wilc_tx_complete);
+ vif->netstats.tx_bytes += skb->len;
if (queue_count > FLOW_CONTROL_UPPER_THRESHOLD) {
int srcu_idx;
diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.h b/drivers/net/wireless/microchip/wilc1000/netdev.h
index bb1a315a7b7e..b3ba87bd3581 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.h
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.h
@@ -277,6 +277,7 @@ struct wilc_wfi_mon_priv {
struct net_device *real_ndev;
};
+void wilc_tx_complete(struct txq_entry_t *tqe);
void wilc_frmw_to_host(struct wilc *wilc, u8 *buff, u32 size, u32 pkt_offset);
void wilc_mac_indicate(struct wilc *wilc);
void wilc_netdev_cleanup(struct wilc *wilc);
diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.c b/drivers/net/wireless/microchip/wilc1000/wlan.c
index 58bbf50081e4..d594178d05d0 100644
--- a/drivers/net/wireless/microchip/wilc1000/wlan.c
+++ b/drivers/net/wireless/microchip/wilc1000/wlan.c
@@ -219,10 +219,7 @@ static void wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev)
tqe = f->pending_acks[i].txqe;
if (tqe) {
wilc_wlan_txq_remove(wilc, tqe->q_num, tqe);
- tqe->status = 1;
- if (tqe->tx_complete_func)
- tqe->tx_complete_func(tqe->priv,
- tqe->status);
+ wilc_tx_complete(tqe);
kfree(tqe);
dropped++;
}
@@ -272,8 +269,6 @@ static int wilc_wlan_txq_add_cfg_pkt(struct wilc_vif *vif, u8 *buffer,
tqe->type = WILC_CFG_PKT;
tqe->buffer = buffer;
tqe->buffer_size = buffer_size;
- tqe->tx_complete_func = NULL;
- tqe->priv = NULL;
tqe->q_num = AC_VO_Q;
tqe->ack_idx = NOT_TCP_ACK;
tqe->vif = vif;
@@ -410,45 +405,30 @@ static inline u8 ac_change(struct wilc *wilc, u8 *ac)
return 1;
}
-int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
- struct tx_complete_data *tx_data, u8 *buffer,
- u32 buffer_size,
- void (*tx_complete_fn)(void *, int))
+int wilc_wlan_txq_add_net_pkt(struct net_device *dev, struct sk_buff *skb)
{
struct txq_entry_t *tqe;
struct wilc_vif *vif = netdev_priv(dev);
- struct wilc *wilc;
+ struct wilc *wilc = vif->wilc;
u8 q_num;
- wilc = vif->wilc;
-
- if (wilc->quit) {
- tx_complete_fn(tx_data, 0);
- return 0;
- }
-
- if (!wilc->initialized) {
- tx_complete_fn(tx_data, 0);
- return 0;
- }
+ if (wilc->quit || !wilc->initialized)
+ return -1;
tqe = kmalloc(sizeof(*tqe), GFP_ATOMIC);
+ if (!tqe)
+ return -1;
- if (!tqe) {
- tx_complete_fn(tx_data, 0);
- return 0;
- }
tqe->type = WILC_NET_PKT;
- tqe->buffer = buffer;
- tqe->buffer_size = buffer_size;
- tqe->tx_complete_func = tx_complete_fn;
- tqe->priv = tx_data;
+ tqe->skb = skb;
+ tqe->buffer = skb->data;
+ tqe->buffer_size = skb->len;
tqe->vif = vif;
- q_num = ac_classify(wilc, tx_data->skb);
+ q_num = ac_classify(wilc, skb);
tqe->q_num = q_num;
if (ac_change(wilc, &q_num)) {
- tx_complete_fn(tx_data, 0);
+ wilc_tx_complete(tqe);
kfree(tqe);
return 0;
}
@@ -459,43 +439,30 @@ int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
tcp_process(dev, tqe);
wilc_wlan_txq_add_to_tail(dev, q_num, tqe);
} else {
- tx_complete_fn(tx_data, 0);
+ wilc_tx_complete(tqe);
kfree(tqe);
}
return wilc->txq_entries;
}
-int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, void *priv, u8 *buffer,
- u32 buffer_size,
- void (*tx_complete_fn)(void *, int))
+int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, u8 *buffer,
+ u32 buffer_size)
{
struct txq_entry_t *tqe;
struct wilc_vif *vif = netdev_priv(dev);
- struct wilc *wilc;
-
- wilc = vif->wilc;
+ struct wilc *wilc = vif->wilc;
- if (wilc->quit) {
- tx_complete_fn(priv, 0);
- return 0;
- }
+ if (wilc->quit || !wilc->initialized)
+ return -1;
- if (!wilc->initialized) {
- tx_complete_fn(priv, 0);
- return 0;
- }
tqe = kmalloc(sizeof(*tqe), GFP_ATOMIC);
+ if (!tqe)
+ return -1;
- if (!tqe) {
- tx_complete_fn(priv, 0);
- return 0;
- }
tqe->type = WILC_MGMT_PKT;
tqe->buffer = buffer;
tqe->buffer_size = buffer_size;
- tqe->tx_complete_func = tx_complete_fn;
- tqe->priv = priv;
tqe->q_num = AC_BE_Q;
tqe->ack_idx = NOT_TCP_ACK;
tqe->vif = vif;
@@ -916,9 +883,7 @@ int wilc_wlan_handle_txq(struct wilc *wilc, u32 *txq_count)
tqe->buffer, tqe->buffer_size);
offset += vmm_sz;
i++;
- tqe->status = 1;
- if (tqe->tx_complete_func)
- tqe->tx_complete_func(tqe->priv, tqe->status);
+ wilc_tx_complete(tqe);
if (tqe->ack_idx != NOT_TCP_ACK &&
tqe->ack_idx < MAX_PENDING_ACKS)
vif->ack_filter.pending_acks[tqe->ack_idx].txqe = NULL;
@@ -1243,8 +1208,7 @@ void wilc_wlan_cleanup(struct net_device *dev)
wilc->quit = 1;
for (ac = 0; ac < NQUEUES; ac++) {
while ((tqe = wilc_wlan_txq_remove_from_head(wilc, ac))) {
- if (tqe->tx_complete_func)
- tqe->tx_complete_func(tqe->priv, 0);
+ wilc_tx_complete(tqe);
kfree(tqe);
}
}
diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.h b/drivers/net/wireless/microchip/wilc1000/wlan.h
index a72cd5cac81d..f5ba836c595a 100644
--- a/drivers/net/wireless/microchip/wilc1000/wlan.h
+++ b/drivers/net/wireless/microchip/wilc1000/wlan.h
@@ -323,15 +323,13 @@ enum ip_pkt_priority {
struct txq_entry_t {
struct list_head list;
- int type;
+ u8 type;
u8 q_num;
- int ack_idx;
+ s16 ack_idx;
+ struct sk_buff *skb;
u8 *buffer;
- int buffer_size;
- void *priv;
- int status;
+ u32 buffer_size;
struct wilc_vif *vif;
- void (*tx_complete_func)(void *priv, int status);
};
struct txq_fw_recv_queue_stat {
@@ -378,12 +376,6 @@ struct wilc_hif_func {
#define WILC_MAX_CFG_FRAME_SIZE 1468
-struct tx_complete_data {
- int size;
- void *buff;
- struct sk_buff *skb;
-};
-
struct wilc_cfg_cmd_hdr {
u8 cmd_type;
u8 seq_no;
@@ -407,10 +399,7 @@ int wilc_wlan_firmware_download(struct wilc *wilc, const u8 *buffer,
u32 buffer_size);
int wilc_wlan_start(struct wilc *wilc);
int wilc_wlan_stop(struct wilc *wilc, struct wilc_vif *vif);
-int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
- struct tx_complete_data *tx_data, u8 *buffer,
- u32 buffer_size,
- void (*tx_complete_fn)(void *, int));
+int wilc_wlan_txq_add_net_pkt(struct net_device *dev, struct sk_buff *skb);
int wilc_wlan_handle_txq(struct wilc *wl, u32 *txq_count);
void wilc_handle_isr(struct wilc *wilc);
void wilc_wlan_cleanup(struct net_device *dev);
@@ -418,8 +407,8 @@ int wilc_wlan_cfg_set(struct wilc_vif *vif, int start, u16 wid, u8 *buffer,
u32 buffer_size, int commit, u32 drv_handler);
int wilc_wlan_cfg_get(struct wilc_vif *vif, int start, u16 wid, int commit,
u32 drv_handler);
-int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, void *priv, u8 *buffer,
- u32 buffer_size, void (*func)(void *, int));
+int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, u8 *buffer,
+ u32 buffer_size);
void wilc_enable_tcp_ack_filter(struct wilc_vif *vif, bool value);
int wilc_wlan_get_num_conn_ifcs(struct wilc *wilc);
netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *dev);
--
2.41.0
On 7/14/23 07:27, Dmitry Antipov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Not a formal description but some thoughts on Ajay's review:
>
> 1. An initial intention was to remove intermediate data structures
> used only to pass parameters, and related 'kmalloc()/kfree()' calls
> used to manage these structures.
This version of patch, which has no dummy variables and few function
arguments removed, looks better.
>
> 2. Why not use the common cleanup function for all type of frames?
> Since the type is always known (it's recorded in 'struct txq_entry_t'),
> cleanup function may call 'dev_kfree_skb()' or 'kfree()' for plain
> buffers. This also shrinks 'struct txq_entry_t' by one pointer field.>
> 3. IMHO it makes sense to make 'struct txq_entry_t' as small as
> possible. To avoid the redundancy (of storing 'skb' nearby to raw
> buffer and its size) for WILC_NET_PKT frames, it's possible to use
> a union, e.g.:
>
> struct txq_entry_t {
> int type;
> ...
> union {
> /* Used by WILC_NET_PKT entries */
> struct sk_buff *skb;
> /* Used by WILC_MGMT_PKT and WILC_CFG_PKT entries */
> struct {
> u8 *buffer;
> u32 size;
> } data;
> } u;
> ...
> };
>
> but this will require some wrappers to access frame data and size,
> e.g.:
>
> static inline u8 *txq_entry_data(struct txq_entry_t *tqe)
> {
> return (tqe->type == WILC_NET_PKT
> ? tqe->u.skb->data : tqe->u.data.buffer);
> }
>
> static inline u32 txq_entry_size(struct txq_entry_t *tqe)
> {
> return (tqe->type == WILC_NET_PKT
> ? tqe->u.skb->len : tqe->u.data.size);
> }
>
> I'm not sure that this makes sense just to shrink 'struct txq_entry_t'
> by one more pointer field.
>
I think the code without the union, which requires extra wrapper API's,
is clean. The tx_complete function takes care of freeing up the
appropriate buffer so skb buffer would have no impact for mgmt/cfg frames.
> Signed-off-by: Dmitry Antipov <[email protected]>
> ---
> .../wireless/microchip/wilc1000/cfg80211.c | 37 ++-------
> drivers/net/wireless/microchip/wilc1000/mon.c | 47 +++--------
> .../net/wireless/microchip/wilc1000/netdev.c | 34 ++++----
> .../net/wireless/microchip/wilc1000/netdev.h | 1 +
> .../net/wireless/microchip/wilc1000/wlan.c | 80 +++++--------------
> .../net/wireless/microchip/wilc1000/wlan.h | 25 ++----
> 6 files changed, 66 insertions(+), 158 deletions(-)
>
> diff --git a/drivers/net/wireless/microchip/wilc1000/cfg80211.c b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
> index b545d93c6e37..7d244c6c1a92 100644
> --- a/drivers/net/wireless/microchip/wilc1000/cfg80211.c
> +++ b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
> @@ -54,11 +54,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
> };
> #endif
>
> -struct wilc_p2p_mgmt_data {
> - int size;
> - u8 *buff;
> -};
> -
> struct wilc_p2p_pub_act_frame {
> u8 category;
> u8 action;
> @@ -1086,14 +1081,6 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff, u32 size)
> cfg80211_rx_mgmt(&priv->wdev, freq, 0, buff, size, 0);
> }
>
> -static void wilc_wfi_mgmt_tx_complete(void *priv, int status)
> -{
> - struct wilc_p2p_mgmt_data *pv_data = priv;
> -
> - kfree(pv_data->buff);
> - kfree(pv_data);
> -}
> -
> static void wilc_wfi_remain_on_channel_expired(void *data, u64 cookie)
> {
> struct wilc_vif *vif = data;
> @@ -1172,7 +1159,6 @@ static int mgmt_tx(struct wiphy *wiphy,
> const u8 *buf = params->buf;
> size_t len = params->len;
> const struct ieee80211_mgmt *mgmt;
> - struct wilc_p2p_mgmt_data *mgmt_tx;
> struct wilc_vif *vif = netdev_priv(wdev->netdev);
> struct wilc_priv *priv = &vif->priv;
> struct host_if_drv *wfi_drv = priv->hif_drv;
> @@ -1181,6 +1167,7 @@ static int mgmt_tx(struct wiphy *wiphy,
> int ie_offset = offsetof(struct ieee80211_mgmt, u) + sizeof(*d);
> const u8 *vendor_ie;
> int ret = 0;
> + u8 *copy;
>
> *cookie = get_random_u32();
> priv->tx_cookie = *cookie;
> @@ -1189,21 +1176,12 @@ static int mgmt_tx(struct wiphy *wiphy,
> if (!ieee80211_is_mgmt(mgmt->frame_control))
> goto out;
>
> - mgmt_tx = kmalloc(sizeof(*mgmt_tx), GFP_KERNEL);
> - if (!mgmt_tx) {
> + copy = kmemdup(buf, len, GFP_KERNEL);
> + if (!copy) {
> ret = -ENOMEM;
> goto out;
> }
>
> - mgmt_tx->buff = kmemdup(buf, len, GFP_KERNEL);
> - if (!mgmt_tx->buff) {
> - ret = -ENOMEM;
> - kfree(mgmt_tx);
> - goto out;
> - }
> -
> - mgmt_tx->size = len;
> -
> if (ieee80211_is_probe_resp(mgmt->frame_control)) {
> wilc_set_mac_chnl_num(vif, chan->hw_value);
> vif->wilc->op_ch = chan->hw_value;
> @@ -1230,8 +1208,7 @@ static int mgmt_tx(struct wiphy *wiphy,
> goto out_set_timeout;
>
> vendor_ie = cfg80211_find_vendor_ie(WLAN_OUI_WFA, WLAN_OUI_TYPE_WFA_P2P,
> - mgmt_tx->buff + ie_offset,
> - len - ie_offset);
> + copy + ie_offset, len - ie_offset);
> if (!vendor_ie)
> goto out_set_timeout;
>
> @@ -1243,10 +1220,8 @@ static int mgmt_tx(struct wiphy *wiphy,
>
> out_txq_add_pkt:
>
> - wilc_wlan_txq_add_mgmt_pkt(wdev->netdev, mgmt_tx,
> - mgmt_tx->buff, mgmt_tx->size,
> - wilc_wfi_mgmt_tx_complete);
> -
> + if (wilc_wlan_txq_add_mgmt_pkt(wdev->netdev, copy, len) < 0)
> + kfree(copy);
> out:
>
> return ret;
> diff --git a/drivers/net/wireless/microchip/wilc1000/mon.c b/drivers/net/wireless/microchip/wilc1000/mon.c
> index 03b7229a0ff5..28c642af943d 100644
> --- a/drivers/net/wireless/microchip/wilc1000/mon.c
> +++ b/drivers/net/wireless/microchip/wilc1000/mon.c
> @@ -95,48 +95,25 @@ void wilc_wfi_monitor_rx(struct net_device *mon_dev, u8 *buff, u32 size)
> netif_rx(skb);
> }
>
> -struct tx_complete_mon_data {
> - int size;
> - void *buff;
> -};
> -
> -static void mgmt_tx_complete(void *priv, int status)
> -{
> - struct tx_complete_mon_data *pv_data = priv;
> - /*
> - * in case of fully hosting mode, the freeing will be done
> - * in response to the cfg packet
> - */
> - kfree(pv_data->buff);
> -
> - kfree(pv_data);
> -}
> -
> -static int mon_mgmt_tx(struct net_device *dev, const u8 *buf, size_t len)
> +static int mon_mgmt_tx(struct net_device *dev, const u8 *buf, u32 len)
> {
> - struct tx_complete_mon_data *mgmt_tx = NULL;
> + u8 *copy;
> + int ret = -EFAULT;
>
> if (!dev)
> - return -EFAULT;
> + return ret;
>
> netif_stop_queue(dev);
> - mgmt_tx = kmalloc(sizeof(*mgmt_tx), GFP_ATOMIC);
> - if (!mgmt_tx)
> - return -ENOMEM;
> -
> - mgmt_tx->buff = kmemdup(buf, len, GFP_ATOMIC);
> - if (!mgmt_tx->buff) {
> - kfree(mgmt_tx);
> - return -ENOMEM;
> + copy = kmemdup(buf, len, GFP_ATOMIC);
> + if (!copy) {
> + ret = -ENOMEM;
> + } else {
> + if (wilc_wlan_txq_add_mgmt_pkt(dev, copy, len) < 0)
> + kfree(copy);
> + ret = 0;
> }
> -
> - mgmt_tx->size = len;
> -
> - wilc_wlan_txq_add_mgmt_pkt(dev, mgmt_tx, mgmt_tx->buff, mgmt_tx->size,
> - mgmt_tx_complete);
> -
> netif_wake_queue(dev);
> - return 0;
> + return ret;
> }
'mon_mgmt_tx' can be structed as below
{
u8 *copy;
int ret;
if (!dev)
return -EFAULT;
netif_stop_queue(dev);
copy = kmemdup(buf, len, GFP_ATOMIC);
if (!copy) {
netif_wake_queue(dev);
return -ENOMEM;
}
ret = wilc_wlan_txq_add_mgmt_pkt(dev, copy, len);
if (ret < 0)
kfree(copy);
netif_wake_queue(dev);
}
>
> static netdev_tx_t wilc_wfi_mon_xmit(struct sk_buff *skb,
> diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
> index e9f59de31b0b..82143d4d16e1 100644
> --- a/drivers/net/wireless/microchip/wilc1000/netdev.c
> +++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
> @@ -713,19 +713,28 @@ static void wilc_set_multicast_list(struct net_device *dev)
> kfree(mc_list);
> }
>
> -static void wilc_tx_complete(void *priv, int status)
> +void wilc_tx_complete(struct txq_entry_t *tqe)
> {
> - struct tx_complete_data *pv_data = priv;
> -
> - dev_kfree_skb(pv_data->skb);
> - kfree(pv_data);
> + switch (tqe->type) {
> + case WILC_NET_PKT:
> + dev_kfree_skb(tqe->skb);
> + break;
> + case WILC_MGMT_PKT:
> + kfree(tqe->buffer);
> + break;
> + case WILC_CFG_PKT:
> + /* nothing */
> + break;
> + default:
> + netdev_err(tqe->vif->ndev, "bad packet type %d\n", tqe->type);
> + break;
> + }
> }
>
> netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *ndev)
> {
> struct wilc_vif *vif = netdev_priv(ndev);
> struct wilc *wilc = vif->wilc;
> - struct tx_complete_data *tx_data = NULL;
> int queue_count;
>
> if (skb->dev != ndev) {
> @@ -734,22 +743,15 @@ netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *ndev)
> return NETDEV_TX_OK;
> }
>
> - tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
> - if (!tx_data) {
> + queue_count = wilc_wlan_txq_add_net_pkt(ndev, skb);
> + if (queue_count < 0) {
> dev_kfree_skb(skb);
> netif_wake_queue(ndev);
> return NETDEV_TX_OK;
> }
>
> - tx_data->buff = skb->data;
> - tx_data->size = skb->len;
> - tx_data->skb = skb;
> -
> vif->netstats.tx_packets++;
> - vif->netstats.tx_bytes += tx_data->size;
> - queue_count = wilc_wlan_txq_add_net_pkt(ndev, tx_data,
> - tx_data->buff, tx_data->size,
> - wilc_tx_complete);
> + vif->netstats.tx_bytes += skb->len;
>
> if (queue_count > FLOW_CONTROL_UPPER_THRESHOLD) {
> int srcu_idx;
> diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.h b/drivers/net/wireless/microchip/wilc1000/netdev.h
> index bb1a315a7b7e..b3ba87bd3581 100644
> --- a/drivers/net/wireless/microchip/wilc1000/netdev.h
> +++ b/drivers/net/wireless/microchip/wilc1000/netdev.h
> @@ -277,6 +277,7 @@ struct wilc_wfi_mon_priv {
> struct net_device *real_ndev;
> };
>
> +void wilc_tx_complete(struct txq_entry_t *tqe);
> void wilc_frmw_to_host(struct wilc *wilc, u8 *buff, u32 size, u32 pkt_offset);
> void wilc_mac_indicate(struct wilc *wilc);
> void wilc_netdev_cleanup(struct wilc *wilc);
> diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.c b/drivers/net/wireless/microchip/wilc1000/wlan.c
> index 58bbf50081e4..d594178d05d0 100644
> --- a/drivers/net/wireless/microchip/wilc1000/wlan.c
> +++ b/drivers/net/wireless/microchip/wilc1000/wlan.c
> @@ -219,10 +219,7 @@ static void wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev)
> tqe = f->pending_acks[i].txqe;
> if (tqe) {
> wilc_wlan_txq_remove(wilc, tqe->q_num, tqe);
> - tqe->status = 1;
> - if (tqe->tx_complete_func)
> - tqe->tx_complete_func(tqe->priv,
> - tqe->status);
> + wilc_tx_complete(tqe);
> kfree(tqe);
> dropped++;
> }
> @@ -272,8 +269,6 @@ static int wilc_wlan_txq_add_cfg_pkt(struct wilc_vif *vif, u8 *buffer,
> tqe->type = WILC_CFG_PKT;
> tqe->buffer = buffer;
> tqe->buffer_size = buffer_size;
> - tqe->tx_complete_func = NULL;
> - tqe->priv = NULL;
> tqe->q_num = AC_VO_Q;
> tqe->ack_idx = NOT_TCP_ACK;
> tqe->vif = vif;
> @@ -410,45 +405,30 @@ static inline u8 ac_change(struct wilc *wilc, u8 *ac)
> return 1;
> }
>
> -int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
> - struct tx_complete_data *tx_data, u8 *buffer,
> - u32 buffer_size,
> - void (*tx_complete_fn)(void *, int))
> +int wilc_wlan_txq_add_net_pkt(struct net_device *dev, struct sk_buff *skb)
> {
> struct txq_entry_t *tqe;
> struct wilc_vif *vif = netdev_priv(dev);
> - struct wilc *wilc;
> + struct wilc *wilc = vif->wilc;
> u8 q_num;
>
> - wilc = vif->wilc;
> -
> - if (wilc->quit) {
> - tx_complete_fn(tx_data, 0);
> - return 0;
> - }
> -
> - if (!wilc->initialized) {
> - tx_complete_fn(tx_data, 0);
> - return 0;
> - }
> + if (wilc->quit || !wilc->initialized)
> + return -1;
Use macro's to return the error. like -EINVAL.
>
> tqe = kmalloc(sizeof(*tqe), GFP_ATOMIC);
> + if (!tqe)
> + return -1;
>
Same as above.
> - if (!tqe) {
> - tx_complete_fn(tx_data, 0);
> - return 0;
> - }
> tqe->type = WILC_NET_PKT;
> - tqe->buffer = buffer;
> - tqe->buffer_size = buffer_size;
> - tqe->tx_complete_func = tx_complete_fn;
> - tqe->priv = tx_data;
> + tqe->skb = skb;
> + tqe->buffer = skb->data;
> + tqe->buffer_size = skb->len;
> tqe->vif = vif;
>
> - q_num = ac_classify(wilc, tx_data->skb);
> + q_num = ac_classify(wilc, skb);
> tqe->q_num = q_num;
> if (ac_change(wilc, &q_num)) {
> - tx_complete_fn(tx_data, 0);
> + wilc_tx_complete(tqe);
> kfree(tqe);
> return 0;
> }
> @@ -459,43 +439,30 @@ int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
> tcp_process(dev, tqe);
> wilc_wlan_txq_add_to_tail(dev, q_num, tqe);
> } else {
> - tx_complete_fn(tx_data, 0);
> + wilc_tx_complete(tqe);
> kfree(tqe);
> }
>
> return wilc->txq_entries;
> }
>
> -int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, void *priv, u8 *buffer,
> - u32 buffer_size,
> - void (*tx_complete_fn)(void *, int))
> +int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, u8 *buffer,
> + u32 buffer_size)
> {
> struct txq_entry_t *tqe;
> struct wilc_vif *vif = netdev_priv(dev);
> - struct wilc *wilc;
> -
> - wilc = vif->wilc;
> + struct wilc *wilc = vif->wilc;
>
> - if (wilc->quit) {
> - tx_complete_fn(priv, 0);
> - return 0;
> - }
> + if (wilc->quit || !wilc->initialized)
> + return -1;
Same as above.
>
> - if (!wilc->initialized) {
> - tx_complete_fn(priv, 0);
> - return 0;
> - }
> tqe = kmalloc(sizeof(*tqe), GFP_ATOMIC);
> + if (!tqe)
> + return -1;
Same as above.
>
> - if (!tqe) {
> - tx_complete_fn(priv, 0);
> - return 0;
> - }
> tqe->type = WILC_MGMT_PKT;
> tqe->buffer = buffer;
> tqe->buffer_size = buffer_size;
> - tqe->tx_complete_func = tx_complete_fn;
> - tqe->priv = priv;
> tqe->q_num = AC_BE_Q;
> tqe->ack_idx = NOT_TCP_ACK;
> tqe->vif = vif;
> @@ -916,9 +883,7 @@ int wilc_wlan_handle_txq(struct wilc *wilc, u32 *txq_count)
> tqe->buffer, tqe->buffer_size);
> offset += vmm_sz;
> i++;
> - tqe->status = 1;
> - if (tqe->tx_complete_func)
> - tqe->tx_complete_func(tqe->priv, tqe->status);
> + wilc_tx_complete(tqe);
> if (tqe->ack_idx != NOT_TCP_ACK &&
> tqe->ack_idx < MAX_PENDING_ACKS)
> vif->ack_filter.pending_acks[tqe->ack_idx].txqe = NULL;
> @@ -1243,8 +1208,7 @@ void wilc_wlan_cleanup(struct net_device *dev)
> wilc->quit = 1;
> for (ac = 0; ac < NQUEUES; ac++) {
> while ((tqe = wilc_wlan_txq_remove_from_head(wilc, ac))) {
> - if (tqe->tx_complete_func)
> - tqe->tx_complete_func(tqe->priv, 0);
> + wilc_tx_complete(tqe);
> kfree(tqe);
> }
> }
> diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.h b/drivers/net/wireless/microchip/wilc1000/wlan.h
> index a72cd5cac81d..f5ba836c595a 100644
> --- a/drivers/net/wireless/microchip/wilc1000/wlan.h
> +++ b/drivers/net/wireless/microchip/wilc1000/wlan.h
> @@ -323,15 +323,13 @@ enum ip_pkt_priority {
>
> struct txq_entry_t {
> struct list_head list;
> - int type;
> + u8 type;
> u8 q_num;
> - int ack_idx;
> + s16 ack_idx;
> + struct sk_buff *skb;
> u8 *buffer;
> - int buffer_size;
> - void *priv;
> - int status;
> + u32 buffer_size;
> struct wilc_vif *vif;
> - void (*tx_complete_func)(void *priv, int status);
> };
>
> struct txq_fw_recv_queue_stat {
> @@ -378,12 +376,6 @@ struct wilc_hif_func {
>
> #define WILC_MAX_CFG_FRAME_SIZE 1468
>
> -struct tx_complete_data {
> - int size;
> - void *buff;
> - struct sk_buff *skb;
> -};
> -
> struct wilc_cfg_cmd_hdr {
> u8 cmd_type;
> u8 seq_no;
> @@ -407,10 +399,7 @@ int wilc_wlan_firmware_download(struct wilc *wilc, const u8 *buffer,
> u32 buffer_size);
> int wilc_wlan_start(struct wilc *wilc);
> int wilc_wlan_stop(struct wilc *wilc, struct wilc_vif *vif);
> -int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
> - struct tx_complete_data *tx_data, u8 *buffer,
> - u32 buffer_size,
> - void (*tx_complete_fn)(void *, int));
> +int wilc_wlan_txq_add_net_pkt(struct net_device *dev, struct sk_buff *skb);
> int wilc_wlan_handle_txq(struct wilc *wl, u32 *txq_count);
> void wilc_handle_isr(struct wilc *wilc);
> void wilc_wlan_cleanup(struct net_device *dev);
> @@ -418,8 +407,8 @@ int wilc_wlan_cfg_set(struct wilc_vif *vif, int start, u16 wid, u8 *buffer,
> u32 buffer_size, int commit, u32 drv_handler);
> int wilc_wlan_cfg_get(struct wilc_vif *vif, int start, u16 wid, int commit,
> u32 drv_handler);
> -int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, void *priv, u8 *buffer,
> - u32 buffer_size, void (*func)(void *, int));
> +int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, u8 *buffer,
> + u32 buffer_size);
> void wilc_enable_tcp_ack_filter(struct wilc_vif *vif, bool value);
> int wilc_wlan_get_num_conn_ifcs(struct wilc *wilc);
> netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *dev);
> --
> 2.41.0
>
Redesign 'struct txq_entry_t' to avoid callback functions and
replace 'wilc_wfi_mgmt_tx_complete()/mgmt_tx_complete()' with
'wilc_tx_complete()' which operates on 'struct txq_entry_t'
directly. Drop callback-specific 'struct wilc_p2p_mgmt_data',
'struct tx_complete_mon_data' and 'struct tx_complete_data'
with related 'kmalloc()/kfree()' calls, adjust related code.
Signed-off-by: Dmitry Antipov <[email protected]>
---
v2: drop unused function arguments and use convenient error codes
(Ajay Kathat)
---
.../wireless/microchip/wilc1000/cfg80211.c | 37 ++-------
drivers/net/wireless/microchip/wilc1000/mon.c | 47 +++--------
.../net/wireless/microchip/wilc1000/netdev.c | 34 ++++----
.../net/wireless/microchip/wilc1000/netdev.h | 1 +
.../net/wireless/microchip/wilc1000/wlan.c | 80 +++++--------------
.../net/wireless/microchip/wilc1000/wlan.h | 25 ++----
6 files changed, 66 insertions(+), 158 deletions(-)
diff --git a/drivers/net/wireless/microchip/wilc1000/cfg80211.c b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
index b545d93c6e37..7d244c6c1a92 100644
--- a/drivers/net/wireless/microchip/wilc1000/cfg80211.c
+++ b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
@@ -54,11 +54,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
};
#endif
-struct wilc_p2p_mgmt_data {
- int size;
- u8 *buff;
-};
-
struct wilc_p2p_pub_act_frame {
u8 category;
u8 action;
@@ -1086,14 +1081,6 @@ void wilc_wfi_p2p_rx(struct wilc_vif *vif, u8 *buff, u32 size)
cfg80211_rx_mgmt(&priv->wdev, freq, 0, buff, size, 0);
}
-static void wilc_wfi_mgmt_tx_complete(void *priv, int status)
-{
- struct wilc_p2p_mgmt_data *pv_data = priv;
-
- kfree(pv_data->buff);
- kfree(pv_data);
-}
-
static void wilc_wfi_remain_on_channel_expired(void *data, u64 cookie)
{
struct wilc_vif *vif = data;
@@ -1172,7 +1159,6 @@ static int mgmt_tx(struct wiphy *wiphy,
const u8 *buf = params->buf;
size_t len = params->len;
const struct ieee80211_mgmt *mgmt;
- struct wilc_p2p_mgmt_data *mgmt_tx;
struct wilc_vif *vif = netdev_priv(wdev->netdev);
struct wilc_priv *priv = &vif->priv;
struct host_if_drv *wfi_drv = priv->hif_drv;
@@ -1181,6 +1167,7 @@ static int mgmt_tx(struct wiphy *wiphy,
int ie_offset = offsetof(struct ieee80211_mgmt, u) + sizeof(*d);
const u8 *vendor_ie;
int ret = 0;
+ u8 *copy;
*cookie = get_random_u32();
priv->tx_cookie = *cookie;
@@ -1189,21 +1176,12 @@ static int mgmt_tx(struct wiphy *wiphy,
if (!ieee80211_is_mgmt(mgmt->frame_control))
goto out;
- mgmt_tx = kmalloc(sizeof(*mgmt_tx), GFP_KERNEL);
- if (!mgmt_tx) {
+ copy = kmemdup(buf, len, GFP_KERNEL);
+ if (!copy) {
ret = -ENOMEM;
goto out;
}
- mgmt_tx->buff = kmemdup(buf, len, GFP_KERNEL);
- if (!mgmt_tx->buff) {
- ret = -ENOMEM;
- kfree(mgmt_tx);
- goto out;
- }
-
- mgmt_tx->size = len;
-
if (ieee80211_is_probe_resp(mgmt->frame_control)) {
wilc_set_mac_chnl_num(vif, chan->hw_value);
vif->wilc->op_ch = chan->hw_value;
@@ -1230,8 +1208,7 @@ static int mgmt_tx(struct wiphy *wiphy,
goto out_set_timeout;
vendor_ie = cfg80211_find_vendor_ie(WLAN_OUI_WFA, WLAN_OUI_TYPE_WFA_P2P,
- mgmt_tx->buff + ie_offset,
- len - ie_offset);
+ copy + ie_offset, len - ie_offset);
if (!vendor_ie)
goto out_set_timeout;
@@ -1243,10 +1220,8 @@ static int mgmt_tx(struct wiphy *wiphy,
out_txq_add_pkt:
- wilc_wlan_txq_add_mgmt_pkt(wdev->netdev, mgmt_tx,
- mgmt_tx->buff, mgmt_tx->size,
- wilc_wfi_mgmt_tx_complete);
-
+ if (wilc_wlan_txq_add_mgmt_pkt(wdev->netdev, copy, len) < 0)
+ kfree(copy);
out:
return ret;
diff --git a/drivers/net/wireless/microchip/wilc1000/mon.c b/drivers/net/wireless/microchip/wilc1000/mon.c
index 03b7229a0ff5..28c642af943d 100644
--- a/drivers/net/wireless/microchip/wilc1000/mon.c
+++ b/drivers/net/wireless/microchip/wilc1000/mon.c
@@ -95,48 +95,25 @@ void wilc_wfi_monitor_rx(struct net_device *mon_dev, u8 *buff, u32 size)
netif_rx(skb);
}
-struct tx_complete_mon_data {
- int size;
- void *buff;
-};
-
-static void mgmt_tx_complete(void *priv, int status)
-{
- struct tx_complete_mon_data *pv_data = priv;
- /*
- * in case of fully hosting mode, the freeing will be done
- * in response to the cfg packet
- */
- kfree(pv_data->buff);
-
- kfree(pv_data);
-}
-
-static int mon_mgmt_tx(struct net_device *dev, const u8 *buf, size_t len)
+static int mon_mgmt_tx(struct net_device *dev, const u8 *buf, u32 len)
{
- struct tx_complete_mon_data *mgmt_tx = NULL;
+ u8 *copy;
+ int ret = -EFAULT;
if (!dev)
- return -EFAULT;
+ return ret;
netif_stop_queue(dev);
- mgmt_tx = kmalloc(sizeof(*mgmt_tx), GFP_ATOMIC);
- if (!mgmt_tx)
- return -ENOMEM;
-
- mgmt_tx->buff = kmemdup(buf, len, GFP_ATOMIC);
- if (!mgmt_tx->buff) {
- kfree(mgmt_tx);
- return -ENOMEM;
+ copy = kmemdup(buf, len, GFP_ATOMIC);
+ if (!copy) {
+ ret = -ENOMEM;
+ } else {
+ if (wilc_wlan_txq_add_mgmt_pkt(dev, copy, len) < 0)
+ kfree(copy);
+ ret = 0;
}
-
- mgmt_tx->size = len;
-
- wilc_wlan_txq_add_mgmt_pkt(dev, mgmt_tx, mgmt_tx->buff, mgmt_tx->size,
- mgmt_tx_complete);
-
netif_wake_queue(dev);
- return 0;
+ return ret;
}
static netdev_tx_t wilc_wfi_mon_xmit(struct sk_buff *skb,
diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.c b/drivers/net/wireless/microchip/wilc1000/netdev.c
index e9f59de31b0b..82143d4d16e1 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.c
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.c
@@ -713,19 +713,28 @@ static void wilc_set_multicast_list(struct net_device *dev)
kfree(mc_list);
}
-static void wilc_tx_complete(void *priv, int status)
+void wilc_tx_complete(struct txq_entry_t *tqe)
{
- struct tx_complete_data *pv_data = priv;
-
- dev_kfree_skb(pv_data->skb);
- kfree(pv_data);
+ switch (tqe->type) {
+ case WILC_NET_PKT:
+ dev_kfree_skb(tqe->skb);
+ break;
+ case WILC_MGMT_PKT:
+ kfree(tqe->buffer);
+ break;
+ case WILC_CFG_PKT:
+ /* nothing */
+ break;
+ default:
+ netdev_err(tqe->vif->ndev, "bad packet type %d\n", tqe->type);
+ break;
+ }
}
netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *ndev)
{
struct wilc_vif *vif = netdev_priv(ndev);
struct wilc *wilc = vif->wilc;
- struct tx_complete_data *tx_data = NULL;
int queue_count;
if (skb->dev != ndev) {
@@ -734,22 +743,15 @@ netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *ndev)
return NETDEV_TX_OK;
}
- tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
- if (!tx_data) {
+ queue_count = wilc_wlan_txq_add_net_pkt(ndev, skb);
+ if (queue_count < 0) {
dev_kfree_skb(skb);
netif_wake_queue(ndev);
return NETDEV_TX_OK;
}
- tx_data->buff = skb->data;
- tx_data->size = skb->len;
- tx_data->skb = skb;
-
vif->netstats.tx_packets++;
- vif->netstats.tx_bytes += tx_data->size;
- queue_count = wilc_wlan_txq_add_net_pkt(ndev, tx_data,
- tx_data->buff, tx_data->size,
- wilc_tx_complete);
+ vif->netstats.tx_bytes += skb->len;
if (queue_count > FLOW_CONTROL_UPPER_THRESHOLD) {
int srcu_idx;
diff --git a/drivers/net/wireless/microchip/wilc1000/netdev.h b/drivers/net/wireless/microchip/wilc1000/netdev.h
index bb1a315a7b7e..b3ba87bd3581 100644
--- a/drivers/net/wireless/microchip/wilc1000/netdev.h
+++ b/drivers/net/wireless/microchip/wilc1000/netdev.h
@@ -277,6 +277,7 @@ struct wilc_wfi_mon_priv {
struct net_device *real_ndev;
};
+void wilc_tx_complete(struct txq_entry_t *tqe);
void wilc_frmw_to_host(struct wilc *wilc, u8 *buff, u32 size, u32 pkt_offset);
void wilc_mac_indicate(struct wilc *wilc);
void wilc_netdev_cleanup(struct wilc *wilc);
diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.c b/drivers/net/wireless/microchip/wilc1000/wlan.c
index 58bbf50081e4..7493fe053efd 100644
--- a/drivers/net/wireless/microchip/wilc1000/wlan.c
+++ b/drivers/net/wireless/microchip/wilc1000/wlan.c
@@ -219,10 +219,7 @@ static void wilc_wlan_txq_filter_dup_tcp_ack(struct net_device *dev)
tqe = f->pending_acks[i].txqe;
if (tqe) {
wilc_wlan_txq_remove(wilc, tqe->q_num, tqe);
- tqe->status = 1;
- if (tqe->tx_complete_func)
- tqe->tx_complete_func(tqe->priv,
- tqe->status);
+ wilc_tx_complete(tqe);
kfree(tqe);
dropped++;
}
@@ -272,8 +269,6 @@ static int wilc_wlan_txq_add_cfg_pkt(struct wilc_vif *vif, u8 *buffer,
tqe->type = WILC_CFG_PKT;
tqe->buffer = buffer;
tqe->buffer_size = buffer_size;
- tqe->tx_complete_func = NULL;
- tqe->priv = NULL;
tqe->q_num = AC_VO_Q;
tqe->ack_idx = NOT_TCP_ACK;
tqe->vif = vif;
@@ -410,45 +405,30 @@ static inline u8 ac_change(struct wilc *wilc, u8 *ac)
return 1;
}
-int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
- struct tx_complete_data *tx_data, u8 *buffer,
- u32 buffer_size,
- void (*tx_complete_fn)(void *, int))
+int wilc_wlan_txq_add_net_pkt(struct net_device *dev, struct sk_buff *skb)
{
struct txq_entry_t *tqe;
struct wilc_vif *vif = netdev_priv(dev);
- struct wilc *wilc;
+ struct wilc *wilc = vif->wilc;
u8 q_num;
- wilc = vif->wilc;
-
- if (wilc->quit) {
- tx_complete_fn(tx_data, 0);
- return 0;
- }
-
- if (!wilc->initialized) {
- tx_complete_fn(tx_data, 0);
- return 0;
- }
+ if (wilc->quit || !wilc->initialized)
+ return -EINVAL;
tqe = kmalloc(sizeof(*tqe), GFP_ATOMIC);
+ if (!tqe)
+ return -ENOMEM;
- if (!tqe) {
- tx_complete_fn(tx_data, 0);
- return 0;
- }
tqe->type = WILC_NET_PKT;
- tqe->buffer = buffer;
- tqe->buffer_size = buffer_size;
- tqe->tx_complete_func = tx_complete_fn;
- tqe->priv = tx_data;
+ tqe->skb = skb;
+ tqe->buffer = skb->data;
+ tqe->buffer_size = skb->len;
tqe->vif = vif;
- q_num = ac_classify(wilc, tx_data->skb);
+ q_num = ac_classify(wilc, skb);
tqe->q_num = q_num;
if (ac_change(wilc, &q_num)) {
- tx_complete_fn(tx_data, 0);
+ wilc_tx_complete(tqe);
kfree(tqe);
return 0;
}
@@ -459,43 +439,30 @@ int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
tcp_process(dev, tqe);
wilc_wlan_txq_add_to_tail(dev, q_num, tqe);
} else {
- tx_complete_fn(tx_data, 0);
+ wilc_tx_complete(tqe);
kfree(tqe);
}
return wilc->txq_entries;
}
-int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, void *priv, u8 *buffer,
- u32 buffer_size,
- void (*tx_complete_fn)(void *, int))
+int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, u8 *buffer,
+ u32 buffer_size)
{
struct txq_entry_t *tqe;
struct wilc_vif *vif = netdev_priv(dev);
- struct wilc *wilc;
-
- wilc = vif->wilc;
+ struct wilc *wilc = vif->wilc;
- if (wilc->quit) {
- tx_complete_fn(priv, 0);
- return 0;
- }
+ if (wilc->quit || !wilc->initialized)
+ return -EINVAL;
- if (!wilc->initialized) {
- tx_complete_fn(priv, 0);
- return 0;
- }
tqe = kmalloc(sizeof(*tqe), GFP_ATOMIC);
+ if (!tqe)
+ return -ENOMEM;
- if (!tqe) {
- tx_complete_fn(priv, 0);
- return 0;
- }
tqe->type = WILC_MGMT_PKT;
tqe->buffer = buffer;
tqe->buffer_size = buffer_size;
- tqe->tx_complete_func = tx_complete_fn;
- tqe->priv = priv;
tqe->q_num = AC_BE_Q;
tqe->ack_idx = NOT_TCP_ACK;
tqe->vif = vif;
@@ -916,9 +883,7 @@ int wilc_wlan_handle_txq(struct wilc *wilc, u32 *txq_count)
tqe->buffer, tqe->buffer_size);
offset += vmm_sz;
i++;
- tqe->status = 1;
- if (tqe->tx_complete_func)
- tqe->tx_complete_func(tqe->priv, tqe->status);
+ wilc_tx_complete(tqe);
if (tqe->ack_idx != NOT_TCP_ACK &&
tqe->ack_idx < MAX_PENDING_ACKS)
vif->ack_filter.pending_acks[tqe->ack_idx].txqe = NULL;
@@ -1243,8 +1208,7 @@ void wilc_wlan_cleanup(struct net_device *dev)
wilc->quit = 1;
for (ac = 0; ac < NQUEUES; ac++) {
while ((tqe = wilc_wlan_txq_remove_from_head(wilc, ac))) {
- if (tqe->tx_complete_func)
- tqe->tx_complete_func(tqe->priv, 0);
+ wilc_tx_complete(tqe);
kfree(tqe);
}
}
diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.h b/drivers/net/wireless/microchip/wilc1000/wlan.h
index a72cd5cac81d..f5ba836c595a 100644
--- a/drivers/net/wireless/microchip/wilc1000/wlan.h
+++ b/drivers/net/wireless/microchip/wilc1000/wlan.h
@@ -323,15 +323,13 @@ enum ip_pkt_priority {
struct txq_entry_t {
struct list_head list;
- int type;
+ u8 type;
u8 q_num;
- int ack_idx;
+ s16 ack_idx;
+ struct sk_buff *skb;
u8 *buffer;
- int buffer_size;
- void *priv;
- int status;
+ u32 buffer_size;
struct wilc_vif *vif;
- void (*tx_complete_func)(void *priv, int status);
};
struct txq_fw_recv_queue_stat {
@@ -378,12 +376,6 @@ struct wilc_hif_func {
#define WILC_MAX_CFG_FRAME_SIZE 1468
-struct tx_complete_data {
- int size;
- void *buff;
- struct sk_buff *skb;
-};
-
struct wilc_cfg_cmd_hdr {
u8 cmd_type;
u8 seq_no;
@@ -407,10 +399,7 @@ int wilc_wlan_firmware_download(struct wilc *wilc, const u8 *buffer,
u32 buffer_size);
int wilc_wlan_start(struct wilc *wilc);
int wilc_wlan_stop(struct wilc *wilc, struct wilc_vif *vif);
-int wilc_wlan_txq_add_net_pkt(struct net_device *dev,
- struct tx_complete_data *tx_data, u8 *buffer,
- u32 buffer_size,
- void (*tx_complete_fn)(void *, int));
+int wilc_wlan_txq_add_net_pkt(struct net_device *dev, struct sk_buff *skb);
int wilc_wlan_handle_txq(struct wilc *wl, u32 *txq_count);
void wilc_handle_isr(struct wilc *wilc);
void wilc_wlan_cleanup(struct net_device *dev);
@@ -418,8 +407,8 @@ int wilc_wlan_cfg_set(struct wilc_vif *vif, int start, u16 wid, u8 *buffer,
u32 buffer_size, int commit, u32 drv_handler);
int wilc_wlan_cfg_get(struct wilc_vif *vif, int start, u16 wid, int commit,
u32 drv_handler);
-int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, void *priv, u8 *buffer,
- u32 buffer_size, void (*func)(void *, int));
+int wilc_wlan_txq_add_mgmt_pkt(struct net_device *dev, u8 *buffer,
+ u32 buffer_size);
void wilc_enable_tcp_ack_filter(struct wilc_vif *vif, bool value);
int wilc_wlan_get_num_conn_ifcs(struct wilc *wilc);
netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct net_device *dev);
--
2.41.0
<[email protected]> writes:
> Hi Dmitry,
>
> On 7/13/23 01:26, Dmitry Antipov wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>> know the content is safe
>>
>> Drop unused second argument of TX callback functions and use
>> 'struct txq_entry_t *' as the only argument, thus removing
>> 'struct wilc_p2p_mgmt_data', 'struct tx_complete_mon_data'
>> and 'struct tx_complete_data' (actually intended just to
>> pass callbacks parameters) as well. This also shrinks
>> 'struct txq_entry_t' by 'priv' field and eliminates a few
>> 'kmalloc()/kfree()' calls (at the cost of having dummy
>> stack-allocated 'struct txq_entry_t' instances).
>
> I'm just curious to know if you have tested this patch with the real
> hardware.
There was no response from Dmitry but I suspect this was not tested on a
real device. Submitting patches like this without any real testing is a
bad idea as the risk of regressions is high. Please _always_ test
patches on a real device. Only exception are trivial patches which can
be easily reviewed.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 7/25/23 18:08, Kalle Valo wrote:
> There was no response from Dmitry but I suspect this was not tested on a
> real device. Submitting patches like this without any real testing is a
> bad idea as the risk of regressions is high. Please _always_ test
> patches on a real device. Only exception are trivial patches which can
> be easily reviewed.
Note there was a 2nd version of this, see
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected].
I'm not insisting on accepting this patch as is, but hope that developers at
Microchip should be capable to consider it for their next major driver update.
Dmitry
Dmitry Antipov <[email protected]> wrote:
> Redesign 'struct txq_entry_t' to avoid callback functions and
> replace 'wilc_wfi_mgmt_tx_complete()/mgmt_tx_complete()' with
> 'wilc_tx_complete()' which operates on 'struct txq_entry_t'
> directly. Drop callback-specific 'struct wilc_p2p_mgmt_data',
> 'struct tx_complete_mon_data' and 'struct tx_complete_data'
> with related 'kmalloc()/kfree()' calls, adjust related code.
>
> Signed-off-by: Dmitry Antipov <[email protected]>
This needs testing on real device before I would dare to take it.
Patch set to Changes Requested.
--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches