Date:   Sat, 4 Mar 2023 11:18:24 +0100
From:   Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
To:     Lorenz Brun <lorenz@brun.one>
Cc:     Thierry Reding <thierry.reding@gmail.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        linux-pwm@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org
Subject: Re: [PATCH] pwm: mediatek: support inverted polarity
Message-ID: <20230304101824.ja7otxatkrrv2xxq@pengutronix.de>
References: <20230303205821.2285418-1-lorenz@brun.one>
 <20230303211725.7wtxdxjqpxlrp77b@pengutronix.de>
 <J6UYQR.PWF59DFFYYO71@brun.one>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="gtzva4fuv4iy5elv"
Content-Disposition: inline
In-Reply-To: <J6UYQR.PWF59DFFYYO71@brun.one>
Precedence: bulk


--gtzva4fuv4iy5elv
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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=F6nig
> <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.
> > >=20
> > >  This implements the same solution as in pwm-meson and just inverts
> > > the
> > >  duty cycle instead, which results in the same outcome.
> >=20
> > 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@pengu=
tronix.de/T/#meda75ffbd4ef2048991ea2cd091c0c14b1bb09c2
> >=20
> 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 t=
hat
> 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.
>=20
> 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
=2Eusage_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 =3D 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 =3D .period -
=2Eduty_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
=2Eduty_offset =3D .period (or .period - 1?).

Of course that would be a bigger effort.

Best regards
Uwe

--=20
Pengutronix e.K.                           | Uwe Kleine-K=F6nig            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

--gtzva4fuv4iy5elv
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEfnIqFpAYrP8+dKQLwfwUeK3K7AkFAmQDGu0ACgkQwfwUeK3K
7AmsnAf/fWQuuTY0c+1r1b43FElwpoyXMnsuq1QzKB5KQRU60v/Z2fTek2f8fy0A
f+0KbZCb+OGdv6V0Ct83X41A4tIYWGDVs1dJYX6ZxbGFbaCWZ6dRIU/P9qpni2BO
3ezvTFrtY75hX6mrXNlEAmOG7oliIyw4vcSA+J4uR3z+lg8xNuj8/EVaWv1UVo8l
n8uUfImwADeST1FTJqTZnWTXywWoITc4Cql5ITWn+ZAmCTfTGBOkc3v1hvta7rOs
vBoIajVUbbL9QU9m/uuKaBk/YRLiuIIBxVdrNgV+RzY0mG3w/EpfSdXVVDPmz7vd
1pZRniBxLaHUN7aoSwm50Uczd6/UvA==
=Q76Z
-----END PGP SIGNATURE-----

--gtzva4fuv4iy5elv--