Re: [PATCH 3/3] mmc: sh_mmcif: calculate best clock with PLL

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Morimoto-san,

On Tuesday 21 April 2015 07:43:33 Kuninori Morimoto wrote:
> Hi Ulf
> 
> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > MMCIF IP on R-Car series has PLL (= parent clock) which was not
> > implemented on old SH-Mobile series. MMCIF can use more high speed
> > access if it is possible to use PLL.
> > This patch adds PLL support for 8a7790/r8a7791.
> > 
> > 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@renesas.com>
> > Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
> 
> Here, I got feedback in off-line, and it indicates this explain
> is using strange nameing (= MMCIF parent clock is divider, not PLL).
> I would like to send v2 patch

Could you please CC the linux-sh list when you'll send v2 ?

> > ---
> > 
> >  drivers/mmc/host/sh_mmcif.c | 79 ++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 76 insertions(+), 3 deletions(-)
> > 
> > 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
> > @@ -57,6 +57,7 @@
> > 
> >  #include <linux/mmc/slot-gpio.h>
> >  #include <linux/mod_devicetable.h>
> >  #include <linux/mutex.h>
> > 
> > +#include <linux/of_device.h>
> > 
> >  #include <linux/pagemap.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/pm_qos.h>
> > 
> > @@ -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,
> > +};
> > +
> > 
> >  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;
> > +		}
> > +
> > +		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;
> > +				}
> > +			}
> > +		}
> > +
> > +		dev_dbg(&host->pd->dev, "clk %d/%d (%d, %x)\n",
> > +			(parent_freq / (1 << (clkdiv + 1))), clk,
> > +			parent_freq, clkdiv);
> > +
> > +		clk_set_rate(host->hclk, parent_freq);
> > +		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);
> > +		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",
> > +			pclk->max, pclk->min,
> > +			host->mmc->f_max, host->mmc->f_min);
> > 
> >  	}
> >  	
> >  	return ret;
> > 
> > @@ -1389,6 +1457,7 @@ static int sh_mmcif_probe(struct platform_device
> > *pdev)> 
> >  	struct device *dev = &pdev->dev;
> >  	struct sh_mmcif_plat_data *pd = dev->platform_data;
> >  	struct resource *res;
> > 
> > +	const struct of_device_id *of_id = of_match_device(mmcif_of_match, 
dev);
> > 
> >  	void __iomem *reg;
> >  	const char *name;
> > 
> > @@ -1421,6 +1490,10 @@ static int sh_mmcif_probe(struct platform_device
> > *pdev)> 
> >  	host->pd = pdev;
> > 
> > +	host->parent_clk = NULL;
> > +	if (of_id)
> > +		host->parent_clk = of_id->data;
> > +
> > 
> >  	spin_lock_init(&host->lock);
> >  	
> >  	mmc->ops = &sh_mmcif_ops;

-- 
Regards,

Laurent Pinchart