[PATCH] realtek: ZyXEL GS1900-48: drop gpio-restart

[Sander Vanheule]

On Sun, 2022-02-20 at 21:13 +0100, Birger Koblitz wrote:
> Hi,
> 
> during development I came across situations where the RTL839x
> SoC's own reset did not always completely reset its state.
> U-Boot was no longer able to boot via tftp afterwards.
> This is the same situation we see on the RTL838x.
> While users will hopefully never mess up the SoC as during
> development, I would prefer a solution that reliably
> resets the device over a solution that avoids a warning.

If you did not see all three warnings on your device, that would mean it's
behaving differently. As Daniel noted, the fact that the third warning is even
emitted, means that gpio-restart failed to restart the device and is returning
from its handler. That either means the GPIO is incorrect (although it matches
the data from the stock firmware), or the RTL8231's output cannot be set to the
intended value.

At least for my device it is not just about avoiding an ugly warning. gpio-
restart effectively does nothing, so there's no point in setting it up.

> At least we should have a comment in, that states that
> once this is fixed in libgpiod this should be the way
> to reset the device. And we should look into fixing libgpiod.

There will be a good reason that gpio-restart is using gpiod_set_value() instead
of gpiod_set_value_cansleep(). I don't know that much about the kernel, but
since the system is being torn down, it is likely certain things cannot be used
anymore. Even if it would work with the current driver, once the RTL8231 is
controlled through an MDIO bus (from a kernel perspective), things might change.

> 
> Cheers,
>    Birger
> 
> 
> On 20/02/2022 19:50, Sander Vanheule wrote:
> > GPIO 5 on the RTL8231 can be used to reset the system, but gpio-restart
> > uses gpiod_set_value. This triggers a kernel warning and backtrace for
> > GPIO pins that can sleep, such as the RTL8231's. Two warnings are
> > emitted by libgpiod, and a third warning by gpio-restart itself after it
> > fails to restart the system:
> > 
> > [  106.654008] ------------[ cut here ]------------
> > [  106.659240] WARNING: CPU: 0 PID: 4279 at drivers/gpio/gpiolib.c:3098
> > gpiod_set_value+0x7c/0x108
> >                 [ Stack dump and call trace ]
> > [  106.826218] ---[ end trace d1de50b401f5a153 ]---
> > [  106.962992] ------------[ cut here ]------------
> > [  106.968208] WARNING: CPU: 0 PID: 4279 at drivers/gpio/gpiolib.c:3098
> > gpiod_set_value+0x7c/0x108
> >                 [ Stack dump and call trace ]
> > [  107.136718] ---[ end trace d1de50b401f5a154 ]---
> > [  111.087092] ------------[ cut here ]------------
> > [  111.092271] WARNING: CPU: 0 PID: 4279 at drivers/power/reset/gpio-
> > restart.c:46 gpio_restart_notify+0xc0/0xdc
> >                 [ Stack dump and call trace ]
> > [  111.256629] ---[ end trace d1de50b401f5a155 ]---
> > 
> > By removing gpio-restart from this device, we skip the restart-by-GPIO
> > attempt and rely only on the watchdog for restarts, which is already the
> > de facto behaviour.
> > 
> > Signed-off-by: Sander Vanheule <sander at svanheule.net>
> > ---
> >   target/linux/realtek/dts-5.10/rtl8393_zyxel_gs1900-48.dts | 5 -----
> >   1 file changed, 5 deletions(-)
> > 
> > diff --git a/target/linux/realtek/dts-5.10/rtl8393_zyxel_gs1900-48.dts
> > b/target/linux/realtek/dts-5.10/rtl8393_zyxel_gs1900-48.dts
> > index dd392c5a9beb..e0e79068d2bc 100644
> > --- a/target/linux/realtek/dts-5.10/rtl8393_zyxel_gs1900-48.dts
> > +++ b/target/linux/realtek/dts-5.10/rtl8393_zyxel_gs1900-48.dts
> > @@ -39,11 +39,6 @@
> >                 gpio-controller;
> >         };
> >   
> > -       gpio-restart {
> > -               compatible = "gpio-restart";
> > -               gpios = <&gpio1 5 GPIO_ACTIVE_LOW>;
> > -       };
> > -
> >         keys {
> >                 compatible = "gpio-keys-polled";
> >                 poll-interval = <20>;