[PATCH 0/3] PCIe controller driver for Marvell Armada 3700

[PATCH 0/3] PCIe controller driver for Marvell Armada 3700

[Thomas Petazzoni]

Hello,

This small patch series adds a new driver for the PCIe controller
found on Marvell Armada 3700 ARM64 SoCs. Like any other new driver
submission, this patch series includes: a Device Tree binding
documentation patch, a patch adding the driver itself, and a patch
adding the Device Tree description of the hardware.

Thanks in advance for your reviews!

Thomas

Thomas Petazzoni (3):
  dt-bindings: add DT binding for the Aardvark PCIe controller
  PCI: host: new PCI host controller driver for Marvell Armada 3700
  arm64: dts: marvell: PCIe support for Armada 3700

 .../devicetree/bindings/pci/aardvark-pci.txt       |   51 +
 MAINTAINERS                                        |    7 +
 arch/arm64/boot/dts/marvell/armada-3720-db.dts     |    5 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi       |   23 +
 drivers/pci/host/Kconfig                           |    9 +
 drivers/pci/host/Makefile                          |    1 +
 drivers/pci/host/pci-aardvark.c                    | 1029 ++++++++++++++++++++
 7 files changed, 1125 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/aardvark-pci.txt
 create mode 100644 drivers/pci/host/pci-aardvark.c

-- 
2.7.4

[PATCH 0/3] PCIe controller driver for Marvell Armada 3700

[Thomas Petazzoni]

Hello,

This small patch series adds a new driver for the PCIe controller
found on Marvell Armada 3700 ARM64 SoCs. Like any other new driver
submission, this patch series includes: a Device Tree binding
documentation patch, a patch adding the driver itself, and a patch
adding the Device Tree description of the hardware.

Thanks in advance for your reviews!

Thomas

Thomas Petazzoni (3):
  dt-bindings: add DT binding for the Aardvark PCIe controller
  PCI: host: new PCI host controller driver for Marvell Armada 3700
  arm64: dts: marvell: PCIe support for Armada 3700

 .../devicetree/bindings/pci/aardvark-pci.txt       |   51 +
 MAINTAINERS                                        |    7 +
 arch/arm64/boot/dts/marvell/armada-3720-db.dts     |    5 +
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi       |   23 +
 drivers/pci/host/Kconfig                           |    9 +
 drivers/pci/host/Makefile                          |    1 +
 drivers/pci/host/pci-aardvark.c                    | 1029 ++++++++++++++++++++
 7 files changed, 1125 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/aardvark-pci.txt
 create mode 100644 drivers/pci/host/pci-aardvark.c

-- 
2.7.4

[PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Thomas Petazzoni]

This commit adds the documentation for the Device Tree binding used to
describe the Aardvark PCIe controller, found on Marvell Armada 3700
ARM64 SoCs.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 .../devicetree/bindings/pci/aardvark-pci.txt       | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/aardvark-pci.txt

diff --git a/Documentation/devicetree/bindings/pci/aardvark-pci.txt b/Documentation/devicetree/bindings/pci/aardvark-pci.txt
new file mode 100644
index 0000000..3517234
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/aardvark-pci.txt
@@ -0,0 +1,51 @@
+Aardvark PCIe controller
+
+This PCIe controller is used on the Marvell Armada 3700 ARM64 SoC.
+
+The Device Tree node describing an Aardvark PCIe controller must
+contain the following properties:
+
+ - compatible: Should be "marvell,armada-3700-pcie"
+ - reg: range of registers for the PCIe controller
+ - interrupts: the interrupt line of the PCIe controller
+ - #address-cells: set to <3>
+ - #size-cells: set to <2>
+ - device_type: set to "pci"
+ - ranges: ranges for the PCI memory and I/O regions
+ - #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.
+ - bus-range: PCI bus numbers covered
+
+In addition, the Device Tree describing an Aardvark PCIe controller
+must include a sub-node that describes the legacy interrupt controller
+built into the PCIe controller. This sub-node must have the following
+properties:
+
+ - interrupt-controller
+ - #interrupt-cells: set to <1>
+
+Example:
+
+	pcie0: pcie@d0070000 {
+		compatible = "marvell,armada-3700-pcie";
+		device_type = "pci";
+		status = "disabled";
+		reg = <0 0xd0070000 0 0x20000>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		bus-range = <0x00 0xff>;
+		interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+		#interrupt-cells = <1>;
+		ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
+			  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
+		interrupt-map-mask = <0 0 0 7>;
+		interrupt-map = <0 0 0 1 &pcie_intc 0>,
+				<0 0 0 2 &pcie_intc 1>,
+				<0 0 0 3 &pcie_intc 2>,
+				<0 0 0 4 &pcie_intc 3>;
+		pcie_intc: interrupt-controller {
+			interrupt-controller;
+			#interrupt-cells = <1>;
+		};
+	};
-- 
2.7.4

[PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Thomas Petazzoni]

This commit adds the documentation for the Device Tree binding used to
describe the Aardvark PCIe controller, found on Marvell Armada 3700
ARM64 SoCs.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 .../devicetree/bindings/pci/aardvark-pci.txt       | 51 ++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/aardvark-pci.txt

diff --git a/Documentation/devicetree/bindings/pci/aardvark-pci.txt b/Documentation/devicetree/bindings/pci/aardvark-pci.txt
new file mode 100644
index 0000000..3517234
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/aardvark-pci.txt
@@ -0,0 +1,51 @@
+Aardvark PCIe controller
+
+This PCIe controller is used on the Marvell Armada 3700 ARM64 SoC.
+
+The Device Tree node describing an Aardvark PCIe controller must
+contain the following properties:
+
+ - compatible: Should be "marvell,armada-3700-pcie"
+ - reg: range of registers for the PCIe controller
+ - interrupts: the interrupt line of the PCIe controller
+ - #address-cells: set to <3>
+ - #size-cells: set to <2>
+ - device_type: set to "pci"
+ - ranges: ranges for the PCI memory and I/O regions
+ - #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.
+ - bus-range: PCI bus numbers covered
+
+In addition, the Device Tree describing an Aardvark PCIe controller
+must include a sub-node that describes the legacy interrupt controller
+built into the PCIe controller. This sub-node must have the following
+properties:
+
+ - interrupt-controller
+ - #interrupt-cells: set to <1>
+
+Example:
+
+	pcie0: pcie at d0070000 {
+		compatible = "marvell,armada-3700-pcie";
+		device_type = "pci";
+		status = "disabled";
+		reg = <0 0xd0070000 0 0x20000>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		bus-range = <0x00 0xff>;
+		interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+		#interrupt-cells = <1>;
+		ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
+			  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
+		interrupt-map-mask = <0 0 0 7>;
+		interrupt-map = <0 0 0 1 &pcie_intc 0>,
+				<0 0 0 2 &pcie_intc 1>,
+				<0 0 0 3 &pcie_intc 2>,
+				<0 0 0 4 &pcie_intc 3>;
+		pcie_intc: interrupt-controller {
+			interrupt-controller;
+			#interrupt-cells = <1>;
+		};
+	};
-- 
2.7.4

Re: [PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Arnd Bergmann]

On Thursday, June 2, 2016 11:09:43 AM CEST Thomas Petazzoni wrote:
> +               ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> +                         0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> 

Any reason for not having a 64-bit MEM prefetchable area in the example?
Does the host not support that?

	Arnd

[PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Arnd Bergmann]

On Thursday, June 2, 2016 11:09:43 AM CEST Thomas Petazzoni wrote:
> +               ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> +                         0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> 

Any reason for not having a 64-bit MEM prefetchable area in the example?
Does the host not support that?

	Arnd

Re: [PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Thomas Petazzoni]

Hello,

Thanks for your review!

On Thu, 02 Jun 2016 11:35:38 +0200, Arnd Bergmann wrote:
> On Thursday, June 2, 2016 11:09:43 AM CEST Thomas Petazzoni wrote:
> > +               ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > +                         0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> >   
> 
> Any reason for not having a 64-bit MEM prefetchable area in the example?
> Does the host not support that?

I'll have to admit I am not sure how to find this out from the
datasheet. My datasheet says about the PCIe controller:

"""
64-bit PCIe address and system address space for outbound transactions
"""

So I guess this would indicate that a 64-bit MEM area is possible.
However, since anyway the area used above is at 0xe8000000 for a length
of 0x1000000, what would be the benefit of declaring this range as a
64-bit one ?

Regarding the prefetchable aspect, I couldn't find any reference in the
datasheet. However, the original driver code explicitly errors out if
there is no non-prefetchable memory area, so I guess prefetchable
areas is not supported.

In of_bus_pci_get_flags(), both the 32-bit and 64-bit cases are handled
in the same way, so is this distinction actually being used by the
kernel?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Thomas Petazzoni]

Hello,

Thanks for your review!

On Thu, 02 Jun 2016 11:35:38 +0200, Arnd Bergmann wrote:
> On Thursday, June 2, 2016 11:09:43 AM CEST Thomas Petazzoni wrote:
> > +               ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > +                         0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> >   
> 
> Any reason for not having a 64-bit MEM prefetchable area in the example?
> Does the host not support that?

I'll have to admit I am not sure how to find this out from the
datasheet. My datasheet says about the PCIe controller:

"""
64-bit PCIe address and system address space for outbound transactions
"""

So I guess this would indicate that a 64-bit MEM area is possible.
However, since anyway the area used above is at 0xe8000000 for a length
of 0x1000000, what would be the benefit of declaring this range as a
64-bit one ?

Regarding the prefetchable aspect, I couldn't find any reference in the
datasheet. However, the original driver code explicitly errors out if
there is no non-prefetchable memory area, so I guess prefetchable
areas is not supported.

In of_bus_pci_get_flags(), both the 32-bit and 64-bit cases are handled
in the same way, so is this distinction actually being used by the
kernel?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Arnd Bergmann]

On Wednesday, June 8, 2016 4:27:50 PM CEST Thomas Petazzoni wrote:
> Hello,
> 
> Thanks for your review!
> 
> On Thu, 02 Jun 2016 11:35:38 +0200, Arnd Bergmann wrote:
> > On Thursday, June 2, 2016 11:09:43 AM CEST Thomas Petazzoni wrote:
> > > +               ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > > +                         0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> > >   
> > 
> > Any reason for not having a 64-bit MEM prefetchable area in the example?
> > Does the host not support that?
> 
> I'll have to admit I am not sure how to find this out from the
> datasheet. My datasheet says about the PCIe controller:
> 
> """
> 64-bit PCIe address and system address space for outbound transactions
> """
> 
> So I guess this would indicate that a 64-bit MEM area is possible.
> However, since anyway the area used above is at 0xe8000000 for a length
> of 0x1000000, what would be the benefit of declaring this range as a
> 64-bit one ?
> 
> Regarding the prefetchable aspect, I couldn't find any reference in the
> datasheet. However, the original driver code explicitly errors out if
> there is no non-prefetchable memory area, so I guess prefetchable
> areas is not supported.
> 
> In of_bus_pci_get_flags(), both the 32-bit and 64-bit cases are handled
> in the same way, so is this distinction actually being used by the
> kernel?

Each device needs to have at least one non-prefetcheable range, which
is why some drivers check for that. Non-prefetchable BARs always use
32-bit addressing, so 64-bit BARs are by definition prefetchable,
but you can also have prefetchable registers in the first 4GB in some
cases.

Some devices require large prefetchable BARs (hundreds of MB, or more),
so if you have the option, you should enable that in the host, as the
16MB you have available for non-prefetchable devices is not going to be
sufficient.

	Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Arnd Bergmann]

On Wednesday, June 8, 2016 4:27:50 PM CEST Thomas Petazzoni wrote:
> Hello,
> 
> Thanks for your review!
> 
> On Thu, 02 Jun 2016 11:35:38 +0200, Arnd Bergmann wrote:
> > On Thursday, June 2, 2016 11:09:43 AM CEST Thomas Petazzoni wrote:
> > > +               ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> > > +                         0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> > >   
> > 
> > Any reason for not having a 64-bit MEM prefetchable area in the example?
> > Does the host not support that?
> 
> I'll have to admit I am not sure how to find this out from the
> datasheet. My datasheet says about the PCIe controller:
> 
> """
> 64-bit PCIe address and system address space for outbound transactions
> """
> 
> So I guess this would indicate that a 64-bit MEM area is possible.
> However, since anyway the area used above is at 0xe8000000 for a length
> of 0x1000000, what would be the benefit of declaring this range as a
> 64-bit one ?
> 
> Regarding the prefetchable aspect, I couldn't find any reference in the
> datasheet. However, the original driver code explicitly errors out if
> there is no non-prefetchable memory area, so I guess prefetchable
> areas is not supported.
> 
> In of_bus_pci_get_flags(), both the 32-bit and 64-bit cases are handled
> in the same way, so is this distinction actually being used by the
> kernel?

Each device needs to have at least one non-prefetcheable range, which
is why some drivers check for that. Non-prefetchable BARs always use
32-bit addressing, so 64-bit BARs are by definition prefetchable,
but you can also have prefetchable registers in the first 4GB in some
cases.

Some devices require large prefetchable BARs (hundreds of MB, or more),
so if you have the option, you should enable that in the host, as the
16MB you have available for non-prefetchable devices is not going to be
sufficient.

	Arnd

Re: [PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Andrew Lunn]

> +In addition, the Device Tree describing an Aardvark PCIe controller
> +must include a sub-node that describes the legacy interrupt controller
> +built into the PCIe controller. This sub-node must have the following
> +properties:
> +
> + - interrupt-controller
> + - #interrupt-cells: set to <1>
> +
> +Example:
> +
> +	pcie0: pcie@d0070000 {
> +		compatible = "marvell,armada-3700-pcie";
> +		device_type = "pci";
> +		status = "disabled";
> +		reg = <0 0xd0070000 0 0x20000>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		bus-range = <0x00 0xff>;
> +		interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
> +		#interrupt-cells = <1>;
> +		ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> +			  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> +		interrupt-map-mask = <0 0 0 7>;
> +		interrupt-map = <0 0 0 1 &pcie_intc 0>,
> +				<0 0 0 2 &pcie_intc 1>,
> +				<0 0 0 3 &pcie_intc 2>,
> +				<0 0 0 4 &pcie_intc 3>;
> +		pcie_intc: interrupt-controller {
> +			interrupt-controller;
> +			#interrupt-cells = <1>;
> +		};

Hi Thomas

It is possible to list PCIe devices on the bus here as child
nodes. I've done this when i needed a phandle to an intel ethernet
controller on the PCIe bus, which i know is soldered onto the board.

I think your current implementation simply uses the first child
node. It would be good to document that ordering is important. It must
be the first child node, and any pcie devices children must come
afterwards.

      Andrew

[PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Andrew Lunn]

> +In addition, the Device Tree describing an Aardvark PCIe controller
> +must include a sub-node that describes the legacy interrupt controller
> +built into the PCIe controller. This sub-node must have the following
> +properties:
> +
> + - interrupt-controller
> + - #interrupt-cells: set to <1>
> +
> +Example:
> +
> +	pcie0: pcie at d0070000 {
> +		compatible = "marvell,armada-3700-pcie";
> +		device_type = "pci";
> +		status = "disabled";
> +		reg = <0 0xd0070000 0 0x20000>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		bus-range = <0x00 0xff>;
> +		interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
> +		#interrupt-cells = <1>;
> +		ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
> +			  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
> +		interrupt-map-mask = <0 0 0 7>;
> +		interrupt-map = <0 0 0 1 &pcie_intc 0>,
> +				<0 0 0 2 &pcie_intc 1>,
> +				<0 0 0 3 &pcie_intc 2>,
> +				<0 0 0 4 &pcie_intc 3>;
> +		pcie_intc: interrupt-controller {
> +			interrupt-controller;
> +			#interrupt-cells = <1>;
> +		};

Hi Thomas

It is possible to list PCIe devices on the bus here as child
nodes. I've done this when i needed a phandle to an intel ethernet
controller on the PCIe bus, which i know is soldered onto the board.

I think your current implementation simply uses the first child
node. It would be good to document that ordering is important. It must
be the first child node, and any pcie devices children must come
afterwards.

      Andrew

Re: [PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Arnd Bergmann]

On Thursday, June 2, 2016 2:24:05 PM CEST Andrew Lunn wrote:
> 
> It is possible to list PCIe devices on the bus here as child
> nodes. I've done this when i needed a phandle to an intel ethernet
> controller on the PCIe bus, which i know is soldered onto the board.
> 
> I think your current implementation simply uses the first child
> node. It would be good to document that ordering is important. It must
> be the first child node, and any pcie devices children must come
> afterwards.

I think some other PCI hosts just move the interrupt-controller
and #interrupt-cells properties into the PCI host node itself,
which avoids the ambiguity here.

	Arnd

[PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Arnd Bergmann]

On Thursday, June 2, 2016 2:24:05 PM CEST Andrew Lunn wrote:
> 
> It is possible to list PCIe devices on the bus here as child
> nodes. I've done this when i needed a phandle to an intel ethernet
> controller on the PCIe bus, which i know is soldered onto the board.
> 
> I think your current implementation simply uses the first child
> node. It would be good to document that ordering is important. It must
> be the first child node, and any pcie devices children must come
> afterwards.

I think some other PCI hosts just move the interrupt-controller
and #interrupt-cells properties into the PCI host node itself,
which avoids the ambiguity here.

	Arnd

Re: [PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Thomas Petazzoni]

Hello,

On Thu, 02 Jun 2016 14:34:14 +0200, Arnd Bergmann wrote:

> I think some other PCI hosts just move the interrupt-controller
> and #interrupt-cells properties into the PCI host node itself,
> which avoids the ambiguity here.

Do you have an example of this? I have modeled my DT binding after the
one used by the pci-dra7xx driver, which is in the same situation as I
am in terms of interrupt handling.

But I can indeed try to make the top-level PCIe controller node the
interrupt controller. Let me try that quickly.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Thomas Petazzoni]

Hello,

On Thu, 02 Jun 2016 14:34:14 +0200, Arnd Bergmann wrote:

> I think some other PCI hosts just move the interrupt-controller
> and #interrupt-cells properties into the PCI host node itself,
> which avoids the ambiguity here.

Do you have an example of this? I have modeled my DT binding after the
one used by the pci-dra7xx driver, which is in the same situation as I
am in terms of interrupt handling.

But I can indeed try to make the top-level PCIe controller node the
interrupt controller. Let me try that quickly.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Arnd Bergmann]

On Thursday, June 2, 2016 2:45:42 PM CEST Thomas Petazzoni wrote:
> 
> On Thu, 02 Jun 2016 14:34:14 +0200, Arnd Bergmann wrote:
> 
> > I think some other PCI hosts just move the interrupt-controller
> > and #interrupt-cells properties into the PCI host node itself,
> > which avoids the ambiguity here.
> 
> Do you have an example of this? I have modeled my DT binding after the
> one used by the pci-dra7xx driver, which is in the same situation as I
> am in terms of interrupt handling.
> 
> But I can indeed try to make the top-level PCIe controller node the
> interrupt controller. Let me try that quickly.
> 

Looking at the binding files, I see only this one:

Documentation/devicetree/bindings/pci/altera-pcie.txt

and a couple of others using a child node.

	Arnd

[PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Arnd Bergmann]

On Thursday, June 2, 2016 2:45:42 PM CEST Thomas Petazzoni wrote:
> 
> On Thu, 02 Jun 2016 14:34:14 +0200, Arnd Bergmann wrote:
> 
> > I think some other PCI hosts just move the interrupt-controller
> > and #interrupt-cells properties into the PCI host node itself,
> > which avoids the ambiguity here.
> 
> Do you have an example of this? I have modeled my DT binding after the
> one used by the pci-dra7xx driver, which is in the same situation as I
> am in terms of interrupt handling.
> 
> But I can indeed try to make the top-level PCIe controller node the
> interrupt controller. Let me try that quickly.
> 

Looking at the binding files, I see only this one:

Documentation/devicetree/bindings/pci/altera-pcie.txt

and a couple of others using a child node.

	Arnd

Re: [PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Thomas Petazzoni]

Hello,

On Thu, 02 Jun 2016 14:34:14 +0200, Arnd Bergmann wrote:

> I think some other PCI hosts just move the interrupt-controller
> and #interrupt-cells properties into the PCI host node itself,
> which avoids the ambiguity here.

