| Message ID | 20090805164639.76cbbb0f@dxy.sh.intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wed, 5 Aug 2009 16:46:39 +0800 Alek Du <alek.du@intel.com> wrote: > From f701d50e53d794309d92980ffa77d54606d44601 Mon Sep 17 00:00:00 2001 > From: Alek Du <alek.du@intel.com> > Date: Wed, 5 Aug 2009 15:25:26 +0800 > Subject: [PATCH] pci: check saved state before restore > > Without the check, the config space may be filled with zeros. Though > the driver should try to avoid call restoring before saving, but the > pci layer also should check this. > > Signed-off-by: Alek Du <alek.du@intel.com> > --- > drivers/pci/pci.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index dbd0f94..7b70312 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -846,6 +846,8 @@ 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); If we check here, we can probably remove some of the checks in the suspend/resume paths. Rafael?
On Friday 07 August 2009, Jesse Barnes wrote: > On Wed, 5 Aug 2009 16:46:39 +0800 > Alek Du <alek.du@intel.com> wrote: > > > From f701d50e53d794309d92980ffa77d54606d44601 Mon Sep 17 00:00:00 2001 > > From: Alek Du <alek.du@intel.com> > > Date: Wed, 5 Aug 2009 15:25:26 +0800 > > Subject: [PATCH] pci: check saved state before restore > > > > Without the check, the config space may be filled with zeros. Though > > the driver should try to avoid call restoring before saving, but the > > pci layer also should check this. > > > > Signed-off-by: Alek Du <alek.du@intel.com> > > --- > > drivers/pci/pci.c | 2 ++ > > 1 files changed, 2 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index dbd0f94..7b70312 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -846,6 +846,8 @@ 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); > > If we check here, we can probably remove some of the checks in the > suspend/resume paths. Rafael? Well, I don't really think the patch is correct, because pci_restore_state() is not only used for PM. Namely, pci_reset_function() uses it. Best, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday 07 August 2009, Rafael J. Wysocki wrote: > On Friday 07 August 2009, Jesse Barnes wrote: > > On Wed, 5 Aug 2009 16:46:39 +0800 > > Alek Du <alek.du@intel.com> wrote: > > > > > From f701d50e53d794309d92980ffa77d54606d44601 Mon Sep 17 00:00:00 2001 > > > From: Alek Du <alek.du@intel.com> > > > Date: Wed, 5 Aug 2009 15:25:26 +0800 > > > Subject: [PATCH] pci: check saved state before restore > > > > > > Without the check, the config space may be filled with zeros. Though > > > the driver should try to avoid call restoring before saving, but the > > > pci layer also should check this. > > > > > > Signed-off-by: Alek Du <alek.du@intel.com> > > > --- > > > drivers/pci/pci.c | 2 ++ > > > 1 files changed, 2 insertions(+), 0 deletions(-) > > > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > > index dbd0f94..7b70312 100644 > > > --- a/drivers/pci/pci.c > > > +++ b/drivers/pci/pci.c > > > @@ -846,6 +846,8 @@ 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); > > > > If we check here, we can probably remove some of the checks in the > > suspend/resume paths. Rafael? > > Well, I don't really think the patch is correct, because pci_restore_state() > is not only used for PM. Namely, pci_reset_function() uses it. Ah, ok, it also uses pci_save_state() before. But Alek, can you please modify the last line of pci_restore_standard_config() in your patch too? Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rafael, >Subject: Re: [PATCH] pci: check saved state before restore > >On Friday 07 August 2009, Rafael J. Wysocki wrote: >> On Friday 07 August 2009, Jesse Barnes wrote: >> > On Wed, 5 Aug 2009 16:46:39 +0800 >> > Alek Du <alek.du@intel.com> wrote: >> > >> > > From f701d50e53d794309d92980ffa77d54606d44601 Mon Sep 17 >00:00:00 2001 >> > > From: Alek Du <alek.du@intel.com> > >Ah, ok, it also uses pci_save_state() before. > >But Alek, can you please modify the last line of >pci_restore_standard_config() in your patch too? > Ok, I will submit a new patch soon. Thanks, Alek -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index dbd0f94..7b70312 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -846,6 +846,8 @@ 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); -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in