[v2,1/6] dt-bindings: pci: Update iProc PCI binding for INTx support
diff mbox series

Message ID 1566982488-9673-2-git-send-email-srinath.mannam@broadcom.com
State New
Headers show
Series
  • PAXB INTx support with proper model
Related show

Commit Message

Srinath Mannam Aug. 28, 2019, 8:54 a.m. UTC
From: Ray Jui <ray.jui@broadcom.com>

Update the iProc PCIe binding document for better modeling of the legacy
interrupt (INTx) support

Signed-off-by: Ray Jui <ray.jui@broadcom.com>
Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
---
 .../devicetree/bindings/pci/brcm,iproc-pcie.txt    | 48 ++++++++++++++++++----
 1 file changed, 41 insertions(+), 7 deletions(-)

Comments

Rob Herring Sept. 2, 2019, 1:38 p.m. UTC | #1
On Wed, 28 Aug 2019 14:24:43 +0530, Srinath Mannam wrote:
> From: Ray Jui <ray.jui@broadcom.com>
> 
> Update the iProc PCIe binding document for better modeling of the legacy
> interrupt (INTx) support
> 
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
> ---
>  .../devicetree/bindings/pci/brcm,iproc-pcie.txt    | 48 ++++++++++++++++++----
>  1 file changed, 41 insertions(+), 7 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Lorenzo Pieralisi Oct. 16, 2019, 10:16 a.m. UTC | #2
On Wed, Aug 28, 2019 at 02:24:43PM +0530, Srinath Mannam wrote:
> From: Ray Jui <ray.jui@broadcom.com>
> 
> Update the iProc PCIe binding document for better modeling of the legacy
> interrupt (INTx) support
> 
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
> ---
>  .../devicetree/bindings/pci/brcm,iproc-pcie.txt    | 48 ++++++++++++++++++----
>  1 file changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
> index df065aa..f23decb 100644
> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
> @@ -13,9 +13,6 @@ controller, used in Stingray
>    PAXB-based root complex is used for external endpoint devices. PAXC-based
>  root complex is connected to emulated endpoint devices internal to the ASIC
>  - reg: base address and length of the PCIe controller I/O register space
> -- #interrupt-cells: set to <1>
> -- interrupt-map-mask and interrupt-map, standard PCI properties to define the
> -  mapping of the PCIe interface to interrupt numbers
>  - linux,pci-domain: PCI domain ID. Should be unique for each host controller
>  - bus-range: PCI bus numbers covered
>  - #address-cells: set to <3>
> @@ -41,6 +38,21 @@ Required:
>  - brcm,pcie-ob-axi-offset: The offset from the AXI address to the internal
>  address used by the iProc PCIe core (not the PCIe address)
>  
> +Legacy interrupt (INTx) support (optional):
> +
> +Note INTx is for PAXB only.
> +- interrupt-map-mask and interrupt-map, standard PCI properties to define
> +the mapping of the PCIe interface to interrupt numbers
> +
> +In addition, a sub-node that describes the legacy interrupt controller built
> +into the PCIe controller.
> +This sub-node must have the following properties:
> + - compatible: must be "brcm,iproc-intc"
> + - interrupt-controller: claims itself as an interrupt controller for INTx
> + - #interrupt-cells: set to <1>
> + - interrupts: interrupt line wired to the generic GIC for INTx support
> + - interrupt-parent: Phandle to the parent interrupt controller
> +
>  MSI support (optional):
>  
>  For older platforms without MSI integrated in the GIC, iProc PCIe core provides
> @@ -77,8 +89,11 @@ Example:
>  		reg = <0x18012000 0x1000>;
>  
>  		#interrupt-cells = <1>;
> -		interrupt-map-mask = <0 0 0 0>;
> -		interrupt-map = <0 0 0 0 &gic GIC_SPI 100 IRQ_TYPE_NONE>;
> +		interrupt-map-mask = <0 0 0 7>;
> +		interrupt-map = <0 0 0 1 &pcie0_intc 1>,
> +				<0 0 0 2 &pcie0_intc 2>,
> +				<0 0 0 3 &pcie0_intc 3>,
> +				<0 0 0 4 &pcie0_intc 4>;

This is not how the interrupt controller works in your PCI host
bridge.

You are mapping INT{A,B,C,D} to pin 0,1,2,3 of the interrupt
controller.

This is how it is meant to be (which is also removing the completely
bogus need for the (+1) in the irq domain allocation (ie the domain
size is PCI_NUM_INTX not (PCI_NUM_INTX + 1))):

interrupt-map = <0 0 0 1 &pcie0_intc 0>,
		<0 0 0 2 &pcie0_intc 1>,
		<0 0 0 3 &pcie0_intc 2>,
		<0 0 0 4 &pcie0_intc 3>;

We need to write common bindings and kernel code to deal with these PCI
host controller interrupt controllers they are almost all implemented
wrongly in the kernel and copy and paste does the rest.

The IRQ domain subsequent patch is wrong too, please have a look
at how:

drivers/pci/controller/pci-ftpci100.c

