From: "Liang, Prike" <Prike.Liang@amd.com>
To: Keith Busch <kbusch@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>,
"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
"Chaitanya.Kulkarni@wdc.com" <Chaitanya.Kulkarni@wdc.com>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"stable@vger.kernel.org" <stable@vger.kernel.org>,
"Deucher, Alexander" <Alexander.Deucher@amd.com>,
"S-k, Shyam-sundar" <Shyam-sundar.S-k@amd.com>
Subject: RE: [PATCH v4 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt
Date: Fri, 16 Apr 2021 06:51:06 +0000 [thread overview]
Message-ID: <BYAPR12MB3238528F1D91A921FF1201EAFB4C9@BYAPR12MB3238.namprd12.prod.outlook.com> (raw)
In-Reply-To: <20210415222544.GA2760247@dhcp-10-100-145-180.wdc.com>
[AMD Public Use]
> From: Keith Busch <kbusch@kernel.org>
> Sent: Friday, April 16, 2021 6:26 AM
> To: Liang, Prike <Prike.Liang@amd.com>
> Cc: Christoph Hellwig <hch@infradead.org>; linux-nvme@lists.infradead.org;
> Chaitanya.Kulkarni@wdc.com; gregkh@linuxfoundation.org;
> stable@vger.kernel.org; Deucher, Alexander
> <Alexander.Deucher@amd.com>; S-k, Shyam-sundar <Shyam-sundar.S-
> k@amd.com>
> Subject: Re: [PATCH v4 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt
>
> On Thu, Apr 15, 2021 at 09:41:53AM +0000, Liang, Prike wrote:
> > > From: Christoph Hellwig <hch@infradead.org>
> >
> > > I'd also much prefer if the flag is used on every pci_dev that is
> > > affected by the broken behavior rather than requiring another lookup in
> the driver.
> > Sorry can't get the meaning, could you give more detail how to implement
> this?
>
> The suggestion is child devices of the pci bus inherit the quirk so drivers don't
> need to look up the parent device that requires it.
>
> That makes sense for a couple reasons. For one, your hard-coded 0:0.0
> probably aligns to actual implementations, but I did't find a spec requirement
> that the host bridge occupy that BDf, so not having to look up a fixed location
> is more flexible.
>
> If I understand the suggestion correctly, I think it's probably easier to thread
> the quirk through the pci_bus->bus_flags. Does the below
> (untested) make sense?
>
Thanks Busch for elaborate clarification. The PCIe RC bus flag can pass to NVMe device successfully and it works well for this case. I will update the patch accordingly.
> ---
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index
> d47bb18b976a..022ff6cf202f 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2834,6 +2834,9 @@ static bool nvme_acpi_storage_d3(struct pci_dev
> *dev)
> acpi_status status;
> u8 val;
>
> +if (dev->bus->bus_flags & PCI_BUS_FLAGS_DISABLE_ON_S2I)
> +return true;
> +
> /*
> * Look for _DSD property specifying that the storage device on the
> port
> * must use D3 to support deep platform power savings during diff --
> git a/drivers/pci/probe.c b/drivers/pci/probe.c index
> 953f15abc850..34ba691ec545 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -558,10 +558,13 @@ static struct pci_bus *pci_alloc_bus(struct pci_bus
> *parent)
> INIT_LIST_HEAD(&b->resources);
> b->max_bus_speed = PCI_SPEED_UNKNOWN;
> b->cur_bus_speed = PCI_SPEED_UNKNOWN;
> +if (parent) {
> #ifdef CONFIG_PCI_DOMAINS_GENERIC
> -if (parent)
> b->domain_nr = parent->domain_nr;
> #endif
> +if (parent->bus_flags & PCI_BUS_FLAGS_DISABLE_ON_S2I)
> +b->bus_flags |= PCI_BUS_FLAGS_DISABLE_ON_S2I;
> +}
> return b;
> }
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> 653660e3ba9e..e8f74661138a 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -312,6 +312,14 @@ static void quirk_nopciamd(struct pci_dev *dev) }
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD,
> PCI_DEVICE_ID_AMD_8151_0,quirk_nopciamd);
>
> +static void quirk_amd_s2i_fixup(struct pci_dev *dev) {
> +dev->bus->bus_flags |= PCI_BUS_FLAGS_DISABLE_ON_S2I;
> +pci_info(dev, "AMD simple suspend opt enabled\n");
> +
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_AMD, 0x1630,
> +quirk_amd_s2i_fixup);
> +
> /* Triton requires workarounds to be used by the drivers */ static void
> quirk_triton(struct pci_dev *dev) { diff --git a/include/linux/pci.h
> b/include/linux/pci.h index 86c799c97b77..7072e2ec88a2 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -240,6 +240,8 @@ enum pci_bus_flags {
> PCI_BUS_FLAGS_NO_MMRBC= (__force pci_bus_flags_t) 2,
> PCI_BUS_FLAGS_NO_AERSID= (__force pci_bus_flags_t) 4,
> PCI_BUS_FLAGS_NO_EXTCFG= (__force pci_bus_flags_t) 8,
> +/* Driver must pci_disable_device() for suspend-to-idle */
> +PCI_BUS_FLAGS_DISABLE_ON_S2I= (__force pci_bus_flags_t) 16,
> };
>
> /* Values from Link Status register, PCIe r3.1, sec 7.8.8 */
> --
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
next prev parent reply other threads:[~2021-04-16 7:06 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-15 3:52 [PATCH v4 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt Prike Liang
2021-04-15 3:52 ` [PATCH v4 2/2] nvme-pci: add AMD PCIe quirk for suspend/resume Prike Liang
2021-04-15 6:29 ` Greg KH
2021-04-15 7:39 ` Liang, Prike
2021-04-15 8:20 ` [PATCH v4 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt Christoph Hellwig
2021-04-15 9:41 ` Liang, Prike
2021-04-15 22:25 ` Keith Busch
2021-04-16 6:51 ` Liang, Prike [this message]
2021-04-30 17:50 ` Bjorn Helgaas
2021-05-03 7:14 ` Christoph Hellwig
2021-05-03 14:57 ` Keith Busch
2021-05-03 15:50 ` Bjorn Helgaas
2021-05-06 3:22 ` Liang, Prike
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=BYAPR12MB3238528F1D91A921FF1201EAFB4C9@BYAPR12MB3238.namprd12.prod.outlook.com \
--to=prike.liang@amd.com \
--cc=Alexander.Deucher@amd.com \
--cc=Chaitanya.Kulkarni@wdc.com \
--cc=Shyam-sundar.S-k@amd.com \
--cc=gregkh@linuxfoundation.org \
--cc=hch@infradead.org \
--cc=kbusch@kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=stable@vger.kernel.org \
/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).