Hello, 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? Thanks, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com