diff mbox series

wifi: wilc1000: simplify and cleanup TX paths

Message ID 20230714142726.129126-1-dmantipov@yandex.ru
State Superseded
Headers show
Series wifi: wilc1000: simplify and cleanup TX paths | expand

Commit Message

Dmitry Antipov July 14, 2023, 2:27 p.m. UTC
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 <dmantipov@yandex.ru>
---
 .../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(-)

Comments

Ajay Singh July 14, 2023, 11:05 p.m. UTC | #1
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 <dmantipov@yandex.ru>
> ---
>  .../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
>
diff mbox series

Patch

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);