On 03/04/2013 11:11 AM, Felipe Balbi wrote: > 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.
Ok, yes I see that now for ARMv5 onwards. Grant has pushed the patch, but may be we can update this as an optimisation separately. Thanks Jon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
