diff mbox series

[1/3] wifi: ath9k: cleanup ath_txq_skb_done()

Message ID 20241209155027.636400-1-dmantipov@yandex.ru
State New
Headers show
Series [1/3] wifi: ath9k: cleanup ath_txq_skb_done() | expand

Commit Message

Dmitry Antipov Dec. 9, 2024, 3:50 p.m. UTC
Since 'txq' argument of 'ath_txq_skb_done()' is actually
(mis|un)used, convert the former to local variable and
adjust all related users. Compile tested only.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/net/wireless/ath/ath9k/xmit.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Toke Høiland-Jørgensen Dec. 10, 2024, 9:58 a.m. UTC | #1
Dmitry Antipov <dmantipov@yandex.ru> writes:

> Prefer 'ktime_t' over 'struct timespec64' for 'struct ath_chanctx' and
> 'struct ath_softc' timestamps, choose standard kernel time API over an
> ad-hoc math in 'chanctx_event_delta()' and 'ath9k_hw_get_tsf_offset()',
> adjust related users. Compile tested only.
>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>

Nice cleanup! Just one formatting nit:

[...]
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index 2f137856a823..cf664a0dedaa 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -247,10 +247,8 @@ static bool ath_complete_reset(struct ath_softc *sc, bool start)
>  	if (!sc->cur_chan->offchannel && start) {
>  		/* restore per chanctx TSF timer */
>  		if (sc->cur_chan->tsf_val) {
> -			u32 offset;
> -
> -			offset = ath9k_hw_get_tsf_offset(&sc->cur_chan->tsf_ts,
> -							 NULL);
> +			u32 offset = ath9k_hw_get_tsf_offset
> +				(sc->cur_chan->tsf_ts, 0);

This turned into a really odd line break. Let's just keep the variable
definition on its own line like it was before, so we can keep the
function call the way it is as well...

-Toke
Toke Høiland-Jørgensen Dec. 10, 2024, 9:59 a.m. UTC | #2
Dmitry Antipov <dmantipov@yandex.ru> writes:

> Since 'txq' argument of 'ath_txq_skb_done()' is actually
> (mis|un)used, convert the former to local variable and
> adjust all related users. Compile tested only.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>

Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
Dmitry Antipov Dec. 10, 2024, 11:37 a.m. UTC | #3
On 12/10/24 12:58 PM, Toke Høiland-Jørgensen wrote:

> Nice cleanup! Just one formatting nit:

Thanks. Feel free to merge with any formatting adjustments.

Dmitry
Toke Høiland-Jørgensen Dec. 14, 2024, 5:29 p.m. UTC | #4
Toke Høiland-Jørgensen <toke@toke.dk> writes:

> Dmitry Antipov <dmantipov@yandex.ru> writes:
>
>> Prefer 'ktime_t' over 'struct timespec64' for 'struct ath_chanctx' and
>> 'struct ath_softc' timestamps, choose standard kernel time API over an
>> ad-hoc math in 'chanctx_event_delta()' and 'ath9k_hw_get_tsf_offset()',
>> adjust related users. Compile tested only.
>>
>> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
>
> Nice cleanup! Just one formatting nit:
>
> [...]
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index 2f137856a823..cf664a0dedaa 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -247,10 +247,8 @@ static bool ath_complete_reset(struct ath_softc *sc, bool start)
>>  	if (!sc->cur_chan->offchannel && start) {
>>  		/* restore per chanctx TSF timer */
>>  		if (sc->cur_chan->tsf_val) {
>> -			u32 offset;
>> -
>> -			offset = ath9k_hw_get_tsf_offset(&sc->cur_chan->tsf_ts,
>> -							 NULL);
>> +			u32 offset = ath9k_hw_get_tsf_offset
>> +				(sc->cur_chan->tsf_ts, 0);
>
> This turned into a really odd line break. Let's just keep the variable
> definition on its own line like it was before, so we can keep the
> function call the way it is as well...

And since Jeff has agreed to fix the line break when applying, with that
fixed:

Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 35aa47a9db90..fae96e3d66dd 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -208,10 +208,10 @@  static void ath_set_rates(struct ieee80211_vif *vif, struct ieee80211_sta *sta,
 				       ARRAY_SIZE(bf->rates));
 }
 
-static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
-			     struct sk_buff *skb)
+static void ath_txq_skb_done(struct ath_softc *sc, struct sk_buff *skb)
 {
 	struct ath_frame_info *fi = get_frame_info(skb);
+	struct ath_txq *txq;
 	int q = fi->txq;
 
 	if (q < 0)
@@ -294,7 +294,7 @@  static void ath_tx_flush_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
 		fi = get_frame_info(skb);
 		bf = fi->bf;
 		if (!bf) {
-			ath_txq_skb_done(sc, txq, skb);
+			ath_txq_skb_done(sc, skb);
 			ieee80211_free_txskb(sc->hw, skb);
 			continue;
 		}
@@ -962,7 +962,7 @@  ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
 			bf->bf_state.stale = false;
 
 		if (!bf) {
-			ath_txq_skb_done(sc, txq, skb);
+			ath_txq_skb_done(sc, skb);
 			ieee80211_free_txskb(sc->hw, skb);
 			continue;
 		}
@@ -2379,7 +2379,7 @@  int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
 
 	bf = ath_tx_setup_buffer(sc, txq, tid, skb);
 	if (!bf) {
-		ath_txq_skb_done(sc, txq, skb);
+		ath_txq_skb_done(sc, skb);
 		if (txctl->paprd)
 			dev_kfree_skb_any(skb);
 		else
@@ -2514,7 +2514,7 @@  static void ath_tx_complete(struct ath_softc *sc, struct sk_buff *skb,
 	}
 	spin_unlock_irqrestore(&sc->sc_pm_lock, flags);
 
-	ath_txq_skb_done(sc, txq, skb);
+	ath_txq_skb_done(sc, skb);
 	tx_info->status.status_driver_data[0] = sta;
 	__skb_queue_tail(&txq->complete_q, skb);
 }