Re: [PATCH v2 1/3] genirq/affinity: Add irq_update_affinity_desc()

[John Garry]

Hi Marc,

> > > Right, but if the driver is removed then the interrupts should be
> > > deallocated, right?
> >
> > When removing the driver we just call free_irq(), which removes the
> > handler and disables the interrupt.
> >
> > But about the irq_desc, this is created when the mapping is created in
> > irq_create_fwspec_mapping(), and I don't see this being torn down in
> > the driver removal, so persistent in that regard.
>
> If the irq_descs are created via the platform_get_irq() calls in
> platform_get_irqs_affinity(), I'd expect some equivalent helper to
> tear things down as a result, calling irq_dispose_mapping() behind the
> scenes.

So this looks lacking in the kernel AFAICS...

So is there a reason for which irq dispose mapping is not a requirement 
for drivers when finished with the irq? because of shared interrupts?

> > So for pci msi I can see that we free the irq_desc in
> > pci_disable_msi() -> free_msi_irqs() -> msi_domain_free_irqs() ...
> >
> > So what I am missing here?
>
> I'm not sure the paths are strictly equivalent. On the PCI side, we
> can have something that completely driver agnostic, as it is all
> architectural. In your case, only the endpoint driver knows about what
> happens, and needs to free things accordingly.
>
> Finally, there is the issue in your driver that everything is
> requested using devm_request_irq, which cannot play nicely with an
> explicit irq_desc teardown. You'll probably need to provide the
> equivalent devm helpers for your driver to safely be taken down.

Yeah, so since we use the devm irq request method, we also need a devm 
dispose release method as we can't dispose the irq mapping in the 
remove() method, prior to the irq_free() in the later devm release method.

But it looks like there is more to it than that, which I'm worried is 
far from non-trivial. For example, just calling irq_dispose_mapping() 
for removal and then plaform_get_irq()->acpi_get_irq() second time fails 
as it looks like more tidy-up is needed for removal...

Cheers,
John