linux-mips
[Top] [All Lists]

Re: [PATCH v2 10/11] MIPS: BCM63XX: add external irq support for non 634

To: Maxime Bizon <mbizon@freebox.fr>
Subject: Re: [PATCH v2 10/11] MIPS: BCM63XX: add external irq support for non 6348 CPUs.
From: Jonas Gorski <jonas.gorski+openwrt@gmail.com>
Date: Wed, 9 Nov 2011 13:35:15 +0100
Cc: ralf@linux-mips.org, linux-mips@linux-mips.org
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:from:date :x-google-sender-auth:message-id:subject:to:cc:content-type :content-transfer-encoding; bh=icLXgRVQRO0EwX71lOuhNWANM/IQ+0cR4Uck+hfeZ4c=; b=Kh0OPDMjFE/y8WQbWtjDkm6lEpFsS8HIa/ATuVZY7J299K5Hm1M5VXzTJyZi/ApERa /3y6yLjCPF+WhS3wowShtkLtflIr9rG6/EDu3ygB/mj2U2/KrFbagSW9br2/YrIG+3s5 ct0FH5+Q5/D+ZWsa/pWQSqDie50+CcFC4EnTc=
In-reply-to: <1320430175-13725-11-git-send-email-mbizon@freebox.fr>
References: <1320430175-13725-1-git-send-email-mbizon@freebox.fr> <1320430175-13725-11-git-send-email-mbizon@freebox.fr>
Sender: linux-mips-bounce@linux-mips.org
Hi Maxime,

On 4 November 2011 19:09, Maxime Bizon <mbizon@freebox.fr> wrote:
> Signed-off-by: Maxime Bizon <mbizon@freebox.fr>
> ---
>  arch/mips/bcm63xx/irq.c                           |  131 
> ++++++++++++++++-----
>  arch/mips/bcm63xx/setup.c                         |   30 ++++-
>  arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h |   31 ++++--
>  3 files changed, 149 insertions(+), 43 deletions(-)
>
> diff --git a/arch/mips/bcm63xx/irq.c b/arch/mips/bcm63xx/irq.c
> index fde8bb4..d159ba7 100644
> --- a/arch/mips/bcm63xx/irq.c
> +++ b/arch/mips/bcm63xx/irq.c
> @@ -34,6 +34,9 @@ static void __internal_irq_unmask_64(unsigned int irq) 
> __maybe_unused;
>  #define is_ext_irq_cascaded    0
>  #define ext_irq_start          0
>  #define ext_irq_end            0
> +#define ext_irq_count          4
> +#define ext_irq_cfg_reg1       PERF_EXTIRQ_CFG_REG_6338
> +#define ext_irq_cfg_reg2       0
>  #endif
>  #ifdef CONFIG_BCM63XX_CPU_6345
>  #define irq_stat_reg           PERF_IRQSTAT_6345_REG
> @@ -42,6 +45,9 @@ static void __internal_irq_unmask_64(unsigned int irq) 
> __maybe_unused;
>  #define is_ext_irq_cascaded    0
>  #define ext_irq_start          0
>  #define ext_irq_end            0
> +#define ext_irq_count          0
> +#define ext_irq_cfg_reg1       0

According to my sources BCM6345 has an EXTIRQ_CFG_REG, too (same place
as the others; 0x14).

