Patchwork ahci: don't enable port irq before handler is registered

login
register
mail settings
Submitter Maxime Bizon
Date March 15, 2011, 9:22 p.m.
Message ID <1300224158.10827.52.camel@sakura.staff.proxad.net>
Download mbox | patch
Permalink /patch/87084/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Maxime Bizon - March 15, 2011, 9:22 p.m.
From: Maxime Bizon <mbizon@freebox.fr>

The ahci_pmp_attach() & ahci_pmp_detach() unmask port irq, but they
are also called during port initialization, before ahci host irq
handler is registered. On ce4100 platform, this sometimes triggers
"irq 4: nobody cared" message when loading driver.

Fixed this by not touching the register if the port is in frozen
state, and mark all uninitialized port as frozen.

Signed-off-by: Maxime Bizon <mbizon@freebox.fr>
---
 drivers/ata/libahci.c     |   15 +++++++++++++--
 drivers/ata/libata-core.c |    2 +-
 2 files changed, 14 insertions(+), 3 deletions(-)
Tejun Heo - March 16, 2011, 9:04 a.m.
On Tue, Mar 15, 2011 at 10:22:38PM +0100, Maxime Bizon wrote:
> 
> From: Maxime Bizon <mbizon@freebox.fr>
> 
> The ahci_pmp_attach() & ahci_pmp_detach() unmask port irq, but they
> are also called during port initialization, before ahci host irq
> handler is registered. On ce4100 platform, this sometimes triggers
> "irq 4: nobody cared" message when loading driver.
> 
> Fixed this by not touching the register if the port is in frozen
> state, and mark all uninitialized port as frozen.
> 
> Signed-off-by: Maxime Bizon <mbizon@freebox.fr>

 Acked-by: Tejun Heo <tj@kernel.org>

But just one nit.

> -	writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
> +
> +	/*
> +	 * This may be called while we are still initializing the ahci
> +	 * host, and interrupt handler is not yet registered at that
> +	 * time. So don't mess with irq mask register unless the port
> +	 * is actually enabled.
> +	 */

Can you augment the comment so that it indicates that hardware IRQ
mask shouldn't be modified while the port is frozen and pp->intr_mask
will be restored when the port is thawed and note that the port starts
frozen?

Thanks.
Tejun Heo - March 16, 2011, 9:05 a.m.
On Wed, Mar 16, 2011 at 10:04:53AM +0100, Tejun Heo wrote:
>  Acked-by: Tejun Heo <tj@kernel.org>
> 
> But just one nit.

Ooh, please also add

 Cc: stable@kernel.org

Thanks.

Patch

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 26d4523..eb5afdd 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1897,7 +1897,15 @@  static void ahci_pmp_attach(struct ata_port *ap)
 	ahci_enable_fbs(ap);
 
 	pp->intr_mask |= PORT_IRQ_BAD_PMP;
-	writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
+
+	/*
+	 * This may be called while we are still initializing the ahci
+	 * host, and interrupt handler is not yet registered at that
+	 * time. So don't mess with irq mask register unless the port
+	 * is actually enabled.
+	 */
+	if (!(ap->pflags & ATA_PFLAG_FROZEN))
+		writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
 }
 
 static void ahci_pmp_detach(struct ata_port *ap)
@@ -1913,7 +1921,10 @@  static void ahci_pmp_detach(struct ata_port *ap)
 	writel(cmd, port_mmio + PORT_CMD);
 
 	pp->intr_mask &= ~PORT_IRQ_BAD_PMP;
-	writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
+
+	/* see comment above in ahci_pmp_attach() */
+	if (!(ap->pflags & ATA_PFLAG_FROZEN))
+		writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
 }
 
 int ahci_port_resume(struct ata_port *ap)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index d4e52e2..02d5703 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5480,7 +5480,7 @@  struct ata_port *ata_port_alloc(struct ata_host *host)
 	if (!ap)
 		return NULL;
 	
-	ap->pflags |= ATA_PFLAG_INITIALIZING;
+	ap->pflags |= ATA_PFLAG_INITIALIZING | ATA_PFLAG_FROZEN;
 	ap->lock = &host->lock;
 	ap->print_id = -1;
 	ap->host = host;