Dear Krzysztof,
Thanks for reviewing...
On Fri, 28 Feb 2025 08:11:04 +0100
Krzysztof Kozlowski <krzk@xxxxxxxxxx> 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
