Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1195498 > unrolled thread

Re: Regression in v4.2-rc1 caused by hierarchical irqdomain changes

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: Regression in v4.2-rc1 caused by hierarchical irqdomain  changes Thomas Gleixner <tglx@linutronix.de> - 2015-07-29 23:10 +0200
    Re: Regression in v4.2-rc1 caused by hierarchical irqdomain changes Jiang Liu <jiang.liu@linux.intel.com> - 2015-07-30 06:10 +0200
    [Bugfix] x86, irq: Fix regression caused by commit d32932d02e18 Jiang Liu <jiang.liu@linux.intel.com> - 2015-07-30 10:00 +0200
      Re: [Bugfix] x86, irq: Fix regression caused by commit  d32932d02e18 Thomas Gleixner <tglx@linutronix.de> - 2015-07-30 10:40 +0200
        Re: [Bugfix] x86, irq: Fix regression caused by commit d32932d02e18 Jiang Liu <jiang.liu@linux.intel.com> - 2015-07-30 11:00 +0200
      Re: [Bugfix] x86, irq: Fix regression caused by commit d32932d02e18 Matt Fleming <matt@codeblueprint.co.uk> - 2015-07-30 11:10 +0200
        Re: [Bugfix] x86, irq: Fix regression caused by commit d32932d02e18 Jiang Liu <jiang.liu@linux.intel.com> - 2015-07-30 11:20 +0200
          Re: [Bugfix] x86, irq: Fix regression caused by commit d32932d02e18 Matt Fleming <matt@codeblueprint.co.uk> - 2015-07-30 11:50 +0200
      [tip:x86/urgent] x86/irq:   Use the caller provided polarity setting in mp_check_pin_attr() tip-bot for Jiang Liu <tipbot@zytor.com> - 2015-07-30 21:30 +0200

#1195498 — Re: Regression in v4.2-rc1 caused by hierarchical irqdomain changes

On Mon, 27 Jul 2015, Matt Fleming wrote:
> [BAD]
>   3:          1          0          0          0  IR-IO-APIC   3-edge    
>   6:          0          0          0          0  IR-IO-APIC   6-edge      dw_dmac
>   7:         15          1          0          0  IR-IO-APIC   7-edge      INT3432:00, INT3433:00

[GOOD]
>   3:         16         17         49         20  IR-IO-APIC-fasteoi   AudioDSP, dw_dmac
>   6:          0          0          0          0  IR-IO-APIC-fasteoi   dw_dmac
>   7:       2662       3072      12307       5419  IR-IO-APIC-fasteoi   INT3432:00, INT3433:00

So the old code uses fasteoi while the new one uses edge.

Jiang????



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1195705 — Re: Regression in v4.2-rc1 caused by hierarchical irqdomain changes

On 2015/7/30 5:03, Thomas Gleixner wrote:
> On Mon, 27 Jul 2015, Matt Fleming wrote:
>> [BAD]
>>   3:          1          0          0          0  IR-IO-APIC   3-edge    
>>   6:          0          0          0          0  IR-IO-APIC   6-edge      dw_dmac
>>   7:         15          1          0          0  IR-IO-APIC   7-edge      INT3432:00, INT3433:00
> 
> [GOOD]
>>   3:         16         17         49         20  IR-IO-APIC-fasteoi   AudioDSP, dw_dmac
>>   6:          0          0          0          0  IR-IO-APIC-fasteoi   dw_dmac
>>   7:       2662       3072      12307       5419  IR-IO-APIC-fasteoi   INT3432:00, INT3433:00
> 
> So the old code uses fasteoi while the new one uses edge.
> 
> Jiang????
Sorry for the slow response.
We have reproduced this regression on Surface Pro 3, but are still
trying to figure out the root cause.
Thanks!
Gerry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1195777 — [Bugfix] x86, irq: Fix regression caused by commit d32932d02e18

Commit d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical
irqdomain interfaces") introduced a regression when converting IOAPIC
to hierarchy irqdomain, which sets wrong irq flow handler for IOAPIC
pins on Surface Pro 3 and causes failure when detecting I2C controllers.
It's caused by using stale attribute value when setting up IOAPIC pins.
With this patch applied, all IOAPIC IRQ configuration are identical
to v4.1 on Surface Pro 3.

Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
Reported-by: Matt Fleming <matt@codeblueprint.co.uk>
Tested-and-reported-by: Chen Yu <yu.c.chen@intel.com>
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
---
Hi Matt,
	Could you please help to test this patch?
