[PATCH] pinctrl/nomadik: use irq_create_mapping()

[PATCH] pinctrl/nomadik: use irq_create_mapping()

[Linus Walleij]

From: Linus Walleij <linus.walleij@linaro.org>

Since in the DT case, the linear domain path will not allocate
descriptors for the IRQs, we need to use irq_create_mapping()
for mapping hwirqs to Linux IRQs, so these descriptors get
created on-the-fly in this case.

Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/pinctrl/pinctrl-nomadik.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
index 01aea1c..d1d3cb9 100644
--- a/drivers/pinctrl/pinctrl-nomadik.c
+++ b/drivers/pinctrl/pinctrl-nomadik.c
@@ -931,7 +931,7 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
 	while (status) {
 		int bit = __ffs(status);
 
-		generic_handle_irq(irq_find_mapping(nmk_chip->domain, bit));
+		generic_handle_irq(irq_create_mapping(nmk_chip->domain, bit));
 		status &= ~BIT(bit);
 	}
 
@@ -1056,7 +1056,7 @@ static int nmk_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
 	struct nmk_gpio_chip *nmk_chip =
 		container_of(chip, struct nmk_gpio_chip, chip);
 
-	return irq_find_mapping(nmk_chip->domain, offset);
+	return irq_create_mapping(nmk_chip->domain, offset);
 }
 
 #ifdef CONFIG_DEBUG_FS
-- 
1.7.11.3

[PATCH] pinctrl/nomadik: use irq_create_mapping()

[Linus Walleij]

From: Linus Walleij <linus.walleij@linaro.org>

Since in the DT case, the linear domain path will not allocate
descriptors for the IRQs, we need to use irq_create_mapping()
for mapping hwirqs to Linux IRQs, so these descriptors get
created on-the-fly in this case.

Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/pinctrl/pinctrl-nomadik.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
index 01aea1c..d1d3cb9 100644
--- a/drivers/pinctrl/pinctrl-nomadik.c
+++ b/drivers/pinctrl/pinctrl-nomadik.c
@@ -931,7 +931,7 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
 	while (status) {
 		int bit = __ffs(status);
 
-		generic_handle_irq(irq_find_mapping(nmk_chip->domain, bit));
+		generic_handle_irq(irq_create_mapping(nmk_chip->domain, bit));
 		status &= ~BIT(bit);
 	}
 
@@ -1056,7 +1056,7 @@ static int nmk_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
 	struct nmk_gpio_chip *nmk_chip =
 		container_of(chip, struct nmk_gpio_chip, chip);
 
-	return irq_find_mapping(nmk_chip->domain, offset);
+	return irq_create_mapping(nmk_chip->domain, offset);
 }
 
 #ifdef CONFIG_DEBUG_FS
-- 
1.7.11.3

Re: [PATCH] pinctrl/nomadik: use irq_create_mapping()

[Stephen Warren]

On 10/19/2012 09:09 AM, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> Since in the DT case, the linear domain path will not allocate
> descriptors for the IRQs, we need to use irq_create_mapping()
> for mapping hwirqs to Linux IRQs, so these descriptors get
> created on-the-fly in this case.

> @@ -931,7 +931,7 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
>  	while (status) {
>  		int bit = __ffs(status);
>  
> -		generic_handle_irq(irq_find_mapping(nmk_chip->domain, bit));
> +		generic_handle_irq(irq_create_mapping(nmk_chip->domain, bit));

Surely this one can remain as irq_find_mapping() since isn't
nmk_gpio_to_irq() guaranteed to have been called first for this GPIO/IRQ?

[PATCH] pinctrl/nomadik: use irq_create_mapping()

[Stephen Warren]

On 10/19/2012 09:09 AM, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> Since in the DT case, the linear domain path will not allocate
> descriptors for the IRQs, we need to use irq_create_mapping()
> for mapping hwirqs to Linux IRQs, so these descriptors get
> created on-the-fly in this case.

> @@ -931,7 +931,7 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
>  	while (status) {
>  		int bit = __ffs(status);
>  
> -		generic_handle_irq(irq_find_mapping(nmk_chip->domain, bit));
> +		generic_handle_irq(irq_create_mapping(nmk_chip->domain, bit));

Surely this one can remain as irq_find_mapping() since isn't
nmk_gpio_to_irq() guaranteed to have been called first for this GPIO/IRQ?

Re: [PATCH] pinctrl/nomadik: use irq_create_mapping()

[Linus Walleij]

On Fri, Oct 19, 2012 at 6:22 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/19/2012 09:09 AM, Linus Walleij wrote:
>> From: Linus Walleij <linus.walleij@linaro.org>
>
>> @@ -931,7 +931,7 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
>>       while (status) {
>>               int bit = __ffs(status);
>>
>> -             generic_handle_irq(irq_find_mapping(nmk_chip->domain, bit));
>> +             generic_handle_irq(irq_create_mapping(nmk_chip->domain, bit));
>
> Surely this one can remain as irq_find_mapping() since isn't
> nmk_gpio_to_irq() guaranteed to have been called first for this GPIO/IRQ?

It's an IRQ handler so it should be robust to spurious IRQs due to
transient hardware states etc I believe.

So if there is a transient IRQ before gpio_to_irq() is called -> boom.

Yours,
Linus Walleij

[PATCH] pinctrl/nomadik: use irq_create_mapping()

[Linus Walleij]

On Fri, Oct 19, 2012 at 6:22 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/19/2012 09:09 AM, Linus Walleij wrote:
>> From: Linus Walleij <linus.walleij@linaro.org>
>
>> @@ -931,7 +931,7 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
>>       while (status) {
>>               int bit = __ffs(status);
>>
>> -             generic_handle_irq(irq_find_mapping(nmk_chip->domain, bit));
>> +             generic_handle_irq(irq_create_mapping(nmk_chip->domain, bit));
>
> Surely this one can remain as irq_find_mapping() since isn't
> nmk_gpio_to_irq() guaranteed to have been called first for this GPIO/IRQ?

It's an IRQ handler so it should be robust to spurious IRQs due to
transient hardware states etc I believe.

So if there is a transient IRQ before gpio_to_irq() is called -> boom.

Yours,
Linus Walleij

Re: [PATCH] pinctrl/nomadik: use irq_create_mapping()

[Stephen Warren]

On 10/22/2012 02:14 AM, Linus Walleij wrote:
> On Fri, Oct 19, 2012 at 6:22 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 10/19/2012 09:09 AM, Linus Walleij wrote:
>>> From: Linus Walleij <linus.walleij@linaro.org>
>>
>>> @@ -931,7 +931,7 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
>>>       while (status) {
>>>               int bit = __ffs(status);
>>>
>>> -             generic_handle_irq(irq_find_mapping(nmk_chip->domain, bit));
>>> +             generic_handle_irq(irq_create_mapping(nmk_chip->domain, bit));
>>
>> Surely this one can remain as irq_find_mapping() since isn't
>> nmk_gpio_to_irq() guaranteed to have been called first for this GPIO/IRQ?
> 
> It's an IRQ handler so it should be robust to spurious IRQs due to
> transient hardware states etc I believe.
> 
> So if there is a transient IRQ before gpio_to_irq() is called -> boom.

I wonder though (a) why it would be unmasked in HW, and (b) why the
software would even look at the status bit if no handler were registered?

[PATCH] pinctrl/nomadik: use irq_create_mapping()

[Stephen Warren]

On 10/22/2012 02:14 AM, Linus Walleij wrote:
> On Fri, Oct 19, 2012 at 6:22 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 10/19/2012 09:09 AM, Linus Walleij wrote:
>>> From: Linus Walleij <linus.walleij@linaro.org>
>>
>>> @@ -931,7 +931,7 @@ static void __nmk_gpio_irq_handler(unsigned int irq, struct irq_desc *desc,
>>>       while (status) {
>>>               int bit = __ffs(status);
>>>
>>> -             generic_handle_irq(irq_find_mapping(nmk_chip->domain, bit));
>>> +             generic_handle_irq(irq_create_mapping(nmk_chip->domain, bit));
>>
>> Surely this one can remain as irq_find_mapping() since isn't
>> nmk_gpio_to_irq() guaranteed to have been called first for this GPIO/IRQ?
> 
> It's an IRQ handler so it should be robust to spurious IRQs due to
> transient hardware states etc I believe.
> 
> So if there is a transient IRQ before gpio_to_irq() is called -> boom.

I wonder though (a) why it would be unmasked in HW, and (b) why the
software would even look at the status bit if no handler were registered?

Re: [PATCH] pinctrl/nomadik: use irq_create_mapping()

[Linus Walleij]

On Mon, Oct 22, 2012 at 10:08 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/22/2012 02:14 AM, Linus Walleij wrote:

>> It's an IRQ handler so it should be robust to spurious IRQs due to
>> transient hardware states etc I believe.
>>
>> So if there is a transient IRQ before gpio_to_irq() is called -> boom.
>
> I wonder though (a) why it would be unmasked in HW, and (b) why the
> software would even look at the status bit if no handler were registered?

That's true of course ... OK I'll update the patch.

Still I'm not feeling good about the irq_create_mapping/irq_find_mapping
separation, I think a lot of drivers just get this wrong and it's causing
bugs... it'd be way better if there was just one of them and we could
count on descriptors being allocated after adding any kind of irqdomain
but I have no clue how hard it would be to achieve this.

Yours,
Linus Walleij

[PATCH] pinctrl/nomadik: use irq_create_mapping()

[Linus Walleij]

On Mon, Oct 22, 2012 at 10:08 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/22/2012 02:14 AM, Linus Walleij wrote:

>> It's an IRQ handler so it should be robust to spurious IRQs due to
>> transient hardware states etc I believe.
>>
>> So if there is a transient IRQ before gpio_to_irq() is called -> boom.
>
> I wonder though (a) why it would be unmasked in HW, and (b) why the
> software would even look at the status bit if no handler were registered?

That's true of course ... OK I'll update the patch.

Still I'm not feeling good about the irq_create_mapping/irq_find_mapping
separation, I think a lot of drivers just get this wrong and it's causing
bugs... it'd be way better if there was just one of them and we could
count on descriptors being allocated after adding any kind of irqdomain
but I have no clue how hard it would be to achieve this.

Yours,
Linus Walleij