Re: [PATCH v2 2/9] mmc: tmio, sh_mobile_sdhi: Add support for variable input clock frequency

[Wolfram Sang <wsa@the-dreams.de>]

[-- Attachment #1: Type: text/plain, Size: 2072 bytes --]


> >   1. The SDHI/MMC clocks now run much slower than before. Perhaps this is
> >      intentional, and a consequence of finding the best way to drive the SD
> >      card at the target frequency?
> 
> I don't think is generally a problem.  Probably even saves a little
> power.

If you insert an SD card, frequencies should be back to normal. This
happens on Lager at least.

> 
> >   2. On r8a7740, the situation is worse: the HP ("High-speed Peripheral")
> >      clock is also scaled down from 99 MHz to 12.375 MHz.
> >      As the HP clock is the parent of lots of on-chip devices, this may affect
> >      performance for all of them.
> >
> > On r8a73a4, r8a7791, and sh73a0, the SDHI clocks are children of the pll1_div2
> > clocks, which are fixed.
> > On r8a7740, the SDHI and MMC clocks are children of the HP clock,
> > which is also scaled down, affecting all other siblings.
> [...]
> 
> That seems like a bug in the clock driver.  If it doesn't have
> independent dividers for each clock client then it shouldn't allow any
> client to change the frequency.

I tend to agree.

However, I just found out that we don't check the result of
clk_set_rate(). Probably something like this is missing?


diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 5923ce7e0fccb3..e51d7b01d39a3b 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -167,7 +167,7 @@ static unsigned int sh_mobile_sdhi_clk_update(struct tmio_mmc_host *host,
 {
 	struct sh_mobile_sdhi *priv = host_to_priv(host);
 	unsigned int freq, best_freq, diff_min, diff;
-	int i;
+	int i, ret;
 
 	diff_min = ~0;
 	best_freq = 0;
@@ -195,9 +195,9 @@ static unsigned int sh_mobile_sdhi_clk_update(struct tmio_mmc_host *host,
 		}
 	}
 
-	clk_set_rate(priv->clk, best_freq);
+	ret = clk_set_rate(priv->clk, best_freq);
 
-	return best_freq;
+	return ret == 0 ? best_freq : clk_get_rate(priv->clk);
 }
 
 static void sh_mobile_sdhi_clk_disable(struct tmio_mmc_host *host)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]