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

[David Jander <david@protonic.nl>]

On Mon, 3 Mar 2025 15:18:40 +0100
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 03/03/2025 12:40, David Jander wrote:
> >>
> >> 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?  
> 
> No, because that flag is already supported via PWM_POLARITY_INVERTED.
> 
> Do not tie bindings to specific implementation. If PWM core is changed
> to always handle PWM_POLARITY_INVERTED even if controller does not
> support it, would the binding became outdated?

Understood. I guess I will have to start fixing PWM drivers then... @uwe, do
you have objections to trying to add a (very trivial) helper for PWM
controllers that can't do inversion in hardware and start to patch some of
them if needed?

> > 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.  
> 
> Sure and use existing bindings for that. If implementation has problems,
> fix implementation.

Sounds reasonable.

Best regards,

-- 
David Jander