All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] genirq: irqdomain_hierarchy: fixes
@ 2015-08-12 17:45 ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:45 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel,
	linux-kernel, Grygorii Strashko

Hi All,

I've had able to identify and reproduce four issues related to switching on
using IRQ domain hierarchy on TI OMAP DRA7 (dra7-evm). Most of them were
discovered during testing of Suspend to RAM and IRQ wakeup functionality.
In my opinion, most of these issue could also affect on other ARM SoC if
they are using IRQ domain hierarchy.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
 ARM GIC <- OMAP wakeupgen <- TI CBAR

Issue 1 - patch 1(new): IRQ re-triggering is not working if IRQ domain
hierarchy used in combination with ARM GIC and irq_chip_retrigger_hierarchy()
is used by child IRQ domain.

Issue 2 - patch 2(new): IRQ wakeup configuration is not propagated properly
through IRQ domains hierarchy if irq_chip_set_wake_parent() is used and
parent IRQ domain don't implement .irq_set_wake(), but has 
IRQCHIP_SKIP_SET_WAKE flag set.

Issue 3 - patches 3-5: ARM GIC IRQ triggering type is not configured
properly when IRQ is routed through IRQ domain hierarchy and
system started using DT. As result, system will start using default
ARM GIC configuration, ignore DT IRQ triggering configuration,
and value of desc->irq_data.state_use_accessors = 0.

/ # cat /proc/interrupts -- before
230:          0          0      CBAR  30 Edge    -00000000  48051000.gpio
263:          0          0      CBAR 116 Edge    -00000000  48053000.gpio
296:        246          0      CBAR  67 Edge    -00000000  OMAP UART0
311:        341          0      CBAR  51 Edge    -00000000  48070000.i2c
312:          3          0      CBAR  52 Edge    -00000000  48072000.i2c
313:          0          0      CBAR  56 Edge    -00000000  48060000.i2c
314:        255          0      CBAR  78 Edge    -00000000  mmc0
315:         40          0      CBAR  81 Edge    -00000000  mmc1
316:         13          0      CBAR  91 Edge    -00000000  mmc2
320:          0          0      CBAR  49 Edge    -00000000  4a140000.sata
324:          0          0      CBAR   1 Edge    -00000000  48078000.elm
325:          0          0      CBAR  15 Edge    -00000000  gpmc
333:          0          0      CBAR 335 Edge    -00000000  48484000.ethernet
334:          0          0      CBAR 336 Edge    -00000000  48484000.ethernet
415:          0          0   pcf857x   2 Edge    -00000000  btnUser1
416:          0          0   pcf857x   3 Edge    -00000000  btnUser2
445:          0          0   pinctrl 992 Edge    -00010000  4806a000.seria

/ # cat /proc/interrupts -- after
230:          0          0      CBAR  30 Level   -00002004  48051000.gpio
263:          0          0      CBAR 116 Level   -00002004  48053000.gpio
296:        567          0      CBAR  67 Level   -00002004  OMAP UART0
311:        363          0      CBAR  51 Level   -00002004  48070000.i2c
312:          3          0      CBAR  52 Level   -00002004  48072000.i2c
313:          0          0      CBAR  56 Level   -00002004  48060000.i2c
314:        197          0      CBAR  78 Level   -00002004  mmc0
315:         40          0      CBAR  81 Level   -00002004  mmc1
316:         13          0      CBAR  91 Level   -00002004  mmc2
320:          0          0      CBAR  49 Level   -00002004  4a140000.sata
324:          0          0      CBAR   1 Level   -00002004  48078000.elm
325:          0          0      CBAR  15 Level   -00002004  gpmc
333:          0          0      CBAR 335 Level   -00002004  48484000.ethernet
334:          0          0      CBAR 336 Level   -00002004  48484000.ethernet
415:          0          0   pcf857x   2 Edge    -00000000  btnUser1
416:          4          2   pcf857x   3 Edge    -00000000  btnUser2
445:          0          0   pinctrl 992 Edge    -00010000  4806a000.serial

-xxxxxxxx are current values of desc->irq_data.state_use_accessors

Issue 4 - patch 5(new): ARM GIC IRQs are not masked during suspend even
if they are not wakeup source. 
	This issue related to IRQCHIP MASK ON SUSPEND flag usage and,
honestly, it's not clear how this flag has to be processed in
case of IRQ domain hierarchy - previously there was one IRQ chip per IRQ
desc and if it has IRQCHIP MASK ON SUSPEND flag set we had to mask IRQ.
But now there are up to three (DRA7) IRQ chip per IRQ desc, and all of them
could have different IRQCHIP MASK ON SUSPEND flag's configuration.
	Here, to fix an issue, I've just added IRQCHIP MASK ON SUSPEND flag
to TI Crossbar which is the last IRQ domain in hierarchy.

IRQ domain hierarchy code was merged in 4.1, so it seems these patches
are material for stable 4.1+.

Tested on:
 TI OMAP DRA7, dra7-evm

Series based on:
 Linux 4.2-rc6

Changes in v2:
- initial patch has been split on three;
- added three more patechs.

Link on v1:
 https://lkml.org/lkml/2015/8/11/179

Grygorii Strashko (6):
  genirq: fix irq_chip_retrigger_hierarchy
  genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
  genirq: introduce irq_chip_set_type_parent() helper
  irqchip: crossbar: fix arm gic irq type configuration
  ARM: OMAP: wakeupgen: fix arm gic irq type configuration
  irqchip: crossbar: fix irq masking at suspend

 arch/arm/mach-omap2/omap-wakeupgen.c |  1 +
 drivers/irqchip/irq-crossbar.c       |  2 ++
 include/linux/irq.h                  |  1 +
 kernel/irq/chip.c                    | 26 +++++++++++++++++++++++++-
 4 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.5.0


^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 0/6] genirq: irqdomain_hierarchy: fixes
@ 2015-08-12 17:45 ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:45 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel,
	linux-kernel, Grygorii Strashko

Hi All,

I've had able to identify and reproduce four issues related to switching on
using IRQ domain hierarchy on TI OMAP DRA7 (dra7-evm). Most of them were
discovered during testing of Suspend to RAM and IRQ wakeup functionality.
In my opinion, most of these issue could also affect on other ARM SoC if
they are using IRQ domain hierarchy.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
 ARM GIC <- OMAP wakeupgen <- TI CBAR

Issue 1 - patch 1(new): IRQ re-triggering is not working if IRQ domain
hierarchy used in combination with ARM GIC and irq_chip_retrigger_hierarchy()
is used by child IRQ domain.

Issue 2 - patch 2(new): IRQ wakeup configuration is not propagated properly
through IRQ domains hierarchy if irq_chip_set_wake_parent() is used and
parent IRQ domain don't implement .irq_set_wake(), but has 
IRQCHIP_SKIP_SET_WAKE flag set.

Issue 3 - patches 3-5: ARM GIC IRQ triggering type is not configured
properly when IRQ is routed through IRQ domain hierarchy and
system started using DT. As result, system will start using default
ARM GIC configuration, ignore DT IRQ triggering configuration,
and value of desc->irq_data.state_use_accessors = 0.

/ # cat /proc/interrupts -- before
230:          0          0      CBAR  30 Edge    -00000000  48051000.gpio
263:          0          0      CBAR 116 Edge    -00000000  48053000.gpio
296:        246          0      CBAR  67 Edge    -00000000  OMAP UART0
311:        341          0      CBAR  51 Edge    -00000000  48070000.i2c
312:          3          0      CBAR  52 Edge    -00000000  48072000.i2c
313:          0          0      CBAR  56 Edge    -00000000  48060000.i2c
314:        255          0      CBAR  78 Edge    -00000000  mmc0
315:         40          0      CBAR  81 Edge    -00000000  mmc1
316:         13          0      CBAR  91 Edge    -00000000  mmc2
320:          0          0      CBAR  49 Edge    -00000000  4a140000.sata
324:          0          0      CBAR   1 Edge    -00000000  48078000.elm
325:          0          0      CBAR  15 Edge    -00000000  gpmc
333:          0          0      CBAR 335 Edge    -00000000  48484000.ethernet
334:          0          0      CBAR 336 Edge    -00000000  48484000.ethernet
415:          0          0   pcf857x   2 Edge    -00000000  btnUser1
416:          0          0   pcf857x   3 Edge    -00000000  btnUser2
445:          0          0   pinctrl 992 Edge    -00010000  4806a000.seria

/ # cat /proc/interrupts -- after
230:          0          0      CBAR  30 Level   -00002004  48051000.gpio
263:          0          0      CBAR 116 Level   -00002004  48053000.gpio
296:        567          0      CBAR  67 Level   -00002004  OMAP UART0
311:        363          0      CBAR  51 Level   -00002004  48070000.i2c
312:          3          0      CBAR  52 Level   -00002004  48072000.i2c
313:          0          0      CBAR  56 Level   -00002004  48060000.i2c
314:        197          0      CBAR  78 Level   -00002004  mmc0
315:         40          0      CBAR  81 Level   -00002004  mmc1
316:         13          0      CBAR  91 Level   -00002004  mmc2
320:          0          0      CBAR  49 Level   -00002004  4a140000.sata
324:          0          0      CBAR   1 Level   -00002004  48078000.elm
325:          0          0      CBAR  15 Level   -00002004  gpmc
333:          0          0      CBAR 335 Level   -00002004  48484000.ethernet
334:          0          0      CBAR 336 Level   -00002004  48484000.ethernet
415:          0          0   pcf857x   2 Edge    -00000000  btnUser1
416:          4          2   pcf857x   3 Edge    -00000000  btnUser2
445:          0          0   pinctrl 992 Edge    -00010000  4806a000.serial

-xxxxxxxx are current values of desc->irq_data.state_use_accessors

Issue 4 - patch 5(new): ARM GIC IRQs are not masked during suspend even
if they are not wakeup source. 
	This issue related to IRQCHIP MASK ON SUSPEND flag usage and,
honestly, it's not clear how this flag has to be processed in
case of IRQ domain hierarchy - previously there was one IRQ chip per IRQ
desc and if it has IRQCHIP MASK ON SUSPEND flag set we had to mask IRQ.
But now there are up to three (DRA7) IRQ chip per IRQ desc, and all of them
could have different IRQCHIP MASK ON SUSPEND flag's configuration.
	Here, to fix an issue, I've just added IRQCHIP MASK ON SUSPEND flag
to TI Crossbar which is the last IRQ domain in hierarchy.

IRQ domain hierarchy code was merged in 4.1, so it seems these patches
are material for stable 4.1+.

Tested on:
 TI OMAP DRA7, dra7-evm

Series based on:
 Linux 4.2-rc6

Changes in v2:
- initial patch has been split on three;
- added three more patechs.

Link on v1:
 https://lkml.org/lkml/2015/8/11/179

Grygorii Strashko (6):
  genirq: fix irq_chip_retrigger_hierarchy
  genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
  genirq: introduce irq_chip_set_type_parent() helper
  irqchip: crossbar: fix arm gic irq type configuration
  ARM: OMAP: wakeupgen: fix arm gic irq type configuration
  irqchip: crossbar: fix irq masking at suspend

 arch/arm/mach-omap2/omap-wakeupgen.c |  1 +
 drivers/irqchip/irq-crossbar.c       |  2 ++
 include/linux/irq.h                  |  1 +
 kernel/irq/chip.c                    | 26 +++++++++++++++++++++++++-
 4 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 0/6] genirq: irqdomain_hierarchy: fixes
@ 2015-08-12 17:45 ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:45 UTC (permalink / raw)
  To: linux-arm-kernel

Hi All,

I've had able to identify and reproduce four issues related to switching on
using IRQ domain hierarchy on TI OMAP DRA7 (dra7-evm). Most of them were
discovered during testing of Suspend to RAM and IRQ wakeup functionality.
In my opinion, most of these issue could also affect on other ARM SoC if
they are using IRQ domain hierarchy.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
 ARM GIC <- OMAP wakeupgen <- TI CBAR

Issue 1 - patch 1(new): IRQ re-triggering is not working if IRQ domain
hierarchy used in combination with ARM GIC and irq_chip_retrigger_hierarchy()
is used by child IRQ domain.

Issue 2 - patch 2(new): IRQ wakeup configuration is not propagated properly
through IRQ domains hierarchy if irq_chip_set_wake_parent() is used and
parent IRQ domain don't implement .irq_set_wake(), but has 
IRQCHIP_SKIP_SET_WAKE flag set.

Issue 3 - patches 3-5: ARM GIC IRQ triggering type is not configured
properly when IRQ is routed through IRQ domain hierarchy and
system started using DT. As result, system will start using default
ARM GIC configuration, ignore DT IRQ triggering configuration,
and value of desc->irq_data.state_use_accessors = 0.

/ # cat /proc/interrupts -- before
230:          0          0      CBAR  30 Edge    -00000000  48051000.gpio
263:          0          0      CBAR 116 Edge    -00000000  48053000.gpio
296:        246          0      CBAR  67 Edge    -00000000  OMAP UART0
311:        341          0      CBAR  51 Edge    -00000000  48070000.i2c
312:          3          0      CBAR  52 Edge    -00000000  48072000.i2c
313:          0          0      CBAR  56 Edge    -00000000  48060000.i2c
314:        255          0      CBAR  78 Edge    -00000000  mmc0
315:         40          0      CBAR  81 Edge    -00000000  mmc1
316:         13          0      CBAR  91 Edge    -00000000  mmc2
320:          0          0      CBAR  49 Edge    -00000000  4a140000.sata
324:          0          0      CBAR   1 Edge    -00000000  48078000.elm
325:          0          0      CBAR  15 Edge    -00000000  gpmc
333:          0          0      CBAR 335 Edge    -00000000  48484000.ethernet
334:          0          0      CBAR 336 Edge    -00000000  48484000.ethernet
415:          0          0   pcf857x   2 Edge    -00000000  btnUser1
416:          0          0   pcf857x   3 Edge    -00000000  btnUser2
445:          0          0   pinctrl 992 Edge    -00010000  4806a000.seria

/ # cat /proc/interrupts -- after
230:          0          0      CBAR  30 Level   -00002004  48051000.gpio
263:          0          0      CBAR 116 Level   -00002004  48053000.gpio
296:        567          0      CBAR  67 Level   -00002004  OMAP UART0
311:        363          0      CBAR  51 Level   -00002004  48070000.i2c
312:          3          0      CBAR  52 Level   -00002004  48072000.i2c
313:          0          0      CBAR  56 Level   -00002004  48060000.i2c
314:        197          0      CBAR  78 Level   -00002004  mmc0
315:         40          0      CBAR  81 Level   -00002004  mmc1
316:         13          0      CBAR  91 Level   -00002004  mmc2
320:          0          0      CBAR  49 Level   -00002004  4a140000.sata
324:          0          0      CBAR   1 Level   -00002004  48078000.elm
325:          0          0      CBAR  15 Level   -00002004  gpmc
333:          0          0      CBAR 335 Level   -00002004  48484000.ethernet
334:          0          0      CBAR 336 Level   -00002004  48484000.ethernet
415:          0          0   pcf857x   2 Edge    -00000000  btnUser1
416:          4          2   pcf857x   3 Edge    -00000000  btnUser2
445:          0          0   pinctrl 992 Edge    -00010000  4806a000.serial

-xxxxxxxx are current values of desc->irq_data.state_use_accessors

Issue 4 - patch 5(new): ARM GIC IRQs are not masked during suspend even
if they are not wakeup source. 
	This issue related to IRQCHIP MASK ON SUSPEND flag usage and,
honestly, it's not clear how this flag has to be processed in
case of IRQ domain hierarchy - previously there was one IRQ chip per IRQ
desc and if it has IRQCHIP MASK ON SUSPEND flag set we had to mask IRQ.
But now there are up to three (DRA7) IRQ chip per IRQ desc, and all of them
could have different IRQCHIP MASK ON SUSPEND flag's configuration.
	Here, to fix an issue, I've just added IRQCHIP MASK ON SUSPEND flag
to TI Crossbar which is the last IRQ domain in hierarchy.

IRQ domain hierarchy code was merged in 4.1, so it seems these patches
are material for stable 4.1+.

Tested on:
 TI OMAP DRA7, dra7-evm

Series based on:
 Linux 4.2-rc6

Changes in v2:
- initial patch has been split on three;
- added three more patechs.

Link on v1:
 https://lkml.org/lkml/2015/8/11/179

Grygorii Strashko (6):
  genirq: fix irq_chip_retrigger_hierarchy
  genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
  genirq: introduce irq_chip_set_type_parent() helper
  irqchip: crossbar: fix arm gic irq type configuration
  ARM: OMAP: wakeupgen: fix arm gic irq type configuration
  irqchip: crossbar: fix irq masking at suspend

 arch/arm/mach-omap2/omap-wakeupgen.c |  1 +
 drivers/irqchip/irq-crossbar.c       |  2 ++
 include/linux/irq.h                  |  1 +
 kernel/irq/chip.c                    | 26 +++++++++++++++++++++++++-
 4 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 1/6] genirq: fix irq_chip_retrigger_hierarchy
  2015-08-12 17:45 ` Grygorii Strashko
  (?)
@ 2015-08-12 17:45   ` Grygorii Strashko
  -1 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:45 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel,
	linux-kernel, Grygorii Strashko

Now irq_chip_retrigger_hierarchy() returns -ENOSYS if it
was not able to find at least one .irq_retrigger() callback
implemented in IRQ domain hierarchy. As result, IRQ
re-triggering is not working now on ARM (TI OMAP) where
ARM GIC is not implemented this callback.
The .irq_retrigger() is optional (see check_irq_resend())
and there are no reasons to fail if it was not found, hence
lets return 0 in this case.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
ARM GIC <- OMAP wakeupgen <- TI CBAR

Failure is reproduced during resume from suspend to RAM:
- wakeup by IRQx
- suspend_enter
  + arch_suspend_enable_irqs
    + handle_fasteoi_irq
      + irq_may_run
        + irq_pm_check_wakeup
          + irq_disable(IRQx)
  + dpm_resume_noirq()
    + resume_device_irqs
      + resume_irqs
        + resume_irq
          + __enable_irq <== IRQx is not re-triggered

Fixes: 85f08c17de26 ('genirq: Introduce helper functions...')
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 kernel/irq/chip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 27f4332..6de638b 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -997,7 +997,7 @@ int irq_chip_retrigger_hierarchy(struct irq_data *data)
 		if (data->chip && data->chip->irq_retrigger)
 			return data->chip->irq_retrigger(data);
 
-	return -ENOSYS;
+	return 0;
 }
 
 /**
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 1/6] genirq: fix irq_chip_retrigger_hierarchy
@ 2015-08-12 17:45   ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:45 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel,
	linux-kernel, Grygorii Strashko

Now irq_chip_retrigger_hierarchy() returns -ENOSYS if it
was not able to find at least one .irq_retrigger() callback
implemented in IRQ domain hierarchy. As result, IRQ
re-triggering is not working now on ARM (TI OMAP) where
ARM GIC is not implemented this callback.
The .irq_retrigger() is optional (see check_irq_resend())
and there are no reasons to fail if it was not found, hence
lets return 0 in this case.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
ARM GIC <- OMAP wakeupgen <- TI CBAR

Failure is reproduced during resume from suspend to RAM:
- wakeup by IRQx
- suspend_enter
  + arch_suspend_enable_irqs
    + handle_fasteoi_irq
      + irq_may_run
        + irq_pm_check_wakeup
          + irq_disable(IRQx)
  + dpm_resume_noirq()
    + resume_device_irqs
      + resume_irqs
        + resume_irq
          + __enable_irq <== IRQx is not re-triggered

Fixes: 85f08c17de26 ('genirq: Introduce helper functions...')
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 kernel/irq/chip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 27f4332..6de638b 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -997,7 +997,7 @@ int irq_chip_retrigger_hierarchy(struct irq_data *data)
 		if (data->chip && data->chip->irq_retrigger)
 			return data->chip->irq_retrigger(data);
 
-	return -ENOSYS;
+	return 0;
 }
 
 /**
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 1/6] genirq: fix irq_chip_retrigger_hierarchy
@ 2015-08-12 17:45   ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:45 UTC (permalink / raw)
  To: linux-arm-kernel

Now irq_chip_retrigger_hierarchy() returns -ENOSYS if it
was not able to find at least one .irq_retrigger() callback
implemented in IRQ domain hierarchy. As result, IRQ
re-triggering is not working now on ARM (TI OMAP) where
ARM GIC is not implemented this callback.
The .irq_retrigger() is optional (see check_irq_resend())
and there are no reasons to fail if it was not found, hence
lets return 0 in this case.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
ARM GIC <- OMAP wakeupgen <- TI CBAR

Failure is reproduced during resume from suspend to RAM:
- wakeup by IRQx
- suspend_enter
  + arch_suspend_enable_irqs
    + handle_fasteoi_irq
      + irq_may_run
        + irq_pm_check_wakeup
          + irq_disable(IRQx)
  + dpm_resume_noirq()
    + resume_device_irqs
      + resume_irqs
        + resume_irq
          + __enable_irq <== IRQx is not re-triggered

Fixes: 85f08c17de26 ('genirq: Introduce helper functions...')
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 kernel/irq/chip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 27f4332..6de638b 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -997,7 +997,7 @@ int irq_chip_retrigger_hierarchy(struct irq_data *data)
 		if (data->chip && data->chip->irq_retrigger)
 			return data->chip->irq_retrigger(data);
 
-	return -ENOSYS;
+	return 0;
 }
 
 /**
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
  2015-08-12 17:45 ` Grygorii Strashko
  (?)
@ 2015-08-12 17:45   ` Grygorii Strashko
  -1 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:45 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel,
	linux-kernel, Grygorii Strashko

The irqchip_set_wake_parent should not fail if IRQ chip
specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
configuration can't be propagated properly through IRQ
domains hierarchy.

In case of TI OMAP DRA7 the issue reproduced with following
configuration:
ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key

gpio_key is wakeup source

Failure is reproduced during suspend/resume to RAM:
suspend:
 - gpio_keys_suspend
   enable_irq_wake
     + pcf857x_irq_set_wake
       + omap_gpio_wake_enable
         + TI CBAR irq_chip_set_wake_parent
           + OMAP wakeupgen has no .irq_set_wake()
           and -ENOSYS will be returned

resume:
 - gpio_keys_resume
   + disable_irq_wake
     + irq_set_irq_wake
       + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);

Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 kernel/irq/chip.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 6de638b..bdb1b9d 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
 int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
 {
 	data = data->parent_data;
+
+	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
+		return 0;
+
 	if (data->chip->irq_set_wake)
 		return data->chip->irq_set_wake(data, on);
 
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-12 17:45   ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:45 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel,
	linux-kernel, Grygorii Strashko

The irqchip_set_wake_parent should not fail if IRQ chip
specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
configuration can't be propagated properly through IRQ
domains hierarchy.

In case of TI OMAP DRA7 the issue reproduced with following
configuration:
ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key

gpio_key is wakeup source

Failure is reproduced during suspend/resume to RAM:
suspend:
 - gpio_keys_suspend
   enable_irq_wake
     + pcf857x_irq_set_wake
       + omap_gpio_wake_enable
         + TI CBAR irq_chip_set_wake_parent
           + OMAP wakeupgen has no .irq_set_wake()
           and -ENOSYS will be returned

resume:
 - gpio_keys_resume
   + disable_irq_wake
     + irq_set_irq_wake
       + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);

Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 kernel/irq/chip.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 6de638b..bdb1b9d 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
 int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
 {
 	data = data->parent_data;
+
+	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
+		return 0;
+
 	if (data->chip->irq_set_wake)
 		return data->chip->irq_set_wake(data, on);
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-12 17:45   ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:45 UTC (permalink / raw)
  To: linux-arm-kernel

The irqchip_set_wake_parent should not fail if IRQ chip
specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
configuration can't be propagated properly through IRQ
domains hierarchy.

In case of TI OMAP DRA7 the issue reproduced with following
configuration:
ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key

gpio_key is wakeup source

Failure is reproduced during suspend/resume to RAM:
suspend:
 - gpio_keys_suspend
   enable_irq_wake
     + pcf857x_irq_set_wake
       + omap_gpio_wake_enable
         + TI CBAR irq_chip_set_wake_parent
           + OMAP wakeupgen has no .irq_set_wake()
           and -ENOSYS will be returned

resume:
 - gpio_keys_resume
   + disable_irq_wake
     + irq_set_irq_wake
       + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);

Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 kernel/irq/chip.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 6de638b..bdb1b9d 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
 int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
 {
 	data = data->parent_data;
+
+	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
+		return 0;
+
 	if (data->chip->irq_set_wake)
 		return data->chip->irq_set_wake(data, on);
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 3/6] genirq: introduce irq_chip_set_type_parent() helper
  2015-08-12 17:45 ` Grygorii Strashko
  (?)
@ 2015-08-12 17:45   ` Grygorii Strashko
  -1 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:45 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel,
	linux-kernel, Grygorii Strashko

It's expected to use this helper when the current
domain doesn't implement .irq_set_type(),  but expect
the parent to do so.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 include/linux/irq.h |  1 +
 kernel/irq/chip.c   | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 92188b0..51744bc 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -484,6 +484,7 @@ extern int irq_chip_set_affinity_parent(struct irq_data *data,
 extern int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on);
 extern int irq_chip_set_vcpu_affinity_parent(struct irq_data *data,
 					     void *vcpu_info);
+extern int irq_chip_set_type_parent(struct irq_data *data, unsigned int type);
 #endif
 
 /* Handling of unhandled and spurious interrupts: */
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index bdb1b9d..b48938b 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -985,6 +985,26 @@ int irq_chip_set_affinity_parent(struct irq_data *data,
 }
 
 /**
+ * irq_chip_set_type_parent - Set IRQ type on the parent interrupt
+ * @data:	Pointer to interrupt specific data
+ * @type:	IRQ_TYPE_{LEVEL,EDGE}_* value - see include/linux/irq.h
+ *
+ * Conditional, as the underlying parent chip might not implement it.
+ */
+int irq_chip_set_type_parent(struct irq_data *data, unsigned int type)
+{
+	data = data->parent_data;
+
+	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
+		return 0;
+
+	if (data->chip->irq_set_type)
+		return data->chip->irq_set_type(data, type);
+
+	return -ENOSYS;
+}
+
+/**
  * irq_chip_retrigger_hierarchy - Retrigger an interrupt in hardware
  * @data:	Pointer to interrupt specific data
  *
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 3/6] genirq: introduce irq_chip_set_type_parent() helper
@ 2015-08-12 17:45   ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:45 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: Grygorii Strashko, linux, jason, nsekhar, linux-kernel, balbi,
	linux-omap, linux-arm-kernel

It's expected to use this helper when the current
domain doesn't implement .irq_set_type(),  but expect
the parent to do so.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 include/linux/irq.h |  1 +
 kernel/irq/chip.c   | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 92188b0..51744bc 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -484,6 +484,7 @@ extern int irq_chip_set_affinity_parent(struct irq_data *data,
 extern int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on);
 extern int irq_chip_set_vcpu_affinity_parent(struct irq_data *data,
 					     void *vcpu_info);
+extern int irq_chip_set_type_parent(struct irq_data *data, unsigned int type);
 #endif
 
 /* Handling of unhandled and spurious interrupts: */
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index bdb1b9d..b48938b 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -985,6 +985,26 @@ int irq_chip_set_affinity_parent(struct irq_data *data,
 }
 
 /**
+ * irq_chip_set_type_parent - Set IRQ type on the parent interrupt
+ * @data:	Pointer to interrupt specific data
+ * @type:	IRQ_TYPE_{LEVEL,EDGE}_* value - see include/linux/irq.h
+ *
+ * Conditional, as the underlying parent chip might not implement it.
+ */
+int irq_chip_set_type_parent(struct irq_data *data, unsigned int type)
+{
+	data = data->parent_data;
+
+	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
+		return 0;
+
+	if (data->chip->irq_set_type)
+		return data->chip->irq_set_type(data, type);
+
+	return -ENOSYS;
+}
+
+/**
  * irq_chip_retrigger_hierarchy - Retrigger an interrupt in hardware
  * @data:	Pointer to interrupt specific data
  *
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 3/6] genirq: introduce irq_chip_set_type_parent() helper
@ 2015-08-12 17:45   ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:45 UTC (permalink / raw)
  To: linux-arm-kernel

It's expected to use this helper when the current
domain doesn't implement .irq_set_type(),  but expect
the parent to do so.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 include/linux/irq.h |  1 +
 kernel/irq/chip.c   | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 92188b0..51744bc 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -484,6 +484,7 @@ extern int irq_chip_set_affinity_parent(struct irq_data *data,
 extern int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on);
 extern int irq_chip_set_vcpu_affinity_parent(struct irq_data *data,
 					     void *vcpu_info);
+extern int irq_chip_set_type_parent(struct irq_data *data, unsigned int type);
 #endif
 
 /* Handling of unhandled and spurious interrupts: */
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index bdb1b9d..b48938b 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -985,6 +985,26 @@ int irq_chip_set_affinity_parent(struct irq_data *data,
 }
 
 /**
+ * irq_chip_set_type_parent - Set IRQ type on the parent interrupt
+ * @data:	Pointer to interrupt specific data
+ * @type:	IRQ_TYPE_{LEVEL,EDGE}_* value - see include/linux/irq.h
+ *
+ * Conditional, as the underlying parent chip might not implement it.
+ */
+int irq_chip_set_type_parent(struct irq_data *data, unsigned int type)
+{
+	data = data->parent_data;
+
+	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
+		return 0;
+
+	if (data->chip->irq_set_type)
+		return data->chip->irq_set_type(data, type);
+
+	return -ENOSYS;
+}
+
+/**
  * irq_chip_retrigger_hierarchy - Retrigger an interrupt in hardware
  * @data:	Pointer to interrupt specific data
  *
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 4/6] irqchip: crossbar: fix arm gic irq type configuration
  2015-08-12 17:45 ` Grygorii Strashko
  (?)
@ 2015-08-12 17:46   ` Grygorii Strashko
  -1 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:46 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel,
	linux-kernel, Grygorii Strashko

It's observed that ARM GIC IRQ triggering type is not configured
properly when IRQ is routed through IRQ domain hierarchy and
system started using DT. As result, system will start using default
ARM GIC configuration, ignore DT IRQ triggering configuration,
and value of desc->irq_data.state_use_accessors = 0.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
ARM GIC <- OMAP wakeupgen <- TI CBAR

Failed call chain:
     irq_create_of_mapping
     irq_set_irq_type
     __irq_set_trigger
     if (!chip || !chip->irq_set_type) {
        return 0; <- return here
     }
Crossbar has no .irq_set_type() defined and, so, IRQ triggering
configuration will not be propagated to parent IRQ domain.

Hence, fix it by using irq_chip_set_type_parent() for
propagation IRQ triggering type to parent IRQ domains.

Fixes: 783d31863fb8 ('irqchip: crossbar: Convert dra7 crossbar...')
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/irqchip/irq-crossbar.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 692fe2b..3ba58e7 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -69,6 +69,7 @@ static struct irq_chip crossbar_chip = {
 	.irq_unmask		= irq_chip_unmask_parent,
 	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.irq_set_wake		= irq_chip_set_wake_parent,
+	.irq_set_type		= irq_chip_set_type_parent,
 #ifdef CONFIG_SMP
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
 #endif
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 4/6] irqchip: crossbar: fix arm gic irq type configuration
@ 2015-08-12 17:46   ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:46 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel,
	linux-kernel, Grygorii Strashko

It's observed that ARM GIC IRQ triggering type is not configured
properly when IRQ is routed through IRQ domain hierarchy and
system started using DT. As result, system will start using default
ARM GIC configuration, ignore DT IRQ triggering configuration,
and value of desc->irq_data.state_use_accessors = 0.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
ARM GIC <- OMAP wakeupgen <- TI CBAR

Failed call chain:
     irq_create_of_mapping
     irq_set_irq_type
     __irq_set_trigger
     if (!chip || !chip->irq_set_type) {
        return 0; <- return here
     }
Crossbar has no .irq_set_type() defined and, so, IRQ triggering
configuration will not be propagated to parent IRQ domain.

Hence, fix it by using irq_chip_set_type_parent() for
propagation IRQ triggering type to parent IRQ domains.

Fixes: 783d31863fb8 ('irqchip: crossbar: Convert dra7 crossbar...')
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/irqchip/irq-crossbar.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 692fe2b..3ba58e7 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -69,6 +69,7 @@ static struct irq_chip crossbar_chip = {
 	.irq_unmask		= irq_chip_unmask_parent,
 	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.irq_set_wake		= irq_chip_set_wake_parent,
+	.irq_set_type		= irq_chip_set_type_parent,
 #ifdef CONFIG_SMP
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
 #endif
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 4/6] irqchip: crossbar: fix arm gic irq type configuration
@ 2015-08-12 17:46   ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:46 UTC (permalink / raw)
  To: linux-arm-kernel

It's observed that ARM GIC IRQ triggering type is not configured
properly when IRQ is routed through IRQ domain hierarchy and
system started using DT. As result, system will start using default
ARM GIC configuration, ignore DT IRQ triggering configuration,
and value of desc->irq_data.state_use_accessors = 0.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
ARM GIC <- OMAP wakeupgen <- TI CBAR

Failed call chain:
     irq_create_of_mapping
     irq_set_irq_type
     __irq_set_trigger
     if (!chip || !chip->irq_set_type) {
        return 0; <- return here
     }
Crossbar has no .irq_set_type() defined and, so, IRQ triggering
configuration will not be propagated to parent IRQ domain.

Hence, fix it by using irq_chip_set_type_parent() for
propagation IRQ triggering type to parent IRQ domains.

Fixes: 783d31863fb8 ('irqchip: crossbar: Convert dra7 crossbar...')
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/irqchip/irq-crossbar.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 692fe2b..3ba58e7 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -69,6 +69,7 @@ static struct irq_chip crossbar_chip = {
 	.irq_unmask		= irq_chip_unmask_parent,
 	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.irq_set_wake		= irq_chip_set_wake_parent,
+	.irq_set_type		= irq_chip_set_type_parent,
 #ifdef CONFIG_SMP
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
 #endif
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 5/6] ARM: OMAP: wakeupgen: fix arm gic irq type configuration
  2015-08-12 17:45 ` Grygorii Strashko
  (?)
@ 2015-08-12 17:46   ` Grygorii Strashko
  -1 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:46 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel,
	linux-kernel, Grygorii Strashko

It's observed that ARM GIC IRQ triggering type is not configured
properly when IRQ is routed through IRQ domain hierarchy and
system started using DT. As result, system will start using default
ARM GIC configuration, ignore DT IRQ triggering configuration,
and value of desc->irq_data.state_use_accessors = 0.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
ARM GIC <- OMAP wakeupgen <- TI CBAR

Failed call chain:
     irq_create_of_mapping
     irq_set_irq_type
     __irq_set_trigger
     if (!chip || !chip->irq_set_type) {
        return 0; <- return here
     }
OMAP wakeupgen has no .irq_set_type() defined and, so, IRQ triggering
configuration will not be propagated to parent IRQ domain.

Hence, fix it by using irq_chip_set_type_parent() for
propagation IRQ triggering type to parent IRQ domains.

Fixes: 7136d457f365 ('ARM: omap: convert wakeupgen to stacked domains')
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/mach-omap2/omap-wakeupgen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-omap2/omap-wakeupgen.c b/arch/arm/mach-omap2/omap-wakeupgen.c
index 8e52621..e1d2e99 100644
--- a/arch/arm/mach-omap2/omap-wakeupgen.c
+++ b/arch/arm/mach-omap2/omap-wakeupgen.c
@@ -392,6 +392,7 @@ static struct irq_chip wakeupgen_chip = {
 	.irq_mask		= wakeupgen_mask,
 	.irq_unmask		= wakeupgen_unmask,
 	.irq_retrigger		= irq_chip_retrigger_hierarchy,
+	.irq_set_type		= irq_chip_set_type_parent,
 	.flags			= IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND,
 #ifdef CONFIG_SMP
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 5/6] ARM: OMAP: wakeupgen: fix arm gic irq type configuration
@ 2015-08-12 17:46   ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:46 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: Grygorii Strashko, linux, jason, nsekhar, linux-kernel, balbi,
	linux-omap, linux-arm-kernel

It's observed that ARM GIC IRQ triggering type is not configured
properly when IRQ is routed through IRQ domain hierarchy and
system started using DT. As result, system will start using default
ARM GIC configuration, ignore DT IRQ triggering configuration,
and value of desc->irq_data.state_use_accessors = 0.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
ARM GIC <- OMAP wakeupgen <- TI CBAR

Failed call chain:
     irq_create_of_mapping
     irq_set_irq_type
     __irq_set_trigger
     if (!chip || !chip->irq_set_type) {
        return 0; <- return here
     }
OMAP wakeupgen has no .irq_set_type() defined and, so, IRQ triggering
configuration will not be propagated to parent IRQ domain.

Hence, fix it by using irq_chip_set_type_parent() for
propagation IRQ triggering type to parent IRQ domains.

Fixes: 7136d457f365 ('ARM: omap: convert wakeupgen to stacked domains')
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/mach-omap2/omap-wakeupgen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-omap2/omap-wakeupgen.c b/arch/arm/mach-omap2/omap-wakeupgen.c
index 8e52621..e1d2e99 100644
--- a/arch/arm/mach-omap2/omap-wakeupgen.c
+++ b/arch/arm/mach-omap2/omap-wakeupgen.c
@@ -392,6 +392,7 @@ static struct irq_chip wakeupgen_chip = {
 	.irq_mask		= wakeupgen_mask,
 	.irq_unmask		= wakeupgen_unmask,
 	.irq_retrigger		= irq_chip_retrigger_hierarchy,
+	.irq_set_type		= irq_chip_set_type_parent,
 	.flags			= IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND,
 #ifdef CONFIG_SMP
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 5/6] ARM: OMAP: wakeupgen: fix arm gic irq type configuration
@ 2015-08-12 17:46   ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:46 UTC (permalink / raw)
  To: linux-arm-kernel

It's observed that ARM GIC IRQ triggering type is not configured
properly when IRQ is routed through IRQ domain hierarchy and
system started using DT. As result, system will start using default
ARM GIC configuration, ignore DT IRQ triggering configuration,
and value of desc->irq_data.state_use_accessors = 0.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
ARM GIC <- OMAP wakeupgen <- TI CBAR

Failed call chain:
     irq_create_of_mapping
     irq_set_irq_type
     __irq_set_trigger
     if (!chip || !chip->irq_set_type) {
        return 0; <- return here
     }
OMAP wakeupgen has no .irq_set_type() defined and, so, IRQ triggering
configuration will not be propagated to parent IRQ domain.

Hence, fix it by using irq_chip_set_type_parent() for
propagation IRQ triggering type to parent IRQ domains.

Fixes: 7136d457f365 ('ARM: omap: convert wakeupgen to stacked domains')
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/mach-omap2/omap-wakeupgen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-omap2/omap-wakeupgen.c b/arch/arm/mach-omap2/omap-wakeupgen.c
index 8e52621..e1d2e99 100644
--- a/arch/arm/mach-omap2/omap-wakeupgen.c
+++ b/arch/arm/mach-omap2/omap-wakeupgen.c
@@ -392,6 +392,7 @@ static struct irq_chip wakeupgen_chip = {
 	.irq_mask		= wakeupgen_mask,
 	.irq_unmask		= wakeupgen_unmask,
 	.irq_retrigger		= irq_chip_retrigger_hierarchy,
+	.irq_set_type		= irq_chip_set_type_parent,
 	.flags			= IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND,
 #ifdef CONFIG_SMP
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 6/6] irqchip: crossbar: fix irq masking at suspend
  2015-08-12 17:45 ` Grygorii Strashko
  (?)
@ 2015-08-12 17:46   ` Grygorii Strashko
  -1 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:46 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel,
	linux-kernel, Grygorii Strashko

All ARM GIC IRQs have to masked during suspend if they are not
wakeup source. Now this is not happen, since switching to
use IRQ domain hierarchy, because suspend_device_irq() only checks flags
in the last IRQ chip in hierarchy for IRQCHIP_MASK_ON_SUSPEND
bit set. And in the case of TI OMAP DRA7 the last IRQ chip is TI Crossbar
which do not have this flag set.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
  ARM GIC <- OMAP wakeupgen <- TI CBAR
ARM GIC - IRQCHIP_MASK_ON_SUSPEND=n
OMAP wakeupgen - IRQCHIP_MASK_ON_SUSPEND=y
TI CBAR - IRQCHIP_MASK_ON_SUSPEND=n

Hence, fix by adding IRQCHIP_MASK_ON_SUSPEND for
TI Crossbar IRQ chip.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/irqchip/irq-crossbar.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 3ba58e7..f5a72cc 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -70,6 +70,7 @@ static struct irq_chip crossbar_chip = {
 	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.irq_set_wake		= irq_chip_set_wake_parent,
 	.irq_set_type		= irq_chip_set_type_parent,
+	.flags			= IRQCHIP_MASK_ON_SUSPEND,
 #ifdef CONFIG_SMP
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
 #endif
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 6/6] irqchip: crossbar: fix irq masking at suspend
@ 2015-08-12 17:46   ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:46 UTC (permalink / raw)
  To: tglx, tony, marc.zyngier
  Cc: Grygorii Strashko, linux, jason, nsekhar, linux-kernel, balbi,
	linux-omap, linux-arm-kernel

All ARM GIC IRQs have to masked during suspend if they are not
wakeup source. Now this is not happen, since switching to
use IRQ domain hierarchy, because suspend_device_irq() only checks flags
in the last IRQ chip in hierarchy for IRQCHIP_MASK_ON_SUSPEND
bit set. And in the case of TI OMAP DRA7 the last IRQ chip is TI Crossbar
which do not have this flag set.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
  ARM GIC <- OMAP wakeupgen <- TI CBAR
ARM GIC - IRQCHIP_MASK_ON_SUSPEND=n
OMAP wakeupgen - IRQCHIP_MASK_ON_SUSPEND=y
TI CBAR - IRQCHIP_MASK_ON_SUSPEND=n

Hence, fix by adding IRQCHIP_MASK_ON_SUSPEND for
TI Crossbar IRQ chip.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/irqchip/irq-crossbar.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 3ba58e7..f5a72cc 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -70,6 +70,7 @@ static struct irq_chip crossbar_chip = {
 	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.irq_set_wake		= irq_chip_set_wake_parent,
 	.irq_set_type		= irq_chip_set_type_parent,
+	.flags			= IRQCHIP_MASK_ON_SUSPEND,
 #ifdef CONFIG_SMP
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
 #endif
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 6/6] irqchip: crossbar: fix irq masking at suspend
@ 2015-08-12 17:46   ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-12 17:46 UTC (permalink / raw)
  To: linux-arm-kernel

All ARM GIC IRQs have to masked during suspend if they are not
wakeup source. Now this is not happen, since switching to
use IRQ domain hierarchy, because suspend_device_irq() only checks flags
in the last IRQ chip in hierarchy for IRQCHIP_MASK_ON_SUSPEND
bit set. And in the case of TI OMAP DRA7 the last IRQ chip is TI Crossbar
which do not have this flag set.

In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
  ARM GIC <- OMAP wakeupgen <- TI CBAR
ARM GIC - IRQCHIP_MASK_ON_SUSPEND=n
OMAP wakeupgen - IRQCHIP_MASK_ON_SUSPEND=y
TI CBAR - IRQCHIP_MASK_ON_SUSPEND=n

Hence, fix by adding IRQCHIP_MASK_ON_SUSPEND for
TI Crossbar IRQ chip.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/irqchip/irq-crossbar.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c
index 3ba58e7..f5a72cc 100644
--- a/drivers/irqchip/irq-crossbar.c
+++ b/drivers/irqchip/irq-crossbar.c
@@ -70,6 +70,7 @@ static struct irq_chip crossbar_chip = {
 	.irq_retrigger		= irq_chip_retrigger_hierarchy,
 	.irq_set_wake		= irq_chip_set_wake_parent,
 	.irq_set_type		= irq_chip_set_type_parent,
+	.flags			= IRQCHIP_MASK_ON_SUSPEND,
 #ifdef CONFIG_SMP
 	.irq_set_affinity	= irq_chip_set_affinity_parent,
 #endif
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
  2015-08-12 17:45   ` Grygorii Strashko
  (?)
@ 2015-08-13  8:54     ` Sudeep Holla
  -1 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13  8:54 UTC (permalink / raw)
  To: Grygorii Strashko, Marc Zyngier
  Cc: tglx, tony, Sudeep Holla, linux, jason, nsekhar, linux-kernel,
	balbi, linux-omap, linux-arm-kernel



On 12/08/15 18:45, Grygorii Strashko wrote:
> The irqchip_set_wake_parent should not fail if IRQ chip
> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
> configuration can't be propagated properly through IRQ
> domains hierarchy.
>
> In case of TI OMAP DRA7 the issue reproduced with following
> configuration:
> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>
> gpio_key is wakeup source
>
> Failure is reproduced during suspend/resume to RAM:
> suspend:
>   - gpio_keys_suspend
>     enable_irq_wake
>       + pcf857x_irq_set_wake
>         + omap_gpio_wake_enable
>           + TI CBAR irq_chip_set_wake_parent
>             + OMAP wakeupgen has no .irq_set_wake()
>             and -ENOSYS will be returned
>
> resume:
>   - gpio_keys_resume
>     + disable_irq_wake
>       + irq_set_irq_wake
>         + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>
> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>   kernel/irq/chip.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 6de638b..bdb1b9d 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
>   int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>   {
>   	data = data->parent_data;
> +
> +	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
> +		return 0;
> +

[Nit] I think the irq core can access data->chip directly. Either way,
it's better to be consistent, the statement following doesn't use helper
function.

Otherwise looks good to me.

Regards,
Sudeep

>   	if (data->chip->irq_set_wake)
>   		return data->chip->irq_set_wake(data, on);
>
>

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-13  8:54     ` Sudeep Holla
  0 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13  8:54 UTC (permalink / raw)
  To: Grygorii Strashko, Marc Zyngier
  Cc: tglx, tony, Sudeep Holla, linux, jason, nsekhar, linux-kernel,
	balbi, linux-omap, linux-arm-kernel



On 12/08/15 18:45, Grygorii Strashko wrote:
> The irqchip_set_wake_parent should not fail if IRQ chip
> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
> configuration can't be propagated properly through IRQ
> domains hierarchy.
>
> In case of TI OMAP DRA7 the issue reproduced with following
> configuration:
> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>
> gpio_key is wakeup source
>
> Failure is reproduced during suspend/resume to RAM:
> suspend:
>   - gpio_keys_suspend
>     enable_irq_wake
>       + pcf857x_irq_set_wake
>         + omap_gpio_wake_enable
>           + TI CBAR irq_chip_set_wake_parent
>             + OMAP wakeupgen has no .irq_set_wake()
>             and -ENOSYS will be returned
>
> resume:
>   - gpio_keys_resume
>     + disable_irq_wake
>       + irq_set_irq_wake
>         + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>
> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>   kernel/irq/chip.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 6de638b..bdb1b9d 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
>   int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>   {
>   	data = data->parent_data;
> +
> +	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
> +		return 0;
> +

[Nit] I think the irq core can access data->chip directly. Either way,
it's better to be consistent, the statement following doesn't use helper
function.

Otherwise looks good to me.

Regards,
Sudeep

>   	if (data->chip->irq_set_wake)
>   		return data->chip->irq_set_wake(data, on);
>
>

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-13  8:54     ` Sudeep Holla
  0 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13  8:54 UTC (permalink / raw)
  To: linux-arm-kernel



On 12/08/15 18:45, Grygorii Strashko wrote:
> The irqchip_set_wake_parent should not fail if IRQ chip
> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
> configuration can't be propagated properly through IRQ
> domains hierarchy.
>
> In case of TI OMAP DRA7 the issue reproduced with following
> configuration:
> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>
> gpio_key is wakeup source
>
> Failure is reproduced during suspend/resume to RAM:
> suspend:
>   - gpio_keys_suspend
>     enable_irq_wake
>       + pcf857x_irq_set_wake
>         + omap_gpio_wake_enable
>           + TI CBAR irq_chip_set_wake_parent
>             + OMAP wakeupgen has no .irq_set_wake()
>             and -ENOSYS will be returned
>
> resume:
>   - gpio_keys_resume
>     + disable_irq_wake
>       + irq_set_irq_wake
>         + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>
> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>   kernel/irq/chip.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 6de638b..bdb1b9d 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
>   int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>   {
>   	data = data->parent_data;
> +
> +	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
> +		return 0;
> +

[Nit] I think the irq core can access data->chip directly. Either way,
it's better to be consistent, the statement following doesn't use helper
function.

Otherwise looks good to me.

Regards,
Sudeep

>   	if (data->chip->irq_set_wake)
>   		return data->chip->irq_set_wake(data, on);
>
>

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 3/6] genirq: introduce irq_chip_set_type_parent() helper
  2015-08-12 17:45   ` Grygorii Strashko
  (?)
@ 2015-08-13  8:58     ` Sudeep Holla
  -1 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13  8:58 UTC (permalink / raw)
  To: Grygorii Strashko, Marc Zyngier
  Cc: tglx, tony, Sudeep Holla, linux, jason, nsekhar, linux-kernel,
	balbi, linux-omap, linux-arm-kernel



On 12/08/15 18:45, Grygorii Strashko wrote:
> It's expected to use this helper when the current
> domain doesn't implement .irq_set_type(),  but expect
> the parent to do so.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>   include/linux/irq.h |  1 +
>   kernel/irq/chip.c   | 20 ++++++++++++++++++++
>   2 files changed, 21 insertions(+)
>
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 92188b0..51744bc 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -484,6 +484,7 @@ extern int irq_chip_set_affinity_parent(struct irq_data *data,
>   extern int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on);
>   extern int irq_chip_set_vcpu_affinity_parent(struct irq_data *data,
>   					     void *vcpu_info);
> +extern int irq_chip_set_type_parent(struct irq_data *data, unsigned int type);
>   #endif
>
>   /* Handling of unhandled and spurious interrupts: */
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index bdb1b9d..b48938b 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -985,6 +985,26 @@ int irq_chip_set_affinity_parent(struct irq_data *data,
>   }
>
>   /**
> + * irq_chip_set_type_parent - Set IRQ type on the parent interrupt
> + * @data:	Pointer to interrupt specific data
> + * @type:	IRQ_TYPE_{LEVEL,EDGE}_* value - see include/linux/irq.h
> + *
> + * Conditional, as the underlying parent chip might not implement it.
> + */
> +int irq_chip_set_type_parent(struct irq_data *data, unsigned int type)
> +{
> +	data = data->parent_data;
> +
> +	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
> +		return 0;
> +

I think this is unrelated here(perhaps copy-paste error ?). I fail to
see how wakeup and trigger type are related.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 3/6] genirq: introduce irq_chip_set_type_parent() helper
@ 2015-08-13  8:58     ` Sudeep Holla
  0 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13  8:58 UTC (permalink / raw)
  To: Grygorii Strashko, Marc Zyngier
  Cc: tglx, tony, Sudeep Holla, linux, jason, nsekhar, linux-kernel,
	balbi, linux-omap, linux-arm-kernel



On 12/08/15 18:45, Grygorii Strashko wrote:
> It's expected to use this helper when the current
> domain doesn't implement .irq_set_type(),  but expect
> the parent to do so.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>   include/linux/irq.h |  1 +
>   kernel/irq/chip.c   | 20 ++++++++++++++++++++
>   2 files changed, 21 insertions(+)
>
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 92188b0..51744bc 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -484,6 +484,7 @@ extern int irq_chip_set_affinity_parent(struct irq_data *data,
>   extern int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on);
>   extern int irq_chip_set_vcpu_affinity_parent(struct irq_data *data,
>   					     void *vcpu_info);
> +extern int irq_chip_set_type_parent(struct irq_data *data, unsigned int type);
>   #endif
>
>   /* Handling of unhandled and spurious interrupts: */
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index bdb1b9d..b48938b 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -985,6 +985,26 @@ int irq_chip_set_affinity_parent(struct irq_data *data,
>   }
>
>   /**
> + * irq_chip_set_type_parent - Set IRQ type on the parent interrupt
> + * @data:	Pointer to interrupt specific data
> + * @type:	IRQ_TYPE_{LEVEL,EDGE}_* value - see include/linux/irq.h
> + *
> + * Conditional, as the underlying parent chip might not implement it.
> + */
> +int irq_chip_set_type_parent(struct irq_data *data, unsigned int type)
> +{
> +	data = data->parent_data;
> +
> +	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
> +		return 0;
> +

I think this is unrelated here(perhaps copy-paste error ?). I fail to
see how wakeup and trigger type are related.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 3/6] genirq: introduce irq_chip_set_type_parent() helper
@ 2015-08-13  8:58     ` Sudeep Holla
  0 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13  8:58 UTC (permalink / raw)
  To: linux-arm-kernel



