diff mbox series

[v3,2/6] dt-bindings: firmware: add i.MX95 SCMI Extension protocol

Message ID 20240412-imx95-bbm-misc-v2-v3-2-4380a4070980@nxp.com (mailing list archive)
State New, archived
Headers show
Series firmware: support i.MX95 SCMI BBM/MISC Extenstion | expand

Commit Message

Peng Fan (OSS) April 12, 2024, 10:47 a.m. UTC 
From: Peng Fan <peng.fan@nxp.com>

Add i.MX SCMI Extension protocols bindings for:
 - Battery Backed Module(BBM) Protocol
   This contains persistent storage (GPR), an RTC, and the ON/OFF button.
   The protocol can also provide access to similar functions implemented via
   external board components.
 - MISC Protocol.
   This includes controls that are misc settings/actions that must be exposed
   from the SM to agents. They are device specific and are usually define to
   access bit fields in various mix block control modules, IOMUX_GPR, and other
   GPR/CSR owned by the SM.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../devicetree/bindings/firmware/arm,scmi.yaml     | 21 +++++++++++++
 .../bindings/firmware/nxp,imx95-scmi.yaml          | 36 ++++++++++++++++++++++
 2 files changed, 57 insertions(+)

Comments

Rob Herring April 12, 2024, 1:34 p.m. UTC | #1 
On Fri, Apr 12, 2024 at 06:47:08PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add i.MX SCMI Extension protocols bindings for:
>  - Battery Backed Module(BBM) Protocol
>    This contains persistent storage (GPR), an RTC, and the ON/OFF button.
>    The protocol can also provide access to similar functions implemented via
>    external board components.
>  - MISC Protocol.
>    This includes controls that are misc settings/actions that must be exposed
>    from the SM to agents. They are device specific and are usually define to
>    access bit fields in various mix block control modules, IOMUX_GPR, and other
>    GPR/CSR owned by the SM.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  .../devicetree/bindings/firmware/arm,scmi.yaml     | 21 +++++++++++++
>  .../bindings/firmware/nxp,imx95-scmi.yaml          | 36 ++++++++++++++++++++++
>  2 files changed, 57 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index 93fb7d05f849..fa2cc910c485 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -247,6 +247,27 @@ properties:
>        reg:
>          const: 0x18
>  
> +  protocol@81:
> +    $ref: '#/$defs/protocol-node'
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        const: 0x81
> +
> +  protocol@84:
> +    type: object
> +    anyOf:
> +      - allOf:
> +          - $ref: /schemas/firmware/nxp,imx95-scmi.yaml
> +          - $ref: '#/$defs/protocol-node'

If you put the ref under the protocol node, then it's 1 schema file per 
protocol per vendor. Also, we then have to list every possible protocol 
node here, and every one listed here will be valid for every vendor.  
What we discussed is putting the list of vendor protocol schemas at the 
top-level here and then the vendor schemas can list out all the protocol 
nodes.

Also, move "$ref: '#/$defs/protocol-node'" to nxp,imx95-scmi.yaml.

> +
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        const: 0x84
> +
>  additionalProperties: false
>  
>  $defs:
> diff --git a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml
> new file mode 100644
> index 000000000000..b84c4a53b78a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml
> @@ -0,0 +1,36 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2024 NXP
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/nxp,imx95-scmi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: i.MX95 System Control and Management Interface(SCMI) Vendor Protocols Extension
> +
> +maintainers:
> +  - Peng Fan <peng.fan@nxp.com>
> +
> +properties:
> +  nxp,wakeup-sources:
> +    description:
> +      Each entry consists of 2 integers, represents the source and electric signal edge
> +    items:
> +      items:
> +        - description: the wakeup source
> +        - description: the wakeup electric signal edge
> +    minItems: 1
> +    maxItems: 32
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +
> +if:
> +  properties:
> +    reg:
> +      const: 0x84

This schema is only included from protocol@84 node, so how can this be 
false?

