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

[Ping-Ke Shih <pkshih@realtek.com>]

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