On 11/27/2018 04:43 AM, Masahiro Yamada wrote: > On Wed, Nov 14, 2018 at 8:44 AM Marek Vasut <marek.va...@gmail.com> wrote: >> >> The TMIO core has a feature where it can automatically disable clock output >> when the bus is not in use. While this is useful, it also interferes with >> switching the bus to 1.8V and other background tasks of the SD/MMC cards, >> which require clock to be enabled. >> >> This patch respects the mmc->clk_disable and only disables the clock when >> the MMC core requests it. Otherwise the clock are continuously generated >> on the bus. >> >> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com> >> Cc: Masahiro Yamada <yamada.masah...@socionext.com> >> --- >> drivers/mmc/tmio-common.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/tmio-common.c b/drivers/mmc/tmio-common.c >> index 424b60ce52..fad2816ca5 100644 >> --- a/drivers/mmc/tmio-common.c >> +++ b/drivers/mmc/tmio-common.c >> @@ -612,10 +612,16 @@ static void tmio_sd_set_clk_rate(struct tmio_sd_priv >> *priv, >> tmio_sd_writel(priv, tmp, TMIO_SD_CLKCTL); >> >> tmp &= ~TMIO_SD_CLKCTL_DIV_MASK; >> - tmp |= val | TMIO_SD_CLKCTL_OFFEN; >> + tmp |= val; >> tmio_sd_writel(priv, tmp, TMIO_SD_CLKCTL); >> >> - tmp |= TMIO_SD_CLKCTL_SCLKEN; >> + if (!mmc->clk_disable) { >> + tmp &= ~TMIO_SD_CLKCTL_OFFEN; >> + tmp |= TMIO_SD_CLKCTL_SCLKEN; >> + } else { >> + tmp |= TMIO_SD_CLKCTL_OFFEN; >> + tmp &= ~TMIO_SD_CLKCTL_SCLKEN; >> + } >> tmio_sd_writel(priv, tmp, TMIO_SD_CLKCTL); >> >> udelay(1000); > > > > What is the point of setting TMIO_SD_CLKCTL_OFFEN > when TMIO_SD_CLKCTL_SCLKEN is unset? > > > Why isn't the code like this? > > > if (!mmc->clk_disable) > tmp |= TMIO_SD_CLKCTL_SCLKEN; > else > tmp &= ~TMIO_SD_CLKCTL_SCLKEN; > > /* Never use OFFEN because it interferes with 1.8 switching */ > tmp &= ~TMIO_SD_CLKCTL_OFFEN; > > tmio_sd_writel(priv, tmp, TMIO_SD_CLKCTL); >
I think this could be done, but I'd have to re-run the entire bulk of tests on all boards to see if nothing broke. I'd prefer to get this in (if it doesn't break your system) and then add a patch on top. > One more thing, you wrote > > if (!mmc->clk_disable) > ... > else > ... > > in this patch. > > > Then, soon you will rewrite it to > > if (mmc->clk_disable) > ... > else > ... > > in another patch: > > http://patchwork.ozlabs.org/patch/998542/ I know, I added the other one afterward, since I discovered another card which had clock problems. -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot