Re: "ahci: drop intx manipulation on msi enable" breaks ULI M1575

[Michael Ellerman <michael@ellerman.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2994 bytes --]

On Thu, 2009-04-09 at 01:32 -0400, Jeff Garzik wrote:
> Michael Ellerman wrote:
> > On Wed, 2009-04-08 at 23:23 -0500, Kumar Gala wrote:
> >> On Apr 8, 2009, at 6:53 PM, Michael Ellerman wrote:
> >>
> >>> On Wed, 2009-04-08 at 17:15 -0500, Timur Tabi wrote:
> >>>> On Wed, Apr 8, 2009 at 4:31 PM, Tejun Heo <tj@kernel.org> wrote:
> >>>>> Hmmm... for now,
> >>>>> I think it would be best to revert the original change.  Jeff, can  
> >>>>> you
> >>>>> please do that?
> >>>> Actually, give me a few days before you do that.  A colleague gave me
> >>>> some suggestions to debug this.
> >>> What device did you say it was? A "ULI M1575" ?
> >>>
> >>> Is that this one?
> >>>
> >>> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AL, 0x1575,  
> >>> hpcd_quirk_uli1575);
> >>>
> >>> static void __devinit hpcd_quirk_uli1575(struct pci_dev *dev)
> >>> {
> >>>        u32 temp32;
> >>>
> >>>        if (!machine_is(mpc86xx_hpcd))
> >>>                return;
> >>>
> >>>        /* Disable INTx */
> >>>        pci_read_config_dword(dev, 0x48, &temp32);
> >>>        pci_write_config_dword(dev, 0x48, (temp32 | 1<<26));
> >>> ..
> >> It is the odd thing is the board he's running on is a mpc86xx_hpcd so  
> >> he shouldn't be hitting the code that actually disables INTx.
> > 
> > Sorry Kumar that's not parsing :)
> > 
> > He is running an mpc86xx_hpcd, so he _should_ be hitting the code that
> > disables INTX?
> 
> The reversed logic of the PCI bit itself also makes for confusing 
> discusion.  In an attempt to be helpful, here is a restatement of what 
> is happening:
> 
> 1) Old 'ahci' used to clear PCI_COMMAND_INTX_DISABLE, thus ensuring INTX 
> interrupts are enabled... if and only if MSI is unavailable.
> 
> 2) Current 'ahci' no longer does this
> 
> 3) As a result, Timur's 'ahci' is no longer receiving interrupts. 
> Presumably this means that BOTH of the following conditions are true
> 
> 	a) INTX is disabled
> 	b) MSI is not available

Agreed.

> Today I am thinking we should either revert the libata commit 
> (a5bfc4714b3f01365aef89a92673f2ceb1ccf246), or poke PCI to twiddle INTX 
> for us at pci_enable_device() time, perhaps.
> 
> I lean towards the former, but maybe the platform folks prefer a third 
> solution?

But the device should have INTX enabled, the core shouldn't need to
twiddle it. If the device comes up with INTX disabled then it needs a
quirk to turn it on.

But in this case we have a quirk to turn INTX _off_, which just seems
odd. Can anyone say _why_ we need a quirk to turn INTX off? Looking at
the git history that quirk has been there ever since the MPC8610 HPCD
support went in (0e65bfe34c1000581746b9889d095241c4cf4a5c).

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]