mmc: sh_mmcif: add PLL support

mmc: sh_mmcif: add PLL support

[Kuninori Morimoto]

Hi Ulf

These patches adds PLL support for sh_mmcif.
Current sh_mmcif driver was used on Renesas SH-Mobile series,
and it is also used on Renesas R-Car series.
But, R-Car series has PLL which was not implemented on SH-Mobile.

These patches are tested by Kobayashi-san on Lager board.

Kuninori Morimoto (3):
      mmc: sh_mmcif: move mmcif_of_match to upside
      mmc: sh_mmcif: cleanup to use dev instead of &pdev->dev
      mmc: sh_mmcif: calculate best clock with PLL

 drivers/mmc/host/sh_mmcif.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 99 insertions(+), 25 deletions(-)

Best regards
---
Kuninori Morimoto

[PATCH 1/3] mmc: sh_mmcif: move mmcif_of_match to upside

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
 drivers/mmc/host/sh_mmcif.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 2b6ef6b..4e81278 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -257,6 +257,12 @@ struct sh_mmcif_host {
 	bool			dma_active;
 };
 
+static const struct of_device_id mmcif_of_match[] = {
+	{ .compatible = "renesas,sh-mmcif" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mmcif_of_match);
+
 static inline void sh_mmcif_bitset(struct sh_mmcif_host *host,
 					unsigned int reg, u32 val)
 {
@@ -1543,12 +1549,6 @@ static int sh_mmcif_resume(struct device *dev)
 }
 #endif
 
-static const struct of_device_id mmcif_of_match[] = {
-	{ .compatible = "renesas,sh-mmcif" },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, mmcif_of_match);
-
 static const struct dev_pm_ops sh_mmcif_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(sh_mmcif_suspend, sh_mmcif_resume)
 };
-- 
1.9.1

[PATCH 2/3] mmc: sh_mmcif: cleanup to use dev instead of &pdev->dev

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
 drivers/mmc/host/sh_mmcif.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 4e81278..5282c5b 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1386,7 +1386,8 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	int ret = 0, irq[2];
 	struct mmc_host *mmc;
 	struct sh_mmcif_host *host;
-	struct sh_mmcif_plat_data *pd = pdev->dev.platform_data;
+	struct device *dev = &pdev->dev;
+	struct sh_mmcif_plat_data *pd = dev->platform_data;
 	struct resource *res;
 	void __iomem *reg;
 	const char *name;
@@ -1394,16 +1395,16 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	irq[0] = platform_get_irq(pdev, 0);
 	irq[1] = platform_get_irq(pdev, 1);
 	if (irq[0] < 0) {
-		dev_err(&pdev->dev, "Get irq error\n");
+		dev_err(dev, "Get irq error\n");
 		return -ENXIO;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, res);
+	reg = devm_ioremap_resource(dev, res);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
-	mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), &pdev->dev);
+	mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), dev);
 	if (!mmc)
 		return -ENOMEM;
 
@@ -1436,20 +1437,20 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, host);
 
-	pm_runtime_enable(&pdev->dev);
+	pm_runtime_enable(dev);
 	host->power = false;
 
-	host->hclk = devm_clk_get(&pdev->dev, NULL);
+	host->hclk = devm_clk_get(dev, NULL);
 	if (IS_ERR(host->hclk)) {
 		ret = PTR_ERR(host->hclk);
-		dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
+		dev_err(dev, "cannot get clock: %d\n", ret);
 		goto err_pm;
 	}
 	ret = sh_mmcif_clk_update(host);
 	if (ret < 0)
 		goto err_pm;
 
-	ret = pm_runtime_resume(&pdev->dev);
+	ret = pm_runtime_resume(dev);
 	if (ret < 0)
 		goto err_clk;
 
@@ -1458,19 +1459,19 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	sh_mmcif_sync_reset(host);
 	sh_mmcif_writel(host->addr, MMCIF_CE_INT_MASK, MASK_ALL);
 
-	name = irq[1] < 0 ? dev_name(&pdev->dev) : "sh_mmc:error";
-	ret = devm_request_threaded_irq(&pdev->dev, irq[0], sh_mmcif_intr,
+	name = irq[1] < 0 ? dev_name(dev) : "sh_mmc:error";
+	ret = devm_request_threaded_irq(dev, irq[0], sh_mmcif_intr,
 					sh_mmcif_irqt, 0, name, host);
 	if (ret) {
-		dev_err(&pdev->dev, "request_irq error (%s)\n", name);
+		dev_err(dev, "request_irq error (%s)\n", name);
 		goto err_clk;
 	}
 	if (irq[1] >= 0) {
-		ret = devm_request_threaded_irq(&pdev->dev, irq[1],
+		ret = devm_request_threaded_irq(dev, irq[1],
 						sh_mmcif_intr, sh_mmcif_irqt,
 						0, "sh_mmc:int", host);
 		if (ret) {
-			dev_err(&pdev->dev, "request_irq error (sh_mmc:int)\n");
+			dev_err(dev, "request_irq error (sh_mmc:int)\n");
 			goto err_clk;
 		}
 	}
@@ -1487,9 +1488,9 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_clk;
 
-	dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
+	dev_pm_qos_expose_latency_limit(dev, 100);
 
-	dev_info(&pdev->dev, "Chip version 0x%04x, clock rate %luMHz\n",
+	dev_info(dev, "Chip version 0x%04x, clock rate %luMHz\n",
 		 sh_mmcif_readl(host->addr, MMCIF_CE_VERSION) & 0xffff,
 		 clk_get_rate(host->hclk) / 1000000UL);
 
@@ -1499,7 +1500,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 err_clk:
 	clk_disable_unprepare(host->hclk);
 err_pm:
-	pm_runtime_disable(&pdev->dev);
+	pm_runtime_disable(dev);
 err_host:
 	mmc_free_host(mmc);
 	return ret;
-- 
1.9.1

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

[Kuninori Morimoto]

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>
---
 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;
-- 
1.9.1

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

[Kuninori Morimoto]

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

> ---
>  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;
> -- 
> 1.9.1
> 
> --
> 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


Best regards
---
Kuninori Morimoto

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

[Laurent Pinchart]

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

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

[Kuninori Morimoto]

Hi Laurent

> > > 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 ?

Ohh bad timing.
I already sent v2...

Best regards
---
Kuninori Morimoto

[PATCH 0/3 v2] mmc: sh_mmcif: add PLL support

[Kuninori Morimoto]

Hi Ulf

These patches are v2 of sh_mmcif.
Current sh_mmcif driver was used on Renesas SH-Mobile series,
and it is also used on Renesas R-Car series.
But, R-Car series can setup parent clock.
We couldn't setup it when SH-Mobile.

These patches are tested by Kobayashi-san on Lager board.

Kuninori Morimoto (3):
      mmc: sh_mmcif: move mmcif_of_match to upside
      mmc: sh_mmcif: cleanup to use dev instead of &pdev->dev
      mmc: sh_mmcif: calculate best clock with parent clock

 drivers/mmc/host/sh_mmcif.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 99 insertions(+), 25 deletions(-)

Best regards
---
Kuninori Morimoto
--
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

[PATCH 1/3 v2] mmc: sh_mmcif: move mmcif_of_match to upside

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v1 -> v2

 - no change

 drivers/mmc/host/sh_mmcif.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 2b6ef6b..4e81278 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -257,6 +257,12 @@ struct sh_mmcif_host {
 	bool			dma_active;
 };
 
+static const struct of_device_id mmcif_of_match[] = {
+	{ .compatible = "renesas,sh-mmcif" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mmcif_of_match);
+
 static inline void sh_mmcif_bitset(struct sh_mmcif_host *host,
 					unsigned int reg, u32 val)
 {
@@ -1543,12 +1549,6 @@ static int sh_mmcif_resume(struct device *dev)
 }
 #endif
 
-static const struct of_device_id mmcif_of_match[] = {
-	{ .compatible = "renesas,sh-mmcif" },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, mmcif_of_match);
-
 static const struct dev_pm_ops sh_mmcif_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(sh_mmcif_suspend, sh_mmcif_resume)
 };
-- 
1.9.1

[PATCH 2/3 v2] mmc: sh_mmcif: cleanup to use dev instead of &pdev->dev

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v2 -> v3

 - no change

 drivers/mmc/host/sh_mmcif.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 4e81278..5282c5b 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1386,7 +1386,8 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	int ret = 0, irq[2];
 	struct mmc_host *mmc;
 	struct sh_mmcif_host *host;
-	struct sh_mmcif_plat_data *pd = pdev->dev.platform_data;
+	struct device *dev = &pdev->dev;
+	struct sh_mmcif_plat_data *pd = dev->platform_data;
 	struct resource *res;
 	void __iomem *reg;
 	const char *name;
@@ -1394,16 +1395,16 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	irq[0] = platform_get_irq(pdev, 0);
 	irq[1] = platform_get_irq(pdev, 1);
 	if (irq[0] < 0) {
-		dev_err(&pdev->dev, "Get irq error\n");
+		dev_err(dev, "Get irq error\n");
 		return -ENXIO;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, res);
+	reg = devm_ioremap_resource(dev, res);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
-	mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), &pdev->dev);
+	mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), dev);
 	if (!mmc)
 		return -ENOMEM;
 
@@ -1436,20 +1437,20 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, host);
 
-	pm_runtime_enable(&pdev->dev);
+	pm_runtime_enable(dev);
 	host->power = false;
 
-	host->hclk = devm_clk_get(&pdev->dev, NULL);
+	host->hclk = devm_clk_get(dev, NULL);
 	if (IS_ERR(host->hclk)) {
 		ret = PTR_ERR(host->hclk);
-		dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
+		dev_err(dev, "cannot get clock: %d\n", ret);
 		goto err_pm;
 	}
 	ret = sh_mmcif_clk_update(host);
 	if (ret < 0)
 		goto err_pm;
 
-	ret = pm_runtime_resume(&pdev->dev);
+	ret = pm_runtime_resume(dev);
 	if (ret < 0)
 		goto err_clk;
 
@@ -1458,19 +1459,19 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	sh_mmcif_sync_reset(host);
 	sh_mmcif_writel(host->addr, MMCIF_CE_INT_MASK, MASK_ALL);
 
-	name = irq[1] < 0 ? dev_name(&pdev->dev) : "sh_mmc:error";
-	ret = devm_request_threaded_irq(&pdev->dev, irq[0], sh_mmcif_intr,
+	name = irq[1] < 0 ? dev_name(dev) : "sh_mmc:error";
+	ret = devm_request_threaded_irq(dev, irq[0], sh_mmcif_intr,
 					sh_mmcif_irqt, 0, name, host);
 	if (ret) {
-		dev_err(&pdev->dev, "request_irq error (%s)\n", name);
+		dev_err(dev, "request_irq error (%s)\n", name);
 		goto err_clk;
 	}
 	if (irq[1] >= 0) {
-		ret = devm_request_threaded_irq(&pdev->dev, irq[1],
+		ret = devm_request_threaded_irq(dev, irq[1],
 						sh_mmcif_intr, sh_mmcif_irqt,
 						0, "sh_mmc:int", host);
 		if (ret) {
-			dev_err(&pdev->dev, "request_irq error (sh_mmc:int)\n");
+			dev_err(dev, "request_irq error (sh_mmc:int)\n");
 			goto err_clk;
 		}
 	}
@@ -1487,9 +1488,9 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_clk;
 
-	dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
+	dev_pm_qos_expose_latency_limit(dev, 100);
 
-	dev_info(&pdev->dev, "Chip version 0x%04x, clock rate %luMHz\n",
+	dev_info(dev, "Chip version 0x%04x, clock rate %luMHz\n",
 		 sh_mmcif_readl(host->addr, MMCIF_CE_VERSION) & 0xffff,
 		 clk_get_rate(host->hclk) / 1000000UL);
 
@@ -1499,7 +1500,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 err_clk:
 	clk_disable_unprepare(host->hclk);
 err_pm:
-	pm_runtime_disable(&pdev->dev);
+	pm_runtime_disable(dev);
 err_host:
 	mmc_free_host(mmc);
 	return ret;
-- 
1.9.1

[PATCH 3/3 v2] mmc: sh_mmcif: calculate best clock with parent clock

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

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@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v1 -> v2

 - tidyup log explain.
   (use "parent clock" instead of "PLL")

 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;
-- 
1.9.1

Re: [PATCH 0/3 v2] mmc: sh_mmcif: add PLL support

[Kuninori Morimoto]

Hi Ulf

> These patches are v2 of sh_mmcif.
> Current sh_mmcif driver was used on Renesas SH-Mobile series,
> and it is also used on Renesas R-Car series.
> But, R-Car series can setup parent clock.
> We couldn't setup it when SH-Mobile.

Sorry for many times.
I will send v3 patches which will be send to SH ML

Best regards
---
Kuninori Morimoto

[PATCH 0/3 v3] mmc: sh_mmcif: add PLL support

[Kuninori Morimoto]

Hi Ulf

These patches are v3 of sh_mmcif.
Current sh_mmcif driver was used on Renesas SH-Mobile series,
and it is also used on Renesas R-Car series.
But, R-Car series can setup parent clock.
We couldn't setup it when SH-Mobile.

These patches are tested by Kobayashi-san on Lager board.

Kuninori Morimoto (3):
      mmc: sh_mmcif: move mmcif_of_match to upside
      mmc: sh_mmcif: cleanup to use dev instead of &pdev->dev
      mmc: sh_mmcif: calculate best clock with parent clock

 drivers/mmc/host/sh_mmcif.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 99 insertions(+), 25 deletions(-)

Best regards
---
Kuninori Morimoto
--
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
--
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

[PATCH 1/3 v3] mmc: sh_mmcif: move mmcif_of_match to upside

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v2 -> v3

 - add SH-ML

 drivers/mmc/host/sh_mmcif.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 2b6ef6b..4e81278 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -257,6 +257,12 @@ struct sh_mmcif_host {
 	bool			dma_active;
 };
 
+static const struct of_device_id mmcif_of_match[] = {
+	{ .compatible = "renesas,sh-mmcif" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mmcif_of_match);
+
 static inline void sh_mmcif_bitset(struct sh_mmcif_host *host,
 					unsigned int reg, u32 val)
 {
@@ -1543,12 +1549,6 @@ static int sh_mmcif_resume(struct device *dev)
 }
 #endif
 
-static const struct of_device_id mmcif_of_match[] = {
-	{ .compatible = "renesas,sh-mmcif" },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, mmcif_of_match);
-
 static const struct dev_pm_ops sh_mmcif_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(sh_mmcif_suspend, sh_mmcif_resume)
 };
-- 
1.9.1

Re: [PATCH 1/3 v3] mmc: sh_mmcif: move mmcif_of_match to upside

[Geert Uytterhoeven]

On Tue, Apr 21, 2015 at 10:26 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

[PATCH 2/3 v3] mmc: sh_mmcif: cleanup to use dev instead of &pdev->dev

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v2 -> v3

 - add SH-ML

 drivers/mmc/host/sh_mmcif.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 4e81278..5282c5b 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1386,7 +1386,8 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	int ret = 0, irq[2];
 	struct mmc_host *mmc;
 	struct sh_mmcif_host *host;
-	struct sh_mmcif_plat_data *pd = pdev->dev.platform_data;
+	struct device *dev = &pdev->dev;
+	struct sh_mmcif_plat_data *pd = dev->platform_data;
 	struct resource *res;
 	void __iomem *reg;
 	const char *name;
@@ -1394,16 +1395,16 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	irq[0] = platform_get_irq(pdev, 0);
 	irq[1] = platform_get_irq(pdev, 1);
 	if (irq[0] < 0) {
-		dev_err(&pdev->dev, "Get irq error\n");
+		dev_err(dev, "Get irq error\n");
 		return -ENXIO;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, res);
+	reg = devm_ioremap_resource(dev, res);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
-	mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), &pdev->dev);
+	mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), dev);
 	if (!mmc)
 		return -ENOMEM;
 
@@ -1436,20 +1437,20 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, host);
 
-	pm_runtime_enable(&pdev->dev);
+	pm_runtime_enable(dev);
 	host->power = false;
 
-	host->hclk = devm_clk_get(&pdev->dev, NULL);
+	host->hclk = devm_clk_get(dev, NULL);
 	if (IS_ERR(host->hclk)) {
 		ret = PTR_ERR(host->hclk);
-		dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
+		dev_err(dev, "cannot get clock: %d\n", ret);
 		goto err_pm;
 	}
 	ret = sh_mmcif_clk_update(host);
 	if (ret < 0)
 		goto err_pm;
 
-	ret = pm_runtime_resume(&pdev->dev);
+	ret = pm_runtime_resume(dev);
 	if (ret < 0)
 		goto err_clk;
 
@@ -1458,19 +1459,19 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	sh_mmcif_sync_reset(host);
 	sh_mmcif_writel(host->addr, MMCIF_CE_INT_MASK, MASK_ALL);
 
