[1/2] bus: ti-sysc: Fix wakeirq sleeping function called from invalid context
diff mbox series

Message ID 20200702174929.26506-1-tony@atomide.com
State New
Headers show
Series
  • [1/2] bus: ti-sysc: Fix wakeirq sleeping function called from invalid context
Related show

Commit Message

Tony Lindgren July 2, 2020, 5:49 p.m. UTC
With CONFIG_DEBUG_ATOMIC_SLEEP enabled we can see the following with
wakeirqs and serial console idled:

BUG: sleeping function called from invalid context at drivers/bus/ti-sysc.c:242
...
(sysc_wait_softreset) from [<c0606894>] (sysc_enable_module+0x48/0x274)
(sysc_enable_module) from [<c0606c5c>] (sysc_runtime_resume+0x19c/0x1d8)
(sysc_runtime_resume) from [<c0606cf0>] (sysc_child_runtime_resume+0x58/0x84)
(sysc_child_runtime_resume) from [<c06eb7bc>] (__rpm_callback+0x30/0x12c)
(__rpm_callback) from [<c06eb8d8>] (rpm_callback+0x20/0x80)
(rpm_callback) from [<c06eb434>] (rpm_resume+0x638/0x7fc)
(rpm_resume) from [<c06eb658>] (__pm_runtime_resume+0x60/0x9c)
(__pm_runtime_resume) from [<c06edc08>] (handle_threaded_wake_irq+0x24/0x60)
(handle_threaded_wake_irq) from [<c01befec>] (irq_thread_fn+0x1c/0x78)
(irq_thread_fn) from [<c01bf30c>] (irq_thread+0x140/0x26c)

We have __pm_runtime_resume() call the sysc_runtime_resume() with spinlock
held and interrupts disabled.

Fixes: d46f9fbec719 ("bus: ti-sysc: Use optional clocks on for enable and wait for softreset bit")
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/bus/ti-sysc.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Comments

Suman Anna July 2, 2020, 6 p.m. UTC | #1
Hi Tony,

On 7/2/20 12:49 PM, Tony Lindgren wrote:
> With CONFIG_DEBUG_ATOMIC_SLEEP enabled we can see the following with
> wakeirqs and serial console idled:

Which devices are these? I have one patch from Tero fixing similar 
errors in OMAP IOMMU driver. Will post that either today or tomorrow.

regards
Suman

> 
> BUG: sleeping function called from invalid context at drivers/bus/ti-sysc.c:242
> ...
> (sysc_wait_softreset) from [<c0606894>] (sysc_enable_module+0x48/0x274)
> (sysc_enable_module) from [<c0606c5c>] (sysc_runtime_resume+0x19c/0x1d8)
> (sysc_runtime_resume) from [<c0606cf0>] (sysc_child_runtime_resume+0x58/0x84)
> (sysc_child_runtime_resume) from [<c06eb7bc>] (__rpm_callback+0x30/0x12c)
> (__rpm_callback) from [<c06eb8d8>] (rpm_callback+0x20/0x80)
> (rpm_callback) from [<c06eb434>] (rpm_resume+0x638/0x7fc)
> (rpm_resume) from [<c06eb658>] (__pm_runtime_resume+0x60/0x9c)
> (__pm_runtime_resume) from [<c06edc08>] (handle_threaded_wake_irq+0x24/0x60)
> (handle_threaded_wake_irq) from [<c01befec>] (irq_thread_fn+0x1c/0x78)
> (irq_thread_fn) from [<c01bf30c>] (irq_thread+0x140/0x26c)
> 
> We have __pm_runtime_resume() call the sysc_runtime_resume() with spinlock
> held and interrupts disabled.
> 
> Fixes: d46f9fbec719 ("bus: ti-sysc: Use optional clocks on for enable and wait for softreset bit")
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>   drivers/bus/ti-sysc.c | 13 ++++++-------
>   1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
> --- a/drivers/bus/ti-sysc.c
> +++ b/drivers/bus/ti-sysc.c
> @@ -236,15 +236,14 @@ static int sysc_wait_softreset(struct sysc *ddata)
>   		syss_done = ddata->cfg.syss_mask;
>   
>   	if (syss_offset >= 0) {
> -		error = readx_poll_timeout(sysc_read_sysstatus, ddata, rstval,
> -					   (rstval & ddata->cfg.syss_mask) ==
> -					   syss_done,
> -					   100, MAX_MODULE_SOFTRESET_WAIT);
> +		error = readx_poll_timeout_atomic(sysc_read_sysstatus, ddata,
> +				rstval, (rstval & ddata->cfg.syss_mask) ==
> +				syss_done, 100, MAX_MODULE_SOFTRESET_WAIT);
>   
>   	} else if (ddata->cfg.quirks & SYSC_QUIRK_RESET_STATUS) {
> -		error = readx_poll_timeout(sysc_read_sysconfig, ddata, rstval,
> -					   !(rstval & sysc_mask),
> -					   100, MAX_MODULE_SOFTRESET_WAIT);
> +		error = readx_poll_timeout_atomic(sysc_read_sysconfig, ddata,
> +				rstval, !(rstval & sysc_mask),
> +				100, MAX_MODULE_SOFTRESET_WAIT);
>   	}
>   
>   	return error;
>
Tony Lindgren July 2, 2020, 7:02 p.m. UTC | #2
* Suman Anna <s-anna@ti.com> [200702 18:01]:
> Hi Tony,
> 
> On 7/2/20 12:49 PM, Tony Lindgren wrote:
> > With CONFIG_DEBUG_ATOMIC_SLEEP enabled we can see the following with
> > wakeirqs and serial console idled:
> 
> Which devices are these? I have one patch from Tero fixing similar errors in
> OMAP IOMMU driver. Will post that either today or tomorrow.

