On 15/04/2024 16:08, Yann Sionneau wrote: > Hello Krzysztof, Arnd, all, > > On 1/22/23 12:54, Krzysztof Kozlowski wrote: >> On 20/01/2023 15:10, Yann Sionneau wrote: >>> From: Jules Maselbas <jmaselbas@xxxxxxxxx> >>> >>> The Power Controller (pwr-ctrl) control cores reset and wake-up >>> procedure. >>> + >>> +int __init kvx_pwr_ctrl_probe(void) >>> +{ >>> + struct device_node *ctrl; >>> + >>> + ctrl = get_pwr_ctrl_node(); >>> + if (!ctrl) { >>> + pr_err("Failed to get power controller node\n"); >>> + return -EINVAL; >>> + } >>> + >>> + if (!of_device_is_compatible(ctrl, "kalray,kvx-pwr-ctrl")) { >>> + pr_err("Failed to get power controller node\n"); >> No. Drivers go to drivers, not to arch directory. This should be a >> proper driver instead of some fake stub doing its own driver matching. >> You need to rework this. > > I am working on a v3 patchset, therefore I am working on a solution for > this "pwr-ctrl" driver that needs to go somewhere else than arch/kvx/. > > The purpose of this "driver" is just to expose a void > kvx_pwr_ctrl_cpu_poweron(unsigned int cpu) function, used by > kernel/smpboot.c function __cpu_up() in order to start secondary CPUs in > SMP config. I might be missing here some bigger picture and maybe my original comment was no appropriate, but IIUC, you might now create dependencies between arch code and drivers. That's also fragile. > > Doing this, on our SoC, requires writing 3 registers in a memory-mapped > device named "power controller". > > I made some researches in drivers/ but I am not sure yet what's a good > place that fits what our device is doing (booting secondary CPUs). > > * drivers/power/reset seems to be for resetting the entire SoC > > * drivers/power/supply seems to be to control power supplies ICs/periph. > > * drivers/reset seems to be for device reset > > * drivers/pmdomain maybe ? > > * drivers/soc ? > Bringup of CPU? Then I would vote for here. You also have existing example: r9a06g032-smp.c But anyway the point is to make it clear - either it is a driver or core code. Not both. The original code was not looking like any other CPU bringup code. Best regards, Krzysztof