I've tried that, and it works fine. Makes the code simpler, the binding
simpler, so: adopted! Thanks for suggesting!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Thomas Petazzoni]

Hello,

On Thu, 02 Jun 2016 14:34:14 +0200, Arnd Bergmann wrote:

> I think some other PCI hosts just move the interrupt-controller
> and #interrupt-cells properties into the PCI host node itself,
> which avoids the ambiguity here.

I've tried that, and it works fine. Makes the code simpler, the binding
simpler, so: adopted! Thanks for suggesting!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Bjorn Helgaas]

On Thu, Jun 02, 2016 at 11:09:43AM +0200, Thomas Petazzoni wrote:
> This commit adds the documentation for the Device Tree binding used to
> describe the Aardvark PCIe controller, found on Marvell Armada 3700
> ARM64 SoCs.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Hi Thomas,

If you post a v2 of these, can you align the subject lines of these
binding, DT, and driver patches so it's easier to find the related
pieces, e.g., add "Armada 3700" to this one or "Aardvark" to the
others?  It looks like maybe Aardvark could be used in several
different systems, so maybe this patch should stay the same and the
drivers/pci patch should mention Aardvark instead of only "Marvell
Armada 3700"?

Bjorn

[PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Bjorn Helgaas]

On Thu, Jun 02, 2016 at 11:09:43AM +0200, Thomas Petazzoni wrote:
> This commit adds the documentation for the Device Tree binding used to
> describe the Aardvark PCIe controller, found on Marvell Armada 3700
> ARM64 SoCs.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Hi Thomas,

If you post a v2 of these, can you align the subject lines of these
binding, DT, and driver patches so it's easier to find the related
pieces, e.g., add "Armada 3700" to this one or "Aardvark" to the
others?  It looks like maybe Aardvark could be used in several
different systems, so maybe this patch should stay the same and the
drivers/pci patch should mention Aardvark instead of only "Marvell
Armada 3700"?

Bjorn

Re: [PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Thomas Petazzoni]

Hello,

On Fri, 10 Jun 2016 10:44:14 -0500, Bjorn Helgaas wrote:

> If you post a v2 of these, can you align the subject lines of these
> binding, DT, and driver patches so it's easier to find the related
> pieces, e.g., add "Armada 3700" to this one or "Aardvark" to the
> others?  It looks like maybe Aardvark could be used in several
> different systems, so maybe this patch should stay the same and the
> drivers/pci patch should mention Aardvark instead of only "Marvell
> Armada 3700"?

Sure thing. I was about to send a v2, so I've fixed up the commit
titles so that they all include Aardvark.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 1/3] dt-bindings: add DT binding for the Aardvark PCIe controller

[Thomas Petazzoni]

Hello,

On Fri, 10 Jun 2016 10:44:14 -0500, Bjorn Helgaas wrote:

> If you post a v2 of these, can you align the subject lines of these
> binding, DT, and driver patches so it's easier to find the related
> pieces, e.g., add "Armada 3700" to this one or "Aardvark" to the
> others?  It looks like maybe Aardvark could be used in several
> different systems, so maybe this patch should stay the same and the
> drivers/pci patch should mention Aardvark instead of only "Marvell
> Armada 3700"?

Sure thing. I was about to send a v2, so I've fixed up the commit
titles so that they all include Aardvark.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Thomas Petazzoni]

This commit adds a new driver for a PCIe controller called Aardvark,
which is used on the Marvell Armada 3700 ARM64 SoC.

