[AMD Public Use]
Ping... Could I get some comments or RB on this latest series.
Thanks,
Prike
> -----Original Message-----
> From: Liang, Prike
> Sent: Thursday, May 6, 2021 11:08 AM
> To: linux-nvme@xxxxxxxxxxxxxxxxxxx; kbusch@xxxxxxxxxx; hch@xxxxxxxxxxxxx;
> Chaitanya.Kulkarni@xxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; linux-
> pci@xxxxxxxxxxxxxxx; Bjorn Helgaas <helgaas@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; S-k, Shyam-sundar <Shyam-sundar.S-
> k@xxxxxxx>; Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>;
> rjw@xxxxxxxxxxxxx
> Subject: RE: [PATCH v5 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt
>
> + linux-pci for further review.
>
> Thanks,
> > -----Original Message-----
> > From: Liang, Prike <Prike.Liang@xxxxxxx>
> > Sent: Thursday, April 22, 2021 9:19 AM
> > To: linux-nvme@xxxxxxxxxxxxxxxxxxx; kbusch@xxxxxxxxxx;
> > hch@xxxxxxxxxxxxx; Chaitanya.Kulkarni@xxxxxxx;
> > gregkh@xxxxxxxxxxxxxxxxxxx
> > Cc: stable@xxxxxxxxxxxxxxx; Deucher, Alexander
> > <Alexander.Deucher@xxxxxxx>; S-k, Shyam-sundar <Shyam-sundar.S-
> > k@xxxxxxx>; Liang, Prike <Prike.Liang@xxxxxxx>; Chaitanya Kulkarni
> > <chaitanya.kulkarni@xxxxxxx>
> > Subject: [PATCH v5 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt
> >
> > In the NVMe controller default suspend-resume seems only save/restore
> > the NVMe link state by APST opt and the NVMe remains in D0 during this
> time.
> > Then the NVMe device will be shutdown by SMU firmware in the s2idle
> > entry and then will lost the NVMe power context during s2idle
> > resume.Finally, the NVMe command queue request will be processed
> > abnormally and result in access timeout.This issue can be settled by
> > using PCIe power set with simple suspend-resume process path instead of
> APST get/set opt.
> >
> > In this patch prepare a PCIe RC bus flag to identify the platform
> > whether need the quirk.
> >
> > Cc: <stable@xxxxxxxxxxxxxxx> # 5.10+
> > Signed-off-by: Prike Liang <Prike.Liang@xxxxxxx>
> > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
> > [ck: split patches for nvme and pcie]
> > Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>
> > Suggested-by: Keith Busch <kbusch@xxxxxxxxxx>
> > Acked-by: Keith Busch <kbusch@xxxxxxxxxx>
> > ---
> > Changes in v2:
> > Fix the patch format and check chip root complex DID instead of PCIe
> > RP to avoid the storage device plugged in internal PCIe RP by USB adaptor.
> >
> > Changes in v3:
> > According to Christoph Hellwig do NVME PCIe related identify opt
> > better in PCIe quirk driver rather than in NVME module.
> >
> > Changes in v4:
> > Split the fix to PCIe and NVMe part and then call the pci_dev_put()
> > put the device reference count and finally refine the commit info.
> >
> > Changes in v5:
> > According to Christoph Hellwig and Keith Busch better use a
> > passthrough
> > device(bus) gloable flag to identify the NVMe shutdown opt rather than
> > look up the device BDF.
> > ---
> > drivers/pci/probe.c | 5 ++++-
> > drivers/pci/quirks.c | 7 +++++++
> > include/linux/pci.h | 2 ++
> > 3 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index
> > 953f15a..34ba691e 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
> > 653660e3..7c4bb8e 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -312,6 +312,13 @@ 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 53f4904..dc65219
> > 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 */
> > --
> > 2.7.4
