regression: ASUS C201 doesnt power off since "mfd: rk808: Convert to device managed resources"

[Urja]

Hi,

On Sunday, July 21st, 2024 at 23.56, Sebastian Reichel <sebastian.reichel at collabora.com> wrote:

> As a first step to understand what is going on can you check if
> rk808_power_off() is being called at all during shutdown (i.e.
> using ftrace or add a simple log print at the start of the
> function)?

I added a printk (well, a pr_emerg(), but anyways) to rk808_power_off, set up the system for serial output (that's why I hadnt tried this before, needing to open the machine), and tried a poweroff.

Instead of my printk, i got:
"reboot: Power off not available: System halted instead".

This led me to poke around the kernel code (note: this is in linux-6.10, because I thought it'd be easiest that way), and what I've figured out is:

rk8xx-core.c registers rk808_power_off in the SYS_OFF_MODE_POWER_OFF_PREPARE phase - not the final POWER_OFF, from where the old-style pm_power_off was called.

Code in linux/reboot.c looks at the POWER_OFF registered handler list and finds it empty, and doesnt even try a poweroff - changes the cmd to a halt.

I'd guess the rk806 systems have some other handler registered for the final power off (maybe firmware-based?), and thus they attempt a poweroff, but the C201 doesnt (it has no runtime firmware services).

Just changing the devm_register_sys_off_handler() call to use SYS_OFF_MODE_POWER_OFF seems to work, the machine does power off.

I grepped for other handlers using POWER_OFF_PREPARE, and found one relevant (plus the ACPI one) in odroid-go-ultra-poweroff.c, with a comment about how regmap_update_bits might sleep - which I assume is the reason rk808_power_off was placed in that phase, too.

So placing it in the final phase might be technically wrong, but it did work just fine before the conversion from pm_power_off (because that's how pm_power_off is called), and given that the machine powers off, this seems kinda irrelevant (as long as the I2C write is fired out, nothing after it matters, i would think.)

But this might be different on rk806 (or other rk8xx), i dont know.

Maybe we should register tha handler in both phases... or register a dummy handler for the final stage. Up to you, I'd just like some kind of an official fix <3, and not to carry an another patch (even if it is just one line).

--
Urja Rannikko