Re: RFC: don't let drivers issue pwm_disable after pwm_config(pwm, 0, period)

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1892 bytes --]

On Fri, Oct 12, 2018 at 11:45:57AM +0200, Uwe Kleine-König wrote:
> On Thu, Oct 11, 2018 at 01:15:44PM +0000, Vokáč Michal wrote:
[...]
> > >> Sidenote: With the current capabilities of the pwm framework there is no
> > >> technical reason to expose to the hardware drivers that the pwm user uses
> > >> inverted logic. The framework could just apply
> > >>
> > >> 	duty = period - duty;
> > >>
> > 
> > I prefer to utilize as much HW capabilities as possible. And it seems
> > like most PWM chips support HW output inversion (to some extent) so to
> > me it seems perfectly OK that that information can be propagated from
> > the upper SW levels to the lowest one.
> 
> If the hardware capability is useful I fully agree. But if inversion can
> be accomplished by a chip that doesn't support inversion in hardware by
> just using duty = period - duty (and so can be accomplished by a chip
> that has inversion support without making use of it) then the drivers
> shouldn't be confronted with it.

These two are orthogonal problems. If all you care about is the power
output of the PWM (as is the case for backlights, LEDs or fans, for
example), then inverted polarity has the same effect as duty = period -
duty.

However, the two don't result in identical waveforms. Again, a backlight
or LED doesn't care about the exact waveform, but there are other uses
for PWMs where these actually are important. I have admittedly never
used any of them myself, but this was brought to my attention a long
time ago when polarity support was introduced. I'm sure you can find the
discussions in some archives if you are inclined to do so.

> (I'm not sure if inversion is really relevant with the current status
> quo, as the expectations are not documented in a place I'm aware of.)

See the kerneldoc for enum pwm_polarity which does document this.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]