ARM: Kirkwood: Support basic hotplug for PCI-E
Subsystems (2)
PATCH 1/1 ARM: Kirkwood: Support basic hotplug for PCI-E
Unconditionally register the PCI-E bus, even if the link is currently down. When the link is brought up the bus can be scanned through /sys/bus/pci/rescan or otherwise. Since the HW has no interrupt for link up, userspace will have to take care of the timing.
An earlier version of this was contingent on CONFIG_HOTPLUG, but that is being removed from the kernel.
This also fixes printing the link up/down message to be displayed on one line (structured logging broke this?)
--- arch/arm/mach-kirkwood/pcie.c | 10 +++------- 1 files changed, 3 insertions(+), 7 deletions(-)
All PCI-E ports are required to support hot plug at the link training level. Our systems support it electrically, and userspace sequences everything to work properly. But the PCI-E root port needs to be registered with the kernel to initiate a rescan via sysfs when things are ready.
arch/arm/mach-kirkwood/pcie.c +3 -7
@@ -244,13 +244,9 @@ static struct hw_pci kirkwood_pci __initdata = {static void __init add_pcie_port (int index,void __iomem *base) { -printk (KERN_INFO "Kirkwood PCIe port %d: " , index); - -if (orion_pcie_link_up (base)) { -printk (KERN_INFO "link up\n" ); - pcie_port_map[num_pcie_ports++] = index; - }else -printk (KERN_INFO "link down, ignoring\n" ); + pcie_port_map[num_pcie_ports++] = index; +pr_info ("Kirkwood PCIe port %d: link %s\n" , index, +orion_pcie_link_up (base) ?"up" :"down" ); }void __init kirkwood_pcie_init (unsigned int portmask)
Signature · 1 line
1.7.5.4
PATCH 1/1 ARM: Kirkwood: Support basic hotplug for PCI-E
Quoted 2 lines · Jason Gunthorpe
Unconditionally register the PCI-E bus, even if the link is currently down.
How does this affect clock gating on boards without PCI-E devices? Will the SoC then power this unconditionally?
thx,
Jason.
Quoted 45 lines
When the link is brought up the bus can be scanned through
/sys/bus/pci/rescanor otherwise. Since the HW has no interrupt for link up, userspace will have to take care of the timing.An earlier version of this was contingent on
CONFIG_HOTPLUG, but that is being removed from the kernel.This also fixes printing the link up/down message to be displayed on one line (structured logging broke this?)
Signed-off-by Jason Gunthorpe <[email protected]>--- arch/arm/mach-kirkwood/pcie.c | 10 +++------- 1 files changed, 3 insertions(+), 7 deletions(-)All PCI-E ports are required to support hot plug at the link training level. Our systems support it electrically, and userspace sequences everything to work properly. But the PCI-E root port needs to be registered with the kernel to initiate a rescan via sysfs when things are ready.
arch/arm/mach-kirkwood/pcie.c +3
-7@@ -244,13 +244,9 @@ static struct hw_pci kirkwood_pci __initdata = {static void __init add_pcie_port (int index,void __iomem *base) { -printk (KERN_INFO "Kirkwood PCIe port %d: " , index); - -if (orion_pcie_link_up (base)) { -printk (KERN_INFO "link up\n" ); - pcie_port_map[num_pcie_ports++] = index; - }else -printk (KERN_INFO "link down, ignoring\n" ); + pcie_port_map[num_pcie_ports++] = index; +pr_info ("Kirkwood PCIe port %d: link %s\n" , index, +orion_pcie_link_up (base) ?"up" :"down" ); }void __init kirkwood_pcie_init (unsigned int portmask)Signature · 1 line
1.7.5.4
PATCH 1/1 ARM: Kirkwood: Support basic hotplug for PCI-E
Quoted 6 lines · Jason Cooper
Quoted 2 lines · Jason Gunthorpe
Unconditionally register the PCI-E bus, even if the link is currently down.
How does this affect clock gating on boards without PCI-E devices? Will the SoC then power this unconditionally?
Hmm, interesting question.. This will definitely hold the clock lock on the PCI-E ports even if they are down at boot, and they will surely consume more power than if they were switched off.
However, if a board has no possible PCI-E devices, then it shouldn't be calling kirkwood_pcie_init, which will let things gate.
So the only case is boards that have pluggable PCI-E devices, where no device is in the plug? That sounds like eval cards, is this a big deal?
I guess another way to deal with this is to make the stuff in kirkwood/pcie.c an actual platform driver and hot attach the entire driver rather than using just using pci/rescan, but that is a fairly extensive change.. And I worry that powering down the interface in the interm will require re-initializing all the registers, which linux doesn't have the code to do at all.
How about fixing this up to have a DT binding and make it a DT controlled option - that seems like a simple middle ground..
What do you think?
Jason
PATCH 1/1 ARM: Kirkwood: Support basic hotplug for PCI-E
Quoted 8 lines · Jason Gunthorpe
Quoted 6 lines · Jason Cooper
Quoted 2 lines · Jason Gunthorpe
Unconditionally register the PCI-E bus, even if the link is currently down.
How does this affect clock gating on boards without PCI-E devices? Will the SoC then power this unconditionally?
Quoted 6 lines
Hmm, interesting question.. This will definitely hold the clock lock on the PCI-E ports even if they are down at boot, and they will surely consume more power than if they were switched off.
However, if a board has no possible PCI-E devices, then it shouldn't be calling
kirkwood_pcie_init, which will let things gate.
Ok, that's the good short-term answer.
Quoted 6 lines
I guess another way to deal with this is to make the stuff in
kirkwood/pcie.can actual platform driver and hot attach the entire driver rather than using just using pci/rescan, but that is a fairly extensive change.. And I worry that powering down the interface in the interm will require re-initializing all the registers, which linux doesn't have the code to do at all.
The goal is to remove everything from mach-kirkwood/, mach-dove/, etc. which means pcie will eventually become a real driver under drivers/. We just aren't there yet.
I'll pull this in as is. Thanks for the patch!
thx,
Jason.
PATCH 1/1 ARM: Kirkwood: Support basic hotplug for PCI-E
In reply to Jason CooperQuoted 6 lines · Jason Cooper
Quoted 2 lines · Jason Gunthorpe
Unconditionally register the PCI-E bus, even if the link is currently down.
How does this affect clock gating on boards without PCI-E devices? Will the SoC then power this unconditionally?
Hi Jason and Jason
I think it does.
The output from /debug/clk will tell.
I think it may also cause problems with udev persistent rules. The IDs will change for devices which are on the second controller and the first controller was previously not registered. I've not idea if this is just a theoretical problem, or a real problem.
Andrew
Quoted 50 lines
thx,
Jason.
Quoted 45 lines
When the link is brought up the bus can be scanned through
/sys/bus/pci/rescanor otherwise. Since the HW has no interrupt for link up, userspace will have to take care of the timing.An earlier version of this was contingent on
CONFIG_HOTPLUG, but that is being removed from the kernel.This also fixes printing the link up/down message to be displayed on one line (structured logging broke this?)
Signed-off-by Jason Gunthorpe <[email protected]>--- arch/arm/mach-kirkwood/pcie.c | 10 +++------- 1 files changed, 3 insertions(+), 7 deletions(-)All PCI-E ports are required to support hot plug at the link training level. Our systems support it electrically, and userspace sequences everything to work properly. But the PCI-E root port needs to be registered with the kernel to initiate a rescan via sysfs when things are ready.
arch/arm/mach-kirkwood/pcie.c +3
-7@@ -244,13 +244,9 @@ static struct hw_pci kirkwood_pci __initdata = {static void __init add_pcie_port (int index,void __iomem *base) { -printk (KERN_INFO "Kirkwood PCIe port %d: " , index); - -if (orion_pcie_link_up (base)) { -printk (KERN_INFO "link up\n" ); - pcie_port_map[num_pcie_ports++] = index; - }else -printk (KERN_INFO "link down, ignoring\n" ); + pcie_port_map[num_pcie_ports++] = index; +pr_info ("Kirkwood PCIe port %d: link %s\n" , index, +orion_pcie_link_up (base) ?"up" :"down" ); }void __init kirkwood_pcie_init (unsigned int portmask)Signature · 1 line
1.7.5.4
PATCH 1/1 ARM: Kirkwood: Support basic hotplug for PCI-E
Quoted 4 lines · Andrew Lunn
I think it may also cause problems with udev persistent rules. The IDs will change for devices which are on the second controller and the first controller was previously not registered. I've not idea if this is just a theoretical problem, or a real problem.
Most udev persistance rules I've seen are based on something more permanent, like a MAC address or whatnot. The PCI-E bus number is usually not used because it is historically not stable.
If a system is pluggable then the bus number already changes based on what ports are plugged, so in a sense this patch makes things a bit better by providing stable bus numbers.
When the PCI-E driver is integrated with DT it can get a busnr resource range unique to each port and then the bus numbers will be stable in almost all cases.
Jason
Jason Cooper
AppliedPATCH 1/1 ARM: Kirkwood: Support basic hotplug for PCI-E
In reply to Jason GunthorpeQuoted 8 lines · Jason Gunthorpe
Unconditionally register the PCI-E bus, even if the link is currently down. When the link is brought up the bus can be scanned through
/sys/bus/pci/rescanor otherwise. Since the HW has no interrupt for link up, userspace will have to take care of the timing.An earlier version of this was contingent on
CONFIG_HOTPLUG, but that is being removed from the kernel.
Quoted 7 lines
This also fixes printing the link up/down message to be displayed on one line (structured logging broke this?)
Signed-off-by Jason Gunthorpe <[email protected]>--- arch/arm/mach-kirkwood/pcie.c | 10 +++------- 1 files changed, 3 insertions(+), 7 deletions(-)
Applied to mvebu/drivers.
thx,
Jason.