* [PATCH] mfd: db8500-prcmu: fix irqdomain usage
@ 2012-12-19 14:42 Linus Walleij
2012-12-19 15:16 ` Lee Jones
2012-12-20 1:32 ` Grant Likely
0 siblings, 2 replies; 6+ messages in thread
From: Linus Walleij @ 2012-12-19 14:42 UTC (permalink / raw)
To: Samuel Ortiz, linux-kernel; +Cc: Anmar Oueja, Linus Walleij, stable, Lee Jones
From: Linus Walleij <linus.walleij@linaro.org>
This fixes two issues with the DB8500 PRCMU irqdomain:
- You have to state the irq base 0 to get a linear domain
for the DT case from irq_domain_add_simple()
- The irqdomain was not used to translate the initial irq
request using irq_create_mapping() making the linear
case fail as it was lacking a proper descriptor.
I took this opportunity to fix two lines of whitespace
errors in related code as I was anyway messing around with
it.
Cc: stable@kernel.org
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Hi Sam, this is a regression and as such should go into the
-rc series. As you can see I also added the Cc: stable tag.
Yours,
Linus Walleij
---
drivers/mfd/db8500-prcmu.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index dc8826d..fcac8e0 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -2524,7 +2524,7 @@ static bool read_mailbox_0(void)
for (n = 0; n < NUM_PRCMU_WAKEUPS; n++) {
if (ev & prcmu_irq_bit[n])
- generic_handle_irq(IRQ_PRCMU_BASE + n);
+ generic_handle_irq(irq_create_mapping(db8500_irq_domain,n));
}
r = true;
break;
@@ -2737,13 +2737,13 @@ static int db8500_irq_map(struct irq_domain *d, unsigned int virq,
}
static struct irq_domain_ops db8500_irq_ops = {
- .map = db8500_irq_map,
- .xlate = irq_domain_xlate_twocell,
+ .map = db8500_irq_map,
+ .xlate = irq_domain_xlate_twocell,
};
static int db8500_irq_init(struct device_node *np)
{
- int irq_base = -1;
+ int irq_base = 0;
/* In the device tree case, just take some IRQs */
if (!np)
--
1.7.11.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: db8500-prcmu: fix irqdomain usage
2012-12-19 14:42 [PATCH] mfd: db8500-prcmu: fix irqdomain usage Linus Walleij
@ 2012-12-19 15:16 ` Lee Jones
2012-12-20 1:32 ` Grant Likely
1 sibling, 0 replies; 6+ messages in thread
From: Lee Jones @ 2012-12-19 15:16 UTC (permalink / raw)
To: Linus Walleij
Cc: Samuel Ortiz, linux-kernel, Anmar Oueja, Linus Walleij, stable
On Wed, 19 Dec 2012, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
>
> This fixes two issues with the DB8500 PRCMU irqdomain:
> - You have to state the irq base 0 to get a linear domain
> for the DT case from irq_domain_add_simple()
> - The irqdomain was not used to translate the initial irq
> request using irq_create_mapping() making the linear
> case fail as it was lacking a proper descriptor.
>
> I took this opportunity to fix two lines of whitespace
> errors in related code as I was anyway messing around with
> it.
>
> Cc: stable@kernel.org
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Hi Sam, this is a regression and as such should go into the
> -rc series. As you can see I also added the Cc: stable tag.
>
> Yours,
> Linus Walleij
> ---
> drivers/mfd/db8500-prcmu.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
> index dc8826d..fcac8e0 100644
> --- a/drivers/mfd/db8500-prcmu.c
> +++ b/drivers/mfd/db8500-prcmu.c
> @@ -2524,7 +2524,7 @@ static bool read_mailbox_0(void)
>
> for (n = 0; n < NUM_PRCMU_WAKEUPS; n++) {
> if (ev & prcmu_irq_bit[n])
> - generic_handle_irq(IRQ_PRCMU_BASE + n);
> + generic_handle_irq(irq_create_mapping(db8500_irq_domain,n));
I thought you were fixing whitespace errors, not introducing them --^
=:-)
> }
> r = true;
> break;
> @@ -2737,13 +2737,13 @@ static int db8500_irq_map(struct irq_domain *d, unsigned int virq,
> }
>
> static struct irq_domain_ops db8500_irq_ops = {
> - .map = db8500_irq_map,
> - .xlate = irq_domain_xlate_twocell,
> + .map = db8500_irq_map,
> + .xlate = irq_domain_xlate_twocell,
That's strange. I wrote this, but have always used 8 space tabbing. :|
> };
>
> static int db8500_irq_init(struct device_node *np)
> {
> - int irq_base = -1;
> + int irq_base = 0;
>
> /* In the device tree case, just take some IRQs */
> if (!np)
> --
> 1.7.11.3
>
Besides the whitespace error it looks good.
Acked-by: Lee Jones <lee.jones@linaro.org>
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: db8500-prcmu: fix irqdomain usage
2012-12-19 14:42 [PATCH] mfd: db8500-prcmu: fix irqdomain usage Linus Walleij
2012-12-19 15:16 ` Lee Jones
@ 2012-12-20 1:32 ` Grant Likely
2012-12-20 7:50 ` Lee Jones
1 sibling, 1 reply; 6+ messages in thread
From: Grant Likely @ 2012-12-20 1:32 UTC (permalink / raw)
To: Linus Walleij, Samuel Ortiz, linux-kernel
Cc: Anmar Oueja, Linus Walleij, stable, Lee Jones
On Wed, 19 Dec 2012 15:42:41 +0100, Linus Walleij <linus.walleij@stericsson.com> wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
>
> This fixes two issues with the DB8500 PRCMU irqdomain:
> - You have to state the irq base 0 to get a linear domain
> for the DT case from irq_domain_add_simple()
> - The irqdomain was not used to translate the initial irq
> request using irq_create_mapping() making the linear
> case fail as it was lacking a proper descriptor.
>
> I took this opportunity to fix two lines of whitespace
> errors in related code as I was anyway messing around with
> it.
>
> Cc: stable@kernel.org
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Hi Sam, this is a regression and as such should go into the
> -rc series. As you can see I also added the Cc: stable tag.
>
> Yours,
> Linus Walleij
> ---
> drivers/mfd/db8500-prcmu.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
> index dc8826d..fcac8e0 100644
> --- a/drivers/mfd/db8500-prcmu.c
> +++ b/drivers/mfd/db8500-prcmu.c
> @@ -2524,7 +2524,7 @@ static bool read_mailbox_0(void)
>
> for (n = 0; n < NUM_PRCMU_WAKEUPS; n++) {
> if (ev & prcmu_irq_bit[n])
> - generic_handle_irq(IRQ_PRCMU_BASE + n);
> + generic_handle_irq(irq_create_mapping(db8500_irq_domain,n));
This looks wrong. It probably works at the moment, but calling
irq_create_mapping when trying to process an IRQ is backwards. There
certainly shouldn't be any processing of an irq that isn't already
mapped. irq_find_mapping() should be used instead.
g.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: db8500-prcmu: fix irqdomain usage
2012-12-20 1:32 ` Grant Likely
@ 2012-12-20 7:50 ` Lee Jones
2012-12-20 9:17 ` Linus Walleij
0 siblings, 1 reply; 6+ messages in thread
From: Lee Jones @ 2012-12-20 7:50 UTC (permalink / raw)
To: Grant Likely
Cc: Linus Walleij, Samuel Ortiz, linux-kernel, Anmar Oueja,
Linus Walleij, stable
On Thu, 20 Dec 2012, Grant Likely wrote:
> On Wed, 19 Dec 2012 15:42:41 +0100, Linus Walleij <linus.walleij@stericsson.com> wrote:
> > From: Linus Walleij <linus.walleij@linaro.org>
> >
> > This fixes two issues with the DB8500 PRCMU irqdomain:
> > - You have to state the irq base 0 to get a linear domain
> > for the DT case from irq_domain_add_simple()
> > - The irqdomain was not used to translate the initial irq
> > request using irq_create_mapping() making the linear
> > case fail as it was lacking a proper descriptor.
> >
> > I took this opportunity to fix two lines of whitespace
> > errors in related code as I was anyway messing around with
> > it.
> >
> > Cc: stable@kernel.org
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > Hi Sam, this is a regression and as such should go into the
> > -rc series. As you can see I also added the Cc: stable tag.
> >
> > Yours,
> > Linus Walleij
> > ---
> > drivers/mfd/db8500-prcmu.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
> > index dc8826d..fcac8e0 100644
> > --- a/drivers/mfd/db8500-prcmu.c
> > +++ b/drivers/mfd/db8500-prcmu.c
> > @@ -2524,7 +2524,7 @@ static bool read_mailbox_0(void)
> >
> > for (n = 0; n < NUM_PRCMU_WAKEUPS; n++) {
> > if (ev & prcmu_irq_bit[n])
> > - generic_handle_irq(IRQ_PRCMU_BASE + n);
> > + generic_handle_irq(irq_create_mapping(db8500_irq_domain,n));
>
> This looks wrong. It probably works at the moment, but calling
> irq_create_mapping when trying to process an IRQ is backwards. There
> certainly shouldn't be any processing of an irq that isn't already
> mapped. irq_find_mapping() should be used instead.
But irq_create_mapping() calls irq_find_mapping() if the irq is
already mapped? Or are you saying that if it isn't already mapped
then there is an issue somewhere else earlier in the code?
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: db8500-prcmu: fix irqdomain usage
2012-12-20 7:50 ` Lee Jones
@ 2012-12-20 9:17 ` Linus Walleij
2012-12-20 10:04 ` Lee Jones
0 siblings, 1 reply; 6+ messages in thread
From: Linus Walleij @ 2012-12-20 9:17 UTC (permalink / raw)
To: Lee Jones
Cc: Grant Likely, Linus Walleij, Samuel Ortiz, linux-kernel,
Anmar Oueja, stable
On Thu, Dec 20, 2012 at 8:50 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 20 Dec 2012, Grant Likely wrote:
>>
>> This looks wrong. It probably works at the moment, but calling
>> irq_create_mapping when trying to process an IRQ is backwards. There
>> certainly shouldn't be any processing of an irq that isn't already
>> mapped. irq_find_mapping() should be used instead.
>
> But irq_create_mapping() calls irq_find_mapping() if the irq is
> already mapped? Or are you saying that if it isn't already mapped
> then there is an issue somewhere else earlier in the code?
I've submitted a v2 patch making the changes I think Grant is
fishing for..
Basically irq_create_mapping() should be in slowpath and
irq_find_mapping() in fastpath with the semantics that
the former has to be called at least once before the other
is ever called.
This is not quite trivial to use ... irqdomain seems like it
will bite us repeatedly for the next few years.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: db8500-prcmu: fix irqdomain usage
2012-12-20 9:17 ` Linus Walleij
@ 2012-12-20 10:04 ` Lee Jones
0 siblings, 0 replies; 6+ messages in thread
From: Lee Jones @ 2012-12-20 10:04 UTC (permalink / raw)
To: Linus Walleij
Cc: Grant Likely, Linus Walleij, Samuel Ortiz, linux-kernel,
Anmar Oueja, stable
On Thu, 20 Dec 2012, Linus Walleij wrote:
> On Thu, Dec 20, 2012 at 8:50 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Thu, 20 Dec 2012, Grant Likely wrote:
> >>
> >> This looks wrong. It probably works at the moment, but calling
> >> irq_create_mapping when trying to process an IRQ is backwards. There
> >> certainly shouldn't be any processing of an irq that isn't already
> >> mapped. irq_find_mapping() should be used instead.
> >
> > But irq_create_mapping() calls irq_find_mapping() if the irq is
> > already mapped? Or are you saying that if it isn't already mapped
> > then there is an issue somewhere else earlier in the code?
>
> I've submitted a v2 patch making the changes I think Grant is
> fishing for..
>
> Basically irq_create_mapping() should be in slowpath and
> irq_find_mapping() in fastpath with the semantics that
> the former has to be called at least once before the other
> is ever called.
Still seems strange to me. Calling irq_create_mapping() on pre-
mapped IRQs is only a few cycles more and it saves the extra
bumph now inserted into patch v2.
> This is not quite trivial to use ... irqdomain seems like it
> will bite us repeatedly for the next few years.
I agree. Personally I like to call irq_create_mapping(), then
I know that if the IRQ isn't already mapped, it soon will be.
This is almost certainly not the way the Grant intended the
API would be used, but seems simpler and diverts the
complexity you speak of above.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-12-20 10:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-19 14:42 [PATCH] mfd: db8500-prcmu: fix irqdomain usage Linus Walleij
2012-12-19 15:16 ` Lee Jones
2012-12-20 1:32 ` Grant Likely
2012-12-20 7:50 ` Lee Jones
2012-12-20 9:17 ` Linus Walleij
2012-12-20 10:04 ` Lee Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).