Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Masahiro Yamada]

Hi Marc,


2016-08-01 16:46 GMT+09:00 Marc Zyngier :
> On 01/08/16 02:28, Masahiro Yamada wrote:
>> 2016-07-29 17:10 GMT+09:00 Marc Zyngier :
>>> On 29/07/16 04:53, Masahiro Yamada wrote:
 Hi.


 I noticed my board would not work any more
 when pulling recent updates.


 I did "git-bisect" and I found the following commit is it.
>>>
>>> It would help if you did post the log showing the failure.
>>>
>>> What if you apply the following patch:
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/diff/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi?h=timers/level-trigger=95e1fd920fcadce81626cfa9bd6af1a361f17e58
>>>
>>
>> Hi Mark,
>>
>> Yes, it worked.
>>
>> But I did not understand why you changed the 3rd cell to 0xf08.
>>
>>
>> The binding of arm,gic-v3.txt says as follows:
>>
>>
>>   The 3rd cell is the flags, encoded as follows:
>> bits[3:0] trigger type and level flags.
>> 1 = edge triggered
>> 4 = level triggered
>>
>>
>> Only 1 and 4 are defined for the bits[3:0].
>
> Ah, I didn't realize you were using GICv3.
>
> If you look at the documentation for the A72 timers:
> http://infocenter.arm.com/help/topic/com.arm.doc.100095_0003_05_en/way1382454511590.html
>
> You'll notice that all timers have an active-low output. Switching to
> "level triggered" fixes the issue in general.
>
>> 0xf04 worked, too.
>>
>> Which is correct?
>
> None of them. 0x04 is the correct answer (as we don't encode the
> affinity in the 3rd cell.


Thank you!





-- 
Best Regards
Masahiro Yamada

Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Masahiro Yamada]

Hi Marc,


2016-08-01 16:46 GMT+09:00 Marc Zyngier :
> On 01/08/16 02:28, Masahiro Yamada wrote:
>> 2016-07-29 17:10 GMT+09:00 Marc Zyngier :
>>> On 29/07/16 04:53, Masahiro Yamada wrote:
 Hi.


 I noticed my board would not work any more
 when pulling recent updates.


 I did "git-bisect" and I found the following commit is it.
>>>
>>> It would help if you did post the log showing the failure.
>>>
>>> What if you apply the following patch:
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/diff/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi?h=timers/level-trigger=95e1fd920fcadce81626cfa9bd6af1a361f17e58
>>>
>>
>> Hi Mark,
>>
>> Yes, it worked.
>>
>> But I did not understand why you changed the 3rd cell to 0xf08.
>>
>>
>> The binding of arm,gic-v3.txt says as follows:
>>
>>
>>   The 3rd cell is the flags, encoded as follows:
>> bits[3:0] trigger type and level flags.
>> 1 = edge triggered
>> 4 = level triggered
>>
>>
>> Only 1 and 4 are defined for the bits[3:0].
>
> Ah, I didn't realize you were using GICv3.
>
> If you look at the documentation for the A72 timers:
> http://infocenter.arm.com/help/topic/com.arm.doc.100095_0003_05_en/way1382454511590.html
>
> You'll notice that all timers have an active-low output. Switching to
> "level triggered" fixes the issue in general.
>
>> 0xf04 worked, too.
>>
>> Which is correct?
>
> None of them. 0x04 is the correct answer (as we don't encode the
> affinity in the 3rd cell.


Thank you!





-- 
Best Regards
Masahiro Yamada

Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Marc Zyngier]

On 01/08/16 02:28, Masahiro Yamada wrote:
> 2016-07-29 17:10 GMT+09:00 Marc Zyngier :
>> On 29/07/16 04:53, Masahiro Yamada wrote:
>>> Hi.
>>>
>>>
>>> I noticed my board would not work any more
>>> when pulling recent updates.
>>>
>>>
>>> I did "git-bisect" and I found the following commit is it.
>>
>> It would help if you did post the log showing the failure.
>>
>> What if you apply the following patch:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/diff/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi?h=timers/level-trigger=95e1fd920fcadce81626cfa9bd6af1a361f17e58
>>
> 
> Hi Mark,
> 
> Yes, it worked.
> 
> But I did not understand why you changed the 3rd cell to 0xf08.
> 
> 
> The binding of arm,gic-v3.txt says as follows:
> 
> 
>   The 3rd cell is the flags, encoded as follows:
> bits[3:0] trigger type and level flags.
> 1 = edge triggered
> 4 = level triggered
> 
> 
> Only 1 and 4 are defined for the bits[3:0].

