* [PATCH 0/2] mmc: renesas_sdhi: reset via reset controller
@ 2021-03-09 9:23 Wolfram Sang
2021-03-09 9:23 ` [PATCH 1/2] mmc: tmio: abort DMA before reset Wolfram Sang
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Wolfram Sang @ 2021-03-09 9:23 UTC (permalink / raw)
To: linux-mmc
Cc: linux-renesas-soc, Yoshihiro Shimoda, Geert Uytterhoeven,
Wolfram Sang
This series enables SDHI to be reset via a reset controller.
Please let me know about your (and BSP team's) thoughts.
Changes since RFC v2 (thanks Geert for the review!):
* use devm_reset_control_get_optional_exclusive()
* select RESET_CONTROLLER
* given reason in commit message of patch 2
Tested on a Salvator-XS with a Renesas R-Car M3-N.
Shimoda-san: I think we need to get this series upstream first. On top
of this, I will work on the better irq_mask handling.
Wolfram Sang (2):
mmc: tmio: abort DMA before reset
mmc: renesas_sdhi: do hard reset if possible
drivers/mmc/host/Kconfig | 1 +
drivers/mmc/host/renesas_sdhi.h | 2 ++
drivers/mmc/host/renesas_sdhi_core.c | 15 ++++++++++++++-
drivers/mmc/host/tmio_mmc_core.c | 4 ++--
4 files changed, 19 insertions(+), 3 deletions(-)
--
2.30.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] mmc: tmio: abort DMA before reset 2021-03-09 9:23 [PATCH 0/2] mmc: renesas_sdhi: reset via reset controller Wolfram Sang @ 2021-03-09 9:23 ` Wolfram Sang 2021-03-10 10:34 ` Niklas Söderlund 2021-03-09 9:23 ` [PATCH 2/2] mmc: renesas_sdhi: do hard reset if possible Wolfram Sang 2021-03-09 9:28 ` [PATCH 0/2] mmc: renesas_sdhi: reset via reset controller Wolfram Sang 2 siblings, 1 reply; 10+ messages in thread From: Wolfram Sang @ 2021-03-09 9:23 UTC (permalink / raw) To: linux-mmc Cc: linux-renesas-soc, Yoshihiro Shimoda, Geert Uytterhoeven, Wolfram Sang We will soon allow resetting the whole IP core via a reset controller. For this case, DMA must be terminated before the actual reset. For the other cases, it is probably better, too. Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> --- drivers/mmc/host/tmio_mmc_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c index 5aa57640d0e6..eca767dcabba 100644 --- a/drivers/mmc/host/tmio_mmc_core.c +++ b/drivers/mmc/host/tmio_mmc_core.c @@ -172,11 +172,11 @@ static void tmio_mmc_reset(struct tmio_mmc_host *host) sd_ctrl_write16(host, CTL_RESET_SD, 0x0001); usleep_range(10000, 11000); + tmio_mmc_abort_dma(host); + if (host->reset) host->reset(host); - tmio_mmc_abort_dma(host); - if (host->pdata->flags & TMIO_MMC_SDIO_IRQ) { sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask); sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001); -- 2.30.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] mmc: tmio: abort DMA before reset 2021-03-09 9:23 ` [PATCH 1/2] mmc: tmio: abort DMA before reset Wolfram Sang @ 2021-03-10 10:34 ` Niklas Söderlund 0 siblings, 0 replies; 10+ messages in thread From: Niklas Söderlund @ 2021-03-10 10:34 UTC (permalink / raw) To: Wolfram Sang Cc: linux-mmc, linux-renesas-soc, Yoshihiro Shimoda, Geert Uytterhoeven Hi Wolfram, Thanks for your patch. On 2021-03-09 10:23:31 +0100, Wolfram Sang wrote: > We will soon allow resetting the whole IP core via a reset controller. > For this case, DMA must be terminated before the actual reset. For the > other cases, it is probably better, too. I agree I think it makes more sens to abort dma before reseting the device. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/mmc/host/tmio_mmc_core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c > index 5aa57640d0e6..eca767dcabba 100644 > --- a/drivers/mmc/host/tmio_mmc_core.c > +++ b/drivers/mmc/host/tmio_mmc_core.c > @@ -172,11 +172,11 @@ static void tmio_mmc_reset(struct tmio_mmc_host *host) > sd_ctrl_write16(host, CTL_RESET_SD, 0x0001); > usleep_range(10000, 11000); > > + tmio_mmc_abort_dma(host); > + > if (host->reset) > host->reset(host); > > - tmio_mmc_abort_dma(host); > - > if (host->pdata->flags & TMIO_MMC_SDIO_IRQ) { > sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask); > sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001); > -- > 2.30.0 > -- Regards, Niklas Söderlund ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] mmc: renesas_sdhi: do hard reset if possible 2021-03-09 9:23 [PATCH 0/2] mmc: renesas_sdhi: reset via reset controller Wolfram Sang 2021-03-09 9:23 ` [PATCH 1/2] mmc: tmio: abort DMA before reset Wolfram Sang @ 2021-03-09 9:23 ` Wolfram Sang 2021-03-10 12:45 ` Yoshihiro Shimoda 2021-03-09 9:28 ` [PATCH 0/2] mmc: renesas_sdhi: reset via reset controller Wolfram Sang 2 siblings, 1 reply; 10+ messages in thread From: Wolfram Sang @ 2021-03-09 9:23 UTC (permalink / raw) To: linux-mmc Cc: linux-renesas-soc, Yoshihiro Shimoda, Geert Uytterhoeven, Wolfram Sang All recent SDHI instances can be reset via the reset controller. If one is found, use it instead of the open coded reset. This is to get a future-proof sane reset state. Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> --- drivers/mmc/host/Kconfig | 1 + drivers/mmc/host/renesas_sdhi.h | 2 ++ drivers/mmc/host/renesas_sdhi_core.c | 15 ++++++++++++++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index b236dfe2e879..1f1b691f10ce 100644 --- a/drivers/mmc/host/Kconfig +++ b/drivers/mmc/host/Kconfig @@ -707,6 +707,7 @@ config MMC_SDHI tristate "Renesas SDHI SD/SDIO controller support" depends on SUPERH || ARCH_RENESAS || COMPILE_TEST select MMC_TMIO_CORE + select RESET_CONTROLLER help This provides support for the SDHI SD/SDIO controller found in Renesas SuperH, ARM and ARM64 based SoCs diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h index cb962c7883dc..53eded81a53e 100644 --- a/drivers/mmc/host/renesas_sdhi.h +++ b/drivers/mmc/host/renesas_sdhi.h @@ -70,6 +70,8 @@ struct renesas_sdhi { DECLARE_BITMAP(smpcmp, BITS_PER_LONG); unsigned int tap_num; unsigned int tap_set; + + struct reset_control *rstc; }; #define host_to_priv(host) \ diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c index 09a5e0dafbef..473f155f6d3d 100644 --- a/drivers/mmc/host/renesas_sdhi_core.c +++ b/drivers/mmc/host/renesas_sdhi_core.c @@ -20,6 +20,7 @@ #include <linux/clk.h> #include <linux/delay.h> +#include <linux/iopoll.h> #include <linux/kernel.h> #include <linux/mfd/tmio.h> #include <linux/mmc/host.h> @@ -32,6 +33,7 @@ #include <linux/platform_device.h> #include <linux/pm_domain.h> #include <linux/regulator/consumer.h> +#include <linux/reset.h> #include <linux/sh_dma.h> #include <linux/slab.h> #include <linux/sys_soc.h> @@ -561,9 +563,16 @@ static int renesas_sdhi_prepare_hs400_tuning(struct mmc_host *mmc, struct mmc_io static void renesas_sdhi_reset(struct tmio_mmc_host *host) { struct renesas_sdhi *priv = host_to_priv(host); + int ret; u16 val; - if (priv->scc_ctl) { + 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); + priv->needs_adjust_hs400 = false; + } else if (priv->scc_ctl) { renesas_sdhi_disable_scc(host->mmc); renesas_sdhi_reset_hs400_mode(host, priv); priv->needs_adjust_hs400 = false; @@ -1077,6 +1086,10 @@ int renesas_sdhi_probe(struct platform_device *pdev, if (ret) goto efree; + priv->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, NULL); + if (IS_ERR(priv->rstc)) + return PTR_ERR(priv->rstc); + ver = sd_ctrl_read16(host, CTL_VERSION); /* GEN2_SDR104 is first known SDHI to use 32bit block count */ if (ver < SDHI_VER_GEN2_SDR104 && mmc_data->max_blk_count > U16_MAX) -- 2.30.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] mmc: renesas_sdhi: do hard reset if possible 2021-03-09 9:23 ` [PATCH 2/2] mmc: renesas_sdhi: do hard reset if possible Wolfram Sang @ 2021-03-10 12:45 ` Yoshihiro Shimoda 2021-03-11 13:17 ` Wolfram Sang 0 siblings, 1 reply; 10+ messages in thread From: Yoshihiro Shimoda @ 2021-03-10 12:45 UTC (permalink / raw) To: Wolfram Sang Cc: linux-renesas-soc@vger.kernel.org, Geert Uytterhoeven, linux-mmc@vger.kernel.org Hi Wolfram-san, Thank you for the patch! > From: Wolfram Sang, Sent: Tuesday, March 9, 2021 6:24 PM > > All recent SDHI instances can be reset via the reset controller. If one > is found, use it instead of the open coded reset. This is to get a > future-proof sane reset state. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- <snip> > @@ -561,9 +563,16 @@ static int renesas_sdhi_prepare_hs400_tuning(struct mmc_host *mmc, struct mmc_io > static void renesas_sdhi_reset(struct tmio_mmc_host *host) > { > struct renesas_sdhi *priv = host_to_priv(host); > + int ret; > u16 val; > > - if (priv->scc_ctl) { > + 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); > + priv->needs_adjust_hs400 = false; After we did hard reset here, sometimes tmio_mmc_reset_work() cannot recover again with "mmcblk0: recovery failed!" message... So, I investigated this issue and I found a reason. > + } else if (priv->scc_ctl) { > renesas_sdhi_disable_scc(host->mmc); I realized this renesas_sdhi_disable_scc() will set CLK_CTL_SCLKEN. So, the previous code can issue CMD13 after tmio_mmc_reset_work() was called. But, after we applied this patch, the CMD13 failed because the clock was disabled. # In other words, if a controller doesn't have scc, the previous code cannot issue # CMD13 in such a case, I guess. So, before we apply this patch, we have to add ->set_clock() calling in tmio_mmc_reset_work() like below: --- diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c index eca767dcabba..a05ccfc7aa0d 100644 --- a/drivers/mmc/host/tmio_mmc_core.c +++ b/drivers/mmc/host/tmio_mmc_core.c @@ -222,6 +222,7 @@ static void tmio_mmc_reset_work(struct work_struct *work) spin_unlock_irqrestore(&host->lock, flags); tmio_mmc_reset(host); + host->set_clock(host, host->clk_cache); /* Ready for new calls */ host->mrq = NULL; --- Best regards, Yoshihiro Shimoda ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] mmc: renesas_sdhi: do hard reset if possible 2021-03-10 12:45 ` Yoshihiro Shimoda @ 2021-03-11 13:17 ` Wolfram Sang 2021-03-12 12:05 ` Yoshihiro Shimoda 0 siblings, 1 reply; 10+ messages in thread From: Wolfram Sang @ 2021-03-11 13:17 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: linux-renesas-soc@vger.kernel.org, Geert Uytterhoeven, linux-mmc@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 983 bytes --] Hi Shimoda-san, > # In other words, if a controller doesn't have scc, the previous code cannot issue > # CMD13 in such a case, I guess. Makes sense. > tmio_mmc_reset(host); > + host->set_clock(host, host->clk_cache); What about putting it into the reset function itself, so it will be always enabled (like for the scc_ctl case)? diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c index 473f155f6d3d..672953e3362d 100644 --- a/drivers/mmc/host/renesas_sdhi_core.c +++ b/drivers/mmc/host/renesas_sdhi_core.c @@ -572,6 +572,7 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host) read_poll_timeout(reset_control_status, ret, ret == 0, 1, 100, false, priv->rstc); priv->needs_adjust_hs400 = false; + renesas_sdhi_set_clock(host, host->clk_cache); } else if (priv->scc_ctl) { renesas_sdhi_disable_scc(host->mmc); renesas_sdhi_reset_hs400_mode(host, priv); If you agree, I will fold this into v2 of this series. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] mmc: renesas_sdhi: do hard reset if possible 2021-03-11 13:17 ` Wolfram Sang @ 2021-03-12 12:05 ` Yoshihiro Shimoda 2021-03-15 12:31 ` Wolfram Sang 0 siblings, 1 reply; 10+ messages in thread From: Yoshihiro Shimoda @ 2021-03-12 12:05 UTC (permalink / raw) To: Wolfram Sang Cc: linux-renesas-soc@vger.kernel.org, Geert Uytterhoeven, linux-mmc@vger.kernel.org Hi Wolfram-san, > From: Wolfram Sang, Sent: Thursday, March 11, 2021 10:18 PM > > tmio_mmc_reset(host); > > + host->set_clock(host, host->clk_cache); > > What about putting it into the reset function itself, so it will be > always enabled (like for the scc_ctl case)? > > diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c > index 473f155f6d3d..672953e3362d 100644 > --- a/drivers/mmc/host/renesas_sdhi_core.c > +++ b/drivers/mmc/host/renesas_sdhi_core.c > @@ -572,6 +572,7 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host) > read_poll_timeout(reset_control_status, ret, ret == 0, 1, 100, > false, priv->rstc); > priv->needs_adjust_hs400 = false; > + renesas_sdhi_set_clock(host, host->clk_cache); > } else if (priv->scc_ctl) { > renesas_sdhi_disable_scc(host->mmc); > renesas_sdhi_reset_hs400_mode(host, priv); > > If you agree, I will fold this into v2 of this series. Adding the function itself seems OK. However, I checked the code, and then adding hard reset into renesas_sdhi_reset() seems to break the following: ----- commit 5b0739d76227fd5a3f02f014385bfa9c86e0404b Author: Wolfram Sang <wsa+renesas@sang-engineering.com> Date: Thu Aug 20 15:25:37 2020 +0200 mmc: tmio: don't reset whole IP core when tuning fails SDHI needs to reset the SCC only, not the whole IP core. So, if tuning fails, don't handle specifics in the generic TMIO core, but in the specific drivers. For SDHI, we need to move around the reset routine a bit. It is not modified. ------ So, perhaps, we have to fix renesas_sdhi_execute_tuning() somehow before adding hard reset. But, what do you think? Best regards, Yoshihiro Shimoda ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] mmc: renesas_sdhi: do hard reset if possible 2021-03-12 12:05 ` Yoshihiro Shimoda @ 2021-03-15 12:31 ` Wolfram Sang 2021-03-15 23:59 ` Yoshihiro Shimoda 0 siblings, 1 reply; 10+ messages in thread From: Wolfram Sang @ 2021-03-15 12:31 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: linux-renesas-soc@vger.kernel.org, Geert Uytterhoeven, linux-mmc@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 1194 bytes --] > Adding the function itself seems OK. However, I checked the code, and then > adding hard reset into renesas_sdhi_reset() seems to break the following: > ----- > commit 5b0739d76227fd5a3f02f014385bfa9c86e0404b > Author: Wolfram Sang <wsa+renesas@sang-engineering.com> > Date: Thu Aug 20 15:25:37 2020 +0200 > > mmc: tmio: don't reset whole IP core when tuning fails > > SDHI needs to reset the SCC only, not the whole IP core. So, if tuning > fails, don't handle specifics in the generic TMIO core, but in the > specific drivers. For SDHI, we need to move around the reset routine a > bit. It is not modified. > ------ > > So, perhaps, we have to fix renesas_sdhi_execute_tuning() somehow before > adding hard reset. But, what do you think? Thanks for the pointer, Shimoda-san! It seems it gets messy again, so time for getting clearer, I think. I will introduce a reset_scc funtion again which is seperate from reset_sdhi, so we have a clear distinction. Then, I will call reset_scc from renesas_sdhi_execute_tuning() and the rest stays with reset_sdhi. I have a prototype patch which I will test some more and hopefully send out later. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] mmc: renesas_sdhi: do hard reset if possible 2021-03-15 12:31 ` Wolfram Sang @ 2021-03-15 23:59 ` Yoshihiro Shimoda 0 siblings, 0 replies; 10+ messages in thread From: Yoshihiro Shimoda @ 2021-03-15 23:59 UTC (permalink / raw) To: Wolfram Sang Cc: linux-renesas-soc@vger.kernel.org, Geert Uytterhoeven, linux-mmc@vger.kernel.org Hi Wolfram-san, > From: Wolfram Sang, Sent: Monday, March 15, 2021 9:31 PM > > > Adding the function itself seems OK. However, I checked the code, and then > > adding hard reset into renesas_sdhi_reset() seems to break the following: > > ----- > > commit 5b0739d76227fd5a3f02f014385bfa9c86e0404b > > Author: Wolfram Sang <wsa+renesas@sang-engineering.com> > > Date: Thu Aug 20 15:25:37 2020 +0200 > > > > mmc: tmio: don't reset whole IP core when tuning fails > > > > SDHI needs to reset the SCC only, not the whole IP core. So, if tuning > > fails, don't handle specifics in the generic TMIO core, but in the > > specific drivers. For SDHI, we need to move around the reset routine a > > bit. It is not modified. > > ------ > > > > So, perhaps, we have to fix renesas_sdhi_execute_tuning() somehow before > > adding hard reset. But, what do you think? > > Thanks for the pointer, Shimoda-san! It seems it gets messy again, so > time for getting clearer, I think. I will introduce a reset_scc funtion > again which is seperate from reset_sdhi, so we have a clear distinction. > Then, I will call reset_scc from renesas_sdhi_execute_tuning() and the > rest stays with reset_sdhi. I have a prototype patch which I will test > some more and hopefully send out later. It's a nice idea! And, thank you for submitting a new patch series. I'll test it later. Best regards, Yoshihiro Shimoda ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] mmc: renesas_sdhi: reset via reset controller 2021-03-09 9:23 [PATCH 0/2] mmc: renesas_sdhi: reset via reset controller Wolfram Sang 2021-03-09 9:23 ` [PATCH 1/2] mmc: tmio: abort DMA before reset Wolfram Sang 2021-03-09 9:23 ` [PATCH 2/2] mmc: renesas_sdhi: do hard reset if possible Wolfram Sang @ 2021-03-09 9:28 ` Wolfram Sang 2 siblings, 0 replies; 10+ messages in thread From: Wolfram Sang @ 2021-03-09 9:28 UTC (permalink / raw) To: linux-mmc; +Cc: linux-renesas-soc, Yoshihiro Shimoda, Geert Uytterhoeven [-- Attachment #1: Type: text/plain, Size: 479 bytes --] On Tue, Mar 09, 2021 at 10:23:30AM +0100, Wolfram Sang wrote: > This series enables SDHI to be reset via a reset controller. > > Please let me know about your (and BSP team's) thoughts. > > Changes since RFC v2 (thanks Geert for the review!): > > * use devm_reset_control_get_optional_exclusive() > * select RESET_CONTROLLER > * given reason in commit message of patch 2 > > Tested on a Salvator-XS with a Renesas R-Car M3-N. Patches are on top of mmc/next. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-03-16 0:00 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-03-09 9:23 [PATCH 0/2] mmc: renesas_sdhi: reset via reset controller Wolfram Sang 2021-03-09 9:23 ` [PATCH 1/2] mmc: tmio: abort DMA before reset Wolfram Sang 2021-03-10 10:34 ` Niklas Söderlund 2021-03-09 9:23 ` [PATCH 2/2] mmc: renesas_sdhi: do hard reset if possible Wolfram Sang 2021-03-10 12:45 ` Yoshihiro Shimoda 2021-03-11 13:17 ` Wolfram Sang 2021-03-12 12:05 ` Yoshihiro Shimoda 2021-03-15 12:31 ` Wolfram Sang 2021-03-15 23:59 ` Yoshihiro Shimoda 2021-03-09 9:28 ` [PATCH 0/2] mmc: renesas_sdhi: reset via reset controller Wolfram Sang
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.