Hello Krzysztof, On 22/07/2024 14:37, Krzysztof Kozlowski wrote: > On 22/07/2024 11:41, ysionneau@xxxxxxxxxxxxx wrote: >> From: Yann Sionneau <ysionneau@xxxxxxxxxxxxx> >> >> The Power Controller (pwr-ctrl) controls cores reset and wake-up >> procedure. >> [...] >> + >> +static bool pwr_ctrl_not_initialized = true; > Do not use inverted meanings. Ack, I will fix this. > >> + >> +/** >> + * kvx_pwr_ctrl_cpu_poweron() - Wakeup a cpu >> + * @cpu: cpu to wakeup >> + */ >> +int __init kvx_pwr_ctrl_cpu_poweron(unsigned int cpu) >> +{ >> + int ret = 0; >> + >> + if (pwr_ctrl_not_initialized) { >> + pr_err("KVX power controller not initialized!\n"); >> + return -ENODEV; >> + } >> + >> + /* Set PE boot address */ >> + writeq((unsigned long long)kvx_start, > Addresses use kernel_ulong_t Ack, I will fix this. > >> + kvx_pwr_controller.regs + KVX_PWR_CTRL_RESET_PC_OFFSET); >> + /* Wake up processor ! */ >> + writeq(1ULL << cpu, > That's BIT Ack, I will fix this and replace with BITULL(cpu). > >> + kvx_pwr_controller.regs + PWR_CTRL_WUP_SET_OFFSET); >> + /* Then clear wakeup to allow processor to sleep */ >> + writeq(1ULL << cpu, > BIT Ack. > >> + kvx_pwr_controller.regs + PWR_CTRL_WUP_CLEAR_OFFSET); >> + >> + return ret; >> +} >> + >> +static const struct smp_operations coolidge_smp_ops __initconst = { >> + .smp_boot_secondary = kvx_pwr_ctrl_cpu_poweron, >> +}; >> + >> +static int __init kvx_pwr_ctrl_probe(void) > That's not a probe, please rename to avoid confusion. Or make it a > proper device driver. Ok, I will probably rename it kvx_pwr_ctrl_init() Thanks! -- Yann