-	name = irq[1] < 0 ? dev_name(&pdev->dev) : "sh_mmc:error";
-	ret = devm_request_threaded_irq(&pdev->dev, irq[0], sh_mmcif_intr,
+	name = irq[1] < 0 ? dev_name(dev) : "sh_mmc:error";
+	ret = devm_request_threaded_irq(dev, irq[0], sh_mmcif_intr,
 					sh_mmcif_irqt, 0, name, host);
 	if (ret) {
-		dev_err(&pdev->dev, "request_irq error (%s)\n", name);
+		dev_err(dev, "request_irq error (%s)\n", name);
 		goto err_clk;
 	}
 	if (irq[1] >= 0) {
-		ret = devm_request_threaded_irq(&pdev->dev, irq[1],
+		ret = devm_request_threaded_irq(dev, irq[1],
 						sh_mmcif_intr, sh_mmcif_irqt,
 						0, "sh_mmc:int", host);
 		if (ret) {
-			dev_err(&pdev->dev, "request_irq error (sh_mmc:int)\n");
+			dev_err(dev, "request_irq error (sh_mmc:int)\n");
 			goto err_clk;
 		}
 	}
@@ -1487,9 +1488,9 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_clk;
 
-	dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
+	dev_pm_qos_expose_latency_limit(dev, 100);
 
-	dev_info(&pdev->dev, "Chip version 0x%04x, clock rate %luMHz\n",
+	dev_info(dev, "Chip version 0x%04x, clock rate %luMHz\n",
 		 sh_mmcif_readl(host->addr, MMCIF_CE_VERSION) & 0xffff,
 		 clk_get_rate(host->hclk) / 1000000UL);
 
@@ -1499,7 +1500,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 err_clk:
 	clk_disable_unprepare(host->hclk);
 err_pm:
-	pm_runtime_disable(&pdev->dev);
+	pm_runtime_disable(dev);
 err_host:
 	mmc_free_host(mmc);
 	return ret;
-- 
1.9.1

Re: [PATCH 2/3 v3] mmc: sh_mmcif: cleanup to use dev instead of &pdev->dev

[Geert Uytterhoeven]

On Tue, Apr 21, 2015 at 10:27 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

[PATCH 3/3 v3] mmc: sh_mmcif: calculate best clock with parent clock

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

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@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v1 -> v2

 - tidyup log explain.
   (use "parent clock" instead of "PLL")

v2 -> v3

 - add SH-ML

 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;
-- 
1.9.1

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

[Geert Uytterhoeven]

Hi Morimoto-san,

On Tue, Apr 21, 2015 at 10:31 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> 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@renesas.com>
> Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>

> 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@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

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

[Laurent Pinchart]

Hello,

On Tuesday 21 April 2015 12:31:21 Geert Uytterhoeven wrote:
> On Tue, Apr 21, 2015 at 10:31 AM, Kuninori Morimoto wrote:
> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > 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@renesas.com>
> > Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
> > 
> > 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 {

I'm not sure I would call this parent clock. It refers to the frequency of the 
functional clock provided to the MMCIF, there's no concept of parent there. 
True, the clock referenced by the MMCIF DT node is an MSTP gate clock, and 
frequency control is implemented in the MSTP clock parent, but that hardware 
architecture is internal to the CPG and shouldn't be considered by the MMCIF.

> > +       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.

The real question is where the constraint comes from. The Gen2 datasheet 
documents the MMC clocks maximum frequency as 97.5 MHz in the CPG section. 
However, I have a feeling the constraint doesn't come from the DIV6 which 
should be able to output higher frequencies, but from the MMCIF, the clock 
distribution tree, or both.

There's also the option of specifying the admissible clock range in DT, either 
in the CPG node or the MMCIF node.

> > +};

-- 
Regards,

Laurent Pinchart

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

[Kuninori Morimoto]

Hi Laurent

> > > +struct sh_mmcif_parent_clk {
> 
> I'm not sure I would call this parent clock. It refers to the frequency of the 
> functional clock provided to the MMCIF, there's no concept of parent there. 
> True, the clock referenced by the MMCIF DT node is an MSTP gate clock, and 
> frequency control is implemented in the MSTP clock parent, but that hardware 
> architecture is internal to the CPG and shouldn't be considered by the MMCIF.

I couldn't understand about last 2 lines.
Do you mean I need to add these method in CPG ?
But, this calculates best clock of parent clock (= from CPG) and divider (= on MMC)
Why CPG need to care about MMC's divider,
and how to set it from CPG ??

> > 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.
> 
> The real question is where the constraint comes from. The Gen2 datasheet 
> documents the MMC clocks maximum frequency as 97.5 MHz in the CPG section. 
> However, I have a feeling the constraint doesn't come from the DIV6 which 
> should be able to output higher frequencies, but from the MMCIF, the clock 
> distribution tree, or both.
> 
> There's also the option of specifying the admissible clock range in DT, either 
> in the CPG node or the MMCIF node.

It needs not only max/min range, but also divider's limit.
I don't know how to do it. but...

 1) add admissible max/min range on CPG node
    add admissible divider range on MMC node
     -> Does this max/min range really from CPG ?
        but, we can reuse this on other DIV6

 2) add admissible max/min range on MMC node
    add admissible divider range on MMC node
     -> reasonable, but we can't reuse it on other DIV6
        what is difference between 2) and 3) ?

 3) add admissible max/min range on MMC driver
    add admissible divider range on MMC driver
     -> This is the reason why we have SoC specific compatible name ?
        These limit came from SoC.

In my opinion, I can accept 1) or 3).

Best regards
---
Kuninori Morimoto

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

[Laurent Pinchart]

Hi Morimoto-san,

Sorry for the late reply.

On Wednesday 22 April 2015 01:05:59 Kuninori Morimoto wrote:
> Hi Laurent
> 
> >>> +struct sh_mmcif_parent_clk {
> > 
> > I'm not sure I would call this parent clock. It refers to the frequency of
> > the functional clock provided to the MMCIF, there's no concept of parent
> > there. True, the clock referenced by the MMCIF DT node is an MSTP gate
> > clock, and frequency control is implemented in the MSTP clock parent, but
> > that hardware architecture is internal to the CPG and shouldn't be
> > considered by the MMCIF.
>
> I couldn't understand about last 2 lines.
> Do you mean I need to add these method in CPG ?
> But, this calculates best clock of parent clock (= from CPG) and divider (=
> on MMC) Why CPG need to care about MMC's divider,
> and how to set it from CPG ??

No, that's not what I meant. The CPG should certainly not care about any MMCIF 
internal divider.

My point was about the name of the structure. However, now that you mention 
the internal MMCIF clock divider, the word "parent" indeed makes sense. Please 
ignore my comment.

> >> 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.
> > 
> > The real question is where the constraint comes from. The Gen2 datasheet
> > documents the MMC clocks maximum frequency as 97.5 MHz in the CPG section.
> > However, I have a feeling the constraint doesn't come from the DIV6 which
> > should be able to output higher frequencies, but from the MMCIF, the clock
> > distribution tree, or both.
> > 
> > There's also the option of specifying the admissible clock range in DT,
> > either in the CPG node or the MMCIF node.
> 
> It needs not only max/min range, but also divider's limit.
> I don't know how to do it. but...
> 
>  1) add admissible max/min range on CPG node
>     add admissible divider range on MMC node
>      -> Does this max/min range really from CPG ?
>         but, we can reuse this on other DIV6
> 
>  2) add admissible max/min range on MMC node
>     add admissible divider range on MMC node
>      -> reasonable, but we can't reuse it on other DIV6
>         what is difference between 2) and 3) ?
> 
>  3) add admissible max/min range on MMC driver
>     add admissible divider range on MMC driver
>      -> This is the reason why we have SoC specific compatible name ?
>         These limit came from SoC.
> 
> In my opinion, I can accept 1) or 3).

Once again it really depends where the constraints come from. If the MMC input 
clock frequency range constraint is dictated by the MMCIF IP core only, 
options 2 and 3 are equivalent and just differ by whether they specify the 
range explicitly in DT (option 2) or implicitly through the compatible name 
(option 3).

If the range constraint comes from the CPG, option 1 is the way to go, 
(although the admissible divider range could also be speficied in the MMC 
driver as in option 3).

If the range constraint is a property of the integration of the MMCIF in the 
SoC then I think option 2 is the best. The per-SoC compatible strings can be 
used to express differences between IP core versions found in different SoCs, 
but they shouldn't be used to express integration properties.

To clarify this with an example, it's fine for the DU driver to use the per-
SoC compatible string to know how many display channels the DU contains, but 
it can't be used to hardcode IRQ numbers in the driver, even if we know which 
IRQ the DU is hooked to for each SoC. The former is a property of the DU 
version, the latter is a property of the integration and should be expressed 
in DT.

-- 
Regards,

Laurent Pinchart

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

[Kuninori Morimoto]

Hi Laurent

> Hi Morimoto-san,
> 
> Sorry for the late reply.

Sorry for my late response too.

> > > I'm not sure I would call this parent clock. It refers to the frequency of
> > > the functional clock provided to the MMCIF, there's no concept of parent
> > > there. True, the clock referenced by the MMCIF DT node is an MSTP gate
> > > clock, and frequency control is implemented in the MSTP clock parent, but
> > > that hardware architecture is internal to the CPG and shouldn't be
> > > considered by the MMCIF.
> >
> > I couldn't understand about last 2 lines.
> > Do you mean I need to add these method in CPG ?
> > But, this calculates best clock of parent clock (= from CPG) and divider (=
> > on MMC) Why CPG need to care about MMC's divider,
> > and how to set it from CPG ??
> 
> No, that's not what I meant. The CPG should certainly not care about any MMCIF 
> internal divider.
> 
> My point was about the name of the structure. However, now that you mention 
> the internal MMCIF clock divider, the word "parent" indeed makes sense. Please 
> ignore my comment.

Thank you for your feedback, and thank you for your understanding.

Best regards
---
Kuninori Morimoto

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

[Kuninori Morimoto]

Hi Geert

Thank you for your review.

> > +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.

As Laurent mentioned in other feedback, parent clock (= CFG::MMCxCKCR) 
is div6, and it can set 1/1 - 1/64 (= 780000000 - 12187500)
But, MMC can use 1/8 - 1/64 (= 97500000 - 12187500), we don't know this limit
came from. we thought that having limit on DIV6 is not good idea.


> > +               /* FIXME */
> > +               if (pclk->max != pclk->min * (pclk->max / pclk->min)) {
> > +                       dev_err(&host->pd->dev, "not assumed parent clk\n");
> > +                       return;
> > +               }
> 
> Why?

This time, max/min = 97500000/12187500 = 8.
But, we don't know the future.
It will be non-assumed result if it was below or similar
max = 91406250
min = 12187500 (max = min * 15/2)


> > +               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?

I don't think so.
it calculates best of parent clock (= from CPG) and divider (= from MMC)
Why CPG driver need to care about MMC's divider ??

> > +               dev_dbg(&host->pd->dev, "clk %d/%d (%d, %x)\n",
> 
> "%u" (all four)
(snip)
> > +               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".
(snip)
> > +               host->clk = clk_get_rate(host->hclk);
> 
> clk_get_rate() returns "unsigned long", while "host->clk" is "unsigned int".
(snip)
> > +               dev_dbg(&host->pd->dev, "parent clk %d/%d, %d/%d\n",
> 
> "%u" (all four)

Thanks !
I will fixup these in v2

Best regards
---
Kuninori Morimoto

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

[Geert Uytterhoeven]

Hi Morimoto-san,

On Wed, Apr 22, 2015 at 3:04 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>> > +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.
>
> As Laurent mentioned in other feedback, parent clock (= CFG::MMCxCKCR)
> is div6, and it can set 1/1 - 1/64 (= 780000000 - 12187500)
> But, MMC can use 1/8 - 1/64 (= 97500000 - 12187500), we don't know this limit
> came from. we thought that having limit on DIV6 is not good idea.

So the upper limit is a limitation of the MMCIF hardware, while the lower
limit is a limitation of the CPG.

Then the upper limit should be either in the driver (as you did, using
different compatible values), or it can be described in DT.
(cfr. "git grep max.*freq -- arch/arm/boot/dts/").
Personally, I'm leaning towards the latter.

IMHO the lower limit doesn't belong here.

>> > +               /* FIXME */
>> > +               if (pclk->max != pclk->min * (pclk->max / pclk->min)) {
>> > +                       dev_err(&host->pd->dev, "not assumed parent clk\n");
>> > +                       return;
>> > +               }
>>
>> Why?
>
> This time, max/min = 97500000/12187500 = 8.
> But, we don't know the future.
> It will be non-assumed result if it was below or similar
> max = 91406250
> min = 12187500 (max = min * 15/2)

You can still calculate a divisor if max is not an integer multiple of min,
can't you?

And currently the check above cannot trigger, as you have hardcoded values
in the driver source that don't trigger it.

>> > +               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?
>
> I don't think so.
> it calculates best of parent clock (= from CPG) and divider (= from MMC)
> Why CPG driver need to care about MMC's divider ??

The MMCIF driver shouldn't need to be aware of the possible values supported
by the CPG driver.

Can't you loop over all MMC dividers, and
  1. calculate the needed parent clock rate for that MMC divider,
  2. use clk_set_rate_range()[*] to find a (close) parent clock rate,
  3. calculate the effective clock rate based on MMC divider and parent
     clock rate.
and use the best effective clock rate found?

[*] It's a pity the clk API doesn't seem to have a function to query or check
    clock rates without actually setting them. Or am I missing something?

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

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

[Ulf Hansson]

On 22 April 2015 at 09:49, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Morimoto-san,
>
> On Wed, Apr 22, 2015 at 3:04 AM, Kuninori Morimoto
> <kuninori.morimoto.gx@renesas.com> wrote:
>>> > +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.
>>
>> As Laurent mentioned in other feedback, parent clock (= CFG::MMCxCKCR)
>> is div6, and it can set 1/1 - 1/64 (= 780000000 - 12187500)
>> But, MMC can use 1/8 - 1/64 (= 97500000 - 12187500), we don't know this limit
>> came from. we thought that having limit on DIV6 is not good idea.
>
> So the upper limit is a limitation of the MMCIF hardware, while the lower
> limit is a limitation of the CPG.
>
> Then the upper limit should be either in the driver (as you did, using
> different compatible values), or it can be described in DT.
> (cfr. "git grep max.*freq -- arch/arm/boot/dts/").
> Personally, I'm leaning towards the latter.
>
> IMHO the lower limit doesn't belong here.
>
>>> > +               /* FIXME */
>>> > +               if (pclk->max != pclk->min * (pclk->max / pclk->min)) {
>>> > +                       dev_err(&host->pd->dev, "not assumed parent clk\n");
>>> > +                       return;
>>> > +               }
>>>
>>> Why?
>>
>> This time, max/min = 97500000/12187500 = 8.
>> But, we don't know the future.
>> It will be non-assumed result if it was below or similar
>> max = 91406250
>> min = 12187500 (max = min * 15/2)
>
> You can still calculate a divisor if max is not an integer multiple of min,
> can't you?
>
> And currently the check above cannot trigger, as you have hardcoded values
> in the driver source that don't trigger it.
>
>>> > +               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?
>>
>> I don't think so.
>> it calculates best of parent clock (= from CPG) and divider (= from MMC)
>> Why CPG driver need to care about MMC's divider ??
>
> The MMCIF driver shouldn't need to be aware of the possible values supported
> by the CPG driver.
>
> Can't you loop over all MMC dividers, and
>   1. calculate the needed parent clock rate for that MMC divider,
>   2. use clk_set_rate_range()[*] to find a (close) parent clock rate,
>   3. calculate the effective clock rate based on MMC divider and parent
>      clock rate.
> and use the best effective clock rate found?
>
> [*] It's a pity the clk API doesn't seem to have a function to query or check
>     clock rates without actually setting them. Or am I missing something?

What about clk_round_rate(), can't that be used?

Kind regards
Uffe

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

[Geert Uytterhoeven]

On Wed, Apr 22, 2015 at 10:18 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> The MMCIF driver shouldn't need to be aware of the possible values supported
>> by the CPG driver.
>>
>> Can't you loop over all MMC dividers, and
>>   1. calculate the needed parent clock rate for that MMC divider,
>>   2. use clk_set_rate_range()[*] to find a (close) parent clock rate,
>>   3. calculate the effective clock rate based on MMC divider and parent
>>      clock rate.
>> and use the best effective clock rate found?
>>
>> [*] It's a pity the clk API doesn't seem to have a function to query or check
>>     clock rates without actually setting them. Or am I missing something?
>
> What about clk_round_rate(), can't that be used?

Thanks, I knew there had to be something, but couldn't find it at first sight.

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

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

[Kuninori Morimoto]

Hi Geert

Thank you for your feedback

> >> Can't you loop over all MMC dividers, and
> >>   1. calculate the needed parent clock rate for that MMC divider,
> >>   2. use clk_set_rate_range()[*] to find a (close) parent clock rate,
> >>   3. calculate the effective clock rate based on MMC divider and parent
> >>      clock rate.
> >> and use the best effective clock rate found?
(snip)
> So the upper limit is a limitation of the MMCIF hardware, while the lower
> limit is a limitation of the CPG.
> 
> Then the upper limit should be either in the driver (as you did, using
> different compatible values), or it can be described in DT.
> (cfr. "git grep max.*freq -- arch/arm/boot/dts/").
> Personally, I'm leaning towards the latter.
> 
> IMHO the lower limit doesn't belong here.

I think I understood your opinion.
I will try this

Best regards
---
Kuninori Morimoto

[PATCH 0/7 v4] mmc: sh_mmcif: add parent clk support

[Kuninori Morimoto]

Hi Ulf, Simon

These patches are v4 of sh_mmcif.
Current sh_mmcif driver was used on Renesas SH-Mobile series,
and it is also used on Renesas R-Car series.
But, R-Car series can setup parent clock.
We couldn't setup it when SH-Mobile.

These patches are tested by Kobayashi-san on Lager board.

Simon

6), 7) are ARM side patches, but these doesn't depend on
1) - 5) patches.

Kuninori Morimoto (7):
      1) mmc: sh_mmcif: move mmcif_of_match to upside
      2) mmc: sh_mmcif: cleanup to use dev instead of &pdev->dev
      3) mmc: sh_mmcif: remove unnecessary int clk from struct sh_mmcif_host
      4) mmc: sh_mmcif: separate sh_mmcif_clk_update() into setup and prepare
      5) mmc: sh_mmcif: calculate best clock with parent clock
      6) ARM: shmobile: r8a7790: add MMCIF parent clock range
      7) ARM: shmobile: r8a7791: add MMCIF parent clock range

 Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |   3 ++
 arch/arm/boot/dts/r8a7790.dtsi                          |   2 ++
 arch/arm/boot/dts/r8a7791.dtsi                          |   1 +
 drivers/mmc/host/sh_mmcif.c                             | 173 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------
 4 files changed, 135 insertions(+), 44 deletions(-)

Best regards
---
Kuninori Morimoto

[PATCH 1/7 v4] mmc: sh_mmcif: move mmcif_of_match to upside

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v3 -> v4

 - no change

 drivers/mmc/host/sh_mmcif.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 2b6ef6b..4e81278 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -257,6 +257,12 @@ struct sh_mmcif_host {
 	bool			dma_active;
 };
 
+static const struct of_device_id mmcif_of_match[] = {
+	{ .compatible = "renesas,sh-mmcif" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mmcif_of_match);
+
 static inline void sh_mmcif_bitset(struct sh_mmcif_host *host,
 					unsigned int reg, u32 val)
 {
@@ -1543,12 +1549,6 @@ static int sh_mmcif_resume(struct device *dev)
 }
 #endif
 
-static const struct of_device_id mmcif_of_match[] = {
-	{ .compatible = "renesas,sh-mmcif" },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, mmcif_of_match);
-
 static const struct dev_pm_ops sh_mmcif_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(sh_mmcif_suspend, sh_mmcif_resume)
 };
-- 
1.9.1

[PATCH 2/7 v4] mmc: sh_mmcif: cleanup to use dev instead of &pdev->dev

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v3 -> v4

 - no change

 drivers/mmc/host/sh_mmcif.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 4e81278..5282c5b 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1386,7 +1386,8 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	int ret = 0, irq[2];
 	struct mmc_host *mmc;
 	struct sh_mmcif_host *host;
-	struct sh_mmcif_plat_data *pd = pdev->dev.platform_data;
+	struct device *dev = &pdev->dev;
+	struct sh_mmcif_plat_data *pd = dev->platform_data;
 	struct resource *res;
 	void __iomem *reg;
 	const char *name;
@@ -1394,16 +1395,16 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	irq[0] = platform_get_irq(pdev, 0);
 	irq[1] = platform_get_irq(pdev, 1);
 	if (irq[0] < 0) {
-		dev_err(&pdev->dev, "Get irq error\n");
+		dev_err(dev, "Get irq error\n");
 		return -ENXIO;
 	}
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	reg = devm_ioremap_resource(&pdev->dev, res);
+	reg = devm_ioremap_resource(dev, res);
 	if (IS_ERR(reg))
 		return PTR_ERR(reg);
 
-	mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), &pdev->dev);
+	mmc = mmc_alloc_host(sizeof(struct sh_mmcif_host), dev);
 	if (!mmc)
 		return -ENOMEM;
 