Ah, I didn't realize you were using GICv3.

If you look at the documentation for the A72 timers:
http://infocenter.arm.com/help/topic/com.arm.doc.100095_0003_05_en/way1382454511590.html

You'll notice that all timers have an active-low output. Switching to
"level triggered" fixes the issue in general.

> 0xf04 worked, too.
> 
> Which is correct?

None of them. 0x04 is the correct answer (as we don't encode the
affinity in the 3rd cell.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Marc Zyngier]

On 01/08/16 02:28, Masahiro Yamada wrote:
> 2016-07-29 17:10 GMT+09:00 Marc Zyngier :
>> On 29/07/16 04:53, Masahiro Yamada wrote:
>>> Hi.
>>>
>>>
>>> I noticed my board would not work any more
>>> when pulling recent updates.
>>>
>>>
>>> I did "git-bisect" and I found the following commit is it.
>>
>> It would help if you did post the log showing the failure.
>>
>> What if you apply the following patch:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/diff/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi?h=timers/level-trigger=95e1fd920fcadce81626cfa9bd6af1a361f17e58
>>
> 
> Hi Mark,
> 
> Yes, it worked.
> 
> But I did not understand why you changed the 3rd cell to 0xf08.
> 
> 
> The binding of arm,gic-v3.txt says as follows:
> 
> 
>   The 3rd cell is the flags, encoded as follows:
> bits[3:0] trigger type and level flags.
> 1 = edge triggered
> 4 = level triggered
> 
> 
> Only 1 and 4 are defined for the bits[3:0].

Ah, I didn't realize you were using GICv3.

If you look at the documentation for the A72 timers:
http://infocenter.arm.com/help/topic/com.arm.doc.100095_0003_05_en/way1382454511590.html

You'll notice that all timers have an active-low output. Switching to
"level triggered" fixes the issue in general.

> 0xf04 worked, too.
> 
> Which is correct?

None of them. 0x04 is the correct answer (as we don't encode the
affinity in the 3rd cell.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Masahiro Yamada]

Hi Jon,

2016-07-29 17:31 GMT+09:00 Jon Hunter :
>
> On 29/07/16 04:53, Masahiro Yamada wrote:
>> Hi.
>>
>> I noticed my board would not work any more
>> when pulling recent updates.
>>
>> I did "git-bisect" and I found the following commit is it.
>>
>> commit 1e2a7d78499ec8859d2b469051b7b80bad3b08aa
>> Author: Jon Hunter 
>> Date:   Tue Jun 7 16:12:28 2016 +0100
>>
>> irqdomain: Don't set type when mapping an IRQ
>>
>> With reverting it, everything works fine for me.
>>
>> My board is arch/arm64/boot/dts/socionext/uniphier-ph1-ld20-ref.dts
>>
>> Is anything wrong with IRQ settings in my Device Tree?
>
> Most likely.
>
>> If 1e2a7d784 is really a buggy commit,
>> could you do something please?
>
> Before this commit bad IRQ type settings in device-tree were not getting
> reported and so that's why most likely it is bad IRQ type settings. As
> Marc mentioned without any more details (which IRQ for which device is
> failing) we cannot confirm. So I would look at the failing IRQ which is
> now failing when being requested and see if the type in device-tree is
> correct.
>


I added
bootargs = "earlycon keep_bootcon"


and the following was what I got.
It looks to me that it silently died without any clear error message.




