[PATCH 0/3] OMAP/pinmux: remove misuse of IRQF_NO_SUSPEND flag

[PATCH 0/3] OMAP/pinmux: remove misuse of IRQF_NO_SUSPEND flag

[Sudeep Holla]

Hi Tony/Grygorii,

Just revieving the old threads [1][2]. I have put all the necessary
patches together so that it can tested. Can you give it a go ?

Regards,
Sudeep

[1] https://www.spinics.net/lists/linux-gpio/msg09879.html
[2] http://lkml.iu.edu/hypermail/linux/kernel/1509.2/03937.html

Sudeep Holla (3):
  pinctrl: single: Use a separate lockdep class
  pinctrl: single: remove misuse of IRQF_NO_SUSPEND flag
  ARM: OMAP2+: remove misuse of IRQF_NO_SUSPEND flag

 arch/arm/mach-omap2/mux.c        |  4 ++--
 arch/arm/mach-omap2/pm34xx.c     |  9 ++++-----
 arch/arm/mach-omap2/prm_common.c |  1 +
 drivers/pinctrl/pinctrl-single.c | 15 ++++++++++++---
 4 files changed, 19 insertions(+), 10 deletions(-)

-- 
1.9.1

[PATCH 1/3] pinctrl: single: Use a separate lockdep class

[Sudeep Holla]

The single pinmux controller can be cascaded to the other interrupt
controllers. Hence when propagating wake-up settings to its parent
interrupt controller, there's possiblity of detecting possible recursive
locking and getting lockdep warning.

This patch avoids this false positive by using a separate lockdep class
for this single pinctrl interrupts.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-gpio@vger.kernel.org
Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/pinctrl/pinctrl-single.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index d24e5f1d1525..fb126d56ad40 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -255,6 +255,13 @@ static enum pin_config_param pcs_bias[] = {
 };
 
 /*
+ * This lock class tells lockdep that irqchip core that this single
+ * pinctrl can be in a different category than its parents, so it won't
+ * report false recursion.
+ */
+static struct lock_class_key pcs_lock_class;
+
+/*
  * REVISIT: Reads and writes could eventually use regmap or something
  * generic. But at least on omaps, some mux registers are performance
  * critical as they may need to be remuxed every time before and after
@@ -1713,6 +1720,7 @@ static int pcs_irqdomain_map(struct irq_domain *d, unsigned int irq,
 	irq_set_chip_data(irq, pcs_soc);
 	irq_set_chip_and_handler(irq, &pcs->chip,
 				 handle_level_irq);
+	irq_set_lockdep_class(irq, &pcs_lock_class);
 	irq_set_noprobe(irq);
 
 	return 0;
-- 
1.9.1

[PATCH 2/3] pinctrl: single: remove misuse of IRQF_NO_SUSPEND flag

[Sudeep Holla]

From: Sudeep Holla <Sudeep.Holla@arm.com>

The IRQF_NO_SUSPEND flag is used to identify the interrupts that should
be left enabled so as to allow them to work as expected during the
suspend-resume cycle, but doesn't guarantee that it will wake the system
from a suspended state, enable_irq_wake is recommended to be used for
the wakeup.

This patch removes the use of IRQF_NO_SUSPEND flags replacing it with
irq_set_irq_wake instead.

Cc: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: linux-gpio@vger.kernel.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/pinctrl/pinctrl-single.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index fb126d56ad40..03977a75a9e2 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -1619,12 +1619,14 @@ static void pcs_irq_unmask(struct irq_data *d)
  */
 static int pcs_irq_set_wake(struct irq_data *d, unsigned int state)
 {
+	struct pcs_soc_data *pcs_soc = irq_data_get_irq_chip_data(d);
+
 	if (state)
 		pcs_irq_unmask(d);
 	else
 		pcs_irq_mask(d);
 
-	return 0;
+	return irq_set_irq_wake(pcs_soc->irq, state);
 }
 
 /**
@@ -1760,8 +1762,7 @@ static int pcs_irq_init_chained_handler(struct pcs_device *pcs,
 		int res;
 
 		res = request_irq(pcs_soc->irq, pcs_irq_handler,
-				  IRQF_SHARED | IRQF_NO_SUSPEND |
-				  IRQF_NO_THREAD,
+				  IRQF_SHARED | IRQF_NO_THREAD,
 				  name, pcs_soc);
 		if (res) {
 			pcs_soc->irq = -1;
-- 
1.9.1

[PATCH 3/3] ARM: OMAP2+: remove misuse of IRQF_NO_SUSPEND flag

[Sudeep Holla]

The IRQF_NO_SUSPEND flag is used to identify the interrupts that should
be left enabled so as to allow them to work as expected during the
suspend-resume cycle, but doesn't guarantee that it will wake the system
from a suspended state, enable_irq_wake is recommended to be used for
the wakeup.

This patch removes the use of IRQF_NO_SUSPEND flags replacing it with
enable_irq_wake instead.

Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: linux-omap@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm/mach-omap2/mux.c        | 4 ++--
 arch/arm/mach-omap2/pm34xx.c     | 9 ++++-----
 arch/arm/mach-omap2/prm_common.c | 1 +
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
index 176eef6ef338..12012bef8e63 100644
--- a/arch/arm/mach-omap2/mux.c
+++ b/arch/arm/mach-omap2/mux.c
@@ -810,13 +810,13 @@ int __init omap_mux_late_init(void)
 		return 0;
 
 	ret = request_irq(omap_prcm_event_to_irq("io"),
-		omap_hwmod_mux_handle_irq, IRQF_SHARED | IRQF_NO_SUSPEND,
+		omap_hwmod_mux_handle_irq, IRQF_SHARED,
 			"hwmod_io", omap_mux_late_init);
 
 	if (ret)
 		pr_warn("mux: Failed to setup hwmod io irq %d\n", ret);
 
-	return 0;
+	return enable_irq_wake(omap_prcm_event_to_irq("io"));
 }
 
 static void __init omap_mux_package_fixup(struct omap_mux *p,
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 2dbd3785ee6f..49604e0023c4 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -472,23 +472,22 @@ int __init omap3_pm_init(void)
 	prcm_setup_regs();
 
 	ret = request_irq(omap_prcm_event_to_irq("wkup"),
-		_prcm_int_handle_wakeup, IRQF_NO_SUSPEND, "pm_wkup", NULL);
+		_prcm_int_handle_wakeup, 0, "pm_wkup", NULL);
 
 	if (ret) {
 		pr_err("pm: Failed to request pm_wkup irq\n");
 		goto err1;
 	}
+	enable_irq_wake(omap_prcm_event_to_irq("wkup"));
 
 	/* IO interrupt is shared with mux code */
 	ret = request_irq(omap_prcm_event_to_irq("io"),
-		_prcm_int_handle_io, IRQF_SHARED | IRQF_NO_SUSPEND, "pm_io",
-		omap3_pm_init);
-	enable_irq(omap_prcm_event_to_irq("io"));
-
+		_prcm_int_handle_io, IRQF_SHARED, "pm_io", omap3_pm_init);
 	if (ret) {
 		pr_err("pm: Failed to request pm_io irq\n");
 		goto err2;
 	}
