From: wangyijing@huawei•com (Yijing Wang)
To: linux-arm-kernel@lists•infradead.org
Subject: [Patch Part2 v4 21/31] PCI/MSI: enhance PCI MSI core to support hierarchy irqdomain
Date: Thu, 6 Nov 2014 13:42:19 +0800 [thread overview]
Message-ID: <545B0A3B.40805@huawei.com> (raw)
In-Reply-To: <545B01DC.5060107@linux.intel.com>
On 2014/11/6 13:06, Jiang Liu wrote:
> On 2014/11/6 9:58, Yijing Wang wrote:
>>>>
>>>> @@ -1098,3 +1099,128 @@ int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
>>>> return nvec;
>>>> }
>>>> EXPORT_SYMBOL(pci_enable_msix_range);
>>>> +
>>>> +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
>>>
>>> Space, not tab.
>>>
>>>> +static inline irq_hw_number_t
>>>> +msi_get_hwirq(struct pci_dev *pdev, struct msi_desc *msidesc)
>>>
>>> The convention in this file is "struct pci_dev *dev". And "struct msi_desc
>>> *desc" (or maybe "*entry"). Try to converge things, not diverge them.
>>>
>>>> +{
>>>> + return (irq_hw_number_t)msidesc->msi_attrib.entry_nr |
>>>> + PCI_DEVID(pdev->bus->number, pdev->devfn) << 11 |
>>>> + (pci_domain_nr(pdev->bus) & 0xFFFFFFFF) << 27;
>>>
>>> Where does this bit layout come from? Is this defined in the spec
>>> somewhere? A reference would help.
>>
>> Currently, more and more Non-PCI device use MSI(or similar MSI mechanism), like DMAR fault irq
>> and HPET FSB irq. And we have to add additional code to support the MSI capability.
>> So I hope we can decouple MSI code and PCI code, then we can unify all MSI(or Message Based interrupt)
>> in one framework.
> Hi Yijing,
> I have a following patch to share more code among MSI/DMAR/HPET,
> which is one step forward as you suggested. Will send out that patch set
> soon.
That's Great! :)
> Regards!
> Gerry
>
>>
>> Thanks!
>> Yijing.
>>
>>>
>>>> +}
>>>> +
>>>> +static int msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
>>>> + unsigned int nr_irqs, void *arg)
>>>> +{
>>>> + int i, ret;
>>>> + irq_hw_number_t hwirq = arch_msi_irq_domain_get_hwirq(arg);
>>>> +
>>>> + if (irq_find_mapping(domain, hwirq) > 0)
>>>> + return -EEXIST;
>>>> +
>>>> + ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
>>>> + if (ret >= 0)
>>>
>>> if (ret < 0)
>>> return ret;
>>>
>>> and un-indent the mainline code below. Then it's obvious that this is the
>>> normal case, not the error case.
>>>
>>>> + for (i = 0; i < nr_irqs; i++) {
>>>> + irq_domain_set_hwirq_and_chip(domain, virq + i,
>>>> + hwirq + i, &msi_chip, (void *)(long)i);
>>>> + __irq_set_handler(virq + i, handle_edge_irq, 0, "edge");
>>>> + }
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> +static void msi_domain_free(struct irq_domain *domain, unsigned int virq,
>>>> + unsigned int nr_irqs)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = 0; i < nr_irqs; i++) {
>>>> + struct msi_desc *msidesc = irq_get_msi_desc(virq);
>>>> +
>>>> + if (msidesc)
>>>> + msidesc->irq = 0;
>>>> + }
>>>> + irq_domain_free_irqs_top(domain, virq, nr_irqs);
>>>> +}
>>>> +
>>>> +static int msi_domain_activate(struct irq_domain *domain,
>>>> + struct irq_data *irq_data)
>>>> +{
>>>> + int ret = 0;
>>>> + struct msi_msg msg;
>>>> +
>>>> + /*
>>>> + * irq_data->chip_data is MSI/MSIx offset.
>>>
>>> "MSI-X", as you wrote on the next line.
>>>
>>>> + * MSI-X message is written per-IRQ, the offset is always 0.
>>>> + * MSI message denotes a contiguous group of IRQs, written for 0th IRQ.
>>>> + */
>>>> + if (!irq_data->chip_data) {
>>>
>>> if (irq_data->chip_data)
>>> return 0;
>>>
>>> and un-indent the mainline code below, and drop the "ret = 0" init above.
>>>
>>>> + ret = irq_chip_compose_msi_msg(irq_data, &msg);
>>>> + if (ret == 0)
>>>
>>> if (ret)
>>> return ret;
>>>
>>>> + write_msi_msg(irq_data->irq, &msg);
>>>> + }
>>>> +
>>>> + return ret;
>>> return 0;
>>>> +}
>>>> +
>>>> +static int msi_domain_deactivate(struct irq_domain *domain,
>>>> + struct irq_data *irq_data)
>>>> +{
>>>> + struct msi_msg msg;
>>>> +
>>>> + if (irq_data->chip_data) {
>>>> + memset(&msg, 0, sizeof(msg));
>>>> + write_msi_msg(irq_data->irq, &msg);
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static struct irq_domain_ops msi_domain_ops = {
>>>> + .alloc = msi_domain_alloc,
>>>> + .free = msi_domain_free,
>>>> + .activate = msi_domain_activate,
>>>> + .deactivate = msi_domain_deactivate,
>>>> +};
>>>> +
>>>> +struct irq_domain *msi_create_irq_domain(struct irq_domain *parent)
>>>> +{
>>>> + struct irq_domain *domain;
>>>> +
>>>> + domain = irq_domain_add_tree(NULL, &msi_domain_ops, NULL);
>>>> + if (domain)
>>>
>>> if (!domain)
>>> return NULL;
>>>
>>> and un-indent this:
>>>
>>>> + domain->parent = parent;
>>>> +
>>>> + return domain;
>>>> +}
>>>> +
>>>> +int msi_irq_domain_alloc_irqs(struct irq_domain *domain, int type,
>>>> + struct pci_dev *dev, void *arg)
>>>> +{
>>>> + int i, virq;
>>>> + struct msi_desc *msidesc;
>>>> + int node = dev_to_node(&dev->dev);
>>>> +
>>>> + list_for_each_entry(msidesc, &dev->msi_list, list) {
>>>> + arch_msi_irq_domain_set_hwirq(arg, msi_get_hwirq(dev, msidesc));
>>>> + virq = irq_domain_alloc_irqs(domain, msidesc->nvec_used,
>>>> + node, arg);
>>>> + if (virq < 0) {
>>>> + /* Special handling for pci_enable_msi_range(). */
>>>> + return (type == PCI_CAP_ID_MSI &&
>>>> + msidesc->nvec_used > 1) ? 1 : -ENOSPC;
>>>
>>> I think "if" would be easier to read than this ternary expression.
>>>
>>>> + }
>>>> + for (i = 0; i < msidesc->nvec_used; i++)
>>>> + irq_set_msi_desc_off(virq + i, i, msidesc);
>>>> + }
>>>> +
>>>> + list_for_each_entry(msidesc, &dev->msi_list, list)
>>>> + if (msidesc->nvec_used == 1)
>>>> + dev_dbg(&dev->dev, "irq %d for MSI/MSI-X\n", virq);
>>>> + else
>>>> + dev_dbg(&dev->dev, "irq [%d-%d] for MSI/MSI-X\n",
>>>> + virq, virq + msidesc->nvec_used - 1);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +#endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
>>>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>>>> index 44f4746d033b..05dcd425f82b 100644
>>>> --- a/include/linux/msi.h
>>>> +++ b/include/linux/msi.h
>>>> @@ -75,4 +75,15 @@ struct msi_chip {
>>>> void (*teardown_irq)(struct msi_chip *chip, unsigned int irq);
>>>> };
>>>>
>>>> +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
>>>
>>> Use a space here, not a tab.
>>>
>>>> +extern struct irq_chip msi_chip;
>>>
>>> I don't think "msi_chip" is a good name. "Chip" only hints that it's a
>>> semiconductor integrated circuit; it doesn't say anything about what it
>>> does. I've suggested "msi_controller" elsewhere.
>>>
>>> Why does this need to be exported? And why should there be only one in a
>>> system?
>>>
>>>> +extern struct irq_domain *msi_create_irq_domain(struct irq_domain *parent);
>>>> +extern int msi_irq_domain_alloc_irqs(struct irq_domain *domain, int type,
>>>> + struct pci_dev *dev, void *arg);
>>>> +
>>>> +extern irq_hw_number_t arch_msi_irq_domain_get_hwirq(void *arg);
>>>> +extern void arch_msi_irq_domain_set_hwirq(void *arg, irq_hw_number_t hwirq);
>>>
>>> Look at the rest of the file and notice that the existing code does not use
>>> "extern" on function declarations.
>>>
>>>> +#endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
>>>
>>> Use a space here (not a tab), like the #endif just below.
>>>
>>>> #endif /* LINUX_MSI_H */
>>>> --
>>>> 1.7.10.4
>>>>
>>>
>>> .
>>>
>>
>>
>
> .
>
--
Thanks!
Yijing
next prev parent reply other threads:[~2014-11-06 5:42 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-04 12:01 [Patch Part2 v4 00/31] Enable hierarchy irqdomian on x86 platforms Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 01/31] irqdomain: Introduce new interfaces to support hierarchy irqdomains Jiang Liu
2014-11-05 23:48 ` Thomas Gleixner
2014-11-06 6:09 ` Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 02/31] irqdomain: Do irq_find_mapping and set_type for hierarchy irqdomain in case OF Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 03/31] genirq: Introduce helper functions to support stacked irq_chip Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 04/31] genirq: Introduce irq_chip.irq_compose_msi_msg() to support stacked irqchip Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 05/31] genirq: Add IRQ_SET_MASK_OK_DONE " Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 06/31] x86, irq: Save destination CPU ID in irq_cfg Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 07/31] x86, irq: Use hierarchy irqdomain to manage CPU interrupt vectors Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 08/31] x86, hpet: Use new irqdomain interfaces to allocate/free IRQ Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 09/31] x86, MSI: " Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 10/31] x86, uv: " Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 11/31] x86, htirq: " Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 12/31] x86, dmar: " Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 13/31] x86: irq_remapping: Introduce new interfaces to support hierarchy irqdomain Jiang Liu
2014-11-06 11:43 ` Yijing Wang
2014-11-04 12:01 ` [Patch Part2 v4 14/31] iommu/vt-d: Change prototypes to prepare for enabling " Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 15/31] iommu/vt-d: Enhance Intel IR driver to suppport " Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 16/31] iommu/amd: Enhance AMD " Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 17/31] x86, hpet: Enhance HPET IRQ to support " Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 18/31] PCI/MSI, trivial: Fix minor syntax issues according to coding styles Jiang Liu
2014-11-05 22:10 ` Bjorn Helgaas
2014-11-05 22:10 ` Bjorn Helgaas
2014-11-04 12:01 ` [Patch Part2 v4 19/31] PCI/MSI: Simplify PCI MSI code by initializing msi_desc.nvec_used earlier Jiang Liu
2014-11-05 22:35 ` Bjorn Helgaas
2014-11-04 12:01 ` [Patch Part2 v4 20/31] PCI/MSI: Kill redundant calling for irq_set_msi_desc() for MSIx interrupts Jiang Liu
2014-11-05 22:45 ` Bjorn Helgaas
2014-11-06 1:32 ` Yijing Wang
2014-11-06 4:04 ` Bjorn Helgaas
2014-11-06 4:31 ` Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 21/31] PCI/MSI: enhance PCI MSI core to support hierarchy irqdomain Jiang Liu
2014-11-05 23:09 ` Bjorn Helgaas
2014-11-06 1:58 ` Yijing Wang
2014-11-06 4:10 ` Bjorn Helgaas
2014-11-06 4:54 ` Yijing Wang
2014-11-06 5:06 ` Jiang Liu
2014-11-06 5:42 ` Yijing Wang [this message]
2014-11-06 4:58 ` Jiang Liu
2014-11-06 5:28 ` Bjorn Helgaas
2014-11-06 10:01 ` Thomas Gleixner
2014-11-06 10:30 ` Thomas Gleixner
2014-11-06 11:41 ` Jiang Liu
2014-11-06 11:59 ` Thomas Gleixner
2014-11-04 12:01 ` [Patch Part2 v4 22/31] x86, PCI, MSI: Use hierarchy irqdomain to manage MSI interrupts Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 23/31] x86, irq: Directly call native_compose_msi_msg() for DMAR IRQ Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 24/31] iommu/vt-d: Clean up unused MSI related code Jiang Liu
2014-11-04 12:01 ` [Patch Part2 v4 25/31] iommu/amd: " Jiang Liu
2014-11-04 12:02 ` [Patch Part2 v4 26/31] x86: irq_remapping: " Jiang Liu
2014-11-04 12:02 ` [Patch Part2 v4 27/31] x86, irq: Clean up unused MSI related code and interfaces Jiang Liu
2014-11-04 12:02 ` [Patch Part2 v4 28/31] iommu/vt-d: Refine the interfaces to create IRQ for DMAR unit Jiang Liu
2014-11-04 12:02 ` [Patch Part2 v4 29/31] x86, irq: Use hierarchy irqdomain to manage DMAR interrupts Jiang Liu
2014-11-04 12:02 ` [Patch Part2 v4 30/31] x86, htirq: Use hierarchy irqdomain to manage Hypertransport interrupts Jiang Liu
2014-11-04 12:02 ` [Patch Part2 v4 31/31] x86, uv: Use hierarchy irqdomain to manage UV interrupts Jiang Liu
2014-11-04 14:47 ` [Patch Part2 v4 00/31] Enable hierarchy irqdomian on x86 platforms Joerg Roedel
2014-11-04 15:12 ` Jiang Liu
2014-11-04 15:32 ` Joerg Roedel
2014-11-05 8:51 ` Joerg Roedel
2014-11-05 9:04 ` Jiang Liu
2014-11-05 9:41 ` Jiang Liu
2014-11-05 9:58 ` Joerg Roedel
2014-11-05 10:28 ` Jiang Liu
2014-11-05 11:10 ` Joerg Roedel
2014-11-06 13:07 ` Joerg Roedel
2014-11-06 13:35 ` Jiang Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=545B0A3B.40805@huawei.com \
--to=wangyijing@huawei$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox