2020-10-28 22:17:14

by Tsuchiya Yuto

[permalink] [raw]
Subject: [RFC PATCH 1/3] mwifiex: pcie: add DMI-based quirk impl for Surface devices

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 <[email protected]>
---
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


2020-11-02 10:32:45

by Tsuchiya Yuto

[permalink] [raw]
Subject: Re: [RFC PATCH 1/3] mwifiex: pcie: add DMI-based quirk impl for Surface devices

On Wed, 2020-10-28 at 17:27 +0200, Andy Shevchenko wrote:
> On Wed, Oct 28, 2020 at 11:27:51PM +0900, Tsuchiya Yuto wrote:
>> 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.
>
> I guess you might need to resend this (and possible other PCI pm related)
> patches to linux-pci@ and Bjorn in Cc.
>
> They may advise possible other (better) solutions.

Thanks for the advice! I also feel it's better. And if I resend this to
the linux-pci mailing list, I feel that I should try to use
drivers/pci/quirks.c instead if possible. But if I do so with this quirk
implementation, it might be possible that it'll be too big change.

So, I'll just resend this series (with changes so that it can be used
for powr_save quirk as well like I said in the reply to another series)
to the linux-pci mailing list and Bjorn (and linux-wireless mailing list
as well) and see what they think.

>> Signed-off-by: Tsuchiya Yuto <[email protected]>
>> ---
>> 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
>>
>