> +then:
> +  properties:
> +    nxp,wakeup-sources: true
> +else:
> +  properties:
> +    nxp,wakeup-sources: false
> +
> +additionalProperties: true
> 
> -- 
> 2.37.1
>
Peng Fan April 12, 2024, 1:50 p.m. UTC | #2 
Hi Rob,

> Subject: Re: [PATCH v3 2/6] dt-bindings: firmware: add i.MX95 SCMI
> Extension protocol
> 
> On Fri, Apr 12, 2024 at 06:47:08PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add i.MX SCMI Extension protocols bindings for:
> >  - Battery Backed Module(BBM) Protocol
> >    This contains persistent storage (GPR), an RTC, and the ON/OFF button.
> >    The protocol can also provide access to similar functions implemented via
> >    external board components.
> >  - MISC Protocol.
> >    This includes controls that are misc settings/actions that must be exposed
> >    from the SM to agents. They are device specific and are usually define to
> >    access bit fields in various mix block control modules, IOMUX_GPR, and
> other
> >    GPR/CSR owned by the SM.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  .../devicetree/bindings/firmware/arm,scmi.yaml     | 21 +++++++++++++
> >  .../bindings/firmware/nxp,imx95-scmi.yaml          | 36
> ++++++++++++++++++++++
> >  2 files changed, 57 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > index 93fb7d05f849..fa2cc910c485 100644
> > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > @@ -247,6 +247,27 @@ properties:
> >        reg:
> >          const: 0x18
> >
> > +  protocol@81:
> > +    $ref: '#/$defs/protocol-node'
> > +    unevaluatedProperties: false
> > +
> > +    properties:
> > +      reg:
> > +        const: 0x81
> > +
> > +  protocol@84:
> > +    type: object
> > +    anyOf:
> > +      - allOf:
> > +          - $ref: /schemas/firmware/nxp,imx95-scmi.yaml
> > +          - $ref: '#/$defs/protocol-node'
> 
> If you put the ref under the protocol node, then it's 1 schema file per protocol
> per vendor. Also, we then have to list every possible protocol node here, and
> every one listed here will be valid for every vendor.
> What we discussed is putting the list of vendor protocol schemas at the top-
> level here and then the vendor schemas can list out all the protocol nodes.
> 
> Also, move "$ref: '#/$defs/protocol-node'" to nxp,imx95-scmi.yaml.

In arm,scmi.yaml top level, add below:
+anyOf:
+  - $ref: /schemas/firmware/nxp,imx95-scmi.yaml

And also add a protocol node:
  protocol@84:                                                                                      
    $ref: '#/$defs/protocol-node'                                                                   
                                                                                                    
    properties:                                                                                     
      reg:                                                                                          
        const: 0x84
But here I not add unevaludatedProperties = false; otherwise the vendor
yaml new properties will not work.

In nxp,imx95-scmi.yaml:
properties:                                                                                         
  protocol@84:                                                                                      
    $ref: '/schemas/firmware/arm,scmi.yaml#/$defs/protocol-node'                                    
    unevaluatedProperties: false                                                                    
                                                                                                    
    properties:                                                                                     
      reg:                                                                                          
        const: 0x84                                                                                 
                                                                                                    
      nxp,wakeup-sources:                                                                           
        description:                                                                                
          Each entry consists of 2 integers, represents the source and electric signal edge         
        items:                                                                                      
          items:                                                                                    
            - description: the wakeup source                                                        
            - description: the wakeup electric signal edge                                          
        minItems: 1                                                                                 
        maxItems: 32                                                                                
        $ref: /schemas/types.yaml#/definitions/uint32-matrix                                        
                                                                                                    
additionalProperties: true

Are the upper looks good to you?