> +#define ext_irq_cfg_reg2       0
>  #endif
>  #ifdef CONFIG_BCM63XX_CPU_6348
>  #define irq_stat_reg           PERF_IRQSTAT_6348_REG
> @@ -50,6 +56,9 @@ static void __internal_irq_unmask_64(unsigned int irq) 
> __maybe_unused;
>  #define is_ext_irq_cascaded    0
>  #define ext_irq_start          0
>  #define ext_irq_end            0
> +#define ext_irq_count          4
> +#define ext_irq_cfg_reg1       PERF_EXTIRQ_CFG_REG_6348
> +#define ext_irq_cfg_reg2       0
>  #endif
>  #ifdef CONFIG_BCM63XX_CPU_6358
>  #define irq_stat_reg           PERF_IRQSTAT_6358_REG
> @@ -58,6 +67,9 @@ static void __internal_irq_unmask_64(unsigned int irq) 
> __maybe_unused;
>  #define is_ext_irq_cascaded    1
>  #define ext_irq_start          (BCM_6358_EXT_IRQ0 - IRQ_INTERNAL_BASE)
>  #define ext_irq_end            (BCM_6358_EXT_IRQ3 - IRQ_INTERNAL_BASE)
> +#define ext_irq_count          4
> +#define ext_irq_cfg_reg1       PERF_EXTIRQ_CFG_REG_6358
> +#define ext_irq_cfg_reg2       0
>  #endif
>
>  #if irq_bits == 32
> @@ -81,7 +93,9 @@ static inline void bcm63xx_init_irq(void)
>  static u32 irq_stat_addr, irq_mask_addr;
>  static void (*dispatch_internal)(void);
>  static int is_ext_irq_cascaded;
> +static unsigned int ext_irq_count;
>  static unsigned int ext_irq_start, ext_irq_end;
> +static unsigned int ext_irq_cfg_reg1, ext_irq_cfg_reg2;
>  static void (*internal_irq_mask)(unsigned int irq);
>  static void (*internal_irq_unmask)(unsigned int irq);
>
> @@ -107,14 +121,18 @@ static void bcm63xx_init_irq(void)
>                irq_stat_addr += PERF_IRQSTAT_6348_REG;
>                irq_mask_addr += PERF_IRQMASK_6348_REG;
>                irq_bits = 32;
> +               ext_irq_count = 4;
> +               ext_irq_cfg_reg1 = PERF_EXTIRQ_CFG_REG_6348;
>                break;
>        case BCM6358_CPU_ID:
>                irq_stat_addr += PERF_IRQSTAT_6358_REG;
>                irq_mask_addr += PERF_IRQMASK_6358_REG;
>                irq_bits = 32;
> +               ext_irq_count = 4;
>                is_ext_irq_cascaded = 1;
>                ext_irq_start = BCM_6358_EXT_IRQ0 - IRQ_INTERNAL_BASE;
>                ext_irq_end = BCM_6358_EXT_IRQ3 - IRQ_INTERNAL_BASE;
> +               ext_irq_cfg_reg1 = PERF_EXTIRQ_CFG_REG_6358;
>                break;

You missed BCM6338 (and BCM6345) here.

>        default:
>                BUG();

(snip)

