[PATCH/RFC] mmc: sh_mmcif: Increase MMCIF clock rate to 97.5MHz

[PATCH/RFC] mmc: sh_mmcif: Increase MMCIF clock rate to 97.5MHz

[Yoshihiro Kaneko]

From: Shinobu Uehara <shinobu.uehara.xc@renesas.com>

Signed-off-by: Shinobu Uehara <shinobu.uehara.xc@renesas.com>
Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
---

This patch is based on mmc-next branch of Chris Ball's mmc tree.

 drivers/mmc/host/sh_mmcif.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
index 656fbba..fe9a8b7 100644
--- a/drivers/mmc/host/sh_mmcif.c
+++ b/drivers/mmc/host/sh_mmcif.c
@@ -1,6 +1,7 @@
 /*
  * MMCIF eMMC driver.
  *
+ * Copyright (C) 2014 Renesas Electronics Corporation
  * Copyright (C) 2010 Renesas Solutions Corp.
  * Yusuke Goda <yusuke.goda.sx@renesas.com>
  *
@@ -57,6 +58,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>
@@ -1371,6 +1373,8 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 	struct resource *res;
 	void __iomem *reg;
 	const char *name;
+	struct device_node *np = pdev->dev.of_node;
+	int clk_rate;
 
 	irq[0] = platform_get_irq(pdev, 0);
 	irq[1] = platform_get_irq(pdev, 1);
@@ -1433,6 +1437,16 @@ static int sh_mmcif_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
 		goto eclkget;
 	}
+
+	if (np && !of_property_read_u32(np, "renesas,clk-rate", &clk_rate)) {
+		if (clk_rate) {
+			ret = clk_set_rate(host->hclk, clk_rate);
+			if (ret < 0)
+				dev_err(&pdev->dev,
+					"cannot set clock rate: %d\n", ret);
+		}
+	}
+
 	ret = sh_mmcif_clk_update(host);
 	if (ret < 0)
 		goto eclkupdate;
-- 
1.9.1

Re: [PATCH/RFC] mmc: sh_mmcif: Increase MMCIF clock rate to 97.5MHz

[Ulf Hansson]

On 11 November 2014 07:42, Yoshihiro Kaneko <ykaneko0929@gmail.com> wrote:
> From: Shinobu Uehara <shinobu.uehara.xc@renesas.com>

Some more info in the commit message please.

>
> Signed-off-by: Shinobu Uehara <shinobu.uehara.xc@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> ---
>
> This patch is based on mmc-next branch of Chris Ball's mmc tree.

Please use:

git.linaro.org/people/ulf.hansson/mmc.git

>
>  drivers/mmc/host/sh_mmcif.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index 656fbba..fe9a8b7 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
> @@ -1,6 +1,7 @@
>  /*
>   * MMCIF eMMC driver.
>   *
> + * Copyright (C) 2014 Renesas Electronics Corporation
>   * Copyright (C) 2010 Renesas Solutions Corp.
>   * Yusuke Goda <yusuke.goda.sx@renesas.com>
>   *
> @@ -57,6 +58,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>
> @@ -1371,6 +1373,8 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>         struct resource *res;
>         void __iomem *reg;
>         const char *name;
> +       struct device_node *np = pdev->dev.of_node;
> +       int clk_rate;
>
>         irq[0] = platform_get_irq(pdev, 0);
>         irq[1] = platform_get_irq(pdev, 1);
> @@ -1433,6 +1437,16 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>                 dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
>                 goto eclkget;
>         }
> +
> +       if (np && !of_property_read_u32(np, "renesas,clk-rate", &clk_rate)) {

Why do you need a DT binding for this?

> +               if (clk_rate) {
> +                       ret = clk_set_rate(host->hclk, clk_rate);
> +                       if (ret < 0)
> +                               dev_err(&pdev->dev,
> +                                       "cannot set clock rate: %d\n", ret);
> +               }
> +       }
> +
>         ret = sh_mmcif_clk_update(host);
>         if (ret < 0)
>                 goto eclkupdate;
> --
> 1.9.1
>

Kind regards
Uffe

Re: [PATCH/RFC] mmc: sh_mmcif: Increase MMCIF clock rate to 97.5MHz

[Sergei Shtylyov]

Hello.

On 11/11/2014 9:42 AM, Yoshihiro Kaneko wrote:

> From: Shinobu Uehara <shinobu.uehara.xc@renesas.com>

> Signed-off-by: Shinobu Uehara <shinobu.uehara.xc@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> ---

> This patch is based on mmc-next branch of Chris Ball's mmc tree.

>   drivers/mmc/host/sh_mmcif.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)

> diff --git a/drivers/mmc/host/sh_mmcif.c b/drivers/mmc/host/sh_mmcif.c
> index 656fbba..fe9a8b7 100644
> --- a/drivers/mmc/host/sh_mmcif.c
> +++ b/drivers/mmc/host/sh_mmcif.c
[...]
> @@ -1371,6 +1373,8 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>   	struct resource *res;
>   	void __iomem *reg;
>   	const char *name;
> +	struct device_node *np = pdev->dev.of_node;
> +	int clk_rate;

    'u32'.

>
>   	irq[0] = platform_get_irq(pdev, 0);
>   	irq[1] = platform_get_irq(pdev, 1);
> @@ -1433,6 +1437,16 @@ static int sh_mmcif_probe(struct platform_device *pdev)
>   		dev_err(&pdev->dev, "cannot get clock: %d\n", ret);
>   		goto eclkget;
>   	}
> +
> +	if (np && !of_property_read_u32(np, "renesas,clk-rate", &clk_rate)) {

    Doesn't the standard "clock-frequency" prop fit here?

WBR, Sergei

Re: [PATCH/RFC] mmc: sh_mmcif: Increase MMCIF clock rate to 97.5MHz

[Kuninori Morimoto]

Hi Kaneko-san

> From: Shinobu Uehara <shinobu.uehara.xc@renesas.com>
> 
> Signed-off-by: Shinobu Uehara <shinobu.uehara.xc@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> ---
(snip)
> +	if (np && !of_property_read_u32(np, "renesas,clk-rate", &clk_rate)) {
> +		if (clk_rate) {
> +			ret = clk_set_rate(host->hclk, clk_rate);
> +			if (ret < 0)
> +				dev_err(&pdev->dev,
> +					"cannot set clock rate: %d\n", ret);
> +		}
> +	}

I guess this is came from BSP, and you have similar
bindings on sh_mobile_sdhi.c too.
But, we need to use "standard" method, not BSP original

Best regards
---
Kuninori Morimoto

Re: [PATCH/RFC] mmc: sh_mmcif: Increase MMCIF clock rate to 97.5MHz

[Simon Horman]

On Thu, Nov 13, 2014 at 12:15:58AM +0000, Kuninori Morimoto wrote:
> 
> Hi Kaneko-san
> 
> > From: Shinobu Uehara <shinobu.uehara.xc@renesas.com>
> > 
> > Signed-off-by: Shinobu Uehara <shinobu.uehara.xc@renesas.com>
> > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> > ---
> (snip)
> > +	if (np && !of_property_read_u32(np, "renesas,clk-rate", &clk_rate)) {
> > +		if (clk_rate) {
> > +			ret = clk_set_rate(host->hclk, clk_rate);
> > +			if (ret < 0)
> > +				dev_err(&pdev->dev,
> > +					"cannot set clock rate: %d\n", ret);
> > +		}
> > +	}
> 
> I guess this is came from BSP, and you have similar
> bindings on sh_mobile_sdhi.c too.
> But, we need to use "standard" method, not BSP original

Hi Morimoto-san,

you are correct in assuming that this patch came from the BSP.

Is your opinion that a) changing the clock rate is good but
b) it needs to be done using a standard method?

If so, is the main change required to use a standard binding string
rather than "renesas,clk-rate"? Is the driver change above needed at all?

Re: [PATCH/RFC] mmc: sh_mmcif: Increase MMCIF clock rate to 97.5MHz

[Kuninori Morimoto]

Hi Simon

> you are correct in assuming that this patch came from the BSP.
> 
> Is your opinion that a) changing the clock rate is good but
> b) it needs to be done using a standard method?
> 
> If so, is the main change required to use a standard binding string
> rather than "renesas,clk-rate"? Is the driver change above needed at all?

I'm not 100% understand, but, maybe these settings are required for
MMC/SHDI's good performance.
I guess we can use CCF method for it ?

Re: [PATCH/RFC] mmc: sh_mmcif: Increase MMCIF clock rate to 97.5MHz

[Simon Horman]

On Thu, Nov 13, 2014 at 01:34:13AM +0000, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > you are correct in assuming that this patch came from the BSP.
> > 
> > Is your opinion that a) changing the clock rate is good but
> > b) it needs to be done using a standard method?
> > 
> > If so, is the main change required to use a standard binding string
> > rather than "renesas,clk-rate"? Is the driver change above needed at all?
> 
> I'm not 100% understand, but, maybe these settings are required for
> MMC/SHDI's good performance.
> I guess we can use CCF method for it ?

That does sound like an idea worth investigating.
I wonder what the best way is to move this forwards.