"irq 4: nobody cared" when loading ahci driver on ce4100

On Tue, 2011-03-15 at 08:19 +0100, Tejun Heo wrote:

> Hello,
> 
> On Tue, Mar 15, 2011 at 05:20:40AM +0100, Maxime Bizon wrote:
> > On Mon, 2011-03-14 at 18:59 -0600, Robert Hancock wrote:
> > 
> > > Where is ahci_thaw being called? It shouldn't be called before the IRQ
> > > handler is registered - I think it should only be called from the
> > > error 
> > 
> > it is not
> > 
> > ahci_pmp_attach/ahci_pmp_detach are the one setting the irq_mask too
> > soon
> 
> Does the following patch fix the problem?  Thanks.

no it doesn't, but this one does:

On Tue, 2011-03-15 at 15:48 +0100, Maxime Bizon wrote:

> no it doesn't, but this one does:

sorry it does not compile, missing ')', but actually works

I will send a proper patch if you agree with this solution

> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index 26d4523..6a01e3d 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -1897,7 +1897,9 @@ 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);
> +
> +	if (!(ap->pflags & (ATA_PFLAG_FROZEN | ATA_PFLAG_INITIALIZING))
> +		writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
>  }
>  
>  static void ahci_pmp_detach(struct ata_port *ap)
> @@ -1913,7 +1915,9 @@ 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);
> +
> +	if (!(ap->pflags & (ATA_PFLAG_FROZEN | ATA_PFLAG_INITIALIZING))
> +		writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
>  }
>  
>  int ahci_port_resume(struct ata_port *ap)
> 
> 
> 
>

On Tue, Mar 15, 2011 at 04:03:42PM +0100, Maxime Bizon wrote:
> 
> On Tue, 2011-03-15 at 15:48 +0100, Maxime Bizon wrote:
> 
> > no it doesn't, but this one does:
> 
> sorry it does not compile, missing ')', but actually works
> 
> I will send a proper patch if you agree with this solution

Hmm... I don't really like INITIALIZING being tested there.  It's much
more cumbersome to explain.  Can you please change ata_port_alloc() to
set pflags to INITIALIZING|FROZEN?  And add a comment in ahci briefly
noting why the test is necessary?

Thanks.

On Tue, 2011-03-15 at 16:19 +0100, Tejun Heo wrote:

> Hmm... I don't really like INITIALIZING being tested there.  It's much
> more cumbersome to explain.  Can you please change ata_port_alloc() to
> set pflags to INITIALIZING|FROZEN?  And add a comment in ahci briefly
> noting why the test is necessary?

will do

I'm trying to test the patch on linus tree instead of old vendor kernel,
and I will submit it once it's done.