> @@ -297,58 +335,93 @@ static void bcm63xx_external_irq_unmask(struct irq_data 
> *d)
>  static void bcm63xx_external_irq_clear(struct irq_data *d)
>  {
>        unsigned int irq = d->irq - IRQ_EXTERNAL_BASE;
> -       u32 reg;
> +       u32 reg, regaddr;
> +
> +       regaddr = get_ext_irq_perf_reg(irq);
> +       reg = bcm_perf_readl(regaddr);
>
> -       reg = bcm_perf_readl(PERF_EXTIRQ_CFG_REG);
> -       reg |= EXTIRQ_CFG_CLEAR(irq);
> -       bcm_perf_writel(reg, PERF_EXTIRQ_CFG_REG);
> +       if (BCMCPU_IS_6348())
> +               reg |= EXTIRQ_CFG_CLEAR_6348(irq % 4);
> +       else
> +               reg |= EXTIRQ_CFG_CLEAR(irq % 4);
> +
> +       bcm_perf_writel(reg, regaddr);
>  }
>
>  static int bcm63xx_external_irq_set_type(struct irq_data *d,
>                                         unsigned int flow_type)
>  {
>        unsigned int irq = d->irq - IRQ_EXTERNAL_BASE;
> -       u32 reg;
> +       u32 reg, regaddr;
> +       int levelsense, sense, bothedge;
>
>        flow_type &= IRQ_TYPE_SENSE_MASK;
>
>        if (flow_type == IRQ_TYPE_NONE)
>                flow_type = IRQ_TYPE_LEVEL_LOW;
>
> -       reg = bcm_perf_readl(PERF_EXTIRQ_CFG_REG);
> +       levelsense = sense = bothedge = 0;
>        switch (flow_type) {
>        case IRQ_TYPE_EDGE_BOTH:
> -               reg &= ~EXTIRQ_CFG_LEVELSENSE(irq);
> -               reg |= EXTIRQ_CFG_BOTHEDGE(irq);
> +               bothedge = 1;
>                break;
>
>        case IRQ_TYPE_EDGE_RISING:
> -               reg &= ~EXTIRQ_CFG_LEVELSENSE(irq);
> -               reg |= EXTIRQ_CFG_SENSE(irq);
> -               reg &= ~EXTIRQ_CFG_BOTHEDGE(irq);
> +               sense = 1;
>                break;
>
>        case IRQ_TYPE_EDGE_FALLING:
> -               reg &= ~EXTIRQ_CFG_LEVELSENSE(irq);
> -               reg &= ~EXTIRQ_CFG_SENSE(irq);
> -               reg &= ~EXTIRQ_CFG_BOTHEDGE(irq);
>                break;
>
>        case IRQ_TYPE_LEVEL_HIGH:
> -               reg |= EXTIRQ_CFG_LEVELSENSE(irq);
> -               reg |= EXTIRQ_CFG_SENSE(irq);
> +               levelsense = 1;
> +               sense = 1;
>                break;
>
>        case IRQ_TYPE_LEVEL_LOW:
> -               reg |= EXTIRQ_CFG_LEVELSENSE(irq);
> -               reg &= ~EXTIRQ_CFG_SENSE(irq);
> +               levelsense = 1;
>                break;
>
>        default:
>                printk(KERN_ERR "bogus flow type combination given !\n");
>                return -EINVAL;
>        }
> -       bcm_perf_writel(reg, PERF_EXTIRQ_CFG_REG);
> +
> +       regaddr = get_ext_irq_perf_reg(irq);
> +       reg = bcm_perf_readl(regaddr);
> +       irq %= 4;
> +
> +       if (BCMCPU_IS_6348()) {
> +               if (levelsense)
> +                       reg |= EXTIRQ_CFG_LEVELSENSE_6348(irq);
> +               else
> +                       reg &= ~EXTIRQ_CFG_LEVELSENSE_6348(irq);
> +               if (sense)
> +                       reg |= EXTIRQ_CFG_SENSE_6348(irq);
> +               else
> +                       reg &= ~EXTIRQ_CFG_SENSE_6348(irq);
> +               if (bothedge)
> +                       reg |= EXTIRQ_CFG_BOTHEDGE_6348(irq);
> +               else
> +                       reg &= ~EXTIRQ_CFG_BOTHEDGE_6348(irq);
> +       }
> +
> +       if (BCMCPU_IS_6338() || BCMCPU_IS_6358()) {

This is also valid for BCM6345. Also you can make this an else if ()
of the if() above.

> +               if (levelsense)
> +                       reg |= EXTIRQ_CFG_LEVELSENSE(irq);
> +               else
> +                       reg &= ~EXTIRQ_CFG_LEVELSENSE(irq);
> +               if (sense)
> +                       reg |= EXTIRQ_CFG_SENSE(irq);
> +               else
> +                       reg &= ~EXTIRQ_CFG_SENSE(irq);
> +               if (bothedge)
> +                       reg |= EXTIRQ_CFG_BOTHEDGE(irq);
> +               else
> +                       reg &= ~EXTIRQ_CFG_BOTHEDGE(irq);
> +       }
> +
> +       bcm_perf_writel(reg, regaddr);
>
>        irqd_set_trigger_type(d, flow_type);
>        if (flow_type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH))