This patch is based on work done by Hezi Shahmoon
<hezi.shahmoon@marvell.com> and Marcin Wojtas <mw@semihalf.com>.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 MAINTAINERS                     |    7 +
 drivers/pci/host/Kconfig        |    9 +
 drivers/pci/host/Makefile       |    1 +
 drivers/pci/host/pci-aardvark.c | 1029 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 1046 insertions(+)
 create mode 100644 drivers/pci/host/pci-aardvark.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7304d2e..373215c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8740,6 +8740,13 @@ L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 F:	drivers/pci/host/*mvebu*
 
+PCI DRIVER FOR AARDVARK (Marvell Armada 3700)
+M:	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+L:	linux-pci@vger.kernel.org
+L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
+S:	Maintained
+F:	drivers/pci/host/pci-aardvark.c
+
 PCI DRIVER FOR NVIDIA TEGRA
 M:	Thierry Reding <thierry.reding@gmail.com>
 L:	linux-tegra@vger.kernel.org
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 5d2374e..5f085fd 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -16,6 +16,15 @@ config PCI_MVEBU
 	depends on ARM
 	depends on OF
 
+config PCI_AARDVARK
+	bool "Aardvark PCIe controller"
+	depends on ARCH_MVEBU && ARM64
+	depends on OF
+	select PCI_MSI
+	help
+	 Add support for Aardvark 64bit PCIe Host Controller. This
+	 controller is part of the South Bridge of the Marvel Armada
+	 3700 SoC.
 
 config PCIE_XILINX_NWL
 	bool "NWL PCIe Core"
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 9c8698e..66f45b6 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
 obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
 obj-$(CONFIG_PCI_HYPERV) += pci-hyperv.o
 obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
+obj-$(CONFIG_PCI_AARDVARK) += pci-aardvark.o
 obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
 obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
 obj-$(CONFIG_PCIE_RCAR) += pcie-rcar.o
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c
new file mode 100644
index 0000000..c204bc0
--- /dev/null
+++ b/drivers/pci/host/pci-aardvark.c
@@ -0,0 +1,1029 @@
+/*
+ * Driver for the Aardvark PCIe controller, used on Marvell Armada
+ * 3700.
+ *
+ * Copyright (C) 2016 Marvell
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/of_pci.h>
+
+/* PCIe core registers */
+#define PCIE_CORE_CMD_STATUS_REG				0x4
+#define     PCIE_CORE_CMD_IO_ACCESS_EN				BIT(0)
+#define     PCIE_CORE_CMD_MEM_ACCESS_EN				BIT(1)
+#define     PCIE_CORE_CMD_MEM_IO_REQ_EN				BIT(2)
+#define PCIE_CORE_DEV_CTRL_STATS_REG				0xC8
+#define     PCIE_CORE_DEV_CTRL_STATS_RELAX_ORDER_DISABLE	(0 << 4)
+#define     PCIE_CORE_DEV_CTRL_STATS_MAX_PAYLOAD_SZ_SHIFT	5
+#define     PCIE_CORE_DEV_CTRL_STATS_SNOOP_DISABLE		(0 << 11)
+#define     PCIE_CORE_DEV_CTRL_STATS_MAX_RD_REQ_SIZE_SHIFT	12
+#define PCIE_CORE_LINK_CTRL_STAT_REG				0xD0
+#define     PCIE_CORE_LINK_L0S_ENTRY				BIT(0)
+#define     PCIE_CORE_LINK_TRAINING				BIT(5)
+#define     PCIE_CORE_LINK_WIDTH_SHIFT				20
+#define PCIE_CORE_ERR_CAPCTL_REG				0x118
+#define     PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX			BIT(5)
+#define     PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX_EN			BIT(6)
+#define     PCIE_CORE_ERR_CAPCTL_ECRC_CHCK			BIT(7)
+#define     PCIE_CORE_ERR_CAPCTL_ECRC_CHCK_RCV			BIT(8)
+
+/* PIO registers base address and register offsets */
+#define PIO_BASE_ADDR				0x4000
+#define PIO_CTRL				(PIO_BASE_ADDR + 0x0)
+#define PIO_STAT				(PIO_BASE_ADDR + 0x4)
+#define   PIO_COMPLETION_STATUS_SHIFT		7
+#define   PIO_COMPLETION_STATUS_MASK		GENMASK(9, 7)
+#define   PIO_COMPLETION_STATUS_OK		0
+#define   PIO_COMPLETION_STATUS_UR		1
+#define   PIO_COMPLETION_STATUS_CRS		2
+#define   PIO_COMPLETION_STATUS_CA		4
+#define   PIO_NON_POSTED_REQ			BIT(0)
+#define PIO_ADDR_LS				(PIO_BASE_ADDR + 0x8)
+#define PIO_ADDR_MS				(PIO_BASE_ADDR + 0xc)
+#define PIO_WR_DATA				(PIO_BASE_ADDR + 0x10)
+#define PIO_WR_DATA_STRB			(PIO_BASE_ADDR + 0x14)
+#define PIO_RD_DATA				(PIO_BASE_ADDR + 0x18)
+#define PIO_START				(PIO_BASE_ADDR + 0x1c)
+#define PIO_ISR					(PIO_BASE_ADDR + 0x20)
+#define PIO_ISRM				(PIO_BASE_ADDR + 0x24)
+
+/* Aardvark Control registers */
+#define CONTROL_BASE_ADDR			0x4800
+#define PCIE_CORE_CTRL0_REG			(CONTROL_BASE_ADDR + 0x0)
+#define     PCIE_GEN_SEL_MSK			0x3
+#define     PCIE_GEN_SEL_SHIFT			0x0
+#define     SPEED_GEN_1				0
+#define     SPEED_GEN_2				1
+#define     SPEED_GEN_3				2
+#define     IS_RC_MSK				1
+#define     IS_RC_SHIFT				2
+#define     LANE_CNT_MSK			0x18
+#define     LANE_CNT_SHIFT			0x3
+#define     LANE_COUNT_1			(0 << LANE_CNT_SHIFT)
+#define     LANE_COUNT_2			(1 << LANE_CNT_SHIFT)
+#define     LANE_COUNT_4			(2 << LANE_CNT_SHIFT)
+#define     LANE_COUNT_8			(3 << LANE_CNT_SHIFT)
+#define     LINK_TRAINING_EN			BIT(6)
+#define     LEGACY_INTA				BIT(28)
+#define     LEGACY_INTB				BIT(29)
+#define     LEGACY_INTC				BIT(30)
+#define     LEGACY_INTD				BIT(31)
+#define PCIE_CORE_CTRL1_REG			(CONTROL_BASE_ADDR + 0x4)
+#define     HOT_RESET_GEN			BIT(0)
+#define PCIE_CORE_CTRL2_REG			(CONTROL_BASE_ADDR + 0x8)
+#define     PCIE_CORE_CTRL2_RESERVED		0x7
+#define     PCIE_CORE_CTRL2_TD_ENABLE		BIT(4)
+#define     PCIE_CORE_CTRL2_STRICT_ORDER_ENABLE	BIT(5)
+#define     PCIE_CORE_CTRL2_OB_WIN_ENABLE	BIT(6)
+#define     PCIE_CORE_CTRL2_MSI_ENABLE		BIT(10)
+#define PCIE_ISR0_REG				(CONTROL_BASE_ADDR + 0x40)
+#define PCIE_ISR0_MASK_REG			(CONTROL_BASE_ADDR + 0x44)
+#define     PCIE_ISR0_FLR_INT			BIT(26)
+#define     PCIE_ISR0_MSG_LTR			BIT(25)
+#define     PCIE_ISR0_MSI_INT_PENDING		BIT(24)
+#define     PCIE_ISR0_INTD_DEASSERT		BIT(23)
+#define     PCIE_ISR0_INTC_DEASSERT		BIT(22)
+#define     PCIE_ISR0_INTB_DEASSERT		BIT(21)
+#define     PCIE_ISR0_INTA_DEASSERT		BIT(20)
+#define     PCIE_ISR0_INTD_ASSERT		BIT(19)
+#define     PCIE_ISR0_INTC_ASSERT		BIT(18)
+#define     PCIE_ISR0_INTB_ASSERT		BIT(17)
+#define     PCIE_ISR0_INTA_ASSERT		BIT(16)
+#define     PCIE_ISR0_FAT_ERR			BIT(13)
+#define     PCIE_ISR0_NFAT_ERR			BIT(12)
+#define     PCIE_ISR0_CORR_ERR			BIT(11)
+#define     PCIE_ISR0_LMI_LOCAL_INT		BIT(10)
+#define     PCIE_ISR0_LEGACY_INT_SENT		BIT(9)
+#define     PCIE_ISR0_MSG_PM_ACTIVE_STATE_NAK	BIT(8)
+#define     PCIE_ISR0_MSG_PM_PME		BIT(7)
+#define     PCIE_ISR0_MSG_PM_TURN_OFF		BIT(6)
+#define     PCIE_ISR0_MSG_PME_TO_ACK		BIT(5)
+#define     PCIE_ISR0_INB_DP_FERR_PERR_IRQ	BIT(4)
+#define     PCIE_ISR0_OUTB_DP_FERR_PERR_IRQ	BIT(3)
+#define     PCIE_ISR0_INBOUND_MSG		BIT(2)
+#define     PCIE_ISR0_LINK_DOWN			BIT(1)
+#define     PCIE_ISR0_HOT_RESET			BIT(0)
+#define     PCIE_ISR0_INTX_ASSERT(val)		BIT(16 + (val))
+#define     PCIE_ISR0_INTX_DEASSERT(val)	BIT(20 + (val))
+#define     PCIE_ISR0_INTX_MASK			GENMASK(23, 16)
+#define	    PCIE_ISR0_ALL_MASK			GENMASK(26, 0)
+#define PCIE_ISR1_REG				(CONTROL_BASE_ADDR + 0x48)
+#define PCIE_ISR1_MASK_REG			(CONTROL_BASE_ADDR + 0x4C)
+#define     PCIE_ISR1_POWER_STATE_CHANGE	BIT(4)
+#define     PCIE_ISR1_FLUSH			BIT(5)
+#define     PCIE_ISR1_ALL_MASK			GENMASK(5, 4)
+#define PCIE_MSI_ADDR_LOW_REG			(CONTROL_BASE_ADDR + 0x50)
+#define PCIE_MSI_ADDR_HIGH_REG			(CONTROL_BASE_ADDR + 0x54)
+#define PCIE_MSI_STATUS_REG			(CONTROL_BASE_ADDR + 0x58)
+#define PCIE_MSI_MASK_REG			(CONTROL_BASE_ADDR + 0x5C)
+#define PCIE_MSI_PAYLOAD_REG			(CONTROL_BASE_ADDR + 0x9C)
+
+/* PCIe window configuration */
+#define OB_WIN_BASE_ADDR			0x4c00
+#define OB_WIN_BLOCK_SIZE			0x20
+#define OB_WIN_REG_ADDR(win, offset)		(OB_WIN_BASE_ADDR + \
+						 OB_WIN_BLOCK_SIZE * (win) + \
+						 (offset))
+#define OB_WIN_MATCH_LS(win)			OB_WIN_REG_ADDR(win, 0x00)
+#define OB_WIN_MATCH_MS(win)			OB_WIN_REG_ADDR(win, 0x04)
+#define OB_WIN_REMAP_LS(win)			OB_WIN_REG_ADDR(win, 0x08)
+#define OB_WIN_REMAP_MS(win)			OB_WIN_REG_ADDR(win, 0x0c)
+#define OB_WIN_MASK_LS(win)			OB_WIN_REG_ADDR(win, 0x10)
+#define OB_WIN_MASK_MS(win)			OB_WIN_REG_ADDR(win, 0x14)
+#define OB_WIN_ACTIONS(win)			OB_WIN_REG_ADDR(win, 0x18)
+
+/* PCIe window types */
+#define     OB_PCIE_MEM				0x0
+#define     OB_PCIE_IO				0x4
+#define     OB_PCIE_CONFIG0			0x8
+#define     OB_PCIE_CONFIG1			0x9
+#define     OB_PCIE_MSG				0xc
+#define     OB_PCIE_MSG_VENDOR			0xd
+
+/* LMI registers base address and register offsets */
+#define LMI_BASE_ADDR				0x6000
+#define CFG_REG					(LMI_BASE_ADDR + 0x0)
+#define     LTSSM_SHIFT				24
+#define     LTSSM_MASK				0x3f
+#define     LTSSM_L0				0x10
+#define     RC_BAR_CONFIG			0x300
+
+/* PCIe core controller registers */
+#define CTRL_CORE_BASE_ADDR			0x18000
+#define CTRL_CONFIG_REG				(CTRL_CORE_BASE_ADDR + 0x0)
+#define     CTRL_MODE_SHIFT			0x0
+#define     CTRL_MODE_MASK			0x1
+#define     PCIE_CORE_MODE_DIRECT		0x0
+#define     PCIE_CORE_MODE_COMMAND		0x1
+
+/* PCIe Central Interrupts Registers */
+#define CENTRAL_INT_BASE_ADDR			0x1b000
+#define HOST_CTRL_INT_STATUS_REG		(CENTRAL_INT_BASE_ADDR + 0x0)
+#define HOST_CTRL_INT_MASK_REG			(CENTRAL_INT_BASE_ADDR + 0x4)
+#define     PCIE_IRQ_CMDQ_INT			BIT(0)
+#define     PCIE_IRQ_MSI_STATUS_INT		BIT(1)
+#define     PCIE_IRQ_CMD_SENT_DONE		BIT(3)
+#define     PCIE_IRQ_DMA_INT			BIT(4)
+#define     PCIE_IRQ_IB_DXFERDONE		BIT(5)
+#define     PCIE_IRQ_OB_DXFERDONE		BIT(6)
+#define     PCIE_IRQ_OB_RXFERDONE		BIT(7)
+#define     PCIE_IRQ_COMPQ_INT			BIT(12)
+#define     PCIE_IRQ_DIR_RD_DDR_DET		BIT(13)
+#define     PCIE_IRQ_DIR_WR_DDR_DET		BIT(14)
+#define     PCIE_IRQ_CORE_INT			BIT(16)
+#define     PCIE_IRQ_CORE_INT_PIO		BIT(17)
+#define     PCIE_IRQ_DPMU_INT			BIT(18)
+#define     PCIE_IRQ_PCIE_MIS_INT		BIT(19)
+#define     PCIE_IRQ_MSI_INT1_DET		BIT(20)
+#define     PCIE_IRQ_MSI_INT2_DET		BIT(21)
+#define     PCIE_IRQ_RC_DBELL_DET		BIT(22)
+#define     PCIE_IRQ_EP_STATUS			BIT(23)
+#define     PCIE_IRQ_ALL_MASK			0xfff0fb
+#define     PCIE_IRQ_ENABLE_INTS_MASK		PCIE_IRQ_CORE_INT
+
+/* Transaction types */
+#define PCIE_CONFIG_RD_TYPE0			0x8
+#define PCIE_CONFIG_RD_TYPE1			0x9
+#define PCIE_CONFIG_WR_TYPE0			0xa
+#define PCIE_CONFIG_WR_TYPE1			0xb
+
+/* PCI_BDF shifts 8bit, so we need extra 4bit shift */
+#define PCIE_BDF(dev)				(dev << 4)
+#define PCIE_CONF_BUS(bus)			(((bus) & 0xff) << 20)
+#define PCIE_CONF_DEV(dev)			(((dev) & 0x1f) << 15)
+#define PCIE_CONF_FUNC(fun)			(((fun) & 0x7)	<< 12)
+#define PCIE_CONF_REG(reg)			((reg) & 0xffc)
+#define PCIE_CONF_ADDR(bus, devfn, where)	\
+	(PCIE_CONF_BUS(bus) | PCIE_CONF_DEV(PCI_SLOT(devfn))	| \
+	 PCIE_CONF_FUNC(PCI_FUNC(devfn)) | PCIE_CONF_REG(where))
+
+#define PIO_TIMEOUT_NUM			(1000)
+#define PCIE_LINKUP_TIMEOUT		(200)
+
+#define LEGACY_IRQ_NUM			4
+#define MSI_IRQ_NUM			32
+
+struct advk_pcie {
+	struct platform_device *pdev;
+	void __iomem *base;
+	struct list_head resources;
+	struct irq_domain *irq_domain;
+	struct irq_chip irq_chip;
+	struct msi_controller msi;
+	struct irq_domain *msi_domain;
+	struct irq_chip msi_irq_chip;
+	DECLARE_BITMAP(msi_irq_in_use, MSI_IRQ_NUM);
+	struct mutex msi_used_lock;
+	u16 msi_msg;
+};
+
+static inline void advk_writel(struct advk_pcie *pcie, u32 val, u64 reg)
+{
+	writel(val, pcie->base + reg);
+}
+
+static inline u32 advk_readl(struct advk_pcie *pcie, u64 reg)
+{
+	return readl(pcie->base + reg);
+}
+
+static bool advk_pcie_link_up(struct advk_pcie *pcie)
+{
+	int timeout;
+	u32 ltssm_state;
+	u32 val;
+
+	timeout = PCIE_LINKUP_TIMEOUT;
+	do {
+		val = advk_readl(pcie, CFG_REG);
+		ltssm_state = (val >> LTSSM_SHIFT) & LTSSM_MASK;
+		timeout--;
+	/* Use ltssm < LTSSM_L0 instead of ltssm != LTSSM_L0 */
+	} while (ltssm_state < LTSSM_L0 && timeout > 0);
+
+	return (timeout > 0);
+}
+
+/*
+ * Set PCIe address window register which could be used for memory
+ * mapping.
+ */
+static void advk_pcie_set_ob_win(struct advk_pcie *pcie,
+				 u32 win_num, u32 match_ms,
+				 u32 match_ls, u32 mask_ms,
+				 u32 mask_ls, u32 remap_ms,
+				 u32 remap_ls, u32 action)
+{
+	advk_writel(pcie, match_ls, OB_WIN_MATCH_LS(win_num));
+	advk_writel(pcie, match_ms, OB_WIN_MATCH_MS(win_num));
+	advk_writel(pcie, mask_ms, OB_WIN_MASK_MS(win_num));
+	advk_writel(pcie, mask_ls, OB_WIN_MASK_LS(win_num));
+	advk_writel(pcie, remap_ms, OB_WIN_REMAP_MS(win_num));
+	advk_writel(pcie, remap_ls, OB_WIN_REMAP_LS(win_num));
+	advk_writel(pcie, action, OB_WIN_ACTIONS(win_num));
+	advk_writel(pcie, match_ls | BIT(0), OB_WIN_MATCH_LS(win_num));
+}
+
+static void advk_pcie_setup_hw(struct advk_pcie *pcie)
+{
+	u32 reg;
+	int i;
+
+	/* Point PCIe unit MBUS decode windows to DRAM space. */
+	for (i = 0; i < 8; i++)
+		advk_pcie_set_ob_win(pcie, i, 0, 0, 0, 0, 0, 0, 0);
+
+	/* Set to Direct mode. */
+	reg = advk_readl(pcie, CTRL_CONFIG_REG);
+	reg &= ~(CTRL_MODE_MASK << CTRL_MODE_SHIFT);
+	reg |= ((PCIE_CORE_MODE_DIRECT & CTRL_MODE_MASK) << CTRL_MODE_SHIFT);
+	advk_writel(pcie, reg, CTRL_CONFIG_REG);
+
+	/* Set PCI global control register to RC mode */
+	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
+	reg |= (IS_RC_MSK << IS_RC_SHIFT);
+	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
+
+	/*
+	 * Set Advanced Error Capabilities and Control PF0 register
+	 */
+	reg = PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX |
+		PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX_EN |
+		PCIE_CORE_ERR_CAPCTL_ECRC_CHCK |
+		PCIE_CORE_ERR_CAPCTL_ECRC_CHCK_RCV;
+	advk_writel(pcie, reg, PCIE_CORE_ERR_CAPCTL_REG);
+
+	/*
+	 * Set PCIe Device Control and Status 1 PF0 register
+	 */
+	reg = PCIE_CORE_DEV_CTRL_STATS_RELAX_ORDER_DISABLE |
+		(7 << PCIE_CORE_DEV_CTRL_STATS_MAX_PAYLOAD_SZ_SHIFT) |
+		PCIE_CORE_DEV_CTRL_STATS_SNOOP_DISABLE |
+		PCIE_CORE_DEV_CTRL_STATS_MAX_RD_REQ_SIZE_SHIFT;
+	advk_writel(pcie, reg, PCIE_CORE_DEV_CTRL_STATS_REG);
+
+	/*
+	 * Program PCIe Control 2 to disable strict ordering.
+	 */
+	reg = PCIE_CORE_CTRL2_RESERVED |
+		PCIE_CORE_CTRL2_TD_ENABLE;
+	advk_writel(pcie, reg, PCIE_CORE_CTRL2_REG);
+
+	/* Set GEN2 */
+	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
+	reg &= ~PCIE_GEN_SEL_MSK;
+	reg |= SPEED_GEN_2;
+	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
+
+	/* Set lane X1 */
+	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
+	reg &= ~LANE_CNT_MSK;
+	reg |= LANE_COUNT_1;
+	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
+
+	/* Enable link training */
+	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
+	reg |= LINK_TRAINING_EN;
+	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
+
+	/* Enable MSI */
+	reg = advk_readl(pcie, PCIE_CORE_CTRL2_REG);
+	reg |= PCIE_CORE_CTRL2_MSI_ENABLE;
+	advk_writel(pcie, reg, PCIE_CORE_CTRL2_REG);
+
+	/* Clear all interrupts. */
+	advk_writel(pcie, PCIE_ISR0_ALL_MASK, PCIE_ISR0_REG);
+	advk_writel(pcie, PCIE_ISR1_ALL_MASK, PCIE_ISR1_REG);
+	advk_writel(pcie, PCIE_IRQ_ALL_MASK, HOST_CTRL_INT_STATUS_REG);
+
+	/* Disable All ISR0/1 Sources */
+	reg = PCIE_ISR0_ALL_MASK;
+	reg &= ~PCIE_ISR0_MSI_INT_PENDING;
+	advk_writel(pcie, reg, PCIE_ISR0_MASK_REG);
+
+	advk_writel(pcie, PCIE_ISR1_ALL_MASK, PCIE_ISR1_MASK_REG);
+
+	/* Unmask all MSI's */
+	advk_writel(pcie, 0, PCIE_MSI_MASK_REG);
+
+	/* Enable summary interrupt for GIC SPI source */
+	reg = PCIE_IRQ_ALL_MASK & (~PCIE_IRQ_ENABLE_INTS_MASK);
+	advk_writel(pcie, reg, HOST_CTRL_INT_MASK_REG);
+
+	/* Start link training */
+	reg = advk_readl(pcie, PCIE_CORE_LINK_CTRL_STAT_REG);
+	reg |= PCIE_CORE_LINK_TRAINING;
+	advk_writel(pcie, reg, PCIE_CORE_LINK_CTRL_STAT_REG);
+
+	advk_pcie_link_up(pcie);
+
+	reg = PCIE_CORE_LINK_L0S_ENTRY |
+		(1 << PCIE_CORE_LINK_WIDTH_SHIFT);
+	advk_writel(pcie, reg, PCIE_CORE_LINK_CTRL_STAT_REG);
+
+	reg = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG);
+	reg |= PCIE_CORE_CMD_MEM_ACCESS_EN |
+		PCIE_CORE_CMD_IO_ACCESS_EN |
+		PCIE_CORE_CMD_MEM_IO_REQ_EN;
+	advk_writel(pcie, reg, PCIE_CORE_CMD_STATUS_REG);
+}
+
+/*
+ * This routine is used to enable or disable AXI address window
+ * location generation.
+ * Disabled: No address window mapping. Use AXI user fields
+ * provided by the AXI fabric.
+ * Enabled: Enable the address window mapping. The HAL bridge
+ * generates the AXI user field locally. Use the local generated AXI user
+ * fields.
+ * It should be disabled when accessing the PCIe device by PIO.
+ * It should be enabled when accessing the PCIe device by memory
+ * access directly.
+ */
+static void advk_pcie_enable_axi_addr_gen(struct advk_pcie *pcie, bool enable)
+{
+	u32 reg;
+
+	reg = advk_readl(pcie, PCIE_CORE_CTRL2_REG);
+	if (enable)
+		reg |= PCIE_CORE_CTRL2_OB_WIN_ENABLE;
+	else
+		reg &= ~PCIE_CORE_CTRL2_OB_WIN_ENABLE;
+	advk_writel(pcie, reg, PCIE_CORE_CTRL2_REG);
+}
+
+static void advk_pcie_check_pio_status(struct advk_pcie *pcie)
+{
+	u32 reg;
+	unsigned int status;
+	char *strcomp_status, *str_posted;
+
+	reg = advk_readl(pcie, PIO_STAT);
+	status = (reg & PIO_COMPLETION_STATUS_MASK) >>
+		PIO_COMPLETION_STATUS_SHIFT;
+
+	if (!status)
+		return;
+
+	switch (status) {
+	case PIO_COMPLETION_STATUS_UR:
+		strcomp_status = "UR";
+		break;
+	case PIO_COMPLETION_STATUS_CRS:
+		strcomp_status = "CRS";
+		break;
+	case PIO_COMPLETION_STATUS_CA:
+		strcomp_status = "CA";
+		break;
+	default:
+		strcomp_status = "Unknown";
+		break;
+	}
+
+	if (reg & PIO_NON_POSTED_REQ)
+		str_posted = "Non-posted";
+	else
+		str_posted = "Posted";
+
+	dev_err(&pcie->pdev->dev,
+		"%s PIO Response Status: %s, %#x @ %#x\n",
+		str_posted, strcomp_status, reg,
+		advk_readl(pcie, PIO_ADDR_LS));
+}
+
+static int advk_pcie_wait_pio(struct advk_pcie *pcie)
+{
+	int i;
+	u32 reg, is_done;
+
+	for (i = 0; i < PIO_TIMEOUT_NUM; i++) {
+		reg = advk_readl(pcie, PIO_START);
+		is_done = advk_readl(pcie, PIO_ISR);
+		if ((!reg) && is_done)
+			break;
+	}
+
+	if (i == PIO_TIMEOUT_NUM) {
+		dev_err(&pcie->pdev->dev, "config read/write timed out\n");
+		return -ETIME;
+	}
+
+	return 0;
+}
+
+static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn,
+			     int where, int size, u32 *val)
+{
+	struct advk_pcie *pcie = bus->sysdata;
+	u32 reg;
+	int ret;
+
+	if (PCI_SLOT(devfn) != 0) {
+		*val = 0xffffffff;
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	advk_pcie_enable_axi_addr_gen(pcie, false);
+
+	/* Start PIO */
+	advk_writel(pcie, 0, PIO_START);
+	advk_writel(pcie, 1, PIO_ISR);
+
+	/* Program the control register */
+	if (bus->number ==  0)
+		advk_writel(pcie, PCIE_CONFIG_RD_TYPE0, PIO_CTRL);
+	else
+		advk_writel(pcie, PCIE_CONFIG_RD_TYPE1, PIO_CTRL);
+
+	/* Program the address registers */
+	reg = PCIE_BDF(devfn) | PCIE_CONF_REG(where);
+	advk_writel(pcie, reg, PIO_ADDR_LS);
+	advk_writel(pcie, 0, PIO_ADDR_MS);
+
+	/* Program the data strobe */
+	advk_writel(pcie, 0xf, PIO_WR_DATA_STRB);
+
+	/* Start the transfer */
+	advk_writel(pcie, 1, PIO_START);
+
+	ret = advk_pcie_wait_pio(pcie);
+	if (ret < 0) {
+		advk_pcie_enable_axi_addr_gen(pcie, true);
+		return PCIBIOS_SET_FAILED;
+	}
+
+	advk_pcie_check_pio_status(pcie);
+
+	/* Get the read result */
+	*val = advk_readl(pcie, PIO_RD_DATA);
+	if (size == 1)
+		*val = (*val >> (8 * (where & 3))) & 0xff;
+	else if (size == 2)
+		*val = (*val >> (8 * (where & 3))) & 0xffff;
+
+	advk_pcie_enable_axi_addr_gen(pcie, true);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int advk_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
+				int where, int size, u32 val)
+{
+	struct advk_pcie *pcie = bus->sysdata;
+	u32 reg;
+	u32 data_strobe = 0x0;
+	int offset;
+	int ret;
+
+	if (PCI_SLOT(devfn) != 0)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	advk_pcie_enable_axi_addr_gen(pcie, false);
+
+	/* Start PIO */
+	advk_writel(pcie, 0, PIO_START);
+	advk_writel(pcie, 1, PIO_ISR);
+
+	if (bus->number == 0)
+		advk_writel(pcie, PCIE_CONFIG_WR_TYPE0, PIO_CTRL);
+	else
+		advk_writel(pcie, PCIE_CONFIG_WR_TYPE1, PIO_CTRL);
+
+	/* Program the address registers */
+	reg = PCIE_CONF_ADDR(bus->number, devfn, where);
+	advk_writel(pcie, reg, PIO_ADDR_LS);
+	advk_writel(pcie, 0, PIO_ADDR_MS);
+
+	if (where % size) {
+		advk_pcie_enable_axi_addr_gen(pcie, true);
+		return PCIBIOS_SET_FAILED;
+	}
+
+	/* Calculate the write strobe */
+	offset      = where & 0x3;
+	reg         = val << (8 * offset);
+	data_strobe = GENMASK(size - 1, 0) << offset;
+
+	/* Program the data register */
+	advk_writel(pcie, reg, PIO_WR_DATA);
+
+	/* Program the data strobe */
+	advk_writel(pcie, data_strobe, PIO_WR_DATA_STRB);
+
+	/* Start the transfer */
+	advk_writel(pcie, 1, PIO_START);
+
+	ret = advk_pcie_wait_pio(pcie);
+	if (ret < 0) {
+		advk_pcie_enable_axi_addr_gen(pcie, true);
+		return PCIBIOS_SET_FAILED;
+	}
+
+	advk_pcie_check_pio_status(pcie);
+
+	advk_pcie_enable_axi_addr_gen(pcie, true);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static struct pci_ops advk_pcie_ops = {
+	.read = advk_pcie_rd_conf,
+	.write = advk_pcie_wr_conf,
+};
+
+static int advk_pcie_alloc_msi(struct advk_pcie *pcie)
+{
+	int hwirq;
+
+	mutex_lock(&pcie->msi_used_lock);
+	hwirq = find_first_zero_bit(pcie->msi_irq_in_use, MSI_IRQ_NUM);
+	if (hwirq >= MSI_IRQ_NUM)
+		hwirq = -ENOSPC;
+	else
+		set_bit(hwirq, pcie->msi_irq_in_use);
+	mutex_unlock(&pcie->msi_used_lock);
+
+	return hwirq;
+}
+
+static void advk_pcie_free_msi(struct advk_pcie *pcie, int hwirq)
+{
+	mutex_lock(&pcie->msi_used_lock);
+	if (!test_bit(hwirq, pcie->msi_irq_in_use))
+		pr_err("trying to free unused MSI#%d\n", hwirq);
+	else
+		clear_bit(hwirq, pcie->msi_irq_in_use);
+	mutex_unlock(&pcie->msi_used_lock);
+}
+
+static int advk_pcie_setup_msi_irq(struct msi_controller *chip,
+				   struct pci_dev *pdev,
+				   struct msi_desc *desc)
+{
+	struct advk_pcie *pcie = pdev->bus->sysdata;
+	struct msi_msg msg;
+	int virq, hwirq;
+	phys_addr_t msi_msg_phys;
+
+	/* We support MSI, but not MSI-X */
+	if (desc->msi_attrib.is_msix)
+		return -EINVAL;
+
+	hwirq = advk_pcie_alloc_msi(pcie);
+	if (hwirq < 0)
+		return hwirq;
+
+	virq = irq_create_mapping(pcie->msi_domain, hwirq);
+	if (!virq) {
+		advk_pcie_free_msi(pcie, hwirq);
+		return -EINVAL;
+	}
+
+	irq_set_msi_desc(virq, desc);
+
+	msi_msg_phys = virt_to_phys(&pcie->msi_msg);
+
+	msg.address_lo = lower_32_bits(msi_msg_phys);
+	msg.address_hi = upper_32_bits(msi_msg_phys);
+	msg.data = virq;
+
+	pci_write_msi_msg(virq, &msg);
+
+	return 0;
+}
+
+static void advk_pcie_teardown_msi_irq(struct msi_controller *chip,
+				       unsigned int irq)
+{
+	struct irq_data *d = irq_get_irq_data(irq);
+	struct msi_desc *msi = irq_data_get_msi_desc(d);
+	struct advk_pcie *pcie = msi_desc_to_pci_sysdata(msi);
+	unsigned long hwirq = d->hwirq;
+
+	irq_dispose_mapping(irq);
+	advk_pcie_free_msi(pcie, hwirq);
+}
+
+static int advk_pcie_msi_map(struct irq_domain *domain,
+			     unsigned int virq, irq_hw_number_t hw)
+{
+	struct advk_pcie *pcie = domain->host_data;
+
+	irq_set_chip_and_handler(virq, &pcie->msi_irq_chip,
+				 handle_simple_irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops advk_pcie_msi_irq_ops = {
+	.map = advk_pcie_msi_map,
+};
+
+static void advk_pcie_irq_mask(struct irq_data *d)
+{
+	struct advk_pcie *pcie = d->domain->host_data;
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	u32 mask;
+
+	mask = advk_readl(pcie, PCIE_ISR0_MASK_REG);
+	mask |= PCIE_ISR0_INTX_ASSERT(hwirq) |
+		PCIE_ISR0_INTX_DEASSERT(hwirq);
+	advk_writel(pcie, mask, PCIE_ISR0_MASK_REG);
+}
+
+static void advk_pcie_irq_unmask(struct irq_data *d)
+{
+	struct advk_pcie *pcie = d->domain->host_data;
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	u32 mask;
+
+	mask = advk_readl(pcie, PCIE_ISR0_MASK_REG);
+	mask &= ~(PCIE_ISR0_INTX_ASSERT(hwirq) |
+		  PCIE_ISR0_INTX_DEASSERT(hwirq));
+	advk_writel(pcie, mask, PCIE_ISR0_MASK_REG);
+}
+
+static int advk_pcie_irq_map(struct irq_domain *h,
+			     unsigned int virq, irq_hw_number_t hwirq)
+{
+	struct advk_pcie *pcie = h->host_data;
+
+	advk_pcie_irq_mask(irq_get_irq_data(virq));
+	irq_set_status_flags(virq, IRQ_LEVEL);
+	irq_set_chip_and_handler(virq, &pcie->irq_chip,
+				 handle_level_irq);
+	irq_set_chip_data(virq, pcie);
+
+	return 0;
+}
+
+static const struct irq_domain_ops advk_pcie_irq_domain_ops = {
+	.map = advk_pcie_irq_map,
+	.xlate = irq_domain_xlate_onecell,
+};
+
+static int advk_pcie_init_irq(struct advk_pcie *pcie)
+{
+	struct device *dev = &pcie->pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct device_node *pcie_intc_node;
+	struct irq_chip *irq_chip, *msi_irq_chip;
+	struct msi_controller *msi;
+	phys_addr_t msi_msg_phys;
+
+	pcie_intc_node =  of_get_next_child(node, NULL);
+	if (!pcie_intc_node) {
+		dev_err(dev, "No PCIe Intc node found\n");
+		return PTR_ERR(pcie_intc_node);
+	}
+
+	irq_chip = &pcie->irq_chip;
+
+	irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-irq",
+					dev_name(dev));
+	if (!irq_chip->name)
+		return -ENOMEM;
+	irq_chip->irq_mask = advk_pcie_irq_mask;
+	irq_chip->irq_mask_ack = advk_pcie_irq_mask;
+	irq_chip->irq_unmask = advk_pcie_irq_unmask;
+
+	pcie->irq_domain =
+		irq_domain_add_linear(pcie_intc_node, LEGACY_IRQ_NUM,
+				      &advk_pcie_irq_domain_ops, pcie);
+	if (!pcie->irq_domain) {
+		dev_err(dev, "Failed to get a INTx IRQ domain\n");
+		return PTR_ERR(pcie->irq_domain);
+	}
+
+	msi_irq_chip = &pcie->msi_irq_chip;
+
+	msi_irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-msi",
+						 dev_name(dev));
+	if (!msi_irq_chip->name) {
+		irq_domain_remove(pcie->irq_domain);
+		return -ENOMEM;
+	}
+
+	msi_irq_chip->irq_enable = pci_msi_unmask_irq;
+	msi_irq_chip->irq_disable = pci_msi_mask_irq;
+	msi_irq_chip->irq_mask = pci_msi_mask_irq;
+	msi_irq_chip->irq_unmask = pci_msi_unmask_irq;
+
+	msi = &pcie->msi;
+
+	msi->setup_irq = advk_pcie_setup_msi_irq;
+	msi->teardown_irq = advk_pcie_teardown_msi_irq;
+	msi->of_node = node;
+
+	mutex_init(&pcie->msi_used_lock);
+
+	msi_msg_phys = virt_to_phys(&pcie->msi_msg);
+
+	advk_writel(pcie, lower_32_bits(msi_msg_phys),
+		    PCIE_MSI_ADDR_LOW_REG);
+	advk_writel(pcie, upper_32_bits(msi_msg_phys),
+		    PCIE_MSI_ADDR_HIGH_REG);
+
+	pcie->msi_domain =
+		irq_domain_add_linear(NULL, MSI_IRQ_NUM,
+				      &advk_pcie_msi_irq_ops, pcie);
+	if (!pcie->msi_domain) {
+		irq_domain_remove(pcie->irq_domain);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static void advk_pcie_handle_msi(struct advk_pcie *pcie)
+{
+	u32 msi_val, msi_mask, msi_status, msi_idx;
+	u16 msi_data;
+
+	msi_mask = advk_readl(pcie, PCIE_MSI_MASK_REG);
+	msi_val = advk_readl(pcie, PCIE_MSI_STATUS_REG);
+	msi_status = msi_val & ~msi_mask;
+
+	for (msi_idx = 0; msi_idx < MSI_IRQ_NUM; msi_idx++) {
+		if (!(BIT(msi_idx) & msi_status))
+			continue;
+
+		advk_writel(pcie, BIT(msi_idx), PCIE_MSI_STATUS_REG);
+		msi_data = advk_readl(pcie, PCIE_MSI_PAYLOAD_REG) & 0xFF;
+		generic_handle_irq(msi_data);
+	}
+
+	advk_writel(pcie, PCIE_ISR0_MSI_INT_PENDING,
+		    PCIE_ISR0_REG);
+}
+
+static void advk_pcie_handle_int(struct advk_pcie *pcie)
+{
+	u32 val, mask, status;
+
+	val = advk_readl(pcie, PCIE_ISR0_REG);
+	mask = advk_readl(pcie, PCIE_ISR0_MASK_REG);
+	status = val & ((~mask) & PCIE_ISR0_ALL_MASK);
+
+	if (!status) {
+		advk_writel(pcie, val, PCIE_ISR0_REG);
+		return;
+	}
+
+	/* Process MSI interrupts */
+	if (status & PCIE_ISR0_MSI_INT_PENDING)
+		advk_pcie_handle_msi(pcie);
+
+	/* Process legacy interrupts */
+	do {
+		int i;
+
+		/*
+		 * As documented in the datasheet, we need to raise an
+		 * interrupt for each ASSERT(i) bit that is set, and
+		 * continue to deliver the interrupt until the
+		 * DEASSERT(i) bit gets set
+		 */
+		for (i = 0; i < LEGACY_IRQ_NUM; i++) {
+			if (!(status & PCIE_ISR0_INTX_ASSERT(i)))
+				continue;
+
+			do {
+				int virq;
+
+				advk_writel(pcie, PCIE_ISR0_INTX_ASSERT(i),
+					    PCIE_ISR0_REG);
+				virq = irq_find_mapping(pcie->irq_domain, i);
+				generic_handle_irq(virq);
+				status = advk_readl(pcie, PCIE_ISR0_REG);
+			} while (!(status & PCIE_ISR0_INTX_DEASSERT(i)));
+
+			advk_writel(pcie, PCIE_ISR0_INTX_DEASSERT(i),
+				    PCIE_ISR0_REG);
+		}
+
+		status = advk_readl(pcie, PCIE_ISR0_REG);
+	} while (status & PCIE_ISR0_INTX_MASK);
+}
+
+static irqreturn_t advk_pcie_irq_handler(int irq, void *arg)
+{
+	struct advk_pcie *pcie = arg;
+	u32 status;
+
+	status = advk_readl(pcie, HOST_CTRL_INT_STATUS_REG);
+	if (!(status & PCIE_IRQ_CORE_INT))
+		return IRQ_NONE;
+
+	advk_pcie_handle_int(pcie);
+
+	/* Clear interrupt */
+	advk_writel(pcie, PCIE_IRQ_CORE_INT, HOST_CTRL_INT_STATUS_REG);
+
+	return IRQ_HANDLED;
+}
+
+static void advk_pcie_release_of_pci_ranges(struct advk_pcie *pcie)
+{
+	pci_free_resource_list(&pcie->resources);
+}
+
+static int advk_pcie_parse_request_of_pci_ranges(struct advk_pcie *pcie)
+{
+	int err, res_valid = 0;
+	struct device *dev = &pcie->pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct resource_entry *win;
+	resource_size_t iobase;
+
+	INIT_LIST_HEAD(&pcie->resources);
+
+	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pcie->resources,
+					       &iobase);
+	if (err)
+		return err;
+
+	resource_list_for_each_entry(win, &pcie->resources) {
+		struct resource *parent, *res = win->res;
+
+		switch (resource_type(res)) {
+		case IORESOURCE_IO:
+			parent = &ioport_resource;
+			advk_pcie_set_ob_win(pcie, 1,
+					     upper_32_bits(res->start),
+					     lower_32_bits(res->start),
+					     0,	0xF8000000, 0,
+					     lower_32_bits(res->start),
+					     OB_PCIE_IO);
+			err = pci_remap_iospace(res, iobase);
+			if (err) {
+				dev_warn(dev, "error %d: failed to map resource %pR\n",
+					 err, res);
+				continue;
+			}
+			break;
+		case IORESOURCE_MEM:
+			parent = &iomem_resource;
+			advk_pcie_set_ob_win(pcie, 0,
+					     upper_32_bits(res->start),
+					     lower_32_bits(res->start),
+					     0x0, 0xF8000000, 0,
+					     lower_32_bits(res->start),
+					     (2 << 20) | OB_PCIE_MEM);
+			res_valid |= !(res->flags & IORESOURCE_PREFETCH);
+			break;
+		default:
+			continue;
+		}
+
+		err = devm_request_resource(dev, parent, res);
+		if (err)
+			goto out_release_res;
+	}
+
+	if (!res_valid) {
+		dev_err(dev, "non-prefetchable memory resource required\n");
+		err = -EINVAL;
+		goto out_release_res;
+	}
+
+	return 0;
+
+out_release_res:
+	advk_pcie_release_of_pci_ranges(pcie);
+	return err;
+}
+
+static int advk_pcie_probe(struct platform_device *pdev)
+{
+	struct advk_pcie *pcie;
+	struct resource *res;
+	struct pci_bus *bus, *child;
+	int ret, irq;
+
+	pcie = devm_kzalloc(&pdev->dev, sizeof(struct advk_pcie),
+			    GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->pdev = pdev;
+	platform_set_drvdata(pdev, pcie);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pcie->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pcie->base)) {
+		dev_err(&pdev->dev, "failed to map registers\n");
+		return PTR_ERR(pcie->base);
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	ret = devm_request_irq(&pdev->dev, irq, advk_pcie_irq_handler,
+			       IRQF_SHARED | IRQF_NO_THREAD, "advk-pcie",
+			       pcie);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register interrupt\n");
+		return ret;
+	}
+
+	ret = advk_pcie_parse_request_of_pci_ranges(pcie);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to parse resources\n");
+		return ret;
+	}
+
+	advk_pcie_setup_hw(pcie);
+	advk_pcie_enable_axi_addr_gen(pcie, true);
+
+	ret = advk_pcie_init_irq(pcie);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to initialize irq\n");
+		return ret;
+	}
+
+	bus = pci_scan_root_bus_msi(&pdev->dev, 0, &advk_pcie_ops,
+				    pcie, &pcie->resources, &pcie->msi);
+	if (!bus)
+		return -ENOMEM;
+
+	pci_bus_assign_resources(bus);
+
+	list_for_each_entry(child, &bus->children, node)
+		pcie_bus_configure_settings(child);
+
+	pci_bus_add_devices(bus);
+
+	return 0;
+}
+
+static const struct of_device_id advk_pcie_of_match_table[] = {
+	{ .compatible = "marvell,armada-3700-pcie", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, advk_pcie_of_match_table);
+
+static struct platform_driver advk_pcie_driver = {
+	.driver = {
+		.name = "advk-pcie",
+		.of_match_table = advk_pcie_of_match_table,
+		/* Driver unloading/unbinding currently not supported */
+		.suppress_bind_attrs = true,
+	},
+	.probe = advk_pcie_probe,
+};
+module_platform_driver(advk_pcie_driver);
+
+MODULE_AUTHOR("Hezi Shahmoon <hezi.shahmoon@marvell.com>");
+MODULE_DESCRIPTION("Aardvark PCIe driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

[PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Thomas Petazzoni]

This commit adds a new driver for a PCIe controller called Aardvark,
which is used on the Marvell Armada 3700 ARM64 SoC.

This patch is based on work done by Hezi Shahmoon
<hezi.shahmoon@marvell.com> and Marcin Wojtas <mw@semihalf.com>.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 MAINTAINERS                     |    7 +
 drivers/pci/host/Kconfig        |    9 +
 drivers/pci/host/Makefile       |    1 +
 drivers/pci/host/pci-aardvark.c | 1029 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 1046 insertions(+)
 create mode 100644 drivers/pci/host/pci-aardvark.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7304d2e..373215c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8740,6 +8740,13 @@ L:	linux-arm-kernel at lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
 F:	drivers/pci/host/*mvebu*
 
+PCI DRIVER FOR AARDVARK (Marvell Armada 3700)
+M:	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+L:	linux-pci at vger.kernel.org
+L:	linux-arm-kernel at lists.infradead.org (moderated for non-subscribers)
+S:	Maintained
+F:	drivers/pci/host/pci-aardvark.c
+
 PCI DRIVER FOR NVIDIA TEGRA
 M:	Thierry Reding <thierry.reding@gmail.com>
 L:	linux-tegra at vger.kernel.org
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 5d2374e..5f085fd 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -16,6 +16,15 @@ config PCI_MVEBU
 	depends on ARM
 	depends on OF
 
+config PCI_AARDVARK
+	bool "Aardvark PCIe controller"
+	depends on ARCH_MVEBU && ARM64
+	depends on OF
+	select PCI_MSI
+	help
+	 Add support for Aardvark 64bit PCIe Host Controller. This
+	 controller is part of the South Bridge of the Marvel Armada
+	 3700 SoC.
 
 config PCIE_XILINX_NWL
 	bool "NWL PCIe Core"
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 9c8698e..66f45b6 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
 obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
 obj-$(CONFIG_PCI_HYPERV) += pci-hyperv.o
 obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
+obj-$(CONFIG_PCI_AARDVARK) += pci-aardvark.o
 obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
 obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
 obj-$(CONFIG_PCIE_RCAR) += pcie-rcar.o
diff --git a/drivers/pci/host/pci-aardvark.c b/drivers/pci/host/pci-aardvark.c
new file mode 100644
index 0000000..c204bc0
--- /dev/null
+++ b/drivers/pci/host/pci-aardvark.c
@@ -0,0 +1,1029 @@
+/*
+ * Driver for the Aardvark PCIe controller, used on Marvell Armada
+ * 3700.
+ *
+ * Copyright (C) 2016 Marvell
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of_address.h>
+#include <linux/of_pci.h>
+
+/* PCIe core registers */
+#define PCIE_CORE_CMD_STATUS_REG				0x4
+#define     PCIE_CORE_CMD_IO_ACCESS_EN				BIT(0)
+#define     PCIE_CORE_CMD_MEM_ACCESS_EN				BIT(1)
+#define     PCIE_CORE_CMD_MEM_IO_REQ_EN				BIT(2)
+#define PCIE_CORE_DEV_CTRL_STATS_REG				0xC8
+#define     PCIE_CORE_DEV_CTRL_STATS_RELAX_ORDER_DISABLE	(0 << 4)
+#define     PCIE_CORE_DEV_CTRL_STATS_MAX_PAYLOAD_SZ_SHIFT	5
+#define     PCIE_CORE_DEV_CTRL_STATS_SNOOP_DISABLE		(0 << 11)
+#define     PCIE_CORE_DEV_CTRL_STATS_MAX_RD_REQ_SIZE_SHIFT	12
+#define PCIE_CORE_LINK_CTRL_STAT_REG				0xD0
+#define     PCIE_CORE_LINK_L0S_ENTRY				BIT(0)
+#define     PCIE_CORE_LINK_TRAINING				BIT(5)
+#define     PCIE_CORE_LINK_WIDTH_SHIFT				20
+#define PCIE_CORE_ERR_CAPCTL_REG				0x118
+#define     PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX			BIT(5)
+#define     PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX_EN			BIT(6)
+#define     PCIE_CORE_ERR_CAPCTL_ECRC_CHCK			BIT(7)
+#define     PCIE_CORE_ERR_CAPCTL_ECRC_CHCK_RCV			BIT(8)
+
+/* PIO registers base address and register offsets */
+#define PIO_BASE_ADDR				0x4000
+#define PIO_CTRL				(PIO_BASE_ADDR + 0x0)
+#define PIO_STAT				(PIO_BASE_ADDR + 0x4)
+#define   PIO_COMPLETION_STATUS_SHIFT		7
+#define   PIO_COMPLETION_STATUS_MASK		GENMASK(9, 7)
+#define   PIO_COMPLETION_STATUS_OK		0
+#define   PIO_COMPLETION_STATUS_UR		1
+#define   PIO_COMPLETION_STATUS_CRS		2
+#define   PIO_COMPLETION_STATUS_CA		4
+#define   PIO_NON_POSTED_REQ			BIT(0)
+#define PIO_ADDR_LS				(PIO_BASE_ADDR + 0x8)
+#define PIO_ADDR_MS				(PIO_BASE_ADDR + 0xc)
+#define PIO_WR_DATA				(PIO_BASE_ADDR + 0x10)
+#define PIO_WR_DATA_STRB			(PIO_BASE_ADDR + 0x14)
+#define PIO_RD_DATA				(PIO_BASE_ADDR + 0x18)
+#define PIO_START				(PIO_BASE_ADDR + 0x1c)
+#define PIO_ISR					(PIO_BASE_ADDR + 0x20)
+#define PIO_ISRM				(PIO_BASE_ADDR + 0x24)
+
+/* Aardvark Control registers */
+#define CONTROL_BASE_ADDR			0x4800
+#define PCIE_CORE_CTRL0_REG			(CONTROL_BASE_ADDR + 0x0)
+#define     PCIE_GEN_SEL_MSK			0x3
+#define     PCIE_GEN_SEL_SHIFT			0x0
+#define     SPEED_GEN_1				0
+#define     SPEED_GEN_2				1
+#define     SPEED_GEN_3				2
+#define     IS_RC_MSK				1
+#define     IS_RC_SHIFT				2
+#define     LANE_CNT_MSK			0x18
+#define     LANE_CNT_SHIFT			0x3
+#define     LANE_COUNT_1			(0 << LANE_CNT_SHIFT)
+#define     LANE_COUNT_2			(1 << LANE_CNT_SHIFT)
+#define     LANE_COUNT_4			(2 << LANE_CNT_SHIFT)
+#define     LANE_COUNT_8			(3 << LANE_CNT_SHIFT)
+#define     LINK_TRAINING_EN			BIT(6)
+#define     LEGACY_INTA				BIT(28)
+#define     LEGACY_INTB				BIT(29)
+#define     LEGACY_INTC				BIT(30)
+#define     LEGACY_INTD				BIT(31)
+#define PCIE_CORE_CTRL1_REG			(CONTROL_BASE_ADDR + 0x4)
+#define     HOT_RESET_GEN			BIT(0)
+#define PCIE_CORE_CTRL2_REG			(CONTROL_BASE_ADDR + 0x8)
+#define     PCIE_CORE_CTRL2_RESERVED		0x7
+#define     PCIE_CORE_CTRL2_TD_ENABLE		BIT(4)
+#define     PCIE_CORE_CTRL2_STRICT_ORDER_ENABLE	BIT(5)
+#define     PCIE_CORE_CTRL2_OB_WIN_ENABLE	BIT(6)
+#define     PCIE_CORE_CTRL2_MSI_ENABLE		BIT(10)
+#define PCIE_ISR0_REG				(CONTROL_BASE_ADDR + 0x40)
+#define PCIE_ISR0_MASK_REG			(CONTROL_BASE_ADDR + 0x44)
+#define     PCIE_ISR0_FLR_INT			BIT(26)
+#define     PCIE_ISR0_MSG_LTR			BIT(25)
+#define     PCIE_ISR0_MSI_INT_PENDING		BIT(24)
+#define     PCIE_ISR0_INTD_DEASSERT		BIT(23)
+#define     PCIE_ISR0_INTC_DEASSERT		BIT(22)
+#define     PCIE_ISR0_INTB_DEASSERT		BIT(21)
+#define     PCIE_ISR0_INTA_DEASSERT		BIT(20)
+#define     PCIE_ISR0_INTD_ASSERT		BIT(19)
+#define     PCIE_ISR0_INTC_ASSERT		BIT(18)
+#define     PCIE_ISR0_INTB_ASSERT		BIT(17)
+#define     PCIE_ISR0_INTA_ASSERT		BIT(16)
+#define     PCIE_ISR0_FAT_ERR			BIT(13)
+#define     PCIE_ISR0_NFAT_ERR			BIT(12)
+#define     PCIE_ISR0_CORR_ERR			BIT(11)
+#define     PCIE_ISR0_LMI_LOCAL_INT		BIT(10)
+#define     PCIE_ISR0_LEGACY_INT_SENT		BIT(9)
+#define     PCIE_ISR0_MSG_PM_ACTIVE_STATE_NAK	BIT(8)
+#define     PCIE_ISR0_MSG_PM_PME		BIT(7)
+#define     PCIE_ISR0_MSG_PM_TURN_OFF		BIT(6)
+#define     PCIE_ISR0_MSG_PME_TO_ACK		BIT(5)
+#define     PCIE_ISR0_INB_DP_FERR_PERR_IRQ	BIT(4)
+#define     PCIE_ISR0_OUTB_DP_FERR_PERR_IRQ	BIT(3)
+#define     PCIE_ISR0_INBOUND_MSG		BIT(2)
+#define     PCIE_ISR0_LINK_DOWN			BIT(1)
+#define     PCIE_ISR0_HOT_RESET			BIT(0)
+#define     PCIE_ISR0_INTX_ASSERT(val)		BIT(16 + (val))
+#define     PCIE_ISR0_INTX_DEASSERT(val)	BIT(20 + (val))
+#define     PCIE_ISR0_INTX_MASK			GENMASK(23, 16)
+#define	    PCIE_ISR0_ALL_MASK			GENMASK(26, 0)
+#define PCIE_ISR1_REG				(CONTROL_BASE_ADDR + 0x48)
+#define PCIE_ISR1_MASK_REG			(CONTROL_BASE_ADDR + 0x4C)
+#define     PCIE_ISR1_POWER_STATE_CHANGE	BIT(4)
+#define     PCIE_ISR1_FLUSH			BIT(5)
+#define     PCIE_ISR1_ALL_MASK			GENMASK(5, 4)
+#define PCIE_MSI_ADDR_LOW_REG			(CONTROL_BASE_ADDR + 0x50)
+#define PCIE_MSI_ADDR_HIGH_REG			(CONTROL_BASE_ADDR + 0x54)
+#define PCIE_MSI_STATUS_REG			(CONTROL_BASE_ADDR + 0x58)
+#define PCIE_MSI_MASK_REG			(CONTROL_BASE_ADDR + 0x5C)
+#define PCIE_MSI_PAYLOAD_REG			(CONTROL_BASE_ADDR + 0x9C)
+
+/* PCIe window configuration */
+#define OB_WIN_BASE_ADDR			0x4c00
+#define OB_WIN_BLOCK_SIZE			0x20
+#define OB_WIN_REG_ADDR(win, offset)		(OB_WIN_BASE_ADDR + \
+						 OB_WIN_BLOCK_SIZE * (win) + \
+						 (offset))
+#define OB_WIN_MATCH_LS(win)			OB_WIN_REG_ADDR(win, 0x00)
+#define OB_WIN_MATCH_MS(win)			OB_WIN_REG_ADDR(win, 0x04)
+#define OB_WIN_REMAP_LS(win)			OB_WIN_REG_ADDR(win, 0x08)
+#define OB_WIN_REMAP_MS(win)			OB_WIN_REG_ADDR(win, 0x0c)
+#define OB_WIN_MASK_LS(win)			OB_WIN_REG_ADDR(win, 0x10)
+#define OB_WIN_MASK_MS(win)			OB_WIN_REG_ADDR(win, 0x14)
+#define OB_WIN_ACTIONS(win)			OB_WIN_REG_ADDR(win, 0x18)
+
+/* PCIe window types */
+#define     OB_PCIE_MEM				0x0
+#define     OB_PCIE_IO				0x4
+#define     OB_PCIE_CONFIG0			0x8
+#define     OB_PCIE_CONFIG1			0x9
+#define     OB_PCIE_MSG				0xc
+#define     OB_PCIE_MSG_VENDOR			0xd
+
+/* LMI registers base address and register offsets */
+#define LMI_BASE_ADDR				0x6000
+#define CFG_REG					(LMI_BASE_ADDR + 0x0)
+#define     LTSSM_SHIFT				24
+#define     LTSSM_MASK				0x3f
+#define     LTSSM_L0				0x10
+#define     RC_BAR_CONFIG			0x300
+
+/* PCIe core controller registers */
+#define CTRL_CORE_BASE_ADDR			0x18000
+#define CTRL_CONFIG_REG				(CTRL_CORE_BASE_ADDR + 0x0)
+#define     CTRL_MODE_SHIFT			0x0
+#define     CTRL_MODE_MASK			0x1
+#define     PCIE_CORE_MODE_DIRECT		0x0
+#define     PCIE_CORE_MODE_COMMAND		0x1
+
+/* PCIe Central Interrupts Registers */
+#define CENTRAL_INT_BASE_ADDR			0x1b000
+#define HOST_CTRL_INT_STATUS_REG		(CENTRAL_INT_BASE_ADDR + 0x0)
+#define HOST_CTRL_INT_MASK_REG			(CENTRAL_INT_BASE_ADDR + 0x4)
+#define     PCIE_IRQ_CMDQ_INT			BIT(0)
+#define     PCIE_IRQ_MSI_STATUS_INT		BIT(1)
+#define     PCIE_IRQ_CMD_SENT_DONE		BIT(3)
+#define     PCIE_IRQ_DMA_INT			BIT(4)
+#define     PCIE_IRQ_IB_DXFERDONE		BIT(5)
+#define     PCIE_IRQ_OB_DXFERDONE		BIT(6)
+#define     PCIE_IRQ_OB_RXFERDONE		BIT(7)
+#define     PCIE_IRQ_COMPQ_INT			BIT(12)
+#define     PCIE_IRQ_DIR_RD_DDR_DET		BIT(13)
+#define     PCIE_IRQ_DIR_WR_DDR_DET		BIT(14)
+#define     PCIE_IRQ_CORE_INT			BIT(16)
+#define     PCIE_IRQ_CORE_INT_PIO		BIT(17)
+#define     PCIE_IRQ_DPMU_INT			BIT(18)
+#define     PCIE_IRQ_PCIE_MIS_INT		BIT(19)
+#define     PCIE_IRQ_MSI_INT1_DET		BIT(20)
+#define     PCIE_IRQ_MSI_INT2_DET		BIT(21)
+#define     PCIE_IRQ_RC_DBELL_DET		BIT(22)
+#define     PCIE_IRQ_EP_STATUS			BIT(23)
+#define     PCIE_IRQ_ALL_MASK			0xfff0fb
+#define     PCIE_IRQ_ENABLE_INTS_MASK		PCIE_IRQ_CORE_INT
+
+/* Transaction types */
+#define PCIE_CONFIG_RD_TYPE0			0x8
+#define PCIE_CONFIG_RD_TYPE1			0x9
+#define PCIE_CONFIG_WR_TYPE0			0xa
+#define PCIE_CONFIG_WR_TYPE1			0xb
+
+/* PCI_BDF shifts 8bit, so we need extra 4bit shift */
+#define PCIE_BDF(dev)				(dev << 4)
+#define PCIE_CONF_BUS(bus)			(((bus) & 0xff) << 20)
+#define PCIE_CONF_DEV(dev)			(((dev) & 0x1f) << 15)
+#define PCIE_CONF_FUNC(fun)			(((fun) & 0x7)	<< 12)
+#define PCIE_CONF_REG(reg)			((reg) & 0xffc)
+#define PCIE_CONF_ADDR(bus, devfn, where)	\
+	(PCIE_CONF_BUS(bus) | PCIE_CONF_DEV(PCI_SLOT(devfn))	| \
+	 PCIE_CONF_FUNC(PCI_FUNC(devfn)) | PCIE_CONF_REG(where))
+
+#define PIO_TIMEOUT_NUM			(1000)
+#define PCIE_LINKUP_TIMEOUT		(200)
+
+#define LEGACY_IRQ_NUM			4
+#define MSI_IRQ_NUM			32
+
+struct advk_pcie {
+	struct platform_device *pdev;
+	void __iomem *base;
+	struct list_head resources;
+	struct irq_domain *irq_domain;
+	struct irq_chip irq_chip;
+	struct msi_controller msi;
+	struct irq_domain *msi_domain;
+	struct irq_chip msi_irq_chip;
+	DECLARE_BITMAP(msi_irq_in_use, MSI_IRQ_NUM);
+	struct mutex msi_used_lock;
+	u16 msi_msg;
+};
+
+static inline void advk_writel(struct advk_pcie *pcie, u32 val, u64 reg)
+{
+	writel(val, pcie->base + reg);
+}
+
+static inline u32 advk_readl(struct advk_pcie *pcie, u64 reg)
+{
+	return readl(pcie->base + reg);
+}
+
+static bool advk_pcie_link_up(struct advk_pcie *pcie)
+{
+	int timeout;
+	u32 ltssm_state;
+	u32 val;
+
+	timeout = PCIE_LINKUP_TIMEOUT;
+	do {
+		val = advk_readl(pcie, CFG_REG);
+		ltssm_state = (val >> LTSSM_SHIFT) & LTSSM_MASK;
+		timeout--;
+	/* Use ltssm < LTSSM_L0 instead of ltssm != LTSSM_L0 */
+	} while (ltssm_state < LTSSM_L0 && timeout > 0);
+
+	return (timeout > 0);
+}
+
+/*
+ * Set PCIe address window register which could be used for memory
+ * mapping.
+ */
+static void advk_pcie_set_ob_win(struct advk_pcie *pcie,
+				 u32 win_num, u32 match_ms,
+				 u32 match_ls, u32 mask_ms,
+				 u32 mask_ls, u32 remap_ms,
+				 u32 remap_ls, u32 action)
+{
+	advk_writel(pcie, match_ls, OB_WIN_MATCH_LS(win_num));
+	advk_writel(pcie, match_ms, OB_WIN_MATCH_MS(win_num));
+	advk_writel(pcie, mask_ms, OB_WIN_MASK_MS(win_num));
+	advk_writel(pcie, mask_ls, OB_WIN_MASK_LS(win_num));
+	advk_writel(pcie, remap_ms, OB_WIN_REMAP_MS(win_num));
+	advk_writel(pcie, remap_ls, OB_WIN_REMAP_LS(win_num));
+	advk_writel(pcie, action, OB_WIN_ACTIONS(win_num));
+	advk_writel(pcie, match_ls | BIT(0), OB_WIN_MATCH_LS(win_num));
+}
+
+static void advk_pcie_setup_hw(struct advk_pcie *pcie)
+{
+	u32 reg;
+	int i;
+
+	/* Point PCIe unit MBUS decode windows to DRAM space. */
+	for (i = 0; i < 8; i++)
+		advk_pcie_set_ob_win(pcie, i, 0, 0, 0, 0, 0, 0, 0);
+
+	/* Set to Direct mode. */
+	reg = advk_readl(pcie, CTRL_CONFIG_REG);
+	reg &= ~(CTRL_MODE_MASK << CTRL_MODE_SHIFT);
+	reg |= ((PCIE_CORE_MODE_DIRECT & CTRL_MODE_MASK) << CTRL_MODE_SHIFT);
+	advk_writel(pcie, reg, CTRL_CONFIG_REG);
+
+	/* Set PCI global control register to RC mode */
+	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
+	reg |= (IS_RC_MSK << IS_RC_SHIFT);
+	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
+
+	/*
+	 * Set Advanced Error Capabilities and Control PF0 register
+	 */
+	reg = PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX |
+		PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX_EN |
+		PCIE_CORE_ERR_CAPCTL_ECRC_CHCK |
+		PCIE_CORE_ERR_CAPCTL_ECRC_CHCK_RCV;
+	advk_writel(pcie, reg, PCIE_CORE_ERR_CAPCTL_REG);
+
+	/*
+	 * Set PCIe Device Control and Status 1 PF0 register
+	 */
+	reg = PCIE_CORE_DEV_CTRL_STATS_RELAX_ORDER_DISABLE |
+		(7 << PCIE_CORE_DEV_CTRL_STATS_MAX_PAYLOAD_SZ_SHIFT) |
+		PCIE_CORE_DEV_CTRL_STATS_SNOOP_DISABLE |
+		PCIE_CORE_DEV_CTRL_STATS_MAX_RD_REQ_SIZE_SHIFT;
+	advk_writel(pcie, reg, PCIE_CORE_DEV_CTRL_STATS_REG);
+
+	/*
+	 * Program PCIe Control 2 to disable strict ordering.
+	 */
+	reg = PCIE_CORE_CTRL2_RESERVED |
+		PCIE_CORE_CTRL2_TD_ENABLE;
+	advk_writel(pcie, reg, PCIE_CORE_CTRL2_REG);
+
+	/* Set GEN2 */
+	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
+	reg &= ~PCIE_GEN_SEL_MSK;
+	reg |= SPEED_GEN_2;
+	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
+
+	/* Set lane X1 */
+	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
+	reg &= ~LANE_CNT_MSK;
+	reg |= LANE_COUNT_1;
+	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
+
+	/* Enable link training */
+	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
+	reg |= LINK_TRAINING_EN;
+	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
+
+	/* Enable MSI */
+	reg = advk_readl(pcie, PCIE_CORE_CTRL2_REG);
+	reg |= PCIE_CORE_CTRL2_MSI_ENABLE;
+	advk_writel(pcie, reg, PCIE_CORE_CTRL2_REG);
+
+	/* Clear all interrupts. */
+	advk_writel(pcie, PCIE_ISR0_ALL_MASK, PCIE_ISR0_REG);
+	advk_writel(pcie, PCIE_ISR1_ALL_MASK, PCIE_ISR1_REG);
+	advk_writel(pcie, PCIE_IRQ_ALL_MASK, HOST_CTRL_INT_STATUS_REG);
+
+	/* Disable All ISR0/1 Sources */
+	reg = PCIE_ISR0_ALL_MASK;
+	reg &= ~PCIE_ISR0_MSI_INT_PENDING;
+	advk_writel(pcie, reg, PCIE_ISR0_MASK_REG);
+
+	advk_writel(pcie, PCIE_ISR1_ALL_MASK, PCIE_ISR1_MASK_REG);
+
+	/* Unmask all MSI's */
+	advk_writel(pcie, 0, PCIE_MSI_MASK_REG);
+
+	/* Enable summary interrupt for GIC SPI source */
+	reg = PCIE_IRQ_ALL_MASK & (~PCIE_IRQ_ENABLE_INTS_MASK);
+	advk_writel(pcie, reg, HOST_CTRL_INT_MASK_REG);
+
+	/* Start link training */
+	reg = advk_readl(pcie, PCIE_CORE_LINK_CTRL_STAT_REG);
+	reg |= PCIE_CORE_LINK_TRAINING;
+	advk_writel(pcie, reg, PCIE_CORE_LINK_CTRL_STAT_REG);
+
+	advk_pcie_link_up(pcie);
+
+	reg = PCIE_CORE_LINK_L0S_ENTRY |
+		(1 << PCIE_CORE_LINK_WIDTH_SHIFT);
+	advk_writel(pcie, reg, PCIE_CORE_LINK_CTRL_STAT_REG);
+
+	reg = advk_readl(pcie, PCIE_CORE_CMD_STATUS_REG);
+	reg |= PCIE_CORE_CMD_MEM_ACCESS_EN |
+		PCIE_CORE_CMD_IO_ACCESS_EN |
+		PCIE_CORE_CMD_MEM_IO_REQ_EN;
+	advk_writel(pcie, reg, PCIE_CORE_CMD_STATUS_REG);
+}
+
+/*
+ * This routine is used to enable or disable AXI address window
+ * location generation.
+ * Disabled: No address window mapping. Use AXI user fields
+ * provided by the AXI fabric.
+ * Enabled: Enable the address window mapping. The HAL bridge
+ * generates the AXI user field locally. Use the local generated AXI user
+ * fields.
+ * It should be disabled when accessing the PCIe device by PIO.
+ * It should be enabled when accessing the PCIe device by memory
+ * access directly.
+ */
+static void advk_pcie_enable_axi_addr_gen(struct advk_pcie *pcie, bool enable)
+{
+	u32 reg;
+
+	reg = advk_readl(pcie, PCIE_CORE_CTRL2_REG);
+	if (enable)
+		reg |= PCIE_CORE_CTRL2_OB_WIN_ENABLE;
+	else
+		reg &= ~PCIE_CORE_CTRL2_OB_WIN_ENABLE;
+	advk_writel(pcie, reg, PCIE_CORE_CTRL2_REG);
+}
+
+static void advk_pcie_check_pio_status(struct advk_pcie *pcie)
+{
+	u32 reg;
+	unsigned int status;
+	char *strcomp_status, *str_posted;
+
+	reg = advk_readl(pcie, PIO_STAT);
+	status = (reg & PIO_COMPLETION_STATUS_MASK) >>
+		PIO_COMPLETION_STATUS_SHIFT;
+
+	if (!status)
+		return;
+
+	switch (status) {
+	case PIO_COMPLETION_STATUS_UR:
+		strcomp_status = "UR";
+		break;
+	case PIO_COMPLETION_STATUS_CRS:
+		strcomp_status = "CRS";
+		break;
+	case PIO_COMPLETION_STATUS_CA:
+		strcomp_status = "CA";
+		break;
+	default:
+		strcomp_status = "Unknown";
+		break;
+	}
+
+	if (reg & PIO_NON_POSTED_REQ)
+		str_posted = "Non-posted";
+	else
+		str_posted = "Posted";
+
+	dev_err(&pcie->pdev->dev,
+		"%s PIO Response Status: %s, %#x @ %#x\n",
+		str_posted, strcomp_status, reg,
+		advk_readl(pcie, PIO_ADDR_LS));
+}
+
+static int advk_pcie_wait_pio(struct advk_pcie *pcie)
+{
+	int i;
+	u32 reg, is_done;
+
+	for (i = 0; i < PIO_TIMEOUT_NUM; i++) {
+		reg = advk_readl(pcie, PIO_START);
+		is_done = advk_readl(pcie, PIO_ISR);
+		if ((!reg) && is_done)
+			break;
+	}
+
+	if (i == PIO_TIMEOUT_NUM) {
+		dev_err(&pcie->pdev->dev, "config read/write timed out\n");
+		return -ETIME;
+	}
+
+	return 0;
+}
+
+static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn,
+			     int where, int size, u32 *val)
+{
+	struct advk_pcie *pcie = bus->sysdata;
+	u32 reg;
+	int ret;
+
+	if (PCI_SLOT(devfn) != 0) {
+		*val = 0xffffffff;
+		return PCIBIOS_DEVICE_NOT_FOUND;
+	}
+
+	advk_pcie_enable_axi_addr_gen(pcie, false);
+
+	/* Start PIO */
+	advk_writel(pcie, 0, PIO_START);
+	advk_writel(pcie, 1, PIO_ISR);
+
+	/* Program the control register */
+	if (bus->number ==  0)
+		advk_writel(pcie, PCIE_CONFIG_RD_TYPE0, PIO_CTRL);
+	else
+		advk_writel(pcie, PCIE_CONFIG_RD_TYPE1, PIO_CTRL);
+
+	/* Program the address registers */
+	reg = PCIE_BDF(devfn) | PCIE_CONF_REG(where);
+	advk_writel(pcie, reg, PIO_ADDR_LS);
+	advk_writel(pcie, 0, PIO_ADDR_MS);
+
+	/* Program the data strobe */
+	advk_writel(pcie, 0xf, PIO_WR_DATA_STRB);
+
+	/* Start the transfer */
+	advk_writel(pcie, 1, PIO_START);
+
+	ret = advk_pcie_wait_pio(pcie);
+	if (ret < 0) {
+		advk_pcie_enable_axi_addr_gen(pcie, true);
+		return PCIBIOS_SET_FAILED;
+	}
+
+	advk_pcie_check_pio_status(pcie);
+
+	/* Get the read result */
+	*val = advk_readl(pcie, PIO_RD_DATA);
+	if (size == 1)
+		*val = (*val >> (8 * (where & 3))) & 0xff;
+	else if (size == 2)
+		*val = (*val >> (8 * (where & 3))) & 0xffff;
+
+	advk_pcie_enable_axi_addr_gen(pcie, true);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static int advk_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
+				int where, int size, u32 val)
+{
+	struct advk_pcie *pcie = bus->sysdata;
+	u32 reg;
+	u32 data_strobe = 0x0;
+	int offset;
+	int ret;
+
+	if (PCI_SLOT(devfn) != 0)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	advk_pcie_enable_axi_addr_gen(pcie, false);
+
+	/* Start PIO */
+	advk_writel(pcie, 0, PIO_START);
+	advk_writel(pcie, 1, PIO_ISR);
+
+	if (bus->number == 0)
+		advk_writel(pcie, PCIE_CONFIG_WR_TYPE0, PIO_CTRL);
+	else
+		advk_writel(pcie, PCIE_CONFIG_WR_TYPE1, PIO_CTRL);
+
+	/* Program the address registers */
+	reg = PCIE_CONF_ADDR(bus->number, devfn, where);
+	advk_writel(pcie, reg, PIO_ADDR_LS);
+	advk_writel(pcie, 0, PIO_ADDR_MS);
+
+	if (where % size) {
+		advk_pcie_enable_axi_addr_gen(pcie, true);
+		return PCIBIOS_SET_FAILED;
+	}
+
+	/* Calculate the write strobe */
+	offset      = where & 0x3;
+	reg         = val << (8 * offset);
+	data_strobe = GENMASK(size - 1, 0) << offset;
+
+	/* Program the data register */
+	advk_writel(pcie, reg, PIO_WR_DATA);
+
+	/* Program the data strobe */
+	advk_writel(pcie, data_strobe, PIO_WR_DATA_STRB);
+
+	/* Start the transfer */
+	advk_writel(pcie, 1, PIO_START);
+
+	ret = advk_pcie_wait_pio(pcie);
+	if (ret < 0) {
+		advk_pcie_enable_axi_addr_gen(pcie, true);
+		return PCIBIOS_SET_FAILED;
+	}
+
+	advk_pcie_check_pio_status(pcie);
+
+	advk_pcie_enable_axi_addr_gen(pcie, true);
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+static struct pci_ops advk_pcie_ops = {
+	.read = advk_pcie_rd_conf,
+	.write = advk_pcie_wr_conf,
+};
+
+static int advk_pcie_alloc_msi(struct advk_pcie *pcie)
+{
+	int hwirq;
+
+	mutex_lock(&pcie->msi_used_lock);
+	hwirq = find_first_zero_bit(pcie->msi_irq_in_use, MSI_IRQ_NUM);
+	if (hwirq >= MSI_IRQ_NUM)
+		hwirq = -ENOSPC;
+	else
+		set_bit(hwirq, pcie->msi_irq_in_use);
+	mutex_unlock(&pcie->msi_used_lock);
+
+	return hwirq;
+}
+
+static void advk_pcie_free_msi(struct advk_pcie *pcie, int hwirq)
+{
+	mutex_lock(&pcie->msi_used_lock);
+	if (!test_bit(hwirq, pcie->msi_irq_in_use))
+		pr_err("trying to free unused MSI#%d\n", hwirq);
+	else
+		clear_bit(hwirq, pcie->msi_irq_in_use);
+	mutex_unlock(&pcie->msi_used_lock);
+}
+
+static int advk_pcie_setup_msi_irq(struct msi_controller *chip,
+				   struct pci_dev *pdev,
+				   struct msi_desc *desc)
+{
+	struct advk_pcie *pcie = pdev->bus->sysdata;
+	struct msi_msg msg;
+	int virq, hwirq;
+	phys_addr_t msi_msg_phys;
+
+	/* We support MSI, but not MSI-X */
+	if (desc->msi_attrib.is_msix)
+		return -EINVAL;
+
+	hwirq = advk_pcie_alloc_msi(pcie);
+	if (hwirq < 0)
+		return hwirq;
+
+	virq = irq_create_mapping(pcie->msi_domain, hwirq);
+	if (!virq) {
+		advk_pcie_free_msi(pcie, hwirq);
+		return -EINVAL;
+	}
+
+	irq_set_msi_desc(virq, desc);
+
+	msi_msg_phys = virt_to_phys(&pcie->msi_msg);
+
+	msg.address_lo = lower_32_bits(msi_msg_phys);
+	msg.address_hi = upper_32_bits(msi_msg_phys);
+	msg.data = virq;
+
+	pci_write_msi_msg(virq, &msg);
+
+	return 0;
+}
+
+static void advk_pcie_teardown_msi_irq(struct msi_controller *chip,
+				       unsigned int irq)
+{
+	struct irq_data *d = irq_get_irq_data(irq);
+	struct msi_desc *msi = irq_data_get_msi_desc(d);
+	struct advk_pcie *pcie = msi_desc_to_pci_sysdata(msi);
+	unsigned long hwirq = d->hwirq;
+
+	irq_dispose_mapping(irq);
+	advk_pcie_free_msi(pcie, hwirq);
+}
+
+static int advk_pcie_msi_map(struct irq_domain *domain,
+			     unsigned int virq, irq_hw_number_t hw)
+{
+	struct advk_pcie *pcie = domain->host_data;
+
+	irq_set_chip_and_handler(virq, &pcie->msi_irq_chip,
+				 handle_simple_irq);
+
+	return 0;
+}
+
+static const struct irq_domain_ops advk_pcie_msi_irq_ops = {
+	.map = advk_pcie_msi_map,
+};
+
+static void advk_pcie_irq_mask(struct irq_data *d)
+{
+	struct advk_pcie *pcie = d->domain->host_data;
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	u32 mask;
+
+	mask = advk_readl(pcie, PCIE_ISR0_MASK_REG);
+	mask |= PCIE_ISR0_INTX_ASSERT(hwirq) |
+		PCIE_ISR0_INTX_DEASSERT(hwirq);
+	advk_writel(pcie, mask, PCIE_ISR0_MASK_REG);
+}
+
+static void advk_pcie_irq_unmask(struct irq_data *d)
+{
+	struct advk_pcie *pcie = d->domain->host_data;
+	irq_hw_number_t hwirq = irqd_to_hwirq(d);
+	u32 mask;
+
+	mask = advk_readl(pcie, PCIE_ISR0_MASK_REG);
+	mask &= ~(PCIE_ISR0_INTX_ASSERT(hwirq) |
+		  PCIE_ISR0_INTX_DEASSERT(hwirq));
+	advk_writel(pcie, mask, PCIE_ISR0_MASK_REG);
+}
+
+static int advk_pcie_irq_map(struct irq_domain *h,
+			     unsigned int virq, irq_hw_number_t hwirq)
+{
+	struct advk_pcie *pcie = h->host_data;
+
+	advk_pcie_irq_mask(irq_get_irq_data(virq));
+	irq_set_status_flags(virq, IRQ_LEVEL);
+	irq_set_chip_and_handler(virq, &pcie->irq_chip,
+				 handle_level_irq);
+	irq_set_chip_data(virq, pcie);
+
+	return 0;
+}
+
+static const struct irq_domain_ops advk_pcie_irq_domain_ops = {
+	.map = advk_pcie_irq_map,
+	.xlate = irq_domain_xlate_onecell,
+};
+
+static int advk_pcie_init_irq(struct advk_pcie *pcie)
+{
+	struct device *dev = &pcie->pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct device_node *pcie_intc_node;
+	struct irq_chip *irq_chip, *msi_irq_chip;
+	struct msi_controller *msi;
+	phys_addr_t msi_msg_phys;
+
+	pcie_intc_node =  of_get_next_child(node, NULL);
+	if (!pcie_intc_node) {
+		dev_err(dev, "No PCIe Intc node found\n");
+		return PTR_ERR(pcie_intc_node);
+	}
+
+	irq_chip = &pcie->irq_chip;
+
+	irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-irq",
+					dev_name(dev));
+	if (!irq_chip->name)
+		return -ENOMEM;
+	irq_chip->irq_mask = advk_pcie_irq_mask;
+	irq_chip->irq_mask_ack = advk_pcie_irq_mask;
+	irq_chip->irq_unmask = advk_pcie_irq_unmask;
+
+	pcie->irq_domain =
+		irq_domain_add_linear(pcie_intc_node, LEGACY_IRQ_NUM,
+				      &advk_pcie_irq_domain_ops, pcie);
+	if (!pcie->irq_domain) {
+		dev_err(dev, "Failed to get a INTx IRQ domain\n");
+		return PTR_ERR(pcie->irq_domain);
+	}
+
+	msi_irq_chip = &pcie->msi_irq_chip;
+
+	msi_irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-msi",
+						 dev_name(dev));
+	if (!msi_irq_chip->name) {
+		irq_domain_remove(pcie->irq_domain);
+		return -ENOMEM;
+	}
+
+	msi_irq_chip->irq_enable = pci_msi_unmask_irq;
+	msi_irq_chip->irq_disable = pci_msi_mask_irq;
+	msi_irq_chip->irq_mask = pci_msi_mask_irq;
+	msi_irq_chip->irq_unmask = pci_msi_unmask_irq;
+
+	msi = &pcie->msi;
+
+	msi->setup_irq = advk_pcie_setup_msi_irq;
+	msi->teardown_irq = advk_pcie_teardown_msi_irq;
+	msi->of_node = node;
+
+	mutex_init(&pcie->msi_used_lock);
+
+	msi_msg_phys = virt_to_phys(&pcie->msi_msg);
+
+	advk_writel(pcie, lower_32_bits(msi_msg_phys),
+		    PCIE_MSI_ADDR_LOW_REG);
+	advk_writel(pcie, upper_32_bits(msi_msg_phys),
+		    PCIE_MSI_ADDR_HIGH_REG);
+
+	pcie->msi_domain =
+		irq_domain_add_linear(NULL, MSI_IRQ_NUM,
+				      &advk_pcie_msi_irq_ops, pcie);
+	if (!pcie->msi_domain) {
+		irq_domain_remove(pcie->irq_domain);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static void advk_pcie_handle_msi(struct advk_pcie *pcie)
+{
+	u32 msi_val, msi_mask, msi_status, msi_idx;
+	u16 msi_data;
+
+	msi_mask = advk_readl(pcie, PCIE_MSI_MASK_REG);
+	msi_val = advk_readl(pcie, PCIE_MSI_STATUS_REG);
+	msi_status = msi_val & ~msi_mask;
+
+	for (msi_idx = 0; msi_idx < MSI_IRQ_NUM; msi_idx++) {
+		if (!(BIT(msi_idx) & msi_status))
+			continue;
+
+		advk_writel(pcie, BIT(msi_idx), PCIE_MSI_STATUS_REG);
+		msi_data = advk_readl(pcie, PCIE_MSI_PAYLOAD_REG) & 0xFF;
+		generic_handle_irq(msi_data);
+	}
+
+	advk_writel(pcie, PCIE_ISR0_MSI_INT_PENDING,
+		    PCIE_ISR0_REG);
+}
+
+static void advk_pcie_handle_int(struct advk_pcie *pcie)
+{
+	u32 val, mask, status;
+
+	val = advk_readl(pcie, PCIE_ISR0_REG);
+	mask = advk_readl(pcie, PCIE_ISR0_MASK_REG);
+	status = val & ((~mask) & PCIE_ISR0_ALL_MASK);
+
+	if (!status) {
+		advk_writel(pcie, val, PCIE_ISR0_REG);
+		return;
+	}
+
+	/* Process MSI interrupts */
+	if (status & PCIE_ISR0_MSI_INT_PENDING)
+		advk_pcie_handle_msi(pcie);
+
+	/* Process legacy interrupts */
+	do {
+		int i;
+
+		/*
+		 * As documented in the datasheet, we need to raise an
+		 * interrupt for each ASSERT(i) bit that is set, and
+		 * continue to deliver the interrupt until the
+		 * DEASSERT(i) bit gets set
+		 */
+		for (i = 0; i < LEGACY_IRQ_NUM; i++) {
+			if (!(status & PCIE_ISR0_INTX_ASSERT(i)))
+				continue;
+
+			do {
+				int virq;
+
+				advk_writel(pcie, PCIE_ISR0_INTX_ASSERT(i),
+					    PCIE_ISR0_REG);
+				virq = irq_find_mapping(pcie->irq_domain, i);
+				generic_handle_irq(virq);
+				status = advk_readl(pcie, PCIE_ISR0_REG);
+			} while (!(status & PCIE_ISR0_INTX_DEASSERT(i)));
+
+			advk_writel(pcie, PCIE_ISR0_INTX_DEASSERT(i),
+				    PCIE_ISR0_REG);
+		}
+
+		status = advk_readl(pcie, PCIE_ISR0_REG);
+	} while (status & PCIE_ISR0_INTX_MASK);
+}
+
+static irqreturn_t advk_pcie_irq_handler(int irq, void *arg)
+{
+	struct advk_pcie *pcie = arg;
+	u32 status;
+
+	status = advk_readl(pcie, HOST_CTRL_INT_STATUS_REG);
+	if (!(status & PCIE_IRQ_CORE_INT))
+		return IRQ_NONE;
+
+	advk_pcie_handle_int(pcie);
+
+	/* Clear interrupt */
+	advk_writel(pcie, PCIE_IRQ_CORE_INT, HOST_CTRL_INT_STATUS_REG);
+
+	return IRQ_HANDLED;
+}
+
+static void advk_pcie_release_of_pci_ranges(struct advk_pcie *pcie)
+{
+	pci_free_resource_list(&pcie->resources);
+}
+
+static int advk_pcie_parse_request_of_pci_ranges(struct advk_pcie *pcie)
+{
+	int err, res_valid = 0;
+	struct device *dev = &pcie->pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct resource_entry *win;
+	resource_size_t iobase;
+
+	INIT_LIST_HEAD(&pcie->resources);
+
+	err = of_pci_get_host_bridge_resources(np, 0, 0xff, &pcie->resources,
+					       &iobase);
+	if (err)
+		return err;
+
+	resource_list_for_each_entry(win, &pcie->resources) {
+		struct resource *parent, *res = win->res;
+
+		switch (resource_type(res)) {
+		case IORESOURCE_IO:
+			parent = &ioport_resource;
+			advk_pcie_set_ob_win(pcie, 1,
+					     upper_32_bits(res->start),
+					     lower_32_bits(res->start),
+					     0,	0xF8000000, 0,
+					     lower_32_bits(res->start),
+					     OB_PCIE_IO);
+			err = pci_remap_iospace(res, iobase);
+			if (err) {
+				dev_warn(dev, "error %d: failed to map resource %pR\n",
+					 err, res);
+				continue;
+			}
+			break;
+		case IORESOURCE_MEM:
+			parent = &iomem_resource;
+			advk_pcie_set_ob_win(pcie, 0,
+					     upper_32_bits(res->start),
+					     lower_32_bits(res->start),
+					     0x0, 0xF8000000, 0,
+					     lower_32_bits(res->start),
+					     (2 << 20) | OB_PCIE_MEM);
+			res_valid |= !(res->flags & IORESOURCE_PREFETCH);
+			break;
+		default:
+			continue;
+		}
+
+		err = devm_request_resource(dev, parent, res);
+		if (err)
+			goto out_release_res;
+	}
+
+	if (!res_valid) {
+		dev_err(dev, "non-prefetchable memory resource required\n");
+		err = -EINVAL;
+		goto out_release_res;
+	}
+
+	return 0;
+
+out_release_res:
+	advk_pcie_release_of_pci_ranges(pcie);
+	return err;
+}
+
+static int advk_pcie_probe(struct platform_device *pdev)
+{
+	struct advk_pcie *pcie;
+	struct resource *res;
+	struct pci_bus *bus, *child;
+	int ret, irq;
+
+	pcie = devm_kzalloc(&pdev->dev, sizeof(struct advk_pcie),
+			    GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	pcie->pdev = pdev;
+	platform_set_drvdata(pdev, pcie);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pcie->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pcie->base)) {
+		dev_err(&pdev->dev, "failed to map registers\n");
+		return PTR_ERR(pcie->base);
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	ret = devm_request_irq(&pdev->dev, irq, advk_pcie_irq_handler,
+			       IRQF_SHARED | IRQF_NO_THREAD, "advk-pcie",
+			       pcie);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register interrupt\n");
+		return ret;
+	}
+
+	ret = advk_pcie_parse_request_of_pci_ranges(pcie);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to parse resources\n");
+		return ret;
+	}
+
+	advk_pcie_setup_hw(pcie);
+	advk_pcie_enable_axi_addr_gen(pcie, true);
+
+	ret = advk_pcie_init_irq(pcie);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to initialize irq\n");
+		return ret;
+	}
+
+	bus = pci_scan_root_bus_msi(&pdev->dev, 0, &advk_pcie_ops,
+				    pcie, &pcie->resources, &pcie->msi);
+	if (!bus)
+		return -ENOMEM;
+
+	pci_bus_assign_resources(bus);
+
+	list_for_each_entry(child, &bus->children, node)
+		pcie_bus_configure_settings(child);
+
+	pci_bus_add_devices(bus);
+
+	return 0;
+}
+
+static const struct of_device_id advk_pcie_of_match_table[] = {
+	{ .compatible = "marvell,armada-3700-pcie", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, advk_pcie_of_match_table);
+
+static struct platform_driver advk_pcie_driver = {
+	.driver = {
+		.name = "advk-pcie",
+		.of_match_table = advk_pcie_of_match_table,
+		/* Driver unloading/unbinding currently not supported */
+		.suppress_bind_attrs = true,
+	},
+	.probe = advk_pcie_probe,
+};
+module_platform_driver(advk_pcie_driver);
+
+MODULE_AUTHOR("Hezi Shahmoon <hezi.shahmoon@marvell.com>");
+MODULE_DESCRIPTION("Aardvark PCIe driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Arnd Bergmann]

On Thursday, June 2, 2016 11:09:44 AM CEST Thomas Petazzoni wrote:

> +static bool advk_pcie_link_up(struct advk_pcie *pcie)
> +{
> +	int timeout;
> +	u32 ltssm_state;
> +	u32 val;
> +
> +	timeout = PCIE_LINKUP_TIMEOUT;
> +	do {
> +		val = advk_readl(pcie, CFG_REG);
> +		ltssm_state = (val >> LTSSM_SHIFT) & LTSSM_MASK;
> +		timeout--;
> +	/* Use ltssm < LTSSM_L0 instead of ltssm != LTSSM_L0 */
> +	} while (ltssm_state < LTSSM_L0 && timeout > 0);
> +
> +	return (timeout > 0);
> +}

Maybe wait for some amount of time to have passed here (using 
time_before(jiffies, end)) rather than a number of retries,
for robustness and determinism.

> +	/* Point PCIe unit MBUS decode windows to DRAM space. */
> +	for (i = 0; i < 8; i++)
> +		advk_pcie_set_ob_win(pcie, i, 0, 0, 0, 0, 0, 0, 0);

Is this actually mbus based, or is the comment copied from some
other Marvell driver?

> +static int advk_pcie_setup_msi_irq(struct msi_controller *chip,
> +				   struct pci_dev *pdev,
> +				   struct msi_desc *desc)
> +{
> +	struct advk_pcie *pcie = pdev->bus->sysdata;
> +	struct msi_msg msg;
> +	int virq, hwirq;
> +	phys_addr_t msi_msg_phys;
> +
> +	/* We support MSI, but not MSI-X */
> +	if (desc->msi_attrib.is_msix)
> +		return -EINVAL;
> +
> +	hwirq = advk_pcie_alloc_msi(pcie);
> +	if (hwirq < 0)
> +		return hwirq;
> +
> +	virq = irq_create_mapping(pcie->msi_domain, hwirq);
> +	if (!virq) {
> +		advk_pcie_free_msi(pcie, hwirq);
> +		return -EINVAL;
> +	}

What is the version of the GIC in the Armada 3700? If you have GICv3
or GICv2m, could you use that instead of the built-in MSI logic?

We typically handle this using the msi-map or msi-parent properties
pointing to either the gic or the PCI host, depending on which one
you want to use, but either of them should work, and the GIC should
be more efficient because you can distribute the interrupts of the
PCI devices over all CPUs by workload, rather than having to
multiplex all MSI through a single GIC interrupt.

	Arnd

[PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Arnd Bergmann]

On Thursday, June 2, 2016 11:09:44 AM CEST Thomas Petazzoni wrote:

> +static bool advk_pcie_link_up(struct advk_pcie *pcie)
> +{
> +	int timeout;
> +	u32 ltssm_state;
> +	u32 val;
> +
> +	timeout = PCIE_LINKUP_TIMEOUT;
> +	do {
> +		val = advk_readl(pcie, CFG_REG);
> +		ltssm_state = (val >> LTSSM_SHIFT) & LTSSM_MASK;
> +		timeout--;
> +	/* Use ltssm < LTSSM_L0 instead of ltssm != LTSSM_L0 */
> +	} while (ltssm_state < LTSSM_L0 && timeout > 0);
> +
> +	return (timeout > 0);
> +}

Maybe wait for some amount of time to have passed here (using 
time_before(jiffies, end)) rather than a number of retries,
for robustness and determinism.

> +	/* Point PCIe unit MBUS decode windows to DRAM space. */
> +	for (i = 0; i < 8; i++)
> +		advk_pcie_set_ob_win(pcie, i, 0, 0, 0, 0, 0, 0, 0);

Is this actually mbus based, or is the comment copied from some
other Marvell driver?

> +static int advk_pcie_setup_msi_irq(struct msi_controller *chip,
> +				   struct pci_dev *pdev,
> +				   struct msi_desc *desc)
> +{
> +	struct advk_pcie *pcie = pdev->bus->sysdata;
> +	struct msi_msg msg;
> +	int virq, hwirq;
> +	phys_addr_t msi_msg_phys;
> +
> +	/* We support MSI, but not MSI-X */
> +	if (desc->msi_attrib.is_msix)
> +		return -EINVAL;
> +
> +	hwirq = advk_pcie_alloc_msi(pcie);
> +	if (hwirq < 0)
> +		return hwirq;
> +
> +	virq = irq_create_mapping(pcie->msi_domain, hwirq);
> +	if (!virq) {
> +		advk_pcie_free_msi(pcie, hwirq);
> +		return -EINVAL;
> +	}

What is the version of the GIC in the Armada 3700? If you have GICv3
or GICv2m, could you use that instead of the built-in MSI logic?

We typically handle this using the msi-map or msi-parent properties
pointing to either the gic or the PCI host, depending on which one
you want to use, but either of them should work, and the GIC should
be more efficient because you can distribute the interrupts of the
PCI devices over all CPUs by workload, rather than having to
multiplex all MSI through a single GIC interrupt.

	Arnd

Re: [PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Thomas Petazzoni]

Hello,

On Thu, 02 Jun 2016 11:46:04 +0200, Arnd Bergmann wrote:

> > +	timeout = PCIE_LINKUP_TIMEOUT;
> > +	do {
> > +		val = advk_readl(pcie, CFG_REG);
> > +		ltssm_state = (val >> LTSSM_SHIFT) & LTSSM_MASK;
> > +		timeout--;
> > +	/* Use ltssm < LTSSM_L0 instead of ltssm != LTSSM_L0 */
> > +	} while (ltssm_state < LTSSM_L0 && timeout > 0);
> > +
> > +	return (timeout > 0);
> > +}  
> 
> Maybe wait for some amount of time to have passed here (using 
> time_before(jiffies, end)) rather than a number of retries,
> for robustness and determinism.

Will. There was another similar loop in the driver which was doing a
number of retries rather than a timeout-bounded loop, so I've fixed
that one as well.

> > +	/* Point PCIe unit MBUS decode windows to DRAM space. */
> > +	for (i = 0; i < 8; i++)
> > +		advk_pcie_set_ob_win(pcie, i, 0, 0, 0, 0, 0, 0, 0);  
> 
> Is this actually mbus based, or is the comment copied from some
> other Marvell driver?

No, it's not MBUS based, the address decoding logic is very different
from previous mvebu SoCs, I guess it's just a "familiar" name to say
that it's configuring address decoding. I'll remove the comment.

> What is the version of the GIC in the Armada 3700? If you have GICv3
> or GICv2m, could you use that instead of the built-in MSI logic?
> 
> We typically handle this using the msi-map or msi-parent properties
> pointing to either the gic or the PCI host, depending on which one
> you want to use, but either of them should work, and the GIC should
> be more efficient because you can distribute the interrupts of the
> PCI devices over all CPUs by workload, rather than having to
> multiplex all MSI through a single GIC interrupt.

There is a GIC-500, but Marcin told me that attempts to use MSI with it
have not been successful so far. There will be investigation on this
topic in the future, but for the moment, we'd like to have the MSI
functionality built into the PCIe driver supported. We can migrate
later to GIC-500 powered MSIs once working.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Thomas Petazzoni]

Hello,

On Thu, 02 Jun 2016 11:46:04 +0200, Arnd Bergmann wrote:

> > +	timeout = PCIE_LINKUP_TIMEOUT;
> > +	do {
> > +		val = advk_readl(pcie, CFG_REG);
> > +		ltssm_state = (val >> LTSSM_SHIFT) & LTSSM_MASK;
> > +		timeout--;
> > +	/* Use ltssm < LTSSM_L0 instead of ltssm != LTSSM_L0 */
> > +	} while (ltssm_state < LTSSM_L0 && timeout > 0);
> > +
> > +	return (timeout > 0);
> > +}  
> 
> Maybe wait for some amount of time to have passed here (using 
> time_before(jiffies, end)) rather than a number of retries,
> for robustness and determinism.

Will. There was another similar loop in the driver which was doing a
number of retries rather than a timeout-bounded loop, so I've fixed
that one as well.

> > +	/* Point PCIe unit MBUS decode windows to DRAM space. */
> > +	for (i = 0; i < 8; i++)
> > +		advk_pcie_set_ob_win(pcie, i, 0, 0, 0, 0, 0, 0, 0);  
> 
> Is this actually mbus based, or is the comment copied from some
> other Marvell driver?

No, it's not MBUS based, the address decoding logic is very different
from previous mvebu SoCs, I guess it's just a "familiar" name to say
that it's configuring address decoding. I'll remove the comment.

> What is the version of the GIC in the Armada 3700? If you have GICv3
> or GICv2m, could you use that instead of the built-in MSI logic?
> 
> We typically handle this using the msi-map or msi-parent properties
> pointing to either the gic or the PCI host, depending on which one
> you want to use, but either of them should work, and the GIC should
> be more efficient because you can distribute the interrupts of the
> PCI devices over all CPUs by workload, rather than having to
> multiplex all MSI through a single GIC interrupt.

There is a GIC-500, but Marcin told me that attempts to use MSI with it
have not been successful so far. There will be investigation on this
topic in the future, but for the moment, we'd like to have the MSI
functionality built into the PCIe driver supported. We can migrate
later to GIC-500 powered MSIs once working.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Arnd Bergmann]

