| Message ID | 20230212231638.1134219-66-sjg@chromium.org |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | RFC: Migrate to split config | expand |
On Sun, Feb 12, 2023 at 04:16:08PM -0700, Simon Glass wrote: > The logic here is strange since the call to hw_watchdog_init() depends on > CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG but the code it calls is only > enabled if !CONFIG_IS_ENABLED(WDT). This seems to work, but with split > config we get a build error in SPL with CONFIG_WATCHDOG, since it is > interpreted as CONFIG_SPL_WATCHDOG. > > To mimic the behaviour before split config takes effect, use the > PPL_WATCHDOG symbol. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > (no changes since v1) > > arch/arm/mach-omap2/boot-common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/mach-omap2/boot-common.c b/arch/arm/mach-omap2/boot-common.c > index 9a342a1bf95..401d12c6ca3 100644 > --- a/arch/arm/mach-omap2/boot-common.c > +++ b/arch/arm/mach-omap2/boot-common.c > @@ -302,7 +302,7 @@ void spl_board_init(void) > #if defined(CONFIG_AM33XX) && defined(CONFIG_SPL_MUSB_NEW) > arch_misc_init(); > #endif > -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) > +#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_PPL_WATCHDOG) > hw_watchdog_init(); > #endif > #ifdef CONFIG_AM33XX No, we want watchdog enabled in SPL.
On Tue, Feb 14, 2023 at 11:34:47AM -0500, Tom Rini wrote: > On Sun, Feb 12, 2023 at 04:16:08PM -0700, Simon Glass wrote: > > The logic here is strange since the call to hw_watchdog_init() depends on > > CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG but the code it calls is only > > enabled if !CONFIG_IS_ENABLED(WDT). This seems to work, but with split > > config we get a build error in SPL with CONFIG_WATCHDOG, since it is > > interpreted as CONFIG_SPL_WATCHDOG. > > > > To mimic the behaviour before split config takes effect, use the > > PPL_WATCHDOG symbol. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > (no changes since v1) > > > > arch/arm/mach-omap2/boot-common.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/arm/mach-omap2/boot-common.c b/arch/arm/mach-omap2/boot-common.c > > index 9a342a1bf95..401d12c6ca3 100644 > > --- a/arch/arm/mach-omap2/boot-common.c > > +++ b/arch/arm/mach-omap2/boot-common.c > > @@ -302,7 +302,7 @@ void spl_board_init(void) > > #if defined(CONFIG_AM33XX) && defined(CONFIG_SPL_MUSB_NEW) > > arch_misc_init(); > > #endif > > -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) > > +#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_PPL_WATCHDOG) > > hw_watchdog_init(); > > #endif > > #ifdef CONFIG_AM33XX > > No, we want watchdog enabled in SPL. Or perhaps, the same question as in the socfpga patch also applies here.
Hi Tom, On Tue, 14 Feb 2023 at 09:43, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Feb 14, 2023 at 11:34:47AM -0500, Tom Rini wrote: > > On Sun, Feb 12, 2023 at 04:16:08PM -0700, Simon Glass wrote: > > > The logic here is strange since the call to hw_watchdog_init() depends on > > > CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG but the code it calls is only > > > enabled if !CONFIG_IS_ENABLED(WDT). This seems to work, but with split > > > config we get a build error in SPL with CONFIG_WATCHDOG, since it is > > > interpreted as CONFIG_SPL_WATCHDOG. > > > > > > To mimic the behaviour before split config takes effect, use the > > > PPL_WATCHDOG symbol. > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > --- > > > > > > (no changes since v1) > > > > > > arch/arm/mach-omap2/boot-common.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/arch/arm/mach-omap2/boot-common.c b/arch/arm/mach-omap2/boot-common.c > > > index 9a342a1bf95..401d12c6ca3 100644 > > > --- a/arch/arm/mach-omap2/boot-common.c > > > +++ b/arch/arm/mach-omap2/boot-common.c > > > @@ -302,7 +302,7 @@ void spl_board_init(void) > > > #if defined(CONFIG_AM33XX) && defined(CONFIG_SPL_MUSB_NEW) > > > arch_misc_init(); > > > #endif > > > -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) > > > +#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_PPL_WATCHDOG) > > > hw_watchdog_init(); > > > #endif > > > #ifdef CONFIG_AM33XX > > > > No, we want watchdog enabled in SPL. > > Or perhaps, the same question as in the socfpga patch also applies here. Yes and with a similar answer. By using PPL_WATCHDOG we ensure that the code is enabled in SPL as well as PPL. I am not sure what the plan is for getting rid of the old watchdog stuff. +Stefan Roese for that Regards, Simon
Hi Simon & Tom, On 2/22/23 20:16, Simon Glass wrote: > Hi Tom, > > On Tue, 14 Feb 2023 at 09:43, Tom Rini <trini@konsulko.com> wrote: >> >> On Tue, Feb 14, 2023 at 11:34:47AM -0500, Tom Rini wrote: >>> On Sun, Feb 12, 2023 at 04:16:08PM -0700, Simon Glass wrote: >>>> The logic here is strange since the call to hw_watchdog_init() depends on >>>> CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG but the code it calls is only >>>> enabled if !CONFIG_IS_ENABLED(WDT). This seems to work, but with split >>>> config we get a build error in SPL with CONFIG_WATCHDOG, since it is >>>> interpreted as CONFIG_SPL_WATCHDOG. >>>> >>>> To mimic the behaviour before split config takes effect, use the >>>> PPL_WATCHDOG symbol. >>>> >>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>>> --- >>>> >>>> (no changes since v1) >>>> >>>> arch/arm/mach-omap2/boot-common.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/arch/arm/mach-omap2/boot-common.c b/arch/arm/mach-omap2/boot-common.c >>>> index 9a342a1bf95..401d12c6ca3 100644 >>>> --- a/arch/arm/mach-omap2/boot-common.c >>>> +++ b/arch/arm/mach-omap2/boot-common.c >>>> @@ -302,7 +302,7 @@ void spl_board_init(void) >>>> #if defined(CONFIG_AM33XX) && defined(CONFIG_SPL_MUSB_NEW) >>>> arch_misc_init(); >>>> #endif >>>> -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) >>>> +#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_PPL_WATCHDOG) >>>> hw_watchdog_init(); >>>> #endif >>>> #ifdef CONFIG_AM33XX >>> >>> No, we want watchdog enabled in SPL. >> >> Or perhaps, the same question as in the socfpga patch also applies here. > > Yes and with a similar answer. By using PPL_WATCHDOG we ensure that > the code is enabled in SPL as well as PPL. > > I am not sure what the plan is for getting rid of the old watchdog stuff. > > +Stefan Roese for that Frankly, I don't know what PPL_WATCHDOG is. Searching the current code base shows nothing. As to the plans of getting rid of the old watchdog stuff (HW_WATCHDOG): My understanding is that this can't easily be removed. HW_WATCHDOG mostly addresses platforms which use (need to use) a very early watchdog driver. This can't be solved (easily) with the normal WDT UCLASS driver. This problem is similar to the early timer functionality AFAIU. Perhaps it makes sense to at least rename all this HW_WATCHDOG stuff to some "early" names instead. I'm not 100% sure here though. Ideas and comments are welcome. Thanks, Stefan
diff --git a/arch/arm/mach-omap2/boot-common.c b/arch/arm/mach-omap2/boot-common.c index 9a342a1bf95..401d12c6ca3 100644 --- a/arch/arm/mach-omap2/boot-common.c +++ b/arch/arm/mach-omap2/boot-common.c @@ -302,7 +302,7 @@ void spl_board_init(void) #if defined(CONFIG_AM33XX) && defined(CONFIG_SPL_MUSB_NEW) arch_misc_init(); #endif -#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) +#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_PPL_WATCHDOG) hw_watchdog_init(); #endif #ifdef CONFIG_AM33XX
The logic here is strange since the call to hw_watchdog_init() depends on CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG but the code it calls is only enabled if !CONFIG_IS_ENABLED(WDT). This seems to work, but with split config we get a build error in SPL with CONFIG_WATCHDOG, since it is interpreted as CONFIG_SPL_WATCHDOG. To mimic the behaviour before split config takes effect, use the PPL_WATCHDOG symbol. Signed-off-by: Simon Glass <sjg@chromium.org> --- (no changes since v1) arch/arm/mach-omap2/boot-common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)