[0.00] Booting Linux on physical CPU 0x0
[0.00] Linux version 4.7.0-07845-g56a940f-dirty
(yamada@beagle) (gcc version 5.3.1 20160113 (Linaro GCC 5.3-2016.02) )
#56 SMP PREEMPT Mon Aug 1 11:15:22 JST 2016
[0.00] Boot CPU: AArch64 Processor [410fd082]
[0.00] earlycon: uniphier0 at MMIO 0x54006800 (options
'115200n8')
[0.00] bootconsole [uniphier0] enabled
[0.00] debug: skip boot console de-registration.
[0.00] efi: Getting EFI parameters from FDT:
[0.00] efi: UEFI not found.
[0.00] cma: Reserved 16 MiB at 0xfec0
[0.00] percpu: Embedded 21 pages/cpu @8000bff7d000 s48256
r8192 d29568 u86016
[0.00] Detected PIPT I-cache on CPU0
[0.00] Built 1 zonelists in Zone order, mobility grouping on.
Total pages: 774144
[0.00] Kernel command line: earlycon keep_bootcon
[0.00] log_buf_len individual max cpu contribution: 4096 bytes
[0.00] log_buf_len total cpu_extra contributions: 12288 bytes
[0.00] log_buf_len min size: 16384 bytes
[0.00] log_buf_len: 32768 bytes
[0.00] early log buf free: 14832(90%)
[0.00] PID hash table entries: 4096 (order: 3, 32768 bytes)
[0.00] Dentry cache hash table entries: 524288 (order: 10,
4194304 bytes)
[0.00] Inode-cache hash table entries: 262144 (order: 9, 2097152 bytes)
[0.00] software IO TLB [mem 0xfac0-0xfec0] (64MB)
mapped at [80007ac0-80007ebf]
[0.00] Memory: 2992924K/3145728K available (7676K kernel code,
657K rwdata, 3208K rodata, 960K init, 269K bss, 136420K reserved,
16384K cma-reserved)
[0.00] Virtual kernel memory layout:
[0.00] modules : 0x - 0x0800
(   128 MB)
[0.00] vmalloc : 0x0800 - 0x7dffbfff
(129022 GB)
[0.00]   .text : 0x0808 - 0x0880
(  7680 KB)
[0.00] .rodata : 0x0880 - 0x08b3
(  3264 KB)
[0.00]   .init : 0x08b3 - 0x08c2
(   960 KB)
[0.00]   .data : 0x08c2 - 0x08cc4600
(   658 KB)
[0.00].bss : 0x08cc4600 - 0x08d07cb0
(   270 KB)
[0.00] fixed   : 0x7dfffe7fd000 - 0x7dfffec0
(  4108 KB)
[0.00] PCI I/O : 0x7dfffee0 - 0x7de0
(16 MB)
[0.00] vmemmap : 0x7e00 - 0x8000
(  2048 GB maximum)
[0.00]   0x7e00 - 0x7e000300
(48 MB actual)
[0.00] memory  : 0x8000 - 0x8000c000
(  3072 MB)
[0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
[0.00] Preemptible hierarchical RCU implementation.
[0.00]  Build-time adjustment of leaf fanout to 64.
[0.00]  RCU restricting CPUs from NR_CPUS=64 to nr_cpu_ids=4.
[0.00] RCU: Adjusting geometry for rcu_fanout_leaf=64, nr_cpu_ids=4
[0.00] NR_IRQS:64 nr_irqs:64 0
[0.00] GICv3: GIC: Using split EOI/Deactivate mode
[0.00] GICv3: CPU0: found redistributor 0 region 0:0x5fe8
[0.00] Architected cp15 timer(s) running at 50.00MHz (phys).
[0.00] clocksource: arch_sys_counter: mask: 0xff
max_cycles: 0xb8812736b, max_idle_ns: 440795202655 ns
[0.02] sched_clock: 56 bits at 50MHz, resolution 20ns, wraps
every 4398046511100ns
[0.008148] Console: colour dummy device 80x25
[0.012595] console [tty0] enabled
[0.016008] Calibrating delay loop (skipped), value calculated
using timer frequency.. 100.00 BogoMIPS (lpj=20)
[

Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Masahiro Yamada]

Hi Jon,

2016-07-29 17:31 GMT+09:00 Jon Hunter :
>
> On 29/07/16 04:53, Masahiro Yamada wrote:
>> Hi.
>>
>> I noticed my board would not work any more
>> when pulling recent updates.
>>
>> I did "git-bisect" and I found the following commit is it.
>>
>> commit 1e2a7d78499ec8859d2b469051b7b80bad3b08aa
>> Author: Jon Hunter 
>> Date:   Tue Jun 7 16:12:28 2016 +0100
>>
>> irqdomain: Don't set type when mapping an IRQ
>>
>> With reverting it, everything works fine for me.
>>
>> My board is arch/arm64/boot/dts/socionext/uniphier-ph1-ld20-ref.dts
>>
>> Is anything wrong with IRQ settings in my Device Tree?
>
> Most likely.
>
>> If 1e2a7d784 is really a buggy commit,
>> could you do something please?
>
> Before this commit bad IRQ type settings in device-tree were not getting
> reported and so that's why most likely it is bad IRQ type settings. As
> Marc mentioned without any more details (which IRQ for which device is
> failing) we cannot confirm. So I would look at the failing IRQ which is
> now failing when being requested and see if the type in device-tree is
> correct.
>


I added
bootargs = "earlycon keep_bootcon"


and the following was what I got.
It looks to me that it silently died without any clear error message.




[0.00] Booting Linux on physical CPU 0x0
[0.00] Linux version 4.7.0-07845-g56a940f-dirty
(yamada@beagle) (gcc version 5.3.1 20160113 (Linaro GCC 5.3-2016.02) )
#56 SMP PREEMPT Mon Aug 1 11:15:22 JST 2016
[0.00] Boot CPU: AArch64 Processor [410fd082]
[0.00] earlycon: uniphier0 at MMIO 0x54006800 (options
'115200n8')
[0.00] bootconsole [uniphier0] enabled
[0.00] debug: skip boot console de-registration.
[0.00] efi: Getting EFI parameters from FDT:
[0.00] efi: UEFI not found.
[0.00] cma: Reserved 16 MiB at 0xfec0
[0.00] percpu: Embedded 21 pages/cpu @8000bff7d000 s48256
r8192 d29568 u86016
[0.00] Detected PIPT I-cache on CPU0
[0.00] Built 1 zonelists in Zone order, mobility grouping on.
Total pages: 774144
[0.00] Kernel command line: earlycon keep_bootcon
[0.00] log_buf_len individual max cpu contribution: 4096 bytes
[0.00] log_buf_len total cpu_extra contributions: 12288 bytes
[0.00] log_buf_len min size: 16384 bytes
[0.00] log_buf_len: 32768 bytes
[0.00] early log buf free: 14832(90%)
[0.00] PID hash table entries: 4096 (order: 3, 32768 bytes)
[0.00] Dentry cache hash table entries: 524288 (order: 10,
4194304 bytes)
[0.00] Inode-cache hash table entries: 262144 (order: 9, 2097152 bytes)
[0.00] software IO TLB [mem 0xfac0-0xfec0] (64MB)
mapped at [80007ac0-80007ebf]
[0.00] Memory: 2992924K/3145728K available (7676K kernel code,
657K rwdata, 3208K rodata, 960K init, 269K bss, 136420K reserved,
16384K cma-reserved)
[0.00] Virtual kernel memory layout:
[0.00] modules : 0x - 0x0800
(   128 MB)
[0.00] vmalloc : 0x0800 - 0x7dffbfff
(129022 GB)
[0.00]   .text : 0x0808 - 0x0880
(  7680 KB)
[0.00] .rodata : 0x0880 - 0x08b3
(  3264 KB)
[0.00]   .init : 0x08b3 - 0x08c2
(   960 KB)
[0.00]   .data : 0x08c2 - 0x08cc4600
(   658 KB)
[0.00].bss : 0x08cc4600 - 0x08d07cb0
(   270 KB)
[0.00] fixed   : 0x7dfffe7fd000 - 0x7dfffec0
(  4108 KB)
[0.00] PCI I/O : 0x7dfffee0 - 0x7de0
(16 MB)
[0.00] vmemmap : 0x7e00 - 0x8000
(  2048 GB maximum)
[0.00]   0x7e00 - 0x7e000300
(48 MB actual)
[0.00] memory  : 0x8000 - 0x8000c000
(  3072 MB)
[0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
[0.00] Preemptible hierarchical RCU implementation.
[0.00]  Build-time adjustment of leaf fanout to 64.
[0.00]  RCU restricting CPUs from NR_CPUS=64 to nr_cpu_ids=4.
[0.00] RCU: Adjusting geometry for rcu_fanout_leaf=64, nr_cpu_ids=4
[0.00] NR_IRQS:64 nr_irqs:64 0
[0.00] GICv3: GIC: Using split EOI/Deactivate mode
[0.00] GICv3: CPU0: found redistributor 0 region 0:0x5fe8
[0.00] Architected cp15 timer(s) running at 50.00MHz (phys).
[0.00] clocksource: arch_sys_counter: mask: 0xff
max_cycles: 0xb8812736b, max_idle_ns: 440795202655 ns
[0.02] sched_clock: 56 bits at 50MHz, resolution 20ns, wraps
every 4398046511100ns
[0.008148] Console: colour dummy device 80x25
[0.012595] console [tty0] enabled
[0.016008] Calibrating delay loop (skipped), value calculated
using timer frequency.. 100.00 BogoMIPS (lpj=20)
[0.026468] pid_max: default: 32768 minimum: 

Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Masahiro Yamada]

2016-07-29 17:10 GMT+09:00 Marc Zyngier :
> On 29/07/16 04:53, Masahiro Yamada wrote:
>> Hi.
>>
>>
>> I noticed my board would not work any more
>> when pulling recent updates.
>>
>>
>> I did "git-bisect" and I found the following commit is it.
>
> It would help if you did post the log showing the failure.
>
> What if you apply the following patch:
>
> https://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/diff/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi?h=timers/level-trigger=95e1fd920fcadce81626cfa9bd6af1a361f17e58
>

Hi Mark,

Yes, it worked.

But I did not understand why you changed the 3rd cell to 0xf08.


The binding of arm,gic-v3.txt says as follows:


  The 3rd cell is the flags, encoded as follows:
bits[3:0] trigger type and level flags.
1 = edge triggered
4 = level triggered


Only 1 and 4 are defined for the bits[3:0].




0xf04 worked, too.

Which is correct?






Best Regards
Masahiro Yamada

Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Masahiro Yamada]