On Thursday, June 9, 2016 11:19:21 AM CEST Thomas Petazzoni wrote:
> > What is the version of the GIC in the Armada 3700? If you have GICv3
> > or GICv2m, could you use that instead of the built-in MSI logic?
> > 
> > We typically handle this using the msi-map or msi-parent properties
> > pointing to either the gic or the PCI host, depending on which one
> > you want to use, but either of them should work, and the GIC should
> > be more efficient because you can distribute the interrupts of the
> > PCI devices over all CPUs by workload, rather than having to
> > multiplex all MSI through a single GIC interrupt.
> 
> There is a GIC-500, but Marcin told me that attempts to use MSI with it
> have not been successful so far. There will be investigation on this
> topic in the future, but for the moment, we'd like to have the MSI
> functionality built into the PCIe driver supported. We can migrate
> later to GIC-500 powered MSIs once working.

I think you should still add the msi-parent or msi-map properties then,
just have them point at the PCI host node instead of the GIC-500,
and evaluate that to get the built-in controller.

	Arnd

[PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Arnd Bergmann]

On Thursday, June 9, 2016 11:19:21 AM CEST Thomas Petazzoni wrote:
> > What is the version of the GIC in the Armada 3700? If you have GICv3
> > or GICv2m, could you use that instead of the built-in MSI logic?
> > 
> > We typically handle this using the msi-map or msi-parent properties
> > pointing to either the gic or the PCI host, depending on which one
> > you want to use, but either of them should work, and the GIC should
> > be more efficient because you can distribute the interrupts of the
> > PCI devices over all CPUs by workload, rather than having to
> > multiplex all MSI through a single GIC interrupt.
> 
> There is a GIC-500, but Marcin told me that attempts to use MSI with it
> have not been successful so far. There will be investigation on this
> topic in the future, but for the moment, we'd like to have the MSI
> functionality built into the PCIe driver supported. We can migrate
> later to GIC-500 powered MSIs once working.

