On 23. 08. 19 11:47, Linus Walleij wrote: > The microblaze uses the legacy APIs to dig out a GPIO pin > defined in the root of the device tree to issue a hard > reset of the platform. > > Asserting a hard reset should be done using the standard > DT-enabled and fully GPIO descriptor aware driver in > drivers/power/reset/gpio-restart.c using the bindings > from Documentation/devicetree/bindings/power/reset/gpio-restart.txt > > To achieve this, first make sure microblaze makes use of > the standard kernel restart path utilizing do_kernel_restart() > from . Put in some grace time and an > emergency print if the restart does not properly assert. > > As this is basic platform functionality we patch the DTS > file and defconfig in one go for a lockstep change. > > Cc: Arnd Bergmann > Cc: Michal Simek > Signed-off-by: Linus Walleij > --- > ChangeLog v1->v2: > - Fix the reset GPIOs property in the DTS file to be > just "gpios" as expected by the driver. > > Hi Michal, would be great if you could test and fix this up > so I can get rid of one more site where legacy GPIO is used. > I am unsure of the appropriate polarity and delays, hence the > comments in the DTS file. > --- > arch/microblaze/boot/dts/system.dts | 16 +++++- > arch/microblaze/configs/mmu_defconfig | 2 + > arch/microblaze/configs/nommu_defconfig | 2 + > arch/microblaze/include/asm/setup.h | 1 - > arch/microblaze/kernel/reset.c | 76 ------------------------- > arch/microblaze/mm/init.c | 11 ++++ > 6 files changed, 30 insertions(+), 78 deletions(-) > > diff --git a/arch/microblaze/boot/dts/system.dts b/arch/microblaze/boot/dts/system.dts > index 5a8a9d090c37..5b236527176e 100644 > --- a/arch/microblaze/boot/dts/system.dts > +++ b/arch/microblaze/boot/dts/system.dts > @@ -18,7 +18,6 @@ > #address-cells = <1>; > #size-cells = <1>; > compatible = "xlnx,microblaze"; > - hard-reset-gpios = <&LEDs_8Bit 2 1>; > model = "testing"; > DDR2_SDRAM: memory@90000000 { > device_type = "memory"; > @@ -281,6 +280,21 @@ > gpios = <&LEDs_8Bit 7 1>; > }; > } ; > + > + gpio-restart { > + compatible = "gpio-restart"; > + /* > + * FIXME: is this active low or active high? > + * the current flag (1) indicates active low. > + * delay measures are templates, should be adjusted > + * to datasheet or trial-and-error with real hardware. > + */ > + gpios = <&LEDs_8Bit 2 1>; > + active-delay = <100>; > + inactive-delay = <10>; > + wait-delay = <100>; > + }; > + > RS232_Uart_1: serial@84000000 { > clock-frequency = <125000000>; > compatible = "xlnx,xps-uartlite-1.00.a"; > diff --git a/arch/microblaze/configs/mmu_defconfig b/arch/microblaze/configs/mmu_defconfig > index 92fd4e95b488..ae8d7d407ff4 100644 > --- a/arch/microblaze/configs/mmu_defconfig > +++ b/arch/microblaze/configs/mmu_defconfig > @@ -59,6 +59,8 @@ CONFIG_SPI_XILINX=y > CONFIG_GPIOLIB=y > CONFIG_GPIO_SYSFS=y > CONFIG_GPIO_XILINX=y > +CONFIG_POWER_RESET=y > +CONFIG_POWER_RESET_GPIO_RESTART=y > # CONFIG_HWMON is not set > CONFIG_WATCHDOG=y > CONFIG_XILINX_WATCHDOG=y > diff --git a/arch/microblaze/configs/nommu_defconfig b/arch/microblaze/configs/nommu_defconfig > index 06d69a6e192d..a2a6be511551 100644 > --- a/arch/microblaze/configs/nommu_defconfig > +++ b/arch/microblaze/configs/nommu_defconfig > @@ -62,6 +62,8 @@ CONFIG_SPI_XILINX=y > CONFIG_GPIOLIB=y > CONFIG_GPIO_SYSFS=y > CONFIG_GPIO_XILINX=y > +CONFIG_POWER_RESET=y > +CONFIG_POWER_RESET_GPIO_RESTART=y > # CONFIG_HWMON is not set > CONFIG_WATCHDOG=y > CONFIG_XILINX_WATCHDOG=y > diff --git a/arch/microblaze/include/asm/setup.h b/arch/microblaze/include/asm/setup.h > index ce9b7b786156..54d634ed98e6 100644 > --- a/arch/microblaze/include/asm/setup.h > +++ b/arch/microblaze/include/asm/setup.h > @@ -29,7 +29,6 @@ void machine_early_init(const char *cmdline, unsigned int ram, > unsigned int fdt, unsigned int msr, unsigned int tlb0, > unsigned int tlb1); > > -void machine_restart(char *cmd); > void machine_shutdown(void); > void machine_halt(void); > void machine_power_off(void); > diff --git a/arch/microblaze/kernel/reset.c b/arch/microblaze/kernel/reset.c > index fcbe1daf6316..b56af4eb91bf 100644 > --- a/arch/microblaze/kernel/reset.c > +++ b/arch/microblaze/kernel/reset.c > @@ -10,82 +10,6 @@ > #include > #include > > -/* Trigger specific functions */ > -#ifdef CONFIG_GPIOLIB > - > -#include > - > -static int handle; /* reset pin handle */ > -static unsigned int reset_val; > - > -static int of_platform_reset_gpio_probe(void) > -{ > - int ret; > - handle = of_get_named_gpio(of_find_node_by_path("/"), > - "hard-reset-gpios", 0); > - > - if (!gpio_is_valid(handle)) { > - pr_info("Skipping unavailable RESET gpio %d (%s)\n", > - handle, "reset"); > - return -ENODEV; > - } > - > - ret = gpio_request(handle, "reset"); > - if (ret < 0) { > - pr_info("GPIO pin is already allocated\n"); > - return ret; > - } > - > - /* get current setup value */ > - reset_val = gpio_get_value(handle); > - /* FIXME maybe worth to perform any action */ > - pr_debug("Reset: Gpio output state: 0x%x\n", reset_val); > - > - /* Setup GPIO as output */ > - ret = gpio_direction_output(handle, 0); > - if (ret < 0) > - goto err; > - > - /* Setup output direction */ > - gpio_set_value(handle, 0); > - > - pr_info("RESET: Registered gpio device: %d, current val: %d\n", > - handle, reset_val); > - return 0; > -err: > - gpio_free(handle); > - return ret; > -} > -device_initcall(of_platform_reset_gpio_probe); > - > - > -static void gpio_system_reset(void) > -{ > - if (gpio_is_valid(handle)) > - gpio_set_value(handle, 1 - reset_val); > - else > - pr_notice("Reset GPIO unavailable - halting!\n"); > -} > -#else > -static void gpio_system_reset(void) > -{ > - pr_notice("No reset GPIO present - halting!\n"); > -} > - > -void of_platform_reset_gpio_probe(void) > -{ > - return; > -} > -#endif > - > -void machine_restart(char *cmd) > -{ > - pr_notice("Machine restart...\n"); > - gpio_system_reset(); > - while (1) > - ; > -} > - > void machine_shutdown(void) > { > pr_notice("Machine shutdown...\n"); > diff --git a/arch/microblaze/mm/init.c b/arch/microblaze/mm/init.c > index a015a951c8b7..4a45e037107f 100644 > --- a/arch/microblaze/mm/init.c > +++ b/arch/microblaze/mm/init.c > @@ -17,6 +17,8 @@ > #include > #include > #include > +#include > +#include > > #include > #include > @@ -265,6 +267,15 @@ static void __init mmu_init_hw(void) > : : : "r11"); > } > > +void machine_restart(char *cmd) > +{ > + do_kernel_restart(cmd); > + /* Give the restart hook 1 s to take us down */ > + mdelay(1000); > + pr_emerg("Reboot failed -- System halted\n"); > + while (1); > +} > + > /* > * MMU_init sets up the basic memory mappings for the kernel, > * including both RAM and possibly some I/O regions, > I will test this for sure. What's the reason to add machine_restart to mm/ folder? You can simply keep it in a location where it was. Do you know why of_find_gpio can failed in connection to gpio-xilinx driver? Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs