Re: [PATCH] pwm: mediatek: support inverted polarity

[Uwe Kleine-König]

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@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx/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/ |

Attachment: signature.asc
Description: PGP signature