2020-04-17 07:48:03

by Tony Chuang

[permalink] [raw]
Subject: [PATCH 28/40] rtw88: 8723d: Add shutdown callback to disable BT USB suspend

From: Ping-Ke Shih <[email protected]>

Without this patch, wifi card can't initialize properly due to BT in USB
suspend state. So, we disable BT USB suspend (wakeup) in shutdown callback
that is the moment before rebooting. To save BT USB power, we can't do this
in 'remove' callback.

Signed-off-by: Ping-Ke Shih <[email protected]>
Signed-off-by: Yan-Hsuan Chuang <[email protected]>
---
drivers/net/wireless/realtek/rtw88/main.h | 1 +
drivers/net/wireless/realtek/rtw88/pci.c | 17 +++++++++++++++++
drivers/net/wireless/realtek/rtw88/reg.h | 1 +
drivers/net/wireless/realtek/rtw88/rtw8723d.c | 6 ++++++
4 files changed, 25 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 987573ddeefc..9421397ee444 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -793,6 +793,7 @@ struct rtw_regulatory {

struct rtw_chip_ops {
int (*mac_init)(struct rtw_dev *rtwdev);
+ void (*shutdown)(struct rtw_dev *rtwdev);
int (*read_efuse)(struct rtw_dev *rtwdev, u8 *map);
void (*phy_set_param)(struct rtw_dev *rtwdev);
void (*set_channel)(struct rtw_dev *rtwdev, u8 channel,
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 8a8d746d3349..9f5edb8ea7a9 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -1577,6 +1577,22 @@ static void rtw_pci_remove(struct pci_dev *pdev)
ieee80211_free_hw(hw);
}

+static void rtw_pci_shutdown(struct pci_dev *pdev)
+{
+ struct ieee80211_hw *hw = pci_get_drvdata(pdev);
+ struct rtw_dev *rtwdev;
+ struct rtw_chip_info *chip;
+
+ if (!hw)
+ return;
+
+ rtwdev = hw->priv;
+ chip = rtwdev->chip;
+
+ if (chip->ops->shutdown)
+ chip->ops->shutdown(rtwdev);
+}
+
static const struct pci_device_id rtw_pci_id_table[] = {
#ifdef CONFIG_RTW88_8822BE
{ RTK_PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0xB822, rtw8822b_hw_spec) },
@@ -1597,6 +1613,7 @@ static struct pci_driver rtw_pci_driver = {
.probe = rtw_pci_probe,
.remove = rtw_pci_remove,
.driver.pm = RTW_PM_OPS,
+ .shutdown = rtw_pci_shutdown,
};
module_pci_driver(rtw_pci_driver);

diff --git a/drivers/net/wireless/realtek/rtw88/reg.h b/drivers/net/wireless/realtek/rtw88/reg.h
index d57de1a6cdcc..5a3e9cc7c400 100644
--- a/drivers/net/wireless/realtek/rtw88/reg.h
+++ b/drivers/net/wireless/realtek/rtw88/reg.h
@@ -83,6 +83,7 @@
#define BIT_DBG_GNT_WL_BT BIT(27)
#define BIT_LTE_MUX_CTRL_PATH BIT(26)
#define REG_HCI_OPT_CTRL 0x0074
+#define BIT_USB_SUS_DIS BIT(8)

#define REG_AFE_CTRL_4 0x0078
#define BIT_CK320M_AFE_EN BIT(4)
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8723d.c b/drivers/net/wireless/realtek/rtw88/rtw8723d.c
index 4fb361d91a0b..b58915e5e1de 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8723d.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8723d.c
@@ -580,6 +580,11 @@ static int rtw8723d_mac_init(struct rtw_dev *rtwdev)
return 0;
}

+static void rtw8723d_shutdown(struct rtw_dev *rtwdev)
+{
+ rtw_write16_set(rtwdev, REG_HCI_OPT_CTRL, BIT_USB_SUS_DIS);
+}
+
static void rtw8723d_cfg_ldo25(struct rtw_dev *rtwdev, bool enable)
{
u8 ldo_pwr;
@@ -1834,6 +1839,7 @@ static struct rtw_chip_ops rtw8723d_ops = {
.query_rx_desc = rtw8723d_query_rx_desc,
.set_channel = rtw8723d_set_channel,
.mac_init = rtw8723d_mac_init,
+ .shutdown = rtw8723d_shutdown,
.read_rf = rtw_phy_read_rf_sipi,
.write_rf = rtw_phy_write_rf_reg_sipi,
.set_tx_power_index = rtw8723d_set_tx_power_index,
--
2.17.1


Subject: Re: [PATCH 28/40] rtw88: 8723d: Add shutdown callback to disable BT USB suspend

On 2020-04-17 15:46:41 [+0800], [email protected] wrote:
> From: Ping-Ke Shih <[email protected]>
>
> Without this patch, wifi card can't initialize properly due to BT in USB
> suspend state. So, we disable BT USB suspend (wakeup) in shutdown callback
> that is the moment before rebooting. To save BT USB power, we can't do this
> in 'remove' callback.

So you can't initialize the USB part because it is in suspend and the
only way to avoid it to disable it on the PCI side. That means you don't
see it enumerated on the USB bus at all?

> Signed-off-by: Ping-Ke Shih <[email protected]>
> Signed-off-by: Yan-Hsuan Chuang <[email protected]>

Sebastian

2020-05-06 02:40:43

by Tony Chuang

[permalink] [raw]
Subject: RE: [PATCH 28/40] rtw88: 8723d: Add shutdown callback to disable BT USB suspend

> On 2020-04-17 15:46:41 [+0800], [email protected] wrote:
> > From: Ping-Ke Shih <[email protected]>
> >
> > Without this patch, wifi card can't initialize properly due to BT in USB
> > suspend state. So, we disable BT USB suspend (wakeup) in shutdown callback
> > that is the moment before rebooting. To save BT USB power, we can't do this
> > in 'remove' callback.
>
> So you can't initialize the USB part because it is in suspend and the
> only way to avoid it to disable it on the PCI side. That means you don't
> see it enumerated on the USB bus at all?

Yes, if we don't disable it on PCI side, then the USB part cannot be
probed on USB bus.

>
> > Signed-off-by: Ping-Ke Shih <[email protected]>
> > Signed-off-by: Yan-Hsuan Chuang <[email protected]>
>
> Sebastian
>

Yen-Hsuan

Subject: Re: [PATCH 28/40] rtw88: 8723d: Add shutdown callback to disable BT USB suspend

On 2020-05-06 02:35:21 [+0000], Tony Chuang wrote:
> > On 2020-04-17 15:46:41 [+0800], [email protected] wrote:
> > > From: Ping-Ke Shih <[email protected]>
> > >
> > > Without this patch, wifi card can't initialize properly due to BT in USB
> > > suspend state. So, we disable BT USB suspend (wakeup) in shutdown callback
> > > that is the moment before rebooting. To save BT USB power, we can't do this
> > > in 'remove' callback.
> >
> > So you can't initialize the USB part because it is in suspend and the
> > only way to avoid it to disable it on the PCI side. That means you don't
> > see it enumerated on the USB bus at all?
>
> Yes, if we don't disable it on PCI side, then the USB part cannot be
> probed on USB bus.

We talk here about USB's runtime-suspend / autosuspend? If so, are you
aware of commit
7ecacafc24063 ("Bluetooth: btusb: Disable runtime suspend on Realtek devices")

or is this an attempt to get rid of this change in favour of this one
(so that the device can enter suspend-mode)?

Sebastian

2020-05-07 04:27:11

by Tony Chuang

[permalink] [raw]
Subject: RE: [PATCH 28/40] rtw88: 8723d: Add shutdown callback to disable BT USB suspend

Sebastian Andrzej Siewior <[email protected]> writes
> On 2020-05-06 02:35:21 [+0000], Tony Chuang wrote:
> > > On 2020-04-17 15:46:41 [+0800], [email protected] wrote:
> > > > From: Ping-Ke Shih <[email protected]>
> > > >
> > > > Without this patch, wifi card can't initialize properly due to BT in USB
> > > > suspend state. So, we disable BT USB suspend (wakeup) in shutdown
> callback
> > > > that is the moment before rebooting. To save BT USB power, we can't do
> this
> > > > in 'remove' callback.
> > >
> > > So you can't initialize the USB part because it is in suspend and the
> > > only way to avoid it to disable it on the PCI side. That means you don't
> > > see it enumerated on the USB bus at all?
> >
> > Yes, if we don't disable it on PCI side, then the USB part cannot be
> > probed on USB bus.
>
> We talk here about USB's runtime-suspend / autosuspend? If so, are you
> aware of commit
> 7ecacafc24063 ("Bluetooth: btusb: Disable runtime suspend on Realtek
> devices")
>
> or is this an attempt to get rid of this change in favour of this one
> (so that the device can enter suspend-mode)?
>

Ping-Ke, can you please help to check on this ?
Looks like Kai-Heng is doing the much same thing here.

But it's still worth to do it in wifi side I think, because it's difficult to
make sure the synchronization of BT and Wifi patch.

Yen-Hsuan

Subject: Re: [PATCH 28/40] rtw88: 8723d: Add shutdown callback to disable BT USB suspend

On 2020-05-07 04:26:24 [+0000], Tony Chuang wrote:
>
> Ping-Ke, can you please help to check on this ?
> Looks like Kai-Heng is doing the much same thing here.
>
> But it's still worth to do it in wifi side I think, because it's difficult to
> make sure the synchronization of BT and Wifi patch.

Yes. It sounds reasonable to remove the patch in BT so the device is not
always avoiding the suspend mode.

I don't remember if I asked this: Shouldn't the USB reset get the device
out of suspend? I thought this is part of the USB test. Could this be
fixed in BT's firmware?

> Yen-Hsuan

Sebastian

2020-05-11 06:45:07

by Ping-Ke Shih

[permalink] [raw]
Subject: RE: [PATCH 28/40] rtw88: 8723d: Add shutdown callback to disable BT USB suspend

Hi Tony,

> -----Original Message-----
> From: Tony Chuang
> Sent: Thursday, May 07, 2020 12:26 PM
> To: Sebastian Andrzej Siewior
> Cc: [email protected]; Pkshih; [email protected]; [email protected]; Kevin
> Yang
> Subject: RE: [PATCH 28/40] rtw88: 8723d: Add shutdown callback to disable BT USB suspend
>
> Sebastian Andrzej Siewior <[email protected]> writes
> > On 2020-05-06 02:35:21 [+0000], Tony Chuang wrote:
> > > > On 2020-04-17 15:46:41 [+0800], [email protected] wrote:
> > > > > From: Ping-Ke Shih <[email protected]>
> > > > >
> > > > > Without this patch, wifi card can't initialize properly due to BT in USB
> > > > > suspend state. So, we disable BT USB suspend (wakeup) in shutdown
> > callback
> > > > > that is the moment before rebooting. To save BT USB power, we can't do
> > this
> > > > > in 'remove' callback.
> > > >
> > > > So you can't initialize the USB part because it is in suspend and the
> > > > only way to avoid it to disable it on the PCI side. That means you don't
> > > > see it enumerated on the USB bus at all?
> > >
> > > Yes, if we don't disable it on PCI side, then the USB part cannot be
> > > probed on USB bus.
> >
> > We talk here about USB's runtime-suspend / autosuspend? If so, are you
> > aware of commit
> > 7ecacafc24063 ("Bluetooth: btusb: Disable runtime suspend on Realtek
> > devices")
> >
> > or is this an attempt to get rid of this change in favour of this one
> > (so that the device can enter suspend-mode)?
> >
>
> Ping-Ke, can you please help to check on this ?
> Looks like Kai-Heng is doing the much same thing here.
>

The Kai-Heng's patch turns off suspend entirely, so I believe if the patch
is existing, this patch doesn't affect the result.
However, the patch seems like a temporal fix, so this patch is needed.


> But it's still worth to do it in wifi side I think, because it's difficult to
> make sure the synchronization of BT and Wifi patch.
>
Agree.


Thank you
PK