Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
From: Keith Busch <kbusch@kernel.org>
To: "Liang, Prike" <Prike.Liang@amd.com>
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: Thu, 15 Apr 2021 15:25:44 -0700
Message-ID: <20210415222544.GA2760247@dhcp-10-100-145-180.wdc.com> (raw)
In-Reply-To: <BYAPR12MB3238E0366477A6CDE7AC9B94FB4D9@BYAPR12MB3238.namprd12.prod.outlook.com>

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?

---
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

  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-15  3:52 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 [this message]
2021-04-16  6:51       ` Liang, Prike
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=20210415222544.GA2760247@dhcp-10-100-145-180.wdc.com \
    --to=kbusch@kernel.org \
    --cc=Alexander.Deucher@amd.com \
    --cc=Chaitanya.Kulkarni@wdc.com \
    --cc=Prike.Liang@amd.com \
    --cc=Shyam-sundar.S-k@amd.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@infradead.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

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org
	public-inbox-index linux-nvme

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git