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@xxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxx> > > --- > > (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.
Attachment:
signature.asc
Description: PGP signature
