On Mon, Mar 04, 2013 at 11:06:12AM -0600, Jon Hunter wrote: > > On 03/02/2013 05:48 AM, Felipe Balbi wrote: > > On Fri, Mar 01, 2013 at 11:22:47AM -0600, Jon Hunter wrote: > >> Currently the OMAP GPIO driver uses a legacy mapping for the GPIO IRQ > >> domain. This is not necessary because we do not need to assign a > >> specific interrupt number to the GPIO IRQ domain. Therefore, convert > >> the OMAP GPIO driver to use a linear mapping instead. > >> > >> Please note that this also allows to simplify the logic in the OMAP > >> gpio_irq_handler() routine, by using irq_find_mapping() to obtain the > >> virtual irq number from the GPIO bank and bank index. > >> > >> Reported-by: Linus Walleij <[email protected]> > >> Signed-off-by: Jon Hunter <[email protected]> > > > > Reviewed-by: Felipe Balbi <[email protected]> > > > > Just one suggestion below for a later patch. > > > >> @@ -680,7 +686,7 @@ static void gpio_irq_handler(unsigned int irq, struct > >> irq_desc *desc) > >> { > >> void __iomem *isr_reg = NULL; > >> u32 isr; > >> - unsigned int gpio_irq, gpio_index; > >> + unsigned int i; > >> struct gpio_bank *bank; > >> int unmasked = 0; > >> struct irq_chip *chip = irq_desc_get_chip(desc); > >> @@ -721,15 +727,10 @@ static void gpio_irq_handler(unsigned int irq, > >> struct irq_desc *desc) > >> if (!isr) > >> break; > >> > >> - gpio_irq = bank->irq_base; > >> - for (; isr != 0; isr >>= 1, gpio_irq++) { > >> - int gpio = irq_to_gpio(bank, gpio_irq); > >> - > >> + for (i = 0; isr != 0; isr >>= 1, i++) { > >> if (!(isr & 1)) > >> continue; > > > > this will iterate over all 32 GPIOs, a better way to handle this would > > be to have something like: > > Worse case, if only bit 31 was set then I agree this is not that > efficient. Or even if one bit is set. However, the loop itself will > iterate while isr != 0 so not always over each bit. No different to the > existing code. > > > while (isr) { > > unsigned long bit = __ffs(isr); > > > > /* clear this bit */ > > isr &= ~bit; > > > > generic_handle_irq(irq_find_mapping(bank->domain, bit); > > } > > > > this way you will only iterate the amount of bits enabled in the isr > > register. > > Definitely cleaner but I am wondering which approach would be more > efficient from an instruction standpoint. This could definitely be much > more efficient if there is only a couple bits set.
__ffs() is done with CLZ instruction, so it's pretty fast. -- balbi
signature.asc
Description: Digital signature
