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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YouPdNrBS2xwWwlI@sirena.org.uk>
Date:   Mon, 23 May 2022 14:43:16 +0100
From:   Mark Brown <broonie@...nel.org>
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
Cc:     lgirdwood@...il.com, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, matthias.bgg@...il.com,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH 2/4] regulator: Add driver for MT6331 PMIC regulators

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.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)