diff mbox series

[3/5] dt-bindings: firmware: arm,scmi: Introduce more transport properties

Message ID 20241018080602.3952869-4-cristian.marussi@arm.com (mailing list archive)
State New, archived
Headers show
Series Expose SCMI Transport properties | expand

Commit Message

Cristian Marussi Oct. 18, 2024, 8:06 a.m. UTC
Depending on specific hardware and firmware design choices, it may be
possible for different platforms to end up having different requirements
regarding the same transport characteristics.

Introduce max-msg-size and max-msg properties to describe such platform
specific transport constraints, since they cannot be discovered otherwise.

Cc: devicetree@vger.kernel.org
Cc: Rob Herring (Arm) <robh@kernel.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 .../devicetree/bindings/firmware/arm,scmi.yaml   | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Rob Herring (Arm) Oct. 18, 2024, 1:33 p.m. UTC | #1
On Fri, Oct 18, 2024 at 09:06:00AM +0100, Cristian Marussi wrote:
> Depending on specific hardware and firmware design choices, it may be
> possible for different platforms to end up having different requirements
> regarding the same transport characteristics.
> 
> Introduce max-msg-size and max-msg properties to describe such platform
> specific transport constraints, since they cannot be discovered otherwise.
> 
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring (Arm) <robh@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
>  .../devicetree/bindings/firmware/arm,scmi.yaml   | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> index 54d7d11bfed4..42852ed887f2 100644
> --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> @@ -131,6 +131,22 @@ properties:
>        be a non-zero value if set.
>      minimum: 1
>  
> +  max-msg-size:

Vendor prefix needed.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      An optional value, expressed in bytes, representing the maximum size
> +      allowed for the payload of messages transmitted on this transport.
> +      If set it is recommended to be greater or equal than the minimum size
> +      required to support all the messages defined by the set of protocols
> +      implemented on this platform.

Sounds kind of broken if less than the minimum...

> +
> +  max-msg:

Vendor prefix and could be a bit more specific what this is.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      An optional value representing the maximum number of concurrent in-flight
> +      messages allowed by this transport. If set, the value should be non-zero.
> +    minimum: 1
> +
>    arm,smc-id:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      description:
> -- 
> 2.46.1
>
Rob Herring (Arm) Oct. 18, 2024, 1:37 p.m. UTC | #2
On Fri, Oct 18, 2024 at 09:06:00AM +0100, Cristian Marussi wrote:
> Depending on specific hardware and firmware design choices, it may be
> possible for different platforms to end up having different requirements
> regarding the same transport characteristics.
> 
> Introduce max-msg-size and max-msg properties to describe such platform
> specific transport constraints, since they cannot be discovered otherwise.
> 
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring (Arm) <robh@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Also, you missed Conor and that not the right address for Krzysztof. Use 
get_maintainers.pl or a tool that uses it (b4).

Also, no need to store maintainer addresses in the commit msg.

Rob
Cristian Marussi Oct. 18, 2024, 2:10 p.m. UTC | #3
On Fri, Oct 18, 2024 at 08:33:40AM -0500, Rob Herring wrote:
> On Fri, Oct 18, 2024 at 09:06:00AM +0100, Cristian Marussi wrote:
> > Depending on specific hardware and firmware design choices, it may be
> > possible for different platforms to end up having different requirements
> > regarding the same transport characteristics.
> > 

Hi Rob,

thanks for having a look.

> > Introduce max-msg-size and max-msg properties to describe such platform
> > specific transport constraints, since they cannot be discovered otherwise.
> > 
> > Cc: devicetree@vger.kernel.org
> > Cc: Rob Herring (Arm) <robh@kernel.org>
> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> > ---
> >  .../devicetree/bindings/firmware/arm,scmi.yaml   | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > index 54d7d11bfed4..42852ed887f2 100644
> > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > @@ -131,6 +131,22 @@ properties:
> >        be a non-zero value if set.
> >      minimum: 1
> >  
> > +  max-msg-size:
> 
> Vendor prefix needed.

Ok, but these are common properties like max-rx-timeout-ms and others in this
bindings so I did not expect to have to explicit set a vendor...well I
really dont know what's the rule here, so I went for similarity.

> 
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      An optional value, expressed in bytes, representing the maximum size
> > +      allowed for the payload of messages transmitted on this transport.
> > +      If set it is recommended to be greater or equal than the minimum size
> > +      required to support all the messages defined by the set of protocols
> > +      implemented on this platform.
> 
> Sounds kind of broken if less than the minimum...
> 

Well, absolutely but the minimum payload really depends on how many
protocols the SCMI server in firmware supports on this platform...the
set of implemented messages (some are optional) AND the spec version
number...so I cannot state really a fixed safe minimum here...it was
just a polite warning for the user not to shot himself in the foot...
..but whoever missteps over this will see a lot of comms errors
indeed...I will drop the warning in V2

> > +
> > +  max-msg:
> 
> Vendor prefix and could be a bit more specific what this is.

Ok I will explain more in v2.

Thanks,
Cristian
Cristian Marussi Oct. 18, 2024, 2:11 p.m. UTC | #4
On Fri, Oct 18, 2024 at 08:37:25AM -0500, Rob Herring wrote:
> On Fri, Oct 18, 2024 at 09:06:00AM +0100, Cristian Marussi wrote:
> > Depending on specific hardware and firmware design choices, it may be
> > possible for different platforms to end up having different requirements
> > regarding the same transport characteristics.
> > 
> > Introduce max-msg-size and max-msg properties to describe such platform
> > specific transport constraints, since they cannot be discovered otherwise.
> > 
> > Cc: devicetree@vger.kernel.org
> > Cc: Rob Herring (Arm) <robh@kernel.org>
> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Also, you missed Conor and that not the right address for Krzysztof. Use 
> get_maintainers.pl or a tool that uses it (b4).
> 
> Also, no need to store maintainer addresses in the commit msg.

Ok, my bad I ahev not checked with the tool this time but just grabbed
from a recent commit log. I will fix in V2

Thanks,
Cristian
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 54d7d11bfed4..42852ed887f2 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -131,6 +131,22 @@  properties:
       be a non-zero value if set.
     minimum: 1
 
+  max-msg-size:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      An optional value, expressed in bytes, representing the maximum size
+      allowed for the payload of messages transmitted on this transport.
+      If set it is recommended to be greater or equal than the minimum size
+      required to support all the messages defined by the set of protocols
+      implemented on this platform.
+
+  max-msg:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      An optional value representing the maximum number of concurrent in-flight
+      messages allowed by this transport. If set, the value should be non-zero.
+    minimum: 1
+
   arm,smc-id:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: