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] [day] [month] [year] [list]

Date:   Mon, 23 May 2022 15:53:22 +0200
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     Mark Brown <broonie@...nel.org>
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

Il 23/05/22 15:43, Mark Brown ha scritto:
> 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.

Ok then, I'll simply remove that!

Thanks,
Angelo