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

[Stefan Roese]

Hi Simon,

On 30.09.22 15:28, Simon Glass wrote:
> 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.

Ok, I'll move forward this way then.

Thanks,
Stefan