Received-SPF: pass (google.com: domain of linux-kernel+bounces-9676-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Fri, 22 Dec 2023 11:53:44 +0000
From: Mark Brown <broonie@kernel.org>
To: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= <u.kleine-koenig@pengutronix.de>
Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	lgirdwood@gmail.com, linux-pwm@vger.kernel.org,
	linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org,
	Thierry Reding <thierry.reding@gmail.com>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Dmitry Rokosov <ddrokosov@sberdevices.ru>
Subject: Re: [RFC PATCH v1] regulator: pwm-regulator: Fix continuous
 get_voltage for disabled PWM
Message-ID: <54905287-855d-47b4-880f-bfb1b8b1f49d@sirena.org.uk>
References: <20231221211222.1380658-1-martin.blumenstingl@googlemail.com>
 <0c99b575-5cf2-4bd6-8cfd-af19f5fd58da@sirena.org.uk>
 <jx2vspy4cfuidf3mwmje675igspxdpgrenw5jz35ud5bm65klc@s4ovjad4fusg>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="KJDHjVYfXsFaJRDn"
Content-Disposition: inline
In-Reply-To: <jx2vspy4cfuidf3mwmje675igspxdpgrenw5jz35ud5bm65klc@s4ovjad4fusg>


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

On Thu, Dec 21, 2023 at 11:07:41PM +0100, Uwe Kleine-K=F6nig wrote:
> On Thu, Dec 21, 2023 at 09:45:49PM +0000, Mark Brown wrote:

> > > -       voltage =3D pwm_get_relative_duty_cycle(&pstate, duty_unit);
> > > +       if (pstate.enabled)
> > > +               voltage =3D pwm_get_relative_duty_cycle(&pstate, duty=
_unit);
> > > +       else if (max_uV_duty < min_uV_duty)
> > > +               voltage =3D max_uV_duty;
> > > +       else
> > > +               voltage =3D min_uV_duty;

> > AFAICT this means that enabling the PWM changes the voltage read back
> > which isn't what we expect (other than a change from 0 to target) and is
> > likely to cause issues.  get_voltage() should not change after an
> > enable(), and indeed I'm unclear how this change works?  I'd expect a
> > change in the init_state() function, possibly one that programs the PWM
> > to reflect the actual hardware state but I'm not 100% confident on that
> > without digging into the PWM API more.

> What is your question here? Looking at pwm_regulator_set_voltage() I
> think this lacks a

> 	pstate.enabled =3D true;

> which might also fix Martin's problem?

That's not really a question, it's a statement - I don't see how the
change works at all and as it stands it introduces a problem with the
behaviour when the regulator is enabled.

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

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

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmWFeMcACgkQJNaLcl1U
h9DDmAf+MtgUxQLU77WJx9BvRbmHOIj3BusmZ5Vz4wKLhcxHaZ6kCJ6cqgwBBMv0
HLih1IgKLkKVBYuFGW3v3I2a8KjNglIqo9zDArCmFI16cB7zJ/XB7/oOQoBipnxd
ttQtIshQIlBeLbxFQcdK9iecfoyYw4uxVoDdmCwo4OKvTIGYz8DEY2ahHEZdDl20
BpfrAA9JeECG/IuFn3SbSlrKzJ5uNwnctynsPfTOMVqKh7bD6kK2OpVx7W2huR7G
Gsd8LXK2UGABOa5zc4dC5pe4pG2aqRbwRHoUbHOlC6N1ZF0NNP4cmDxQ8YnJZZG3
sQafS5nyBfCGJlWOmwecR7JkXeEFhg==
=Hl8j
-----END PGP SIGNATURE-----

--KJDHjVYfXsFaJRDn--