lkml.org 
[lkml]   [2012]   [Aug]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [BUGFIX -v3 3/4] PCI/PM: Fix config reg access for D3cold and bridge suspending
    From
    Date
    On Wed, 2012-08-08 at 09:07 +0800, Huang Ying wrote:
    > This patch fixes the following bug:
    >
    > http://marc.info/?l=linux-pci&m=134338059022620&w=2
    >
    > Where lspci does not work properly if a device and the corresponding
    > parent bridge (such as PCIe port) is suspended. This is because the
    > device configuration space registers will be not accessible if the
    > corresponding parent bridge is suspended or the device is put into
    > D3cold state.
    >
    > To solve the issue, the bridge/PCIe port connected to the device is
    > put into active state before read/write configuration space registers.
    > If the device is in D3cold state, it will be put into active state
    > too.
    >
    > To avoid resume/suspend PCIe port for each configuration register
    > read/write, a small delay is added before the PCIe port to go
    > suspended.
    >
    > Reported-by: Bjorn Mork <bjorn@mork.no>
    > Signed-off-by: Huang Ying <ying.huang@intel.com>
    > ---
    > drivers/pci/pci-sysfs.c | 43 +++++++++++++++++++++++++++++++++++++++++
    > drivers/pci/pcie/portdrv_pci.c | 9 ++++++++
    > 2 files changed, 52 insertions(+)
    >
    > --- a/drivers/pci/pci-sysfs.c
    > +++ b/drivers/pci/pci-sysfs.c
    > @@ -458,6 +458,41 @@ boot_vga_show(struct device *dev, struct
    > }
    > struct device_attribute vga_attr = __ATTR_RO(boot_vga);
    >
    > +static void
    > +pci_config_pm_runtime_get(struct pci_dev *pdev)
    > +{
    > + struct device *dev = &pdev->dev;
    > + struct device *parent = dev->parent;
    > +
    > + if (parent)
    > + pm_runtime_get_sync(parent);
    > + pm_runtime_get_noresume(dev);
    > + /*
    > + * pdev->current_state is set to PCI_D3cold during suspending,
    > + * so wait until suspending completes
    > + */
    > + pm_runtime_barrier(dev);
    > + if (pdev->current_state == PCI_D3cold) {
    > + /*
    > + * Already called pm_runtime_get_noresume above, so
    > + * just calling pm_runtime_resume is sufficient, need
    > + * not to call pm_runtime_get_sync.
    > + */
    > + pm_runtime_resume(dev);

    Hi, Rafael,

    Do you OK with this new version and the comments added above?

    Best Regards,
    Huang Ying

    > + }
    > +}
    > +
    > +static void
    > +pci_config_pm_runtime_put(struct pci_dev *pdev)
    > +{
    > + struct device *dev = &pdev->dev;
    > + struct device *parent = dev->parent;
    > +
    > + pm_runtime_put(dev);
    > + if (parent)
    > + pm_runtime_put_sync(parent);
    > +}
    > +
    > static ssize_t
    > pci_read_config(struct file *filp, struct kobject *kobj,
    > struct bin_attribute *bin_attr,
    > @@ -484,6 +519,8 @@ pci_read_config(struct file *filp, struc
    > size = count;
    > }
    >
    > + pci_config_pm_runtime_get(dev);
    > +
    > if ((off & 1) && size) {
    > u8 val;
    > pci_user_read_config_byte(dev, off, &val);
    > @@ -529,6 +566,8 @@ pci_read_config(struct file *filp, struc
    > --size;
    > }
    >
    > + pci_config_pm_runtime_put(dev);
    > +
    > return count;
    > }
    >
    > @@ -549,6 +588,8 @@ pci_write_config(struct file* filp, stru
    > count = size;
    > }
    >
    > + pci_config_pm_runtime_get(dev);
    > +
    > if ((off & 1) && size) {
    > pci_user_write_config_byte(dev, off, data[off - init_off]);
    > off++;
    > @@ -587,6 +628,8 @@ pci_write_config(struct file* filp, stru
    > --size;
    > }
    >
    > + pci_config_pm_runtime_put(dev);
    > +
    > return count;
    > }
    >
    > --- a/drivers/pci/pcie/portdrv_pci.c
    > +++ b/drivers/pci/pcie/portdrv_pci.c
    > @@ -140,9 +140,17 @@ static int pcie_port_runtime_resume(stru
    > {
    > return 0;
    > }
    > +
    > +static int pcie_port_runtime_idle(struct device *dev)
    > +{
    > + /* Delay for a short while to prevent too frequent suspend/resume */
    > + pm_schedule_suspend(dev, 10);
    > + return -EBUSY;
    > +}
    > #else
    > #define pcie_port_runtime_suspend NULL
    > #define pcie_port_runtime_resume NULL
    > +#define pcie_port_runtime_idle NULL
    > #endif
    >
    > static const struct dev_pm_ops pcie_portdrv_pm_ops = {
    > @@ -155,6 +163,7 @@ static const struct dev_pm_ops pcie_port
    > .resume_noirq = pcie_port_resume_noirq,
    > .runtime_suspend = pcie_port_runtime_suspend,
    > .runtime_resume = pcie_port_runtime_resume,
    > + .runtime_idle = pcie_port_runtime_idle,
    > };
    >
    > #define PCIE_PORTDRV_PM_OPS (&pcie_portdrv_pm_ops)




    \
     
     \ /
      Last update: 2012-08-10 05:21    [W:0.032 / U:0.620 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site