Skip to content

Commit

Permalink
mwifiex: Add quirk resetting the PCI bridge on MS Surface devices
Browse files Browse the repository at this point in the history
The most recent firmware of the 88W8897 card reports a hardcoded LTR
value to the system during initialization, probably as an (unsuccessful)
attempt of the developers to fix firmware crashes. This LTR value
prevents most of the Microsoft Surface devices from entering deep
powersaving states (either platform C-State 10 or S0ix state), because
the exit latency of that state would be higher than what the card can
tolerate.

Turns out the card works just the same (including the firmware crashes)
no matter if that hardcoded LTR value is reported or not, so it's kind
of useless and only prevents us from saving power.

To get rid of those hardcoded LTR reports, it's possible to reset the
PCI bridge device after initializing the cards firmware. I'm not exactly
sure why that works, maybe the power management subsystem of the PCH
resets its stored LTR values when doing a function level reset of the
bridge device. Doing the reset once after starting the wifi firmware
works very well, probably because the firmware only reports that LTR
value a single time during firmware startup.
  • Loading branch information
jonas2515 committed Mar 26, 2021
1 parent b2c74ef commit bb36502
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 8 deletions.
12 changes: 12 additions & 0 deletions drivers/net/wireless/marvell/mwifiex/pcie.c
Original file line number Diff line number Diff line change
Expand Up @@ -1753,9 +1753,21 @@ mwifiex_pcie_send_boot_cmd(struct mwifiex_adapter *adapter, struct sk_buff *skb)
static int mwifiex_pcie_init_fw_port(struct mwifiex_adapter *adapter)
{
struct pcie_service_card *card = adapter->card;
struct pci_dev *pdev = card->dev;
struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
const struct mwifiex_pcie_card_reg *reg = card->pcie.reg;
int tx_wrap = card->txbd_wrptr & reg->tx_wrap_mask;

/* Trigger a function level reset of the PCI bridge device, this makes
* the firmware of PCIe 88W8897 cards stop reporting a fixed LTR value
* that prevents the system from entering package C10 and S0ix powersaving
* states.
* We need to do it here because it must happen after firmware
* initialization and this function is called after that is done.
*/
if (card->quirks & QUIRK_DO_FLR_ON_BRIDGE)
pci_reset_function(parent_pdev);

/* Write the RX ring read pointer in to reg->rx_rdptr */
if (mwifiex_write_reg(adapter, reg->rx_rdptr, card->rxbd_rdptr |
tx_wrap)) {
Expand Down
26 changes: 18 additions & 8 deletions drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Pro 4"),
},
.driver_data = (void *)(QUIRK_FW_RST_D3COLD |
QUIRK_NO_BRIDGE_D3),
QUIRK_NO_BRIDGE_D3 |
QUIRK_DO_FLR_ON_BRIDGE),
},
{
.ident = "Surface Pro 5",
Expand All @@ -43,7 +44,8 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "Surface_Pro_1796"),
},
.driver_data = (void *)(QUIRK_FW_RST_D3COLD |
QUIRK_NO_BRIDGE_D3),
QUIRK_NO_BRIDGE_D3 |
QUIRK_DO_FLR_ON_BRIDGE),
},
{
.ident = "Surface Pro 5 (LTE)",
Expand All @@ -53,7 +55,8 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "Surface_Pro_1807"),
},
.driver_data = (void *)(QUIRK_FW_RST_D3COLD |
QUIRK_NO_BRIDGE_D3),
QUIRK_NO_BRIDGE_D3 |
QUIRK_DO_FLR_ON_BRIDGE),
},
{
.ident = "Surface Pro 6",
Expand All @@ -62,7 +65,8 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Pro 6"),
},
.driver_data = (void *)(QUIRK_FW_RST_D3COLD |
QUIRK_NO_BRIDGE_D3),
QUIRK_NO_BRIDGE_D3 |
QUIRK_DO_FLR_ON_BRIDGE),
},
{
.ident = "Surface Book 1",
Expand All @@ -71,7 +75,8 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Book"),
},
.driver_data = (void *)(QUIRK_FW_RST_D3COLD |
QUIRK_NO_BRIDGE_D3),
QUIRK_NO_BRIDGE_D3 |
QUIRK_DO_FLR_ON_BRIDGE),
},
{
.ident = "Surface Book 2",
Expand All @@ -80,7 +85,8 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Book 2"),
},
.driver_data = (void *)(QUIRK_FW_RST_D3COLD |
QUIRK_NO_BRIDGE_D3),
QUIRK_NO_BRIDGE_D3 |
QUIRK_DO_FLR_ON_BRIDGE),
},
{
.ident = "Surface Laptop 1",
Expand All @@ -89,7 +95,8 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Laptop"),
},
.driver_data = (void *)(QUIRK_FW_RST_D3COLD |
QUIRK_NO_BRIDGE_D3),
QUIRK_NO_BRIDGE_D3 |
QUIRK_DO_FLR_ON_BRIDGE),
},
{
.ident = "Surface Laptop 2",
Expand All @@ -98,7 +105,8 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Laptop 2"),
},
.driver_data = (void *)(QUIRK_FW_RST_D3COLD |
QUIRK_NO_BRIDGE_D3),
QUIRK_NO_BRIDGE_D3 |
QUIRK_DO_FLR_ON_BRIDGE),
},
{
.ident = "Surface 3",
Expand Down Expand Up @@ -147,6 +155,8 @@ void mwifiex_initialize_quirks(struct pcie_service_card *card)
if (card->quirks & QUIRK_NO_BRIDGE_D3)
dev_info(&pdev->dev,
"quirk no_brigde_d3 enabled\n");
if (card->quirks & QUIRK_DO_FLR_ON_BRIDGE)
dev_info(&pdev->dev, "quirk do_flr_on_bridge enabled\n");
}

static void mwifiex_pcie_set_power_d3cold(struct pci_dev *pdev)
Expand Down
1 change: 1 addition & 0 deletions drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
*/
#define QUIRK_FW_RST_WSID_S3 BIT(1)
#define QUIRK_NO_BRIDGE_D3 BIT(2)
#define QUIRK_DO_FLR_ON_BRIDGE BIT(3)

void mwifiex_initialize_quirks(struct pcie_service_card *card);
int mwifiex_pcie_reset_d3cold_quirk(struct pci_dev *pdev);
Expand Down

0 comments on commit bb36502

Please sign in to comment.