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

[Krzysztof Kozlowski <krzk@kernel.org>]

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?

> 
> 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.

Best regards,
Krzysztof