> 
> > +
> > +    unevaluatedProperties: false
> > +
> > +    properties:
> > +      reg:
> > +        const: 0x84
> > +
> >  additionalProperties: false
> >
> >  $defs:
> > diff --git
> > a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml
> > b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml
> > new file mode 100644
> > index 000000000000..b84c4a53b78a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml
> > @@ -0,0 +1,36 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) # Copyright 2024
> > +NXP %YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fschemas%2Ffirmware%2Fnxp%2Cimx95-
> scmi.yaml%23&data=05%7C
> >
> +02%7Cpeng.fan%40nxp.com%7Ca73ba3e8c48044f31aed08dc5af538e8%7C
> 686ea1d3
> >
> +bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638485256510784219%7CUnk
> nown%7CTWF
> >
> +pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> VCI6
> >
> +Mn0%3D%7C0%7C%7C%7C&sdata=Ml5yAdb4A0Bmg%2BVKroJTJsE9dAF6B
> h2mbJzgyxB4w
> > +xs%3D&reserved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fmeta-
> schemas%2Fcore.yaml%23&data=05%7C02%7Cpeng.fan%40nx
> >
> +p.com%7Ca73ba3e8c48044f31aed08dc5af538e8%7C686ea1d3bc2b4c6fa9
> 2cd99c5c
> >
> +301635%7C0%7C0%7C638485256510797603%7CUnknown%7CTWFpbGZs
> b3d8eyJWIjoiM
> >
> +C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7
> C%7C%7
> >
> +C&sdata=6%2FsoexEJRrjL3DaXX5wwtjd1ZSN7nqmh0YjAQEmH5Ow%3D&res
> erved=0
> > +
> > +title: i.MX95 System Control and Management Interface(SCMI) Vendor
> > +Protocols Extension
> > +
> > +maintainers:
> > +  - Peng Fan <peng.fan@nxp.com>
> > +
> > +properties:
> > +  nxp,wakeup-sources:
> > +    description:
> > +      Each entry consists of 2 integers, represents the source and electric
> signal edge
> > +    items:
> > +      items:
> > +        - description: the wakeup source
> > +        - description: the wakeup electric signal edge
> > +    minItems: 1
> > +    maxItems: 32
> > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +
> > +if:
> > +  properties:
> > +    reg:
> > +      const: 0x84
> 
> This schema is only included from protocol@84 node, so how can this be
> false?

Just in case adding other ID in this file in future. This is no need with
per file containing all the nxp protocols.

Thanks,
Peng.
> 
> > +then:
> > +  properties:
> > +    nxp,wakeup-sources: true
> > +else:
> > +  properties:
> > +    nxp,wakeup-sources: false
> > +
> > +additionalProperties: true
> >
> > --
> > 2.37.1
> >
Rob Herring April 16, 2024, 1:07 p.m. UTC | #3 
On Fri, Apr 12, 2024 at 01:50:37PM +0000, Peng Fan wrote:
> Hi Rob,
> 
> > Subject: Re: [PATCH v3 2/6] dt-bindings: firmware: add i.MX95 SCMI
> > Extension protocol
> > 
> > On Fri, Apr 12, 2024 at 06:47:08PM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > Add i.MX SCMI Extension protocols bindings for:
> > >  - Battery Backed Module(BBM) Protocol
> > >    This contains persistent storage (GPR), an RTC, and the ON/OFF button.
> > >    The protocol can also provide access to similar functions implemented via
> > >    external board components.
> > >  - MISC Protocol.
> > >    This includes controls that are misc settings/actions that must be exposed
> > >    from the SM to agents. They are device specific and are usually define to
> > >    access bit fields in various mix block control modules, IOMUX_GPR, and
> > other
> > >    GPR/CSR owned by the SM.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  .../devicetree/bindings/firmware/arm,scmi.yaml     | 21 +++++++++++++
> > >  .../bindings/firmware/nxp,imx95-scmi.yaml          | 36
> > ++++++++++++++++++++++
> > >  2 files changed, 57 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > index 93fb7d05f849..fa2cc910c485 100644
> > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > @@ -247,6 +247,27 @@ properties:
> > >        reg:
> > >          const: 0x18
> > >
> > > +  protocol@81:
> > > +    $ref: '#/$defs/protocol-node'
> > > +    unevaluatedProperties: false
> > > +
> > > +    properties:
> > > +      reg:
> > > +        const: 0x81
> > > +
> > > +  protocol@84:
> > > +    type: object
> > > +    anyOf:
> > > +      - allOf:
> > > +          - $ref: /schemas/firmware/nxp,imx95-scmi.yaml
> > > +          - $ref: '#/$defs/protocol-node'
> > 
> > If you put the ref under the protocol node, then it's 1 schema file per protocol
> > per vendor. Also, we then have to list every possible protocol node here, and
> > every one listed here will be valid for every vendor.
> > What we discussed is putting the list of vendor protocol schemas at the top-
> > level here and then the vendor schemas can list out all the protocol nodes.
> > 
> > Also, move "$ref: '#/$defs/protocol-node'" to nxp,imx95-scmi.yaml.
> 
> In arm,scmi.yaml top level, add below:
> +anyOf:
> +  - $ref: /schemas/firmware/nxp,imx95-scmi.yaml
> 
> And also add a protocol node:
>   protocol@84:                                                                                      
>     $ref: '#/$defs/protocol-node'                                                                   
>                                                                                                     
>     properties:                                                                                     
>       reg:                                                                                          
>         const: 0x84
> But here I not add unevaludatedProperties = false; otherwise the vendor
> yaml new properties will not work.

