[U-Boot] [PATCH 1/2] rockchip: rk3399-puma: add code to allow forcing a power-on reset

[U-Boot] [PATCH 1/2] rockchip: rk3399-puma: add code to allow forcing a power-on reset

[Philipp Tomsich]

The reset circuitry in the RK3399 only resets 'almost all logic' when
a software reset is performed.  To make our software maintenance
easier in the future, we want to have the option (controlled by a DTS
property) to force all reset causes other than a power-on reset to
trigger a power-on reset via a GPIO trigger.

This adds the necessary support to the rk3399-puma (i.e. RK3399-Q7)
board-support and the documentation for the new property
(sysreset-gpio) within the /config-node.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>
---

 board/theobroma-systems/puma_rk3399/puma-rk3399.c | 46 +++++++++++++++++++++++
 doc/device-tree-bindings/config.txt               |  6 +++
 2 files changed, 52 insertions(+)

diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
index 2b4988e..79dbdf4 100644
--- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
+++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
@@ -9,7 +9,10 @@
 #include <misc.h>
 #include <dm/pinctrl.h>
 #include <dm/uclass-internal.h>
+#include <asm/gpio.h>
 #include <asm/setup.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/cru_rk3399.h>
 #include <asm/arch/periph.h>
 #include <power/regulator.h>
 #include <spl.h>
@@ -32,9 +35,52 @@ int board_init(void)
 	return 0;
 }
 
+static void rk3399_force_power_on_reset(void)
+{
+	ofnode node;
+	struct gpio_desc sysreset_gpio;
+
+	debug("%s: trying to force a power-on reset\n", __func__);
+
+	node = ofnode_path("/config");
+	if (!ofnode_valid(node)) {
+		debug("%s: no /config node?\n", __func__);
+		return;
+	}
+
+	gpio_request_by_name_nodev(node, "sysreset-gpio", 0,
+				   &sysreset_gpio, GPIOD_IS_OUT);
+
+	if (!dm_gpio_is_valid(&sysreset_gpio)) {
+		debug("%s: could not find a /config/sysreset-gpio\n", __func__);
+		return;
+	}
+
+	dm_gpio_set_value(&sysreset_gpio, 1);
+}
+
 void spl_board_init(void)
 {
 	int  ret;
+	struct rk3399_cru *cru = rockchip_get_cru();
+
+	/*
+	 * The RK3399 resets only 'almost all logic' (see also in the TRM
+	 * "3.9.4 Global software reset"), when issuing a software reset.
+	 * This may cause issues during boot-up for some configurations of
+	 * the application software stack.
+	 *
+	 * To work around this, we test whether the last reset reason was
+	 * a power-on reset and (if not) issue an overtemp-reset to reset
+	 * the entire module.
+	 *
+	 * While this was previously fixed by modifying the various places
+	 * that could generate a software reset (e.g. U-Boot's sysreset
+	 * driver, the ATF or Linux), we now have it here to ensure that
+	 * we no longer have to track this through the various components.
+	 */
+	if (cru->glb_rst_st != 0)
+		rk3399_force_power_on_reset();
 
 	/*
 	 * Turning the eMMC and SPI back on (if disabled via the Qseven
diff --git a/doc/device-tree-bindings/config.txt b/doc/device-tree-bindings/config.txt
index 15e4349..6cdc16d 100644
--- a/doc/device-tree-bindings/config.txt
+++ b/doc/device-tree-bindings/config.txt
@@ -46,3 +46,9 @@ u-boot,spl-payload-offset
 	If present (and SPL is controlled by the device-tree), this allows
 	to override the CONFIG_SYS_SPI_U_BOOT_OFFS setting using a value
 	from the device-tree.
+
+sysreset-gpio
+	If present (and supported by the specific board), indicates a
+	GPIO that can be set to trigger a system reset.  It is assumed
+	that such a system reset will effect a complete platform reset,
+	being roughly equivalent to a power-on reset.
-- 
2.1.4

[U-Boot] [PATCH 1/2] rockchip: rk3399-puma: add code to allow forcing a power-on reset

[Philipp Tomsich]

On the RK3399-Q7, we want to trigger a full platform reset (so the
various software stacks supported don't have to deal with the same
complexities over and over again) in case that anything other than a
power-on reset occurred.

To do so, this defines the /config/sysreset-gpio property and has it
point to a GPIO that will perform a power-on reset of the entire
platform.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>
---

 arch/arm/dts/rk3399-puma.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/dts/rk3399-puma.dtsi b/arch/arm/dts/rk3399-puma.dtsi
index 96bd4fe..314214c 100644
--- a/arch/arm/dts/rk3399-puma.dtsi
+++ b/arch/arm/dts/rk3399-puma.dtsi
@@ -16,6 +16,7 @@
 		u-boot,mmc-env-offset = <0x4000>;      /* @  16KB */
 		u-boot,efi-partition-entries-offset = <0x200000>; /* 2MB */
 		u-boot,boot-led = "module_led";
+		sysreset-gpio = <&gpio1 RK_PA6 GPIO_ACTIVE_HIGH>;
 	};
 
 	chosen {
@@ -550,6 +551,10 @@
 	status = "okay";
 };
 
