Received-SPF: pass (google.com: domain of linux-kernel+bounces-133381-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240405003025.739603-1-tgamblin@baylibre.com> <fhrderkiwqzc7fw2hptz2la3aco72wlossqbrr4m42qllcpcpw@vhxflah2ex2a>
In-Reply-To: <fhrderkiwqzc7fw2hptz2la3aco72wlossqbrr4m42qllcpcpw@vhxflah2ex2a>
From: David Lechner <dlechner@baylibre.com>
Date: Fri, 5 Apr 2024 12:03:56 -0500
Message-ID: <CAMknhBFgXdH63_N738qMt6tzK_zWdB-OWvYTWitmBuQNqrN+LQ@mail.gmail.com>
Subject: Re: [RFC PATCH 0/3] pwm: add support for duty_offset
To: =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
Cc: Trevor Gamblin <tgamblin@baylibre.com>, linux-pwm@vger.kernel.org, 
	linux-kernel@vger.kernel.org, michael.hennerich@analog.com, 
	nuno.sa@analog.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

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:

..

> > 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.

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.

For example the consumer driver might require two coordinated signals like =
this:
      _                   _
PWM0 | |_________________| |_________________
           ___                 ___
PWM1 _____|   |_______________|   |__________

PWM0: duty_offset =3D 0, duty_cycle =3D 1, period =3D 10
PWM1: duty_offset =3D 2, duty_cycle =3D 2, period =3D 10

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?