| Message ID | 20220220185038.943824-1-sander@svanheule.net |
|---|---|
| State | Under Review |
| Delegated to: | Sander Vanheule |
| Headers | show |
| Series | realtek: ZyXEL GS1900-48: drop gpio-restart | expand |
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. 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. 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@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>;
On Sun, Feb 20, 2022 at 09:13:24PM +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. > 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. In the case shown by Sander, gpio-restart isn't used and/or doesn't have the desired effect of restarting the device anyway, resulting in the next lower priority restart handler (in this case the watchdog) to carry out the restart. Ie. what is there right now is a no-op, at least on the device which Sander has used to capture the log shown below. > 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@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>; > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
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@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>;
Hi Daniel, I understand that. My preferred solution would be to leave this in but comment the node out so that we can use it in the future once we either have made the 8231 gpio driver do busy waiting and avoid anything that sleeps, or fix libgpiod. The intention is to have every switch use a gpio for reset. Some switches currently don't work at all because of functionality no longer in the upstreamed spi-nor driver and no reset gpio being known: https://forum.openwrt.org/t/support-for-rtl838x-based-managed-switches/57875/784 On others users have to do a manual switch on/off because the network is not properly reset by a software reset: https://forum.openwrt.org/t/support-for-rtl838x-based-managed-switches/57875/948 https://forum.openwrt.org/t/support-for-rtl838x-based-managed-switches/57875/782 The 838x platform was definitely built for an external reset to be used, and I have good reason to believe the 839x platform does not entirely fix this. So the watchdog should be avoided if there is another way to reset the device. Cheers, Birger On 20/02/2022 21:25, Daniel Golle wrote: > On Sun, Feb 20, 2022 at 09:13:24PM +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. >> 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. > > In the case shown by Sander, gpio-restart isn't used and/or > doesn't have the desired effect of restarting the device anyway, > resulting in the next lower priority restart handler (in this case > the watchdog) to carry out the restart. > Ie. what is there right now is a no-op, at least on the device which > Sander has used to capture the log shown below. > > >> 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@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>; >> >> _______________________________________________ >> openwrt-devel mailing list >> openwrt-devel@lists.openwrt.org >> https://lists.openwrt.org/mailman/listinfo/openwrt-devel > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel >
Hi, On 21/02/2022 10:04, Sander Vanheule wrote: > 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. For me that would be a reason not to do it that way, because we will not be able to support some devices (all that use an RTL8231 GPIO for the hardware reset). At least we would not be able to reliably reset them and they would either not reboot (because of the 3/4 byte issue) or a messed up network configuration. Cheers, birger
On Mon, Feb 21, 2022 at 10:04:13AM +0100, Sander Vanheule wrote: > 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. > Thinking about it another night I think you are both right: The GPIO does trigger a reliable reset, it's just **not fast enough**, hence the warning about that it can sleep. > > 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 The reason is simple: The restart code doesn't have any timeout, it expects an immediate action and if it even reaches the next instruction, that would mean the previous reset handler has failed. > 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. Yes, it may not even be possible to have it use *_cansleep() and then wait a defined amount of time, because that would mean spawning another thread/worker/whatever for the timeout... Not using bit-banging to control the RTL8231 could solve that. MDIO access code typically just actively loop-waits until the busy-bit of the bus control register has cleared -- no sleep()'ing involved there. > > > > > 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@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>;
On Mon, 2022-02-21 at 12:09 +0000, Daniel Golle wrote: > On Mon, Feb 21, 2022 at 10:04:13AM +0100, Sander Vanheule wrote: > > 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. > > > > Thinking about it another night I think you are both right: > The GPIO does trigger a reliable reset, it's just **not fast enough**, > hence the warning about that it can sleep. > I just checked with my multimeter, and while the GPIO5 on the RTL8231 does go high/low when I set the output high/low from Linux, my device certainly doesn't reset. The other GPIO lines on the chip do work, since SFP modules are correctly detected. Birger, just to be sure, can you confirm that your device does reset with GPIO5 on the RTL8231? > > > > 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 > > The reason is simple: > The restart code doesn't have any timeout, it expects an immediate > action and if it even reaches the next instruction, that would mean > the previous reset handler has failed. Not sure what you mean here, gpio-restart does have a few built-in delays. With the used default values this should give the following waveform (voltages inverted with GPIO_ACTIVE_LOW): |< 100ms >|< 100 ms >|< 3000 ms >|< Restart failed _____|_________| |_______________|__ [ active ] _____X \__________/ [inactive] ^ Restart should already occur here There is some debouncing circuitry between the hard reset button and the trace leading to the SoC, so maybe 100ms wouldn't be enough. Even if setting the GPIO is slow (or happens asynchronously), I think 3s should be enough to allow the correct level to be asserted. > > > 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. > > Yes, it may not even be possible to have it use *_cansleep() and then > wait a defined amount of time, because that would mean spawning another > thread/worker/whatever for the timeout... > > Not using bit-banging to control the RTL8231 could solve that. > MDIO access code typically just actively loop-waits until the busy-bit > of the bus control register has cleared -- no sleep()'ing involved > there. The current driver for the RTL82131 doesn't use bit-banging, but talks to the AUX-MDIO control registers directly to perform register reads and writes. To wait for MDIO transfers to finish, udelay() is used, so there's already no sleep()'ing at the moment. Best, Sander > > > > > > > > > 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@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>;
On Mon, Feb 21, 2022 at 08:33:06PM +0100, Sander Vanheule wrote: > On Mon, 2022-02-21 at 12:09 +0000, Daniel Golle wrote: > > On Mon, Feb 21, 2022 at 10:04:13AM +0100, Sander Vanheule wrote: > > > 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. > > > > > > > Thinking about it another night I think you are both right: > > The GPIO does trigger a reliable reset, it's just **not fast enough**, > > hence the warning about that it can sleep. > > > > I just checked with my multimeter, and while the GPIO5 on the RTL8231 does go high/low > when I set the output high/low from Linux, my device certainly doesn't reset. The other > GPIO lines on the chip do work, since SFP modules are correctly detected. > > Birger, just to be sure, can you confirm that your device does reset with GPIO5 on the > RTL8231? > > > > > > > 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 > > > > The reason is simple: > > The restart code doesn't have any timeout, it expects an immediate > > action and if it even reaches the next instruction, that would mean > > the previous reset handler has failed. > > Not sure what you mean here, gpio-restart does have a few built-in delays. With the used > default values this should give the following waveform (voltages inverted with > GPIO_ACTIVE_LOW): > > |< 100ms >|< 100 ms >|< 3000 ms >|< Restart failed > _____|_________| |_______________|__ [ active ] > _____X \__________/ [inactive] > > ^ Restart should already occur here > > There is some debouncing circuitry between the hard reset button and the trace leading to > the SoC, so maybe 100ms wouldn't be enough. Even if setting the GPIO is slow (or happens > asynchronously), I think 3s should be enough to allow the correct level to be asserted. Absolutely. 3s should be much more than enough. Thank you for investigating this in such great depth! > > > > > > 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. > > > > Yes, it may not even be possible to have it use *_cansleep() and then > > wait a defined amount of time, because that would mean spawning another > > thread/worker/whatever for the timeout... > > > > Not using bit-banging to control the RTL8231 could solve that. > > MDIO access code typically just actively loop-waits until the busy-bit > > of the bus control register has cleared -- no sleep()'ing involved > > there. > > The current driver for the RTL82131 doesn't use bit-banging, but talks to the AUX-MDIO > control registers directly to perform register reads and writes. To wait for MDIO > transfers to finish, udelay() is used, so there's already no sleep()'ing at the moment. Ah ok, so it's more of a RTL83xx+RTL8231 driver at this point ;) Again, thank you for enlightening me! Cheers Daniel > > Best, > Sander > > > > > > > > > > > > > > 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@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>; >
Hi, > > I just checked with my multimeter, and while the GPIO5 on the RTL8231 does go high/low > when I set the output high/low from Linux, my device certainly doesn't reset. The other > GPIO lines on the chip do work, since SFP modules are correctly detected. > > Birger, just to be sure, can you confirm that your device does reset with GPIO5 on the > RTL8231? Yes, it does.There is a warning, but then it reliably resets. That was why I left it in as is. Cheers, Birger
On Mon, 2022-02-21 at 21:23 +0100, Birger Koblitz wrote: > Hi, > > > > I just checked with my multimeter, and while the GPIO5 on the RTL8231 does go high/low > > when I set the output high/low from Linux, my device certainly doesn't reset. The > > other > > GPIO lines on the chip do work, since SFP modules are correctly detected. > > > > Birger, just to be sure, can you confirm that your device does reset with GPIO5 on the > > RTL8231? > > Yes, it does.There is a warning, but then it reliably resets. That was why I left it > in as is. I had another hard look at my board, to check if something may be wrong physically, but I cannot find anything. My device's board looks identical to the pictures on the switch wiki [1], which I think you uploaded earlier. There is some reset logic on the board [2], but I cannot figure out how GPIO5 would be connected to it electrically. Unless I missed a via connecting to that pin on the RTL8231, GPIO5 only appears to lead to TP2. GPIO5/TP2 does not appear to be connected electrically to any part of the circuit next to SW1. I could add a bodge wire to connect TP2 to pad U25:3, but gpio-restart should really work on unmodified hardware. Does my description of the hard reset circuit on the wiki match your board? FWIW, the GPIO description reported by the stock firmware on the XGS1250-12 claims that internal GPIO21 can be used to reset the board, but you noted in the DTS it actually only resets the phy-s. So it wouldn't be the first time the reported board config is partially incorrect. Best, Sander [1] https://svanheule.net/switches/gs1900-48#board_details [2] https://svanheule.net/switches/gs1900-48#hard_reset_circuit
Hi, the information on the external GPIO resetting the board of the Zyxel GS1900-48 comes from the hardware configuration reported by the stock firmware. It says: GS1900# show board [...] ====== Reset ================= Type: GPIO GPIO: EXT_5 [...] Using the rtk gpio commands in u-boot this can be confirmed. Similarly the information on the XGS1250 comes from the output of the "show tech-support" command which shows: **************************** ZYXEL XGS1250-12 **************************** ============================ Board GPIO ============================ Device Pin Direction Default Current Purpose ------- ---- ---------- -------- -------- -------- INT 0 OUT 0 0 INT 1 IN 1 1 FAN_CONTROL INT 6 IN 1 0 HW_VER_BIT1 INT 7 IN 1 1 HW_VER_BIT0 INT 11 OUT 0 0 FAN_EN INT 15 OUT 0 0 TX_DIS_P11 INT 16 IN 0 0 SFP_Present_P11 INT 17 IN 0 1 SFP_LOS_P11 INT 20 IN 0 0 TX_FAULT_P11 INT 21 OUT 1 1 RESET_PHY INT 22 IN 1 1 RST_BTN_OUT I also tested this in u-boot and toggling that output 21 definitely only resets the PHYs (port leds turn off), not the entire board. Cheers, Birger On 22/02/2022 23:00, Sander Vanheule wrote: > On Mon, 2022-02-21 at 21:23 +0100, Birger Koblitz wrote: >> Hi, >>> >>> I just checked with my multimeter, and while the GPIO5 on the RTL8231 does go high/low >>> when I set the output high/low from Linux, my device certainly doesn't reset. The >>> other >>> GPIO lines on the chip do work, since SFP modules are correctly detected. >>> >>> Birger, just to be sure, can you confirm that your device does reset with GPIO5 on the >>> RTL8231? >> >> Yes, it does.There is a warning, but then it reliably resets. That was why I left it >> in as is. > > I had another hard look at my board, to check if something may be wrong physically, but I > cannot find anything. My device's board looks identical to the pictures on the switch wiki > [1], which I think you uploaded earlier. > > There is some reset logic on the board [2], but I cannot figure out how GPIO5 would be > connected to it electrically. Unless I missed a via connecting to that pin on the RTL8231, > GPIO5 only appears to lead to TP2. GPIO5/TP2 does not appear to be connected electrically > to any part of the circuit next to SW1. I could add a bodge wire to connect TP2 to pad > U25:3, but gpio-restart should really work on unmodified hardware. > > Does my description of the hard reset circuit on the wiki match your board? > > FWIW, the GPIO description reported by the stock firmware on the XGS1250-12 claims that > internal GPIO21 can be used to reset the board, but you noted in the DTS it actually only > resets the phy-s. So it wouldn't be the first time the reported board config is partially > incorrect. > > > Best, > Sander > > [1] https://svanheule.net/switches/gs1900-48#board_details > [2] https://svanheule.net/switches/gs1900-48#hard_reset_circuit >
On Tue, 2022-02-22 at 23:39 +0100, Birger Koblitz wrote: > Hi, > > the information on the external GPIO resetting the board of > the Zyxel GS1900-48 comes from the hardware configuration > reported by the stock firmware. It says: > GS1900# show board > [...] > ====== Reset ================= > Type: GPIO > GPIO: EXT_5 > [...] > Using the rtk gpio commands in u-boot this can be confirmed. Can you list the commands that you used to test this? My bootloader only supports "rtk network ..." and "cst pinSet ...". > On 22/02/2022 23:00, Sander Vanheule wrote: > > On Mon, 2022-02-21 at 21:23 +0100, Birger Koblitz wrote: > > > Hi, > > > > > > > > I just checked with my multimeter, and while the GPIO5 on the RTL8231 does go > > > > high/low > > > > when I set the output high/low from Linux, my device certainly doesn't reset. The > > > > other > > > > GPIO lines on the chip do work, since SFP modules are correctly detected. > > > > > > > > Birger, just to be sure, can you confirm that your device does reset with GPIO5 on > > > > the > > > > RTL8231? > > > > > > Yes, it does.There is a warning, but then it reliably resets. That was why I left it > > > in as is. > > > > I had another hard look at my board, to check if something may be wrong physically, > > but I > > cannot find anything. My device's board looks identical to the pictures on the switch > > wiki > > [1], which I think you uploaded earlier. > > > > There is some reset logic on the board [2], but I cannot figure out how GPIO5 would be > > connected to it electrically. Unless I missed a via connecting to that pin on the > > RTL8231, > > GPIO5 only appears to lead to TP2. GPIO5/TP2 does not appear to be connected > > electrically > > to any part of the circuit next to SW1. I could add a bodge wire to connect TP2 to pad > > U25:3, but gpio-restart should really work on unmodified hardware. > > > > [1] https://svanheule.net/switches/gs1900-48#board_details > > [2] https://svanheule.net/switches/gs1900-48#hard_reset_circuit Having another look at the source code of gpio-restart, the WARNING-s I reported in the patch's commit message occur at the following points of the GPIO output waveform: |< 100ms >|< 100 ms >|< 3000 ms >|< Restart failed _____|_________| |_______________|__ [ active ] _____X \__________/ [inactive] | | | | | | | ^ WARN @ drivers/power/reset/gpio-restart.c:46 | | | | | ^ WARN @ drivers/gpio/gpiolib.c:3098 | ^ WARN @ drivers/gpio/gpiolib.c:3098 | ^ Restart should already occur here If everything is set up correctly, the system should restart before execution reaches the point where a warning can be emitted. If you say that you see a warning (any at all), AFAICT that means gpio-restart is not working. As they say, the proof of the pudding is in the eating, so I soldered a jumper wire between the RTL8231's GPIO5 pin (U38:25) and the line driven by the hard reset button (U25:3) [https://svanheule.net/switches/gs1900-48#hard_reset_circuit]. As expected from the analysis above, this results in a system rebooting without _any_ warning (using an initramfs from yesterday's snapshot builds): root@OpenWrt:/# reboot root@OpenWrt:/# [ 185.092891] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported [ 185.101879] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported [ 185.111835] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported [ 185.120484] rtl83xx_fib4_del: found a route with id 1, nh-id 0 [ 185.127681] rtl83xx-switch switch@1b000000: unknown nexthop, id 0 [ 185.149505] rtl83xx-switch switch@1b000000: unknown nexthop, id 0 [ 185.157262] rtl83xx_fib4_del: found a route with id 2, nh-id 0 [ 185.164418] rtl83xx-switch switch@1b000000: unknown nexthop, id 0 [ 185.173391] rtl83xx_fib4_del: no such gateway: 0.0.0.0 [ 185.225492] device lan01 left promiscuous mode [ 185.230976] switch: port 1(lan01) entered disabled state ... [ 187.735562] device lan50 left promiscuous mode [ 187.741075] switch: port 50(lan50) entered disabled state [ 187.794104] in rtl838x_eth_stop [ 187.797945] rtl838x-eth 1b00a300.ethernet eth0: Link is Down [ 188.329431] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported [ 188.337562] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported [ 188.345649] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported [ 188.353736] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported [ 188.543709] rtl83xx_fib4_del: no such gateway: 0.0.0.0 [ 188.549982] rtl83xx_fib4_del: no such gateway: 0.0.0.0 [ 188.559077] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported [ 188.567226] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported [ 188.576283] rtl83xx_fib4_del: no such gateway: 0.0.0.0 [ 188.582679] rtl83xx_fib4_del: no such gateway: 0.0.0.0 [ 192.871878] reboot: Restarting system U-Boot Version: 2.0.0.59413 (Jul 08 2015 - 10:01:28) CPU: 750MHz DRAM: 128 MB FLASH: 16 MB Model: ZyXEL_GS1900_48 SN: S182L05000541 Best, Sander
Hi Sander, I don't have the GS1900-48 at hand or even my records. I am on vacation and I will only be back in 10 days, we actually got stuck here in quarantine... You can also test the GPIO access in U-Boot using md.l and mw.l using the the indirect access register. I tested this once for the indirect table access registers and that worked. But I looked up when the GS1900-48 was released and it was around since at least 2013. I would be surprised if there are _not_ different versions around. We know that there _are_ actually different version with regards to the SoC. They use at least 3 different processor generations including different work-arounds necessary. Have a look at the forum on issues with reading non-existing PHYs and the port flooding table, which in SoC revisions < D cannot be read and requires a shadow table (not implemented, since we always flood to all ports, because port isolation overrides this anyway). On the warning: At least the initial warning came from the lock debugging I compiled in, which is based on Daniel's standard settings. IIRC, the lengthy warning was about sleeping in the wrong context, maybe there was also something about an issue with lock contention. Cheers, Birger On 24/02/2022 21:19, Sander Vanheule wrote: > On Tue, 2022-02-22 at 23:39 +0100, Birger Koblitz wrote: >> Hi, >> >> the information on the external GPIO resetting the board of >> the Zyxel GS1900-48 comes from the hardware configuration >> reported by the stock firmware. It says: >> GS1900# show board >> [...] >> ====== Reset ================= >> Type: GPIO >> GPIO: EXT_5 >> [...] >> Using the rtk gpio commands in u-boot this can be confirmed. > > Can you list the commands that you used to test this? My bootloader only supports "rtk > network ..." and "cst pinSet ...". > > >> On 22/02/2022 23:00, Sander Vanheule wrote: >>> On Mon, 2022-02-21 at 21:23 +0100, Birger Koblitz wrote: >>>> Hi, >>>>> >>>>> I just checked with my multimeter, and while the GPIO5 on the RTL8231 does go >>>>> high/low >>>>> when I set the output high/low from Linux, my device certainly doesn't reset. The >>>>> other >>>>> GPIO lines on the chip do work, since SFP modules are correctly detected. >>>>> >>>>> Birger, just to be sure, can you confirm that your device does reset with GPIO5 on >>>>> the >>>>> RTL8231? >>>> >>>> Yes, it does.There is a warning, but then it reliably resets. That was why I left it >>>> in as is. >>> >>> I had another hard look at my board, to check if something may be wrong physically, >>> but I >>> cannot find anything. My device's board looks identical to the pictures on the switch >>> wiki >>> [1], which I think you uploaded earlier. >>> >>> There is some reset logic on the board [2], but I cannot figure out how GPIO5 would be >>> connected to it electrically. Unless I missed a via connecting to that pin on the >>> RTL8231, >>> GPIO5 only appears to lead to TP2. GPIO5/TP2 does not appear to be connected >>> electrically >>> to any part of the circuit next to SW1. I could add a bodge wire to connect TP2 to pad >>> U25:3, but gpio-restart should really work on unmodified hardware. >>> >>> [1] https://svanheule.net/switches/gs1900-48#board_details >>> [2] https://svanheule.net/switches/gs1900-48#hard_reset_circuit > > > Having another look at the source code of gpio-restart, the WARNING-s I reported in the > patch's commit message occur at the following points of the GPIO output waveform: > > |< 100ms >|< 100 ms >|< 3000 ms >|< Restart failed > _____|_________| |_______________|__ [ active ] > _____X \__________/ [inactive] > | | | | > | | | ^ WARN @ drivers/power/reset/gpio-restart.c:46 > | | | > | | ^ WARN @ drivers/gpio/gpiolib.c:3098 > | ^ WARN @ drivers/gpio/gpiolib.c:3098 > | > ^ Restart should already occur here > > > If everything is set up correctly, the system should restart before execution reaches the > point where a warning can be emitted. If you say that you see a warning (any at all), > AFAICT that means gpio-restart is not working. > > As they say, the proof of the pudding is in the eating, so I soldered a jumper wire > between the RTL8231's GPIO5 pin (U38:25) and the line driven by the hard reset button > (U25:3) [https://svanheule.net/switches/gs1900-48#hard_reset_circuit]. > As expected from the analysis above, this results in a system rebooting without _any_ > warning (using an initramfs from yesterday's snapshot builds): > > root@OpenWrt:/# reboot > root@OpenWrt:/# [ 185.092891] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > [ 185.101879] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > [ 185.111835] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > [ 185.120484] rtl83xx_fib4_del: found a route with id 1, nh-id 0 > [ 185.127681] rtl83xx-switch switch@1b000000: unknown nexthop, id 0 > [ 185.149505] rtl83xx-switch switch@1b000000: unknown nexthop, id 0 > [ 185.157262] rtl83xx_fib4_del: found a route with id 2, nh-id 0 > [ 185.164418] rtl83xx-switch switch@1b000000: unknown nexthop, id 0 > [ 185.173391] rtl83xx_fib4_del: no such gateway: 0.0.0.0 > [ 185.225492] device lan01 left promiscuous mode > [ 185.230976] switch: port 1(lan01) entered disabled state > ... > [ 187.735562] device lan50 left promiscuous mode > [ 187.741075] switch: port 50(lan50) entered disabled state > [ 187.794104] in rtl838x_eth_stop > [ 187.797945] rtl838x-eth 1b00a300.ethernet eth0: Link is Down > [ 188.329431] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > [ 188.337562] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > [ 188.345649] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > [ 188.353736] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > [ 188.543709] rtl83xx_fib4_del: no such gateway: 0.0.0.0 > [ 188.549982] rtl83xx_fib4_del: no such gateway: 0.0.0.0 > [ 188.559077] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > [ 188.567226] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > [ 188.576283] rtl83xx_fib4_del: no such gateway: 0.0.0.0 > [ 188.582679] rtl83xx_fib4_del: no such gateway: 0.0.0.0 > [ 192.871878] reboot: Restarting system > > > U-Boot Version: 2.0.0.59413 (Jul 08 2015 - 10:01:28) > > CPU: 750MHz > DRAM: 128 MB > FLASH: 16 MB > Model: ZyXEL_GS1900_48 > SN: S182L05000541 > > > Best, > Sander >
Hi Birger, On Fri, 2022-02-25 at 19:58 +0100, Birger Koblitz wrote: > Hi Sander, > > I don't have the GS1900-48 at hand or even my records. I am on vacation and I will only > be back in 10 days, we actually got stuck here in quarantine... Have you been able to perform any checks by now to dispute my claims, or can this patch be merged? Best, Sander > You can also test the GPIO access in U-Boot using md.l and mw.l using the the indirect > access register. I tested this once for the indirect table access registers and that > worked. > But I looked up when the GS1900-48 was released and it was around since at least 2013. > I would be surprised if there are _not_ different versions around. We know that there > _are_ actually different version with regards to the SoC. They use at least 3 different > processor generations including different work-arounds necessary. Have a look at the > forum on issues with reading non-existing PHYs and the port flooding table, which in SoC > revisions > < D cannot be read and requires a shadow table (not implemented, since we always flood > to > all ports, because port isolation overrides this anyway). > On the warning: At least the initial warning came from the lock debugging I compiled in, > which is based on Daniel's standard settings. IIRC, the lengthy warning was about > sleeping in the wrong context, maybe there was also something about an issue with lock > contention. > > Cheers, > Birger > > > On 24/02/2022 21:19, Sander Vanheule wrote: > > On Tue, 2022-02-22 at 23:39 +0100, Birger Koblitz wrote: > > > Hi, > > > > > > the information on the external GPIO resetting the board of > > > the Zyxel GS1900-48 comes from the hardware configuration > > > reported by the stock firmware. It says: > > > GS1900# show board > > > [...] > > > ====== Reset ================= > > > Type: GPIO > > > GPIO: EXT_5 > > > [...] > > > Using the rtk gpio commands in u-boot this can be confirmed. > > > > Can you list the commands that you used to test this? My bootloader only supports "rtk > > network ..." and "cst pinSet ...". > > > > > > > On 22/02/2022 23:00, Sander Vanheule wrote: > > > > On Mon, 2022-02-21 at 21:23 +0100, Birger Koblitz wrote: > > > > > Hi, > > > > > > > > > > > > I just checked with my multimeter, and while the GPIO5 on the RTL8231 does go > > > > > > high/low > > > > > > when I set the output high/low from Linux, my device certainly doesn't reset. > > > > > > The > > > > > > other > > > > > > GPIO lines on the chip do work, since SFP modules are correctly detected. > > > > > > > > > > > > Birger, just to be sure, can you confirm that your device does reset with > > > > > > GPIO5 on > > > > > > the > > > > > > RTL8231? > > > > > > > > > > Yes, it does.There is a warning, but then it reliably resets. That was why I > > > > > left it > > > > > in as is. > > > > > > > > I had another hard look at my board, to check if something may be wrong > > > > physically, > > > > but I > > > > cannot find anything. My device's board looks identical to the pictures on the > > > > switch > > > > wiki > > > > [1], which I think you uploaded earlier. > > > > > > > > There is some reset logic on the board [2], but I cannot figure out how GPIO5 > > > > would be > > > > connected to it electrically. Unless I missed a via connecting to that pin on the > > > > RTL8231, > > > > GPIO5 only appears to lead to TP2. GPIO5/TP2 does not appear to be connected > > > > electrically > > > > to any part of the circuit next to SW1. I could add a bodge wire to connect TP2 to > > > > pad > > > > U25:3, but gpio-restart should really work on unmodified hardware. > > > > > > > > [1] https://svanheule.net/switches/gs1900-48#board_details > > > > [2] https://svanheule.net/switches/gs1900-48#hard_reset_circuit > > > > > > Having another look at the source code of gpio-restart, the WARNING-s I reported in > > the > > patch's commit message occur at the following points of the GPIO output waveform: > > > > |< 100ms >|< 100 ms >|< 3000 ms >|< Restart failed > > _____|_________| |_______________|__ [ active ] > > _____X \__________/ [inactive] > > | | | | > > | | | ^ WARN @ drivers/power/reset/gpio- > > restart.c:46 > > | | | > > | | ^ WARN @ drivers/gpio/gpiolib.c:3098 > > | ^ WARN @ drivers/gpio/gpiolib.c:3098 > > | > > ^ Restart should already occur here > > > > > > If everything is set up correctly, the system should restart before execution reaches > > the > > point where a warning can be emitted. If you say that you see a warning (any at all), > > AFAICT that means gpio-restart is not working. > > > > As they say, the proof of the pudding is in the eating, so I soldered a jumper wire > > between the RTL8231's GPIO5 pin (U38:25) and the line driven by the hard reset button > > (U25:3) [https://svanheule.net/switches/gs1900-48#hard_reset_circuit]. > > As expected from the analysis above, this results in a system rebooting without _any_ > > warning (using an initramfs from yesterday's snapshot builds): > > > > root@OpenWrt:/# reboot > > root@OpenWrt:/# [ 185.092891] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not > > supported > > [ 185.101879] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > > [ 185.111835] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > > [ 185.120484] rtl83xx_fib4_del: found a route with id 1, nh-id 0 > > [ 185.127681] rtl83xx-switch switch@1b000000: unknown nexthop, id 0 > > [ 185.149505] rtl83xx-switch switch@1b000000: unknown nexthop, id 0 > > [ 185.157262] rtl83xx_fib4_del: found a route with id 2, nh-id 0 > > [ 185.164418] rtl83xx-switch switch@1b000000: unknown nexthop, id 0 > > [ 185.173391] rtl83xx_fib4_del: no such gateway: 0.0.0.0 > > [ 185.225492] device lan01 left promiscuous mode > > [ 185.230976] switch: port 1(lan01) entered disabled state > > ... > > [ 187.735562] device lan50 left promiscuous mode > > [ 187.741075] switch: port 50(lan50) entered disabled state > > [ 187.794104] in rtl838x_eth_stop > > [ 187.797945] rtl838x-eth 1b00a300.ethernet eth0: Link is Down > > [ 188.329431] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > > [ 188.337562] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > > [ 188.345649] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > > [ 188.353736] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > > [ 188.543709] rtl83xx_fib4_del: no such gateway: 0.0.0.0 > > [ 188.549982] rtl83xx_fib4_del: no such gateway: 0.0.0.0 > > [ 188.559077] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > > [ 188.567226] rtl83xx_fib_event: FIB_RULE ADD/DELL for IPv6 not supported > > [ 188.576283] rtl83xx_fib4_del: no such gateway: 0.0.0.0 > > [ 188.582679] rtl83xx_fib4_del: no such gateway: 0.0.0.0 > > [ 192.871878] reboot: Restarting system > > > > > > U-Boot Version: 2.0.0.59413 (Jul 08 2015 - 10:01:28) > > > > CPU: 750MHz > > DRAM: 128 MB > > FLASH: 16 MB > > Model: ZyXEL_GS1900_48 > > SN: S182L05000541 > > > > > > Best, > > Sander > >
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>;
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@svanheule.net> --- target/linux/realtek/dts-5.10/rtl8393_zyxel_gs1900-48.dts | 5 ----- 1 file changed, 5 deletions(-)