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

[Peng Fan]

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

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