+	enable_irq_wake(omap_prcm_event_to_irq("io"));
 
 	ret = pwrdm_for_each(pwrdms_setup, NULL);
 	if (ret) {
diff --git a/arch/arm/mach-omap2/prm_common.c b/arch/arm/mach-omap2/prm_common.c
index 5b2f5138d938..7af688273e0d 100644
--- a/arch/arm/mach-omap2/prm_common.c
+++ b/arch/arm/mach-omap2/prm_common.c
@@ -338,6 +338,7 @@ int omap_prcm_register_chain_handler(struct omap_prcm_irq_setup *irq_setup)
 		ct->chip.irq_ack = irq_gc_ack_set_bit;
 		ct->chip.irq_mask = irq_gc_mask_clr_bit;
 		ct->chip.irq_unmask = irq_gc_mask_set_bit;
+		ct->chip.flags = IRQCHIP_SKIP_SET_WAKE;
 
 		ct->regs.ack = irq_setup->ack + i * 4;
 		ct->regs.mask = irq_setup->mask + i * 4;
-- 
1.9.1

Re: [PATCH 3/3] ARM: OMAP2+: remove misuse of IRQF_NO_SUSPEND flag

[Grygorii Strashko]

Hi Sudeep,

On 02/01/2016 08:28 PM, Sudeep Holla wrote:
> The IRQF_NO_SUSPEND flag is used to identify the interrupts that should
> be left enabled so as to allow them to work as expected during the
> suspend-resume cycle, but doesn't guarantee that it will wake the system
> from a suspended state, enable_irq_wake is recommended to be used for
> the wakeup.
> 
> This patch removes the use of IRQF_NO_SUSPEND flags replacing it with
> enable_irq_wake instead.

And sorry for delayed reply - I've spent some time investigating it, but
It was during Christmas holidays and finally I lost track of it :)

