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

[Jeff Garzik <jeff@garzik.org>]

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

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?

	Jeff