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