Re: [PATCH (v4) 2/11] MIPS: bmips: Add bcm6345-l2-timer interrupt controller
flat view
From: Thomas Gleixner <hidden>
Date: 2015-11-27 08:38:56
Also in:
linux-mips, linux-watchdog, lkml
Simon, On Thu, 26 Nov 2015, Simon Arlott wrote:
quoted hunk ↗ jump to hunk
+static inline u32 bcm6345_timer_read_int_status(struct bcm6345_timer *timer) +{ + if (timer->interrupt_bits == 32) + return __raw_readl(timer->base + timer->regs[TIMER_INT_STATUS]); + else + return __raw_readb(timer->base + timer->regs[TIMER_INT_STATUS]); +}
Instead of having that pile of conditionals you could just define two functions and have a function pointer in struct bcm6345_timer which you initialize at init time.
quoted hunk ↗ jump to hunk
+static inline void bcm6345_timer_write_control(struct bcm6345_timer *timer, + unsigned int id, u32 val) +{ + if (id >= timer->nr_timers) { + WARN(1, "%s: %d >= %d", __func__, id, timer->nr_timers);
This is more than silly. You call that form the init function via: for (i = 0; i < timer->nr_timers; i++) Hmm?
quoted hunk ↗ jump to hunk
+static void bcm6345_timer_unmask(struct irq_data *d) +{ + struct bcm6345_timer *timer = irq_data_get_irq_chip_data(d); + unsigned long flags; + u8 val; + + if (d->hwirq < timer->nr_timers) {
Again. You can have two different interrupt chips without that completely undocumented and non obvious conditional. BTW, how are those simple interrupts masked at all?
quoted hunk ↗ jump to hunk
+ raw_spin_lock_irqsave(&timer->lock, flags); + val = bcm6345_timer_read_int_enable(timer); + val |= BIT(d->hwirq); + bcm6345_timer_write_int_enable(timer, val); + raw_spin_unlock_irqrestore(&timer->lock, flags); + } +}
quoted hunk ↗ jump to hunk
+ raw_spin_lock_init(&timer->lock); + timer->regs = regs; + timer->interrupt_bits = interrupt_bits; + timer->nr_timers = nr_timers; + timer->nr_interrupts = nr_timers + 1;
What is that extra interrupt about? For the casual reader this looks like a bug ... Comments exist for a reason. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html