Re: [PATCH/RFC 0/3] pmdomain: renesas: rmobile-sysc: Remove serial console handling

From: Tomi Valkeinen
Date: Wed Jun 05 2024 - 06:41:59 EST


Hi Ulf,

On 05/06/2024 12:34, Ulf Hansson wrote:
+ Tomi

On Mon, 27 May 2024 at 14:41, Geert Uytterhoeven
<geert+renesas@xxxxxxxxx> wrote:

Hi all,

Since commit a47cf07f60dcb02d ("serial: core: Call
device_set_awake_path() for console port"), the serial driver properly
handles the case where the serial console is part of the awake path, and
it looked like we could start removing special serial console handling
from PM Domain drivers like the R-Mobile SYSC PM Domain driver.
Unfortunately the devil is in the details, as usual...

Earlycon relies on the serial port to be initialized by the firmware
and/or bootloader. Linux is not aware of any hardware dependencies that
must be met to keep the port working, and thus cannot guarantee they
stay met, until the full serial driver takes over.

E.g. all unused clocks and unused PM Domains are disabled in a late
initcall. As this happens after the full serial driver has taken over,
the serial port's clock and/or PM Domain are no longer deemed unused,
and this is typically not a problem.

However, if the serial port's clock or PM Domain is shared with another
device, and that other device is runtime-suspended before the full
serial driver has probed, the serial port's clock and/or PM Domain will
be disabled inadvertently. Any subsequent serial console output will
cause a crash or system lock-up. E.g. on R/SH-Mobile SoCs, the serial
ports share their PM Domain with several other I/O devices. After the
use of pwm (Armadillo-800-EVA) or i2c (KZM-A9-GT) during early boot,
before the full serial driver takes over, the PM Domain containing the
early serial port is powered down, causing a lock-up when booted with
"earlycon".

Hi Geert,

Thanks for the detailed description of the problem! As pointed out in
regards to another similar recent patch [1], this is indeed a generic
problem, not limited to the serial console handling.

At Linaro Connect a few weeks ago I followed up with Saravana from the
earlier discussions at LPC last fall. We now have a generic solution
for genpd drafted on plain paper, based on fw_devlink and the
->sync_state() callback. I am currently working on the genpd series,
while Saravana will re-spin the series (can't find the link to the
last version) for the clock framework. Ideally, we want these things
to work in a very similar way.

That said, allow me to post the series for genpd in a week or two to
see if it can solve your problem too, for the serial console.

Both the genpd and the clock solutions will make suppliers depend on all their consumers to be probed, right?

I think it is a solution, and should be worked on, but it has the drawback that suppliers that have consumers that will possibly never be probed, will also never be able to turn off unused resources.

This was specifically the case with the TI ti-sci pmdomain case I was looking at: the genpd driver (ti_sci_pm_domains.c) provides a lot of genpds for totally unrelated devices, and so if, e.g., you don't have or don't want to load a driver for the GPU, all PDs are affected.

Even here the solutions you mention will help: instead of things getting broken because genpds get turned off while they are actually in use, the genpds will be kept enabled, thus fixing the breakage. Unfortunately, they'll be kept enabled forever.

I've been ill for quite a while so I haven't had the chance to look at this more, but before that I was hacking around a bit with something I named .partial_sync_state(). .sync_state() gets called when all the consumers have probed, but .partial_sync_state() gets called when _a_ consumer has been probed.

For the .sync_state() things are easy for the driver, as it knows everything related has been probed, but for .partial_sync_state() the driver needs to track resources internally. .partial_sync_state() will tell the driver that a consumer device has probed, the driver can then find out which specific resources (genpds in my case) that consumer refers to, and then... Well, that's how far I got with my hacks =).

So, I don't know if this .partial_sync_state() can even work, but I think we do need something more on top of the .sync_state().

Tomi


Kind regards
Uffe

[1]
https://lore.kernel.org/linux-arm-kernel/CAPDyKFqShuq98qV5nSPzSqwLLUZ7LxLvp1eihGRBkU4qUKdWwQ@xxxxxxxxxxxxxx/


This RFC patch series aims to provide a mechanism for handling this, and
to fix it for the PM Domain case:
1. The first patch provides a mechanism to let the clock and/or PM
Domain subsystem or drivers handle this, by exporting the clock and
PM Domain dependencies for the serial port, as available in the
system's device tree,
2. The second patch introduces a new flag to handle a PM domain that
must be kept powered-on during early boot, and by setting this flag
if the PM Domain contains the serial console (originally I handled
this inside rmobile-sysc, but it turned out to be easy to
generalize this to other platforms in the core PM Domain code).
3. The third patch removes the no longer needed special console
handling from the R-Mobile SYSC PM Domain driver.

I did not fix the similar clock issue, as it is more complex (there can
be multiple clocks, and each clock provider can have its own value of
#clock-cells), and I do not need it for Renesas ARM platforms.

I will defer to Sarvana here, but ideally his series for the clock
framework should solve this case too.


This has been tested on the APE6-EVM, Armadillo-800-EVA, and KZM-A9-GT
development boards, with and without earlycon, including s2ram with and
without no_console_suspend.

Notes:
- This should not be needed on RZ/G3S, where each serial port device
has its own PM Domain,
- drivers/clk/imx/clk.c and drivers/pmdomain/imx/scu-pd.c have special
handling for the of_stdout device, but is probably not affected, as
each serial port seems to share its PM Domain only with the serial
port's clock controller.

Thanks for your comments!

Geert Uytterhoeven (3):
earlycon: Export clock and PM Domain info from FDT
pmdomain: core: Avoid earlycon power-down
pmdomain: renesas: rmobile-sysc: Remove serial console handling

drivers/pmdomain/core.c | 24 ++++++++++++++++--
drivers/pmdomain/renesas/rmobile-sysc.c | 33 +------------------------
drivers/tty/serial/earlycon.c | 14 ++++++++++-
include/linux/pm_domain.h | 4 +++
include/linux/serial_core.h | 10 ++++++++
5 files changed, 50 insertions(+), 35 deletions(-)

--

Kind regards
Uffe