From: Ulf Hansson <ulf.hansson@linaro.org>
To: Wolfram Sang <wsa@kernel.org>
Cc: linux-mmc@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Wolfram Sang <wsa+renesas@sang-engineering.com>,
Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Subject: Re: [PATCH] mmc: tmio: avoid glitches when resetting
Date: Tue, 12 Jul 2022 13:09:02 +0200 [thread overview]
Message-ID: <CAPDyKFphYtMhFHu0c+WF9hsm7Y9KgjCqKqSZ_uDTY3ddCYZ-7w@mail.gmail.com> (raw)
In-Reply-To: <20220625131722.1397-1-wsa@kernel.org>
On Sat, 25 Jun 2022 at 15:17, Wolfram Sang <wsa@kernel.org> wrote:
>
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> If we reset because of an error, we need to preserve values for the
> clock frequency. Otherwise, glitches may be seen on the bus.
>
> To achieve that, we introduce a 'preserve' parameter to the reset
> function and the IP core specific reset callbacks to handle everything
> accordingly.
>
> Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Applied for next, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/host/renesas_sdhi_core.c | 29 ++++++++++++++--------------
> drivers/mmc/host/tmio_mmc.c | 2 +-
> drivers/mmc/host/tmio_mmc.h | 6 +++++-
> drivers/mmc/host/tmio_mmc_core.c | 28 +++++++++++++++++++++------
> 4 files changed, 42 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index 4404ca1f98d8..5fa365d0c7fd 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -49,9 +49,6 @@
> #define HOST_MODE_GEN3_32BIT (HOST_MODE_GEN3_WMODE | HOST_MODE_GEN3_BUSWIDTH)
> #define HOST_MODE_GEN3_64BIT 0
>
> -#define CTL_SDIF_MODE 0xe6
> -#define SDIF_MODE_HS400 BIT(0)
> -
> #define SDHI_VER_GEN2_SDR50 0x490c
> #define SDHI_VER_RZ_A1 0x820b
> /* very old datasheets said 0x490c for SDR104, too. They are wrong! */
> @@ -562,23 +559,25 @@ static void renesas_sdhi_scc_reset(struct tmio_mmc_host *host, struct renesas_sd
> }
>
> /* only populated for TMIO_MMC_MIN_RCAR2 */
> -static void renesas_sdhi_reset(struct tmio_mmc_host *host)
> +static void renesas_sdhi_reset(struct tmio_mmc_host *host, bool preserve)
> {
> struct renesas_sdhi *priv = host_to_priv(host);
> int ret;
> u16 val;
>
> - if (priv->rstc) {
> - reset_control_reset(priv->rstc);
> - /* Unknown why but without polling reset status, it will hang */
> - read_poll_timeout(reset_control_status, ret, ret == 0, 1, 100,
> - false, priv->rstc);
> - /* At least SDHI_VER_GEN2_SDR50 needs manual release of reset */
> - sd_ctrl_write16(host, CTL_RESET_SD, 0x0001);
> - priv->needs_adjust_hs400 = false;
> - renesas_sdhi_set_clock(host, host->clk_cache);
> - } else if (priv->scc_ctl) {
> - renesas_sdhi_scc_reset(host, priv);
> + if (!preserve) {
> + if (priv->rstc) {
> + reset_control_reset(priv->rstc);
> + /* Unknown why but without polling reset status, it will hang */
> + read_poll_timeout(reset_control_status, ret, ret == 0, 1, 100,
> + false, priv->rstc);
> + /* At least SDHI_VER_GEN2_SDR50 needs manual release of reset */
> + sd_ctrl_write16(host, CTL_RESET_SD, 0x0001);
> + priv->needs_adjust_hs400 = false;
> + renesas_sdhi_set_clock(host, host->clk_cache);
> + } else if (priv->scc_ctl) {
> + renesas_sdhi_scc_reset(host, priv);
> + }
> }
>
> if (sd_ctrl_read16(host, CTL_VERSION) >= SDHI_VER_GEN3_SD) {
> diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
> index b55a29c53d9c..53a2ad9a24b8 100644
> --- a/drivers/mmc/host/tmio_mmc.c
> +++ b/drivers/mmc/host/tmio_mmc.c
> @@ -75,7 +75,7 @@ static void tmio_mmc_set_clock(struct tmio_mmc_host *host,
> tmio_mmc_clk_start(host);
> }
>
> -static void tmio_mmc_reset(struct tmio_mmc_host *host)
> +static void tmio_mmc_reset(struct tmio_mmc_host *host, bool preserve)
> {
> sd_ctrl_write16(host, CTL_RESET_SDIO, 0x0000);
> usleep_range(10000, 11000);
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index e754bb3f5c32..501613c74406 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -42,6 +42,7 @@
> #define CTL_DMA_ENABLE 0xd8
> #define CTL_RESET_SD 0xe0
> #define CTL_VERSION 0xe2
> +#define CTL_SDIF_MODE 0xe6 /* only known on R-Car 2+ */
>
> /* Definitions for values the CTL_STOP_INTERNAL_ACTION register can take */
> #define TMIO_STOP_STP BIT(0)
> @@ -98,6 +99,9 @@
> /* Definitions for values the CTL_DMA_ENABLE register can take */
> #define DMA_ENABLE_DMASDRW BIT(1)
>
> +/* Definitions for values the CTL_SDIF_MODE register can take */
> +#define SDIF_MODE_HS400 BIT(0) /* only known on R-Car 2+ */
> +
> /* Define some IRQ masks */
> /* This is the mask used at reset by the chip */
> #define TMIO_MASK_ALL 0x837f031d
> @@ -181,7 +185,7 @@ struct tmio_mmc_host {
> int (*multi_io_quirk)(struct mmc_card *card,
> unsigned int direction, int blk_size);
> int (*write16_hook)(struct tmio_mmc_host *host, int addr);
> - void (*reset)(struct tmio_mmc_host *host);
> + void (*reset)(struct tmio_mmc_host *host, bool preserve);
> bool (*check_retune)(struct tmio_mmc_host *host, struct mmc_request *mrq);
> void (*fixup_request)(struct tmio_mmc_host *host, struct mmc_request *mrq);
> unsigned int (*get_timeout_cycles)(struct tmio_mmc_host *host);
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index a5850d83908b..437048bb8027 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -179,8 +179,17 @@ static void tmio_mmc_set_bus_width(struct tmio_mmc_host *host,
> sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, reg);
> }
>
> -static void tmio_mmc_reset(struct tmio_mmc_host *host)
> +static void tmio_mmc_reset(struct tmio_mmc_host *host, bool preserve)
> {
> + u16 card_opt, clk_ctrl, sdif_mode;
> +
> + if (preserve) {
> + card_opt = sd_ctrl_read16(host, CTL_SD_MEM_CARD_OPT);
> + clk_ctrl = sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL);
> + if (host->pdata->flags & TMIO_MMC_MIN_RCAR2)
> + sdif_mode = sd_ctrl_read16(host, CTL_SDIF_MODE);
> + }
> +
> /* FIXME - should we set stop clock reg here */
> sd_ctrl_write16(host, CTL_RESET_SD, 0x0000);
> usleep_range(10000, 11000);
> @@ -190,7 +199,7 @@ static void tmio_mmc_reset(struct tmio_mmc_host *host)
> tmio_mmc_abort_dma(host);
>
> if (host->reset)
> - host->reset(host);
> + host->reset(host, preserve);
>
> sd_ctrl_write32_as_16_and_16(host, CTL_IRQ_MASK, host->sdcard_irq_mask_all);
> host->sdcard_irq_mask = host->sdcard_irq_mask_all;
> @@ -206,6 +215,13 @@ static void tmio_mmc_reset(struct tmio_mmc_host *host)
> sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001);
> }
>
> + if (preserve) {
> + sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, card_opt);
> + sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, clk_ctrl);
> + if (host->pdata->flags & TMIO_MMC_MIN_RCAR2)
> + sd_ctrl_write16(host, CTL_SDIF_MODE, sdif_mode);
> + }
> +
> if (host->mmc->card)
> mmc_retune_needed(host->mmc);
> }
> @@ -248,7 +264,7 @@ static void tmio_mmc_reset_work(struct work_struct *work)
>
> spin_unlock_irqrestore(&host->lock, flags);
>
> - tmio_mmc_reset(host);
> + tmio_mmc_reset(host, true);
>
> /* Ready for new calls */
> host->mrq = NULL;
> @@ -961,7 +977,7 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> tmio_mmc_power_off(host);
> /* For R-Car Gen2+, we need to reset SDHI specific SCC */
> if (host->pdata->flags & TMIO_MMC_MIN_RCAR2)
> - tmio_mmc_reset(host);
> + tmio_mmc_reset(host, false);
>
> host->set_clock(host, 0);
> break;
> @@ -1189,7 +1205,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
> _host->sdcard_irq_mask_all = TMIO_MASK_ALL;
>
> _host->set_clock(_host, 0);
> - tmio_mmc_reset(_host);
> + tmio_mmc_reset(_host, false);
>
> spin_lock_init(&_host->lock);
> mutex_init(&_host->ios_lock);
> @@ -1285,7 +1301,7 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
> struct tmio_mmc_host *host = dev_get_drvdata(dev);
>
> tmio_mmc_clk_enable(host);
> - tmio_mmc_reset(host);
> + tmio_mmc_reset(host, false);
>
> if (host->clk_cache)
> host->set_clock(host, host->clk_cache);
> --
> 2.35.1
>
prev parent reply other threads:[~2022-07-12 11:09 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-25 13:17 [PATCH] mmc: tmio: avoid glitches when resetting Wolfram Sang
2022-07-12 11:09 ` Ulf Hansson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAPDyKFphYtMhFHu0c+WF9hsm7Y9KgjCqKqSZ_uDTY3ddCYZ-7w@mail.gmail.com \
--to=ulf.hansson@linaro.org \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=wsa+renesas@sang-engineering.com \
--cc=wsa@kernel.org \
--cc=yoshihiro.shimoda.uh@renesas.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.