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
