On Mon, Dec 21, 2020 at 06:51:40PM +0000, Mark Brown wrote: > > with some kind of inheritance scheme where platform device remained as > > only instantiated directly in board files, while drivers could bind to > > OF/DT/ACPI/FPGA/etc device instantiations with minimal duplication & > > boilerplate. > > Like I said in my previous message that is essentially what we have now. > It's not worded in quite that way but it's how all the non-enumerable > buses work. I think it is about half way there. We jammed everything into platform device and platform bus and then had a few api aspects to tell if which of the subtypes it might be. That functions sort of like an object model with inheritance, but a single type and 'is it a XXX' queries is not quite the same thing. > 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? IMHO the point of the bus type is to tell the driver what API set you have. If you have a of_device then you have an OF node and can do all the of operations. Same for PCI/ACPI/etc. 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. 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 are represented by struct platform_device and fiddling in the core done to make that work OK. It is much easier to identify what a bus_type is (the unique collection of APIs) and thus when to create those. If the bus_type should contain struct platform_device or a unqiue struct then becomes a different question. Yes that is very hacky, but it feels less hacky than the platform bus/device is everything and can be used everwhere idea. > > The only problem with mfd as far as SOF is concerned was Greg was not > > happy when he saw PCI stuff in the MFD subsystem. > > This is a huge part of the problem here - there's no clearly articulated > logic, it's all coming back to these sorts of opinion statements about > specific cases which aren't really something you can base anything > on. 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.. > 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" eg INT33A1 is pmc_core_driver so the device was moved from acpi_bus to platform_bus? How does that make sense?? Why is pmc_core_driver a platform device instead of ACPI? Because some platforms don't have ACPI and the board file properly creates a platform device in C code. > 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. 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' Yes, it is super hacky, but at least the bus side would follow the basic architecture.. > 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'. Jason
