2020-07-02 17:52:26

by Tony Lindgren

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

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 <[email protected]>
---
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


2020-07-02 18:01:36

by Suman Anna

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

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 <[email protected]>
> ---
> 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;
>

2020-07-02 19:05:18

by Tony Lindgren

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

* Suman Anna <[email protected]> [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

2020-07-02 19:14:10

by Tony Lindgren

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

* Tony Lindgren <[email protected]> [200702 19:03]:
> * Suman Anna <[email protected]> [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