[PATCH] clk: exynos5250: Fix divider values for sclk_mmc{0,1,2,3}

[PATCH] clk: exynos5250: Fix divider values for sclk_mmc{0,1,2,3}

[Tushar Behera]

In legacy setup, sclk_mmc{0,1,2,3} used PRE_RATIO bit-field (8-bit wide)
instead of RATIO bit-field (4-bit wide) for dividing clock rate.

With current common clock setup, we are using RATIO bit-field which
is creating FIFO read errors while accessing eMMC. Changing over to
use PRE_RATIO bit-field fixes this issue.

dwmmc_exynos 12200000.dwmmc0: data FIFO error (status=00008020)
mmcblk0: error -5 transferring data, sector 1, nr 7, cmd response 0x900, card status 0x0
end_request: I/O error, dev mmcblk0, sector 1

Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
CC: Thomas Abraham <thomas.abraham@linaro.org>
---

Based on Kukjin's for-next branch.
commit d58f6a153f40 ("Merge branch 'next/clk-exynos-2' into for-next")

 drivers/clk/samsung/clk-exynos5250.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index 1152125..2c46fbd 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -274,10 +274,10 @@ struct samsung_div_clock exynos5250_div_clks[] __initdata = {
 	DIV(none, "div_pcm0", "sclk_audio0", DIV_MAU, 4, 8),
 	DIV(none, "div_sata", "mout_sata", DIV_FSYS0, 20, 4),
 	DIV(none, "div_usb3", "mout_usb3", DIV_FSYS0, 24, 4),
-	DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 0, 4),
-	DIV(none, "div_mmc1", "mout_mmc1", DIV_FSYS1, 16, 4),
-	DIV(none, "div_mmc2", "mout_mmc2", DIV_FSYS2, 0, 4),
-	DIV(none, "div_mmc3", "mout_mmc3", DIV_FSYS2, 16, 4),
+	DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 8, 8),
+	DIV(none, "div_mmc1", "mout_mmc1", DIV_FSYS1, 24, 8),
+	DIV(none, "div_mmc2", "mout_mmc2", DIV_FSYS2, 8, 8),
+	DIV(none, "div_mmc3", "mout_mmc3", DIV_FSYS2, 24, 8),
 	DIV(none, "div_uart0", "mout_uart0", DIV_PERIC0, 0, 4),
 	DIV(none, "div_uart1", "mout_uart1", DIV_PERIC0, 4, 4),
 	DIV(none, "div_uart2", "mout_uart2", DIV_PERIC0, 8, 4),
-- 
1.7.9.5

Re: [PATCH] clk: exynos5250: Fix divider values for sclk_mmc{0,1,2,3}

[Mike Turquette]

Quoting Tushar Behera (2013-04-02 01:20:40)
> In legacy setup, sclk_mmc{0,1,2,3} used PRE_RATIO bit-field (8-bit wide)
> instead of RATIO bit-field (4-bit wide) for dividing clock rate.
> 
> With current common clock setup, we are using RATIO bit-field which
> is creating FIFO read errors while accessing eMMC. Changing over to
> use PRE_RATIO bit-field fixes this issue.
> 
> dwmmc_exynos 12200000.dwmmc0: data FIFO error (status=00008020)
> mmcblk0: error -5 transferring data, sector 1, nr 7, cmd response 0x900, card status 0x0
> end_request: I/O error, dev mmcblk0, sector 1
> 
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> CC: Thomas Abraham <thomas.abraham@linaro.org>

I guess this will be applied through the samsung tree, so:

Acked-by: Mike Turquette <mturquette@linaro.org>

> ---
> 
> Based on Kukjin's for-next branch.
> commit d58f6a153f40 ("Merge branch 'next/clk-exynos-2' into for-next")
> 
>  drivers/clk/samsung/clk-exynos5250.c |    8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
> index 1152125..2c46fbd 100644
> --- a/drivers/clk/samsung/clk-exynos5250.c
> +++ b/drivers/clk/samsung/clk-exynos5250.c
> @@ -274,10 +274,10 @@ struct samsung_div_clock exynos5250_div_clks[] __initdata = {
>         DIV(none, "div_pcm0", "sclk_audio0", DIV_MAU, 4, 8),
>         DIV(none, "div_sata", "mout_sata", DIV_FSYS0, 20, 4),
>         DIV(none, "div_usb3", "mout_usb3", DIV_FSYS0, 24, 4),
> -       DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 0, 4),
> -       DIV(none, "div_mmc1", "mout_mmc1", DIV_FSYS1, 16, 4),
> -       DIV(none, "div_mmc2", "mout_mmc2", DIV_FSYS2, 0, 4),
> -       DIV(none, "div_mmc3", "mout_mmc3", DIV_FSYS2, 16, 4),
> +       DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 8, 8),
> +       DIV(none, "div_mmc1", "mout_mmc1", DIV_FSYS1, 24, 8),
> +       DIV(none, "div_mmc2", "mout_mmc2", DIV_FSYS2, 8, 8),
> +       DIV(none, "div_mmc3", "mout_mmc3", DIV_FSYS2, 24, 8),
>         DIV(none, "div_uart0", "mout_uart0", DIV_PERIC0, 0, 4),
>         DIV(none, "div_uart1", "mout_uart1", DIV_PERIC0, 4, 4),
>         DIV(none, "div_uart2", "mout_uart2", DIV_PERIC0, 8, 4),
> -- 
> 1.7.9.5

RE: [PATCH] clk: exynos5250: Fix divider values for sclk_mmc{0,1,2,3}

[Kukjin Kim]

Mike Turquette wrote:
> 
> Quoting Tushar Behera (2013-04-02 01:20:40)
> > In legacy setup, sclk_mmc{0,1,2,3} used PRE_RATIO bit-field (8-bit wide)
> > instead of RATIO bit-field (4-bit wide) for dividing clock rate.
> >
> > With current common clock setup, we are using RATIO bit-field which
> > is creating FIFO read errors while accessing eMMC. Changing over to
> > use PRE_RATIO bit-field fixes this issue.
> >
> > dwmmc_exynos 12200000.dwmmc0: data FIFO error (status=00008020)
> > mmcblk0: error -5 transferring data, sector 1, nr 7, cmd response 0x900,
> card status 0x0
> > end_request: I/O error, dev mmcblk0, sector 1
> >
> > Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> > CC: Thomas Abraham <thomas.abraham@linaro.org>
> 
> I guess this will be applied through the samsung tree, so:
> 
> Acked-by: Mike Turquette <mturquette@linaro.org>
> 
Thanks, applied.

- Kukjin

Re: [PATCH] clk: exynos5250: Fix divider values for sclk_mmc{0,1,2,3}

[Doug Anderson]

Hi,

On Mon, Apr 8, 2013 at 12:22 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> Mike Turquette wrote:
>>
>> Quoting Tushar Behera (2013-04-02 01:20:40)
>> > In legacy setup, sclk_mmc{0,1,2,3} used PRE_RATIO bit-field (8-bit wide)
>> > instead of RATIO bit-field (4-bit wide) for dividing clock rate.
>> >
>> > With current common clock setup, we are using RATIO bit-field which
>> > is creating FIFO read errors while accessing eMMC. Changing over to
>> > use PRE_RATIO bit-field fixes this issue.
>> >
>> > dwmmc_exynos 12200000.dwmmc0: data FIFO error (status=00008020)
>> > mmcblk0: error -5 transferring data, sector 1, nr 7, cmd response 0x900,
>> card status 0x0
>> > end_request: I/O error, dev mmcblk0, sector 1
>> >
>> > Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>> > CC: Thomas Abraham <thomas.abraham@linaro.org>
>>
>> I guess this will be applied through the samsung tree, so:
>>
>> Acked-by: Mike Turquette <mturquette@linaro.org>
>>
> Thanks, applied.

I haven't yet had time to dig / track down why, but this patch totally
messes up access to the eMMC on the ARM Chromebook (exynos5250-snow).
I suddenly start getting FIFO errors like you show above.  When I
revert your change then I'm all happy.

Perhaps I need a device tree setting change as well?  I always forget
how the "samsung,dw-mshc-ciu-div" / "samsung,dw-mshc-sdr-timing"
properties work...

For the short term I'm going to revert locally since I've got a few
other things to do over the next few days.  If nobody else gets around
to it then I'll try to find time to dig further.

---

Log messages at boot before your change applied:

localhost ~ # dmesg | grep mmc[a-z]*0
[    1.460000] dwmmc_exynos 12200000.dwmmc0: Using internal DMA controller.
[    1.465000] dwmmc_exynos 12200000.dwmmc0: Version ID is 241a
[    1.475000] dwmmc_exynos 12200000.dwmmc0: DW MMC controller at irq
107, 32 bit host data width, 128 deep fifo
[    1.485000] mmc0: no vmmc regulator found
[    1.510000] mmc_host mmc0: Bus speed (slot 0) = 100000000Hz (slot
req 400000Hz, actual 400000HZ div = 125)
[    1.530000] dwmmc_exynos 12200000.dwmmc0: 1 slots initialized
[    1.750000] mmc0: BKOPS_EN bit is not set
[    1.750000] mmc_host mmc0: Bus speed (slot 0) = 100000000Hz (slot
req 52000000Hz, actual 50000000HZ div = 1)
[    1.755000] mmc0: new high speed DDR MMC card at address 0001
[    1.755000] mmcblk0: mmc0:0001 SEM16G 14.6 GiB
[    1.780000] mmcblk0boot0: mmc0:0001 SEM16G partition 1 2.00 MiB
[    1.785000] mmcblk0boot1: mmc0:0001 SEM16G partition 2 2.00 MiB
[    1.790000] mmcblk0rpmb: mmc0:0001 SEM16G partition 3 128 KiB
[    1.800000]  mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10 p11 p12
[    1.825000]  mmcblk0boot1: unknown partition table
[    1.835000]  mmcblk0boot0: unknown partition table


Log messages at boot after your change (note that the bus speed is
reported as different which is what lead me to your change):

localhost ~ # dmesg | grep mmc[a-z]*0
[    1.440000] dwmmc_exynos 12200000.dwmmc0: Using internal DMA controller.
[    1.445000] dwmmc_exynos 12200000.dwmmc0: Version ID is 241a
[    1.455000] dwmmc_exynos 12200000.dwmmc0: DW MMC controller at irq
107, 32 bit host data width, 128 deep fifo
[    1.465000] mmc0: no vmmc regulator found
[    1.490000] mmc_host mmc0: Bus speed (slot 0) = 200000000Hz (slot
req 400000Hz, actual 400000HZ div = 250)
[    1.510000] dwmmc_exynos 12200000.dwmmc0: 1 slots initialized
[    1.760000] mmc0: BKOPS_EN bit is not set
[    1.770000] mmc_host mmc0: Bus speed (slot 0) = 200000000Hz (slot
req 52000000Hz, actual 50000000HZ div = 2)
[    1.770000] mmc0: new high speed DDR MMC card at address 0001
[    1.785000] mmcblk0: mmc0:0001 SEM16G 14.6 GiB
[    1.855000] mmcblk0boot0: mmc0:0001 SEM16G partition 1 2.00 MiB
[    1.860000] mmcblk0boot1: mmc0:0001 SEM16G partition 2 2.00 MiB
[    1.950000] mmcblk0rpmb: mmc0:0001 SEM16G partition 3 128 KiB
[    1.955000] mmcblk0: error -84 transferring data, sector 0, nr 8,
cmd response 0x900, card status 0xb00
[    1.965000] mmcblk0: retrying using single block read
[    1.970000] mmcblk0: error -84 transferring data, sector 0, nr 8,
cmd response 0x900, card status 0x0
[    1.980000] end_request: I/O error, dev mmcblk0, sector 0
[    1.985000] mmcblk0: error -84 transferring data, sector 1, nr 7,
cmd response 0x900, card status 0x0
[    1.995000] end_request: I/O error, dev mmcblk0, sector 1
[    2.000000] mmcblk0: error -84 transferring data, sector 2, nr 6,
cmd response 0x900, card status 0x0
[    2.010000] end_request: I/O error, dev mmcblk0, sector 2
[    2.015000] mmcblk0: error -84 transferring data, sector 3, nr 5,
cmd response 0x900, card status 0x0
[    2.025000] end_request: I/O error, dev mmcblk0, sector 3
[    2.030000] mmcblk0: error -84 transferring data, sector 4, nr 4,
cmd response 0x900, card status 0x0
[    2.040000] end_request: I/O error, dev mmcblk0, sector 4
[    2.045000] dwmmc_exynos 12200000.dwmmc0: data FIFO error (status=00008008)
[    2.050000] mmcblk0: error -5 transferring data, sector 5, nr 3,
cmd response 0x900, card status 0x0
[    2.060000] end_request: I/O error, dev mmcblk0, sector 5
...
...
...
...

-Doug

Re: [PATCH] clk: exynos5250: Fix divider values for sclk_mmc{0,1,2,3}

[Olof Johansson]

Hi,

On Tue, Apr 16, 2013 at 12:35 PM, Doug Anderson <dianders@chromium.org> wrote:
> Hi,
>
> On Mon, Apr 8, 2013 at 12:22 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:
>> Mike Turquette wrote:
>>>
>>> Quoting Tushar Behera (2013-04-02 01:20:40)
>>> > In legacy setup, sclk_mmc{0,1,2,3} used PRE_RATIO bit-field (8-bit wide)
>>> > instead of RATIO bit-field (4-bit wide) for dividing clock rate.
>>> >
>>> > With current common clock setup, we are using RATIO bit-field which
>>> > is creating FIFO read errors while accessing eMMC. Changing over to
>>> > use PRE_RATIO bit-field fixes this issue.
>>> >
>>> > dwmmc_exynos 12200000.dwmmc0: data FIFO error (status=00008020)
>>> > mmcblk0: error -5 transferring data, sector 1, nr 7, cmd response 0x900,
>>> card status 0x0
>>> > end_request: I/O error, dev mmcblk0, sector 1
>>> >
>>> > Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>>> > CC: Thomas Abraham <thomas.abraham@linaro.org>
>>>
>>> I guess this will be applied through the samsung tree, so:
>>>
>>> Acked-by: Mike Turquette <mturquette@linaro.org>
>>>
>> Thanks, applied.
>
> I haven't yet had time to dig / track down why, but this patch totally
> messes up access to the eMMC on the ARM Chromebook (exynos5250-snow).
> I suddenly start getting FIFO errors like you show above.  When I
> revert your change then I'm all happy.
>
> Perhaps I need a device tree setting change as well?  I always forget
> how the "samsung,dw-mshc-ciu-div" / "samsung,dw-mshc-sdr-timing"
> properties work...
>
> For the short term I'm going to revert locally since I've got a few
> other things to do over the next few days.  If nobody else gets around
> to it then I'll try to find time to dig further.

Unless I hear differently within 24 hours, I am going to revert this
in arm-soc (since that is where it is merged right now).

It is obviously causing regressions on existing platforms. I am _NOT_
happy to see dead silence about this for 6 days. Tushar??



-Olof

Re: [PATCH] clk: exynos5250: Fix divider values for sclk_mmc{0,1,2,3}

[Tushar Behera]

On 04/22/2013 11:10 PM, Olof Johansson wrote:
> Hi,
> 
> On Tue, Apr 16, 2013 at 12:35 PM, Doug Anderson <dianders@chromium.org> wrote:
>> Hi,
>>
>> On Mon, Apr 8, 2013 at 12:22 AM, Kukjin Kim <kgene.kim@samsung.com> wrote:
>>> Mike Turquette wrote:
>>>>
>>>> Quoting Tushar Behera (2013-04-02 01:20:40)
>>>>> In legacy setup, sclk_mmc{0,1,2,3} used PRE_RATIO bit-field (8-bit wide)
>>>>> instead of RATIO bit-field (4-bit wide) for dividing clock rate.
>>>>>
>>>>> With current common clock setup, we are using RATIO bit-field which
>>>>> is creating FIFO read errors while accessing eMMC. Changing over to
>>>>> use PRE_RATIO bit-field fixes this issue.
>>>>>
>>>>> dwmmc_exynos 12200000.dwmmc0: data FIFO error (status=00008020)
>>>>> mmcblk0: error -5 transferring data, sector 1, nr 7, cmd response 0x900,
>>>> card status 0x0
>>>>> end_request: I/O error, dev mmcblk0, sector 1
>>>>>
>>>>> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
>>>>> CC: Thomas Abraham <thomas.abraham@linaro.org>
>>>>
>>>> I guess this will be applied through the samsung tree, so:
>>>>
>>>> Acked-by: Mike Turquette <mturquette@linaro.org>
>>>>
>>> Thanks, applied.
>>
>> I haven't yet had time to dig / track down why, but this patch totally
>> messes up access to the eMMC on the ARM Chromebook (exynos5250-snow).
>> I suddenly start getting FIFO errors like you show above.  When I
>> revert your change then I'm all happy.
>>
>> Perhaps I need a device tree setting change as well?  I always forget
>> how the "samsung,dw-mshc-ciu-div" / "samsung,dw-mshc-sdr-timing"
>> properties work...
>>
>> For the short term I'm going to revert locally since I've got a few
>> other things to do over the next few days.  If nobody else gets around
>> to it then I'll try to find time to dig further.
> 
> Unless I hear differently within 24 hours, I am going to revert this
> in arm-soc (since that is where it is merged right now).
> 

