Re: [PATCH 1/2] dt-bindings: crypto: fsl,sec-v4.0-mon: add optional force shutdown time property

[Ian Ray <ian.ray@gehealthcare.com>]

On Fri, Mar 14, 2025 at 09:44:03AM +0100, Krzysztof Kozlowski wrote:
> On Thu, Mar 13, 2025 at 01:44:52PM +0200, Ian Ray wrote:
> > Add an optional property to configure the force shutdown time.
> 
> And why is this hardware property? You described the desired Linux
> feature or behavior, not the actual hardware. The bindings are about the
> latter, so instead you need to rephrase the property and its description
> to match actual hardware capabilities/features/configuration etc.
> 
> 
> >
> > Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
> > ---
> >  .../devicetree/bindings/crypto/fsl,sec-v4.0-mon.yaml      | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/crypto/fsl,sec-v4.0-mon.yaml b/Documentation/devicetree/bindings/crypto/fsl,sec-v4.0-mon.yaml
> > index e879bc0be8e2..ee282c352535 100644
> > --- a/Documentation/devicetree/bindings/crypto/fsl,sec-v4.0-mon.yaml
> > +++ b/Documentation/devicetree/bindings/crypto/fsl,sec-v4.0-mon.yaml
> > @@ -111,6 +111,14 @@ properties:
> >          maxItems: 1
> >          default: 116
> >
> > +      force-shutdown-time:
> 
> Missing unit suffix
> 
> Missing vendor prefix
> 
> > +        description: |
> 
> Do not need '|' unless you need to preserve formatting.
> 
> > +          Configure the long-press force shutdown time.
> > +          Setting this value to zero disables the long-press detection.
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        enum: [0, 5, 10, 15]
> > +        default: 5
> 
> Why existing power-off-time-sec does not work? Isn't input schema
> already referenced for this node? Looks like you are duplicating things
> for no gain.

Thank you -- I somehow completely missed the existing property (which as
you note) is exactly what's needed.

I will send a V2 which uses the existing property.

> 
> Best regards,
> Krzysztof

Thank you for the review,
Ian