+&gpio1 {
+	u-boot,dm-pre-reloc;
+};
+
 &gpio3 {
 	u-boot,dm-pre-reloc;
 };
-- 
2.1.4

[U-Boot] [PATCH 1/2] rockchip: rk3399-puma: add code to allow forcing a power-on reset

[Simon Glass]

Hi Philipp,

On 26 November 2017 at 16:22, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> The reset circuitry in the RK3399 only resets 'almost all logic' when
> a software reset is performed.  To make our software maintenance
> easier in the future, we want to have the option (controlled by a DTS
> property) to force all reset causes other than a power-on reset to
> trigger a power-on reset via a GPIO trigger.
>
> This adds the necessary support to the rk3399-puma (i.e. RK3399-Q7)
> board-support and the documentation for the new property
> (sysreset-gpio) within the /config-node.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>
> ---
>
>  board/theobroma-systems/puma_rk3399/puma-rk3399.c | 46 +++++++++++++++++++++++
>  doc/device-tree-bindings/config.txt               |  6 +++
>  2 files changed, 52 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

>
> diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> index 2b4988e..79dbdf4 100644
> --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
> @@ -9,7 +9,10 @@
>  #include <misc.h>
>  #include <dm/pinctrl.h>
>  #include <dm/uclass-internal.h>
> +#include <asm/gpio.h>
>  #include <asm/setup.h>
> +#include <asm/arch/clock.h>
> +#include <asm/arch/cru_rk3399.h>
>  #include <asm/arch/periph.h>
>  #include <power/regulator.h>
>  #include <spl.h>
> @@ -32,9 +35,52 @@ int board_init(void)
>         return 0;
>  }
>
> +static void rk3399_force_power_on_reset(void)
> +{
> +       ofnode node;
> +       struct gpio_desc sysreset_gpio;
> +
> +       debug("%s: trying to force a power-on reset\n", __func__);
> +
> +       node = ofnode_path("/config");
> +       if (!ofnode_valid(node)) {
> +               debug("%s: no /config node?\n", __func__);
> +               return;
> +       }
> +
> +       gpio_request_by_name_nodev(node, "sysreset-gpio", 0,
> +                                  &sysreset_gpio, GPIOD_IS_OUT);
> +
> +       if (!dm_gpio_is_valid(&sysreset_gpio)) {

If you don't support this GPIO being option, you should check the
return value of gpio_request_by_name_nodev() instead. If it is < 0
then it did not find a GPIO.

> +               debug("%s: could not find a /config/sysreset-gpio\n", __func__);
> +               return;
> +       }
> +
> +       dm_gpio_set_value(&sysreset_gpio, 1);
> +}
> +
>  void spl_board_init(void)
>  {
>         int  ret;
> +       struct rk3399_cru *cru = rockchip_get_cru();
> +
> +       /*
> +        * The RK3399 resets only 'almost all logic' (see also in the TRM
> +        * "3.9.4 Global software reset"), when issuing a software reset.
> +        * This may cause issues during boot-up for some configurations of
> +        * the application software stack.
> +        *
> +        * To work around this, we test whether the last reset reason was
> +        * a power-on reset and (if not) issue an overtemp-reset to reset
> +        * the entire module.
> +        *
> +        * While this was previously fixed by modifying the various places
> +        * that could generate a software reset (e.g. U-Boot's sysreset
> +        * driver, the ATF or Linux), we now have it here to ensure that
> +        * we no longer have to track this through the various components.
> +        */
> +       if (cru->glb_rst_st != 0)
> +               rk3399_force_power_on_reset();
>
>         /*
>          * Turning the eMMC and SPI back on (if disabled via the Qseven
> diff --git a/doc/device-tree-bindings/config.txt b/doc/device-tree-bindings/config.txt
> index 15e4349..6cdc16d 100644
> --- a/doc/device-tree-bindings/config.txt
> +++ b/doc/device-tree-bindings/config.txt
> @@ -46,3 +46,9 @@ u-boot,spl-payload-offset
>         If present (and SPL is controlled by the device-tree), this allows
>         to override the CONFIG_SYS_SPI_U_BOOT_OFFS setting using a value
>         from the device-tree.
> +
> +sysreset-gpio
> +       If present (and supported by the specific board), indicates a
> +       GPIO that can be set to trigger a system reset.  It is assumed
> +       that such a system reset will effect a complete platform reset,
> +       being roughly equivalent to a power-on reset.

Is there no kernel binding for this sort of thing?

Regards,
Simon

[U-Boot] [PATCH 1/2] rockchip: rk3399-puma: add code to allow forcing a power-on reset

[Dr. Philipp Tomsich]

> On 27 Nov 2017, at 18:10, Simon Glass <sjg@chromium.org> wrote:
> 
> Hi Philipp,
> 
> On 26 November 2017 at 16:22, Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com> wrote:
>> The reset circuitry in the RK3399 only resets 'almost all logic' when
>> a software reset is performed.  To make our software maintenance
>> easier in the future, we want to have the option (controlled by a DTS
>> property) to force all reset causes other than a power-on reset to
>> trigger a power-on reset via a GPIO trigger.
>> 
>> This adds the necessary support to the rk3399-puma (i.e. RK3399-Q7)
>> board-support and the documentation for the new property
>> (sysreset-gpio) within the /config-node.
>> 
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>
>> ---
>> 
>> board/theobroma-systems/puma_rk3399/puma-rk3399.c | 46 +++++++++++++++++++++++
>> doc/device-tree-bindings/config.txt               |  6 +++
>> 2 files changed, 52 insertions(+)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
>> 
>> diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
>> index 2b4988e..79dbdf4 100644
>> --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
>> +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
>> @@ -9,7 +9,10 @@
>> #include <misc.h>
>> #include <dm/pinctrl.h>
>> #include <dm/uclass-internal.h>
>> +#include <asm/gpio.h>
>> #include <asm/setup.h>
>> +#include <asm/arch/clock.h>
>> +#include <asm/arch/cru_rk3399.h>
>> #include <asm/arch/periph.h>
>> #include <power/regulator.h>
>> #include <spl.h>
>> @@ -32,9 +35,52 @@ int board_init(void)
>>        return 0;
>> }
>> 
>> +static void rk3399_force_power_on_reset(void)
>> +{
>> +       ofnode node;
>> +       struct gpio_desc sysreset_gpio;
>> +
>> +       debug("%s: trying to force a power-on reset\n", __func__);
>> +
>> +       node = ofnode_path("/config");
>> +       if (!ofnode_valid(node)) {
>> +               debug("%s: no /config node?\n", __func__);
>> +               return;
>> +       }
>> +
>> +       gpio_request_by_name_nodev(node, "sysreset-gpio", 0,
>> +                                  &sysreset_gpio, GPIOD_IS_OUT);
>> +
>> +       if (!dm_gpio_is_valid(&sysreset_gpio)) {
> 
> If you don't support this GPIO being option, you should check the
> return value of gpio_request_by_name_nodev() instead. If it is < 0
> then it did not find a GPIO.

We want to treat this as an option, if either the software stack is known
to handle partial resets correctly or if the software stack already always
generates a cold-reset of the entire platform.

> 
>> +               debug("%s: could not find a /config/sysreset-gpio\n", __func__);
>> +               return;
>> +       }
>> +
>> +       dm_gpio_set_value(&sysreset_gpio, 1);
>> +}
>> +
>> void spl_board_init(void)
>> {
>>        int  ret;
>> +       struct rk3399_cru *cru = rockchip_get_cru();
>> +
>> +       /*
>> +        * The RK3399 resets only 'almost all logic' (see also in the TRM
>> +        * "3.9.4 Global software reset"), when issuing a software reset.
>> +        * This may cause issues during boot-up for some configurations of
>> +        * the application software stack.
>> +        *
>> +        * To work around this, we test whether the last reset reason was
>> +        * a power-on reset and (if not) issue an overtemp-reset to reset
>> +        * the entire module.
>> +        *
>> +        * While this was previously fixed by modifying the various places
>> +        * that could generate a software reset (e.g. U-Boot's sysreset
>> +        * driver, the ATF or Linux), we now have it here to ensure that
>> +        * we no longer have to track this through the various components.
>> +        */
>> +       if (cru->glb_rst_st != 0)
>> +               rk3399_force_power_on_reset();
>> 
>>        /*
>>         * Turning the eMMC and SPI back on (if disabled via the Qseven
>> diff --git a/doc/device-tree-bindings/config.txt b/doc/device-tree-bindings/config.txt
>> index 15e4349..6cdc16d 100644
>> --- a/doc/device-tree-bindings/config.txt
>> +++ b/doc/device-tree-bindings/config.txt
>> @@ -46,3 +46,9 @@ u-boot,spl-payload-offset
>>        If present (and SPL is controlled by the device-tree), this allows
>>        to override the CONFIG_SYS_SPI_U_BOOT_OFFS setting using a value
>>        from the device-tree.
>> +
>> +sysreset-gpio
>> +       If present (and supported by the specific board), indicates a
>> +       GPIO that can be set to trigger a system reset.  It is assumed
>> +       that such a system reset will effect a complete platform reset,
>> +       being roughly equivalent to a power-on reset.
> 
> Is there no kernel binding for this sort of thing?

Not to my knowledge (and grep didn’t find anything either).

Note that the kernel (for ARM64) requests the reset via PSCI (from
the ATF) and the ATF then needs to do “the right thing”.  Many other
platforms have relied on their watchdog timers to effect a reset in the
past.  So in other words: most of the kernel doesn’t really care about
a setting like this.

> 
> Regards,
> Simon

[U-Boot] [PATCH 1/2] rockchip: rk3399-puma: add code to allow forcing a power-on reset

[Simon Glass]

Hi Philipp,

On 27 November 2017 at 10:16, Dr. Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
>
>> On 27 Nov 2017, at 18:10, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Philipp,
>>
>> On 26 November 2017 at 16:22, Philipp Tomsich
>> <philipp.tomsich@theobroma-systems.com> wrote:
>>> The reset circuitry in the RK3399 only resets 'almost all logic' when
>>> a software reset is performed.  To make our software maintenance
>>> easier in the future, we want to have the option (controlled by a DTS
>>> property) to force all reset causes other than a power-on reset to
>>> trigger a power-on reset via a GPIO trigger.
>>>
>>> This adds the necessary support to the rk3399-puma (i.e. RK3399-Q7)
>>> board-support and the documentation for the new property
>>> (sysreset-gpio) within the /config-node.
>>>
>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>> Tested-by: Klaus Goger <klaus.goger@theobroma-systems.com>
>>> ---
>>>
>>> board/theobroma-systems/puma_rk3399/puma-rk3399.c | 46 +++++++++++++++++++++++
>>> doc/device-tree-bindings/config.txt               |  6 +++
>>> 2 files changed, 52 insertions(+)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>>>
>>> diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
>>> index 2b4988e..79dbdf4 100644
>>> --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c
>>> +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c
>>> @@ -9,7 +9,10 @@
>>> #include <misc.h>
>>> #include <dm/pinctrl.h>
>>> #include <dm/uclass-internal.h>
>>> +#include <asm/gpio.h>
>>> #include <asm/setup.h>
>>> +#include <asm/arch/clock.h>
>>> +#include <asm/arch/cru_rk3399.h>
>>> #include <asm/arch/periph.h>
>>> #include <power/regulator.h>
>>> #include <spl.h>
>>> @@ -32,9 +35,52 @@ int board_init(void)
>>>        return 0;
>>> }
>>>
>>> +static void rk3399_force_power_on_reset(void)
>>> +{
>>> +       ofnode node;
>>> +       struct gpio_desc sysreset_gpio;
>>> +
>>> +       debug("%s: trying to force a power-on reset\n", __func__);
>>> +
>>> +       node = ofnode_path("/config");
>>> +       if (!ofnode_valid(node)) {
>>> +               debug("%s: no /config node?\n", __func__);
>>> +               return;
>>> +       }
>>> +
>>> +       gpio_request_by_name_nodev(node, "sysreset-gpio", 0,
>>> +                                  &sysreset_gpio, GPIOD_IS_OUT);
>>> +
>>> +       if (!dm_gpio_is_valid(&sysreset_gpio)) {
>>
>> If you don't support this GPIO being option, you should check the
>> return value of gpio_request_by_name_nodev() instead. If it is < 0
>> then it did not find a GPIO.
>
> We want to treat this as an option, if either the software stack is known
> to handle partial resets correctly or if the software stack already always
> generates a cold-reset of the entire platform.

OK, so you should be able to do:

if (gpio_request_by_name_nodev(...)) {
   debug(...);
   return;
}

There is no need to call dm_gpio_is_valid() separately.

>
>>
>>> +               debug("%s: could not find a /config/sysreset-gpio\n", __func__);
>>> +               return;
>>> +       }
>>> +
>>> +       dm_gpio_set_value(&sysreset_gpio, 1);
>>> +}
>>> +
>>> void spl_board_init(void)
>>> {
>>>        int  ret;
>>> +       struct rk3399_cru *cru = rockchip_get_cru();
>>> +
>>> +       /*
>>> +        * The RK3399 resets only 'almost all logic' (see also in the TRM
>>> +        * "3.9.4 Global software reset"), when issuing a software reset.
>>> +        * This may cause issues during boot-up for some configurations of
>>> +        * the application software stack.
>>> +        *
>>> +        * To work around this, we test whether the last reset reason was
>>> +        * a power-on reset and (if not) issue an overtemp-reset to reset
>>> +        * the entire module.
>>> +        *
>>> +        * While this was previously fixed by modifying the various places
>>> +        * that could generate a software reset (e.g. U-Boot's sysreset
>>> +        * driver, the ATF or Linux), we now have it here to ensure that
>>> +        * we no longer have to track this through the various components.
>>> +        */
>>> +       if (cru->glb_rst_st != 0)
>>> +               rk3399_force_power_on_reset();
>>>
>>>        /*
>>>         * Turning the eMMC and SPI back on (if disabled via the Qseven
>>> diff --git a/doc/device-tree-bindings/config.txt b/doc/device-tree-bindings/config.txt
>>> index 15e4349..6cdc16d 100644
>>> --- a/doc/device-tree-bindings/config.txt
>>> +++ b/doc/device-tree-bindings/config.txt
>>> @@ -46,3 +46,9 @@ u-boot,spl-payload-offset
>>>        If present (and SPL is controlled by the device-tree), this allows
>>>        to override the CONFIG_SYS_SPI_U_BOOT_OFFS setting using a value
>>>        from the device-tree.
>>> +
>>> +sysreset-gpio
>>> +       If present (and supported by the specific board), indicates a
>>> +       GPIO that can be set to trigger a system reset.  It is assumed
>>> +       that such a system reset will effect a complete platform reset,
>>> +       being roughly equivalent to a power-on reset.
>>
>> Is there no kernel binding for this sort of thing?
>
> Not to my knowledge (and grep didn’t find anything either).
>
> Note that the kernel (for ARM64) requests the reset via PSCI (from
> the ATF) and the ATF then needs to do “the right thing”.  Many other
> platforms have relied on their watchdog timers to effect a reset in the
> past.  So in other words: most of the kernel doesn’t really care about
> a setting like this.

Regards,
Simon