On Fri, Sep 11, 2015 at 2:22 AM, Y Vo <yvo@xxxxxxx> wrote: > Add support to configure GPIO line as input, output or external IRQ pin. > > Signed-off-by: Y Vo <yvo@xxxxxxx> Mostly OK but... > #define XGENE_MAX_GPIO_DS 22 > #define XGENE_MAX_GPIO_DS_IRQ 6 > +#define XGENE_GPIO8_HWIRQ 0x48 > +#define XGENE_GPIO8_IDX 8 (...) > +static int xgene_irq_to_line(u32 irq) > +{ > + u32 offset = irq_get_irq_data(irq)->hwirq - XGENE_GPIO8_HWIRQ; > + > + return (offset < XGENE_MAX_GPIO_DS_IRQ) ? > + (offset + XGENE_GPIO8_IDX) : -EINVAL; > +} What is this hardcoded IRQ 0x48 business? This patch needs kerneldoc for this xgene_irq_to_line() to explain what exactly is going on, or noone will understand it enough to debug or refactor the code. I hope this 0x48 is not some way of compensating for Linux internal IRQ offsets, that is what irqdomain is for. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html