Re: [PATCH v3 4/8] dt-bindings: firmware: arm,scmi: Add atomic_threshold optional property

[Cristian Marussi]

On Tue, Feb 08, 2022 at 08:12:39AM -0800, Florian Fainelli wrote:
> 
> 
> On 2/8/2022 7:44 AM, Cristian Marussi wrote:
> > SCMI protocols in the platform can optionally signal to the OSPM agent
> > the expected execution latency for a specific resource/operation pair.
> > 
> > Introduce an SCMI system wide optional property to describe a global time
> > threshold which can be configured on a per-platform base to determine the
> > opportunity, or not, for an SCMI command advertised to have a higher
> > latency than the threshold, to be considered for atomic operations:
> > high-latency SCMI synchronous commands should be preferably issued in the
> > usual non-atomic mode.
> > 

Hi Florian,

thanks for the feedback first of all.

> > Cc: Rob Herring <robh+dt@xxxxxxxxxx>
> > Cc: devicetree@xxxxxxxxxxxxxxx
> > Signed-off-by: Cristian Marussi <cristian.marussi@xxxxxxx>
> > ---
> > v1 --> v2
> > - rephrased the property description
> > ---
> >   .../devicetree/bindings/firmware/arm,scmi.yaml        | 11 +++++++++++
> >   1 file changed, 11 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > index eae15df36eef..646bdf2873b5 100644
> > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > @@ -81,6 +81,15 @@ properties:
> >     '#size-cells':
> >       const: 0
> > +  atomic_threshold:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      An optional time value, expressed in microseconds, representing, on this
> > +      platform, the threshold above which any SCMI command, advertised to have
> > +      an higher-than-threshold execution latency, should not be considered for
> > +      atomic mode of operation, even if requested.
> > +      If left unconfigured defaults to zero.
> 
> Underscores in properties is not usually something that is desired, and it
> might be a good idea to put the unit in the property name, how about:
> atomic-threshold-us?
> 

I was indeed in doubt if it was desirable or not to have the units
embedded in the name ... thanks for the hint about the underscores too,

I'll fix nextV following your advice.

Thanks,
Cristian