2016-07-29 17:10 GMT+09:00 Marc Zyngier :
> On 29/07/16 04:53, Masahiro Yamada wrote:
>> Hi.
>>
>>
>> I noticed my board would not work any more
>> when pulling recent updates.
>>
>>
>> I did "git-bisect" and I found the following commit is it.
>
> It would help if you did post the log showing the failure.
>
> What if you apply the following patch:
>
> https://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/diff/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi?h=timers/level-trigger=95e1fd920fcadce81626cfa9bd6af1a361f17e58
>

Hi Mark,

Yes, it worked.

But I did not understand why you changed the 3rd cell to 0xf08.


The binding of arm,gic-v3.txt says as follows:


  The 3rd cell is the flags, encoded as follows:
bits[3:0] trigger type and level flags.
1 = edge triggered
4 = level triggered


Only 1 and 4 are defined for the bits[3:0].




0xf04 worked, too.

Which is correct?






Best Regards
Masahiro Yamada

Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Jon Hunter]

On 29/07/16 04:53, Masahiro Yamada wrote:
> Hi.
> 
> I noticed my board would not work any more
> when pulling recent updates.
> 
> I did "git-bisect" and I found the following commit is it.
> 
> commit 1e2a7d78499ec8859d2b469051b7b80bad3b08aa
> Author: Jon Hunter 
> Date:   Tue Jun 7 16:12:28 2016 +0100
> 
> irqdomain: Don't set type when mapping an IRQ
> 
> With reverting it, everything works fine for me.
> 
> My board is arch/arm64/boot/dts/socionext/uniphier-ph1-ld20-ref.dts
> 
> Is anything wrong with IRQ settings in my Device Tree?

