On Mon, 15 May 2023 12:25:54 +0100, Lee Jones lee@kernel.org wrote:
On Fri, 12 May 2023, Marc Zyngier wrote:
On Fri, 12 May 2023 16:39:33 +0100, Charles Keepax ckeepax@opensource.cirrus.com wrote:
On Fri, May 12, 2023 at 04:10:05PM +0100, Marc Zyngier wrote:
On Fri, 12 May 2023 13:28:35 +0100, Charles Keepax ckeepax@opensource.cirrus.com wrote:
The CS42L43 is an audio CODEC with integrated MIPI SoundWire interface (Version 1.2.1 compliant), I2C, SPI, and I2S/TDM interfaces designed for portable applications. It provides a high dynamic range, stereo DAC for headphone output, two integrated Class D amplifiers for loudspeakers, and two ADCs for wired headset microphone input or stereo line input. PDM inputs are provided for digital microphones.
The IRQ chip provides IRQ functionality both to other parts of the cs42l43 device and to external devices that wish to use its IRQs.
Sorry, but this isn't much of an interrupt controller driver. A modern interrupt controller driver is firmware-driven (DT or ACPI, pick your poison), uses irq domains, and uses the irqchip API.
Apologies but I really need a little help clarifying the issues here. I am totally happy to fix things up but might need a couple pointers.
- uses the irqchip API / uses irq domains
The driver does use both the irqchip API and domains, what part of the IRQ API are we not using that we should be?
The driver registers an irq domain using irq_domain_create_linear. It requests its parent IRQ using request_threaded_irq. It passes IRQs onto the devices requesting IRQs from it using handle_nested_irq and irq_find_mapping.
Is the objection here that regmap is making these calls for us, rather than them being hard coded into this driver?
That's one of the reasons. Look at the existing irqchip drivers: they have nothing in common with yours. The regmap irqchip abstraction may be convenient for what you are doing, but the result isn't really an irqchip driver. It is something that is a small bit of a larger device and not an interrupt controller driver on its own. The irqchip subsystem is there for "first class" interrupt controllers.
I'm not aware of another subsystem that deals with !IRQChip level IRQ controllers. Where do simple or "second class" interrupt controllers go?
This isn't an interrupt controller. This is internal signalling, local to a single component that has been artificially broken into discrete bits, including an interrupt controller. The only *real* interrupts here are the GPIOs.
I'm happy to see an interrupt controller for the GPIOs. But the rest is just internal muck that doesn't really belong here. Where should it go? Together with the rest of the stuff that manages the block as a whole. Which looks like the MFD subsystem to me.
Thanks,
M.