| Message ID | 1364902014-943-1-git-send-email-Yuanquan.Chen@freescale.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | 37f02195bee9c25ce44e25204f40b7961a6d7c9d |
| Delegated to: | Michael Ellerman |
| Headers | show |
On Tue, 2013-04-02 at 19:26 +0800, Yuanquan Chen wrote: > So we move the DMA & IRQ initialization code from pcibios_setup_devices() and > construct a new function pcibios_enable_device. We call this function in > pcibios_enable_device, which will be called by PCI-e rescan code. At the > meanwhile, we avoid the the impact on cardbus. I also validate this patch with > silicon's PCIe-sata which encounters the IRQ issue. My worry is that this delays the setup of the IRQ and DMA to very late in the process, possibly after the quirks have been run, which can be problematic. We have platform hooks that might try to "fixup" specific IRQ issues on some platforms (especially macs) which I worry might fail if delayed that way (I may be wrong, I don't have a specific case in mind, but I would feel better if we kept setting up these things earlier). Cheers, Ben.
On 04/02/2013 11:10 PM, Benjamin Herrenschmidt wrote: > On Tue, 2013-04-02 at 19:26 +0800, Yuanquan Chen wrote: >> So we move the DMA & IRQ initialization code from pcibios_setup_devices() and >> construct a new function pcibios_enable_device. We call this function in >> pcibios_enable_device, which will be called by PCI-e rescan code. At the >> meanwhile, we avoid the the impact on cardbus. I also validate this patch with >> silicon's PCIe-sata which encounters the IRQ issue. > My worry is that this delays the setup of the IRQ and DMA to very late in > the process, possibly after the quirks have been run, which can be > problematic. We have platform hooks that might try to "fixup" specific > IRQ issues on some platforms (especially macs) which I worry might fail > if delayed that way (I may be wrong, I don't have a specific case in mind, > but I would feel better if we kept setting up these things earlier). > > Cheers, > Ben. > Hi Ben, I have checked all the quirk functions which are declared in kernel arch/powerpc with command : grep DECLARE_PCI_FIXUP_ `find arch/powerpc/ *.[hc]` All the quirk function are defined as DECLARE_PCI_FIXUP_EARLY , DECLARE_PCI_FIXUP_HEADER and DECLARE_PCI_FIXUP_FINAL, except quirk_uli5229() in arch/powerpc/platforms/fsl_uli1575.c, which is defined both as DECLARE_PCI_FIXUP_HEADER and DECLARE_PCI_FIXUP_RESUME. So the quirk_uli5229() will also be called with PCI pm module. The quirk functions defined as xxx_FINAL, HEADER and EARLY, will be called in the path: pci_scan_child_bus()->pci_scan_slot()->pci_scan_single_device()->pci_scan_device()->pci_setup_device() ->pci_device_add() the pci_scan_slot() is called earlier than pcibios_fixup_bus() even for the first scan of PCI-e bus, so the quirk functions on powerpc platform is called before the DMA & IRQ fixup. So in reality, the delay of DMA & IRQ fixup won't affect anything. Regards, Yuanquan > > >
On 04/03/2013 12:08 PM, Chen Yuanquan-B41889 wrote: > On 04/02/2013 11:10 PM, Benjamin Herrenschmidt wrote: >> On Tue, 2013-04-02 at 19:26 +0800, Yuanquan Chen wrote: >>> So we move the DMA & IRQ initialization code from >>> pcibios_setup_devices() and >>> construct a new function pcibios_enable_device. We call this >>> function in >>> pcibios_enable_device, which will be called by PCI-e rescan code. At >>> the >>> meanwhile, we avoid the the impact on cardbus. I also validate this >>> patch with >>> silicon's PCIe-sata which encounters the IRQ issue. >> My worry is that this delays the setup of the IRQ and DMA to very >> late in >> the process, possibly after the quirks have been run, which can be >> problematic. We have platform hooks that might try to "fixup" specific >> IRQ issues on some platforms (especially macs) which I worry might fail >> if delayed that way (I may be wrong, I don't have a specific case in >> mind, >> but I would feel better if we kept setting up these things earlier). >> >> Cheers, >> Ben. >> > > Hi Ben, > > I have checked all the quirk functions which are declared in kernel > arch/powerpc > with command : > grep DECLARE_PCI_FIXUP_ `find arch/powerpc/ *.[hc]` > > All the quirk function are defined as DECLARE_PCI_FIXUP_EARLY , > DECLARE_PCI_FIXUP_HEADER > and DECLARE_PCI_FIXUP_FINAL, except quirk_uli5229() in > arch/powerpc/platforms/fsl_uli1575.c, which is > defined both as DECLARE_PCI_FIXUP_HEADER and DECLARE_PCI_FIXUP_RESUME. > So the quirk_uli5229() > will also be called with PCI pm module. The quirk functions defined as > xxx_FINAL, HEADER and EARLY, > will be called in the path: > > pci_scan_child_bus()->pci_scan_slot()->pci_scan_single_device()->pci_scan_device()->pci_setup_device() > > ->pci_device_add() > > the pci_scan_slot() is called earlier than pcibios_fixup_bus() even > for the first scan of PCI-e bus, so the quirk > functions on powerpc platform is called before the DMA & IRQ fixup. So > in reality, the delay of DMA & IRQ fixup > won't affect anything. > > Regards, > Yuanquan > Hi Ben, How do you think about this? Do you have any comment? Thanks, Yuanquan >> >> >> >
On 04/10/2013 05:08 PM, Chen Yuanquan-B41889 wrote: > On 04/03/2013 12:08 PM, Chen Yuanquan-B41889 wrote: >> On 04/02/2013 11:10 PM, Benjamin Herrenschmidt wrote: >>> On Tue, 2013-04-02 at 19:26 +0800, Yuanquan Chen wrote: >>>> So we move the DMA & IRQ initialization code from >>>> pcibios_setup_devices() and >>>> construct a new function pcibios_enable_device. We call this >>>> function in >>>> pcibios_enable_device, which will be called by PCI-e rescan code. >>>> At the >>>> meanwhile, we avoid the the impact on cardbus. I also validate this >>>> patch with >>>> silicon's PCIe-sata which encounters the IRQ issue. >>> My worry is that this delays the setup of the IRQ and DMA to very >>> late in >>> the process, possibly after the quirks have been run, which can be >>> problematic. We have platform hooks that might try to "fixup" specific >>> IRQ issues on some platforms (especially macs) which I worry might fail >>> if delayed that way (I may be wrong, I don't have a specific case in >>> mind, >>> but I would feel better if we kept setting up these things earlier). >>> >>> Cheers, >>> Ben. >>> >> >> Hi Ben, >> >> I have checked all the quirk functions which are declared in kernel >> arch/powerpc >> with command : >> grep DECLARE_PCI_FIXUP_ `find arch/powerpc/ *.[hc]` >> >> All the quirk function are defined as DECLARE_PCI_FIXUP_EARLY , >> DECLARE_PCI_FIXUP_HEADER >> and DECLARE_PCI_FIXUP_FINAL, except quirk_uli5229() in >> arch/powerpc/platforms/fsl_uli1575.c, which is >> defined both as DECLARE_PCI_FIXUP_HEADER and >> DECLARE_PCI_FIXUP_RESUME. So the quirk_uli5229() >> will also be called with PCI pm module. The quirk functions defined >> as xxx_FINAL, HEADER and EARLY, >> will be called in the path: >> >> pci_scan_child_bus()->pci_scan_slot()->pci_scan_single_device()->pci_scan_device()->pci_setup_device() >> >> ->pci_device_add() >> >> the pci_scan_slot() is called earlier than pcibios_fixup_bus() even >> for the first scan of PCI-e bus, so the quirk >> functions on powerpc platform is called before the DMA & IRQ fixup. >> So in reality, the delay of DMA & IRQ fixup >> won't affect anything. >> >> Regards, >> Yuanquan >> > > Hi Ben, > > How do you think about this? Do you have any comment? > > Thanks, > Yuanquan > Hi Bjorn, There's no response from Ben. How do you think about this patch? What's your advice? Thanks, Yuanquan >>> >>> >>> >> > > >
On Tue, 2013-04-23 at 17:26 +0800, Chen Yuanquan-B41889 wrote: > There's no response from Ben. How do you think about this patch? > What's > your advice? Didn't Michael put your patch in -next while I was on vacation ? I'll check tomorrow. Cheers, Ben.
On 04/23/2013 06:05 PM, Benjamin Herrenschmidt wrote: > On Tue, 2013-04-23 at 17:26 +0800, Chen Yuanquan-B41889 wrote: >> There's no response from Ben. How do you think about this patch? >> What's >> your advice? > Didn't Michael put your patch in -next while I was on vacation ? I'll > check tomorrow. > > Cheers, > Ben. > Hi Ben, I have checked the latest kernel(v3.9-rc7). This patch has been applied. Thanks, Yuanquan > > >
On Tue, Apr 23, 2013 at 06:51:09PM +0800, Chen Yuanquan-B41889 wrote: > On 04/23/2013 06:05 PM, Benjamin Herrenschmidt wrote: > >On Tue, 2013-04-23 at 17:26 +0800, Chen Yuanquan-B41889 wrote: > >>There's no response from Ben. How do you think about this patch? > >>What's > >>your advice? > >Didn't Michael put your patch in -next while I was on vacation ? I'll > >check tomorrow. > > > >Cheers, > >Ben. > > > > Hi Ben, > > I have checked the latest kernel(v3.9-rc7). This patch has been applied. No it's not in v3.9-rc7. It is in my test branch, which is in linux-next. http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit?id=37f02195bee9c25ce44e25204f40b7961a6d7c9d cheers
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index fa12ae4..0324758 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -1023,6 +1023,27 @@ void pcibios_setup_bus_self(struct pci_bus *bus) ppc_md.pci_dma_bus_setup(bus); } +void pcibios_setup_device(struct pci_dev *dev) +{ + /* Fixup NUMA node as it may not be setup yet by the generic + * code and is needed by the DMA init + */ + set_dev_node(&dev->dev, pcibus_to_node(dev->bus)); + + /* Hook up default DMA ops */ + set_dma_ops(&dev->dev, pci_dma_ops); + set_dma_offset(&dev->dev, PCI_DRAM_OFFSET); + + /* Additional platform DMA/iommu setup */ + if (ppc_md.pci_dma_dev_setup) + ppc_md.pci_dma_dev_setup(dev); + + /* Read default IRQs and fixup if necessary */ + pci_read_irq_line(dev); + if (ppc_md.pci_irq_fixup) + ppc_md.pci_irq_fixup(dev); +} + void pcibios_setup_bus_devices(struct pci_bus *bus) { struct pci_dev *dev; @@ -1037,23 +1058,7 @@ void pcibios_setup_bus_devices(struct pci_bus *bus) if (dev->is_added) continue; - /* Fixup NUMA node as it may not be setup yet by the generic - * code and is needed by the DMA init - */ - set_dev_node(&dev->dev, pcibus_to_node(dev->bus)); - - /* Hook up default DMA ops */ - set_dma_ops(&dev->dev, pci_dma_ops); - set_dma_offset(&dev->dev, PCI_DRAM_OFFSET); - - /* Additional platform DMA/iommu setup */ - if (ppc_md.pci_dma_dev_setup) - ppc_md.pci_dma_dev_setup(dev); - - /* Read default IRQs and fixup if necessary */ - pci_read_irq_line(dev); - if (ppc_md.pci_irq_fixup) - ppc_md.pci_irq_fixup(dev); + pcibios_setup_device(dev); } } @@ -1494,6 +1499,10 @@ int pcibios_enable_device(struct pci_dev *dev, int mask) if (ppc_md.pcibios_enable_device_hook(dev)) return -EINVAL; + /* avoid pcie irq fix up impact on cardbus */ + if (dev->hdr_type != PCI_HEADER_TYPE_CARDBUS) + pcibios_setup_device(dev); + return pci_enable_resources(dev, mask); }
Powerpc initializes the DMA and IRQ information in pci_scan_child_bus()-> pcibios_fixup_bus()->pcibios_setup_bus_devices(). But for the devices which are hotpluged, bus->is added has been set for the first scan of the PCI-e bus, so the initialization code won't be called. Then the hotpluged devices' driver will fail to load. For example : The PCI-e device 0001:03:00.0 is the Intel PCI-e e1000e network card, remove it from the system: # echo 1 > /sys/bus/pci/devices/0001\:03\:00.0/remove # e1000e 0001:03:00.0 eth0: removed PHC Rescan it from it's bus: # echo 1 > /sys/bus/pci/devices/0001\:02\:00.0/rescan ... e1000e 0001:03:00.0: Disabling ASPM L0s L1 e1000e 0001:03:00.0: No usable DMA configuration, aborting e1000e: probe of 0001:03:00.0 failed with error -5 So we move the DMA & IRQ initialization code from pcibios_setup_devices() and construct a new function pcibios_enable_device. We call this function in pcibios_enable_device, which will be called by PCI-e rescan code. At the meanwhile, we avoid the the impact on cardbus. I also validate this patch with silicon's PCIe-sata which encounters the IRQ issue. Signed-off-by: Yuanquan Chen <Yuanquan.Chen@freescale.com> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Hiroo Matsumoto <matsumoto.hiroo@jp.fujitsu.com> --- arch/powerpc/kernel/pci-common.c | 43 +++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 17 deletions(-)