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

[David Jander <david@protonic.nl>]

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