Messages in this thread | | | Date | Wed, 16 Nov 2022 16:44:05 +0000 | From | Mark Brown <> | Subject | Re: [PATCH 09/12] irqchip: cirrus: Add driver for Cirrus Logic CS48L31/32/33 codecs |
| |
On Fri, Nov 11, 2022 at 08:00:10AM +0000, Marc Zyngier wrote: > Mark Brown <broonie@kernel.org> wrote: > > On Thu, Nov 10, 2022 at 06:47:20PM +0000, Marc Zyngier wrote:
> > > 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.
It turns out a bunch of these systems are intended to be used with Linux, and even where the vendor does care about Linux we also have to consider what's tasteful for ACPI.
> > You'd need to look at having the MFD add additional > > description via swnode or something to try to get things going. MFD
...
> > 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
...
> > 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
> 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.
Yeah, I'm definitely not in the least bit convinced that the irqchip code is a good home for this sort of glue (especially the interrupt consumers) for the reasons you mention - my concern was more that the firmware interface also has issues, and that putting things into firmware is also putting them into ABI which is much harder to do a good job with later. [unhandled content-type:application/pgp-signature] | |