From: Amey Narkhede <ameynarkhede03@gmail.com>
To: "Jonas Dreßler" <verdre@v0yd.nl>
Cc: "Alex Williamson" <alex.williamson@redhat.com>,
"Tsuchiya Yuto" <kitakar@gmail.com>,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
"Maximilian Luz" <luzmaximilian@gmail.com>,
"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
"Bjorn Helgaas" <bhelgaas@google.com>,
"Pali Rohár" <pali@kernel.org>,
"Krzysztof Wilczyński" <kw@linux.com>,
"Amitkumar Karwar" <amitkarwar@gmail.com>,
"Ganapathi Bhat" <ganapathi017@gmail.com>,
"Sharvari Harisangam" <sharvari.harisangam@nxp.com>,
"Xinming Hu" <huxinming820@gmail.com>,
"Kalle Valo" <kvalo@codeaurora.org>,
"David S. Miller" <davem@davemloft.net>,
"Jakub Kicinski" <kuba@kernel.org>
Subject: Re: [RFC PATCH 2/3] mwifiex: pcie: add reset_d3cold quirk for Surface gen4+ devices
Date: Sun, 23 May 2021 00:14:16 +0530 [thread overview]
Message-ID: <20210522184416.mscbmay27jciy2hv@archlinux> (raw)
In-Reply-To: <20210522131827.67551-3-verdre@v0yd.nl>
On 21/05/22 03:18PM, Jonas Dreßler wrote:
> From: Tsuchiya Yuto <kitakar@gmail.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@gmail.com>
> Signed-off-by: Jonas Dreßler <verdre@v0yd.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
>
next prev parent reply other threads:[~2021-05-22 18:44 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-22 13:18 [RFC PATCH 0/3] mwifiex: Add quirks for MS Surface devices Jonas Dreßler
2021-05-22 13:18 ` [RFC PATCH 1/3] mwifiex: pcie: add DMI-based quirk implementation for " Jonas Dreßler
2021-05-22 13:18 ` [RFC PATCH 2/3] mwifiex: pcie: add reset_d3cold quirk for Surface gen4+ devices Jonas Dreßler
2021-05-22 18:44 ` Amey Narkhede [this message]
2021-05-23 10:30 ` Jonas Dreßler
[not found] ` <1a844abf-2259-ff4f-d49d-de95870345dc@mailbox.org>
2021-05-24 20:27 ` Amey Narkhede
2021-07-09 14:10 ` Jonas Dreßler
2021-06-04 21:08 ` Bjorn Helgaas
2021-05-22 13:18 ` [RFC PATCH 3/3] mwifiex: pcie: add reset_wsid quirk for Surface 3 Jonas Dreßler
2021-06-04 21:14 ` Bjorn Helgaas
2021-06-04 21:26 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210522184416.mscbmay27jciy2hv@archlinux \
--to=ameynarkhede03@gmail.com \
--cc=alex.williamson@redhat.com \
--cc=amitkarwar@gmail.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=bhelgaas@google.com \
--cc=davem@davemloft.net \
--cc=ganapathi017@gmail.com \
--cc=huxinming820@gmail.com \
--cc=kitakar@gmail.com \
--cc=kuba@kernel.org \
--cc=kvalo@codeaurora.org \
--cc=kw@linux.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=luzmaximilian@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pali@kernel.org \
--cc=sharvari.harisangam@nxp.com \
--cc=verdre@v0yd.nl \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).