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

[Stefan Roese]

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?

Thanks,
Stefan