On 16.9.2020 11.06, Greg KH wrote: > On Mon, Aug 31, 2020 at 06:52:46AM +0000, Nehal Bakulchandra Shah wrote: >> From: Nehal Bakulchandra Shah <Nehal-Bakulchandra.shah@xxxxxxx> >> >> On some platform of AMD, S3 fails with HCE and SRE errors.To fix this, >> sparse controller enable bit has to be disabled. >> >> Signed-off-by: Nehal Bakulchandra Shah <Nehal-Bakulchandra.shah@xxxxxxx> >> --- >> drivers/usb/host/xhci-pci.c | 12 ++++++++++++ >> drivers/usb/host/xhci.h | 1 + >> 2 files changed, 13 insertions(+) >> >> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c >> @@ -371,6 +374,15 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct pci_device_id *id) >> /* USB 2.0 roothub is stored in the PCI device now. */ >> hcd = dev_get_drvdata(&dev->dev); >> xhci = hcd_to_xhci(hcd); >> + >> + if (xhci->quirks & XHCI_DISABLE_SPARSE) { >> + u32 reg; >> + >> + reg = readl(hcd->regs + 0xC12C); >> + reg &= ~BIT(17); > > And is this the proper place to be testing for quirks and applying them? > Why not do the above in the xhci_pci_quirks() call? xhci_pci_quirks() is a confusing name, it actually only sets quirk flags based on PCI vendor/device. Anyway, point is still valid, this level of bitops in probe isn't very nice. Turn it into a separate function, and call it from xhci_pci_setup(), or if flag needs to be cleared more often, and is related to S3 problems then possibly from xhci_pci_suspend() -Mathias