Re: [PATCH 04/10] timer: cadence-ttc: Add timer_early functions

[Michal Simek <michal.simek@amd.com>]

Hi Simon and Stefan,

On 9/28/22 03:54, Simon Glass wrote:
>> Hi Simon,
>> Hi Michal,
>>
>> On 25.09.22 16:15, Simon Glass wrote:
>>> Hi Stefan,
>>>
>>> On Wed, 21 Sept 2022 at 08:06, Stefan Roese <sr@denx.de> wrote:
>>>>
>>>> Currently this timer driver provides timer_get_boot_us() to support the
>>>> BOOTSTAGE functionality. This patch adds the timer_early functions so
>>>> that the "normal" timer functions can be used, when CONFIG_TIMER_EARLY
>>>> is enabled.
>>>>
>>>> timer_get_boot_us() will get removed in a follow-up patch, once the
>>>> BOOTSTAGE interface is migrated to timer_get_us().
>>>>
>>>> Signed-off-by: Stefan Roese <sr@denx.de>
>>>> Cc: Michal Simek <michal.simek@xilinx.com>
>>>> ---
>>>>    drivers/timer/cadence-ttc.c | 25 +++++++++++++++++++++++++
>>>>    1 file changed, 25 insertions(+)
>>>>
>>>> diff --git a/drivers/timer/cadence-ttc.c b/drivers/timer/cadence-ttc.c
>>>> index 2eff45060ad6..e26c7923a140 100644
>>>> --- a/drivers/timer/cadence-ttc.c
>>>> +++ b/drivers/timer/cadence-ttc.c
>>>> @@ -58,6 +58,31 @@ ulong timer_get_boot_us(void)
>>>>    }
>>>>    #endif
>>>>
>>>> +unsigned long notrace timer_early_get_rate(void)
>>>> +{
>>>> +       return 1;
>>>> +}
>>>> +
>>>> +u64 notrace timer_early_get_count(void)
>>>> +{
>>>> +       u64 ticks = 0;
>>>> +       u32 rate = 1;
>>>> +       u64 us;
>>>> +       int ret;
>>>> +
>>>> +       ret = dm_timer_init();
>>>
>>> I don't think you can call this if you want to support bootstage,
>>> since driver model may not be inited.
>>
>> Yes, thanks for noticing. Still, this code is copied from the original
>> timer_get_boot_us() function in this driver. Which also has problems
>> with early timer access AFAICT.
> 
> Yes, good point.
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>

I looked at it and kind of interesting code. For getting this working with this 
whole series if ttc gets u-boot,dm-pre-reloc everthing is working fine.

diff --git a/arch/arm/dts/zynqmp-r5.dts b/arch/arm/dts/zynqmp-r5.dts
index a72172ef2ea4..8059931f2162 100644
--- a/arch/arm/dts/zynqmp-r5.dts
+++ b/arch/arm/dts/zynqmp-r5.dts
@@ -56,6 +56,7 @@

                 ttc0: timer@ff110000 {
                         compatible = "cdns,ttc";
+                       u-boot,dm-pre-reloc;
                         status = "okay";
                         reg = <0xff110000 0x1000>;
                         timer-width = <32>;

What I see based on behavior. Every bootstage call is asking for 
timer_early_get_count() and because dm_timer_init() is failing 0 is returned.

Inside initf_dm() dm_init_and_scan() is called and initialized timer uclass also 
with timer instance. With u-boot,dm-pre-reloc also cadence timer driver.
On the next dm_timer_init() gd->timer is initialized and value is returned and 
initf_dm() passes.

Here is the log and output from bootstage.

U-Boot 2022.10-rc5-00130-g1be5bed207c9 (Sep 30 2022 - 14:00:08 +0200)

Model: Xilinx ZynqMP R5
DRAM:  512 MiB
Core:  7 devices, 7 uclasses, devicetree: embed
MMC:
Loading Environment from nowhere... OK
In:    serial@ff010000
Out:   serial@ff010000
Err:   serial@ff010000
Net:   No ethernet found.
ZynqMP r5> dm tree
  Class     Index  Probed  Driver                Name
-----------------------------------------------------------
  root          0  [ + ]   root_driver           root_driver
  clk           0  [ + ]   fixed_clock           |-- clk100
  simple_bus    0  [ + ]   simple_bus            |-- amba
  timer         0  [ + ]   cadence_ttc           |   |-- timer@ff110000
  serial        0  [ + ]   serial_zynq           |   `-- serial@ff010000
  bootstd       0  [   ]   bootstd_drv           `-- bootstd
  bootmeth      0  [   ]   bootmeth_distro           `-- distro
ZynqMP r5> bootstage report
Timer summary in microseconds (8 records):
        Mark    Elapsed  Stage
           0          0  reset
           0          0  board_init_f
           0          0  dm_f
           0          0  dm_r
      16,319     16,319  eth_common_init
      18,061      1,742  main_loop
      18,281        220  cli_loop
      29,249     10,968  board_init_r

Accumulated time:

Would be good if you can also add u-boot,dm-pre-reloc before 9/10 patch which 
enables CONFIG_TIMER_EARLY.

Thanks,
Michal