[OpenWrt-Devel] [PATCH] ath79: ar7100: remove IRQ code from PCI driver

Dmitry Tunin hanipouspilot at gmail.com
Wed Aug 22 12:57:42 EDT 2018


Looking into irq-ath79-misc.c I found a few things to cleanup.

1. It appears that two chips are declared: "qca,ar7100-misc-intc" and
"qca,ar7240-misc-intc".
The only difference is mask-ack proc. The registers seem to work the
same way and practically we are using "qca,ar7240-misc-intc" on ar7100
according to dtsi.
I found that the other chip was declared only when looking into the code.

At the same time the code I removed from the PCI driver used the
ar7100 type of mask-ack, that worked also OK.

I see no reason of having two different ways of mask-ack for similar
hardware. We need to chose one that looks better and drop the other.

2. Instead of declaring two different chips this way, since we are
reusing this driver for PCI, I suggest to declare also
"qca,ath79-pci-intc" that will
have a different chip name = "PCI", so that "MISC and "PCI" will be
seen separately in /proc/interrupts.

3. I suggest removing legacy __init ath79_misc_irq_init() and leaving
only the OF stuff.

Any comments?
ср, 22 авг. 2018 г. в 14:47, Chuanhong Guo <gch981213 at gmail.com>:
>
> Dmitry Tunin <hanipouspilot at gmail.com> 于2018年8月22日周三 下午5:07写道:
> >
> > Currently all PCI devices get the same IRQ that affects performance badly.
> >
> > This commit adresses this problem and cleans the code.
> >
> > ar7100 has a special PCI interrupt controller at 18060018 that works exactly
> > the same way as misc interrupt controller.
> >
> > This patch does the following:
> >
> > 1. Removes all IRQ handling code from the PCI driver.
> > 2. Defines pci-intc interrupt controller at 18060018 in dtsi.
> > 3. Removes interrupt-controller property from PCI node.
> > 4. Sets a correct interrupt mask for PCI devices.
> >
> > Run tested on DIR-825 B1.
> >
> > Signed-off-by: Dmitry Tunin <hanipouspilot at gmail.com>
> > ---
> >  target/linux/ath79/dts/ar7100.dtsi                 |  21 +++-
> >  .../0036-MIPS-ath79-remove-irq-code-from-pci.patch | 117 +++++++++++++++++++++
> >  2 files changed, 133 insertions(+), 5 deletions(-)
> >  create mode 100644 target/linux/ath79/patches-4.14/0036-MIPS-ath79-remove-irq-code-from-pci.patch
> >
> > diff --git a/target/linux/ath79/dts/ar7100.dtsi b/target/linux/ath79/dts/ar7100.dtsi
> > index 8994a7d..0632050 100644
> > --- a/target/linux/ath79/dts/ar7100.dtsi
> > +++ b/target/linux/ath79/dts/ar7100.dtsi
> > @@ -88,6 +88,14 @@
> >                                 clock-names = "wdt";
> >                         };
> >
> > +                       pci_intc: interrupt-controller at 18060018 {
> > +                               compatible = "qca,ar7240-misc-intc";
> > +                               reg = <0x18060018 0x4>;
> I guess this should be <0x18060018 0x8>?
> Interrupt status is at 0x18060018-0x1806001c and interrupt mask is at
> 0x1806001c-0x18060020. The length of used memory space is 8 bytes.
> BTW I noticed that reg of miscintc is <0x18060010 0x4> but the
> register of interrupt status and mask is 0x18060010-0x18060018, also 8
> bytes.
> Are these two mistakes or my misunderstanding of the size in reg property?
> > +                               interrupt-parent = <&cpuintc>;
> > +                               interrupts = <2>;
> > +                               interrupt-controller;
> > +                               #interrupt-cells = <1>;
> > +                       };
> >
> >                         rst: reset-controller at 18060024 {
> >                                 compatible = "qca,ar7100-reset";
> > @@ -105,14 +113,17 @@
> >                                 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-controller;
> > +                               interrupt-parent = <&pci_intc>;
> > +                               interrupts = <4>;
> > +
> >                                 #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 0
> > +                                                0x9000 0 0 0 &pci_intc 1
> > +                                                0x9800 0 0 0 &pci_intc 2>;
> > +
> >                                 status = "disabled";
> >                         };
> >                 };
> > diff --git a/target/linux/ath79/patches-4.14/0036-MIPS-ath79-remove-irq-code-from-pci.patch b/target/linux/ath79/patches-4.14/0036-MIPS-ath79-remove-irq-code-from-pci.patch
> > new file mode 100644
> > index 0000000..b3fc19a
> > --- /dev/null
> > +++ b/target/linux/ath79/patches-4.14/0036-MIPS-ath79-remove-irq-code-from-pci.patch
> > @@ -0,0 +1,117 @@
> > +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
> > +@@ -269,103 +269,6 @@ static struct pci_ops ar71xx_pci_ops = {
> > +       .write  = ar71xx_pci_write_config,
> > + };
> > +
> > +-static void ar71xx_pci_irq_handler(struct irq_desc *desc)
> > +-{
> > +-      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;
> > +-
> > +-      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(apc->domain, 1));
> > +-
> > +-      else if (pending & AR71XX_PCI_INT_DEV1)
> > +-              generic_handle_irq(irq_linear_revmap(apc->domain, 2));
> > +-
> > +-      else if (pending & AR71XX_PCI_INT_DEV2)
> > +-              generic_handle_irq(irq_linear_revmap(apc->domain, 3));
> > +-
> > +-      else if (pending & AR71XX_PCI_INT_CORE)
> > +-              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)
> > +-{
> > +-      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 = 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);
> > +-
> > +-      /* 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 = 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);
> > +-
> > +-      /* 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 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;
> > +-
> > +-      __raw_writel(0, base + AR71XX_RESET_REG_PCI_INT_ENABLE);
> > +-      __raw_writel(0, base + AR71XX_RESET_REG_PCI_INT_STATUS);
> > +-
> > +-      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);
> > +-}
> > +-
> > + static void ar71xx_pci_reset(void)
> > + {
> > +       ath79_device_reset_set(AR71XX_RESET_PCI_BUS | AR71XX_RESET_PCI_CORE);
> > +@@ -419,8 +322,6 @@ static int ar71xx_pci_probe(struct platf
> > +       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);
> > +
> > +       __ath79_pci_swizzle_b = ar71xx_pci_swizzle_b;
> > --
> > 2.7.4
> >
> >
> > _______________________________________________
> > openwrt-devel mailing list
> > openwrt-devel at lists.openwrt.org
> > https://lists.openwrt.org/mailman/listinfo/openwrt-devel

_______________________________________________
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