So, what we have:
1) bad news: it will not work :(

The PRCM wake up is handled in the following way on OMAP3+
-- PRCM IRQ omap_prcm_irq_handler()
   |- (a) "wkup" event -> _prcm_int_handle_wakeup()
   |- "io" event (shared)
       |- (b) _prcm_int_handle_io()
       |- (c) pcs_irq_handler()
(mux is for legacy platform - can be ignored for now)

All "wkup"/"io" events must be handled in PRCM registers before
PRCM IRQ status bits can be acked and reset.

It means that all IRQs (a), (b) (c) have to be enabled at the
moment when PRCM IRQ is fired. Unfortunately this can't be 
guaranteed by IR PM core and omap_prcm_irq_handler() 
will stuck forever on Resume :(.

Resume example:
 omap_prcm_irq_handler()
[1] -> prcm_irq_setup->read_pending_irqs(pending);
    -> is there pending irqs?
	-> io event
	    -> (c) pcs_irq (enabled | wakeup src)
		-> irq_pm_check_wakeup() 
		   -> (disabled | suspended| pending)
			pcs_irq_handler() not run
   ->  goto [1] ----- OOps dead-loop
	  

2) Potential good news: It could be fixed the same way as it's done
omap34xx (and as it was before :

diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 58920bc..a0f8265 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -221,8 +221,7 @@ static int omap_pm_enter(suspend_state_t suspend_state)
 static int omap_pm_begin(suspend_state_t state)
 {
        cpu_idle_poll_ctrl(true);
-       if (cpu_is_omap34xx())
-               omap_prcm_irq_prepare();
+       omap_prcm_irq_prepare();
        return 0;
 }
 
@@ -233,8 +232,7 @@ static void omap_pm_end(void)
 
 static void omap_pm_finish(void)
 {
-       if (cpu_is_omap34xx())
-               omap_prcm_irq_complete();
+       omap_prcm_irq_complete();
 }

Another option is to convert omap_prcm_irq_handler to the generic handler
(now chained) and, probably, make it threaded and all cascaded IRQs as
nested threaded (this is just a theory).

I'll be on business trip next two weeks and will not be able to help more with it
Sorry.



> 
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: linux-omap@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   arch/arm/mach-omap2/mux.c        | 4 ++--
>   arch/arm/mach-omap2/pm34xx.c     | 9 ++++-----
>   arch/arm/mach-omap2/prm_common.c | 1 +
>   3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
> index 176eef6ef338..12012bef8e63 100644
> --- a/arch/arm/mach-omap2/mux.c
> +++ b/arch/arm/mach-omap2/mux.c
> @@ -810,13 +810,13 @@ int __init omap_mux_late_init(void)
>   		return 0;
>   
>   	ret = request_irq(omap_prcm_event_to_irq("io"),
> -		omap_hwmod_mux_handle_irq, IRQF_SHARED | IRQF_NO_SUSPEND,
> +		omap_hwmod_mux_handle_irq, IRQF_SHARED,
>   			"hwmod_io", omap_mux_late_init);
>   
>   	if (ret)
>   		pr_warn("mux: Failed to setup hwmod io irq %d\n", ret);
>   
> -	return 0;
> +	return enable_irq_wake(omap_prcm_event_to_irq("io"));
>   }
>   
>   static void __init omap_mux_package_fixup(struct omap_mux *p,
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 2dbd3785ee6f..49604e0023c4 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -472,23 +472,22 @@ int __init omap3_pm_init(void)
>   	prcm_setup_regs();
>   
>   	ret = request_irq(omap_prcm_event_to_irq("wkup"),
> -		_prcm_int_handle_wakeup, IRQF_NO_SUSPEND, "pm_wkup", NULL);
> +		_prcm_int_handle_wakeup, 0, "pm_wkup", NULL);
>   
>   	if (ret) {
>   		pr_err("pm: Failed to request pm_wkup irq\n");
>   		goto err1;
>   	}
> +	enable_irq_wake(omap_prcm_event_to_irq("wkup"));
>   
>   	/* IO interrupt is shared with mux code */
>   	ret = request_irq(omap_prcm_event_to_irq("io"),
> -		_prcm_int_handle_io, IRQF_SHARED | IRQF_NO_SUSPEND, "pm_io",
> -		omap3_pm_init);
> -	enable_irq(omap_prcm_event_to_irq("io"));
> -
> +		_prcm_int_handle_io, IRQF_SHARED, "pm_io", omap3_pm_init);
>   	if (ret) {
>   		pr_err("pm: Failed to request pm_io irq\n");
>   		goto err2;
>   	}
> +	enable_irq_wake(omap_prcm_event_to_irq("io"));
>   
>   	ret = pwrdm_for_each(pwrdms_setup, NULL);
>   	if (ret) {
> diff --git a/arch/arm/mach-omap2/prm_common.c b/arch/arm/mach-omap2/prm_common.c
> index 5b2f5138d938..7af688273e0d 100644
> --- a/arch/arm/mach-omap2/prm_common.c
> +++ b/arch/arm/mach-omap2/prm_common.c
> @@ -338,6 +338,7 @@ int omap_prcm_register_chain_handler(struct omap_prcm_irq_setup *irq_setup)
>   		ct->chip.irq_ack = irq_gc_ack_set_bit;
>   		ct->chip.irq_mask = irq_gc_mask_clr_bit;
>   		ct->chip.irq_unmask = irq_gc_mask_set_bit;
> +		ct->chip.flags = IRQCHIP_SKIP_SET_WAKE;
>   
>   		ct->regs.ack = irq_setup->ack + i * 4;
>   		ct->regs.mask = irq_setup->mask + i * 4;
> 


-- 
regards,
-grygorii

Re: [PATCH 3/3] ARM: OMAP2+: remove misuse of IRQF_NO_SUSPEND flag

[Sudeep Holla]

On 04/02/16 13:14, Grygorii Strashko wrote:
> Hi Sudeep,
>
> On 02/01/2016 08:28 PM, Sudeep Holla wrote:
>> The IRQF_NO_SUSPEND flag is used to identify the interrupts that should
>> be left enabled so as to allow them to work as expected during the
>> suspend-resume cycle, but doesn't guarantee that it will wake the system
>> from a suspended state, enable_irq_wake is recommended to be used for
>> the wakeup.
>>
>> This patch removes the use of IRQF_NO_SUSPEND flags replacing it with
>> enable_irq_wake instead.
>
> And sorry for delayed reply - I've spent some time investigating it, but
> It was during Christmas holidays and finally I lost track of it :)
>