Most likely.

> If 1e2a7d784 is really a buggy commit,
> could you do something please?

Before this commit bad IRQ type settings in device-tree were not getting
reported and so that's why most likely it is bad IRQ type settings. As
Marc mentioned without any more details (which IRQ for which device is
failing) we cannot confirm. So I would look at the failing IRQ which is
now failing when being requested and see if the type in device-tree is
correct.

Cheers
Jon

-- 
nvpublic

Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Jon Hunter]

On 29/07/16 04:53, Masahiro Yamada wrote:
> Hi.
> 
> I noticed my board would not work any more
> when pulling recent updates.
> 
> I did "git-bisect" and I found the following commit is it.
> 
> commit 1e2a7d78499ec8859d2b469051b7b80bad3b08aa
> Author: Jon Hunter 
> Date:   Tue Jun 7 16:12:28 2016 +0100
> 
> irqdomain: Don't set type when mapping an IRQ
> 
> With reverting it, everything works fine for me.
> 
> My board is arch/arm64/boot/dts/socionext/uniphier-ph1-ld20-ref.dts
> 
> Is anything wrong with IRQ settings in my Device Tree?

Most likely.

> If 1e2a7d784 is really a buggy commit,
> could you do something please?

Before this commit bad IRQ type settings in device-tree were not getting
reported and so that's why most likely it is bad IRQ type settings. As
Marc mentioned without any more details (which IRQ for which device is
failing) we cannot confirm. So I would look at the failing IRQ which is
now failing when being requested and see if the type in device-tree is
correct.

Cheers
Jon

-- 
nvpublic

Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Marc Zyngier]

On 29/07/16 04:53, Masahiro Yamada wrote:
> Hi.
> 
> 
> I noticed my board would not work any more
> when pulling recent updates.
> 
> 
> I did "git-bisect" and I found the following commit is it.

It would help if you did post the log showing the failure.

What if you apply the following patch:

https://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/diff/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi?h=timers/level-trigger=95e1fd920fcadce81626cfa9bd6af1a361f17e58

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Marc Zyngier]

On 29/07/16 04:53, Masahiro Yamada wrote:
> Hi.
> 
> 
> I noticed my board would not work any more
> when pulling recent updates.
> 
> 
> I did "git-bisect" and I found the following commit is it.

It would help if you did post the log showing the failure.

What if you apply the following patch:

https://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/diff/arch/arm64/boot/dts/socionext/uniphier-ph1-ld20.dtsi?h=timers/level-trigger=95e1fd920fcadce81626cfa9bd6af1a361f17e58

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Masahiro Yamada]