I will have a look at it today.

> It is obviously causing regressions on existing platforms. I am _NOT_
> happy to see dead silence about this for 6 days. Tushar??
> 

Apologies.

> 
> 
> -Olof
> 


-- 
Tushar Behera

[PATCH] clk: exynos5250: Fix parent clock for sclk_mmc{0,1,2,3}

[Tushar Behera]

commit 688f7d8c9fef ("clk: exynos5250: Fix divider values for
sclk_mmc{0,1,2,3}") incorrectly sets the divider for sclk_mmc{0,1,2,3}
to fix the wrong clock value. Though this fixed issue with Arndale,
it created regressions for other boards like Snow.

On Exynos5250, sclk_mmc<n> is generated like below (as per the clock
names in drivers/clk/samsung/clk-exynos5250.c)

mout_group1_p ==> mout_mmc<n> ==>
		div_mmc<n> ==> div_mmc_pre<n> => sclk_mmc<n>

Earlier div_mmc<n> was set as the parent for sclk_mmc<n>, hence
div_mmc_pre<n> was not getting referred in kernel code and depending
on its value set during preboot, sclk_mmc<n> value was different for
various boards.

Setting the correct clock generation path should fix the issues
reported in above referenced commit. The changes committed during the
earlier patch has also been reverted here.

Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
CC: Doug Anderson <dianders@chromium.org>
---
Doug,

Would you please test whether this patch works for Snow?


 drivers/clk/samsung/clk-exynos5250.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
index 7290faa..bb54606 100644
--- a/drivers/clk/samsung/clk-exynos5250.c
+++ b/drivers/clk/samsung/clk-exynos5250.c
@@ -276,10 +276,10 @@ struct samsung_div_clock exynos5250_div_clks[] __initdata = {
 	DIV(none, "div_pcm0", "sclk_audio0", DIV_MAU, 4, 8),
 	DIV(none, "div_sata", "mout_sata", DIV_FSYS0, 20, 4),
 	DIV(none, "div_usb3", "mout_usb3", DIV_FSYS0, 24, 4),
-	DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 8, 8),
-	DIV(none, "div_mmc1", "mout_mmc1", DIV_FSYS1, 24, 8),
-	DIV(none, "div_mmc2", "mout_mmc2", DIV_FSYS2, 8, 8),
-	DIV(none, "div_mmc3", "mout_mmc3", DIV_FSYS2, 24, 8),
+	DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 0, 4),
+	DIV(none, "div_mmc1", "mout_mmc1", DIV_FSYS1, 16, 4),
+	DIV(none, "div_mmc2", "mout_mmc2", DIV_FSYS2, 0, 4),
+	DIV(none, "div_mmc3", "mout_mmc3", DIV_FSYS2, 16, 4),
 	DIV(none, "div_uart0", "mout_uart0", DIV_PERIC0, 0, 4),
 	DIV(none, "div_uart1", "mout_uart1", DIV_PERIC0, 4, 4),
 	DIV(none, "div_uart2", "mout_uart2", DIV_PERIC0, 8, 4),
@@ -421,13 +421,13 @@ struct samsung_gate_clock exynos5250_gate_clks[] __initdata = {
 			SRC_MASK_DISP1_0, 20, 0, 0),
 	GATE(sclk_audio0, "sclk_audio0", "div_audio0",
 			SRC_MASK_MAU, 0, CLK_SET_RATE_PARENT, 0),
