linux-kernel - Re: [PATCH] pwm: mediatek: support inverted polarity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230304101824.ja7otxatkrrv2xxq@pengutronix.de>
Date:   Sat, 4 Mar 2023 11:18:24 +0100
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Lorenz Brun <lorenz@...n.one>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        linux-pwm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org
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@...gutronix.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/ |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)