Hi.


I noticed my board would not work any more
when pulling recent updates.


I did "git-bisect" and I found the following commit is it.



commit 1e2a7d78499ec8859d2b469051b7b80bad3b08aa
Author: Jon Hunter 
Date:   Tue Jun 7 16:12:28 2016 +0100

irqdomain: Don't set type when mapping an IRQ




With reverting it, everything works fine for me.

My board is arch/arm64/boot/dts/socionext/uniphier-ph1-ld20-ref.dts

Is anything wrong with IRQ settings in my Device Tree?


If 1e2a7d784 is really a buggy commit,
could you do something please?







2016-06-08 0:12 GMT+09:00 Jon Hunter :
> Some IRQ chips, such as GPIO controllers or secondary level interrupt
> controllers, may require require additional runtime power management
> control to ensure they are accessible. For such IRQ chips, it makes sense
> to enable the IRQ chip when interrupts are requested and disabled them
> again once all interrupts have been freed.
>
> When mapping an IRQ, the IRQ type settings are read and then programmed.
> The mapping of the IRQ happens before the IRQ is requested and so the
> programming of the type settings occurs before the IRQ is requested. This
> is a problem for IRQ chips that require additional power management
> control because they may not be accessible yet. Therefore, when mapping
> the IRQ, don't program the type settings, just save them and then program
> these saved settings when the IRQ is requested (so long as if they are not
> overridden via the call to request the IRQ).
>
> Add a stub function for irq_domain_free_irqs() to avoid any compilation
> errors when CONFIG_IRQ_DOMAIN_HIERARCHY is not selected.
>
> Signed-off-by: Jon Hunter 
> Reviewed-by: Marc Zyngier 
> ---
>  include/linux/irqdomain.h |  3 +++
>  kernel/irq/irqdomain.c| 23 ++-
>  2 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index f1f36e04d885..317503763314 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -452,6 +452,9 @@ static inline int irq_domain_alloc_irqs(struct irq_domain 
> *domain,
> return -1;
>  }
>
> +static inline void irq_domain_free_irqs(unsigned int virq,
> +   unsigned int nr_irqs) { }
> +
>  static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
>  {
> return false;
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index f3ff1eb8dd09..caa6a63d26f0 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -567,6 +567,7 @@ static void of_phandle_args_to_fwspec(struct 
> of_phandle_args *irq_data,
>  unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
>  {
> struct irq_domain *domain;
> +   struct irq_data *irq_data;
> irq_hw_number_t hwirq;
> unsigned int type = IRQ_TYPE_NONE;
> int virq;
> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec 
> *fwspec)
>  * it now and return the interrupt number.
>  */
> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
> -   irq_set_irq_type(virq, type);
> +   irq_data = irq_get_irq_data(virq);
> +   if (!irq_data)
> +   return 0;
> +
> +   irqd_set_trigger_type(irq_data, type);
> return virq;
> }
>
> @@ -634,10 +639,18 @@ unsigned int irq_create_fwspec_mapping(struct 
> irq_fwspec *fwspec)
> return virq;
> }
>
> -   /* Set type if specified and different than the current one */
> -   if (type != IRQ_TYPE_NONE &&
> -   type != irq_get_trigger_type(virq))
> -   irq_set_irq_type(virq, type);
> +   irq_data = irq_get_irq_data(virq);
> +   if (!irq_data) {
> +   if (irq_domain_is_hierarchy(domain))
> +   irq_domain_free_irqs(virq, 1);
> +   else
> +   irq_dispose_mapping(virq);
> +   return 0;
> +   }
> +
> +   /* Store trigger type */
> +   irqd_set_trigger_type(irq_data, type);
> +
> return virq;
>  }
>  EXPORT_SYMBOL_GPL(irq_create_fwspec_mapping);
> --
> 2.1.4
>



-- 
Best Regards
Masahiro Yamada

Re: [PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Masahiro Yamada]

Hi.


I noticed my board would not work any more
when pulling recent updates.


I did "git-bisect" and I found the following commit is it.



commit 1e2a7d78499ec8859d2b469051b7b80bad3b08aa
Author: Jon Hunter 
Date:   Tue Jun 7 16:12:28 2016 +0100

irqdomain: Don't set type when mapping an IRQ




With reverting it, everything works fine for me.

My board is arch/arm64/boot/dts/socionext/uniphier-ph1-ld20-ref.dts

Is anything wrong with IRQ settings in my Device Tree?


If 1e2a7d784 is really a buggy commit,
could you do something please?







