diff mbox

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

Message ID 1415688142-20023-1-git-send-email-ykaneko0929@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Geert Uytterhoeven
Headers show

Commit Message

Yoshihiro Kaneko Nov. 11, 2014, 6:42 a.m. UTC
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(+)

Comments

Ulf Hansson Nov. 12, 2014, 8:37 a.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov Nov. 12, 2014, 1:40 p.m. UTC | #2
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

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kuninori Morimoto Nov. 13, 2014, 12:15 a.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Horman Nov. 13, 2014, 12:24 a.m. UTC | #4
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?
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kuninori Morimoto Nov. 13, 2014, 1:34 a.m. UTC | #5
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 ?

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Horman Nov. 21, 2014, 4:09 a.m. UTC | #6
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;