@@ -1436,20 +1437,20 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, host);
 
-	pm_runtime_enable(&pdev->dev);
+	pm_runtime_enable(dev);
 	host->power = false;
 
-	host->hclk = devm_clk_get(&pdev->dev, NULL);
+	host->hclk = devm_clk_get(dev, NULL);
 	if (IS_ERR(host->hclk)) {
 		ret = PTR_ERR(host->hclk);
-		dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
+		dev_err(dev, "cannot get clock: %d\n", ret);
 		goto err_pm;
 	}
 	ret = sh_mmcif_clk_update(host);
 	if (ret < 0)
 		goto err_pm;
 
-	ret = pm_runtime_resume(&pdev->dev);
+	ret = pm_runtime_resume(dev);
 	if (ret < 0)
 		goto err_clk;
 
@@ -1458,19 +1459,19 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	sh_mmcif_sync_reset(host);
 	sh_mmcif_writel(host->addr, MMCIF_CE_INT_MASK, MASK_ALL);
 
-	name = irq[1] < 0 ? dev_name(&pdev->dev) : "sh_mmc:error";
-	ret = devm_request_threaded_irq(&pdev->dev, irq[0], sh_mmcif_intr,
+	name = irq[1] < 0 ? dev_name(dev) : "sh_mmc:error";
+	ret = devm_request_threaded_irq(dev, irq[0], sh_mmcif_intr,
 					sh_mmcif_irqt, 0, name, host);
 	if (ret) {
-		dev_err(&pdev->dev, "request_irq error (%s)\n", name);
+		dev_err(dev, "request_irq error (%s)\n", name);
 		goto err_clk;
 	}
 	if (irq[1] >= 0) {
-		ret = devm_request_threaded_irq(&pdev->dev, irq[1],
+		ret = devm_request_threaded_irq(dev, irq[1],
 						sh_mmcif_intr, sh_mmcif_irqt,
 						0, "sh_mmc:int", host);
 		if (ret) {
-			dev_err(&pdev->dev, "request_irq error (sh_mmc:int)\n");
+			dev_err(dev, "request_irq error (sh_mmc:int)\n");
 			goto err_clk;
 		}
 	}
@@ -1487,9 +1488,9 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_clk;
 
-	dev_pm_qos_expose_latency_limit(&pdev->dev, 100);
+	dev_pm_qos_expose_latency_limit(dev, 100);
 
-	dev_info(&pdev->dev, "Chip version 0x%04x, clock rate %luMHz\n",
+	dev_info(dev, "Chip version 0x%04x, clock rate %luMHz\n",
 		 sh_mmcif_readl(host->addr, MMCIF_CE_VERSION) & 0xffff,
 		 clk_get_rate(host->hclk) / 1000000UL);
 
@@ -1499,7 +1500,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 err_clk:
 	clk_disable_unprepare(host->hclk);
 err_pm:
-	pm_runtime_disable(&pdev->dev);
+	pm_runtime_disable(dev);
 err_host:
 	mmc_free_host(mmc);
 	return ret;
-- 
1.9.1

[PATCH 3/7 v4] mmc: sh_mmcif: remove unnecessary int clk from struct sh_mmcif_host

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

struct sh_mmcif_host has 1) int clk, 2) struct clock *hclk,
and host->clk = clk_get_rate(host->hclk).
This int clk is not necessary. Let's remove it.
And, current hclk is confusable naming. Let's rename it to clk.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v3 -> v4

 - new patch, this is cleanup patch

 drivers/mmc/host/sh_mmcif.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 5282c5b..ff4f8c0 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -228,8 +228,7 @@ struct sh_mmcif_host {
 	struct mmc_host *mmc;
 	struct mmc_request *mrq;
 	struct platform_device *pd;
-	struct clk *hclk;
-	unsigned int clk;
+	struct clk *clk;
 	int bus_width;
 	unsigned char timing;
 	bool sd_error;
@@ -484,17 +483,18 @@ static void sh_mmcif_clock_control(struct sh_mmcif_host *host, unsigned int clk)
 {
 	struct sh_mmcif_plat_data *p = host->pd->dev.platform_data;
 	bool sup_pclk = p ? p->sup_pclk : false;
+	unsigned int current_clk = clk_get_rate(host->clk);
 
 	sh_mmcif_bitclr(host, MMCIF_CE_CLK_CTRL, CLK_ENABLE);
 	sh_mmcif_bitclr(host, MMCIF_CE_CLK_CTRL, CLK_CLEAR);
 
 	if (!clk)
 		return;
-	if (sup_pclk && clk == host->clk)
+	if (sup_pclk && clk == current_clk)
 		sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_SUP_PCLK);
 	else
 		sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_CLEAR &
-				((fls(DIV_ROUND_UP(host->clk,
+				((fls(DIV_ROUND_UP(current_clk,
 						   clk) - 1) - 1) << 16));
 
 	sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_ENABLE);
@@ -980,12 +980,13 @@ static void sh_mmcif_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 static int sh_mmcif_clk_update(struct sh_mmcif_host *host)
 {
-	int ret = clk_prepare_enable(host->hclk);
+	int ret = clk_prepare_enable(host->clk);
 
 	if (!ret) {
-		host->clk = clk_get_rate(host->hclk);
-		host->mmc->f_max = host->clk / 2;
-		host->mmc->f_min = host->clk / 512;
+		unsigned int clk = clk_get_rate(host->clk);
+
+		host->mmc->f_max = clk / 2;
+		host->mmc->f_min = clk / 512;
 	}
 
 	return ret;
@@ -1034,7 +1035,7 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		}
 		if (host->power) {
 			pm_runtime_put_sync(&host->pd->dev);
-			clk_disable_unprepare(host->hclk);
+			clk_disable_unprepare(host->clk);
 			host->power = false;
 			if (ios->power_mode == MMC_POWER_OFF)
 				sh_mmcif_set_power(host, ios);
@@ -1440,9 +1441,9 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	pm_runtime_enable(dev);
 	host->power = false;
 
-	host->hclk = devm_clk_get(dev, NULL);
-	if (IS_ERR(host->hclk)) {
-		ret = PTR_ERR(host->hclk);
+	host->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(host->clk)) {
+		ret = PTR_ERR(host->clk);
 		dev_err(dev, "cannot get clock: %d\n", ret);
 		goto err_pm;
 	}
@@ -1492,13 +1493,13 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 
 	dev_info(dev, "Chip version 0x%04x, clock rate %luMHz\n",
 		 sh_mmcif_readl(host->addr, MMCIF_CE_VERSION) & 0xffff,
-		 clk_get_rate(host->hclk) / 1000000UL);
+		 clk_get_rate(host->clk) / 1000000UL);
 
-	clk_disable_unprepare(host->hclk);
+	clk_disable_unprepare(host->clk);
 	return ret;
 
 err_clk:
-	clk_disable_unprepare(host->hclk);
+	clk_disable_unprepare(host->clk);
 err_pm:
 	pm_runtime_disable(dev);
 err_host:
@@ -1511,7 +1512,7 @@ static int sh_mmcif_remove(struct platform_device *pdev)
 	struct sh_mmcif_host *host = platform_get_drvdata(pdev);
 
 	host->dying = true;
-	clk_prepare_enable(host->hclk);
+	clk_prepare_enable(host->clk);
 	pm_runtime_get_sync(&pdev->dev);
 
 	dev_pm_qos_hide_latency_limit(&pdev->dev);
@@ -1526,7 +1527,7 @@ static int sh_mmcif_remove(struct platform_device *pdev)
 	 */
 	cancel_delayed_work_sync(&host->timeout_work);
 
-	clk_disable_unprepare(host->hclk);
+	clk_disable_unprepare(host->clk);
 	mmc_free_host(host->mmc);
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-- 
1.9.1

Re: [PATCH 3/7 v4] mmc: sh_mmcif: remove unnecessary int clk from struct sh_mmcif_host

[Geert Uytterhoeven]

On Thu, Apr 23, 2015 at 10:15 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> struct sh_mmcif_host has 1) int clk, 2) struct clock *hclk,
> and host->clk = clk_get_rate(host->hclk).
> This int clk is not necessary. Let's remove it.
> And, current hclk is confusable naming. Let's rename it to clk.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

[PATCH 4/7 v4] mmc: sh_mmcif: separate sh_mmcif_clk_update() into setup and prepare

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current sh_mmcif_clk_update() is called from probe() and set_ios(),
but, the purpose of later one is just clk_prepare_enable().
No need to setup mmc->f_max/f_min in many times.
This patch separe sh_mmcif_clk_update() into clk_prepare_enable()
and mmc->f_max/f_min setup.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v3 -> v4

 - new patch, this is tidyup patch

 drivers/mmc/host/sh_mmcif.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index ff4f8c0..d2f1158 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -978,18 +978,12 @@ static void sh_mmcif_request(struct mmc_host *mmc, struct mmc_request *mrq)
 	sh_mmcif_start_cmd(host, mrq);
 }
 
-static int sh_mmcif_clk_update(struct sh_mmcif_host *host)
+static void sh_mmcif_clk_setup(struct sh_mmcif_host *host)
 {
-	int ret = clk_prepare_enable(host->clk);
+	unsigned int clk = clk_get_rate(host->clk);
 
-	if (!ret) {
-		unsigned int clk = clk_get_rate(host->clk);
-
-		host->mmc->f_max = clk / 2;
-		host->mmc->f_min = clk / 512;
-	}
-
-	return ret;
+	host->mmc->f_max = clk / 2;
+	host->mmc->f_min = clk / 512;
 }
 
 static void sh_mmcif_set_power(struct sh_mmcif_host *host, struct mmc_ios *ios)
@@ -1046,7 +1040,8 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 	if (ios->clock) {
 		if (!host->power) {
-			sh_mmcif_clk_update(host);
+			clk_prepare_enable(host->clk);
+
 			pm_runtime_get_sync(&host->pd->dev);
 			host->power = true;
 			sh_mmcif_sync_reset(host);
@@ -1447,10 +1442,13 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 		dev_err(dev, "cannot get clock: %d\n", ret);
 		goto err_pm;
 	}
-	ret = sh_mmcif_clk_update(host);
+
+	ret = clk_prepare_enable(host->clk);
 	if (ret < 0)
 		goto err_pm;
 
+	sh_mmcif_clk_setup(host);
+
 	ret = pm_runtime_resume(dev);
 	if (ret < 0)
 		goto err_clk;
-- 
1.9.1

Re: [PATCH 4/7 v4] mmc: sh_mmcif: separate sh_mmcif_clk_update() into setup and prepare

[Geert Uytterhoeven]

On Thu, Apr 23, 2015 at 10:16 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Current sh_mmcif_clk_update() is called from probe() and set_ios(),
> but, the purpose of later one is just clk_prepare_enable().
> No need to setup mmc->f_max/f_min in many times.
> This patch separe sh_mmcif_clk_update() into clk_prepare_enable()
> and mmc->f_max/f_min setup.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

[PATCH 5/7 v4] mmc: sh_mmcif: calculate best clock with parent clock

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

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.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v3 -> v4

 - add new clk-range on DT
 - use clk_round_rate() for parent_clk
 - struct sh_mmcif_ver instead of sh_mmcif_parent_clk

 .../devicetree/bindings/mmc/renesas,mmcif.txt      |  3 +
 drivers/mmc/host/sh_mmcif.c                        | 89 +++++++++++++++++++++-
 2 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
index 299081f..eb50f4e 100644
--- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
+++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
@@ -14,6 +14,8 @@ Required properties:
 
 - clocks: reference to the functional clock
 
+- clk-range: parent clock range
+
 - dmas: reference to the DMA channels, one per channel name listed in the
   dma-names property.
 - dma-names: must contain "tx" for the transmit DMA channel and "rx" for the
@@ -29,4 +31,5 @@ Example: R8A7790 (R-Car H2) MMCIF0
 		clocks = <&mstp3_clks R8A7790_CLK_MMCIF0>;
 		dmas = <&dmac0 0xd1>, <&dmac0 0xd2>;
 		dma-names = "tx", "rx";
+		clk-range = <12187500 97500000>;
 	};
diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index d2f1158..437aa3c 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,13 @@ enum mmcif_wait_for {
 	MMCIF_WAIT_FOR_STOP,
 };
 
+/*
+ * difference for each SoC
+ */
+struct sh_mmcif_ver {
+	u32 clkdiv_map;	 /* see CE_CLK_CTRL::CLKDIV */
+};
+
 struct sh_mmcif_host {
 	struct mmc_host *mmc;
 	struct mmc_request *mrq;
@@ -248,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_ver *ver;
 
 	/* DMA support */
 	struct dma_chan		*chan_rx;
@@ -256,8 +265,14 @@ struct sh_mmcif_host {
 	bool			dma_active;
 };
 
+static const struct sh_mmcif_ver sh_mmcif_gen2 = {
+	.clkdiv_map = 0x3ff,
+};
+
 static const struct of_device_id mmcif_of_match[] = {
 	{ .compatible = "renesas,sh-mmcif" },
+	{ .compatible = "renesas,mmcif-r8a7790", .data = &sh_mmcif_gen2},
+	{ .compatible = "renesas,mmcif-r8a7791", .data = &sh_mmcif_gen2},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, mmcif_of_match);
@@ -490,12 +505,50 @@ static void sh_mmcif_clock_control(struct sh_mmcif_host *host, unsigned int clk)
 
 	if (!clk)
 		return;
-	if (sup_pclk && clk == current_clk)
+
+	if (host->ver) {
+		const struct sh_mmcif_ver *ver = host->ver;
+		unsigned int freq, best_freq, myclk, clkdiv, div, diff_min, diff;
+		int i;
+
+		clkdiv = 0;
+		diff_min = ~0;
+		best_freq = 0;
+		for (i = fls(ver->clkdiv_map); i >= 0; i--) {
+			if (!((1 << i) & ver->clkdiv_map))
+				continue;
+
+			/*
+			 * clk = parent_freq / div
+			 * -> parent_freq = clk x div
+			 */
+
+			div = 1 << (i + 1);
+			freq = clk_round_rate(host->clk, clk * div);
+			myclk = freq / div;
+			diff = (myclk > clk) ? myclk - clk : clk - myclk;
+
+			if (diff <= diff_min) {
+				best_freq = freq;
+				clkdiv = i;
+				diff_min = diff;
+			}
+		}
+
+		dev_dbg(&host->pd->dev, "clk %u/%u (%u, 0x%x)\n",
+			(best_freq / (1 << (clkdiv + 1))), clk,
+			best_freq, clkdiv);
+
+		clk_set_rate(host->clk, best_freq);
+		sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL,
+				CLK_CLEAR & (clkdiv << 16));
+	} else if (sup_pclk && clk == current_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(current_clk,
 						   clk) - 1) - 1) << 16));
+	}
 
 	sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_ENABLE);
 }
@@ -980,10 +1033,42 @@ static void sh_mmcif_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 static void sh_mmcif_clk_setup(struct sh_mmcif_host *host)
 {
+	struct device *dev = &host->pd->dev;
+	const struct of_device_id *of_id = of_match_device(mmcif_of_match, dev);
 	unsigned int clk = clk_get_rate(host->clk);
 
+	if (of_id) {
+		struct device_node *np = dev->of_node;
+		const struct sh_mmcif_ver *ver = of_id->data;
+		u32 range[2]; /* min/max */
+
+		if (!ver)
+			goto sh_mmcif_clk_setup_default;
+
+		if (0 != of_property_read_u32_array(np, "clk-range",
+						    range, ARRAY_SIZE(range)))
+			goto sh_mmcif_clk_setup_default;
+
+		if (range[0] > range[1])
+			goto sh_mmcif_clk_setup_default;
+
+		host->mmc->f_min = range[0] / (1 << fls(ver->clkdiv_map));
+		host->mmc->f_max = range[1] / (1 << ffs(ver->clkdiv_map));
+
+		dev_dbg(dev, "parent clk <%d - %d> (%d/%d)\n",
+			range[0], range[1],
+			host->mmc->f_max, host->mmc->f_min);
+
+		host->ver = ver;
+		return;
+	}
+
+sh_mmcif_clk_setup_default:
 	host->mmc->f_max = clk / 2;
 	host->mmc->f_min = clk / 512;
+
+	dev_dbg(dev, "clk max/min = %d/%d\n",
+		host->mmc->f_max, host->mmc->f_min);
 }
 
 static void sh_mmcif_set_power(struct sh_mmcif_host *host, struct mmc_ios *ios)
-- 
1.9.1

Re: [PATCH 5/7 v4] mmc: sh_mmcif: calculate best clock with parent clock

[Laurent Pinchart]

Hi Morimoto-san,

Thank you for the patch.

I'm CC'ing the device-tree mailing list as well as Mike Turquette, please see 
below for the discussion.

On Thursday 23 April 2015 08:17:23 Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> 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.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
> ---
> v3 -> v4
> 
>  - add new clk-range on DT
>  - use clk_round_rate() for parent_clk
>  - struct sh_mmcif_ver instead of sh_mmcif_parent_clk
> 
>  .../devicetree/bindings/mmc/renesas,mmcif.txt      |  3 +
>  drivers/mmc/host/sh_mmcif.c                        | 89 ++++++++++++++++++-
>  2 files changed, 90 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt index
> 299081f..eb50f4e 100644
> --- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> +++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> @@ -14,6 +14,8 @@ Required properties:
> 
>  - clocks: reference to the functional clock
> 
> +- clk-range: parent clock range
> +

I think the idea of a DT property to specify the admissible range for clock 
frequencies is good. However, I have a couple of small comments regarding the 
implementation.

First of all, this doesn't seem Renesas-specific to me (feel free to disagree, 
but in that case the property should probably be called renesas,clk-range). 
Wouldn't it make sense to specify the property in 
Documentation/devicetree/bindings/clock/clock-bindings.txt instead ?

Then, I think the documentation should be clarified a bit, as "parent clock 
range" is probably a bit vague for people who haven't followed the development 
of this patch series. How about something like "range of admissible 
frequencies for the input clock" ?

There's already a clock-ranges property with a totally different meaning. 
That's quite confusing, it would thus be good to rename clk-range. How about 
clock-freq-range or clock-frequency-range ?

Finally, if a DT node references several clocks in its clocks property, we 
need a way to specify the range for each of them.

