| Message ID | 1244879535-3351-3-git-send-email-yu.zhao@intel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Sat, 13 Jun 2009 15:52:15 +0800 Yu Zhao <yu.zhao@intel.com> wrote: > PCI-to-PCI Bridge 1.2 specifies that the Secondary Bus Reset bit can > force the assertion of RST# on the secondary interface, which can be > used to reset all devices including subordinates under this bus. This > can be used to reset a function if this function is the only device > under this bus. > > Reviewed-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com> > Signed-off-by: Yu Zhao <yu.zhao@intel.com> > --- Applied this series, thanks guys.
Hi, I'm sorry for the very delayed comment. May I ask you about timing parameters? Yu Zhao wrote: > PCI-to-PCI Bridge 1.2 specifies that the Secondary Bus Reset bit can > force the assertion of RST# on the secondary interface, which can be > used to reset all devices including subordinates under this bus. This > can be used to reset a function if this function is the only device > under this bus. > > Reviewed-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com> > Signed-off-by: Yu Zhao <yu.zhao@intel.com> > --- > drivers/pci/pci.c | 31 +++++++++++++++++++++++++++++++ > 1 files changed, 31 insertions(+), 0 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 2e58acc..c56a4a0 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -2162,6 +2162,33 @@ static int pci_pm_reset(struct pci_dev *dev, int probe) > return 0; > } > > +static int pci_parent_bus_reset(struct pci_dev *dev, int probe) > +{ > + u16 ctrl; > + struct pci_dev *pdev; > + > + if (dev->subordinate) > + return -ENOTTY; > + > + list_for_each_entry(pdev, &dev->bus->devices, bus_list) > + if (pdev != dev) > + return -ENOTTY; > + > + if (probe) > + return 0; > + > + pci_read_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, &ctrl); > + ctrl |= PCI_BRIDGE_CTL_BUS_RESET; > + pci_write_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, ctrl); > + msleep(100); Why 100 msec here? Is this based on T_rst (1 msec)? > + > + ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET; > + pci_write_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, ctrl); > + msleep(100); > + I think 100 msec here is not enough for PCI/PCI-X. I think T_rhfa is required here for PCI/PCI-X. Thanks, Kenji Kaneshige > + return 0; > +} > + > static int pci_dev_reset(struct pci_dev *dev, int probe) > { > int rc; > @@ -2183,6 +2210,10 @@ static int pci_dev_reset(struct pci_dev *dev, int probe) > goto done; > > rc = pci_pm_reset(dev, probe); > + if (rc != -ENOTTY) > + goto done; > + > + rc = pci_parent_bus_reset(dev, probe); > done: > if (!probe) { > up(&dev->dev.sem); -- 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
Kenji Kaneshige wrote: > Hi, > > I'm sorry for the very delayed comment. May I ask you about timing > parameters? >> + pci_read_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, &ctrl); >> + ctrl |= PCI_BRIDGE_CTL_BUS_RESET; >> + pci_write_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, ctrl); >> + msleep(100); > > Why 100 msec here? Is this based on T_rst (1 msec)? Yes, this is Trst. >> + >> + ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET; >> + pci_write_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, ctrl); >> + msleep(100); >> + > > I think 100 msec here is not enough for PCI/PCI-X. I think T_rhfa > is required here for PCI/PCI-X. Yes, should be Trhfa here. PCI spec says "Device operational parameters at frequencies under 16 MHz may be guaranteed by design rather than by testing", so the Trhfa should be 2s (i.e. 2^25 clock / 16 MHz). Will send a patch to update these values. Thanks, Yu -- 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 2e58acc..c56a4a0 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -2162,6 +2162,33 @@ static int pci_pm_reset(struct pci_dev *dev, int probe) return 0; } +static int pci_parent_bus_reset(struct pci_dev *dev, int probe) +{ + u16 ctrl; + struct pci_dev *pdev; + + if (dev->subordinate) + return -ENOTTY; + + list_for_each_entry(pdev, &dev->bus->devices, bus_list) + if (pdev != dev) + return -ENOTTY; + + if (probe) + return 0; + + pci_read_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, &ctrl); + ctrl |= PCI_BRIDGE_CTL_BUS_RESET; + pci_write_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, ctrl); + msleep(100); + + ctrl &= ~PCI_BRIDGE_CTL_BUS_RESET; + pci_write_config_word(dev->bus->self, PCI_BRIDGE_CONTROL, ctrl); + msleep(100); + + return 0; +} + static int pci_dev_reset(struct pci_dev *dev, int probe) { int rc; @@ -2183,6 +2210,10 @@ static int pci_dev_reset(struct pci_dev *dev, int probe) goto done; rc = pci_pm_reset(dev, probe); + if (rc != -ENOTTY) + goto done; + + rc = pci_parent_bus_reset(dev, probe); done: if (!probe) { up(&dev->dev.sem);