Thanks!
Gerry
---
 arch/x86/kernel/apic/io_apic.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 845dc0df2002..206052e55517 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -943,7 +943,7 @@ static bool mp_check_pin_attr(int irq, struct irq_alloc_info *info)
 	 */
 	if (irq < nr_legacy_irqs() && data->count == 1) {
 		if (info->ioapic_trigger != data->trigger)
-			mp_register_handler(irq, data->trigger);
+			mp_register_handler(irq, info->ioapic_trigger);
 		data->entry.trigger = data->trigger = info->ioapic_trigger;
 		data->entry.polarity = data->polarity = info->ioapic_polarity;
 	}
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1195817 — Re: [Bugfix] x86, irq: Fix regression caused by commit d32932d02e18

On Thu, 30 Jul 2015, Jiang Liu wrote:

> Commit d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical
> irqdomain interfaces") introduced a regression when converting IOAPIC
> to hierarchy irqdomain, which sets wrong irq flow handler for IOAPIC
> pins on Surface Pro 3 and causes failure when detecting I2C controllers.
> It's caused by using stale attribute value when setting up IOAPIC pins.
> With this patch applied, all IOAPIC IRQ configuration are identical
> to v4.1 on Surface Pro 3.
>  
> Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
> Reported-by: Matt Fleming <matt@codeblueprint.co.uk>
> Tested-and-reported-by: Chen Yu <yu.c.chen@intel.com>
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
> Hi Matt,
> 	Could you please help to test this patch?
> Thanks!
> Gerry
> ---
>  arch/x86/kernel/apic/io_apic.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 845dc0df2002..206052e55517 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -943,7 +943,7 @@ static bool mp_check_pin_attr(int irq, struct irq_alloc_info *info)
>  	 */
>  	if (irq < nr_legacy_irqs() && data->count == 1) {
>  		if (info->ioapic_trigger != data->trigger)
> -			mp_register_handler(irq, data->trigger);
> +			mp_register_handler(irq, info->ioapic_trigger);
>  		data->entry.trigger = data->trigger = info->ioapic_trigger;
>  		data->entry.polarity = data->polarity = info->ioapic_polarity;

Pretty obvious, but I was staring at this very piece of code for hours
without spotting it.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1195823 — Re: [Bugfix] x86, irq: Fix regression caused by commit d32932d02e18

On 2015/7/30 16:33, Thomas Gleixner wrote:
> On Thu, 30 Jul 2015, Jiang Liu wrote:
> 
>> Commit d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical
>> irqdomain interfaces") introduced a regression when converting IOAPIC
>> to hierarchy irqdomain, which sets wrong irq flow handler for IOAPIC
>> pins on Surface Pro 3 and causes failure when detecting I2C controllers.
>> It's caused by using stale attribute value when setting up IOAPIC pins.
>> With this patch applied, all IOAPIC IRQ configuration are identical
>> to v4.1 on Surface Pro 3.
>>  
>> Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
>> Reported-by: Matt Fleming <matt@codeblueprint.co.uk>
>> Tested-and-reported-by: Chen Yu <yu.c.chen@intel.com>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> ---
>> Hi Matt,
>> 	Could you please help to test this patch?
>> Thanks!
>> Gerry
>> ---
>>  arch/x86/kernel/apic/io_apic.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>> index 845dc0df2002..206052e55517 100644
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -943,7 +943,7 @@ static bool mp_check_pin_attr(int irq, struct irq_alloc_info *info)
>>  	 */
>>  	if (irq < nr_legacy_irqs() && data->count == 1) {
>>  		if (info->ioapic_trigger != data->trigger)
>> -			mp_register_handler(irq, data->trigger);
>> +			mp_register_handler(irq, info->ioapic_trigger);
>>  		data->entry.trigger = data->trigger = info->ioapic_trigger;
>>  		data->entry.polarity = data->polarity = info->ioapic_polarity;
> 
> Pretty obvious, but I was staring at this very piece of code for hours
> without spotting it.
You have given me the most valuable hints about the difference in
irq flow handler:)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1195826 — Re: [Bugfix] x86, irq: Fix regression caused by commit d32932d02e18

On Thu, 30 Jul, at 03:51:32PM, Jiang Liu wrote:
> Commit d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical
> irqdomain interfaces") introduced a regression when converting IOAPIC
> to hierarchy irqdomain, which sets wrong irq flow handler for IOAPIC
> pins on Surface Pro 3 and causes failure when detecting I2C controllers.
> It's caused by using stale attribute value when setting up IOAPIC pins.
> With this patch applied, all IOAPIC IRQ configuration are identical
> to v4.1 on Surface Pro 3.
> 
> Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
> Reported-by: Matt Fleming <matt@codeblueprint.co.uk>
> Tested-and-reported-by: Chen Yu <yu.c.chen@intel.com>
> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
> ---
> Hi Matt,
> 	Could you please help to test this patch?
> Thanks!
> Gerry
> ---
>  arch/x86/kernel/apic/io_apic.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Works for me! What I actually did was apply this fixup commit ontop of
commit d32932d02e18, just to be sure that it fixes the issue. I'm about
to apply it on top of -rc4 just to be sure.

But failing any issues arising,

Tested-by: Matt Fleming <matt.fleming@intel.com>

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1195834 — Re: [Bugfix] x86, irq: Fix regression caused by commit d32932d02e18

On 2015/7/30 17:07, Matt Fleming wrote:
> On Thu, 30 Jul, at 03:51:32PM, Jiang Liu wrote:
>> Commit d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical
>> irqdomain interfaces") introduced a regression when converting IOAPIC
>> to hierarchy irqdomain, which sets wrong irq flow handler for IOAPIC
>> pins on Surface Pro 3 and causes failure when detecting I2C controllers.
>> It's caused by using stale attribute value when setting up IOAPIC pins.
>> With this patch applied, all IOAPIC IRQ configuration are identical
>> to v4.1 on Surface Pro 3.
>>
>> Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
>> Reported-by: Matt Fleming <matt@codeblueprint.co.uk>
>> Tested-and-reported-by: Chen Yu <yu.c.chen@intel.com>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> ---
>> Hi Matt,
>> 	Could you please help to test this patch?
>> Thanks!
>> Gerry
>> ---
>>  arch/x86/kernel/apic/io_apic.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Works for me! What I actually did was apply this fixup commit ontop of
> commit d32932d02e18, just to be sure that it fixes the issue. I'm about
> to apply it on top of -rc4 just to be sure.
Hi Matt,
	Chen found there are other issues with 4.2-rc4 which
breaks Surface Pro 3's I2C and multitouch drivers. If you run into
the same trouble, please contact Chen Yu <yu.c.chen@intel.com>
for details:)
Thanks!
Gerry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1195869 — Re: [Bugfix] x86, irq: Fix regression caused by commit d32932d02e18

