On Mon, Jan 04, 2021 at 02:08:31PM -0400, Jason Gunthorpe wrote:
On Mon, Dec 21, 2020 at 06:51:40PM +0000, Mark Brown wrote:
BTW I did have a bit of a scan through some of the ACPI devices and for a good proportion of them it seems fairly clear that they are not platform devices at all - they were mostly interacting with ACPI firmware functionality rather than hardware, something you can't really do with FDT at all.
Right, that is kind of the point. We also have cases where ACPI devices are just an ioresource list and don't have any special ACPIness. IIRC DT has a similar issue where there are DT drivers that just don't work without the OF stuff. Why are they platform drivers?
There *might* be some very legacy ones for actual OF systems but that's not really at all a thing for FDT which is essentially all DT usage at this point - to the extent that things won't work it's due to the non-enumarability of the hardware so it's just a question of where the data comes from rather than one of communicating with a firmware (and for more generic things like GPIOs exactly where the data comes from ends up abstracted away from the driver anyway which is all some devices need). The idiom with DT is more that it's just a different way of filling out the data you'd get from a board file, it's not a runtime thing like ACPI.
We fake this idea out by being able to convert platform to DT and OF, but if platform is to be the universal device then why do we have PCI device and not a 'platform to pci' operator instead? None of this is consistent.
If it were more common for there to be IPs that appear as both PCI and platform devices (Intel do it a bit but otherwise it's quite rare) I'd guess we would actually have helpers for that translation.
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.
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 agree with this, IMHO there is no really cohesive explanation for when to create a bus vs use the "universal bus" (platform) that can also explain the things platform is already doing.
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.
Personally I'm even struggling to identify a practical problem that we're trying to solve here. Like Alexandre says what would an mfd_driver actually buy us?
Well, there is the minor issue of name collision eg /sys/bus/XX/devices/* must list all devices in the system with no collisions.
The owner of the bus is supposed to define the stable naming scheme and all the devices are supposed to follow it. platform doesn't have this:
$ ls /sys/bus/platform/devices/ ACPI000C:00 dell-smbios.0 'Fixed MDIO bus.0' INT33A1:00 microcode PNP0C04:00 PNP0C0B:03 PNP0C14:00 alarmtimer.0.auto dell-smbios.1 GHES.0 intel_rapl_msr.0 MSFT0101:00 PNP0C0B:00 PNP0C0B:04 PNP0C14:01 coretemp.0 efi-framebuffer.0 GHES.1 iTCO_wdt pcspkr PNP0C0B:01 PNP0C0C:00 reg-dummy dcdbas eisa.0 INT0800:00 kgdboc PNP0103:00 PNP0C0B:02 PNP0C0E:00 serial8250
Why are ACPI names in here? It looks like "because platform drivers were used to bind to ACPI devices"
I think we decided that the ACPI namespace was sufficiently divergent from our general conventions that we could just safely use the string, ICBW though.
I have some bad news for you about the hardware description problem space. Among other things we have a bunch of platform devices that don't have any resources exposed through the resource API but are still things like chips on a board, doing some combination of exposing resources for other devices (eg, a fixed voltage regulator) and consuming things like clocks or GPIOs that don't appear in the resource API.
So in these cases how do I use the generic platform bus API to find the GPIOs, regulators, and so on to connect with?
If drivers take a platform device and immediately covert it to an OF object and use OF APIs to find those connections then it really *never* was a platform device in the first place and coding an OF driver as platform is an abuse.
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);
with the strings being defined according to something in the hardware spec so there's a good chance of them working generically (and realistically it's only DT that's actually putting these names in firmware, otherwise it's just board files that we totally control, so this really is fine).
A decent step would be to accept that 'platform_device' is something weird and special and split its bus_type. Only devices created direclty in C code should be on the platform_bus, OF/ACPI/etc should all be on their own bus_types, even though they all use the same 'struct platform_bus'
...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.
that but it is not clear what the upside of doing that would be, especially given the amount of upheval it would generate and the classification issues that will inevitably result.
Well, I think the upside for existing is very small, but I would like to see a shared idea about how to answer questions like 'when should I use a existing device type' and 'when should I make a new device type'.
Yes, very much so.