[PATCH] mmc: dw_mmc: add fixed divider for ciu_clk on SoCFPGA

[PATCH] mmc: dw_mmc: add fixed divider for ciu_clk on SoCFPGA

[dinguyen]

From: Dinh Nguyen <dinguyen@opensource.altera.com>

The ciu_clk(Card Interface Unit Clock) on the SoCFPGA platform has a fixed
divider of 4. Add the fixed clock divide code in the platform's clock
setup code.

Technically, this code could be be moved to dw_mmc-socfpga, like the Rockchip
and Exynos, but I don't think it's necessary for a small change that is
needed for the entire SoCFPGA family of SoCs.

Also, "altr,dw-mshc-ciu-div" can be introduced to respresent the ciu divider,
but since this divider is common across all SoCFPGA, I don't think its
necessary.

Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
CC: Seungwon Jeon <tgih.jun@samsung.com>
CC: Jaehoon Chung <jh80.chung@samsung.com>
CC: Chris Ball <chris@printf.net>
CC: Ulf Hansson <ulf.hansson@linaro.org>
CC: Doug Anderson <dianders@chromium.org>
---
 drivers/mmc/host/dw_mmc-pltfm.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
index ec6dbcd..c92f790 100644
--- a/drivers/mmc/host/dw_mmc-pltfm.c
+++ b/drivers/mmc/host/dw_mmc-pltfm.c
@@ -26,13 +26,22 @@
 #include "dw_mmc.h"
 #include "dw_mmc-pltfm.h"
 
+#define SOCFPGA_CIU_CLK_DIV	4
+
 static void dw_mci_pltfm_prepare_command(struct dw_mci *host, u32 *cmdr)
 {
 	*cmdr |= SDMMC_CMD_USE_HOLD_REG;
 }
 
