arm64: dts: marvell: armada-37xx: Set linux,pci-domain to zero
Subsystems (3)
PATCH 1/1 arm64: dts: marvell: armada-37xx: Set linux,pci-domain to zero
Since commit 526a76991b7b ("PCI: aardvark: Implement driver 'remove' function and allow to build it as module") PCIe controller driver for Armada 37xx can be dynamically loaded and unloaded at runtime. Also driver allows dynamic binding and unbinding of PCIe controller device.
Kernel PCI subsystem assigns by default dynamically allocated PCI domain number (starting from zero) for this PCIe controller every time when device is bound. So PCI domain changes after every unbind / bind operation.
Alternative way for assigning PCI domain number is to use static allocated numbers defined in Device Tree. This option has requirement that every PCI controller in system must have defined PCI bus number in Device Tree.
Armada 37xx has only one PCIe controller, so assign for it PCI domain 0 in Device Tree.
After this change PCI domain on Armada 37xx is always zero, even after repeated unbind and bind operations.
--- arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 1 + 1 file changed, 1 insertion(+)
arch/arm64/boot/dts/marvell/armada-37xx.dtsi +1 -0
@@ -495,6 +495,7 @@ <0 0 0 2 &pcie_intc 1>, <0 0 0 3 &pcie_intc 2>, <0 0 0 4 &pcie_intc 3>; + linux,pci-domain = <0>; max-link-speed = <2>; phys = <&comphy1 0>; pcie_intc: interrupt-controller {
Signature · 1 line
2.20.1
Quoted 9 lines · Pali Rohár
Since commit
526a76991b7b("PCI: aardvark: Implement driver 'remove' function and allow to build it as module") PCIe controller driver for Armada 37xx can be dynamically loaded and unloaded at runtime. Also driver allows dynamic binding and unbinding of PCIe controller device.Kernel PCI subsystem assigns by default dynamically allocated PCI domain number (starting from zero) for this PCIe controller every time when device is bound. So PCI domain changes after every unbind / bind operation.
PCI host bridges as a module are relatively new, so seems likely a bug to me.
Quoted 3 lines
Alternative way for assigning PCI domain number is to use static allocated numbers defined in Device Tree. This option has requirement that every PCI controller in system must have defined PCI bus number in Device Tree.
That seems entirely pointless from a DT point of view with a single PCI bridge.
Quoted 27 lines
Armada 37xx has only one PCIe controller, so assign for it PCI domain 0 in Device Tree.
After this change PCI domain on Armada 37xx is always zero, even after repeated unbind and bind operations.
Signed-off-by Pali Rohár <[email protected]>Fixes 526a76991b7b ("PCI: aardvark: Implement driver 'remove' function and allow to build it as module")--- arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 1 + 1 file changed, 1 insertion(+)arch/arm64/boot/dts/marvell/armada-37xx.dtsi +1
-1@@ -495,6 +495,7 @@ <0 0 0 2 &pcie_intc 1>, <0 0 0 3 &pcie_intc 2>, <0 0 0 4 &pcie_intc 3>; + linux,pci-domain = <0>; max-link-speed = <2>; phys = <&comphy1 0>; pcie_intc: interrupt-controller { --2.20.1
Quoted 7 lines · On Tuesday 13 April 2021 13:17:29 Rob Herring wrote:
Quoted 6 lines · Pali Rohár
Since commit
526a76991b7b("PCI: aardvark: Implement driver 'remove' function and allow to build it as module") PCIe controller driver for Armada 37xx can be dynamically loaded and unloaded at runtime. Also driver allows dynamic binding and unbinding of PCIe controller device.
Quoted 5 lines
Quoted 3 lines
Kernel PCI subsystem assigns by default dynamically allocated PCI domain number (starting from zero) for this PCIe controller every time when device is bound. So PCI domain changes after every unbind / bind operation.
PCI host bridges as a module are relatively new, so seems likely a bug to me.
Why a bug? It is there since 5.10 and it is working.
Quoted 5 lines
Quoted 3 lines
Alternative way for assigning PCI domain number is to use static allocated numbers defined in Device Tree. This option has requirement that every PCI controller in system must have defined PCI bus number in Device Tree.
That seems entirely pointless from a DT point of view with a single PCI bridge.
If domain id is not specified in DT then kernel uses counter and assigns counter++. So it is not pointless if we want to have stable domain id.
Quoted 27 lines
Quoted 27 lines
Armada 37xx has only one PCIe controller, so assign for it PCI domain 0 in Device Tree.
After this change PCI domain on Armada 37xx is always zero, even after repeated unbind and bind operations.
Signed-off-by Pali Rohár <[email protected]>Fixes 526a76991b7b ("PCI: aardvark: Implement driver 'remove' function and allow to build it as module")--- arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 1 + 1 file changed, 1 insertion(+)arch/arm64/boot/dts/marvell/armada-37xx.dtsi +1
-1@@ -495,6 +495,7 @@ <0 0 0 2 &pcie_intc 1>, <0 0 0 3 &pcie_intc 2>, <0 0 0 4 &pcie_intc 3>; + linux,pci-domain = <0>; max-link-speed = <2>; phys = <&comphy1 0>; pcie_intc: interrupt-controller { --2.20.1
Quoted 16 lines · On Thu, 15 Apr 2021 10:36:40 +0200 Pali Rohár <[email protected]> wrote:
Quoted 7 lines · On Tuesday 13 April 2021 13:17:29 Rob Herring wrote:
Quoted 6 lines · Pali Rohár
Since commit
526a76991b7b("PCI: aardvark: Implement driver 'remove' function and allow to build it as module") PCIe controller driver for Armada 37xx can be dynamically loaded and unloaded at runtime. Also driver allows dynamic binding and unbinding of PCIe controller device.Quoted 5 lines
Quoted 3 lines
Kernel PCI subsystem assigns by default dynamically allocated PCI domain number (starting from zero) for this PCIe controller every time when device is bound. So PCI domain changes after every unbind / bind operation.
PCI host bridges as a module are relatively new, so seems likely a bug to me.
Why a bug? It is there since 5.10 and it is working.
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Alternative way for assigning PCI domain number is to use static allocated numbers defined in Device Tree. This option has requirement that every PCI controller in system must have defined PCI bus number in Device Tree.
That seems entirely pointless from a DT point of view with a single PCI bridge.
If domain id is not specified in DT then kernel uses counter and assigns counter++. So it is not pointless if we want to have stable domain id.
What Rob is trying to say is that
- the bug is that kernel assigns counter++
- device-tree should not be used to fix problems with how kernel does things
- if a device has only one PCIe controller, it is pointless to define it's pci-domain. If there were multiple controllers, then it would make sense, but there is only one
Quoted 12 lines · Marek Behun
Quoted 8 lines · On Thu, 15 Apr 2021 10:36:40 +0200 Pali Rohár <[email protected]> wrote:
Quoted 7 lines · On Tuesday 13 April 2021 13:17:29 Rob Herring wrote:
Quoted 6 lines · Pali Rohár
Since commit
526a76991b7b("PCI: aardvark: Implement driver 'remove' function and allow to build it as module") PCIe controller driver for Armada 37xx can be dynamically loaded and unloaded at runtime. Also driver allows dynamic binding and unbinding of PCIe controller device.
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Kernel PCI subsystem assigns by default dynamically allocated PCI domain number (starting from zero) for this PCIe controller every time when device is bound. So PCI domain changes after every unbind / bind operation.
PCI host bridges as a module are relatively new, so seems likely a bug to me.
Why a bug? It is there since 5.10 and it is working.
I mean historically, the PCI subsystem didn't even support host bridges as a module. They weren't even proper drivers and it was all arch specific code. Most of the host bridge drivers are still built-in only. This seems like a small detail that was easily overlooked. unbind is not a well tested path.
Quoted 9 lines
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Alternative way for assigning PCI domain number is to use static allocated numbers defined in Device Tree. This option has requirement that every PCI controller in system must have defined PCI bus number in Device Tree.
That seems entirely pointless from a DT point of view with a single PCI bridge.
If domain id is not specified in DT then kernel uses counter and assigns counter++. So it is not pointless if we want to have stable domain id.
Quoted 7 lines
What Rob is trying to say is that
- the bug is that kernel assigns counter++
- device-tree should not be used to fix problems with how kernel does things
- if a device has only one PCIe controller, it is pointless to define it's pci-domain. If there were multiple controllers, then it would make sense, but there is only one
Yes. I think what we want here is a domain bitmap rather than a counter and we assign the lowest free bit. That could also allow for handling a mixture of fixed domain numbers and dynamically assigned ones.
You could create scenarios where the numbers change on you, but it wouldn't be any different than say plugging in USB serial adapters. You get the same ttyUSBx device when you re-attach unless there's been other ttyUSBx devices attached/detached.
Rob
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoted 19 lines · On Thursday 15 April 2021 10:13:17 Rob Herring wrote:
Quoted 18 lines · Marek Behun
Quoted 14 lines · On Thu, 15 Apr 2021 10:36:40 +0200 Pali Rohár <[email protected]> wrote:
Quoted 12 lines · On Tuesday 13 April 2021 13:17:29 Rob Herring wrote:
Quoted 9 lines · Pali Rohár
Since commit
526a76991b7b("PCI: aardvark: Implement driver 'remove' function and allow to build it as module") PCIe controller driver for Armada 37xx can be dynamically loaded and unloaded at runtime. Also driver allows dynamic binding and unbinding of PCIe controller device.Kernel PCI subsystem assigns by default dynamically allocated PCI domain number (starting from zero) for this PCIe controller every time when device is bound. So PCI domain changes after every unbind / bind operation.
PCI host bridges as a module are relatively new, so seems likely a bug to me.
Quoted 7 lines
Quoted 1 line
Quoted 1 line
Why a bug? It is there since 5.10 and it is working.
I mean historically, the PCI subsystem didn't even support host bridges as a module. They weren't even proper drivers and it was all arch specific code. Most of the host bridge drivers are still built-in only. This seems like a small detail that was easily overlooked. unbind is not a well tested path.
Ok! Just to note that during my testing I have not spotted any issue.
Quoted 17 lines
Quoted 9 lines
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Alternative way for assigning PCI domain number is to use static allocated numbers defined in Device Tree. This option has requirement that every PCI controller in system must have defined PCI bus number in Device Tree.
That seems entirely pointless from a DT point of view with a single PCI bridge.
If domain id is not specified in DT then kernel uses counter and assigns counter++. So it is not pointless if we want to have stable domain id.
Quoted 7 lines
What Rob is trying to say is that
- the bug is that kernel assigns counter++
- device-tree should not be used to fix problems with how kernel does things
- if a device has only one PCIe controller, it is pointless to define it's pci-domain. If there were multiple controllers, then it would make sense, but there is only one
Quoted 4 lines
Yes. I think what we want here is a domain bitmap rather than a counter and we assign the lowest free bit. That could also allow for handling a mixture of fixed domain numbers and dynamically assigned ones.
Currently this code is implemented in pci_bus_find_domain_nr() function. IIRC domain number is 16bit integer, so plain bitmap would consume 8 kB of memory. I'm not sure if it is fine or some other tree-based structure for allocated domain numbers is needed.
Quoted 4 lines
You could create scenarios where the numbers change on you, but it wouldn't be any different than say plugging in USB serial adapters. You get the same ttyUSBx device when you re-attach unless there's been other ttyUSBx devices attached/detached.
This should be fine for most scenarios. Dynamically attaching / detaching PCI domain is not such common action...
Will you implement this new feature?
Andrew Lunn
maintainerQuoted 4 lines
Currently this code is implemented in
pci_bus_find_domain_nr()function. IIRC domain number is 16bit integer, so plain bitmap would consume 8 kB of memory. I'm not sure if it is fine or some other tree-based structure for allocated domain numbers is needed.
Hi Pali
Have a look at lib/idr.c
Andrew
Quoted 10 lines · On Saturday 17 April 2021 17:19:38 Andrew Lunn wrote:
Quoted 4 lines
Currently this code is implemented in
pci_bus_find_domain_nr()function. IIRC domain number is 16bit integer, so plain bitmap would consume 8 kB of memory. I'm not sure if it is fine or some other tree-based structure for allocated domain numbers is needed.Hi Pali
Have a look at
lib/idr.cAndrew
Great! So number allocation is already implemented in kernel (via radix trees).
Quoted 53 lines · Pali Rohár
Quoted 19 lines · On Thursday 15 April 2021 10:13:17 Rob Herring wrote:
Quoted 18 lines · Marek Behun
Quoted 14 lines · On Thu, 15 Apr 2021 10:36:40 +0200 Pali Rohár <[email protected]> wrote:
Quoted 12 lines · On Tuesday 13 April 2021 13:17:29 Rob Herring wrote:
Quoted 9 lines · Pali Rohár
Since commit
526a76991b7b("PCI: aardvark: Implement driver 'remove' function and allow to build it as module") PCIe controller driver for Armada 37xx can be dynamically loaded and unloaded at runtime. Also driver allows dynamic binding and unbinding of PCIe controller device.Kernel PCI subsystem assigns by default dynamically allocated PCI domain number (starting from zero) for this PCIe controller every time when device is bound. So PCI domain changes after every unbind / bind operation.
PCI host bridges as a module are relatively new, so seems likely a bug to me.
Quoted 7 lines
Quoted 1 line
Quoted 1 line
Why a bug? It is there since 5.10 and it is working.
I mean historically, the PCI subsystem didn't even support host bridges as a module. They weren't even proper drivers and it was all arch specific code. Most of the host bridge drivers are still built-in only. This seems like a small detail that was easily overlooked. unbind is not a well tested path.
Ok! Just to note that during my testing I have not spotted any issue.
Quoted 17 lines
Quoted 9 lines
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Alternative way for assigning PCI domain number is to use static allocated numbers defined in Device Tree. This option has requirement that every PCI controller in system must have defined PCI bus number in Device Tree.
That seems entirely pointless from a DT point of view with a single PCI bridge.
If domain id is not specified in DT then kernel uses counter and assigns counter++. So it is not pointless if we want to have stable domain id.
Quoted 7 lines
What Rob is trying to say is that
- the bug is that kernel assigns counter++
- device-tree should not be used to fix problems with how kernel does things
- if a device has only one PCIe controller, it is pointless to define it's pci-domain. If there were multiple controllers, then it would make sense, but there is only oneQuoted 4 lines
Yes. I think what we want here is a domain bitmap rather than a counter and we assign the lowest free bit. That could also allow for handling a mixture of fixed domain numbers and dynamically assigned ones.
Quoted 4 lines
Currently this code is implemented in
pci_bus_find_domain_nr()function. IIRC domain number is 16bit integer, so plain bitmap would consume 8 kB of memory. I'm not sure if it is fine or some other tree-based structure for allocated domain numbers is needed.
It's an atomic_t but then shortened (potentially) to an 'int'. Surely we don't need 8k (or 2^31) host bridges? Seems like we could start with 64 and bump it as needed. Or the idr route is another option if that works. We'd need to get the lowest free value and be able to reserve values (when specified by firmware).
Quoted 9 lines
Quoted 4 lines
You could create scenarios where the numbers change on you, but it wouldn't be any different than say plugging in USB serial adapters. You get the same ttyUSBx device when you re-attach unless there's been other ttyUSBx devices attached/detached.
This should be fine for most scenarios. Dynamically attaching / detaching PCI domain is not such common action...
Will you implement this new feature?
Yes, after I find a DT binding co-maintainer.
Rob
Quoted 59 lines · On Friday 23 April 2021 10:33:38 Rob Herring wrote:
Quoted 53 lines · Pali Rohár
Quoted 19 lines · On Thursday 15 April 2021 10:13:17 Rob Herring wrote:
Quoted 18 lines · Marek Behun
Quoted 14 lines · On Thu, 15 Apr 2021 10:36:40 +0200 Pali Rohár <[email protected]> wrote:
Quoted 12 lines · On Tuesday 13 April 2021 13:17:29 Rob Herring wrote:
Quoted 9 lines · Pali Rohár
Since commit
526a76991b7b("PCI: aardvark: Implement driver 'remove' function and allow to build it as module") PCIe controller driver for Armada 37xx can be dynamically loaded and unloaded at runtime. Also driver allows dynamic binding and unbinding of PCIe controller device.Kernel PCI subsystem assigns by default dynamically allocated PCI domain number (starting from zero) for this PCIe controller every time when device is bound. So PCI domain changes after every unbind / bind operation.
PCI host bridges as a module are relatively new, so seems likely a bug to me.
Quoted 7 lines
Quoted 1 line
Quoted 1 line
Why a bug? It is there since 5.10 and it is working.
I mean historically, the PCI subsystem didn't even support host bridges as a module. They weren't even proper drivers and it was all arch specific code. Most of the host bridge drivers are still built-in only. This seems like a small detail that was easily overlooked. unbind is not a well tested path.
Ok! Just to note that during my testing I have not spotted any issue.
Quoted 17 lines
Quoted 9 lines
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Alternative way for assigning PCI domain number is to use static allocated numbers defined in Device Tree. This option has requirement that every PCI controller in system must have defined PCI bus number in Device Tree.
That seems entirely pointless from a DT point of view with a single PCI bridge.
If domain id is not specified in DT then kernel uses counter and assigns counter++. So it is not pointless if we want to have stable domain id.
Quoted 7 lines
What Rob is trying to say is that
- the bug is that kernel assigns counter++
- device-tree should not be used to fix problems with how kernel does things
- if a device has only one PCIe controller, it is pointless to define it's pci-domain. If there were multiple controllers, then it would make sense, but there is only oneQuoted 4 lines
Yes. I think what we want here is a domain bitmap rather than a counter and we assign the lowest free bit. That could also allow for handling a mixture of fixed domain numbers and dynamically assigned ones.
Quoted 4 lines
Currently this code is implemented in
pci_bus_find_domain_nr()function. IIRC domain number is 16bit integer, so plain bitmap would consume 8 kB of memory. I'm not sure if it is fine or some other tree-based structure for allocated domain numbers is needed.
Quoted 5 lines
It's an
atomic_tbut then shortened (potentially) to an 'int'. Surely we don't need 8k (or 2^31) host bridges? Seems like we could start with 64 and bump it as needed. Or the idr route is another option if that works. We'd need to get the lowest free value and be able to reserve values (when specified by firmware).
Seems that idr_alloc() supports both required operations as you can ask idr_alloc() for allocating specific id (if is available).
Quoted 13 lines
Quoted 9 lines
Quoted 4 lines
You could create scenarios where the numbers change on you, but it wouldn't be any different than say plugging in USB serial adapters. You get the same ttyUSBx device when you re-attach unless there's been other ttyUSBx devices attached/detached.
This should be fine for most scenarios. Dynamically attaching / detaching PCI domain is not such common action...
Will you implement this new feature?
Yes, after I find a DT binding co-maintainer.
Rob