Just drop 'protocol@84' entirely here and change the top-level 
additionalProperties to unevaluatedProperties.

> 
> In nxp,imx95-scmi.yaml:
> properties:                                                                                         
>   protocol@84:                                                                                      
>     $ref: '/schemas/firmware/arm,scmi.yaml#/$defs/protocol-node'                                    
>     unevaluatedProperties: false                                                                    
>                                                                                                     
>     properties:                                                                                     
>       reg:                                                                                          
>         const: 0x84                                                                                 
>                                                                                                     
>       nxp,wakeup-sources:                                                                           
>         description:                                                                                
>           Each entry consists of 2 integers, represents the source and electric signal edge         
>         items:                                                                                      
>           items:                                                                                    
>             - description: the wakeup source                                                        
>             - description: the wakeup electric signal edge                                          

Constraints on the items values?

>         minItems: 1                                                                                 
>         maxItems: 32                                                                                
>         $ref: /schemas/types.yaml#/definitions/uint32-matrix                                        
>                                                                                                     
> additionalProperties: true
> 
> Are the upper looks good to you?

Yes, other than the one comment.

Rob
Peng Fan April 16, 2024, 1:19 p.m. UTC | #4 
> Subject: Re: [PATCH v3 2/6] dt-bindings: firmware: add i.MX95 SCMI
> Extension protocol
> 
> On Fri, Apr 12, 2024 at 01:50:37PM +0000, Peng Fan wrote:
> > Hi Rob,
> >
> > > Subject: Re: [PATCH v3 2/6] dt-bindings: firmware: add i.MX95 SCMI
> > > Extension protocol
> > >
> > > On Fri, Apr 12, 2024 at 06:47:08PM +0800, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > Add i.MX SCMI Extension protocols bindings for:
> > > >  - Battery Backed Module(BBM) Protocol
> > > >    This contains persistent storage (GPR), an RTC, and the ON/OFF button.
> > > >    The protocol can also provide access to similar functions implemented
> via
> > > >    external board components.
> > > >  - MISC Protocol.
> > > >    This includes controls that are misc settings/actions that must be
> exposed
> > > >    from the SM to agents. They are device specific and are usually define
> to
> > > >    access bit fields in various mix block control modules,
> > > > IOMUX_GPR, and
> > > other
> > > >    GPR/CSR owned by the SM.
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > >  .../devicetree/bindings/firmware/arm,scmi.yaml     | 21 +++++++++++++
> > > >  .../bindings/firmware/nxp,imx95-scmi.yaml          | 36
> > > ++++++++++++++++++++++
> > > >  2 files changed, 57 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > index 93fb7d05f849..fa2cc910c485 100644
> > > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > @@ -247,6 +247,27 @@ properties:
> > > >        reg:
> > > >          const: 0x18
> > > >
> > > > +  protocol@81:
> > > > +    $ref: '#/$defs/protocol-node'
> > > > +    unevaluatedProperties: false
> > > > +
> > > > +    properties:
> > > > +      reg:
> > > > +        const: 0x81
> > > > +
> > > > +  protocol@84:
> > > > +    type: object
> > > > +    anyOf:
> > > > +      - allOf:
> > > > +          - $ref: /schemas/firmware/nxp,imx95-scmi.yaml
> > > > +          - $ref: '#/$defs/protocol-node'
> > >
> > > If you put the ref under the protocol node, then it's 1 schema file
> > > per protocol per vendor. Also, we then have to list every possible
> > > protocol node here, and every one listed here will be valid for every
> vendor.
> > > What we discussed is putting the list of vendor protocol schemas at
> > > the top- level here and then the vendor schemas can list out all the
> protocol nodes.
> > >
> > > Also, move "$ref: '#/$defs/protocol-node'" to nxp,imx95-scmi.yaml.
> >
> > In arm,scmi.yaml top level, add below:
> > +anyOf:
> > +  - $ref: /schemas/firmware/nxp,imx95-scmi.yaml
> >
> > And also add a protocol node:
> >   protocol@84:
> >     $ref: '#/$defs/protocol-node'
> >
> >     properties:
> >       reg:
> >         const: 0x84
> > But here I not add unevaludatedProperties = false; otherwise the
> > vendor yaml new properties will not work.
> 
> Just drop 'protocol@84' entirely here and change the top-level
> additionalProperties to unevaluatedProperties.

