On Tue, Jun 13, 2023 at 2:08 PM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> Typically, the cmd->busy_timeout contains the current value of the
> timeout that should be used for the commands that have the flags
> MMC_RSP_BUSY set for it.
>
> The stm variant already uses cmd->busy_timeout, but also assigns a
> default timeout, just to make sure if the core has failed to set
> cmd->busy_timeout (it shouldn't but who knows).
I generalized the STM32 solution with the core-specified timeout
and default and used that.
If we know the core will always provide correct timeouts we should
probably delete hacks like this though (but that would be a separate
patch).
> > +static void busy_timeout_work(struct work_struct *work)
> > +{
> > + struct mmci_host *host =
> > + container_of(work, struct mmci_host, busy_timeout_work.work);
> > + u32 status;
> > +
> > + dev_dbg(mmc_dev(host->mmc), "timeout waiting for busy IRQ\n");
> > + status = readl(host->base + MMCISTATUS);
> > + mmci_cmd_irq(host, host->cmd, status);
>
> There's no locking here. I assume that's because we call
> cancel_delayed_work_sync() from an atomic context and we don't want
> that to hang.
>
> However, can't the call to mmci_cmd_irq() race with a proper irq being
> managed in parallel?
It will not be a problem AFAIC: the MMCI is using level IRQs, meaning it
will handle all flags that are set for command or data IRQs before exiting
the IRQ handler, the order of the IRQ handling if several fire at the same
time is actually dependent on the order the IRQ flags are checked in the
code.
When the interrupt handler exits, all IRQs should be handled and the
respective IRQ lines and thus the IRQ from the MMCI should be
de-asserted.
In this case, our problem is that an interrupt is not fired at all, but if
the actual IRQ (that should have been fired) or a totally different IRQ
(such as an error) is fired at the same time doesn't matter: the pending
IRQs will be handled, and if the timer then fires an additional IRQ
the IRQ handler will check if there are any IRQs to handle and then
conclude there isn't and then just return.
At least the way I read it.
I made a v4, sending it out so you can check!
Yours,
Linus Walleij
