diff mbox series

[v6,15/16] dt-bindings: imx6q-pcie: Add iMX95 pcie endpoint compatible string

Message ID 20231224183242.1675372-16-Frank.Li@nxp.com
State New
Headers
Series PCI: imx6: Clean up and add imx95 pci support |

Commit Message

Frank Li Dec. 24, 2023, 6:32 p.m. UTC 
  Add i.MX95 PCIe "fsl,imx95-pcie-ep" compatible string.
Add reg-name: "atu", "dbi2", "dma" and "app".
Reuse PCI linux,pci-domain as controller id at endpoint.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---

Notes:
    Change from v1 to v3
    - new patches at v3

 .../bindings/pci/fsl,imx6q-pcie-ep.yaml       | 57 ++++++++++++++++---
 1 file changed, 49 insertions(+), 8 deletions(-)

Comments

Krzysztof Kozlowski Dec. 25, 2023, 7:16 p.m. UTC | #1 
On 24/12/2023 19:32, Frank Li wrote:
> Add i.MX95 PCIe "fsl,imx95-pcie-ep" compatible string.
> Add reg-name: "atu", "dbi2", "dma" and "app".
> Reuse PCI linux,pci-domain as controller id at endpoint.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> 

...

> +# reuse PCI linux,pci-domain as controller id at Endpoint
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - fsl,imx95-pcie-ep
> +    then:
> +      properties:
> +        linux,pci-domain: true

Same comment: why do you need? Don't ignore my feedback. You responded
you will fix it, but it is still here...

Best regards,
Krzysztof
Frank Li Dec. 26, 2023, 4:12 p.m. UTC | #2 
On Mon, Dec 25, 2023 at 08:16:17PM +0100, Krzysztof Kozlowski wrote:
> On 24/12/2023 19:32, Frank Li wrote:
> > Add i.MX95 PCIe "fsl,imx95-pcie-ep" compatible string.
> > Add reg-name: "atu", "dbi2", "dma" and "app".
> > Reuse PCI linux,pci-domain as controller id at endpoint.
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > 
> 
> ...
> 
> > +# reuse PCI linux,pci-domain as controller id at Endpoint
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - fsl,imx95-pcie-ep
> > +    then:
> > +      properties:
> > +        linux,pci-domain: true
> 
> Same comment: why do you need? Don't ignore my feedback. You responded
> you will fix it, but it is still here...

DTB_CHECK report error after I remove it. linux,pci-domain is only define
in pci, not pci-ep.

So I add comments about this. linux,pci-domain was resued ad controller id.

If include pci.yaml, there are too much other properties was involved, but
not used by pci-ep.

Frank

> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Dec. 26, 2023, 7:01 p.m. UTC | #3 
On 26/12/2023 17:12, Frank Li wrote:
> On Mon, Dec 25, 2023 at 08:16:17PM +0100, Krzysztof Kozlowski wrote:
>> On 24/12/2023 19:32, Frank Li wrote:
>>> Add i.MX95 PCIe "fsl,imx95-pcie-ep" compatible string.
>>> Add reg-name: "atu", "dbi2", "dma" and "app".
>>> Reuse PCI linux,pci-domain as controller id at endpoint.
>>>
>>> Signed-off-by: Frank Li <Frank.Li@nxp.com>
>>> ---
>>>
>>
>> ...
>>
>>> +# reuse PCI linux,pci-domain as controller id at Endpoint
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          enum:
>>> +            - fsl,imx95-pcie-ep
>>> +    then:
>>> +      properties:
>>> +        linux,pci-domain: true
>>
>> Same comment: why do you need? Don't ignore my feedback. You responded
>> you will fix it, but it is still here...
> 
> DTB_CHECK report error after I remove it. linux,pci-domain is only define
> in pci, not pci-ep.

Ah, thank you, indeed.

> 
> So I add comments about this. linux,pci-domain was resued ad controller id.

However maybe there is reason why it is not for endpoints. The
description is saying it is valid only for host bridge, so maybe it
should not be used for endpoint case?
> 
> If include pci.yaml, there are too much other properties was involved, but
> not used by pci-ep.