2016-06-08 0:12 GMT+09:00 Jon Hunter :
> Some IRQ chips, such as GPIO controllers or secondary level interrupt
> controllers, may require require additional runtime power management
> control to ensure they are accessible. For such IRQ chips, it makes sense
> to enable the IRQ chip when interrupts are requested and disabled them
> again once all interrupts have been freed.
>
> When mapping an IRQ, the IRQ type settings are read and then programmed.
> The mapping of the IRQ happens before the IRQ is requested and so the
> programming of the type settings occurs before the IRQ is requested. This
> is a problem for IRQ chips that require additional power management
> control because they may not be accessible yet. Therefore, when mapping
> the IRQ, don't program the type settings, just save them and then program
> these saved settings when the IRQ is requested (so long as if they are not
> overridden via the call to request the IRQ).
>
> Add a stub function for irq_domain_free_irqs() to avoid any compilation
> errors when CONFIG_IRQ_DOMAIN_HIERARCHY is not selected.
>
> Signed-off-by: Jon Hunter 
> Reviewed-by: Marc Zyngier 
> ---
>  include/linux/irqdomain.h |  3 +++
>  kernel/irq/irqdomain.c| 23 ++-
>  2 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index f1f36e04d885..317503763314 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -452,6 +452,9 @@ static inline int irq_domain_alloc_irqs(struct irq_domain 
> *domain,
> return -1;
>  }
>
> +static inline void irq_domain_free_irqs(unsigned int virq,
> +   unsigned int nr_irqs) { }
> +
>  static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
>  {
> return false;
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index f3ff1eb8dd09..caa6a63d26f0 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -567,6 +567,7 @@ static void of_phandle_args_to_fwspec(struct 
> of_phandle_args *irq_data,
>  unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
>  {
> struct irq_domain *domain;
> +   struct irq_data *irq_data;
> irq_hw_number_t hwirq;
> unsigned int type = IRQ_TYPE_NONE;
> int virq;
> @@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec 
> *fwspec)
>  * it now and return the interrupt number.
>  */
> if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
> -   irq_set_irq_type(virq, type);
> +   irq_data = irq_get_irq_data(virq);
> +   if (!irq_data)
> +   return 0;
> +
> +   irqd_set_trigger_type(irq_data, type);
> return virq;
> }
>
> @@ -634,10 +639,18 @@ unsigned int irq_create_fwspec_mapping(struct 
> irq_fwspec *fwspec)
> return virq;
> }
>
> -   /* Set type if specified and different than the current one */
> -   if (type != IRQ_TYPE_NONE &&
> -   type != irq_get_trigger_type(virq))
> -   irq_set_irq_type(virq, type);
> +   irq_data = irq_get_irq_data(virq);
> +   if (!irq_data) {
> +   if (irq_domain_is_hierarchy(domain))
> +   irq_domain_free_irqs(virq, 1);
> +   else
> +   irq_dispose_mapping(virq);
> +   return 0;
> +   }
> +
> +   /* Store trigger type */
> +   irqd_set_trigger_type(irq_data, type);
> +
> return virq;
>  }
>  EXPORT_SYMBOL_GPL(irq_create_fwspec_mapping);
> --
> 2.1.4
>



-- 
Best Regards
Masahiro Yamada

[PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Jon Hunter]

Some IRQ chips, such as GPIO controllers or secondary level interrupt
controllers, may require require additional runtime power management
control to ensure they are accessible. For such IRQ chips, it makes sense
to enable the IRQ chip when interrupts are requested and disabled them
again once all interrupts have been freed.

When mapping an IRQ, the IRQ type settings are read and then programmed.
The mapping of the IRQ happens before the IRQ is requested and so the
programming of the type settings occurs before the IRQ is requested. This
is a problem for IRQ chips that require additional power management
control because they may not be accessible yet. Therefore, when mapping
the IRQ, don't program the type settings, just save them and then program
these saved settings when the IRQ is requested (so long as if they are not
overridden via the call to request the IRQ).

Add a stub function for irq_domain_free_irqs() to avoid any compilation
errors when CONFIG_IRQ_DOMAIN_HIERARCHY is not selected.

Signed-off-by: Jon Hunter 
Reviewed-by: Marc Zyngier 
---
 include/linux/irqdomain.h |  3 +++
 kernel/irq/irqdomain.c| 23 ++-
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index f1f36e04d885..317503763314 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -452,6 +452,9 @@ static inline int irq_domain_alloc_irqs(struct irq_domain 
*domain,
return -1;
 }
 
