diff mbox series

[v3,08/13] dt-bindings: imx6q-pcie: Add imx95 pcie compatible string

Message ID 20231211215842.134823-9-Frank.Li@nxp.com (mailing list archive)
State Superseded
Delegated to: Lorenzo Pieralisi
Headers show
Series PCI: imx6: Clean up and add imx95 pci support | expand

Commit Message

Frank Li Dec. 11, 2023, 9:58 p.m. UTC
From: Richard Zhu <hongxing.zhu@nxp.com>

Add i.MX95 PCIe "fsl,imx95-pcie" compatible string.
Add "atu" and "serdes" to reg-names.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---

Notes:
    Change from v2 to v3
    - Remove krzy's ACK tag
    - Add condition check for imx95, which required more reg-names then old
    platform, so need Krzy review again,
    
    Change from v1 to v2
    - add Krzy's ACK tag

 .../bindings/pci/fsl,imx6q-pcie.yaml           | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Rob Herring (Arm) Dec. 12, 2023, 10:44 p.m. UTC | #1
On Mon, Dec 11, 2023 at 04:58:37PM -0500, Frank Li wrote:
> From: Richard Zhu <hongxing.zhu@nxp.com>
> 
> Add i.MX95 PCIe "fsl,imx95-pcie" compatible string.
> Add "atu" and "serdes" to reg-names.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
> 
> Notes:
>     Change from v2 to v3
>     - Remove krzy's ACK tag
>     - Add condition check for imx95, which required more reg-names then old
>     platform, so need Krzy review again,
>     
>     Change from v1 to v2
>     - add Krzy's ACK tag
> 
>  .../bindings/pci/fsl,imx6q-pcie.yaml           | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> index 81bbb8728f0f9..b8fcf8258f031 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> @@ -29,6 +29,7 @@ properties:
>        - fsl,imx8mq-pcie
>        - fsl,imx8mm-pcie
>        - fsl,imx8mp-pcie
> +      - fsl,imx95-pcie
>  
>    reg:
>      items:
> @@ -90,6 +91,22 @@ required:
>  allOf:
>    - $ref: /schemas/pci/snps,dw-pcie.yaml#
>    - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - fsl,imx95-pcie
> +    then:
> +      properties:
> +        reg:
> +          minItems: 4
> +        reg-names:
> +          items:
> +            - const: dbi
> +            - const: serdes

Did you test this? It should fail because 'serdes' would need to be 
added to snps,dw-pcie.yaml.

Is this really not a separate phy block? A separate node would be 
ideal. If not, there's already a 'phy' name you can use here. We don't 
want more random names.

Rob
Frank Li Dec. 12, 2023, 11:28 p.m. UTC | #2
On Tue, Dec 12, 2023 at 04:44:26PM -0600, Rob Herring wrote:
> On Mon, Dec 11, 2023 at 04:58:37PM -0500, Frank Li wrote:
> > From: Richard Zhu <hongxing.zhu@nxp.com>
> > 
> > Add i.MX95 PCIe "fsl,imx95-pcie" compatible string.
> > Add "atu" and "serdes" to reg-names.
> > 
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> > 
> > Notes:
> >     Change from v2 to v3
> >     - Remove krzy's ACK tag
> >     - Add condition check for imx95, which required more reg-names then old
> >     platform, so need Krzy review again,
> >     
> >     Change from v1 to v2
> >     - add Krzy's ACK tag
> > 
> >  .../bindings/pci/fsl,imx6q-pcie.yaml           | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > index 81bbb8728f0f9..b8fcf8258f031 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > @@ -29,6 +29,7 @@ properties:
> >        - fsl,imx8mq-pcie
> >        - fsl,imx8mm-pcie
> >        - fsl,imx8mp-pcie
> > +      - fsl,imx95-pcie
> >  
> >    reg:
> >      items:
> > @@ -90,6 +91,22 @@ required:
> >  allOf:
> >    - $ref: /schemas/pci/snps,dw-pcie.yaml#
> >    - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          enum:
> > +            - fsl,imx95-pcie
> > +    then:
> > +      properties:
> > +        reg:
> > +          minItems: 4
> > +        reg-names:
> > +          items:
> > +            - const: dbi
> > +            - const: serdes
> 
> Did you test this? It should fail because 'serdes' would need to be 
> added to snps,dw-pcie.yaml.

I run "make dt_binding_check DT_SCHEMA_FILES=/pci/", no error report.
And PCIe function can work.

> 
> Is this really not a separate phy block?

This is misc block, which included phy and also include some registers
about SID for each PCI devices. I plan do it later.


> A separate node would be 
> ideal. If not, there's already a 'phy' name you can use here. We don't 
> want more random names.

Chip reference manual call it as 'serdes'. If there are already similar
name, I can reuse it.

> 
> Rob
Krzysztof Kozlowski Dec. 13, 2023, 6:28 a.m. UTC | #3
On 13/12/2023 00:28, Frank Li wrote:
	>>>      items:
>>> @@ -90,6 +91,22 @@ required:
>>>  allOf:
>>>    - $ref: /schemas/pci/snps,dw-pcie.yaml#
>>>    - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          enum:
>>> +            - fsl,imx95-pcie
>>> +    then:
>>> +      properties:
>>> +        reg:
>>> +          minItems: 4
>>> +        reg-names:
>>> +          items:
>>> +            - const: dbi
>>> +            - const: serdes
>>
>> Did you test this? It should fail because 'serdes' would need to be 
>> added to snps,dw-pcie.yaml.
> 
> I run "make dt_binding_check DT_SCHEMA_FILES=/pci/", no error report.
> And PCIe function can work.

Did you test your DTS. The answer is, like Rob suspected: no, you did
not test it.

This fails.

Best regards,
Krzysztof
Rob Herring (Arm) Dec. 13, 2023, 2:36 p.m. UTC | #4
On Tue, Dec 12, 2023 at 06:28:43PM -0500, Frank Li wrote:
> On Tue, Dec 12, 2023 at 04:44:26PM -0600, Rob Herring wrote:
> > On Mon, Dec 11, 2023 at 04:58:37PM -0500, Frank Li wrote:
> > > From: Richard Zhu <hongxing.zhu@nxp.com>
> > > 
> > > Add i.MX95 PCIe "fsl,imx95-pcie" compatible string.
> > > Add "atu" and "serdes" to reg-names.
> > > 
> > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > ---
> > > 
> > > Notes:
> > >     Change from v2 to v3
> > >     - Remove krzy's ACK tag
> > >     - Add condition check for imx95, which required more reg-names then old
> > >     platform, so need Krzy review again,
> > >     
> > >     Change from v1 to v2
> > >     - add Krzy's ACK tag
> > > 
> > >  .../bindings/pci/fsl,imx6q-pcie.yaml           | 18 ++++++++++++++++++
> > >  1 file changed, 18 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > index 81bbb8728f0f9..b8fcf8258f031 100644
> > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > @@ -29,6 +29,7 @@ properties:
> > >        - fsl,imx8mq-pcie
> > >        - fsl,imx8mm-pcie
> > >        - fsl,imx8mp-pcie
> > > +      - fsl,imx95-pcie
> > >  
> > >    reg:
> > >      items:
> > > @@ -90,6 +91,22 @@ required:
> > >  allOf:
> > >    - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > >    - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          enum:
> > > +            - fsl,imx95-pcie
> > > +    then:
> > > +      properties:
> > > +        reg:
> > > +          minItems: 4
> > > +        reg-names:
> > > +          items:
> > > +            - const: dbi
> > > +            - const: serdes
> > 
> > Did you test this? It should fail because 'serdes' would need to be 
> > added to snps,dw-pcie.yaml.
> 
> I run "make dt_binding_check DT_SCHEMA_FILES=/pci/", no error report.

Only because you have no example. What about your actual .dts?

> And PCIe function can work.
> 
> > 
> > Is this really not a separate phy block?
> 
> This is misc block, which included phy and also include some registers
> about SID for each PCI devices. I plan do it later.

Sounds like it should be a separate node and use the phy binding. Do it 
correctly from the start, not later. Later is an ABI break.

What is SID?

Rob
Frank Li Dec. 13, 2023, 3:39 p.m. UTC | #5
On Wed, Dec 13, 2023 at 08:36:15AM -0600, Rob Herring wrote:
> On Tue, Dec 12, 2023 at 06:28:43PM -0500, Frank Li wrote:
> > On Tue, Dec 12, 2023 at 04:44:26PM -0600, Rob Herring wrote:
> > > On Mon, Dec 11, 2023 at 04:58:37PM -0500, Frank Li wrote:
> > > > From: Richard Zhu <hongxing.zhu@nxp.com>
> > > > 
> > > > Add i.MX95 PCIe "fsl,imx95-pcie" compatible string.
> > > > Add "atu" and "serdes" to reg-names.
> > > > 
> > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > > ---
> > > > 
> > > > Notes:
> > > >     Change from v2 to v3
> > > >     - Remove krzy's ACK tag
> > > >     - Add condition check for imx95, which required more reg-names then old
> > > >     platform, so need Krzy review again,
> > > >     
> > > >     Change from v1 to v2
> > > >     - add Krzy's ACK tag
> > > > 
> > > >  .../bindings/pci/fsl,imx6q-pcie.yaml           | 18 ++++++++++++++++++
> > > >  1 file changed, 18 insertions(+)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > index 81bbb8728f0f9..b8fcf8258f031 100644
> > > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > @@ -29,6 +29,7 @@ properties:
> > > >        - fsl,imx8mq-pcie
> > > >        - fsl,imx8mm-pcie
> > > >        - fsl,imx8mp-pcie
> > > > +      - fsl,imx95-pcie
> > > >  
> > > >    reg:
> > > >      items:
> > > > @@ -90,6 +91,22 @@ required:
> > > >  allOf:
> > > >    - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > >    - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
> > > > +  - if:
> > > > +      properties:
> > > > +        compatible:
> > > > +          enum:
> > > > +            - fsl,imx95-pcie
> > > > +    then:
> > > > +      properties:
> > > > +        reg:
> > > > +          minItems: 4
> > > > +        reg-names:
> > > > +          items:
> > > > +            - const: dbi
> > > > +            - const: serdes
> > > 
> > > Did you test this? It should fail because 'serdes' would need to be 
> > > added to snps,dw-pcie.yaml.
> > 
> > I run "make dt_binding_check DT_SCHEMA_FILES=/pci/", no error report.
> 
> Only because you have no example. What about your actual .dts?

I see. 95 is quite new. Still have not good base yet.
I may just take take care this session.

> 
> > And PCIe function can work.
> > 
> > > 
> > > Is this really not a separate phy block?
> > 
> > This is misc block, which included phy and also include some registers
> > about SID for each PCI devices. I plan do it later.
> 
> Sounds like it should be a separate node and use the phy binding. Do it 
> correctly from the start, not later. Later is an ABI break.

Actually, I considerred phy binding. The major problem is LUT (look up
table) for MSI and SMMU. LUT need be config according to some PCI device
information. I have not find good hook for that at PHY driver.

> 
> What is SID?

Stream ID, each device master have SID, which pass to IOMMU and GIC ITS.

Frank

> 
> Rob
Frank Li Dec. 13, 2023, 4:46 p.m. UTC | #6
On Wed, Dec 13, 2023 at 10:39:20AM -0500, Frank Li wrote:
> On Wed, Dec 13, 2023 at 08:36:15AM -0600, Rob Herring wrote:
> > On Tue, Dec 12, 2023 at 06:28:43PM -0500, Frank Li wrote:
> > > On Tue, Dec 12, 2023 at 04:44:26PM -0600, Rob Herring wrote:
> > > > On Mon, Dec 11, 2023 at 04:58:37PM -0500, Frank Li wrote:
> > > > > From: Richard Zhu <hongxing.zhu@nxp.com>
> > > > > 
> > > > > Add i.MX95 PCIe "fsl,imx95-pcie" compatible string.
> > > > > Add "atu" and "serdes" to reg-names.
> > > > > 
> > > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > > > ---
> > > > > 
> > > > > Notes:
> > > > >     Change from v2 to v3
> > > > >     - Remove krzy's ACK tag
> > > > >     - Add condition check for imx95, which required more reg-names then old
> > > > >     platform, so need Krzy review again,
> > > > >     
> > > > >     Change from v1 to v2
> > > > >     - add Krzy's ACK tag
> > > > > 
> > > > >  .../bindings/pci/fsl,imx6q-pcie.yaml           | 18 ++++++++++++++++++
> > > > >  1 file changed, 18 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > > index 81bbb8728f0f9..b8fcf8258f031 100644
> > > > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > > @@ -29,6 +29,7 @@ properties:
> > > > >        - fsl,imx8mq-pcie
> > > > >        - fsl,imx8mm-pcie
> > > > >        - fsl,imx8mp-pcie
> > > > > +      - fsl,imx95-pcie
> > > > >  
> > > > >    reg:
> > > > >      items:
> > > > > @@ -90,6 +91,22 @@ required:
> > > > >  allOf:
> > > > >    - $ref: /schemas/pci/snps,dw-pcie.yaml#
> > > > >    - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
> > > > > +  - if:
> > > > > +      properties:
> > > > > +        compatible:
> > > > > +          enum:
> > > > > +            - fsl,imx95-pcie
> > > > > +    then:
> > > > > +      properties:
> > > > > +        reg:
> > > > > +          minItems: 4
> > > > > +        reg-names:
> > > > > +          items:
> > > > > +            - const: dbi
> > > > > +            - const: serdes
> > > > 
> > > > Did you test this? It should fail because 'serdes' would need to be 
> > > > added to snps,dw-pcie.yaml.
> > > 
> > > I run "make dt_binding_check DT_SCHEMA_FILES=/pci/", no error report.
> > 
> > Only because you have no example. What about your actual .dts?
> 
> I see. 95 is quite new. Still have not good base yet.
> I may just take take care this session.
> 
> > 
> > > And PCIe function can work.
> > > 
> > > > 
> > > > Is this really not a separate phy block?
> > > 
> > > This is misc block, which included phy and also include some registers
> > > about SID for each PCI devices. I plan do it later.
> > 
> > Sounds like it should be a separate node and use the phy binding. Do it 
> > correctly from the start, not later. Later is an ABI break.
> 
> Actually, I considerred phy binding. The major problem is LUT (look up
> table) for MSI and SMMU. LUT need be config according to some PCI device
> information. I have not find good hook for that at PHY driver.
> 
> > 
> > What is SID?
> 
> Stream ID, each device master have SID, which pass to IOMMU and GIC ITS.
> 
> Frank

Similar case at

commit c6523c4a301d3adff7ddcf57515b9c847beb7566
Author: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date:   Fri May 6 18:21:02 2022 +0300

    dt-bindings: PCI: qcom: Specify reg-names explicitly
    
    Instead of specifying the enum of possible reg-names, specify them
    explicitly. This allows us to specify which chipsets need the "atu"
    regions and which do not. Also it clearly describes which platforms
    enumerate PCIe cores using the dbi region and which use parf region for
    that.
    
    Link: https://lore.kernel.org/r/20220506152107.1527552-4-dmitry.baryshkov@linaro.org
    Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
    Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
    Acked-by: Rob Herring <robh@kernel.org>


+            - const: dbi # DesignWare PCIe registers
+            - const: elbi # External local bus interface registers
+            - const: parf # Qualcomm specific registers
                      ^^^^

+            - const: config # PCIe configuration space

Qualcomm called "part",  nxp call "serdes"

Frank

> 
> > 
> > Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index 81bbb8728f0f9..b8fcf8258f031 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -29,6 +29,7 @@  properties:
       - fsl,imx8mq-pcie
       - fsl,imx8mm-pcie
       - fsl,imx8mp-pcie
+      - fsl,imx95-pcie
 
   reg:
     items:
@@ -90,6 +91,22 @@  required:
 allOf:
   - $ref: /schemas/pci/snps,dw-pcie.yaml#
   - $ref: /schemas/pci/fsl,imx6q-pcie-common.yaml#
+  - if:
+      properties:
+        compatible:
+          enum:
+            - fsl,imx95-pcie
+    then:
+      properties:
+        reg:
+          minItems: 4
+        reg-names:
+          items:
+            - const: dbi
+            - const: serdes
+            - const: atu
+            - const: config
+
   - if:
       properties:
         compatible:
@@ -111,6 +128,7 @@  allOf:
         compatible:
           enum:
             - fsl,imx8mq-pcie
+            - fsl,imx95-pcie
     then:
       properties:
         clocks: