Message-ID: <2577051.irdbgypaU6@workhorse>
Date: Tue, 27 May 2025 13:18:20 +0200
From: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
 Quentin Schulz <foss+kernel@...il.net>, Lee Jones <lee@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>,
 Sebastian Reichel <sebastian.reichel@...labora.com>,
 linux-rockchip@...ts.infradead.org
Cc: Lukasz Czechowski <lukasz.czechowski@...umatec.com>,
 Daniel Semkowicz <dse@...umatec.com>, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 linux-kernel@...r.kernel.org, Quentin Schulz <quentin.schulz@...rry.de>
Subject:
 Re: [PATCH 1/4] dt-bindings: mfd: rk806: allow to customize PMIC reset method

Hi Quentin,

On Tuesday, 27 May 2025 11:26:49 Central European Summer Time Quentin Schulz wrote:
> Hi Krzysztof,
> 
> On 5/27/25 11:08 AM, Krzysztof Kozlowski wrote:
> > On 27/05/2025 10:48, Quentin Schulz wrote:
> [...]
> >>
> >> likely a purpose to it. Especially if they also change the
> >> silicon-default in their own downstream fork AND provide you with a way
> >> to change their new default from Device Tree.
> >>
> >> We can hardcode the change in the driver without using DT, but I wager
> >> we're going to see a revert in a few releases because it broke some
> >> devices. It may break in subtle ways as well, for example our boards
> >> seem to be working just fine except that because the PMIC doesn't
> >> entirely reset the power rails, our companion microcontroller doesn't
> >> detect the reboot.
> >>
> >> If it's deemed a SW policy by the DT binding people, is there a way to
> >> customize this without having it hardcoded to the same value for all
> >> users of RK806 and without relying on module params?
> > 
> > sysfs, reboot mode etc. I don't know what is the right here, because you
> > did not explain the actual hardware issue being fixed here. You only
> > described that bootloader does something, so you want to write something
> > else there.
> > 
> 
> We have a companion microcontroller on the PCB of both products which 
> needs to detect if the board was reset. When the board is reset, the MCU 
> FW does a few things, like essentially resetting its internal logic such 
> as the PWM controller (so the beeper stops beeping), watchdogs and 
> reinit most user-exposed registers so that it's like "fresh" out of 
> reset (even though it actually wasn't reset since it's continuously 
> powered, not from the PMIC).
> 
> To detect a reboot, it senses one of the power rails (DCDC8; vcc_3v3_s3 
> on our boards) from the PMIC. This power rail is only "restarted" when 
> RST_FUN is set to 0 ("restart PMU" mode) according to our experiments.
> 
> I assume it is possible other boards do not want this (all?) power rail 
> to be quickly interrupted when rebooting? But that I do not know.

I agree that this sounds like a pretty big change in behavior, yes. I am
somewhat suspicious of any silent mainline difference from silicon defaults
as being the result of cargo-culting from downstream hacks to make things
work, and are unresolved technical debt in need of cleanup.

On the RK3576 board I'm currently working on, where an RK806 is used as
well, then DCDC8 cutting out would wreak havoc on warm reboots I'd wager
as it's used for a lot of 1.8V IO voltage supply things, including one
instance where the DCDC8 rail going low would feed into a downstream
regulator that's being kept enabled as long as a different supply is on.

If you don't want to deal with DT bindings people (sysfs for reset
behaviour? What?) a workaround for this could be to add the necessary
register write to your bootloader's (probably u-boot?) board init code.

I do think however that "what does this board hardware expect to happen to
power rails on reset" is a pretty strongly board specific non-enumerable
hardware difference that belongs in DT as a declarative property, but
perhaps in a different form than the bare register contents for this, so
that it can hopefully be used as a more generic (even if vendor) property
for future PMICs going forward. Think regulator-always-on but for this
specific case.

> 
> Cheers,
> Quentin
> 

Kind regards,
Nicolas Frattaroli

Powered by blists - more mailing lists