Re: [RFC PATCH 7/7] dt-bindings: motion: Add motion-simple-pwm bindings

David Jander
2025-03-03T11:40:36Z

Dear Uwe,

Thanks for chiming in!

On Fri, 28 Feb 2025 16:18:05 +0100
Uwe Kleine-König <ukleinek@kernel.org> wrote:

> Hey David,
>
> On Fri, Feb 28, 2025 at 11:09:31AM +0100, David Jander wrote:
> > On Fri, 28 Feb 2025 10:37:48 +0100
> > Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > > On 28/02/2025 10:22, David Jander wrote:
> > > >
> > > >>> +
> > > >>> + motion,pwm-inverted:
> > > >>> + $ref: /schemas/types.yaml#/definitions/flag
> > > >>
> > > >> And PWM flag does not work?
> > > >
> > > > I have seen PWM controllers that don't seem to support the
> > > > PWM_POLARITY_INVERTED flag and those where it just doesn't work. Should all
> > >
> > >
> > > Shouldn't the controllers be fixed? Or let's rephrase the question: why
> > > only this PWM consumer needs this property and none of others need it?
> >
> > CCing Uwe Kleine-Koenig and linux-pwm mailing list.
> >
> > I know that at least in kernel 6.11 the pwm-stm32.c PWM driver doesn't
> > properly invert the PWM signal when specifying PWM_POLARITY_INVERTED. I agree
> > this is a probably bug that needs fixing if still present in 6.14-rc. Besides
> > that, if linux-pwm agrees that every single PWM driver _must_ properly support
> > this flag, I will drop this consumer flag an start fixing broken PWM drivers
> > that I encounter. I agree that it makes more sense this way, but I wanted to
> > be sure.
>
> Some hardwares cannot support PWM_POLARITY_INVERTED. Affected drivers
> include:
>
> pwm-adp5585
> pwm-ntxec
> pwm-raspberrypi-poe
> pwm-rz-mtu3 (software limitation only)
> pwm-sunplus
> pwm-twl-led (not completely sure, that one is strange)
>
> . ISTR that there is a driver that does only support inverted polarity,
> but I don't find it. For an overview I recommend reading through the
> output of:
>
> for f in drivers/pwm/pwm-*; do
> echo $f;
> sed -rn '/Limitations:/,/\*\/?$/p' $f;
> echo;
> done | less
>
> . (Note not all drivers have commentary in the right format to unveil
> their limitations.)
>
> For most use-cases you can just do
>
> .duty_cycle = .period - .duty_cycle

Yes, that is exactly what the relevant code in motion/simple-pwm.c does when
the "pwm-inverted" flag is present in the DT node.

> instead of inverting polarity, but there is no abstraction in the PWM
> bindings for that and also no helpers in the PWM framework. The problem
> is more or less ignored, so if you have a device with
>
> pwms = <&pwm0 0 PWM_POLARITY_INVERTED>;
>
> and the PWM chip in question doesn't support that, the pwm API functions
> will fail. So the system designer better makes sure that the PWM
> hardware can cope with the needed polarity.

Thanks for clarifying this!

@Krzysztof, do you think that given this situation it is acceptable to include
the "pwm-inverted" flag in the dt-schema of the simple PWM motor driver?

The need for an inverted PWM signal is something very common in the case of
H-bridge motor drivers, where the PWM signal represents the actual logical
output level of each of the two halves of the bridge. Often the high-side
switches are used as the free-wheel position, so that 100% duty-cycle on both
channels is actually standstill, while 0% duty-cycle on one channel is full
speed in either direction. This isn't always the case though, hence the
importance for this to be able to be selected.

Best regards,

--
David Jander