* [RFC 13/14] irq_domain: Remove 'new' irq_domain in favour of the ppc one
2012-01-13 0:31 ` [RFC 13/14] irq_domain: Remove 'new' irq_domain in favour of the ppc one Rob Herring
@ 2012-01-13 0:47 ` Grant Likely
2012-01-13 0:53 ` Rob Herring
2012-01-13 2:20 ` Grant Likely
2012-01-17 2:43 ` Michael Bohan
2 siblings, 1 reply; 8+ messages in thread
From: Grant Likely @ 2012-01-13 0:47 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 12, 2012 at 5:31 PM, Rob Herring <robherring2@gmail.com> wrote:
> Adding lakml...
>
> On 01/11/2012 03:27 PM, Grant Likely wrote:
>> On Wed, Jan 11, 2012 at 2:15 PM, Rob Herring <robherring2@gmail.com> wrote:
>>> Grant,
>>>
>>> On 01/11/2012 02:22 PM, Grant Likely wrote:
>>>> This patch removes the simplistic implementation of irq_domains and enables
>>>> the powerpc infrastructure for all irq_domain users. ?The powerpc
>>>> infrastructure includes support for complex mappings between Linux and
>>>> hardware irq numbers, and can manage allocation of irq_descs.
>>>>
>>>> This patch also converts the few users of irq_domain_add()/irq_domain_del()
>>>> to call irq_domain_add_legacy() instead.
>>>
>>> So what is the non-legacy way? Legacy implies we don't want to do it
>>> that way. I guess until we remove all non-DT platforms with GIC we are
>>> stuck with legacy. That seems like it could be a ways out until we get
>>> there.
>>
>> Non-legacy is letting the irq_domain manage the irq_desc allocations.
>> Some of the controllers will be easy to convert, some will be more
>> difficult. ?The primary thing that really blocks getting away from the
>> legacy method is anything that expects hardcoded #defined irq numbers.
>> ?The goal is to convert all users over to the linear revmap method.
>>
>
> So I gave this a spin on highbank. I ran into a couple problems.
>
> I had to revert "irqdesc: Consolidate irq reservation logic" which is in
> your branch, but not this series. irq_alloc_desc_from was returning -EEXIST.
Hmmm... I thought I sorted that out. Thanks for letting me know.
>
> The GIC code did not work which I think is specific to using gic_of_init
> which makes irq_start = -1. With that it still doesn't work. It dies in
> gic_set_type... I've found one problem which I'll reply inline to, but I
> think this is a dead end path anyway.
Haha, I'm not surprised. That last patch was only compile tested on
platforms using the gic. I'm not surprised that I flubbed it.
> You have removed the irq_alloc_descs call from the GIC which is a step
> backwards. Several of the ARM DT enabled platforms are at the point they
> can fully support dynamic virq base for each irqchip. I changed the
> domain from legacy to linear and got things working.
> The issue with
I hadn't actually intended to remove the irq_alloc_descs in this
patch. That was a leftover hunk from when I was playing with going
straight to irq_domain_add_linear(). For this specific patch, I'll
put the alloc back in and test it that way. A follow-on patch can do
a proper conversion to the linear revmap.
> linear is for SPARSE_IRQ. The default behavior on ARM for SPARSE_IRQ is
> all nr_irqs are allocated at boot time before any controller is
> initialized. The only platform with a GIC and requiring SPARSE_IRQ is
> shmobile, but it is also the only one that calls irq_alloc_desc
> functions for it's interrupts. So I think we are okay there. The problem
> occurs when enabling SPARSE_IRQ for a non-DT platform with a GIC and
> with irqchips that don't call irq_alloc_desc for their irqs. IMHO, this
> should be an okay trade-off. There's no advantage to enabling SPARSE_IRQ
> on ARM for platforms that don't require it. All the platforms with a GIC
> have active work to convert to DT (except shmobile which I think is
> okay), so it's a temporary issue.
Actually, I believe Thomas' long term goal is to always enable
SPARSE_IRQ and remove the option entirely, so it should still be
properly resolved. I'll take a look next week if I don't get to it
tomorrow. I need to resurrect my vexpress qemu test environment so I
can test the permutations.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 13/14] irq_domain: Remove 'new' irq_domain in favour of the ppc one
2012-01-13 0:47 ` Grant Likely
@ 2012-01-13 0:53 ` Rob Herring
0 siblings, 0 replies; 8+ messages in thread
From: Rob Herring @ 2012-01-13 0:53 UTC (permalink / raw)
To: linux-arm-kernel
On 01/12/2012 06:47 PM, Grant Likely wrote:
> On Thu, Jan 12, 2012 at 5:31 PM, Rob Herring <robherring2@gmail.com> wrote:
>> Adding lakml...
>>
>> On 01/11/2012 03:27 PM, Grant Likely wrote:
>>> On Wed, Jan 11, 2012 at 2:15 PM, Rob Herring <robherring2@gmail.com> wrote:
>>>> Grant,
>>>>
>>>> On 01/11/2012 02:22 PM, Grant Likely wrote:
>>>>> This patch removes the simplistic implementation of irq_domains and enables
>>>>> the powerpc infrastructure for all irq_domain users. The powerpc
>>>>> infrastructure includes support for complex mappings between Linux and
>>>>> hardware irq numbers, and can manage allocation of irq_descs.
>>>>>
>>>>> This patch also converts the few users of irq_domain_add()/irq_domain_del()
>>>>> to call irq_domain_add_legacy() instead.
>>>>
>>>> So what is the non-legacy way? Legacy implies we don't want to do it
>>>> that way. I guess until we remove all non-DT platforms with GIC we are
>>>> stuck with legacy. That seems like it could be a ways out until we get
>>>> there.
>>>
>>> Non-legacy is letting the irq_domain manage the irq_desc allocations.
>>> Some of the controllers will be easy to convert, some will be more
>>> difficult. The primary thing that really blocks getting away from the
>>> legacy method is anything that expects hardcoded #defined irq numbers.
>>> The goal is to convert all users over to the linear revmap method.
>>>
>>
>> So I gave this a spin on highbank. I ran into a couple problems.
>>
>> I had to revert "irqdesc: Consolidate irq reservation logic" which is in
>> your branch, but not this series. irq_alloc_desc_from was returning -EEXIST.
>
> Hmmm... I thought I sorted that out. Thanks for letting me know.
>
>>
>> The GIC code did not work which I think is specific to using gic_of_init
>> which makes irq_start = -1. With that it still doesn't work. It dies in
>> gic_set_type... I've found one problem which I'll reply inline to, but I
>> think this is a dead end path anyway.
>
> Haha, I'm not surprised. That last patch was only compile tested on
> platforms using the gic. I'm not surprised that I flubbed it.
>
>> You have removed the irq_alloc_descs call from the GIC which is a step
>> backwards. Several of the ARM DT enabled platforms are at the point they
>> can fully support dynamic virq base for each irqchip. I changed the
>> domain from legacy to linear and got things working.
>> The issue with
>
> I hadn't actually intended to remove the irq_alloc_descs in this
> patch. That was a leftover hunk from when I was playing with going
> straight to irq_domain_add_linear(). For this specific patch, I'll
> put the alloc back in and test it that way. A follow-on patch can do
> a proper conversion to the linear revmap.
>
>> linear is for SPARSE_IRQ. The default behavior on ARM for SPARSE_IRQ is
>> all nr_irqs are allocated at boot time before any controller is
>> initialized. The only platform with a GIC and requiring SPARSE_IRQ is
>> shmobile, but it is also the only one that calls irq_alloc_desc
>> functions for it's interrupts. So I think we are okay there. The problem
>> occurs when enabling SPARSE_IRQ for a non-DT platform with a GIC and
>> with irqchips that don't call irq_alloc_desc for their irqs. IMHO, this
>> should be an okay trade-off. There's no advantage to enabling SPARSE_IRQ
>> on ARM for platforms that don't require it. All the platforms with a GIC
>> have active work to convert to DT (except shmobile which I think is
>> okay), so it's a temporary issue.
>
> Actually, I believe Thomas' long term goal is to always enable
> SPARSE_IRQ and remove the option entirely, so it should still be
> properly resolved. I'll take a look next week if I don't get to it
> tomorrow. I need to resurrect my vexpress qemu test environment so I
> can test the permutations.
>
Agreed. I think that is the path to the removing include of mach/irqs.h
as well, and I have a patch series to do just that when SPARSE_IRQ is
enabled. It also has the problem of breaking platforms which don't NEED
to enable SPARSE_IRQ. I'll try to get it sent out soon.
Rob
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 13/14] irq_domain: Remove 'new' irq_domain in favour of the ppc one
2012-01-13 0:31 ` [RFC 13/14] irq_domain: Remove 'new' irq_domain in favour of the ppc one Rob Herring
2012-01-13 0:47 ` Grant Likely
@ 2012-01-13 2:20 ` Grant Likely
2012-01-17 2:43 ` Michael Bohan
2 siblings, 0 replies; 8+ messages in thread
From: Grant Likely @ 2012-01-13 2:20 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jan 12, 2012 at 06:31:28PM -0600, Rob Herring wrote:
> Adding lakml...
>
> On 01/11/2012 03:27 PM, Grant Likely wrote:
> > On Wed, Jan 11, 2012 at 2:15 PM, Rob Herring <robherring2@gmail.com> wrote:
> >> Grant,
> >>
> >> On 01/11/2012 02:22 PM, Grant Likely wrote:
> >>> This patch removes the simplistic implementation of irq_domains and enables
> >>> the powerpc infrastructure for all irq_domain users. The powerpc
> >>> infrastructure includes support for complex mappings between Linux and
> >>> hardware irq numbers, and can manage allocation of irq_descs.
> >>>
> >>> This patch also converts the few users of irq_domain_add()/irq_domain_del()
> >>> to call irq_domain_add_legacy() instead.
> >>
> >> So what is the non-legacy way? Legacy implies we don't want to do it
> >> that way. I guess until we remove all non-DT platforms with GIC we are
> >> stuck with legacy. That seems like it could be a ways out until we get
> >> there.
> >
> > Non-legacy is letting the irq_domain manage the irq_desc allocations.
> > Some of the controllers will be easy to convert, some will be more
> > difficult. The primary thing that really blocks getting away from the
> > legacy method is anything that expects hardcoded #defined irq numbers.
> > The goal is to convert all users over to the linear revmap method.
> >
>
> So I gave this a spin on highbank. I ran into a couple problems.
>
> I had to revert "irqdesc: Consolidate irq reservation logic" which is in
> your branch, but not this series. irq_alloc_desc_from was returning -EEXIST.
Gah, I flubbed that patch too. Try this on top of it:
---
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 8a9e2ec..11feb2f 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -375,6 +375,9 @@ __irq_reserve_irqs(int irq, unsigned int from, unsigned int cnt)
}
bitmap_set(allocated_irqs, start, cnt);
+ mutex_unlock(&sparse_irq_lock);
+ return start;
+
err:
mutex_unlock(&sparse_irq_lock);
return ret;
@@ -408,7 +411,8 @@ EXPORT_SYMBOL_GPL(__irq_alloc_descs);
*/
int irq_reserve_irqs(unsigned int from, unsigned int cnt)
{
- return __irq_reserve_irqs(from, from, cnt);
+ int start = __irq_reserve_irqs(from, from, cnt);
+ return start < 0 ? start : 0;
}
/**
^ permalink raw reply related [flat|nested] 8+ messages in thread* [RFC 13/14] irq_domain: Remove 'new' irq_domain in favour of the ppc one
2012-01-13 0:31 ` [RFC 13/14] irq_domain: Remove 'new' irq_domain in favour of the ppc one Rob Herring
2012-01-13 0:47 ` Grant Likely
2012-01-13 2:20 ` Grant Likely
@ 2012-01-17 2:43 ` Michael Bohan
2012-01-17 3:42 ` Benjamin Herrenschmidt
2 siblings, 1 reply; 8+ messages in thread
From: Michael Bohan @ 2012-01-17 2:43 UTC (permalink / raw)
To: linux-arm-kernel
On 1/12/2012 4:31 PM, Rob Herring wrote:
> You have removed the irq_alloc_descs call from the GIC which is a step
> backwards. Several of the ARM DT enabled platforms are at the point they
> can fully support dynamic virq base for each irqchip. I changed the
> domain from legacy to linear and got things working. The issue with
> linear is for SPARSE_IRQ. The default behavior on ARM for SPARSE_IRQ is
> all nr_irqs are allocated at boot time before any controller is
> initialized. The only platform with a GIC and requiring SPARSE_IRQ is
> shmobile, but it is also the only one that calls irq_alloc_desc
> functions for it's interrupts. So I think we are okay there. The problem
> occurs when enabling SPARSE_IRQ for a non-DT platform with a GIC and
> with irqchips that don't call irq_alloc_desc for their irqs. IMHO, this
> should be an okay trade-off. There's no advantage to enabling SPARSE_IRQ
> on ARM for platforms that don't require it. All the platforms with a GIC
> have active work to convert to DT (except shmobile which I think is
> okay), so it's a temporary issue.
I thought I would chime in here since this is very relevant to what
we're doing on arm-msm. We are using Device Tree with the GIC. We also
have several other interrupt chips, including one for a device that
supports up to 32768 sparsely populated interrupts. Hence we are using
SPARSE_IRQ to only allocate irq_descs on demand for this device.
To support this, I had to modify irq_domain_add() to not 'register' the
domain. Eg. simply add the domain to the list, but do not initialize the
hwirq values in the irq_data structure. This is because our irq_descs
are allocated later based on Device Tree topology. This information is
not available during of_irq_init().
But then I was left with the problem of managing irq_bases for multiple
chip drivers. The problem with irq_alloc_desc() is that it doesn't allow
you to allocate a range without allocating its corresponding resources.
Meaning, I'd like to reserve a chunk of virqs, but not utilize any
resources for them until I know they're necessary. This is where
irq_domains comes in.
In order to support this properly, I decided it made sense to add a new
API called irq_domain_find_free_range(). This walks the irq domains
list, now sorted by domain->irq_base, to find the first available range
of the size specified. Thus every irq chip then only needs to know the
worst case number of interrupts it could ever support. The framework
takes care of finding the specific virq base. And all of this
information is available at of_irq_init() time. This all assumes that
every chip driver registers with irq_domains.
I was planning on sending these patches out, but it seems like with
Grant's patches, they may no longer be up to date. I was curious if any
thought was given to supporting configurations like this the one I
mentioned with the advent of these patches. I am happy to help test if
you can steer me in the right direction.
Thanks,
Mike
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 13/14] irq_domain: Remove 'new' irq_domain in favour of the ppc one
2012-01-17 2:43 ` Michael Bohan
@ 2012-01-17 3:42 ` Benjamin Herrenschmidt
2012-01-18 0:28 ` Grant Likely
0 siblings, 1 reply; 8+ messages in thread
From: Benjamin Herrenschmidt @ 2012-01-17 3:42 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 2012-01-16 at 18:43 -0800, Michael Bohan wrote:
>
> I was planning on sending these patches out, but it seems like with
> Grant's patches, they may no longer be up to date. I was curious if any
> thought was given to supporting configurations like this the one I
> mentioned with the advent of these patches. I am happy to help test if
> you can steer me in the right direction.
I haven't had a chance to look in detail at what Grant is doing in his
latest series, but the ppc domain scheme that he's basing it on has the
concept of sparse interrupt domains.
For these, we use a radix tree for the reverse map (we do not rely on a
linear range) and we "allocate" linux IRQ numbers on-demand as we create
mapping for individual HW interrupts.
Cheers,
Ben.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC 13/14] irq_domain: Remove 'new' irq_domain in favour of the ppc one
2012-01-17 3:42 ` Benjamin Herrenschmidt
@ 2012-01-18 0:28 ` Grant Likely
2012-01-18 2:16 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 8+ messages in thread
From: Grant Likely @ 2012-01-18 0:28 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jan 16, 2012 at 8:42 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Mon, 2012-01-16 at 18:43 -0800, Michael Bohan wrote:
>>
>> I was planning on sending these patches out, but it seems like with
>> Grant's patches, they may no longer be up to date. I was curious if any
>> thought was given to supporting configurations like this the one I
>> mentioned with the advent of these patches. I am happy to help test if
>> you can steer me in the right direction.
>
> I haven't had a chance to look in detail at what Grant is doing in his
> latest series, but the ppc domain scheme that he's basing it on has the
> concept of sparse interrupt domains.
>
> For these, we use a radix tree for the reverse map (we do not rely on a
> linear range) and we "allocate" linux IRQ numbers on-demand as we create
> mapping for individual HW interrupts.
My latest patches are primarily a direct port of the ppc code to be
generic for all architectures, and it does pretty much what you want.
As Ben suggests, you should look at the radix revmap code. Use the
irq_domain_add_tree() function for creating the irq_domain, and there
is no need to even pre-allocate the number of hwirqs that you want to
support. It uses a radix tree to manage arbitrary mappings from hwirq
number to linux numbers.
The other option is the linear map which maintains a static array with
one entry per hwirq number. Use irq_domain_add_linear() to create one
and pass in the maximum number of hwirqs supported by the controller.
It will allocate an array of irq_desc* so that irq_descs can be
allocated dynamically.
It is also important to note that for both these revmaps, the actual
virq number is irrelevant. The irq_domain is entirely responsible for
allocation and management. There will be no need for a 'chunk of
virqs' assigned to an irq controller.
BTW: Ben, if you can carve out some time, I'd appreciate a look over
my series and make sure you're okay with it. Once it's had some
testing by arm and ppc folks, I want to get it into linux-next.
g.
^ permalink raw reply [flat|nested] 8+ messages in thread