From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754988AbbLIKIF (ORCPT ); Wed, 9 Dec 2015 05:08:05 -0500 Received: from foss.arm.com ([217.140.101.70]:48570 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754356AbbLIKH7 (ORCPT ); Wed, 9 Dec 2015 05:07:59 -0500 Date: Wed, 9 Dec 2015 10:13:55 +0000 From: Marc Zyngier To: Suravee Suthikulpanit Cc: , , , Lorenzo Pieralisi , Will Deacon , Catalin Marinas , , , , , , , Subject: Re: [PATCH v5 1/5] acpi: pci: Setup MSI domain for ACPI based pci devices Message-ID: <20151209101355.68eab0cc@why.wild-wind.fr.eu.org> In-Reply-To: <1449625686-5594-2-git-send-email-Suravee.Suthikulpanit@amd.com> References: <1449625686-5594-1-git-send-email-Suravee.Suthikulpanit@amd.com> <1449625686-5594-2-git-send-email-Suravee.Suthikulpanit@amd.com> Organization: ARM Ltd X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 8 Dec 2015 17:48:02 -0800 Suravee Suthikulpanit 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 > --- > 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 > #include > +#include > #include > +#include > #include > #include > #include > @@ -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 M. -- Without deviation from the norm, progress is not possible. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marc Zyngier 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 Message-ID: <20151209101355.68eab0cc@why.wild-wind.fr.eu.org> References: <1449625686-5594-1-git-send-email-Suravee.Suthikulpanit@amd.com> <1449625686-5594-2-git-send-email-Suravee.Suthikulpanit@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1449625686-5594-2-git-send-email-Suravee.Suthikulpanit@amd.com> Sender: linux-kernel-owner@vger.kernel.org To: Suravee Suthikulpanit Cc: tglx@linutronix.de, jason@lakedaemon.net, rjw@rjwysocki.net, Lorenzo Pieralisi , Will Deacon , Catalin Marinas , 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 List-Id: linux-acpi@vger.kernel.org On Tue, 8 Dec 2015 17:48:02 -0800 Suravee Suthikulpanit 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 > --- > 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 > #include > +#include > #include > +#include > #include > #include > #include > @@ -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 M. -- Without deviation from the norm, progress is not possible. From mboxrd@z Thu Jan 1 00:00:00 1970 From: marc.zyngier@arm.com (Marc Zyngier) Date: Wed, 9 Dec 2015 10:13:55 +0000 Subject: [PATCH v5 1/5] acpi: pci: Setup MSI domain for ACPI based pci devices In-Reply-To: <1449625686-5594-2-git-send-email-Suravee.Suthikulpanit@amd.com> References: <1449625686-5594-1-git-send-email-Suravee.Suthikulpanit@amd.com> <1449625686-5594-2-git-send-email-Suravee.Suthikulpanit@amd.com> Message-ID: <20151209101355.68eab0cc@why.wild-wind.fr.eu.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, 8 Dec 2015 17:48:02 -0800 Suravee Suthikulpanit 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 > --- > 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 > #include > +#include > #include > +#include > #include > #include > #include > @@ -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 M. -- Without deviation from the norm, progress is not possible.