Re: [PATCH 2/4] PCI: Add new bitmap for keeping track of supported reset mechanisms
On Friday 12 March 2021 23:04:50 ameynarkhed...@gmail.com wrote:
> From: Amey Narkhede
>
> Introduce a new bitmap reset_methods in struct pci_dev
> to keep track of reset mechanisms supported by the
> device. Also refactor probing and reset functions
> to take advantage of calling convention of reset
> functions.
>
> Signed-off-by: Amey Narkhede
> ---
> Reviewed-by: Alex Williamson
> Reviewed-by: Raphael Norwitz
>
> drivers/pci/pci.c | 106
> drivers/pci/pci.h | 11 -
> drivers/pci/probe.c | 5 +--
> include/linux/pci.h | 10 +
> 4 files changed, 79 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 4a7c084a3..407b44e85 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -40,6 +40,26 @@ const char *pci_power_names[] = {
> };
> EXPORT_SYMBOL_GPL(pci_power_names);
>
> +static int pci_af_flr(struct pci_dev *dev, int probe);
> +static int pci_pm_reset(struct pci_dev *dev, int probe);
> +static int pci_dev_reset_slot_function(struct pci_dev *dev, int probe);
> +static int pci_parent_bus_reset(struct pci_dev *dev, int probe);
> +
> +/*
> + * The ordering for functions in pci_reset_fn_methods
> + * is required for bitmap positions defined
> + * in reset_methods in struct pci_dev
> + */
> +const struct pci_reset_fn_method pci_reset_fn_methods[] = {
> + { .reset_fn = _dev_specific_reset, .name = "device_specific" },
> + { .reset_fn = _flr, .name = "flr" },
> + { .reset_fn = _af_flr, .name = "af_flr" },
> + { .reset_fn = _pm_reset, .name = "pm" },
> + { .reset_fn = _dev_reset_slot_function, .name = "slot" },
> + { .reset_fn = _parent_bus_reset, .name = "bus" },
Hello Amey! In the list of reset methods is missing PCIe Warm Reset.
Could you extend and prepare API also for PCIe Warm Reset? According to
PCI Express mini card and m.2 electromechanical specifications, PCIe
Warm Reset can be triggered by PERST# signal and more kernel drivers can
internally control PERST#. Just there is no kernel API and therefore
PCIe Warm Reset nor PERST# signal is unified.
> + { 0 },
> +};
> +
> int isa_dma_bridge_buggy;
> EXPORT_SYMBOL(isa_dma_bridge_buggy);
>
> @@ -5080,71 +5100,59 @@ static void pci_dev_restore(struct pci_dev *dev)
> */
> int __pci_reset_function_locked(struct pci_dev *dev)
> {
> - int rc;
> + int i, rc = -ENOTTY;
> + const struct pci_reset_fn_method *reset;
>
> might_sleep();
>
> - /*
> - * A reset method returns -ENOTTY if it doesn't support this device
> - * and we should try the next method.
> - *
> - * If it returns 0 (success), we're finished. If it returns any
> - * other error, we're also finished: this indicates that further
> - * reset mechanisms might be broken on the device.
> - */
> - rc = pci_dev_specific_reset(dev, 0);
> - if (rc != -ENOTTY)
> - return rc;
> - rc = pcie_flr(dev, 0);
> - if (rc != -ENOTTY)
> - return rc;
> - rc = pci_af_flr(dev, 0);
> - if (rc != -ENOTTY)
> - return rc;
> - rc = pci_pm_reset(dev, 0);
> - if (rc != -ENOTTY)
> - return rc;
> - rc = pci_dev_reset_slot_function(dev, 0);
> - if (rc != -ENOTTY)
> - return rc;
> - return pci_parent_bus_reset(dev, 0);
> + for (i = 0, reset = pci_reset_fn_methods; reset->reset_fn; i++,
> reset++) {
> + if (!(dev->reset_methods & (1 << i)))
> + continue;
> +
> + /*
> + * A reset method returns -ENOTTY if it doesn't support this
> device
> + * and we should try the next method.
> + *
> + * If it returns 0 (success), we're finished. If it returns any
> + * other error, we're also finished: this indicates that further
> + * reset mechanisms might be broken on the device.
> + */
> + rc = reset->reset_fn(dev, 0);
> + if (rc != -ENOTTY)
> + return rc;
> + }
> + return rc;
> }
> EXPORT_SYMBOL_GPL(__pci_reset_function_locked);
>
> /**
> - * pci_probe_reset_function - check whether the device can be safely reset
> - * @dev: PCI device to reset
> + * pci_init_reset_methods - check whether device can be safely reset
> + * and store supported reset mechanisms.
> + * @dev: PCI device to check for reset mechanisms
> *
> * Some devices allow an individual function to be reset without affecting
> * other functions in the same device. The PCI device must be responsive
> - * to PCI config space in order to use this function.
> + * to reads and writes to its PCI config space in order to use this function.
> *
> - * Returns 0 if the device function can be reset or negative if the
> - * device doesn't support resetting a single function.
> + * Stores reset mechanisms supported by device in reset_methods bitmap
> + * field of struct pci_dev
[PATCH 2/4] PCI: Add new bitmap for keeping track of supported reset mechanisms
From: Amey Narkhede
Introduce a new bitmap reset_methods in struct pci_dev
to keep track of reset mechanisms supported by the
device. Also refactor probing and reset functions
to take advantage of calling convention of reset
functions.
Signed-off-by: Amey Narkhede
---
Reviewed-by: Alex Williamson
Reviewed-by: Raphael Norwitz
drivers/pci/pci.c | 106
drivers/pci/pci.h | 11 -
drivers/pci/probe.c | 5 +--
include/linux/pci.h | 10 +
4 files changed, 79 insertions(+), 53 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 4a7c084a3..407b44e85 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -40,6 +40,26 @@ const char *pci_power_names[] = {
};
EXPORT_SYMBOL_GPL(pci_power_names);
+static int pci_af_flr(struct pci_dev *dev, int probe);
+static int pci_pm_reset(struct pci_dev *dev, int probe);
+static int pci_dev_reset_slot_function(struct pci_dev *dev, int probe);
+static int pci_parent_bus_reset(struct pci_dev *dev, int probe);
+
+/*
+ * The ordering for functions in pci_reset_fn_methods
+ * is required for bitmap positions defined
+ * in reset_methods in struct pci_dev
+ */
+const struct pci_reset_fn_method pci_reset_fn_methods[] = {
+ { .reset_fn = _dev_specific_reset, .name = "device_specific" },
+ { .reset_fn = _flr, .name = "flr" },
+ { .reset_fn = _af_flr, .name = "af_flr" },
+ { .reset_fn = _pm_reset, .name = "pm" },
+ { .reset_fn = _dev_reset_slot_function, .name = "slot" },
+ { .reset_fn = _parent_bus_reset, .name = "bus" },
+ { 0 },
+};
+
int isa_dma_bridge_buggy;
EXPORT_SYMBOL(isa_dma_bridge_buggy);
@@ -5080,71 +5100,59 @@ static void pci_dev_restore(struct pci_dev *dev)
*/
int __pci_reset_function_locked(struct pci_dev *dev)
{
- int rc;
+ int i, rc = -ENOTTY;
+ const struct pci_reset_fn_method *reset;
might_sleep();
- /*
-* A reset method returns -ENOTTY if it doesn't support this device
-* and we should try the next method.
-*
-* If it returns 0 (success), we're finished. If it returns any
-* other error, we're also finished: this indicates that further
-* reset mechanisms might be broken on the device.
-*/
- rc = pci_dev_specific_reset(dev, 0);
- if (rc != -ENOTTY)
- return rc;
- rc = pcie_flr(dev, 0);
- if (rc != -ENOTTY)
- return rc;
- rc = pci_af_flr(dev, 0);
- if (rc != -ENOTTY)
- return rc;
- rc = pci_pm_reset(dev, 0);
- if (rc != -ENOTTY)
- return rc;
- rc = pci_dev_reset_slot_function(dev, 0);
- if (rc != -ENOTTY)
- return rc;
- return pci_parent_bus_reset(dev, 0);
+ for (i = 0, reset = pci_reset_fn_methods; reset->reset_fn; i++,
reset++) {
+ if (!(dev->reset_methods & (1 << i)))
+ continue;
+
+ /*
+* A reset method returns -ENOTTY if it doesn't support this
device
+* and we should try the next method.
+*
+* If it returns 0 (success), we're finished. If it returns any
+* other error, we're also finished: this indicates that further
+* reset mechanisms might be broken on the device.
+*/
+ rc = reset->reset_fn(dev, 0);
+ if (rc != -ENOTTY)
+ return rc;
+ }
+ return rc;
}
EXPORT_SYMBOL_GPL(__pci_reset_function_locked);
/**
- * pci_probe_reset_function - check whether the device can be safely reset
- * @dev: PCI device to reset
+ * pci_init_reset_methods - check whether device can be safely reset
+ * and store supported reset mechanisms.
+ * @dev: PCI device to check for reset mechanisms
*
* Some devices allow an individual function to be reset without affecting
* other functions in the same device. The PCI device must be responsive
- * to PCI config space in order to use this function.
+ * to reads and writes to its PCI config space in order to use this function.
*
- * Returns 0 if the device function can be reset or negative if the
- * device doesn't support resetting a single function.
+ * Stores reset mechanisms supported by device in reset_methods bitmap
+ * field of struct pci_dev
*/
-int pci_probe_reset_function(struct pci_dev *dev)
+void pci_init_reset_methods(struct pci_dev *dev)
{
- int rc;
+ int i, rc;
+ const struct pci_reset_fn_method *reset;
- might_sleep();
+ dev->reset_methods = 0;
- rc = pci_dev_specific_reset(dev, 1);
- if (rc != -ENOTTY)
- return rc;
- rc = pcie_flr(dev, 1);
- if (rc != -ENOTTY)
- return rc;
- rc = pci_af_flr(dev, 1);
- if (rc != -ENOTTY)
- return rc;
- rc = pci_pm_reset(dev, 1);
- if (rc != -ENOTTY)
- return
