Re: [PATCH 09/12] irqchip: cirrus: Add driver for Cirrus Logic CS48L31/32/33 codecs

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2925 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]