From: ddaney@caviumnetworks•com (David Daney)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v3 3/4] PCI/MSI: Add helper function pci_msi_domain_get_msi_rid().
Date: Thu, 1 Oct 2015 09:13:12 -0700 [thread overview]
Message-ID: <560D5B98.5050400@caviumnetworks.com> (raw)
In-Reply-To: <560CFBDC.4020901@arm.com>
On 10/01/2015 02:24 AM, Marc Zyngier wrote:
> Hi David,
>
> On 30/09/15 23:47, David Daney wrote:
>> From: David Daney <david.daney@cavium•com>
>>
>> Add pci_msi_domain_get_msi_rid() to return the MSI requester id (RID).
>> Initially needed by gic-v3 based systems. It will be used by follow on
>> patch to drivers/irqchip/irq-gic-v3-its-pci-msi.c
>>
>> Initially supports mapping the RID via OF device tree. In the future,
>> this could be extended to use ACPI _IORT tables as well.
>>
>> Signed-off-by: David Daney <david.daney@cavium•com>
>> ---
>> drivers/pci/msi.c | 31 +++++++++++++++++++++++++++++++
>> include/linux/msi.h | 1 +
>> 2 files changed, 32 insertions(+)
>>
>> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
>> index d449714..92b6dc9 100644
>> --- a/drivers/pci/msi.c
>> +++ b/drivers/pci/msi.c
>> @@ -20,6 +20,7 @@
>> #include <linux/io.h>
>> #include <linux/slab.h>
>> #include <linux/irqdomain.h>
>> +#include <linux/of_irq.h>
>>
>> #include "pci.h"
>>
>> @@ -1327,4 +1328,34 @@ struct irq_domain *pci_msi_create_default_irq_domain(struct device_node *node,
>>
>> return domain;
>> }
>> +
>> +struct get_mis_id_data {
>> + u32 alias;
>> +};
>> +
>> +static int get_msi_id_cb(struct pci_dev *pdev, u16 alias, void *data)
>> +{
>> + struct get_mis_id_data *s = data;
>> +
>> + s->alias = alias;
>> + return 0;
>> +}
>
> Why not use a naked u32, since you only have a single field in this
> structure? Or is it that you are anticipating other fields there?
In this case, I think using a pointer to u32 is a good idea. It would
simplify the source code somewhat. Although, I think the generated
binary would likely be the same. I don't foresee adding things to this
structure. If it becomes necessary in the future, we can just go back
to using a pointer to a structure.
>
>> +/**
>> + * pci_msi_domain_get_msi_rid - Get the MSI requester id (RID)
>> + * @domain: The interrupt domain
>> + * @pdev: The PCI device.
>> + *
>> + * The RID for a device is formed from the alias, with a firmware
>> + * supplied mapping applied
>> + *
>> + * Returns: The RID.
>> + */
>> +u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev)
>> +{
>> + struct get_mis_id_data d;
>> +
>> + d.alias = 0;
>> + pci_for_each_dma_alias(pdev, get_msi_id_cb, &d);
>> + return of_msi_map_rid(&pdev->dev, domain->of_node, d.alias);
>
> Should you check whether domain->of_node is NULL first? I don't think
> of_msi_map_rid would have any problem with that, but a domain that is
> not backed by an of_node makes me feel a bit uneasy and would tend to
> indicate that we're not using DT.
Yes, that makes sense. As you observe, I think it probably works as is,
but it would be good to make it more clear. This is especially true
when we add ACPI support. We will want to be clear on which of
device-tree or ACPI we are using.
>
>> +}
>> #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
>> diff --git a/include/linux/msi.h b/include/linux/msi.h
>> index ad939d0..56e3b76 100644
>> --- a/include/linux/msi.h
>> +++ b/include/linux/msi.h
>> @@ -293,6 +293,7 @@ irq_hw_number_t pci_msi_domain_calc_hwirq(struct pci_dev *dev,
>> struct msi_desc *desc);
>> int pci_msi_domain_check_cap(struct irq_domain *domain,
>> struct msi_domain_info *info, struct device *dev);
>> +u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev);
>> #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
>>
>> #endif /* LINUX_MSI_H */
>>
>
> Otherwise looks good to me.
I will send what I hope is the final revision of the patches later today.
David Daney
next prev parent reply other threads:[~2015-10-01 16:13 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-30 22:46 [PATCH v3 0/4] MSI, of, irqchip/gicv3-its: Handle "msi-map" properties David Daney
2015-09-30 22:46 ` [PATCH v3 1/4] Docs: dt: Add PCI MSI map bindings David Daney
2015-10-01 13:02 ` Rob Herring
2015-09-30 22:47 ` [PATCH v3 2/4] of/irq: Add new function of_msi_map_rid() David Daney
2015-10-01 13:08 ` Rob Herring
2015-09-30 22:47 ` [PATCH v3 3/4] PCI/MSI: Add helper function pci_msi_domain_get_msi_rid() David Daney
2015-10-01 9:24 ` Marc Zyngier
2015-10-01 16:13 ` David Daney [this message]
2015-10-01 16:33 ` Marc Zyngier
2015-09-30 22:47 ` [PATCH v3 4/4] irqchip/gic-v3-its: Add handling of PCI requester id David Daney
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=560D5B98.5050400@caviumnetworks.com \
--to=ddaney@caviumnetworks$(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