On Thu, Sep 07, 2023 at 04:17:10PM +0800, Szuying Chen wrote: > When a CRC error occurs, the HBA asserts an interrupt to indicate an > interface fatal error(PxIS.IFS). The ISR clear PxIE and PxIS, then do > error recovery. Before recovery process, HBA receives another SDB FIS > with the error(PxIS.TFES) from device. This can't be serviced due to > PxIE be cleared already. During error recovery process, the HBA can't > issue any new command after setting PxCMD.ST to 1 due to PxIS.TFES > still alive. > > According to AHCI 1.3.1 - section 6.2.2 specification. Fatal errors > (signified by the setting of PxIS.HBFS, PxIS.HBDS, PxIS.IFS or PxIS.TFES) > will cause the HBA to enter the ERR:Fatal state. In this state, the HBA > shall not issue any new commands. > > To avoid this, introduce the function ahci_port_clear_pending_irq() > to clear pending interrupts before executing a COMRESET. This follows > the AHCI 1.3.1 - section 6.2.2.2 specification. > > Signed-off-by: Szuying Chen <Chloe_Chen@xxxxxxxxxxxxxx> > --- > V1->V2: On suggestion by Damien to renamed helper function and modified > ahci_port_init() to make use of the helper. > V2->V3: On suggestion by Niklas to modify commit log and delete the extra > describe. > V3->V4: On suggestion by Damien to modify problem statement on commit log. > > drivers/ata/libahci.c | 35 +++++++++++++++++++++++------------ > 1 file changed, 23 insertions(+), 12 deletions(-) > > diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c > index e2bacedf28ef..f1263364fa97 100644 > --- a/drivers/ata/libahci.c > +++ b/drivers/ata/libahci.c > @@ -1256,6 +1256,26 @@ static ssize_t ahci_activity_show(struct ata_device *dev, char *buf) > return sprintf(buf, "%d\n", emp->blink_policy); > } > > +static void ahci_port_clear_pending_irq(struct ata_port *ap) > +{ > + struct ahci_host_priv *hpriv = ap->host->private_data; > + void __iomem *port_mmio = ahci_port_base(ap); > + u32 tmp; > + > + /* clear SError */ > + tmp = readl(port_mmio + PORT_SCR_ERR); > + dev_dbg(ap->host->dev, "PORT_SCR_ERR 0x%x\n", tmp); > + writel(tmp, port_mmio + PORT_SCR_ERR); > + > + /* clear port IRQ */ > + tmp = readl(port_mmio + PORT_IRQ_STAT); > + dev_dbg(ap->host->dev, "PORT_IRQ_STAT 0x%x\n", tmp); > + if (tmp) > + writel(tmp, port_mmio + PORT_IRQ_STAT); > + > + writel(1 << ap->port_no, hpriv->mmio + HOST_IRQ_STAT); > +} > + > static void ahci_port_init(struct device *dev, struct ata_port *ap, > int port_no, void __iomem *mmio, > void __iomem *port_mmio) > @@ -1270,18 +1290,7 @@ static void ahci_port_init(struct device *dev, struct ata_port *ap, > if (rc) > dev_warn(dev, "%s (%d)\n", emsg, rc); > > - /* clear SError */ > - tmp = readl(port_mmio + PORT_SCR_ERR); > - dev_dbg(dev, "PORT_SCR_ERR 0x%x\n", tmp); > - writel(tmp, port_mmio + PORT_SCR_ERR); > - > - /* clear port IRQ */ > - tmp = readl(port_mmio + PORT_IRQ_STAT); > - dev_dbg(dev, "PORT_IRQ_STAT 0x%x\n", tmp); > - if (tmp) > - writel(tmp, port_mmio + PORT_IRQ_STAT); > - > - writel(1 << port_no, mmio + HOST_IRQ_STAT); > + ahci_port_clear_pending_irq(ap); > > /* mark esata ports */ > tmp = readl(port_mmio + PORT_CMD); > @@ -1603,6 +1612,8 @@ int ahci_do_hardreset(struct ata_link *link, unsigned int *class, > tf.status = ATA_BUSY; > ata_tf_to_fis(&tf, 0, 0, d2h_fis); > > + ahci_port_clear_pending_irq(ap); > + > rc = sata_link_hardreset(link, timing, deadline, online, > ahci_check_ready); > > -- > 2.39.2 > Reviewed-by: Niklas Cassel <niklas.cassel@xxxxxxx>