[PATCH v1 0/4] Add dbi2 and atu for i.MX8M PCIe EP

[PATCH v1 0/4] Add dbi2 and atu for i.MX8M PCIe EP

[Richard Zhu]

Ideally, dbi2 and atu base addresses should be fetched from DT.
Add dbi2 and atu base addresses for i.MX8M PCIe EP here.

[PATCH v1 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and "atu"
[PATCH v1 2/4] dts: arm64: imx8mq: Add dbi2 and atu reg for i.MX8MQ
[PATCH v1 3/4] dts: arm64: imx8mp: Add dbi2 and atu reg for i.MX8MP
[PATCH v1 4/4] dts: arm64: imx8mm: Add dbi2 and atu reg for i.MX8MM

Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml | 13 +++++++++----
arch/arm64/boot/dts/freescale/imx8mm.dtsi                    |  6 ++++--
arch/arm64/boot/dts/freescale/imx8mp.dtsi                    |  7 +++++--
arch/arm64/boot/dts/freescale/imx8mq.dtsi                    |  8 +++++---
4 files changed, 23 insertions(+), 11 deletions(-)

[PATCH v1 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and "atu" for i.MX8M PCIe Endpoint

[Richard Zhu]

Add reg-name: "dbi2", "atu" for i.MX8M PCIe Endpoint.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 .../devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml  | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
index a06f75df8458..309e8953dc91 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
@@ -65,11 +65,13 @@ allOf:
     then:
       properties:
         reg:
-          minItems: 2
-          maxItems: 2
+          minItems: 4
+          maxItems: 4
         reg-names:
           items:
             - const: dbi
+            - const: dbi2
+            - const: atu
             - const: addr_space
 
   - if:
@@ -129,8 +131,11 @@ examples:
 
     pcie_ep: pcie-ep@33800000 {
       compatible = "fsl,imx8mp-pcie-ep";
-      reg = <0x33800000 0x000400000>, <0x18000000 0x08000000>;
-      reg-names = "dbi", "addr_space";
+      reg = <0x33800000 0x100000>,
+            <0x33900000 0x100000>,
+            <0x33b00000 0x100000>,
+            <0x18000000 0x8000000>;
+      reg-names = "dbi", "dbi2", "atu", "addr_space";
       clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
                <&clk IMX8MP_CLK_HSIO_AXI>,
                <&clk IMX8MP_CLK_PCIE_ROOT>;
-- 
2.37.1

[PATCH v1 2/4] dts: arm64: imx8mq: Add dbi2 and atu reg for i.MX8MQ PCIe EP

[Richard Zhu]

Add dbi2 and iatu reg for i.MX8MQ PCIe EP.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index e03186bbc415..4f0fe69ef601 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -1819,9 +1819,11 @@ pcie1: pcie@33c00000 {
 
 		pcie1_ep: pcie-ep@33c00000 {
 			compatible = "fsl,imx8mq-pcie-ep";
-			reg = <0x33c00000 0x000400000>,
-			      <0x20000000 0x08000000>;
-			reg-names = "dbi", "addr_space";
+			reg = <0x33c00000 0x100000>,
+			      <0x33d00000 0x100000>,
+			      <0x33f00000 0x100000>,
+			      <0x20000000 0x8000000>;
+			reg-names = "dbi", "dbi2", "atu", "addr_space";
 			num-lanes = <1>;
 			interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "dma";
-- 
2.37.1

[PATCH v1 3/4] dts: arm64: imx8mp: Add dbi2 and atu reg for i.MX8MP PCIe EP

[Richard Zhu]

Add dbi2 and iatu reg for i.MX8MP PCIe EP.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 603dfe80216f..062bb4a107f2 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -2125,8 +2125,11 @@ pcie: pcie@33800000 {
 
 		pcie_ep: pcie-ep@33800000 {
 			compatible = "fsl,imx8mp-pcie-ep";
-			reg = <0x33800000 0x000400000>, <0x18000000 0x08000000>;
-			reg-names = "dbi", "addr_space";
+			reg = <0x33800000 0x100000>,
+			      <0x33900000 0x100000>,
+			      <0x33b00000 0x100000>,
+			      <0x18000000 0x8000000>;
+			reg-names = "dbi", "dbi2", "atu", "addr_space";
 			clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
 				 <&clk IMX8MP_CLK_HSIO_AXI>,
 				 <&clk IMX8MP_CLK_PCIE_ROOT>;
-- 
2.37.1

[PATCH v1 4/4] dts: arm64: imx8mm: Add dbi2 and atu reg for i.MX8MM PCIe EP

[Richard Zhu]

Add dbi2 and iatu reg for i.MX8MM PCIe EP.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 9535dedcef59..78462b591d29 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -1375,9 +1375,11 @@ pcie0: pcie@33800000 {
 
 		pcie0_ep: pcie-ep@33800000 {
 			compatible = "fsl,imx8mm-pcie-ep";
-			reg = <0x33800000 0x400000>,
+			reg = <0x33800000 0x100000>,
+			      <0x33900000 0x100000>,
+			      <0x33b00000 0x100000>,
 			      <0x18000000 0x8000000>;
-			reg-names = "dbi", "addr_space";
+			reg-names = "dbi", "dbi2", "atu", "addr_space";
 			num-lanes = <1>;
 			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "dma";
-- 
2.37.1

Re: [PATCH v1 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and "atu" for i.MX8M PCIe Endpoint

[Conor Dooley]

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

On Mon, Jul 22, 2024 at 03:56:16PM +0800, Richard Zhu wrote:
> Add reg-name: "dbi2", "atu" for i.MX8M PCIe Endpoint.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  .../devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml  | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> index a06f75df8458..309e8953dc91 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> @@ -65,11 +65,13 @@ allOf:
>      then:
>        properties:
>          reg:
> -          minItems: 2
> -          maxItems: 2
> +          minItems: 4
> +          maxItems: 4
>          reg-names:
>            items:
>              - const: dbi
> +            - const: dbi2
> +            - const: atu

New properties in the middle of the list is potentially an ABI break.
Why not add them at the end?

>              - const: addr_space
>  
>    - if:
> @@ -129,8 +131,11 @@ examples:
>  
>      pcie_ep: pcie-ep@33800000 {
>        compatible = "fsl,imx8mp-pcie-ep";
> -      reg = <0x33800000 0x000400000>, <0x18000000 0x08000000>;
> -      reg-names = "dbi", "addr_space";
> +      reg = <0x33800000 0x100000>,
> +            <0x33900000 0x100000>,
> +            <0x33b00000 0x100000>,
> +            <0x18000000 0x8000000>;
> +      reg-names = "dbi", "dbi2", "atu", "addr_space";
>        clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
>                 <&clk IMX8MP_CLK_HSIO_AXI>,
>                 <&clk IMX8MP_CLK_PCIE_ROOT>;
> -- 
> 2.37.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and "atu" for i.MX8M PCIe Endpoint

[Frank Li]

On Mon, Jul 22, 2024 at 05:37:14PM +0100, Conor Dooley wrote:
> On Mon, Jul 22, 2024 at 03:56:16PM +0800, Richard Zhu wrote:
> > Add reg-name: "dbi2", "atu" for i.MX8M PCIe Endpoint.
> > 
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  .../devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml  | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> > index a06f75df8458..309e8953dc91 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> > @@ -65,11 +65,13 @@ allOf:
> >      then:
> >        properties:
> >          reg:
> > -          minItems: 2
> > -          maxItems: 2
> > +          minItems: 4
> > +          maxItems: 4
> >          reg-names:
> >            items:
> >              - const: dbi
> > +            - const: dbi2
> > +            - const: atu
> 
> New properties in the middle of the list is potentially an ABI break.
> Why not add them at the end?

Because it ref to snps,dw-pcie-ep.yaml, which already defined the reg
name orders. we using reg-names to get reg resource, I don't think it break
the ABI. Driver already auto detect both 'dbi2' or no 'dbi2' case.

Frank

> 
> >              - const: addr_space
> >  
> >    - if:
> > @@ -129,8 +131,11 @@ examples:
> >  
> >      pcie_ep: pcie-ep@33800000 {
> >        compatible = "fsl,imx8mp-pcie-ep";
> > -      reg = <0x33800000 0x000400000>, <0x18000000 0x08000000>;
> > -      reg-names = "dbi", "addr_space";
> > +      reg = <0x33800000 0x100000>,
> > +            <0x33900000 0x100000>,
> > +            <0x33b00000 0x100000>,
> > +            <0x18000000 0x8000000>;
> > +      reg-names = "dbi", "dbi2", "atu", "addr_space";
> >        clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> >                 <&clk IMX8MP_CLK_HSIO_AXI>,
> >                 <&clk IMX8MP_CLK_PCIE_ROOT>;
> > -- 
> > 2.37.1
> > 

Re: [PATCH v1 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and "atu" for i.MX8M PCIe Endpoint

[Conor Dooley]

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

On Mon, Jul 22, 2024 at 04:47:29PM -0400, Frank Li wrote:
> On Mon, Jul 22, 2024 at 05:37:14PM +0100, Conor Dooley wrote:
> > On Mon, Jul 22, 2024 at 03:56:16PM +0800, Richard Zhu wrote:
> > > Add reg-name: "dbi2", "atu" for i.MX8M PCIe Endpoint.
> > > 
> > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > ---
> > >  .../devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml  | 13 +++++++++----
> > >  1 file changed, 9 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> > > index a06f75df8458..309e8953dc91 100644
> > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> > > @@ -65,11 +65,13 @@ allOf:
> > >      then:
> > >        properties:
> > >          reg:
> > > -          minItems: 2
> > > -          maxItems: 2
> > > +          minItems: 4
> > > +          maxItems: 4
> > >          reg-names:
> > >            items:
> > >              - const: dbi
> > > +            - const: dbi2
> > > +            - const: atu
> > 
> > New properties in the middle of the list is potentially an ABI break.
> > Why not add them at the end?
> 
> Because it ref to snps,dw-pcie-ep.yaml, which already defined the reg
> name orders.

Are you sure that it defines an order for reg? If it did, it would not
allow what you already have in this binding. The order is actually
defined in this file.

> we using reg-names to get reg resource, I don't think it break
> the ABI. Driver already auto detect both 'dbi2' or no 'dbi2' case.

Linux's might, another might not. I don't see any point in breaking the
ABI when you can just put the entries at the end of he list and have no
problems at all.

Thanks,
Conor.

> > >              - const: addr_space
> > >  
> > >    - if:
> > > @@ -129,8 +131,11 @@ examples:
> > >  
> > >      pcie_ep: pcie-ep@33800000 {
> > >        compatible = "fsl,imx8mp-pcie-ep";
> > > -      reg = <0x33800000 0x000400000>, <0x18000000 0x08000000>;
> > > -      reg-names = "dbi", "addr_space";
> > > +      reg = <0x33800000 0x100000>,
> > > +            <0x33900000 0x100000>,
> > > +            <0x33b00000 0x100000>,
> > > +            <0x18000000 0x8000000>;
> > > +      reg-names = "dbi", "dbi2", "atu", "addr_space";
> > >        clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> > >                 <&clk IMX8MP_CLK_HSIO_AXI>,
> > >                 <&clk IMX8MP_CLK_PCIE_ROOT>;
> > > -- 
> > > 2.37.1
> > > 
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and "atu" for i.MX8M PCIe Endpoint

[Frank Li]

On Tue, Jul 23, 2024 at 03:35:11PM +0100, Conor Dooley wrote:
> On Mon, Jul 22, 2024 at 04:47:29PM -0400, Frank Li wrote:
> > On Mon, Jul 22, 2024 at 05:37:14PM +0100, Conor Dooley wrote:
> > > On Mon, Jul 22, 2024 at 03:56:16PM +0800, Richard Zhu wrote:
> > > > Add reg-name: "dbi2", "atu" for i.MX8M PCIe Endpoint.
> > > > 
> > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > > ---
> > > >  .../devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml  | 13 +++++++++----
> > > >  1 file changed, 9 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> > > > index a06f75df8458..309e8953dc91 100644
> > > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> > > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
> > > > @@ -65,11 +65,13 @@ allOf:
> > > >      then:
> > > >        properties:
> > > >          reg:
> > > > -          minItems: 2
> > > > -          maxItems: 2
> > > > +          minItems: 4
> > > > +          maxItems: 4
> > > >          reg-names:
> > > >            items:
> > > >              - const: dbi
> > > > +            - const: dbi2
> > > > +            - const: atu
> > > 
> > > New properties in the middle of the list is potentially an ABI break.
> > > Why not add them at the end?
> > 
> > Because it ref to snps,dw-pcie-ep.yaml, which already defined the reg
> > name orders.
> 
> Are you sure that it defines an order for reg? If it did, it would not
> allow what you already have in this binding. The order is actually
> defined in this file.

Sorry, I missed oneOf in snps,dw-pcie-ep.yaml. You are right.

Frank

> 
> > we using reg-names to get reg resource, I don't think it break
> > the ABI. Driver already auto detect both 'dbi2' or no 'dbi2' case.
> 
> Linux's might, another might not. I don't see any point in breaking the
> ABI when you can just put the entries at the end of he list and have no
> problems at all.
> 
> Thanks,
> Conor.
> 
> > > >              - const: addr_space
> > > >  
> > > >    - if:
> > > > @@ -129,8 +131,11 @@ examples:
> > > >  
> > > >      pcie_ep: pcie-ep@33800000 {
> > > >        compatible = "fsl,imx8mp-pcie-ep";
> > > > -      reg = <0x33800000 0x000400000>, <0x18000000 0x08000000>;
> > > > -      reg-names = "dbi", "addr_space";
> > > > +      reg = <0x33800000 0x100000>,
> > > > +            <0x33900000 0x100000>,
> > > > +            <0x33b00000 0x100000>,
> > > > +            <0x18000000 0x8000000>;
> > > > +      reg-names = "dbi", "dbi2", "atu", "addr_space";
> > > >        clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
> > > >                 <&clk IMX8MP_CLK_HSIO_AXI>,
> > > >                 <&clk IMX8MP_CLK_PCIE_ROOT>;
> > > > -- 
> > > > 2.37.1
> > > > 
> > 
> >