(snip)

> diff --git a/arch/mips/bcm63xx/setup.c b/arch/mips/bcm63xx/setup.c
> index 04a3499..d209f85 100644
> --- a/arch/mips/bcm63xx/setup.c
> +++ b/arch/mips/bcm63xx/setup.c
> @@ -63,13 +63,33 @@ static void bcm6348_a1_reboot(void)
>
>  void bcm63xx_machine_reboot(void)
>  {
> -       u32 reg;
> +       u32 reg, perf_regs[2] = { 0, 0 };
> +       unsigned int i;
>
>        /* mask and clear all external irq */
> -       reg = bcm_perf_readl(PERF_EXTIRQ_CFG_REG);
> -       reg &= ~EXTIRQ_CFG_MASK_ALL;
> -       reg |= EXTIRQ_CFG_CLEAR_ALL;
> -       bcm_perf_writel(reg, PERF_EXTIRQ_CFG_REG);
> +       switch (bcm63xx_get_cpu_id()) {
> +       case BCM6338_CPU_ID:
> +               perf_regs[0] = PERF_EXTIRQ_CFG_REG_6338;
> +               break;
> +       case BCM6348_CPU_ID:
> +               perf_regs[0] = PERF_EXTIRQ_CFG_REG_6348;
> +               break;
> +       case BCM6358_CPU_ID:
> +               perf_regs[0] = PERF_EXTIRQ_CFG_REG_6358;
> +               break;

Missing the BCM6345 again.

> +       }
> +
> +       for (i = 0; i < 2; i++) {
> +               reg = bcm_perf_readl(perf_regs[i]);

perf_regs[1] is (currently) always 0, so you are actually reading ...

> +               if (BCMCPU_IS_6348()) {
> +                       reg &= ~EXTIRQ_CFG_MASK_ALL_6348;
> +                       reg |= EXTIRQ_CFG_CLEAR_ALL_6348;
> +               } else {
> +                       reg &= ~EXTIRQ_CFG_MASK_ALL;
> +                       reg |= EXTIRQ_CFG_CLEAR_ALL;
> +               }
> +               bcm_perf_writel(reg, perf_regs[i]);

... and writing to the REV_ID register. While it's probably harmless
as this is a read-only register, it doesn't look right.

Since BCM6368 and BCM6816 seem to be the only two actually having more
than one EXTIRQ register, I think it would make more sense to just
modify the first and check for the CPUID for the second instead of a
two element loop.

so I would do something like

               if (BCMCPU_IS_6348()) {
                  /* do the 6348 modify of the first extirq reg */
               } else {
                  /* do the normal modify of the first extirq reg */
               }

And then add in your 6368 patch (maybe to the else branch)

               if (BCMCPU_IS_6368()) {
                  /* modify the second ext_irq reg */
               }

> +       }
>
>        if (BCMCPU_IS_6348() && (bcm63xx_get_cpu_rev() == 0xa1))
>                bcm6348_a1_reboot();
> diff --git a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h 
> b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h
> index 25676cd..2b3a2d6 100644
> --- a/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h
> +++ b/arch/mips/include/asm/mach-bcm63xx/bcm63xx_regs.h
> @@ -100,16 +100,29 @@
>  #define PERF_IRQSTAT_6358_REG          0x10
>
>  /* External Interrupt Configuration register */
> -#define PERF_EXTIRQ_CFG_REG            0x14
> +#define PERF_EXTIRQ_CFG_REG_6338       0x14

Missing the 6345 here again.

> +#define PERF_EXTIRQ_CFG_REG_6348       0x14
> +#define PERF_EXTIRQ_CFG_REG_6358       0x14

(snip)

Regards
Jonas
<Prev in Thread] Current Thread [Next in Thread>