Re: [PATCH 2/4] regulator: Add driver for MT6331 PMIC regulators

Mark Brown
2022-05-23T13:43:30Z

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

On Mon, May 23, 2022 at 03:22:39PM +0200, AngeloGioacchino Del Regno wrote:
> Il 23/05/22 15:00, Mark Brown ha scritto:

> > Right, my point here is that it looks awfully like the documentation
> > (this came from documentation I guess?) is including some extra bits
> > that get ignored in the voltage selection field here. That seems like a
> > weird choice somewhere along the line.

> I wish I had a datasheet for these parts...

> All of this comes from analyzing a running device on the downstream 3.4 kernel
> and on understanding the (not really readable) downstream kernel code...
> ..but yes, I agree on the fact that this seems to be a weird choice.

> Ah, besides, I hooked up an oscilloscope to the VCAM_IO and I can see that the
> vreg really does react as expected upon setting the upper bits.. but since it
> still works without, we can safely ignore them, which makes us able to simplify
> the driver (as no custom code for that will be required) and, at the same time,
> avoid seeing a table of values repeated 4 times in a row.

That seems safer in general if we don't know what those bits are doing -
it could be some kind of mode control or something.

> ...so, the regulator will indeed shut itself off and clear either/both the QI_EN
> and/or its relative bit in the enable register... I've also just found hints of
> the latter (enable register being set to 0) downstream, so I'm sure that this is
> indeed right.

That's still not quite the same thing as a status bit, if the regulator
disables itself then it won't try to turn itself back on. Note that the
core will fall back on using the enable state if there's no status op so
there's no need for this logic, you can just omit the status op and the
behaviour will be the same.

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