linux-kernel - Re: [RFC PATCH 06/11] dt-bindings: soc: microchip: document the two simple-mfd syscons on PolarFire SoC

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: <20240815-afloat-baton-77a6d4e47b18@spud>
Date: Thu, 15 Aug 2024 21:42:22 +0100
From: Conor Dooley <conor@...nel.org>
To: Rob Herring <robh@...nel.org>
Cc: devicetree@...r.kernel.org, Conor Dooley <conor.dooley@...rochip.com>,
	Lee Jones <lee@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 06/11] dt-bindings: soc: microchip: document the two
 simple-mfd syscons on PolarFire SoC

On Thu, Aug 15, 2024 at 02:00:03PM -0600, Rob Herring wrote:
> On Thu, Aug 15, 2024 at 03:01:09PM +0100, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@...rochip.com>
> > 
> > There are two syscons on PolarFire SoC that provide various functionality of
> > use to the OS.
> > 
> > The first of these is the "control-scb" region, that contains the "tvs"
> > temperature and voltage sensors and the control/status registers for the
> > system controller's mailbox. The mailbox has a dedicated node, so
> > there's no need for a child node describing it, looking the syscon up by
> > compatible is sufficient.
> > 
> > The second, "mss-top-sysreg", contains clocks, pinctrl, resets, and
> > interrupt controller and more. For this RFC, only the reset controller
> > child is described as that's all that is described by the existing
> > bindings. The clock controller already has a dedicated node, and will
> > retain it as there are other clock regions, so like the mailbox,
> > a compatible-based lookup of the syscon is sufficient to keep the clock
> > driver working as before so no child is needed.
> 
> I'm confused. The reset controller is reused from somewhere else?

There's already a driver for it on this device, but probed via the
auxiliary bus, and the #reset-cells property is in the clock controller
node. The only devices that use this driver are the various different
logic element SKUs (which all share a compatible, they're identical as
far as an OS is concerned) and an upcoming SoC that is effectively a
zero logic element SKU.

> I 
> thought you didn't expect any reuse of the IP happening.

> If a child node 
> makes it possible to enable the h/w without any s/w changes, then that 
> is a compelling argument for having a child node.

No, in both cases there'd be software changes - they're just simpler
with a child node.

> 
> > Signed-off-by: Conor Dooley <conor.dooley@...rochip.com>
> > ---
> > (I'll split this in two later, it's just easier when I have the same
> > questions about both...)
> > 
> > Are these things entitled to have child nodes for the reset and sensor
> > nodes, or should the properties be in the parent and the OS probe the
> > drivers for the functions? That's something that, despite supposedly
> > being a maintainer, I do not understand the rules (of thumb?) for.
> 
> Besides the is it an independent, reusable IP block test, my test 
> generally is do the child nodes have their own DT resources? Say 
> you have phy registers mixed in some syscon and clocks which only go to 
> the phy. Then a child node with "clocks" makes sense. If your only 
> property is #phy-cells, then a child node doesn't make sense. Of course 
> you could reach different conclusions based on the completeness of the 
> binding.

AFAIK, none of these things are consumers of resources like that, other
than the interrupt controller, which has an interrupts property. I think
that could justify a child node (and I think a dedicated binding,
because it is a confusing irq mux that that kernel doesn't appear to
have anything else similar to).

> > Secondly, is it okay to make the "pragmatic" decision to not have a
> > child clock node and keep routing the clocks via the existing & retained
> > clock node (and therefore not update the various clocks nodes in the
> > consumers)? Doing so would require a lot more hocus pocus with the clock
> > driver than this series does, as the same driver would no longer be
> > suitable for the before/after bindings.
> 
> In the 2 cases here, I don't think you need child nodes. I would expect 
> pinctrl to have one though if only as a container for all the pinctrl 
> child nodes.

Good to know for when that gets written. Hopefully not by me, I have
enough messes to sort out as is!

Cheers,
Conor.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)