>  - dmas: reference to the DMA channels, one per channel name listed in the
>    dma-names property.
>  - dma-names: must contain "tx" for the transmit DMA channel and "rx" for
> the @@ -29,4 +31,5 @@ Example: R8A7790 (R-Car H2) MMCIF0
>  		clocks = <&mstp3_clks R8A7790_CLK_MMCIF0>;
>  		dmas = <&dmac0 0xd1>, <&dmac0 0xd2>;
>  		dma-names = "tx", "rx";
> +		clk-range = <12187500 97500000>;
>  	};
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index d2f1158..437aa3c 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,13 @@ enum mmcif_wait_for {
>  	MMCIF_WAIT_FOR_STOP,
>  };
> 
> +/*
> + * difference for each SoC
> + */
> +struct sh_mmcif_ver {
> +	u32 clkdiv_map;	 /* see CE_CLK_CTRL::CLKDIV */
> +};
> +
>  struct sh_mmcif_host {
>  	struct mmc_host *mmc;
>  	struct mmc_request *mrq;
> @@ -248,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_ver *ver;
> 
>  	/* DMA support */
>  	struct dma_chan		*chan_rx;
> @@ -256,8 +265,14 @@ struct sh_mmcif_host {
>  	bool			dma_active;
>  };
> 
> +static const struct sh_mmcif_ver sh_mmcif_gen2 = {
> +	.clkdiv_map = 0x3ff,
> +};
> +
>  static const struct of_device_id mmcif_of_match[] = {
>  	{ .compatible = "renesas,sh-mmcif" },
> +	{ .compatible = "renesas,mmcif-r8a7790", .data = &sh_mmcif_gen2},
> +	{ .compatible = "renesas,mmcif-r8a7791", .data = &sh_mmcif_gen2},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, mmcif_of_match);
> @@ -490,12 +505,50 @@ static void sh_mmcif_clock_control(struct
> sh_mmcif_host *host, unsigned int clk)
> 
>  	if (!clk)
>  		return;
> -	if (sup_pclk && clk == current_clk)
> +
> +	if (host->ver) {
> +		const struct sh_mmcif_ver *ver = host->ver;
> +		unsigned int freq, best_freq, myclk, clkdiv, div, diff_min, diff;
> +		int i;
> +
> +		clkdiv = 0;
> +		diff_min = ~0;
> +		best_freq = 0;
> +		for (i = fls(ver->clkdiv_map); i >= 0; i--) {
> +			if (!((1 << i) & ver->clkdiv_map))
> +				continue;
> +
> +			/*
> +			 * clk = parent_freq / div
> +			 * -> parent_freq = clk x div
> +			 */
> +
> +			div = 1 << (i + 1);
> +			freq = clk_round_rate(host->clk, clk * div);
> +			myclk = freq / div;
> +			diff = (myclk > clk) ? myclk - clk : clk - myclk;
> +
> +			if (diff <= diff_min) {
> +				best_freq = freq;
> +				clkdiv = i;
> +				diff_min = diff;
> +			}
> +		}
> +
> +		dev_dbg(&host->pd->dev, "clk %u/%u (%u, 0x%x)\n",
> +			(best_freq / (1 << (clkdiv + 1))), clk,
> +			best_freq, clkdiv);
> +
> +		clk_set_rate(host->clk, best_freq);
> +		sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL,
> +				CLK_CLEAR & (clkdiv << 16));
> +	} else if (sup_pclk && clk == current_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(current_clk,
>  						   clk) - 1) - 1) << 16));
> +	}
> 
>  	sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_ENABLE);
>  }
> @@ -980,10 +1033,42 @@ static void sh_mmcif_request(struct mmc_host *mmc,
> struct mmc_request *mrq)
> 
>  static void sh_mmcif_clk_setup(struct sh_mmcif_host *host)
>  {
> +	struct device *dev = &host->pd->dev;
> +	const struct of_device_id *of_id = of_match_device(mmcif_of_match, dev);
>  	unsigned int clk = clk_get_rate(host->clk);
> 
> +	if (of_id) {
> +		struct device_node *np = dev->of_node;
> +		const struct sh_mmcif_ver *ver = of_id->data;
> +		u32 range[2]; /* min/max */
> +
> +		if (!ver)
> +			goto sh_mmcif_clk_setup_default;
> +
> +		if (0 != of_property_read_u32_array(np, "clk-range",
> +						    range, ARRAY_SIZE(range)))
> +			goto sh_mmcif_clk_setup_default;
> +
> +		if (range[0] > range[1])
> +			goto sh_mmcif_clk_setup_default;
> +
> +		host->mmc->f_min = range[0] / (1 << fls(ver->clkdiv_map));
> +		host->mmc->f_max = range[1] / (1 << ffs(ver->clkdiv_map));
> +
> +		dev_dbg(dev, "parent clk <%d - %d> (%d/%d)\n",
> +			range[0], range[1],
> +			host->mmc->f_max, host->mmc->f_min);
> +
> +		host->ver = ver;
> +		return;
> +	}
> +
> +sh_mmcif_clk_setup_default:
>  	host->mmc->f_max = clk / 2;
>  	host->mmc->f_min = clk / 512;
> +
> +	dev_dbg(dev, "clk max/min = %d/%d\n",
> +		host->mmc->f_max, host->mmc->f_min);
>  }
> 
>  static void sh_mmcif_set_power(struct sh_mmcif_host *host, struct mmc_ios
> *ios)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 5/7 v4] mmc: sh_mmcif: calculate best clock with parent clock

[Kuninori Morimoto]

Hi Laurent

Thank you for your feedback

> > diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> > b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt index
> > 299081f..eb50f4e 100644
> > --- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> > +++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
> > @@ -14,6 +14,8 @@ Required properties:
> > 
> >  - clocks: reference to the functional clock
> > 
> > +- clk-range: parent clock range
> > +
> 
> I think the idea of a DT property to specify the admissible range for clock 
> frequencies is good. However, I have a couple of small comments regarding the 
> implementation.
> 
> First of all, this doesn't seem Renesas-specific to me (feel free to disagree, 
> but in that case the property should probably be called renesas,clk-range). 
> Wouldn't it make sense to specify the property in 
> Documentation/devicetree/bindings/clock/clock-bindings.txt instead ?
> 
> Then, I think the documentation should be clarified a bit, as "parent clock 
> range" is probably a bit vague for people who haven't followed the development 
> of this patch series. How about something like "range of admissible 
> frequencies for the input clock" ?
> 
> There's already a clock-ranges property with a totally different meaning. 
> That's quite confusing, it would thus be good to rename clk-range. How about 
> clock-freq-range or clock-frequency-range ?
> 
> Finally, if a DT node references several clocks in its clocks property, we 
> need a way to specify the range for each of them.

I didn't tell you about this, but actually I created v5 patch
(it is under test stage now, I will send it to ML soon)
which doesn't add new DT property.

This v4 added new property for clock frequencies, and it was Geert's idea.
This is just my opinion, and I don't know this is good match for DT,
but, this clock frequencies range depends on each SoC, and we are already
using SoC based "compatible" on DT. This means we can (should ?) get each SoC
specific feature (which is not related to each platform/board) from "compatible".
Thus, v5 patch gets clock frequency range from SoC based "compatible".

Thank you for your help, please check my next v5 patch.

Best regards
---
Kuninori Morimoto

[PATCH 6/7 v4] ARM: shmobile: r8a7790: add MMCIF parent clock range

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v3 -> v4

 - new patch

 arch/arm/boot/dts/r8a7790.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index f3b8430..ac61636 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -467,6 +467,7 @@
 		dma-names = "tx", "rx";
 		reg-io-width = <4>;
 		status = "disabled";
+		clk-range = <12187500 97500000>;
 	};
 
 	mmcif1: mmc@ee220000 {
@@ -478,6 +479,7 @@
 		dma-names = "tx", "rx";
 		reg-io-width = <4>;
 		status = "disabled";
+		clk-range = <12187500 97500000>;
 	};
 
 	pfc: pfc@e6060000 {
-- 
1.9.1

Re: [PATCH 6/7 v4] ARM: shmobile: r8a7790: add MMCIF parent clock range

[Simon Horman]

Hi Morimoto-san,

On Thu, Apr 23, 2015 at 08:18:04AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>

Could you indicate if:
* you think this and the next patch of the series is ready to be merged or;
* I should wait for patch 5 to be merged/reworked/...

> ---
> v3 -> v4
> 
>  - new patch
> 
>  arch/arm/boot/dts/r8a7790.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index f3b8430..ac61636 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -467,6 +467,7 @@
>  		dma-names = "tx", "rx";
>  		reg-io-width = <4>;
>  		status = "disabled";
> +		clk-range = <12187500 97500000>;
>  	};
>  
>  	mmcif1: mmc@ee220000 {
> @@ -478,6 +479,7 @@
>  		dma-names = "tx", "rx";
>  		reg-io-width = <4>;
>  		status = "disabled";
> +		clk-range = <12187500 97500000>;
>  	};
>  
>  	pfc: pfc@e6060000 {
> -- 
> 1.9.1
> 

Re: [PATCH 6/7 v4] ARM: shmobile: r8a7790: add MMCIF parent clock range

[Kuninori Morimoto]

Hi Simon

> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
> 
> Could you indicate if:
> * you think this and the next patch of the series is ready to be merged or;
> * I should wait for patch 5 to be merged/reworked/...

Thank you for your feedback.
I think I need to send v5 patch of 5/6/7 patches.
Please don't care about v4 patches.

Best regards
---
Kuninori Morimoto

Re: [PATCH 6/7 v4] ARM: shmobile: r8a7790: add MMCIF parent clock range

[Simon Horman]

On Mon, May 11, 2015 at 02:53:43AM +0000, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > > 
> > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > > Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
> > 
> > Could you indicate if:
> > * you think this and the next patch of the series is ready to be merged or;
> > * I should wait for patch 5 to be merged/reworked/...
> 
> Thank you for your feedback.
> I think I need to send v5 patch of 5/6/7 patches.
> Please don't care about v4 patches.

Thanks, got it.

[PATCH 7/7 v4] ARM: shmobile: r8a7791: add MMCIF parent clock range

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v3 -> v4

 - new patch

 arch/arm/boot/dts/r8a7791.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 3e9f824..47eb36a 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -471,6 +471,7 @@
 		dma-names = "tx", "rx";
 		reg-io-width = <4>;
 		status = "disabled";
+		clk-range = <12187500 97500000>;
 	};
 
 	sdhi0: sd@ee100000 {
-- 
1.9.1

Re: [PATCH 0/7 v4] mmc: sh_mmcif: add parent clk support

[Laurent Pinchart]

Hi Morimoto-san,

Thank you for the patches.

On Thursday 23 April 2015 08:11:36 Kuninori Morimoto wrote:
> Hi Ulf, Simon
> 
> These patches are v4 of sh_mmcif.
> Current sh_mmcif driver was used on Renesas SH-Mobile series,
> and it is also used on Renesas R-Car series.
> But, R-Car series can setup parent clock.
> We couldn't setup it when SH-Mobile.
> 
> These patches are tested by Kobayashi-san on Lager board.
> 
> Simon
> 
> 6), 7) are ARM side patches, but these doesn't depend on
> 1) - 5) patches.
>
> Kuninori Morimoto (7):
>   1) mmc: sh_mmcif: move mmcif_of_match to upside
>   2) mmc: sh_mmcif: cleanup to use dev instead of &pdev->dev
>   3) mmc: sh_mmcif: remove unnecessary int clk from struct sh_mmcif_host
>   4) mmc: sh_mmcif: separate sh_mmcif_clk_update() into setup and prepare

For patches 1-4,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I'll comment separately on patch 5.

>   5) mmc: sh_mmcif: calculate best clock with parent clock
>   6) ARM: shmobile: r8a7790: add MMCIF parent clock range
>   7) ARM: shmobile: r8a7791: add MMCIF parent clock range
> 
>  Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |   3 ++
>  arch/arm/boot/dts/r8a7790.dtsi                          |   2 ++
>  arch/arm/boot/dts/r8a7791.dtsi                          |   1 +
>  drivers/mmc/host/sh_mmcif.c                             | 173 +++++++++----
>  4 files changed, 135 insertions(+), 44 deletions(-)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/7 v4] mmc: sh_mmcif: add parent clk support

[Ulf Hansson]

On 23 April 2015 at 12:07, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Morimoto-san,
>
> Thank you for the patches.
>
> On Thursday 23 April 2015 08:11:36 Kuninori Morimoto wrote:
>> Hi Ulf, Simon
>>
>> These patches are v4 of sh_mmcif.
>> Current sh_mmcif driver was used on Renesas SH-Mobile series,
>> and it is also used on Renesas R-Car series.
>> But, R-Car series can setup parent clock.
>> We couldn't setup it when SH-Mobile.
>>
>> These patches are tested by Kobayashi-san on Lager board.
>>
>> Simon
>>
>> 6), 7) are ARM side patches, but these doesn't depend on
>> 1) - 5) patches.
>>
>> Kuninori Morimoto (7):
>>   1) mmc: sh_mmcif: move mmcif_of_match to upside
>>   2) mmc: sh_mmcif: cleanup to use dev instead of &pdev->dev
>>   3) mmc: sh_mmcif: remove unnecessary int clk from struct sh_mmcif_host
>>   4) mmc: sh_mmcif: separate sh_mmcif_clk_update() into setup and prepare
>
> For patches 1-4,
>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> I'll comment separately on patch 5.
>
>>   5) mmc: sh_mmcif: calculate best clock with parent clock
>>   6) ARM: shmobile: r8a7790: add MMCIF parent clock range
>>   7) ARM: shmobile: r8a7791: add MMCIF parent clock range
>>
>>  Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |   3 ++
>>  arch/arm/boot/dts/r8a7790.dtsi                          |   2 ++
>>  arch/arm/boot/dts/r8a7791.dtsi                          |   1 +
>>  drivers/mmc/host/sh_mmcif.c                             | 173 +++++++++----
>>  4 files changed, 135 insertions(+), 44 deletions(-)
>
> --

Thanks, applied patches 1->4.

I am delaying patch 5, until it gets acked.

Kind regards
Uffe

[PATCH 0/3 v5] mmc: sh_mmcif: add parent clk support

[Kuninori Morimoto]

Hi Ulf

These patches are v5 of sh_mmcif clock support.
1) and 2) are new patches for cleanup,
and 3) is v5 patch for it.

Kuninori Morimoto (3):
      1) mmc: sh_mmcif: add sh_mmcif_host_to_dev() macro and use it.
      2) mmc: sh_mmcif: use sh_mmcif_xxx prefix for all functions
      5) mmc: sh_mmcif: calculate best clock with parent clock

 drivers/mmc/host/sh_mmcif.c | 232 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------------
 1 file changed, 167 insertions(+), 65 deletions(-)

Best regards
---
Kuninori Morimoto
--
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

[PATCH 1/3 v5] mmc: sh_mmcif: add sh_mmcif_host_to_dev() macro and use it.

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current sh_mmcif driver is directly using &host->pd->dev in all place.
It is not big problem, but it is unreadable, and it can be cause of
future bug. This patch adds new sh_mmcif_host_to_dev() and use it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v4 -> v5

 - new patch for cleanup

 drivers/mmc/host/sh_mmcif.c | 114 +++++++++++++++++++++++++++-----------------
 1 file changed, 71 insertions(+), 43 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 068b9a6..29835ae 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -262,6 +262,8 @@ static const struct of_device_id mmcif_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, mmcif_of_match);
 
+#define sh_mmcif_host_to_dev(host) (&host->pd->dev)
+
 static inline void sh_mmcif_bitset(struct sh_mmcif_host *host,
 					unsigned int reg, u32 val)
 {
@@ -278,11 +280,12 @@ static void mmcif_dma_complete(void *arg)
 {
 	struct sh_mmcif_host *host = arg;
 	struct mmc_request *mrq = host->mrq;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 
-	dev_dbg(&host->pd->dev, "Command completed\n");
+	dev_dbg(dev, "Command completed\n");
 
 	if (WARN(!mrq || !mrq->data, "%s: NULL data in DMA completion!\n",
-		 dev_name(&host->pd->dev)))
+		 dev_name(dev)))
 		return;
 
 	complete(&host->dma_complete);
@@ -294,6 +297,7 @@ static void sh_mmcif_start_dma_rx(struct sh_mmcif_host *host)
 	struct scatterlist *sg = data->sg;
 	struct dma_async_tx_descriptor *desc = NULL;
 	struct dma_chan *chan = host->chan_rx;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	dma_cookie_t cookie = -EINVAL;
 	int ret;
 
@@ -312,7 +316,7 @@ static void sh_mmcif_start_dma_rx(struct sh_mmcif_host *host)
 		sh_mmcif_bitset(host, MMCIF_CE_BUF_ACC, BUF_ACC_DMAREN);
 		dma_async_issue_pending(chan);
 	}
-	dev_dbg(&host->pd->dev, "%s(): mapped %d -> %d, cookie %d\n",
+	dev_dbg(dev, "%s(): mapped %d -> %d, cookie %d\n",
 		__func__, data->sg_len, ret, cookie);
 
 	if (!desc) {
@@ -328,12 +332,12 @@ static void sh_mmcif_start_dma_rx(struct sh_mmcif_host *host)
 			host->chan_tx = NULL;
 			dma_release_channel(chan);
 		}
-		dev_warn(&host->pd->dev,
+		dev_warn(dev,
 			 "DMA failed: %d, falling back to PIO\n", ret);
 		sh_mmcif_bitclr(host, MMCIF_CE_BUF_ACC, BUF_ACC_DMAREN | BUF_ACC_DMAWEN);
 	}
 
-	dev_dbg(&host->pd->dev, "%s(): desc %p, cookie %d, sg[%d]\n", __func__,
+	dev_dbg(dev, "%s(): desc %p, cookie %d, sg[%d]\n", __func__,
 		desc, cookie, data->sg_len);
 }
 
@@ -343,6 +347,7 @@ static void sh_mmcif_start_dma_tx(struct sh_mmcif_host *host)
 	struct scatterlist *sg = data->sg;
 	struct dma_async_tx_descriptor *desc = NULL;
 	struct dma_chan *chan = host->chan_tx;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	dma_cookie_t cookie = -EINVAL;
 	int ret;
 
@@ -361,7 +366,7 @@ static void sh_mmcif_start_dma_tx(struct sh_mmcif_host *host)
 		sh_mmcif_bitset(host, MMCIF_CE_BUF_ACC, BUF_ACC_DMAWEN);
 		dma_async_issue_pending(chan);
 	}
