[PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add SCMI System Power Protocol bindings, and the protocol id is 0x12.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 4591523b51a0..d99f1577476e 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -141,6 +141,14 @@ properties:
     required:
       - '#power-domain-cells'
 
+  protocol@12:
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
+    properties:
+      reg:
+        const: 0x12
+
   protocol@13:
     $ref: '#/$defs/protocol-node'
     unevaluatedProperties: false
-- 
2.37.1

Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[Cristian Marussi]

On Mon, Feb 26, 2024 at 09:02:43PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add SCMI System Power Protocol bindings, and the protocol id is 0x12.
> 
Hi,

yes this is something I spotted in the past it was missing and I posted
a similar patch but I was told that a protocol node without any specific
additional properties is already being described by the general protocol
node described above.

Thanks,
Cristian

> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index 4591523b51a0..d99f1577476e 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -141,6 +141,14 @@ properties:
>      required:
>        - '#power-domain-cells'
>  
> +  protocol@12:
> +    $ref: '#/$defs/protocol-node'
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        const: 0x12
> +
>    protocol@13:
>      $ref: '#/$defs/protocol-node'
>      unevaluatedProperties: false
> -- 
> 2.37.1
> 

Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[Cristian Marussi]

On Mon, Feb 26, 2024 at 01:28:31PM +0000, Cristian Marussi wrote:
> On Mon, Feb 26, 2024 at 09:02:43PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> > 
> > Add SCMI System Power Protocol bindings, and the protocol id is 0x12.
> > 
> Hi,
> 
> yes this is something I spotted in the past it was missing and I posted
> a similar patch but I was told that a protocol node without any specific
> additional properties is already being described by the general protocol
> node described above.
> 


I was referring to this old thread...

https://lore.kernel.org/all/20230124222023.316089-1-robh@kernel.org/

Thanks,
Cristian

RE: [PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[Peng Fan]

> Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power
> protocol
>
> On Mon, Feb 26, 2024 at 01:28:31PM +0000, Cristian Marussi wrote:
> > On Mon, Feb 26, 2024 at 09:02:43PM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > Add SCMI System Power Protocol bindings, and the protocol id is 0x12.
> > >
> > Hi,
> >
> > yes this is something I spotted in the past it was missing and I
> > posted a similar patch but I was told that a protocol node without any
> > specific additional properties is already being described by the
> > general protocol node described above.

Without this patch, there is dtbs_check warning.

scmi: 'protocol@12' does not match any of the regexes: 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/firmware/arm,scmi.yaml#

Regards,
Peng.

> >
>
>
> I was referring to this old thread...
>
> https://lore.ke/
> rnel.org%2Fall%2F20230124222023.316089-1-
> robh%40kernel.org%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C73d2d
> 867857f409cfd2608dc36dbf475%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> 7C0%7C0%7C638445565544388755%7CUnknown%7CTWFpbGZsb3d8eyJWI
> joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
> 0%7C%7C%7C&sdata=SDKSIbYQq5YS4M7SJXyWwaPLVvQK6wIPtjUYi%2FW3
> p6k%3D&reserved=0
>
> Thanks,
> Cristian

Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[Sudeep Holla]

On Tue, Feb 27, 2024 at 01:01:41AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power
> > protocol
> >
> > On Mon, Feb 26, 2024 at 01:28:31PM +0000, Cristian Marussi wrote:
> > > On Mon, Feb 26, 2024 at 09:02:43PM +0800, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > Add SCMI System Power Protocol bindings, and the protocol id is 0x12.
> > > >
> > > Hi,
> > >
> > > yes this is something I spotted in the past it was missing and I
> > > posted a similar patch but I was told that a protocol node without any
> > > specific additional properties is already being described by the
> > > general protocol node described above.
> 
> Without this patch, there is dtbs_check warning.
> 
> scmi: 'protocol@12' does not match any of the regexes: 'pinctrl-[0-9]+'
> from schema $id: http://devicetree.org/schemas/firmware/arm,scmi.yaml#
> 

Why are you adding protocol@12 to the device tree ? Does it have a
dedicated channel ? If not, you shouldn't need to add it.

-- 
Regards,
Sudeep

RE: [PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[Peng Fan]

> Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power
> protocol
> 
> On Tue, Feb 27, 2024 at 01:01:41AM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system
> > > power protocol
> > >
> > > On Mon, Feb 26, 2024 at 01:28:31PM +0000, Cristian Marussi wrote:
> > > > On Mon, Feb 26, 2024 at 09:02:43PM +0800, Peng Fan (OSS) wrote:
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > >
> > > > > Add SCMI System Power Protocol bindings, and the protocol id is 0x12.
> > > > >
> > > > Hi,
> > > >
> > > > yes this is something I spotted in the past it was missing and I
> > > > posted a similar patch but I was told that a protocol node without
> > > > any specific additional properties is already being described by
> > > > the general protocol node described above.
> >
> > Without this patch, there is dtbs_check warning.
> >
> > scmi: 'protocol@12' does not match any of the regexes: 'pinctrl-[0-9]+'
> > from schema $id:
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevic
> >
> etree.org%2Fschemas%2Ffirmware%2Farm%2Cscmi.yaml%23&data=05%7C0
> 2%7Cpen
> >
> g.fan%40nxp.com%7Ccac77deb5f6a4b20460a08dc392ead40%7C686ea1d3b
> c2b4c6fa
> >
> 92cd99c5c301635%7C0%7C0%7C638448119832543335%7CUnknown%7CT
> WFpbGZsb3d8e
> >
> yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> %7C0%
> >
> 7C%7C%7C&sdata=6MldIOUQ4hxn%2BRffwJJJ3jxXXtHCSxLUOa4JMWB0htU%
> 3D&reserv
> > ed=0
> >
> 
> Why are you adding protocol@12 to the device tree ? Does it have a
> dedicated channel ? If not, you shouldn't need to add it.

No dedicated channel.
The idea is we have multile Agent, the M7 agent may ask to shutdown Linux
Agent. So the linux agent need use protocol@12 to do the action.

For now, we have not finish implementing this in linux side, just add
the node in dts.

Regards,
Peng.
> 
> --
> Regards,
> Sudeep

Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[Cristian Marussi]

On Fri, Mar 01, 2024 at 12:41:29AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power
> > protocol
> > 
> > On Tue, Feb 27, 2024 at 01:01:41AM +0000, Peng Fan wrote:
> > > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system
> > > > power protocol
> > > >
> > > > On Mon, Feb 26, 2024 at 01:28:31PM +0000, Cristian Marussi wrote:
> > > > > On Mon, Feb 26, 2024 at 09:02:43PM +0800, Peng Fan (OSS) wrote:
> > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > >
> > > > > > Add SCMI System Power Protocol bindings, and the protocol id is 0x12.
> > > > > >
> > > > > Hi,
> > > > >
> > > > > yes this is something I spotted in the past it was missing and I
> > > > > posted a similar patch but I was told that a protocol node without
> > > > > any specific additional properties is already being described by
> > > > > the general protocol node described above.
> > >
> > > Without this patch, there is dtbs_check warning.
> > >
> > > scmi: 'protocol@12' does not match any of the regexes: 'pinctrl-[0-9]+'
> > > from schema $id:
> > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevic
> > >
> > etree.org%2Fschemas%2Ffirmware%2Farm%2Cscmi.yaml%23&data=05%7C0
> > 2%7Cpen
> > >
> > g.fan%40nxp.com%7Ccac77deb5f6a4b20460a08dc392ead40%7C686ea1d3b
> > c2b4c6fa
> > >
> > 92cd99c5c301635%7C0%7C0%7C638448119832543335%7CUnknown%7CT
> > WFpbGZsb3d8e
> > >
> > yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> > %7C0%
> > >
> > 7C%7C%7C&sdata=6MldIOUQ4hxn%2BRffwJJJ3jxXXtHCSxLUOa4JMWB0htU%
> > 3D&reserv
> > > ed=0
> > >
> > 
> > Why are you adding protocol@12 to the device tree ? Does it have a
> > dedicated channel ? If not, you shouldn't need to add it.
> 
> No dedicated channel.
> The idea is we have multile Agent, the M7 agent may ask to shutdown Linux
> Agent. So the linux agent need use protocol@12 to do the action.
> 

Ok, so indeed you need, AFAICR, the node in the DT even without a
dedicated channel nor anynone referring it: no DT node means no protocol
initialization.

> For now, we have not finish implementing this in linux side, just add
> the node in dts.

On the linux side you should not need to do anything but adding the node
@12, the existent drivers/firmware/arm_scmi/system_power_control.c
driver when loaded will shutdown the system upon receiving the SytemPower
notification....what lacks Linux side in these regards ?

Thanks,
Cristian

RE: [PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[Peng Fan]

> Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power
> protocol
> 
> On Fri, Mar 01, 2024 at 12:41:29AM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system
> > > power protocol
> > >
> > > On Tue, Feb 27, 2024 at 01:01:41AM +0000, Peng Fan wrote:
> > > > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support
> > > > > system power protocol
> > > > >
> > > > > On Mon, Feb 26, 2024 at 01:28:31PM +0000, Cristian Marussi wrote:
> > > > > > On Mon, Feb 26, 2024 at 09:02:43PM +0800, Peng Fan (OSS) wrote:
> > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > >
> > > > > > > Add SCMI System Power Protocol bindings, and the protocol id is
> 0x12.
> > > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > yes this is something I spotted in the past it was missing and
> > > > > > I posted a similar patch but I was told that a protocol node
> > > > > > without any specific additional properties is already being
> > > > > > described by the general protocol node described above.
> > > >
> > > > Without this patch, there is dtbs_check warning.
> > > >
> > > > scmi: 'protocol@12' does not match any of the regexes: 'pinctrl-[0-9]+'
> > > > from schema $id:
> > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fd
> > > >
> evic%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C3ef5d152a8e5463e9b
> 3408
> > > >
> dc39c47e41%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63844
> 87632
> > > >
> 86203182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
> V2luM
> > > >
> zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=kRBV9FPk6
> Nrgu
> > > > Hs2SHpL74VlNUH6cYO1kAsW74CkaTk%3D&reserved=0
> > > >
> > >
> etree.org%2Fschemas%2Ffirmware%2Farm%2Cscmi.yaml%23&data=05%7C0
> > > 2%7Cpen
> > > >
> > >
> g.fan%40nxp.com%7Ccac77deb5f6a4b20460a08dc392ead40%7C686ea1d3b
> > > c2b4c6fa
> > > >
> > >
> 92cd99c5c301635%7C0%7C0%7C638448119832543335%7CUnknown%7CT
> > > WFpbGZsb3d8e
> > > >
> > >
> yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> > > %7C0%
> > > >
> > >
> 7C%7C%7C&sdata=6MldIOUQ4hxn%2BRffwJJJ3jxXXtHCSxLUOa4JMWB0htU%
> > > 3D&reserv
> > > > ed=0
> > > >
> > >
> > > Why are you adding protocol@12 to the device tree ? Does it have a
> > > dedicated channel ? If not, you shouldn't need to add it.
> >
> > No dedicated channel.
> > The idea is we have multile Agent, the M7 agent may ask to shutdown
> > Linux Agent. So the linux agent need use protocol@12 to do the action.
> >
> 
> Ok, so indeed you need, AFAICR, the node in the DT even without a dedicated
> channel nor anynone referring it: no DT node means no protocol initialization.

Yeah, so this binding update is needed, right?

> 
> > For now, we have not finish implementing this in linux side, just add
> > the node in dts.
> 
> On the linux side you should not need to do anything but adding the node
> @12, the existent drivers/firmware/arm_scmi/system_power_control.c
> driver when loaded will shutdown the system upon receiving the
> SytemPower notification....what lacks Linux side in these regards ?

" Ignoring unsupported system_state:" we have new defined
number here.

Regards,
Peng.

> 
> Thanks,
> Cristian

Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[Sudeep Holla]

On Fri, Mar 01, 2024 at 09:22:24AM +0000, Peng Fan wrote:
> " Ignoring unsupported system_state:" we have new defined
> number here.

Why ?

-- 
Regards,
Sudeep

Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[Cristian Marussi]

On Fri, Mar 01, 2024 at 09:22:24AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power
> > protocol
> > 
> > On Fri, Mar 01, 2024 at 12:41:29AM +0000, Peng Fan wrote:
> > > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system
> > > > power protocol
> > > >
> > > > On Tue, Feb 27, 2024 at 01:01:41AM +0000, Peng Fan wrote:
> > > > > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support
> > > > > > system power protocol
> > > > > >
> > > > > > On Mon, Feb 26, 2024 at 01:28:31PM +0000, Cristian Marussi wrote:
> > > > > > > On Mon, Feb 26, 2024 at 09:02:43PM +0800, Peng Fan (OSS) wrote:
> > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > >
> > > > > > > > Add SCMI System Power Protocol bindings, and the protocol id is
> > 0x12.
> > > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > yes this is something I spotted in the past it was missing and
> > > > > > > I posted a similar patch but I was told that a protocol node
> > > > > > > without any specific additional properties is already being
> > > > > > > described by the general protocol node described above.
> > > > >
> > > > > Without this patch, there is dtbs_check warning.
> > > > >
> > > > > scmi: 'protocol@12' does not match any of the regexes: 'pinctrl-[0-9]+'
> > > > > from schema $id:
> > > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fd
> > > > >
> > evic%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C3ef5d152a8e5463e9b
> > 3408
> > > > >
> > dc39c47e41%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63844
> > 87632
> > > > >
> > 86203182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
> > V2luM
> > > > >
> > zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=kRBV9FPk6
> > Nrgu
> > > > > Hs2SHpL74VlNUH6cYO1kAsW74CkaTk%3D&reserved=0
> > > > >
> > > >
> > etree.org%2Fschemas%2Ffirmware%2Farm%2Cscmi.yaml%23&data=05%7C0
> > > > 2%7Cpen
> > > > >
> > > >
> > g.fan%40nxp.com%7Ccac77deb5f6a4b20460a08dc392ead40%7C686ea1d3b
> > > > c2b4c6fa
> > > > >
> > > >
> > 92cd99c5c301635%7C0%7C0%7C638448119832543335%7CUnknown%7CT
> > > > WFpbGZsb3d8e
> > > > >
> > > >
> > yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> > > > %7C0%
> > > > >
> > > >
> > 7C%7C%7C&sdata=6MldIOUQ4hxn%2BRffwJJJ3jxXXtHCSxLUOa4JMWB0htU%
> > > > 3D&reserv
> > > > > ed=0
> > > > >
> > > >
> > > > Why are you adding protocol@12 to the device tree ? Does it have a
> > > > dedicated channel ? If not, you shouldn't need to add it.
> > >
> > > No dedicated channel.
> > > The idea is we have multile Agent, the M7 agent may ask to shutdown
> > > Linux Agent. So the linux agent need use protocol@12 to do the action.
> > >
> > 
> > Ok, so indeed you need, AFAICR, the node in the DT even without a dedicated
> > channel nor anynone referring it: no DT node means no protocol initialization.
> 
> Yeah, so this binding update is needed, right?
> 
> > 
> > > For now, we have not finish implementing this in linux side, just add
> > > the node in dts.
> > 
> > On the linux side you should not need to do anything but adding the node
> > @12, the existent drivers/firmware/arm_scmi/system_power_control.c
> > driver when loaded will shutdown the system upon receiving the
> > SytemPower notification....what lacks Linux side in these regards ?
> 
> " Ignoring unsupported system_state:" we have new defined
> number here.
> 

So I suppose you are using the Vendor defined system states and writing
your own driver to handle these instead of system_power_control ?

Thanks,
Cristian

Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[Cristian Marussi]

On Fri, Mar 01, 2024 at 10:37:49AM +0000, Cristian Marussi wrote:
> On Fri, Mar 01, 2024 at 09:22:24AM +0000, Peng Fan wrote:
> > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power
> > > protocol
> > > 
> > > On Fri, Mar 01, 2024 at 12:41:29AM +0000, Peng Fan wrote:
> > > > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system
> > > > > power protocol
> > > > >
> > > > > On Tue, Feb 27, 2024 at 01:01:41AM +0000, Peng Fan wrote:
> > > > > > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support
> > > > > > > system power protocol
> > > > > > >
> > > > > > > On Mon, Feb 26, 2024 at 01:28:31PM +0000, Cristian Marussi wrote:
> > > > > > > > On Mon, Feb 26, 2024 at 09:02:43PM +0800, Peng Fan (OSS) wrote:
> > > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > > >
> > > > > > > > > Add SCMI System Power Protocol bindings, and the protocol id is
> > > 0x12.
> > > > > > > > >
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > yes this is something I spotted in the past it was missing and
> > > > > > > > I posted a similar patch but I was told that a protocol node
> > > > > > > > without any specific additional properties is already being
> > > > > > > > described by the general protocol node described above.
> > > > > >
> > > > > > Without this patch, there is dtbs_check warning.
> > > > > >
> > > > > > scmi: 'protocol@12' does not match any of the regexes: 'pinctrl-[0-9]+'
> > > > > > from schema $id:
> > > > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fd
> > > > > >
> > > evic%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C3ef5d152a8e5463e9b
> > > 3408
> > > > > >
> > > dc39c47e41%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63844
> > > 87632
> > > > > >
> > > 86203182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
> > > V2luM
> > > > > >
> > > zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=kRBV9FPk6
> > > Nrgu
> > > > > > Hs2SHpL74VlNUH6cYO1kAsW74CkaTk%3D&reserved=0
> > > > > >
> > > > >
> > > etree.org%2Fschemas%2Ffirmware%2Farm%2Cscmi.yaml%23&data=05%7C0
> > > > > 2%7Cpen
> > > > > >
> > > > >
> > > g.fan%40nxp.com%7Ccac77deb5f6a4b20460a08dc392ead40%7C686ea1d3b
> > > > > c2b4c6fa
> > > > > >
> > > > >
> > > 92cd99c5c301635%7C0%7C0%7C638448119832543335%7CUnknown%7CT
> > > > > WFpbGZsb3d8e
> > > > > >
> > > > >
> > > yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> > > > > %7C0%
> > > > > >
> > > > >
> > > 7C%7C%7C&sdata=6MldIOUQ4hxn%2BRffwJJJ3jxXXtHCSxLUOa4JMWB0htU%
> > > > > 3D&reserv
> > > > > > ed=0
> > > > > >
> > > > >
> > > > > Why are you adding protocol@12 to the device tree ? Does it have a
> > > > > dedicated channel ? If not, you shouldn't need to add it.
> > > >
> > > > No dedicated channel.
> > > > The idea is we have multile Agent, the M7 agent may ask to shutdown
> > > > Linux Agent. So the linux agent need use protocol@12 to do the action.
> > > >
> > > 
> > > Ok, so indeed you need, AFAICR, the node in the DT even without a dedicated
> > > channel nor anynone referring it: no DT node means no protocol initialization.
> > 
> > Yeah, so this binding update is needed, right?
> > 
> > > 
> > > > For now, we have not finish implementing this in linux side, just add
> > > > the node in dts.
> > > 
> > > On the linux side you should not need to do anything but adding the node
> > > @12, the existent drivers/firmware/arm_scmi/system_power_control.c
> > > driver when loaded will shutdown the system upon receiving the
> > > SytemPower notification....what lacks Linux side in these regards ?
> > 
> > " Ignoring unsupported system_state:" we have new defined
> > number here.
> > 
> 
> So I suppose you are using the Vendor defined system states and writing
> your own driver to handle these instead of system_power_control ?

Or some vendor extensions to the std driver ?
IOW what is the plan...custom vendor states was left out from
system_power_control waiting for a real usecase...that seems to have
shown up eventually :D

Thanks,
Cristian

RE: [PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[Peng Fan]

> Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power
> protocol
> 
> On Fri, Mar 01, 2024 at 10:37:49AM +0000, Cristian Marussi wrote:
> > On Fri, Mar 01, 2024 at 09:22:24AM +0000, Peng Fan wrote:
> > > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support
> > > > system power protocol
> > > >
> > > > On Fri, Mar 01, 2024 at 12:41:29AM +0000, Peng Fan wrote:
> > > > > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support
> > > > > > system power protocol
> > > > > >
> > > > > > On Tue, Feb 27, 2024 at 01:01:41AM +0000, Peng Fan wrote:
> > > > > > > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi:
> > > > > > > > support system power protocol
> > > > > > > >
> > > > > > > > On Mon, Feb 26, 2024 at 01:28:31PM +0000, Cristian Marussi
> wrote:
> > > > > > > > > On Mon, Feb 26, 2024 at 09:02:43PM +0800, Peng Fan (OSS)
> wrote:
> > > > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > > > >
> > > > > > > > > > Add SCMI System Power Protocol bindings, and the
> > > > > > > > > > protocol id is
> > > > 0x12.
> > > > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > yes this is something I spotted in the past it was
> > > > > > > > > missing and I posted a similar patch but I was told that
> > > > > > > > > a protocol node without any specific additional
> > > > > > > > > properties is already being described by the general protocol
> node described above.
> > > > > > >
> > > > > > > Without this patch, there is dtbs_check warning.
> > > > > > >
> > > > > > > scmi: 'protocol@12' does not match any of the regexes: 'pinctrl-[0-
> 9]+'
> > > > > > > from schema $id:
> > > > > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%
> > > > > > >
> 2F%2Fd%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7Caf8c2c02cfc644
> > > > > > >
> 981a3a08dc39dce87d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> > > > > > >
> 0%7C638448868151253291%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> wL
> > > > > > >
> jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C
> > > > > > > %7C%7C&sdata=hRNs%2BMA%2BPR3rdUSEMg%2FzqO4nN1YP4EX
> DSikXxZMhJ
> > > > > > > HQ%3D&reserved=0
> > > > > > >
> > > >
> evic%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C3ef5d152a8e5463e9b
> > > > 3408
> > > > > > >
> > > >
> dc39c47e41%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63844
> > > > 87632
> > > > > > >
> > > >
> 86203182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
> > > > V2luM
> > > > > > >
> > > >
> zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=kRBV9FPk6
> > > > Nrgu
> > > > > > > Hs2SHpL74VlNUH6cYO1kAsW74CkaTk%3D&reserved=0
> > > > > > >
> > > > > >
> > > >
> etree.org%2Fschemas%2Ffirmware%2Farm%2Cscmi.yaml%23&data=05%7C0
> > > > > > 2%7Cpen
> > > > > > >
> > > > > >
> > > >
> g.fan%40nxp.com%7Ccac77deb5f6a4b20460a08dc392ead40%7C686ea1d3b
> > > > > > c2b4c6fa
> > > > > > >
> > > > > >
> > > >
> 92cd99c5c301635%7C0%7C0%7C638448119832543335%7CUnknown%7CT
> > > > > > WFpbGZsb3d8e
> > > > > > >
> > > > > >
> > > >
> yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> > > > > > %7C0%
> > > > > > >
> > > > > >
> > > >
> 7C%7C%7C&sdata=6MldIOUQ4hxn%2BRffwJJJ3jxXXtHCSxLUOa4JMWB0htU%
> > > > > > 3D&reserv
> > > > > > > ed=0
> > > > > > >
> > > > > >
> > > > > > Why are you adding protocol@12 to the device tree ? Does it
> > > > > > have a dedicated channel ? If not, you shouldn't need to add it.
> > > > >
> > > > > No dedicated channel.
> > > > > The idea is we have multile Agent, the M7 agent may ask to
> > > > > shutdown Linux Agent. So the linux agent need use protocol@12 to do
> the action.
> > > > >
> > > >
> > > > Ok, so indeed you need, AFAICR, the node in the DT even without a
> > > > dedicated channel nor anynone referring it: no DT node means no
> protocol initialization.
> > >
> > > Yeah, so this binding update is needed, right?
> > >
> > > >
> > > > > For now, we have not finish implementing this in linux side,
> > > > > just add the node in dts.
> > > >
> > > > On the linux side you should not need to do anything but adding
> > > > the node @12, the existent
> > > > drivers/firmware/arm_scmi/system_power_control.c
> > > > driver when loaded will shutdown the system upon receiving the
> > > > SytemPower notification....what lacks Linux side in these regards ?
> > >
> > > " Ignoring unsupported system_state:" we have new defined number
> > > here.
> > >
> >
> > So I suppose you are using the Vendor defined system states and
> > writing your own driver to handle these instead of system_power_control ?
> 
> Or some vendor extensions to the std driver ?
> IOW what is the plan...custom vendor states was left out from
> system_power_control waiting for a real usecase...that seems to have shown
> up eventually :D

I have not implemented the linux side driver as of now, but we do need
Extension:

Ignoring unsupported system_state: 0x80000003, we use this state indicating
others are requesting linux to suspend.

And when system is doing emergency reset, linux will get a notification, and got
force reset, we are thinking to let linux do some disk flush or whatelse, the
Time window is small.

Regards,
Peng.

> 
> Thanks,
> Cristian

Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power protocol

[Cristian Marussi]

On Mon, Mar 04, 2024 at 02:18:10AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support system power
> > protocol
> > 
> > On Fri, Mar 01, 2024 at 10:37:49AM +0000, Cristian Marussi wrote:
> > > On Fri, Mar 01, 2024 at 09:22:24AM +0000, Peng Fan wrote:
> > > > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support
> > > > > system power protocol
> > > > >
> > > > > On Fri, Mar 01, 2024 at 12:41:29AM +0000, Peng Fan wrote:
> > > > > > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi: support
> > > > > > > system power protocol
> > > > > > >
> > > > > > > On Tue, Feb 27, 2024 at 01:01:41AM +0000, Peng Fan wrote:
> > > > > > > > > Subject: Re: [PATCH] dt-bindings: firmware: arm,scmi:
> > > > > > > > > support system power protocol
> > > > > > > > >
> > > > > > > > > On Mon, Feb 26, 2024 at 01:28:31PM +0000, Cristian Marussi
> > wrote:
> > > > > > > > > > On Mon, Feb 26, 2024 at 09:02:43PM +0800, Peng Fan (OSS)
> > wrote:
> > > > > > > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > > > > > >
> > > > > > > > > > > Add SCMI System Power Protocol bindings, and the
> > > > > > > > > > > protocol id is
> > > > > 0x12.
> > > > > > > > > > >
> > > > > > > > > > Hi,
> > > > > > > > > >
> > > > > > > > > > yes this is something I spotted in the past it was
> > > > > > > > > > missing and I posted a similar patch but I was told that
> > > > > > > > > > a protocol node without any specific additional
> > > > > > > > > > properties is already being described by the general protocol
> > node described above.
> > > > > > > >
> > > > > > > > Without this patch, there is dtbs_check warning.
> > > > > > > >
> > > > > > > > scmi: 'protocol@12' does not match any of the regexes: 'pinctrl-[0-
> > 9]+'
> > > > > > > > from schema $id:
> > > > > > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%
> > > > > > > >
> > 2F%2Fd%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7Caf8c2c02cfc644
> > > > > > > >
> > 981a3a08dc39dce87d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> > > > > > > >
> > 0%7C638448868151253291%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> > wL
> > > > > > > >
> > jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C
> > > > > > > > %7C%7C&sdata=hRNs%2BMA%2BPR3rdUSEMg%2FzqO4nN1YP4EX
> > DSikXxZMhJ
> > > > > > > > HQ%3D&reserved=0
> > > > > > > >
> > > > >
> > evic%2F&data=05%7C02%7Cpeng.fan%40nxp.com%7C3ef5d152a8e5463e9b
> > > > > 3408
> > > > > > > >
> > > > >
> > dc39c47e41%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63844
> > > > > 87632
> > > > > > > >
> > > > >
> > 86203182%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
> > > > > V2luM
> > > > > > > >
> > > > >
> > zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=kRBV9FPk6
> > > > > Nrgu
> > > > > > > > Hs2SHpL74VlNUH6cYO1kAsW74CkaTk%3D&reserved=0
> > > > > > > >
> > > > > > >
> > > > >
> > etree.org%2Fschemas%2Ffirmware%2Farm%2Cscmi.yaml%23&data=05%7C0
> > > > > > > 2%7Cpen
> > > > > > > >
> > > > > > >
> > > > >
> > g.fan%40nxp.com%7Ccac77deb5f6a4b20460a08dc392ead40%7C686ea1d3b
> > > > > > > c2b4c6fa
> > > > > > > >
> > > > > > >
> > > > >
> > 92cd99c5c301635%7C0%7C0%7C638448119832543335%7CUnknown%7CT
> > > > > > > WFpbGZsb3d8e
> > > > > > > >
> > > > > > >
> > > > >
> > yJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> > > > > > > %7C0%
> > > > > > > >
> > > > > > >
> > > > >
> > 7C%7C%7C&sdata=6MldIOUQ4hxn%2BRffwJJJ3jxXXtHCSxLUOa4JMWB0htU%
> > > > > > > 3D&reserv
> > > > > > > > ed=0
> > > > > > > >
> > > > > > >
> > > > > > > Why are you adding protocol@12 to the device tree ? Does it
> > > > > > > have a dedicated channel ? If not, you shouldn't need to add it.
> > > > > >
> > > > > > No dedicated channel.
> > > > > > The idea is we have multile Agent, the M7 agent may ask to
> > > > > > shutdown Linux Agent. So the linux agent need use protocol@12 to do
> > the action.
> > > > > >
> > > > >
> > > > > Ok, so indeed you need, AFAICR, the node in the DT even without a
> > > > > dedicated channel nor anynone referring it: no DT node means no
> > protocol initialization.
> > > >
> > > > Yeah, so this binding update is needed, right?
> > > >
> > > > >
> > > > > > For now, we have not finish implementing this in linux side,
> > > > > > just add the node in dts.
> > > > >
> > > > > On the linux side you should not need to do anything but adding
> > > > > the node @12, the existent
> > > > > drivers/firmware/arm_scmi/system_power_control.c
> > > > > driver when loaded will shutdown the system upon receiving the
> > > > > SytemPower notification....what lacks Linux side in these regards ?
> > > >
> > > > " Ignoring unsupported system_state:" we have new defined number
> > > > here.
> > > >
> > >
> > > So I suppose you are using the Vendor defined system states and
> > > writing your own driver to handle these instead of system_power_control ?
> > 
> > Or some vendor extensions to the std driver ?
> > IOW what is the plan...custom vendor states was left out from
> > system_power_control waiting for a real usecase...that seems to have shown
> > up eventually :D
> 
> I have not implemented the linux side driver as of now, but we do need
> Extension:
> 
> Ignoring unsupported system_state: 0x80000003, we use this state indicating
> others are requesting linux to suspend.
> 
> And when system is doing emergency reset, linux will get a notification, and got
> force reset, we are thinking to let linux do some disk flush or whatelse, the
> Time window is small.

mmm ... now I am puzzled....why do you need extra custom state since
this behaviour just sounds like the normal SYSTEM_POWER notification
graceful shutdown request (that can carry a platform timeout)...

...some other agents requested a shutdown so Linuux get a notification
SYSTEM_POWER(shutdown) with possibly timeout advised by the platform to
complete the shutdown...then scmi_power_control existing upstream driver
issues a orderly_poweroff (which carries out an emergency_sync) and then
finally userspace can do whatever it wants hooking its own shutdown-callbacks
into whatever pid_1 process it has...if all of this exceeds the advised
timeout for a graceful shutdown Linux will just force-shutdown without
waiting futher for userspace shutdown-callbacks to complete...

so wqhat do you need more than this to justify a new custom vendor
system_power state AND a dedicated driver other than scmi_power_control
IF you really have just to shutdown based on some SYSTEM_POWER
notification sent by the platform when some  other agents decided to
shutdown ?

Thanks,
Cristian