That's fine, usually that's the case in general will all of us :)

[..]
>
> Another option is to convert omap_prcm_irq_handler to the generic handler
> (now chained) and, probably, make it threaded and all cascaded IRQs as
> nested threaded (this is just a theory).
>

Since I don't have any knowledge of OMAP, I will completely depend on
your for anything OMAP specific.

> I'll be on business trip next two weeks and will not be able to help more with it
> Sorry.
>

No problem, let me know once you get a chance to try out things at your end.

-- 
Regards,
Sudeep

Re: [PATCH 0/3] OMAP/pinmux: remove misuse of IRQF_NO_SUSPEND flag

[Linus Walleij]

On Mon, Feb 1, 2016 at 7:28 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:

>   pinctrl: single: Use a separate lockdep class
>   pinctrl: single: remove misuse of IRQF_NO_SUSPEND flag
>   ARM: OMAP2+: remove misuse of IRQF_NO_SUSPEND flag
>
>  arch/arm/mach-omap2/mux.c        |  4 ++--
>  arch/arm/mach-omap2/pm34xx.c     |  9 ++++-----
>  arch/arm/mach-omap2/prm_common.c |  1 +
>  drivers/pinctrl/pinctrl-single.c | 15 ++++++++++++---

Are these patches orthogonal so I can merge the pinctrl patch independently
of the OMAP patches if Tony ACKs this?

If not: should it be merged by me or through ARM SoC/Tony?

Anyways waiting for maintainer feedback, but the above is good to know.

Yours,
Linus Walleij

Re: [PATCH 0/3] OMAP/pinmux: remove misuse of IRQF_NO_SUSPEND flag

[Sudeep Holla]

Hi Linus,

