2013-04-11 09:09:49

by Linus Walleij

[permalink] [raw]
Subject: [PATCH] MFD: ab8500: pass AB8500 IRQ to debugfs code by resource

From: Linus Walleij <[email protected]>

The AB8500 debug code which was merged in parallell with the
multiplatform work incidentally introduced a new instance using
the <mach/irqs.h> header which is now deleted, causing this
build regression:

drivers/mfd/ab8500-debugfs.c:95:23:
fatal error: mach/irqs.h: No such file or directory
compilation terminated.
make[4]: *** [drivers/mfd/ab8500-debugfs.o] Error 1

The code most certainly never worked with device tree either
since that does not rely on this kind of hard-coded interrupt
numbers.

Fix the problem at the root by passing it as a named resource
from the ab8500-core driver.

Cc: Arnd Bergmann <[email protected]>
Cc: Lee Jones <[email protected]>
Signed-off-by: Linus Walleij <[email protected]>
---
Sam this would have to go into the v3.10-targetted stuff...
Either immediately before you send it upstream or as a fixup.
---
drivers/mfd/ab8500-core.c | 11 +++++++++++
drivers/mfd/ab8500-debugfs.c | 14 ++++++++++----
2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/ab8500-core.c b/drivers/mfd/ab8500-core.c
index 8e8a016..dd72914 100644
--- a/drivers/mfd/ab8500-core.c
+++ b/drivers/mfd/ab8500-core.c
@@ -868,6 +868,11 @@ static struct resource ab8500_chargalg_resources[] = {};
#ifdef CONFIG_DEBUG_FS
static struct resource ab8500_debug_resources[] = {
{
+ .name = "IRQ_AB8500",
+ /* Number will be filled in */
+ .flags = IORESOURCE_IRQ,
+ },
+ {
.name = "IRQ_FIRST",
.start = AB8500_INT_MAIN_EXT_CH_NOT_OK,
.end = AB8500_INT_MAIN_EXT_CH_NOT_OK,
@@ -1712,6 +1717,12 @@ static int ab8500_probe(struct platform_device *pdev)
if (ret)
return ret;

+#if CONFIG_DEBUG_FS
+ /* Pass to debugfs */
+ ab8500_debug_resources[0].start = ab8500->irq;
+ ab8500_debug_resources[0].end = ab8500->irq;
+#endif
+
if (is_ab9540(ab8500))
ret = mfd_add_devices(ab8500->dev, 0, ab9540_devs,
ARRAY_SIZE(ab9540_devs), NULL,
diff --git a/drivers/mfd/ab8500-debugfs.c b/drivers/mfd/ab8500-debugfs.c
index b88bbbc..bd39146 100644
--- a/drivers/mfd/ab8500-debugfs.c
+++ b/drivers/mfd/ab8500-debugfs.c
@@ -91,12 +91,10 @@
#include <linux/ctype.h>
#endif

-/* TODO: this file should not reference IRQ_DB8500_AB8500! */
-#include <mach/irqs.h>
-
static u32 debug_bank;
static u32 debug_address;

+static int irq_ab8500;
static int irq_first;
static int irq_last;
static u32 *irq_count;
@@ -1589,7 +1587,7 @@ void ab8500_debug_register_interrupt(int line)
{
if (line < num_interrupt_lines) {
num_interrupts[line]++;
- if (suspend_test_wake_cause_interrupt_is_mine(IRQ_DB8500_AB8500))
+ if (suspend_test_wake_cause_interrupt_is_mine(irq_ab8500))
num_wake_interrupts[line]++;
}
}
@@ -2959,6 +2957,14 @@ static int ab8500_debug_probe(struct platform_device *plf)
if (!event_name)
goto out_freedev_attr;

+ irq_ab8500 = platform_get_irq_byname(plf, "IRQ_AB8500");
+ if (irq_ab8500 < 0) {
+ dev_err(&plf->dev, "AB8500 irq not found, err %d\n",
+ irq_first);
+ ret = irq_ab8500;
+ goto out_freeevent_name;
+ }
+
irq_first = platform_get_irq_byname(plf, "IRQ_FIRST");
if (irq_first < 0) {
dev_err(&plf->dev, "First irq not found, err %d\n",
--
1.7.11.3


2013-04-11 14:34:03

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] MFD: ab8500: pass AB8500 IRQ to debugfs code by resource

On Thursday 11 April 2013, Linus Walleij wrote:
> From: Linus Walleij <[email protected]>
>
> The AB8500 debug code which was merged in parallell with the
> multiplatform work incidentally introduced a new instance using
> the <mach/irqs.h> header which is now deleted, causing this
> build regression:
>
> drivers/mfd/ab8500-debugfs.c:95:23:
> fatal error: mach/irqs.h: No such file or directory
> compilation terminated.
> make[4]: *** [drivers/mfd/ab8500-debugfs.o] Error 1
>
> The code most certainly never worked with device tree either
> since that does not rely on this kind of hard-coded interrupt
> numbers.
>
> Fix the problem at the root by passing it as a named resource
> from the ab8500-core driver.

I think this won't work because the interrupts in ab8500_debug_resources
are now local numbers relative to the ab8500->domain irq_domain,
while IRQ_DB8500_AB8500 is an global interrupt number.

Arnd

2013-04-12 07:50:05

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] MFD: ab8500: pass AB8500 IRQ to debugfs code by resource

On Thu, Apr 11, 2013 at 4:33 PM, Arnd Bergmann <[email protected]> wrote:
> On Thursday 11 April 2013, Linus Walleij wrote:
>> From: Linus Walleij <[email protected]>
>>
>> The AB8500 debug code which was merged in parallell with the
>> multiplatform work incidentally introduced a new instance using
>> the <mach/irqs.h> header which is now deleted, causing this
>> build regression:
>>
>> drivers/mfd/ab8500-debugfs.c:95:23:
>> fatal error: mach/irqs.h: No such file or directory
>> compilation terminated.
>> make[4]: *** [drivers/mfd/ab8500-debugfs.o] Error 1
>>
>> The code most certainly never worked with device tree either
>> since that does not rely on this kind of hard-coded interrupt
>> numbers.
>>
>> Fix the problem at the root by passing it as a named resource
>> from the ab8500-core driver.
>
> I think this won't work because the interrupts in ab8500_debug_resources
> are now local numbers relative to the ab8500->domain irq_domain,
> while IRQ_DB8500_AB8500 is an global interrupt number.

Actually, well, this one IRQ (the one I start to pass) is already
domain-mapped to the global IRQ numberspace, so that one will
be correct.

However the debug driver patch, i.e. the entire debugfs driver,
is completely irqdomain-unaware and has been broken for
device tree since forever and broken for non-dt builds since
the IRQ numbers were made local.

I guess I'll just have to cook a second patch to fix this up...

Yours,
Linus Walleij

2013-04-12 08:08:59

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] MFD: ab8500: pass AB8500 IRQ to debugfs code by resource

On Friday 12 April 2013, Linus Walleij wrote:
> >
> > I think this won't work because the interrupts in ab8500_debug_resources
> > are now local numbers relative to the ab8500->domain irq_domain,
> > while IRQ_DB8500_AB8500 is an global interrupt number.
>
> Actually, well, this one IRQ (the one I start to pass) is already
> domain-mapped to the global IRQ numberspace, so that one will
> be correct.

I don't know if we're talking about the same thing here.

My point was that passing a domain-mapped IRQ number into mfd_add_devices
with another domain will result in the attempt to map that number into
another domain, which does not work.

> However the debug driver patch, i.e. the entire debugfs driver,
> is completely irqdomain-unaware and has been broken for
> device tree since forever and broken for non-dt builds since
> the IRQ numbers were made local.
>
> I guess I'll just have to cook a second patch to fix this up...

The debug driver should not need to worry about domains, as long
as the ab8500->domain passed into mfd_add_devices is correctly
set up.

Arnd

2013-04-12 08:41:37

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] MFD: ab8500: pass AB8500 IRQ to debugfs code by resource

On Fri, Apr 12, 2013 at 10:08 AM, Arnd Bergmann <[email protected]> wrote:
> On Friday 12 April 2013, Linus Walleij wrote:

>> Actually, well, this one IRQ (the one I start to pass) is already
>> domain-mapped to the global IRQ numberspace, so that one will
>> be correct.
>
> I don't know if we're talking about the same thing here.
>
> My point was that passing a domain-mapped IRQ number into mfd_add_devices
> with another domain will result in the attempt to map that number into
> another domain, which does not work.

Ah, yes you're right ... now I need to find some other way
to pass this resource ... hm.

Yours,
Linus Walleij

2013-04-12 10:48:59

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] MFD: ab8500: pass AB8500 IRQ to debugfs code by resource

