| Message ID | 1524880132-100904-1-git-send-email-shawn.lin@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Sat, Apr 28, 2018 at 09:48:52AM +0800, Shawn Lin wrote: > It seems host drivers which allocate IRQ domain for INTx > and the related code block looks highly similar to each other. > Add a new helper, pci_alloc_intx_irqd(), to avoid code duplication > as much as possible. > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> Maybe add a _host to the name as in pci_host_alloc_intx_irqdomain, and potentially move it to a different file than irq.c, as that contains helpers for drivers.
Hi Shawn,
I love your patch! Perhaps something to improve:
[auto build test WARNING on pci/next]
[also build test WARNING on v4.17-rc2 next-20180426]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Shawn-Lin/Add-new-helper-to-allocate-IRQ-domain-for-host-drivers/20180430-025537
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: s390-allmodconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=s390
All warnings (new ones prefixed by >>):
In file included from arch/s390/include/asm/hw_irq.h:6:0,
from include/linux/irq.h:522,
from include/linux/of_irq.h:7,
from drivers/dma/mv_xor.c:26:
>> include/linux/pci.h:1455:35: warning: 'struct irq_domain_ops' declared inside parameter list will not be visible outside of this definition or declaration
bool general_xlate, const struct irq_domain_ops *intx_domain_ops,
^~~~~~~~~~~~~~
vim +1455 include/linux/pci.h
1453
1454 extern struct irq_domain *pci_alloc_intx_irqd(struct device *dev, void *host,
> 1455 bool general_xlate, const struct irq_domain_ops *intx_domain_ops,
1456 struct device_node *local_intc);
1457 #ifdef CONFIG_PCIEPORTBUS
1458 extern bool pcie_ports_disabled;
1459 #else
1460 #define pcie_ports_disabled true
1461 #endif
1462
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Shawn,
I love your patch! Yet something to improve:
[auto build test ERROR on pci/next]
[also build test ERROR on v4.17-rc3 next-20180426]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Shawn-Lin/Add-new-helper-to-allocate-IRQ-domain-for-host-drivers/20180430-025537
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh
All errors (new ones prefixed by >>):
In file included from arch/sh/kernel/io.c:12:0:
>> include/linux/pci.h:1692:27: error: 'pci_alloc_intx_irqd' defined but not used [-Werror=unused-function]
static struct irq_domain *pci_alloc_intx_irqd(struct device *dev, void *host,
^~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
vim +/pci_alloc_intx_irqd +1692 include/linux/pci.h
1691
> 1692 static struct irq_domain *pci_alloc_intx_irqd(struct device *dev, void *host,
1693 bool general_xlate, const struct irq_domain_ops *intx_domain_ops,
1694 struct device_node *local_intc)
1695 { return ERR_PTR(-EINVAL); }
1696 #endif /* CONFIG_PCI */
1697
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/pci/irq.c b/drivers/pci/irq.c index 2a808e1..57fe0f4 100644 --- a/drivers/pci/irq.c +++ b/drivers/pci/irq.c @@ -118,3 +118,71 @@ void pci_free_irq(struct pci_dev *dev, unsigned int nr, void *dev_id) kfree(free_irq(pci_irq_vector(dev, nr), dev_id)); } EXPORT_SYMBOL(pci_free_irq); + +static int pcie_intx_map(struct irq_domain *domain, unsigned int irq, + irq_hw_number_t hwirq) +{ + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq); + irq_set_chip_data(irq, domain->host_data); + + return 0; +} + +static struct irq_domain_ops pci_intx_domain_ops = { + .map = pcie_intx_map, +}; + +/** + * pci_alloc_intx_irqd() - Allocate INTx IRQ domain + * @dev: device associated with the PCI controller. + * @host: pointer to host specific data struct + * @general_xlate: flag for whether use pci_irqd_intx_xlate() helper + * @intx_domain_ops: pointer to driver specific struct irq_domain_ops + * @local_intc: pointer to driver specific interrupt controller node + * + * A simple helper for drivers to allocate IRQ domain for INTx. If + * intx_domain_ops is NULL, use pci_intx_domain_ops by default. And if + * local_intc is present, then use it firstly, otherwise, fallback to get + * interrupt controller node from @dev. + * + * Returns valid pointer of struct irq_domain on success, or PTR_ERR(-EINVAL) + * if failure occurred. + */ +struct irq_domain *pci_alloc_intx_irqd(struct device *dev, void *host, + bool general_xlate, const struct irq_domain_ops *intx_domain_ops, + struct device_node *local_intc) +{ + struct device_node *intc = local_intc; + struct irq_domain *domain; + const struct irq_domain_ops *irqd_ops; + bool need_put = false; + + if (!intc) { + intc = of_get_next_child(dev->of_node, NULL); + if (!intc) { + dev_err(dev, "missing child interrupt-controller node\n"); + return ERR_PTR(-EINVAL); + } + need_put = true; + } + + if (intx_domain_ops) { + irqd_ops = intx_domain_ops; + } else { + if (general_xlate) + pci_intx_domain_ops.xlate = &pci_irqd_intx_xlate; + irqd_ops = &pci_intx_domain_ops; + } + + domain = irq_domain_add_linear(intc, PCI_NUM_INTX, + irqd_ops, host); + if (!domain) { + dev_err(dev, "failed to get a INTx IRQ domain\n"); + if (need_put) + of_node_put(intc); + return ERR_PTR(-EINVAL); + } + + return domain; +} +EXPORT_SYMBOL_GPL(pci_alloc_intx_irqd); diff --git a/include/linux/pci.h b/include/linux/pci.h index 73178a2..18cc39f 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -31,6 +31,8 @@ #include <linux/device.h> #include <linux/interrupt.h> #include <linux/io.h> +#include <linux/irq.h> +#include <linux/of_irq.h> #include <linux/resource_ext.h> #include <uapi/linux/pci.h> @@ -1449,6 +1451,9 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d, return 0; } +extern struct irq_domain *pci_alloc_intx_irqd(struct device *dev, void *host, + bool general_xlate, const struct irq_domain_ops *intx_domain_ops, + struct device_node *local_intc); #ifdef CONFIG_PCIEPORTBUS extern bool pcie_ports_disabled; #else @@ -1683,6 +1688,11 @@ static inline int pci_irqd_intx_xlate(struct irq_domain *d, unsigned long *out_hwirq, unsigned int *out_type) { return -EINVAL; } + +static struct irq_domain *pci_alloc_intx_irqd(struct device *dev, void *host, + bool general_xlate, const struct irq_domain_ops *intx_domain_ops, + struct device_node *local_intc) +{ return ERR_PTR(-EINVAL); } #endif /* CONFIG_PCI */ /* Include architecture-dependent settings and functions */
It seems host drivers which allocate IRQ domain for INTx and the related code block looks highly similar to each other. Add a new helper, pci_alloc_intx_irqd(), to avoid code duplication as much as possible. Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- Changes in v2: - fix typo and move the code to C file. drivers/pci/irq.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 10 ++++++++ 2 files changed, 78 insertions(+)