Re: [PATCH] spi: Add HiSilicon SPI controller driver support

[Lukas Wunner <lukas@xxxxxxxxx>]

On Mon, Mar 01, 2021 at 01:54:05PM +0000, Mark Brown wrote:
> On Mon, Mar 01, 2021 at 07:56:11PM +0800, Jay Fang wrote:
> > +	ret = devm_request_irq(dev, hs->irq, hisi_spi_irq, IRQF_SHARED,
> > +				dev_name(dev), master);
> > +	if (ret < 0) {
> > +		dev_err(dev, "failed to get IRQ=%d, ret=%d\n", hs->irq, ret);
> > +		return ret;
> > +	}
> 
> This will free the IRQ *after* the controller is unregistered, it's
> better to manually free the interrupt

Transfers may still be ongoing until spi_unregister_controller() returns.
(It's called from devres_release_all() in this case.)  Since the IRQ is
presumably necessary to handle those transfers, freeing the IRQ after
unregistering is actually correct.  So the code looks fine in principle.

However, because the IRQ is requested with IRQF_SHARED, the handler may
be invoked at any time, even after the controller has been unregistered.
It is therefore necessary to quiesce the SPI controller's interrupt on
unregistering and it is also necessary to check in the IRQ handler whether
an interrupt is actually pending (and bail out if not).

Thanks,

Lukas