On 11/27/19 4:40 PM, Claudius Heine wrote:
> On 27/11/2019 16.21, Marek Vasut wrote:
>> On 11/27/19 4:17 PM, Claudius Heine wrote:
>>> On 27/11/2019 16.12, Marek Vasut wrote:
>>>> On 11/27/19 4:09 PM, Claudius Heine wrote:
>>>>> Hi Marek,
>>>>>
>>>>> On 27/11/2019 15.47, Marek Vasut wrote:
>>>>>> On 11/27/19 3:20 PM, Claudius Heine wrote:
>>>>>>> In case CONFIG_SYSRESET is set, do_reset from reset.c will not be
>>>>>>> available
>>>>>>> anywere, even if SYSRESET is disabled for SPL in the board specific
>>>>>>> header
>>>>>>> file like this:
>>>>>>>
>>>>>>> #if defined(CONFIG_SPL_BUILD)
>>>>>>> #undef CONFIG_WDT
>>>>>>> #undef CONFIG_WATCHDOG
>>>>>>> #undef CONFIG_SYSRESET
>>>>>>> #define CONFIG_HW_WATCHDOG
>>>>>>> #endif
>>>>>>>
>>>>>>> 'do_reset' is called from SPL for instance from the panic handler in
>>>>>>> case
>>>>>>> SPL_USB_SDP is enabled and PANIC_HANG is not set.
>>>>>>>
>>>>>>> Setting PANIC_HANG would solve this issue, but it also changes the
>>>>>>> behavior
>>>>>>> in case a panic occurs.
>>>>>>>
>>>>>>> Signed-off-by: Claudius Heine <c...@denx.de>
>>>>>>> ---
>>>>>>> arch/arm/lib/Makefile | 2 --
>>>>>>> arch/arm/lib/reset.c | 2 ++
>>>>>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
>>>>>>> index 9de9a9acee..763eb4498f 100644
>>>>>>> --- a/arch/arm/lib/Makefile
>>>>>>> +++ b/arch/arm/lib/Makefile
>>>>>>> @@ -56,9 +56,7 @@ obj-y += interrupts_64.o
>>>>>>> else
>>>>>>> obj-y += interrupts.o
>>>>>>> endif
>>>>>>> -ifndef CONFIG_SYSRESET
>>>>>>> obj-y += reset.o
>>>>>>> -endif
>>>>>>>
>>>>>>> obj-y += cache.o
>>>>>>> obj-$(CONFIG_SYS_ARM_CACHE_CP15) += cache-cp15.o
>>>>>>> diff --git a/arch/arm/lib/reset.c b/arch/arm/lib/reset.c
>>>>>>> index f3ea116e87..11e680be1d 100644
>>>>>>> --- a/arch/arm/lib/reset.c
>>>>>>> +++ b/arch/arm/lib/reset.c
>>>>>>> @@ -22,6 +22,7 @@
>>>>>>>
>>>>>>> #include <common.h>
>>>>>>>
>>>>>>> +#if !defined(CONFIG_SYSRESET)
>>>>>>> __weak void reset_misc(void)
>>>>>>> {
>>>>>>> }
>>>>>>> @@ -40,3 +41,4 @@ int do_reset(cmd_tbl_t *cmdtp, int flag, int argc,
>>>>>>> char * const argv[])
>>>>>>> /*NOTREACHED*/
>>>>>>> return 0;
>>>>>>> }
>>>>>>> +#endif
>>>>>>
>>>>>> Does this mean there's now one huge ifdef around the entire source file?
>>>>>> That's odd.
>>>>>
>>>>> Right. Other suggestions?
>>>>>
>>>>> Maybe having 'do_reset' here as a weak instead, so that sysreset can
>>>>> overwrite it? But then the other definitions in arch/*/lib/reset.c
>>>>> should probably be the same for consistency sake?
>>>>>
>>>>> I tried with this patch not to change anything in case SYSRESET is
>>>>> enabled too much and since the file isn't too large I thought that would
>>>>> be ok for now.
>>>>
>>>> What if sysreset implemented do_reset ? Wouldn't that solve the issue ?
>>>
>>> Not sure what you mean... sysreset implements do_reset:
>>>
>>> https://gitlab.denx.de/u-boot/u-boot/blob/master/drivers/sysreset/sysreset-uclass.c#L112
>>>
>>> But the SPL does not have sysreset in this case, so it needs something
>>> different.
>>
>> Oh, so you need CONFIG_$(SPL_TPL_)SYSRESET then ?
>
> Well that would probably not enough. I would also need settings for the
> watchdog, because the SPL does not have DM support, so while u-boot uses
> CONFIG_WATCHDOG the SPL uses CONFIG_HW_WATCHDOG.
>
> Easier that changing all this is something like this in the board header
> file (as I described in the commit description):
>
> #if defined(CONFIG_SPL_BUILD)
> #undef CONFIG_WDT
> #undef CONFIG_WATCHDOG
> #undef CONFIG_SYSRESET
> #define CONFIG_HW_WATCHDOG
> #endif
Can't we add DM watchdog to SPL instead ?
> In case of imx6, that way the SPL uses the hw_watchdog_reset from the
> imx watchdog driver instead of the 'watchdog_reset'.
>
> 'watchdog_reset' is not available since that is implemented in
> wdt-uclass.c and CONFIG_SPL_WDT depends on SPL_DM.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
