[RFC/PATCH 6/7] MPIC MSI backend

[Segher Boessenkool]

> Agreed, though I prefer the naming to be ht_find_capability().

Of course, I just took "_msi" out of the name here ;-)

> Yes, I will do a cleanup pass on the MPIC code there. I think it  
> should
> only program the chip for polarity/sense in startup() and thus can
> "override" the native LSI setting of that vector when the interrupt is
> flagged as MSI without actually losing the original information.

You mean when we (in the future) reserve a range of vectors
for MSI allocation at MPIC startup?  Sure, that works.

> Right now, as you noticed, it's a non-issue for MSIs coming off HT, as
> both HT interrupts and MSIs are programmed to be edge in the MPIC
> itself, however, this is not ok with interrupts from the PCIe slot fow
> which, according to the device-tree on my Quad G5, the LSI is level  
> low.

["The PCIe slot" == the 16x PCIe slot on PowerMacs here, for
the record; it isn't connected over a bridge to HT, but is
directly connected to the CPC945 (U4) chip.]

There are more problems there (with this current code): the LSI
IRQ will always be #3, which cannot be used as an MSI IRQ; and
the MSI address to use can not be found on a parent HT bridge
(as there is none, duh).  That last problem makes the patch
perfectly safe for the CPC945 PCIe port though :-)

> Something else I'd like to do is add a generic IRQF_MSI purely for the
> sake of displaying "MSI" instead of "Edge" in /proc/interrupts :-)

Yes definitely.

> Though it might be useful for some backends to track what interrupts
> have been configured as MSI too.

That, too.

>>> ++static int msi_mpic_setup_msi_msg(struct pci_dev *pdev,
>>> +		struct msix_entry *entry, struct msi_msg *msg, int type)
>>> +{
>>> +	u64 addr;
>>> +
>>> +	addr = find_ht_magic_addr(pdev);
>>> +	msg->address_lo = addr & 0xFFFFFFFF;
>>> +	msg->address_hi = addr >> 32;
>>
>> You don't seem the check whether the "magic address" is outside
>> of 32-bit range and the device can do 32-bit only?
>
> Good point. It happens not to be on our chipset / firmware but we  
> should
> probably check in the check() callback. If it's the case, then we  
> refuse
> to enable MSI for that device.

Can't we just check here?  If not, what's the point of having
a return code here?  although doing the check in check() might
be conceptually nicer, sure.


Segher