On 12/08/15 18:45, Grygorii Strashko wrote:
> It's expected to use this helper when the current
> domain doesn't implement .irq_set_type(),  but expect
> the parent to do so.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>   include/linux/irq.h |  1 +
>   kernel/irq/chip.c   | 20 ++++++++++++++++++++
>   2 files changed, 21 insertions(+)
>
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 92188b0..51744bc 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -484,6 +484,7 @@ extern int irq_chip_set_affinity_parent(struct irq_data *data,
>   extern int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on);
>   extern int irq_chip_set_vcpu_affinity_parent(struct irq_data *data,
>   					     void *vcpu_info);
> +extern int irq_chip_set_type_parent(struct irq_data *data, unsigned int type);
>   #endif
>
>   /* Handling of unhandled and spurious interrupts: */
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index bdb1b9d..b48938b 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -985,6 +985,26 @@ int irq_chip_set_affinity_parent(struct irq_data *data,
>   }
>
>   /**
> + * irq_chip_set_type_parent - Set IRQ type on the parent interrupt
> + * @data:	Pointer to interrupt specific data
> + * @type:	IRQ_TYPE_{LEVEL,EDGE}_* value - see include/linux/irq.h
> + *
> + * Conditional, as the underlying parent chip might not implement it.
> + */
> +int irq_chip_set_type_parent(struct irq_data *data, unsigned int type)
> +{
> +	data = data->parent_data;
> +
> +	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
> +		return 0;
> +

I think this is unrelated here(perhaps copy-paste error ?). I fail to
see how wakeup and trigger type are related.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 1/6] genirq: fix irq_chip_retrigger_hierarchy
  2015-08-12 17:45   ` Grygorii Strashko
  (?)
@ 2015-08-13  9:26     ` Marc Zyngier
  -1 siblings, 0 replies; 65+ messages in thread
