On Thursday 02 October 2008, Pakaravoor, Jagadeesh wrote: > > /* > > - * twl4030_irq_thread() runs as a kernel thread. It queries the twl4030 > > - * interrupt controller to see which modules are generating interrupt requests > > - * and then calls the desc->handle method for each module requesting service. > > + * This thread processes interrupts reported by the Primary Interrupt Handler. > > */ > > Why do we need to remove that piece of comment? > > Can we not keep the comment this way? We "can", but "should" we? The general policy on comments is that they should not be translating the C code into English. Not only are such comments pointless (the C code says the same thing, more precisely) but they also are more likely to become obsolete as the code changes. (That's a well known problem: comments usually don't get updated when the code changes.) So in my opinion we "should" not have this comment just paraphrase (in English) what the code does, one sentence per code block. > /* > - * twl4030_irq_thread() runs as a kernel thread. It queries the twl4030 > - * interrupt controller to see which modules are generating interrupt requests > + * This thread processes interrupts reported by the Primary Interrupt Handler, > * and then calls the desc->handle method for each module requesting service. Plus, it's desc->handle_irq() not desc->handle(). ;) I expect all the details of the IRQ handling to get overhauled in a while, anyway. Mainline will be getting some threaded IRQ infrastructure, and I'd expect it to help out here. (And then there's the way every SIH handler seems to be getting its own irq_chip and handler thread. I'm sure that can be done much better.) - Dave > > */ > > -- > Jagadeesh > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
