Re: [PATCH] clk: renesas: r8a7795: Fix SD clocks

[Stephen Boyd]

On 08/10, Geert Uytterhoeven wrote:
> From: Yoshihiro Shimoda 
> 
> According to the datasheet, SDn clocks are from the SDSRC clock. And
> the SDSRC has a 1/2 divider. So, we should have ".sdsrc" as an internal
> core clock. Otherwise, since the sdhi driver will calculate clock for
> a sd card using the wrong parent clock rate, and then performance will
> be not good.
> 
> Fixes: 90c073e53909da85 ("clk: shmobile: r8a7795: Add SD divider support")
> Signed-off-by: Yoshihiro Shimoda 
> Acked-by: Dirk Behme 
> Tested-by: Wolfram Sang 
> Signed-off-by: Geert Uytterhoeven 
> Cc: sta...@vger.kernel.org
> ---

Applied to clk-fixes

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] clk: renesas: r8a7795: Fix SD clocks

[Simon Horman]

On Thu, Aug 11, 2016 at 10:57:20AM +0200, Simon Horman wrote:
> On Wed, Aug 10, 2016 at 09:29:43AM +0200, Geert Uytterhoeven wrote:
> > From: Yoshihiro Shimoda 
> > 
> > According to the datasheet, SDn clocks are from the SDSRC clock. And
> > the SDSRC has a 1/2 divider. So, we should have ".sdsrc" as an internal
> > core clock. Otherwise, since the sdhi driver will calculate clock for
> > a sd card using the wrong parent clock rate, and then performance will
> > be not good.
> > 
> > Fixes: 90c073e53909da85 ("clk: shmobile: r8a7795: Add SD divider support")
> > Signed-off-by: Yoshihiro Shimoda 
> > Acked-by: Dirk Behme 
> > Tested-by: Wolfram Sang 
> > Signed-off-by: Geert Uytterhoeven 
> > Cc: sta...@vger.kernel.org
> > ---
> > SDHI was enabled on r8a7795/salvator-x in v4.6.
> > Please apply as a fix for v4.8.
> 
> Thanks, done.

My bad, please ignore the above, this patch is not for my tree.

Re: [PATCH] clk: renesas: r8a7795: Fix SD clocks

[Simon Horman]

On Wed, Aug 10, 2016 at 09:29:43AM +0200, Geert Uytterhoeven wrote:
> From: Yoshihiro Shimoda 
> 
> According to the datasheet, SDn clocks are from the SDSRC clock. And
> the SDSRC has a 1/2 divider. So, we should have ".sdsrc" as an internal
> core clock. Otherwise, since the sdhi driver will calculate clock for
> a sd card using the wrong parent clock rate, and then performance will
> be not good.
> 
> Fixes: 90c073e53909da85 ("clk: shmobile: r8a7795: Add SD divider support")
> Signed-off-by: Yoshihiro Shimoda 
> Acked-by: Dirk Behme 
> Tested-by: Wolfram Sang 
> Signed-off-by: Geert Uytterhoeven 
> Cc: sta...@vger.kernel.org
> ---
> SDHI was enabled on r8a7795/salvator-x in v4.6.
> Please apply as a fix for v4.8.

Thanks, done.

[PATCH] clk: renesas: r8a7795: Fix SD clocks

[Geert Uytterhoeven]

From: Yoshihiro Shimoda 

According to the datasheet, SDn clocks are from the SDSRC clock. And
the SDSRC has a 1/2 divider. So, we should have ".sdsrc" as an internal
core clock. Otherwise, since the sdhi driver will calculate clock for
a sd card using the wrong parent clock rate, and then performance will
be not good.

Fixes: 90c073e53909da85 ("clk: shmobile: r8a7795: Add SD divider support")
Signed-off-by: Yoshihiro Shimoda 
Acked-by: Dirk Behme 
Tested-by: Wolfram Sang 
Signed-off-by: Geert Uytterhoeven 
Cc: sta...@vger.kernel.org
---
SDHI was enabled on r8a7795/salvator-x in v4.6.
Please apply as a fix for v4.8.

Thanks!

 drivers/clk/renesas/r8a7795-cpg-mssr.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c 