On Friday 12 April 2013, Linus Walleij wrote:
> On Fri, Apr 12, 2013 at 10:08 AM, Arnd Bergmann <[email protected]> wrote:
> > On Friday 12 April 2013, Linus Walleij wrote:
>
> >> Actually, well, this one IRQ (the one I start to pass) is already
> >> domain-mapped to the global IRQ numberspace, so that one will
> >> be correct.
> >
> > I don't know if we're talking about the same thing here.
> >
> > My point was that passing a domain-mapped IRQ number into mfd_add_devices
> > with another domain will result in the attempt to map that number into
> > another domain, which does not work.
>
> Ah, yes you're right ... now I need to find some other way
> to pass this resource ... hm.
>

What is it actually needed for? In the code I'm looking at in linux-next
it only gets passed to a function that ignores it.

Arnd

2013-04-12 13:43:55

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] MFD: ab8500: pass AB8500 IRQ to debugfs code by resource

On Fri, Apr 12, 2013 at 12:48 PM, Arnd Bergmann <[email protected]> wrote:
> On Friday 12 April 2013, Linus Walleij wrote:
>> On Fri, Apr 12, 2013 at 10:08 AM, Arnd Bergmann <[email protected]> wrote:
>> > On Friday 12 April 2013, Linus Walleij wrote:
>>
>> >> Actually, well, this one IRQ (the one I start to pass) is already
>> >> domain-mapped to the global IRQ numberspace, so that one will
>> >> be correct.
>> >
>> > I don't know if we're talking about the same thing here.
>> >
>> > My point was that passing a domain-mapped IRQ number into mfd_add_devices
>> > with another domain will result in the attempt to map that number into
>> > another domain, which does not work.
>>
>> Ah, yes you're right ... now I need to find some other way
>> to pass this resource ... hm.
>
> What is it actually needed for? In the code I'm looking at in linux-next
> it only gets passed to a function that ignores it.

Well that is a __attribute((weak)) function. It is overridden
by suspend/resume code in the machine to ask the suspend
code whether the AB8500 woke the system up.

Arguably not such a nice design :-/

However that suspend code is being posted to the linux-pm
mailinglist as we speak, so I don't want to make their life
miserable by just deleting some hook they're actually using
for testing.

Yours,
Linus Walleij