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

[Uwe Kleine-König]

On Sat, Mar 11, 2023 at 10:00:50PM +0100, Martin Blumenstingl 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...

And that's the problem. As the hardware doesn't support inverted
polarity there is no way to implement it correctly. The only right way
would be to return -EINVAL in this case, but this might break some
consumers.

I have an idea how to evolve the PWM API. That's by introducing an
.offset parameter to struct pwm_state. This would describe the following
PWM signal:


   ___________/¯¯¯¯¯¯¯¯¯\_______________/¯¯¯¯¯¯¯¯¯\____
   ^                         ^                         ^                         ^                         ^
   <------ period ---------->
   <- offset->
              <--------> duty_cycle

This is more general than polarity: It can describe normal polarity
(.offset = 0) and inversed polarity (.offset = .period - .duty_cycle).

Then the policy to implement a pwm_state like that would probably be:

 - Pick the biggest period not bigger than requested
 - for that period pick the biggest duty cycle not bigger than requested
 - for that period and duty_cycle pick the biggest offset not bigger
   than requested.

With these rules in place it would be allowed to configure normal
polarity for a request with inverted polarity, but not the other way
around. Then the algorithm currently implemented in the meson driver
would be allowed.

A consumer that doesn't care about the offset (i.e. most drivers) could
just pass .offset = .period - 1.

To be practical for consumers who care about polarity, we first would
need a way to test the capabilities of a PWM though. I have an idea for
that, too, but today this is still vapourware.

> > Fixes: c375bcbaabdb ("pwm: meson: Read the full hardware state in meson_pwm_get_state()")
> > Reported-by: Munehisa Kamata <kamatam at amazon.com>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig at pengutronix.de>
> 
> Acked-by: Martin Blumenstingl <martin.blumenstingl at googlemail.com>

Thanks
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/20230311/45c37271/attachment.sig>