On 2014/11/18 21:16, Yun Wu (Abel) wrote:
> On 2014/11/18 20:43, Jiang Liu wrote:
>
>> On 2014/11/18 19:47, Yun Wu (Abel) wrote:
>>> On 2014/11/18 18:02, Thomas Gleixner wrote:
>>>
>>>> On Tue, 18 Nov 2014, Yun Wu (Abel) wrote:
>>>>> On 2014/11/12 21:42, Thomas Gleixner wrote:
>>>>>> +int irq_chip_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>>>>>> +{
>>>>>> + struct irq_data *pos = NULL;
>>>>>> +
>>>>>> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
>>>>>> + for (; data; data = data->parent_data)
>>>>>> +#endif
>>>>>> + if (data->chip && data->chip->irq_compose_msi_msg)
>>>>>> + pos = data;
>>>>>> + if (!pos)
>>>>>> + return -ENOSYS;
>>>>>> +
>>>>>> + pos->chip->irq_compose_msi_msg(pos, msg);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>
>>>>> Adding message composing routine to struct irq_chip is OK to me, and it
>>>>> should
>>>>> be because it is interrupt controllers' duty to compose messages (so that
>>>>> they
>>>>> can parse the messages correctly without any pre-defined rules that
>>>>> endpoint
>>>>> devices absolutely need not to know).
>>>>> However a problem comes out when deciding which parameters should be
>>>>> passed to
>>>>> this routine. A message can associate with multiple interrupts, which
>>>>> makes me
>>>>> think composing messages for each interrupt is not that appropriate. And
>>>>> we
>>>>> can take a look at the new routine irq_chip_compose_msi_msg(). It is
>>>>> called by
>>>>> msi_domain_activate() which will be called by irq_domain_activate_irq() in
>>>>> irq_startup() for each interrupt descriptor, result in composing a
>>>>> message for
>>>>> each interrupt, right? (Unless requiring a judge on the parameter @data
>>>>> when
>>>>> implementing the irq_compose_msi_msg() callback that only compose message
>>>>> for
>>>>> the first entry of that message. But I really don't like that...)
>>>>
>>>> No, that's not correct. You are looking at some random stale version
>>>> of this. The current state of affairs is in
>>>>
>>>> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/irqdomain
>>>>
>>>> See also https://lkml.org/lkml/2014/11/17/764
>>>>
>>>> In activate we write the message, which is the right point to do so.
>>>>
>>>
>>> I checked the current state, it seems to be the same.
>>> Yes, the decision of postponing the actual hardware programming to the point
>>> where the interrupt actually gets used is right, but here above I was
>>> talking
>>> another thing.
>>> As I mentioned, a message can associate with multiple interrupts. Enabling
>>> any of them will call irq_startup(). So if we don't want to compose or write
>>> messages repeatedly, we'd better require performing some checks before
>>> activating the interrupts.
>> Hi Yun,
>> Seems you are talking about the case of multiple MSI support.
>> Yes, we have special treatment for multiple MSI, which only writes PCI
>> MSI registers when starting up the first MSI interrupt.
>> void pci_msi_domain_write_msg(struct irq_data *irq_data, struct msi_msg
>> *msg)
>> {
>> struct msi_desc *desc = irq_data->msi_desc;
>>
>> /*
>> * For MSI-X desc->irq is always equal to irq_data->irq. For
>> * MSI only the first interrupt of MULTI MSI passes the test.
>> */
>> if (desc->irq == irq_data->irq)
>> __pci_write_msi_msg(desc, msg);
>> }
>
>
> Yes, I picked the case of multiple MSI support.
> The check should also be performed when composing messages. That's why
> I don't like its parameters. The @data only indicates one interrupt,
> while I prefer doing compose/write in the unit of message descriptor.
Hi Yun,
The common abstraction is that every message interrupt could be
controlled independently, so have compose_msi_msg()/write_msi_msg() per
interrupt. MSI is abstracted as an special message signaled interrupt
with hardware limitation where multiple interrupts sharing the same
hardware registers. So we filter in pci_msi_domain_write_msg(). On the
other handle, the generic MSI framework caches msi_msg in msi_desc,
so we don't filter compose_msi_msg().
Regards!
Gerry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
