Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Message-ID: <35c14b437f8c4b61e2fc14c6b156bf5de330a493.camel@sipsolutions.net>
Subject: Re: [PATCH v3] wifi: mac80211: add exported tpt_led_trig function
 for softmac driver
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Yi-Chia Hsieh <yi-chia.hsieh@mediatek.com>,
        Felix Fietkau <nbd@nbd.name>
Cc:     Lorenzo Bianconi <lorenzo.bianconi@redhat.com>,
        Ryder Lee <ryder.lee@mediatek.com>,
        Shayne Chen <shayne.chen@mediatek.com>,
        Evelyn Tsai <evelyn.tsai@mediatek.com>,
        Money Wang <money.wang@mediatek.com>,
        Peter Chiu <chui-hao.chiu@mediatek.com>,
        Benjamin Lin <benjamin-jw.lin@mediatek.com>,
        linux-wireless@vger.kernel.org, linux-mediatek@lists.infradead.org
Date:   Mon, 25 Sep 2023 08:38:14 +0200
In-Reply-To: <038706d9bbf434642013e880300d4f597f13b514.1695329290.git.yi-chia.hsieh@mediatek.com>
References: <038706d9bbf434642013e880300d4f597f13b514.1695329290.git.yi-chia.hsieh@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk


> +++ b/include/net/mac80211.h
> @@ -4733,6 +4733,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_trx(struct ieee80211_hw *hw,
> +				  int tx_bytes, int rx_bytes);

Hm, I think you misunderstood what I said.

I still think you need to have a well-documented function or two here,
but now you can make the exported versions of this be inlines calling
this new function.

But also this now fails the build if !CONFIG_MAC80211_LEDS, you need to
have an ifdef somewhere.

> +void __ieee80211_tpt_led_trig_trx(struct ieee80211_hw *hw,
> +				  int tx_bytes, int rx_bytes)
> +{
> +	struct ieee80211_local *local =3D hw_to_local(hw);
> +
> +	if (atomic_read(&local->tpt_led_active)) {
> +		local->tpt_led_trigger->tx_bytes +=3D tx_bytes;
> +		local->tpt_led_trigger->rx_bytes +=3D rx_bytes;
> +	}
> +}
> +EXPORT_SYMBOL(__ieee80211_tpt_led_trig_trx);

Even that won't build.

> --- a/net/mac80211/led.h
> +++ b/net/mac80211/led.h
> @@ -71,8 +71,7 @@ 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 +=3D bytes;
> +	__ieee80211_tpt_led_trig_trx(&local->hw, bytes, 0);
>  #endif

Ah. Actually what I was thinking is that __ieee80211_tpt_led_trig_trx is
still an inline, and then the ieee80211_tpt_led_trig_trx() exported to
drivers can call this alongside these inlines - that way mac80211 still
gets the (trivial) code inlined.

johannes