RFT: microblaze: Switch to standard restart handler

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 <linux/reboot.h>. 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 <arnd@arndb.de>
Cc: Michal Simek <monstr@monstr.eu>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
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(-)

Hi Linus,

On 21. 08. 19 10:33, 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 <linux/reboot.h>. 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 <arnd@arndb.de>
> Cc: Michal Simek <monstr@monstr.eu>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> 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..c4e8bed50cb0 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.
> +			 */
> +			hard-reset-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 <linux/init.h>
>  #include <linux/of_platform.h>
>  
> -/* Trigger specific functions */
> -#ifdef CONFIG_GPIOLIB
> -
> -#include <linux/of_gpio.h>
> -
> -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 <linux/slab.h>
>  #include <linux/swap.h>
>  #include <linux/export.h>
> +#include <linux/delay.h>
> +#include <linux/reboot.h>
>  
>  #include <asm/page.h>
>  #include <asm/mmu_context.h>
> @@ -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,
> 

This should got to kernel/reset.c.

Anyway I have tested it and I have issue to use restart-gpio driver and

I am using gpio-xilinx driver which drives that pin.

[    6.475449] restart-gpio amba_pl:gpio-restart: GPIO lookup for
consumer (null)
[    6.482887] restart-gpio amba_pl:gpio-restart: using device tree for
GPIO lookup
[    6.495623] restart-gpio amba_pl:gpio-restart: using lookup tables
for GPIO lookup
[    6.505016] restart-gpio amba_pl:gpio-restart: No GPIO consumer
(null) found
[    6.512275] restart-gpio amba_pl:gpio-restart: Could net get reset GPIO
[    6.519076] restart-gpio: probe of amba_pl:gpio-restart failed with
error -2

Is there an issue with gpio-xilinx driver that of_find_gpio() is failing?

Thanks,
Michal