Re: [PATCH] mmc: meson: move mmc_request_done() call to irq thread

[Martin Blumenstingl]

Hi Martin,

On Tue, Jun 6, 2023 at 8:59 AM Martin Hundebøll <martin@xxxxxxxxxx> wrote:
[...]
> @@ -1040,6 +1034,13 @@ static irqreturn_t meson_mmc_irq_thread(int irq, void *dev_id)
>         if (WARN_ON(!cmd))
>                 return IRQ_NONE;
>
> +       if (!meson_mmc_bounce_buf_read(cmd->data) &&
> +           !meson_mmc_get_next_command(cmd)) {
> +               meson_mmc_request_done(host->mmc, cmd->mrq);
> +
> +               return IRQ_HANDLED;
> +       }
> +
I'm wondering if this is changing the behavior when cmd->error is non-zero.

Let's take the "status & IRQ_CRC_ERR" case from meson_mmc_irq() - previously:
- cmd->error is set
- regmap_update_bits(host->regmap, SD_EMMC_START, START_DESC_BUSY, 0);
- IRQ_WAKE_THREAD
- then inside meson_mmc_irq_thread():
-- meson_mmc_wait_desc_stop(host);
-- meson_mmc_request_done(host->mmc, cmd->mrq);

With this patch:
- cmd->error is set
- regmap_update_bits(host->regmap, SD_EMMC_START, START_DESC_BUSY, 0);
- IRQ_WAKE_THREAD
- then inside meson_mmc_irq_thread():
-- !meson_mmc_bounce_buf_read(cmd->data) &&
!meson_mmc_get_next_command(cmd) can be true, which means we only
call:
--- meson_mmc_request_done(host->mmc, cmd->mrq);
--- (note the missing meson_mmc_wait_desc_stop() call before
meson_mmc_request_done())

What do you think?
Maybe we just need to remove the code from meson_mmc_irq() and just
omit any modifications to meson_mmc_irq_thread() (as it seems all
checks that you're adding are already present in that function below
the cmd->error case).


Best regards,
Martin