From: Marc Zyngier @ 2015-08-13  9:26 UTC (permalink / raw)
  To: Grygorii Strashko, tglx, tony
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel,
	linux-kernel, Jiang Liu

[adding Jiang to the cc list]

On 12/08/15 18:45, Grygorii Strashko wrote:
> Now irq_chip_retrigger_hierarchy() returns -ENOSYS if it
> was not able to find at least one .irq_retrigger() callback
> implemented in IRQ domain hierarchy. As result, IRQ
> re-triggering is not working now on ARM (TI OMAP) where
> ARM GIC is not implemented this callback.
> The .irq_retrigger() is optional (see check_irq_resend())
> and there are no reasons to fail if it was not found, hence
> lets return 0 in this case.
> 
> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
> ARM GIC <- OMAP wakeupgen <- TI CBAR
> 
> Failure is reproduced during resume from suspend to RAM:
> - wakeup by IRQx
> - suspend_enter
>   + arch_suspend_enable_irqs
>     + handle_fasteoi_irq
>       + irq_may_run
>         + irq_pm_check_wakeup
>           + irq_disable(IRQx)
>   + dpm_resume_noirq()
>     + resume_device_irqs
>       + resume_irqs
>         + resume_irq
>           + __enable_irq <== IRQx is not re-triggered
> 
> Fixes: 85f08c17de26 ('genirq: Introduce helper functions...')
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  kernel/irq/chip.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 27f4332..6de638b 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -997,7 +997,7 @@ int irq_chip_retrigger_hierarchy(struct irq_data *data)
>  		if (data->chip && data->chip->irq_retrigger)
>  			return data->chip->irq_retrigger(data);
>  
> -	return -ENOSYS;
> +	return 0;
>  }
>  
>  /**
> 

I think this makes sense. Not having an irq_retrigger or having an
irq_retrigger that returns zero are the same thing.

Actually, we don't even distinguish between a retrigger that
successfully poked the HW, and a retrigger that returned an error. Both
are considered to not to require a SW retrigger... maybe we should fix
that too. Jiang, Thomas?

Anyway, for this patch:

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

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

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 1/6] genirq: fix irq_chip_retrigger_hierarchy
@ 2015-08-13  9:26     ` Marc Zyngier
  0 siblings, 0 replies; 65+ messages in thread
From: Marc Zyngier @ 2015-08-13  9:26 UTC (permalink / raw)
  To: Grygorii Strashko, tglx, tony
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel,
	linux-kernel, Jiang Liu

[adding Jiang to the cc list]

On 12/08/15 18:45, Grygorii Strashko wrote:
> Now irq_chip_retrigger_hierarchy() returns -ENOSYS if it
> was not able to find at least one .irq_retrigger() callback
> implemented in IRQ domain hierarchy. As result, IRQ
> re-triggering is not working now on ARM (TI OMAP) where
> ARM GIC is not implemented this callback.
> The .irq_retrigger() is optional (see check_irq_resend())
> and there are no reasons to fail if it was not found, hence
> lets return 0 in this case.
> 
> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
> ARM GIC <- OMAP wakeupgen <- TI CBAR
> 
> Failure is reproduced during resume from suspend to RAM:
> - wakeup by IRQx
> - suspend_enter
>   + arch_suspend_enable_irqs
>     + handle_fasteoi_irq
>       + irq_may_run
>         + irq_pm_check_wakeup
>           + irq_disable(IRQx)
>   + dpm_resume_noirq()
>     + resume_device_irqs
>       + resume_irqs
>         + resume_irq
>           + __enable_irq <== IRQx is not re-triggered
> 
> Fixes: 85f08c17de26 ('genirq: Introduce helper functions...')
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  kernel/irq/chip.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 27f4332..6de638b 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -997,7 +997,7 @@ int irq_chip_retrigger_hierarchy(struct irq_data *data)
>  		if (data->chip && data->chip->irq_retrigger)
>  			return data->chip->irq_retrigger(data);
>  
> -	return -ENOSYS;
> +	return 0;
>  }
>  
>  /**
> 

I think this makes sense. Not having an irq_retrigger or having an
irq_retrigger that returns zero are the same thing.

Actually, we don't even distinguish between a retrigger that
successfully poked the HW, and a retrigger that returned an error. Both
are considered to not to require a SW retrigger... maybe we should fix
that too. Jiang, Thomas?

Anyway, for this patch:

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

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

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 1/6] genirq: fix irq_chip_retrigger_hierarchy
@ 2015-08-13  9:26     ` Marc Zyngier
  0 siblings, 0 replies; 65+ messages in thread
From: Marc Zyngier @ 2015-08-13  9:26 UTC (permalink / raw)
  To: linux-arm-kernel

[adding Jiang to the cc list]

On 12/08/15 18:45, Grygorii Strashko wrote:
> Now irq_chip_retrigger_hierarchy() returns -ENOSYS if it
> was not able to find at least one .irq_retrigger() callback
> implemented in IRQ domain hierarchy. As result, IRQ
> re-triggering is not working now on ARM (TI OMAP) where
> ARM GIC is not implemented this callback.
> The .irq_retrigger() is optional (see check_irq_resend())
> and there are no reasons to fail if it was not found, hence
> lets return 0 in this case.
> 
> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
> ARM GIC <- OMAP wakeupgen <- TI CBAR
> 
> Failure is reproduced during resume from suspend to RAM:
> - wakeup by IRQx
> - suspend_enter
>   + arch_suspend_enable_irqs
>     + handle_fasteoi_irq
>       + irq_may_run
>         + irq_pm_check_wakeup
>           + irq_disable(IRQx)
>   + dpm_resume_noirq()
>     + resume_device_irqs
>       + resume_irqs
>         + resume_irq
>           + __enable_irq <== IRQx is not re-triggered
> 
> Fixes: 85f08c17de26 ('genirq: Introduce helper functions...')
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  kernel/irq/chip.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 27f4332..6de638b 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -997,7 +997,7 @@ int irq_chip_retrigger_hierarchy(struct irq_data *data)
>  		if (data->chip && data->chip->irq_retrigger)
>  			return data->chip->irq_retrigger(data);
>  
> -	return -ENOSYS;
> +	return 0;
>  }
>  
>  /**
> 

I think this makes sense. Not having an irq_retrigger or having an
irq_retrigger that returns zero are the same thing.

Actually, we don't even distinguish between a retrigger that
successfully poked the HW, and a retrigger that returned an error. Both
are considered to not to require a SW retrigger... maybe we should fix
that too. Jiang, Thomas?

Anyway, for this patch:

Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>

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

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 6/6] irqchip: crossbar: fix irq masking at suspend
  2015-08-12 17:46   ` Grygorii Strashko
  (?)
@ 2015-08-13  9:30     ` Sudeep Holla
  -1 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13  9:30 UTC (permalink / raw)
  To: Grygorii Strashko, Marc Zyngier
  Cc: tglx, tony, Sudeep Holla, linux, jason, nsekhar, linux-kernel,
	balbi, linux-omap, linux-arm-kernel



On 12/08/15 18:46, Grygorii Strashko wrote:
> All ARM GIC IRQs have to masked during suspend if they are not
> wakeup source. Now this is not happen, since switching to
> use IRQ domain hierarchy, because suspend_device_irq() only checks flags
> in the last IRQ chip in hierarchy for IRQCHIP_MASK_ON_SUSPEND
> bit set. And in the case of TI OMAP DRA7 the last IRQ chip is TI Crossbar
> which do not have this flag set.
>
> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
>    ARM GIC <- OMAP wakeupgen <- TI CBAR
> ARM GIC - IRQCHIP_MASK_ON_SUSPEND=n

May be this won't affect your platform or this patch but even GIC marks
IRQCHIP_MASK_ON_SUSPEND=y now since GIC doesn't provide any facility to
configure the wakeup source and keeps all the interrupt source enabled.

We have this flag enabled now as it's always safer to mask all the non
wakeup interrupts are masked at the chip level when suspending.

Also the beginning of the commit message contradicts when you also say
in the following statement IRQCHIP_MASK_ON_SUSPEND=n. So you may need to
update the log.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 6/6] irqchip: crossbar: fix irq masking at suspend
@ 2015-08-13  9:30     ` Sudeep Holla
  0 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13  9:30 UTC (permalink / raw)
  To: Grygorii Strashko, Marc Zyngier
  Cc: tglx, tony, Sudeep Holla, linux, jason, nsekhar, linux-kernel,
	balbi, linux-omap, linux-arm-kernel



On 12/08/15 18:46, Grygorii Strashko wrote:
> All ARM GIC IRQs have to masked during suspend if they are not
> wakeup source. Now this is not happen, since switching to
> use IRQ domain hierarchy, because suspend_device_irq() only checks flags
> in the last IRQ chip in hierarchy for IRQCHIP_MASK_ON_SUSPEND
> bit set. And in the case of TI OMAP DRA7 the last IRQ chip is TI Crossbar
> which do not have this flag set.
>
> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
>    ARM GIC <- OMAP wakeupgen <- TI CBAR
> ARM GIC - IRQCHIP_MASK_ON_SUSPEND=n

May be this won't affect your platform or this patch but even GIC marks
IRQCHIP_MASK_ON_SUSPEND=y now since GIC doesn't provide any facility to
configure the wakeup source and keeps all the interrupt source enabled.

We have this flag enabled now as it's always safer to mask all the non
wakeup interrupts are masked at the chip level when suspending.

Also the beginning of the commit message contradicts when you also say
in the following statement IRQCHIP_MASK_ON_SUSPEND=n. So you may need to
update the log.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 6/6] irqchip: crossbar: fix irq masking at suspend
@ 2015-08-13  9:30     ` Sudeep Holla
  0 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13  9:30 UTC (permalink / raw)
  To: linux-arm-kernel



On 12/08/15 18:46, Grygorii Strashko wrote:
> All ARM GIC IRQs have to masked during suspend if they are not
> wakeup source. Now this is not happen, since switching to
> use IRQ domain hierarchy, because suspend_device_irq() only checks flags
> in the last IRQ chip in hierarchy for IRQCHIP_MASK_ON_SUSPEND
> bit set. And in the case of TI OMAP DRA7 the last IRQ chip is TI Crossbar
> which do not have this flag set.
>
> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
>    ARM GIC <- OMAP wakeupgen <- TI CBAR
> ARM GIC - IRQCHIP_MASK_ON_SUSPEND=n

May be this won't affect your platform or this patch but even GIC marks
IRQCHIP_MASK_ON_SUSPEND=y now since GIC doesn't provide any facility to
configure the wakeup source and keeps all the interrupt source enabled.

We have this flag enabled now as it's always safer to mask all the non
wakeup interrupts are masked at the chip level when suspending.

Also the beginning of the commit message contradicts when you also say
in the following statement IRQCHIP_MASK_ON_SUSPEND=n. So you may need to
update the log.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 5/6] ARM: OMAP: wakeupgen: fix arm gic irq type configuration
  2015-08-12 17:46   ` Grygorii Strashko
@ 2015-08-13  9:50     ` Tony Lindgren
  -1 siblings, 0 replies; 65+ messages in thread
From: Tony Lindgren @ 2015-08-13  9:50 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: tglx, marc.zyngier, linux, nsekhar, jason, balbi, linux-omap,
	linux-arm-kernel, linux-kernel

* Grygorii Strashko <grygorii.strashko@ti.com> [150812 10:49]:
> It's observed that ARM GIC IRQ triggering type is not configured
> properly when IRQ is routed through IRQ domain hierarchy and
> system started using DT. As result, system will start using default
> ARM GIC configuration, ignore DT IRQ triggering configuration,
> and value of desc->irq_data.state_use_accessors = 0.
> 
> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
> ARM GIC <- OMAP wakeupgen <- TI CBAR
> 
> Failed call chain:
>      irq_create_of_mapping
>      irq_set_irq_type
>      __irq_set_trigger
>      if (!chip || !chip->irq_set_type) {
>         return 0; <- return here
>      }
> OMAP wakeupgen has no .irq_set_type() defined and, so, IRQ triggering
> configuration will not be propagated to parent IRQ domain.
> 
> Hence, fix it by using irq_chip_set_type_parent() for
> propagation IRQ triggering type to parent IRQ domains.
> 
> Fixes: 7136d457f365 ('ARM: omap: convert wakeupgen to stacked domains')
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Makse sense to merge this along with the irqchip changes once those
are ready, so feel free to add to this one:

Acked-by: Tony Lindgren <tony@atomide.com>

> ---
>  arch/arm/mach-omap2/omap-wakeupgen.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-omap2/omap-wakeupgen.c b/arch/arm/mach-omap2/omap-wakeupgen.c
> index 8e52621..e1d2e99 100644
> --- a/arch/arm/mach-omap2/omap-wakeupgen.c
> +++ b/arch/arm/mach-omap2/omap-wakeupgen.c
> @@ -392,6 +392,7 @@ static struct irq_chip wakeupgen_chip = {
>  	.irq_mask		= wakeupgen_mask,
>  	.irq_unmask		= wakeupgen_unmask,
>  	.irq_retrigger		= irq_chip_retrigger_hierarchy,
> +	.irq_set_type		= irq_chip_set_type_parent,
>  	.flags			= IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND,
>  #ifdef CONFIG_SMP
>  	.irq_set_affinity	= irq_chip_set_affinity_parent,
> -- 
> 2.5.0
> 

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 5/6] ARM: OMAP: wakeupgen: fix arm gic irq type configuration
@ 2015-08-13  9:50     ` Tony Lindgren
  0 siblings, 0 replies; 65+ messages in thread
From: Tony Lindgren @ 2015-08-13  9:50 UTC (permalink / raw)
  To: linux-arm-kernel

* Grygorii Strashko <grygorii.strashko@ti.com> [150812 10:49]:
> It's observed that ARM GIC IRQ triggering type is not configured
> properly when IRQ is routed through IRQ domain hierarchy and
> system started using DT. As result, system will start using default
> ARM GIC configuration, ignore DT IRQ triggering configuration,
> and value of desc->irq_data.state_use_accessors = 0.
> 
> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
> ARM GIC <- OMAP wakeupgen <- TI CBAR
> 
> Failed call chain:
>      irq_create_of_mapping
>      irq_set_irq_type
>      __irq_set_trigger
>      if (!chip || !chip->irq_set_type) {
>         return 0; <- return here
>      }
> OMAP wakeupgen has no .irq_set_type() defined and, so, IRQ triggering
> configuration will not be propagated to parent IRQ domain.
> 
> Hence, fix it by using irq_chip_set_type_parent() for
> propagation IRQ triggering type to parent IRQ domains.
> 
> Fixes: 7136d457f365 ('ARM: omap: convert wakeupgen to stacked domains')
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Makse sense to merge this along with the irqchip changes once those
are ready, so feel free to add to this one:

Acked-by: Tony Lindgren <tony@atomide.com>

> ---
>  arch/arm/mach-omap2/omap-wakeupgen.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-omap2/omap-wakeupgen.c b/arch/arm/mach-omap2/omap-wakeupgen.c
> index 8e52621..e1d2e99 100644
> --- a/arch/arm/mach-omap2/omap-wakeupgen.c
> +++ b/arch/arm/mach-omap2/omap-wakeupgen.c
> @@ -392,6 +392,7 @@ static struct irq_chip wakeupgen_chip = {
>  	.irq_mask		= wakeupgen_mask,
>  	.irq_unmask		= wakeupgen_unmask,
>  	.irq_retrigger		= irq_chip_retrigger_hierarchy,
> +	.irq_set_type		= irq_chip_set_type_parent,
>  	.flags			= IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND,
>  #ifdef CONFIG_SMP
>  	.irq_set_affinity	= irq_chip_set_affinity_parent,
> -- 
> 2.5.0
> 

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
  2015-08-13  8:54     ` Sudeep Holla
  (?)
@ 2015-08-13  9:51       ` Grygorii Strashko
  -1 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13  9:51 UTC (permalink / raw)
  To: Sudeep Holla, Marc Zyngier
  Cc: tglx, tony, linux, jason, nsekhar, linux-kernel, balbi,
	linux-omap, linux-arm-kernel

On 08/13/2015 11:54 AM, Sudeep Holla wrote:
>
>
> On 12/08/15 18:45, Grygorii Strashko wrote:
>> The irqchip_set_wake_parent should not fail if IRQ chip
>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>> configuration can't be propagated properly through IRQ
>> domains hierarchy.
>>
>> In case of TI OMAP DRA7 the issue reproduced with following
>> configuration:
>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>
>> gpio_key is wakeup source
>>
>> Failure is reproduced during suspend/resume to RAM:
>> suspend:
>>   - gpio_keys_suspend
>>     enable_irq_wake
>>       + pcf857x_irq_set_wake
>>         + omap_gpio_wake_enable
>>           + TI CBAR irq_chip_set_wake_parent
>>             + OMAP wakeupgen has no .irq_set_wake()
>>             and -ENOSYS will be returned
>>
>> resume:
>>   - gpio_keys_resume
>>     + disable_irq_wake
>>       + irq_set_irq_wake
>>         + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>
>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   kernel/irq/chip.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index 6de638b..bdb1b9d 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct
>> irq_data *data, void *vcpu_info)
>>   int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>   {
>>       data = data->parent_data;
>> +
>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>> +        return 0;
>> +
>
> [Nit] I think the irq core can access data->chip directly. Either way,
> it's better to be consistent, the statement following doesn't use helper
> function.

thanks. I'll change it to:
	if (data->chip->flags & IRQCHIP_SKIP_SET_WAKE)
		return 0;

>
> Otherwise looks good to me.

Does it means that I can add your Reviewed-by: with above change?


>
>>       if (data->chip->irq_set_wake)
>>           return data->chip->irq_set_wake(data, on);
>>
>>


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-13  9:51       ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13  9:51 UTC (permalink / raw)
  To: Sudeep Holla, Marc Zyngier
  Cc: tglx, tony, linux, jason, nsekhar, linux-kernel, balbi,
	linux-omap, linux-arm-kernel

On 08/13/2015 11:54 AM, Sudeep Holla wrote:
>
>
> On 12/08/15 18:45, Grygorii Strashko wrote:
>> The irqchip_set_wake_parent should not fail if IRQ chip
>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>> configuration can't be propagated properly through IRQ
>> domains hierarchy.
>>
>> In case of TI OMAP DRA7 the issue reproduced with following
>> configuration:
>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>
>> gpio_key is wakeup source
>>
>> Failure is reproduced during suspend/resume to RAM:
>> suspend:
>>   - gpio_keys_suspend
>>     enable_irq_wake
>>       + pcf857x_irq_set_wake
>>         + omap_gpio_wake_enable
>>           + TI CBAR irq_chip_set_wake_parent
>>             + OMAP wakeupgen has no .irq_set_wake()
>>             and -ENOSYS will be returned
>>
>> resume:
>>   - gpio_keys_resume
>>     + disable_irq_wake
>>       + irq_set_irq_wake
>>         + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>
>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   kernel/irq/chip.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index 6de638b..bdb1b9d 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct
>> irq_data *data, void *vcpu_info)
>>   int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>   {
>>       data = data->parent_data;
>> +
>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>> +        return 0;
>> +
>
> [Nit] I think the irq core can access data->chip directly. Either way,
> it's better to be consistent, the statement following doesn't use helper
> function.

thanks. I'll change it to:
	if (data->chip->flags & IRQCHIP_SKIP_SET_WAKE)
		return 0;

>
> Otherwise looks good to me.

Does it means that I can add your Reviewed-by: with above change?


>
>>       if (data->chip->irq_set_wake)
>>           return data->chip->irq_set_wake(data, on);
>>
>>


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-13  9:51       ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13  9:51 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/13/2015 11:54 AM, Sudeep Holla wrote:
>
>
> On 12/08/15 18:45, Grygorii Strashko wrote:
>> The irqchip_set_wake_parent should not fail if IRQ chip
>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>> configuration can't be propagated properly through IRQ
>> domains hierarchy.
>>
>> In case of TI OMAP DRA7 the issue reproduced with following
>> configuration:
>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>
>> gpio_key is wakeup source
>>
>> Failure is reproduced during suspend/resume to RAM:
>> suspend:
>>   - gpio_keys_suspend
>>     enable_irq_wake
>>       + pcf857x_irq_set_wake
>>         + omap_gpio_wake_enable
>>           + TI CBAR irq_chip_set_wake_parent
>>             + OMAP wakeupgen has no .irq_set_wake()
>>             and -ENOSYS will be returned
>>
>> resume:
>>   - gpio_keys_resume
>>     + disable_irq_wake
>>       + irq_set_irq_wake
>>         + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>
>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   kernel/irq/chip.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index 6de638b..bdb1b9d 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct
>> irq_data *data, void *vcpu_info)
>>   int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>   {
>>       data = data->parent_data;
>> +
>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>> +        return 0;
>> +
>
> [Nit] I think the irq core can access data->chip directly. Either way,
> it's better to be consistent, the statement following doesn't use helper
> function.

thanks. I'll change it to:
	if (data->chip->flags & IRQCHIP_SKIP_SET_WAKE)
		return 0;

>
> Otherwise looks good to me.

Does it means that I can add your Reviewed-by: with above change?


>
>>       if (data->chip->irq_set_wake)
>>           return data->chip->irq_set_wake(data, on);
>>
>>


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 3/6] genirq: introduce irq_chip_set_type_parent() helper
  2015-08-13  8:58     ` Sudeep Holla
  (?)
@ 2015-08-13  9:54       ` Grygorii Strashko
  -1 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13  9:54 UTC (permalink / raw)
  To: Sudeep Holla, Marc Zyngier
  Cc: tglx, tony, linux, jason, nsekhar, linux-kernel, balbi,
	linux-omap, linux-arm-kernel

On 08/13/2015 11:58 AM, Sudeep Holla wrote:
>
>
> On 12/08/15 18:45, Grygorii Strashko wrote:
>> It's expected to use this helper when the current
>> domain doesn't implement .irq_set_type(),  but expect
>> the parent to do so.
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   include/linux/irq.h |  1 +
>>   kernel/irq/chip.c   | 20 ++++++++++++++++++++
>>   2 files changed, 21 insertions(+)
>>
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index 92188b0..51744bc 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -484,6 +484,7 @@ extern int irq_chip_set_affinity_parent(struct
>> irq_data *data,
>>   extern int irq_chip_set_wake_parent(struct irq_data *data, unsigned
>> int on);
>>   extern int irq_chip_set_vcpu_affinity_parent(struct irq_data *data,
>>                            void *vcpu_info);
>> +extern int irq_chip_set_type_parent(struct irq_data *data, unsigned
>> int type);
>>   #endif
>>
>>   /* Handling of unhandled and spurious interrupts: */
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index bdb1b9d..b48938b 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -985,6 +985,26 @@ int irq_chip_set_affinity_parent(struct irq_data
>> *data,
>>   }
>>
>>   /**
>> + * irq_chip_set_type_parent - Set IRQ type on the parent interrupt
>> + * @data:    Pointer to interrupt specific data
>> + * @type:    IRQ_TYPE_{LEVEL,EDGE}_* value - see include/linux/irq.h
>> + *
>> + * Conditional, as the underlying parent chip might not implement it.
>> + */
>> +int irq_chip_set_type_parent(struct irq_data *data, unsigned int type)
>> +{
>> +    data = data->parent_data;
>> +
>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>> +        return 0;
>> +
>
> I think this is unrelated here(perhaps copy-paste error ?). I fail to
> see how wakeup and trigger type are related.

Oh. Yes, it's copy-paste error. thanks, will fix.

-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 3/6] genirq: introduce irq_chip_set_type_parent() helper
@ 2015-08-13  9:54       ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13  9:54 UTC (permalink / raw)
  To: Sudeep Holla, Marc Zyngier
  Cc: tglx, tony, linux, jason, nsekhar, linux-kernel, balbi,
	linux-omap, linux-arm-kernel

On 08/13/2015 11:58 AM, Sudeep Holla wrote:
>
>
> On 12/08/15 18:45, Grygorii Strashko wrote:
>> It's expected to use this helper when the current
>> domain doesn't implement .irq_set_type(),  but expect
>> the parent to do so.
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   include/linux/irq.h |  1 +
>>   kernel/irq/chip.c   | 20 ++++++++++++++++++++
>>   2 files changed, 21 insertions(+)
>>
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index 92188b0..51744bc 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -484,6 +484,7 @@ extern int irq_chip_set_affinity_parent(struct
>> irq_data *data,
>>   extern int irq_chip_set_wake_parent(struct irq_data *data, unsigned
>> int on);
>>   extern int irq_chip_set_vcpu_affinity_parent(struct irq_data *data,
>>                            void *vcpu_info);
>> +extern int irq_chip_set_type_parent(struct irq_data *data, unsigned
>> int type);
>>   #endif
>>
>>   /* Handling of unhandled and spurious interrupts: */
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index bdb1b9d..b48938b 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -985,6 +985,26 @@ int irq_chip_set_affinity_parent(struct irq_data
>> *data,
>>   }
>>
>>   /**
>> + * irq_chip_set_type_parent - Set IRQ type on the parent interrupt
>> + * @data:    Pointer to interrupt specific data
>> + * @type:    IRQ_TYPE_{LEVEL,EDGE}_* value - see include/linux/irq.h
>> + *
>> + * Conditional, as the underlying parent chip might not implement it.
>> + */
>> +int irq_chip_set_type_parent(struct irq_data *data, unsigned int type)
>> +{
>> +    data = data->parent_data;
>> +
>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>> +        return 0;
>> +
>
> I think this is unrelated here(perhaps copy-paste error ?). I fail to
> see how wakeup and trigger type are related.

Oh. Yes, it's copy-paste error. thanks, will fix.

-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 3/6] genirq: introduce irq_chip_set_type_parent() helper
@ 2015-08-13  9:54       ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13  9:54 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/13/2015 11:58 AM, Sudeep Holla wrote:
>
>
> On 12/08/15 18:45, Grygorii Strashko wrote:
>> It's expected to use this helper when the current
>> domain doesn't implement .irq_set_type(),  but expect
>> the parent to do so.
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   include/linux/irq.h |  1 +
>>   kernel/irq/chip.c   | 20 ++++++++++++++++++++
>>   2 files changed, 21 insertions(+)
>>
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index 92188b0..51744bc 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -484,6 +484,7 @@ extern int irq_chip_set_affinity_parent(struct
>> irq_data *data,
>>   extern int irq_chip_set_wake_parent(struct irq_data *data, unsigned
>> int on);
>>   extern int irq_chip_set_vcpu_affinity_parent(struct irq_data *data,
>>                            void *vcpu_info);
>> +extern int irq_chip_set_type_parent(struct irq_data *data, unsigned
>> int type);
>>   #endif
>>
>>   /* Handling of unhandled and spurious interrupts: */
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index bdb1b9d..b48938b 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -985,6 +985,26 @@ int irq_chip_set_affinity_parent(struct irq_data
>> *data,
>>   }
>>
>>   /**
>> + * irq_chip_set_type_parent - Set IRQ type on the parent interrupt
>> + * @data:    Pointer to interrupt specific data
>> + * @type:    IRQ_TYPE_{LEVEL,EDGE}_* value - see include/linux/irq.h
>> + *
>> + * Conditional, as the underlying parent chip might not implement it.
>> + */
>> +int irq_chip_set_type_parent(struct irq_data *data, unsigned int type)
>> +{
>> +    data = data->parent_data;
>> +
>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>> +        return 0;
>> +
>
> I think this is unrelated here(perhaps copy-paste error ?). I fail to
> see how wakeup and trigger type are related.

Oh. Yes, it's copy-paste error. thanks, will fix.

-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
  2015-08-12 17:45   ` Grygorii Strashko
  (?)
@ 2015-08-13 10:01     ` Marc Zyngier
  -1 siblings, 0 replies; 65+ messages in thread
From: Marc Zyngier @ 2015-08-13 10:01 UTC (permalink / raw)
  To: Grygorii Strashko, tglx, tony
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel, linux-kernel

On 12/08/15 18:45, Grygorii Strashko wrote:
> The irqchip_set_wake_parent should not fail if IRQ chip
> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
> configuration can't be propagated properly through IRQ
> domains hierarchy.
> 
> In case of TI OMAP DRA7 the issue reproduced with following
> configuration:
> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
> 
> gpio_key is wakeup source
> 
> Failure is reproduced during suspend/resume to RAM:
> suspend:
>  - gpio_keys_suspend
>    enable_irq_wake
>      + pcf857x_irq_set_wake
>        + omap_gpio_wake_enable
>          + TI CBAR irq_chip_set_wake_parent
>            + OMAP wakeupgen has no .irq_set_wake()

Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.

>            and -ENOSYS will be returned
> 
> resume:
>  - gpio_keys_resume
>    + disable_irq_wake
>      + irq_set_irq_wake
>        + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
> 
> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  kernel/irq/chip.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 6de638b..bdb1b9d 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
>  int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>  {
>  	data = data->parent_data;
> +
> +	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
> +		return 0;
> +
>  	if (data->chip->irq_set_wake)
>  		return data->chip->irq_set_wake(data, on);
>  
> 

We have a more general issue with chip flags, and how they combine
within a stack of irqchips.

What if you remove the irq_chip_set_wake_parent from the crossbar
driver, and instead set IRQCHIP_SKIP_SET_WAKE?

Thanks,

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

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-13 10:01     ` Marc Zyngier
  0 siblings, 0 replies; 65+ messages in thread
From: Marc Zyngier @ 2015-08-13 10:01 UTC (permalink / raw)
  To: Grygorii Strashko, tglx, tony
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel, linux-kernel

On 12/08/15 18:45, Grygorii Strashko wrote:
> The irqchip_set_wake_parent should not fail if IRQ chip
> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
> configuration can't be propagated properly through IRQ
> domains hierarchy.
> 
> In case of TI OMAP DRA7 the issue reproduced with following
> configuration:
> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
> 
> gpio_key is wakeup source
> 
> Failure is reproduced during suspend/resume to RAM:
> suspend:
>  - gpio_keys_suspend
>    enable_irq_wake
>      + pcf857x_irq_set_wake
>        + omap_gpio_wake_enable
>          + TI CBAR irq_chip_set_wake_parent
>            + OMAP wakeupgen has no .irq_set_wake()

Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.

>            and -ENOSYS will be returned
> 
> resume:
>  - gpio_keys_resume
>    + disable_irq_wake
>      + irq_set_irq_wake
>        + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
> 
> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  kernel/irq/chip.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 6de638b..bdb1b9d 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
>  int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>  {
>  	data = data->parent_data;
> +
> +	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
> +		return 0;
> +
>  	if (data->chip->irq_set_wake)
>  		return data->chip->irq_set_wake(data, on);
>  
> 

We have a more general issue with chip flags, and how they combine
within a stack of irqchips.

What if you remove the irq_chip_set_wake_parent from the crossbar
driver, and instead set IRQCHIP_SKIP_SET_WAKE?

Thanks,

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

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-13 10:01     ` Marc Zyngier
  0 siblings, 0 replies; 65+ messages in thread
From: Marc Zyngier @ 2015-08-13 10:01 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/08/15 18:45, Grygorii Strashko wrote:
> The irqchip_set_wake_parent should not fail if IRQ chip
> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
> configuration can't be propagated properly through IRQ
> domains hierarchy.
> 
> In case of TI OMAP DRA7 the issue reproduced with following
> configuration:
> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
> 
> gpio_key is wakeup source
> 
> Failure is reproduced during suspend/resume to RAM:
> suspend:
>  - gpio_keys_suspend
>    enable_irq_wake
>      + pcf857x_irq_set_wake
>        + omap_gpio_wake_enable
>          + TI CBAR irq_chip_set_wake_parent
>            + OMAP wakeupgen has no .irq_set_wake()

Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.

>            and -ENOSYS will be returned
> 
> resume:
>  - gpio_keys_resume
>    + disable_irq_wake
>      + irq_set_irq_wake
>        + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
> 
> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  kernel/irq/chip.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 6de638b..bdb1b9d 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
>  int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>  {
>  	data = data->parent_data;
> +
> +	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
> +		return 0;
> +
>  	if (data->chip->irq_set_wake)
>  		return data->chip->irq_set_wake(data, on);
>  
> 

We have a more general issue with chip flags, and how they combine
within a stack of irqchips.

What if you remove the irq_chip_set_wake_parent from the crossbar
driver, and instead set IRQCHIP_SKIP_SET_WAKE?

Thanks,

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

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 6/6] irqchip: crossbar: fix irq masking at suspend
  2015-08-13  9:30     ` Sudeep Holla
  (?)
@ 2015-08-13 10:17       ` Grygorii Strashko
  -1 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13 10:17 UTC (permalink / raw)
  To: Sudeep Holla, Marc Zyngier
  Cc: tglx, tony, linux, jason, nsekhar, linux-kernel, balbi,
	linux-omap, linux-arm-kernel

On 08/13/2015 12:30 PM, Sudeep Holla wrote:
> 
> 
> On 12/08/15 18:46, Grygorii Strashko wrote:
>> All ARM GIC IRQs have to masked during suspend if they are not
>> wakeup source. Now this is not happen, since switching to
>> use IRQ domain hierarchy, because suspend_device_irq() only checks flags
>> in the last IRQ chip in hierarchy for IRQCHIP_MASK_ON_SUSPEND
>> bit set. And in the case of TI OMAP DRA7 the last IRQ chip is TI Crossbar
>> which do not have this flag set.
>>
>> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
>>    ARM GIC <- OMAP wakeupgen <- TI CBAR
>> ARM GIC - IRQCHIP_MASK_ON_SUSPEND=n
> 
> May be this won't affect your platform or this patch but even GIC marks
> IRQCHIP_MASK_ON_SUSPEND=y now since GIC doesn't provide any facility to
> configure the wakeup source and keeps all the interrupt source enabled.

That's true for next, bur not true for 4.2-rc6 or 4.1 :(

> 
> We have this flag enabled now as it's always safer to mask all the non
> wakeup interrupts are masked at the chip level when suspending.

Indeed, but that's do not work in case of IRQ domain hierarchy and
it's do not clear how should it work?
I've tried to describe this problem in cover letter actually.

> 
> Also the beginning of the commit message contradicts when you also say
> in the following statement IRQCHIP_MASK_ON_SUSPEND=n. So you may need to
> update the log.

I'll try to reword. What I've tried to mention that IRQs masking on
suspend is default expected behavior and that how it was before
switching to IRQ domain hierarchy.

"All ARM GIC IRQs have to masked during suspend if they are not
 wakeup source - this is expected behavior and that's how it was before
 switching to IRQ domain hierarchy. ..."
ok?

-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 6/6] irqchip: crossbar: fix irq masking at suspend
@ 2015-08-13 10:17       ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13 10:17 UTC (permalink / raw)
  To: Sudeep Holla, Marc Zyngier
  Cc: tglx, tony, linux, jason, nsekhar, linux-kernel, balbi,
	linux-omap, linux-arm-kernel

On 08/13/2015 12:30 PM, Sudeep Holla wrote:
> 
> 
> On 12/08/15 18:46, Grygorii Strashko wrote:
>> All ARM GIC IRQs have to masked during suspend if they are not
>> wakeup source. Now this is not happen, since switching to
>> use IRQ domain hierarchy, because suspend_device_irq() only checks flags
>> in the last IRQ chip in hierarchy for IRQCHIP_MASK_ON_SUSPEND
>> bit set. And in the case of TI OMAP DRA7 the last IRQ chip is TI Crossbar
>> which do not have this flag set.
>>
>> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
>>    ARM GIC <- OMAP wakeupgen <- TI CBAR
>> ARM GIC - IRQCHIP_MASK_ON_SUSPEND=n
> 
> May be this won't affect your platform or this patch but even GIC marks
> IRQCHIP_MASK_ON_SUSPEND=y now since GIC doesn't provide any facility to
> configure the wakeup source and keeps all the interrupt source enabled.

That's true for next, bur not true for 4.2-rc6 or 4.1 :(

> 
> We have this flag enabled now as it's always safer to mask all the non
> wakeup interrupts are masked at the chip level when suspending.

Indeed, but that's do not work in case of IRQ domain hierarchy and
it's do not clear how should it work?
I've tried to describe this problem in cover letter actually.

> 
> Also the beginning of the commit message contradicts when you also say
> in the following statement IRQCHIP_MASK_ON_SUSPEND=n. So you may need to
> update the log.

I'll try to reword. What I've tried to mention that IRQs masking on
suspend is default expected behavior and that how it was before
switching to IRQ domain hierarchy.

"All ARM GIC IRQs have to masked during suspend if they are not
 wakeup source - this is expected behavior and that's how it was before
 switching to IRQ domain hierarchy. ..."
ok?

-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 6/6] irqchip: crossbar: fix irq masking at suspend
@ 2015-08-13 10:17       ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13 10:17 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/13/2015 12:30 PM, Sudeep Holla wrote:
> 
> 
> On 12/08/15 18:46, Grygorii Strashko wrote:
>> All ARM GIC IRQs have to masked during suspend if they are not
>> wakeup source. Now this is not happen, since switching to
>> use IRQ domain hierarchy, because suspend_device_irq() only checks flags
>> in the last IRQ chip in hierarchy for IRQCHIP_MASK_ON_SUSPEND
>> bit set. And in the case of TI OMAP DRA7 the last IRQ chip is TI Crossbar
>> which do not have this flag set.
>>
>> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
>>    ARM GIC <- OMAP wakeupgen <- TI CBAR
>> ARM GIC - IRQCHIP_MASK_ON_SUSPEND=n
> 
> May be this won't affect your platform or this patch but even GIC marks
> IRQCHIP_MASK_ON_SUSPEND=y now since GIC doesn't provide any facility to
> configure the wakeup source and keeps all the interrupt source enabled.

That's true for next, bur not true for 4.2-rc6 or 4.1 :(

> 
> We have this flag enabled now as it's always safer to mask all the non
> wakeup interrupts are masked at the chip level when suspending.

Indeed, but that's do not work in case of IRQ domain hierarchy and
it's do not clear how should it work?
I've tried to describe this problem in cover letter actually.

> 
> Also the beginning of the commit message contradicts when you also say
> in the following statement IRQCHIP_MASK_ON_SUSPEND=n. So you may need to
> update the log.

I'll try to reword. What I've tried to mention that IRQs masking on
suspend is default expected behavior and that how it was before
switching to IRQ domain hierarchy.

"All ARM GIC IRQs have to masked during suspend if they are not
 wakeup source - this is expected behavior and that's how it was before
 switching to IRQ domain hierarchy. ..."
ok?

-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
  2015-08-13 10:01     ` Marc Zyngier
  (?)
@ 2015-08-13 10:31       ` Grygorii Strashko
  -1 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13 10:31 UTC (permalink / raw)
  To: Marc Zyngier, tglx, tony
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel, linux-kernel

On 08/13/2015 01:01 PM, Marc Zyngier wrote:
> On 12/08/15 18:45, Grygorii Strashko wrote:
>> The irqchip_set_wake_parent should not fail if IRQ chip
>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>> configuration can't be propagated properly through IRQ
>> domains hierarchy.
>>
>> In case of TI OMAP DRA7 the issue reproduced with following
>> configuration:
>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>
>> gpio_key is wakeup source
>>
>> Failure is reproduced during suspend/resume to RAM:
>> suspend:
>>   - gpio_keys_suspend
>>     enable_irq_wake
>>       + pcf857x_irq_set_wake
>>         + omap_gpio_wake_enable
>>           + TI CBAR irq_chip_set_wake_parent
>>             + OMAP wakeupgen has no .irq_set_wake()
>
> Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.
>
>>             and -ENOSYS will be returned
>>
>> resume:
>>   - gpio_keys_resume
>>     + disable_irq_wake
>>       + irq_set_irq_wake
>>         + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>
>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   kernel/irq/chip.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index 6de638b..bdb1b9d 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
>>   int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>   {
>>   	data = data->parent_data;
>> +
>> +	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>> +		return 0;
>> +
>>   	if (data->chip->irq_set_wake)
>>   		return data->chip->irq_set_wake(data, on);
>>
>>
>
> We have a more general issue with chip flags, and how they combine
> within a stack of irqchips.

Indeed. Problem looks similar to IRQCHIP_MASK_ON_SUSPEND flag usage.

>
> What if you remove the irq_chip_set_wake_parent from the crossbar
> driver, and instead set IRQCHIP_SKIP_SET_WAKE?

I've thought about this and it should work for me.
One question - what if crossbar will be not the last one in
IRQ domains hierarchy?


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-13 10:31       ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13 10:31 UTC (permalink / raw)
  To: Marc Zyngier, tglx, tony
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel, linux-kernel

On 08/13/2015 01:01 PM, Marc Zyngier wrote:
> On 12/08/15 18:45, Grygorii Strashko wrote:
>> The irqchip_set_wake_parent should not fail if IRQ chip
>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>> configuration can't be propagated properly through IRQ
>> domains hierarchy.
>>
>> In case of TI OMAP DRA7 the issue reproduced with following
>> configuration:
>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>
>> gpio_key is wakeup source
>>
>> Failure is reproduced during suspend/resume to RAM:
>> suspend:
>>   - gpio_keys_suspend
>>     enable_irq_wake
>>       + pcf857x_irq_set_wake
>>         + omap_gpio_wake_enable
>>           + TI CBAR irq_chip_set_wake_parent
>>             + OMAP wakeupgen has no .irq_set_wake()
>
> Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.
>
>>             and -ENOSYS will be returned
>>
>> resume:
>>   - gpio_keys_resume
>>     + disable_irq_wake
>>       + irq_set_irq_wake
>>         + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>
>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   kernel/irq/chip.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index 6de638b..bdb1b9d 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
>>   int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>   {
>>   	data = data->parent_data;
>> +
>> +	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>> +		return 0;
>> +
>>   	if (data->chip->irq_set_wake)
>>   		return data->chip->irq_set_wake(data, on);
>>
>>
>
> We have a more general issue with chip flags, and how they combine
> within a stack of irqchips.

Indeed. Problem looks similar to IRQCHIP_MASK_ON_SUSPEND flag usage.

>
> What if you remove the irq_chip_set_wake_parent from the crossbar
> driver, and instead set IRQCHIP_SKIP_SET_WAKE?

I've thought about this and it should work for me.
One question - what if crossbar will be not the last one in
IRQ domains hierarchy?


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-13 10:31       ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13 10:31 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/13/2015 01:01 PM, Marc Zyngier wrote:
> On 12/08/15 18:45, Grygorii Strashko wrote:
>> The irqchip_set_wake_parent should not fail if IRQ chip
>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>> configuration can't be propagated properly through IRQ
>> domains hierarchy.
>>
>> In case of TI OMAP DRA7 the issue reproduced with following
>> configuration:
>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>
>> gpio_key is wakeup source
>>
>> Failure is reproduced during suspend/resume to RAM:
>> suspend:
>>   - gpio_keys_suspend
>>     enable_irq_wake
>>       + pcf857x_irq_set_wake
>>         + omap_gpio_wake_enable
>>           + TI CBAR irq_chip_set_wake_parent
>>             + OMAP wakeupgen has no .irq_set_wake()
>
> Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.
>
>>             and -ENOSYS will be returned
>>
>> resume:
>>   - gpio_keys_resume
>>     + disable_irq_wake
>>       + irq_set_irq_wake
>>         + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>
>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   kernel/irq/chip.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>> index 6de638b..bdb1b9d 100644
>> --- a/kernel/irq/chip.c
>> +++ b/kernel/irq/chip.c
>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
>>   int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>   {
>>   	data = data->parent_data;
>> +
>> +	if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>> +		return 0;
>> +
>>   	if (data->chip->irq_set_wake)
>>   		return data->chip->irq_set_wake(data, on);
>>
>>
>
> We have a more general issue with chip flags, and how they combine
> within a stack of irqchips.

Indeed. Problem looks similar to IRQCHIP_MASK_ON_SUSPEND flag usage.

>
> What if you remove the irq_chip_set_wake_parent from the crossbar
> driver, and instead set IRQCHIP_SKIP_SET_WAKE?

I've thought about this and it should work for me.
One question - what if crossbar will be not the last one in
IRQ domains hierarchy?


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
  2015-08-13  9:51       ` Grygorii Strashko
  (?)
@ 2015-08-13 10:33         ` Sudeep Holla
  -1 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13 10:33 UTC (permalink / raw)
  To: Grygorii Strashko, Marc Zyngier
  Cc: Sudeep Holla, linux, jason, tony, nsekhar, linux-kernel, balbi,
	tglx, linux-omap, linux-arm-kernel



On 13/08/15 10:51, Grygorii Strashko wrote:
> On 08/13/2015 11:54 AM, Sudeep Holla wrote:
>>
>>
>> On 12/08/15 18:45, Grygorii Strashko wrote:

[...]

>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>> index 6de638b..bdb1b9d 100644
>>> --- a/kernel/irq/chip.c
>>> +++ b/kernel/irq/chip.c
>>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct
>>> irq_data *data, void *vcpu_info)
>>>    int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>>    {
>>>        data = data->parent_data;
>>> +
>>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>>> +        return 0;
>>> +
>>
>> [Nit] I think the irq core can access data->chip directly. Either way,
>> it's better to be consistent, the statement following doesn't use helper
>> function.
>
> thanks. I'll change it to:
> 	if (data->chip->flags & IRQCHIP_SKIP_SET_WAKE)
> 		return 0;
>
>>
>> Otherwise looks good to me.
>
> Does it means that I can add your Reviewed-by: with above change?
>

Yes you can.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-13 10:33         ` Sudeep Holla
  0 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13 10:33 UTC (permalink / raw)
  To: Grygorii Strashko, Marc Zyngier
  Cc: Sudeep Holla, linux, jason, tony, nsekhar, linux-kernel, balbi,
	tglx, linux-omap, linux-arm-kernel



On 13/08/15 10:51, Grygorii Strashko wrote:
> On 08/13/2015 11:54 AM, Sudeep Holla wrote:
>>
>>
>> On 12/08/15 18:45, Grygorii Strashko wrote:

[...]

>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>> index 6de638b..bdb1b9d 100644
>>> --- a/kernel/irq/chip.c
>>> +++ b/kernel/irq/chip.c
>>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct
>>> irq_data *data, void *vcpu_info)
>>>    int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>>    {
>>>        data = data->parent_data;
>>> +
>>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>>> +        return 0;
>>> +
>>
>> [Nit] I think the irq core can access data->chip directly. Either way,
>> it's better to be consistent, the statement following doesn't use helper
>> function.
>
> thanks. I'll change it to:
> 	if (data->chip->flags & IRQCHIP_SKIP_SET_WAKE)
> 		return 0;
>
>>
>> Otherwise looks good to me.
>
> Does it means that I can add your Reviewed-by: with above change?
>

Yes you can.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-13 10:33         ` Sudeep Holla
  0 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13 10:33 UTC (permalink / raw)
  To: linux-arm-kernel



On 13/08/15 10:51, Grygorii Strashko wrote:
> On 08/13/2015 11:54 AM, Sudeep Holla wrote:
>>
>>
>> On 12/08/15 18:45, Grygorii Strashko wrote:

[...]

>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>> index 6de638b..bdb1b9d 100644
>>> --- a/kernel/irq/chip.c
>>> +++ b/kernel/irq/chip.c
>>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct
>>> irq_data *data, void *vcpu_info)
>>>    int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>>    {
>>>        data = data->parent_data;
>>> +
>>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>>> +        return 0;
>>> +
>>
>> [Nit] I think the irq core can access data->chip directly. Either way,
>> it's better to be consistent, the statement following doesn't use helper
>> function.
>
> thanks. I'll change it to:
> 	if (data->chip->flags & IRQCHIP_SKIP_SET_WAKE)
> 		return 0;
>
>>
>> Otherwise looks good to me.
>
> Does it means that I can add your Reviewed-by: with above change?
>

Yes you can.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 6/6] irqchip: crossbar: fix irq masking at suspend
  2015-08-13 10:17       ` Grygorii Strashko
  (?)
@ 2015-08-13 11:04         ` Sudeep Holla
  -1 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13 11:04 UTC (permalink / raw)
  To: Grygorii Strashko, Marc Zyngier
  Cc: Sudeep Holla, tglx, tony, linux, jason, nsekhar, linux-kernel,
	balbi, linux-omap, linux-arm-kernel



On 13/08/15 11:17, Grygorii Strashko wrote:
> On 08/13/2015 12:30 PM, Sudeep Holla wrote:
>>
>>
>> On 12/08/15 18:46, Grygorii Strashko wrote:
>>> All ARM GIC IRQs have to masked during suspend if they are not
>>> wakeup source. Now this is not happen, since switching to
>>> use IRQ domain hierarchy, because suspend_device_irq() only checks flags
>>> in the last IRQ chip in hierarchy for IRQCHIP_MASK_ON_SUSPEND
>>> bit set. And in the case of TI OMAP DRA7 the last IRQ chip is TI Crossbar
>>> which do not have this flag set.
>>>
>>> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
>>>     ARM GIC <- OMAP wakeupgen <- TI CBAR
>>> ARM GIC - IRQCHIP_MASK_ON_SUSPEND=n
>>
>> May be this won't affect your platform or this patch but even GIC marks
>> IRQCHIP_MASK_ON_SUSPEND=y now since GIC doesn't provide any facility to
>> configure the wakeup source and keeps all the interrupt source enabled.
>
> That's true for next, bur not true for 4.2-rc6 or 4.1 :(
>

True, I just wanted to ensure there is no assumption.

[...]
>>
>> Also the beginning of the commit message contradicts when you also say
>> in the following statement IRQCHIP_MASK_ON_SUSPEND=n. So you may need to
>> update the log.
>
> I'll try to reword. What I've tried to mention that IRQs masking on
> suspend is default expected behavior and that how it was before
> switching to IRQ domain hierarchy.
>
> "All ARM GIC IRQs have to masked during suspend if they are not
>   wakeup source - this is expected behavior and that's how it was before
>   switching to IRQ domain hierarchy. ..."
> ok?
>

My mistake I referred the code after it was converted to use stack
domains. So I missed to understand that you were using gic_arch_extn
flags before to override GIC flags as gic_set_irqchip_flags was not
used when I removed it. Sorry for the noise, you can retain the commit
log as is.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 6/6] irqchip: crossbar: fix irq masking at suspend
@ 2015-08-13 11:04         ` Sudeep Holla
  0 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13 11:04 UTC (permalink / raw)
  To: Grygorii Strashko, Marc Zyngier
  Cc: Sudeep Holla, tglx, tony, linux, jason, nsekhar, linux-kernel,
	balbi, linux-omap, linux-arm-kernel



On 13/08/15 11:17, Grygorii Strashko wrote:
> On 08/13/2015 12:30 PM, Sudeep Holla wrote:
>>
>>
>> On 12/08/15 18:46, Grygorii Strashko wrote:
>>> All ARM GIC IRQs have to masked during suspend if they are not
>>> wakeup source. Now this is not happen, since switching to
>>> use IRQ domain hierarchy, because suspend_device_irq() only checks flags
>>> in the last IRQ chip in hierarchy for IRQCHIP_MASK_ON_SUSPEND
>>> bit set. And in the case of TI OMAP DRA7 the last IRQ chip is TI Crossbar
>>> which do not have this flag set.
>>>
>>> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
>>>     ARM GIC <- OMAP wakeupgen <- TI CBAR
>>> ARM GIC - IRQCHIP_MASK_ON_SUSPEND=n
>>
>> May be this won't affect your platform or this patch but even GIC marks
>> IRQCHIP_MASK_ON_SUSPEND=y now since GIC doesn't provide any facility to
>> configure the wakeup source and keeps all the interrupt source enabled.
>
> That's true for next, bur not true for 4.2-rc6 or 4.1 :(
>

True, I just wanted to ensure there is no assumption.

[...]
>>
>> Also the beginning of the commit message contradicts when you also say
>> in the following statement IRQCHIP_MASK_ON_SUSPEND=n. So you may need to
>> update the log.
>
> I'll try to reword. What I've tried to mention that IRQs masking on
> suspend is default expected behavior and that how it was before
> switching to IRQ domain hierarchy.
>
> "All ARM GIC IRQs have to masked during suspend if they are not
>   wakeup source - this is expected behavior and that's how it was before
>   switching to IRQ domain hierarchy. ..."
> ok?
>

My mistake I referred the code after it was converted to use stack
domains. So I missed to understand that you were using gic_arch_extn
flags before to override GIC flags as gic_set_irqchip_flags was not
used when I removed it. Sorry for the noise, you can retain the commit
log as is.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 6/6] irqchip: crossbar: fix irq masking at suspend
@ 2015-08-13 11:04         ` Sudeep Holla
  0 siblings, 0 replies; 65+ messages in thread
From: Sudeep Holla @ 2015-08-13 11:04 UTC (permalink / raw)
  To: linux-arm-kernel



On 13/08/15 11:17, Grygorii Strashko wrote:
> On 08/13/2015 12:30 PM, Sudeep Holla wrote:
>>
>>
>> On 12/08/15 18:46, Grygorii Strashko wrote:
>>> All ARM GIC IRQs have to masked during suspend if they are not
>>> wakeup source. Now this is not happen, since switching to
>>> use IRQ domain hierarchy, because suspend_device_irq() only checks flags
>>> in the last IRQ chip in hierarchy for IRQCHIP_MASK_ON_SUSPEND
>>> bit set. And in the case of TI OMAP DRA7 the last IRQ chip is TI Crossbar
>>> which do not have this flag set.
>>>
>>> In case of TI OMAP DRA7 the following IRQ hierarchy is defined:
>>>     ARM GIC <- OMAP wakeupgen <- TI CBAR
>>> ARM GIC - IRQCHIP_MASK_ON_SUSPEND=n
>>
>> May be this won't affect your platform or this patch but even GIC marks
>> IRQCHIP_MASK_ON_SUSPEND=y now since GIC doesn't provide any facility to
>> configure the wakeup source and keeps all the interrupt source enabled.
>
> That's true for next, bur not true for 4.2-rc6 or 4.1 :(
>

True, I just wanted to ensure there is no assumption.

[...]
>>
>> Also the beginning of the commit message contradicts when you also say
>> in the following statement IRQCHIP_MASK_ON_SUSPEND=n. So you may need to
>> update the log.
>
> I'll try to reword. What I've tried to mention that IRQs masking on
> suspend is default expected behavior and that how it was before
> switching to IRQ domain hierarchy.
>
> "All ARM GIC IRQs have to masked during suspend if they are not
>   wakeup source - this is expected behavior and that's how it was before
>   switching to IRQ domain hierarchy. ..."
> ok?
>

My mistake I referred the code after it was converted to use stack
domains. So I missed to understand that you were using gic_arch_extn
flags before to override GIC flags as gic_set_irqchip_flags was not
used when I removed it. Sorry for the noise, you can retain the commit
log as is.

Regards,
Sudeep

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
  2015-08-13 10:31       ` Grygorii Strashko
  (?)
@ 2015-08-13 12:58         ` Grygorii Strashko
  -1 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13 12:58 UTC (permalink / raw)
  To: Marc Zyngier, tglx, tony
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel, linux-kernel

On 08/13/2015 01:31 PM, Grygorii Strashko wrote:
> On 08/13/2015 01:01 PM, Marc Zyngier wrote:
>> On 12/08/15 18:45, Grygorii Strashko wrote:
>>> The irqchip_set_wake_parent should not fail if IRQ chip
>>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>>> configuration can't be propagated properly through IRQ
>>> domains hierarchy.
>>>
>>> In case of TI OMAP DRA7 the issue reproduced with following
>>> configuration:
>>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>>
>>> gpio_key is wakeup source
>>>
>>> Failure is reproduced during suspend/resume to RAM:
>>> suspend:
>>>   - gpio_keys_suspend
>>>     enable_irq_wake
>>>       + pcf857x_irq_set_wake
>>>         + omap_gpio_wake_enable
>>>           + TI CBAR irq_chip_set_wake_parent
>>>             + OMAP wakeupgen has no .irq_set_wake()
>>
>> Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.
>>
>>>             and -ENOSYS will be returned
>>>
>>> resume:
>>>   - gpio_keys_resume
>>>     + disable_irq_wake
>>>       + irq_set_irq_wake
>>>         + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>>
>>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> ---
>>>   kernel/irq/chip.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>> index 6de638b..bdb1b9d 100644
>>> --- a/kernel/irq/chip.c
>>> +++ b/kernel/irq/chip.c
>>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct 
>>> irq_data *data, void *vcpu_info)
>>>   int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>>   {
>>>       data = data->parent_data;
>>> +
>>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>>> +        return 0;
>>> +
>>>       if (data->chip->irq_set_wake)
>>>           return data->chip->irq_set_wake(data, on);
>>>
>>>
>>
>> We have a more general issue with chip flags, and how they combine
>> within a stack of irqchips.
> 
> Indeed. Problem looks similar to IRQCHIP_MASK_ON_SUSPEND flag usage.
> 
>>
>> What if you remove the irq_chip_set_wake_parent from the crossbar
>> driver, and instead set IRQCHIP_SKIP_SET_WAKE?
> 
> I've thought about this and it should work for me.
> One question - what if crossbar will be not the last one in
> IRQ domains hierarchy?
> 

I can confirm, if I revert this patch, add IRQCHIP_SKIP_SET_WAKE to
the crossbar and remove irq_chip_set_wake_parent wakeups still works.
What do you prefer me to do: add additional patch for the crossbar,
drop/keep this patch?

-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-13 12:58         ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13 12:58 UTC (permalink / raw)
  To: Marc Zyngier, tglx, tony
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel, linux-kernel

On 08/13/2015 01:31 PM, Grygorii Strashko wrote:
> On 08/13/2015 01:01 PM, Marc Zyngier wrote:
>> On 12/08/15 18:45, Grygorii Strashko wrote:
>>> The irqchip_set_wake_parent should not fail if IRQ chip
>>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>>> configuration can't be propagated properly through IRQ
>>> domains hierarchy.
>>>
>>> In case of TI OMAP DRA7 the issue reproduced with following
>>> configuration:
>>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>>
>>> gpio_key is wakeup source
>>>
>>> Failure is reproduced during suspend/resume to RAM:
>>> suspend:
>>>   - gpio_keys_suspend
>>>     enable_irq_wake
>>>       + pcf857x_irq_set_wake
>>>         + omap_gpio_wake_enable
>>>           + TI CBAR irq_chip_set_wake_parent
>>>             + OMAP wakeupgen has no .irq_set_wake()
>>
>> Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.
>>
>>>             and -ENOSYS will be returned
>>>
>>> resume:
>>>   - gpio_keys_resume
>>>     + disable_irq_wake
>>>       + irq_set_irq_wake
>>>         + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>>
>>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> ---
>>>   kernel/irq/chip.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>> index 6de638b..bdb1b9d 100644
>>> --- a/kernel/irq/chip.c
>>> +++ b/kernel/irq/chip.c
>>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct 
>>> irq_data *data, void *vcpu_info)
>>>   int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>>   {
>>>       data = data->parent_data;
>>> +
>>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>>> +        return 0;
>>> +
>>>       if (data->chip->irq_set_wake)
>>>           return data->chip->irq_set_wake(data, on);
>>>
>>>
>>
>> We have a more general issue with chip flags, and how they combine
>> within a stack of irqchips.
> 
> Indeed. Problem looks similar to IRQCHIP_MASK_ON_SUSPEND flag usage.
> 
>>
>> What if you remove the irq_chip_set_wake_parent from the crossbar
>> driver, and instead set IRQCHIP_SKIP_SET_WAKE?
> 
> I've thought about this and it should work for me.
> One question - what if crossbar will be not the last one in
> IRQ domains hierarchy?
> 

I can confirm, if I revert this patch, add IRQCHIP_SKIP_SET_WAKE to
the crossbar and remove irq_chip_set_wake_parent wakeups still works.
What do you prefer me to do: add additional patch for the crossbar,
drop/keep this patch?

-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-13 12:58         ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-13 12:58 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/13/2015 01:31 PM, Grygorii Strashko wrote:
> On 08/13/2015 01:01 PM, Marc Zyngier wrote:
>> On 12/08/15 18:45, Grygorii Strashko wrote:
>>> The irqchip_set_wake_parent should not fail if IRQ chip
>>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>>> configuration can't be propagated properly through IRQ
>>> domains hierarchy.
>>>
>>> In case of TI OMAP DRA7 the issue reproduced with following
>>> configuration:
>>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>>
>>> gpio_key is wakeup source
>>>
>>> Failure is reproduced during suspend/resume to RAM:
>>> suspend:
>>>   - gpio_keys_suspend
>>>     enable_irq_wake
>>>       + pcf857x_irq_set_wake
>>>         + omap_gpio_wake_enable
>>>           + TI CBAR irq_chip_set_wake_parent
>>>             + OMAP wakeupgen has no .irq_set_wake()
>>
>> Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.
>>
>>>             and -ENOSYS will be returned
>>>
>>> resume:
>>>   - gpio_keys_resume
>>>     + disable_irq_wake
>>>       + irq_set_irq_wake
>>>         + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>>
>>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> ---
>>>   kernel/irq/chip.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>> index 6de638b..bdb1b9d 100644
>>> --- a/kernel/irq/chip.c
>>> +++ b/kernel/irq/chip.c
>>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct 
>>> irq_data *data, void *vcpu_info)
>>>   int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>>   {
>>>       data = data->parent_data;
>>> +
>>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>>> +        return 0;
>>> +
>>>       if (data->chip->irq_set_wake)
>>>           return data->chip->irq_set_wake(data, on);
>>>
>>>
>>
>> We have a more general issue with chip flags, and how they combine
>> within a stack of irqchips.
> 
> Indeed. Problem looks similar to IRQCHIP_MASK_ON_SUSPEND flag usage.
> 
>>
>> What if you remove the irq_chip_set_wake_parent from the crossbar
>> driver, and instead set IRQCHIP_SKIP_SET_WAKE?
> 
> I've thought about this and it should work for me.
> One question - what if crossbar will be not the last one in
> IRQ domains hierarchy?
> 

I can confirm, if I revert this patch, add IRQCHIP_SKIP_SET_WAKE to
the crossbar and remove irq_chip_set_wake_parent wakeups still works.
What do you prefer me to do: add additional patch for the crossbar,
drop/keep this patch?

-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
  2015-08-13 12:58         ` Grygorii Strashko
  (?)
@ 2015-08-14 10:18           ` Grygorii Strashko
  -1 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-14 10:18 UTC (permalink / raw)
  To: Marc Zyngier, tglx, tony, sudeep.holla
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel, linux-kernel

On 08/13/2015 03:58 PM, Grygorii Strashko wrote:
> On 08/13/2015 01:31 PM, Grygorii Strashko wrote:
>> On 08/13/2015 01:01 PM, Marc Zyngier wrote:
>>> On 12/08/15 18:45, Grygorii Strashko wrote:
>>>> The irqchip_set_wake_parent should not fail if IRQ chip
>>>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>>>> configuration can't be propagated properly through IRQ
>>>> domains hierarchy.
>>>>
>>>> In case of TI OMAP DRA7 the issue reproduced with following
>>>> configuration:
>>>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>>>
>>>> gpio_key is wakeup source
>>>>
>>>> Failure is reproduced during suspend/resume to RAM:
>>>> suspend:
>>>>    - gpio_keys_suspend
>>>>      enable_irq_wake
>>>>        + pcf857x_irq_set_wake
>>>>          + omap_gpio_wake_enable
>>>>            + TI CBAR irq_chip_set_wake_parent
>>>>              + OMAP wakeupgen has no .irq_set_wake()
>>>
>>> Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.
>>>
>>>>              and -ENOSYS will be returned
>>>>
>>>> resume:
>>>>    - gpio_keys_resume
>>>>      + disable_irq_wake
>>>>        + irq_set_irq_wake
>>>>          + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>>>
>>>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>> ---
>>>>    kernel/irq/chip.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>>> index 6de638b..bdb1b9d 100644
>>>> --- a/kernel/irq/chip.c
>>>> +++ b/kernel/irq/chip.c
>>>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct
>>>> irq_data *data, void *vcpu_info)
>>>>    int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>>>    {
>>>>        data = data->parent_data;
>>>> +
>>>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>>>> +        return 0;
>>>> +
>>>>        if (data->chip->irq_set_wake)
>>>>            return data->chip->irq_set_wake(data, on);
>>>>
>>>>
>>>
>>> We have a more general issue with chip flags, and how they combine
>>> within a stack of irqchips.
>>
>> Indeed. Problem looks similar to IRQCHIP_MASK_ON_SUSPEND flag usage.
>>
>>>
>>> What if you remove the irq_chip_set_wake_parent from the crossbar
>>> driver, and instead set IRQCHIP_SKIP_SET_WAKE?
>>
>> I've thought about this and it should work for me.
>> One question - what if crossbar will be not the last one in
>> IRQ domains hierarchy?
>>
> 
> I can confirm, if I revert this patch, add IRQCHIP_SKIP_SET_WAKE to
> the crossbar and remove irq_chip_set_wake_parent wakeups still works.
> What do you prefer me to do: add additional patch for the crossbar,
> drop/keep this patch?
> 

OK. There are two possibilities to fix set_wake functionality for TI OMAPs
where below HW configurations are used:
  OMAP4/5: GIC <- OMAP wakeupgen
  DRA7: GIC <- OMAP wakeupgen <- TI CBAR


1) ensure that IRQCHIP_SKIP_SET_WAKE flag is set only for GIC and
use irq_chip_set_wake_parent() in both wakeupgen and crossbar 
 [this patch is required]

2) ensure that IRQCHIP_SKIP_SET_WAKE flag is set and drop 
.irq_set_wake()/irq_chip_set_wake_parent() for all IRQ chips
in IRQ domains hierarchy.
 [this patch can be dropped]

I'm going to select approach 2 and re-send.


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-14 10:18           ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-14 10:18 UTC (permalink / raw)
  To: Marc Zyngier, tglx, tony, sudeep.holla
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel, linux-kernel

On 08/13/2015 03:58 PM, Grygorii Strashko wrote:
> On 08/13/2015 01:31 PM, Grygorii Strashko wrote:
>> On 08/13/2015 01:01 PM, Marc Zyngier wrote:
>>> On 12/08/15 18:45, Grygorii Strashko wrote:
>>>> The irqchip_set_wake_parent should not fail if IRQ chip
>>>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>>>> configuration can't be propagated properly through IRQ
>>>> domains hierarchy.
>>>>
>>>> In case of TI OMAP DRA7 the issue reproduced with following
>>>> configuration:
>>>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>>>
>>>> gpio_key is wakeup source
>>>>
>>>> Failure is reproduced during suspend/resume to RAM:
>>>> suspend:
>>>>    - gpio_keys_suspend
>>>>      enable_irq_wake
>>>>        + pcf857x_irq_set_wake
>>>>          + omap_gpio_wake_enable
>>>>            + TI CBAR irq_chip_set_wake_parent
>>>>              + OMAP wakeupgen has no .irq_set_wake()
>>>
>>> Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.
>>>
>>>>              and -ENOSYS will be returned
>>>>
>>>> resume:
>>>>    - gpio_keys_resume
>>>>      + disable_irq_wake
>>>>        + irq_set_irq_wake
>>>>          + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>>>
>>>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>> ---
>>>>    kernel/irq/chip.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>>> index 6de638b..bdb1b9d 100644
>>>> --- a/kernel/irq/chip.c
>>>> +++ b/kernel/irq/chip.c
>>>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct
>>>> irq_data *data, void *vcpu_info)
>>>>    int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>>>    {
>>>>        data = data->parent_data;
>>>> +
>>>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>>>> +        return 0;
>>>> +
>>>>        if (data->chip->irq_set_wake)
>>>>            return data->chip->irq_set_wake(data, on);
>>>>
>>>>
>>>
>>> We have a more general issue with chip flags, and how they combine
>>> within a stack of irqchips.
>>
>> Indeed. Problem looks similar to IRQCHIP_MASK_ON_SUSPEND flag usage.
>>
>>>
>>> What if you remove the irq_chip_set_wake_parent from the crossbar
>>> driver, and instead set IRQCHIP_SKIP_SET_WAKE?
>>
>> I've thought about this and it should work for me.
>> One question - what if crossbar will be not the last one in
>> IRQ domains hierarchy?
>>
> 
> I can confirm, if I revert this patch, add IRQCHIP_SKIP_SET_WAKE to
> the crossbar and remove irq_chip_set_wake_parent wakeups still works.
> What do you prefer me to do: add additional patch for the crossbar,
> drop/keep this patch?
> 

OK. There are two possibilities to fix set_wake functionality for TI OMAPs
where below HW configurations are used:
  OMAP4/5: GIC <- OMAP wakeupgen
  DRA7: GIC <- OMAP wakeupgen <- TI CBAR


1) ensure that IRQCHIP_SKIP_SET_WAKE flag is set only for GIC and
use irq_chip_set_wake_parent() in both wakeupgen and crossbar 
 [this patch is required]

