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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7eeef2c6-7375-4e41-aad6-ca0a39e95e2e@linaro.org>
Date: Wed, 17 Apr 2024 21:20:53 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Yann Sionneau <ysionneau@...rayinc.com>,
 Yann Sionneau <ysionneau@...ray.eu>, Arnd Bergmann <arnd@...db.de>,
 Jonathan Corbet <corbet@....net>, Thomas Gleixner <tglx@...utronix.de>,
 Marc Zyngier <maz@...nel.org>, Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Will Deacon <will@...nel.org>, Peter Zijlstra <peterz@...radead.org>,
 Boqun Feng <boqun.feng@...il.com>, Mark Rutland <mark.rutland@....com>,
 Eric Biederman <ebiederm@...ssion.com>, Kees Cook <keescook@...omium.org>,
 Oleg Nesterov <oleg@...hat.com>, Ingo Molnar <mingo@...hat.com>,
 Waiman Long <longman@...hat.com>,
 "Aneesh Kumar K.V" <aneesh.kumar@...ux.ibm.com>,
 Andrew Morton <akpm@...ux-foundation.org>, Nick Piggin <npiggin@...il.com>,
 Paul Moore <paul@...l-moore.com>, Eric Paris <eparis@...hat.com>,
 Christian Brauner <brauner@...nel.org>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 Jules Maselbas <jmaselbas@...ray.eu>,
 Guillaume Thouvenin <gthouvenin@...ray.eu>,
 Clement Leger <clement@...ment-leger.fr>,
 Vincent Chardon <vincent.chardon@...ys-design.com>,
 Marc Poulhiès <dkm@...aplop.net>,
 Julian Vetter <jvetter@...ray.eu>, Samuel Jones <sjones@...ray.eu>,
 Ashley Lesdalons <alesdalons@...ray.eu>, Thomas Costis <tcostis@...ray.eu>,
 Marius Gligor <mgligor@...ray.eu>, Jonathan Borne <jborne@...ray.eu>,
 Julien Villette <jvillette@...ray.eu>, Luc Michel <lmichel@...ray.eu>,
 Louis Morhet <lmorhet@...ray.eu>, Julien Hascoet <jhascoet@...ray.eu>,
 Jean-Christophe Pince <jcpince@...il.com>,
 Guillaume Missonnier <gmissonnier@...ray.eu>, Alex Michon
 <amichon@...ray.eu>, Huacai Chen <chenhuacai@...nel.org>,
 WANG Xuerui <git@...0n.name>, Shaokun Zhang <zhangshaokun@...ilicon.com>,
 John Garry <john.garry@...wei.com>,
 Guangbin Huang <huangguangbin2@...wei.com>,
 Bharat Bhushan <bbhushan2@...vell.com>, Bibo Mao <maobibo@...ngson.cn>,
 Atish Patra <atishp@...shpatra.org>, "Jason A. Donenfeld" <Jason@...c4.com>,
 Qi Liu <liuqi115@...wei.com>, Jiaxun Yang <jiaxun.yang@...goat.com>,
 Catalin Marinas <catalin.marinas@....com>, Mark Brown <broonie@...nel.org>,
 Janosch Frank <frankja@...ux.ibm.com>, Alexey Dobriyan <adobriyan@...il.com>
Cc: Benjamin Mugnier <mugnier.benjamin@...il.com>, linux-doc@...r.kernel.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 linux-mm@...ck.org, linux-arch@...r.kernel.org, linux-audit@...hat.com,
 linux-riscv@...ts.infradead.org, bpf@...r.kernel.org
Subject: Re: [RFC PATCH v2 30/31] kvx: Add power controller driver

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@...ray.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.

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