diff mbox series

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

Message ID 20240226130243.3820915-1-peng.fan@oss.nxp.com (mailing list archive)
State New, archived
Headers show
Series dt-bindings: firmware: arm,scmi: support system power protocol | expand

Commit Message

Peng Fan (OSS) Feb. 26, 2024, 1:02 p.m. UTC
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(+)

Comments

Cristian Marussi Feb. 26, 2024, 1:28 p.m. UTC | #1
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
>
Cristian Marussi Feb. 26, 2024, 3:02 p.m. UTC | #2
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
Peng Fan Feb. 27, 2024, 1:01 a.m. UTC | #3
> 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
Sudeep Holla Feb. 29, 2024, 1:59 p.m. UTC | #4
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.
Peng Fan March 1, 2024, 12:41 a.m. UTC | #5
> 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
Cristian Marussi March 1, 2024, 7:52 a.m. UTC | #6
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
Peng Fan March 1, 2024, 9:22 a.m. UTC | #7
> 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
Sudeep Holla March 1, 2024, 9:31 a.m. UTC | #8
On Fri, Mar 01, 2024 at 09:22:24AM +0000, Peng Fan wrote:
> " Ignoring unsupported system_state:" we have new defined
> number here.

Why ?
Cristian Marussi March 1, 2024, 10:37 a.m. UTC | #9
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
Cristian Marussi March 1, 2024, 10:46 a.m. UTC | #10
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
Peng Fan March 4, 2024, 2:18 a.m. UTC | #11
> 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
Cristian Marussi March 4, 2024, 8:29 a.m. UTC | #12
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
diff mbox series

Patch

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