Re: [U-Boot] [RFC PATCH] ARM: reset: Move SYSRESET condition from Makefile into source file
On 11/28/19 9:10 AM, Claudius Heine wrote:
> On 27/11/2019 17.05, Marek Vasut wrote:
>> 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
> ---
> 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
>
> +#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 ?
>
> Do you mean implementing SPL_DM support for this board so that we can
> use the DM watchdog and sysreset?
Isn't SPL DM already implemented ?
[...]
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH] ARM: reset: Move SYSRESET condition from Makefile into source file
On Wed, Nov 27, 2019 at 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
> >> ---
> >> 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
> >>
> >> +#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
>
> 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.
That seems totally unrelated to this patch.
I think this patch should change the Makefile to use
CONFIG_$(SPL_TPL_)SYSRESET and you need to solve your watchdog
issue in a separate patch.
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH] ARM: reset: Move SYSRESET condition from Makefile into source file
On 27/11/2019 17.05, Marek Vasut wrote:
> 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
---
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
+#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 ?
Do you mean implementing SPL_DM support for this board so that we can
use the DM watchdog and sysreset?
Well you know more about this than me. But it probably comes down to
technical limitations like size of the SPL.
Lukasz has done something similar with the display5 board [1], but that
uses PANIC_HANG to avoid this issue.
[1]
https://gitlab.denx.de/u-boot/u-boot/blob/4b19b89ca4a866b7baa642533e6dbd67cd832d27/include/configs/display5.h#L343
>
>> 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
Re: [U-Boot] [RFC PATCH] ARM: reset: Move SYSRESET condition from Makefile into source file
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
>>> ---
>>> 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
>>>
>>> +#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
Re: [U-Boot] [RFC PATCH] ARM: reset: Move SYSRESET condition from Makefile into source file
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
>> ---
>> 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
>>
>> +#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
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
Re: [U-Boot] [RFC PATCH] ARM: reset: Move SYSRESET condition from Makefile into source file
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
> ---
> 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
>
> +#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 ?
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH] ARM: reset: Move SYSRESET condition from Makefile into source file
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
---
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
+#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.
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de
PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
Keyserver: hkp://pool.sks-keyservers.net
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH] ARM: reset: Move SYSRESET condition from Makefile into source file
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
>>> ---
>>> 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
>>>
>>> +#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 ?
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH] ARM: reset: Move SYSRESET condition from Makefile into source file
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
>> ---
>> 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
>>
>> +#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.
regards
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: c...@denx.de
PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
Keyserver: hkp://pool.sks-keyservers.net
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [RFC PATCH] ARM: reset: Move SYSRESET condition from Makefile into source file
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
> ---
> 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
>
> +#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.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