models the legacy IRQ domain and follow it.
>  
>  		linux,pci-domain = <0>;
>  
> @@ -98,6 +113,14 @@ Example:
>  
>  		msi-parent = <&msi0>;
>  
> +		pcie0_intc: interrupt-controller {
> +			compatible = "brcm,iproc-intc";
> +			interrupt-controller;
> +			#interrupt-cells = <1>;
> +			interrupt-parent = <&gic>;
> +			interrupts = <GIC_SPI 100 IRQ_TYPE_NONE>;
> +		};
> +
>  		/* iProc event queue based MSI */
>  		msi0: msi@18012000 {
>  			compatible = "brcm,iproc-msi";
> @@ -115,8 +138,11 @@ Example:
>  		reg = <0x18013000 0x1000>;
>  
>  		#interrupt-cells = <1>;
> -		interrupt-map-mask = <0 0 0 0>;
> -		interrupt-map = <0 0 0 0 &gic GIC_SPI 106 IRQ_TYPE_NONE>;
> +		interrupt-map-mask = <0 0 0 7>;
> +		interrupt-map = <0 0 0 1 &pcie1_intc 1>,
> +				<0 0 0 2 &pcie1_intc 2>,
> +				<0 0 0 3 &pcie1_intc 3>,
> +				<0 0 0 4 &pcie1_intc 4>;
>  
>  		linux,pci-domain = <1>;
>  
> @@ -130,4 +156,12 @@ Example:
>  
>  		phys = <&phy 1 6>;
>  		phy-names = "pcie-phy";
> +
> +		pcie1_intc: interrupt-controller {
> +			compatible = "brcm,iproc-intc";
> +			interrupt-controller;
> +			#interrupt-cells = <1>;
> +			interrupt-parent = <&gic>;
> +			interrupts = <GIC_SPI 106 IRQ_TYPE_NONE>;
> +		};
>  	};
> -- 
> 2.7.4
>

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
index df065aa..f23decb 100644
--- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.txt
@@ -13,9 +13,6 @@  controller, used in Stingray
   PAXB-based root complex is used for external endpoint devices. PAXC-based
 root complex is connected to emulated endpoint devices internal to the ASIC
 - reg: base address and length of the PCIe controller I/O register space
-- #interrupt-cells: set to <1>
-- interrupt-map-mask and interrupt-map, standard PCI properties to define the
-  mapping of the PCIe interface to interrupt numbers
 - linux,pci-domain: PCI domain ID. Should be unique for each host controller
 - bus-range: PCI bus numbers covered
 - #address-cells: set to <3>
@@ -41,6 +38,21 @@  Required:
 - brcm,pcie-ob-axi-offset: The offset from the AXI address to the internal
 address used by the iProc PCIe core (not the PCIe address)
 
+Legacy interrupt (INTx) support (optional):
+
+Note INTx is for PAXB only.
+- interrupt-map-mask and interrupt-map, standard PCI properties to define
+the mapping of the PCIe interface to interrupt numbers
+
+In addition, a sub-node that describes the legacy interrupt controller built
+into the PCIe controller.
+This sub-node must have the following properties:
+ - compatible: must be "brcm,iproc-intc"
+ - interrupt-controller: claims itself as an interrupt controller for INTx
+ - #interrupt-cells: set to <1>
+ - interrupts: interrupt line wired to the generic GIC for INTx support
+ - interrupt-parent: Phandle to the parent interrupt controller
+
 MSI support (optional):
 
 For older platforms without MSI integrated in the GIC, iProc PCIe core provides
@@ -77,8 +89,11 @@  Example:
 		reg = <0x18012000 0x1000>;
 
 		#interrupt-cells = <1>;
-		interrupt-map-mask = <0 0 0 0>;
-		interrupt-map = <0 0 0 0 &gic GIC_SPI 100 IRQ_TYPE_NONE>;
+		interrupt-map-mask = <0 0 0 7>;
+		interrupt-map = <0 0 0 1 &pcie0_intc 1>,
+				<0 0 0 2 &pcie0_intc 2>,
+				<0 0 0 3 &pcie0_intc 3>,
+				<0 0 0 4 &pcie0_intc 4>;
 
 		linux,pci-domain = <0>;
 
@@ -98,6 +113,14 @@  Example:
 
 		msi-parent = <&msi0>;
 
+		pcie0_intc: interrupt-controller {
+			compatible = "brcm,iproc-intc";
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			interrupt-parent = <&gic>;
+			interrupts = <GIC_SPI 100 IRQ_TYPE_NONE>;
+		};
+
 		/* iProc event queue based MSI */
 		msi0: msi@18012000 {
 			compatible = "brcm,iproc-msi";
@@ -115,8 +138,11 @@  Example:
 		reg = <0x18013000 0x1000>;
 
 		#interrupt-cells = <1>;
-		interrupt-map-mask = <0 0 0 0>;
-		interrupt-map = <0 0 0 0 &gic GIC_SPI 106 IRQ_TYPE_NONE>;
+		interrupt-map-mask = <0 0 0 7>;
+		interrupt-map = <0 0 0 1 &pcie1_intc 1>,
+				<0 0 0 2 &pcie1_intc 2>,
+				<0 0 0 3 &pcie1_intc 3>,
+				<0 0 0 4 &pcie1_intc 4>;
 
 		linux,pci-domain = <1>;
 
@@ -130,4 +156,12 @@  Example:
 
 		phys = <&phy 1 6>;
 		phy-names = "pcie-phy";
+
+		pcie1_intc: interrupt-controller {
+			compatible = "brcm,iproc-intc";
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			interrupt-parent = <&gic>;
+			interrupts = <GIC_SPI 106 IRQ_TYPE_NONE>;
+		};
 	};