Best regards,
Krzysztof
Frank Li Dec. 26, 2023, 9:53 p.m. UTC | #4 
On Tue, Dec 26, 2023 at 08:01:53PM +0100, Krzysztof Kozlowski wrote:
> On 26/12/2023 17:12, Frank Li wrote:
> > On Mon, Dec 25, 2023 at 08:16:17PM +0100, Krzysztof Kozlowski wrote:
> >> On 24/12/2023 19:32, Frank Li wrote:
> >>> Add i.MX95 PCIe "fsl,imx95-pcie-ep" compatible string.
> >>> Add reg-name: "atu", "dbi2", "dma" and "app".
> >>> Reuse PCI linux,pci-domain as controller id at endpoint.
> >>>
> >>> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> >>> ---
> >>>
> >>
> >> ...
> >>
> >>> +# reuse PCI linux,pci-domain as controller id at Endpoint
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          enum:
> >>> +            - fsl,imx95-pcie-ep
> >>> +    then:
> >>> +      properties:
> >>> +        linux,pci-domain: true
> >>
> >> Same comment: why do you need? Don't ignore my feedback. You responded
> >> you will fix it, but it is still here...
> > 
> > DTB_CHECK report error after I remove it. linux,pci-domain is only define
> > in pci, not pci-ep.
> 
> Ah, thank you, indeed.
> 
> > 
> > So I add comments about this. linux,pci-domain was resued ad controller id.
> 
> However maybe there is reason why it is not for endpoints. The
> description is saying it is valid only for host bridge, so maybe it
> should not be used for endpoint case?

EP side, it is not PCI bus. So it is reasonable that linux,pci-doamin not
in EP side.

iMX6 host driver(and other some host controller drivers) already use it as
"controller id". EP driver is mostly reused with host bridge drivers. I
think needn't create new property such as "controller_id" for EP only.

A comments should be enough for this case.

Frank

> > 
> > If include pci.yaml, there are too much other properties was involved, but
> > not used by pci-ep.
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Dec. 27, 2023, noon UTC | #5 
On 26/12/2023 22:53, Frank Li wrote:
>>> DTB_CHECK report error after I remove it. linux,pci-domain is only define
>>> in pci, not pci-ep.
>>
>> Ah, thank you, indeed.
>>
>>>
>>> So I add comments about this. linux,pci-domain was resued ad controller id.
>>
>> However maybe there is reason why it is not for endpoints. The
>> description is saying it is valid only for host bridge, so maybe it
>> should not be used for endpoint case?
> 
> EP side, it is not PCI bus. So it is reasonable that linux,pci-doamin not
> in EP side.
> 
> iMX6 host driver(and other some host controller drivers) already use it as
> "controller id". EP driver is mostly reused with host bridge drivers. I
> think needn't create new property such as "controller_id" for EP only.
> 
> A comments should be enough for this case.

Hm, ok, seems fine for me. I don't know PCI good enough to judge, so
unless Rob or Conor say something else, it looks good.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
index ee155ed5f1811..f4d6ae5dab785 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml
@@ -22,14 +22,7 @@  properties:
       - fsl,imx8mm-pcie-ep
       - fsl,imx8mq-pcie-ep
       - fsl,imx8mp-pcie-ep
-
-  reg:
-    minItems: 2
-
-  reg-names:
-    items:
-      - const: dbi
-      - const: addr_space
+      - fsl,imx95-pcie-ep
 
   clocks:
     minItems: 3
@@ -62,11 +55,48 @@  required:
 allOf:
   - $ref: /schemas/pci/snps,dw-pcie-ep.yaml#
   - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx8mm-pcie-ep
+            - fsl,imx8mq-pcie-ep
+            - fsl,imx8mp-pcie-ep
+    then:
+      properties:
+        reg:
+          minItems: 2
+          maxItems: 2
+        reg-names:
+          items:
+            - const: dbi
+            - const: addr_space
+
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx95-pcie-ep
+    then:
+      properties:
+        reg:
+          minItems: 6
+          maxItems: 6
+        reg-names:
+          items:
+            - const: dbi
+            - const: atu
+            - const: dbi2
+            - const: app
+            - const: dma
+            - const: addr_space
+
   - if:
       properties:
         compatible:
           enum:
             - fsl,imx8mq-pcie-ep
+            - fsl,imx95-pcie-ep
     then:
       properties:
         clocks:
@@ -87,6 +117,17 @@  allOf:
             - const: pcie_bus
             - const: pcie_aux
 
+# reuse PCI linux,pci-domain as controller id at Endpoint
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx95-pcie-ep
+    then:
+      properties:
+        linux,pci-domain: true
+      required:
+        - linux,pci-domain
 
 unevaluatedProperties: false