On 13/02/16 14:42, Linus Walleij wrote:
> On Mon, Feb 1, 2016 at 7:28 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>>    pinctrl: single: Use a separate lockdep class
>>    pinctrl: single: remove misuse of IRQF_NO_SUSPEND flag
>>    ARM: OMAP2+: remove misuse of IRQF_NO_SUSPEND flag
>>
>>   arch/arm/mach-omap2/mux.c        |  4 ++--
>>   arch/arm/mach-omap2/pm34xx.c     |  9 ++++-----
>>   arch/arm/mach-omap2/prm_common.c |  1 +
>>   drivers/pinctrl/pinctrl-single.c | 15 ++++++++++++---
>
> Are these patches orthogonal so I can merge the pinctrl patch independently
> of the OMAP patches if Tony ACKs this?
>

Thanks for the follow up.
IIUC they are dependent and remember Tony/Grygorii wanted them to be
merged together(at-least for bisectibility)

> If not: should it be merged by me or through ARM SoC/Tony?
>

I think so.

> Anyways waiting for maintainer feedback, but the above is good to know.
>

Grygorii was still not sure if this works and he couldn't give it a
test. He was traveling and may need more time to get back on this.

-- 
Regards,
Sudeep

Re: [PATCH 0/3] OMAP/pinmux: remove misuse of IRQF_NO_SUSPEND flag

[Tony Lindgren]

* Sudeep Holla <sudeep.holla@arm.com> [160215 02:03]:
> Hi Linus,
> 
> On 13/02/16 14:42, Linus Walleij wrote:
> >On Mon, Feb 1, 2016 at 7:28 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> >>   pinctrl: single: Use a separate lockdep class
> >>   pinctrl: single: remove misuse of IRQF_NO_SUSPEND flag
> >>   ARM: OMAP2+: remove misuse of IRQF_NO_SUSPEND flag
> >>
> >>  arch/arm/mach-omap2/mux.c        |  4 ++--
> >>  arch/arm/mach-omap2/pm34xx.c     |  9 ++++-----
> >>  arch/arm/mach-omap2/prm_common.c |  1 +
> >>  drivers/pinctrl/pinctrl-single.c | 15 ++++++++++++---
> >
> >Are these patches orthogonal so I can merge the pinctrl patch independently
> >of the OMAP patches if Tony ACKs this?
> >
> 
> Thanks for the follow up.
> IIUC they are dependent and remember Tony/Grygorii wanted them to be
> merged together(at-least for bisectibility)

AFAIK this series needs to be kept together to keep things
working.

> >If not: should it be merged by me or through ARM SoC/Tony?
> >
> 
> I think so.
> 
> >Anyways waiting for maintainer feedback, but the above is good to know.
> >
> 
> Grygorii was still not sure if this works and he couldn't give it a
> test. He was traveling and may need more time to get back on this.

He said it does not work so clearly more work is needed.

Regards,

Tony

Re: [PATCH 0/3] OMAP/pinmux: remove misuse of IRQF_NO_SUSPEND flag

[Grygorii Strashko]

On 02/15/2016 10:27 PM, Tony Lindgren wrote:
> * Sudeep Holla <sudeep.holla@arm.com> [160215 02:03]:
>> Hi Linus,
>>
>> On 13/02/16 14:42, Linus Walleij wrote:
>>> On Mon, Feb 1, 2016 at 7:28 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>
>>>>    pinctrl: single: Use a separate lockdep class
>>>>    pinctrl: single: remove misuse of IRQF_NO_SUSPEND flag
>>>>    ARM: OMAP2+: remove misuse of IRQF_NO_SUSPEND flag
>>>>
>>>>   arch/arm/mach-omap2/mux.c        |  4 ++--
>>>>   arch/arm/mach-omap2/pm34xx.c     |  9 ++++-----
>>>>   arch/arm/mach-omap2/prm_common.c |  1 +
>>>>   drivers/pinctrl/pinctrl-single.c | 15 ++++++++++++---
>>>
>>> Are these patches orthogonal so I can merge the pinctrl patch independently
>>> of the OMAP patches if Tony ACKs this?
>>>
>>
>> Thanks for the follow up.
>> IIUC they are dependent and remember Tony/Grygorii wanted them to be
>> merged together(at-least for bisectibility)
> 
> AFAIK this series needs to be kept together to keep things
> working.
> 
>>> If not: should it be merged by me or through ARM SoC/Tony?
>>>
>>
>> I think so.
>>
>>> Anyways waiting for maintainer feedback, but the above is good to know.
>>>
>>
>> Grygorii was still not sure if this works and he couldn't give it a
>> test. He was traveling and may need more time to get back on this.
> 
> He said it does not work so clearly more work is needed.
> 

