diff mbox series

[v5,1/2] dt-bindings: mfd: syscon: Add ti,j721e-acspcie-proxy-ctrl compatible

Message ID 20250110100331.1642242-1-romain.naour@smile.fr (mailing list archive)
State New
Headers show
Series [v5,1/2] dt-bindings: mfd: syscon: Add ti,j721e-acspcie-proxy-ctrl compatible | expand

Commit Message

Romain Naour Jan. 10, 2025, 10:03 a.m. UTC 
From: Romain Naour <romain.naour@skf.com>

The ACSPCIE_PROXY_CTRL registers within the CTRL_MMR space of TI's J721e
SoC are used to drive the reference clock to the PCIe Endpoint device via
the PAD IO Buffers. Add the compatible for allowing the PCIe driver to
obtain the regmap for the ACSPCIE_CTRL register within the System
Controller device-tree node in order to enable the PAD IO Buffers.

Using the ti,j721e-acspcie-proxy-ctrl compatible imply to use "Proxy1"
address (1A090h) instead of "Proxy0" (18090h) to access
CTRLMMR_ACSPCIE0_CTRL register:

  CTRLMMR_ACSPCIE0_CTRL Register (Proxy0 Offset = 18090h; Proxy1 Offset = 1A090h)

"Proxy0" is used as the default access path that can be locked with the
help of "CTRLMMR_LOCK0_KICK0" and "CTRLMMR_LOCK0_KICK1" registers.

The Technical Reference Manual for J721e SoC with details of the
ASCPCIE_CTRL registers is available at:
https://www.ti.com/lit/zip/spruil1

Signed-off-by: Romain Naour <romain.naour@skf.com>
---
v5:
  - Add missing change to the J721e system controller binding
    to avoid DT check warning when the new acspcie0_proxy_ctrl (syscon)
    will be added to J721e system controller node (Andrew Davis).

  https://lore.kernel.org/linux-devicetree/90f47fae-a493-471d-8fe6-e7df741161be@ti.com/

  - Explain why "Proxy1" address (1A090h) should be used while using
    ti,j721e-acspcie-proxy-ctrl compatible (Siddharth Vadapalli).

  https://lore.kernel.org/linux-devicetree/begojbvvrpyjfr3pye7mqwiw73ucw5ynepdfujssr4jx4vs33a@pwahnph3qesl/

v4: Add missing change in the second list (From Andrew Davis) [1]
  Rebase after the ti,j784s4-acspcie-proxy-ctrl compatible fix [2]
  [1] https://lore.kernel.org/linux-devicetree/20250103174524.28768-1-afd@ti.com/
  [2] https://lore.kernel.org/linux-devicetree/20250103174524.28768-2-afd@ti.com/

v3: new commit
---
 Documentation/devicetree/bindings/mfd/syscon.yaml           | 2 ++
 .../bindings/soc/ti/ti,j721e-system-controller.yaml         | 6 ++++++
 2 files changed, 8 insertions(+)

Comments

Rob Herring (Arm) Jan. 10, 2025, 3:35 p.m. UTC | #1 
On Fri, Jan 10, 2025 at 11:03:30AM +0100, Romain Naour wrote:
> From: Romain Naour <romain.naour@skf.com>
> 
> The ACSPCIE_PROXY_CTRL registers within the CTRL_MMR space of TI's J721e
> SoC are used to drive the reference clock to the PCIe Endpoint device via
> the PAD IO Buffers. Add the compatible for allowing the PCIe driver to
> obtain the regmap for the ACSPCIE_CTRL register within the System
> Controller device-tree node in order to enable the PAD IO Buffers.
> 
> Using the ti,j721e-acspcie-proxy-ctrl compatible imply to use "Proxy1"
> address (1A090h) instead of "Proxy0" (18090h) to access
> CTRLMMR_ACSPCIE0_CTRL register:
> 
>   CTRLMMR_ACSPCIE0_CTRL Register (Proxy0 Offset = 18090h; Proxy1 Offset = 1A090h)
> 
> "Proxy0" is used as the default access path that can be locked with the
> help of "CTRLMMR_LOCK0_KICK0" and "CTRLMMR_LOCK0_KICK1" registers.
> 
> The Technical Reference Manual for J721e SoC with details of the
> ASCPCIE_CTRL registers is available at:
> https://www.ti.com/lit/zip/spruil1
> 
> Signed-off-by: Romain Naour <romain.naour@skf.com>
> ---
> v5:
>   - Add missing change to the J721e system controller binding
>     to avoid DT check warning when the new acspcie0_proxy_ctrl (syscon)
>     will be added to J721e system controller node (Andrew Davis).
> 
>   https://lore.kernel.org/linux-devicetree/90f47fae-a493-471d-8fe6-e7df741161be@ti.com/
> 
>   - Explain why "Proxy1" address (1A090h) should be used while using
>     ti,j721e-acspcie-proxy-ctrl compatible (Siddharth Vadapalli).
> 
>   https://lore.kernel.org/linux-devicetree/begojbvvrpyjfr3pye7mqwiw73ucw5ynepdfujssr4jx4vs33a@pwahnph3qesl/
> 
> v4: Add missing change in the second list (From Andrew Davis) [1]
>   Rebase after the ti,j784s4-acspcie-proxy-ctrl compatible fix [2]
>   [1] https://lore.kernel.org/linux-devicetree/20250103174524.28768-1-afd@ti.com/
>   [2] https://lore.kernel.org/linux-devicetree/20250103174524.28768-2-afd@ti.com/
> 
> v3: new commit
> ---
>  Documentation/devicetree/bindings/mfd/syscon.yaml           | 2 ++
>  .../bindings/soc/ti/ti,j721e-system-controller.yaml         | 6 ++++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
> index 0e68c69e7bc9..1f3e67f432e7 100644
> --- a/Documentation/devicetree/bindings/mfd/syscon.yaml
> +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
> @@ -115,6 +115,7 @@ select:
>            - ti,am625-dss-oldi-io-ctrl
>            - ti,am62p-cpsw-mac-efuse
>            - ti,am654-dss-oldi-io-ctrl
> +          - ti,j721e-acspcie-proxy-ctrl
>            - ti,j784s4-acspcie-proxy-ctrl
>            - ti,j784s4-pcie-ctrl
>            - ti,keystone-pllctrl
> @@ -213,6 +214,7 @@ properties:
>            - ti,am625-dss-oldi-io-ctrl
>            - ti,am62p-cpsw-mac-efuse
>            - ti,am654-dss-oldi-io-ctrl

> +          - ti,j721e-acspcie-proxy-ctrl
>            - ti,j784s4-acspcie-proxy-ctrl

How do these 2 compare? Are they compatible?

>            - ti,j784s4-pcie-ctrl
>            - ti,keystone-pllctrl
> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml b/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
> index 378e9cc5fac2..16929218d611 100644
> --- a/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
> +++ b/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
> @@ -68,6 +68,12 @@ patternProperties:
>      description:
>        The node corresponding to SoC chip identification.
>  
> +  "^syscon@[0-9a-f]+$":
> +    type: object
> +    $ref: /schemas/mfd/syscon.yaml#
> +    description:
> +      This is the ASPCIe control region.

So this is a syscon child of a syscon. The primary reason for 'syscon' 
compatible is to create a regmap. Why can't you use the parent's syscon?

Rob
Andrew Davis Jan. 10, 2025, 4:17 p.m. UTC | #2 
On 1/10/25 9:35 AM, Rob Herring wrote:
> On Fri, Jan 10, 2025 at 11:03:30AM +0100, Romain Naour wrote:
>> From: Romain Naour <romain.naour@skf.com>
>>
>> The ACSPCIE_PROXY_CTRL registers within the CTRL_MMR space of TI's J721e
>> SoC are used to drive the reference clock to the PCIe Endpoint device via
>> the PAD IO Buffers. Add the compatible for allowing the PCIe driver to
>> obtain the regmap for the ACSPCIE_CTRL register within the System
>> Controller device-tree node in order to enable the PAD IO Buffers.
>>
>> Using the ti,j721e-acspcie-proxy-ctrl compatible imply to use "Proxy1"
>> address (1A090h) instead of "Proxy0" (18090h) to access
>> CTRLMMR_ACSPCIE0_CTRL register:
>>
>>    CTRLMMR_ACSPCIE0_CTRL Register (Proxy0 Offset = 18090h; Proxy1 Offset = 1A090h)
>>
>> "Proxy0" is used as the default access path that can be locked with the
>> help of "CTRLMMR_LOCK0_KICK0" and "CTRLMMR_LOCK0_KICK1" registers.
>>
>> The Technical Reference Manual for J721e SoC with details of the
>> ASCPCIE_CTRL registers is available at:
>> https://www.ti.com/lit/zip/spruil1
>>
>> Signed-off-by: Romain Naour <romain.naour@skf.com>
>> ---
>> v5:
>>    - Add missing change to the J721e system controller binding
>>      to avoid DT check warning when the new acspcie0_proxy_ctrl (syscon)
>>      will be added to J721e system controller node (Andrew Davis).
>>
>>    https://lore.kernel.org/linux-devicetree/90f47fae-a493-471d-8fe6-e7df741161be@ti.com/
>>
>>    - Explain why "Proxy1" address (1A090h) should be used while using
>>      ti,j721e-acspcie-proxy-ctrl compatible (Siddharth Vadapalli).
>>
>>    https://lore.kernel.org/linux-devicetree/begojbvvrpyjfr3pye7mqwiw73ucw5ynepdfujssr4jx4vs33a@pwahnph3qesl/
>>
>> v4: Add missing change in the second list (From Andrew Davis) [1]
>>    Rebase after the ti,j784s4-acspcie-proxy-ctrl compatible fix [2]
>>    [1] https://lore.kernel.org/linux-devicetree/20250103174524.28768-1-afd@ti.com/
>>    [2] https://lore.kernel.org/linux-devicetree/20250103174524.28768-2-afd@ti.com/
>>
>> v3: new commit
>> ---
>>   Documentation/devicetree/bindings/mfd/syscon.yaml           | 2 ++
>>   .../bindings/soc/ti/ti,j721e-system-controller.yaml         | 6 ++++++
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
>> index 0e68c69e7bc9..1f3e67f432e7 100644
>> --- a/Documentation/devicetree/bindings/mfd/syscon.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
>> @@ -115,6 +115,7 @@ select:
>>             - ti,am625-dss-oldi-io-ctrl
>>             - ti,am62p-cpsw-mac-efuse
>>             - ti,am654-dss-oldi-io-ctrl
>> +          - ti,j721e-acspcie-proxy-ctrl
>>             - ti,j784s4-acspcie-proxy-ctrl
>>             - ti,j784s4-pcie-ctrl
>>             - ti,keystone-pllctrl
>> @@ -213,6 +214,7 @@ properties:
>>             - ti,am625-dss-oldi-io-ctrl
>>             - ti,am62p-cpsw-mac-efuse
>>             - ti,am654-dss-oldi-io-ctrl
> 
>> +          - ti,j721e-acspcie-proxy-ctrl
>>             - ti,j784s4-acspcie-proxy-ctrl
> 
> How do these 2 compare? Are they compatible?
> 

Yes, they are 100% identical and compatible, but we were told
to make a new string anyway.. [0]

[0] https://lore.kernel.org/all/1bfdf1f1-7542-4149-a85d-2ac4b659b26b@kernel.org/


>>             - ti,j784s4-pcie-ctrl
>>             - ti,keystone-pllctrl
>> diff --git a/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml b/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
>> index 378e9cc5fac2..16929218d611 100644
>> --- a/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
>> +++ b/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
>> @@ -68,6 +68,12 @@ patternProperties:
>>       description:
>>         The node corresponding to SoC chip identification.
>>   
>> +  "^syscon@[0-9a-f]+$":
>> +    type: object
>> +    $ref: /schemas/mfd/syscon.yaml#
>> +    description:
>> +      This is the ASPCIe control region.
> 
> So this is a syscon child of a syscon. The primary reason for 'syscon'
> compatible is to create a regmap. Why can't you use the parent's syscon?
> 

The parent node will not be a syscon soon. We made this whole bus a "syscon"
so we could just poke any register we wanted which was a hacky solution we
want to fix. The parent will be converted into a normal "simple-bus".

Most of the IP in this region can be described using normal DT devices,
but there are still just a couple registers like this where we need a raw
syscon (or we could make a proper device driver for these registers, but
that might be excessive, instead seems easy enough to just poke them
directly from the PCIe driver).

Andrew

> Rob
Rob Herring (Arm) Jan. 10, 2025, 5:35 p.m. UTC | #3 
On Fri, Jan 10, 2025 at 10:17:15AM -0600, Andrew Davis wrote:
> On 1/10/25 9:35 AM, Rob Herring wrote:
> > On Fri, Jan 10, 2025 at 11:03:30AM +0100, Romain Naour wrote:
> > > From: Romain Naour <romain.naour@skf.com>
> > > 
> > > The ACSPCIE_PROXY_CTRL registers within the CTRL_MMR space of TI's J721e
> > > SoC are used to drive the reference clock to the PCIe Endpoint device via
> > > the PAD IO Buffers. Add the compatible for allowing the PCIe driver to
> > > obtain the regmap for the ACSPCIE_CTRL register within the System
> > > Controller device-tree node in order to enable the PAD IO Buffers.
> > > 
> > > Using the ti,j721e-acspcie-proxy-ctrl compatible imply to use "Proxy1"
> > > address (1A090h) instead of "Proxy0" (18090h) to access
> > > CTRLMMR_ACSPCIE0_CTRL register:
> > > 
> > >    CTRLMMR_ACSPCIE0_CTRL Register (Proxy0 Offset = 18090h; Proxy1 Offset = 1A090h)
> > > 
> > > "Proxy0" is used as the default access path that can be locked with the
> > > help of "CTRLMMR_LOCK0_KICK0" and "CTRLMMR_LOCK0_KICK1" registers.
> > > 
> > > The Technical Reference Manual for J721e SoC with details of the
> > > ASCPCIE_CTRL registers is available at:
> > > https://www.ti.com/lit/zip/spruil1
> > > 
> > > Signed-off-by: Romain Naour <romain.naour@skf.com>
> > > ---
> > > v5:
> > >    - Add missing change to the J721e system controller binding
> > >      to avoid DT check warning when the new acspcie0_proxy_ctrl (syscon)
> > >      will be added to J721e system controller node (Andrew Davis).
> > > 
> > >    https://lore.kernel.org/linux-devicetree/90f47fae-a493-471d-8fe6-e7df741161be@ti.com/
> > > 
> > >    - Explain why "Proxy1" address (1A090h) should be used while using
> > >      ti,j721e-acspcie-proxy-ctrl compatible (Siddharth Vadapalli).
> > > 
> > >    https://lore.kernel.org/linux-devicetree/begojbvvrpyjfr3pye7mqwiw73ucw5ynepdfujssr4jx4vs33a@pwahnph3qesl/
> > > 
> > > v4: Add missing change in the second list (From Andrew Davis) [1]
> > >    Rebase after the ti,j784s4-acspcie-proxy-ctrl compatible fix [2]
> > >    [1] https://lore.kernel.org/linux-devicetree/20250103174524.28768-1-afd@ti.com/
> > >    [2] https://lore.kernel.org/linux-devicetree/20250103174524.28768-2-afd@ti.com/
> > > 
> > > v3: new commit
> > > ---
> > >   Documentation/devicetree/bindings/mfd/syscon.yaml           | 2 ++
> > >   .../bindings/soc/ti/ti,j721e-system-controller.yaml         | 6 ++++++
> > >   2 files changed, 8 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
> > > index 0e68c69e7bc9..1f3e67f432e7 100644
> > > --- a/Documentation/devicetree/bindings/mfd/syscon.yaml
> > > +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
> > > @@ -115,6 +115,7 @@ select:
> > >             - ti,am625-dss-oldi-io-ctrl
> > >             - ti,am62p-cpsw-mac-efuse
> > >             - ti,am654-dss-oldi-io-ctrl
> > > +          - ti,j721e-acspcie-proxy-ctrl
> > >             - ti,j784s4-acspcie-proxy-ctrl
> > >             - ti,j784s4-pcie-ctrl
> > >             - ti,keystone-pllctrl
> > > @@ -213,6 +214,7 @@ properties:
> > >             - ti,am625-dss-oldi-io-ctrl
> > >             - ti,am62p-cpsw-mac-efuse
> > >             - ti,am654-dss-oldi-io-ctrl
> > 
> > > +          - ti,j721e-acspcie-proxy-ctrl
> > >             - ti,j784s4-acspcie-proxy-ctrl
> > 
> > How do these 2 compare? Are they compatible?
> > 
> 
> Yes, they are 100% identical and compatible, but we were told
> to make a new string anyway.. [0]
> 
> [0] https://lore.kernel.org/all/1bfdf1f1-7542-4149-a85d-2ac4b659b26b@kernel.org/

Then you should have:

"ti,j721e-acspcie-proxy-ctrl", "ti,j784s4-acspcie-proxy-ctrl", "syscon"

> 
> 
> > >             - ti,j784s4-pcie-ctrl
> > >             - ti,keystone-pllctrl
> > > diff --git a/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml b/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
> > > index 378e9cc5fac2..16929218d611 100644
> > > --- a/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
> > > +++ b/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
> > > @@ -68,6 +68,12 @@ patternProperties:
> > >       description:
> > >         The node corresponding to SoC chip identification.
> > > +  "^syscon@[0-9a-f]+$":
> > > +    type: object
> > > +    $ref: /schemas/mfd/syscon.yaml#
> > > +    description:
> > > +      This is the ASPCIe control region.
> > 
> > So this is a syscon child of a syscon. The primary reason for 'syscon'
> > compatible is to create a regmap. Why can't you use the parent's syscon?
> > 
> 
> The parent node will not be a syscon soon. We made this whole bus a "syscon"
> so we could just poke any register we wanted which was a hacky solution we
> want to fix. The parent will be converted into a normal "simple-bus".

Sigh... And that can be done without ABI breakage?

> Most of the IP in this region can be described using normal DT devices,
> but there are still just a couple registers like this where we need a raw
> syscon (or we could make a proper device driver for these registers, but
> that might be excessive, instead seems easy enough to just poke them
> directly from the PCIe driver).

Given it was already a syscon, keeping that is fine.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index 0e68c69e7bc9..1f3e67f432e7 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -115,6 +115,7 @@  select:
           - ti,am625-dss-oldi-io-ctrl
           - ti,am62p-cpsw-mac-efuse
           - ti,am654-dss-oldi-io-ctrl
+          - ti,j721e-acspcie-proxy-ctrl
           - ti,j784s4-acspcie-proxy-ctrl
           - ti,j784s4-pcie-ctrl
           - ti,keystone-pllctrl
@@ -213,6 +214,7 @@  properties:
           - ti,am625-dss-oldi-io-ctrl
           - ti,am62p-cpsw-mac-efuse
           - ti,am654-dss-oldi-io-ctrl
+          - ti,j721e-acspcie-proxy-ctrl
           - ti,j784s4-acspcie-proxy-ctrl
           - ti,j784s4-pcie-ctrl
           - ti,keystone-pllctrl
diff --git a/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml b/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
index 378e9cc5fac2..16929218d611 100644
--- a/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
+++ b/Documentation/devicetree/bindings/soc/ti/ti,j721e-system-controller.yaml
@@ -68,6 +68,12 @@  patternProperties:
     description:
       The node corresponding to SoC chip identification.
 
+  "^syscon@[0-9a-f]+$":
+    type: object
+    $ref: /schemas/mfd/syscon.yaml#
+    description:
+      This is the ASPCIe control region.
+
 required:
   - compatible
   - reg