Re: [PATCH v5 09/18] arm64: dts: qcom: qrb5165-rb5: model the PMU of the QCA6391

Mark Brown
2024-02-19T19:59:56Z

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

On Mon, Feb 19, 2024 at 07:48:20PM +0100, Bartosz Golaszewski wrote:
> On Mon, Feb 19, 2024 at 7:03 PM Mark Brown <broonie@kernel.org> wrote:
> > On Fri, Feb 16, 2024 at 09:32:06PM +0100, Bartosz Golaszewski wrote:

> > > + vreg_pmu_aon_0p59: ldo1 {
> > > + regulator-name = "vreg_pmu_aon_0p59";
> > > + regulator-min-microvolt = <540000>;
> > > + regulator-max-microvolt = <840000>;
> > > + };

> > That's a *very* wide voltage range for a supply that's got a name ending
> > in _0_p59 which sounds a lot like it should be fixed at 0.59V.
> > Similarly for a bunch of the other supplies, and I'm not seeing any
> > evidence that the consumers do any voltage changes here? There doesn't
> > appear to be any logic here, I'm not convinced these are validated or
> > safe constraints.

> No, the users don't request any regulators (or rather: software
> representations thereof) because - as per the cover letter - no
> regulators are created by the PMU driver. This is what is physically
> on the board - as the schematics and the datasheet define it. I took

The above makes no sense. How can constraints be "what is physically on
the board", particularly variable constrants when there isn't even a
consumer? What values are you taking from which documentation?

The cover letter and binding both claimed (buried after large amounts of
changelog) that these PMUs were exposing regulators to consumers and the
DTS puports to do exactly that...

> the values from the docs verbatim. In C, we create a power sequencing
> provider which doesn't use the regulator framework at all.

For something that doesn't use the regulator framework at all what
appears to be a provider in patch 16 ("power: pwrseq: add a driver for
the QCA6390 PMU module") seems to have a lot of regualtor API calls?

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