Hello, On Tue May 7, 2024 at 4:52 PM CEST, Théo Lebrun wrote: > On Sat May 4, 2024 at 4:34 AM CEST, Stephen Boyd wrote: > > Quoting Théo Lebrun (2024-05-03 07:20:45) > > > This builds on previous EyeQ5 system-controller revisions[0], supporting > > > EyeQ5, EyeQ6L and EyeQ6H. We expose a few OLB system-controller > > > features here: > > > - Clocks: some read-only PLLs derived from main crystal and some > > > divider clocks based on PLLs. > > > - Resets. > > > - Pin controller, only on EyeQ5 (rest will use generic pinctrl-single). > > > > > > EyeQ6H is special in that it has seven instances of this > > > system-controller. Those are spread around and cannot be seen as a > > > single device, hence are exposed as seven DT nodes and seven > > > compatibles. > > > > > > This revision differs from previous in that it exposes all devices as a > > > single DT node. Driver-wise, a MFD registers multiple cells for each > > > device. Each driver is still in isolation from one another, each in > > > their respective subsystem. > > > > Why can't you use auxiliary device and driver APIs? > > Good question. Reasons I see: > > - I didn't know about auxdev beforehand. I discussed the rework with a > few colleagues and none mentioned it either. > > - It feels simpler to let each device access iomem resources. From my > understanding, an auxdev is supposed to make function calls to its > parent without inheriting iomem access. That sounds like it will put > the register logic/knowledge inside a single driver, which could or > could not be a better option. > > Implementing a function like this feels like cheating: > int olb_read(struct device *dev, u32 offset, u32 *val); > > With an MFD, we hand over a part of the iomem resource to each child > and they deal with it however they like. > > - Syscon is what I picked to share parts of OLB to other devices that > need it. Currently that is only for I2C speed mode but other devices > have wrapping-related registers. MFD and syscon are deeply connected > so an MFD felt natural. > > - That would require picking one device that is platform driver, the > rest being all aux devices. Clock driver appears to be the one, same > as two existing mpfs and starfive-jh7110 that use auxdev for clk and > reset. > > Main reason I see for picking auxdev is that it forces devices to > interact with a defined internal API. That can lead to nicer > abstractions rather than inheriting resources as is being done in MFD. > > Are there other reasons? Self replying myself. I gave myself some time to think about that but I still have more thought now that I've written the previous email, and re-read almost all old revisions of this series. I do like this auxdev proposal. More so than current MFD revision. One really nice feature is that it centralises access to iomem. I've noticed recently a register that has most its fields for reset but one lost bit dealing with a clock mux. Logic to handle that would be in one location. Also, I just noticed you hinted at auxiliary devices in previous emails, which I thought was a generic term. I did not see it as a specific kernel infrastructure to be used. Sorry about that. Regards, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
