LinuxLists.cc - [PATCH v1 wireless-next 0/2] wifi: rtw88: two small error propagation fixes

2023-02-26 22:10:52

Subject: [PATCH v1 wireless-next 0/2] wifi: rtw88: two small error propagation fixes

While updating the rtw88 sdio patches I found two more improvements for
the existing code. This series targets wireless-next and is meant to be
applied on top of my previous series v2 "rtw88: Add additional SDIO
support bits" from [0]. I decide to target wireless-next because these
patches are not fixing any issues visible to the end user. Worst case
is that the code is shadowing some error codes.

The next series that I'll send after this is the addition of the rtw88
SDIO HCI and adding the first driver(s) to utilize this code.

[0] https://lore.kernel.org/lkml/[email protected]/

Martin Blumenstingl (2):
wifi: rtw88: mac: Return the original error from rtw_pwr_seq_parser()
wifi: rtw88: mac: Return the original error from
rtw_mac_power_switch()

drivers/net/wireless/realtek/rtw88/mac.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

--
2.39.2

2023-02-26 22:10:55

by Martin Blumenstingl

[permalink] [raw]

Subject: [PATCH v1 wireless-next 1/2] wifi: rtw88: mac: Return the original error from rtw_pwr_seq_parser()

rtw_pwr_seq_parser() calls rtw_sub_pwr_seq_parser() which can either
return -EBUSY, -EINVAL or 0. Propagate the original error code instead
of unconditionally returning -EBUSY in case of an error.

Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/net/wireless/realtek/rtw88/mac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
index 1c9530a0eb69..4749d75fefee 100644
--- a/drivers/net/wireless/realtek/rtw88/mac.c
+++ b/drivers/net/wireless/realtek/rtw88/mac.c
@@ -236,7 +236,7 @@ static int rtw_pwr_seq_parser(struct rtw_dev *rtwdev,

ret = rtw_sub_pwr_seq_parser(rtwdev, intf_mask, cut_mask, cmd);
if (ret)
- return -EBUSY;
+ return ret;

idx++;
} while (1);
--
2.39.2

2023-02-26 22:10:57

by Martin Blumenstingl

[permalink] [raw]

Subject: [PATCH v1 wireless-next 2/2] wifi: rtw88: mac: Return the original error from rtw_mac_power_switch()

rtw_mac_power_switch() calls rtw_pwr_seq_parser() which can return
-EINVAL, -EBUSY or 0. Propagate the original error code instead of
unconditionally returning -EINVAL in case of an error.

Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/net/wireless/realtek/rtw88/mac.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
index 4749d75fefee..f3a566cf979b 100644
--- a/drivers/net/wireless/realtek/rtw88/mac.c
+++ b/drivers/net/wireless/realtek/rtw88/mac.c
@@ -250,6 +250,7 @@ static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on)
const struct rtw_pwr_seq_cmd **pwr_seq;
u8 rpwm;
bool cur_pwr;
+ int ret;

if (rtw_chip_wcpu_11ac(rtwdev)) {
rpwm = rtw_read8(rtwdev, rtwdev->hci.rpwm_addr);
@@ -273,8 +274,9 @@ static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on)
return -EALREADY;

pwr_seq = pwr_on ? chip->pwr_on_seq : chip->pwr_off_seq;
- if (rtw_pwr_seq_parser(rtwdev, pwr_seq))
- return -EINVAL;
+ ret = rtw_pwr_seq_parser(rtwdev, pwr_seq);
+ if (ret)
+ return ret;

if (pwr_on)
set_bit(RTW_FLAG_POWERON, rtwdev->flags);
--
2.39.2

2023-03-01 01:51:52

by Ping-Ke Shih

[permalink] [raw]

Subject: RE: [PATCH v1 wireless-next 2/2] wifi: rtw88: mac: Return the original error from rtw_mac_power_switch()

> -----Original Message-----
> From: Martin Blumenstingl <[email protected]>
> Sent: Monday, February 27, 2023 6:10 AM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected];
> Ping-Ke Shih <[email protected]>; Neo Jou <[email protected]>; Martin Blumenstingl
> <[email protected]>
> Subject: [PATCH v1 wireless-next 2/2] wifi: rtw88: mac: Return the original error from
> rtw_mac_power_switch()
>
> rtw_mac_power_switch() calls rtw_pwr_seq_parser() which can return
> -EINVAL, -EBUSY or 0. Propagate the original error code instead of
> unconditionally returning -EINVAL in case of an error.
>
> Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
> Signed-off-by: Martin Blumenstingl <[email protected]>

Reviewed-by: Ping-Ke Shih <[email protected]>

> ---
> drivers/net/wireless/realtek/rtw88/mac.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
> index 4749d75fefee..f3a566cf979b 100644
> --- a/drivers/net/wireless/realtek/rtw88/mac.c
> +++ b/drivers/net/wireless/realtek/rtw88/mac.c
> @@ -250,6 +250,7 @@ static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on)
> const struct rtw_pwr_seq_cmd **pwr_seq;
> u8 rpwm;
> bool cur_pwr;
> + int ret;
>
> if (rtw_chip_wcpu_11ac(rtwdev)) {
> rpwm = rtw_read8(rtwdev, rtwdev->hci.rpwm_addr);
> @@ -273,8 +274,9 @@ static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on)
> return -EALREADY;

I think a reason why we don't propagate return value is special deal of EALREADY
by caller. Since this driver becomes stable and no others use EALREADY as error code,
this patchset will be okay.

>
> pwr_seq = pwr_on ? chip->pwr_on_seq : chip->pwr_off_seq;
> - if (rtw_pwr_seq_parser(rtwdev, pwr_seq))
> - return -EINVAL;
> + ret = rtw_pwr_seq_parser(rtwdev, pwr_seq);
> + if (ret)
> + return ret;
>
> if (pwr_on)
> set_bit(RTW_FLAG_POWERON, rtwdev->flags);
> --
> 2.39.2

2023-03-01 01:52:23

by Ping-Ke Shih

[permalink] [raw]

Subject: RE: [PATCH v1 wireless-next 1/2] wifi: rtw88: mac: Return the original error from rtw_pwr_seq_parser()

2023-03-06 10:10:36

by Kalle Valo

[permalink] [raw]

Subject: Re: [PATCH v1 wireless-next 1/2] wifi: rtw88: mac: Return the original error from rtw_pwr_seq_parser()

Martin Blumenstingl <[email protected]> wrote:

> rtw_pwr_seq_parser() calls rtw_sub_pwr_seq_parser() which can either
> return -EBUSY, -EINVAL or 0. Propagate the original error code instead
> of unconditionally returning -EBUSY in case of an error.
>
> Fixes: e3037485c68e ("rtw88: new Realtek 802.11ac driver")
> Signed-off-by: Martin Blumenstingl <[email protected]>
> Reviewed-by: Ping-Ke Shih <[email protected]>

2 patches applied to wireless-next.git, thanks.

b7ed9fa2cb76 wifi: rtw88: mac: Return the original error from rtw_pwr_seq_parser()
15c8e267dfa6 wifi: rtw88: mac: Return the original error from rtw_mac_power_switch()

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches