[PATCH 1/5] dt-bindings: arm: keystone: add ti,j7200-sci compatible

Thomas Richard posted 5 patches 2 years ago
[PATCH 1/5] dt-bindings: arm: keystone: add ti,j7200-sci compatible
Posted by Thomas Richard 2 years ago
On j7200, during suspend to ram the soc is powered-off.
At resume requested irqs shall be restored which is a different behavior
from other platforms.

Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
---
 Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
index c24ad0968f3e..53d9c58dcd70 100644
--- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
+++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
@@ -40,6 +40,8 @@ properties:
       - description: System controller on TI AM654 SoC
         items:
           - const: ti,am654-sci
+      - description: System controller on TI J7200 SOC
+          - const: ti,j7200-sci
 
   reg-names:
     description: |

-- 
2.39.2
Re: [PATCH 1/5] dt-bindings: arm: keystone: add ti,j7200-sci compatible
Posted by Nishanth Menon 2 years ago
On 16:31-20231129, Thomas Richard wrote:
> On j7200, during suspend to ram the soc is powered-off.
> At resume requested irqs shall be restored which is a different behavior
> from other platforms.
> 
> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> ---
>  Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> index c24ad0968f3e..53d9c58dcd70 100644
> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> @@ -40,6 +40,8 @@ properties:
>        - description: System controller on TI AM654 SoC
>          items:
>            - const: ti,am654-sci
> +      - description: System controller on TI J7200 SOC
> +          - const: ti,j7200-sci
>  
>    reg-names:
>      description: |
> 
> -- 
> 2.39.2
> 

Sorry, but I don't see why this is any different for all K3 devices.
they all follow the same pattern of usage.

Also, constraints you are speaking off is also present even for
am654-sci. just handle this in the driver.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D
Re: [PATCH 1/5] dt-bindings: arm: keystone: add ti,j7200-sci compatible
Posted by Andrew Davis 2 years ago
On 11/29/23 9:31 AM, Thomas Richard wrote:
> On j7200, during suspend to ram the soc is powered-off.
> At resume requested irqs shall be restored which is a different behavior
> from other platforms.

Why is J7200 different? All K3 can/will support off mode suspend
to RAM. The only difference is you are adding support for it to this
one SoC first. You are describing a software behavior, not hardware.
Using a compatible to describe if a SW feature is enabled is not a
correct use of DT.

Andrew

> 
> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> ---
>   Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> index c24ad0968f3e..53d9c58dcd70 100644
> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> @@ -40,6 +40,8 @@ properties:
>         - description: System controller on TI AM654 SoC
>           items:
>             - const: ti,am654-sci
> +      - description: System controller on TI J7200 SOC
> +          - const: ti,j7200-sci
>   
>     reg-names:
>       description: |
>
Re: [PATCH 1/5] dt-bindings: arm: keystone: add ti,j7200-sci compatible
Posted by Conor Dooley 2 years ago
On Wed, Nov 29, 2023 at 04:31:17PM +0100, Thomas Richard wrote:
> On j7200, during suspend to ram the soc is powered-off.
> At resume requested irqs shall be restored which is a different behavior
> from other platforms.
> 
> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.
Re: [PATCH 1/5] dt-bindings: arm: keystone: add ti,j7200-sci compatible
Posted by Conor Dooley 2 years ago
On Wed, Nov 29, 2023 at 03:34:20PM +0000, Conor Dooley wrote:
> On Wed, Nov 29, 2023 at 04:31:17PM +0100, Thomas Richard wrote:
> > On j7200, during suspend to ram the soc is powered-off.
> > At resume requested irqs shall be restored which is a different behavior
> > from other platforms.
> > 
> > Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> 
> Acked-by: Conor Dooley <conor.dooley@microchip.com>

Un-Acked. Your dts patch contradicts this one.

Is the programming model compatible with the existing devices? To be
compatible, the existing device only need to support a compatible subset
of behaviours.
If so, this patch is wrong. If not, then the dts one is.

Thanks,
Conor.
Re: [PATCH 1/5] dt-bindings: arm: keystone: add ti,j7200-sci compatible
Posted by Conor Dooley 2 years ago
On Wed, Nov 29, 2023 at 03:38:04PM +0000, Conor Dooley wrote:
> On Wed, Nov 29, 2023 at 03:34:20PM +0000, Conor Dooley wrote:
> > On Wed, Nov 29, 2023 at 04:31:17PM +0100, Thomas Richard wrote:
> > > On j7200, during suspend to ram the soc is powered-off.
> > > At resume requested irqs shall be restored which is a different behavior
> > > from other platforms.
> > > 
> > > Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> > 
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Un-Acked. Your dts patch contradicts this one.
> 
> Is the programming model compatible with the existing devices? To be
> compatible, the existing device only need to support a compatible subset
> of behaviours.
> If so, this patch is wrong. If not, then the dts one is.

Given Andrew's response, it looks like the dts patch is the correct one
of the two, and this patch should document the k2g as a fallback for the
jh7200.

Cheers,
Conor.