2) ensure that IRQCHIP_SKIP_SET_WAKE flag is set and drop 
.irq_set_wake()/irq_chip_set_wake_parent() for all IRQ chips
in IRQ domains hierarchy.
 [this patch can be dropped]

I'm going to select approach 2 and re-send.


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-14 10:18           ` Grygorii Strashko
  0 siblings, 0 replies; 65+ messages in thread
From: Grygorii Strashko @ 2015-08-14 10:18 UTC (permalink / raw)
  To: linux-arm-kernel

On 08/13/2015 03:58 PM, Grygorii Strashko wrote:
> On 08/13/2015 01:31 PM, Grygorii Strashko wrote:
>> On 08/13/2015 01:01 PM, Marc Zyngier wrote:
>>> On 12/08/15 18:45, Grygorii Strashko wrote:
>>>> The irqchip_set_wake_parent should not fail if IRQ chip
>>>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>>>> configuration can't be propagated properly through IRQ
>>>> domains hierarchy.
>>>>
>>>> In case of TI OMAP DRA7 the issue reproduced with following
>>>> configuration:
>>>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>>>
>>>> gpio_key is wakeup source
>>>>
>>>> Failure is reproduced during suspend/resume to RAM:
>>>> suspend:
>>>>    - gpio_keys_suspend
>>>>      enable_irq_wake
>>>>        + pcf857x_irq_set_wake
>>>>          + omap_gpio_wake_enable
>>>>            + TI CBAR irq_chip_set_wake_parent
>>>>              + OMAP wakeupgen has no .irq_set_wake()
>>>
>>> Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.
>>>
>>>>              and -ENOSYS will be returned
>>>>
>>>> resume:
>>>>    - gpio_keys_resume
>>>>      + disable_irq_wake
>>>>        + irq_set_irq_wake
>>>>          + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>>>
>>>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>> ---
>>>>    kernel/irq/chip.c | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>>> index 6de638b..bdb1b9d 100644
>>>> --- a/kernel/irq/chip.c
>>>> +++ b/kernel/irq/chip.c
>>>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct
>>>> irq_data *data, void *vcpu_info)
>>>>    int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>>>    {
>>>>        data = data->parent_data;
>>>> +
>>>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>>>> +        return 0;
>>>> +
>>>>        if (data->chip->irq_set_wake)
>>>>            return data->chip->irq_set_wake(data, on);
>>>>
>>>>
>>>
>>> We have a more general issue with chip flags, and how they combine
>>> within a stack of irqchips.
>>
>> Indeed. Problem looks similar to IRQCHIP_MASK_ON_SUSPEND flag usage.
>>
>>>
>>> What if you remove the irq_chip_set_wake_parent from the crossbar
>>> driver, and instead set IRQCHIP_SKIP_SET_WAKE?
>>
>> I've thought about this and it should work for me.
>> One question - what if crossbar will be not the last one in
>> IRQ domains hierarchy?
>>
> 
> I can confirm, if I revert this patch, add IRQCHIP_SKIP_SET_WAKE to
> the crossbar and remove irq_chip_set_wake_parent wakeups still works.
> What do you prefer me to do: add additional patch for the crossbar,
> drop/keep this patch?
> 

OK. There are two possibilities to fix set_wake functionality for TI OMAPs
where below HW configurations are used:
  OMAP4/5: GIC <- OMAP wakeupgen
  DRA7: GIC <- OMAP wakeupgen <- TI CBAR


1) ensure that IRQCHIP_SKIP_SET_WAKE flag is set only for GIC and
use irq_chip_set_wake_parent() in both wakeupgen and crossbar 
 [this patch is required]

2) ensure that IRQCHIP_SKIP_SET_WAKE flag is set and drop 
.irq_set_wake()/irq_chip_set_wake_parent() for all IRQ chips
in IRQ domains hierarchy.
 [this patch can be dropped]

I'm going to select approach 2 and re-send.


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
  2015-08-14 10:18           ` Grygorii Strashko
  (?)
@ 2015-08-14 10:39             ` Marc Zyngier
  -1 siblings, 0 replies; 65+ messages in thread
From: Marc Zyngier @ 2015-08-14 10:39 UTC (permalink / raw)
  To: Grygorii Strashko, tglx, tony, Sudeep Holla
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel, linux-kernel

On 14/08/15 11:18, Grygorii Strashko wrote:
> On 08/13/2015 03:58 PM, Grygorii Strashko wrote:
>> On 08/13/2015 01:31 PM, Grygorii Strashko wrote:
>>> On 08/13/2015 01:01 PM, Marc Zyngier wrote:
>>>> On 12/08/15 18:45, Grygorii Strashko wrote:
>>>>> The irqchip_set_wake_parent should not fail if IRQ chip
>>>>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>>>>> configuration can't be propagated properly through IRQ
>>>>> domains hierarchy.
>>>>>
>>>>> In case of TI OMAP DRA7 the issue reproduced with following
>>>>> configuration:
>>>>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>>>>
>>>>> gpio_key is wakeup source
>>>>>
>>>>> Failure is reproduced during suspend/resume to RAM:
>>>>> suspend:
>>>>>    - gpio_keys_suspend
>>>>>      enable_irq_wake
>>>>>        + pcf857x_irq_set_wake
>>>>>          + omap_gpio_wake_enable
>>>>>            + TI CBAR irq_chip_set_wake_parent
>>>>>              + OMAP wakeupgen has no .irq_set_wake()
>>>>
>>>> Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.
>>>>
>>>>>              and -ENOSYS will be returned
>>>>>
>>>>> resume:
>>>>>    - gpio_keys_resume
>>>>>      + disable_irq_wake
>>>>>        + irq_set_irq_wake
>>>>>          + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>>>>
>>>>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>>> ---
>>>>>    kernel/irq/chip.c | 4 ++++
>>>>>    1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>>>> index 6de638b..bdb1b9d 100644
>>>>> --- a/kernel/irq/chip.c
>>>>> +++ b/kernel/irq/chip.c
>>>>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct
>>>>> irq_data *data, void *vcpu_info)
>>>>>    int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>>>>    {
>>>>>        data = data->parent_data;
>>>>> +
>>>>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>>>>> +        return 0;
>>>>> +
>>>>>        if (data->chip->irq_set_wake)
>>>>>            return data->chip->irq_set_wake(data, on);
>>>>>
>>>>>
>>>>
>>>> We have a more general issue with chip flags, and how they combine
>>>> within a stack of irqchips.
>>>
>>> Indeed. Problem looks similar to IRQCHIP_MASK_ON_SUSPEND flag usage.
>>>
>>>>
>>>> What if you remove the irq_chip_set_wake_parent from the crossbar
>>>> driver, and instead set IRQCHIP_SKIP_SET_WAKE?
>>>
>>> I've thought about this and it should work for me.
>>> One question - what if crossbar will be not the last one in
>>> IRQ domains hierarchy?
>>>
>>
>> I can confirm, if I revert this patch, add IRQCHIP_SKIP_SET_WAKE to
>> the crossbar and remove irq_chip_set_wake_parent wakeups still works.
>> What do you prefer me to do: add additional patch for the crossbar,
>> drop/keep this patch?
>>
> 
> OK. There are two possibilities to fix set_wake functionality for TI OMAPs
> where below HW configurations are used:
>   OMAP4/5: GIC <- OMAP wakeupgen
>   DRA7: GIC <- OMAP wakeupgen <- TI CBAR
> 
> 
> 1) ensure that IRQCHIP_SKIP_SET_WAKE flag is set only for GIC and
> use irq_chip_set_wake_parent() in both wakeupgen and crossbar 
>  [this patch is required]
> 
> 2) ensure that IRQCHIP_SKIP_SET_WAKE flag is set and drop 
> .irq_set_wake()/irq_chip_set_wake_parent() for all IRQ chips
> in IRQ domains hierarchy.
>  [this patch can be dropped]
> 
> I'm going to select approach 2 and re-send.

Yeah, I'd like to go for the minimal approach for now, and work out what
exactly are the propagation semantics (I had something at some point,
need to find what I did with those patches...).

Thanks,

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

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-14 10:39             ` Marc Zyngier
  0 siblings, 0 replies; 65+ messages in thread
