[PATCH 5/6] wifi: mac80211: export ieee80211_tpt_led_trig_tx/rx for driver

[Jeff Johnson]

On 8/25/2023 10:47 AM, Yi-Chia Hsieh wrote:
> Whenever the H/W path is enabled and traffic is in the binding state,
> mac80211 is not aware of the traffic. Consequently, the LED does not
> blink for that reason.
> 
> The ieee80211_tpt_led_trig_tx/rx functions are exported for the driver
> so that we can report the tx and rx bytes from the driver when
> the H/W path is being used.
> 
> Signed-off-by: Yi-Chia Hsieh <yi-chia.hsieh at mediatek.com>
> ---
>   include/net/mac80211.h | 17 +++++++++++++++++
>   net/mac80211/led.c     | 18 ++++++++++++++++++
>   net/mac80211/led.h     | 18 ------------------
>   net/mac80211/rx.c      |  2 +-
>   net/mac80211/tx.c      |  4 ++--
>   5 files changed, 38 insertions(+), 21 deletions(-)
> 
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 3a8a2d2c58c3..53804822dc8d 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -4729,6 +4729,8 @@ __ieee80211_create_tpt_led_trigger(struct ieee80211_hw *hw,
>   				   unsigned int flags,
>   				   const struct ieee80211_tpt_blink *blink_table,
>   				   unsigned int blink_table_len);
> +void __ieee80211_tpt_led_trig_tx(struct ieee80211_hw *hw, int bytes);
> +void __ieee80211_tpt_led_trig_rx(struct ieee80211_hw *hw, int bytes);

suggest these should be in led.h since they are local to mac80211 and 
should not be directly invoked by clients of mac80211

>   #endif
>   /**
>    * ieee80211_get_tx_led_name - get name of TX LED
> @@ -4839,6 +4841,21 @@ ieee80211_create_tpt_led_trigger(struct ieee80211_hw *hw, unsigned int flags,
>   #endif
>   }
>   
> +static inline void
> +ieee80211_tpt_led_trig_tx(struct ieee80211_hw *hw, int bytes)
> +{
> +#ifdef CPTCFG_MAC80211_LEDS

why isn't this using CONFIG_MAC80211_LEDS??

> +	__ieee80211_tpt_led_trig_tx(hw, bytes);
> +#endif
> +}
> +
> +static inline void
> +ieee80211_tpt_led_trig_rx(struct ieee80211_hw *hw, int bytes)
> +{
> +#ifdef CPTCFG_MAC80211_LEDS
> +	__ieee80211_tpt_led_trig_rx(hw, bytes);
> +#endif
> +}
>   /**
>    * ieee80211_unregister_hw - Unregister a hardware device
>    *
> diff --git a/net/mac80211/led.c b/net/mac80211/led.c
> index 2dc732147e85..af03a2ef5c6a 100644
> --- a/net/mac80211/led.c
> +++ b/net/mac80211/led.c
> @@ -319,6 +319,24 @@ __ieee80211_create_tpt_led_trigger(struct ieee80211_hw *hw,
>   }
>   EXPORT_SYMBOL(__ieee80211_create_tpt_led_trigger);
>   
> +void __ieee80211_tpt_led_trig_tx(struct ieee80211_hw *hw, int bytes)
> +{
> +	struct ieee80211_local *local = hw_to_local(hw);
> +
> +	if (atomic_read(&local->tpt_led_active))
> +		local->tpt_led_trigger->tx_bytes += bytes;
> +}
> +EXPORT_SYMBOL(__ieee80211_tpt_led_trig_tx);
> +
> +void __ieee80211_tpt_led_trig_rx(struct ieee80211_hw *hw, int bytes)
> +{
> +	struct ieee80211_local *local = hw_to_local(hw);
> +
> +	if (atomic_read(&local->tpt_led_active))
> +		local->tpt_led_trigger->rx_bytes += bytes;
> +}
> +EXPORT_SYMBOL(__ieee80211_tpt_led_trig_rx);
> +

why are you making these exported implementations instead of keeping 
them as inline in led.h?

>   static void ieee80211_start_tpt_led_trig(struct ieee80211_local *local)
>   {
>   	struct tpt_led_trigger *tpt_trig = local->tpt_led_trigger;
> diff --git a/net/mac80211/led.h b/net/mac80211/led.h
> index d25f13346b82..98db4356d0de 100644
> --- a/net/mac80211/led.h
> +++ b/net/mac80211/led.h
> @@ -66,21 +66,3 @@ static inline void ieee80211_mod_tpt_led_trig(struct ieee80211_local *local,
>   {
>   }
>   #endif
> -
> -static inline void
> -ieee80211_tpt_led_trig_tx(struct ieee80211_local *local, int bytes)
> -{
> -#ifdef CONFIG_MAC80211_LEDS
> -	if (atomic_read(&local->tpt_led_active))
> -		local->tpt_led_trigger->tx_bytes += bytes;
> -#endif
> -}
> -
> -static inline void
> -ieee80211_tpt_led_trig_rx(struct ieee80211_local *local, int bytes)
> -{
> -#ifdef CONFIG_MAC80211_LEDS
> -	if (atomic_read(&local->tpt_led_active))
> -		local->tpt_led_trigger->rx_bytes += bytes;
> -#endif
> -}
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 4f707d2a160f..5747d7dac4d7 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -5375,7 +5375,7 @@ void ieee80211_rx_list(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta,
>   	if (skb) {
>   		if ((status->flag & RX_FLAG_8023) ||
>   			ieee80211_is_data_present(hdr->frame_control))
> -			ieee80211_tpt_led_trig_rx(local, skb->len);
> +			ieee80211_tpt_led_trig_rx(&local->hw, skb->len);

what is the rationale for changing the signature, especially given that 
the first thing the implementations do is local = hw_to_local(hw)

>   
>   		if (status->flag & RX_FLAG_8023)
>   			__ieee80211_rx_handle_8023(hw, pubsta, skb, list);
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 7fe7280e8437..234de8d3b8bb 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -4320,7 +4320,7 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb,
>   	len = 0;
>    out:
>   	if (len)
> -		ieee80211_tpt_led_trig_tx(local, len);
> +		ieee80211_tpt_led_trig_tx(&local->hw, len);
>   	rcu_read_unlock();
>   }
>   
> @@ -4646,7 +4646,7 @@ static void ieee80211_8023_xmit(struct ieee80211_sub_if_data *sdata,
>   	sta->deflink.tx_stats.packets[queue] += skbs;
>   	sta->deflink.tx_stats.bytes[queue] += len;
>   
> -	ieee80211_tpt_led_trig_tx(local, len);
> +	ieee80211_tpt_led_trig_tx(&local->hw, len);
>   
>   	ieee80211_tx_8023(sdata, skb, sta, false);
>