From: Marc Zyngier <marc.zyngier@arm.com>
To: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Cc: <tglx@linutronix.de>, <jason@lakedaemon.net>, <rjw@rjwysocki.net>,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Will Deacon <will.deacon@arm.com>,
Catalin Marinas <Catalin.Marinas@arm.com>,
<hanjun.guo@linaro.org>, <tomasz.nowicki@linaro.org>,
<graeme.gregory@linaro.org>, <dhdang@apm.com>,
<linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, <linux-acpi@vger.kernel.org>
Subject: Re: [PATCH v5 1/5] acpi: pci: Setup MSI domain for ACPI based pci devices
Date: Wed, 9 Dec 2015 10:13:55 +0000 [thread overview]
Message-ID: <20151209101355.68eab0cc@why.wild-wind.fr.eu.org> (raw)
In-Reply-To: <1449625686-5594-2-git-send-email-Suravee.Suthikulpanit@amd.com>
On Tue, 8 Dec 2015 17:48:02 -0800
Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> wrote:
> This patch introduces pci_msi_register_fwnode_provider() for irqchip
> to register a callback, to provide a way to determine appropriate MSI
> domain for a pci device.
>
> It also introduces pci_host_bridge_acpi_msi_domain(), which returns
> the MSI domain of the specified PCI host bridge with DOMAIN_BUS_PCI_MSI
> bus token. Then, it is assigned to pci device.
>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> ---
> drivers/pci/pci-acpi.c | 32 ++++++++++++++++++++++++++++++++
> drivers/pci/probe.c | 2 ++
> include/linux/irqdomain.h | 5 +++++
> include/linux/pci.h | 10 ++++++++++
> 4 files changed, 49 insertions(+)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index a32ba75..a555d7e 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -9,7 +9,9 @@
>
> #include <linux/delay.h>
> #include <linux/init.h>
> +#include <linux/irqdomain.h>
> #include <linux/pci.h>
> +#include <linux/msi.h>
> #include <linux/pci_hotplug.h>
> #include <linux/module.h>
> #include <linux/pci-aspm.h>
> @@ -689,6 +691,36 @@ static struct acpi_bus_type acpi_pci_bus = {
> .cleanup = pci_acpi_cleanup,
> };
>
> +
> +static struct fwnode_handle *(*pci_msi_get_fwnode_cb)(struct device *dev);
> +
> +/**
> + * pci_msi_register_fwnode_provider - Register callback to retrieve fwnode
> + * @fn: The interrupt domain to retrieve
This is the wrong description. Maybe something like@
* @fn: Callback matching a device to a fwnode that identifies a PCI
MSI domain
Or something along those lines.
> + *
> + * This should be called by irqchip driver, which is the parent of
> + * the MSI domain to provide callback interface to query fwnode.
> + */
> +void
> +pci_msi_register_fwnode_provider(struct fwnode_handle *(*fn)(struct device *))
> +{
> + pci_msi_get_fwnode_cb = fn;
> +}
> +
> +struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus)
> +{
Documentation?
> + struct irq_domain *dom = NULL;
> + struct fwnode_handle *fwnode = NULL;
> +
> + if (pci_msi_get_fwnode_cb)
> + fwnode = pci_msi_get_fwnode_cb(&bus->dev);
> +
> + if (fwnode)
> + dom = irq_find_matching_fwnode(fwnode,
> + DOMAIN_BUS_PCI_MSI);
> + return dom;
> +}
> +
Suggestion:
struct fwnode_handle *fwnode;
if (!pci_msi_get_fwnode_cb)
return NULL;
fwnode = pci_msi_get_fwnode_cb(&bus->dev);
if (!fwnode)
return NULL;
return irq_find_matching_fwnode(...);
My own personal taste, though.
> static int __init acpi_pci_init(void)
> {
> int ret;
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index edb1984..553a029 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -672,6 +672,8 @@ static struct irq_domain *pci_host_bridge_msi_domain(struct pci_bus *bus)
> * should be called from here.
> */
> d = pci_host_bridge_of_msi_domain(bus);
> + if (!d)
> + d = pci_host_bridge_acpi_msi_domain(bus);
>
> return d;
> }
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index d5e5c5b..a06feda 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -410,6 +410,11 @@ static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
> static inline void irq_dispose_mapping(unsigned int virq) { }
> static inline void irq_domain_activate_irq(struct irq_data *data) { }
> static inline void irq_domain_deactivate_irq(struct irq_data *data) { }
> +static inline struct irq_domain *irq_find_matching_fwnode(
> + struct fwnode_handle *fwnode, enum irq_domain_bus_token bus_token)
> +{
> + return NULL;
> +}
> #endif /* !CONFIG_IRQ_DOMAIN */
>
> #endif /* _LINUX_IRQDOMAIN_H */
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 6ae25aa..d86378c 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1946,6 +1946,16 @@ static inline struct irq_domain *
> pci_host_bridge_of_msi_domain(struct pci_bus *bus) { return NULL; }
> #endif /* CONFIG_OF */
>
> +#ifdef CONFIG_ACPI
> +struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus);
> +
> +void
> +pci_msi_register_fwnode_provider(struct fwnode_handle *(*fn)(struct device *));
> +#else
> +static inline struct irq_domain *
> +pci_host_bridge_acpi_msi_domain(struct pci_bus *bus) { return NULL; }
> +#endif
> +
> #ifdef CONFIG_EEH
> static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
> {
Other than the couple of nits above:
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
M.
--
Without deviation from the norm, progress is not possible.
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <marc.zyngier@arm.com>
To: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Cc: tglx@linutronix.de, jason@lakedaemon.net, rjw@rjwysocki.net,
Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
Will Deacon <will.deacon@arm.com>,
Catalin Marinas <Catalin.Marinas@arm.com>,
hanjun.guo@linaro.org, tomasz.nowicki@linaro.org,
graeme.gregory@linaro.org, dhdang@apm.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org
Subject: Re: [PATCH v5 1/5] acpi: pci: Setup MSI domain for ACPI based pci devices
Date: Wed, 9 Dec 2015 10:13:55 +0000 [thread overview]
Message-ID: <20151209101355.68eab0cc@why.wild-wind.fr.eu.org> (raw)
In-Reply-To: <1449625686-5594-2-git-send-email-Suravee.Suthikulpanit@amd.com>
On Tue, 8 Dec 2015 17:48:02 -0800
Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> wrote:
> This patch introduces pci_msi_register_fwnode_provider() for irqchip
> to register a callback, to provide a way to determine appropriate MSI
> domain for a pci device.
>
> It also introduces pci_host_bridge_acpi_msi_domain(), which returns
> the MSI domain of the specified PCI host bridge with DOMAIN_BUS_PCI_MSI
> bus token. Then, it is assigned to pci device.
>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> ---
> drivers/pci/pci-acpi.c | 32 ++++++++++++++++++++++++++++++++
> drivers/pci/probe.c | 2 ++
> include/linux/irqdomain.h | 5 +++++
> include/linux/pci.h | 10 ++++++++++
> 4 files changed, 49 insertions(+)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index a32ba75..a555d7e 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -9,7 +9,9 @@
>
> #include <linux/delay.h>
> #include <linux/init.h>
> +#include <linux/irqdomain.h>
> #include <linux/pci.h>
> +#include <linux/msi.h>
> #include <linux/pci_hotplug.h>
> #include <linux/module.h>
> #include <linux/pci-aspm.h>
> @@ -689,6 +691,36 @@ static struct acpi_bus_type acpi_pci_bus = {
> .cleanup = pci_acpi_cleanup,
> };
>
> +
> +static struct fwnode_handle *(*pci_msi_get_fwnode_cb)(struct device *dev);
> +
> +/**
> + * pci_msi_register_fwnode_provider - Register callback to retrieve fwnode
> + * @fn: The interrupt domain to retrieve
This is the wrong description. Maybe something like@
* @fn: Callback matching a device to a fwnode that identifies a PCI
MSI domain
Or something along those lines.
> + *
> + * This should be called by irqchip driver, which is the parent of
> + * the MSI domain to provide callback interface to query fwnode.
> + */
> +void
> +pci_msi_register_fwnode_provider(struct fwnode_handle *(*fn)(struct device *))
> +{
> + pci_msi_get_fwnode_cb = fn;
> +}
> +
> +struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus)
> +{
Documentation?
> + struct irq_domain *dom = NULL;
> + struct fwnode_handle *fwnode = NULL;
> +
> + if (pci_msi_get_fwnode_cb)
> + fwnode = pci_msi_get_fwnode_cb(&bus->dev);
> +
> + if (fwnode)
> + dom = irq_find_matching_fwnode(fwnode,
> + DOMAIN_BUS_PCI_MSI);
> + return dom;
> +}
> +
Suggestion:
struct fwnode_handle *fwnode;
if (!pci_msi_get_fwnode_cb)
return NULL;
fwnode = pci_msi_get_fwnode_cb(&bus->dev);
if (!fwnode)
return NULL;
return irq_find_matching_fwnode(...);
My own personal taste, though.
> static int __init acpi_pci_init(void)
> {
> int ret;
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index edb1984..553a029 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -672,6 +672,8 @@ static struct irq_domain *pci_host_bridge_msi_domain(struct pci_bus *bus)
> * should be called from here.
> */
> d = pci_host_bridge_of_msi_domain(bus);
> + if (!d)
> + d = pci_host_bridge_acpi_msi_domain(bus);
>
> return d;
> }
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index d5e5c5b..a06feda 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -410,6 +410,11 @@ static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
> static inline void irq_dispose_mapping(unsigned int virq) { }
> static inline void irq_domain_activate_irq(struct irq_data *data) { }
> static inline void irq_domain_deactivate_irq(struct irq_data *data) { }
> +static inline struct irq_domain *irq_find_matching_fwnode(
> + struct fwnode_handle *fwnode, enum irq_domain_bus_token bus_token)
> +{
> + return NULL;
> +}
> #endif /* !CONFIG_IRQ_DOMAIN */
>
> #endif /* _LINUX_IRQDOMAIN_H */
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 6ae25aa..d86378c 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1946,6 +1946,16 @@ static inline struct irq_domain *
> pci_host_bridge_of_msi_domain(struct pci_bus *bus) { return NULL; }
> #endif /* CONFIG_OF */
>
> +#ifdef CONFIG_ACPI
> +struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus);
> +
> +void
> +pci_msi_register_fwnode_provider(struct fwnode_handle *(*fn)(struct device *));
> +#else
> +static inline struct irq_domain *
> +pci_host_bridge_acpi_msi_domain(struct pci_bus *bus) { return NULL; }
> +#endif
> +
> #ifdef CONFIG_EEH
> static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
> {
Other than the couple of nits above:
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
M.
--
Without deviation from the norm, progress is not possible.
WARNING: multiple messages have this Message-ID (diff)
From: marc.zyngier@arm.com (Marc Zyngier)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 1/5] acpi: pci: Setup MSI domain for ACPI based pci devices
Date: Wed, 9 Dec 2015 10:13:55 +0000 [thread overview]
Message-ID: <20151209101355.68eab0cc@why.wild-wind.fr.eu.org> (raw)
In-Reply-To: <1449625686-5594-2-git-send-email-Suravee.Suthikulpanit@amd.com>
On Tue, 8 Dec 2015 17:48:02 -0800
Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> wrote:
> This patch introduces pci_msi_register_fwnode_provider() for irqchip
> to register a callback, to provide a way to determine appropriate MSI
> domain for a pci device.
>
> It also introduces pci_host_bridge_acpi_msi_domain(), which returns
> the MSI domain of the specified PCI host bridge with DOMAIN_BUS_PCI_MSI
> bus token. Then, it is assigned to pci device.
>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> ---
> drivers/pci/pci-acpi.c | 32 ++++++++++++++++++++++++++++++++
> drivers/pci/probe.c | 2 ++
> include/linux/irqdomain.h | 5 +++++
> include/linux/pci.h | 10 ++++++++++
> 4 files changed, 49 insertions(+)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index a32ba75..a555d7e 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -9,7 +9,9 @@
>
> #include <linux/delay.h>
> #include <linux/init.h>
> +#include <linux/irqdomain.h>
> #include <linux/pci.h>
> +#include <linux/msi.h>
> #include <linux/pci_hotplug.h>
> #include <linux/module.h>
> #include <linux/pci-aspm.h>
> @@ -689,6 +691,36 @@ static struct acpi_bus_type acpi_pci_bus = {
> .cleanup = pci_acpi_cleanup,
> };
>
> +
> +static struct fwnode_handle *(*pci_msi_get_fwnode_cb)(struct device *dev);
> +
> +/**
> + * pci_msi_register_fwnode_provider - Register callback to retrieve fwnode
> + * @fn: The interrupt domain to retrieve
This is the wrong description. Maybe something like@
* @fn: Callback matching a device to a fwnode that identifies a PCI
MSI domain
Or something along those lines.
> + *
> + * This should be called by irqchip driver, which is the parent of
> + * the MSI domain to provide callback interface to query fwnode.
> + */
> +void
> +pci_msi_register_fwnode_provider(struct fwnode_handle *(*fn)(struct device *))
> +{
> + pci_msi_get_fwnode_cb = fn;
> +}
> +
> +struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus)
> +{
Documentation?
> + struct irq_domain *dom = NULL;
> + struct fwnode_handle *fwnode = NULL;
> +
> + if (pci_msi_get_fwnode_cb)
> + fwnode = pci_msi_get_fwnode_cb(&bus->dev);
> +
> + if (fwnode)
> + dom = irq_find_matching_fwnode(fwnode,
> + DOMAIN_BUS_PCI_MSI);
> + return dom;
> +}
> +
Suggestion:
struct fwnode_handle *fwnode;
if (!pci_msi_get_fwnode_cb)
return NULL;
fwnode = pci_msi_get_fwnode_cb(&bus->dev);
if (!fwnode)
return NULL;
return irq_find_matching_fwnode(...);
My own personal taste, though.
> static int __init acpi_pci_init(void)
> {
> int ret;
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index edb1984..553a029 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -672,6 +672,8 @@ static struct irq_domain *pci_host_bridge_msi_domain(struct pci_bus *bus)
> * should be called from here.
> */
> d = pci_host_bridge_of_msi_domain(bus);
> + if (!d)
> + d = pci_host_bridge_acpi_msi_domain(bus);
>
> return d;
> }
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index d5e5c5b..a06feda 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -410,6 +410,11 @@ static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
> static inline void irq_dispose_mapping(unsigned int virq) { }
> static inline void irq_domain_activate_irq(struct irq_data *data) { }
> static inline void irq_domain_deactivate_irq(struct irq_data *data) { }
> +static inline struct irq_domain *irq_find_matching_fwnode(
> + struct fwnode_handle *fwnode, enum irq_domain_bus_token bus_token)
> +{
> + return NULL;
> +}
> #endif /* !CONFIG_IRQ_DOMAIN */
>
> #endif /* _LINUX_IRQDOMAIN_H */
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 6ae25aa..d86378c 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1946,6 +1946,16 @@ static inline struct irq_domain *
> pci_host_bridge_of_msi_domain(struct pci_bus *bus) { return NULL; }
> #endif /* CONFIG_OF */
>
> +#ifdef CONFIG_ACPI
> +struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus);
> +
> +void
> +pci_msi_register_fwnode_provider(struct fwnode_handle *(*fn)(struct device *));
> +#else
> +static inline struct irq_domain *
> +pci_host_bridge_acpi_msi_domain(struct pci_bus *bus) { return NULL; }
> +#endif
> +
> #ifdef CONFIG_EEH
> static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
> {
Other than the couple of nits above:
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
M.
--
Without deviation from the norm, progress is not possible.
next prev parent reply other threads:[~2015-12-09 10:08 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-09 1:48 [PATCH v5 0/5] gicv2m: acpi: Add ACPI support for GICv2m MSI Suravee Suthikulpanit
2015-12-09 1:48 ` Suravee Suthikulpanit
2015-12-09 1:48 ` Suravee Suthikulpanit
2015-12-09 1:48 ` [PATCH v5 1/5] acpi: pci: Setup MSI domain for ACPI based pci devices Suravee Suthikulpanit
2015-12-09 1:48 ` Suravee Suthikulpanit
2015-12-09 1:48 ` Suravee Suthikulpanit
2015-12-09 10:13 ` Marc Zyngier [this message]
2015-12-09 10:13 ` Marc Zyngier
2015-12-09 10:13 ` Marc Zyngier
2015-12-09 17:41 ` Suravee Suthikulanit
2015-12-09 17:41 ` Suravee Suthikulanit
2015-12-09 17:41 ` Suravee Suthikulanit
2015-12-09 1:48 ` [PATCH v5 2/5] irqdomain: introduce is_fwnode_irqchip helper Suravee Suthikulpanit
2015-12-09 1:48 ` Suravee Suthikulpanit
2015-12-09 1:48 ` Suravee Suthikulpanit
2015-12-09 10:20 ` Marc Zyngier
2015-12-09 10:20 ` Marc Zyngier
2015-12-09 10:20 ` Marc Zyngier
2015-12-09 1:48 ` [PATCH v5 3/5] irqdomain: Introduce irq_domain_get_irqchip_fwnode_name helper function Suravee Suthikulpanit
2015-12-09 1:48 ` Suravee Suthikulpanit
2015-12-09 1:48 ` Suravee Suthikulpanit
2015-12-09 1:48 ` [PATCH v5 4/5] gicv2m: Refactor to prepare for ACPI support Suravee Suthikulpanit
2015-12-09 1:48 ` Suravee Suthikulpanit
2015-12-09 1:48 ` Suravee Suthikulpanit
2015-12-09 10:25 ` Marc Zyngier
2015-12-09 10:25 ` Marc Zyngier
2015-12-09 10:25 ` Marc Zyngier
2015-12-09 1:48 ` [PATCH v5 5/5] gicv2m: acpi: Introducing GICv2m " Suravee Suthikulpanit
2015-12-09 1:48 ` Suravee Suthikulpanit
2015-12-09 1:48 ` Suravee Suthikulpanit
2015-12-09 10:38 ` Marc Zyngier
2015-12-09 10:38 ` Marc Zyngier
2015-12-09 10:38 ` Marc Zyngier
2015-12-09 18:02 ` Suravee Suthikulanit
2015-12-09 18:02 ` Suravee Suthikulanit
2015-12-09 18:02 ` Suravee Suthikulanit
2015-12-09 18:16 ` Marc Zyngier
2015-12-09 18:16 ` Marc Zyngier
2015-12-09 18:51 ` Suravee Suthikulanit
2015-12-09 18:51 ` Suravee Suthikulanit
2015-12-09 18:51 ` Suravee Suthikulanit
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=20151209101355.68eab0cc@why.wild-wind.fr.eu.org \
--to=marc.zyngier@arm.com \
--cc=Catalin.Marinas@arm.com \
--cc=Suravee.Suthikulpanit@amd.com \
--cc=dhdang@apm.com \
--cc=graeme.gregory@linaro.org \
--cc=hanjun.guo@linaro.org \
--cc=jason@lakedaemon.net \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=rjw@rjwysocki.net \
--cc=tglx@linutronix.de \
--cc=tomasz.nowicki@linaro.org \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.