Add the code specific to SDIO HCI in the MAC power on sequence. This is
based on the RTL8822BS and RTL8822CS vendor drivers.
Co-developed-by: Jernej Skrabec <[email protected]>
Signed-off-by: Jernej Skrabec <[email protected]>
Signed-off-by: Martin Blumenstingl <[email protected]>
---
Changes since v1:
- only access REG_SDIO_HIMR for RTW_HCI_TYPE_SDIO
- use proper BIT_HCI_SUS_REQ, BIT_HCI_RESUME_RDY and BIT_SDIO_PAD_E5
macros as suggested by Ping-Ke
drivers/net/wireless/realtek/rtw88/mac.c | 46 +++++++++++++++++++++---
1 file changed, 42 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
index cfdfc8a2c836..17704394cca3 100644
--- a/drivers/net/wireless/realtek/rtw88/mac.c
+++ b/drivers/net/wireless/realtek/rtw88/mac.c
@@ -7,6 +7,7 @@
#include "reg.h"
#include "fw.h"
#include "debug.h"
+#include "sdio.h"
void rtw_set_channel_mac(struct rtw_dev *rtwdev, u8 channel, u8 bw,
u8 primary_ch_idx)
@@ -60,6 +61,7 @@ EXPORT_SYMBOL(rtw_set_channel_mac);
static int rtw_mac_pre_system_cfg(struct rtw_dev *rtwdev)
{
+ unsigned int retry;
u32 value32;
u8 value8;
@@ -77,6 +79,28 @@ static int rtw_mac_pre_system_cfg(struct rtw_dev *rtwdev)
case RTW_HCI_TYPE_PCIE:
rtw_write32_set(rtwdev, REG_HCI_OPT_CTRL, BIT_USB_SUS_DIS);
break;
+ case RTW_HCI_TYPE_SDIO:
+ rtw_write8_clr(rtwdev, REG_SDIO_HSUS_CTRL, BIT_HCI_SUS_REQ);
+
+ for (retry = 0; retry < RTW_PWR_POLLING_CNT; retry++) {
+ if (rtw_read8(rtwdev, REG_SDIO_HSUS_CTRL) & BIT_HCI_RESUME_RDY)
+ break;
+
+ usleep_range(10, 50);
+ }
+
+ if (retry == RTW_PWR_POLLING_CNT) {
+ rtw_err(rtwdev, "failed to poll REG_SDIO_HSUS_CTRL[1]");
+ return -ETIMEDOUT;
+ }
+
+ if (rtw_sdio_is_sdio30_supported(rtwdev))
+ rtw_write8_set(rtwdev, REG_HCI_OPT_CTRL + 2,
+ BIT_SDIO_PAD_E5 >> 16);
+ else
+ rtw_write8_clr(rtwdev, REG_HCI_OPT_CTRL + 2,
+ BIT_SDIO_PAD_E5 >> 16);
+ break;
case RTW_HCI_TYPE_USB:
break;
default:
@@ -248,6 +272,7 @@ static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on)
{
const struct rtw_chip_info *chip = rtwdev->chip;
const struct rtw_pwr_seq_cmd **pwr_seq;
+ u32 imr;
u8 rpwm;
bool cur_pwr;
int ret;
@@ -273,18 +298,24 @@ static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on)
if (pwr_on == cur_pwr)
return -EALREADY;
+ if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_SDIO) {
+ imr = rtw_read32(rtwdev, REG_SDIO_HIMR);
+ rtw_write32(rtwdev, REG_SDIO_HIMR, 0);
+ }
+
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)
- return ret;
+
+ if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_SDIO)
+ rtw_write32(rtwdev, REG_SDIO_HIMR, imr);
if (pwr_on)
set_bit(RTW_FLAG_POWERON, rtwdev->flags);
- return 0;
+ return ret;
}
static int __rtw_mac_init_system_cfg(struct rtw_dev *rtwdev)
@@ -455,6 +486,9 @@ static void download_firmware_reg_backup(struct rtw_dev *rtwdev,
rtw_write16(rtwdev, REG_FIFOPAGE_INFO_1, 0x200);
rtw_write32(rtwdev, REG_RQPN_CTRL_2, bckp[bckp_idx - 1].val);
+ if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_SDIO)
+ rtw_read32(rtwdev, REG_SDIO_FREE_TXPG);
+
/* Disable beacon related functions */
tmp = rtw_read8(rtwdev, REG_BCN_CTRL);
bckp[bckp_idx].len = 1;
@@ -1067,8 +1101,12 @@ static int txdma_queue_mapping(struct rtw_dev *rtwdev)
if (rtw_chip_wcpu_11ac(rtwdev))
rtw_write32(rtwdev, REG_H2CQ_CSR, BIT_H2CQ_FULL);
- if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_USB)
+ if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_SDIO) {
+ rtw_read32(rtwdev, REG_SDIO_FREE_TXPG);
+ rtw_write32(rtwdev, REG_SDIO_TX_CTRL, 0);
+ } else if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_USB) {
rtw_write8_set(rtwdev, REG_TXDMA_PQ_MAP, BIT_RXDMA_ARBBW_EN);
+ }
return 0;
}
--
2.39.2
> -----Original Message-----
> From: Martin Blumenstingl <[email protected]>
> Sent: Saturday, March 11, 2023 4:29 AM
> To: [email protected]
> Cc: Yan-Hsuan Chuang <[email protected]>; Kalle Valo <[email protected]>; Ulf Hansson
> <[email protected]>; [email protected]; [email protected];
> [email protected]; Chris Morgan <[email protected]>; Nitin Gupta <[email protected]>;
> Neo Jou <[email protected]>; Ping-Ke Shih <[email protected]>; Jernej Skrabec <[email protected]>;
> Martin Blumenstingl <[email protected]>
> Subject: [PATCH v2 RFC 3/9] wifi: rtw88: mac: Support SDIO specific bits in the power on sequence
>
> Add the code specific to SDIO HCI in the MAC power on sequence. This is
> based on the RTL8822BS and RTL8822CS vendor drivers.
>
> Co-developed-by: Jernej Skrabec <[email protected]>
> Signed-off-by: Jernej Skrabec <[email protected]>
> Signed-off-by: Martin Blumenstingl <[email protected]>
> ---
> Changes since v1:
> - only access REG_SDIO_HIMR for RTW_HCI_TYPE_SDIO
> - use proper BIT_HCI_SUS_REQ, BIT_HCI_RESUME_RDY and BIT_SDIO_PAD_E5
> macros as suggested by Ping-Ke
>
>
> drivers/net/wireless/realtek/rtw88/mac.c | 46 +++++++++++++++++++++---
> 1 file changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
> index cfdfc8a2c836..17704394cca3 100644
> --- a/drivers/net/wireless/realtek/rtw88/mac.c
> +++ b/drivers/net/wireless/realtek/rtw88/mac.c
> @@ -7,6 +7,7 @@
> #include "reg.h"
> #include "fw.h"
> #include "debug.h"
> +#include "sdio.h"
>
> void rtw_set_channel_mac(struct rtw_dev *rtwdev, u8 channel, u8 bw,
> u8 primary_ch_idx)
> @@ -60,6 +61,7 @@ EXPORT_SYMBOL(rtw_set_channel_mac);
>
> static int rtw_mac_pre_system_cfg(struct rtw_dev *rtwdev)
> {
> + unsigned int retry;
> u32 value32;
> u8 value8;
>
> @@ -77,6 +79,28 @@ static int rtw_mac_pre_system_cfg(struct rtw_dev *rtwdev)
> case RTW_HCI_TYPE_PCIE:
> rtw_write32_set(rtwdev, REG_HCI_OPT_CTRL, BIT_USB_SUS_DIS);
> break;
> + case RTW_HCI_TYPE_SDIO:
> + rtw_write8_clr(rtwdev, REG_SDIO_HSUS_CTRL, BIT_HCI_SUS_REQ);
> +
> + for (retry = 0; retry < RTW_PWR_POLLING_CNT; retry++) {
> + if (rtw_read8(rtwdev, REG_SDIO_HSUS_CTRL) & BIT_HCI_RESUME_RDY)
> + break;
> +
> + usleep_range(10, 50);
> + }
> +
> + if (retry == RTW_PWR_POLLING_CNT) {
> + rtw_err(rtwdev, "failed to poll REG_SDIO_HSUS_CTRL[1]");
> + return -ETIMEDOUT;
> + }
> +
> + if (rtw_sdio_is_sdio30_supported(rtwdev))
> + rtw_write8_set(rtwdev, REG_HCI_OPT_CTRL + 2,
> + BIT_SDIO_PAD_E5 >> 16);
> + else
> + rtw_write8_clr(rtwdev, REG_HCI_OPT_CTRL + 2,
> + BIT_SDIO_PAD_E5 >> 16);
> + break;
> case RTW_HCI_TYPE_USB:
> break;
> default:
> @@ -248,6 +272,7 @@ static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on)
> {
> const struct rtw_chip_info *chip = rtwdev->chip;
> const struct rtw_pwr_seq_cmd **pwr_seq;
> + u32 imr;
> u8 rpwm;
> bool cur_pwr;
> int ret;
> @@ -273,18 +298,24 @@ static int rtw_mac_power_switch(struct rtw_dev *rtwdev, bool pwr_on)
> if (pwr_on == cur_pwr)
> return -EALREADY;
>
> + if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_SDIO) {
> + imr = rtw_read32(rtwdev, REG_SDIO_HIMR);
> + rtw_write32(rtwdev, REG_SDIO_HIMR, 0);
> + }
> +
> 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)
> - return ret;
> +
> + if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_SDIO)
> + rtw_write32(rtwdev, REG_SDIO_HIMR, imr);
>
> if (pwr_on)
> set_bit(RTW_FLAG_POWERON, rtwdev->flags);
If failed to power on, it still set RTW_FLAG_POWERON. Is it reasonable?
Did you meet real problem here?
Maybe, here can be
if (pwr_on && !ret)
set_bit(RTW_FLAG_POWERON, rtwdev->flags);
>
> - return 0;
> + return ret;
> }
>
> static int __rtw_mac_init_system_cfg(struct rtw_dev *rtwdev)
> @@ -455,6 +486,9 @@ static void download_firmware_reg_backup(struct rtw_dev *rtwdev,
> rtw_write16(rtwdev, REG_FIFOPAGE_INFO_1, 0x200);
> rtw_write32(rtwdev, REG_RQPN_CTRL_2, bckp[bckp_idx - 1].val);
>
> + if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_SDIO)
> + rtw_read32(rtwdev, REG_SDIO_FREE_TXPG);
> +
> /* Disable beacon related functions */
> tmp = rtw_read8(rtwdev, REG_BCN_CTRL);
> bckp[bckp_idx].len = 1;
> @@ -1067,8 +1101,12 @@ static int txdma_queue_mapping(struct rtw_dev *rtwdev)
> if (rtw_chip_wcpu_11ac(rtwdev))
> rtw_write32(rtwdev, REG_H2CQ_CSR, BIT_H2CQ_FULL);
>
> - if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_USB)
> + if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_SDIO) {
> + rtw_read32(rtwdev, REG_SDIO_FREE_TXPG);
> + rtw_write32(rtwdev, REG_SDIO_TX_CTRL, 0);
> + } else if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_USB) {
> rtw_write8_set(rtwdev, REG_TXDMA_PQ_MAP, BIT_RXDMA_ARBBW_EN);
> + }
>
> return 0;
> }
> --
> 2.39.2
Hello Ping-Ke,
On Mon, Mar 13, 2023 at 10:05 AM Ping-Ke Shih <[email protected]> wrote:
[...]
> > pwr_seq = pwr_on ? chip->pwr_on_seq : chip->pwr_off_seq;
> > ret = rtw_pwr_seq_parser(rtwdev, pwr_seq);
> > - if (ret)
> > - return ret;
> > +
> > + if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_SDIO)
> > + rtw_write32(rtwdev, REG_SDIO_HIMR, imr);
> >
> > if (pwr_on)
> > set_bit(RTW_FLAG_POWERON, rtwdev->flags);
>
> If failed to power on, it still set RTW_FLAG_POWERON. Is it reasonable?
That sounds very reasonable to me!
> Did you meet real problem here?
>
> Maybe, here can be
>
> if (pwr_on && !ret)
> set_bit(RTW_FLAG_POWERON, rtwdev->flags);
I can't remember any issue that I've seen. I'll verify this at the end
of the week (until then I am pretty busy with my daytime job) and then
go with your suggestion.
Thanks again as always - your feedback is really appreciated!
Also thank you for commenting on the other patches. I'll take a closer
look at your feedback at the end of the week and send another version
of this series.
Best regards,
Martin
> -----Original Message-----
> From: Martin Blumenstingl <[email protected]>
> Sent: Tuesday, March 14, 2023 4:12 AM
> To: Ping-Ke Shih <[email protected]>
> Cc: [email protected]; Yan-Hsuan Chuang <[email protected]>; Kalle Valo
> <[email protected]>; Ulf Hansson <[email protected]>; [email protected];
> [email protected]; [email protected]; Chris Morgan <[email protected]>; Nitin Gupta
> <[email protected]>; Neo Jou <[email protected]>; Jernej Skrabec <[email protected]>
> Subject: Re: [PATCH v2 RFC 3/9] wifi: rtw88: mac: Support SDIO specific bits in the power on sequence
>
> Hello Ping-Ke,
>
> On Mon, Mar 13, 2023 at 10:05 AM Ping-Ke Shih <[email protected]> wrote:
> [...]
> > > pwr_seq = pwr_on ? chip->pwr_on_seq : chip->pwr_off_seq;
> > > ret = rtw_pwr_seq_parser(rtwdev, pwr_seq);
> > > - if (ret)
> > > - return ret;
> > > +
> > > + if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_SDIO)
> > > + rtw_write32(rtwdev, REG_SDIO_HIMR, imr);
> > >
> > > if (pwr_on)
> > > set_bit(RTW_FLAG_POWERON, rtwdev->flags);
> >
> > If failed to power on, it still set RTW_FLAG_POWERON. Is it reasonable?
> That sounds very reasonable to me!
Let me clear here more.
Consider a use case:
1. Initially, it enters this function with pwr_on = true, and RTW_FLAG_POWERON is unset.
2. rtw_pwr_seq_parser() return error, so power state is uncertain.
3. Unconditionally, set RTW_FLAG_POWERON.
rtw_mac_power_on() will try to power off/on once again if it fails, so
in step 3 setting RTW_FLAG_POWERON only if rtw_pwr_seq_parser() returns 0
can have the same values as step 1, when it retries to power on.
Honestly, we don't have perfect error handle for error of rtw_pwr_seq_parser(),
but still want to make things easier understand.
>
> > Did you meet real problem here?
> >
> > Maybe, here can be
> >
> > if (pwr_on && !ret)
> > set_bit(RTW_FLAG_POWERON, rtwdev->flags);
> I can't remember any issue that I've seen. I'll verify this at the end
> of the week (until then I am pretty busy with my daytime job) and then
> go with your suggestion.
Thanks. Wait for you.
> Thanks again as always - your feedback is really appreciated!
>
> Also thank you for commenting on the other patches. I'll take a closer
> look at your feedback at the end of the week and send another version
> of this series.
>
I also thank you for cooking these patches voluntarily for people who
can use their own wifi happily. :-)
Ping-Ke