From: Marc Zyngier @ 2015-08-14 10:39 UTC (permalink / raw)
  To: Grygorii Strashko, tglx, tony, Sudeep Holla
  Cc: linux, nsekhar, jason, balbi, linux-omap, linux-arm-kernel, linux-kernel

On 14/08/15 11:18, Grygorii Strashko wrote:
> On 08/13/2015 03:58 PM, Grygorii Strashko wrote:
>> On 08/13/2015 01:31 PM, Grygorii Strashko wrote:
>>> On 08/13/2015 01:01 PM, Marc Zyngier wrote:
>>>> On 12/08/15 18:45, Grygorii Strashko wrote:
>>>>> The irqchip_set_wake_parent should not fail if IRQ chip
>>>>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>>>>> configuration can't be propagated properly through IRQ
>>>>> domains hierarchy.
>>>>>
>>>>> In case of TI OMAP DRA7 the issue reproduced with following
>>>>> configuration:
>>>>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>>>>
>>>>> gpio_key is wakeup source
>>>>>
>>>>> Failure is reproduced during suspend/resume to RAM:
>>>>> suspend:
>>>>>    - gpio_keys_suspend
>>>>>      enable_irq_wake
>>>>>        + pcf857x_irq_set_wake
>>>>>          + omap_gpio_wake_enable
>>>>>            + TI CBAR irq_chip_set_wake_parent
>>>>>              + OMAP wakeupgen has no .irq_set_wake()
>>>>
>>>> Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.
>>>>
>>>>>              and -ENOSYS will be returned
>>>>>
>>>>> resume:
>>>>>    - gpio_keys_resume
>>>>>      + disable_irq_wake
>>>>>        + irq_set_irq_wake
>>>>>          + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>>>>
>>>>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>>> ---
>>>>>    kernel/irq/chip.c | 4 ++++
>>>>>    1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>>>> index 6de638b..bdb1b9d 100644
>>>>> --- a/kernel/irq/chip.c
>>>>> +++ b/kernel/irq/chip.c
>>>>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct
>>>>> irq_data *data, void *vcpu_info)
>>>>>    int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>>>>    {
>>>>>        data = data->parent_data;
>>>>> +
>>>>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>>>>> +        return 0;
>>>>> +
>>>>>        if (data->chip->irq_set_wake)
>>>>>            return data->chip->irq_set_wake(data, on);
>>>>>
>>>>>
>>>>
>>>> We have a more general issue with chip flags, and how they combine
>>>> within a stack of irqchips.
>>>
>>> Indeed. Problem looks similar to IRQCHIP_MASK_ON_SUSPEND flag usage.
>>>
>>>>
>>>> What if you remove the irq_chip_set_wake_parent from the crossbar
>>>> driver, and instead set IRQCHIP_SKIP_SET_WAKE?
>>>
>>> I've thought about this and it should work for me.
>>> One question - what if crossbar will be not the last one in
>>> IRQ domains hierarchy?
>>>
>>
>> I can confirm, if I revert this patch, add IRQCHIP_SKIP_SET_WAKE to
>> the crossbar and remove irq_chip_set_wake_parent wakeups still works.
>> What do you prefer me to do: add additional patch for the crossbar,
>> drop/keep this patch?
>>
> 
> OK. There are two possibilities to fix set_wake functionality for TI OMAPs
> where below HW configurations are used:
>   OMAP4/5: GIC <- OMAP wakeupgen
>   DRA7: GIC <- OMAP wakeupgen <- TI CBAR
> 
> 
> 1) ensure that IRQCHIP_SKIP_SET_WAKE flag is set only for GIC and
> use irq_chip_set_wake_parent() in both wakeupgen and crossbar 
>  [this patch is required]
> 
> 2) ensure that IRQCHIP_SKIP_SET_WAKE flag is set and drop 
> .irq_set_wake()/irq_chip_set_wake_parent() for all IRQ chips
> in IRQ domains hierarchy.
>  [this patch can be dropped]
> 
> I'm going to select approach 2 and re-send.

Yeah, I'd like to go for the minimal approach for now, and work out what
exactly are the propagation semantics (I had something at some point,
need to find what I did with those patches...).

Thanks,

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

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE
@ 2015-08-14 10:39             ` Marc Zyngier
  0 siblings, 0 replies; 65+ messages in thread
From: Marc Zyngier @ 2015-08-14 10:39 UTC (permalink / raw)
  To: linux-arm-kernel

On 14/08/15 11:18, Grygorii Strashko wrote:
> On 08/13/2015 03:58 PM, Grygorii Strashko wrote:
>> On 08/13/2015 01:31 PM, Grygorii Strashko wrote:
>>> On 08/13/2015 01:01 PM, Marc Zyngier wrote:
>>>> On 12/08/15 18:45, Grygorii Strashko wrote:
>>>>> The irqchip_set_wake_parent should not fail if IRQ chip
>>>>> specifies IRQCHIP_SKIP_SET_WAKE. Otherwise, IRQ wakeup
>>>>> configuration can't be propagated properly through IRQ
>>>>> domains hierarchy.
>>>>>
>>>>> In case of TI OMAP DRA7 the issue reproduced with following
>>>>> configuration:
>>>>> ARM GIC<-OMAP wakeupgen<-TI CBAR<-GPIO<-GPIO pcf857x<-gpio_key
>>>>>
>>>>> gpio_key is wakeup source
>>>>>
>>>>> Failure is reproduced during suspend/resume to RAM:
>>>>> suspend:
>>>>>    - gpio_keys_suspend
>>>>>      enable_irq_wake
>>>>>        + pcf857x_irq_set_wake
>>>>>          + omap_gpio_wake_enable
>>>>>            + TI CBAR irq_chip_set_wake_parent
>>>>>              + OMAP wakeupgen has no .irq_set_wake()
>>>>
>>>> Most importantly, wakeupgen has IRQCHIP_SKIP_SET_WAKE set.
>>>>
>>>>>              and -ENOSYS will be returned
>>>>>
>>>>> resume:
>>>>>    - gpio_keys_resume
>>>>>      + disable_irq_wake
>>>>>        + irq_set_irq_wake
>>>>>          + WARN(1, "Unbalanced IRQ %d wake disable\n", irq);
>>>>>
>>>>> Fixes: 08b55e2a9208 ('genirq: Add irqchip_set_wake_parent')
>>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>>> ---
>>>>>    kernel/irq/chip.c | 4 ++++
>>>>>    1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>>>> index 6de638b..bdb1b9d 100644
>>>>> --- a/kernel/irq/chip.c
>>>>> +++ b/kernel/irq/chip.c
>>>>> @@ -1024,6 +1024,10 @@ int irq_chip_set_vcpu_affinity_parent(struct
>>>>> irq_data *data, void *vcpu_info)
>>>>>    int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
>>>>>    {
>>>>>        data = data->parent_data;
>>>>> +
>>>>> +    if (irq_data_get_irq_chip(data)->flags & IRQCHIP_SKIP_SET_WAKE)
>>>>> +        return 0;
>>>>> +
>>>>>        if (data->chip->irq_set_wake)
>>>>>            return data->chip->irq_set_wake(data, on);
>>>>>
>>>>>
>>>>
>>>> We have a more general issue with chip flags, and how they combine
>>>> within a stack of irqchips.
>>>
>>> Indeed. Problem looks similar to IRQCHIP_MASK_ON_SUSPEND flag usage.
>>>
>>>>
>>>> What if you remove the irq_chip_set_wake_parent from the crossbar
>>>> driver, and instead set IRQCHIP_SKIP_SET_WAKE?
>>>
>>> I've thought about this and it should work for me.
>>> One question - what if crossbar will be not the last one in
>>> IRQ domains hierarchy?
>>>
>>
>> I can confirm, if I revert this patch, add IRQCHIP_SKIP_SET_WAKE to
>> the crossbar and remove irq_chip_set_wake_parent wakeups still works.
>> What do you prefer me to do: add additional patch for the crossbar,
>> drop/keep this patch?
>>
> 
> OK. There are two possibilities to fix set_wake functionality for TI OMAPs
> where below HW configurations are used:
>   OMAP4/5: GIC <- OMAP wakeupgen
>   DRA7: GIC <- OMAP wakeupgen <- TI CBAR
> 
> 
> 1) ensure that IRQCHIP_SKIP_SET_WAKE flag is set only for GIC and
> use irq_chip_set_wake_parent() in both wakeupgen and crossbar 
>  [this patch is required]
> 
> 2) ensure that IRQCHIP_SKIP_SET_WAKE flag is set and drop 
> .irq_set_wake()/irq_chip_set_wake_parent() for all IRQ chips
> in IRQ domains hierarchy.
>  [this patch can be dropped]
> 
> I'm going to select approach 2 and re-send.

Yeah, I'd like to go for the minimal approach for now, and work out what
exactly are the propagation semantics (I had something at some point,
need to find what I did with those patches...).

Thanks,

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

^ permalink raw reply	[flat|nested] 65+ messages in thread

end of thread, other threads:[~2015-08-14 10:40 UTC | newest]

Thread overview: 65+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-12 17:45 [PATCH v2 0/6] genirq: irqdomain_hierarchy: fixes Grygorii Strashko
2015-08-12 17:45 ` Grygorii Strashko
2015-08-12 17:45 ` Grygorii Strashko
2015-08-12 17:45 ` [PATCH v2 1/6] genirq: fix irq_chip_retrigger_hierarchy Grygorii Strashko
2015-08-12 17:45   ` Grygorii Strashko
2015-08-12 17:45   ` Grygorii Strashko
2015-08-13  9:26   ` Marc Zyngier
2015-08-13  9:26     ` Marc Zyngier
2015-08-13  9:26     ` Marc Zyngier
2015-08-12 17:45 ` [PATCH v2 2/6] genirq: fix irqchip_set_wake_parent if IRQCHIP_SKIP_SET_WAKE Grygorii Strashko
2015-08-12 17:45   ` Grygorii Strashko
2015-08-12 17:45   ` Grygorii Strashko
2015-08-13  8:54   ` Sudeep Holla
2015-08-13  8:54     ` Sudeep Holla
2015-08-13  8:54     ` Sudeep Holla
2015-08-13  9:51     ` Grygorii Strashko
2015-08-13  9:51       ` Grygorii Strashko
2015-08-13  9:51       ` Grygorii Strashko
2015-08-13 10:33       ` Sudeep Holla
2015-08-13 10:33         ` Sudeep Holla
2015-08-13 10:33         ` Sudeep Holla
2015-08-13 10:01   ` Marc Zyngier
2015-08-13 10:01     ` Marc Zyngier
2015-08-13 10:01     ` Marc Zyngier
2015-08-13 10:31     ` Grygorii Strashko
2015-08-13 10:31       ` Grygorii Strashko
2015-08-13 10:31       ` Grygorii Strashko
2015-08-13 12:58       ` Grygorii Strashko
2015-08-13 12:58         ` Grygorii Strashko
2015-08-13 12:58         ` Grygorii Strashko
2015-08-14 10:18         ` Grygorii Strashko
2015-08-14 10:18           ` Grygorii Strashko
2015-08-14 10:18           ` Grygorii Strashko
2015-08-14 10:39           ` Marc Zyngier
2015-08-14 10:39             ` Marc Zyngier
2015-08-14 10:39             ` Marc Zyngier
2015-08-12 17:45 ` [PATCH v2 3/6] genirq: introduce irq_chip_set_type_parent() helper Grygorii Strashko
2015-08-12 17:45   ` Grygorii Strashko
2015-08-12 17:45   ` Grygorii Strashko
2015-08-13  8:58   ` Sudeep Holla
2015-08-13  8:58     ` Sudeep Holla
2015-08-13  8:58     ` Sudeep Holla
2015-08-13  9:54     ` Grygorii Strashko
2015-08-13  9:54       ` Grygorii Strashko
2015-08-13  9:54       ` Grygorii Strashko
2015-08-12 17:46 ` [PATCH v2 4/6] irqchip: crossbar: fix arm gic irq type configuration Grygorii Strashko
2015-08-12 17:46   ` Grygorii Strashko
2015-08-12 17:46   ` Grygorii Strashko
2015-08-12 17:46 ` [PATCH v2 5/6] ARM: OMAP: wakeupgen: " Grygorii Strashko
2015-08-12 17:46   ` Grygorii Strashko
2015-08-12 17:46   ` Grygorii Strashko
2015-08-13  9:50   ` Tony Lindgren
2015-08-13  9:50     ` Tony Lindgren
2015-08-12 17:46 ` [PATCH v2 6/6] irqchip: crossbar: fix irq masking at suspend Grygorii Strashko
2015-08-12 17:46   ` Grygorii Strashko
2015-08-12 17:46   ` Grygorii Strashko
2015-08-13  9:30   ` Sudeep Holla
2015-08-13  9:30     ` Sudeep Holla
2015-08-13  9:30     ` Sudeep Holla
2015-08-13 10:17     ` Grygorii Strashko
2015-08-13 10:17       ` Grygorii Strashko
2015-08-13 10:17       ` Grygorii Strashko
2015-08-13 11:04       ` Sudeep Holla
2015-08-13 11:04         ` Sudeep Holla
2015-08-13 11:04         ` Sudeep Holla

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.