* [RFC PATCH 1/3] mwifiex: pcie: add DMI-based quirk impl for Surface devices
2020-10-28 14:27 [RFC PATCH 0/3] mwifiex: add fw reset quirks for Microsoft Surface Tsuchiya Yuto
@ 2020-10-28 14:27 ` Tsuchiya Yuto
[not found] ` <20201028152710.GU4077@smile.fi.intel.com>
2020-10-28 14:27 ` [RFC PATCH 2/3] mwifiex: pcie: add reset_d3cold quirk for Surface gen4+ devices Tsuchiya Yuto
2020-10-28 14:27 ` [RFC PATCH 3/3] mwifiex: pcie: add reset_wsid quirk for Surface 3 Tsuchiya Yuto
2 siblings, 1 reply; 10+ messages in thread
From: Tsuchiya Yuto @ 2020-10-28 14:27 UTC (permalink / raw)
To: Amitkumar Karwar, Ganapathi Bhat, Xinming Hu, Kalle Valo,
David S. Miller, Jakub Kicinski
Cc: linux-wireless, netdev, linux-kernel, Maximilian Luz,
Andy Shevchenko, verdre, Tsuchiya Yuto
This commit adds quirk implementation based on DMI matching with DMI
table for Microsoft Surface devices that uses mwifiex chip (currently,
all the devices that use mwifiex equip PCIe-88W8897 chip).
This implementation can be used for quirks later.
Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
---
drivers/net/wireless/marvell/mwifiex/Makefile | 1 +
drivers/net/wireless/marvell/mwifiex/pcie.c | 4 +
drivers/net/wireless/marvell/mwifiex/pcie.h | 2 +
.../wireless/marvell/mwifiex/pcie_quirks.c | 114 ++++++++++++++++++
.../wireless/marvell/mwifiex/pcie_quirks.h | 11 ++
5 files changed, 132 insertions(+)
create mode 100644 drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
create mode 100644 drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
diff --git a/drivers/net/wireless/marvell/mwifiex/Makefile b/drivers/net/wireless/marvell/mwifiex/Makefile
index fdfd9bf15ed46..8a1e7c5b9c6e2 100644
--- a/drivers/net/wireless/marvell/mwifiex/Makefile
+++ b/drivers/net/wireless/marvell/mwifiex/Makefile
@@ -49,6 +49,7 @@ mwifiex_sdio-y += sdio.o
obj-$(CONFIG_MWIFIEX_SDIO) += mwifiex_sdio.o
mwifiex_pcie-y += pcie.o
+mwifiex_pcie-y += pcie_quirks.o
obj-$(CONFIG_MWIFIEX_PCIE) += mwifiex_pcie.o
mwifiex_usb-y += usb.o
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 6a10ff0377a24..362cf10debfa0 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -27,6 +27,7 @@
#include "wmm.h"
#include "11n.h"
#include "pcie.h"
+#include "pcie_quirks.h"
#define PCIE_VERSION "1.0"
#define DRV_NAME "Marvell mwifiex PCIe"
@@ -410,6 +411,9 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
return ret;
}
+ /* check quirks */
+ mwifiex_initialize_quirks(card);
+
if (mwifiex_add_card(card, &card->fw_done, &pcie_ops,
MWIFIEX_PCIE, &pdev->dev)) {
pr_err("%s failed\n", __func__);
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.h b/drivers/net/wireless/marvell/mwifiex/pcie.h
index 843d57eda8201..09839a3bd1753 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.h
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.h
@@ -242,6 +242,8 @@ struct pcie_service_card {
struct mwifiex_msix_context share_irq_ctx;
struct work_struct work;
unsigned long work_flags;
+
+ unsigned long quirks;
};
static inline int
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
new file mode 100644
index 0000000000000..929aee2b0a60a
--- /dev/null
+++ b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * File for PCIe quirks.
+ */
+
+/* The low-level PCI operations will be performed in this file. Therefore,
+ * let's use dev_*() instead of mwifiex_dbg() here to avoid troubles (e.g.
+ * to avoid using mwifiex_adapter struct before init or wifi is powered
+ * down, or causes NULL ptr deref).
+ */
+
+#include <linux/dmi.h>
+
+#include "pcie_quirks.h"
+
+/* quirk table based on DMI matching */
+static const struct dmi_system_id mwifiex_quirk_table[] = {
+ {
+ .ident = "Surface Pro 4",
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Pro 4"),
+ },
+ .driver_data = 0,
+ },
+ {
+ .ident = "Surface Pro 5",
+ .matches = {
+ /* match for SKU here due to generic product name "Surface Pro" */
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "Surface_Pro_1796"),
+ },
+ .driver_data = 0,
+ },
+ {
+ .ident = "Surface Pro 5 (LTE)",
+ .matches = {
+ /* match for SKU here due to generic product name "Surface Pro" */
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "Surface_Pro_1807"),
+ },
+ .driver_data = 0,
+ },
+ {
+ .ident = "Surface Pro 6",
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Pro 6"),
+ },
+ .driver_data = 0,
+ },
+ {
+ .ident = "Surface Book 1",
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Book"),
+ },
+ .driver_data = 0,
+ },
+ {
+ .ident = "Surface Book 2",
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Book 2"),
+ },
+ .driver_data = 0,
+ },
+ {
+ .ident = "Surface Laptop 1",
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Laptop"),
+ },
+ .driver_data = 0,
+ },
+ {
+ .ident = "Surface Laptop 2",
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Laptop 2"),
+ },
+ .driver_data = 0,
+ },
+ {
+ .ident = "Surface 3",
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
+ },
+ .driver_data = 0,
+ },
+ {
+ .ident = "Surface Pro 3",
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Pro 3"),
+ },
+ .driver_data = 0,
+ },
+ {}
+};
+
+void mwifiex_initialize_quirks(struct pcie_service_card *card)
+{
+ struct pci_dev *pdev = card->dev;
+ const struct dmi_system_id *dmi_id;
+
+ dmi_id = dmi_first_match(mwifiex_quirk_table);
+ if (dmi_id)
+ card->quirks = (uintptr_t)dmi_id->driver_data;
+
+ if (!card->quirks)
+ dev_info(&pdev->dev, "no quirks enabled\n");
+}
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
new file mode 100644
index 0000000000000..5326ae7e56713
--- /dev/null
+++ b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Header file for PCIe quirks.
+ */
+
+#include "pcie.h"
+
+/* quirks */
+// quirk flags can be added here
+
+void mwifiex_initialize_quirks(struct pcie_service_card *card);
--
2.29.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH 2/3] mwifiex: pcie: add reset_d3cold quirk for Surface gen4+ devices
2020-10-28 14:27 [RFC PATCH 0/3] mwifiex: add fw reset quirks for Microsoft Surface Tsuchiya Yuto
2020-10-28 14:27 ` [RFC PATCH 1/3] mwifiex: pcie: add DMI-based quirk impl for Surface devices Tsuchiya Yuto
@ 2020-10-28 14:27 ` Tsuchiya Yuto
2020-10-28 14:27 ` [RFC PATCH 3/3] mwifiex: pcie: add reset_wsid quirk for Surface 3 Tsuchiya Yuto
2 siblings, 0 replies; 10+ messages in thread
From: Tsuchiya Yuto @ 2020-10-28 14:27 UTC (permalink / raw)
To: Amitkumar Karwar, Ganapathi Bhat, Xinming Hu, Kalle Valo,
David S. Miller, Jakub Kicinski
Cc: linux-wireless, netdev, linux-kernel, Maximilian Luz,
Andy Shevchenko, verdre, Tsuchiya Yuto
To reset mwifiex on Surface gen4+ (Pro 4 or later gen) devices, it
seems that putting the wifi device into D3cold is required according
to errata.inf file on Windows installation (Windows/INF/errata.inf).
This patch adds a function that performs power-cycle (put into D3cold
then D0) and call the function at the end of reset_prepare().
Note: Need to also reset the parent device (bridge) of wifi on SB1;
it might be because the bridge of wifi always reports it's in D3hot.
When I tried to reset only the wifi device (not touching parent), it gave
the following error and the reset failed:
acpi device:4b: Cannot transition to power state D0 for parent in D3hot
mwifiex_pcie 0000:03:00.0: can't change power state from D3cold to D0 (config space inaccessible)
Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
---
Current issue:
* After reset with this quirk, ASPM settings don't get restored.
Below is the "sudo lspci -nnvvv" diff before/after fw reset on Surface Book 1:
#
# 03:00.0 Ethernet controller [0200]: Marvell Technology Group Ltd. 88W8897 [AVASTAR] 802.11ac Wireless [11ab:2b38]
#
@@ -574,9 +574,9 @@
Capabilities: [168 v1] L1 PM Substates
L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+
PortCommonModeRestoreTime=70us PortTPowerOnTime=10us
- L1SubCtl1: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+
- T_CommonMode=0us LTR1.2_Threshold=163840ns
- L1SubCtl2: T_PwrOn=44us
+ L1SubCtl1: PCI-PM_L1.2- PCI-PM_L1.1- ASPM_L1.2- ASPM_L1.1-
+ T_CommonMode=0us LTR1.2_Threshold=0ns
+ L1SubCtl2: T_PwrOn=10us
Kernel driver in use: mwifiex_pcie
Kernel modules: mwifiex_pcie
#
# no changes on root port of wifi regarding ASPM
#
As you see, all of the L1 substates are disabled after fw reset. LTR
value is also changed.
drivers/net/wireless/marvell/mwifiex/pcie.c | 7 ++
.../wireless/marvell/mwifiex/pcie_quirks.c | 73 +++++++++++++++++--
.../wireless/marvell/mwifiex/pcie_quirks.h | 3 +-
3 files changed, 74 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 362cf10debfa0..c0c4b5a9149ab 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -529,6 +529,13 @@ static void mwifiex_pcie_reset_prepare(struct pci_dev *pdev)
mwifiex_shutdown_sw(adapter);
clear_bit(MWIFIEX_IFACE_WORK_DEVICE_DUMP, &card->work_flags);
clear_bit(MWIFIEX_IFACE_WORK_CARD_RESET, &card->work_flags);
+
+ /* For Surface gen4+ devices, we need to put wifi into D3cold right
+ * before performing FLR
+ */
+ if (card->quirks & QUIRK_FW_RST_D3COLD)
+ mwifiex_pcie_reset_d3cold_quirk(pdev);
+
mwifiex_dbg(adapter, INFO, "%s, successful\n", __func__);
}
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
index 929aee2b0a60a..edc739c542fea 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
@@ -21,7 +21,7 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Pro 4"),
},
- .driver_data = 0,
+ .driver_data = (void *)QUIRK_FW_RST_D3COLD,
},
{
.ident = "Surface Pro 5",
@@ -30,7 +30,7 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "Surface_Pro_1796"),
},
- .driver_data = 0,
+ .driver_data = (void *)QUIRK_FW_RST_D3COLD,
},
{
.ident = "Surface Pro 5 (LTE)",
@@ -39,7 +39,7 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
DMI_EXACT_MATCH(DMI_PRODUCT_SKU, "Surface_Pro_1807"),
},
- .driver_data = 0,
+ .driver_data = (void *)QUIRK_FW_RST_D3COLD,
},
{
.ident = "Surface Pro 6",
@@ -47,7 +47,7 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Pro 6"),
},
- .driver_data = 0,
+ .driver_data = (void *)QUIRK_FW_RST_D3COLD,
},
{
.ident = "Surface Book 1",
@@ -55,7 +55,7 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Book"),
},
- .driver_data = 0,
+ .driver_data = (void *)QUIRK_FW_RST_D3COLD,
},
{
.ident = "Surface Book 2",
@@ -63,7 +63,7 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Book 2"),
},
- .driver_data = 0,
+ .driver_data = (void *)QUIRK_FW_RST_D3COLD,
},
{
.ident = "Surface Laptop 1",
@@ -71,7 +71,7 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Laptop"),
},
- .driver_data = 0,
+ .driver_data = (void *)QUIRK_FW_RST_D3COLD,
},
{
.ident = "Surface Laptop 2",
@@ -79,7 +79,7 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Laptop 2"),
},
- .driver_data = 0,
+ .driver_data = (void *)QUIRK_FW_RST_D3COLD,
},
{
.ident = "Surface 3",
@@ -111,4 +111,61 @@ void mwifiex_initialize_quirks(struct pcie_service_card *card)
if (!card->quirks)
dev_info(&pdev->dev, "no quirks enabled\n");
+ if (card->quirks & QUIRK_FW_RST_D3COLD)
+ dev_info(&pdev->dev, "quirk reset_d3cold enabled\n");
+}
+
+static void mwifiex_pcie_set_power_d3cold(struct pci_dev *pdev)
+{
+ dev_info(&pdev->dev, "putting into D3cold...\n");
+
+ pci_save_state(pdev);
+ if (pci_is_enabled(pdev))
+ pci_disable_device(pdev);
+ pci_set_power_state(pdev, PCI_D3cold);
+}
+
+static int mwifiex_pcie_set_power_d0(struct pci_dev *pdev)
+{
+ int ret;
+
+ dev_info(&pdev->dev, "putting into D0...\n");
+
+ pci_set_power_state(pdev, PCI_D0);
+ ret = pci_enable_device(pdev);
+ if (ret) {
+ dev_err(&pdev->dev, "pci_enable_device failed\n");
+ return ret;
+ }
+ pci_restore_state(pdev);
+
+ return 0;
+}
+
+int mwifiex_pcie_reset_d3cold_quirk(struct pci_dev *pdev)
+{
+ struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
+ int ret;
+
+ /* Power-cycle (put into D3cold then D0) */
+ dev_info(&pdev->dev, "Using reset_d3cold quirk to perform FW reset\n");
+
+ /* We need to perform power-cycle also for bridge of wifi because
+ * on some devices (e.g. Surface Book 1), the OS for some reasons
+ * can't know the real power state of the bridge.
+ * When tried to power-cycle only wifi, the reset failed with the
+ * following dmesg log:
+ * "Cannot transition to power state D0 for parent in D3hot".
+ */
+ mwifiex_pcie_set_power_d3cold(pdev);
+ mwifiex_pcie_set_power_d3cold(parent_pdev);
+
+ ret = mwifiex_pcie_set_power_d0(parent_pdev);
+ if (ret)
+ return ret;
+ ret = mwifiex_pcie_set_power_d0(pdev);
+ if (ret)
+ return ret;
+
+ return 0;
}
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
index 5326ae7e56713..8b9dcb5070d87 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
+++ b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
@@ -6,6 +6,7 @@
#include "pcie.h"
/* quirks */
-// quirk flags can be added here
+#define QUIRK_FW_RST_D3COLD BIT(0)
void mwifiex_initialize_quirks(struct pcie_service_card *card);
+int mwifiex_pcie_reset_d3cold_quirk(struct pci_dev *pdev);
--
2.29.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH 3/3] mwifiex: pcie: add reset_wsid quirk for Surface 3
2020-10-28 14:27 [RFC PATCH 0/3] mwifiex: add fw reset quirks for Microsoft Surface Tsuchiya Yuto
2020-10-28 14:27 ` [RFC PATCH 1/3] mwifiex: pcie: add DMI-based quirk impl for Surface devices Tsuchiya Yuto
2020-10-28 14:27 ` [RFC PATCH 2/3] mwifiex: pcie: add reset_d3cold quirk for Surface gen4+ devices Tsuchiya Yuto
@ 2020-10-28 14:27 ` Tsuchiya Yuto
2 siblings, 0 replies; 10+ messages in thread
From: Tsuchiya Yuto @ 2020-10-28 14:27 UTC (permalink / raw)
To: Amitkumar Karwar, Ganapathi Bhat, Xinming Hu, Kalle Valo,
David S. Miller, Jakub Kicinski
Cc: linux-wireless, netdev, linux-kernel, Maximilian Luz,
Andy Shevchenko, verdre, Tsuchiya Yuto
This commit adds reset_wsid quirk and uses this quirk for Surface 3 on
card reset.
To reset mwifiex on Surface 3, it seems that calling the _DSM method
exists in \_SB.WSID [1] device is required.
On Surface 3, calling the _DSM method removes/re-probes the card by
itself. So, need to place the reset function before performing FLR and
skip performing any other reset-related works.
Note that Surface Pro 3 also has the WSID device [2], but it seems to need
more work. This commit only supports Surface 3 yet.
[1] https://github.com/linux-surface/acpidumps/blob/05cba925f3a515f222acb5b3551a032ddde958fe/surface_3/dsdt.dsl#L11947-L12011
[2] https://github.com/linux-surface/acpidumps/blob/05cba925f3a515f222acb5b3551a032ddde958fe/surface_pro_3/dsdt.dsl#L12164-L12216
Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
---
Current issues:
1) After reset with this quirk, ASPM settings don't get restored.
Below is the "sudo lspci -nnvvv" diff before/after fw reset on Surface 3:
#
# root port of wifi:
# 00:1c.0 PCI bridge [0604]: Intel Corporation Atom/Celeron/Pentium Processor x5-E8000/J3xxx/N3xxx Series PCI Express Port #1 [8086:22c8] (rev 20) (prog-if 00 [Normal decode])
#
@@ -103,7 +103,7 @@
DevSta: CorrErr+ NonFatalErr- FatalErr- UnsupReq- AuxPwr+ TransPend-
LnkCap: Port #1, Speed 5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s <512ns, L1 <16us
ClockPM- Surprise- LLActRep+ BwNot+ ASPMOptComp-
- LnkCtl: ASPM L0s L1 Enabled; RCB 64 bytes, Disabled- CommClk+
+ LnkCtl: ASPM Disabled; RCB 64 bytes, Disabled- CommClk+
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 2.5GT/s (downgraded), Width x1 (ok)
TrErr- Train- SlotClk+ DLActive+ BWMgmt+ ABWMgmt-
#
# no changes on wifi regarding ASPM
#
As you see, the ASPM of wifi root port is disabled after fw reset (thus
breaks S0ix).
2) Creating a new device driver for the WSID device is better?
then export the reset function, and call it from mwifiex. As the comments
in code states, on S3, calling the _DSM methods immediately
remove/re-probe mwifiex. So, resetting the wifi externally is a better
idea? (Also in this way, hopefully supporting SP3 may become easier.)
drivers/net/wireless/marvell/mwifiex/pcie.c | 10 +++
.../wireless/marvell/mwifiex/pcie_quirks.c | 77 ++++++++++++++++++-
.../wireless/marvell/mwifiex/pcie_quirks.h | 5 ++
3 files changed, 91 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index c0c4b5a9149ab..b3aacd19cc48f 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2966,6 +2966,16 @@ static void mwifiex_pcie_card_reset_work(struct mwifiex_adapter *adapter)
{
struct pcie_service_card *card = adapter->card;
+ /* On Surface 3, reset_wsid method removes then re-probes card by
+ * itself. So, need to place it here and skip performing any other
+ * reset-related works.
+ */
+ if (card->quirks & QUIRK_FW_RST_WSID_S3) {
+ mwifiex_pcie_reset_wsid_quirk(card->dev);
+ /* skip performing any other reset-related works */
+ return;
+ }
+
/* We can't afford to wait here; remove() might be waiting on us. If we
* can't grab the device lock, maybe we'll get another chance later.
*/
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
index edc739c542fea..f0a6fa0a7ae5f 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
@@ -9,10 +9,21 @@
* down, or causes NULL ptr deref).
*/
+#include <linux/acpi.h>
#include <linux/dmi.h>
#include "pcie_quirks.h"
+/* For reset_wsid quirk */
+#define ACPI_WSID_PATH "\\_SB.WSID"
+#define WSID_REV 0x0
+#define WSID_FUNC_WIFI_PWR_OFF 0x1
+#define WSID_FUNC_WIFI_PWR_ON 0x2
+/* WSID _DSM UUID: "534ea3bf-fcc2-4e7a-908f-a13978f0c7ef" */
+static const guid_t wsid_dsm_guid =
+ GUID_INIT(0x534ea3bf, 0xfcc2, 0x4e7a,
+ 0x90, 0x8f, 0xa1, 0x39, 0x78, 0xf0, 0xc7, 0xef);
+
/* quirk table based on DMI matching */
static const struct dmi_system_id mwifiex_quirk_table[] = {
{
@@ -87,7 +98,7 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
},
- .driver_data = 0,
+ .driver_data = (void *)QUIRK_FW_RST_WSID_S3,
},
{
.ident = "Surface Pro 3",
@@ -113,6 +124,9 @@ void mwifiex_initialize_quirks(struct pcie_service_card *card)
dev_info(&pdev->dev, "no quirks enabled\n");
if (card->quirks & QUIRK_FW_RST_D3COLD)
dev_info(&pdev->dev, "quirk reset_d3cold enabled\n");
+ if (card->quirks & QUIRK_FW_RST_WSID_S3)
+ dev_info(&pdev->dev,
+ "quirk reset_wsid for Surface 3 enabled\n");
}
static void mwifiex_pcie_set_power_d3cold(struct pci_dev *pdev)
@@ -169,3 +183,64 @@ int mwifiex_pcie_reset_d3cold_quirk(struct pci_dev *pdev)
return 0;
}
+
+int mwifiex_pcie_reset_wsid_quirk(struct pci_dev *pdev)
+{
+ acpi_handle handle;
+ union acpi_object *obj;
+ acpi_status status;
+
+ dev_info(&pdev->dev, "Using reset_wsid quirk to perform FW reset\n");
+
+ status = acpi_get_handle(NULL, ACPI_WSID_PATH, &handle);
+ if (ACPI_FAILURE(status)) {
+ dev_err(&pdev->dev, "No ACPI handle for path %s\n",
+ ACPI_WSID_PATH);
+ return -ENODEV;
+ }
+
+ if (!acpi_has_method(handle, "_DSM")) {
+ dev_err(&pdev->dev, "_DSM method not found\n");
+ return -ENODEV;
+ }
+
+ if (!acpi_check_dsm(handle, &wsid_dsm_guid,
+ WSID_REV, WSID_FUNC_WIFI_PWR_OFF)) {
+ dev_err(&pdev->dev,
+ "_DSM method doesn't support wifi power off func\n");
+ return -ENODEV;
+ }
+
+ if (!acpi_check_dsm(handle, &wsid_dsm_guid,
+ WSID_REV, WSID_FUNC_WIFI_PWR_ON)) {
+ dev_err(&pdev->dev,
+ "_DSM method doesn't support wifi power on func\n");
+ return -ENODEV;
+ }
+
+ /* card will be removed immediately after this call on Surface 3 */
+ dev_info(&pdev->dev, "turning wifi off...\n");
+ obj = acpi_evaluate_dsm(handle, &wsid_dsm_guid,
+ WSID_REV, WSID_FUNC_WIFI_PWR_OFF,
+ NULL);
+ if (!obj) {
+ dev_err(&pdev->dev,
+ "device _DSM execution failed for turning wifi off\n");
+ return -EIO;
+ }
+ ACPI_FREE(obj);
+
+ /* card will be re-probed immediately after this call on Surface 3 */
+ dev_info(&pdev->dev, "turning wifi on...\n");
+ obj = acpi_evaluate_dsm(handle, &wsid_dsm_guid,
+ WSID_REV, WSID_FUNC_WIFI_PWR_ON,
+ NULL);
+ if (!obj) {
+ dev_err(&pdev->dev,
+ "device _DSM execution failed for turning wifi on\n");
+ return -EIO;
+ }
+ ACPI_FREE(obj);
+
+ return 0;
+}
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
index 8b9dcb5070d87..3ef7440418e32 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
+++ b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
@@ -7,6 +7,11 @@
/* quirks */
#define QUIRK_FW_RST_D3COLD BIT(0)
+/* Surface 3 and Surface Pro 3 have the same _DSM method but need to
+ * be handled differently. Currently, only S3 is supported.
+ */
+#define QUIRK_FW_RST_WSID_S3 BIT(1)
void mwifiex_initialize_quirks(struct pcie_service_card *card);
int mwifiex_pcie_reset_d3cold_quirk(struct pci_dev *pdev);
+int mwifiex_pcie_reset_wsid_quirk(struct pci_dev *pdev);
--
2.29.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH 3/3] mwifiex: pcie: add reset_wsid quirk for Surface 3
2021-05-22 13:18 [RFC PATCH 0/3] mwifiex: Add quirks for MS Surface devices Jonas Dreßler
@ 2021-05-22 13:18 ` Jonas Dreßler
2021-05-22 22:01 ` kernel test robot
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Jonas Dreßler @ 2021-05-22 13:18 UTC (permalink / raw)
To: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, Kalle Valo, David S. Miller, Jakub Kicinski
Cc: Jonas Dreßler, Tsuchiya Yuto, linux-wireless, netdev,
linux-kernel, linux-pci, Maximilian Luz, Andy Shevchenko,
Bjorn Helgaas, Pali Rohár
From: Tsuchiya Yuto <kitakar@gmail.com>
This commit adds reset_wsid quirk and uses this quirk for Surface 3 on
card reset.
To reset mwifiex on Surface 3, it seems that calling the _DSM method
exists in \_SB.WSID [1] device is required.
On Surface 3, calling the _DSM method removes/re-probes the card by
itself. So, need to place the reset function before performing FLR and
skip performing any other reset-related works.
Note that Surface Pro 3 also has the WSID device [2], but it seems to need
more work. This commit only supports Surface 3 yet.
[1] https://github.com/linux-surface/acpidumps/blob/05cba925f3a515f222acb5b3551a032ddde958fe/surface_3/dsdt.dsl#L11947-L12011
[2] https://github.com/linux-surface/acpidumps/blob/05cba925f3a515f222acb5b3551a032ddde958fe/surface_pro_3/dsdt.dsl#L12164-L12216
Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
Signed-off-by: Jonas Dreßler <verdre@v0yd.nl>
---
drivers/net/wireless/marvell/mwifiex/pcie.c | 10 ++
.../wireless/marvell/mwifiex/pcie_quirks.c | 91 +++++++++++++++++++
.../wireless/marvell/mwifiex/pcie_quirks.h | 6 ++
3 files changed, 107 insertions(+)
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index d9acfea395ad..6e049236ae1a 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2969,6 +2969,16 @@ static void mwifiex_pcie_card_reset_work(struct mwifiex_adapter *adapter)
{
struct pcie_service_card *card = adapter->card;
+ /* On Surface 3, reset_wsid method removes then re-probes card by
+ * itself. So, need to place it here and skip performing any other
+ * reset-related works.
+ */
+ if (card->quirks & QUIRK_FW_RST_WSID_S3) {
+ mwifiex_pcie_reset_wsid_quirk(card->dev);
+ /* skip performing any other reset-related works */
+ return;
+ }
+
/* We can't afford to wait here; remove() might be waiting on us. If we
* can't grab the device lock, maybe we'll get another chance later.
*/
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
index b5f214fc1212..f0a6fa0a7ae5 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
@@ -9,10 +9,21 @@
* down, or causes NULL ptr deref).
*/
+#include <linux/acpi.h>
#include <linux/dmi.h>
#include "pcie_quirks.h"
+/* For reset_wsid quirk */
+#define ACPI_WSID_PATH "\\_SB.WSID"
+#define WSID_REV 0x0
+#define WSID_FUNC_WIFI_PWR_OFF 0x1
+#define WSID_FUNC_WIFI_PWR_ON 0x2
+/* WSID _DSM UUID: "534ea3bf-fcc2-4e7a-908f-a13978f0c7ef" */
+static const guid_t wsid_dsm_guid =
+ GUID_INIT(0x534ea3bf, 0xfcc2, 0x4e7a,
+ 0x90, 0x8f, 0xa1, 0x39, 0x78, 0xf0, 0xc7, 0xef);
+
/* quirk table based on DMI matching */
static const struct dmi_system_id mwifiex_quirk_table[] = {
{
@@ -81,6 +92,22 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
},
.driver_data = (void *)QUIRK_FW_RST_D3COLD,
},
+ {
+ .ident = "Surface 3",
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
+ },
+ .driver_data = (void *)QUIRK_FW_RST_WSID_S3,
+ },
+ {
+ .ident = "Surface Pro 3",
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Pro 3"),
+ },
+ .driver_data = 0,
+ },
{}
};
@@ -97,6 +124,9 @@ void mwifiex_initialize_quirks(struct pcie_service_card *card)
dev_info(&pdev->dev, "no quirks enabled\n");
if (card->quirks & QUIRK_FW_RST_D3COLD)
dev_info(&pdev->dev, "quirk reset_d3cold enabled\n");
+ if (card->quirks & QUIRK_FW_RST_WSID_S3)
+ dev_info(&pdev->dev,
+ "quirk reset_wsid for Surface 3 enabled\n");
}
static void mwifiex_pcie_set_power_d3cold(struct pci_dev *pdev)
@@ -153,3 +183,64 @@ int mwifiex_pcie_reset_d3cold_quirk(struct pci_dev *pdev)
return 0;
}
+
+int mwifiex_pcie_reset_wsid_quirk(struct pci_dev *pdev)
+{
+ acpi_handle handle;
+ union acpi_object *obj;
+ acpi_status status;
+
+ dev_info(&pdev->dev, "Using reset_wsid quirk to perform FW reset\n");
+
+ status = acpi_get_handle(NULL, ACPI_WSID_PATH, &handle);
+ if (ACPI_FAILURE(status)) {
+ dev_err(&pdev->dev, "No ACPI handle for path %s\n",
+ ACPI_WSID_PATH);
+ return -ENODEV;
+ }
+
+ if (!acpi_has_method(handle, "_DSM")) {
+ dev_err(&pdev->dev, "_DSM method not found\n");
+ return -ENODEV;
+ }
+
+ if (!acpi_check_dsm(handle, &wsid_dsm_guid,
+ WSID_REV, WSID_FUNC_WIFI_PWR_OFF)) {
+ dev_err(&pdev->dev,
+ "_DSM method doesn't support wifi power off func\n");
+ return -ENODEV;
+ }
+
+ if (!acpi_check_dsm(handle, &wsid_dsm_guid,
+ WSID_REV, WSID_FUNC_WIFI_PWR_ON)) {
+ dev_err(&pdev->dev,
+ "_DSM method doesn't support wifi power on func\n");
+ return -ENODEV;
+ }
+
+ /* card will be removed immediately after this call on Surface 3 */
+ dev_info(&pdev->dev, "turning wifi off...\n");
+ obj = acpi_evaluate_dsm(handle, &wsid_dsm_guid,
+ WSID_REV, WSID_FUNC_WIFI_PWR_OFF,
+ NULL);
+ if (!obj) {
+ dev_err(&pdev->dev,
+ "device _DSM execution failed for turning wifi off\n");
+ return -EIO;
+ }
+ ACPI_FREE(obj);
+
+ /* card will be re-probed immediately after this call on Surface 3 */
+ dev_info(&pdev->dev, "turning wifi on...\n");
+ obj = acpi_evaluate_dsm(handle, &wsid_dsm_guid,
+ WSID_REV, WSID_FUNC_WIFI_PWR_ON,
+ NULL);
+ if (!obj) {
+ dev_err(&pdev->dev,
+ "device _DSM execution failed for turning wifi on\n");
+ return -EIO;
+ }
+ ACPI_FREE(obj);
+
+ return 0;
+}
diff --git a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
index 549093067813..d90ada3f2daa 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
+++ b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
@@ -7,5 +7,11 @@
#define QUIRK_FW_RST_D3COLD BIT(0)
+/* Surface 3 and Surface Pro 3 have the same _DSM method but need to
+ * be handled differently. Currently, only S3 is supported.
+ */
+#define QUIRK_FW_RST_WSID_S3 BIT(1)
+
void mwifiex_initialize_quirks(struct pcie_service_card *card);
int mwifiex_pcie_reset_d3cold_quirk(struct pci_dev *pdev);
+int mwifiex_pcie_reset_wsid_quirk(struct pci_dev *pdev);
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 3/3] mwifiex: pcie: add reset_wsid quirk for Surface 3
2021-05-22 13:18 ` [RFC PATCH 3/3] mwifiex: pcie: add reset_wsid quirk for Surface 3 Jonas Dreßler
@ 2021-05-22 22:01 ` kernel test robot
2021-05-23 2:32 ` kernel test robot
2021-06-04 21:14 ` Bjorn Helgaas
2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-05-22 22:01 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3203 bytes --]
Hi "Jonas,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on wireless-drivers/master v5.13-rc2 next-20210521]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jonas-Dre-ler/mwifiex-Add-quirks-for-MS-Surface-devices/20210522-222428
base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/664dc875005de6def700fef775192ea019ebc8ab
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jonas-Dre-ler/mwifiex-Add-quirks-for-MS-Surface-devices/20210522-222428
git checkout 664dc875005de6def700fef775192ea019ebc8ab
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/net/wireless/marvell/mwifiex/pcie_quirks.c: In function 'mwifiex_pcie_reset_wsid_quirk':
>> drivers/net/wireless/marvell/mwifiex/pcie_quirks.c:202:7: error: implicit declaration of function 'acpi_has_method'; did you mean 'acpi_has_watchdog'? [-Werror=implicit-function-declaration]
202 | if (!acpi_has_method(handle, "_DSM")) {
| ^~~~~~~~~~~~~~~
| acpi_has_watchdog
>> drivers/net/wireless/marvell/mwifiex/pcie_quirks.c:207:7: error: implicit declaration of function 'acpi_check_dsm'; did you mean 'acpi_check_region'? [-Werror=implicit-function-declaration]
207 | if (!acpi_check_dsm(handle, &wsid_dsm_guid,
| ^~~~~~~~~~~~~~
| acpi_check_region
cc1: some warnings being treated as errors
vim +202 drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
186
187 int mwifiex_pcie_reset_wsid_quirk(struct pci_dev *pdev)
188 {
189 acpi_handle handle;
190 union acpi_object *obj;
191 acpi_status status;
192
193 dev_info(&pdev->dev, "Using reset_wsid quirk to perform FW reset\n");
194
195 status = acpi_get_handle(NULL, ACPI_WSID_PATH, &handle);
196 if (ACPI_FAILURE(status)) {
197 dev_err(&pdev->dev, "No ACPI handle for path %s\n",
198 ACPI_WSID_PATH);
199 return -ENODEV;
200 }
201
> 202 if (!acpi_has_method(handle, "_DSM")) {
203 dev_err(&pdev->dev, "_DSM method not found\n");
204 return -ENODEV;
205 }
206
> 207 if (!acpi_check_dsm(handle, &wsid_dsm_guid,
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 69115 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 3/3] mwifiex: pcie: add reset_wsid quirk for Surface 3
2021-05-22 13:18 ` [RFC PATCH 3/3] mwifiex: pcie: add reset_wsid quirk for Surface 3 Jonas Dreßler
2021-05-22 22:01 ` kernel test robot
@ 2021-05-23 2:32 ` kernel test robot
2021-06-04 21:14 ` Bjorn Helgaas
2 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-05-23 2:32 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3389 bytes --]
Hi "Jonas,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on wireless-drivers-next/master]
[also build test ERROR on wireless-drivers/master v5.13-rc2 next-20210521]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jonas-Dre-ler/mwifiex-Add-quirks-for-MS-Surface-devices/20210522-222428
base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master
config: riscv-randconfig-r004-20210523 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project e84a9b9bb3051c35dea993cdad7b3d2575638f85)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/664dc875005de6def700fef775192ea019ebc8ab
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Jonas-Dre-ler/mwifiex-Add-quirks-for-MS-Surface-devices/20210522-222428
git checkout 664dc875005de6def700fef775192ea019ebc8ab
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/net/wireless/marvell/mwifiex/pcie_quirks.c:202:7: error: implicit declaration of function 'acpi_has_method' [-Werror,-Wimplicit-function-declaration]
if (!acpi_has_method(handle, "_DSM")) {
^
drivers/net/wireless/marvell/mwifiex/pcie_quirks.c:202:7: note: did you mean 'acpi_has_watchdog'?
include/linux/acpi.h:1288:20: note: 'acpi_has_watchdog' declared here
static inline bool acpi_has_watchdog(void) { return false; }
^
>> drivers/net/wireless/marvell/mwifiex/pcie_quirks.c:207:7: error: implicit declaration of function 'acpi_check_dsm' [-Werror,-Wimplicit-function-declaration]
if (!acpi_check_dsm(handle, &wsid_dsm_guid,
^
2 errors generated.
vim +/acpi_has_method +202 drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
186
187 int mwifiex_pcie_reset_wsid_quirk(struct pci_dev *pdev)
188 {
189 acpi_handle handle;
190 union acpi_object *obj;
191 acpi_status status;
192
193 dev_info(&pdev->dev, "Using reset_wsid quirk to perform FW reset\n");
194
195 status = acpi_get_handle(NULL, ACPI_WSID_PATH, &handle);
196 if (ACPI_FAILURE(status)) {
197 dev_err(&pdev->dev, "No ACPI handle for path %s\n",
198 ACPI_WSID_PATH);
199 return -ENODEV;
200 }
201
> 202 if (!acpi_has_method(handle, "_DSM")) {
203 dev_err(&pdev->dev, "_DSM method not found\n");
204 return -ENODEV;
205 }
206
> 207 if (!acpi_check_dsm(handle, &wsid_dsm_guid,
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 34104 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 3/3] mwifiex: pcie: add reset_wsid quirk for Surface 3
2021-05-22 13:18 ` [RFC PATCH 3/3] mwifiex: pcie: add reset_wsid quirk for Surface 3 Jonas Dreßler
2021-05-22 22:01 ` kernel test robot
2021-05-23 2:32 ` kernel test robot
@ 2021-06-04 21:14 ` Bjorn Helgaas
2021-06-04 21:26 ` Andy Shevchenko
2 siblings, 1 reply; 10+ messages in thread
From: Bjorn Helgaas @ 2021-06-04 21:14 UTC (permalink / raw)
To: Jonas Dreßler
Cc: Amitkumar Karwar, Ganapathi Bhat, Sharvari Harisangam,
Xinming Hu, Kalle Valo, David S. Miller, Jakub Kicinski,
Tsuchiya Yuto, linux-wireless, netdev, linux-kernel, linux-pci,
Maximilian Luz, Andy Shevchenko, Bjorn Helgaas, Pali Rohár
On Sat, May 22, 2021 at 03:18:27PM +0200, Jonas Dreßler wrote:
> From: Tsuchiya Yuto <kitakar@gmail.com>
>
> This commit adds reset_wsid quirk and uses this quirk for Surface 3 on
> card reset.
>
> To reset mwifiex on Surface 3, it seems that calling the _DSM method
> exists in \_SB.WSID [1] device is required.
>
> On Surface 3, calling the _DSM method removes/re-probes the card by
> itself. So, need to place the reset function before performing FLR and
> skip performing any other reset-related works.
Maybe this is a nit-pick, but I understand "probing" to be something
the OS does, namely what we normally call "enumeration," i.e.,
discovering a device.
So it sounds like the _DSM causes a logical hot-removal of the card,
which the PCI hotplug driver should notice and it should remove the
driver and remove the pci_dev.
And the _DSM also causes a hot-add (reading the code below, it looks
like this is actually a second _DSM), which the PCI hotplug driver
should also notice and enumerate the bus (i.e., it reads config space
looking for a device). This all would cause a new pci_dev to be
allocated, resources assigned for its BARs, and the driver .probe()
method to be called again?
That seems like a lot, so maybe I didn't understand what's actually
happening.
> Note that Surface Pro 3 also has the WSID device [2], but it seems to need
> more work. This commit only supports Surface 3 yet.
>
> [1] https://github.com/linux-surface/acpidumps/blob/05cba925f3a515f222acb5b3551a032ddde958fe/surface_3/dsdt.dsl#L11947-L12011
> [2] https://github.com/linux-surface/acpidumps/blob/05cba925f3a515f222acb5b3551a032ddde958fe/surface_pro_3/dsdt.dsl#L12164-L12216
>
> Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
> Signed-off-by: Jonas Dreßler <verdre@v0yd.nl>
> ---
> drivers/net/wireless/marvell/mwifiex/pcie.c | 10 ++
> .../wireless/marvell/mwifiex/pcie_quirks.c | 91 +++++++++++++++++++
> .../wireless/marvell/mwifiex/pcie_quirks.h | 6 ++
> 3 files changed, 107 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index d9acfea395ad..6e049236ae1a 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -2969,6 +2969,16 @@ static void mwifiex_pcie_card_reset_work(struct mwifiex_adapter *adapter)
> {
> struct pcie_service_card *card = adapter->card;
>
> + /* On Surface 3, reset_wsid method removes then re-probes card by
> + * itself. So, need to place it here and skip performing any other
> + * reset-related works.
> + */
> + if (card->quirks & QUIRK_FW_RST_WSID_S3) {
> + mwifiex_pcie_reset_wsid_quirk(card->dev);
> + /* skip performing any other reset-related works */
> + return;
> + }
> +
> /* We can't afford to wait here; remove() might be waiting on us. If we
> * can't grab the device lock, maybe we'll get another chance later.
> */
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
> index b5f214fc1212..f0a6fa0a7ae5 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
> @@ -9,10 +9,21 @@
> * down, or causes NULL ptr deref).
> */
>
> +#include <linux/acpi.h>
> #include <linux/dmi.h>
>
> #include "pcie_quirks.h"
>
> +/* For reset_wsid quirk */
> +#define ACPI_WSID_PATH "\\_SB.WSID"
> +#define WSID_REV 0x0
> +#define WSID_FUNC_WIFI_PWR_OFF 0x1
> +#define WSID_FUNC_WIFI_PWR_ON 0x2
> +/* WSID _DSM UUID: "534ea3bf-fcc2-4e7a-908f-a13978f0c7ef" */
> +static const guid_t wsid_dsm_guid =
> + GUID_INIT(0x534ea3bf, 0xfcc2, 0x4e7a,
> + 0x90, 0x8f, 0xa1, 0x39, 0x78, 0xf0, 0xc7, 0xef);
> +
> /* quirk table based on DMI matching */
> static const struct dmi_system_id mwifiex_quirk_table[] = {
> {
> @@ -81,6 +92,22 @@ static const struct dmi_system_id mwifiex_quirk_table[] = {
> },
> .driver_data = (void *)QUIRK_FW_RST_D3COLD,
> },
> + {
> + .ident = "Surface 3",
> + .matches = {
> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
> + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface 3"),
> + },
> + .driver_data = (void *)QUIRK_FW_RST_WSID_S3,
> + },
> + {
> + .ident = "Surface Pro 3",
> + .matches = {
> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
> + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Pro 3"),
> + },
> + .driver_data = 0,
> + },
> {}
> };
>
> @@ -97,6 +124,9 @@ void mwifiex_initialize_quirks(struct pcie_service_card *card)
> dev_info(&pdev->dev, "no quirks enabled\n");
> if (card->quirks & QUIRK_FW_RST_D3COLD)
> dev_info(&pdev->dev, "quirk reset_d3cold enabled\n");
> + if (card->quirks & QUIRK_FW_RST_WSID_S3)
> + dev_info(&pdev->dev,
> + "quirk reset_wsid for Surface 3 enabled\n");
> }
>
> static void mwifiex_pcie_set_power_d3cold(struct pci_dev *pdev)
> @@ -153,3 +183,64 @@ int mwifiex_pcie_reset_d3cold_quirk(struct pci_dev *pdev)
>
> return 0;
> }
> +
> +int mwifiex_pcie_reset_wsid_quirk(struct pci_dev *pdev)
> +{
> + acpi_handle handle;
> + union acpi_object *obj;
> + acpi_status status;
> +
> + dev_info(&pdev->dev, "Using reset_wsid quirk to perform FW reset\n");
> +
> + status = acpi_get_handle(NULL, ACPI_WSID_PATH, &handle);
> + if (ACPI_FAILURE(status)) {
> + dev_err(&pdev->dev, "No ACPI handle for path %s\n",
> + ACPI_WSID_PATH);
> + return -ENODEV;
> + }
> +
> + if (!acpi_has_method(handle, "_DSM")) {
> + dev_err(&pdev->dev, "_DSM method not found\n");
> + return -ENODEV;
> + }
> +
> + if (!acpi_check_dsm(handle, &wsid_dsm_guid,
> + WSID_REV, WSID_FUNC_WIFI_PWR_OFF)) {
> + dev_err(&pdev->dev,
> + "_DSM method doesn't support wifi power off func\n");
> + return -ENODEV;
> + }
> +
> + if (!acpi_check_dsm(handle, &wsid_dsm_guid,
> + WSID_REV, WSID_FUNC_WIFI_PWR_ON)) {
> + dev_err(&pdev->dev,
> + "_DSM method doesn't support wifi power on func\n");
> + return -ENODEV;
> + }
> +
> + /* card will be removed immediately after this call on Surface 3 */
> + dev_info(&pdev->dev, "turning wifi off...\n");
> + obj = acpi_evaluate_dsm(handle, &wsid_dsm_guid,
> + WSID_REV, WSID_FUNC_WIFI_PWR_OFF,
> + NULL);
> + if (!obj) {
> + dev_err(&pdev->dev,
> + "device _DSM execution failed for turning wifi off\n");
> + return -EIO;
> + }
> + ACPI_FREE(obj);
> +
> + /* card will be re-probed immediately after this call on Surface 3 */
> + dev_info(&pdev->dev, "turning wifi on...\n");
> + obj = acpi_evaluate_dsm(handle, &wsid_dsm_guid,
> + WSID_REV, WSID_FUNC_WIFI_PWR_ON,
> + NULL);
> + if (!obj) {
> + dev_err(&pdev->dev,
> + "device _DSM execution failed for turning wifi on\n");
> + return -EIO;
> + }
> + ACPI_FREE(obj);
> +
> + return 0;
> +}
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
> index 549093067813..d90ada3f2daa 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
> @@ -7,5 +7,11 @@
>
> #define QUIRK_FW_RST_D3COLD BIT(0)
>
> +/* Surface 3 and Surface Pro 3 have the same _DSM method but need to
> + * be handled differently. Currently, only S3 is supported.
> + */
> +#define QUIRK_FW_RST_WSID_S3 BIT(1)
> +
> void mwifiex_initialize_quirks(struct pcie_service_card *card);
> int mwifiex_pcie_reset_d3cold_quirk(struct pci_dev *pdev);
> +int mwifiex_pcie_reset_wsid_quirk(struct pci_dev *pdev);
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH 3/3] mwifiex: pcie: add reset_wsid quirk for Surface 3
2021-06-04 21:14 ` Bjorn Helgaas
@ 2021-06-04 21:26 ` Andy Shevchenko
0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2021-06-04 21:26 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Jonas Dreßler, Amitkumar Karwar, Ganapathi Bhat,
Sharvari Harisangam, Xinming Hu, Kalle Valo, David S. Miller,
Jakub Kicinski, Tsuchiya Yuto, linux-wireless, netdev,
linux-kernel, linux-pci, Maximilian Luz, Andy Shevchenko,
Bjorn Helgaas, Pali Rohár
On Sat, Jun 5, 2021 at 12:15 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Sat, May 22, 2021 at 03:18:27PM +0200, Jonas Dreßler wrote:
> > From: Tsuchiya Yuto <kitakar@gmail.com>
> >
> > This commit adds reset_wsid quirk and uses this quirk for Surface 3 on
> > card reset.
> >
> > To reset mwifiex on Surface 3, it seems that calling the _DSM method
> > exists in \_SB.WSID [1] device is required.
> >
> > On Surface 3, calling the _DSM method removes/re-probes the card by
> > itself. So, need to place the reset function before performing FLR and
> > skip performing any other reset-related works.
>
> Maybe this is a nit-pick, but I understand "probing" to be something
> the OS does, namely what we normally call "enumeration," i.e.,
> discovering a device.
>
> So it sounds like the _DSM causes a logical hot-removal of the card,
> which the PCI hotplug driver should notice and it should remove the
> driver and remove the pci_dev.
>
> And the _DSM also causes a hot-add (reading the code below, it looks
> like this is actually a second _DSM),
_DSM can be only one (single) per device node in ACPI.
But _DSM may have "functions", that's what we see here.
> which the PCI hotplug driver
> should also notice and enumerate the bus (i.e., it reads config space
> looking for a device). This all would cause a new pci_dev to be
> allocated, resources assigned for its BARs, and the driver .probe()
> method to be called again?
>
> That seems like a lot, so maybe I didn't understand what's actually
> happening.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 10+ messages in thread