On 7/08/20 4:50 am, Bough Chen wrote: > >> -----Original Message----- >> From: Adrian Hunter [mailto:adrian.hunter@xxxxxxxxx] >> Sent: 2020年8月6日 20:11 >> To: Bough Chen <haibo.chen@xxxxxxx>; ulf.hansson@xxxxxxxxxx; >> linux-mmc@xxxxxxxxxxxxxxx >> Cc: dl-linux-imx <linux-imx@xxxxxxx>; shawnguo@xxxxxxxxxx; >> s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx >> Subject: Re: [PATCH] mmc: host: sdhci-esdhc-imx: reset usdhc before sending >> tuning command for manual tuning method >> >> On 6/08/20 2:48 pm, haibo.chen@xxxxxxx wrote: >>> From: Haibo Chen <haibo.chen@xxxxxxx> >>> >>> According to IC suggestion, everytime before sending the tuning >>> command, need to reset the usdhc, so to reset the tuning circuit, to >>> let every tuning command work well for the manual tuning method. For >>> standard tuning method, IC already add the reset operation in the logic. >>> >>> Signed-off-by: Haibo Chen <haibo.chen@xxxxxxx> >>> --- >>> drivers/mmc/host/sdhci-esdhc-imx.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c >>> b/drivers/mmc/host/sdhci-esdhc-imx.c >>> index a76b4513fbec..e4694eb1b914 100644 >>> --- a/drivers/mmc/host/sdhci-esdhc-imx.c >>> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c >>> @@ -990,6 +990,7 @@ static void esdhc_prepare_tuning(struct sdhci_host >>> *host, u32 val) >>> >>> /* FIXME: delay a bit for card to be ready for next tuning due to errors */ >>> mdelay(1); >>> + sdhci_reset(host, SDHCI_RESET_ALL); >> >> Doesn't that reset all registers i.e. the entire ios state? >> > > Hi Adrian, > For i.MX usdhc, RESET_ALL operation do not impact any register value, it just reset the internal logic setting. I already verified that. > Here I just want to set the SDHCI_RESET_ALL bit, but I notice that the API sdhci_reset() will also touch the host->clock, so I'm not sure whether it is good enough to use this API directly. Any suggestion? At least, current patch can fix the manual tuning issue on imx7d/imx8qxp. It might be safer to write SDHCI_RESET_ALL directly because, for your case, it does not have exactly the same semantics as sdhci_reset(). It is up to you, but either way, it would be worth adding a comment. > >>> >>> reg = readl(host->ioaddr + ESDHC_MIX_CTRL); >>> reg |= ESDHC_MIX_CTRL_EXE_TUNE | ESDHC_MIX_CTRL_SMPCLK_SEL | >>> >