Hi!
Comments inline:
Dmitry Tunin <hanipouspi...@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@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@18060024.
Nodes in dts are supposed to be ordered by register address.
> +
> pcie0: pcie-controller@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 <j...@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 <j...@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 <j...@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 <j...@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 <j...@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 <j...@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 <al...@free.fr>
> ++ * Copyright (C) 2010-2011 Jaiganesh Narayanan <jnaraya...@atheros.com>
> ++ * Copyright (C) 2008-2011 Gabor Juhos <juh...@openwrt.org>
> ++ * Copyright (C) 2008 Imre Kaloz <ka...@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@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