+static int dw_mci_socfpga_setup_clock(struct dw_mci *host)
+{
+	host->bus_hz /= SOCFPGA_CIU_CLK_DIV;
+	return 0;
+}
+
 static const struct dw_mci_drv_data socfpga_drv_data = {
 	.prepare_command	= dw_mci_pltfm_prepare_command,
+	.setup_clock		= dw_mci_socfpga_setup_clock,
 };
 
 static const struct dw_mci_drv_data pistachio_drv_data = {
-- 
2.2.1

Re: [PATCH] mmc: dw_mmc: add fixed divider for ciu_clk on SoCFPGA

[Doug Anderson]

Dinh,

On Fri, Apr 10, 2015 at 6:56 AM,  <dinguyen@opensource.altera.com> wrote:
> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>
> The ciu_clk(Card Interface Unit Clock) on the SoCFPGA platform has a fixed
> divider of 4. Add the fixed clock divide code in the platform's clock
> setup code.

It might actually be better to do this a different way for SoCFPGA.  I
sorta wish we had done it differently for Rockchip as well, but at
this point you end up with the complexity of changing device tree
bindings in conjunction with code and it gets ugly.

Specifically, you've probably got the following clocks:

SD_prediv = 400MHz
-> SD postdiv = 100MHz
-> SD sample = 100MHz, shifted
-> SD drive = 100MHz, shifted

Right now you're specifying "SD_prediv" as the SD card clock.  If you
instead expose "SD postdiv" as a new clock (from your clock driver)
that is "SD prediv" divided by 4 then you'll magically get all the
behavior that you want with no modifications to dw_mmc.  Just make
sure that "SD postdiv" passes on rate changes to its parent (that's
just a flag in the common clock framework).


At some point in time you'll also want to expose the sample and drive
clocks once you get UHS modes working.  Alexandru posted some patches
for this a while ago to support tuning in dw_mmc using just drive and
sample clocks, but the patch still needed some more work.  Either he
or I will probably pick it up again soon.

-Doug

Re: [PATCH] mmc: dw_mmc: add fixed divider for ciu_clk on SoCFPGA

[Dinh Nguyen]

On 4/10/15 10:15 AM, Doug Anderson wrote:
> Dinh,
> 
> On Fri, Apr 10, 2015 at 6:56 AM,  <dinguyen@opensource.altera.com> wrote:
>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>
>> The ciu_clk(Card Interface Unit Clock) on the SoCFPGA platform has a fixed
>> divider of 4. Add the fixed clock divide code in the platform's clock
>> setup code.
> 
> It might actually be better to do this a different way for SoCFPGA.  I
> sorta wish we had done it differently for Rockchip as well, but at
> this point you end up with the complexity of changing device tree
> bindings in conjunction with code and it gets ugly.

Yes, I started going down this path and realized that.

> 
> Specifically, you've probably got the following clocks:
> 
> SD_prediv = 400MHz
> -> SD postdiv = 100MHz
> -> SD sample = 100MHz, shifted
> -> SD drive = 100MHz, shifted
> 
> Right now you're specifying "SD_prediv" as the SD card clock.  If you
> instead expose "SD postdiv" as a new clock (from your clock driver)
> that is "SD prediv" divided by 4 then you'll magically get all the
> behavior that you want with no modifications to dw_mmc.  Just make
> sure that "SD postdiv" passes on rate changes to its parent (that's
> just a flag in the common clock framework).
> 

That's a great idea, thanks for pointing that out.

> 
> At some point in time you'll also want to expose the sample and drive
> clocks once you get UHS modes working.  Alexandru posted some patches
> for this a while ago to support tuning in dw_mmc using just drive and
> sample clocks, but the patch still needed some more work.  Either he
> or I will probably pick it up again soon.

I think I have already done this by representing the sdmmc_clk with a
"clk-phase" property for this clock.

Dinh

Re: [PATCH] mmc: dw_mmc: add fixed divider for ciu_clk on SoCFPGA

[Doug Anderson]

Dinh,

On Fri, Apr 10, 2015 at 11:08 AM, Dinh Nguyen <dinh.linux@gmail.com> wrote:
>
>
> On 4/10/15 10:15 AM, Doug Anderson wrote:
>> Dinh,
>>
>> On Fri, Apr 10, 2015 at 6:56 AM,  <dinguyen@opensource.altera.com> wrote:
>>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>>
>>> The ciu_clk(Card Interface Unit Clock) on the SoCFPGA platform has a fixed
>>> divider of 4. Add the fixed clock divide code in the platform's clock
>>> setup code.
>>
>> It might actually be better to do this a different way for SoCFPGA.  I
>> sorta wish we had done it differently for Rockchip as well, but at
>> this point you end up with the complexity of changing device tree
>> bindings in conjunction with code and it gets ugly.
>
> Yes, I started going down this path and realized that.
>
>>
>> Specifically, you've probably got the following clocks:
>>
>> SD_prediv = 400MHz
>> -> SD postdiv = 100MHz
>> -> SD sample = 100MHz, shifted
>> -> SD drive = 100MHz, shifted
>>
>> Right now you're specifying "SD_prediv" as the SD card clock.  If you
>> instead expose "SD postdiv" as a new clock (from your clock driver)
>> that is "SD prediv" divided by 4 then you'll magically get all the
>> behavior that you want with no modifications to dw_mmc.  Just make
>> sure that "SD postdiv" passes on rate changes to its parent (that's
>> just a flag in the common clock framework).
>>
>
> That's a great idea, thanks for pointing that out.
>
>>
>> At some point in time you'll also want to expose the sample and drive
>> clocks once you get UHS modes working.  Alexandru posted some patches
>> for this a while ago to support tuning in dw_mmc using just drive and
>> sample clocks, but the patch still needed some more work.  Either he
>> or I will probably pick it up again soon.
>
> I think I have already done this by representing the sdmmc_clk with a
> "clk-phase" property for this clock.

I think there needs to be a separate sample and drive clock at least,
since they can have different phases, right?  I haven't looked, but if
that's what you have then you're all set!  :)  Above I was suggesting
a 3rd clock instead of arbitrarily using either the sample/drive clock
as the main clock for dw_mmc.

-Doug