[OpenWrt-Devel] [PATCH] ath79: ar71xx create a separate driver for ar71xx pci interrupt controller.
Chuanhong Guo
gch981213 at gmail.com
Tue Aug 21 12:03:11 EDT 2018
Hi!
Comments inline:
Dmitry Tunin <hanipouspilot at gmail.com> 于2018年8月21日周二 下午10:59写道：
>
> It is based on Chuanhong Guo work.
>
> PCI interrupt controller is not part of PCI. It is a part of reset controller
> with 0x18060018, 0x1806001c control registers.
>
> This should fix a bug with one IRQ for all PCI devices and also handle the PCI_CORE interrupt.
No it doesn't. Assigning it in dts doesn't mean that we "handled" it.
But I still like this patch because it's doing a cleanup of the messy code.
>
> I am not sure that this kind of cascading is good for performance, but this way we are more flexible when cofiguring IRQs.
It shouldn't affect performance because doing this separation doesn't
change the way it dispatches PCI IRQ.
>
> Run tested on DIR-825 B1.
> ---
> target/linux/ath79/dts/ar7100.dtsi | 22 ++-
> ...turn-pci-ar71xx-driver-into-a-pure-OF-dri.patch | 168 ++++++++---------
> .../0036-MIPS-ath79-add-pci-intc.patch | 205 +++++++++++++++++++++
> 3 files changed, 306 insertions(+), 89 deletions(-)
> create mode 100644 target/linux/ath79/patches-4.14/0036-MIPS-ath79-add-pci-intc.patch
>
> diff --git a/target/linux/ath79/dts/ar7100.dtsi b/target/linux/ath79/dts/ar7100.dtsi
> index 8994a7d..6dc1751 100644
> --- a/target/linux/ath79/dts/ar7100.dtsi
> +++ b/target/linux/ath79/dts/ar7100.dtsi
> @@ -96,6 +96,16 @@
> #reset-cells = <1>;
> };
>
> + pci_intc: interrupt-controller at 18060018 {
> + compatible = "qca,ar7100-pci-intc";
> + reg = <0x18060018 0x4>;
> + interrupt-parent = <&cpuintc>;
> + interrupts = <2>;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + };
> +
For other chips this node is a subnode of reset-controller. But in
this case here we can just put this above reset-controller at 18060024.
Nodes in dts are supposed to be ordered by register address.
> +
> pcie0: pcie-controller at 180c0000 {
> compatible = "qca,ar7100-pci";
> #address-cells = <3>;
> @@ -105,14 +115,16 @@
> reg-names = "cfg_base";
> ranges = <0x2000000 0 0x10000000 0x10000000 0 0x07000000 /* pci memory */
> 0x1000000 0 0x00000000 0x0000000 0 0x000001>; /* io space */
> - interrupt-parent = <&cpuintc>;
> - interrupts = <2>;
> + interrupt-parent = <&pci_intc>;
> + interrupts = <4>;
Do we really need this? We don't even have a handler that actually do
anything for it. I think the above two lines can simply be dropped.
>
> - interrupt-controller;
> #interrupt-cells = <1>;
>
> - interrupt-map-mask = <0 0 0 1>;
> - interrupt-map = <0 0 0 0 &pcie0 0>;
> + interrupt-map-mask = <0xf800 0 0 0>;
> + interrupt-map = <0x8800 0 0 0 &pci_intc 1
> + 0x9000 0 0 0 &pci_intc 2
> + 0x9800 0 0 0 &pci_intc 3>;
> +
> status = "disabled";
> };
> };
> diff --git a/target/linux/ath79/patches-4.14/0020-MIPS-ath79-turn-pci-ar71xx-driver-into-a-pure-OF-dri.patch b/target/linux/ath79/patches-4.14/0020-MIPS-ath79-turn-pci-ar71xx-driver-into-a-pure-OF-dri.patch
> index ea3514a..95ca6d1 100644
> --- a/target/linux/ath79/patches-4.14/0020-MIPS-ath79-turn-pci-ar71xx-driver-into-a-pure-OF-dri.patch
> +++ b/target/linux/ath79/patches-4.14/0020-MIPS-ath79-turn-pci-ar71xx-driver-into-a-pure-OF-dri.patch
It's not a good idea to mix the removal of IRQ part into this patch. I
suggest using a separated patch to do the removing.
> @@ -9,8 +9,10 @@ Signed-off-by: John Crispin <john at phrozen.org>
> arch/mips/pci/pci-ar71xx.c | 81 +++++++++++++++++++++++-----------------------
> 1 file changed, 40 insertions(+), 41 deletions(-)
>
> ---- a/arch/mips/pci/pci-ar71xx.c
> -+++ b/arch/mips/pci/pci-ar71xx.c
> +Index: linux-4.14.65/arch/mips/pci/pci-ar71xx.c
> +===================================================================
> +--- linux-4.14.65.orig/arch/mips/pci/pci-ar71xx.c
> ++++ linux-4.14.65/arch/mips/pci/pci-ar71xx.c
> @@ -18,8 +18,11 @@
> #include <linux/pci.h>
> #include <linux/pci_regs.h>
> @@ -38,89 +40,86 @@ Signed-off-by: John Crispin <john at phrozen.org>
> };
>
> /* Byte lane enable bits */
> -@@ -228,29 +232,30 @@ static struct pci_ops ar71xx_pci_ops = {
> +@@ -226,96 +230,6 @@ static struct pci_ops ar71xx_pci_ops = {
> + .write = ar71xx_pci_write_config,
> + };
>
> - static void ar71xx_pci_irq_handler(struct irq_desc *desc)
> - {
> +-static void ar71xx_pci_irq_handler(struct irq_desc *desc)
> +-{
> - struct ar71xx_pci_controller *apc;
> - void __iomem *base = ath79_reset_base;
> -+ struct irq_chip *chip = irq_desc_get_chip(desc);
> -+ struct ar71xx_pci_controller *apc = irq_desc_get_handler_data(desc);
> - u32 pending;
> -
> +- void __iomem *base = ath79_reset_base;
> +- u32 pending;
> +-
> - apc = irq_desc_get_handler_data(desc);
> -
> -+ chained_irq_enter(chip, desc);
> - pending = __raw_readl(base + AR71XX_RESET_REG_PCI_INT_STATUS) &
> - __raw_readl(base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> -
> - if (pending & AR71XX_PCI_INT_DEV0)
> +- pending = __raw_readl(base + AR71XX_RESET_REG_PCI_INT_STATUS) &
> +- __raw_readl(base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> +-
> +- if (pending & AR71XX_PCI_INT_DEV0)
> - generic_handle_irq(apc->irq_base + 0);
> -+ generic_handle_irq(irq_linear_revmap(apc->domain, 1));
> -
> - else if (pending & AR71XX_PCI_INT_DEV1)
> +-
> +- else if (pending & AR71XX_PCI_INT_DEV1)
> - generic_handle_irq(apc->irq_base + 1);
> -+ generic_handle_irq(irq_linear_revmap(apc->domain, 2));
> -
> - else if (pending & AR71XX_PCI_INT_DEV2)
> +-
> +- else if (pending & AR71XX_PCI_INT_DEV2)
> - generic_handle_irq(apc->irq_base + 2);
> -+ generic_handle_irq(irq_linear_revmap(apc->domain, 3));
> -
> - else if (pending & AR71XX_PCI_INT_CORE)
> +-
> +- else if (pending & AR71XX_PCI_INT_CORE)
> - generic_handle_irq(apc->irq_base + 4);
> -+ generic_handle_irq(irq_linear_revmap(apc->domain, 4));
> -
> - else
> - spurious_interrupt();
> -+ chained_irq_exit(chip, desc);
> - }
> -
> - static void ar71xx_pci_irq_unmask(struct irq_data *d)
> -@@ -261,7 +266,7 @@ static void ar71xx_pci_irq_unmask(struct
> - u32 t;
> -
> - apc = irq_data_get_irq_chip_data(d);
> +-
> +- else
> +- spurious_interrupt();
> +-}
> +-
> +-static void ar71xx_pci_irq_unmask(struct irq_data *d)
> +-{
> +- struct ar71xx_pci_controller *apc;
> +- unsigned int irq;
> +- void __iomem *base = ath79_reset_base;
> +- u32 t;
> +-
> +- apc = irq_data_get_irq_chip_data(d);
> - irq = d->irq - apc->irq_base;
> -+ irq = irq_linear_revmap(apc->domain, d->irq);
> -
> - t = __raw_readl(base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> - __raw_writel(t | (1 << irq), base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> -@@ -278,7 +283,7 @@ static void ar71xx_pci_irq_mask(struct i
> - u32 t;
> -
> - apc = irq_data_get_irq_chip_data(d);
> +-
> +- t = __raw_readl(base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> +- __raw_writel(t | (1 << irq), base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> +-
> +- /* flush write */
> +- __raw_readl(base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> +-}
> +-
> +-static void ar71xx_pci_irq_mask(struct irq_data *d)
> +-{
> +- struct ar71xx_pci_controller *apc;
> +- unsigned int irq;
> +- void __iomem *base = ath79_reset_base;
> +- u32 t;
> +-
> +- apc = irq_data_get_irq_chip_data(d);
> - irq = d->irq - apc->irq_base;
> -+ irq = irq_linear_revmap(apc->domain, d->irq);
> -
> - t = __raw_readl(base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> - __raw_writel(t & ~(1 << irq), base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> -@@ -294,24 +299,30 @@ static struct irq_chip ar71xx_pci_irq_ch
> - .irq_mask_ack = ar71xx_pci_irq_mask,
> - };
> -
> -+static int ar71xx_pci_irq_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
> -+{
> -+ struct ar71xx_pci_controller *apc = d->host_data;
> -+
> -+ irq_set_chip_and_handler(irq, &ar71xx_pci_irq_chip, handle_level_irq);
> -+ irq_set_chip_data(irq, apc);
> -+
> -+ return 0;
> -+}
> -+
> -+static const struct irq_domain_ops ar71xx_pci_domain_ops = {
> -+ .xlate = irq_domain_xlate_onecell,
> -+ .map = ar71xx_pci_irq_map,
> -+};
> -+
> - static void ar71xx_pci_irq_init(struct ar71xx_pci_controller *apc)
> - {
> - void __iomem *base = ath79_reset_base;
> +-
> +- t = __raw_readl(base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> +- __raw_writel(t & ~(1 << irq), base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> +-
> +- /* flush write */
> +- __raw_readl(base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> +-}
> +-
> +-static struct irq_chip ar71xx_pci_irq_chip = {
> +- .name = "AR71XX PCI",
> +- .irq_mask = ar71xx_pci_irq_mask,
> +- .irq_unmask = ar71xx_pci_irq_unmask,
> +- .irq_mask_ack = ar71xx_pci_irq_mask,
> +-};
> +-
> +-static void ar71xx_pci_irq_init(struct ar71xx_pci_controller *apc)
> +-{
> +- void __iomem *base = ath79_reset_base;
> - int i;
> -
> - __raw_writel(0, base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> - __raw_writel(0, base + AR71XX_RESET_REG_PCI_INT_STATUS);
> -
> +-
> +- __raw_writel(0, base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> +- __raw_writel(0, base + AR71XX_RESET_REG_PCI_INT_STATUS);
> +-
> - BUILD_BUG_ON(ATH79_PCI_IRQ_COUNT < AR71XX_PCI_IRQ_COUNT);
> -
> - apc->irq_base = ATH79_PCI_IRQ_BASE;
> @@ -131,12 +130,14 @@ Signed-off-by: John Crispin <john at phrozen.org>
> - irq_set_chip_data(i, apc);
> - }
> -
> -+ apc->domain = irq_domain_add_linear(apc->np, AR71XX_PCI_IRQ_COUNT,
> -+ &ar71xx_pci_domain_ops, apc);
> - irq_set_chained_handler_and_data(apc->irq, ar71xx_pci_irq_handler,
> - apc);
> - }
> -@@ -328,6 +339,11 @@ static void ar71xx_pci_reset(void)
> +- irq_set_chained_handler_and_data(apc->irq, ar71xx_pci_irq_handler,
> +- apc);
> +-}
> +-
> + static void ar71xx_pci_reset(void)
> + {
> + ath79_device_reset_set(AR71XX_RESET_PCI_BUS | AR71XX_RESET_PCI_CORE);
> +@@ -328,6 +242,11 @@ static void ar71xx_pci_reset(void)
> mdelay(100);
> }
>
> @@ -148,7 +149,7 @@ Signed-off-by: John Crispin <john at phrozen.org>
> static int ar71xx_pci_probe(struct platform_device *pdev)
> {
> struct ar71xx_pci_controller *apc;
> -@@ -348,26 +364,6 @@ static int ar71xx_pci_probe(struct platf
> +@@ -348,26 +267,6 @@ static int ar71xx_pci_probe(struct platf
> if (apc->irq < 0)
> return -EINVAL;
>
> @@ -175,7 +176,7 @@ Signed-off-by: John Crispin <john at phrozen.org>
> ar71xx_pci_reset();
>
> /* setup COMMAND register */
> -@@ -378,11 +374,13 @@ static int ar71xx_pci_probe(struct platf
> +@@ -378,11 +277,12 @@ static int ar71xx_pci_probe(struct platf
> /* clear bus errors */
> ar71xx_pci_check_error(apc, 1);
>
> @@ -187,11 +188,10 @@ Signed-off-by: John Crispin <john at phrozen.org>
> apc->pci_ctrl.io_resource = &apc->io_res;
> + pci_load_of_ranges(&apc->pci_ctrl, pdev->dev.of_node);
> +
> -+ ar71xx_pci_irq_init(apc);
>
> register_pci_controller(&apc->pci_ctrl);
>
> -@@ -393,6 +391,7 @@ static struct platform_driver ar71xx_pci
> +@@ -393,6 +293,7 @@ static struct platform_driver ar71xx_pci
> .probe = ar71xx_pci_probe,
> .driver = {
> .name = "ar71xx-pci",
> diff --git a/target/linux/ath79/patches-4.14/0036-MIPS-ath79-add-pci-intc.patch b/target/linux/ath79/patches-4.14/0036-MIPS-ath79-add-pci-intc.patch
> new file mode 100644
> index 0000000..23d6a55
> --- /dev/null
> +++ b/target/linux/ath79/patches-4.14/0036-MIPS-ath79-add-pci-intc.patch
> @@ -0,0 +1,205 @@
> +Index: linux-4.14.65/drivers/irqchip/Makefile
> +===================================================================
> +--- linux-4.14.65.orig/drivers/irqchip/Makefile
> ++++ linux-4.14.65/drivers/irqchip/Makefile
> +@@ -5,6 +5,7 @@ obj-$(CONFIG_ALPINE_MSI) += irq-alpine-
> + obj-$(CONFIG_ATH79) += irq-ath79-cpu.o
> + obj-$(CONFIG_ATH79) += irq-ath79-intc.o
> + obj-$(CONFIG_ATH79) += irq-ath79-misc.o
> ++obj-$(CONFIG_ATH79) += irq-ath79-pci.o
> + obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2835.o
> + obj-$(CONFIG_ARCH_BCM2835) += irq-bcm2836.o
> + obj-$(CONFIG_ARCH_EXYNOS) += exynos-combiner.o
> +Index: linux-4.14.65/drivers/irqchip/irq-ath79-pci.c
> +===================================================================
> +--- /dev/null
> ++++ linux-4.14.65/drivers/irqchip/irq-ath79-pci.c
I haven't read the code thoroghly but it seemed that irq-ath79-misc
works in a similar way. Is it possible to use the misc intc driver for
PCI?
> +@@ -0,0 +1,188 @@
> ++/*
> ++ * Atheros AR71xx PCI interrupt controller
> ++ *
> ++ * Copyright (C) 2015 Alban Bedel <albeu at free.fr>
> ++ * Copyright (C) 2010-2011 Jaiganesh Narayanan <jnarayanan at atheros.com>
> ++ * Copyright (C) 2008-2011 Gabor Juhos <juhosg at openwrt.org>
> ++ * Copyright (C) 2008 Imre Kaloz <kaloz at openwrt.org>
> ++ *
> ++ * Parts of this file are based on Atheros' 2.6.15/2.6.31 BSP
> ++ *
> ++ * This program is free software; you can redistribute it and/or modify it
> ++ * under the terms of the GNU General Public License version 2 as published
> ++ * by the Free Software Foundation.
> ++ */
> ++
> ++#include <linux/irqchip.h>
> ++#include <linux/irqchip/chained_irq.h>
> ++#include <linux/of_address.h>
> ++#include <linux/of_irq.h>
> ++
> ++#define AR71XX_RESET_REG_PCI_INT_STATUS 0
> ++#define AR71XX_RESET_REG_PCI_INT_ENABLE 4
> ++
> ++#define AR71XX_PCI_INT_CORE BIT(4)
> ++#define AR71XX_PCI_INT_DEV2 BIT(2)
> ++#define AR71XX_PCI_INT_DEV1 BIT(1)
> ++#define AR71XX_PCI_INT_DEV0 BIT(0)
> ++
> ++#define ATH79_PCI_IRQ_COUNT 5
> ++
> ++static void ath79_pci_irq_handler(struct irq_desc *desc)
> ++{
> ++ struct irq_domain *domain = irq_desc_get_handler_data(desc);
> ++ struct irq_chip *chip = irq_desc_get_chip(desc);
> ++ void __iomem *base = domain->host_data;
> ++ u32 pending;
> ++
> ++ chained_irq_enter(chip, desc);
> ++
> ++ pending = __raw_readl(base + AR71XX_RESET_REG_PCI_INT_STATUS) &
> ++ __raw_readl(base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> ++
> ++ if (pending & AR71XX_PCI_INT_DEV0)
> ++ generic_handle_irq(irq_linear_revmap(domain, 1));
> ++
> ++ else if (pending & AR71XX_PCI_INT_DEV1)
> ++ generic_handle_irq(irq_linear_revmap(domain, 2));
> ++
> ++ else if (pending & AR71XX_PCI_INT_DEV2)
> ++ generic_handle_irq(irq_linear_revmap(domain, 3));
> ++
> ++ else if (pending & AR71XX_PCI_INT_CORE)
> ++ generic_handle_irq(irq_linear_revmap(domain, 4));
> ++
> ++ else
> ++ spurious_interrupt();
> ++
> ++ chained_irq_exit(chip, desc);
> ++}
> ++
> ++static void ar71xx_pci_irq_unmask(struct irq_data *d)
> ++{
> ++ void __iomem *base = irq_data_get_irq_chip_data(d);
> ++ u32 t;
> ++
> ++ t = __raw_readl(base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> ++
> ++ switch (d->hwirq) {
> ++ case 1:
> ++ t |= AR71XX_PCI_INT_DEV0;
> ++ break;
> ++ case 2:
> ++ t |= AR71XX_PCI_INT_DEV1;
> ++ break;
> ++ case 3:
> ++ t |= AR71XX_PCI_INT_DEV2;
> ++ break;
> ++ case 4:
> ++ t |= AR71XX_PCI_INT_CORE;
> ++ break;
> ++ default:
> ++ WARN(1, "Incorrect IRQ used for PCI device.");
> ++ return;
> ++ }
> ++
> ++ __raw_writel(t, base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> ++
> ++ /* flush write */
> ++ __raw_readl(base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> ++}
> ++
> ++static void ar71xx_pci_irq_mask(struct irq_data *d)
> ++{
> ++ void __iomem *base = irq_data_get_irq_chip_data(d);
> ++ u32 t;
> ++
> ++ t = __raw_readl(base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> ++
> ++ switch (d->hwirq) {
> ++ case 1:
> ++ t &= (~AR71XX_PCI_INT_DEV0);
> ++ break;
> ++ case 2:
> ++ t &= (~AR71XX_PCI_INT_DEV1);
> ++ break;
> ++ case 3:
> ++ t &= (~AR71XX_PCI_INT_DEV2);
> ++ break;
> ++ case 4:
> ++ t &= (~AR71XX_PCI_INT_CORE);
> ++ break;
> ++ default:
> ++ WARN(1, "Incorrect IRQ used for PCI device.");
> ++ return;
> ++ }
> ++
> ++ __raw_writel(t, base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> ++
> ++ /* flush write */
> ++ __raw_readl(base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> ++}
> ++
> ++static struct irq_chip ar71xx_pci_irq_chip = {
> ++ .name = "AR71XX PCI",
> ++ .irq_unmask = ar71xx_pci_irq_unmask,
> ++ .irq_mask = ar71xx_pci_irq_mask,
> ++ .irq_mask_ack = ar71xx_pci_irq_mask,
> ++};
> ++
> ++static int ar71xx_pci_irq_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
> ++{
> ++ struct ar71xx_pci_controller *apc = d->host_data;
> ++
> ++ irq_set_chip_and_handler(irq, &ar71xx_pci_irq_chip, handle_level_irq);
> ++ irq_set_chip_data(irq, apc);
> ++
> ++ return 0;
> ++}
> ++
> ++static const struct irq_domain_ops pci_irq_domain_ops = {
> ++ .xlate = irq_domain_xlate_onecell,
> ++ .map = ar71xx_pci_irq_map,
> ++};
> ++
> ++static void __init ath79_pci_intc_domain_init(
> ++ struct irq_domain *domain, int irq)
> ++{
> ++ void __iomem *base = domain->host_data;
> ++
> ++ /* Disable and clear all interrupts */
> ++ __raw_writel(0, base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> ++ __raw_writel(0, base + AR71XX_RESET_REG_PCI_INT_STATUS);
> ++
> ++ irq_set_chained_handler_and_data(irq, ath79_pci_irq_handler, domain);
> ++}
> ++
> ++static int __init ar7100_pci_intc_of_init(
> ++ struct device_node *node, struct device_node *parent)
> ++{
> ++ struct irq_domain *domain;
> ++ void __iomem *base;
> ++ int irq;
> ++
> ++ irq = irq_of_parse_and_map(node, 0);
> ++ if (!irq) {
> ++ pr_err("Failed to get PCI IRQ\n");
> ++ return -EINVAL;
> ++ }
> ++
> ++ base = of_iomap(node, 0);
> ++ if (!base) {
> ++ pr_err("Failed to get PCI IRQ registers\n");
> ++ return -ENOMEM;
> ++ }
> ++
> ++ domain = irq_domain_add_linear(node, ATH79_PCI_IRQ_COUNT,
> ++ &pci_irq_domain_ops, base);
> ++ if (!domain) {
> ++ pr_err("Failed to add PCI irqdomain\n");
> ++ return -EINVAL;
> ++ }
> ++
> ++ ath79_pci_intc_domain_init(domain, irq);
> ++ return 0;
> ++}
> ++
> ++IRQCHIP_DECLARE(ar7100_pci_intc, "qca,ar7100-pci-intc",
> ++ ar7100_pci_intc_of_init);
> --
> 2.7.4
>
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
More information about the openwrt-devel
mailing list