| Message ID | 20210429004907.29044-1-sdonthineni@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [v4,1/2] PCI: Add support for a function level reset based on _RST method | expand |
On Wed, 28 Apr 2021 19:49:06 -0500 Shanker Donthineni <sdonthineni@nvidia.com> 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@nvidia.com> > --- > 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@gmail.com/ > > 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
Hi Alex, On 4/30/21 1:39 PM, Alex Williamson wrote: > External email: Use caution opening links or attachments > > > On Wed, 28 Apr 2021 19:49:06 -0500 > Shanker Donthineni <sdonthineni@nvidia.com> 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@nvidia.com> >> --- >> 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@gmail.com/ >> >> 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, > Thanks, I will change to "{ .reset_fn = &pci_dev_acpi_reset, .name = "acpi" }"
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" }, { .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
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@nvidia.com> --- 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@gmail.com/ drivers/pci/pci.c | 30 ++++++++++++++++++++++++++++++ drivers/pci/probe.c | 2 +- include/linux/pci.h | 2 +- 3 files changed, 32 insertions(+), 2 deletions(-)