| Message ID | 20160415202803.GA3105@katana (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
> > 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. So, we need MSTP clocks which do not use CLK_SET_RATE_PARENT on r8a7740. Probably the best way is to use the "renesas,r8a7740-mstp-clocks" compatible entry to determine the difference? Geert?
On Fri, Apr 15, 2016 at 10:41:57PM +0200, Wolfram Sang wrote: > > > 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. > > So, we need MSTP clocks which do not use CLK_SET_RATE_PARENT on r8a7740. > Probably the best way is to use the "renesas,r8a7740-mstp-clocks" > compatible entry to determine the difference? Geert? Well, I like the idea of a virtual sd clock even better. I sent a series doing that a minute ago to the renesas-soc list.
On Sun, Apr 17, 2016 at 2:49 PM, Wolfram Sang <wsa@the-dreams.de> wrote: > On Fri, Apr 15, 2016 at 10:41:57PM +0200, Wolfram Sang wrote: >> > > 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. >> >> So, we need MSTP clocks which do not use CLK_SET_RATE_PARENT on r8a7740. >> Probably the best way is to use the "renesas,r8a7740-mstp-clocks" >> compatible entry to determine the difference? Geert? > > Well, I like the idea of a virtual sd clock even better. I sent a series > doing that a minute ago to the renesas-soc list. While I don't doubt the "virtual sd" clock will work, this doesn't match the hardware, so I have to object against it. Hence I think it should be handled in the driver. This is indeed more difficult to do with clk-mstp.c than with renesas-cpg-mssr.c... BTW, this behavior seems to be introduced by commit e44df332f30bf3040c60c1ed6674d1431fdb48b9 Author: Ben Dooks <ben.dooks@codethink.co.uk> Date: Mon Mar 31 18:47:27 2014 +0100 clk: shmobile: fix setting paretn clock rate Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Wolfram, On Fri, Apr 15, 2016 at 10:41 PM, Wolfram Sang <wsa@the-dreams.de> wrote: >> > 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. > > So, we need MSTP clocks which do not use CLK_SET_RATE_PARENT on r8a7740. > Probably the best way is to use the "renesas,r8a7740-mstp-clocks" > compatible entry to determine the difference? Geert? To fix this regression in the short term, can we just enable this feature in the sh_mobile_sdhi driver on SoCs where we know it's safe to change the clock, i.e. on R-Car Gen2 and H3? Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
> To fix this regression in the short term, can we just enable this feature in > the sh_mobile_sdhi driver on SoCs where we know it's safe to change the clock, > i.e. on R-Car Gen2 and H3? I'll try a workaround next week.
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)