Hi Linus,
On Tue, Apr 17, 2012 at 09:30:49AM +0200, Linus Walleij wrote:
> +static irqreturn_t ab8500_hierarchical_irq(int irq, void *dev)
> +{
> + struct ab8500 *ab8500 = dev;
> + int i;
> +
> + dev_vdbg(ab8500->dev, "interrupt\n");
> +
> + /* Hierarchical interrupt version */
> + for (i = 0; i < AB8500_IT_LATCHHIER_NUM; i++) {
> + int status;
> + u8 hier_val;
> +
> + status = get_register_interruptible(ab8500, AB8500_INTERRUPT,
> + AB8500_IT_LATCHHIER1_REG + i, &hier_val);
> + if (status < 0 || hier_val == 0)
> + continue;
> +
> + do {
> + int latch_bit = __ffs(hier_val);
> + u8 offset = i * 8 + latch_bit;
> + u8 latch_val;
> +
> + /* Fix inconsistent ITFromLatch25 bit mapping... */
> + if (unlikely(offset == 17))
> + offset = 24;
> +
> + status = get_register_interruptible(ab8500,
> + AB8500_INTERRUPT,
> + AB8500_IT_LATCH1_REG + offset,
> + &latch_val);
> + if (status < 0 || latch_val == 0)
> + goto discard;
> +
> + do {
> + int int_bit = __ffs(latch_val);
> + int line;
> + int j;
> +
> + for (j = 0; j < ab8500->mask_size; j++)
> + if (ab8500->irq_reg_offset[j] == offset)
> + break;
> +
> + if (j >= ab8500->mask_size) {
> + dev_err(ab8500->dev,
> + "Register offset 0x%2x not declared\n",
> + offset);
> + return IRQ_HANDLED;
> + }
> +
> + line = j * 8 + int_bit;
> +
> + handle_nested_irq(ab8500->irq_base + line);
> + latch_val &= ~(1 << int_bit);
> + } while (latch_val);
> +discard:
> + hier_val &= ~(1 << latch_bit);
> + } while (hier_val);
> + }
> + return IRQ_HANDLED;
> +}
With 4 nested loops, this one is getting difficult to read. Have you
considered splitting the loops into separate routines ?
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
On Tue, May 1, 2012 at 1:31 PM, Samuel Ortiz <[email protected]> wrote:
> On Tue, Apr 17, 2012 at 09:30:49AM +0200, Linus Walleij wrote:
>> +static irqreturn_t ab8500_hierarchical_irq(int irq, void *dev)
>> +{
>> + ? ? /* ?Hierarchical interrupt version */
>> + ? ? for (i = 0; i < AB8500_IT_LATCHHIER_NUM; i++) {
>> + ? ? ? ? ? ? do {
>> + ? ? ? ? ? ? ? ? ? ? int latch_bit = __ffs(hier_val);
>> + ? ? ? ? ? ? ? ? ? ? do {
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? int int_bit = __ffs(latch_val);
>> + ? ? ? ? ? ? ? ? ? ? } while (latch_val);
>> + ? ? ? ? ? ? } while (hier_val);
>> + ? ? }
>> + ? ? return IRQ_HANDLED;
>> +}
>
> With 4 nested loops, this one is getting difficult to read. Have you
> considered splitting the loops into separate routines ?
I get the point :-D
Michel do you think this can be done?
Yours,
Linus Walleij