ARM/Kernel: Problems with multiple VIC registration

ARM/Kernel: Problems with multiple VIC registration

[Amit Virdi]

Hi All,

I've picked Linux kernel 3.8-rc4 and added the support for my ARM based 
board in it.
- The SoC has 2 VICs
- valid_sources are 32 (~0)
- NR_IRQS is defined as 104; => irq_desc[104]
- I'm not populating devices thro' DT

The first controller is registered throu' irq_domain_add_linear API. As 
a result of this call, a new domain is allocated to the VIC device.

In the next step IRQ mapping, irq_create_mapping is called and this maps 
all the IRQs in the Linux IRQ space. However in doing so, it uses 
irq_desc[1]...irq_desc[32]. irq_dec[0] is not used (checked using a 
debugger)!

When I'm trying to register the second VIC (it's throu' 
irq_domain_add_legacy), I'm getting warnings since irq_get_irq_data 
returns irq_data of irq_desc[32] which has already been mapped.

In short, irq_domain_add_legacy returns with NULL irq_domain.

I checked that multiple VIC registration works in Linux 3.4
Is it something that I'm missing while adapting to the new framework, or 
there's some undetected problem?

I've attached the logfile generated with debug prints ON. Would 
appreciate any help...

Thanks
Amit Virdi
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: logfile.txt
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130205/e095a301/attachment-0001.txt>

ARM/Kernel: Problems with multiple VIC registration

[Rob Herring]

Adding Linus W.

On 02/05/2013 03:48 AM, Amit Virdi wrote:
> Hi All,
> 
> I've picked Linux kernel 3.8-rc4 and added the support for my ARM based
> board in it.

What about rc6? Linus has been fixing some VIC issues in this area.

Rob

> - The SoC has 2 VICs
> - valid_sources are 32 (~0)
> - NR_IRQS is defined as 104; => irq_desc[104]
> - I'm not populating devices thro' DT
> 
> The first controller is registered throu' irq_domain_add_linear API. As
> a result of this call, a new domain is allocated to the VIC device.
> 
> In the next step IRQ mapping, irq_create_mapping is called and this maps
> all the IRQs in the Linux IRQ space. However in doing so, it uses
> irq_desc[1]...irq_desc[32]. irq_dec[0] is not used (checked using a
> debugger)!
> 
> When I'm trying to register the second VIC (it's throu'
> irq_domain_add_legacy), I'm getting warnings since irq_get_irq_data
> returns irq_data of irq_desc[32] which has already been mapped.
> 
> In short, irq_domain_add_legacy returns with NULL irq_domain.
> 
> I checked that multiple VIC registration works in Linux 3.4
> Is it something that I'm missing while adapting to the new framework, or
> there's some undetected problem?
> 
> I've attached the logfile generated with debug prints ON. Would
> appreciate any help...
> 
> Thanks
> Amit Virdi

ARM/Kernel: Problems with multiple VIC registration

[Linus Walleij]

On Tue, Feb 5, 2013 at 10:48 AM, Amit Virdi <amit.virdi@st.com> wrote:

> The first controller is registered throu' irq_domain_add_linear API. As a
> result of this call, a new domain is allocated to the VIC device.

This is a bug. Do you have this patch in your tree:
http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7611/1

Yours,
Linus Walleij

ARM/Kernel: Problems with multiple VIC registration

[Amit Virdi]

On 2/5/2013 9:57 PM, Linus Walleij wrote:
> On Tue, Feb 5, 2013 at 10:48 AM, Amit Virdi<amit.virdi@st.com>  wrote:
>
>> The first controller is registered throu' irq_domain_add_linear API. As a
>> result of this call, a new domain is allocated to the VIC device.
>
> This is a bug. Do you have this patch in your tree:
> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7611/1
>

Yes, I have this patch (from rc2) and still the bug!

Regards
Amit Virdi

ARM/Kernel: Problems with multiple VIC registration

[Amit Virdi]

> On 02/05/2013 03:48 AM, Amit Virdi wrote:
>> Hi All,
>>
>> I've picked Linux kernel 3.8-rc4 and added the support for my ARM based
>> board in it.
>
> What about rc6? Linus has been fixing some VIC issues in this area.
>

Same behavior with rc6. I guess the fix provided by Linus W. is not 
addressing this issue.

Regards
Amit Virdi

ARM/Kernel: Problems with multiple VIC registration

[Linus Walleij]

On Wed, Feb 6, 2013 at 4:52 AM, Amit Virdi <amit.virdi@st.com> wrote:
> On 2/5/2013 9:57 PM, Linus Walleij wrote:
>>
>> On Tue, Feb 5, 2013 at 10:48 AM, Amit Virdi<amit.virdi@st.com>  wrote:
>>
>>> The first controller is registered throu' irq_domain_add_linear API. As a
>>> result of this call, a new domain is allocated to the VIC device.
>>
>>
>> This is a bug. Do you have this patch in your tree:
>> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7611/1
>
> Yes, I have this patch (from rc2) and still the bug!

You have to post your code for your SoC board support for us to help
out.

There are several things you can do wrong. For example a common mistake
is to offset the IRQs on a new platform from zero. So that the first
parameter to vic_init() is zero. This is not legal because Linux IRQ 0
is NO_IRQ and shall not be used. If this is your problems, IRQs shall
offset from something like 64 so you can even use SPARSE_IRQ.

Yours,
Linus Walleij

ARM/Kernel: Problems with multiple VIC registration

[Amit Virdi]

Linus,

On 2/6/2013 2:45 PM, Linus Walleij wrote:
> On Wed, Feb 6, 2013 at 4:52 AM, Amit Virdi<amit.virdi@st.com>  wrote:
>> On 2/5/2013 9:57 PM, Linus Walleij wrote:
>>>
>>> On Tue, Feb 5, 2013 at 10:48 AM, Amit Virdi<amit.virdi@st.com>   wrote:
>>>
>>>> The first controller is registered throu' irq_domain_add_linear API. As a
>>>> result of this call, a new domain is allocated to the VIC device.
>>>
>>>
>>> This is a bug. Do you have this patch in your tree:
>>> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=7611/1
>>
>> Yes, I have this patch (from rc2) and still the bug!
>
> You have to post your code for your SoC board support for us to help
> out.
>
> There are several things you can do wrong. For example a common mistake
> is to offset the IRQs on a new platform from zero. So that the first
> parameter to vic_init() is zero. This is not legal because Linux IRQ 0
> is NO_IRQ and shall not be used. If this is your problems, IRQs shall
> offset from something like 64 so you can even use SPARSE_IRQ.
>

Yes, I was using 0 as the first offset in IRQs and my IRQs were defined 
from 0 to n. I changed it to 1 to n+1 and I'm now not using 0. Every 
problem seems to have resolved now.

Thanks
Amit Virdi

ARM/Kernel: Problems with multiple VIC registration

[Linus Walleij]

On Tue, Feb 12, 2013 at 1:38 PM, Amit Virdi <amit.virdi@st.com> wrote:
> On 2/6/2013 2:45 PM, Linus Walleij wrote:

>> There are several things you can do wrong. For example a common mistake
>> is to offset the IRQs on a new platform from zero. So that the first
>> parameter to vic_init() is zero. This is not legal because Linux IRQ 0
>> is NO_IRQ and shall not be used. If this is your problems, IRQs shall
>> offset from something like 64 so you can even use SPARSE_IRQ.
>
> Yes, I was using 0 as the first offset in IRQs and my IRQs were defined from
> 0 to n. I changed it to 1 to n+1 and I'm now not using 0. Every problem
> seems to have resolved now.

1 and n+1 is only going to work as long as you're defining
NR_IRQS in the platform or in the .nr_irqs member of
the MACHINE record.

If you use device tree or SPARSE_IRQ (which, by the way,
you should turn on right now) you will run into trouble.

Bump it to 64+n.

Else you will just have to fix all this when you get to
submitting board support upstream.

Yours,
Linus Walleij

ARM/Kernel: Problems with multiple VIC registration

[Amit Virdi]

On 2/12/2013 6:41 PM, Linus Walleij wrote:
> On Tue, Feb 12, 2013 at 1:38 PM, Amit Virdi<amit.virdi@st.com>  wrote:
>> On 2/6/2013 2:45 PM, Linus Walleij wrote:
>
>>> There are several things you can do wrong. For example a common mistake
>>> is to offset the IRQs on a new platform from zero. So that the first
>>> parameter to vic_init() is zero. This is not legal because Linux IRQ 0
>>> is NO_IRQ and shall not be used. If this is your problems, IRQs shall
>>> offset from something like 64 so you can even use SPARSE_IRQ.
>>
>> Yes, I was using 0 as the first offset in IRQs and my IRQs were defined from
>> 0 to n. I changed it to 1 to n+1 and I'm now not using 0. Every problem
>> seems to have resolved now.
>
> 1 and n+1 is only going to work as long as you're defining
> NR_IRQS in the platform or in the .nr_irqs member of
> the MACHINE record.
>
> If you use device tree or SPARSE_IRQ (which, by the way,
> you should turn on right now) you will run into trouble.
>
> Bump it to 64+n.
>
> Else you will just have to fix all this when you get to
> submitting board support upstream.
>

Okay, thanks for your support!!

Regards
Amit Virdi