On Mon, 2012-04-23 at 16:30 -0600, Grant Likely wrote:
> The mpc8xx driver uses a reference to NR_IRQS that is buggy.  It uses
> NR_IRQs for the array size of the ppc_cached_irq_mask bitmap, but
> NR_IRQs could be smaller than the number of hardware irqs that
> ppc_cached_irq_mask tracks.

Joakim, any chance you can test this ASAP ? :-)

Thanks !

Cheers,
Ben.

> Also, while fixing that problem, it became apparent that the interrupt
> controller only supports 32 interrupt numbers, but it is written as if
> it supports multiple register banks which is more complicated.
> 
> This patch pulls out the buggy reference to NR_IRQs and fixes the size
> of the ppc_cached_irq_mask to match the number of HW irqs.  It also
> drops the now-unnecessary code since ppc_cached_irq_mask is no longer
> an array.
> 
> v2: - Rename ppc_cached_irq_mask to mpc8xx_cached_irq_mask
>     - Drop duplicate forward declaration
> 
> Signed-off-by: Grant Likely <grant.lik...@secretlab.ca>
> Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> ---
>  arch/powerpc/sysdev/mpc8xx_pic.c |   61 
> +++++++++++++-------------------------
>  1 file changed, 20 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/mpc8xx_pic.c 
> b/arch/powerpc/sysdev/mpc8xx_pic.c
> index d5f5416..b724622 100644
> --- a/arch/powerpc/sysdev/mpc8xx_pic.c
> +++ b/arch/powerpc/sysdev/mpc8xx_pic.c
> @@ -18,69 +18,45 @@
>  extern int cpm_get_irq(struct pt_regs *regs);
>  
>  static struct irq_domain *mpc8xx_pic_host;
> -#define NR_MASK_WORDS   ((NR_IRQS + 31) / 32)
> -static unsigned long ppc_cached_irq_mask[NR_MASK_WORDS];
> +static unsigned long mpc8xx_cached_irq_mask;
>  static sysconf8xx_t __iomem *siu_reg;
>  
> -int cpm_get_irq(struct pt_regs *regs);
> +static inline unsigned long mpc8xx_irqd_to_bit(struct irq_data *d)
> +{
> +     return 0x80000000 >> irqd_to_hwirq(d);
> +}
>  
>  static void mpc8xx_unmask_irq(struct irq_data *d)
>  {
> -     int     bit, word;
> -     unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d);
> -
> -     bit = irq_nr & 0x1f;
> -     word = irq_nr >> 5;
> -
> -     ppc_cached_irq_mask[word] |= (1 << (31-bit));
> -     out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]);
> +     mpc8xx_cached_irq_mask |= mpc8xx_irqd_to_bit(d);
> +     out_be32(&siu_reg->sc_simask, mpc8xx_cached_irq_mask);
>  }
>  
>  static void mpc8xx_mask_irq(struct irq_data *d)
>  {
> -     int     bit, word;
> -     unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d);
> -
> -     bit = irq_nr & 0x1f;
> -     word = irq_nr >> 5;
> -
> -     ppc_cached_irq_mask[word] &= ~(1 << (31-bit));
> -     out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]);
> +     mpc8xx_cached_irq_mask &= ~mpc8xx_irqd_to_bit(d);
> +     out_be32(&siu_reg->sc_simask, mpc8xx_cached_irq_mask);
>  }
>  
>  static void mpc8xx_ack(struct irq_data *d)
>  {
> -     int     bit;
> -     unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d);
> -
> -     bit = irq_nr & 0x1f;
> -     out_be32(&siu_reg->sc_sipend, 1 << (31-bit));
> +     out_be32(&siu_reg->sc_sipend, mpc8xx_irqd_to_bit(d));
>  }
>  
>  static void mpc8xx_end_irq(struct irq_data *d)
>  {
> -     int bit, word;
> -     unsigned int irq_nr = (unsigned int)irqd_to_hwirq(d);
> -
> -     bit = irq_nr & 0x1f;
> -     word = irq_nr >> 5;
> -
> -     ppc_cached_irq_mask[word] |= (1 << (31-bit));
> -     out_be32(&siu_reg->sc_simask, ppc_cached_irq_mask[word]);
> +     mpc8xx_cached_irq_mask |= mpc8xx_irqd_to_bit(d);
> +     out_be32(&siu_reg->sc_simask, mpc8xx_cached_irq_mask);
>  }
>  
>  static int mpc8xx_set_irq_type(struct irq_data *d, unsigned int flow_type)
>  {
> -     if (flow_type & IRQ_TYPE_EDGE_FALLING) {
> -             irq_hw_number_t hw = (unsigned int)irqd_to_hwirq(d);
> +     /* only external IRQ senses are programmable */
> +     if ((flow_type & IRQ_TYPE_EDGE_FALLING) && !(irqd_to_hwirq(d) & 1)) {
>               unsigned int siel = in_be32(&siu_reg->sc_siel);
> -
> -             /* only external IRQ senses are programmable */
> -             if ((hw & 1) == 0) {
> -                     siel |= (0x80000000 >> hw);
> -                     out_be32(&siu_reg->sc_siel, siel);
> -                     __irq_set_handler_locked(d->irq, handle_edge_irq);
> -             }
> +             siel |= mpc8xx_irqd_to_bit(d);
> +             out_be32(&siu_reg->sc_siel, siel);
> +             __irq_set_handler_locked(d->irq, handle_edge_irq);
>       }
>       return 0;
>  }
> @@ -132,6 +108,9 @@ static int mpc8xx_pic_host_xlate(struct irq_domain *h, 
> struct device_node *ct,
>               IRQ_TYPE_EDGE_FALLING,
>       };
>  
> +     if (intspec[0] > 0x1f)
> +             return 0;
> +
>       *out_hwirq = intspec[0];
>       if (intsize > 1 && intspec[1] < 4)
>               *out_flags = map_pic_senses[intspec[1]];


_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to