diff mbox series

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

Message ID 20231129-j7200-tisci-s2r-v1-1-c1d5964ed574@bootlin.com
State New
Headers
Series Add suspend/resume support in ti_sci driver for j7200 |

Commit Message

Thomas Richard Nov. 29, 2023, 3:31 p.m. UTC 
  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(+)

Comments

Conor Dooley Nov. 29, 2023, 3:34 p.m. UTC | #1 
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.
Conor Dooley Nov. 29, 2023, 3:38 p.m. UTC | #2 
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.
Andrew Davis Nov. 29, 2023, 3:54 p.m. UTC | #3 
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: |
>
Conor Dooley Nov. 30, 2023, 3:42 p.m. UTC | #4 
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.
Nishanth Menon Dec. 1, 2023, 7:46 a.m. UTC | #5 
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.
diff mbox series

Patch

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: |