Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The `source` argument in `apic_set_timer_period_hz()` and `apic_set_timer_period_khz()` is completely unused, leading to dead code and lost debug logging. --
commit a7b2ed4060ee2d44136d35fad056804351b581ca Author: Sean Christopherson <[email protected]> x86/apic: Add CONFIG_X86_LOCAL_APIC=n stubs for apic_set_timer_period_{,k}hz() This commit adds stubs for apic_set_timer_period_hz() and apic_set_timer_period_khz() when the kernel is built without local APIC support. It also removes the corresponding #ifdef guards from the calling code in various x86 subsystems. > diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h > index cd84a94688a2f..035998555e992 100644 > --- a/arch/x86/include/asm/apic.h > +++ b/arch/x86/include/asm/apic.h > @@ -189,6 +189,8 @@ static inline void disable_local_APIC(void) { } > # define setup_boot_APIC_clock x86_init_noop > # define setup_secondary_APIC_clock x86_init_noop > static inline void lapic_update_tsc_freq(void) { } > +static inline void apic_set_timer_period_hz(u64 period_hz, const char > *source) { } > +static inline void apic_set_timer_period_khz(u64 period_khz, const char > *source) { } [Severity: Medium] Is it intentional that the source argument is completely unused here in the stubs? Looking at the main implementation in arch/x86/kernel/apic/apic.c: void apic_set_timer_period_khz(u64 period_khz, const char *source) { lapic_timer_period = mul_u64_u32_div(period_khz, 1000, HZ); } The source parameter is discarded without being used for debug logging. Since any code calling apic_set_timer_period_hz() or apic_set_timer_period_khz() passes a string literal describing the source (such as "VMware hypervisor" or "CPUID 0x15/0x16"), this results in dropped debug information. Could the source parameter be removed from the API entirely to clean up the dead code, or should the boot-time logging be restored? > static inline void init_bsp_APIC(void) { } > static inline void apic_intr_mode_select(void) { } > static inline void apic_intr_mode_init(void) { } [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
