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

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

[Tony Lindgren]

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(-)

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;
-- 
2.27.0

[PATCH 2/2] bus: ti-sysc: Fix sleeping function called from invalid context for RTC quirk

[Tony Lindgren]

With CONFIG_DEBUG_ATOMIC_SLEEP enabled we can see the following with RTC probe:

BUG: sleeping function called from invalid context at drivers/bus/ti-sysc.c:1736
...
(sysc_quirk_rtc) from [<c060d01c>] (sysc_write_sysconfig+0x1c/0x60)
(sysc_write_sysconfig) from [<c060d9f4>] (sysc_enable_module+0x11c/0x274)
(sysc_enable_module) from [<c060f37c>] (sysc_probe+0xe9c/0x1380)
(sysc_probe) from [<c06e9384>] (platform_drv_probe+0x48/0x98)

Fixes: e8639e1c986a ("bus: ti-sysc: Handle module unlock quirk needed for some RTC")
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/bus/ti-sysc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 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
@@ -1727,8 +1727,8 @@ static void sysc_quirk_rtc(struct sysc *ddata, bool lock)
 
 	local_irq_save(flags);
 	/* RTC_STATUS BUSY bit may stay active for 1/32768 seconds (~30 usec) */
-	error = readl_poll_timeout(ddata->module_va + 0x44, val,
-				   !(val & BIT(0)), 100, 50);
+	error = readl_poll_timeout_atomic(ddata->module_va + 0x44, val,
+					  !(val & BIT(0)), 100, 50);
 	if (error)
 		dev_warn(ddata->dev, "rtc busy timeout\n");
 	/* Now we have ~15 microseconds to read/write various registers */
-- 
2.27.0

Re: [PATCH 1/2] bus: ti-sysc: Fix wakeirq sleeping function called from invalid context

[Suman Anna]

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;
> 

Re: [PATCH 1/2] bus: ti-sysc: Fix wakeirq sleeping function called from invalid context

[Tony Lindgren]

* 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

Re: [PATCH 1/2] bus: ti-sysc: Fix wakeirq sleeping function called from invalid context

[Tony Lindgren]

* 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