On Mon, Jan 4, 2021 at 4:14 PM Jason Gunthorpe jgg@nvidia.com 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...
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?