Quoting Greg Kroah-Hartman (2021-10-06 22:37:40) > On Wed, Oct 06, 2021 at 12:37:47PM -0700, Stephen Boyd wrote: > > > > Let's make the component driver into an actual device driver that has > > probe/remove/shutdown functions. The driver will only be bound to the > > aggregate device once all component drivers have called component_add() > > to indicate they're ready to assemble the aggregate driver. This allows > > us to attach shutdown logic (and in the future runtime PM logic) to the > > aggregate driver so that it runs the hooks in the correct order. > > Why are you creating a new bus type and not using the auxiliary bus > instead? > > You have seen Documentation/driver-api/auxiliary_bus.rst, right? > Nope, but I read it now. Thanks for the pointer. My read of it is that the auxiliary bus is a way to slice up a single IP block into multiple devices and then have drivers attach to those different "pieces" of the IP. It avoids polluting the platform bus with devices that don't belong on the platform bus because they are sub components of a larger IP block that sits on the platform bus. The aggregate bus is solving the reverse problem. It is rounding up a collection of IP blocks that live on some bus (platform, i2c, spi, whatever) and presenting them as a single aggregate device (sound card, display card, whatever) whenever all the component devices call component_add(). For example, we don't want to do operations on the entire display pipeline until all the devices that make up the display are probed and drivers are attached. I suppose the aggregate_device in this patch series has a 1:1 relationship with the drm class_type that makes up /sys/class/drm/cardN but there's also a couple sound users and a power_supply user so I don't know the equivalent there. Long term, maybe all of this component code could be placed directly into the driver core? That's probably even more invasive of a change but I imagine we could make device links with component_add() as we're already doing with these patches and then have driver core call some class function pointer when all the links are probed. That would handle the 'bind/probe' callback for the aggregate device but it won't handle the component_bind_all() path where we call bind_component() for each component device that makes up the aggregate device. Maybe we can add even more devices for the components and then call probe there too. Sorry that's a long-winded non-answer. I don't think they're solving the same problem so using the same bus type looks wrong. We'd have to take two different paths depending on what type of device it is (aggregate vs. auxiliary) so there's not much of anything that is shared code-wise.
