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

[Rob Herring]

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@xxxxxxx>
> > >
> > > 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@xxxxxxx>
> > > ---
> > >  .../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