On Monday, July 19, 2010, Brad Peters wrote:
> Patch Overview:
> The pci_restore_state API is shared by both power management code and Extended
> Error Handling (EEH) code on Power. This patch adds an additional recovery
> function to pci_restore_state API. The problem being addressed is that Power
> Management semantics only allow the saved state of PCI device to be restored
> once per save. With this patch, EEH is able to restore the saved state
> each time a PCI error is detected, enabling recovery in the face of repeated errors.
>
> There was some discussion of renaming the existing and new functions to more
> clearly break out unconditional restore from the default conditional one, but a
> name change seemed a heavy-handed change to force on the 200+ current users.
>
> Bit more detail:
> PCI device drivers which support EEH/AER save their pci state once during
> driver initialization and during EEH/AER error recovery, restore the
> original saved state. What we found was that our pci driver code would
> recover from the first EEH error and fail to recover on subsequent
> EEH errors. This issue results from pci_restore_state() function
> restoring the state during initialization on the first EEH error.
>
> What this patch does is to provide the pci_force_restore_state() for use
> by PCI drivers which support EEH/AER that require the original saved
> state be restored each time an EEH/AER error is detected.
>
>
> Signed-off by: Brad Peters <bpeters@xxxxxxxxxx>
> Signed-off by: Richard A Lary <rlary@xxxxxxxxxxxxxxxxxx>
>
> diff -uNrp -X linux-2.6.34/Documentation/dontdiff
> linux-2.6.34.orig/drivers/pci/pci.c linux-2.6.34/drivers/pci/pci.c
> --- linux-2.6.34.orig/drivers/pci/pci.c 2010-05-16 14:17:36.000000000 -0700
> +++ linux-2.6.34/drivers/pci/pci.c 2010-05-26 17:16:20.000000000 -0700
> @@ -920,19 +920,11 @@ pci_save_state(struct pci_dev *dev)
> return 0;
> }
>
> -/**
> - * pci_restore_state - Restore the saved state of a PCI device
> - * @dev: - PCI device that we're dealing with
> - */
> -int
> -pci_restore_state(struct pci_dev *dev)
> +static void __pci_restore_state(struct pci_dev *dev)
> {
> int i;
> u32 val;
>
> - if (!dev->state_saved)
> - return 0;
> -
> /* PCI Express register must be restored first */
> pci_restore_pcie_state(dev);
>
> @@ -953,12 +945,44 @@ pci_restore_state(struct pci_dev *dev)
> pci_restore_pcix_state(dev);
> pci_restore_msi_state(dev);
> pci_restore_iov_state(dev);
> +}
> +
> +
> +/**
> + * pci_restore_state - Restore the saved state of a PCI device
> + * only if dev->state_saved is not 0. Used by
> + * power management suspend/restore routines.
> + * @dev: - PCI device that we're dealing with
> + */
> +int
> +pci_restore_state(struct pci_dev *dev)
> +{
> +
> + if (!dev->state_saved)
> + return 0;
> +
> + __pci_restore_state(dev);
>
> dev->state_saved = false;
>
> return 0;
> }
>
> +/**
> + * pci_force_restore_state - Restore the saved state of a PCI device
> + * even if dev->state_saved is 0. Used by
> + * EEH and AER PCI error recovery.
> + * @dev: - PCI device that we're dealing with
> + */
> +int
> +pci_force_restore_state(struct pci_dev *dev)
> +{
> + __pci_restore_state(dev);
> +
> + return 0;
> +}
I'm not sure if that wrapper is necessary. In my opinion you can just use
__pci_restore_state() driectly wherever you want to disregard state_saved.
That would make life somewhat easier to people trying to understand the code
using it.
Still, if you really really want a wrapper, why don't you put it into
include/linux/pci.h as a static inline instead?
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
