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

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

[Martin Blumenstingl]

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/20230218152944.48842-1-martin.blumenstingl@googlemail.com/


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

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

[Martin Blumenstingl]

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 <martin.blumenstingl@googlemail.com>
---
 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

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

[Ping-Ke Shih]

> -----Original Message-----
> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Sent: Monday, February 27, 2023 6:10 AM
> To: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; kvalo@kernel.org; tony0620emma@gmail.com;
> Ping-Ke Shih <pkshih@realtek.com>; Neo Jou <neojou@gmail.com>; Martin Blumenstingl
> <martin.blumenstingl@googlemail.com>
> 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 <martin.blumenstingl@googlemail.com>

Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>

> ---
>  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

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

[Kalle Valo]

Martin Blumenstingl <martin.blumenstingl@googlemail.com> 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 <martin.blumenstingl@googlemail.com>
> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>

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/20230226221004.138331-2-martin.blumenstingl@googlemail.com/

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

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

[Martin Blumenstingl]

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 <martin.blumenstingl@googlemail.com>
---
 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

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

[Ping-Ke Shih]

> -----Original Message-----
> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Sent: Monday, February 27, 2023 6:10 AM
> To: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; kvalo@kernel.org; tony0620emma@gmail.com;
> Ping-Ke Shih <pkshih@realtek.com>; Neo Jou <neojou@gmail.com>; Martin Blumenstingl
> <martin.blumenstingl@googlemail.com>
> 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 <martin.blumenstingl@googlemail.com>

Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>

> ---
>  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