b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index d359c92e13a65c00..e38bf60c0ff4e28c 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -69,6 +69,7 @@ static const struct cpg_core_clk r8a7795_core_clks[] 
__initconst = {
DEF_FIXED(".s1",CLK_S1,CLK_PLL1_DIV2,  3, 1),
DEF_FIXED(".s2",CLK_S2,CLK_PLL1_DIV2,  4, 1),
DEF_FIXED(".s3",CLK_S3,CLK_PLL1_DIV2,  6, 1),
+   DEF_FIXED(".sdsrc", CLK_SDSRC, CLK_PLL1_DIV2,  2, 1),
 
/* Core Clock Outputs */
DEF_FIXED("ztr",R8A7795_CLK_ZTR,   CLK_PLL1_DIV2,  6, 1),
@@ -87,10 +88,10 @@ static const struct cpg_core_clk r8a7795_core_clks[] 
__initconst = {
DEF_FIXED("s3d2",   R8A7795_CLK_S3D2,  CLK_S3, 2, 1),
DEF_FIXED("s3d4",   R8A7795_CLK_S3D4,  CLK_S3, 4, 1),
 
-   DEF_GEN3_SD("sd0",  R8A7795_CLK_SD0,   CLK_PLL1_DIV2, 0x0074),
-   DEF_GEN3_SD("sd1",  R8A7795_CLK_SD1,   CLK_PLL1_DIV2, 0x0078),
-   DEF_GEN3_SD("sd2",  R8A7795_CLK_SD2,   CLK_PLL1_DIV2, 0x0268),
-   DEF_GEN3_SD("sd3",  R8A7795_CLK_SD3,   CLK_PLL1_DIV2, 0x026c),
+   DEF_GEN3_SD("sd0",  R8A7795_CLK_SD0,   CLK_SDSRC, 0x0074),
+   DEF_GEN3_SD("sd1",  R8A7795_CLK_SD1,   CLK_SDSRC, 0x0078),
+   DEF_GEN3_SD("sd2",  R8A7795_CLK_SD2,   CLK_SDSRC, 0x0268),
+   DEF_GEN3_SD("sd3",  R8A7795_CLK_SD3,   CLK_SDSRC, 0x026c),
 
DEF_FIXED("cl", R8A7795_CLK_CL,CLK_PLL1_DIV2, 48, 1),
DEF_FIXED("cp", R8A7795_CLK_CP,CLK_EXTAL,  2, 1),
-- 
1.9.1

Re: [PATCH] clk: renesas: r8a7795: Fix SD clocks

[Wolfram Sang]

On Wed, Jul 20, 2016 at 02:06:24PM +0200, Wolfram Sang wrote:
> 
> > Wolfram, Dirk: any comments?
> 
> Looks proper.
> 
> Reviewed-by: Wolfram Sang 
> 
> I couldn't really test it, though, since I still have problems with the
> Gen3 DMA series.

... which are gone now since this series landed in renesas-drivers.
Dunno why but DMA works now, so I can see Shimoda-san's patch makes a
difference if we apply a missing fix which I will send in a second. Long
story short:

Tested-by: Wolfram Sang 

Re: [PATCH] clk: renesas: r8a7795: Fix SD clocks

[Wolfram Sang]

> Wolfram, Dirk: any comments?

Looks proper.

Reviewed-by: Wolfram Sang 

I couldn't really test it, though, since I still have problems with the
Gen3 DMA series.



signature.asc
Description: PGP signature

Re: [PATCH] clk: renesas: r8a7795: Fix SD clocks

[Dirk Behme]

On 18.07.2016 12:53, Geert Uytterhoeven wrote:

Hi Shimoda-san,

On Wed, Jul 13, 2016 at 5:20 AM, Yoshihiro Shimoda
 wrote:

According to the datasheet, SDn clocks are from the SDSRC clock. And
the SDSRC has a 1/2 divider. So, we should have ".sdsrc" as an internal
core clock. Otherwise, since the sdhi driver will calculate clock for
a sd card using the wrong parent clock rate, and then performance will
be not good.

Signed-off-by: Yoshihiro Shimoda 


Reviewed-by: Geert Uytterhoeven 
Fixes: 90c073e53909da85 ("clk: shmobile: r8a7795: Add SD divider support")
although this won't apply to v4.6 as-is, due to the move from
drivers/clk/shmobile/
to drivers/clk/renesas, and s/DEF_SD/DEF_GEN3_SD/.

This causes the following changes:

--- clk_summary.old 2016-07-18 12:45:07.788501000 +0200
+++ clk_summary 2016-07-18 12:47:43.27966 +0200
@@ -34,23 +34,24 @@
.pll3  00  159936
0 0
.pll2  00  119952
0 0
.pll1  11  159936
0 0
-  .pll1_div2  55   79968
0 0
+  .pll1_div2  44   79968
0 0
  hdmi 002499
0 0
 hdmi0 002499
0 0
 hdmi1 002499
0 0
  cl   001666
0 0
- sd3  111250
0 0
-sdif3 121250
0 0
- sd2  009996
0 0
-sdif2 009996
0 0
- sd1  009996
0 0
-sdif1 009996
0 0
- sd0  111250
0 0
-sdif0 121250
0 0
  zx   00   39984
0 0
  zt   00   19992
0 0
  ztrd2006664
0 0
  ztr  00   13328
0 0
+ .sdsrc   22   39984
0 0
+sd3   11 625
0 0
+   sdif3  12 625
0 0
+sd2   004998
0 0
+   sdif2  004998
0 0
+sd1   004998
0 0
+   sdif1  004998
0 0
+sd0   11 625
0 0
+   sdif0  12 625
0 0
  .s3  33   13328
0 0
 s3d4  593332
0 0
scu-all0   123332
0 0

Note that I still have a Salvator-X with a 16.67 MHz i.s.o. 33.33 Mhz crystal.

Wolfram, Dirk: any comments?



I think this has changed (corrected?) in the manual since

90c073e53909da85 ("clk: shmobile: r8a7795: Add SD divider support")

Acked-by: Dirk Behme 

Best regards

Dirk



---
 drivers/clk/renesas/r8a7795-cpg-mssr.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c 
b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index ca5519c..5f99f7c 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -91,6 +91,7 @@ static const struct cpg_core_clk r8a7795_core_clks[] 
__initconst = {
DEF_FIXED(".s1",CLK_S1,CLK_PLL1_DIV2,  3, 1),
DEF_FIXED(".s2",CLK_S2,CLK_PLL1_DIV2,  4, 1),
DEF_FIXED(".s3",CLK_S3,CLK_PLL1_DIV2,  6, 1),
+   DEF_FIXED(".sdsrc", CLK_SDSRC, CLK_PLL1_DIV2,  2, 1),

/* Core Clock Outputs */
DEF_FIXED("ztr",R8A7795_CLK_ZTR,   CLK_PLL1_DIV2,  6, 1),
@@ -109,10 +110,10 @@ static const struct cpg_core_clk r8a7795_core_clks[] 
__initconst = {
DEF_FIXED("s3d2",   R8A7795_CLK_S3D2,  CLK_S3, 2, 1),
DEF_FIXED("s3d4",   

[PATCH] clk: renesas: r8a7795: Fix SD clocks

[Yoshihiro Shimoda]

According to the datasheet, SDn clocks are from the SDSRC clock. And
the SDSRC has a 1/2 divider. So, we should have ".sdsrc" as an internal
core clock. Otherwise, since the sdhi driver will calculate clock for
a sd card using the wrong parent clock rate, and then performance will
be not good.

Signed-off-by: Yoshihiro Shimoda 
---
 drivers/clk/renesas/r8a7795-cpg-mssr.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/renesas/r8a7795-cpg-mssr.c 
b/drivers/clk/renesas/r8a7795-cpg-mssr.c
index ca5519c..5f99f7c 100644
--- a/drivers/clk/renesas/r8a7795-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7795-cpg-mssr.c
@@ -91,6 +91,7 @@ static const struct cpg_core_clk r8a7795_core_clks[] 
__initconst = {
DEF_FIXED(".s1",CLK_S1,CLK_PLL1_DIV2,  3, 1),
DEF_FIXED(".s2",CLK_S2,CLK_PLL1_DIV2,  4, 1),
DEF_FIXED(".s3",CLK_S3,CLK_PLL1_DIV2,  6, 1),
+   DEF_FIXED(".sdsrc", CLK_SDSRC, CLK_PLL1_DIV2,  2, 1),
 
/* Core Clock Outputs */
DEF_FIXED("ztr",R8A7795_CLK_ZTR,   CLK_PLL1_DIV2,  6, 1),
@@ -109,10 +110,10 @@ static const struct cpg_core_clk r8a7795_core_clks[] 
__initconst = {
DEF_FIXED("s3d2",   R8A7795_CLK_S3D2,  CLK_S3, 2, 1),
DEF_FIXED("s3d4",   R8A7795_CLK_S3D4,  CLK_S3, 4, 1),
 
-   DEF_GEN3_SD("sd0",  R8A7795_CLK_SD0,   CLK_PLL1_DIV2, 0x0074),
-   DEF_GEN3_SD("sd1",  R8A7795_CLK_SD1,   CLK_PLL1_DIV2, 0x0078),
-   DEF_GEN3_SD("sd2",  R8A7795_CLK_SD2,   CLK_PLL1_DIV2, 0x0268),
-   DEF_GEN3_SD("sd3",  R8A7795_CLK_SD3,   CLK_PLL1_DIV2, 0x026c),
+   DEF_GEN3_SD("sd0",  R8A7795_CLK_SD0,   CLK_SDSRC, 0x0074),
+   DEF_GEN3_SD("sd1",  R8A7795_CLK_SD1,   CLK_SDSRC, 0x0078),
+   DEF_GEN3_SD("sd2",  R8A7795_CLK_SD2,   CLK_SDSRC, 0x0268),
+   DEF_GEN3_SD("sd3",  R8A7795_CLK_SD3,   CLK_SDSRC, 0x026c),
 
DEF_FIXED("cl", R8A7795_CLK_CL,CLK_PLL1_DIV2, 48, 1),
DEF_FIXED("cp", R8A7795_CLK_CP,CLK_EXTAL,  2, 1),
-- 
1.9.1