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

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

[Richard Zhu]

v2 changes:
Thanks for Conor's comments.
- Place the new added properties at the end.

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

[PATCH v2 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and "atu"
[PATCH v2 2/4] dts: arm64: imx8mq: Add dbi2 and atu reg for i.MX8MQ
[PATCH v2 3/4] dts: arm64: imx8mp: Add dbi2 and atu reg for i.MX8MP
[PATCH v2 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                    |  8 +++++---
arch/arm64/boot/dts/freescale/imx8mp.dtsi                    |  7 +++++--
arch/arm64/boot/dts/freescale/imx8mq.dtsi                    |  8 +++++---
4 files changed, 24 insertions(+), 12 deletions(-)

[PATCH v2 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..84ca12e8b25b 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
@@ -65,12 +65,14 @@ allOf:
     then:
       properties:
         reg:
-          minItems: 2
-          maxItems: 2
+          minItems: 4
+          maxItems: 4
         reg-names:
           items:
             - const: dbi
             - const: addr_space
+            - const: dbi2
+            - const: atu
 
   - if:
       properties:
@@ -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>,
+            <0x18000000 0x8000000>,
+            <0x33900000 0x100000>,
+            <0x33b00000 0x100000>;
+      reg-names = "dbi", "addr_space", "dbi2", "atu";
       clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
                <&clk IMX8MP_CLK_HSIO_AXI>,
                <&clk IMX8MP_CLK_PCIE_ROOT>;
-- 
2.37.1

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

[Krzysztof Kozlowski]

On 24/07/2024 05:03, Richard Zhu wrote:
> Add reg-name: "dbi2", "atu" for i.MX8M PCIe Endpoint.

This we see in the diff. What I do not see is why? Hardware changed? How
come?


Best regards,
Krzysztof

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

[Hongxing Zhu]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 2024年7月24日 14:07
> To: Hongxing Zhu <hongxing.zhu@nxp.com>; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; shawnguo@kernel.org;
> l.stach@pengutronix.de
> Cc: devicetree@vger.kernel.org; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> kernel@pengutronix.de; imx@lists.linux.dev
> Subject: Re: [PATCH v2 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and
> "atu" for i.MX8M PCIe Endpoint
> 
> On 24/07/2024 05:03, Richard Zhu wrote:
> > Add reg-name: "dbi2", "atu" for i.MX8M PCIe Endpoint.
> 
> This we see in the diff. What I do not see is why? Hardware changed? How come?
> 
For i.MX8M PCIe EP, the dbi2 and atu address are pre-defined in the driver.
This method is not good.
In commit b7d67c6130ee ("PCI: imx6: Add iMX95 Endpoint (EP) support"),
Frank suggests to fetch the dbi2 and atu from DT directly.
This series is preparation to do that for i.MX8M PCIe EP.

Best Regards
Richard Zhu
> 
> Best regards,
> Krzysztof

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

[Krzysztof Kozlowski]

On 24/07/2024 08:26, Hongxing Zhu wrote:
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: 2024年7月24日 14:07
>> To: Hongxing Zhu <hongxing.zhu@nxp.com>; robh@kernel.org;
>> krzk+dt@kernel.org; conor+dt@kernel.org; shawnguo@kernel.org;
>> l.stach@pengutronix.de
>> Cc: devicetree@vger.kernel.org; linux-pci@vger.kernel.org;
>> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
>> kernel@pengutronix.de; imx@lists.linux.dev
>> Subject: Re: [PATCH v2 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and
>> "atu" for i.MX8M PCIe Endpoint
>>
>> On 24/07/2024 05:03, Richard Zhu wrote:
>>> Add reg-name: "dbi2", "atu" for i.MX8M PCIe Endpoint.
>>
>> This we see in the diff. What I do not see is why? Hardware changed? How come?
>>
> For i.MX8M PCIe EP, the dbi2 and atu address are pre-defined in the driver.
> This method is not good.
> In commit b7d67c6130ee ("PCI: imx6: Add iMX95 Endpoint (EP) support"),
> Frank suggests to fetch the dbi2 and atu from DT directly.
> This series is preparation to do that for i.MX8M PCIe EP.

This all must be explained in commit msg.

Anyway, this will be an ABI break, so explain exactly why it is OK to
break the ABI.

Best regards,
Krzysztof

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

[Hongxing Zhu]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 2024年7月24日 14:33
> To: Hongxing Zhu <hongxing.zhu@nxp.com>; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; shawnguo@kernel.org;
> l.stach@pengutronix.de
> Cc: devicetree@vger.kernel.org; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> kernel@pengutronix.de; imx@lists.linux.dev
> Subject: Re: [PATCH v2 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and
> "atu" for i.MX8M PCIe Endpoint
> 
> On 24/07/2024 08:26, Hongxing Zhu wrote:
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: 2024年7月24日 14:07
> >> To: Hongxing Zhu <hongxing.zhu@nxp.com>; robh@kernel.org;
> >> krzk+dt@kernel.org; conor+dt@kernel.org; shawnguo@kernel.org;
> >> l.stach@pengutronix.de
> >> Cc: devicetree@vger.kernel.org; linux-pci@vger.kernel.org;
> >> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> >> kernel@pengutronix.de; imx@lists.linux.dev
> >> Subject: Re: [PATCH v2 1/4] dt-bindings: imx6q-pcie: Add reg-name
> >> "dbi2" and "atu" for i.MX8M PCIe Endpoint
> >>
> >> On 24/07/2024 05:03, Richard Zhu wrote:
> >>> Add reg-name: "dbi2", "atu" for i.MX8M PCIe Endpoint.
> >>
> >> This we see in the diff. What I do not see is why? Hardware changed? How
> come?
> >>
> > For i.MX8M PCIe EP, the dbi2 and atu address are pre-defined in the driver.
> > This method is not good.
> > In commit b7d67c6130ee ("PCI: imx6: Add iMX95 Endpoint (EP) support"),
> > Frank suggests to fetch the dbi2 and atu from DT directly.
> > This series is preparation to do that for i.MX8M PCIe EP.
> 
> This all must be explained in commit msg.
> 
> Anyway, this will be an ABI break, so explain exactly why it is OK to break the ABI.
Okay, all the explanation would be added later.
Thanks for your comments.

Best Regards
Richard Zhu
> 
> Best regards,
> Krzysztof

Re: [PATCH v2 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: 1687 bytes --]

On Wed, Jul 24, 2024 at 08:32:34AM +0200, Krzysztof Kozlowski wrote:
> On 24/07/2024 08:26, Hongxing Zhu wrote:
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: 2024年7月24日 14:07
> >> To: Hongxing Zhu <hongxing.zhu@nxp.com>; robh@kernel.org;
> >> krzk+dt@kernel.org; conor+dt@kernel.org; shawnguo@kernel.org;
> >> l.stach@pengutronix.de
> >> Cc: devicetree@vger.kernel.org; linux-pci@vger.kernel.org;
> >> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> >> kernel@pengutronix.de; imx@lists.linux.dev
> >> Subject: Re: [PATCH v2 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and
> >> "atu" for i.MX8M PCIe Endpoint
> >>
> >> On 24/07/2024 05:03, Richard Zhu wrote:
> >>> Add reg-name: "dbi2", "atu" for i.MX8M PCIe Endpoint.
> >>
> >> This we see in the diff. What I do not see is why? Hardware changed? How come?
> >>
> > For i.MX8M PCIe EP, the dbi2 and atu address are pre-defined in the driver.
> > This method is not good.
> > In commit b7d67c6130ee ("PCI: imx6: Add iMX95 Endpoint (EP) support"),
> > Frank suggests to fetch the dbi2 and atu from DT directly.
> > This series is preparation to do that for i.MX8M PCIe EP.
> 
> This all must be explained in commit msg.
> 
> Anyway, this will be an ABI break, so explain exactly why it is OK to
> break the ABI.

And the driver needs to be written in such a way that if only two reg
properties are provided, it falls back to the old method of acquiring
the two new reg regions. I didn't see a driver patch on v1, so I missed
that this is what was actually the plan - I thought that this was just
adding two missing regions.

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

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

[Hongxing Zhu]

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: 2024年7月25日 0:24
> To: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Hongxing Zhu <hongxing.zhu@nxp.com>; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org; shawnguo@kernel.org;
> l.stach@pengutronix.de; devicetree@vger.kernel.org; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> kernel@pengutronix.de; imx@lists.linux.dev
> Subject: Re: [PATCH v2 1/4] dt-bindings: imx6q-pcie: Add reg-name "dbi2" and
> "atu" for i.MX8M PCIe Endpoint
> 
> On Wed, Jul 24, 2024 at 08:32:34AM +0200, Krzysztof Kozlowski wrote:
> > On 24/07/2024 08:26, Hongxing Zhu wrote:
> > >> -----Original Message-----
> > >> From: Krzysztof Kozlowski <krzk@kernel.org>
> > >> Sent: 2024年7月24日 14:07
> > >> To: Hongxing Zhu <hongxing.zhu@nxp.com>; robh@kernel.org;
> > >> krzk+dt@kernel.org; conor+dt@kernel.org; shawnguo@kernel.org;
> > >> l.stach@pengutronix.de
> > >> Cc: devicetree@vger.kernel.org; linux-pci@vger.kernel.org;
> > >> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > >> kernel@pengutronix.de; imx@lists.linux.dev
> > >> Subject: Re: [PATCH v2 1/4] dt-bindings: imx6q-pcie: Add reg-name
> > >> "dbi2" and "atu" for i.MX8M PCIe Endpoint
> > >>
> > >> On 24/07/2024 05:03, Richard Zhu wrote:
> > >>> Add reg-name: "dbi2", "atu" for i.MX8M PCIe Endpoint.
> > >>
> > >> This we see in the diff. What I do not see is why? Hardware changed? How
> come?
> > >>
> > > For i.MX8M PCIe EP, the dbi2 and atu address are pre-defined in the driver.
> > > This method is not good.
> > > In commit b7d67c6130ee ("PCI: imx6: Add iMX95 Endpoint (EP)
> > > support"), Frank suggests to fetch the dbi2 and atu from DT directly.
> > > This series is preparation to do that for i.MX8M PCIe EP.
> >
> > This all must be explained in commit msg.
> >
> > Anyway, this will be an ABI break, so explain exactly why it is OK to
> > break the ABI.
> 
> And the driver needs to be written in such a way that if only two reg properties are
> provided, it falls back to the old method of acquiring the two new reg regions. I
> didn't see a driver patch on v1, so I missed that this is what was actually the plan - I
> thought that this was just adding two missing regions.
Hi Corner:
Thanks for your comments.
Yes, the current driver just uses this way to handle dbi2 and atu base
 address assignments.
If only two reg properties are provided, driver would falls back to the
 old method.

Best Regards
Richard Zhu

[PATCH v2 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..d51de8d899b2 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>,
+			      <0x20000000 0x8000000>,
+			      <0x33d00000 0x100000>,
+			      <0x33f00000 0x100000>;
+			reg-names = "dbi", "addr_space", "dbi2", "atu";
 			num-lanes = <1>;
 			interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "dma";
-- 
2.37.1

[PATCH v2 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..53748227db10 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>,
+			      <0x18000000 0x8000000>,
+			      <0x33900000 0x100000>,
+			      <0x33b00000 0x100000>;
+			reg-names = "dbi", "addr_space", "dbi2", "atu";
 			clocks = <&clk IMX8MP_CLK_HSIO_ROOT>,
 				 <&clk IMX8MP_CLK_HSIO_AXI>,
 				 <&clk IMX8MP_CLK_PCIE_ROOT>;
-- 
2.37.1

[PATCH v2 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 | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 9535dedcef59..4de3bf22902b 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>,
-			      <0x18000000 0x8000000>;
-			reg-names = "dbi", "addr_space";
+			reg = <0x33800000 0x100000>,
+			      <0x18000000 0x8000000>,
+			      <0x33900000 0x100000>,
+			      <0x33b00000 0x100000>;
+			reg-names = "dbi", "addr_space", "dbi2", "atu";
 			num-lanes = <1>;
 			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "dma";
-- 
2.37.1