Messages in this thread | | | Date | Sat, 4 Mar 2023 11:18:24 +0100 | From | Uwe Kleine-König <> | Subject | Re: [PATCH] pwm: mediatek: support inverted polarity |
| |
Hello Lorenz,
On Fri, Mar 03, 2023 at 11:23:07PM +0100, Lorenz Brun wrote: > On Fri, Mar 3 2023 at 22:17:25 +01:00:00, Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> wrote: > > On Fri, Mar 03, 2023 at 09:58:21PM +0100, Lorenz Brun wrote: > > > According to the MT7986 Reference Manual the Mediatek PWM > > > controller > > > doesn't appear to have support for inverted polarity. > > > > > > This implements the same solution as in pwm-meson and just inverts > > > the > > > duty cycle instead, which results in the same outcome. > > > > This idea is broken. This was recently discussed on the linux-pwm list > > and I hope will be fixed soon. See > > https://lore.kernel.org/linux-pwm/20230228093911.bh2sbp4tyfir2z5g@pengutronix.de/T/#meda75ffbd4ef2048991ea2cd091c0c14b1bb09c2 > > > Is the issue here emulating PWM_POLARITY_INVERSED by inverting the period or > the overflow issues? > This driver currently rejects PWM_POLARITY_INVERSED, but the problem is that > I have a board which inverts the output of the PWM peripheral (low-side > MOSFET for higher-voltage open-drain output), thus I need to set the PWM > node to output an inverted signal so that the final open-drain output > behaves correctly as the signal has been inverted twice now. > > In my specific case this logic could also be added to pwm-fan, but this > would lead to more complexity there as this type of circuit is generally > handled by the PWM driver.
The issue is clear, and I'm sure the motivation was similar for meson.
However just inverting duty_cycle might hurt consumers who rely on actually inversed polarity.
There is an approach available: You could implement support for .usage_power. However I don't like that concept because its semantic is unclear (but in the past there is no agreement about that betweeen Thierry and me).
My favourite would be to add a u64 duty_offset to struct pwm_state that would allow to request something like:
________ ________ ________ ___/ \________/ \________/ \______ ^ ^ ^ ^ <-> duty_offset <-------> duty_cycle <----------------> period
Then todays requests would be equivalent to .duty_offset = 0, and drivers would be advised to implement the biggest duty_offset not bigger than requested (i.e. similar to how period and duty_cycle work).
This could even replace .polarity by setting .duty_offset = .period - .duty_cycle. And a consumer who doesn't care about polarity but only about percentage of the active time during a period could signal that by .duty_offset = .period (or .period - 1?).
Of course that would be a bigger effort.
Best regards Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [unhandled content-type:application/pgp-signature] | |