I think you should still add the msi-parent or msi-map properties then,
just have them point at the PCI host node instead of the GIC-500,
and evaluate that to get the built-in controller.

	Arnd

Re: [PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Thomas Petazzoni]

Hello,

On Thu, 09 Jun 2016 14:19:07 +0200, Arnd Bergmann wrote:

> > There is a GIC-500, but Marcin told me that attempts to use MSI with it
> > have not been successful so far. There will be investigation on this
> > topic in the future, but for the moment, we'd like to have the MSI
> > functionality built into the PCIe driver supported. We can migrate
> > later to GIC-500 powered MSIs once working.  
> 
> I think you should still add the msi-parent or msi-map properties then,
> just have them point at the PCI host node instead of the GIC-500,
> and evaluate that to get the built-in controller.

Sure, I'll have a look at doing that.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Thomas Petazzoni]

Hello,

On Thu, 09 Jun 2016 14:19:07 +0200, Arnd Bergmann wrote:

> > There is a GIC-500, but Marcin told me that attempts to use MSI with it
> > have not been successful so far. There will be investigation on this
> > topic in the future, but for the moment, we'd like to have the MSI
> > functionality built into the PCIe driver supported. We can migrate
> > later to GIC-500 powered MSIs once working.  
> 
> I think you should still add the msi-parent or msi-map properties then,
> just have them point at the PCI host node instead of the GIC-500,
> and evaluate that to get the built-in controller.

