[PATCH 07/10] board_f/r: Allow selection of CONFIG_TIMER_EARLY w/o CONFIG_TIMER

[Simon Glass]

Hi Stefan,

On Thu, 29 Sept 2022 at 23:36, Stefan Roese <sr at denx.de> wrote:
>
> Hi Simon,
>
> On 28.09.22 03:54, Simon Glass wrote:
> > Hi Stefan,
> >
> > On Mon, 26 Sept 2022 at 07:52, Stefan Roese <sr at denx.de> wrote:
> >>
> >> Hi Simon,
> >>
> >> On 25.09.22 16:15, Simon Glass wrote:
> >>> Hi Stefan,
> >>>
> >>> On Wed, 21 Sept 2022 at 08:06, Stefan Roese <sr at denx.de> wrote:
> >>>>
> >>>> The early timer functions provided via CONFIG_TIMER_EARLY don't need
> >>>> CONFIG_TIMER to be enabled, as they don't make use of the DM timer
> >>>> and uclass interface. This patch now allow the selection of
> >>>> CONFIG_TIMER_EARLY w/o CONFIG_TIMER, enabling this early timer
> >>>> functionality also for non CONFIG_TIMER drivers.
> >>>>
> >>>> With this change it's necessary to guard the dm_timer_init() call
> >>>> in initr_dm_devices() & initf_dm() additionally via CONFIG_TIMER.
> >>>>
> >>>> Signed-off-by: Stefan Roese <sr at denx.de>
> >>>> ---
> >>>>    common/board_f.c      | 2 +-
> >>>>    common/board_r.c      | 2 +-
> >>>>    drivers/timer/Kconfig | 1 -
> >>>>    3 files changed, 2 insertions(+), 3 deletions(-)
> >>>
> >>> I don't like this as it complicates the logic and also seems to be
> >>> adding a new feature to legacy code.
> >>>
> >>> Instead, let's enable the early timer only for driver model.
> >>
> >> Hmmm, not sure how this should work. Do you have this in mind (instead
> >> of this patch)?
> >>
> >> diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
> >> index fd8745ffc2e0..30d6efe98f29 100644
> >> --- a/drivers/timer/Kconfig
> >> +++ b/drivers/timer/Kconfig
> >> @@ -39,7 +39,7 @@ config VPL_TIMER
> >>
> >>    config TIMER_EARLY
> >>           bool "Allow timer to be used early in U-Boot"
> >> -       depends on TIMER
> >> +       depends on DM
> >>           # initr_bootstage() requires a timer and is called before
> >> initr_dm()
> >>           # so only the early timer is available
> >>           default y if X86 && BOOTSTAGE
> >>
> >> This results in some compilation errors, like this:
> >>
> >> $ make stm32mp15_basic_defconfig
> >> $ make -sj
> >> /opt/kernel.org/gcc-11.1.0-nolibc/arm-linux-gnueabi/bin/arm-linux-gnueabi-ld.bfd:
> >> common/board_f.o: in function `initf_dm':
> >> /home/stefan/git/u-boot/u-boot-marvell/common/board_f.c:791: undefined
> >> reference to `dm_timer_init'
> >> /opt/kernel.org/gcc-11.1.0-nolibc/arm-linux-gnueabi/bin/arm-linux-gnueabi-ld.bfd:
> >> common/board_r.o: in function `initr_dm_devices':
> >> /home/stefan/git/u-boot/u-boot-marvell/common/board_r.c:258: undefined
> >> reference to `dm_timer_init'
> >> make: *** [Makefile:1790: u-boot] Error 1
> >>
> >> I might be missing something. Or it's not that easy and we still need
> >> my original implementation.
> >
> > Well, given that TIMER depends on DM, I don't think you need to change that.
> >
> > Also, TIMER_EARLY depends on TIMER (as it should), you should just be
> > able to check for IS_ENABLED(CONFIG_TIMER)
> >
> > We can disable the early timer stuff for things that don't use CONFIG_TIMER
>
> The result of this would be, that BOOTSTAGE is not available for
> platforms that don't have TIMER_EARLY (& TIMER) enabled, like
> stm32mp15_basic and some others. Actually I think this is a good
> move, as it "encourages" people to move to enable CONFIG_TIMER.
>
> So should I move forward this way? Depending BOOTSTAGE on TIMER?

Yes I think soas it is simplified things and we want people to migrate anyway.

Regards,
Simon