| Message ID | 20230713082616.112160-1-dmantipov@yandex.ru |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: wilc1000: simplify TX callback functions | expand |
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 <dmantipov@yandex.ru> > --- > .../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 >
<Ajay.Kathat@microchip.com> 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.
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/20230717084400.57882-1-dmantipov@yandex.ru. 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
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);
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 <dmantipov@yandex.ru> --- .../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(-)