I noticed this testing Andy Schevchenko's pending generic serial PM
patches. It happens on any omap variant with kernel serial console
detached and uart idled. Then just wait for the autosuspend timeout
to expire and type a character on the serial console :)

Regards,

Tony
Tony Lindgren July 2, 2020, 7:11 p.m. UTC | #3
* Tony Lindgren <tony@atomide.com> [200702 19:03]:
> * Suman Anna <s-anna@ti.com> [200702 18:01]:
> > Hi Tony,
> > 
> > On 7/2/20 12:49 PM, Tony Lindgren wrote:
> > > With CONFIG_DEBUG_ATOMIC_SLEEP enabled we can see the following with
> > > wakeirqs and serial console idled:
> > 
> > Which devices are these? I have one patch from Tero fixing similar errors in
> > OMAP IOMMU driver. Will post that either today or tomorrow.
> 
> I noticed this testing Andy Schevchenko's pending generic serial PM
> patches. It happens on any omap variant with kernel serial console
> detached and uart idled. Then just wait for the autosuspend timeout
> to expire and type a character on the serial console :)

And BTW, Andy's series involves the removal of pm_runtime_irq_safe()
from the serial drivers that we still have. So this won't trigger
currently with the uart. But the issue could trigger with other
drivers though.

Regards,

Tony

Patch
diff mbox series

diff --git a/drivers/bus/ti-sysc.c b/drivers/bus/ti-sysc.c
--- a/drivers/bus/ti-sysc.c
+++ b/drivers/bus/ti-sysc.c
@@ -236,15 +236,14 @@  static int sysc_wait_softreset(struct sysc *ddata)
 		syss_done = ddata->cfg.syss_mask;
 
 	if (syss_offset >= 0) {
-		error = readx_poll_timeout(sysc_read_sysstatus, ddata, rstval,
-					   (rstval & ddata->cfg.syss_mask) ==
-					   syss_done,
-					   100, MAX_MODULE_SOFTRESET_WAIT);
+		error = readx_poll_timeout_atomic(sysc_read_sysstatus, ddata,
+				rstval, (rstval & ddata->cfg.syss_mask) ==
+				syss_done, 100, MAX_MODULE_SOFTRESET_WAIT);
 
 	} else if (ddata->cfg.quirks & SYSC_QUIRK_RESET_STATUS) {
-		error = readx_poll_timeout(sysc_read_sysconfig, ddata, rstval,
-					   !(rstval & sysc_mask),
-					   100, MAX_MODULE_SOFTRESET_WAIT);
+		error = readx_poll_timeout_atomic(sysc_read_sysconfig, ddata,
+				rstval, !(rstval & sysc_mask),
+				100, MAX_MODULE_SOFTRESET_WAIT);
 	}
 
 	return error;