mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ahci: libahci: clear pending interrupt status
@ 2023-08-10  9:31 Szuying Chen
  2023-08-10  9:59 ` Damien Le Moal
  0 siblings, 1 reply; 6+ messages in thread
From: Szuying Chen @ 2023-08-10  9:31 UTC (permalink / raw)
  To: dlemoal, linux-ide, linux-kernel; +Cc: Jesse1_Chang, Richard_Hsu, Szuying Chen

On 8/10/23 14:12, Damien Le Moal wrote: 
>    On 8/10/23 14:05, Szuying Chen wrote:
>    > When ISR handle interface fatal error with error recovery after clear PxIS
>    > and PxIE. Another FIS(SDB FIS with err) that set PxIS.IFS to 1 is recevied
>    > during error recovery, which causing the HBA not issue any new commands
>    > after cmd.ST set 1.
>
>    This is not very clear. If there was a fatal error, the drive should be in
>    error state and no other SDB FIS can be received as the drive does absolutely
>    nothing while in error state (it only waits for a read log 10h command to be>
>    issued to get it out of error state). So if you are seeing 2 SDB FIS with
>    errors one after the other, you have a buggy device...
>
>    However, I may be misunderstanding your issue here. Could you provide more
>    details and a dmesg output example of the issue ?

According to AHCI 1.3.1 specification ch6.1.9, when an R_ERR is received
on an H2D Data FIS in normal operation, the HBA sets PxIS.IFS to 1
(fatal error) and halts operation. Referring to SATA 3.0 specification we
know the device will halt queued command processing and transmit SDB FIS to
host with ERR bit in Status field set to one(set PxIS.TFES to 1).

In our case, the ISR handles fatal errors(PxIS.IFS) and enters error 
recovery after cleaning up PxIS and PxIE. Then a SDB FIS is received 
with interrupt bit(PxIS.TFES) set to 1. According to AHCI 1.3.1 
specification ch6.2.2, HBA can't issue(cmd.ST set to 1) any new commands
under PxIS.TFES alive during error recovery.

>    >
>    > Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
>    > ---
>    >  drivers/ata/libahci.c | 12 ++++++++++++
>    >  1 file changed, 12 insertions(+)
>    >
>    > diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
>    > index 06aec35f88f2..0ae51fd95d46 100644
>    > --- a/drivers/ata/libahci.c
>    > +++ b/drivers/ata/libahci.c
>    > @@ -679,9 +679,21 @@ static int ahci_scr_write(struct ata_link *link, unsigned int sc_reg, u32 val)
>    >
>    >  void ahci_start_engine(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);
>    > +     writel(tmp, port_mmio + PORT_SCR_ERR);
>    > +
>    > +     /* clear port IRQ */
>    > +     tmp = readl(port_mmio + PORT_IRQ_STAT);
>    > +     if (tmp)
>    > +             writel(tmp, port_mmio + PORT_IRQ_STAT);
>    > +
>    > +     writel(1 << ap->port_no, hpriv->mmio + PORT_IRQ_STAT);
>    > +
>    >       /* start DMA */
>    >       tmp = readl(port_mmio + PORT_CMD);
>    >       tmp |= PORT_CMD_START;
>    > --
>    > 2.39.2
>   >
>
>   -- 
>   Damien Le Moal
>    Western Digital Research
>
Thanks. 


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] ahci: libahci: clear pending interrupt status
@ 2023-08-31  3:09 Szuying Chen
  2023-08-31  5:22 ` Damien Le Moal
  0 siblings, 1 reply; 6+ messages in thread
From: Szuying Chen @ 2023-08-31  3:09 UTC (permalink / raw)
  To: Niklas.Cassel, dlemoal, linux-ide, linux-kernel
  Cc: Jesse1_Chang, Richard_Hsu, Chloe_Chen

This patch adds the function to clear pending interrupt before COMRESET.
It follows the  AHCI1.3.1 - section6.2.2.2 specification.

Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
---
 drivers/ata/libahci.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 06aec35f88f2..1ae788251a6c 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1584,6 +1584,23 @@ static int ahci_pmp_retry_softreset(struct ata_link *link, unsigned int *class,
 	return rc;
 }

+static void PortClearPendingInterrupt(struct ata_port *ap)
+{
+	struct ahci_host_priv *hpriv = ap->host->private_data;
+	void __iomem *port_mmio = ahci_port_base(ap);
+	u32 tmp;
+
+	/* clear port SERR */
+	tmp = readl(port_mmio + PORT_SCR_ERR);
+	writel(tmp, port_mmio + PORT_SCR_ERR);
+
+	/* clear port IRQ */
+	tmp = readl(port_mmio + PORT_IRQ_STAT);
+	writel(tmp, port_mmio + PORT_IRQ_STAT);
+
+	writel(1 << ap->port_no, hpriv->mmio + HOST_IRQ_STAT);
+}
+
 int ahci_do_hardreset(struct ata_link *link, unsigned int *class,
 		      unsigned long deadline, bool *online)
 {
@@ -1602,6 +1619,9 @@ int ahci_do_hardreset(struct ata_link *link, unsigned int *class,
 	tf.status = ATA_BUSY;
 	ata_tf_to_fis(&tf, 0, 0, d2h_fis);

+	/* clear pending Interrupt */
+	PortClearPendingInterrupt(ap);
+
 	rc = sata_link_hardreset(link, timing, deadline, online,
 				 ahci_check_ready);

--
2.39.2


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] ahci: libahci: clear pending interrupt status
@ 2023-08-10  5:05 Szuying Chen
  2023-08-10  5:12 ` Damien Le Moal
  0 siblings, 1 reply; 6+ messages in thread
From: Szuying Chen @ 2023-08-10  5:05 UTC (permalink / raw)
  To: dlemoal, linux-ide, linux-kernel
  Cc: Jesse1_Chang, Richard_Hsu, Chloe_chen, Szuying Chen

When ISR handle interface fatal error with error recovery after clear PxIS
and PxIE. Another FIS(SDB FIS with err) that set PxIS.IFS to 1 is recevied
during error recovery, which causing the HBA not issue any new commands
after cmd.ST set 1.

Signed-off-by: Szuying Chen <Chloe_Chen@asmedia.com.tw>
---
 drivers/ata/libahci.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 06aec35f88f2..0ae51fd95d46 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -679,9 +679,21 @@ static int ahci_scr_write(struct ata_link *link, unsigned int sc_reg, u32 val)

 void ahci_start_engine(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);
+	writel(tmp, port_mmio + PORT_SCR_ERR);
+
+	/* clear port IRQ */
+	tmp = readl(port_mmio + PORT_IRQ_STAT);
+	if (tmp)
+		writel(tmp, port_mmio + PORT_IRQ_STAT);
+
+	writel(1 << ap->port_no, hpriv->mmio + PORT_IRQ_STAT);
+
 	/* start DMA */
 	tmp = readl(port_mmio + PORT_CMD);
 	tmp |= PORT_CMD_START;
--
2.39.2


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-31  5:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-10  9:31 [PATCH] ahci: libahci: clear pending interrupt status Szuying Chen
2023-08-10  9:59 ` Damien Le Moal
  -- strict thread matches above, loose matches on Subject: below --
2023-08-31  3:09 Szuying Chen
2023-08-31  5:22 ` Damien Le Moal
2023-08-10  5:05 Szuying Chen
2023-08-10  5:12 ` Damien Le Moal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®