[PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[Rob Herring]

The SCMI protocol child nodes are missing any constraints on unknown
properties. Specifically, either 'unevaluatedProperties' or
'additionalProperties' is needed. The current structure with a regex
match for all child nodes doesn't work for this purpose, so let's move
the common properties '$defs' entry which each specific protocol node
can reference and set 'unevaluatedProperties: false'.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../bindings/firmware/arm,scmi.yaml           | 43 ++++++++++++++-----
 1 file changed, 33 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 176796931a22..2f7c51c75e85 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -100,7 +100,9 @@ properties:
       Channel specifier required when using OP-TEE transport.
 
   protocol@11:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:
         const: 0x11
@@ -112,7 +114,9 @@ properties:
       - '#power-domain-cells'
 
   protocol@13:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:
         const: 0x13
@@ -124,7 +128,9 @@ properties:
       - '#clock-cells'
 
   protocol@14:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:
         const: 0x14
@@ -136,7 +142,9 @@ properties:
       - '#clock-cells'
 
   protocol@15:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:
         const: 0x15
@@ -148,7 +156,9 @@ properties:
       - '#thermal-sensor-cells'
 
   protocol@16:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:
         const: 0x16
@@ -160,20 +170,31 @@ properties:
       - '#reset-cells'
 
   protocol@17:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:
         const: 0x17
 
       regulators:
         type: object
+        additionalProperties: false
         description:
           The list of all regulators provided by this SCMI controller.
 
+        properties:
+          '#address-cells':
+            const: 1
+
+          '#size-cells':
+            const: 0
+
         patternProperties:
-          '^regulators@[0-9a-f]+$':
+          '^regulator@[0-9a-f]+$':
             type: object
             $ref: "../regulator/regulator.yaml#"
+            unevaluatedProperties: false
 
             properties:
               reg:
@@ -184,15 +205,17 @@ properties:
               - reg
 
   protocol@18:
-    type: object
+    $ref: '#/$defs/protocol-node'
+    unevaluatedProperties: false
+
     properties:
       reg:
         const: 0x18
 
 additionalProperties: false
 
-patternProperties:
-  '^protocol@[0-9a-f]+$':
+$defs:
+  protocol-node:
     type: object
     description:
       Each sub-node represents a protocol supported. If the platform
-- 
2.39.0

Re: [PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[Sudeep Holla]

On Tue, Jan 24, 2023 at 04:20:23PM -0600, Rob Herring wrote:
> The SCMI protocol child nodes are missing any constraints on unknown
> properties. Specifically, either 'unevaluatedProperties' or
> 'additionalProperties' is needed. The current structure with a regex
> match for all child nodes doesn't work for this purpose, so let's move
> the common properties '$defs' entry which each specific protocol node
> can reference and set 'unevaluatedProperties: false'.
>

Makes sense to me. Also thanks for $defs example, wasn't aware of how
to do that.

Can you please take it though your tree ? Assuming that,

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

--
Regards,
Sudeep

Re: [PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[Cristian Marussi]

On Tue, Jan 24, 2023 at 04:20:23PM -0600, Rob Herring wrote:
> The SCMI protocol child nodes are missing any constraints on unknown
> properties. Specifically, either 'unevaluatedProperties' or
> 'additionalProperties' is needed. The current structure with a regex
> match for all child nodes doesn't work for this purpose, so let's move
> the common properties '$defs' entry which each specific protocol node
> can reference and set 'unevaluatedProperties: false'.
> 

Hi Rob,

> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/firmware/arm,scmi.yaml           | 43 ++++++++++++++-----
>  1 file changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index 176796931a22..2f7c51c75e85 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -100,7 +100,9 @@ properties:
>        Channel specifier required when using OP-TEE transport.
>  
>    protocol@11:
> -    type: object
> +    $ref: '#/$defs/protocol-node'
> +    unevaluatedProperties: false
> +
>      properties:
>        reg:
>          const: 0x11
> @@ -112,7 +114,9 @@ properties:
>        - '#power-domain-cells'
>  
>    protocol@13:
> -    type: object
> +    $ref: '#/$defs/protocol-node'
> +    unevaluatedProperties: false
> +
>      properties:
>        reg:
>          const: 0x13
> @@ -124,7 +128,9 @@ properties:
>        - '#clock-cells'
>  
>    protocol@14:
> -    type: object
> +    $ref: '#/$defs/protocol-node'
> +    unevaluatedProperties: false
> +
>      properties:
>        reg:
>          const: 0x14
> @@ -136,7 +142,9 @@ properties:
>        - '#clock-cells'
>  
>    protocol@15:
> -    type: object
> +    $ref: '#/$defs/protocol-node'
> +    unevaluatedProperties: false
> +
>      properties:
>        reg:
>          const: 0x15
> @@ -148,7 +156,9 @@ properties:
>        - '#thermal-sensor-cells'
>  
>    protocol@16:
> -    type: object
> +    $ref: '#/$defs/protocol-node'
> +    unevaluatedProperties: false
> +
>      properties:
>        reg:
>          const: 0x16
> @@ -160,20 +170,31 @@ properties:
>        - '#reset-cells'
>  
>    protocol@17:
> -    type: object
> +    $ref: '#/$defs/protocol-node'
> +    unevaluatedProperties: false
> +
>      properties:
>        reg:
>          const: 0x17
>  
>        regulators:
>          type: object
> +        additionalProperties: false
>          description:
>            The list of all regulators provided by this SCMI controller.
>  
> +        properties:
> +          '#address-cells':
> +            const: 1
> +
> +          '#size-cells':
> +            const: 0
> +
>          patternProperties:
> -          '^regulators@[0-9a-f]+$':
> +          '^regulator@[0-9a-f]+$':
>              type: object
>              $ref: "../regulator/regulator.yaml#"
> +            unevaluatedProperties: false
>  
>              properties:
>                reg:
> @@ -184,15 +205,17 @@ properties:
>                - reg
>  
>    protocol@18:
> -    type: object
> +    $ref: '#/$defs/protocol-node'
> +    unevaluatedProperties: false
> +
>      properties:
>        reg:
>          const: 0x18
>  
>  additionalProperties: false
>  
> -patternProperties:
> -  '^protocol@[0-9a-f]+$':
> +$defs:
> +  protocol-node:
>      type: object
>      description:

so now that the catch-all protocol@ patternProperty is gone in favour
of the 'protocol-node' definition and $refs, does that mean that any
current and future SCMI officially published protocol <N> has to be
added to the above explicit protocol list, even though it does not
have any special additional required property beside reg ?
(like protocol@18 above...)

As an example SystemPower protocol@12 is not listed above too and it
has nothing more than a reg=0x12 prop (liek 0x18), but before this patch
was 'covered' by the patternProperty (so Krzysztof shot down, rightly,
my recent attempt to add a distinct protocol@12 def), but now it does not
seem anymore the case...so will we need to add an explicit protocol node
for any future protocol addition ? (SCMI is extensible up to 255
protos..)

Thanks,
Cristian

Re: [PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[Sudeep Holla]

On Wed, Jan 25, 2023 at 01:43:48PM +0000, Cristian Marussi wrote:
> so now that the catch-all protocol@ patternProperty is gone in favour
> of the 'protocol-node' definition and $refs, does that mean that any
> current and future SCMI officially published protocol <N> has to be
> added to the above explicit protocol list, even though it does not
> have any special additional required property beside reg ?
> (like protocol@18 above...)
>

If there are no consumers, should we just not add and deal with it
entirely within the kernel. I know we rely today on presence of node
before we initialise, but hey we have exception for system power protocol
for other reasons, why not add this one too.

In short we shouldn't have to add a node if there are no consumers. It
was one of the topic of discussion initially when SCMI binding was added
and they exist only for the consumers otherwise we don't need it as
everything is discoverable from the interface.

--
Regards,
Sudeep

Re: [PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[Cristian Marussi]

On Wed, Jan 25, 2023 at 02:11:13PM +0000, Sudeep Holla wrote:
> On Wed, Jan 25, 2023 at 01:43:48PM +0000, Cristian Marussi wrote:
> > so now that the catch-all protocol@ patternProperty is gone in favour
> > of the 'protocol-node' definition and $refs, does that mean that any
> > current and future SCMI officially published protocol <N> has to be
> > added to the above explicit protocol list, even though it does not
> > have any special additional required property beside reg ?
> > (like protocol@18 above...)
> >
> 
> If there are no consumers, should we just not add and deal with it
> entirely within the kernel. I know we rely today on presence of node
> before we initialise, but hey we have exception for system power protocol
> for other reasons, why not add this one too.
> 
> In short we shouldn't have to add a node if there are no consumers. It
> was one of the topic of discussion initially when SCMI binding was added
> and they exist only for the consumers otherwise we don't need it as
> everything is discoverable from the interface.
> 

I'm fine with that, just wanted to understand/clarify the rule here.

Thanks,
Cristian

Re: [PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[Rob Herring]

On Wed, Jan 25, 2023 at 8:11 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Wed, Jan 25, 2023 at 01:43:48PM +0000, Cristian Marussi wrote:
> > so now that the catch-all protocol@ patternProperty is gone in favour
> > of the 'protocol-node' definition and $refs, does that mean that any
> > current and future SCMI officially published protocol <N> has to be
> > added to the above explicit protocol list, even though it does not
> > have any special additional required property beside reg ?
> > (like protocol@18 above...)
> >
>
> If there are no consumers, should we just not add and deal with it
> entirely within the kernel. I know we rely today on presence of node
> before we initialise, but hey we have exception for system power protocol
> for other reasons, why not add this one too.
>
> In short we shouldn't have to add a node if there are no consumers. It
> was one of the topic of discussion initially when SCMI binding was added
> and they exist only for the consumers otherwise we don't need it as
> everything is discoverable from the interface.

As you might guess, I agree.

We need to keep 0x18 I suppose, right? I assume it is already in use.
Are there any others that didn't get documented? We'd need to keep
them because old kernels would still need them.

Rob

Re: [PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[Cristian Marussi]

On Wed, Jan 25, 2023 at 02:11:13PM +0000, Sudeep Holla wrote:
> On Wed, Jan 25, 2023 at 01:43:48PM +0000, Cristian Marussi wrote:
> > so now that the catch-all protocol@ patternProperty is gone in favour
> > of the 'protocol-node' definition and $refs, does that mean that any
> > current and future SCMI officially published protocol <N> has to be
> > added to the above explicit protocol list, even though it does not
> > have any special additional required property beside reg ?
> > (like protocol@18 above...)
> >
> 
> If there are no consumers, should we just not add and deal with it
> entirely within the kernel. I know we rely today on presence of node
> before we initialise, but hey we have exception for system power protocol
> for other reasons, why not add this one too.
> 
> In short we shouldn't have to add a node if there are no consumers. It
> was one of the topic of discussion initially when SCMI binding was added
> and they exist only for the consumers otherwise we don't need it as
> everything is discoverable from the interface.

It is fine for me the no-consumers/no-node argument (which anyway would
require a few changes in the core init logic anyway to work this way...),
BUT is it not that ANY protocol (even future-ones) does have, potentially,
consumers indeed, since each protocol-node can potentially have a dedicated
channel and related DT channel-descriptor ? (when multiple channels are
allowed by the transport)

I mean, as an example, you dont strictly need protos 0x18/0x12 nodes for
anything (if we patch the core init as said) UNLESS you want to dedicate
a channel to those protocols; so I'm just checking here if these kind of
scenarios will still be allowed with this binding change, or if I am
missing something.

Thanks,
Cristian

Re: [PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[Sudeep Holla]

On Thu, Jan 26, 2023 at 09:43:44AM +0000, Cristian Marussi wrote:
> On Wed, Jan 25, 2023 at 02:11:13PM +0000, Sudeep Holla wrote:
> > On Wed, Jan 25, 2023 at 01:43:48PM +0000, Cristian Marussi wrote:
> > > so now that the catch-all protocol@ patternProperty is gone in favour
> > > of the 'protocol-node' definition and $refs, does that mean that any
> > > current and future SCMI officially published protocol <N> has to be
> > > added to the above explicit protocol list, even though it does not
> > > have any special additional required property beside reg ?
> > > (like protocol@18 above...)
> > >
> > 
> > If there are no consumers, should we just not add and deal with it
> > entirely within the kernel. I know we rely today on presence of node
> > before we initialise, but hey we have exception for system power protocol
> > for other reasons, why not add this one too.
> > 
> > In short we shouldn't have to add a node if there are no consumers. It
> > was one of the topic of discussion initially when SCMI binding was added
> > and they exist only for the consumers otherwise we don't need it as
> > everything is discoverable from the interface.
> 
> It is fine for me the no-consumers/no-node argument (which anyway would
> require a few changes in the core init logic anyway to work this way...),
> BUT is it not that ANY protocol (even future-ones) does have, potentially,
> consumers indeed, since each protocol-node can potentially have a dedicated
> channel and related DT channel-descriptor ? (when multiple channels are
> allowed by the transport)
> 
> I mean, as an example, you dont strictly need protos 0x18/0x12 nodes for
> anything (if we patch the core init as said) UNLESS you want to dedicate
> a channel to those protocols; so I'm just checking here if these kind of
> scenarios will still be allowed with this binding change, or if I am
> missing something.

Ah, good point on the transport information. Yes we will need a node if
a protocol has a dedicated transport. No one has used so far other than
Juno perf, but we never know. We can always extended the bindings if
needed.

Sorry for missing the dedicated transport part.

-- 
Regards,
Sudeep

Re: [PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[Rob Herring]

On Thu, Jan 26, 2023 at 8:46 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Thu, Jan 26, 2023 at 09:43:44AM +0000, Cristian Marussi wrote:
> > On Wed, Jan 25, 2023 at 02:11:13PM +0000, Sudeep Holla wrote:
> > > On Wed, Jan 25, 2023 at 01:43:48PM +0000, Cristian Marussi wrote:
> > > > so now that the catch-all protocol@ patternProperty is gone in favour
> > > > of the 'protocol-node' definition and $refs, does that mean that any
> > > > current and future SCMI officially published protocol <N> has to be
> > > > added to the above explicit protocol list, even though it does not
> > > > have any special additional required property beside reg ?
> > > > (like protocol@18 above...)
> > > >
> > >
> > > If there are no consumers, should we just not add and deal with it
> > > entirely within the kernel. I know we rely today on presence of node
> > > before we initialise, but hey we have exception for system power protocol
> > > for other reasons, why not add this one too.
> > >
> > > In short we shouldn't have to add a node if there are no consumers. It
> > > was one of the topic of discussion initially when SCMI binding was added
> > > and they exist only for the consumers otherwise we don't need it as
> > > everything is discoverable from the interface.
> >
> > It is fine for me the no-consumers/no-node argument (which anyway would
> > require a few changes in the core init logic anyway to work this way...),
> > BUT is it not that ANY protocol (even future-ones) does have, potentially,
> > consumers indeed, since each protocol-node can potentially have a dedicated
> > channel and related DT channel-descriptor ? (when multiple channels are
> > allowed by the transport)
> >
> > I mean, as an example, you dont strictly need protos 0x18/0x12 nodes for
> > anything (if we patch the core init as said) UNLESS you want to dedicate
> > a channel to those protocols; so I'm just checking here if these kind of
> > scenarios will still be allowed with this binding change, or if I am
> > missing something.
>
> Ah, good point on the transport information. Yes we will need a node if
> a protocol has a dedicated transport. No one has used so far other than
> Juno perf, but we never know. We can always extended the bindings if
> needed.
>
> Sorry for missing the dedicated transport part.

So I need to add back 'protocol@.*' or not?

Rob

Re: [PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[Sudeep Holla]

On Thu, Jan 26, 2023 at 09:25:12AM -0600, Rob Herring wrote:
> On Thu, Jan 26, 2023 at 8:46 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Thu, Jan 26, 2023 at 09:43:44AM +0000, Cristian Marussi wrote:
> > > On Wed, Jan 25, 2023 at 02:11:13PM +0000, Sudeep Holla wrote:
> > > > On Wed, Jan 25, 2023 at 01:43:48PM +0000, Cristian Marussi wrote:
> > > > > so now that the catch-all protocol@ patternProperty is gone in favour
> > > > > of the 'protocol-node' definition and $refs, does that mean that any
> > > > > current and future SCMI officially published protocol <N> has to be
> > > > > added to the above explicit protocol list, even though it does not
> > > > > have any special additional required property beside reg ?
> > > > > (like protocol@18 above...)
> > > > >
> > > >
> > > > If there are no consumers, should we just not add and deal with it
> > > > entirely within the kernel. I know we rely today on presence of node
> > > > before we initialise, but hey we have exception for system power protocol
> > > > for other reasons, why not add this one too.
> > > >
> > > > In short we shouldn't have to add a node if there are no consumers. It
> > > > was one of the topic of discussion initially when SCMI binding was added
> > > > and they exist only for the consumers otherwise we don't need it as
> > > > everything is discoverable from the interface.
> > >
> > > It is fine for me the no-consumers/no-node argument (which anyway would
> > > require a few changes in the core init logic anyway to work this way...),
> > > BUT is it not that ANY protocol (even future-ones) does have, potentially,
> > > consumers indeed, since each protocol-node can potentially have a dedicated
> > > channel and related DT channel-descriptor ? (when multiple channels are
> > > allowed by the transport)
> > >
> > > I mean, as an example, you dont strictly need protos 0x18/0x12 nodes for
> > > anything (if we patch the core init as said) UNLESS you want to dedicate
> > > a channel to those protocols; so I'm just checking here if these kind of
> > > scenarios will still be allowed with this binding change, or if I am
> > > missing something.
> >
> > Ah, good point on the transport information. Yes we will need a node if
> > a protocol has a dedicated transport. No one has used so far other than
> > Juno perf, but we never know. We can always extended the bindings if
> > needed.
> >
> > Sorry for missing the dedicated transport part.
>
> So I need to add back 'protocol@.*' or not?

IMO it is better to know what exactly gets added under each of these protocol
sub-nodes and so better to have entry specific to each known protocols. I
liked that fact with this change as I have seen some crazy vendor extensions
adding all sorts of non-sense defining some vendor protocol. For example [1],
in which case we can catch those better than existing schema which matches
all. So let's not add protocol@.* if possible or until that becomes the only
cleaner way to maintain this.

--
Regards,
Sudeep

[1] https://lore.kernel.org/all/1667451512-9655-2-git-send-email-quic_sibis@quicinc.com/

Re: [PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[Rob Herring]

On Thu, Jan 26, 2023 at 11:04 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Thu, Jan 26, 2023 at 09:25:12AM -0600, Rob Herring wrote:
> > On Thu, Jan 26, 2023 at 8:46 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > On Thu, Jan 26, 2023 at 09:43:44AM +0000, Cristian Marussi wrote:
> > > > On Wed, Jan 25, 2023 at 02:11:13PM +0000, Sudeep Holla wrote:
> > > > > On Wed, Jan 25, 2023 at 01:43:48PM +0000, Cristian Marussi wrote:
> > > > > > so now that the catch-all protocol@ patternProperty is gone in favour
> > > > > > of the 'protocol-node' definition and $refs, does that mean that any
> > > > > > current and future SCMI officially published protocol <N> has to be
> > > > > > added to the above explicit protocol list, even though it does not
> > > > > > have any special additional required property beside reg ?
> > > > > > (like protocol@18 above...)
> > > > > >
> > > > >
> > > > > If there are no consumers, should we just not add and deal with it
> > > > > entirely within the kernel. I know we rely today on presence of node
> > > > > before we initialise, but hey we have exception for system power protocol
> > > > > for other reasons, why not add this one too.
> > > > >
> > > > > In short we shouldn't have to add a node if there are no consumers. It
> > > > > was one of the topic of discussion initially when SCMI binding was added
> > > > > and they exist only for the consumers otherwise we don't need it as
> > > > > everything is discoverable from the interface.
> > > >
> > > > It is fine for me the no-consumers/no-node argument (which anyway would
> > > > require a few changes in the core init logic anyway to work this way...),
> > > > BUT is it not that ANY protocol (even future-ones) does have, potentially,
> > > > consumers indeed, since each protocol-node can potentially have a dedicated
> > > > channel and related DT channel-descriptor ? (when multiple channels are
> > > > allowed by the transport)
> > > >
> > > > I mean, as an example, you dont strictly need protos 0x18/0x12 nodes for
> > > > anything (if we patch the core init as said) UNLESS you want to dedicate
> > > > a channel to those protocols; so I'm just checking here if these kind of
> > > > scenarios will still be allowed with this binding change, or if I am
> > > > missing something.
> > >
> > > Ah, good point on the transport information. Yes we will need a node if
> > > a protocol has a dedicated transport. No one has used so far other than
> > > Juno perf, but we never know. We can always extended the bindings if
> > > needed.
> > >
> > > Sorry for missing the dedicated transport part.
> >
> > So I need to add back 'protocol@.*' or not?
>
> IMO it is better to know what exactly gets added under each of these protocol
> sub-nodes and so better to have entry specific to each known protocols. I
> liked that fact with this change as I have seen some crazy vendor extensions
> adding all sorts of non-sense defining some vendor protocol. For example [1],
> in which case we can catch those better than existing schema which matches
> all. So let's not add protocol@.* if possible or until that becomes the only
> cleaner way to maintain this.

TBC, 'protocol@.*' would not allow anything but the properties defined
in the /$defs/protocol-node. So [1] would throw errors without a
schema addition.

We should either do that along with dropping 'protocol@18' or we keep
protocol 0x18 node and add all other providerless protocols. I don't
think we need the latter to just check unit-address vs. reg. I want to
come up with a better way to do that (dtc does some, but only for
defined bus types).

Rob

Re: [PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[Sudeep Holla]

Hi Rob,

On Fri, Jan 27, 2023 at 12:52:33PM -0600, Rob Herring wrote:
>
> TBC, 'protocol@.*' would not allow anything but the properties defined
> in the /$defs/protocol-node. So [1] would throw errors without a
> schema addition.

Right I clearly missed that, somehow I assumed it would allow.

> We should either do that along with dropping 'protocol@18' or we keep
> protocol 0x18 node and add all other providerless protocols. I don't
> think we need the latter to just check unit-address vs. reg.

I only argument today it to allow protocol specific transport. So we could
delay addition of it until someone needs that way. So far we haven't seen
anyone using it other than performance(even that is not needed with the
introduction of fast channels that are auto discoverable in relatively
newer versions of the spec).

--
Regards,
Sudeep

Re: [PATCH] dt-bindings: firmware: arm,scmi: Restrict protocol child node properties

[Rob Herring]

On Mon, Feb 6, 2023 at 4:47 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> Hi Rob,
>
> On Fri, Jan 27, 2023 at 12:52:33PM -0600, Rob Herring wrote:
> >
> > TBC, 'protocol@.*' would not allow anything but the properties defined
> > in the /$defs/protocol-node. So [1] would throw errors without a
> > schema addition.
>
> Right I clearly missed that, somehow I assumed it would allow.
>
> > We should either do that along with dropping 'protocol@18' or we keep
> > protocol 0x18 node and add all other providerless protocols. I don't
> > think we need the latter to just check unit-address vs. reg.
>
> I only argument today it to allow protocol specific transport. So we could
> delay addition of it until someone needs that way. So far we haven't seen
> anyone using it other than performance(even that is not needed with the
> introduction of fast channels that are auto discoverable in relatively
> newer versions of the spec).

I failed to think about 'protocol@.*' would match on every protocol,
so we have to list them explicitly: '^protocol@(18|xx|yy|zz)$'

Anyways, I think the conclusion is the patch should stay as-is and so
I've applied it.

Rob