Hi Tien Fong,

> 
> Please respin:
> - keep CONFIG_COUNTER_FREQUENCY default at 200000000 for SoCFPGA64, and
> - perform the 400MHz update at runtime in the clock/PLL bring-up path,
>   following the existing pattern in drivers/clk/altera/clk-agilex5.c
>   (around the PLL setup section near line ~403), i.e. only update the
>   counter frequency after the PLL is configured and the clock source has
>   switched.
> - Ensure correct namespace is used  CONFIG_ARCH_SOCFPGA_* .
> 

Thank you for the review. After checking current mainline, I agree
with your feedback and will drop this patch.

Some chronological background on how this change evolved on our side:
2023-09-05 — Initial downstream change (U-Boot v2023.04) Tracked as 
HSD #15014205886, "COUNTER_FREQUENCY is not set properly in U-Boot v2022.07 
above".

While debugging U-Boot v2023.04 on the emulator, we found that
COUNTER_FREQUENCY could no longer be set from
socfpga_soc64_common.h after upstream Kconfig migration.
It needed to be set via CONFIG_COUNTER_FREQUENCY
in Kconfig/defconfig instead.

As an emulator workaround at that time, the frequency
was bumped to 400 MHz.

Later rework — history was partially lost downstream
The change was reworked and merged via our internal
PR #336, and a static 400 MHz default was kept in places. 
hat is how we ended up with this v1 Kconfig-only patch,
which does not match the intended runtime
200 MHz (boot mode) → 400 MHz (after PLL) behavior you described.

Current mainline already has the correct split
1) Early/default counter frequency remains 200 MHz:
https://github.com/u-boot/u-boot/blob/
527115ef6783cec49e5610c523c124b399011361/arch/arm/Kconfig#L33

2_ Post-PLL 400 MHz update is done at runtime via
COUNTER_FREQUENCY_REAL:
https://github.com/u-boot/u-boot/blob/
527115ef6783cec49e5610c523c124b399011361/
drivers/clk/altera/clk-agilex.h#L14

https://github.com/u-boot/u-boot/blob/
527115ef6783cec49e5610c523c124b399011361/
drivers/clk/altera/clk-agilex5.c#L403

As you noted, defaulting CONFIG_COUNTER_FREQUENCY
to 400 MHz would break early timeouts/delays while still
on cs_at_clk at 200 MHz. Since mainline already keeps the
200 MHz default and updates cntfrq_el0 after PLL bring-up,
this v1 patch is redundant.

I will drop this patch, and we should drop in from our
downstream repo too.

Thank you again for the clarification.

Best regards,
Boon Khai

Reply via email to