Hi Morimoto-san,
On Tue, Apr 21, 2015 at 10:31 AM, Kuninori Morimoto
<kuninori.morimoto.gx@xxxxxxxxxxx> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
>
> MMCIF IP on R-Car series has parent clock which can be set
> several rate, and it was not implemented on old SH-Mobile series
> (= SH-Mobile series parent clock was fixed rate)
> R-Car series MMCIF can use more high speed access if it setup
> parent clock. This patch adds parent clock setup method,
> and r8a7790/r8a7791 can use it.
>
> renesas,mmcif already contain renesas,mmcif-r8a7790/r8a7791 on
> compatible string. So there is no update for binding Document.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> Tested-by: Keita Kobayashi <keita.kobayashi.ym@xxxxxxxxxxx>
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index 5282c5b..6ae836b 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -224,6 +225,12 @@ enum mmcif_wait_for {
> MMCIF_WAIT_FOR_STOP,
> };
>
> +struct sh_mmcif_parent_clk {
> + unsigned int max; /* if exist: R-Car has MMC CKCR on CPG */
> + unsigned int min; /* if exist: R-Car has MMC CKCR on CPG */
> + u32 clkdiv_map; /* see CE_CLK_CTRL::CLKDIV */
> +};
> +
> struct sh_mmcif_host {
> struct mmc_host *mmc;
> struct mmc_request *mrq;
> @@ -249,6 +256,7 @@ struct sh_mmcif_host {
> bool ccs_enable; /* Command Completion Signal support */
> bool clk_ctrl2_enable;
> struct mutex thread_lock;
> + const struct sh_mmcif_parent_clk *parent_clk;
>
> /* DMA support */
> struct dma_chan *chan_rx;
> @@ -257,8 +265,16 @@ struct sh_mmcif_host {
> bool dma_active;
> };
>
> +static const struct sh_mmcif_parent_clk mmcif_gen2_parent_clk = {
> + .max = 97500000,
> + .min = 12187500,
> + .clkdiv_map = 0x3ff,
Shouldn't this come from private data in the CPG clock driver, which supplies
the parent clock? Then the mmcif driver will be independent from the parent
clock.
> +};
> +
> static const struct of_device_id mmcif_of_match[] = {
> { .compatible = "renesas,sh-mmcif" },
> + { .compatible = "renesas,mmcif-r8a7790", .data = &mmcif_gen2_parent_clk},
> + { .compatible = "renesas,mmcif-r8a7791", .data = &mmcif_gen2_parent_clk},
> { }
> };
> MODULE_DEVICE_TABLE(of, mmcif_of_match);
> @@ -490,12 +506,51 @@ static void sh_mmcif_clock_control(struct sh_mmcif_host *host, unsigned int clk)
>
> if (!clk)
> return;
> - if (sup_pclk && clk == host->clk)
> +
> + if (host->parent_clk) {
> + const struct sh_mmcif_parent_clk *pclk = host->parent_clk;
> + unsigned int parent_freq, clkdiv, myclk, diff_min, diff;
> + int i, j;
> +
> + /* FIXME */
> + if (pclk->max != pclk->min * (pclk->max / pclk->min)) {
> + dev_err(&host->pd->dev, "not assumed parent clk\n");
> + return;
> + }
Why?
> + parent_freq = 0;
> + clkdiv = 0;
> + diff_min = ~0;
> + for (i = pclk->max / pclk->min; i > 0; i--) {
> + for (j = fls(pclk->clkdiv_map); j >= 0; j--) {
> + if (!((1 << j) & pclk->clkdiv_map))
> + continue;
> +
> + myclk = ((pclk->min * i) / (1 << (j + 1)));
> + diff = (myclk > clk) ? myclk - clk : clk - myclk;
> +
> + if (diff <= diff_min) {
> + parent_freq = pclk->min * i;
> + clkdiv = j;
> + diff_min = diff;
> + }
> + }
> + }
Shouldn't the above be handled by the CPG clock driver, through a clk API?
> + dev_dbg(&host->pd->dev, "clk %d/%d (%d, %x)\n",
"%u" (all four)
> + (parent_freq / (1 << (clkdiv + 1))), clk,
> + parent_freq, clkdiv);
> +
> + clk_set_rate(host->hclk, parent_freq);
Note that the last parameter of clk_set_rate() is "unsigned long", while
parent_freq is "unsigned int".
> + sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL,
> + CLK_CLEAR & (clkdiv << 16));
> + } else if (sup_pclk && clk == host->clk) {
> sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_SUP_PCLK);
> - else
> + } else {
> sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_CLEAR &
> ((fls(DIV_ROUND_UP(host->clk,
> clk) - 1) - 1) << 16));
> + }
>
> sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_ENABLE);
> }
> @@ -982,10 +1037,23 @@ static int sh_mmcif_clk_update(struct sh_mmcif_host *host)
> {
> int ret = clk_prepare_enable(host->hclk);
>
> - if (!ret) {
> + if (ret)
> + return ret;
> +
> + if (!host->parent_clk) {
> host->clk = clk_get_rate(host->hclk);
> host->mmc->f_max = host->clk / 2;
> host->mmc->f_min = host->clk / 512;
> + } else {
> + const struct sh_mmcif_parent_clk *pclk = host->parent_clk;
> +
> + host->clk = clk_get_rate(host->hclk);
clk_get_rate() returns "unsigned long", while "host->clk" is "unsigned int".
> + host->mmc->f_max = pclk->max / (1 << ffs(pclk->clkdiv_map));
> + host->mmc->f_min = pclk->min / (1 << fls(pclk->clkdiv_map));
> +
> + dev_dbg(&host->pd->dev, "parent clk %d/%d, %d/%d\n",
"%u" (all four)
> + pclk->max, pclk->min,
> + host->mmc->f_max, host->mmc->f_min);
> }
>
> return ret;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