Sure, I'll have a look at doing that.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4804 bytes --]

Hi,

[auto build test ERROR on pci/next]
[also build test ERROR on v4.7-rc1 next-20160603]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Thomas-Petazzoni/PCIe-controller-driver-for-Marvell-Armada-3700/20160602-171515
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-lkp (attached as .config)
compiler: gcc-4.9 (Debian 4.9.3-14) 4.9.3
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> drivers/of/Kconfig:4:error: recursive dependency detected!
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/of/Kconfig:4:	symbol OF is selected by X86_INTEL_CE
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:501:	symbol X86_INTEL_CE depends on X86_IO_APIC
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:970:	symbol X86_IO_APIC depends on X86_LOCAL_APIC
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:964:	symbol X86_LOCAL_APIC depends on X86_UP_APIC
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:939:	symbol X86_UP_APIC depends on PCI_MSI
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/pci/Kconfig:11:	symbol PCI_MSI is selected by PCI_AARDVARK
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/pci/host/Kconfig:19:	symbol PCI_AARDVARK depends on OF
--
>> drivers/of/Kconfig:4:error: recursive dependency detected!
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/of/Kconfig:4:	symbol OF is selected by X86_INTEL_CE
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:501:	symbol X86_INTEL_CE depends on X86_IO_APIC
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:970:	symbol X86_IO_APIC depends on X86_LOCAL_APIC
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:964:	symbol X86_LOCAL_APIC depends on X86_UP_APIC
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:939:	symbol X86_UP_APIC depends on PCI_MSI
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/pci/Kconfig:11:	symbol PCI_MSI is selected by PCI_AARDVARK
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/pci/host/Kconfig:19:	symbol PCI_AARDVARK depends on OF
   .config:429:warning: symbol value 'm' invalid for MICROCODE
   .config:2991:warning: symbol value 'm' invalid for FAULT_INJECTION