-	dev_dbg(&host->pd->dev, "%s(): mapped %d -> %d, cookie %d\n",
+	dev_dbg(dev, "%s(): mapped %d -> %d, cookie %d\n",
 		__func__, data->sg_len, ret, cookie);
 
 	if (!desc) {
@@ -377,12 +382,12 @@ static void sh_mmcif_start_dma_tx(struct sh_mmcif_host *host)
 			host->chan_rx = NULL;
 			dma_release_channel(chan);
 		}
-		dev_warn(&host->pd->dev,
+		dev_warn(dev,
 			 "DMA failed: %d, falling back to PIO\n", ret);
 		sh_mmcif_bitclr(host, MMCIF_CE_BUF_ACC, BUF_ACC_DMAREN | BUF_ACC_DMAWEN);
 	}
 
-	dev_dbg(&host->pd->dev, "%s(): desc %p, cookie %d\n", __func__,
+	dev_dbg(dev, "%s(): desc %p, cookie %d\n", __func__,
 		desc, cookie);
 }
 
@@ -395,6 +400,7 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
 	struct dma_chan *chan;
 	void *slave_data = NULL;
 	struct resource *res;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	dma_cap_mask_t mask;
 	int ret;
 
@@ -407,10 +413,10 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
 			(void *)pdata->slave_id_rx;
 
 	chan = dma_request_slave_channel_compat(mask, shdma_chan_filter,
-				slave_data, &host->pd->dev,
+				slave_data, dev,
 				direction == DMA_MEM_TO_DEV ? "tx" : "rx");
 
-	dev_dbg(&host->pd->dev, "%s: %s: got channel %p\n", __func__,
+	dev_dbg(dev, "%s: %s: got channel %p\n", __func__,
 		direction == DMA_MEM_TO_DEV ? "TX" : "RX", chan);
 
 	if (!chan)
@@ -440,12 +446,13 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
 static void sh_mmcif_request_dma(struct sh_mmcif_host *host,
 				 struct sh_mmcif_plat_data *pdata)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	host->dma_active = false;
 
 	if (pdata) {
 		if (pdata->slave_id_tx <= 0 || pdata->slave_id_rx <= 0)
 			return;
-	} else if (!host->pd->dev.of_node) {
+	} else if (!dev->of_node) {
 		return;
 	}
 
@@ -481,7 +488,8 @@ static void sh_mmcif_release_dma(struct sh_mmcif_host *host)
 
 static void sh_mmcif_clock_control(struct sh_mmcif_host *host, unsigned int clk)
 {
-	struct sh_mmcif_plat_data *p = host->pd->dev.platform_data;
+	struct device *dev = sh_mmcif_host_to_dev(host);
+	struct sh_mmcif_plat_data *p = dev->platform_data;
 	bool sup_pclk = p ? p->sup_pclk : false;
 	unsigned int current_clk = clk_get_rate(host->clk);
 
@@ -520,6 +528,7 @@ static void sh_mmcif_sync_reset(struct sh_mmcif_host *host)
 
 static int sh_mmcif_error_manage(struct sh_mmcif_host *host)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	u32 state1, state2;
 	int ret, timeout;
 
@@ -527,8 +536,8 @@ static int sh_mmcif_error_manage(struct sh_mmcif_host *host)
 
 	state1 = sh_mmcif_readl(host->addr, MMCIF_CE_HOST_STS1);
 	state2 = sh_mmcif_readl(host->addr, MMCIF_CE_HOST_STS2);
-	dev_dbg(&host->pd->dev, "ERR HOST_STS1 = %08x\n", state1);
-	dev_dbg(&host->pd->dev, "ERR HOST_STS2 = %08x\n", state2);
+	dev_dbg(dev, "ERR HOST_STS1 = %08x\n", state1);
+	dev_dbg(dev, "ERR HOST_STS2 = %08x\n", state2);
 
 	if (state1 & STS1_CMDSEQ) {
 		sh_mmcif_bitset(host, MMCIF_CE_CMD_CTRL, CMD_CTRL_BREAK);
@@ -540,25 +549,25 @@ static int sh_mmcif_error_manage(struct sh_mmcif_host *host)
 			mdelay(1);
 		}
 		if (!timeout) {
-			dev_err(&host->pd->dev,
+			dev_err(dev,
 				"Forced end of command sequence timeout err\n");
 			return -EIO;
 		}
 		sh_mmcif_sync_reset(host);
-		dev_dbg(&host->pd->dev, "Forced end of command sequence\n");
+		dev_dbg(dev, "Forced end of command sequence\n");
 		return -EIO;
 	}
 
 	if (state2 & STS2_CRC_ERR) {
-		dev_err(&host->pd->dev, " CRC error: state %u, wait %u\n",
+		dev_err(dev, " CRC error: state %u, wait %u\n",
 			host->state, host->wait_for);
 		ret = -EIO;
 	} else if (state2 & STS2_TIMEOUT_ERR) {
-		dev_err(&host->pd->dev, " Timeout: state %u, wait %u\n",
+		dev_err(dev, " Timeout: state %u, wait %u\n",
 			host->state, host->wait_for);
 		ret = -ETIMEDOUT;
 	} else {
-		dev_dbg(&host->pd->dev, " End/Index error: state %u, wait %u\n",
+		dev_dbg(dev, " End/Index error: state %u, wait %u\n",
 			host->state, host->wait_for);
 		ret = -EIO;
 	}
@@ -599,13 +608,14 @@ static void sh_mmcif_single_read(struct sh_mmcif_host *host,
 
 static bool sh_mmcif_read_block(struct sh_mmcif_host *host)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	struct mmc_data *data = host->mrq->data;
 	u32 *p = sg_virt(data->sg);
 	int i;
 
 	if (host->sd_error) {
 		data->error = sh_mmcif_error_manage(host);
-		dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, data->error);
+		dev_dbg(dev, "%s(): %d\n", __func__, data->error);
 		return false;
 	}
 
@@ -640,13 +650,14 @@ static void sh_mmcif_multi_read(struct sh_mmcif_host *host,
 
 static bool sh_mmcif_mread_block(struct sh_mmcif_host *host)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	struct mmc_data *data = host->mrq->data;
 	u32 *p = host->pio_ptr;
 	int i;
 
 	if (host->sd_error) {
 		data->error = sh_mmcif_error_manage(host);
-		dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, data->error);
+		dev_dbg(dev, "%s(): %d\n", __func__, data->error);
 		return false;
 	}
 
@@ -677,13 +688,14 @@ static void sh_mmcif_single_write(struct sh_mmcif_host *host,
 
 static bool sh_mmcif_write_block(struct sh_mmcif_host *host)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	struct mmc_data *data = host->mrq->data;
 	u32 *p = sg_virt(data->sg);
 	int i;
 
 	if (host->sd_error) {
 		data->error = sh_mmcif_error_manage(host);
-		dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, data->error);
+		dev_dbg(dev, "%s(): %d\n", __func__, data->error);
 		return false;
 	}
 
@@ -718,13 +730,14 @@ static void sh_mmcif_multi_write(struct sh_mmcif_host *host,
 
 static bool sh_mmcif_mwrite_block(struct sh_mmcif_host *host)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	struct mmc_data *data = host->mrq->data;
 	u32 *p = host->pio_ptr;
 	int i;
 
 	if (host->sd_error) {
 		data->error = sh_mmcif_error_manage(host);
-		dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, data->error);
+		dev_dbg(dev, "%s(): %d\n", __func__, data->error);
 		return false;
 	}
 
@@ -762,6 +775,7 @@ static void sh_mmcif_get_cmd12response(struct sh_mmcif_host *host,
 static u32 sh_mmcif_set_cmd(struct sh_mmcif_host *host,
 			    struct mmc_request *mrq)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	struct mmc_data *data = mrq->data;
 	struct mmc_command *cmd = mrq->cmd;
 	u32 opc = cmd->opcode;
@@ -781,7 +795,7 @@ static u32 sh_mmcif_set_cmd(struct sh_mmcif_host *host,
 		tmp |= CMD_SET_RTYP_17B;
 		break;
 	default:
-		dev_err(&host->pd->dev, "Unsupported response type.\n");
+		dev_err(dev, "Unsupported response type.\n");
 		break;
 	}
 	switch (opc) {
@@ -809,7 +823,7 @@ static u32 sh_mmcif_set_cmd(struct sh_mmcif_host *host,
 			tmp |= CMD_SET_DATW_8;
 			break;
 		default:
-			dev_err(&host->pd->dev, "Unsupported bus width.\n");
+			dev_err(dev, "Unsupported bus width.\n");
 			break;
 		}
 		switch (host->timing) {
@@ -852,6 +866,8 @@ static u32 sh_mmcif_set_cmd(struct sh_mmcif_host *host,
 static int sh_mmcif_data_trans(struct sh_mmcif_host *host,
 			       struct mmc_request *mrq, u32 opc)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
+
 	switch (opc) {
 	case MMC_READ_MULTIPLE_BLOCK:
 		sh_mmcif_multi_read(host, mrq);
@@ -867,7 +883,7 @@ static int sh_mmcif_data_trans(struct sh_mmcif_host *host,
 		sh_mmcif_single_read(host, mrq);
 		return 0;
 	default:
-		dev_err(&host->pd->dev, "Unsupported CMD%d\n", opc);
+		dev_err(dev, "Unsupported CMD%d\n", opc);
 		return -EINVAL;
 	}
 }
@@ -924,6 +940,8 @@ static void sh_mmcif_start_cmd(struct sh_mmcif_host *host,
 static void sh_mmcif_stop_cmd(struct sh_mmcif_host *host,
 			      struct mmc_request *mrq)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
+
 	switch (mrq->cmd->opcode) {
 	case MMC_READ_MULTIPLE_BLOCK:
 		sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MCMD12DRE);
@@ -932,7 +950,7 @@ static void sh_mmcif_stop_cmd(struct sh_mmcif_host *host,
 		sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MCMD12RBE);
 		break;
 	default:
-		dev_err(&host->pd->dev, "unsupported stop cmd\n");
+		dev_err(dev, "unsupported stop cmd\n");
 		mrq->stop->error = sh_mmcif_error_manage(host);
 		return;
 	}
@@ -943,11 +961,13 @@ static void sh_mmcif_stop_cmd(struct sh_mmcif_host *host,
 static void sh_mmcif_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct sh_mmcif_host *host = mmc_priv(mmc);
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	unsigned long flags;
 
 	spin_lock_irqsave(&host->lock, flags);
 	if (host->state != STATE_IDLE) {
-		dev_dbg(&host->pd->dev, "%s() rejected, state %u\n", __func__, host->state);
+		dev_dbg(dev, "%s() rejected, state %u\n",
+			__func__, host->state);
 		spin_unlock_irqrestore(&host->lock, flags);
 		mrq->cmd->error = -EAGAIN;
 		mmc_request_done(mmc, mrq);
@@ -999,11 +1019,13 @@ static void sh_mmcif_set_power(struct sh_mmcif_host *host, struct mmc_ios *ios)
 static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct sh_mmcif_host *host = mmc_priv(mmc);
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	unsigned long flags;
 
 	spin_lock_irqsave(&host->lock, flags);
 	if (host->state != STATE_IDLE) {
-		dev_dbg(&host->pd->dev, "%s() rejected, state %u\n", __func__, host->state);
+		dev_dbg(dev, "%s() rejected, state %u\n",
+			__func__, host->state);
 		spin_unlock_irqrestore(&host->lock, flags);
 		return;
 	}
@@ -1014,7 +1036,7 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	if (ios->power_mode == MMC_POWER_UP) {
 		if (!host->card_present) {
 			/* See if we also get DMA */
-			sh_mmcif_request_dma(host, host->pd->dev.platform_data);
+			sh_mmcif_request_dma(host, dev->platform_data);
 			host->card_present = true;
 		}
 		sh_mmcif_set_power(host, ios);
@@ -1028,7 +1050,7 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			}
 		}
 		if (host->power) {
-			pm_runtime_put_sync(&host->pd->dev);
+			pm_runtime_put_sync(dev);
 			clk_disable_unprepare(host->clk);
 			host->power = false;
 			if (ios->power_mode == MMC_POWER_OFF)
@@ -1042,7 +1064,7 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		if (!host->power) {
 			clk_prepare_enable(host->clk);
 
-			pm_runtime_get_sync(&host->pd->dev);
+			pm_runtime_get_sync(dev);
 			host->power = true;
 			sh_mmcif_sync_reset(host);
 		}
@@ -1057,7 +1079,8 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 static int sh_mmcif_get_cd(struct mmc_host *mmc)
 {
 	struct sh_mmcif_host *host = mmc_priv(mmc);
-	struct sh_mmcif_plat_data *p = host->pd->dev.platform_data;
+	struct device *dev = sh_mmcif_host_to_dev(host);
+	struct sh_mmcif_plat_data *p = dev->platform_data;
 	int ret = mmc_gpio_get_cd(mmc);
 
 	if (ret >= 0)
@@ -1079,6 +1102,7 @@ static bool sh_mmcif_end_cmd(struct sh_mmcif_host *host)
 {
 	struct mmc_command *cmd = host->mrq->cmd;
 	struct mmc_data *data = host->mrq->data;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	long time;
 
 	if (host->sd_error) {
@@ -1092,7 +1116,7 @@ static bool sh_mmcif_end_cmd(struct sh_mmcif_host *host)
 			cmd->error = sh_mmcif_error_manage(host);
 			break;
 		}
-		dev_dbg(&host->pd->dev, "CMD%d error %d\n",
+		dev_dbg(dev, "CMD%d error %d\n",
 			cmd->opcode, cmd->error);
 		host->sd_error = false;
 		return false;
@@ -1172,6 +1196,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 {
 	struct sh_mmcif_host *host = dev_id;
 	struct mmc_request *mrq;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	bool wait = false;
 	unsigned long flags;
 	int wait_work;
@@ -1186,7 +1211,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 
 	mrq = host->mrq;
 	if (!mrq) {
-		dev_dbg(&host->pd->dev, "IRQ thread state %u, wait %u: NULL mrq!\n",
+		dev_dbg(dev, "IRQ thread state %u, wait %u: NULL mrq!\n",
 			host->state, host->wait_for);
 		mutex_unlock(&host->thread_lock);
 		return IRQ_HANDLED;
@@ -1224,7 +1249,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 	case MMCIF_WAIT_FOR_STOP:
 		if (host->sd_error) {
 			mrq->stop->error = sh_mmcif_error_manage(host);
-			dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, mrq->stop->error);
+			dev_dbg(dev, "%s(): %d\n", __func__, mrq->stop->error);
 			break;
 		}
 		sh_mmcif_get_cmd12response(host, mrq->stop);
@@ -1234,7 +1259,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 	case MMCIF_WAIT_FOR_WRITE_END:
 		if (host->sd_error) {
 			mrq->data->error = sh_mmcif_error_manage(host);
-			dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, mrq->data->error);
+			dev_dbg(dev, "%s(): %d\n", __func__, mrq->data->error);
 		}
 		break;
 	default:
@@ -1277,6 +1302,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 static irqreturn_t sh_mmcif_intr(int irq, void *dev_id)
 {
 	struct sh_mmcif_host *host = dev_id;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	u32 state, mask;
 
 	state = sh_mmcif_readl(host->addr, MMCIF_CE_INT);
@@ -1288,22 +1314,22 @@ static irqreturn_t sh_mmcif_intr(int irq, void *dev_id)
 	sh_mmcif_bitclr(host, MMCIF_CE_INT_MASK, state & MASK_CLEAN);
 
 	if (state & ~MASK_CLEAN)
-		dev_dbg(&host->pd->dev, "IRQ state = 0x%08x incompletely cleared\n",
+		dev_dbg(dev, "IRQ state = 0x%08x incompletely cleared\n",
 			state);
 
 	if (state & INT_ERR_STS || state & ~INT_ALL) {
 		host->sd_error = true;
-		dev_dbg(&host->pd->dev, "int err state = 0x%08x\n", state);
+		dev_dbg(dev, "int err state = 0x%08x\n", state);
 	}
 	if (state & ~(INT_CMD12RBE | INT_CMD12CRE)) {
 		if (!host->mrq)
-			dev_dbg(&host->pd->dev, "NULL IRQ state = 0x%08x\n", state);
+			dev_dbg(dev, "NULL IRQ state = 0x%08x\n", state);
 		if (!host->dma_active)
 			return IRQ_WAKE_THREAD;
 		else if (host->sd_error)
 			mmcif_dma_complete(host);
 	} else {
-		dev_dbg(&host->pd->dev, "Unexpected IRQ 0x%x\n", state);
+		dev_dbg(dev, "Unexpected IRQ 0x%x\n", state);
 	}
 
 	return IRQ_HANDLED;
@@ -1314,6 +1340,7 @@ static void mmcif_timeout_work(struct work_struct *work)
 	struct delayed_work *d = container_of(work, struct delayed_work, work);
 	struct sh_mmcif_host *host = container_of(d, struct sh_mmcif_host, timeout_work);
 	struct mmc_request *mrq = host->mrq;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	unsigned long flags;
 
 	if (host->dying)
@@ -1326,7 +1353,7 @@ static void mmcif_timeout_work(struct work_struct *work)
 		return;
 	}
 
-	dev_err(&host->pd->dev, "Timeout waiting for %u on CMD%u\n",
+	dev_err(dev, "Timeout waiting for %u on CMD%u\n",
 		host->wait_for, mrq->cmd->opcode);
 
 	host->state = STATE_TIMEOUT;
@@ -1363,7 +1390,8 @@ static void mmcif_timeout_work(struct work_struct *work)
 
 static void sh_mmcif_init_ocr(struct sh_mmcif_host *host)
 {
-	struct sh_mmcif_plat_data *pd = host->pd->dev.platform_data;
+	struct device *dev = sh_mmcif_host_to_dev(host);
+	struct sh_mmcif_plat_data *pd = dev->platform_data;
 	struct mmc_host *mmc = host->mmc;
 
 	mmc_regulator_get_supply(mmc);
-- 
1.9.1

[PATCH 2/3 v5] mmc: sh_mmcif: use sh_mmcif_xxx prefix for all functions

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current sh_mmcif driver is using sh_mmcif_xxx and mmcif_xxx
prefix for naming. This patch used sh_mmcif_xxx for all name.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v4 -> v5

 - new patch for cleanup

 drivers/mmc/host/sh_mmcif.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 29835ae..e13c5f41 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -205,14 +205,14 @@
 #define CLKDEV_MMC_DATA		20000000 /* 20MHz */
 #define CLKDEV_INIT		400000   /* 400 KHz */
 
-enum mmcif_state {
+enum sh_mmcif_state {
 	STATE_IDLE,
 	STATE_REQUEST,
 	STATE_IOS,
 	STATE_TIMEOUT,
 };
 
-enum mmcif_wait_for {
+enum sh_mmcif_wait_for {
 	MMCIF_WAIT_FOR_REQUEST,
 	MMCIF_WAIT_FOR_CMD,
 	MMCIF_WAIT_FOR_MREAD,
@@ -237,8 +237,8 @@ struct sh_mmcif_host {
 	void __iomem *addr;
 	u32 *pio_ptr;
 	spinlock_t lock;		/* protect sh_mmcif_host::state */
-	enum mmcif_state state;
-	enum mmcif_wait_for wait_for;
+	enum sh_mmcif_state state;
+	enum sh_mmcif_wait_for wait_for;
 	struct delayed_work timeout_work;
 	size_t blocksize;
 	int sg_idx;
@@ -256,11 +256,11 @@ struct sh_mmcif_host {
 	bool			dma_active;
 };
 
-static const struct of_device_id mmcif_of_match[] = {
+static const struct of_device_id sh_mmcif_of_match[] = {
 	{ .compatible = "renesas,sh-mmcif" },
 	{ }
 };
-MODULE_DEVICE_TABLE(of, mmcif_of_match);
+MODULE_DEVICE_TABLE(of, sh_mmcif_of_match);
 
 #define sh_mmcif_host_to_dev(host) (&host->pd->dev)
 
@@ -276,7 +276,7 @@ static inline void sh_mmcif_bitclr(struct sh_mmcif_host *host,
 	writel(~val & readl(host->addr + reg), host->addr + reg);
 }
 
-static void mmcif_dma_complete(void *arg)
+static void sh_mmcif_dma_complete(void *arg)
 {
 	struct sh_mmcif_host *host = arg;
 	struct mmc_request *mrq = host->mrq;
@@ -310,7 +310,7 @@ static void sh_mmcif_start_dma_rx(struct sh_mmcif_host *host)
 	}
 
 	if (desc) {
-		desc->callback = mmcif_dma_complete;
+		desc->callback = sh_mmcif_dma_complete;
 		desc->callback_param = host;
 		cookie = dmaengine_submit(desc);
 		sh_mmcif_bitset(host, MMCIF_CE_BUF_ACC, BUF_ACC_DMAREN);
@@ -360,7 +360,7 @@ static void sh_mmcif_start_dma_tx(struct sh_mmcif_host *host)
 	}
 
 	if (desc) {
-		desc->callback = mmcif_dma_complete;
+		desc->callback = sh_mmcif_dma_complete;
 		desc->callback_param = host;
 		cookie = dmaengine_submit(desc);
 		sh_mmcif_bitset(host, MMCIF_CE_BUF_ACC, BUF_ACC_DMAWEN);
@@ -1327,7 +1327,7 @@ static irqreturn_t sh_mmcif_intr(int irq, void *dev_id)
 		if (!host->dma_active)
 			return IRQ_WAKE_THREAD;
 		else if (host->sd_error)
-			mmcif_dma_complete(host);
+			sh_mmcif_dma_complete(host);
 	} else {
 		dev_dbg(dev, "Unexpected IRQ 0x%x\n", state);
 	}
@@ -1335,7 +1335,7 @@ static irqreturn_t sh_mmcif_intr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void mmcif_timeout_work(struct work_struct *work)
+static void sh_mmcif_timeout_work(struct work_struct *work)
 {
 	struct delayed_work *d = container_of(work, struct delayed_work, work);
 	struct sh_mmcif_host *host = container_of(d, struct sh_mmcif_host, timeout_work);
@@ -1481,7 +1481,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_clk;
 
-	INIT_DELAYED_WORK(&host->timeout_work, mmcif_timeout_work);
+	INIT_DELAYED_WORK(&host->timeout_work, sh_mmcif_timeout_work);
 
 	sh_mmcif_sync_reset(host);
 	sh_mmcif_writel(host->addr, MMCIF_CE_INT_MASK, MASK_ALL);
@@ -1587,7 +1587,7 @@ static struct platform_driver sh_mmcif_driver = {
 	.driver		= {
 		.name	= DRIVER_NAME,
 		.pm	= &sh_mmcif_dev_pm_ops,
-		.of_match_table = mmcif_of_match,
+		.of_match_table = sh_mmcif_of_match,
 	},
 };
 
-- 
1.9.1

[PATCH 3/3 v5] mmc: sh_mmcif: calculate best clock with parent clock

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

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.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v4 -> v5

 - get clock data from compatible

 drivers/mmc/host/sh_mmcif.c | 92 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 83 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index e13c5f41..7ade263 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,15 @@ enum sh_mmcif_wait_for {
 	MMCIF_WAIT_FOR_STOP,
 };
 
+/*
+ * difference for each SoC
+ */
+struct sh_mmcif_ver {
+	u32 clkdiv_map;		/* see CE_CLK_CTRL::CLKDIV */
+	unsigned int pf_min;	/* parent frequency min */
+	unsigned int pf_max;	/* parent frequency max */
+};
+
 struct sh_mmcif_host {
 	struct mmc_host *mmc;
 	struct mmc_request *mrq;
@@ -248,6 +258,7 @@ struct sh_mmcif_host {
 	bool ccs_enable;		/* Command Completion Signal support */
 	bool clk_ctrl2_enable;
 	struct mutex thread_lock;
+	const struct sh_mmcif_ver *ver;
 
 	/* DMA support */
 	struct dma_chan		*chan_rx;
@@ -256,8 +267,16 @@ struct sh_mmcif_host {
 	bool			dma_active;
 };
 
+static const struct sh_mmcif_ver sh_mmcif_gen2 = {
+	.clkdiv_map	= 0x3ff,
+	.pf_min		= 12187500,
+	.pf_max		= 97500000,
+};
+
 static const struct of_device_id sh_mmcif_of_match[] = {
 	{ .compatible = "renesas,sh-mmcif" },
+	{ .compatible = "renesas,mmcif-r8a7790", .data = &sh_mmcif_gen2 },
+	{ .compatible = "renesas,mmcif-r8a7791", .data = &sh_mmcif_gen2 },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sh_mmcif_of_match);
@@ -492,19 +511,56 @@ static void sh_mmcif_clock_control(struct sh_mmcif_host *host, unsigned int clk)
 	struct sh_mmcif_plat_data *p = dev->platform_data;
 	bool sup_pclk = p ? p->sup_pclk : false;
 	unsigned int current_clk = clk_get_rate(host->clk);
+	unsigned int clkdiv;
 
 	sh_mmcif_bitclr(host, MMCIF_CE_CLK_CTRL, CLK_ENABLE);
 	sh_mmcif_bitclr(host, MMCIF_CE_CLK_CTRL, CLK_CLEAR);
 
 	if (!clk)
 		return;
-	if (sup_pclk && clk == current_clk)
-		sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_SUP_PCLK);
-	else
-		sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_CLEAR &
-				((fls(DIV_ROUND_UP(current_clk,
-						   clk) - 1) - 1) << 16));
 
+	if (host->ver) {
+		const struct sh_mmcif_ver *ver = host->ver;
+		unsigned int freq, best_freq, myclk, div, diff_min, diff;
+		int i;
+
+		clkdiv = 0;
+		diff_min = ~0;
+		best_freq = 0;
+		for (i = 31; i >= 0; i--) {
+			if (!((1 << i) & ver->clkdiv_map))
+				continue;
+
+			/*
+			 * clk = parent_freq / div
+			 * -> parent_freq = clk x div
+			 */
+
+			div = 1 << (i + 1);
+			freq = clk_round_rate(host->clk, clk * div);
+			myclk = freq / div;
+			diff = (myclk > clk) ? myclk - clk : clk - myclk;
+
+			if (diff <= diff_min) {
+				best_freq = freq;
+				clkdiv = i;
+				diff_min = diff;
+			}
+		}
+
+		dev_dbg(dev, "clk %u/%u (%u, 0x%x)\n",
+			(best_freq / (1 << (clkdiv + 1))), clk,
+			best_freq, clkdiv);
+
+		clk_set_rate(host->clk, best_freq);
+		clkdiv = clkdiv << 16;
+	} else if (sup_pclk && clk == current_clk) {
+		clkdiv = CLK_SUP_PCLK;
+	} else {
+		clkdiv = (fls(DIV_ROUND_UP(current_clk, clk) - 1) - 1) << 16;
+	}
+
+	sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_CLEAR & clkdiv);
 	sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_ENABLE);
 }
 
@@ -1000,10 +1056,28 @@ static void sh_mmcif_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 static void sh_mmcif_clk_setup(struct sh_mmcif_host *host)
 {
-	unsigned int clk = clk_get_rate(host->clk);
+	struct device *dev = sh_mmcif_host_to_dev(host);
+	const struct of_device_id *of_id;
+	const struct sh_mmcif_ver *ver = NULL;
+
+	of_id = of_match_device(sh_mmcif_of_match, dev);
+	if (of_id)
+		ver = of_id->data;
+
+	if (ver) {
+		host->mmc->f_min = ver->pf_min / (1 << fls(ver->clkdiv_map));
+		host->mmc->f_max = ver->pf_max / (1 << ffs(ver->clkdiv_map));
+
+		host->ver = ver;
+	} else {
+		unsigned int clk = clk_get_rate(host->clk);
+
+		host->mmc->f_max = clk / 2;
+		host->mmc->f_min = clk / 512;
+	}
 
-	host->mmc->f_max = clk / 2;
-	host->mmc->f_min = clk / 512;
+	dev_dbg(dev, "clk max/min = %d/%d\n",
+		host->mmc->f_max, host->mmc->f_min);
 }
 
 static void sh_mmcif_set_power(struct sh_mmcif_host *host, struct mmc_ios *ios)
-- 
1.9.1

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

[Geert Uytterhoeven]

Hi Morimoto-san,

On Wed, May 13, 2015 at 4:18 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> 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.

Thanks for this update.

I'm still wondering if we can do this without hardcoding frequencies in the
driver...

>From the discussion before:
"So the upper limit is a limitation of the MMCIF hardware, while the lower
 limit is a limitation of the CPG."

> index e13c5f41..7ade263 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c

> @@ -224,6 +225,15 @@ enum sh_mmcif_wait_for {
>         MMCIF_WAIT_FOR_STOP,
>  };
>
> +/*
> + * difference for each SoC
> + */
> +struct sh_mmcif_ver {
> +       u32 clkdiv_map;         /* see CE_CLK_CTRL::CLKDIV */
> +       unsigned int pf_min;    /* parent frequency min */

I don't think you need an explicit lower limit: clk_round_rate() will
never return a frequency that's lower than the parent clock's lower limit.

> +       unsigned int pf_max;    /* parent frequency max */
> +};
> +
>  struct sh_mmcif_host {
>         struct mmc_host *mmc;
>         struct mmc_request *mrq;
> @@ -248,6 +258,7 @@ struct sh_mmcif_host {
>         bool ccs_enable;                /* Command Completion Signal support */
>         bool clk_ctrl2_enable;
>         struct mutex thread_lock;
> +       const struct sh_mmcif_ver *ver;
>
>         /* DMA support */
>         struct dma_chan         *chan_rx;
> @@ -256,8 +267,16 @@ struct sh_mmcif_host {
>         bool                    dma_active;
>  };
>
> +static const struct sh_mmcif_ver sh_mmcif_gen2 = {
> +       .clkdiv_map     = 0x3ff,
> +       .pf_min         = 12187500,
> +       .pf_max         = 97500000,

By any chance, does setting "max-frequency = <97500000>" from the standard
MMC DT bindings have the same effect?

> +};
> +
>  static const struct of_device_id sh_mmcif_of_match[] = {
>         { .compatible = "renesas,sh-mmcif" },
> +       { .compatible = "renesas,mmcif-r8a7790", .data = &sh_mmcif_gen2 },
> +       { .compatible = "renesas,mmcif-r8a7791", .data = &sh_mmcif_gen2 },
>         { }
>  };

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

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

[Kuninori Morimoto]

Hi Geert

Thank you for your feedback

> > +/*
> > + * difference for each SoC
> > + */
> > +struct sh_mmcif_ver {
> > +       u32 clkdiv_map;         /* see CE_CLK_CTRL::CLKDIV */
> > +       unsigned int pf_min;    /* parent frequency min */
> 
> I don't think you need an explicit lower limit: clk_round_rate() will
> never return a frequency that's lower than the parent clock's lower limit.

Indeed... Good catch

> > +static const struct sh_mmcif_ver sh_mmcif_gen2 = {
> > +       .clkdiv_map     = 0x3ff,
> > +       .pf_min         = 12187500,
> > +       .pf_max         = 97500000,
> 
> By any chance, does setting "max-frequency = <97500000>" from the standard
> MMC DT bindings have the same effect?

I'm still not understanding about this.
If we can specify SoC (via compatible), the clock frequency limitation
can be specified in same time.
Why do we need to have flexibility for it ?
What is yuur "any chance" mean ?

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

[Geert Uytterhoeven]

Hi Morimoto-san,

On Wed, May 13, 2015 at 10:37 AM, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>> > +static const struct sh_mmcif_ver sh_mmcif_gen2 = {
>> > +       .clkdiv_map     = 0x3ff,
>> > +       .pf_min         = 12187500,
>> > +       .pf_max         = 97500000,
>>
>> By any chance, does setting "max-frequency = <97500000>" from the standard
>> MMC DT bindings have the same effect?
>
> I'm still not understanding about this.
> If we can specify SoC (via compatible), the clock frequency limitation
> can be specified in same time.
> Why do we need to have flexibility for it ?

It's better if you don't need to update the C source file for every new SoC,
especially if you can use a standard MMC DT property instead.
Still, you need it for clkdiv_map, right?

> What is yuur "any chance" mean ?

Just ignore those words.

Does setting "max-frequency = <97500000>" have the same effect as specifying
the parent clock upper limit in C?

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

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

[Kuninori Morimoto]

Hi Geert

> > I'm still not understanding about this.
> > If we can specify SoC (via compatible), the clock frequency limitation
> > can be specified in same time.
> > Why do we need to have flexibility for it ?
> 
> It's better if you don't need to update the C source file for every new SoC,
> especially if you can use a standard MMC DT property instead.
> Still, you need it for clkdiv_map, right?

Hmm... many people tell me different opinion...
I understand your opinion, but I'm confusing about dilemma.

If we can get all these settings via DT, (maybe) we don't need to
update driver for every new SoC, yes.
But it works if "this is 100% completed driver/HW".

If we get settings from DT, but MMC IP was updated in the future,
then, we need to care about ABI compatibility for it.
And sometimes (anytime ?) Renesas IP is updated strangely.
In such case, keeping ABI compatibility, and adds new feature
will be super difficult.

I discussed about this topic before with one engineer,
and the final answer was
"updating C source file is easier than updating DT / ABI compatibility"

This is the big reason why I want to get settings from driver
as much as possible.
We need to update C source code for every new SoC,
and Yes this is PITA, but easier than keeping ABI compatibility.
And updating C source code for every new SoC can indicates
"someone is caring this driver" to user

Best regards
---
Kuninori Morimoto

[PATCH 0/3 v6] mmc: sh_mmcif: add parent clk support

[Kuninori Morimoto]

Hi Ulf, Simon

These patches are v6 of sh_mmcif clock support.

1) - 2) : cleanup patches
3)      : v6 patch for clk
4) - 5) : DTS patch

Simon

 1) - 3) only : driver uses default clock
 4) - 5) only : driver uses it, but it is updated to default clock
 1) - 5) all  : it uses new method

so, you can check 4) - 5) without thinking about 1) - 3).

Kuninori Morimoto (5):
      1) mmc: sh_mmcif: add sh_mmcif_host_to_dev() macro and use it.
      2) mmc: sh_mmcif: use sh_mmcif_xxx prefix for all functions
      3) mmc: sh_mmcif: calculate best clock with parent clock
      4) ARM: shmobile: r8a7790: add MMCIF max-frequency
      5) ARM: shmobile: r8a7791: add MMCIF max-frequency

 Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |   3 ++
 arch/arm/boot/dts/r8a7790.dtsi                          |   2 +
 arch/arm/boot/dts/r8a7791.dtsi                          |   1 +
 drivers/mmc/host/sh_mmcif.c                             | 220 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------
 4 files changed, 161 insertions(+), 65 deletions(-)

Best regards
---
Kuninori Morimoto
--
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
--
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

[PATCH 1/5 v6] mmc: sh_mmcif: add sh_mmcif_host_to_dev() macro and use it.

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current sh_mmcif driver is directly using &host->pd->dev in all place.
It is not big problem, but it is unreadable, and it can be cause of
future bug. This patch adds new sh_mmcif_host_to_dev() and use it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v5 -> v6

 - no change

 drivers/mmc/host/sh_mmcif.c | 114 +++++++++++++++++++++++++++-----------------
 1 file changed, 71 insertions(+), 43 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 068b9a6..29835ae 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -262,6 +262,8 @@ static const struct of_device_id mmcif_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, mmcif_of_match);
 
+#define sh_mmcif_host_to_dev(host) (&host->pd->dev)
+
 static inline void sh_mmcif_bitset(struct sh_mmcif_host *host,
 					unsigned int reg, u32 val)
 {
@@ -278,11 +280,12 @@ static void mmcif_dma_complete(void *arg)
 {
 	struct sh_mmcif_host *host = arg;
 	struct mmc_request *mrq = host->mrq;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 
-	dev_dbg(&host->pd->dev, "Command completed\n");
+	dev_dbg(dev, "Command completed\n");
 
 	if (WARN(!mrq || !mrq->data, "%s: NULL data in DMA completion!\n",
-		 dev_name(&host->pd->dev)))
+		 dev_name(dev)))
 		return;
 
 	complete(&host->dma_complete);
@@ -294,6 +297,7 @@ static void sh_mmcif_start_dma_rx(struct sh_mmcif_host *host)
 	struct scatterlist *sg = data->sg;
 	struct dma_async_tx_descriptor *desc = NULL;
 	struct dma_chan *chan = host->chan_rx;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	dma_cookie_t cookie = -EINVAL;
 	int ret;
 
@@ -312,7 +316,7 @@ static void sh_mmcif_start_dma_rx(struct sh_mmcif_host *host)
 		sh_mmcif_bitset(host, MMCIF_CE_BUF_ACC, BUF_ACC_DMAREN);
 		dma_async_issue_pending(chan);
 	}
-	dev_dbg(&host->pd->dev, "%s(): mapped %d -> %d, cookie %d\n",
+	dev_dbg(dev, "%s(): mapped %d -> %d, cookie %d\n",
 		__func__, data->sg_len, ret, cookie);
 
 	if (!desc) {
@@ -328,12 +332,12 @@ static void sh_mmcif_start_dma_rx(struct sh_mmcif_host *host)
 			host->chan_tx = NULL;
 			dma_release_channel(chan);
 		}
-		dev_warn(&host->pd->dev,
+		dev_warn(dev,
 			 "DMA failed: %d, falling back to PIO\n", ret);
 		sh_mmcif_bitclr(host, MMCIF_CE_BUF_ACC, BUF_ACC_DMAREN | BUF_ACC_DMAWEN);
 	}
 
-	dev_dbg(&host->pd->dev, "%s(): desc %p, cookie %d, sg[%d]\n", __func__,
+	dev_dbg(dev, "%s(): desc %p, cookie %d, sg[%d]\n", __func__,
 		desc, cookie, data->sg_len);
 }
 
@@ -343,6 +347,7 @@ static void sh_mmcif_start_dma_tx(struct sh_mmcif_host *host)
 	struct scatterlist *sg = data->sg;
 	struct dma_async_tx_descriptor *desc = NULL;
 	struct dma_chan *chan = host->chan_tx;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	dma_cookie_t cookie = -EINVAL;
 	int ret;
 
@@ -361,7 +366,7 @@ static void sh_mmcif_start_dma_tx(struct sh_mmcif_host *host)
 		sh_mmcif_bitset(host, MMCIF_CE_BUF_ACC, BUF_ACC_DMAWEN);
 		dma_async_issue_pending(chan);
 	}
-	dev_dbg(&host->pd->dev, "%s(): mapped %d -> %d, cookie %d\n",
+	dev_dbg(dev, "%s(): mapped %d -> %d, cookie %d\n",
 		__func__, data->sg_len, ret, cookie);
 
 	if (!desc) {
@@ -377,12 +382,12 @@ static void sh_mmcif_start_dma_tx(struct sh_mmcif_host *host)
 			host->chan_rx = NULL;
 			dma_release_channel(chan);
 		}
-		dev_warn(&host->pd->dev,
+		dev_warn(dev,
 			 "DMA failed: %d, falling back to PIO\n", ret);
 		sh_mmcif_bitclr(host, MMCIF_CE_BUF_ACC, BUF_ACC_DMAREN | BUF_ACC_DMAWEN);
 	}
 
-	dev_dbg(&host->pd->dev, "%s(): desc %p, cookie %d\n", __func__,
+	dev_dbg(dev, "%s(): desc %p, cookie %d\n", __func__,
 		desc, cookie);
 }
 
@@ -395,6 +400,7 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
 	struct dma_chan *chan;
 	void *slave_data = NULL;
 	struct resource *res;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	dma_cap_mask_t mask;
 	int ret;
 
@@ -407,10 +413,10 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
 			(void *)pdata->slave_id_rx;
 
 	chan = dma_request_slave_channel_compat(mask, shdma_chan_filter,
-				slave_data, &host->pd->dev,
+				slave_data, dev,
 				direction == DMA_MEM_TO_DEV ? "tx" : "rx");
 
-	dev_dbg(&host->pd->dev, "%s: %s: got channel %p\n", __func__,
+	dev_dbg(dev, "%s: %s: got channel %p\n", __func__,
 		direction == DMA_MEM_TO_DEV ? "TX" : "RX", chan);
 
 	if (!chan)
@@ -440,12 +446,13 @@ sh_mmcif_request_dma_one(struct sh_mmcif_host *host,
 static void sh_mmcif_request_dma(struct sh_mmcif_host *host,
 				 struct sh_mmcif_plat_data *pdata)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	host->dma_active = false;
 
 	if (pdata) {
 		if (pdata->slave_id_tx <= 0 || pdata->slave_id_rx <= 0)
 			return;
-	} else if (!host->pd->dev.of_node) {
+	} else if (!dev->of_node) {
 		return;
 	}
 
@@ -481,7 +488,8 @@ static void sh_mmcif_release_dma(struct sh_mmcif_host *host)
 
 static void sh_mmcif_clock_control(struct sh_mmcif_host *host, unsigned int clk)
 {
-	struct sh_mmcif_plat_data *p = host->pd->dev.platform_data;
+	struct device *dev = sh_mmcif_host_to_dev(host);
+	struct sh_mmcif_plat_data *p = dev->platform_data;
 	bool sup_pclk = p ? p->sup_pclk : false;
 	unsigned int current_clk = clk_get_rate(host->clk);
 
@@ -520,6 +528,7 @@ static void sh_mmcif_sync_reset(struct sh_mmcif_host *host)
 
 static int sh_mmcif_error_manage(struct sh_mmcif_host *host)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	u32 state1, state2;
 	int ret, timeout;
 
@@ -527,8 +536,8 @@ static int sh_mmcif_error_manage(struct sh_mmcif_host *host)
 
 	state1 = sh_mmcif_readl(host->addr, MMCIF_CE_HOST_STS1);
 	state2 = sh_mmcif_readl(host->addr, MMCIF_CE_HOST_STS2);
-	dev_dbg(&host->pd->dev, "ERR HOST_STS1 = %08x\n", state1);
-	dev_dbg(&host->pd->dev, "ERR HOST_STS2 = %08x\n", state2);
+	dev_dbg(dev, "ERR HOST_STS1 = %08x\n", state1);
+	dev_dbg(dev, "ERR HOST_STS2 = %08x\n", state2);
 
 	if (state1 & STS1_CMDSEQ) {
 		sh_mmcif_bitset(host, MMCIF_CE_CMD_CTRL, CMD_CTRL_BREAK);
@@ -540,25 +549,25 @@ static int sh_mmcif_error_manage(struct sh_mmcif_host *host)
 			mdelay(1);
 		}
 		if (!timeout) {
-			dev_err(&host->pd->dev,
+			dev_err(dev,
 				"Forced end of command sequence timeout err\n");
 			return -EIO;
 		}
 		sh_mmcif_sync_reset(host);
-		dev_dbg(&host->pd->dev, "Forced end of command sequence\n");
+		dev_dbg(dev, "Forced end of command sequence\n");
 		return -EIO;
 	}
 
 	if (state2 & STS2_CRC_ERR) {
-		dev_err(&host->pd->dev, " CRC error: state %u, wait %u\n",
+		dev_err(dev, " CRC error: state %u, wait %u\n",
 			host->state, host->wait_for);
 		ret = -EIO;
 	} else if (state2 & STS2_TIMEOUT_ERR) {
-		dev_err(&host->pd->dev, " Timeout: state %u, wait %u\n",
+		dev_err(dev, " Timeout: state %u, wait %u\n",
 			host->state, host->wait_for);
 		ret = -ETIMEDOUT;
 	} else {
-		dev_dbg(&host->pd->dev, " End/Index error: state %u, wait %u\n",
+		dev_dbg(dev, " End/Index error: state %u, wait %u\n",
 			host->state, host->wait_for);
 		ret = -EIO;
 	}
@@ -599,13 +608,14 @@ static void sh_mmcif_single_read(struct sh_mmcif_host *host,
 
 static bool sh_mmcif_read_block(struct sh_mmcif_host *host)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	struct mmc_data *data = host->mrq->data;
 	u32 *p = sg_virt(data->sg);
 	int i;
 
 	if (host->sd_error) {
 		data->error = sh_mmcif_error_manage(host);
-		dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, data->error);
+		dev_dbg(dev, "%s(): %d\n", __func__, data->error);
 		return false;
 	}
 
@@ -640,13 +650,14 @@ static void sh_mmcif_multi_read(struct sh_mmcif_host *host,
 
 static bool sh_mmcif_mread_block(struct sh_mmcif_host *host)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	struct mmc_data *data = host->mrq->data;
 	u32 *p = host->pio_ptr;
 	int i;
 
 	if (host->sd_error) {
 		data->error = sh_mmcif_error_manage(host);
-		dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, data->error);
+		dev_dbg(dev, "%s(): %d\n", __func__, data->error);
 		return false;
 	}
 
@@ -677,13 +688,14 @@ static void sh_mmcif_single_write(struct sh_mmcif_host *host,
 
 static bool sh_mmcif_write_block(struct sh_mmcif_host *host)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	struct mmc_data *data = host->mrq->data;
 	u32 *p = sg_virt(data->sg);
 	int i;
 
 	if (host->sd_error) {
 		data->error = sh_mmcif_error_manage(host);
-		dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, data->error);
+		dev_dbg(dev, "%s(): %d\n", __func__, data->error);
 		return false;
 	}
 
@@ -718,13 +730,14 @@ static void sh_mmcif_multi_write(struct sh_mmcif_host *host,
 
 static bool sh_mmcif_mwrite_block(struct sh_mmcif_host *host)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	struct mmc_data *data = host->mrq->data;
 	u32 *p = host->pio_ptr;
 	int i;
 
 	if (host->sd_error) {
 		data->error = sh_mmcif_error_manage(host);
-		dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, data->error);
+		dev_dbg(dev, "%s(): %d\n", __func__, data->error);
 		return false;
 	}
 
@@ -762,6 +775,7 @@ static void sh_mmcif_get_cmd12response(struct sh_mmcif_host *host,
 static u32 sh_mmcif_set_cmd(struct sh_mmcif_host *host,
 			    struct mmc_request *mrq)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	struct mmc_data *data = mrq->data;
 	struct mmc_command *cmd = mrq->cmd;
 	u32 opc = cmd->opcode;
@@ -781,7 +795,7 @@ static u32 sh_mmcif_set_cmd(struct sh_mmcif_host *host,
 		tmp |= CMD_SET_RTYP_17B;
 		break;
 	default:
-		dev_err(&host->pd->dev, "Unsupported response type.\n");
+		dev_err(dev, "Unsupported response type.\n");
 		break;
 	}
 	switch (opc) {
@@ -809,7 +823,7 @@ static u32 sh_mmcif_set_cmd(struct sh_mmcif_host *host,
 			tmp |= CMD_SET_DATW_8;
 			break;
 		default:
-			dev_err(&host->pd->dev, "Unsupported bus width.\n");
+			dev_err(dev, "Unsupported bus width.\n");
 			break;
 		}
 		switch (host->timing) {
@@ -852,6 +866,8 @@ static u32 sh_mmcif_set_cmd(struct sh_mmcif_host *host,
 static int sh_mmcif_data_trans(struct sh_mmcif_host *host,
 			       struct mmc_request *mrq, u32 opc)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
+
 	switch (opc) {
 	case MMC_READ_MULTIPLE_BLOCK:
 		sh_mmcif_multi_read(host, mrq);
@@ -867,7 +883,7 @@ static int sh_mmcif_data_trans(struct sh_mmcif_host *host,
 		sh_mmcif_single_read(host, mrq);
 		return 0;
 	default:
-		dev_err(&host->pd->dev, "Unsupported CMD%d\n", opc);
+		dev_err(dev, "Unsupported CMD%d\n", opc);
 		return -EINVAL;
 	}
 }
@@ -924,6 +940,8 @@ static void sh_mmcif_start_cmd(struct sh_mmcif_host *host,
 static void sh_mmcif_stop_cmd(struct sh_mmcif_host *host,
 			      struct mmc_request *mrq)
 {
+	struct device *dev = sh_mmcif_host_to_dev(host);
+
 	switch (mrq->cmd->opcode) {
 	case MMC_READ_MULTIPLE_BLOCK:
 		sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MCMD12DRE);
@@ -932,7 +950,7 @@ static void sh_mmcif_stop_cmd(struct sh_mmcif_host *host,
 		sh_mmcif_bitset(host, MMCIF_CE_INT_MASK, MASK_MCMD12RBE);
 		break;
 	default:
-		dev_err(&host->pd->dev, "unsupported stop cmd\n");
+		dev_err(dev, "unsupported stop cmd\n");
 		mrq->stop->error = sh_mmcif_error_manage(host);
 		return;
 	}
@@ -943,11 +961,13 @@ static void sh_mmcif_stop_cmd(struct sh_mmcif_host *host,
 static void sh_mmcif_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct sh_mmcif_host *host = mmc_priv(mmc);
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	unsigned long flags;
 
 	spin_lock_irqsave(&host->lock, flags);
 	if (host->state != STATE_IDLE) {
-		dev_dbg(&host->pd->dev, "%s() rejected, state %u\n", __func__, host->state);
+		dev_dbg(dev, "%s() rejected, state %u\n",
+			__func__, host->state);
 		spin_unlock_irqrestore(&host->lock, flags);
 		mrq->cmd->error = -EAGAIN;
 		mmc_request_done(mmc, mrq);
@@ -999,11 +1019,13 @@ static void sh_mmcif_set_power(struct sh_mmcif_host *host, struct mmc_ios *ios)
 static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 {
 	struct sh_mmcif_host *host = mmc_priv(mmc);
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	unsigned long flags;
 
 	spin_lock_irqsave(&host->lock, flags);
 	if (host->state != STATE_IDLE) {
-		dev_dbg(&host->pd->dev, "%s() rejected, state %u\n", __func__, host->state);
+		dev_dbg(dev, "%s() rejected, state %u\n",
+			__func__, host->state);
 		spin_unlock_irqrestore(&host->lock, flags);
 		return;
 	}
@@ -1014,7 +1036,7 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	if (ios->power_mode == MMC_POWER_UP) {
 		if (!host->card_present) {
 			/* See if we also get DMA */
-			sh_mmcif_request_dma(host, host->pd->dev.platform_data);
+			sh_mmcif_request_dma(host, dev->platform_data);
 			host->card_present = true;
 		}
 		sh_mmcif_set_power(host, ios);
@@ -1028,7 +1050,7 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			}
 		}
 		if (host->power) {
-			pm_runtime_put_sync(&host->pd->dev);
+			pm_runtime_put_sync(dev);
 			clk_disable_unprepare(host->clk);
 			host->power = false;
 			if (ios->power_mode == MMC_POWER_OFF)
@@ -1042,7 +1064,7 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		if (!host->power) {
 			clk_prepare_enable(host->clk);
 
-			pm_runtime_get_sync(&host->pd->dev);
+			pm_runtime_get_sync(dev);
 			host->power = true;
 			sh_mmcif_sync_reset(host);
 		}
@@ -1057,7 +1079,8 @@ static void sh_mmcif_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 static int sh_mmcif_get_cd(struct mmc_host *mmc)
 {
 	struct sh_mmcif_host *host = mmc_priv(mmc);
-	struct sh_mmcif_plat_data *p = host->pd->dev.platform_data;
+	struct device *dev = sh_mmcif_host_to_dev(host);
+	struct sh_mmcif_plat_data *p = dev->platform_data;
 	int ret = mmc_gpio_get_cd(mmc);
 
 	if (ret >= 0)
@@ -1079,6 +1102,7 @@ static bool sh_mmcif_end_cmd(struct sh_mmcif_host *host)
 {
 	struct mmc_command *cmd = host->mrq->cmd;
 	struct mmc_data *data = host->mrq->data;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	long time;
 
 	if (host->sd_error) {
@@ -1092,7 +1116,7 @@ static bool sh_mmcif_end_cmd(struct sh_mmcif_host *host)
 			cmd->error = sh_mmcif_error_manage(host);
 			break;
 		}
-		dev_dbg(&host->pd->dev, "CMD%d error %d\n",
+		dev_dbg(dev, "CMD%d error %d\n",
 			cmd->opcode, cmd->error);
 		host->sd_error = false;
 		return false;
@@ -1172,6 +1196,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 {
 	struct sh_mmcif_host *host = dev_id;
 	struct mmc_request *mrq;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	bool wait = false;
 	unsigned long flags;
 	int wait_work;
@@ -1186,7 +1211,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 
 	mrq = host->mrq;
 	if (!mrq) {
-		dev_dbg(&host->pd->dev, "IRQ thread state %u, wait %u: NULL mrq!\n",
+		dev_dbg(dev, "IRQ thread state %u, wait %u: NULL mrq!\n",
 			host->state, host->wait_for);
 		mutex_unlock(&host->thread_lock);
 		return IRQ_HANDLED;
@@ -1224,7 +1249,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 	case MMCIF_WAIT_FOR_STOP:
 		if (host->sd_error) {
 			mrq->stop->error = sh_mmcif_error_manage(host);
-			dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, mrq->stop->error);
+			dev_dbg(dev, "%s(): %d\n", __func__, mrq->stop->error);
 			break;
 		}
 		sh_mmcif_get_cmd12response(host, mrq->stop);
@@ -1234,7 +1259,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 	case MMCIF_WAIT_FOR_WRITE_END:
 		if (host->sd_error) {
 			mrq->data->error = sh_mmcif_error_manage(host);
-			dev_dbg(&host->pd->dev, "%s(): %d\n", __func__, mrq->data->error);
+			dev_dbg(dev, "%s(): %d\n", __func__, mrq->data->error);
 		}
 		break;
 	default:
@@ -1277,6 +1302,7 @@ static irqreturn_t sh_mmcif_irqt(int irq, void *dev_id)
 static irqreturn_t sh_mmcif_intr(int irq, void *dev_id)
 {
 	struct sh_mmcif_host *host = dev_id;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	u32 state, mask;
 
 	state = sh_mmcif_readl(host->addr, MMCIF_CE_INT);
@@ -1288,22 +1314,22 @@ static irqreturn_t sh_mmcif_intr(int irq, void *dev_id)
 	sh_mmcif_bitclr(host, MMCIF_CE_INT_MASK, state & MASK_CLEAN);
 
 	if (state & ~MASK_CLEAN)
-		dev_dbg(&host->pd->dev, "IRQ state = 0x%08x incompletely cleared\n",
+		dev_dbg(dev, "IRQ state = 0x%08x incompletely cleared\n",
 			state);
 
 	if (state & INT_ERR_STS || state & ~INT_ALL) {
 		host->sd_error = true;
-		dev_dbg(&host->pd->dev, "int err state = 0x%08x\n", state);
+		dev_dbg(dev, "int err state = 0x%08x\n", state);
 	}
 	if (state & ~(INT_CMD12RBE | INT_CMD12CRE)) {
 		if (!host->mrq)
-			dev_dbg(&host->pd->dev, "NULL IRQ state = 0x%08x\n", state);
+			dev_dbg(dev, "NULL IRQ state = 0x%08x\n", state);
 		if (!host->dma_active)
 			return IRQ_WAKE_THREAD;
 		else if (host->sd_error)
 			mmcif_dma_complete(host);
 	} else {
-		dev_dbg(&host->pd->dev, "Unexpected IRQ 0x%x\n", state);
+		dev_dbg(dev, "Unexpected IRQ 0x%x\n", state);
 	}
 
 	return IRQ_HANDLED;
@@ -1314,6 +1340,7 @@ static void mmcif_timeout_work(struct work_struct *work)
 	struct delayed_work *d = container_of(work, struct delayed_work, work);
 	struct sh_mmcif_host *host = container_of(d, struct sh_mmcif_host, timeout_work);
 	struct mmc_request *mrq = host->mrq;
+	struct device *dev = sh_mmcif_host_to_dev(host);
 	unsigned long flags;
 
 	if (host->dying)
@@ -1326,7 +1353,7 @@ static void mmcif_timeout_work(struct work_struct *work)
 		return;
 	}
 
-	dev_err(&host->pd->dev, "Timeout waiting for %u on CMD%u\n",
+	dev_err(dev, "Timeout waiting for %u on CMD%u\n",
 		host->wait_for, mrq->cmd->opcode);
 
 	host->state = STATE_TIMEOUT;
@@ -1363,7 +1390,8 @@ static void mmcif_timeout_work(struct work_struct *work)
 
 static void sh_mmcif_init_ocr(struct sh_mmcif_host *host)
 {
-	struct sh_mmcif_plat_data *pd = host->pd->dev.platform_data;
+	struct device *dev = sh_mmcif_host_to_dev(host);
+	struct sh_mmcif_plat_data *pd = dev->platform_data;
 	struct mmc_host *mmc = host->mmc;
 
 	mmc_regulator_get_supply(mmc);
-- 
1.9.1

[PATCH 2/5 v6] mmc: sh_mmcif: use sh_mmcif_xxx prefix for all functions

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current sh_mmcif driver is using sh_mmcif_xxx and mmcif_xxx
for functions. This patch used sh_mmcif_xxx for all functions.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v5 -> v6

 - no change

 drivers/mmc/host/sh_mmcif.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 29835ae..e13c5f41 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -205,14 +205,14 @@
 #define CLKDEV_MMC_DATA		20000000 /* 20MHz */
 #define CLKDEV_INIT		400000   /* 400 KHz */
 
-enum mmcif_state {
+enum sh_mmcif_state {
 	STATE_IDLE,
 	STATE_REQUEST,
 	STATE_IOS,
 	STATE_TIMEOUT,
 };
 
-enum mmcif_wait_for {
+enum sh_mmcif_wait_for {
 	MMCIF_WAIT_FOR_REQUEST,
 	MMCIF_WAIT_FOR_CMD,
 	MMCIF_WAIT_FOR_MREAD,
@@ -237,8 +237,8 @@ struct sh_mmcif_host {
 	void __iomem *addr;
 	u32 *pio_ptr;
 	spinlock_t lock;		/* protect sh_mmcif_host::state */
-	enum mmcif_state state;
-	enum mmcif_wait_for wait_for;
+	enum sh_mmcif_state state;
+	enum sh_mmcif_wait_for wait_for;
 	struct delayed_work timeout_work;
 	size_t blocksize;
 	int sg_idx;
@@ -256,11 +256,11 @@ struct sh_mmcif_host {
 	bool			dma_active;
 };
 
-static const struct of_device_id mmcif_of_match[] = {
+static const struct of_device_id sh_mmcif_of_match[] = {
 	{ .compatible = "renesas,sh-mmcif" },
 	{ }
 };
-MODULE_DEVICE_TABLE(of, mmcif_of_match);
+MODULE_DEVICE_TABLE(of, sh_mmcif_of_match);
 
 #define sh_mmcif_host_to_dev(host) (&host->pd->dev)
 
@@ -276,7 +276,7 @@ static inline void sh_mmcif_bitclr(struct sh_mmcif_host *host,
 	writel(~val & readl(host->addr + reg), host->addr + reg);
 }
 
-static void mmcif_dma_complete(void *arg)
+static void sh_mmcif_dma_complete(void *arg)
 {
 	struct sh_mmcif_host *host = arg;
 	struct mmc_request *mrq = host->mrq;
@@ -310,7 +310,7 @@ static void sh_mmcif_start_dma_rx(struct sh_mmcif_host *host)
 	}
 
 	if (desc) {
-		desc->callback = mmcif_dma_complete;
+		desc->callback = sh_mmcif_dma_complete;
 		desc->callback_param = host;
 		cookie = dmaengine_submit(desc);
 		sh_mmcif_bitset(host, MMCIF_CE_BUF_ACC, BUF_ACC_DMAREN);
@@ -360,7 +360,7 @@ static void sh_mmcif_start_dma_tx(struct sh_mmcif_host *host)
 	}
 
 	if (desc) {
-		desc->callback = mmcif_dma_complete;
+		desc->callback = sh_mmcif_dma_complete;
 		desc->callback_param = host;
 		cookie = dmaengine_submit(desc);
 		sh_mmcif_bitset(host, MMCIF_CE_BUF_ACC, BUF_ACC_DMAWEN);
@@ -1327,7 +1327,7 @@ static irqreturn_t sh_mmcif_intr(int irq, void *dev_id)
 		if (!host->dma_active)
 			return IRQ_WAKE_THREAD;
 		else if (host->sd_error)
-			mmcif_dma_complete(host);
+			sh_mmcif_dma_complete(host);
 	} else {
 		dev_dbg(dev, "Unexpected IRQ 0x%x\n", state);
 	}
@@ -1335,7 +1335,7 @@ static irqreturn_t sh_mmcif_intr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static void mmcif_timeout_work(struct work_struct *work)
+static void sh_mmcif_timeout_work(struct work_struct *work)
 {
 	struct delayed_work *d = container_of(work, struct delayed_work, work);
 	struct sh_mmcif_host *host = container_of(d, struct sh_mmcif_host, timeout_work);
@@ -1481,7 +1481,7 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_clk;
 
-	INIT_DELAYED_WORK(&host->timeout_work, mmcif_timeout_work);
+	INIT_DELAYED_WORK(&host->timeout_work, sh_mmcif_timeout_work);
 
 	sh_mmcif_sync_reset(host);
 	sh_mmcif_writel(host->addr, MMCIF_CE_INT_MASK, MASK_ALL);
@@ -1587,7 +1587,7 @@ static struct platform_driver sh_mmcif_driver = {
 	.driver		= {
 		.name	= DRIVER_NAME,
 		.pm	= &sh_mmcif_dev_pm_ops,
-		.of_match_table = mmcif_of_match,
+		.of_match_table = sh_mmcif_of_match,
 	},
 };
 
-- 
1.9.1

[PATCH 3/5 v6] mmc: sh_mmcif: calculate best clock with parent clock

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

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 setups parent clock. This patch adds parent clock setup
method. It will be used if DT has "max-frequency", and then, this driver
assumes it is booted on R-Car Gen2 or later SoC. Because SH-Mobile series
(which doesn't boot from DT) and R-Car series (which boots from DT) have
different divider.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v5 -> v6

 - it uses max-frequency
 - don't add new compatible string

 .../devicetree/bindings/mmc/renesas,mmcif.txt      |  3 +
 drivers/mmc/host/sh_mmcif.c                        | 84 +++++++++++++++++++---
 2 files changed, 78 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
index 299081f..d38942f 100644
--- a/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
+++ b/Documentation/devicetree/bindings/mmc/renesas,mmcif.txt
@@ -18,6 +18,8 @@ Required properties:
   dma-names property.
 - dma-names: must contain "tx" for the transmit DMA channel and "rx" for the
   receive DMA channel.
+- max-frequency: Maximum operating clock frequency, driver uses default clock
+  frequency if it is not set.
 
 
 Example: R8A7790 (R-Car H2) MMCIF0
@@ -29,4 +31,5 @@ Example: R8A7790 (R-Car H2) MMCIF0
 		clocks = <&mstp3_clks R8A7790_CLK_MMCIF0>;
 		dmas = <&dmac0 0xd1>, <&dmac0 0xd2>;
 		dma-names = "tx", "rx";
+		max-frequency = <97500000>;
 	};
diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index e13c5f41..554fa9d 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,9 @@ enum sh_mmcif_wait_for {
 	MMCIF_WAIT_FOR_STOP,
 };
 
+/*
+ * difference for each SoC
+ */
 struct sh_mmcif_host {
 	struct mmc_host *mmc;
 	struct mmc_request *mrq;
@@ -248,6 +252,7 @@ struct sh_mmcif_host {
 	bool ccs_enable;		/* Command Completion Signal support */
 	bool clk_ctrl2_enable;
 	struct mutex thread_lock;
+	u32 clkdiv_map;         /* see CE_CLK_CTRL::CLKDIV */
 
 	/* DMA support */
 	struct dma_chan		*chan_rx;
@@ -492,19 +497,55 @@ static void sh_mmcif_clock_control(struct sh_mmcif_host *host, unsigned int clk)
 	struct sh_mmcif_plat_data *p = dev->platform_data;
 	bool sup_pclk = p ? p->sup_pclk : false;
 	unsigned int current_clk = clk_get_rate(host->clk);
+	unsigned int clkdiv;
 
 	sh_mmcif_bitclr(host, MMCIF_CE_CLK_CTRL, CLK_ENABLE);
 	sh_mmcif_bitclr(host, MMCIF_CE_CLK_CTRL, CLK_CLEAR);
 
 	if (!clk)
 		return;
-	if (sup_pclk && clk == current_clk)
-		sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_SUP_PCLK);
-	else
-		sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_CLEAR &
-				((fls(DIV_ROUND_UP(current_clk,
-						   clk) - 1) - 1) << 16));
 
+	if (host->clkdiv_map) {
+		unsigned int freq, best_freq, myclk, div, diff_min, diff;
+		int i;
+
+		clkdiv = 0;
+		diff_min = ~0;
+		best_freq = 0;
+		for (i = 31; i >= 0; i--) {
+			if (!((1 << i) & host->clkdiv_map))
+				continue;
+
+			/*
+			 * clk = parent_freq / div
+			 * -> parent_freq = clk x div
+			 */
+
+			div = 1 << (i + 1);
+			freq = clk_round_rate(host->clk, clk * div);
+			myclk = freq / div;
+			diff = (myclk > clk) ? myclk - clk : clk - myclk;
+
+			if (diff <= diff_min) {
+				best_freq = freq;
+				clkdiv = i;
+				diff_min = diff;
+			}
+		}
+
+		dev_dbg(dev, "clk %u/%u (%u, 0x%x)\n",
+			(best_freq / (1 << (clkdiv + 1))), clk,
+			best_freq, clkdiv);
+
+		clk_set_rate(host->clk, best_freq);
+		clkdiv = clkdiv << 16;
+	} else if (sup_pclk && clk == current_clk) {
+		clkdiv = CLK_SUP_PCLK;
+	} else {
+		clkdiv = (fls(DIV_ROUND_UP(current_clk, clk) - 1) - 1) << 16;
+	}
+
+	sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_CLEAR & clkdiv);
 	sh_mmcif_bitset(host, MMCIF_CE_CLK_CTRL, CLK_ENABLE);
 }
 
@@ -1000,10 +1041,35 @@ static void sh_mmcif_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 static void sh_mmcif_clk_setup(struct sh_mmcif_host *host)
 {
-	unsigned int clk = clk_get_rate(host->clk);
+	struct device *dev = sh_mmcif_host_to_dev(host);
+
+	if (host->mmc->f_max) {
+		unsigned int f_max, f_min, f_min_old;
+
+		f_max = host->mmc->f_max;
+		for (f_min_old = f_max; f_min_old > 2;) {
+			f_min = clk_round_rate(host->clk, f_min_old / 2);
+			if (f_min == f_min_old)
+				break;
+			f_min_old = f_min;
+		}
+
+		/*
+		 * This driver assumes this SoC is R-Car Gen2 or later
+		 */
+		host->clkdiv_map = 0x3ff;
+
+		host->mmc->f_max = f_max / (1 << ffs(host->clkdiv_map));
+		host->mmc->f_min = f_min / (1 << fls(host->clkdiv_map));
+	} else {
+		unsigned int clk = clk_get_rate(host->clk);
+
+		host->mmc->f_max = clk / 2;
+		host->mmc->f_min = clk / 512;
+	}
 
-	host->mmc->f_max = clk / 2;
-	host->mmc->f_min = clk / 512;
+	dev_dbg(dev, "clk max/min = %d/%d\n",
+		host->mmc->f_max, host->mmc->f_min);
 }
 
 static void sh_mmcif_set_power(struct sh_mmcif_host *host, struct mmc_ios *ios)
-- 
1.9.1

[PATCH 4/5 v6] ARM: shmobile: r8a7790: add MMCIF max-frequency

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Tested-by: Keita Kobayashi <keita.kobayashi.ym@renesas.com>
---
v5 -> v6

 - new patch

 arch/arm/boot/dts/r8a7790.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index c9055e3..044f0ef 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -468,6 +468,7 @@
 		dma-names = "tx", "rx";
 		reg-io-width = <4>;
 		status = "disabled";
+		max-frequency = <97500000>;
 	};
 
 	mmcif1: mmc@ee220000 {
@@ -479,6 +480,7 @@
 		dma-names = "tx", "rx";
 		reg-io-width = <4>;
 		status = "disabled";
+		max-frequency = <97500000>;
 	};
 
 	pfc: pfc@e6060000 {
-- 
1.9.1

[PATCH 5/5 v6] ARM: shmobile: r8a7791: add MMCIF max-frequency

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
v5 -> v6

 - no change

 arch/arm/boot/dts/r8a7791.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 30e1e13..fda7dc6 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -472,6 +472,7 @@
 		dma-names = "tx", "rx";
 		reg-io-width = <4>;
 		status = "disabled";
+		max-frequency = <97500000>;
 	};
 
 	sdhi0: sd@ee100000 {
-- 
1.9.1

Re: [PATCH 0/3 v6] mmc: sh_mmcif: add parent clk support

[Ulf Hansson]

On 14 May 2015 at 09:20, Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
> Hi Ulf, Simon
>
> These patches are v6 of sh_mmcif clock support.
>
> 1) - 2) : cleanup patches
> 3)      : v6 patch for clk
> 4) - 5) : DTS patch
>
> Simon
>
>  1) - 3) only : driver uses default clock
>  4) - 5) only : driver uses it, but it is updated to default clock
>  1) - 5) all  : it uses new method
>
> so, you can check 4) - 5) without thinking about 1) - 3).
>
> Kuninori Morimoto (5):
>       1) mmc: sh_mmcif: add sh_mmcif_host_to_dev() macro and use it.
>       2) mmc: sh_mmcif: use sh_mmcif_xxx prefix for all functions
>       3) mmc: sh_mmcif: calculate best clock with parent clock
>       4) ARM: shmobile: r8a7790: add MMCIF max-frequency
>       5) ARM: shmobile: r8a7791: add MMCIF max-frequency
>
>  Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |   3 ++
>  arch/arm/boot/dts/r8a7790.dtsi                          |   2 +
>  arch/arm/boot/dts/r8a7791.dtsi                          |   1 +
>  drivers/mmc/host/sh_mmcif.c                             | 220 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------
>  4 files changed, 161 insertions(+), 65 deletions(-)

Thanks, applied patch 1->3. I leave 4->5 to Simon.

Thanks!

Kind regards
Uffe

Re: [PATCH 0/3 v6] mmc: sh_mmcif: add parent clk support

[Kuninori Morimoto]

Hi Simon, Ulf

> > Kuninori Morimoto (5):
> >       1) mmc: sh_mmcif: add sh_mmcif_host_to_dev() macro and use it.
> >       2) mmc: sh_mmcif: use sh_mmcif_xxx prefix for all functions
> >       3) mmc: sh_mmcif: calculate best clock with parent clock
> >       4) ARM: shmobile: r8a7790: add MMCIF max-frequency
> >       5) ARM: shmobile: r8a7791: add MMCIF max-frequency
(snip)
> Thanks, applied patch 1->3. I leave 4->5 to Simon.

Thank you Ulf.
Simon, please check 4) 5)

Best regards
---
Kuninori Morimoto

Re: [PATCH 0/3 v6] mmc: sh_mmcif: add parent clk support

[Simon Horman]

Hi Morimoto-san,

On Mon, May 25, 2015 at 12:24:59AM +0000, Kuninori Morimoto wrote:
> 
> Hi Simon, Ulf
> 
> > > Kuninori Morimoto (5):
> > >       1) mmc: sh_mmcif: add sh_mmcif_host_to_dev() macro and use it.
> > >       2) mmc: sh_mmcif: use sh_mmcif_xxx prefix for all functions
> > >       3) mmc: sh_mmcif: calculate best clock with parent clock
> > >       4) ARM: shmobile: r8a7790: add MMCIF max-frequency
> > >       5) ARM: shmobile: r8a7791: add MMCIF max-frequency
> (snip)
> > Thanks, applied patch 1->3. I leave 4->5 to Simon.
> 
> Thank you Ulf.
> Simon, please check 4) 5)
> 
> Best regards
> ---
> Kuninori Morimoto
> 

