[PATCH] realtek: do not hardcode restart handler and enable gpio-restart

Sander Vanheule sander at svanheule.net
Tue Oct 5 13:59:37 PDT 2021


Hi Paul,

On Tue, 2021-10-05 at 16:35 +0300, Paul Fertser wrote:
> Most boards supported by this target have a dedicated GPIO line
> connected to SoC's /RESET to allow full clean reliable reboot.
> 
> Use regular kernel notifier methods for default restart routines to
> allow higher-priority handlers (such as gpio-restart) to override it.
> 
> Signed-off-by: Paul Fertser <fercerpav at gmail.com>

This looks like a good improvement to the current restart handling, I think. However, I
also suspect that this logic should actually go into in a separate reset driver, and a
clock driver to handle the PLL. The reset/PLL registers are in the switch-core register
space and we don't have any mainline drivers there yet, so I don't know how we're supposed
to access the jumble of functionality there. As far as I'm concerned though, this can all
stay together in one place for now.

One alternative could be to create some extra driver(s) in OpenWrt already, to get rid of
setup.c (and prom.c at some later point) and work towards the upstream base for
RTL838x/RTL839x. Anyone else with an opinion on this?

I also see that you're removing the _machine_halt function, while this isn't mentioned in
the commit message. I had already removed _machine_halt in f4b687d1f053 ("realtek: use
kernel defined halt"), but apparently it quietly got reintroduced in 8faffa00cb6b
("realtek: add support for the RTL9300 timer").

> ---
>  target/linux/realtek/config-5.10              |  1 +
>  .../files-5.10/arch/mips/rtl838x/setup.c      | 75 ++++++++++++-------
>  2 files changed, 48 insertions(+), 28 deletions(-)
> 

[...]

>  
>  static void __init rtl838x_setup(void)
>  {
> -       pr_info("Registering _machine_restart\n");
> -       _machine_restart = rtl838x_restart;
> -       _machine_halt = rtl838x_halt;
> +       pr_info("Registering rtl838x_restart_notify\n");

I don't think it's very useful to log this, not at the INFO level anyway. Maybe just drop
this, and the ones for the other SoC families.

Best,
Sander






More information about the openwrt-devel mailing list