| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20210522184416.mscbmay27jciy2hv@archlinux>
Date: Sun, 23 May 2021 00:14:16 +0530
From: Amey Narkhede <ameynarkhede03@...il.com>
To: Jonas Dreßler <verdre@...d.nl>
Cc: Alex Williamson <alex.williamson@...hat.com>,
Tsuchiya Yuto <kitakar@...il.com>,
linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
Maximilian Luz <luzmaximilian@...il.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Pali Rohár <pali@...nel.org>,
Krzysztof Wilczyński <kw@...ux.com>,
Amitkumar Karwar <amitkarwar@...il.com>,
Ganapathi Bhat <ganapathi017@...il.com>,
Sharvari Harisangam <sharvari.harisangam@....com>,
Xinming Hu <huxinming820@...il.com>,
Kalle Valo <kvalo@...eaurora.org>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>
Subject: Re: [RFC PATCH 2/3] mwifiex: pcie: add reset_d3cold quirk for
Surface gen4+ devices
On 21/05/22 03:18PM, Jonas Dreßler wrote:
> From: Tsuchiya Yuto <kitakar@...il.com>
>
> 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)
>
May I know how did you reset only the wifi device when you encountered
this error?
> Signed-off-by: Tsuchiya Yuto <kitakar@...il.com>
> Signed-off-by: Jonas Dreßler <verdre@...d.nl>
> ---
> drivers/net/wireless/marvell/mwifiex/pcie.c | 7 +
> .../wireless/marvell/mwifiex/pcie_quirks.c | 123 ++++++++++++++++++
> .../wireless/marvell/mwifiex/pcie_quirks.h | 3 +
> 3 files changed, 133 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index 02fdce926de5..d9acfea395ad 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -528,6 +528,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__);
>
> card->pci_reset_ongoing = true;
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
> index 4064f99b36ba..b5f214fc1212 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.c
> @@ -15,6 +15,72 @@
>
> /* 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 = (void *)QUIRK_FW_RST_D3COLD,
> + },
> + {
> + .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 = (void *)QUIRK_FW_RST_D3COLD,
> + },
> + {
> + .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 = (void *)QUIRK_FW_RST_D3COLD,
> + },
> + {
> + .ident = "Surface Pro 6",
> + .matches = {
> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
> + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Pro 6"),
> + },
> + .driver_data = (void *)QUIRK_FW_RST_D3COLD,
> + },
> + {
> + .ident = "Surface Book 1",
> + .matches = {
> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
> + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Book"),
> + },
> + .driver_data = (void *)QUIRK_FW_RST_D3COLD,
> + },
> + {
> + .ident = "Surface Book 2",
> + .matches = {
> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
> + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Book 2"),
> + },
> + .driver_data = (void *)QUIRK_FW_RST_D3COLD,
> + },
> + {
> + .ident = "Surface Laptop 1",
> + .matches = {
> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
> + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Laptop"),
> + },
> + .driver_data = (void *)QUIRK_FW_RST_D3COLD,
> + },
> + {
> + .ident = "Surface Laptop 2",
> + .matches = {
> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
> + DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Surface Laptop 2"),
> + },
> + .driver_data = (void *)QUIRK_FW_RST_D3COLD,
> + },
> {}
> };
>
> @@ -29,4 +95,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);
> +}
pci_set_power_state with PCI_D3cold state calls
pci_bus_set_current_state(dev->subordinate, PCI_D3cold).
Maybe this was the reason for the earlier problem you had?
Not 100% sure about this though CCing: Alex
> +
> +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);
On the side note just save and restore is enough in this case?
What would be the device <-> driver state after the reset as you
are calling this on parent_pdev below so that affects other
devices on bus?
Thanks,
Amey
> +
> + 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 7a1fe3b3a61a..549093067813 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie_quirks.h
> @@ -5,4 +5,7 @@
>
> #include "pcie.h"
>
> +#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.31.1
>
Powered by blists - more mailing lists