On 30/08/2024 10:52 am, Roger Pau Monne wrote: > Split the logic that deals with probing and fetching the CMOS time into a > separate helper. While moving the code also take the opportunity to reduce > the > scope of some local variables. > > No functional change intended. > > Signed-off-by: Roger Pau Monné <[email protected]>
This does look like a straight rearrangement, so Acked-by: Andrew Cooper <[email protected]>, although... > +static unsigned long get_cmos_time(void) > +{ > + struct rtc_time rtc; > + static bool __read_mostly cmos_rtc_probe; > + boolean_param("cmos-rtc-probe", cmos_rtc_probe); > + > + if ( efi_enabled(EFI_RS) ) > + { > + unsigned long res = efi_get_time(); > + > + if ( res ) > + return res; > + } > + > + if ( likely(!(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC)) ) > + cmos_rtc_probe = false; > + else if ( system_state < SYS_STATE_smp_boot && !cmos_rtc_probe ) > + panic("System with no CMOS RTC advertised must be booted from EFI" > + " (or with command line option \"cmos-rtc-probe\")\n"); > + > + if ( unlikely(!read_cmos_time(&rtc, cmos_rtc_probe)) ) > panic("No CMOS RTC found - system must be booted from EFI\n"); ... this really does show some (preexisting) tangled logic. All of this should live in an init function, and not be re-evaluated each time we call get_wallclock_time(), not that we call it very often. cmos_rtc_probe should be __initdata or at least __ro_after_init, but it gets written on every pass through the function on most systems. Lets focus on not crashing. Cleanup can come later. ~Andrew