vim +4 drivers/of/Kconfig

5ab5fc7e Grant Likely     2010-07-05   1  config DTC
5ab5fc7e Grant Likely     2010-07-05   2  	bool
5ab5fc7e Grant Likely     2010-07-05   3  
0166dc11 Rob Herring      2015-05-28  @4  menuconfig OF
0166dc11 Rob Herring      2015-05-28   5  	bool "Device Tree and Open Firmware support"
0166dc11 Rob Herring      2015-05-28   6  	help
0166dc11 Rob Herring      2015-05-28   7  	  This option enables the device tree infrastructure.
0166dc11 Rob Herring      2015-05-28   8  	  It is automatically selected by platforms that need it or can
0166dc11 Rob Herring      2015-05-28   9  	  be enabled manually for unittests, overlays or
0166dc11 Rob Herring      2015-05-28  10  	  compile-coverage.
bcbefae2 Stephen Rothwell 2010-06-29  11  
0166dc11 Rob Herring      2015-05-28  12  if OF

:::::: The code at line 4 was first introduced by commit
:::::: 0166dc11be911213e0b1b764488c671be4c48cf3 of: make CONFIG_OF user selectable

:::::: TO: Rob Herring <robh@kernel.org>
:::::: CC: Rob Herring <robh@kernel.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 23344 bytes --]

[PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[kbuild test robot]

Hi,

[auto build test ERROR on pci/next]
[also build test ERROR on v4.7-rc1 next-20160603]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Thomas-Petazzoni/PCIe-controller-driver-for-Marvell-Armada-3700/20160602-171515
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-lkp (attached as .config)
compiler: gcc-4.9 (Debian 4.9.3-14) 4.9.3
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> drivers/of/Kconfig:4:error: recursive dependency detected!
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/of/Kconfig:4:	symbol OF is selected by X86_INTEL_CE
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:501:	symbol X86_INTEL_CE depends on X86_IO_APIC
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:970:	symbol X86_IO_APIC depends on X86_LOCAL_APIC
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:964:	symbol X86_LOCAL_APIC depends on X86_UP_APIC
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:939:	symbol X86_UP_APIC depends on PCI_MSI
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/pci/Kconfig:11:	symbol PCI_MSI is selected by PCI_AARDVARK
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/pci/host/Kconfig:19:	symbol PCI_AARDVARK depends on OF
--
>> drivers/of/Kconfig:4:error: recursive dependency detected!
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/of/Kconfig:4:	symbol OF is selected by X86_INTEL_CE
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:501:	symbol X86_INTEL_CE depends on X86_IO_APIC
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:970:	symbol X86_IO_APIC depends on X86_LOCAL_APIC
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:964:	symbol X86_LOCAL_APIC depends on X86_UP_APIC
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   arch/x86/Kconfig:939:	symbol X86_UP_APIC depends on PCI_MSI
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/pci/Kconfig:11:	symbol PCI_MSI is selected by PCI_AARDVARK
   For a resolution refer to Documentation/kbuild/kconfig-language.txt
   subsection "Kconfig recursive dependency limitations"
   drivers/pci/host/Kconfig:19:	symbol PCI_AARDVARK depends on OF
   .config:429:warning: symbol value 'm' invalid for MICROCODE
   .config:2991:warning: symbol value 'm' invalid for FAULT_INJECTION

vim +4 drivers/of/Kconfig

5ab5fc7e Grant Likely     2010-07-05   1  config DTC
5ab5fc7e Grant Likely     2010-07-05   2  	bool
5ab5fc7e Grant Likely     2010-07-05   3  
0166dc11 Rob Herring      2015-05-28  @4  menuconfig OF
0166dc11 Rob Herring      2015-05-28   5  	bool "Device Tree and Open Firmware support"
0166dc11 Rob Herring      2015-05-28   6  	help
0166dc11 Rob Herring      2015-05-28   7  	  This option enables the device tree infrastructure.
0166dc11 Rob Herring      2015-05-28   8  	  It is automatically selected by platforms that need it or can
0166dc11 Rob Herring      2015-05-28   9  	  be enabled manually for unittests, overlays or
0166dc11 Rob Herring      2015-05-28  10  	  compile-coverage.
bcbefae2 Stephen Rothwell 2010-06-29  11  
0166dc11 Rob Herring      2015-05-28  12  if OF

:::::: The code at line 4 was first introduced by commit
:::::: 0166dc11be911213e0b1b764488c671be4c48cf3 of: make CONFIG_OF user selectable

:::::: TO: Rob Herring <robh@kernel.org>
:::::: CC: Rob Herring <robh@kernel.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/octet-stream
Size: 23344 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160604/1aa84bf0/attachment-0001.obj>

Re: [PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Marcin Wojtas]

Hi Thomas,


> +static const struct irq_domain_ops advk_pcie_msi_irq_ops = {
> +       .map = advk_pcie_msi_map,
> +};
> +
> +static void advk_pcie_irq_mask(struct irq_data *d)
> +{
> +       struct advk_pcie *pcie = d->domain->host_data;
> +       irq_hw_number_t hwirq = irqd_to_hwirq(d);
> +       u32 mask;
> +
> +       mask = advk_readl(pcie, PCIE_ISR0_MASK_REG);
> +       mask |= PCIE_ISR0_INTX_ASSERT(hwirq) |
> +               PCIE_ISR0_INTX_DEASSERT(hwirq);
> +       advk_writel(pcie, mask, PCIE_ISR0_MASK_REG);
> +}
> +
> +static void advk_pcie_irq_unmask(struct irq_data *d)
> +{
> +       struct advk_pcie *pcie = d->domain->host_data;
> +       irq_hw_number_t hwirq = irqd_to_hwirq(d);
> +       u32 mask;
> +
> +       mask = advk_readl(pcie, PCIE_ISR0_MASK_REG);
> +       mask &= ~(PCIE_ISR0_INTX_ASSERT(hwirq) |
> +                 PCIE_ISR0_INTX_DEASSERT(hwirq));

Have you checked my latest improvements of interrupt handling in this
controller? As DEASSERT is polled in advk_pcie_handle_int() I don't
think it should be able to trigger root complex interrupt at all.
Hence I think it shouldn't be unmasked.

> +       advk_writel(pcie, mask, PCIE_ISR0_MASK_REG);
> +}
> +
> +static int advk_pcie_irq_map(struct irq_domain *h,
> +                            unsigned int virq, irq_hw_number_t hwirq)
> +{
> +       struct advk_pcie *pcie = h->host_data;
> +
> +       advk_pcie_irq_mask(irq_get_irq_data(virq));
> +       irq_set_status_flags(virq, IRQ_LEVEL);
> +       irq_set_chip_and_handler(virq, &pcie->irq_chip,
> +                                handle_level_irq);
> +       irq_set_chip_data(virq, pcie);
> +
> +       return 0;
> +}
> +
> +static const struct irq_domain_ops advk_pcie_irq_domain_ops = {
> +       .map = advk_pcie_irq_map,
> +       .xlate = irq_domain_xlate_onecell,
> +};
> +
> +static int advk_pcie_init_irq(struct advk_pcie *pcie)
> +{
> +       struct device *dev = &pcie->pdev->dev;
> +       struct device_node *node = dev->of_node;
> +       struct device_node *pcie_intc_node;
> +       struct irq_chip *irq_chip, *msi_irq_chip;
> +       struct msi_controller *msi;
> +       phys_addr_t msi_msg_phys;
> +
> +       pcie_intc_node =  of_get_next_child(node, NULL);
> +       if (!pcie_intc_node) {
> +               dev_err(dev, "No PCIe Intc node found\n");
> +               return PTR_ERR(pcie_intc_node);
> +       }
> +
> +       irq_chip = &pcie->irq_chip;
> +
> +       irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-irq",
> +                                       dev_name(dev));
> +       if (!irq_chip->name)
> +               return -ENOMEM;
> +       irq_chip->irq_mask = advk_pcie_irq_mask;
> +       irq_chip->irq_mask_ack = advk_pcie_irq_mask;
> +       irq_chip->irq_unmask = advk_pcie_irq_unmask;
> +
> +       pcie->irq_domain =
> +               irq_domain_add_linear(pcie_intc_node, LEGACY_IRQ_NUM,
> +                                     &advk_pcie_irq_domain_ops, pcie);
> +       if (!pcie->irq_domain) {
> +               dev_err(dev, "Failed to get a INTx IRQ domain\n");
> +               return PTR_ERR(pcie->irq_domain);
> +       }
> +
> +       msi_irq_chip = &pcie->msi_irq_chip;
> +
> +       msi_irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-msi",
> +                                                dev_name(dev));
> +       if (!msi_irq_chip->name) {
> +               irq_domain_remove(pcie->irq_domain);
> +               return -ENOMEM;
> +       }
> +
> +       msi_irq_chip->irq_enable = pci_msi_unmask_irq;
> +       msi_irq_chip->irq_disable = pci_msi_mask_irq;
> +       msi_irq_chip->irq_mask = pci_msi_mask_irq;
> +       msi_irq_chip->irq_unmask = pci_msi_unmask_irq;
> +
> +       msi = &pcie->msi;

Do we consciously resign from CONFIG_MSI dependency? I know it should
always be enabled, but just making sure.

> +
> +       msi->setup_irq = advk_pcie_setup_msi_irq;
> +       msi->teardown_irq = advk_pcie_teardown_msi_irq;
> +       msi->of_node = node;
> +
> +       mutex_init(&pcie->msi_used_lock);
> +
> +       msi_msg_phys = virt_to_phys(&pcie->msi_msg);
> +
> +       advk_writel(pcie, lower_32_bits(msi_msg_phys),
> +                   PCIE_MSI_ADDR_LOW_REG);
> +       advk_writel(pcie, upper_32_bits(msi_msg_phys),
> +                   PCIE_MSI_ADDR_HIGH_REG);
> +
> +       pcie->msi_domain =
> +               irq_domain_add_linear(NULL, MSI_IRQ_NUM,
> +                                     &advk_pcie_msi_irq_ops, pcie);
> +       if (!pcie->msi_domain) {
> +               irq_domain_remove(pcie->irq_domain);
> +               return -ENOMEM;
> +       }
> +
> +       return 0;
> +}
> +
> +static void advk_pcie_handle_msi(struct advk_pcie *pcie)
> +{
> +       u32 msi_val, msi_mask, msi_status, msi_idx;
> +       u16 msi_data;
> +
> +       msi_mask = advk_readl(pcie, PCIE_MSI_MASK_REG);
> +       msi_val = advk_readl(pcie, PCIE_MSI_STATUS_REG);
> +       msi_status = msi_val & ~msi_mask;
> +
> +       for (msi_idx = 0; msi_idx < MSI_IRQ_NUM; msi_idx++) {
> +               if (!(BIT(msi_idx) & msi_status))
> +                       continue;
> +
> +               advk_writel(pcie, BIT(msi_idx), PCIE_MSI_STATUS_REG);
> +               msi_data = advk_readl(pcie, PCIE_MSI_PAYLOAD_REG) & 0xFF;
> +               generic_handle_irq(msi_data);
> +       }
> +
> +       advk_writel(pcie, PCIE_ISR0_MSI_INT_PENDING,
> +                   PCIE_ISR0_REG);
> +}
> +
> +static void advk_pcie_handle_int(struct advk_pcie *pcie)
> +{
> +       u32 val, mask, status;
> +
> +       val = advk_readl(pcie, PCIE_ISR0_REG);
> +       mask = advk_readl(pcie, PCIE_ISR0_MASK_REG);
> +       status = val & ((~mask) & PCIE_ISR0_ALL_MASK);
> +
> +       if (!status) {
> +               advk_writel(pcie, val, PCIE_ISR0_REG);
> +               return;
> +       }
> +
> +       /* Process MSI interrupts */
> +       if (status & PCIE_ISR0_MSI_INT_PENDING)
> +               advk_pcie_handle_msi(pcie);
> +
> +       /* Process legacy interrupts */
> +       do {
> +               int i;
> +
> +               /*
> +                * As documented in the datasheet, we need to raise an
> +                * interrupt for each ASSERT(i) bit that is set, and
> +                * continue to deliver the interrupt until the
> +                * DEASSERT(i) bit gets set
> +                */

Below code will result in spurious interrupt flood (I can provide test
details with SATA3.0 card). I know this part of specs very well, but
expected EP operation flow should look like this:
a. EP interrupt
b. INTX_ASSERT signal
c. root complex interrupt
d. clear INTX_ASSERT bit
e. execute EP handler
f. INTX_DEASSERT signal
g. clear INTX_DEASSERT bit
h. exit interrupt

Now between d. and f., in case of any shortest delay we will force
executing EP handler numerous of times, which IMO is not desired. All
of them for e.g. the SATA3.0 card are registered as spurious
interrupts, pretty quickly resulting in "switching to irq poll"
failure after 100k of such happenings. Actually I think that after
executing generic_handle_irq(virq); we should leave root complex isr
asap, the polling for DEASSERT in such case is not resulting in any
benefits. In my code I ignored DEASSERT signal existence - it neither
triggers interrupt nor we don't poll for it. Under big stress nothing
wrong happens. We rely only on interrupts triggered by INTX_ASSERT
signal, so we care only for its status.

> +               for (i = 0; i < LEGACY_IRQ_NUM; i++) {
> +                       if (!(status & PCIE_ISR0_INTX_ASSERT(i)))
> +                               continue;
> +
> +                       do {
> +                               int virq;
> +
> +                               advk_writel(pcie, PCIE_ISR0_INTX_ASSERT(i),
> +                                           PCIE_ISR0_REG);
> +                               virq = irq_find_mapping(pcie->irq_domain, i);
> +                               generic_handle_irq(virq);
> +                               status = advk_readl(pcie, PCIE_ISR0_REG);
> +                       } while (!(status & PCIE_ISR0_INTX_DEASSERT(i)));
> +
> +                       advk_writel(pcie, PCIE_ISR0_INTX_DEASSERT(i),
> +                                   PCIE_ISR0_REG);
> +               }
> +
> +               status = advk_readl(pcie, PCIE_ISR0_REG);
> +       } while (status & PCIE_ISR0_INTX_MASK);

How about simplifying code a bit and remove this do-while loop? IMO
it's enough to do single iterating through INTA/B/C/D and exit - in
such case we wouldn't spent too much time in single top half handler.

I'm looking forward to your feedback.

Best regards,
Marcin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Marcin Wojtas]

Hi Thomas,


> +static const struct irq_domain_ops advk_pcie_msi_irq_ops = {
> +       .map = advk_pcie_msi_map,
> +};
> +
> +static void advk_pcie_irq_mask(struct irq_data *d)
> +{
> +       struct advk_pcie *pcie = d->domain->host_data;
> +       irq_hw_number_t hwirq = irqd_to_hwirq(d);
> +       u32 mask;
> +
> +       mask = advk_readl(pcie, PCIE_ISR0_MASK_REG);
> +       mask |= PCIE_ISR0_INTX_ASSERT(hwirq) |
> +               PCIE_ISR0_INTX_DEASSERT(hwirq);
> +       advk_writel(pcie, mask, PCIE_ISR0_MASK_REG);
> +}
> +
> +static void advk_pcie_irq_unmask(struct irq_data *d)
> +{
> +       struct advk_pcie *pcie = d->domain->host_data;
> +       irq_hw_number_t hwirq = irqd_to_hwirq(d);
> +       u32 mask;
> +
> +       mask = advk_readl(pcie, PCIE_ISR0_MASK_REG);
> +       mask &= ~(PCIE_ISR0_INTX_ASSERT(hwirq) |
> +                 PCIE_ISR0_INTX_DEASSERT(hwirq));

Have you checked my latest improvements of interrupt handling in this
controller? As DEASSERT is polled in advk_pcie_handle_int() I don't
think it should be able to trigger root complex interrupt at all.
Hence I think it shouldn't be unmasked.

> +       advk_writel(pcie, mask, PCIE_ISR0_MASK_REG);
> +}
> +
> +static int advk_pcie_irq_map(struct irq_domain *h,
> +                            unsigned int virq, irq_hw_number_t hwirq)
> +{
> +       struct advk_pcie *pcie = h->host_data;
> +
> +       advk_pcie_irq_mask(irq_get_irq_data(virq));
> +       irq_set_status_flags(virq, IRQ_LEVEL);
> +       irq_set_chip_and_handler(virq, &pcie->irq_chip,
> +                                handle_level_irq);
> +       irq_set_chip_data(virq, pcie);
> +
> +       return 0;
> +}
> +
> +static const struct irq_domain_ops advk_pcie_irq_domain_ops = {
> +       .map = advk_pcie_irq_map,
> +       .xlate = irq_domain_xlate_onecell,
> +};
> +
> +static int advk_pcie_init_irq(struct advk_pcie *pcie)
> +{
> +       struct device *dev = &pcie->pdev->dev;
> +       struct device_node *node = dev->of_node;
> +       struct device_node *pcie_intc_node;
> +       struct irq_chip *irq_chip, *msi_irq_chip;
> +       struct msi_controller *msi;
> +       phys_addr_t msi_msg_phys;
> +
> +       pcie_intc_node =  of_get_next_child(node, NULL);
> +       if (!pcie_intc_node) {
> +               dev_err(dev, "No PCIe Intc node found\n");
> +               return PTR_ERR(pcie_intc_node);
> +       }
> +
> +       irq_chip = &pcie->irq_chip;
> +
> +       irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-irq",
> +                                       dev_name(dev));
> +       if (!irq_chip->name)
> +               return -ENOMEM;
> +       irq_chip->irq_mask = advk_pcie_irq_mask;
> +       irq_chip->irq_mask_ack = advk_pcie_irq_mask;
> +       irq_chip->irq_unmask = advk_pcie_irq_unmask;
> +
> +       pcie->irq_domain =
> +               irq_domain_add_linear(pcie_intc_node, LEGACY_IRQ_NUM,
> +                                     &advk_pcie_irq_domain_ops, pcie);
> +       if (!pcie->irq_domain) {
> +               dev_err(dev, "Failed to get a INTx IRQ domain\n");
> +               return PTR_ERR(pcie->irq_domain);
> +       }
> +
> +       msi_irq_chip = &pcie->msi_irq_chip;
> +
> +       msi_irq_chip->name = devm_kasprintf(dev, GFP_KERNEL, "%s-msi",
> +                                                dev_name(dev));
> +       if (!msi_irq_chip->name) {
> +               irq_domain_remove(pcie->irq_domain);
> +               return -ENOMEM;
> +       }
> +
> +       msi_irq_chip->irq_enable = pci_msi_unmask_irq;
> +       msi_irq_chip->irq_disable = pci_msi_mask_irq;
> +       msi_irq_chip->irq_mask = pci_msi_mask_irq;
> +       msi_irq_chip->irq_unmask = pci_msi_unmask_irq;
> +
> +       msi = &pcie->msi;

Do we consciously resign from CONFIG_MSI dependency? I know it should
always be enabled, but just making sure.

> +
> +       msi->setup_irq = advk_pcie_setup_msi_irq;
> +       msi->teardown_irq = advk_pcie_teardown_msi_irq;
> +       msi->of_node = node;
> +
> +       mutex_init(&pcie->msi_used_lock);
> +
> +       msi_msg_phys = virt_to_phys(&pcie->msi_msg);
> +
> +       advk_writel(pcie, lower_32_bits(msi_msg_phys),
> +                   PCIE_MSI_ADDR_LOW_REG);
> +       advk_writel(pcie, upper_32_bits(msi_msg_phys),
> +                   PCIE_MSI_ADDR_HIGH_REG);
> +
> +       pcie->msi_domain =
> +               irq_domain_add_linear(NULL, MSI_IRQ_NUM,
> +                                     &advk_pcie_msi_irq_ops, pcie);
> +       if (!pcie->msi_domain) {
> +               irq_domain_remove(pcie->irq_domain);
> +               return -ENOMEM;
> +       }
> +
> +       return 0;
> +}
> +
> +static void advk_pcie_handle_msi(struct advk_pcie *pcie)
> +{
> +       u32 msi_val, msi_mask, msi_status, msi_idx;
> +       u16 msi_data;
> +
> +       msi_mask = advk_readl(pcie, PCIE_MSI_MASK_REG);
> +       msi_val = advk_readl(pcie, PCIE_MSI_STATUS_REG);
> +       msi_status = msi_val & ~msi_mask;
> +
> +       for (msi_idx = 0; msi_idx < MSI_IRQ_NUM; msi_idx++) {
> +               if (!(BIT(msi_idx) & msi_status))
> +                       continue;
> +
> +               advk_writel(pcie, BIT(msi_idx), PCIE_MSI_STATUS_REG);
> +               msi_data = advk_readl(pcie, PCIE_MSI_PAYLOAD_REG) & 0xFF;
> +               generic_handle_irq(msi_data);
> +       }
> +
> +       advk_writel(pcie, PCIE_ISR0_MSI_INT_PENDING,
> +                   PCIE_ISR0_REG);
> +}
> +
> +static void advk_pcie_handle_int(struct advk_pcie *pcie)
> +{
> +       u32 val, mask, status;
> +
> +       val = advk_readl(pcie, PCIE_ISR0_REG);
> +       mask = advk_readl(pcie, PCIE_ISR0_MASK_REG);
> +       status = val & ((~mask) & PCIE_ISR0_ALL_MASK);
> +
> +       if (!status) {
> +               advk_writel(pcie, val, PCIE_ISR0_REG);
> +               return;
> +       }
> +
> +       /* Process MSI interrupts */
> +       if (status & PCIE_ISR0_MSI_INT_PENDING)
> +               advk_pcie_handle_msi(pcie);
> +
> +       /* Process legacy interrupts */
> +       do {
> +               int i;
> +
> +               /*
> +                * As documented in the datasheet, we need to raise an
> +                * interrupt for each ASSERT(i) bit that is set, and
> +                * continue to deliver the interrupt until the
> +                * DEASSERT(i) bit gets set
> +                */

Below code will result in spurious interrupt flood (I can provide test
details with SATA3.0 card). I know this part of specs very well, but
expected EP operation flow should look like this:
a. EP interrupt
b. INTX_ASSERT signal
c. root complex interrupt
d. clear INTX_ASSERT bit
e. execute EP handler
f. INTX_DEASSERT signal
g. clear INTX_DEASSERT bit
h. exit interrupt

Now between d. and f., in case of any shortest delay we will force
executing EP handler numerous of times, which IMO is not desired. All
of them for e.g. the SATA3.0 card are registered as spurious
interrupts, pretty quickly resulting in "switching to irq poll"
failure after 100k of such happenings. Actually I think that after
executing generic_handle_irq(virq); we should leave root complex isr
asap, the polling for DEASSERT in such case is not resulting in any
benefits. In my code I ignored DEASSERT signal existence - it neither
triggers interrupt nor we don't poll for it. Under big stress nothing
wrong happens. We rely only on interrupts triggered by INTX_ASSERT
signal, so we care only for its status.

> +               for (i = 0; i < LEGACY_IRQ_NUM; i++) {
> +                       if (!(status & PCIE_ISR0_INTX_ASSERT(i)))
> +                               continue;
> +
> +                       do {
> +                               int virq;
> +
> +                               advk_writel(pcie, PCIE_ISR0_INTX_ASSERT(i),
> +                                           PCIE_ISR0_REG);
> +                               virq = irq_find_mapping(pcie->irq_domain, i);
> +                               generic_handle_irq(virq);
> +                               status = advk_readl(pcie, PCIE_ISR0_REG);
> +                       } while (!(status & PCIE_ISR0_INTX_DEASSERT(i)));
> +
> +                       advk_writel(pcie, PCIE_ISR0_INTX_DEASSERT(i),
> +                                   PCIE_ISR0_REG);
> +               }
> +
> +               status = advk_readl(pcie, PCIE_ISR0_REG);
> +       } while (status & PCIE_ISR0_INTX_MASK);

How about simplifying code a bit and remove this do-while loop? IMO
it's enough to do single iterating through INTA/B/C/D and exit - in
such case we wouldn't spent too much time in single top half handler.

I'm looking forward to your feedback.

Best regards,
Marcin

Re: [PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Thomas Petazzoni]

Hello,

Thanks for your feedback!

On Wed, 8 Jun 2016 17:15:08 +0200, Marcin Wojtas wrote:

> Have you checked my latest improvements of interrupt handling in this
> controller? As DEASSERT is polled in advk_pcie_handle_int() I don't
> think it should be able to trigger root complex interrupt at all.
> Hence I think it shouldn't be unmasked.

Correct. Makes sense. Though you are suggesting below to no longer poll
the DEASSERT signal in the interrupt handler, but you're instead saying
to ignore it entirely.


> > +       msi = &pcie->msi;  
> 
> Do we consciously resign from CONFIG_MSI dependency? I know it should
> always be enabled, but just making sure.

Yes, I discussed this with Arnd, and we decided to simply make MSI
support a hard dependency. You can always disable it by passing
pci=nomsi on the kernel command line (tested to be working).


> Below code will result in spurious interrupt flood (I can provide test
> details with SATA3.0 card). I know this part of specs very well, but
> expected EP operation flow should look like this:
> a. EP interrupt
> b. INTX_ASSERT signal
> c. root complex interrupt
> d. clear INTX_ASSERT bit
> e. execute EP handler
> f. INTX_DEASSERT signal
> g. clear INTX_DEASSERT bit
> h. exit interrupt
> 
> Now between d. and f., in case of any shortest delay we will force
> executing EP handler numerous of times, which IMO is not desired. All
> of them for e.g. the SATA3.0 card are registered as spurious
> interrupts, pretty quickly resulting in "switching to irq poll"
> failure after 100k of such happenings. Actually I think that after
> executing generic_handle_irq(virq); we should leave root complex isr
> asap, the polling for DEASSERT in such case is not resulting in any
> benefits. In my code I ignored DEASSERT signal existence - it neither
> triggers interrupt nor we don't poll for it. Under big stress nothing
> wrong happens. We rely only on interrupts triggered by INTX_ASSERT
> signal, so we care only for its status.

Thanks for the detailed explanation. So in practice, the code should be
just:

	for (i = 0; i < LEGACY_IRQ_NUM; i++) {
		int virq;

		if (!(status & PCIE_ISR0_INTX_ASSERT(i)))
			continue;

		advk_writel(pcie, PCIE_ISR0_INTX_ASSERT(i), PCIE_ISR0_REG);
		virq = irq_find_mapping(pcie->irq_domain, i);
		generic_handle_irq(virq);
	}

and that's it.

Is that what you suggest?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] PCI: host: new PCI host controller driver for Marvell Armada 3700

[Thomas Petazzoni]

Hello,

Thanks for your feedback!

On Wed, 8 Jun 2016 17:15:08 +0200, Marcin Wojtas wrote:

> Have you checked my latest improvements of interrupt handling in this
> controller? As DEASSERT is polled in advk_pcie_handle_int() I don't
> think it should be able to trigger root complex interrupt at all.
> Hence I think it shouldn't be unmasked.

Correct. Makes sense. Though you are suggesting below to no longer poll
the DEASSERT signal in the interrupt handler, but you're instead saying
to ignore it entirely.


> > +       msi = &pcie->msi;  
> 
> Do we consciously resign from CONFIG_MSI dependency? I know it should
> always be enabled, but just making sure.

Yes, I discussed this with Arnd, and we decided to simply make MSI
support a hard dependency. You can always disable it by passing
pci=nomsi on the kernel command line (tested to be working).


> Below code will result in spurious interrupt flood (I can provide test
> details with SATA3.0 card). I know this part of specs very well, but
> expected EP operation flow should look like this:
> a. EP interrupt
> b. INTX_ASSERT signal
> c. root complex interrupt
> d. clear INTX_ASSERT bit
> e. execute EP handler
> f. INTX_DEASSERT signal
> g. clear INTX_DEASSERT bit
> h. exit interrupt
> 
> Now between d. and f., in case of any shortest delay we will force
> executing EP handler numerous of times, which IMO is not desired. All
> of them for e.g. the SATA3.0 card are registered as spurious
> interrupts, pretty quickly resulting in "switching to irq poll"
> failure after 100k of such happenings. Actually I think that after
> executing generic_handle_irq(virq); we should leave root complex isr
> asap, the polling for DEASSERT in such case is not resulting in any
> benefits. In my code I ignored DEASSERT signal existence - it neither
> triggers interrupt nor we don't poll for it. Under big stress nothing
> wrong happens. We rely only on interrupts triggered by INTX_ASSERT
> signal, so we care only for its status.

Thanks for the detailed explanation. So in practice, the code should be
just:

	for (i = 0; i < LEGACY_IRQ_NUM; i++) {
		int virq;

		if (!(status & PCIE_ISR0_INTX_ASSERT(i)))
			continue;

		advk_writel(pcie, PCIE_ISR0_INTX_ASSERT(i), PCIE_ISR0_REG);
		virq = irq_find_mapping(pcie->irq_domain, i);
		generic_handle_irq(virq);
	}

and that's it.

Is that what you suggest?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 3/3] arm64: dts: marvell: PCIe support for Armada 3700

[Thomas Petazzoni]

This commit adds the SoC-level description of the PCIe controller found
on the Marvell Armada 3700, and also enables this PCIe controller on the
development board for this SoC.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-3720-db.dts |  5 +++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi   | 23 +++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 86110a6..1372e9a6 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -76,3 +76,8 @@
 &usb3 {
 	status = "okay";
 };
+
+/* CON17 (PCIe) / CON12 (mini-PCIe) */
+&pcie0 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 9e2efb8..01d3b20 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -141,5 +141,28 @@
 				      <0x1d40000 0x40000>; /* GICR */
 			};
 		};
+
+		pcie0: pcie@d0070000 {
+			compatible = "marvell,armada-3700-pcie";
+			device_type = "pci";
+			status = "disabled";
+			reg = <0 0xd0070000 0 0x20000>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			bus-range = <0x00 0xff>;
+			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+			#interrupt-cells = <1>;
+			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
+				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
+			interrupt-map-mask = <0 0 0 7>;
+			interrupt-map = <0 0 0 1 &pcie_intc 0>,
+					<0 0 0 2 &pcie_intc 1>,
+					<0 0 0 3 &pcie_intc 2>,
+					<0 0 0 4 &pcie_intc 3>;
+			pcie_intc: interrupt-controller {
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
 	};
 };
-- 
2.7.4

[PATCH 3/3] arm64: dts: marvell: PCIe support for Armada 3700

[Thomas Petazzoni]

This commit adds the SoC-level description of the PCIe controller found
on the Marvell Armada 3700, and also enables this PCIe controller on the
development board for this SoC.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm64/boot/dts/marvell/armada-3720-db.dts |  5 +++++
 arch/arm64/boot/dts/marvell/armada-37xx.dtsi   | 23 +++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-3720-db.dts b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
index 86110a6..1372e9a6 100644
--- a/arch/arm64/boot/dts/marvell/armada-3720-db.dts
+++ b/arch/arm64/boot/dts/marvell/armada-3720-db.dts
@@ -76,3 +76,8 @@
 &usb3 {
 	status = "okay";
 };
+
+/* CON17 (PCIe) / CON12 (mini-PCIe) */
+&pcie0 {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
index 9e2efb8..01d3b20 100644
--- a/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-37xx.dtsi
@@ -141,5 +141,28 @@
 				      <0x1d40000 0x40000>; /* GICR */
 			};
 		};
+
+		pcie0: pcie at d0070000 {
+			compatible = "marvell,armada-3700-pcie";
+			device_type = "pci";
+			status = "disabled";
+			reg = <0 0xd0070000 0 0x20000>;
+			#address-cells = <3>;
+			#size-cells = <2>;
+			bus-range = <0x00 0xff>;
+			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+			#interrupt-cells = <1>;
+			ranges = <0x82000000 0 0xe8000000   0 0xe8000000 0 0x1000000 /* Port 0 MEM */
+				  0x81000000 0 0xe9000000   0 0xe9000000 0 0x10000>; /* Port 0 IO*/
+			interrupt-map-mask = <0 0 0 7>;
+			interrupt-map = <0 0 0 1 &pcie_intc 0>,
+					<0 0 0 2 &pcie_intc 1>,
+					<0 0 0 3 &pcie_intc 2>,
+					<0 0 0 4 &pcie_intc 3>;
+			pcie_intc: interrupt-controller {
+				interrupt-controller;
+				#interrupt-cells = <1>;
+			};
+		};
 	};
 };
-- 
2.7.4