Hi Wolfram-san,
Thank you for the patch!
> From: Wolfram Sang, Sent: Thursday, April 8, 2021 12:31 AM
>
> Currently, only SDHI on R-Car Gen2+ reinitializes the irq register
> during reset but it should be done on all instances. We can move it from
> the SDHI driver to the TMIO core, because we now have the
> 'sd_irq_mask_all' variable which carries the proper value to use. That
> also means we can remove the initialization from tmio_mmc_probe()
> because it calls tmio_mmc_reset(), too. We only move that
> tmio_mmc_reset() call there a little to ensure 'sd_irq_mask_all' is
> properly set.
Yes, this is my expectation. However...
> Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> ---
>
> Shimoda-san, I think this is the implementation of what we discussed. It
> passes my tests on a Renesas H3 ES2.0. I'd be happy if you or the BSP
> team could run their additional checks with this patch. Thank you and
> kind regards!
>
> drivers/mmc/host/renesas_sdhi_core.c | 2 --
> drivers/mmc/host/tmio_mmc_core.c | 11 ++++++-----
> 2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index d36181b6f687..635bf31a6735 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -588,8 +588,6 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host)
> renesas_sdhi_scc_reset(host, priv);
> }
>
> - sd_ctrl_write32_as_16_and_16(host, CTL_IRQ_MASK, TMIO_MASK_ALL_RCAR2);
> -
> if (sd_ctrl_read16(host, CTL_VERSION) >= SDHI_VER_GEN3_SD) {
> val = sd_ctrl_read16(host, CTL_SD_MEM_CARD_OPT);
> val |= CARD_OPT_EXTOP;
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index 0c474d78b186..bcd26056d47a 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -192,6 +192,9 @@ static void tmio_mmc_reset(struct tmio_mmc_host *host)
> if (host->reset)
> host->reset(host);
>
> + host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(host, CTL_IRQ_MASK);
> + tmio_mmc_disable_mmc_irqs(host, host->sdcard_irq_mask_all);
> +
This code could not resolve my concern. This code still read
CTL_IRQ_MASK at first. So, if the register value is incorrect
(when "host->reset" didn't exist), the sdcard_irq_mask value
will be not expected value.
So, I'm thinking we should write CTL_IRQ_MASK with sdcard_irq_mask_all
by using sd_ctrl_write32_as_16_and_16() instead of using
tmio_mmc_disable_mmc_irqs() at first.
Best regards,
Yoshihiro Shimoda