Yeah. As I mentioned before patches 2/3 will not work.
I've tried to test suspend with diff I posted [1], but I did it only on 
dra7 and am43 where suspend i more simple than on omap3. Also, I worry
that with such approach some irq can be missed (especially edge ones), 
because PRCM irqs will be re-enabled at very late resume stage 
(right before thaw).

Second option, I see, is to rework PRCM to be threaded IRQ, but in this case
all children IRQs will need to be threaded/nested threaded
(including PCS, legacy OMAP mux, OMAP3 _prcm_int_handle_wakeup/_prcm_int_handle_io).

Are there any objection to convert PCS to threaded IRQ, for example? ;)
 

[1] https://patchwork.ozlabs.org/patch/578915/

-- 
regards,
-grygorii

Re: [PATCH 1/3] pinctrl: single: Use a separate lockdep class

[Grygorii Strashko]

On 02/02/2016 01:28 AM, Sudeep Holla wrote:
> The single pinmux controller can be cascaded to the other interrupt
> controllers. Hence when propagating wake-up settings to its parent
> interrupt controller, there's possiblity of detecting possible recursive
> locking and getting lockdep warning.
> 
> This patch avoids this false positive by using a separate lockdep class
> for this single pinctrl interrupts.
> 
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: linux-gpio@vger.kernel.org
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   drivers/pinctrl/pinctrl-single.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> index d24e5f1d1525..fb126d56ad40 100644
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -255,6 +255,13 @@ static enum pin_config_param pcs_bias[] = {
>   };
>   
>   /*
> + * This lock class tells lockdep that irqchip core that this single
> + * pinctrl can be in a different category than its parents, so it won't
> + * report false recursion.
> + */
> +static struct lock_class_key pcs_lock_class;
> +
> +/*
>    * REVISIT: Reads and writes could eventually use regmap or something
>    * generic. But at least on omaps, some mux registers are performance
>    * critical as they may need to be remuxed every time before and after
> @@ -1713,6 +1720,7 @@ static int pcs_irqdomain_map(struct irq_domain *d, unsigned int irq,
>   	irq_set_chip_data(irq, pcs_soc);
>   	irq_set_chip_and_handler(irq, &pcs->chip,
>   				 handle_level_irq);
> +	irq_set_lockdep_class(irq, &pcs_lock_class);
>   	irq_set_noprobe(irq);
>   
>   	return 0;
> 

I have no objection to this patch and it's independent from other patches in
this series.
Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>  

-- 
regards,
-grygorii

Re: [PATCH 0/3] OMAP/pinmux: remove misuse of IRQF_NO_SUSPEND flag

[Tony Lindgren]

* Grygorii Strashko <grygorii.strashko@ti.com> [160308 03:32]:
> 
> Yeah. As I mentioned before patches 2/3 will not work.
> I've tried to test suspend with diff I posted [1], but I did it only on 
> dra7 and am43 where suspend i more simple than on omap3. Also, I worry
> that with such approach some irq can be missed (especially edge ones), 
> because PRCM irqs will be re-enabled at very late resume stage 
> (right before thaw).

Yeah any tests not done with PM on omap3 with the mainline kernel
are clearly incomplete in this case :)

> Second option, I see, is to rework PRCM to be threaded IRQ, but in this case
> all children IRQs will need to be threaded/nested threaded
> (including PCS, legacy OMAP mux, OMAP3 _prcm_int_handle_wakeup/_prcm_int_handle_io).
> 
> Are there any objection to convert PCS to threaded IRQ, for example? ;)

For PRCM interrupt, I don't think that can be threaded IRQ as
that thing is firing all the time. The consumers for the chained
PRCM interrupt consumers like PCS could probably use threaded IRQ.
When the PCS interrupts fire, we're already in some sleeper
idle state so the extra latency thre does not matter.

Regards,

Tony

> [1] https://patchwork.ozlabs.org/patch/578915/

Re: [PATCH 1/3] pinctrl: single: Use a separate lockdep class

[Linus Walleij]

On Tue, Feb 2, 2016 at 1:28 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:

> The single pinmux controller can be cascaded to the other interrupt
> controllers. Hence when propagating wake-up settings to its parent
> interrupt controller, there's possiblity of detecting possible recursive
> locking and getting lockdep warning.
>
> This patch avoids this false positive by using a separate lockdep class
> for this single pinctrl interrupts.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: linux-gpio@vger.kernel.org
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Patch applied with Grygorii's review tag.

Yours,
Linus Walleij