Re: [PATCH v5 2/7] pwm: pca9685: Support hardware readout - Uwe Kleine-König

From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Thierry Reding <thierry.reding@gmail.com>,
	Clemens Gruber <clemens.gruber@pqgruber.com>,
	Sven Van Asbroeck <thesven73@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"linux-pwm@vger.kernel.org" <linux-pwm@vger.kernel.org>
Subject: Re: [PATCH v5 2/7] pwm: pca9685: Support hardware readout
Date: Mon, 22 Mar 2021 12:22:54 +0100	[thread overview]
Message-ID: <20210322112254.5mjkajkq3wnhgnd5@pengutronix.de> (raw)
In-Reply-To: <CAHp75Ve2FFEMsAv8S18bUDFsH2UkiQ5UvgcRtZ=j30syQtEirw@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2285 bytes --]

Hello Andy,

On Mon, Mar 22, 2021 at 11:38:40AM +0200, Andy Shevchenko wrote:
> On Monday, March 22, 2021, Thierry Reding <thierry.reding@gmail.com> wrote:
> > On Fri, Jan 29, 2021 at 09:37:47PM +0100, Clemens Gruber wrote:
> > > Thierry: Would you accept it if we continue to reset the registers in
> > > .probe?
> >
> > Yes, I think it's fine to continue to reset the registers since that's
> > basically what the driver already does. It'd be great if you could
> > follow up with a patch that removes the reset and leaves the hardware in
> > whatever state the bootloader has set up. Then we can take that patch
> > for a ride and see if there are any complains about it breaking. If
> > there are we can always try to fix them, but as a last resort we can
> > also revert, which then may be something we have to live with. But I
> > think we should at least try to make this consistent with how other
> > drivers do this so that people don't stumble over this particular
> > driver's
> 
> I guess we may miss (a PCB / silicon design flaw or warm boot case) when
> boot loader left device completely untouched and device either in wrong
> state because if failed reset (saw this on PCA9555 which has a
> corresponding errata), or simply we have done a warm reset of the system.
> So, we also have to understand how to properly exit.

I don't think that not resetting is a real problem. My argumentation
goes as follows:

When the PWM driver is loaded and the PWM configuration is invalid, it
was already invalid for the time between power up (or warm start) and
PWM driver load time. Then it doesn't really hurt to keep the PWM
in this invalid state for a little moment longer until the consumer of
the PWM becomes active.

Together with the use cases where not resetting is the right thing to
do, I'm convinced not resetting is the better strategy.

> Another point, CCF has a bit “is critical”, and u guess PWM may get the
> same and make the all assumptions much easier.

So I think complicating the PWM framework for this isn't the right thing
to do.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]