[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 at theobroma-systems.com> wrote:
>
>> 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.

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