On Wed, 28 Apr 2021 19:49:06 -0500 Shanker Donthineni <sdonthineni@xxxxxxxxxx> wrote: > The _RST is a standard method specified in the ACPI specification. It > provides a function level reset when it is described in the acpi_device > context associated with PCI-device. > > Implement a new reset function pci_dev_acpi_reset() for probing RST > method and execute if it is defined in the firmware. The ACPI binding > information is available only after calling device_add(), so move > pci_init_reset_methods() to end of the pci_device_add(). > > The default priority of the acpi reset is set to below device-specific > and above hardware resets. > > Signed-off-by: Shanker Donthineni <sdonthineni@xxxxxxxxxx> > --- > changes since v2: > - fix typo in the commit text > changes since v2: > - rebase patch on top of https://lore.kernel.org/linux-pci/20210409192324.30080-1-ameynarkhede03@xxxxxxxxx/ > > drivers/pci/pci.c | 30 ++++++++++++++++++++++++++++++ > drivers/pci/probe.c | 2 +- > include/linux/pci.h | 2 +- > 3 files changed, 32 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 664cf2d358d6..510f9224a3b0 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -5076,6 +5076,35 @@ static void pci_dev_restore(struct pci_dev *dev) > err_handler->reset_done(dev); > } > > +/** > + * pci_dev_acpi_reset - do a function level reset using _RST method > + * @dev: device to reset > + * @probe: check if _RST method is included in the acpi_device context. > + */ > +static int pci_dev_acpi_reset(struct pci_dev *dev, int probe) > +{ > +#ifdef CONFIG_ACPI > + acpi_handle handle = ACPI_HANDLE(&dev->dev); > + > + /* Return -ENOTTY if _RST method is not included in the dev context */ > + if (!handle || !acpi_has_method(handle, "_RST")) > + return -ENOTTY; > + > + /* Return 0 for probe phase indicating that we can reset this device */ > + if (probe) > + return 0; > + > + /* Invoke _RST() method to perform a function level reset */ > + if (ACPI_FAILURE(acpi_evaluate_object(handle, "_RST", NULL, NULL))) { > + pci_warn(dev, "Failed to reset the device\n"); > + return -EINVAL; > + } > + return 0; > +#else > + return -ENOTTY; > +#endif > +} > + > /* > * The ordering for functions in pci_reset_fn_methods > * is required for reset_methods byte array defined > @@ -5083,6 +5112,7 @@ static void pci_dev_restore(struct pci_dev *dev) > */ > const struct pci_reset_fn_method pci_reset_fn_methods[] = { > { .reset_fn = &pci_dev_specific_reset, .name = "device_specific" }, > + { .reset_fn = &pci_dev_acpi_reset, .name = "acpi_reset" }, Would it make sense to name this "acpi_rst" after the method name? Otherwise "_reset" is a bit redundant to the sysfs attribute, we could simply name it "acpi" to indicate an ACPI based reset. Thanks, Alex > { .reset_fn = &pcie_reset_flr, .name = "flr" }, > { .reset_fn = &pci_af_flr, .name = "af_flr" }, > { .reset_fn = &pci_pm_reset, .name = "pm" }, > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 4764e031a44b..d4becd6ffb52 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -2403,7 +2403,6 @@ static void pci_init_capabilities(struct pci_dev *dev) > pci_rcec_init(dev); /* Root Complex Event Collector */ > > pcie_report_downtraining(dev); > - pci_init_reset_methods(dev); > } > > /* > @@ -2494,6 +2493,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus) > dev->match_driver = false; > ret = device_add(&dev->dev); > WARN_ON(ret < 0); > + pci_init_reset_methods(dev); > } > > struct pci_dev *pci_scan_single_device(struct pci_bus *bus, int devfn) > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 9f8347799634..b4a5d2146542 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -49,7 +49,7 @@ > PCI_STATUS_SIG_TARGET_ABORT | \ > PCI_STATUS_PARITY) > > -#define PCI_RESET_FN_METHODS 5 > +#define PCI_RESET_FN_METHODS 6 > > /* > * The PCI interface treats multi-function devices as independent
