Received-SPF: pass (google.com: domain of linux-kernel+bounces-216179-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Sun, 16 Jun 2024 09:47:28 +0200
From: Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= <u.kleine-koenig@baylibre.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>, 
	linux-kernel@vger.kernel.org, linux-pwm@vger.kernel.org
Subject: Re: [RFC PATCH] regulator: pwm-regulator: Make assumptions about
 disabled PWM consistent
Message-ID: <htgmpafra7fehqfbsusiraed7eny5linma3hrvv3nkuqp3bmuw@zisvojdwymee>
References: <20240610104600.458308-2-u.kleine-koenig@baylibre.com>
 <CAFBinCB+S1wOpD-fCbcTORqXSV00Sd7-1GHUKY+rO859_dkhOA@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="ncc5iil27kc3ytrv"
Content-Disposition: inline
In-Reply-To: <CAFBinCB+S1wOpD-fCbcTORqXSV00Sd7-1GHUKY+rO859_dkhOA@mail.gmail.com>


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

Hello Martin,

On Sun, Jun 16, 2024 at 01:10:32AM +0200, Martin Blumenstingl wrote:
> On Mon, Jun 10, 2024 at 12:46=E2=80=AFPM Uwe Kleine-K=C3=B6nig
> <u.kleine-koenig@baylibre.com> wrote:
> [...]
> > this is my suggestion to fix the concerns I expressed in
> > https://lore.kernel.org/all/hf24mrplr76xtziztkqiscowkh2f3vmceuarecqcwnr=
6udggs6@uiof2rvvqq5v/
> > .
> >
> > It's only compile tested as I don't have a machine with a pwm-regulator.
> > So getting test feedback before applying it would be great.
> Unfortunately this approach breaks my Odroid-C1 board again with the
> same symptoms as before the mentioned commits: random memory
> corruption, preventing the board from booting to userspace.
>=20
> The cause also seems to be the same as before my commits:
> - period (3666ns) and duty cycle (3333ns) in the hardware registers
> the PWM controller when Linux boots, but the PWM output is disabled
> - with a duty cycle of 0 or PWM output being disabled the output of
> the voltage regulator is 1140mV, which is the only allowed voltage for
> that rail (even though the regulator can achieve other voltages)
> - pwm_regulator_enable() calls pwm_enable() which simply takes the
> period and and duty cycle that was read back from the hardware and
> enables the output, resulting in undervolting of a main voltage rail
> of the SoC

Ah, indeed. pwm_enable() looks so innocent, but in fact the details are
difficult. One more reason to drop that caching of parameters in the pwm
core.

> I hope that this (especially the last item) also clarifies the
> question you had in the linked mail on whether updating
> pwm_regulator_init_state() would help/work.
>=20
> Regarding your alternative and preferred approach from the other mail:
> > Alternatively (and IMHO nicer) just keep the pwm_state around and don't
> > use the (mis) feature of the PWM core that pwm_get_state only returns
> > the last set state.
> I tried this to see if it would work also for my Odroid-C1 board and
> I'm happy to report it does - see the attached diff.
> In case you are happy with this approach I can submit it as a proper patc=
h.

Yes, I like it. From a quick glance, I only wonder about the dropped
error message in pwm_regulator_set_voltage(). Probably it's right that
this function is silent, but then this is orthogonal to the patch
we're discussing and should go in a separate patch.

Thanks for your valuable cooperation.

Best regards
Uwe

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

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

iQEzBAABCgAdFiEEP4GsaTp6HlmJrf7Tj4D7WH0S/k4FAmZumI4ACgkQj4D7WH0S
/k4Txwf/f9Fb540sNFCp2PnXCtk5/OiMtk1Vctv3I6yS+cRb9ifz+NIvu7doQHhe
BLS+bk2Hayvr24rts1jVJ70ElCM8MwthQE6xMQExMzBJmjuXrQuKoKKnicKTMciz
bw6ByBnaeaOczYuWQ8aihPTAs4MD6HJ/04hVNX9JiSJ8h7jNd3AnOPMTTI/C6ttS
HoDVPMCKlY+rMoIkfvgv5rniQJDpLkCN1aqYL91WK0YW3/eu5o3tfgNxMjgYu25K
EqVzjPmyh+s1VCJbXFOuSMkJFxq1jgHIjQtfzyM84KuvLD6+6nEmlB3sYrOFS+KU
QZ6L4ciDfY19HFbY+BEN8TOK8vtulw==
=WQXL
-----END PGP SIGNATURE-----

--ncc5iil27kc3ytrv--