On Mon, May 25, 2015 at 12:38:08AM +0000, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > > Thanks, applied patch 1->3. I leave 4->5 to Simon.
> > 
> > Thanks Ulf.
> > 
> > Morimoto-san, can I confirm that I should go ahead and queue up 4->5 ?
> 
> Thank you for your help.
> Yes, please. you don't need to care about 1) - 3) for 4) - 5) patches.
> 

Thanks, I have queued up 5 & 5 for v4.2.

Re: [PATCH 0/3 v6] mmc: sh_mmcif: add parent clk support

[Simon Horman]

On Fri, May 22, 2015 at 04:01:06PM +0200, Ulf Hansson wrote:
> On 14 May 2015 at 09:20, Kuninori Morimoto
> <kuninori.morimoto.gx@renesas.com> wrote:
> >
> > Hi Ulf, Simon
> >
> > These patches are v6 of sh_mmcif clock support.
> >
> > 1) - 2) : cleanup patches
> > 3)      : v6 patch for clk
> > 4) - 5) : DTS patch
> >
> > Simon
> >
> >  1) - 3) only : driver uses default clock
> >  4) - 5) only : driver uses it, but it is updated to default clock
> >  1) - 5) all  : it uses new method
> >
> > so, you can check 4) - 5) without thinking about 1) - 3).
> >
> > Kuninori Morimoto (5):
> >       1) mmc: sh_mmcif: add sh_mmcif_host_to_dev() macro and use it.
> >       2) mmc: sh_mmcif: use sh_mmcif_xxx prefix for all functions
> >       3) mmc: sh_mmcif: calculate best clock with parent clock
> >       4) ARM: shmobile: r8a7790: add MMCIF max-frequency
> >       5) ARM: shmobile: r8a7791: add MMCIF max-frequency
> >
> >  Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |   3 ++
> >  arch/arm/boot/dts/r8a7790.dtsi                          |   2 +
> >  arch/arm/boot/dts/r8a7791.dtsi                          |   1 +
> >  drivers/mmc/host/sh_mmcif.c                             | 220 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------
> >  4 files changed, 161 insertions(+), 65 deletions(-)
> 
> Thanks, applied patch 1->3. I leave 4->5 to Simon.

