Re: [PATCH v1] drivers: clk: agilex5: Setting correct counter frequency in boot mode

"NG, BOON KHAI via U-Boot" <[email protected]>
Newsgroups gmane.comp.boot-loaders.u-boot.general,gmane.comp.boot-loaders.u-boot
Message-ID <SA3PR03MB71875487D1DA481E25FBC552B4DA2@SA3PR03MB7187.namprd03.prod.outlook.com>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.