Thread (14 messages) 14 messages, 5 authors, 2013-03-04
STALE4888d
Revisions (6)
  1. v1 [diff vs current]
  2. v1 [diff vs current]
  3. v1 [diff vs current]
  4. v1 [diff vs current]
  5. v1 [diff vs current]
  6. v1 current

[PATCH 1/2] gpio/omap: convert gpio irq domain to linear mapping

From: Felipe Balbi <hidden>
Date: 2013-03-04 17:11:59
Also in: linux-omap

On Mon, Mar 04, 2013 at 11:06:12AM -0600, Jon Hunter wrote:
On 03/02/2013 05:48 AM, Felipe Balbi wrote:
quoted
On Fri, Mar 01, 2013 at 11:22:47AM -0600, Jon Hunter wrote:
quoted
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 <redacted>
Signed-off-by: Jon Hunter <redacted>
Reviewed-by: Felipe Balbi <redacted>

Just one suggestion below for a later patch.
quoted
@@ -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.
quoted
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130304/4ca672d8/attachment.sig>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help