| Message ID | 1389081848-26506-1-git-send-email-dongsheng.wang@freescale.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Scott Wood |
| Headers | show |
On 2014/1/7 16:04, Dongsheng Wang wrote: > From: Wang Dongsheng <dongsheng.wang@freescale.com> > > the root port bus->self always NULL, so put root port pci device > into root port bus->self. > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com> > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 38e403d..7f2d1ab 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -1472,6 +1472,9 @@ int pci_scan_slot(struct pci_bus *bus, int devfn) > if (!dev->is_added) > nr++; > > + if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) > + bus->self = dev; In this case, bus is the pci root bus I think, so why set bus->self = root port ? "bus->self" should pointer to the pci device that bridge out this bus. > + > for (fn = next_fn(bus, dev, 0); fn > 0; fn = next_fn(bus, dev, fn)) { > dev = pci_scan_single_device(bus, devfn + fn); > if (dev) { >
On 2014/1/7 16:27, Yijing Wang wrote: > On 2014/1/7 16:04, Dongsheng Wang wrote: >> From: Wang Dongsheng <dongsheng.wang@freescale.com> >> >> the root port bus->self always NULL, so put root port pci device >> into root port bus->self. >> >> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com> >> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >> index 38e403d..7f2d1ab 100644 >> --- a/drivers/pci/probe.c >> +++ b/drivers/pci/probe.c >> @@ -1472,6 +1472,9 @@ int pci_scan_slot(struct pci_bus *bus, int devfn) >> if (!dev->is_added) >> nr++; >> >> + if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) >> + bus->self = dev; > > In this case, bus is the pci root bus I think, so why set bus->self = root port ? > "bus->self" should pointer to the pci device that bridge out this bus. Yes, this patch seems wrong. If dev is root port, bus should be root bus, so we shouldn't set root_bus->self = pci_dev_of_root_port. Actually PCI core has correctly setup pci_bus->self for secondary bus of PCIe root port. Thanks! Gerry > >> + >> for (fn = next_fn(bus, dev, 0); fn > 0; fn = next_fn(bus, dev, fn)) { >> dev = pci_scan_single_device(bus, devfn + fn); >> if (dev) { >> > >
> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index > >> 38e403d..7f2d1ab 100644 > >> --- a/drivers/pci/probe.c > >> +++ b/drivers/pci/probe.c > >> @@ -1472,6 +1472,9 @@ int pci_scan_slot(struct pci_bus *bus, int devfn) > >> if (!dev->is_added) > >> nr++; > >> > >> + if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) > >> + bus->self = dev; > > > > In this case, bus is the pci root bus I think, so why set bus->self = root > port ? > > "bus->self" should pointer to the pci device that bridge out this bus. > Yes, this patch seems wrong. If dev is root port, bus should be root bus, so we > shouldn't set root_bus->self = pci_dev_of_root_port. > Why the root bus->self pointer to the bridge? If child bus create from root bus, the child->self will get the bridge(root port) pci device. > Actually PCI core has correctly setup pci_bus->self for secondary bus of PCIe > root port. Yes, right. But the root-bus->self is NULL. I think the root bus should get root port pci device for itself. If there is no bridge at board how to get the root port device? -Dongsheng > > Thanks! > Gerry >
On 2014/1/7 17:51, Dongsheng.Wang@freescale.com wrote: >>>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index >>>> 38e403d..7f2d1ab 100644 >>>> --- a/drivers/pci/probe.c >>>> +++ b/drivers/pci/probe.c >>>> @@ -1472,6 +1472,9 @@ int pci_scan_slot(struct pci_bus *bus, int devfn) >>>> if (!dev->is_added) >>>> nr++; >>>> >>>> + if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) >>>> + bus->self = dev; >>> >>> In this case, bus is the pci root bus I think, so why set bus->self = root >> port ? >>> "bus->self" should pointer to the pci device that bridge out this bus. >> Yes, this patch seems wrong. If dev is root port, bus should be root bus, so we >> shouldn't set root_bus->self = pci_dev_of_root_port. >> > Why the root bus->self pointer to the bridge? > If child bus create from root bus, the child->self will get the bridge(root port) pci device. > >> Actually PCI core has correctly setup pci_bus->self for secondary bus of PCIe >> root port. > Yes, right. But the root-bus->self is NULL. I think the root bus should get root port > pci device for itself. If there is no bridge at board how to get the root port device? Hi Dongsheng, PCI root bus represents PCI host bridge, which has no corresponding PCI device. > > -Dongsheng > >> >> Thanks! >> Gerry >> >
> -----Original Message----- > From: Jiang Liu [mailto:jiang.liu@linux.intel.com] > Sent: Wednesday, January 08, 2014 10:33 AM > To: Wang Dongsheng-B40534; Yijing Wang; bhelgaas@google.com; rjw@rjwysocki.net > Cc: Wood Scott-B07421; galak@codeaurora.org; Zang Roy-R61911; linux- > pci@vger.kernel.org; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 1/2] pci: Fix root port bus->self is NULL > > > > On 2014/1/7 17:51, Dongsheng.Wang@freescale.com wrote: > >>>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index > >>>> 38e403d..7f2d1ab 100644 > >>>> --- a/drivers/pci/probe.c > >>>> +++ b/drivers/pci/probe.c > >>>> @@ -1472,6 +1472,9 @@ int pci_scan_slot(struct pci_bus *bus, int devfn) > >>>> if (!dev->is_added) > >>>> nr++; > >>>> > >>>> + if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) > >>>> + bus->self = dev; > >>> > >>> In this case, bus is the pci root bus I think, so why set bus->self > >>> = root > >> port ? > >>> "bus->self" should pointer to the pci device that bridge out this bus. > >> Yes, this patch seems wrong. If dev is root port, bus should be root > >> bus, so we shouldn't set root_bus->self = pci_dev_of_root_port. > >> > > Why the root bus->self pointer to the bridge? > > If child bus create from root bus, the child->self will get the bridge(root > port) pci device. > > > >> Actually PCI core has correctly setup pci_bus->self for secondary bus > >> of PCIe root port. > > Yes, right. But the root-bus->self is NULL. I think the root bus > > should get root port pci device for itself. If there is no bridge at board how > to get the root port device? > Hi Dongsheng, > PCI root bus represents PCI host bridge, which has no corresponding PCI > device. > Yes, agree. If more than one bridge on the root bus, this patch is wrong. Thanks for your review. :) -Dongsheng > > > > -Dongsheng > > > >> > >> Thanks! > >> Gerry > >> > > >
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 38e403d..7f2d1ab 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1472,6 +1472,9 @@ int pci_scan_slot(struct pci_bus *bus, int devfn) if (!dev->is_added) nr++; + if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) + bus->self = dev; + for (fn = next_fn(bus, dev, 0); fn > 0; fn = next_fn(bus, dev, fn)) { dev = pci_scan_single_device(bus, devfn + fn); if (dev) {