On Thu, 10 Nov 2022 20:36:16 +0000, Mark Brown broonie@kernel.org wrote:
On Thu, Nov 10, 2022 at 06:47:20PM +0000, Marc Zyngier wrote:
Read again what I have written. Having to expose a device-specific API for endpoint drivers to obtain their interrupts, and requiring them to know about some magic values that describe the interrupts source are not a acceptable constructs.
We have firmware descriptions to expose interrupt linkages, and your HW is not special enough to deserve its own top level API. Yes, we accepted such drivers in the past, but it has to stop.
Either you describe the internal structure of your device in DT or ACPI, and make all client drivers use the standard API, or you make this a codec library, purely specific to your device and only used by it. But the current shape is not something I'm prepared to accept.
ACPI gets to be a lot of fun here, it's just not idiomatic to describe the internals of these devices in firmware there and a lot of the systems shipping this stuff are targeted at other OSs and system integrators are therefore not in the least worried about Linux preferences.
Let me reassure the vendors that I do not care about them either. By this standard, we'd all run Windows on x86.
You'd need to look at having the MFD add additional description via swnode or something to try to get things going. MFD does have support for that, though it's currently mainly used with devices that only have ACPI use (axp20x looks like the only potentially DT user, from the git history the swnode bits are apparently for use on ACPI systems). That might get fragile in the DT case since you could have multiple sources for description of the same thing unless you do something like suppress the swnode stuff on DT systems.
Given that swnode is basically DT written out in C code I'm not actually convinced it's that much of a win, unless someone writes some tooling to generate swnode data from DT files you're not getting the benefit of any of the schema validation work that's being done. We'd also need to do some work for regulators to make sure that if we are parsing DT properties on ACPI systems we don't do so from _DSD since ACPI has strong ideas about how power works and we don't want to end up with systems with firmware providing mixed ACPI/DT models without a clear understanding of what we're geting into.
I do also have other concerns in the purely DT case, especially with chip functions like the CODEC where there's a very poor mapping between physical IPs and how Linux is tending to describe things internally at the minute. In particular these devices often have a clock tree portions of which can be visible and useful off chip but which tends to get lumped in with the audio IPs in our current code. Ideally we'd describe that as a clock subdevice (or subdevices if that fits the hardware) using the clock bindings but then that has a bunch of knock on effects the way the code currently is which probably it's probably disproportionate to force an individual driver author to work through. OTOH the DT bindings should be OS neutral ABI so...
I don't think this is a reason to continue on the current path that pretends to have something generic, but instead is literally a board file fragment with baked-in magic numbers.
An irqchip is supposed to offer services to arbitrary clients (endpoint drivers) that are oblivious of the irqchip itself, of the hwirq mapping, and use the standard APIs to obtain a virtual interrupt number. None of that here. This is a monolithic driver, only split across multiple subsystem to satisfy a "not in my backyard" requirement.
If the vendors/authors want to keep the shape of the code as is, they can do it outside of the irqchip code and have some library code with an internal API. At least they will stop pretending that this is a general purpose driver. And the existing madera code can also go in the process.
M.