On Fri, May 12, 2023 at 05:07:45PM +0100, Marc Zyngier wrote: > On Fri, 12 May 2023 16:39:33 +0100, > Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx> wrote: > > 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. > Thank you this is helpful. This device has GPIOs that other devices might want to use for IRQs, so the chip is capable of providing IRQ services to other devices in the system not just itself. This is commonly used where external boosted amps have their IRQs hooked up to the CODEC. I guess if Mark doesn't mind I think the only internal bit of the device that uses the IRQs is the CODEC driver so I could move the IRQ handling in there, it does seem a little odd to me, but I guess I don't have any problems with it. > > Is the objection here the table mapping the register fields that > > are provided as an IRQ on the device? > > I'm referring to this sort of construct: > > + CS42L43_IRQ_REG(HP_STARTUP_DONE, MSM), > + CS42L43_IRQ_REG(HP_SHUTDOWN_DONE, MSM), > + CS42L43_IRQ_REG(HSDET_DONE, MSM), > + CS42L43_IRQ_REG(TIPSENSE_UNPLUG_DB, MSM), > + CS42L43_IRQ_REG(TIPSENSE_PLUG_DB, MSM), > + CS42L43_IRQ_REG(RINGSENSE_UNPLUG_DB, MSM), > + CS42L43_IRQ_REG(RINGSENSE_PLUG_DB, MSM), > + CS42L43_IRQ_REG(TIPSENSE_UNPLUG_PDET, MSM), > + CS42L43_IRQ_REG(TIPSENSE_PLUG_PDET, MSM), > + CS42L43_IRQ_REG(RINGSENSE_UNPLUG_PDET, MSM), > + CS42L43_IRQ_REG(RINGSENSE_PLUG_PDET, MSM), > > Why isn't this described in firmware tables? So we probably could do that for device tree systems, but getting this into ACPI I think will be exceedingly difficult, and that is likely the primary market for the device. > Why doesn't it need to be > carried as part of the driver? Is "CLASS_D_AMP" something an interrupt > controller driver should care about? Ah ok so I think I am starting to understand, if I might paraphrase, your main objection here is that many of the IRQs are fixed purpose signals originating inside the chip itself, rather than external lines that can be hooked up for generic purposes. I guess most "first class" IRQ controllers have a lot more generic IRQs than they do fixed purpose ones. Where as we only have the 3 GPIOs as generic purpose IRQ lines. Thanks, Charles
