Re: [PATCH] powerpc/pci: Improve device hotplug initialization
On 06/01/2013 09:58 PM, Guenter Roeck wrote: On Fri, May 31, 2013 at 03:44:07PM +1000, Benjamin Herrenschmidt wrote: On Thu, 2013-05-30 at 22:14 -0700, Guenter Roeck wrote: On Wed, May 29, 2013 at 05:30:41PM +0800, Chen Yuanquan-B41889 wrote: On 05/29/2013 01:35 AM, Guenter Roeck wrote: bios_add_device(). Drop explicit calls to pcibios_setup_device(); this makes pcibios_setup_bus_devices() a noop function which could eve Yeah, it's more reasonable to do the irq and DMA related initialization in one code path for all devices. Any comments / feedback on the code itself ? Sorry, I haven't had a chance to review it yet, I'm fairly bogged down at the moment. I want to tread carefully because the previous iteration of changing that stuff did break a few platforms in the end. Hi Ben, the comment was actuially directed towards Yuanquan. No problem, take your time. I did my best to test it, but I agree that this is a critical area of the code, and it would be desirable to get additional scrutiny and test feedback. The code has been running in our system (P2020 and P5040) for several months. I was preparing a patch for upstream submission when I noticed commit 37f02195b. After testing ithis commit, I noticed the problems with it and wrote this patch, which aligns the code with our initial patch. I tested it as good as I could on our systems as well as with a P5040 evaluation board and an Intel GE PCIe card. Thanks, Guenter Hi Guenter, Your patch makes sure the initialization of DMA and irq in one code path and also avoids the initialization called two times(in pci bus scanning and pci_enable_device() in device driver) for no-hot-plugged pci device. It's much more reasonable! I don't know why you also remove the function set_dev_node() ? As I know, the function just be called here. I think it should remain in your new function pcibios_add_device(). Regards, Yuanquan ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/pci: Improve device hotplug initialization
On 05/29/2013 01:35 AM, Guenter Roeck wrote: bios_add_device(). Drop explicit calls to pcibios_setup_device(); this makes pcibios_setup_bus_devices() a noop function which could eve Yeah, it's more reasonable to do the irq and DMA related initialization in one code path for all devices. Regards, Yuanquan ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/pci: fix PCI-e devices rescan issue on powerpc platform
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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/pci: fix PCI-e devices rescan issue on powerpc platform
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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/pci: fix PCI-e devices rescan issue on powerpc platform
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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/pci: fix PCI-e devices rescan issue on powerpc platform
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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [linuxppc-release][PATCH] powerpc/pci-hotplug: fix init issue of rescanned pci device
On 12/08/2012 05:15 AM, Bjorn Helgaas wrote:
On Thu, Dec 6, 2012 at 4:23 AM, Chen Yuanquan-B41889
b41...@freescale.com wrote:
On 12/06/2012 05:30 AM, Bjorn Helgaas wrote:
On Wed, Dec 5, 2012 at 2:29 AM, Chen Yuanquan-B41889
b41...@freescale.com wrote:
On 12/05/2012 04:26 PM, Benjamin Herrenschmidt wrote:
On Wed, 2012-12-05 at 16:20 +0800, Chen Yuanquan-B41889 wrote:
On 12/05/2012 03:17 PM, Benjamin Herrenschmidt wrote:
On Wed, 2012-12-05 at 10:31 +0800, Yuanquan Chen wrote:
On powerpc arch, some fixup work of PCI/PCI-e device is just done
during the
first scan at booting time. For the PCI/PCI-e device rescanned after
linux OS
booting up, the fixup work won't be done, which leads to dma_set_mask
error or
irq related issue in rescanned PCI/PCI-e device's driver. So, it does
the same
fixup work for the rescanned device to avoid this issue.
Hrm, the patch is a bit gross. First the code shouldn't be copy/pasted
that way but factored out.
Please, at least format your email properly so I can try to undertand
without needing aspirin.
There's a judgement if (!bus-is_added) before calling of
pcibios_fixup_bus in pci_scan_child_bus, so for the rescanned device,
the fixup won't execute, which leads to fatal error in driver of
rescanned
device on freescale powerpc, no this issues on x86 arch.
First, none of that invalidates my statement that you shouldn't
duplicate a whole block of code like this. Even if your approach is
correct (which is debated separately), at the very least you should
factor the code out into a common function between the two copies.
Remove the judgement, let it to do the pcibios_fixup_bus
directly, the error won't occur for the rescanned device. But it's
general code, not proper to change here, so copy the pcibios_fixup_bus
work to pcibios_enable_device.
I'm surprised also that is_added is false when pcibios_enable_device()
gets called ... that looks strange to me. At what point is that enable
happening in the hotplug sequence ?
All devices are rescanned and then call the pci_enable_devices and
pci_bus_add_devices.
Where ? How ? What is the sequence happening ? In any case, I think if
we need a proper fixup done per-device like that after scan we ought to
create a new hook at the generic level rather than that sort of hack.
echo 1 rescan to trigger dev_rescan_store:
dev_rescan_store-pci_rescan_bus-pci_scan_child_bus,
pci_assign_unassigned_bus_resources,
pci_enable_bridges, pci_bus_add_devices
pci_enable_bridges-pci_enable_device-__pci_enable_device_flags-do_pci_enable_device-
pcibios_enable_device
pci_bus_add_devices-pci_bus_add_device-dev-is_added = 1
Yeah, it's general fixup code for every rescanned PCI/PCI-e device on
powerpc at runtime. So if
we want to call it in a ppc_md member, we need to wrap it as a function
and
assign it in every ppc_md,
it isn't proper for the general code.
Regards,
yuanquan
The patch code will be called by pci_enable_devices. The
dev-is_added
is set in pci_bus_add_device
which is called by pci_bus_add_devices. So dev-is_added is false
when
checking it in pcibios_enable_device
for the rescanned device.
Who calls pci_enable_device() in the rescan case ? Why isn't it left to
the driver ? I don't think we can rely on that behaviour not to change.
How do you trigger the rescan anyway ?
Use the interface under /sys :
echo 1 /sys/bus/pci/devices/xxx/remove
then echo 1 to the pci device which is the bus of the removed device
echo 1 /sys/bus/pci/devices//rescan
the removed device will be scanned and it's driver module will be
loaded
automatically.
Yeah this code path are known to be fishy. I think the problem is at the
generic abstraction level and that's where it needs to be fixed.
Cheers,
Ben.
Regards,
yuanquan
I think the problem needs to be solve at a higher level, I'm adding
linux-pci Bjorn to the CC list.
Cheers,
Ben.
Signed-off-by: Yuanquan Chen b41...@freescale.com
---
arch/powerpc/kernel/pci-common.c | 20
1 file changed, 20 insertions(+)
diff --git a/arch/powerpc/kernel/pci-common.c
b/arch/powerpc/kernel/pci-common.c
index 7f94f76..f0fb070 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1496,6 +1496,26 @@ int pcibios_enable_device(struct pci_dev *dev,
int mask)
if (ppc_md.pcibios_enable_device_hook(dev))
return -EINVAL;
+if (!dev-is_added) {
+ /*
+* 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
[PATCH] Make PCIe hotplug work with Freescale PCIe controllers
-Original Message-
From: Linuxppc-dev [mailto:linuxppc-dev-bounces+tie-
fei.zang=freescale@lists.ozlabs.org] On Behalf Of Rojhalat Ibrahim
Sent: Tuesday, March 12, 2013 5:23 PM
To: Kumar Gala
Cc: linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] Make PCIe hotplug work with Freescale PCIe
controllers
On Monday 11 March 2013 12:17:42 Kumar Gala wrote:
Rather than do it this way, we should do something like:
fsl_indirect_read_config() {
link check
if (link)
indirect_read_config()
}
and just add fsl_indirect_{r,w}_config into fsl_pci.c
- k
Ok, how about this:
Yeah, this patch can solve the problem of PCI-e bus rescan which a PCI-e
EP is added to RC
after RC booting up. If RC boots up without EP added, the original code
will set the PCI-e
bus as no link even if you add a EP to RC during RC's runtime.
Regards,
Yuanquan
Signed-off-by: Rojhalat Ibrahim i...@rtschenk.de
---
arch/powerpc/sysdev/fsl_pci.c | 49
++
1 file changed, 45 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_pci.c
b/arch/powerpc/sysdev/fsl_pci.c index 682084d..693db9f 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -36,6 +36,8 @@
static int fsl_pcie_bus_fixup, is_mpc83xx_pci;
+static struct pci_ops *indirect_pci_ops;
+
static void quirk_fsl_pcie_header(struct pci_dev *dev) {
u8 hdr_type;
@@ -64,6 +66,45 @@ static int __init fsl_pcie_check_link(struct
pci_controller
*hose)
return 0;
}
+static int fsl_indirect_read_config(struct pci_bus *bus, unsigned int
devfn,
+ int offset, int len, u32 *val)
+{
+ struct pci_controller *hose = pci_bus_to_host(bus);
+
+ // check the link status
+ if ((bus-number == hose-first_busno) (devfn == 0)) {
+ u32 ltssm = 0;
+ indirect_pci_ops-read(bus, 0, PCIE_LTSSM, 4, ltssm);
+ if (ltssm PCIE_LTSSM_L0) {
+ hose-indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+ } else {
+ hose-indirect_type = ~PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+ }
+ }
+ return indirect_pci_ops-read(bus, devfn, offset, len, val); }
+
+static int fsl_indirect_write_config(struct pci_bus *bus, unsigned int
devfn,
+int offset, int len, u32 val)
+{
+ return indirect_pci_ops-write(bus, devfn, offset, len, val); }
+
+static struct pci_ops fsl_indirect_pci_ops = {
+ .read = fsl_indirect_read_config,
+ .write = fsl_indirect_write_config,
+};
+
+static void __init fsl_setup_indirect_pci(struct pci_controller* hose,
+ resource_size_t cfg_addr,
+ resource_size_t cfg_data, u32 flags) {
+ setup_indirect_pci(hose, cfg_addr, cfg_data, flags);
+ indirect_pci_ops = hose-ops;
+ hose-ops = fsl_indirect_pci_ops;
+}
+
#if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
#define MAX_PHYS_ADDR_BITS40
@@ -461,8 +502,8 @@ int __init fsl_add_bridge(struct platform_device
*pdev, int is_primary)
hose-first_busno = bus_range ? bus_range[0] : 0x0;
hose-last_busno = bus_range ? bus_range[1] : 0xff;
- setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
- PPC_INDIRECT_TYPE_BIG_ENDIAN);
+ fsl_setup_indirect_pci(hose, rsrc.start, rsrc.start + 0x4,
+ PPC_INDIRECT_TYPE_BIG_ENDIAN);
if (early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP)) {
/* For PCIE read HEADER_TYPE to identify controler mode */ @@
-766,8 +807,8 @@ int __init mpc83xx_add_bridge(struct device_node *dev)
if (ret)
goto err0;
} else {
- setup_indirect_pci(hose, rsrc_cfg.start,
- rsrc_cfg.start + 4, 0);
+ fsl_setup_indirect_pci(hose, rsrc_cfg.start,
+ rsrc_cfg.start + 4, 0);
}
printk(KERN_INFO Found FSL PCI host bridge at 0x%016llx.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Make PCIe hotplug work with Freescale PCIe controllers
On 03/12/2013 06:30 PM, Rojhalat Ibrahim wrote:
On Tuesday 12 March 2013 18:12:20 Chen Yuanquan-B41889 wrote:
-Original Message-
From: Linuxppc-dev [mailto:linuxppc-dev-bounces+tie-
fei.zang=freescale@lists.ozlabs.org] On Behalf Of Rojhalat Ibrahim
Sent: Tuesday, March 12, 2013 5:23 PM
To: Kumar Gala
Cc: linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] Make PCIe hotplug work with Freescale PCIe
controllers
On Monday 11 March 2013 12:17:42 Kumar Gala wrote:
Rather than do it this way, we should do something like:
fsl_indirect_read_config() {
link check
if (link)
indirect_read_config()
}
and just add fsl_indirect_{r,w}_config into fsl_pci.c
- k
Ok, how about this:
Yeah, this patch can solve the problem of PCI-e bus rescan which a PCI-e
EP is added to RC
after RC booting up. If RC boots up without EP added, the original code
will set the PCI-e
bus as no link even if you add a EP to RC during RC's runtime.
Regards,
Yuanquan
Right. The EP is only added if you first do echo 1 /sys/bus/pci/rescan.
Rojhalat
The following patch can solve your issue of only added if you first ...:
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 5b3771a..c1298d0 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -730,11 +730,12 @@ int __devinit pci_scan_bridge(struct pci_bus *bus,
struct
/* Prevent assigning a bus number that already exists.
* This can happen when a bridge is hot-plugged */
- if (pci_find_bus(pci_domain_nr(bus), max+1))
- goto out;
- child = pci_add_new_bus(bus, dev, ++max);
- if (!child)
- goto out;
+ child = pci_find_bus(pci_domain_nr(bus), max+1);
+ if (!child) {
+ child = pci_add_new_bus(bus, dev, ++max);
+ if (!child)
+ goto out;
+ }
buses = (buses 0xff00)
| ((unsigned int)(child-primary) 0)
| ((unsigned int)(child-secondary) 8)
There are still some issues about powerpc PCI-e rescan. For example, add
a Intel e1000e
ethernet card or silicon PCI-e_sata to powerpc PCI-e slot and boot the
board. The EP can
work well with their driver. But if you echo 1
/sys/bus/pci/device/xxx/remove which
corresponds to Intel e1000e ethernet card or silicon PCI-e_sata, then
echo 1 to rescan,
the device can be rescanned, but it will fail to load the corresponded
driver due to hw_irq
and dma_set_mask error. The following patch can solve the problem, but
not a good method
to solve it.
diff --git a/arch/powerpc/kernel/pci-common.c
b/arch/powerpc/kernel/pci-common.c
index 2476a32..f9b7f0f 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1557,6 +1557,19 @@ int pcibios_enable_device(struct pci_dev *dev,
int mask)
if (ppc_md.pcibios_enable_device_hook(dev))
return -EINVAL;
+ if (!dev-is_added) {
+ set_dev_node(dev-dev, pcibus_to_node(dev-bus));
+
+ set_dma_ops(dev-dev, pci_dma_ops);
+ set_dma_offset(dev-dev, PCI_DRAM_OFFSET);
+
+ if (ppc_md.pci_dma_dev_setup)
+ ppc_md.pci_dma_dev_setup(dev);
+
+ pci_read_irq_line(dev);
+ if (ppc_md.pci_irq_fixup)
+ ppc_md.pci_irq_fixup(dev);
+ }
return pci_enable_resources(dev, mask);
}
Regards,
Yuanquan
Signed-off-by: Rojhalat Ibrahim i...@rtschenk.de
---
arch/powerpc/sysdev/fsl_pci.c | 49
++
1 file changed, 45 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_pci.c
b/arch/powerpc/sysdev/fsl_pci.c index 682084d..693db9f 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -36,6 +36,8 @@
static int fsl_pcie_bus_fixup, is_mpc83xx_pci;
+static struct pci_ops *indirect_pci_ops;
+
static void quirk_fsl_pcie_header(struct pci_dev *dev) {
u8 hdr_type;
@@ -64,6 +66,45 @@ static int __init fsl_pcie_check_link(struct
pci_controller
*hose)
return 0;
}
+static int fsl_indirect_read_config(struct pci_bus *bus, unsigned int
devfn,
+ int offset, int len, u32 *val)
+{
+ struct pci_controller *hose = pci_bus_to_host(bus);
+
+ // check the link status
+ if ((bus-number == hose-first_busno) (devfn == 0)) {
+ u32 ltssm = 0;
+ indirect_pci_ops-read(bus, 0, PCIE_LTSSM, 4, ltssm);
+ if (ltssm PCIE_LTSSM_L0) {
+ hose-indirect_type |= PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+ } else {
+ hose-indirect_type = ~PPC_INDIRECT_TYPE_NO_PCIE_LINK;
+ }
+ }
+ return indirect_pci_ops-read
Re: [linuxppc-release][PATCH] powerpc/pci-hotplug: fix init issue of rescanned pci device
On 12/06/2012 05:30 AM, Bjorn Helgaas wrote:
On Wed, Dec 5, 2012 at 2:29 AM, Chen Yuanquan-B41889
b41...@freescale.com wrote:
On 12/05/2012 04:26 PM, Benjamin Herrenschmidt wrote:
On Wed, 2012-12-05 at 16:20 +0800, Chen Yuanquan-B41889 wrote:
On 12/05/2012 03:17 PM, Benjamin Herrenschmidt wrote:
On Wed, 2012-12-05 at 10:31 +0800, Yuanquan Chen wrote:
On powerpc arch, some fixup work of PCI/PCI-e device is just done
during the
first scan at booting time. For the PCI/PCI-e device rescanned after
linux OS
booting up, the fixup work won't be done, which leads to dma_set_mask
error or
irq related issue in rescanned PCI/PCI-e device's driver. So, it does
the same
fixup work for the rescanned device to avoid this issue.
Hrm, the patch is a bit gross. First the code shouldn't be copy/pasted
that way but factored out.
Please, at least format your email properly so I can try to undertand
without needing aspirin.
There's a judgement if (!bus-is_added) before calling of
pcibios_fixup_bus in pci_scan_child_bus, so for the rescanned device,
the fixup won't execute, which leads to fatal error in driver of
rescanned
device on freescale powerpc, no this issues on x86 arch.
First, none of that invalidates my statement that you shouldn't
duplicate a whole block of code like this. Even if your approach is
correct (which is debated separately), at the very least you should
factor the code out into a common function between the two copies.
Remove the judgement, let it to do the pcibios_fixup_bus
directly, the error won't occur for the rescanned device. But it's
general code, not proper to change here, so copy the pcibios_fixup_bus
work to pcibios_enable_device.
I'm surprised also that is_added is false when pcibios_enable_device()
gets called ... that looks strange to me. At what point is that enable
happening in the hotplug sequence ?
All devices are rescanned and then call the pci_enable_devices and
pci_bus_add_devices.
Where ? How ? What is the sequence happening ? In any case, I think if
we need a proper fixup done per-device like that after scan we ought to
create a new hook at the generic level rather than that sort of hack.
echo 1 rescan to trigger dev_rescan_store:
dev_rescan_store-pci_rescan_bus-pci_scan_child_bus,
pci_assign_unassigned_bus_resources,
pci_enable_bridges, pci_bus_add_devices
pci_enable_bridges-pci_enable_device-__pci_enable_device_flags-do_pci_enable_device-
pcibios_enable_device
pci_bus_add_devices-pci_bus_add_device-dev-is_added = 1
Yeah, it's general fixup code for every rescanned PCI/PCI-e device on
powerpc at runtime. So if
we want to call it in a ppc_md member, we need to wrap it as a function and
assign it in every ppc_md,
it isn't proper for the general code.
Regards,
yuanquan
The patch code will be called by pci_enable_devices. The dev-is_added
is set in pci_bus_add_device
which is called by pci_bus_add_devices. So dev-is_added is false when
checking it in pcibios_enable_device
for the rescanned device.
Who calls pci_enable_device() in the rescan case ? Why isn't it left to
the driver ? I don't think we can rely on that behaviour not to change.
How do you trigger the rescan anyway ?
Use the interface under /sys :
echo 1 /sys/bus/pci/devices/xxx/remove
then echo 1 to the pci device which is the bus of the removed device
echo 1 /sys/bus/pci/devices//rescan
the removed device will be scanned and it's driver module will be loaded
automatically.
Yeah this code path are known to be fishy. I think the problem is at the
generic abstraction level and that's where it needs to be fixed.
Cheers,
Ben.
Regards,
yuanquan
I think the problem needs to be solve at a higher level, I'm adding
linux-pci Bjorn to the CC list.
Cheers,
Ben.
Signed-off-by: Yuanquan Chen b41...@freescale.com
---
arch/powerpc/kernel/pci-common.c | 20
1 file changed, 20 insertions(+)
diff --git a/arch/powerpc/kernel/pci-common.c
b/arch/powerpc/kernel/pci-common.c
index 7f94f76..f0fb070 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1496,6 +1496,26 @@ int pcibios_enable_device(struct pci_dev *dev,
int mask)
if (ppc_md.pcibios_enable_device_hook(dev))
return -EINVAL;
+if (!dev-is_added) {
+ /*
+* 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
Re: [linuxppc-release][PATCH] powerpc/pci-hotplug: fix init issue of rescanned pci device
On 12/05/2012 03:17 PM, Benjamin Herrenschmidt wrote:
On Wed, 2012-12-05 at 10:31 +0800, Yuanquan Chen wrote:
On powerpc arch, some fixup work of PCI/PCI-e device is just done during the
first scan at booting time. For the PCI/PCI-e device rescanned after linux OS
booting up, the fixup work won't be done, which leads to dma_set_mask error or
irq related issue in rescanned PCI/PCI-e device's driver. So, it does the same
fixup work for the rescanned device to avoid this issue.
Hrm, the patch is a bit gross. First the code shouldn't be copy/pasted
that way but factored out.
There's a judgement if (!bus-is_added) before calling of
pcibios_fixup_bus
in pci_scan_child_bus, so for the rescanned device, the fixup won't execute,
which leads to fatal error in driver of rescanned device on freescale
powerpc, no
this issues on x86 arch. Remove the judgement, let it to do the
pcibios_fixup_bus
directly, the error won't occur for the rescanned device. But it's
general code, not
proper to change here, so copy the pcibios_fixup_bus work to
pcibios_enable_device.
I'm surprised also that is_added is false when pcibios_enable_device()
gets called ... that looks strange to me. At what point is that enable
happening in the hotplug sequence ?
All devices are rescanned and then call the pci_enable_devices and
pci_bus_add_devices.
The patch code will be called by pci_enable_devices. The dev-is_added
is set in pci_bus_add_device
which is called by pci_bus_add_devices. So dev-is_added is false when
checking it in pcibios_enable_device
for the rescanned device.
How do you trigger the rescan anyway ?
Use the interface under /sys :
echo 1 /sys/bus/pci/devices/xxx/remove
then echo 1 to the pci device which is the bus of the removed device
echo 1 /sys/bus/pci/devices//rescan
the removed device will be scanned and it's driver module will be loaded
automatically.
Regards,
yuanquan
I think the problem needs to be solve at a higher level, I'm adding
linux-pci Bjorn to the CC list.
Cheers,
Ben.
Signed-off-by: Yuanquan Chen b41...@freescale.com
---
arch/powerpc/kernel/pci-common.c | 20
1 file changed, 20 insertions(+)
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 7f94f76..f0fb070 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1496,6 +1496,26 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
if (ppc_md.pcibios_enable_device_hook(dev))
return -EINVAL;
+ if (!dev-is_added) {
+ /*
+* 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);
+ }
return pci_enable_resources(dev, mask);
}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [linuxppc-release][PATCH] powerpc/pci-hotplug: fix init issue of rescanned pci device
On 12/05/2012 04:26 PM, Benjamin Herrenschmidt wrote:
On Wed, 2012-12-05 at 16:20 +0800, Chen Yuanquan-B41889 wrote:
On 12/05/2012 03:17 PM, Benjamin Herrenschmidt wrote:
On Wed, 2012-12-05 at 10:31 +0800, Yuanquan Chen wrote:
On powerpc arch, some fixup work of PCI/PCI-e device is just done during the
first scan at booting time. For the PCI/PCI-e device rescanned after linux OS
booting up, the fixup work won't be done, which leads to dma_set_mask error or
irq related issue in rescanned PCI/PCI-e device's driver. So, it does the same
fixup work for the rescanned device to avoid this issue.
Hrm, the patch is a bit gross. First the code shouldn't be copy/pasted
that way but factored out.
Please, at least format your email properly so I can try to undertand
without needing aspirin.
There's a judgement if (!bus-is_added) before calling of
pcibios_fixup_bus in pci_scan_child_bus, so for the rescanned device,
the fixup won't execute, which leads to fatal error in driver of rescanned
device on freescale powerpc, no this issues on x86 arch.
First, none of that invalidates my statement that you shouldn't
duplicate a whole block of code like this. Even if your approach is
correct (which is debated separately), at the very least you should
factor the code out into a common function between the two copies.
Remove the judgement, let it to do the pcibios_fixup_bus
directly, the error won't occur for the rescanned device. But it's
general code, not proper to change here, so copy the pcibios_fixup_bus
work to pcibios_enable_device.
I'm surprised also that is_added is false when pcibios_enable_device()
gets called ... that looks strange to me. At what point is that enable
happening in the hotplug sequence ?
All devices are rescanned and then call the pci_enable_devices and
pci_bus_add_devices.
Where ? How ? What is the sequence happening ? In any case, I think if
we need a proper fixup done per-device like that after scan we ought to
create a new hook at the generic level rather than that sort of hack.
echo 1 rescan to trigger dev_rescan_store:
dev_rescan_store-pci_rescan_bus-pci_scan_child_bus,
pci_assign_unassigned_bus_resources,
pci_enable_bridges, pci_bus_add_devices
pci_enable_bridges-pci_enable_device-__pci_enable_device_flags-do_pci_enable_device-
pcibios_enable_device
pci_bus_add_devices-pci_bus_add_device-dev-is_added = 1
Yeah, it's general fixup code for every rescanned PCI/PCI-e device on
powerpc at runtime. So if
we want to call it in a ppc_md member, we need to wrap it as a function
and assign it in every ppc_md,
it isn't proper for the general code.
Regards,
yuanquan
The patch code will be called by pci_enable_devices. The dev-is_added
is set in pci_bus_add_device
which is called by pci_bus_add_devices. So dev-is_added is false when
checking it in pcibios_enable_device
for the rescanned device.
Who calls pci_enable_device() in the rescan case ? Why isn't it left to
the driver ? I don't think we can rely on that behaviour not to change.
How do you trigger the rescan anyway ?
Use the interface under /sys :
echo 1 /sys/bus/pci/devices/xxx/remove
then echo 1 to the pci device which is the bus of the removed device
echo 1 /sys/bus/pci/devices//rescan
the removed device will be scanned and it's driver module will be loaded
automatically.
Yeah this code path are known to be fishy. I think the problem is at the
generic abstraction level and that's where it needs to be fixed.
Cheers,
Ben.
Regards,
yuanquan
I think the problem needs to be solve at a higher level, I'm adding
linux-pci Bjorn to the CC list.
Cheers,
Ben.
Signed-off-by: Yuanquan Chen b41...@freescale.com
---
arch/powerpc/kernel/pci-common.c | 20
1 file changed, 20 insertions(+)
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 7f94f76..f0fb070 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1496,6 +1496,26 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
if (ppc_md.pcibios_enable_device_hook(dev))
return -EINVAL;
+ if (!dev-is_added) {
+ /*
+* 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);
+ }
return pci_enable_resources(dev, mask
Re: [PATCH] powerpc/pci-hotplug: fix the rescanned pci device's dma_set_mask issue
On 11/25/2012 08:41 PM, Kumar Gala wrote:
On Nov 22, 2012, at 10:29 PM, Yuanquan Chen wrote:
On powerpc arch, dma_ops of rescanned pci device after system's booting up
won't be
initialized by system, so it will fail to execute the dma_set_mask in the
device's
driver. Initialize it to solve this issue.
Signed-off-by: Yuanquan Chen b41...@freescale.com
---
arch/powerpc/include/asm/dma-mapping.h |7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
This is not the right way to get the dma_ops setup. You need to find some
other point for the hotplug scenario to get the dma_ops setup.
- k
Hi Kumar,
I read the code about pci bus scan and rescan. Only the
pcibios_fixup_bus in pci_scan_child_bus and
pcibios_enable_device in pci_rescan_bus are arch related code. The
pcibios_fixup_bus won't be called
for the rescanned PCI devices due to the bus-is_added has been set for
the first scanning at boot time.
So I think it's more reasonable to do the same work as pcibios_fixup_bus
for rescanned PCI device in
pcibios_enable_device. The patch code is a copy of
pcibios_setup_bus_devices called by pcibios_fixup_bus,
It can solve the dma_set_mask and irq related issues of rescanned PCI
device on powerpc arch. What's
your opinion?
Thanks,
yuanquan
diff --git a/arch/powerpc/kernel/pci-common.c
b/arch/powerpc/kernel/pci-common.c
index 7f94f76..30f7d61 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1496,6 +1496,23 @@ int pcibios_enable_device(struct pci_dev *dev,
int mask)
if (ppc_md.pcibios_enable_device_hook(dev))
return -EINVAL;
+ if (!dev-is_added) {
+ 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);
+ }
+
return pci_enable_resources(dev, mask);
}
diff --git a/arch/powerpc/include/asm/dma-mapping.h
b/arch/powerpc/include/asm/dma-mapping.h
index 7816087..22eae53 100644
--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h
@@ -126,8 +126,11 @@ static inline int dma_supported(struct device *dev, u64
mask)
{
struct dma_map_ops *dma_ops = get_dma_ops(dev);
- if (unlikely(dma_ops == NULL))
- return 0;
+ if (unlikely(dma_ops == NULL)) {
+ set_dma_ops(dev, dma_direct_ops);
+ set_dma_offset(dev, PCI_DRAM_OFFSET);
+ dma_ops = dma_direct_ops;
+ }
if (dma_ops-dma_supported == NULL)
return 1;
return dma_ops-dma_supported(dev, mask);
--
1.7.9.5
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc/pci-hotplug: fix the rescanned pci device's dma_set_mask issue
On 11/25/2012 08:41 PM, Kumar Gala wrote:
On Nov 22, 2012, at 10:29 PM, Yuanquan Chen wrote:
On powerpc arch, dma_ops of rescanned pci device after system's booting up
won't be
initialized by system, so it will fail to execute the dma_set_mask in the
device's
driver. Initialize it to solve this issue.
Signed-off-by: Yuanquan Chen b41...@freescale.com
---
arch/powerpc/include/asm/dma-mapping.h |7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
This is not the right way to get the dma_ops setup. You need to find some
other point for the hotplug scenario to get the dma_ops setup.
- k
Hi Kumar,
I read the code about pci bus scan and rescan. Only the
pcibios_fixup_bus in pci_scan_child_bus and
pcibios_enable_device in pci_rescan_bus are arch related code. The
pcibios_fixup_bus won't be called
for the rescanned PCI devices due to the bus-is_added has been set for
the first scanning at boot time.
So I think it's more reasonable to do the same work as pcibios_fixup_bus
for rescanned PCI device in
pcibios_enable_device. The patch code is a copy of
pcibios_setup_bus_devices called by pcibios_fixup_bus,
It can solve the dma_set_mask and irq related issues of rescanned PCI
device on powerpc arch. What's
your opinion?
Thanks,
yuanquan
diff --git a/arch/powerpc/kernel/pci-common.c
b/arch/powerpc/kernel/pci-common.c
index 7f94f76..30f7d61 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1496,6 +1496,23 @@ int pcibios_enable_device(struct pci_dev *dev,
int mask)
if (ppc_md.pcibios_enable_device_hook(dev))
return -EINVAL;
+ if (!dev-is_added) {
+ 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);
+ }
+
return pci_enable_resources(dev, mask);
}
diff --git a/arch/powerpc/include/asm/dma-mapping.h
b/arch/powerpc/include/asm/dma-mapping.h
index 7816087..22eae53 100644
--- a/arch/powerpc/include/asm/dma-mapping.h
+++ b/arch/powerpc/include/asm/dma-mapping.h
@@ -126,8 +126,11 @@ static inline int dma_supported(struct device *dev, u64
mask)
{
struct dma_map_ops *dma_ops = get_dma_ops(dev);
- if (unlikely(dma_ops == NULL))
- return 0;
+ if (unlikely(dma_ops == NULL)) {
+ set_dma_ops(dev, dma_direct_ops);
+ set_dma_offset(dev, PCI_DRAM_OFFSET);
+ dma_ops = dma_direct_ops;
+ }
if (dma_ops-dma_supported == NULL)
return 1;
return dma_ops-dma_supported(dev, mask);
--
1.7.9.5
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
