linux-kernel - Re: [PATCH] regulator: bd718x7: Ensure SNVS power state is used as requested

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: <52221c62-689c-44d2-b65d-07a5301090b3@gmail.com>
Date: Fri, 2 May 2025 08:24:51 +0300
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Esben Haabendal <esben@...nix.com>, Liam Girdwood <lgirdwood@...il.com>,
 Mark Brown <broonie@...nel.org>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] regulator: bd718x7: Ensure SNVS power state is used as
 requested

Hi Esben,

Oh, it has been a while since I've heard anything from these PMICs :)

On 01/05/2025 17:48, Esben Haabendal wrote:
> With the introduction of the rohm,reset-snvs-powered DT binding [2], the
> PMIC settings were only changed when the new property was not found.
> 
> As mentioned in [1] the default for BD71387 and BD71847 is to switch to
> SNVS power state on watchdog reset.

I suppose you mean READY, not SNVS? Commit seems to state:
"By default only wathcdog reset changes state from poweroff to ready."

> So even with rohm,reset-snvs-powered added to DT, a watchdog reset causes
> transitions through READY instead of SNVS.

The original idea of the rohm,reset-snvs-powered was not to configure 
the SNVS to be the target. The driver was mostly built to assume that 
the PMIC has been configured by earlier stages like uboot, and configs 
in the driver were mostly introduced to make power rail enable states 
controllable by the software - without risking the rails to be left off. 
Thus, AFAIR, the values set by boot (or other power manager MCUs) 
haven't been overwritten is the "rohm,reset-snvs-powered" has been found.

Configuring for example the hardware watchdog related stuff at Linux 
driver boot is somewhat late, since watchdog should probably be running 
already - and hangs might happen prior the driver probe.

> And with the default reboot
> method in mxc_restart() is to cause a watchdog reset, we ended up powering
> off the SNVS domains, and thus losing SNVS state such as SNVS RTC and
> LPGPR, on reboots.
> 
> With this change, the rohm,reset-snvs-powered property results in the PMIC
> configuration being modified so POWEROFF transitions to SNVS for all reset
> types, including watchdog reset.

As far as I can say, this change is, in principle, fine. The 
"rohm,reset-snvs-powered" shouldn't be populated in the device-tree, if 
SNVS is not meant to be used. My only worry is that the BD71837, 47 and 
50 have been on the field since 2018 - and I am not at all sure all the 
device-trees are sane... And if we configure the reset to use SNVS 
state, then the software controlled regulators will not turn ON after 
the reset. Fail to mark them in the device-tree and the device will be 
dead until battery is drained or removed.

Is there a way for you to set the "target state" at boot SW? I think 
that should work as the Linux driver won't touch the target state if 
rohm,reset-snvs-powered is set(?)

This is not NACK to the change, this is asking if we had a safer way, 
both for other users and also for you (since I still think these configs 
should be done prior Linux driver probe)...

Yours,
	-- Matti