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 <san...@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


_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to