diff mbox series

[v3,1/9] wifi: rtw88: Clear RTW_FLAG_POWERON early in rtw_mac_power_switch()

Message ID 20230320213508.2358213-2-martin.blumenstingl@googlemail.com
State Superseded
Headers show
Series rtw88: Add SDIO support | expand

Commit Message

Martin Blumenstingl March 20, 2023, 9:35 p.m. UTC
The SDIO HCI implementation needs to know when the MAC is powered on.
This is needed because 32-bit register access has to be split into 4x
8-bit register access when the MAC is not fully powered on or while
powering off. When the MAC is powered on 32-bit register access can be
used to reduce the number of transfers but splitting into 4x 8-bit
register access still works in that case.

During the power on sequence is how RTW_FLAG_POWERON is only set when
the power on sequence has completed successfully. During power off
however RTW_FLAG_POWERON is set. This means that the upcoming SDIO HCI
implementation does not know that it has to use 4x 8-bit register
accessors. Clear the RTW_FLAG_POWERON flag early when powering off the
MAC so the whole power off sequence is processed with RTW_FLAG_POWERON
unset. This will make it possible to use the RTW_FLAG_POWERON flag in
the upcoming SDIO HCI implementation.

Note that a failure in rtw_pwr_seq_parser() while applying
chip->pwr_off_seq can theoretically result in the RTW_FLAG_POWERON
flag being cleared while the chip is still powered on. However,
depending on when the failure occurs in the power off sequence the
chip may be on or off. Even the original approach of clearing
RTW_FLAG_POWERON only when the power off sequence has been applied
successfully could end up in some corner case where the chip is
powered off but RTW_FLAG_POWERON was not cleared.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
Changes since v2:
- improve patch description about corner cases when clearing
  RTW_FLAG_POWERON

Changes since v1:
- This replaces a previous patch called "rtw88: hci: Add an optional
  power_switch() callback to rtw_hci_ops" which added a new callback
  to the HCI ops.


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

Comments

Ping-Ke Shih March 23, 2023, 2:02 a.m. UTC | #1
> -----Original Message-----
> From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Sent: Tuesday, March 21, 2023 5:35 AM
> To: linux-wireless@vger.kernel.org
> Cc: Yan-Hsuan Chuang <tony0620emma@gmail.com>; Kalle Valo <kvalo@kernel.org>; Ulf Hansson
> <ulf.hansson@linaro.org>; linux-kernel@vger.kernel.org; netdev@vger.kernel.org;
> linux-mmc@vger.kernel.org; Chris Morgan <macroalpha82@gmail.com>; Nitin Gupta <nitin.gupta981@gmail.com>;
> Neo Jou <neojou@gmail.com>; Ping-Ke Shih <pkshih@realtek.com>; Jernej Skrabec <jernej.skrabec@gmail.com>;
> Larry Finger <Larry.Finger@lwfinger.net>; Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Subject: [PATCH v3 1/9] wifi: rtw88: Clear RTW_FLAG_POWERON early in rtw_mac_power_switch()
> 
> The SDIO HCI implementation needs to know when the MAC is powered on.
> This is needed because 32-bit register access has to be split into 4x
> 8-bit register access when the MAC is not fully powered on or while
> powering off. When the MAC is powered on 32-bit register access can be
> used to reduce the number of transfers but splitting into 4x 8-bit
> register access still works in that case.
> 
> During the power on sequence is how RTW_FLAG_POWERON is only set when
> the power on sequence has completed successfully. During power off
> however RTW_FLAG_POWERON is set. This means that the upcoming SDIO HCI
> implementation does not know that it has to use 4x 8-bit register
> accessors. Clear the RTW_FLAG_POWERON flag early when powering off the
> MAC so the whole power off sequence is processed with RTW_FLAG_POWERON
> unset. This will make it possible to use the RTW_FLAG_POWERON flag in
> the upcoming SDIO HCI implementation.
> 
> Note that a failure in rtw_pwr_seq_parser() while applying
> chip->pwr_off_seq can theoretically result in the RTW_FLAG_POWERON
> flag being cleared while the chip is still powered on. However,
> depending on when the failure occurs in the power off sequence the
> chip may be on or off. Even the original approach of clearing
> RTW_FLAG_POWERON only when the power off sequence has been applied
> successfully could end up in some corner case where the chip is
> powered off but RTW_FLAG_POWERON was not cleared.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

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

> ---
> Changes since v2:
> - improve patch description about corner cases when clearing
>   RTW_FLAG_POWERON
> 
> Changes since v1:
> - This replaces a previous patch called "rtw88: hci: Add an optional
>   power_switch() callback to rtw_hci_ops" which added a new callback
>   to the HCI ops.
> 
> 
>  drivers/net/wireless/realtek/rtw88/mac.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
> index f3a566cf979b..cfdfc8a2c836 100644
> --- a/drivers/net/wireless/realtek/rtw88/mac.c
> +++ b/drivers/net/wireless/realtek/rtw88/mac.c
> @@ -273,6 +273,9 @@ static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on)
>         if (pwr_on == cur_pwr)
>                 return -EALREADY;
> 
> +       if (!pwr_on)
> +               clear_bit(RTW_FLAG_POWERON, rtwdev->flags);
> +
>         pwr_seq = pwr_on ? chip->pwr_on_seq : chip->pwr_off_seq;
>         ret = rtw_pwr_seq_parser(rtwdev, pwr_seq);
>         if (ret)
> @@ -280,8 +283,6 @@ static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on)
> 
>         if (pwr_on)
>                 set_bit(RTW_FLAG_POWERON, rtwdev->flags);
> -       else
> -               clear_bit(RTW_FLAG_POWERON, rtwdev->flags);
> 
>         return 0;
>  }
> --
> 2.40.0
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
index f3a566cf979b..cfdfc8a2c836 100644
--- a/drivers/net/wireless/realtek/rtw88/mac.c
+++ b/drivers/net/wireless/realtek/rtw88/mac.c
@@ -273,6 +273,9 @@  static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on)
 	if (pwr_on == cur_pwr)
 		return -EALREADY;
 
+	if (!pwr_on)
+		clear_bit(RTW_FLAG_POWERON, rtwdev->flags);
+
 	pwr_seq = pwr_on ? chip->pwr_on_seq : chip->pwr_off_seq;
 	ret = rtw_pwr_seq_parser(rtwdev, pwr_seq);
 	if (ret)
@@ -280,8 +283,6 @@  static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on)
 
 	if (pwr_on)
 		set_bit(RTW_FLAG_POWERON, rtwdev->flags);
-	else
-		clear_bit(RTW_FLAG_POWERON, rtwdev->flags);
 
 	return 0;
 }