-	GATE(sclk_mmc0, "sclk_mmc0", "div_mmc0",
+	GATE(sclk_mmc0, "sclk_mmc0", "div_mmc_pre0",
 			SRC_MASK_FSYS, 0, CLK_SET_RATE_PARENT, 0),
-	GATE(sclk_mmc1, "sclk_mmc1", "div_mmc1",
+	GATE(sclk_mmc1, "sclk_mmc1", "div_mmc_pre1",
 			SRC_MASK_FSYS, 4, CLK_SET_RATE_PARENT, 0),
-	GATE(sclk_mmc2, "sclk_mmc2", "div_mmc2",
+	GATE(sclk_mmc2, "sclk_mmc2", "div_mmc_pre2",
 			SRC_MASK_FSYS, 8, CLK_SET_RATE_PARENT, 0),
-	GATE(sclk_mmc3, "sclk_mmc3", "div_mmc3",
+	GATE(sclk_mmc3, "sclk_mmc3", "div_mmc_pre3",
 			SRC_MASK_FSYS, 12, CLK_SET_RATE_PARENT, 0),
 	GATE(sclk_sata, "sclk_sata", "div_sata",
 			SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0),
-- 
1.7.9.5

Re: [PATCH] clk: exynos5250: Fix parent clock for sclk_mmc{0,1,2,3}

[Doug Anderson]

Tushar,

Thanks for tracking this down.

On Mon, Apr 22, 2013 at 11:31 PM, Tushar Behera
<tushar.behera@linaro.org> wrote:
> commit 688f7d8c9fef ("clk: exynos5250: Fix divider values for
> sclk_mmc{0,1,2,3}") incorrectly sets the divider for sclk_mmc{0,1,2,3}
> to fix the wrong clock value. Though this fixed issue with Arndale,
> it created regressions for other boards like Snow.
>
> On Exynos5250, sclk_mmc<n> is generated like below (as per the clock
> names in drivers/clk/samsung/clk-exynos5250.c)
>
> mout_group1_p ==> mout_mmc<n> ==>
>                 div_mmc<n> ==> div_mmc_pre<n> => sclk_mmc<n>
>
> Earlier div_mmc<n> was set as the parent for sclk_mmc<n>, hence
> div_mmc_pre<n> was not getting referred in kernel code and depending
> on its value set during preboot, sclk_mmc<n> value was different for
> various boards.
>
> Setting the correct clock generation path should fix the issues
> reported in above referenced commit. The changes committed during the
> earlier patch has also been reverted here.
>
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> CC: Doug Anderson <dianders@chromium.org>
> ---
> Doug,
>
> Would you please test whether this patch works for Snow?

Yup, it works for me.  I did the basic testing of installing to eMMC
with this and then booting from eMMC.  I saw no errors in the console.

I glanced over the patch and it looks reasonable to me.  :)

Reported-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>

Re: [PATCH] clk: exynos5250: Fix parent clock for sclk_mmc{0,1,2,3}

[Mike Turquette]

Quoting Tushar Behera (2013-04-22 23:31:51)
> commit 688f7d8c9fef ("clk: exynos5250: Fix divider values for
> sclk_mmc{0,1,2,3}") incorrectly sets the divider for sclk_mmc{0,1,2,3}
> to fix the wrong clock value. Though this fixed issue with Arndale,
> it created regressions for other boards like Snow.
> 
> On Exynos5250, sclk_mmc<n> is generated like below (as per the clock
> names in drivers/clk/samsung/clk-exynos5250.c)
> 
> mout_group1_p ==> mout_mmc<n> ==>
>                 div_mmc<n> ==> div_mmc_pre<n> => sclk_mmc<n>
> 
> Earlier div_mmc<n> was set as the parent for sclk_mmc<n>, hence
> div_mmc_pre<n> was not getting referred in kernel code and depending
> on its value set during preboot, sclk_mmc<n> value was different for
> various boards.
> 
> Setting the correct clock generation path should fix the issues
> reported in above referenced commit. The changes committed during the
> earlier patch has also been reverted here.
> 
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> CC: Doug Anderson <dianders@chromium.org>

Change looks good to me.

Regards,
Mike

> ---
> Doug,
> 
> Would you please test whether this patch works for Snow?
> 
> 
>  drivers/clk/samsung/clk-exynos5250.c |   16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
> index 7290faa..bb54606 100644
> --- a/drivers/clk/samsung/clk-exynos5250.c
> +++ b/drivers/clk/samsung/clk-exynos5250.c
> @@ -276,10 +276,10 @@ struct samsung_div_clock exynos5250_div_clks[] __initdata = {
>         DIV(none, "div_pcm0", "sclk_audio0", DIV_MAU, 4, 8),
>         DIV(none, "div_sata", "mout_sata", DIV_FSYS0, 20, 4),
>         DIV(none, "div_usb3", "mout_usb3", DIV_FSYS0, 24, 4),
> -       DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 8, 8),
> -       DIV(none, "div_mmc1", "mout_mmc1", DIV_FSYS1, 24, 8),
> -       DIV(none, "div_mmc2", "mout_mmc2", DIV_FSYS2, 8, 8),
> -       DIV(none, "div_mmc3", "mout_mmc3", DIV_FSYS2, 24, 8),
> +       DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 0, 4),
> +       DIV(none, "div_mmc1", "mout_mmc1", DIV_FSYS1, 16, 4),
> +       DIV(none, "div_mmc2", "mout_mmc2", DIV_FSYS2, 0, 4),
> +       DIV(none, "div_mmc3", "mout_mmc3", DIV_FSYS2, 16, 4),
>         DIV(none, "div_uart0", "mout_uart0", DIV_PERIC0, 0, 4),
>         DIV(none, "div_uart1", "mout_uart1", DIV_PERIC0, 4, 4),
>         DIV(none, "div_uart2", "mout_uart2", DIV_PERIC0, 8, 4),
> @@ -421,13 +421,13 @@ struct samsung_gate_clock exynos5250_gate_clks[] __initdata = {
>                         SRC_MASK_DISP1_0, 20, 0, 0),
>         GATE(sclk_audio0, "sclk_audio0", "div_audio0",
>                         SRC_MASK_MAU, 0, CLK_SET_RATE_PARENT, 0),
> -       GATE(sclk_mmc0, "sclk_mmc0", "div_mmc0",
> +       GATE(sclk_mmc0, "sclk_mmc0", "div_mmc_pre0",
>                         SRC_MASK_FSYS, 0, CLK_SET_RATE_PARENT, 0),
> -       GATE(sclk_mmc1, "sclk_mmc1", "div_mmc1",
> +       GATE(sclk_mmc1, "sclk_mmc1", "div_mmc_pre1",
>                         SRC_MASK_FSYS, 4, CLK_SET_RATE_PARENT, 0),
> -       GATE(sclk_mmc2, "sclk_mmc2", "div_mmc2",
> +       GATE(sclk_mmc2, "sclk_mmc2", "div_mmc_pre2",
>                         SRC_MASK_FSYS, 8, CLK_SET_RATE_PARENT, 0),
> -       GATE(sclk_mmc3, "sclk_mmc3", "div_mmc3",
> +       GATE(sclk_mmc3, "sclk_mmc3", "div_mmc_pre3",
>                         SRC_MASK_FSYS, 12, CLK_SET_RATE_PARENT, 0),
>         GATE(sclk_sata, "sclk_sata", "div_sata",
>                         SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0),
> -- 
> 1.7.9.5

Re: [PATCH] clk: exynos5250: Fix parent clock for sclk_mmc{0,1,2,3}

[Kukjin Kim]

On 04/24/13 01:23, Mike Turquette wrote:
> Quoting Tushar Behera (2013-04-22 23:31:51)
>> commit 688f7d8c9fef ("clk: exynos5250: Fix divider values for
>> sclk_mmc{0,1,2,3}") incorrectly sets the divider for sclk_mmc{0,1,2,3}
>> to fix the wrong clock value. Though this fixed issue with Arndale,
>> it created regressions for other boards like Snow.
>>
>> On Exynos5250, sclk_mmc<n>  is generated like below (as per the clock
>> names in drivers/clk/samsung/clk-exynos5250.c)
>>
>> mout_group1_p ==>  mout_mmc<n>  ==>
>>                  div_mmc<n>  ==>  div_mmc_pre<n>  =>  sclk_mmc<n>
>>
>> Earlier div_mmc<n>  was set as the parent for sclk_mmc<n>, hence
>> div_mmc_pre<n>  was not getting referred in kernel code and depending
>> on its value set during preboot, sclk_mmc<n>  value was different for
>> various boards.
>>
>> Setting the correct clock generation path should fix the issues
>> reported in above referenced commit. The changes committed during the
>> earlier patch has also been reverted here.
>>
>> Signed-off-by: Tushar Behera<tushar.behera@linaro.org>
>> CC: Doug Anderson<dianders@chromium.org>
>
> Change looks good to me.
>

+1

Olof, I think, you can pick this up in the arm-soc directly.

If you want:
Acked-by: Kukjin Kim <kgene.kim@samsung.com>

Thanks.

- Kukjin

>> ---
>> Doug,
>>
>> Would you please test whether this patch works for Snow?
>>
>>
>>   drivers/clk/samsung/clk-exynos5250.c |   16 ++++++++--------
>>   1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/clk/samsung/clk-exynos5250.c b/drivers/clk/samsung/clk-exynos5250.c
>> index 7290faa..bb54606 100644
>> --- a/drivers/clk/samsung/clk-exynos5250.c
>> +++ b/drivers/clk/samsung/clk-exynos5250.c
>> @@ -276,10 +276,10 @@ struct samsung_div_clock exynos5250_div_clks[] __initdata = {
>>          DIV(none, "div_pcm0", "sclk_audio0", DIV_MAU, 4, 8),
>>          DIV(none, "div_sata", "mout_sata", DIV_FSYS0, 20, 4),
>>          DIV(none, "div_usb3", "mout_usb3", DIV_FSYS0, 24, 4),
>> -       DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 8, 8),
>> -       DIV(none, "div_mmc1", "mout_mmc1", DIV_FSYS1, 24, 8),
>> -       DIV(none, "div_mmc2", "mout_mmc2", DIV_FSYS2, 8, 8),
>> -       DIV(none, "div_mmc3", "mout_mmc3", DIV_FSYS2, 24, 8),
>> +       DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 0, 4),
>> +       DIV(none, "div_mmc1", "mout_mmc1", DIV_FSYS1, 16, 4),
>> +       DIV(none, "div_mmc2", "mout_mmc2", DIV_FSYS2, 0, 4),
>> +       DIV(none, "div_mmc3", "mout_mmc3", DIV_FSYS2, 16, 4),
>>          DIV(none, "div_uart0", "mout_uart0", DIV_PERIC0, 0, 4),
>>          DIV(none, "div_uart1", "mout_uart1", DIV_PERIC0, 4, 4),
>>          DIV(none, "div_uart2", "mout_uart2", DIV_PERIC0, 8, 4),
>> @@ -421,13 +421,13 @@ struct samsung_gate_clock exynos5250_gate_clks[] __initdata = {
>>                          SRC_MASK_DISP1_0, 20, 0, 0),
>>          GATE(sclk_audio0, "sclk_audio0", "div_audio0",
>>                          SRC_MASK_MAU, 0, CLK_SET_RATE_PARENT, 0),
>> -       GATE(sclk_mmc0, "sclk_mmc0", "div_mmc0",
>> +       GATE(sclk_mmc0, "sclk_mmc0", "div_mmc_pre0",
>>                          SRC_MASK_FSYS, 0, CLK_SET_RATE_PARENT, 0),
>> -       GATE(sclk_mmc1, "sclk_mmc1", "div_mmc1",
>> +       GATE(sclk_mmc1, "sclk_mmc1", "div_mmc_pre1",
>>                          SRC_MASK_FSYS, 4, CLK_SET_RATE_PARENT, 0),
>> -       GATE(sclk_mmc2, "sclk_mmc2", "div_mmc2",
>> +       GATE(sclk_mmc2, "sclk_mmc2", "div_mmc_pre2",
>>                          SRC_MASK_FSYS, 8, CLK_SET_RATE_PARENT, 0),
>> -       GATE(sclk_mmc3, "sclk_mmc3", "div_mmc3",
>> +       GATE(sclk_mmc3, "sclk_mmc3", "div_mmc_pre3",
>>                          SRC_MASK_FSYS, 12, CLK_SET_RATE_PARENT, 0),
>>          GATE(sclk_sata, "sclk_sata", "div_sata",
>>                          SRC_MASK_FSYS, 24, CLK_SET_RATE_PARENT, 0),
>> --
>> 1.7.9.5

Re: [PATCH] clk: exynos5250: Fix parent clock for sclk_mmc{0,1,2,3}

[Olof Johansson]

On Tue, Apr 23, 2013 at 12:01:51PM +0530, Tushar Behera wrote:
> commit 688f7d8c9fef ("clk: exynos5250: Fix divider values for
> sclk_mmc{0,1,2,3}") incorrectly sets the divider for sclk_mmc{0,1,2,3}
> to fix the wrong clock value. Though this fixed issue with Arndale,
> it created regressions for other boards like Snow.
> 
> On Exynos5250, sclk_mmc<n> is generated like below (as per the clock
> names in drivers/clk/samsung/clk-exynos5250.c)
> 
> mout_group1_p ==> mout_mmc<n> ==>
> 		div_mmc<n> ==> div_mmc_pre<n> => sclk_mmc<n>
> 
> Earlier div_mmc<n> was set as the parent for sclk_mmc<n>, hence
> div_mmc_pre<n> was not getting referred in kernel code and depending
> on its value set during preboot, sclk_mmc<n> value was different for
> various boards.
> 
> Setting the correct clock generation path should fix the issues
> reported in above referenced commit. The changes committed during the
> earlier patch has also been reverted here.
> 
> Signed-off-by: Tushar Behera <tushar.behera@linaro.org>
> CC: Doug Anderson <dianders@chromium.org>
> ---
> Doug,
> 
> Would you please test whether this patch works for Snow?

Applied to next/drivers in arm-soc.git.


-Olof