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

David Jander
2025-02-28T09:51:51Z

On Fri, 28 Feb 2025 10:35:38 +0100
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 28/02/2025 09:48, David Jander wrote:
> >
> > Dear Krzysztof,
> >
> > Thanks for reviewing...
> >
> > On Fri, 28 Feb 2025 08:11:04 +0100
> > Krzysztof Kozlowski <krzk@kernel.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?
>
> You should specify all supplies now, because hardware should be fully
> described by binding and DTS.

In the case of the hardware I use for testing all of this, there are several
tmc5240 chips which have their "SLEEPN" pin tied together controlled by a
single GPIO pin that needs to be pulled high before any of these chips can be
talked to. The usual way I know of solving this is by specifying a common
"virtual" supply of type "regulator-fixed" with an enable gpio.
But this isn't strictly a supply that has to do with this chip or driver, so I
don't think it should be specified in the schema. I do need to use it in my
particular case though. Is there a better way of doing this?

> What's more, the necessary supplies (according to datasheet) should be
> required, not optional.

Do you mean that they should be in the binding definition as well? I.e. add
all of Vs, Vdd1v8 and Vcc_io here?

Best regards,

--
David Jander