[PATCH] pwm: meson: Explicitly set .polarity in .get_state()

[Uwe Kleine-König]

On Mon, Mar 13, 2023 at 10:07:48AM +0100, Jerome Brunet wrote:
> 
> On Sat 11 Mar 2023 at 22:00, Martin Blumenstingl <martin.blumenstingl at googlemail.com> wrote:
> 
> > Hi Uwe,
> >
> > On Fri, Mar 10, 2023 at 8:14 PM Uwe Kleine-König
> > <u.kleine-koenig at pengutronix.de> wrote:
> > [...]
> >> There is a complicating fact, that the .apply() callback fakes support
> >> for inversed polarity. This is not (and cannot) be matched by
> >> .get_state(). As fixing this isn't easy, only point it out in a comment
> >> to prevent authors of other drivers from copying that approach.
> > If you have any suggestions on how to fix this then please let us know.
> > I don't recall any board needing support for inverted PWM - but they
> > may be out there somewhere...
> 
> AFAIK, PWM are essentially used for the SDIO 32k clock and voltage
> regulators. I don't recall seeing anything else.
> 
> It should be safe to change polarity if necessary, except for the DVFS
> PWM regulators maybe ? I fear that if we change the PWM setting it might
> trigger a glitch on the supply and possibly stall the CPU.
> 
> That being said, I don't think there is any particular care regarding
> that right now, so maybe it is fine.

Another option is to do something like that:

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 16d79ca5d8f5..25a177a3fa00 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -162,8 +162,10 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
 	duty = state->duty_cycle;
 	period = state->period;
 
-	if (state->polarity == PWM_POLARITY_INVERSED)
+	if (state->polarity == PWM_POLARITY_INVERSED) {
+		WARN_ONCE(1, "Wrongly trying to support inversed polarity. Please report to linux-pwm at vger.kernel.org if you rely on this\n");
 		duty = period - duty;
+	}
 
 	fin_freq = clk_get_rate(channel->clk);
 	if (fin_freq == 0) {

and then drop that faked support in a year or so if nobody spoke up.

Disclaimer: I assume Thierry is not a fan of this approach, he opposed
similar warnings in the past.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-amlogic/attachments/20230313/cf1498cf/attachment.sig>