lkml.org 
[lkml]   [2014]   [Nov]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 04/16] genirq: Introduce irq_chip.irq_compose_msi_msg() to support stacked irqchip
    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


    \
     
     \ /
      Last update: 2014-11-18 15:01    [W:4.428 / U:0.824 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site