| Message ID | 1459525479-20842-3-git-send-email-wsa@the-dreams.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 1, 2016 at 5:44 PM, Wolfram Sang <wsa@the-dreams.de> wrote: > From: Ben Hutchings <ben.hutchings@codethink.co.uk> > > Currently tmio_mmc assumes that the input clock frequency is fixed and > only its own clock divider can be changed. This is not true in the > case of sh_mobile_sdhi; we can use the clock API to change it. > > In tmio_mmc: > - Delegate setting of f_min from tmio to the clk_enable operation (if > implemented), as it can be smaller than f_max / 512 > - Add an optional clk_update operation called from tmio_mmc_set_clock() > that updates the input clock frequency > - Rename tmio_mmc_clk_update() to tmio_mmc_clk_enable(), to avoid > confusion with the clk_update operation > > In sh_mobile_sdhi: > - Make the setting of f_max conditional; it should be set through the > max-frequency property in the device tree in future > - Set f_min based on the input clock's minimum frequency > - Implement the clk_update operation, selecting the best input clock > frequency for the bus frequency that's wanted > > sh_mobile_sdhi_clk_update() is loosely based on Kuninori Morimoto's work > in sh_mmcif. > > Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> This is now commit 2e21101df4fe8bdc ("mmc: tmio, sh_mobile_sdhi: Add support for variable input clock frequency") in the next branch of Ulf's mmc.git. 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? 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. dmesg and /sys/kernel/debug/clk/clk_summary differences below. r8a73a4/ape6evm --------------- -sh_mobile_sdhi ee100000.sd: mmc0 base at 0xee100000 clock rate 88 MHz +sh_mobile_sdhi ee100000.sd: mmc0 base at 0xee100000 max clock rate 88 MHz -sh_mobile_sdhi ee120000.sd: mmc1 base at 0xee120000 clock rate 12 MHz +sh_mobile_sdhi ee120000.sd: mmc1 base at 0xee120000 max clock rate 12 MHz clock enable_cnt prepare_cnt rate accuracy phase ------------------------------------------------------------------------- - sdhi1ck 1 1 12500000 0 0 - sdhi1 2 2 12500000 0 0 - sdhi0ck 1 1 88888888 0 0 - sdhi0 1 2 88888888 0 0 + sdhi1ck 1 1 12698412 0 0 + sdhi1 2 2 12698412 0 0 + sdhi0ck 1 1 12698412 0 0 + sdhi0 1 2 12698412 0 0 r8a7791/koelsch --------------- -sh_mobile_sdhi ee100000.sd: mmc0 base at 0xee100000 clock rate 97 MHz +sh_mobile_sdhi ee100000.sd: mmc0 base at 0xee100000 max clock rate 97 MHz -sh_mobile_sdhi ee140000.sd: mmc1 base at 0xee140000 clock rate 48 MHz +sh_mobile_sdhi ee140000.sd: mmc1 base at 0xee140000 max clock rate 48 MHz -sh_mobile_sdhi ee160000.sd: mmc2 base at 0xee160000 clock rate 48 MHz +sh_mobile_sdhi ee160000.sd: mmc2 base at 0xee160000 max clock rate 48 MHz mmc0 0 0 12187500 0 0 mmcif0 0 0 12187500 0 0 - sd3 1 1 48750000 0 0 - sdhi2 1 2 48750000 0 0 - sd2 1 1 48750000 0 0 - sdhi1 1 2 48750000 0 0 + sd3 1 1 12786885 0 0 + sdhi2 1 2 12786885 0 0 + sd2 1 1 12786885 0 0 + sdhi1 1 2 12786885 0 0 sh73a0/kzm9g ------------ -sh_mobile_sdhi ee100000.sd: mmc0 base at 0xee100000 clock rate 69 MHz +sh_mobile_sdhi ee100000.sd: mmc0 base at 0xee100000 max clock rate 69 MHz -sh_mobile_sdhi ee140000.sd: mmc1 base at 0xee140000 clock rate 69 MHz +sh_mobile_sdhi ee140000.sd: mmc1 base at 0xee140000 max clock rate 69 MHz - sdhi2ck 1 1 69333333 0 0 - sdhi2 2 2 69333333 0 0 + sdhi2ck 1 1 12734693 0 0 + sdhi2 2 2 12734693 0 0 sdhi1ck 0 0 69333333 0 0 sdhi1 0 0 69333333 0 0 - sdhi0ck 1 1 69333333 0 0 - sdhi0 1 2 69333333 0 0 + sdhi0ck 1 1 12734693 0 0 + sdhi0 1 2 12734693 0 0 r8a7740/armadillo ----------------- -sh_mobile_sdhi e6850000.sd: mmc0 base at 0xe6850000 clock rate 99 MHz +sh_mobile_sdhi e6850000.sd: mmc0 base at 0xe6850000 max clock rate 99 MHz -sh_mmcif e6bd0000.mmc: Chip version 0x0003, clock rate 99MHz +sh_mmcif e6bd0000.mmc: Chip version 0x0003, clock rate 12MHz - hp 4 6 99000000 0 0 - tpu0 0 1 99000000 0 0 - gether 1 1 99000000 0 0 - mmc 2 2 99000000 0 0 - sdhi1 0 0 99000000 0 0 - sdhi0 1 2 99000000 0 0 - usbf 0 0 99000000 0 0 - fsi 0 1 99000000 0 0 - usbdmac 0 0 99000000 0 0 - dmac3 0 0 99000000 0 0 - dmac2 0 0 99000000 0 0 - dmac1 0 0 99000000 0 0 - intca 4 4 99000000 0 0 - usphy 0 0 99000000 0 0 - usbfunc 0 0 99000000 0 0 - sdhi2 0 0 99000000 0 0 - usbhost 0 0 99000000 0 0 + hp 4 6 12375000 0 0 + tpu0 0 1 12375000 0 0 + gether 1 1 12375000 0 0 + mmc 2 2 12375000 0 0 + sdhi1 0 0 12375000 0 0 + sdhi0 1 2 12375000 0 0 + usbf 0 0 12375000 0 0 + fsi 0 1 12375000 0 0 + usbdmac 0 0 12375000 0 0 + dmac3 0 0 12375000 0 0 + dmac2 0 0 12375000 0 0 + dmac1 0 0 12375000 0 0 + intca 4 4 12375000 0 0 + usphy 0 0 12375000 0 0 + usbfunc 0 0 12375000 0 0 + sdhi2 0 0 12375000 0 0 + usbhost 0 0 12375000 0 0 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 unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2016-04-12 at 14:55 +0200, Geert Uytterhoeven wrote: > On Fri, Apr 1, 2016 at 5:44 PM, Wolfram Sang <wsa@the-dreams.de> wrote: > > From: Ben Hutchings <ben.hutchings@codethink.co.uk> > > > > Currently tmio_mmc assumes that the input clock frequency is fixed and > > only its own clock divider can be changed. This is not true in the > > case of sh_mobile_sdhi; we can use the clock API to change it. > > > > In tmio_mmc: > > - Delegate setting of f_min from tmio to the clk_enable operation (if > > implemented), as it can be smaller than f_max / 512 > > - Add an optional clk_update operation called from tmio_mmc_set_clock() > > that updates the input clock frequency > > - Rename tmio_mmc_clk_update() to tmio_mmc_clk_enable(), to avoid > > confusion with the clk_update operation > > > > In sh_mobile_sdhi: > > - Make the setting of f_max conditional; it should be set through the > > max-frequency property in the device tree in future > > - Set f_min based on the input clock's minimum frequency > > - Implement the clk_update operation, selecting the best input clock > > frequency for the bus frequency that's wanted > > > > sh_mobile_sdhi_clk_update() is loosely based on Kuninori Morimoto's work > > in sh_mmcif. > > > > Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk> > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > This is now commit 2e21101df4fe8bdc ("mmc: tmio, sh_mobile_sdhi: Add support > for variable input clock frequency") in the next branch of Ulf's mmc.git. > > 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. > 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. Ben.
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c index 55849350202b7d..8fd1d6b29190b6 100644 --- a/drivers/mmc/host/sh_mobile_sdhi.c +++ b/drivers/mmc/host/sh_mobile_sdhi.c @@ -139,7 +139,20 @@ static int sh_mobile_sdhi_clk_enable(struct tmio_mmc_host *host) if (ret < 0) return ret; - mmc->f_max = clk_get_rate(priv->clk); + /* + * The clock driver may not know what maximum frequency + * actually works, so it should be set with the max-frequency + * property which will already have been read to f_max. If it + * was missing, assume the current frequency is the maximum. + */ + if (!mmc->f_max) + mmc->f_max = clk_get_rate(priv->clk); + + /* + * Minimum frequency is the minimum input clock frequency + * divided by our maximum divider. + */ + mmc->f_min = max(clk_round_rate(priv->clk, 1) / 512, 1L); /* enable 16bit data access on SDBUF as default */ sh_mobile_sdhi_sdbuf_width(host, 16); @@ -147,6 +160,44 @@ static int sh_mobile_sdhi_clk_enable(struct tmio_mmc_host *host) return 0; } +static unsigned int sh_mobile_sdhi_clk_update(struct tmio_mmc_host *host, + unsigned int new_clock) +{ + struct sh_mobile_sdhi *priv = host_to_priv(host); + unsigned int freq, best_freq, diff_min, diff; + int i; + + diff_min = ~0; + best_freq = 0; + + /* + * We want the bus clock to be as close as possible to, but no + * greater than, new_clock. As we can divide by 1 << i for + * any i in [0, 9] we want the input clock to be as close as + * possible, but no greater than, new_clock << i. + */ + for (i = min(9, ilog2(UINT_MAX / new_clock)); i >= 0; i--) { + freq = clk_round_rate(priv->clk, new_clock << i); + if (freq > (new_clock << i)) { + /* Too fast; look for a slightly slower option */ + freq = clk_round_rate(priv->clk, + (new_clock << i) / 4 * 3); + if (freq > (new_clock << i)) + continue; + } + + diff = new_clock - (freq >> i); + if (diff <= diff_min) { + best_freq = freq; + diff_min = diff; + } + } + + clk_set_rate(priv->clk, best_freq); + + return best_freq; +} + static void sh_mobile_sdhi_clk_disable(struct tmio_mmc_host *host) { struct sh_mobile_sdhi *priv = host_to_priv(host); @@ -265,6 +316,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev) host->dma = dma_priv; host->write16_hook = sh_mobile_sdhi_write16_hook; host->clk_enable = sh_mobile_sdhi_clk_enable; + host->clk_update = sh_mobile_sdhi_clk_update; host->clk_disable = sh_mobile_sdhi_clk_disable; host->multi_io_quirk = sh_mobile_sdhi_multi_io_quirk; @@ -362,7 +414,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev) } } - dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n", + dev_info(&pdev->dev, "%s base at 0x%08lx max clock rate %u MHz\n", mmc_hostname(host->mmc), (unsigned long) (platform_get_resource(pdev, IORESOURCE_MEM, 0)->start), host->mmc->f_max / 1000000); diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h index 68fd8d791358c1..b44b5890290622 100644 --- a/drivers/mmc/host/tmio_mmc.h +++ b/drivers/mmc/host/tmio_mmc.h @@ -96,6 +96,8 @@ struct tmio_mmc_host { int (*write16_hook)(struct tmio_mmc_host *host, int addr); int (*clk_enable)(struct tmio_mmc_host *host); + unsigned int (*clk_update)(struct tmio_mmc_host *host, + unsigned int new_clock); void (*clk_disable)(struct tmio_mmc_host *host); int (*multi_io_quirk)(struct mmc_card *card, unsigned int direction, int blk_size); diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c index d1160156678ade..ae81b34f17a5a5 100644 --- a/drivers/mmc/host/tmio_mmc_pio.c +++ b/drivers/mmc/host/tmio_mmc_pio.c @@ -160,9 +160,12 @@ static void tmio_mmc_set_clock(struct tmio_mmc_host *host, u32 clk = 0, clock; if (new_clock) { - for (clock = host->mmc->f_min, clk = 0x80000080; - new_clock >= (clock << 1); - clk >>= 1) + if (host->clk_update) + clock = host->clk_update(host, new_clock) / 512; + else + clock = host->mmc->f_min; + + for (clk = 0x80000080; new_clock >= (clock << 1); clk >>= 1) clock <<= 1; /* 1/1 clock is option */ @@ -837,19 +840,12 @@ fail: pm_runtime_put_autosuspend(mmc_dev(mmc)); } -static int tmio_mmc_clk_update(struct tmio_mmc_host *host) +static int tmio_mmc_clk_enable(struct tmio_mmc_host *host) { - struct mmc_host *mmc = host->mmc; - int ret; - if (!host->clk_enable) return -ENOTSUPP; - ret = host->clk_enable(host); - if (!ret) - mmc->f_min = mmc->f_max / 512; - - return ret; + return host->clk_enable(host); } static void tmio_mmc_power_on(struct tmio_mmc_host *host, unsigned short vdd) @@ -1135,7 +1131,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host, mmc->caps & MMC_CAP_NONREMOVABLE || mmc->slot.cd_irq >= 0); - if (tmio_mmc_clk_update(_host) < 0) { + if (tmio_mmc_clk_enable(_host) < 0) { mmc->f_max = pdata->hclk; mmc->f_min = mmc->f_max / 512; } @@ -1263,7 +1259,7 @@ int tmio_mmc_host_runtime_resume(struct device *dev) struct tmio_mmc_host *host = mmc_priv(mmc); tmio_mmc_reset(host); - tmio_mmc_clk_update(host); + tmio_mmc_clk_enable(host); if (host->clk_cache) { tmio_mmc_set_clock(host, host->clk_cache);