Hi Marc,
Marc Zyngier <marc.zyngier@xxxxxxx> wrote on Thu, 7 Mar 2019 16:25:02
+0000:
> On 06/03/2019 10:21, Miquel Raynal wrote:
> > ahci_multi_irqs_intr_hard() is going to be used as interrupt handler
> > to support SATA per-port interrupts. The current logic is to check and
> > clear the SATA port interrupt status register only. To avoid spurious
> > IRQs and interrupt storms, it will be needed to clear the port
> > interrupt bit in the host interrupt status register as well.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> > ---
> > drivers/ata/libahci.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> > index b5f57c69c487..66d4906a5013 100644
> > --- a/drivers/ata/libahci.c
> > +++ b/drivers/ata/libahci.c
> > @@ -1912,7 +1912,10 @@ static void ahci_port_intr(struct ata_port *ap)
> > static irqreturn_t ahci_multi_irqs_intr_hard(int irq, void *dev_instance)
> > {
> > struct ata_port *ap = dev_instance;
> > + struct ata_host *host = ap->host;
> > + struct ahci_host_priv *hpriv = host->private_data;
> > void __iomem *port_mmio = ahci_port_base(ap);
> > + void __iomem *mmio = hpriv->mmio;
> > u32 status;
> >
> > VPRINTK("ENTER\n");
> > @@ -1924,6 +1927,10 @@ static irqreturn_t ahci_multi_irqs_intr_hard(int irq, void *dev_instance)
> > ahci_handle_port_interrupt(ap, port_mmio, status);
> > spin_unlock(ap->lock);
> >
> > + spin_lock(&host->lock);
> > + writel(BIT(ap->port_no), mmio + HOST_IRQ_STAT);
> > + spin_unlock(&host->lock);
>
> What's not clear here is under which circumstances this is required.
> This write should be atomic (if it isn't, you have bigger problems), and
> it is at best unclear what you're avoiding by taking the host lock.
You're right, taking the lock is not needed. It is a relict of a
previous implementation anyway.
Thanks,
Miquèl