Thanks Ulf.

Morimoto-san, can I confirm that I should go ahead and queue up 4->5 ?

Re: [PATCH 0/3 v6] mmc: sh_mmcif: add parent clk support

[Kuninori Morimoto]

Hi Simon

> > Thanks, applied patch 1->3. I leave 4->5 to Simon.
> 
> Thanks Ulf.
> 
> Morimoto-san, can I confirm that I should go ahead and queue up 4->5 ?

Thank you for your help.
Yes, please. you don't need to care about 1) - 3) for 4) - 5) patches.

Re: [PATCH 0/3 v6] mmc: sh_mmcif: add parent clk support

[Ulf Hansson]

On 22 May 2015 at 16:01, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 14 May 2015 at 09:20, Kuninori Morimoto
> <kuninori.morimoto.gx@renesas.com> wrote:
>>
>> Hi Ulf, Simon
>>
>> These patches are v6 of sh_mmcif clock support.
>>
>> 1) - 2) : cleanup patches
>> 3)      : v6 patch for clk
>> 4) - 5) : DTS patch
>>
>> Simon
>>
>>  1) - 3) only : driver uses default clock
>>  4) - 5) only : driver uses it, but it is updated to default clock
>>  1) - 5) all  : it uses new method
>>
>> so, you can check 4) - 5) without thinking about 1) - 3).
>>
>> Kuninori Morimoto (5):
>>       1) mmc: sh_mmcif: add sh_mmcif_host_to_dev() macro and use it.
>>       2) mmc: sh_mmcif: use sh_mmcif_xxx prefix for all functions
>>       3) mmc: sh_mmcif: calculate best clock with parent clock
>>       4) ARM: shmobile: r8a7790: add MMCIF max-frequency
>>       5) ARM: shmobile: r8a7791: add MMCIF max-frequency
>>
>>  Documentation/devicetree/bindings/mmc/renesas,mmcif.txt |   3 ++
>>  arch/arm/boot/dts/r8a7790.dtsi                          |   2 +
>>  arch/arm/boot/dts/r8a7791.dtsi                          |   1 +
>>  drivers/mmc/host/sh_mmcif.c                             | 220 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------------
>>  4 files changed, 161 insertions(+), 65 deletions(-)
>
> Thanks, applied patch 1->3. I leave 4->5 to Simon.

I realized the patch 3 caused a compiler warning. I have fixed it up,
so no worries!

Kind regards
Uffe

Re: [PATCH 0/3 v6] mmc: sh_mmcif: add parent clk support

[Kuninori Morimoto]

Hi Ulf

> >> Kuninori Morimoto (5):
> >>       1) mmc: sh_mmcif: add sh_mmcif_host_to_dev() macro and use it.
> >>       2) mmc: sh_mmcif: use sh_mmcif_xxx prefix for all functions
> >>       3) mmc: sh_mmcif: calculate best clock with parent clock
> >>       4) ARM: shmobile: r8a7790: add MMCIF max-frequency
> >>       5) ARM: shmobile: r8a7791: add MMCIF max-frequency
(snip)
> I realized the patch 3 caused a compiler warning. I have fixed it up,
> so no worries!

Oops. Thank you for your help