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

[Birger Koblitz]

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 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>;
>>
>> _______________________________________________
>> openwrt-devel mailing list
>> openwrt-devel at lists.openwrt.org
>> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
>