Re: [RFC PATCH v2 30/31] kvx: Add power controller driver

From: "Arnd Bergmann" <arnd@arndb.de>
To: "Yann Sionneau" <ysionneau@kalrayinc.com>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>,
	"Yann Sionneau" <ysionneau@kalray.eu>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Marc Zyngier" <maz@kernel.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Will Deacon" <will@kernel.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	"Kees Cook" <keescook@chromium.org>,
	"Oleg Nesterov" <oleg@redhat.com>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Waiman Long" <longman@redhat.com>,
	"Aneesh Kumar" <aneesh.kumar@linux.ibm.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Nicholas Piggin" <npiggin@gmail.com>,
	"Paul Moore" <paul@paul-moore.com>,
	"Eric Paris" <eparis@redhat.com>,
	"Christian Brauner" <brauner@kernel.org>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Jules Maselbas" <jmaselbas@kalray.eu>,
	"Guillaume Thouvenin" <gthouvenin@kalray.eu>,
	"Clement Leger" <clement@clement-leger.fr>,
	"Vincent Chardon" <vincent.chardon@elsys-design.com>,
	"Marc Poulhiès" <dkm@kataplop.net>,
	"Julian Vetter" <jvetter@kalray.eu>,
	"Samuel Jones" <sjones@kalray.eu>,
	"Ashley Lesdalons" <alesdalons@kalray.eu>,
	"Thomas Costis" <tcostis@kalray.eu>,
	"Marius Gligor" <mgligor@kalray.eu>,
	"Jonathan Borne" <jborne@kalray.eu>,
	"Julien Villette" <jvillette@kalray.eu>,
	"Luc Michel" <lmichel@kalray.eu>,
	"Louis Morhet" <lmorhet@kalray.eu>,
	"Julien Hascoet" <jhascoet@kalray.eu>,
	"Jean-Christophe Pince" <jcpince@gmail.com>,
	"Guillaume Missonnier" <gmissonnier@kalray.eu>,
	"Alex Michon" <amichon@kalray.eu>,
	"Huacai Chen" <chenhuacai@kernel.org>,
	"WANG Xuerui" <git@xen0n.name>,
	"Shaokun Zhang" <zhangshaokun@hisilicon.com>,
	"John Garry" <john.garry@huawei.com>,
	"Guangbin Huang" <huangguangbin2@huawei.com>,
	"Bharat Bhushan" <bbhushan2@marvell.com>,
	"Bibo Mao" <maobibo@loongson.cn>,
	"Atish Patra" <atishp@atishpatra.org>,
	"Jason A . Donenfeld" <Jason@zx2c4.com>,
	"Qi Liu" <liuqi115@huawei.com>,
	"Jiaxun Yang" <jiaxun.yang@flygoat.com>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Mark Brown" <broonie@kernel.org>,
	"Janosch Frank" <frankja@linux.ibm.com>,
	"Alexey Dobriyan" <adobriyan@gmail.com>
Cc: "Benjamin Mugnier" <mugnier.benjamin@gmail.com>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linux-mm@kvack.org,
	Linux-Arch <linux-arch@vger.kernel.org>,
	linux-audit@redhat.com, linux-riscv@lists.infradead.org,
	bpf@vger.kernel.org
Subject: Re: [RFC PATCH v2 30/31] kvx: Add power controller driver
Date: Mon, 15 Apr 2024 17:30:31 +0200	[thread overview]
Message-ID: <61d0584f-dfe4-4d8c-a178-78f000d477d4@app.fastmail.com> (raw)
In-Reply-To: <c20b433f-97ef-7faa-5122-9949af41f2fb@kalrayinc.com>

On Mon, Apr 15, 2024, at 16:08, Yann Sionneau wrote:
> On 1/22/23 12:54, Krzysztof Kozlowski wrote:
>> On 20/01/2023 15:10, Yann Sionneau wrote:
>>> From: Jules Maselbas <jmaselbas@kalray.eu>
>>>
>>> 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.
>
> 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 ?

Right, I don't think any of the above are appropriate

> * drivers/soc ?
>
> * drivers/platform ?
>
> * drivers/misc ?

Not drivers/misc, that is mainly for things with a user-space
interface.

drivers/soc is mainly for drivers used by other drivers, but
this would work, especially if you expect to have multiple
SoC variants that all use the same architecture code but
incompatible register layouts

drivers/platform is really for things outside of the SoC
that are used for managing the system, especially across
architectures, so I don't think that is a good fit.

Traditionally we had this code in arch/{arm,mips,powerpc,sh,x86}
and we never created a drier subsystem for it since newer
targets (arm64, riscv, newer arm, most x86) all use a method
that is specified as part of the ISA or firmware interface.

The question what you expect to see with future hardware
iterations: if you think all arch/kvx/ hardware will use the
same code for maybe at least the next five years, I would
suggest you keep it in arch/kvx/kernel/smp.c, but if you
know or expect other implementations to be needed, I can
merge it as a driver through drivers/soc/.

     Arnd