Re: [PATCH/RFC] mmc: tmio: Cancel delayed work before freeing host

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 31 Oct 2023 at 16:48, Geert Uytterhoeven
<geert+renesas@xxxxxxxxx> wrote:
>
> On RZ/Five SMARC EVK, where probing of SDHI fails due to missing pin
> control:
>
>     ------------[ cut here ]------------
>     WARNING: CPU: 0 PID: 0 at kernel/time/timer.c:1739 __run_timers.part.0+0x1e8/0x22a
>     Modules linked in:
>     CPU: 0 PID: 0 Comm: swapper Not tainted 6.6.0-10953-ga37a67d260e6-dirty #86
>     Hardware name: Renesas SMARC EVK based on r9a07g043f01 (DT)
>     epc : __run_timers.part.0+0x1e8/0x22a
>      ra : __run_timers.part.0+0x130/0x22a
>     epc : ffffffff8007540c ra : ffffffff80075354 sp : ffffffc800003e60
>      gp : ffffffff814f5f08 tp : ffffffff8140d5c0 t0 : 0000000000046600
>      t1 : 0000000000000001 t2 : ffffffff81200c10 s0 : ffffffc800003f20
>      s1 : ffffffd8023bc4a0 a0 : 00000000fffee790 a1 : 0000000000000200
>      a2 : 0000000000000200 a3 : ffffffff81489640 a4 : ffffffc800003e60
>      a5 : 0000000000000000 a6 : 0000000000000000 a7 : ffffffc800003e68
>      s2 : 0000000000200000 s3 : 0000000000000122 s4 : 0000000000000000
>      s5 : ffffffffffffffff s6 : ffffffff814896c0 s7 : ffffffff814f58a0
>      s8 : ffffffff80f8bec8 s9 : 0000000000000000 s10: ffffffc800003e60
>      s11: ffffffff81489640 t3 : ffffffff81489678 t4 : 0000000000000240
>      t5 : ffffffd8024ac018 t6 : ffffffd8024ac038
>     status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003
>     [<ffffffff8007540c>] __run_timers.part.0+0x1e8/0x22a
>     [<ffffffff80075472>] run_timer_softirq+0x24/0x4a
>     [<ffffffff80804ec6>] __do_softirq+0xc6/0x212
>     [<ffffffff80027434>] irq_exit_rcu+0x7c/0x9a
>     [<ffffffff807fcd8a>] handle_riscv_irq+0x40/0x4e
>     [<ffffffff807fd7f8>] do_irq+0x40/0x68
>     ---[ end trace 0000000000000000 ]---
>
> What happens?
>
>     renesas_sdhi_probe()
>     {
>         tmio_mmc_host_alloc()
>             mmc_alloc_host()
>                 INIT_DELAYED_WORK(&host->detect, mmc_rescan);
>
>         devm_request_irq(tmio_mmc_irq);
>
>         /*
>          * After this, the interrupt handler may be invoked at any time
>          *
>          *  tmio_mmc_irq()
>          *  {
>          *      __tmio_mmc_card_detect_irq()
>          *          mmc_detect_change()
>          *              _mmc_detect_change()
>          *                  mmc_schedule_delayed_work(&host->detect, delay);
>          *  }
>          */
>
>         tmio_mmc_host_probe()
>             tmio_mmc_init_ocr()
>                     -EPROBE_DEFER
>
>         tmio_mmc_host_free()
>             mmc_free_host()
>     }
>
> When expire_timers() runs later, it warns because the scheduled work was
> freed, and now contains a NULL handler function pointer.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> ---
> Calling __mmc_stop_host() instead works too, but __mmc_stop_host() is an
> internal core function, and is not exported to modules yet.
>
> Perhaps this should be handled by mmc_free_host() instead?

That sounds reasonable to me. Or actually not "instead of"
__mmc_stop_host(), but rather from mmc_free_host() too.

mmc_stop_host() also needs to make sure that mmc_rescan() isn't
currently being executed, so that when setting the
"host->rescan_disable = 1;" it really takes effect.

Do you want to send a patch?

> ---
>  drivers/mmc/host/tmio_mmc_core.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index be7f18fd4836ab29..1e56e78a020d94b9 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -1132,6 +1132,7 @@ EXPORT_SYMBOL_GPL(tmio_mmc_host_alloc);
>
>  void tmio_mmc_host_free(struct tmio_mmc_host *host)
>  {
> +       cancel_delayed_work_sync(&host->mmc->detect);
>         mmc_free_host(host->mmc);
>  }
>  EXPORT_SYMBOL_GPL(tmio_mmc_host_free);

Kind regards
Uffe



[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux