On Mon, Jan 4, 2021 at 4:14 PM Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > > On Mon, Jan 04, 2021 at 09:19:30PM +0000, Mark Brown wrote: > > > > > Regardless of the shortcut to make everything a struct > > > platform_device, I think it was a mistake to put OF devices on > > > platform_bus. Those should have remained on some of_bus even if they > > > > Like I keep saying the same thing applies to all non-enumerable buses - > > exactly the same considerations exist for all the other buses like I2C > > (including the ACPI naming issue you mention below), and for that matter > > with enumerable buses which can have firmware info. > > And most busses do already have their own bus type. ACPI, I2C, PCI, > etc. It is just a few that have been squished into platform, notably > OF. > I'll note that ACPI is an outlier that places devices on 2 buses, where new acpi_driver instances are discouraged [1] in favor of platform_drivers. ACPI scan handlers are awkwardly integrated into the Linux device model. So while I agree with sentiment that an "ACPI bus" should theoretically stand on its own there is legacy to unwind. I only bring that up to keep the focus on how to organize drivers going forward, because trying to map some of these arguments backwards runs into difficulties. [1]: http://lore.kernel.org/r/CAJZ5v0j_ReK3AGDdw7fLvmw_7knECCg2U_huKgJzQeLCy8smug@xxxxxxxxxxxxxx > > > are represented by struct platform_device and fiddling in the core > > > done to make that work OK. > > > > What exactly is the fiddling in the core here, I'm a bit unclear? > > I'm not sure, but I bet there is a small fall out to making bus_type > not 1:1 with the struct device type.. Would have to attempt it to see > > > > This feels like a good conference topic someday.. > > > > We should have this discussion *before* we get too far along with trying > > to implement things, we should at least have some idea where we want to > > head there. > > Well, auxillary bus is clearly following the original bus model > intention with a dedicated bus type with a controlled naming > scheme. The debate here seems to be "what about platform bus" and > "what to do with mfd"? > > > Those APIs all take a struct device for lookup so it's the same call for > > looking things up regardless of the bus the device is on or what > > firmware the system is using - where there are firmware specific lookup > > functions they're generally historical and shouldn't be used for new > > code. It's generally something in the form > > > > api_type *api_get(struct device *dev, const char *name); > > Well, that is a nice improvement since a few years back when I last > worked on this stuff. > > But now it begs the question, why not push harder to make 'struct > device' the generic universal access point and add some resource_get() > API along these lines so even a platform_device * isn't needed? > > Then the path seems much clearer, add a multi-bus-type device_driver > that has a probe(struct device *) and uses the 'universal api_get()' > style interface to find the generic 'resources'. > > The actual bus types and bus structs can then be split properly > without the boilerplate that caused them all to be merged to platform, > even PCI could be substantially merged like this. > > Bonus points to replace the open coded method disptach: > > int gpiod_count(struct device *dev, const char *con_id) > { > int count = -ENOENT; > > if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node) > count = of_gpio_get_count(dev, con_id); > else if (IS_ENABLED(CONFIG_ACPI) && dev && ACPI_HANDLE(dev)) > count = acpi_gpio_count(dev, con_id); > > if (count < 0) > count = platform_gpio_count(dev, con_id); > > With an actual bus specific virtual function: > > return dev->bus->gpio_count(dev); > > > ...and then do the same thing for every other bus with firmware > > bindings. If it's about the firmware interfaces it really isn't a > > platform bus specific thing. It's not clear to me if that's what it is > > though or if this is just some tangent. > > It should be split up based on the unique naming scheme and any bus > specific API elements - like raw access to ACPI or OF data or what > have you for other FW bus types. I agree that the pendulum may have swung too far towards "reuse existing bus_type", and auxiliary-bus unwinds some of that, but does the bus_type really want to be an indirection for driver apis outside of bus-specific operations?
