On 11.03.2025 17:19, Oleksii Kurochko wrote: > --- /dev/null > +++ b/xen/arch/riscv/time.c > @@ -0,0 +1,38 @@ > +#include <xen/device_tree.h> > +#include <xen/init.h> > +#include <xen/lib.h> > +#include <xen/sections.h> > + > +unsigned long __ro_after_init cpu_khz; /* CPU clock frequency in kHz. */ > +unsigned long __read_mostly boot_count;
Why not also __ro_after_init? And what is this variable actually needed for? Common code doesn't use it, so a better name (describing what it really holds) might be desirable, even if this then means not being in sync with Arm code. Furthermore, I can't spot a declaration of this variable. Was it meant to be static? > +static __initdata struct dt_device_node *timer; > + > +/* Set up the timer on the boot CPU (early init function) */ > +static void __init preinit_dt_xen_time(void) > +{ > + static const struct dt_device_match __initconst timer_ids[] = Nit: Stray double blank. > + { > + DT_MATCH_PATH("/cpus"), > + { /* sentinel */ }, > + }; > + u32 rate; Nit: uint32_t please in new code. > + timer = dt_find_matching_node(NULL, timer_ids); > + if ( !timer ) > + panic("Unable to find a compatible timer in the device tree\n"); > + > + dt_device_set_used_by(timer, DOMID_XEN); > + > + if ( !dt_property_read_u32(timer, "timebase-frequency", &rate) ) > + panic("Unable to find clock frequency.\n"); Please be consistent with the full stop in panic messages. (I think there shouldn't be any.) Jan