+static inline void irq_domain_free_irqs(unsigned int virq,
+   unsigned int nr_irqs) { }
+
 static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
 {
return false;
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index f3ff1eb8dd09..caa6a63d26f0 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -567,6 +567,7 @@ static void of_phandle_args_to_fwspec(struct 
of_phandle_args *irq_data,
 unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 {
struct irq_domain *domain;
+   struct irq_data *irq_data;
irq_hw_number_t hwirq;
unsigned int type = IRQ_TYPE_NONE;
int virq;
@@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec 
*fwspec)
 * it now and return the interrupt number.
 */
if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
-   irq_set_irq_type(virq, type);
+   irq_data = irq_get_irq_data(virq);
+   if (!irq_data)
+   return 0;
+
+   irqd_set_trigger_type(irq_data, type);
return virq;
}
 
@@ -634,10 +639,18 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec 
*fwspec)
return virq;
}
 
-   /* Set type if specified and different than the current one */
-   if (type != IRQ_TYPE_NONE &&
-   type != irq_get_trigger_type(virq))
-   irq_set_irq_type(virq, type);
+   irq_data = irq_get_irq_data(virq);
+   if (!irq_data) {
+   if (irq_domain_is_hierarchy(domain))
+   irq_domain_free_irqs(virq, 1);
+   else
+   irq_dispose_mapping(virq);
+   return 0;
+   }
+
+   /* Store trigger type */
+   irqd_set_trigger_type(irq_data, type);
+
return virq;
 }
 EXPORT_SYMBOL_GPL(irq_create_fwspec_mapping);
-- 
2.1.4

[PATCH V6 3/9] irqdomain: Don't set type when mapping an IRQ

[Jon Hunter]

Some IRQ chips, such as GPIO controllers or secondary level interrupt
controllers, may require require additional runtime power management
control to ensure they are accessible. For such IRQ chips, it makes sense
to enable the IRQ chip when interrupts are requested and disabled them
again once all interrupts have been freed.

When mapping an IRQ, the IRQ type settings are read and then programmed.
The mapping of the IRQ happens before the IRQ is requested and so the
programming of the type settings occurs before the IRQ is requested. This
is a problem for IRQ chips that require additional power management
control because they may not be accessible yet. Therefore, when mapping
the IRQ, don't program the type settings, just save them and then program
these saved settings when the IRQ is requested (so long as if they are not
overridden via the call to request the IRQ).

Add a stub function for irq_domain_free_irqs() to avoid any compilation
errors when CONFIG_IRQ_DOMAIN_HIERARCHY is not selected.

Signed-off-by: Jon Hunter 
Reviewed-by: Marc Zyngier 
---
 include/linux/irqdomain.h |  3 +++
 kernel/irq/irqdomain.c| 23 ++-
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index f1f36e04d885..317503763314 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -452,6 +452,9 @@ static inline int irq_domain_alloc_irqs(struct irq_domain 
*domain,
return -1;
 }
 
+static inline void irq_domain_free_irqs(unsigned int virq,
+   unsigned int nr_irqs) { }
+
 static inline bool irq_domain_is_hierarchy(struct irq_domain *domain)
 {
return false;
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index f3ff1eb8dd09..caa6a63d26f0 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -567,6 +567,7 @@ static void of_phandle_args_to_fwspec(struct 
of_phandle_args *irq_data,
 unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 {
struct irq_domain *domain;
+   struct irq_data *irq_data;
irq_hw_number_t hwirq;
unsigned int type = IRQ_TYPE_NONE;
int virq;
@@ -614,7 +615,11 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec 
*fwspec)
 * it now and return the interrupt number.
 */
if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
-   irq_set_irq_type(virq, type);
+   irq_data = irq_get_irq_data(virq);
+   if (!irq_data)
+   return 0;
+
+   irqd_set_trigger_type(irq_data, type);
return virq;
}
 
@@ -634,10 +639,18 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec 
*fwspec)
return virq;
}
 
-   /* Set type if specified and different than the current one */
-   if (type != IRQ_TYPE_NONE &&
-   type != irq_get_trigger_type(virq))
-   irq_set_irq_type(virq, type);
+   irq_data = irq_get_irq_data(virq);
+   if (!irq_data) {
+   if (irq_domain_is_hierarchy(domain))
+   irq_domain_free_irqs(virq, 1);
+   else
+   irq_dispose_mapping(virq);
+   return 0;
+   }
+
+   /* Store trigger type */
+   irqd_set_trigger_type(irq_data, type);
+
return virq;
 }
 EXPORT_SYMBOL_GPL(irq_create_fwspec_mapping);
-- 
2.1.4