Skip to content

Conversation

@najumon1980
Copy link
Contributor

@najumon1980 najumon1980 commented May 6, 2024

fixes for log_timestamp test case failer (issue #72344) due to timer PR regression.
The regression is due to following reason:
sys_clock_hw_cycles_per_sec() used in log_core_init() which invoked during cstart (EARLY init) but HPET driver init is PRE_KERNEL_2. So the z_clock_hw_cycles_per_sec need to init with some default CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC (though it is not right) or ideally need to init z_clock_hw_cycles_per_sec run time during cstart (EARLY init).

Current Fixes: put back the older code for static initialization for z_clock_hw_cycles_per_sec with CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC. This will be later update with run time during HPTET driver init (PRE_KERNEL2) .

fixes for log_timestamp test case failer (issue zephyrproject-rtos#72344) due
to timer PR regression. The regression is due to following reason:
sys_clock_hw_cycles_per_sec() used in log_core_init() which invoked
during cstart (EARLY init) but HPET driver init is PRE_KERNEL_2.
So the z_clock_hw_cycles_per_sec need to init with some default
CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC (though it is not right) or
ideally need to init z_clock_hw_cycles_per_sec run time during
cstart (EARLY init). Current Fixes: put back the older code for
static initialization for z_clock_hw_cycles_per_sec with
CONFIG_SYS_CLOCK_HW_CYCLES_PER_SEC. This will be later update
with run time during HPTET driver init (PRE_KERNEL2).

Signed-off-by: Najumon B.A <najumon.ba@intel.com>
add a static fallback option for timer clock frequency (HW clock
cycle per sec) during early init. Later this will update once
retrieve runtime clock frequency

Signed-off-by: Najumon B.A <najumon.ba@intel.com>

config SYS_CLOCK_HW_CYCLES_PER_SEC
int "System clock's h/w timer frequency"
depends on !TIMER_READS_ITS_FREQUENCY_AT_RUNTIME
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is sort of an abuse. The idea behind the dependency is to prevent apps from accidentally using the "static" clock rate on devices where it might be wrong because the actual rate is selected by the hardware.

Copy link
Contributor

@andyross andyross left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does logging need the time for? Just a timestamp? We might fix it there by detecting a zero denominator and presenting a zero/"early" timestamp to the user. That's what Linux does for very early logs.

@andyross
Copy link
Contributor

andyross commented May 6, 2024

Also: how does HPET get around this? I don't think it initializes before z_cstart() either.

@github-actions
Copy link

github-actions bot commented Jul 6, 2024

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jul 6, 2024
@github-actions github-actions bot closed this Jul 20, 2024
kartben added a commit to kartben/zephyr that referenced this pull request Dec 4, 2025
Last activity: 18 months ago (review)

Recent activities:

- review: Review on 'x86: ia32: linker: move bss/noinit sections after data
  section'
  zephyrproject-rtos#73044
  (2024-05-22T06:47:31Z)
- pr: kernel: timer: fixes for log_timestamp test case failer due to timer
  PR regression
  zephyrproject-rtos#72352
  (2024-05-06T10:47:53Z)
- review: Review on 'Revert 69705 and fix zephyrproject-rtos#72344'
  zephyrproject-rtos#72345
  (2024-05-06T09:53:31Z)

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants