linux-kernel - Re: [RFC PATCH 6/7] dt-bindings: motion: Add adi,tmc5240 bindings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250228094818.0aad5491@erd003.prtnl>
Date: Fri, 28 Feb 2025 09:48:18 +0100
From: David Jander <david@...tonic.nl>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org, Jonathan Corbet
 <corbet@....net>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 devicetree@...r.kernel.org, linux-doc@...r.kernel.org, Nuno Sa
 <nuno.sa@...log.com>, Jonathan Cameron <jic23@...nel.org>, Oleksij Rempel
 <o.rempel@...gutronix.de>
Subject: Re: [RFC PATCH 6/7] dt-bindings: motion: Add adi,tmc5240 bindings


Dear Krzysztof,

Thanks for reviewing...

On Fri, 28 Feb 2025 08:11:04 +0100
Krzysztof Kozlowski <krzk@...nel.org> wrote:

> On Thu, Feb 27, 2025 at 05:28:22PM +0100, David Jander wrote:
> [...]
> > +
> > +  enable-supply:
> > +    description: Optional external enable supply to control SLEEPn pin. Can  
> 
> That's odd. regular pins are not supplies. This must be named after
> physical supplies. There is vdd18, vcc, vcp but nothing about enable
> supply in datasheet.
> 
> > +      be shared between several controllers.  
> 
> Second sentence is both redundant and really not relevant to this
> binding. It's not this binding which decides about sharing.

Good point. I think I should drop the whole property, since it is indeed
irrelevant. If extra supplies need to be specified, they always can be, right?

> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +  - $ref: /schemas/motion/common.yaml#
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    spi {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        motor@0 {
> > +            compatible = "adi,tmc5240";
> > +            reg = <0>;
> > +            interrupts-extended = <&gpiok 7 0>;  
> 
> Include header and use standard defines for flags.

Thanks. I didn't know I could include them here... will fix this.

> 
> > +            clocks = <&clock_tmc5240>;
> > +            enable-supply = <&stpsleepn>;
> > +            spi-max-frequency = <1000000>;  
> 
> Where are any other properties from common schema?

The properties in common.yaml are optional. Do I need to explicitly specify
this somehow (they are not listed under "required:")?
In fact, the tmc5240 driver has its own known constants for these properties
that it fills in. One can overrule them here if needed, but I suppose in the
case of the tmc5240 the regular use-case is not to do that. Should I maybe add
a second example that overrules the defaults?

Best regards,

-- 
David Jander