Thanks, will fix in v4.
> 
> >
> > In nxp,imx95-scmi.yaml:
> > properties:
> >   protocol@84:
> >     $ref: '/schemas/firmware/arm,scmi.yaml#/$defs/protocol-node'
> >     unevaluatedProperties: false
> >
> >     properties:
> >       reg:
> >         const: 0x84
> >
> >       nxp,wakeup-sources:
> >         description:
> >           Each entry consists of 2 integers, represents the source and electric
> signal edge
> >         items:
> >           items:
> >             - description: the wakeup source
> >             - description: the wakeup electric signal edge
> 
> Constraints on the items values?

will add in v4.

> 
> >         minItems: 1
> >         maxItems: 32
> >         $ref: /schemas/types.yaml#/definitions/uint32-matrix
> >
> > additionalProperties: true
> >
> > Are the upper looks good to you?
> 
> Yes, other than the one comment.

Thanks for help reviewing the patch.

Thanks,
Peng.
> 
> Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 93fb7d05f849..fa2cc910c485 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -247,6 +247,27 @@  properties:
       reg:
         const: 0x18
 
+  protocol@81:
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
+    properties:
+      reg:
+        const: 0x81
+
+  protocol@84:
+    type: object
+    anyOf:
+      - allOf:
+          - $ref: /schemas/firmware/nxp,imx95-scmi.yaml
+          - $ref: '#/$defs/protocol-node'
+
+    unevaluatedProperties: false
+
+    properties:
+      reg:
+        const: 0x84
+
 additionalProperties: false
 
 $defs:
diff --git a/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml
new file mode 100644
index 000000000000..b84c4a53b78a
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/nxp,imx95-scmi.yaml
@@ -0,0 +1,36 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2024 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/nxp,imx95-scmi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX95 System Control and Management Interface(SCMI) Vendor Protocols Extension
+
+maintainers:
+  - Peng Fan <peng.fan@nxp.com>
+
+properties:
+  nxp,wakeup-sources:
+    description:
+      Each entry consists of 2 integers, represents the source and electric signal edge
+    items:
+      items:
+        - description: the wakeup source
+        - description: the wakeup electric signal edge
+    minItems: 1
+    maxItems: 32
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+
+if:
+  properties:
+    reg:
+      const: 0x84
+then:
+  properties:
+    nxp,wakeup-sources: true
+else:
+  properties:
+    nxp,wakeup-sources: false
+
+additionalProperties: true