On Thu, 30 Jul, at 05:12:39PM, Jiang Liu wrote:
> Hi Matt,
> 	Chen found there are other issues with 4.2-rc4 which
> breaks Surface Pro 3's I2C and multitouch drivers. If you run into
> the same trouble, please contact Chen Yu <yu.c.chen@intel.com>
> for details:)

No i2c/multitouch issues here on v4.2-rc4, everything works fine!

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1196389 — [tip:x86/urgent] x86/irq: Use the caller provided polarity setting in mp_check_pin_attr()

Commit-ID:  646c4b75494747887f936513b669bb8a2d794459
Gitweb:     http://git.kernel.org/tip/646c4b75494747887f936513b669bb8a2d794459
Author:     Jiang Liu <jiang.liu@linux.intel.com>
AuthorDate: Thu, 30 Jul 2015 15:51:32 +0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 30 Jul 2015 21:15:29 +0200

x86/irq: Use the caller provided polarity setting in mp_check_pin_attr()

Commit d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical
irqdomain interfaces") introduced a regression which causes
malfunction of interrupt lines.

The reason is that the conversion of mp_check_pin_attr() missed to
update the polarity selection of the interrupt pin with the caller
provided setting and instead uses a stale attribute value. That in
turn results in chosing the wrong interrupt flow handler.

Use the caller supplied setting to configure the pin correctly which
also choses the correct interrupt flow handler.

This restores the original behaviour and on the affected
machine/driver (Surface Pro 3, i2c controller) all IOAPIC IRQ
configuration are identical to v4.1.

Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
Reported-and-tested-by: Matt Fleming <matt@codeblueprint.co.uk>
Reported-and-tested-by: Chen Yu <yu.c.chen@intel.com>
Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Chen Yu <yu.c.chen@intel.com>
Cc: Yinghai Lu <yinghai@kernel.org>
Link: http://lkml.kernel.org/r/1438242695-23531-1-git-send-email-jiang.liu@linux.intel.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/apic/io_apic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 845dc0d..206052e 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -943,7 +943,7 @@ static bool mp_check_pin_attr(int irq, struct irq_alloc_info *info)
 	 */
 	if (irq < nr_legacy_irqs() && data->count == 1) {
 		if (info->ioapic_trigger != data->trigger)
-			mp_register_handler(irq, data->trigger);
+			mp_register_handler(irq, info->ioapic_trigger);
 		data->entry.trigger = data->trigger = info->ioapic_trigger;
 		data->entry.polarity = data->polarity = info->ioapic_polarity;
 	}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web