* [PATCH next] wifi: mt76: mt7925: fix error checking in mt7925_mcu_uni_rx/tx_ba()
@ 2025-01-20 9:46 Dan Carpenter
2025-02-18 13:15 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-01-20 9:46 UTC (permalink / raw)
To: Ming Yen Hsieh
Cc: Felix Fietkau, Lorenzo Bianconi, Ryder Lee, Shayne Chen,
Sean Wang, Kalle Valo, Matthias Brugger,
AngeloGioacchino Del Regno, Deren Wu, linux-wireless,
linux-kernel, linux-arm-kernel, linux-mediatek, kernel-janitors
The "ret" variable in mt7925_mcu_uni_[rx/tx]_ba() needs to be signed for
the if (ret < 0) condition to be true.
Also the mt7925_mcu_sta_ba() function returns positive values on success.
The code currently returns whatever non-negative value was returned on
the last iteration. It would be better to return zero on success. This
function is called from mt7925_ampdu_action() which does not check the
return value so the return value doesn't affect runtime. However, it
still makes sense to return zero even though nothing is affected in the
current code.
Fixes: eb2a9a12c609 ("wifi: mt76: mt7925: Update mt7925_mcu_uni_[tx,rx]_ba for MLO")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
drivers/net/wireless/mediatek/mt76/mt7925/mcu.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
index 15815ad84713..b3a00964e802 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
@@ -617,7 +617,8 @@ int mt7925_mcu_uni_tx_ba(struct mt792x_dev *dev,
struct mt792x_bss_conf *mconf;
unsigned long usable_links = ieee80211_vif_usable_links(vif);
struct mt76_wcid *wcid;
- u8 link_id, ret;
+ u8 link_id;
+ int ret;
for_each_set_bit(link_id, &usable_links, IEEE80211_MLD_MAX_NUM_LINKS) {
mconf = mt792x_vif_to_link(mvif, link_id);
@@ -630,10 +631,10 @@ int mt7925_mcu_uni_tx_ba(struct mt792x_dev *dev,
ret = mt7925_mcu_sta_ba(&dev->mt76, &mconf->mt76, wcid, params,
enable, true);
if (ret < 0)
- break;
+ return ret;
}
- return ret;
+ return 0;
}
int mt7925_mcu_uni_rx_ba(struct mt792x_dev *dev,
@@ -647,7 +648,8 @@ int mt7925_mcu_uni_rx_ba(struct mt792x_dev *dev,
struct mt792x_bss_conf *mconf;
unsigned long usable_links = ieee80211_vif_usable_links(vif);
struct mt76_wcid *wcid;
- u8 link_id, ret;
+ u8 link_id;
+ int ret;
for_each_set_bit(link_id, &usable_links, IEEE80211_MLD_MAX_NUM_LINKS) {
mconf = mt792x_vif_to_link(mvif, link_id);
@@ -657,10 +659,10 @@ int mt7925_mcu_uni_rx_ba(struct mt792x_dev *dev,
ret = mt7925_mcu_sta_ba(&dev->mt76, &mconf->mt76, wcid, params,
enable, false);
if (ret < 0)
- break;
+ return ret;
}
- return ret;
+ return 0;
}
static int mt7925_load_clc(struct mt792x_dev *dev, const char *fw_name)
--
2.45.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH next] wifi: mt76: mt7925: fix error checking in mt7925_mcu_uni_rx/tx_ba()
2025-01-20 9:46 [PATCH next] wifi: mt76: mt7925: fix error checking in mt7925_mcu_uni_rx/tx_ba() Dan Carpenter
@ 2025-02-18 13:15 ` Dan Carpenter
2025-02-19 2:15 ` Mingyen Hsieh (謝明諺)
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2025-02-18 13:15 UTC (permalink / raw)
To: Ming Yen Hsieh
Cc: Felix Fietkau, Lorenzo Bianconi, Ryder Lee, Shayne Chen,
Sean Wang, Kalle Valo, Matthias Brugger,
AngeloGioacchino Del Regno, Deren Wu, linux-wireless,
linux-kernel, linux-arm-kernel, linux-mediatek, kernel-janitors
Ping.
regards,
dan carpenter
On Mon, Jan 20, 2025 at 12:46:58PM +0300, Dan Carpenter wrote:
> The "ret" variable in mt7925_mcu_uni_[rx/tx]_ba() needs to be signed for
> the if (ret < 0) condition to be true.
>
> Also the mt7925_mcu_sta_ba() function returns positive values on success.
> The code currently returns whatever non-negative value was returned on
> the last iteration. It would be better to return zero on success. This
> function is called from mt7925_ampdu_action() which does not check the
> return value so the return value doesn't affect runtime. However, it
> still makes sense to return zero even though nothing is affected in the
> current code.
>
> Fixes: eb2a9a12c609 ("wifi: mt76: mt7925: Update mt7925_mcu_uni_[tx,rx]_ba for MLO")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> drivers/net/wireless/mediatek/mt76/mt7925/mcu.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
> index 15815ad84713..b3a00964e802 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
> @@ -617,7 +617,8 @@ int mt7925_mcu_uni_tx_ba(struct mt792x_dev *dev,
> struct mt792x_bss_conf *mconf;
> unsigned long usable_links = ieee80211_vif_usable_links(vif);
> struct mt76_wcid *wcid;
> - u8 link_id, ret;
> + u8 link_id;
> + int ret;
>
> for_each_set_bit(link_id, &usable_links, IEEE80211_MLD_MAX_NUM_LINKS) {
> mconf = mt792x_vif_to_link(mvif, link_id);
> @@ -630,10 +631,10 @@ int mt7925_mcu_uni_tx_ba(struct mt792x_dev *dev,
> ret = mt7925_mcu_sta_ba(&dev->mt76, &mconf->mt76, wcid, params,
> enable, true);
> if (ret < 0)
> - break;
> + return ret;
> }
>
> - return ret;
> + return 0;
> }
>
> int mt7925_mcu_uni_rx_ba(struct mt792x_dev *dev,
> @@ -647,7 +648,8 @@ int mt7925_mcu_uni_rx_ba(struct mt792x_dev *dev,
> struct mt792x_bss_conf *mconf;
> unsigned long usable_links = ieee80211_vif_usable_links(vif);
> struct mt76_wcid *wcid;
> - u8 link_id, ret;
> + u8 link_id;
> + int ret;
>
> for_each_set_bit(link_id, &usable_links, IEEE80211_MLD_MAX_NUM_LINKS) {
> mconf = mt792x_vif_to_link(mvif, link_id);
> @@ -657,10 +659,10 @@ int mt7925_mcu_uni_rx_ba(struct mt792x_dev *dev,
> ret = mt7925_mcu_sta_ba(&dev->mt76, &mconf->mt76, wcid, params,
> enable, false);
> if (ret < 0)
> - break;
> + return ret;
> }
>
> - return ret;
> + return 0;
> }
>
> static int mt7925_load_clc(struct mt792x_dev *dev, const char *fw_name)
> --
> 2.45.2
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH next] wifi: mt76: mt7925: fix error checking in mt7925_mcu_uni_rx/tx_ba()
2025-02-18 13:15 ` Dan Carpenter
@ 2025-02-19 2:15 ` Mingyen Hsieh (謝明諺)
0 siblings, 0 replies; 3+ messages in thread
From: Mingyen Hsieh (謝明諺) @ 2025-02-19 2:15 UTC (permalink / raw)
To: dan.carpenter@linaro.org
Cc: kernel-janitors@vger.kernel.org,
Shayne Chen (陳軒丞), lorenzo@kernel.org,
Ryder Lee, AngeloGioacchino Del Regno, nbd@nbd.name,
kvalo@kernel.org, linux-wireless@vger.kernel.org, Sean Wang,
linux-kernel@vger.kernel.org, matthias.bgg@gmail.com,
linux-arm-kernel@lists.infradead.org,
Deren Wu (武德仁),
linux-mediatek@lists.infradead.org
On Tue, 2025-02-18 at 16:15 +0300, Dan Carpenter wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> Ping.
>
> regards,
> dan carpenter
>
> On Mon, Jan 20, 2025 at 12:46:58PM +0300, Dan Carpenter wrote:
> > The "ret" variable in mt7925_mcu_uni_[rx/tx]_ba() needs to be
> > signed for
> > the if (ret < 0) condition to be true.
> >
> > Also the mt7925_mcu_sta_ba() function returns positive values on
> > success.
> > The code currently returns whatever non-negative value was returned
> > on
> > the last iteration. It would be better to return zero on success.
> > This
> > function is called from mt7925_ampdu_action() which does not check
> > the
> > return value so the return value doesn't affect runtime. However,
> > it
> > still makes sense to return zero even though nothing is affected in
> > the
> > current code.
> >
> > Fixes: eb2a9a12c609 ("wifi: mt76: mt7925: Update
> > mt7925_mcu_uni_[tx,rx]_ba for MLO")
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> > drivers/net/wireless/mediatek/mt76/mt7925/mcu.c | 14 ++++++++-----
> > -
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
> > b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
> > index 15815ad84713..b3a00964e802 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
> > @@ -617,7 +617,8 @@ int mt7925_mcu_uni_tx_ba(struct mt792x_dev
> > *dev,
> > struct mt792x_bss_conf *mconf;
> > unsigned long usable_links = ieee80211_vif_usable_links(vif);
> > struct mt76_wcid *wcid;
> > - u8 link_id, ret;
> > + u8 link_id;
> > + int ret;
> >
> > for_each_set_bit(link_id, &usable_links,
> > IEEE80211_MLD_MAX_NUM_LINKS) {
> > mconf = mt792x_vif_to_link(mvif, link_id);
> > @@ -630,10 +631,10 @@ int mt7925_mcu_uni_tx_ba(struct mt792x_dev
> > *dev,
> > ret = mt7925_mcu_sta_ba(&dev->mt76, &mconf->mt76,
> > wcid, params,
> > enable, true);
> > if (ret < 0)
> > - break;
> > + return ret;
> > }
> >
> > - return ret;
> > + return 0;
> > }
> >
> > int mt7925_mcu_uni_rx_ba(struct mt792x_dev *dev,
> > @@ -647,7 +648,8 @@ int mt7925_mcu_uni_rx_ba(struct mt792x_dev
> > *dev,
> > struct mt792x_bss_conf *mconf;
> > unsigned long usable_links = ieee80211_vif_usable_links(vif);
> > struct mt76_wcid *wcid;
> > - u8 link_id, ret;
> > + u8 link_id;
> > + int ret;
> >
> > for_each_set_bit(link_id, &usable_links,
> > IEEE80211_MLD_MAX_NUM_LINKS) {
> > mconf = mt792x_vif_to_link(mvif, link_id);
> > @@ -657,10 +659,10 @@ int mt7925_mcu_uni_rx_ba(struct mt792x_dev
> > *dev,
> > ret = mt7925_mcu_sta_ba(&dev->mt76, &mconf->mt76,
> > wcid, params,
> > enable, false);
> > if (ret < 0)
> > - break;
> > + return ret;
> > }
> >
> > - return ret;
> > + return 0;
> > }
> >
> > static int mt7925_load_clc(struct mt792x_dev *dev, const char
> > *fw_name)
> > --
> > 2.45.2
> >
Hi Dan,
We plan to revert this patch, and the patch is already ongoing as well.
https://patchwork.kernel.org/project/linux-wireless/patch/20250114020712.704254-1-sean.wang@kernel.org/
Best Regards,
Yen.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-02-19 2:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-20 9:46 [PATCH next] wifi: mt76: mt7925: fix error checking in mt7925_mcu_uni_rx/tx_ba() Dan Carpenter
2025-02-18 13:15 ` Dan Carpenter
2025-02-19 2:15 ` Mingyen Hsieh (謝明諺)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox