ata: Fixes related to edge trigger latch for ahci_xgene driver.
v2 cover ata: Fixes related to edge trigger latch for ahci_xgene driver.
This patch set implements a workaround for an errate in the APM X-Gene SATA host controller with edge interrupt. The HOST_IRQ_STAT misses the edge interrupt from the PORT_IRQ_STAT when clearing the HOST_IRQ_STAT and reporting the PORT_IRQ_STAT happens in same clock cycle. It also implements the capability to override generic interrupt handler.
v1 change:
* Implement the capability for LDD to override interrupt handler.
* ahci_xgene driver implements the edge trigger interrupt handler.
v2 change:
* Remove the AHCI_HFLAG_EDGE_IRQ frim libahci.
Suman Tripathi (3):
ata: Remove the AHCI_HFLAG_EDGE_IRQ support from libahci.
libahci: Implement the capability to override the generic ahci
interrupt handler.
ahci_xgene: Implement the workaround to fix the missing of edge
interrupt for HOST_IRQ_STAT.
drivers/ata/ahci.h | 5 ++- drivers/ata/ahci_xgene.c | 86 +++++++++++++++++++++++++++++++++++++++++++++- drivers/ata/libahci.c | 53 +++++----------------------- 3 files changed, 98 insertions(+), 46 deletions(-)
PATCH v2 1/3 ata: Remove the AHCI_HFLAG_EDGE_IRQ support from libahci.
This patch removes the AHCI_HFLAG_EDGE_IRQ support from libahci. The upcoming patches will have the flexibility to override irq handlers in the LLD's. Controller's implementing a EDGE trigger latch can implement their own interrupt handler inside the driver.
--- drivers/ata/ahci.h | 3 +-- drivers/ata/ahci_xgene.c | 2 +- drivers/ata/libahci.c | 40 ---------------------------------------- 3 files changed, 2 insertions(+), 43 deletions(-)
drivers/ata/ahci.h +1 -2
@@ -240,8 +240,7 @@ enum {error-handling stage) */ AHCI_HFLAG_NO_DEVSLP = (1 << 17), /* no device sleep */ AHCI_HFLAG_NO_FBS = (1 << 18), /* no FBS */ -AHCI_HFLAG_EDGE_IRQ = (1 <<19 ),/* HOST_IRQ_STAT behaves as -Edge Triggered */ +#ifdef CONFIG_PCI_MSI AHCI_HFLAG_MULTI_MSI = (1 <<20 ),/* multiple PCI MSIs */ AHCI_HFLAG_MULTI_MSIX = (1 << 21), /* per-port MSI-X */
drivers/ata/ahci_xgene.c +1 -1
@@ -779,7 +779,7 @@ skip_clk_phy: hpriv->flags =AHCI_HFLAG_NO_NCQ ;break ;case XGENE_AHCI_V2 : - hpriv->flags |=AHCI_HFLAG_YES_FBS |AHCI_HFLAG_EDGE_IRQ ; + hpriv->flags |=AHCI_HFLAG_YES_FBS ;break ;default :break ;
drivers/ata/libahci.c +0 -40
@@ -1843,43 +1843,6 @@ static u32 ahci_handle_port_intr(struct ata_host *host, u32 irq_masked)return handled; } -static irqreturn_t ahci_single_edge_irq_intr (int irq,void *dev_instance) -{ -struct ata_host *host = dev_instance; -struct ahci_host_priv *hpriv; -unsigned int rc =0 ; -void __iomem *mmio; -u32 irq_stat, irq_masked; - -VPRINTK ("ENTER\n" ); - - hpriv = host->private_data; - mmio = hpriv->mmio; - -/* sigh. 0xffffffff is a valid return from h/w */ - irq_stat =readl (mmio +HOST_IRQ_STAT ); -if (!irq_stat) -return IRQ_NONE ; - - irq_masked = irq_stat & hpriv->port_map; - -spin_lock (&host->lock); - -/* -* HOST_IRQ_STAT behaves as edge triggered latch meaning that -* it should be cleared before all the port events are cleared. -*/ -writel (irq_stat, mmio +HOST_IRQ_STAT ); - - rc =ahci_handle_port_intr (host, irq_masked); - -spin_unlock (&host->lock); - -VPRINTK ("EXIT\n" ); - -return IRQ_RETVAL (rc); -} -static irqreturn_t ahci_single_level_irq_intr (int irq,void *dev_instance) {struct ata_host *host = dev_instance;
@@ -2507,9 +2470,6 @@ int ahci_host_activate(struct ata_host *host, struct scsi_host_template *sht)if (hpriv->flags & (AHCI_HFLAG_MULTI_MSI |AHCI_HFLAG_MULTI_MSIX )) rc =ahci_host_activate_multi_irqs (host, sht); -else if (hpriv->flags &AHCI_HFLAG_EDGE_IRQ ) - rc =ata_host_activate (host, irq, ahci_single_edge_irq_intr, -IRQF_SHARED , sht);else rc =ata_host_activate (host, irq, ahci_single_level_irq_intr,IRQF_SHARED , sht);
--
1.7.1
PATCH v2 1/3 ata: Remove the AHCI_HFLAG_EDGE_IRQ support from libahci.
Quoted 6 lines · Suman Tripathi
This patch removes the
AHCI_HFLAG_EDGE_IRQsupport from libahci. The upcoming patches will have the flexibility to override irq handlers in the LLD's. Controller's implementing a EDGE trigger latch can implement their own interrupt handler inside the driver.Signed-off-by Suman Tripathi <[email protected]>
Shouldn't this patch come after the irq override is in place?
1. introduce irq override.
2. remove AHCI_HFLAG_EDGE_IRQ by moving the edge triggered handler to ahci_xgene.
3. Update xgene irq handler to address the erratum.
Thanks.
Signature · 1 line
tejun
PATCH v2 1/3 ata: Remove the AHCI_HFLAG_EDGE_IRQ support from libahci.
Quoted 8 lines · Tejun Heo
Quoted 6 lines · Suman Tripathi
This patch removes the
AHCI_HFLAG_EDGE_IRQsupport from libahci. The upcoming patches will have the flexibility to override irq handlers in the LLD's. Controller's implementing a EDGE trigger latch can implement their own interrupt handler inside the driver.Signed-off-by Suman Tripathi <[email protected]>
Quoted 8 lines
Shouldn't this patch come after the irq override is in place?
1. introduce irq override.
2. remove
AHCI_HFLAG_EDGE_IRQby moving the edge triggered handler toahci_xgene.3. Update xgene irq handler to address the erratum.
Thought of remove the AHCI_HFLAG_EDGE entirely first because we don't require it . Then implement the irq override capability.
Quoted 5 lines
Thanks.
--
tejun
Signature · 3 lines
Thanks, with regards, Suman Tripathi
PATCH v2 2/3 libahci: Implement the capability to override the generic ahci interrupt handler.
In reply to Suman TripathiThis patch implements the capability to override the generic ahci interrupt handler so that LDD drivers can implement there own interrupt handler routines.
--- drivers/ata/ahci.h | 2 ++ drivers/ata/libahci.c | 13 ++++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-)
drivers/ata/ahci.h +2 -0
@@ -359,6 +359,7 @@ struct ahci_host_priv {* be overridden anytime before the host is activated. */ void (*start_engine)(struct ata_port *ap); +irqreturn_t (*ahci_irq_intr)(int irq,void *dev_instance); };#ifdef CONFIG_PCI_MSI
@@ -422,6 +423,7 @@ int ahci_reset_em(struct ata_host *host);void ahci_print_info (struct ata_host *host,const char *scc_s);int ahci_host_activate (struct ata_host *host,struct scsi_host_template *sht);void ahci_error_handler (struct ata_port *ap); +u32 ahci_handle_port_intr (struct ata_host *host,u32 irq_masked);static inline void __iomem *__ahci_port_base (struct ata_host *host,unsigned int port_no)
drivers/ata/libahci.c +10 -3
@@ -1816,7 +1816,7 @@ static irqreturn_t ahci_multi_irqs_intr_hard(int irq, void *dev_instance)return IRQ_HANDLED ; } -static u32 ahci_handle_port_intr (struct ata_host *host,u32 irq_masked) +u32 ahci_handle_port_intr (struct ata_host *host,u32 irq_masked) {unsigned int i, handled =0 ;
@@ -1842,6 +1842,7 @@ static u32 ahci_handle_port_intr(struct ata_host *host, u32 irq_masked)return handled; } +EXPORT_SYMBOL_GPL (ahci_handle_port_intr);static irqreturn_t ahci_single_level_irq_intr (int irq,void *dev_instance) {
@@ -2466,13 +2467,19 @@ int ahci_host_activate(struct ata_host *host, struct scsi_host_template *sht) {struct ahci_host_priv *hpriv = host->private_data;int irq = hpriv->irq; +irqreturn_t (*ahci_irq_handler)(int irq,void *dev_instance);int rc;if (hpriv->flags & (AHCI_HFLAG_MULTI_MSI |AHCI_HFLAG_MULTI_MSIX )) rc =ahci_host_activate_multi_irqs (host, sht); -else - rc =ata_host_activate (host, irq, ahci_single_level_irq_intr, +else { + ahci_irq_handler = hpriv->ahci_irq_intr ? hpriv->ahci_irq_intr : + ahci_single_level_irq_intr; + + rc =ata_host_activate (host, irq, ahci_irq_handler,IRQF_SHARED , sht); + } +return rc; }EXPORT_SYMBOL_GPL (ahci_host_activate);
--
1.7.1
PATCH v2 2/3 libahci: Implement the capability to override the generic ahci interrupt handler.
drivers/ata/libahci.c int ahci_host_activate(struct ata_host *host, struct scsi_host_template *sht) · line 2474
Quoted 9 lines · Suman Tripathi
drivers/ata/libahci.c +1
-0@@ -2466,13 +2467,19 @@ int ahci_host_activate(struct ata_host *host, struct scsi_host_template *sht) {struct ahci_host_priv *hpriv = host->private_data;int irq = hpriv->irq; +irqreturn_t (*ahci_irq_handler)(int irq,void *dev_instance);int rc;if (hpriv->flags & (AHCI_HFLAG_MULTI_MSI |AHCI_HFLAG_MULTI_MSIX )) rc =ahci_host_activate_multi_irqs (host, sht);
You need to add { } to the if body too. Please read CodingStyle.
drivers/ata/libahci.c int ahci_host_activate(struct ata_host *host, struct scsi_host_template *sht) · line 2481
Quoted 9 lines
drivers/ata/libahci.c +6
-2-else - rc =ata_host_activate (host, irq, ahci_single_level_irq_intr, +else { + ahci_irq_handler = hpriv->ahci_irq_intr ? hpriv->ahci_irq_intr : + ahci_single_level_irq_intr; + + rc =ata_host_activate (host, irq, ahci_irq_handler,IRQF_SHARED , sht); + }
Can you please make this handled the same way as ->start_engine? Also, maybe print out a warning message if a driver specifies both AHCI_HFLAG_MULTI_MSI[X] and the irq handler override?
Thanks.
Signature · 1 line
tejun
PATCH v2 2/3 libahci: Implement the capability to override the generic ahci interrupt handler.
Quoted 23 lines · Tejun Heo
drivers/ata/libahci.c int ahci_host_activate(struct ata_host *host, struct scsi_host_template *sht) · line 2474
Quoted 9 lines · Suman Tripathi
drivers/ata/libahci.c +1
-0@@ -2466,13 +2467,19 @@ int ahci_host_activate(struct ata_host *host, struct scsi_host_template *sht) {struct ahci_host_priv *hpriv = host->private_data;int irq = hpriv->irq; +irqreturn_t (*ahci_irq_handler)(int irq,void *dev_instance);int rc;if (hpriv->flags & (AHCI_HFLAG_MULTI_MSI |AHCI_HFLAG_MULTI_MSIX )) rc =ahci_host_activate_multi_irqs (host, sht);You need to add { } to the if body too. Please read CodingStyle.
drivers/ata/libahci.c int ahci_host_activate(struct ata_host *host, struct scsi_host_template *sht) · line 2481
Quoted 9 lines
drivers/ata/libahci.c +6
-2-else - rc =ata_host_activate (host, irq, ahci_single_level_irq_intr, +else { + ahci_irq_handler = hpriv->ahci_irq_intr ? hpriv->ahci_irq_intr : + ahci_single_level_irq_intr; + + rc =ata_host_activate (host, irq, ahci_irq_handler,IRQF_SHARED , sht); + }
Quoted 3 lines
Can you please make this handled the same way as ->start_engine? Also, maybe print out a warning message if a driver specifies both AHCI_HFLAG_MULTI_MSI[X] and the irq handler override?
Ok Sure
Quoted 5 lines
Thanks.
--
tejun
Signature · 3 lines
Thanks, with regards, Suman Tripathi
PATCH v2 3/3 ahci_xgene: Implement the workaround to fix the missing of edge interrupt for HOST_IRQ_STAT.
In reply to Suman TripathiDue to H/W errata, the HOST_IRQ_STAT register misses the edge interrupt when clearing the HOST_IRQ_STAT register and hardware reporting the PORT_IRQ_STAT register at the same clock cycle.
--- drivers/ata/ahci_xgene.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 84 insertions(+), 0 deletions(-)
drivers/ata/ahci_xgene.c +84 -0
@@ -548,6 +548,89 @@ softreset_retry:return rc; } +/** +* xgene_ahci_handle_broken_edge_irq - Handle the broken irq. +* @ata_host: Host that recieved the irq +* @irq_masked: HOST_IRQ_STAT value +* +* For hardware with broken edge trigger latch +* the HOST_IRQ_STAT register misses the edge interrupt +* when clearing of HOST_IRQ_STAT register and hardware +* reporting the PORT_IRQ_STAT register at the +* same clock cycle. +* As such, the algorithm below outlines the workaround. +* +* 1. Read HOST_IRQ_STAT register and save the state. +* 2. Clear the HOST_IRQ_STAT register. +* 3. Read back the HOST_IRQ_STAT register. +* 4. If HOST_IRQ_STAT register equals to zero, then +* traverse the rest of port's PORT_IRQ_STAT register +* to check if an interrupt is triggered at that point else +* go to step 6. +* 5. If PORT_IRQ_STAT register of rest ports is not equal to zero +* then update the state of HOST_IRQ_STAT saved in step 1. +* 6. Handle port interrupts. +* 7. Exit +*/ +static int xgene_ahci_handle_broken_edge_irq (struct ata_host *host, +u32 irq_masked) +{ +struct ahci_host_priv *hpriv = host->private_data; +void __iomem *port_mmio; +int i; + +if (!readl (hpriv->mmio +HOST_IRQ_STAT )) { +for (i =0 ; i < host->n_ports; i++) { +if (irq_masked & (1 << i)) +continue ; + + port_mmio =ahci_port_base (host->ports[i]); +if (readl (port_mmio +PORT_IRQ_STAT )) + irq_masked |= (1 << i); + } + } + +return ahci_handle_port_intr (host, irq_masked); +} + +static irqreturn_t xgene_ahci_irq_intr (int irq,void *dev_instance) +{ +struct ata_host *host = dev_instance; +struct ahci_host_priv *hpriv; +unsigned int rc =0 ; +void __iomem *mmio; +u32 irq_stat, irq_masked; + +VPRINTK ("ENTER\n" ); + + hpriv = host->private_data; + mmio = hpriv->mmio; + +/* sigh. 0xffffffff is a valid return from h/w */ + irq_stat =readl (mmio +HOST_IRQ_STAT ); +if (!irq_stat) +return IRQ_NONE ; + + irq_masked = irq_stat & hpriv->port_map; + +spin_lock (&host->lock); + +/* +* HOST_IRQ_STAT behaves as edge triggered latch meaning that +* it should be cleared before all the port events are cleared. +*/ +writel (irq_stat, mmio +HOST_IRQ_STAT ); + + rc =xgene_ahci_handle_broken_edge_irq (host, irq_masked); + +spin_unlock (&host->lock); + +VPRINTK ("EXIT\n" ); + +return IRQ_RETVAL (rc); + +} +static struct ata_port_operations xgene_ahci_v1_ops = { .inherits = &ahci_ops, .host_stop = xgene_ahci_host_stop,
@@ -780,6 +863,7 @@ skip_clk_phy:break ;case XGENE_AHCI_V2 : hpriv->flags |=AHCI_HFLAG_YES_FBS ; + hpriv->ahci_irq_intr = xgene_ahci_irq_intr;break ;default :break ;
--
1.7.1