Received-SPF: pass (google.com: domain of linux-kernel+bounces-133540-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Fri, 5 Apr 2024 21:55:54 +0200
From: Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
To: David Lechner <dlechner@baylibre.com>
Cc: Trevor Gamblin <tgamblin@baylibre.com>, linux-pwm@vger.kernel.org, 
	linux-kernel@vger.kernel.org, michael.hennerich@analog.com, nuno.sa@analog.com
Subject: Re: [RFC PATCH 0/3] pwm: add support for duty_offset
Message-ID: <gcibnofcdvs3qwygsuvwobckoyipj2a3t72remo5ohauuvf2cb@4ae4byeom67d>
References: <20240405003025.739603-1-tgamblin@baylibre.com>
 <fhrderkiwqzc7fw2hptz2la3aco72wlossqbrr4m42qllcpcpw@vhxflah2ex2a>
 <CAMknhBFgXdH63_N738qMt6tzK_zWdB-OWvYTWitmBuQNqrN+LQ@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="wewextuk6np2tzoq"
Content-Disposition: inline
In-Reply-To: <CAMknhBFgXdH63_N738qMt6tzK_zWdB-OWvYTWitmBuQNqrN+LQ@mail.gmail.com>


--wewextuk6np2tzoq
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hello David,

On Fri, Apr 05, 2024 at 12:03:56PM -0500, David Lechner wrote:
> On Fri, Apr 5, 2024 at 1:23=E2=80=AFAM Uwe Kleine-K=C3=B6nig
> <u.kleine-koenig@pengutronix.de> wrote:
> > On Thu, Apr 04, 2024 at 08:30:22PM -0400, Trevor Gamblin wrote:
>=20
> ...
>=20
> > > 2. Should __pwm_apply() explicitly disallow PWM_POLARITY_INVERSED and
> > > duty_offset together?
> >
> > While there is no technical need for that, a configuration with both
> > PWM_POLARITY_INVERSED and duty_offset > 0 is irritating. So I'd say yes,
> > it should be disallowed. When I created the cdev API I even considered
> > dropping .polarity for lowlevel drivers and convert them all to
> > .duty_offset.
> >
> > Having said that I don't like the addition of .supports_offset to
> > struct pwm_chip, which only signals a new incomplete evolution of the
> > pwm framework. Better adapt all drivers and then assume all of them
> > support it.
>=20
> But not all chips can fully support this feature. I envisioned this
> flag as something consumer drivers would check when they require a
> chip capable of providing a phase offset between two PWM output
> channels. This way, the consumer driver could fail to probe if the PWM
> chip is not up to the task.
>=20
> For example the consumer driver might require two coordinated signals lik=
e this:
>       _                   _
> PWM0 | |_________________| |_________________
>            ___                 ___
> PWM1 _____|   |_______________|   |__________
>=20
> PWM0: duty_offset =3D 0, duty_cycle =3D 1, period =3D 10
> PWM1: duty_offset =3D 2, duty_cycle =3D 2, period =3D 10
>=20
> Do we need to do additional work to support cases like this? Or should
> we just let it fail silently and let it generate incorrect signals if
> someone attempts to use an unsupported hardware configuration?

My vision here is that you can do the following:

	state.duty_offset =3D 0;
	state.duty_cycle =3D 1;
	state.period =3D 10;
	ret =3D pwm_round_state(pwm, &state);
	if (!is_good_enough(state))
		goto err;

This way pwm_apply_* could just continue to work as it does now (i.e.
implement the biggest period not bigger than requested. For that
implement the biggest duty_offset not bigger than requested and for
these values of period and duty_offset implement the biggest duty_cycle
not bigger than requested.)

This has the advantage that the lowlevel driver doesn't need to judge if
the setting it implements is good enough.

To get there, quite some work has to be invested.

Best regards
Uwe

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

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

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

iQEzBAABCgAdFiEEP4GsaTp6HlmJrf7Tj4D7WH0S/k4FAmYQV0IACgkQj4D7WH0S
/k4KhQgApye5alomNROsehvEvK1/wS5Lj9Ak4+5GDtGnadX04W2BVIjid7X3PUoq
T56Cfz402Db10LHyYSimqxXL6g8pr08d5E4u80BNu6hhDHbh4azaqbVZZlUsk6cq
URRSQ+lESFFLRCT/P7kUMZWYtkkTIGFtU6wKBzDC27Jz8GfaIRbI7k4lmqgCChzK
QMT3xRESfs4otfNHol3nEDJRYktq700/5QEXN1IMse8XFw5d2wurC1rrR3F2U8EK
AyZ64WBU8eITxen3DX82SXiUEPXirEVBOlOmVH5bBFcQPoqpHjS+E1fl++whZC6V
4N7T05zPmIf50NIaPdGqwym+lCAEaA==
=EDVO
-----END PGP SIGNATURE-----

--wewextuk6np2tzoq--