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>
