[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 at chromium.org> wrote:
> 
> Hi Philipp,
> 
> On 26 November 2017 at 16:22, Philipp Tomsich
> <philipp.tomsich at 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 at theobroma-systems.com>
>> Tested-by: Klaus Goger <klaus.goger at 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 at 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