All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] gpiolib: irqchip: use irq_find_mapping while removing irqchip
@ 2014-09-25 16:09 Grygorii Strashko
  2014-09-25 16:09 ` [RFC/RFT PATCH 2/2] gpiolib: irqchip: get rid of irq_base Grygorii Strashko
  2014-09-26  8:40 ` [PATCH 1/2] gpiolib: irqchip: use irq_find_mapping while removing irqchip Linus Walleij
  0 siblings, 2 replies; 6+ messages in thread
From: Grygorii Strashko @ 2014-09-25 16:09 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandre Courbot, Yalin.Wang, linux-gpio, Grygorii Strashko

There is no guarantee that VIRQs will be allocated sequentially
for gpio irqchip in gpiochip_irqchip_add().
Therefore, it's unsafe to dispose VIRQ in gpiochip_irqchip_remove()
basing on index relatively to stored irq_base value.

Hence, use irq_find_mapping for VIRQ finding  in gpiochip_irqchip_remove()
instead of irq_base + index.

Reported-by: Wang, Yalin <Yalin.Wang@sonymobile.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/gpio/gpiolib.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 15cc0bb..f6bf368 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -524,7 +524,8 @@ static void gpiochip_irqchip_remove(struct gpio_chip *gpiochip)
 	/* Remove all IRQ mappings and delete the domain */
 	if (gpiochip->irqdomain) {
 		for (offset = 0; offset < gpiochip->ngpio; offset++)
-			irq_dispose_mapping(gpiochip->irq_base + offset);
+			irq_dispose_mapping(
+				irq_find_mapping(gpiochip->irqdomain, offset));
 		irq_domain_remove(gpiochip->irqdomain);
 	}
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [RFC/RFT PATCH 2/2] gpiolib: irqchip: get rid of irq_base
  2014-09-25 16:09 [PATCH 1/2] gpiolib: irqchip: use irq_find_mapping while removing irqchip Grygorii Strashko
@ 2014-09-25 16:09 ` Grygorii Strashko
  2014-09-26  8:44   ` Linus Walleij
  2014-09-26  8:40 ` [PATCH 1/2] gpiolib: irqchip: use irq_find_mapping while removing irqchip Linus Walleij
  1 sibling, 1 reply; 6+ messages in thread
From: Grygorii Strashko @ 2014-09-25 16:09 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandre Courbot, Yalin.Wang, linux-gpio, Grygorii Strashko

Remove irq_base from struct gpio_chip, as it is seems to
be unused.
Aslo, using this field by drivers is unsafe because it's
uncompatible with Sparse IRQ feature.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
Hi Linus,

I've not found users of this field in drivers/gpio/ folder,
so I've decided to created this patch to get more comments.

 drivers/gpio/gpiolib.c      | 9 +--------
 include/linux/gpio/driver.h | 1 -
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f6bf368..8aa84d5 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -605,15 +605,8 @@ int gpiochip_irqchip_add(struct gpio_chip *gpiochip,
 	 * any gpiochip calls. If the first_irq was zero, this is
 	 * necessary to allocate descriptors for all IRQs.
 	 */
-	for (offset = 0; offset < gpiochip->ngpio; offset++) {
+	for (offset = 0; offset < gpiochip->ngpio; offset++)
 		irq_base = irq_create_mapping(gpiochip->irqdomain, offset);
-		if (offset == 0)
-			/*
-			 * Store the base into the gpiochip to be used when
-			 * unmapping the irqs.
-			 */
-			gpiochip->irq_base = irq_base;
-	}
 
 	acpi_gpiochip_request_interrupts(gpiochip);
 
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index e78a237..976276a 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -110,7 +110,6 @@ struct gpio_chip {
 	 */
 	struct irq_chip		*irqchip;
 	struct irq_domain	*irqdomain;
-	unsigned int		irq_base;
 	irq_flow_handler_t	irq_handler;
 	unsigned int		irq_default_type;
 #endif
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] gpiolib: irqchip: use irq_find_mapping while removing irqchip
  2014-09-25 16:09 [PATCH 1/2] gpiolib: irqchip: use irq_find_mapping while removing irqchip Grygorii Strashko
  2014-09-25 16:09 ` [RFC/RFT PATCH 2/2] gpiolib: irqchip: get rid of irq_base Grygorii Strashko
@ 2014-09-26  8:40 ` Linus Walleij
  1 sibling, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2014-09-26  8:40 UTC (permalink / raw)
  To: Grygorii Strashko; +Cc: Alexandre Courbot, Wang, Yalin, linux-gpio

On Thu, Sep 25, 2014 at 6:09 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> There is no guarantee that VIRQs will be allocated sequentially
> for gpio irqchip in gpiochip_irqchip_add().
> Therefore, it's unsafe to dispose VIRQ in gpiochip_irqchip_remove()
> basing on index relatively to stored irq_base value.
>
> Hence, use irq_find_mapping for VIRQ finding  in gpiochip_irqchip_remove()
> instead of irq_base + index.
>
> Reported-by: Wang, Yalin <Yalin.Wang@sonymobile.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Patch applied.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC/RFT PATCH 2/2] gpiolib: irqchip: get rid of irq_base
  2014-09-25 16:09 ` [RFC/RFT PATCH 2/2] gpiolib: irqchip: get rid of irq_base Grygorii Strashko
@ 2014-09-26  8:44   ` Linus Walleij
  2014-09-26 10:44     ` Grygorii Strashko
  0 siblings, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2014-09-26  8:44 UTC (permalink / raw)
  To: Grygorii Strashko; +Cc: Alexandre Courbot, Wang, Yalin, linux-gpio

On Thu, Sep 25, 2014 at 6:09 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> Remove irq_base from struct gpio_chip, as it is seems to
> be unused.
> Aslo, using this field by drivers is unsafe because it's
> uncompatible with Sparse IRQ feature.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> Hi Linus,
>
> I've not found users of this field in drivers/gpio/ folder,
> so I've decided to created this patch to get more comments.

Wut?

git grep '>irq_base'
(...)
drivers/gpio/gpio-max732x.c:    return chip->irq_base + off;
drivers/gpio/gpio-ml-ioh.c:     return chip->irq_base + offset;
drivers/gpio/gpio-pch.c:        chip->irq_base = irq_base;
etc etc

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC/RFT PATCH 2/2] gpiolib: irqchip: get rid of irq_base
  2014-09-26  8:44   ` Linus Walleij
@ 2014-09-26 10:44     ` Grygorii Strashko
  2014-09-26 12:33       ` Linus Walleij
  0 siblings, 1 reply; 6+ messages in thread
From: Grygorii Strashko @ 2014-09-26 10:44 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Alexandre Courbot, Wang, Yalin, linux-gpio

On 09/26/2014 11:44 AM, Linus Walleij wrote:
> On Thu, Sep 25, 2014 at 6:09 PM, Grygorii Strashko
> <grygorii.strashko@ti.com> wrote:
>
>> Remove irq_base from struct gpio_chip, as it is seems to
>> be unused.
>> Aslo, using this field by drivers is unsafe because it's
>> uncompatible with Sparse IRQ feature.
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>> Hi Linus,
>>
>> I've not found users of this field in drivers/gpio/ folder,
>> so I've decided to created this patch to get more comments.
>
> Wut?
>
> git grep '>irq_base'
> (...)
> drivers/gpio/gpio-max732x.c:    return chip->irq_base + off;
^struct max732x_chip *chip;

> drivers/gpio/gpio-ml-ioh.c:     return chip->irq_base + offset;
^struct ioh_gpio *chip

> drivers/gpio/gpio-pch.c:        chip->irq_base = irq_base;
^struct pch_gpio *chip

> etc etc

etc ;)

I've spent some time checking it, but it's possible
that I missed smth or it's used outside gpio directory.

Best regards,
-grygorii

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC/RFT PATCH 2/2] gpiolib: irqchip: get rid of irq_base
  2014-09-26 10:44     ` Grygorii Strashko
@ 2014-09-26 12:33       ` Linus Walleij
  0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2014-09-26 12:33 UTC (permalink / raw)
  To: Grygorii Strashko; +Cc: Alexandre Courbot, Wang, Yalin, linux-gpio

On Fri, Sep 26, 2014 at 12:44 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
> On 09/26/2014 11:44 AM, Linus Walleij wrote:
>>
>> On Thu, Sep 25, 2014 at 6:09 PM, Grygorii Strashko
>> <grygorii.strashko@ti.com> wrote:
>>
>>> Remove irq_base from struct gpio_chip, as it is seems to
>>> be unused.
>>> Aslo, using this field by drivers is unsafe because it's
>>> uncompatible with Sparse IRQ feature.
>>>
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> ---
>>> Hi Linus,
>>>
>>> I've not found users of this field in drivers/gpio/ folder,
>>> so I've decided to created this patch to get more comments.
>>
>>
>> Wut?
>>
>> git grep '>irq_base'
>> (...)
>> drivers/gpio/gpio-max732x.c:    return chip->irq_base + off;
>
> ^struct max732x_chip *chip;
>
>> drivers/gpio/gpio-ml-ioh.c:     return chip->irq_base + offset;
>
> ^struct ioh_gpio *chip
>
>> drivers/gpio/gpio-pch.c:        chip->irq_base = irq_base;
>
> ^struct pch_gpio *chip
>
>> etc etc
>
> etc ;)

Aha now the variable names are confusing me at no end too :-)

> I've spent some time checking it, but it's possible
> that I missed smth or it's used outside gpio directory.

It'd be good if we could estimate this, I'd prefer if we could do
some semantic grep like cocinelle does to see if the (foo)->irq_base
affects a case where (foo) is struct gpio_chip...

But I guess I can also just apply the patch and throw it at
the autobuilders. I'm just worried about cases the autobuilder
would miss.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-09-26 12:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-25 16:09 [PATCH 1/2] gpiolib: irqchip: use irq_find_mapping while removing irqchip Grygorii Strashko
2014-09-25 16:09 ` [RFC/RFT PATCH 2/2] gpiolib: irqchip: get rid of irq_base Grygorii Strashko
2014-09-26  8:44   ` Linus Walleij
2014-09-26 10:44     ` Grygorii Strashko
2014-09-26 12:33       ` Linus Walleij
2014-09-26  8:40 ` [PATCH 1/2] gpiolib: irqchip: use irq_find_mapping while removing irqchip Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.