[PATCH v2] clk: shmobile: r8a7795: Add SDHI clocks

[PATCH v2] clk: shmobile: r8a7795: Add SDHI clocks

[Dirk Behme]

Add R8A7795 SDHI clocks.

Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
---
Changes in v2: Add the missing *H clocks and correct the dividers.

This replaces v1

http://www.spinics.net/lists/linux-sh/msg47464.html

 drivers/clk/shmobile/r8a7795-cpg-mssr.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/shmobile/r8a7795-cpg-mssr.c b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
index 05479e6..f30ed32 100644
--- a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
+++ b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
@@ -100,8 +100,15 @@ 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_FIXED("cl",         R8A7795_CLK_CL,    CLK_PLL1_DIV2, 48, 1),
+	DEF_FIXED("sd0h",       R8A7795_CLK_SD0H,  CLK_PLL1_DIV2,  2, 1),
+	DEF_FIXED("sd0",        R8A7795_CLK_SD0,   CLK_PLL1_DIV2,  8, 1),
+	DEF_FIXED("sd1h",       R8A7795_CLK_SD1H,  CLK_PLL1_DIV2,  2, 1),
+	DEF_FIXED("sd1",        R8A7795_CLK_SD1,   CLK_PLL1_DIV2,  8, 1),
+	DEF_FIXED("sd2h",       R8A7795_CLK_SD2H,  CLK_PLL1_DIV2,  2, 1),
+	DEF_FIXED("sd2",        R8A7795_CLK_SD2,   CLK_PLL1_DIV2,  8, 1),
+	DEF_FIXED("sd3h",       R8A7795_CLK_SD3H,  CLK_PLL1_DIV2,  2, 1),
+	DEF_FIXED("sd3",        R8A7795_CLK_SD3,   CLK_PLL1_DIV2,  8, 1),
 	DEF_FIXED("cp",         R8A7795_CLK_CP,    CLK_EXTAL,      2, 1),
-
 	DEF_DIV6P1("mso",       R8A7795_CLK_MSO,   CLK_PLL1_DIV4, 0x014),
 	DEF_DIV6P1("hdmi",      R8A7795_CLK_HDMI,  CLK_PLL1_DIV2, 0x250),
 };
@@ -120,6 +127,10 @@ static const struct mssr_mod_clk r8a7795_mod_clks[] __initconst = {
 	DEF_MOD("sys-dmac1",		 218,	R8A7795_CLK_S3D1),
 	DEF_MOD("sys-dmac0",		 219,	R8A7795_CLK_S3D1),
 	DEF_MOD("scif2",		 310,	R8A7795_CLK_S3D4),
+	DEF_MOD("sdif3",	       	 311,	R8A7795_CLK_SD3),
+	DEF_MOD("sdif2",	       	 312,	R8A7795_CLK_SD2),
+	DEF_MOD("sdif1",	      	 313,	R8A7795_CLK_SD1),
+	DEF_MOD("sdif0",	     	 314,	R8A7795_CLK_SD0),
 	DEF_MOD("pcie1",		 318,	R8A7795_CLK_S3D1),
 	DEF_MOD("pcie0",		 319,	R8A7795_CLK_S3D1),
 	DEF_MOD("intc-ap",		 408,	R8A7795_CLK_S3D1),
-- 
2.6.4

Re: [PATCH v2] clk: shmobile: r8a7795: Add SDHI clocks

[Dirk Behme]

On 24.12.2015 11:09, Dirk Behme wrote:
> Add R8A7795 SDHI clocks.
>
> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
> ---
> Changes in v2: Add the missing *H clocks and correct the dividers.
>
> This replaces v1
>
> http://www.spinics.net/lists/linux-sh/msg47464.html
>
>   drivers/clk/shmobile/r8a7795-cpg-mssr.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/shmobile/r8a7795-cpg-mssr.c b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
> index 05479e6..f30ed32 100644
> --- a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
> +++ b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
> @@ -100,8 +100,15 @@ 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_FIXED("cl",         R8A7795_CLK_CL,    CLK_PLL1_DIV2, 48, 1),
> +	DEF_FIXED("sd0h",       R8A7795_CLK_SD0H,  CLK_PLL1_DIV2,  2, 1),
> +	DEF_FIXED("sd0",        R8A7795_CLK_SD0,   CLK_PLL1_DIV2,  8, 1),
> +	DEF_FIXED("sd1h",       R8A7795_CLK_SD1H,  CLK_PLL1_DIV2,  2, 1),
> +	DEF_FIXED("sd1",        R8A7795_CLK_SD1,   CLK_PLL1_DIV2,  8, 1),
> +	DEF_FIXED("sd2h",       R8A7795_CLK_SD2H,  CLK_PLL1_DIV2,  2, 1),
> +	DEF_FIXED("sd2",        R8A7795_CLK_SD2,   CLK_PLL1_DIV2,  8, 1),
> +	DEF_FIXED("sd3h",       R8A7795_CLK_SD3H,  CLK_PLL1_DIV2,  2, 1),
> +	DEF_FIXED("sd3",        R8A7795_CLK_SD3,   CLK_PLL1_DIV2,  8, 1),
>   	DEF_FIXED("cp",         R8A7795_CLK_CP,    CLK_EXTAL,      2, 1),
> -
>   	DEF_DIV6P1("mso",       R8A7795_CLK_MSO,   CLK_PLL1_DIV4, 0x014),
>   	DEF_DIV6P1("hdmi",      R8A7795_CLK_HDMI,  CLK_PLL1_DIV2, 0x250),
>   };
> @@ -120,6 +127,10 @@ static const struct mssr_mod_clk r8a7795_mod_clks[] __initconst = {
>   	DEF_MOD("sys-dmac1",		 218,	R8A7795_CLK_S3D1),
>   	DEF_MOD("sys-dmac0",		 219,	R8A7795_CLK_S3D1),
>   	DEF_MOD("scif2",		 310,	R8A7795_CLK_S3D4),
> +	DEF_MOD("sdif3",	       	 311,	R8A7795_CLK_SD3),
> +	DEF_MOD("sdif2",	       	 312,	R8A7795_CLK_SD2),
> +	DEF_MOD("sdif1",	      	 313,	R8A7795_CLK_SD1),
> +	DEF_MOD("sdif0",	     	 314,	R8A7795_CLK_SD0),
>   	DEF_MOD("pcie1",		 318,	R8A7795_CLK_S3D1),
>   	DEF_MOD("pcie0",		 319,	R8A7795_CLK_S3D1),
>   	DEF_MOD("intc-ap",		 408,	R8A7795_CLK_S3D1),


Any comments on this? Could this be applied?

Best regards

Dirk

Re: [PATCH v2] clk: shmobile: r8a7795: Add SDHI clocks

[Geert Uytterhoeven]

Hi Dirk,

On Sat, Jan 9, 2016 at 7:42 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
> On 24.12.2015 11:09, Dirk Behme wrote:
>> Add R8A7795 SDHI clocks.

Thanks for your patch!

>> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
>> ---
>> Changes in v2: Add the missing *H clocks and correct the dividers.
>>
>> This replaces v1
>>
>> http://www.spinics.net/lists/linux-sh/msg47464.html
>>
>>   drivers/clk/shmobile/r8a7795-cpg-mssr.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>> b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>> index 05479e6..f30ed32 100644
>> --- a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>> +++ b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>> @@ -100,8 +100,15 @@ 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_FIXED("cl",         R8A7795_CLK_CL,    CLK_PLL1_DIV2, 48, 1),
>> +       DEF_FIXED("sd0h",       R8A7795_CLK_SD0H,  CLK_PLL1_DIV2,  2, 1),
>> +       DEF_FIXED("sd0",        R8A7795_CLK_SD0,   CLK_PLL1_DIV2,  8, 1),
>> +       DEF_FIXED("sd1h",       R8A7795_CLK_SD1H,  CLK_PLL1_DIV2,  2, 1),
>> +       DEF_FIXED("sd1",        R8A7795_CLK_SD1,   CLK_PLL1_DIV2,  8, 1),
>> +       DEF_FIXED("sd2h",       R8A7795_CLK_SD2H,  CLK_PLL1_DIV2,  2, 1),
>> +       DEF_FIXED("sd2",        R8A7795_CLK_SD2,   CLK_PLL1_DIV2,  8, 1),
>> +       DEF_FIXED("sd3h",       R8A7795_CLK_SD3H,  CLK_PLL1_DIV2,  2, 1),
>> +       DEF_FIXED("sd3",        R8A7795_CLK_SD3,   CLK_PLL1_DIV2,  8, 1),

The dividers for these clocks are not fixed, they are controlled by the
SDnCKCR registers.

Unfortunately the register layout is more complicated than on R-Car Gen2, so
you can no longer use clk_register_divider_table(), but have to write a custom
clock driver.

For an initial version, a simple "read-only" version that just calls
clk_register_fixed_factor() with divider values read from the hardware
registers may be good enough. But for full support, you need a driver that
can program the registers, too.

Just using fixed dividers like you did won't work, as the boot loader/reset
state may be different.
E.g. on my board, which has a 16.66666 MHz crystal, the initial state according
to the register values is:
  - sd[0-2]h are stopped,
  - sd[0-2] run at 25 MHz,
  - sd3h runs at 400 MHz,
  - sd3 runs at 100 MHz.

>> @@ -120,6 +127,10 @@ static const struct mssr_mod_clk r8a7795_mod_clks[]
>> __initconst = {
>>         DEF_MOD("sys-dmac1",             218,   R8A7795_CLK_S3D1),
>>         DEF_MOD("sys-dmac0",             219,   R8A7795_CLK_S3D1),
>>         DEF_MOD("scif2",                 310,   R8A7795_CLK_S3D4),
>> +       DEF_MOD("sdif3",                 311,   R8A7795_CLK_SD3),
>> +       DEF_MOD("sdif2",                 312,   R8A7795_CLK_SD2),
>> +       DEF_MOD("sdif1",                 313,   R8A7795_CLK_SD1),
>> +       DEF_MOD("sdif0",                 314,   R8A7795_CLK_SD0),

This part is OK.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2] clk: shmobile: r8a7795: Add SDHI clocks

[Michael Turquette]

Hi Dirk,

On Fri, Jan 8, 2016 at 10:42 PM, Dirk Behme <dirk.behme@gmail.com> wrote:
> On 24.12.2015 11:09, Dirk Behme wrote:
>>
>> Add R8A7795 SDHI clocks.
>>
>> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>

Please Cc linux-clk@vger.kernel.org for review of clk driver patches.

Thanks,
Mike

>> ---
>> Changes in v2: Add the missing *H clocks and correct the dividers.
>>
>> This replaces v1
>>
>> http://www.spinics.net/lists/linux-sh/msg47464.html
>>
>>   drivers/clk/shmobile/r8a7795-cpg-mssr.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>> b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>> index 05479e6..f30ed32 100644
>> --- a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>> +++ b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>> @@ -100,8 +100,15 @@ 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_FIXED("cl",         R8A7795_CLK_CL,    CLK_PLL1_DIV2, 48, 1),
>> +       DEF_FIXED("sd0h",       R8A7795_CLK_SD0H,  CLK_PLL1_DIV2,  2, 1),
>> +       DEF_FIXED("sd0",        R8A7795_CLK_SD0,   CLK_PLL1_DIV2,  8, 1),
>> +       DEF_FIXED("sd1h",       R8A7795_CLK_SD1H,  CLK_PLL1_DIV2,  2, 1),
>> +       DEF_FIXED("sd1",        R8A7795_CLK_SD1,   CLK_PLL1_DIV2,  8, 1),
>> +       DEF_FIXED("sd2h",       R8A7795_CLK_SD2H,  CLK_PLL1_DIV2,  2, 1),
>> +       DEF_FIXED("sd2",        R8A7795_CLK_SD2,   CLK_PLL1_DIV2,  8, 1),
>> +       DEF_FIXED("sd3h",       R8A7795_CLK_SD3H,  CLK_PLL1_DIV2,  2, 1),
>> +       DEF_FIXED("sd3",        R8A7795_CLK_SD3,   CLK_PLL1_DIV2,  8, 1),
>>         DEF_FIXED("cp",         R8A7795_CLK_CP,    CLK_EXTAL,      2, 1),
>> -
>>         DEF_DIV6P1("mso",       R8A7795_CLK_MSO,   CLK_PLL1_DIV4, 0x014),
>>         DEF_DIV6P1("hdmi",      R8A7795_CLK_HDMI,  CLK_PLL1_DIV2, 0x250),
>>   };
>> @@ -120,6 +127,10 @@ static const struct mssr_mod_clk r8a7795_mod_clks[]
>> __initconst = {
>>         DEF_MOD("sys-dmac1",             218,   R8A7795_CLK_S3D1),
>>         DEF_MOD("sys-dmac0",             219,   R8A7795_CLK_S3D1),
>>         DEF_MOD("scif2",                 310,   R8A7795_CLK_S3D4),
>> +       DEF_MOD("sdif3",                 311,   R8A7795_CLK_SD3),
>> +       DEF_MOD("sdif2",                 312,   R8A7795_CLK_SD2),
>> +       DEF_MOD("sdif1",                 313,   R8A7795_CLK_SD1),
>> +       DEF_MOD("sdif0",                 314,   R8A7795_CLK_SD0),
>>         DEF_MOD("pcie1",                 318,   R8A7795_CLK_S3D1),
>>         DEF_MOD("pcie0",                 319,   R8A7795_CLK_S3D1),
>>         DEF_MOD("intc-ap",               408,   R8A7795_CLK_S3D1),
>
>
>
> Any comments on this? Could this be applied?
>
> Best regards
>
> Dirk
>



-- 
Michael Turquette
CEO
BayLibre - At the Heart of Embedded Linux
http://baylibre.com/

Re: [PATCH v2] clk: shmobile: r8a7795: Add SDHI clocks

[Dirk Behme]

On 14.01.2016 19:24, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> On Sat, Jan 9, 2016 at 7:42 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
>> On 24.12.2015 11:09, Dirk Behme wrote:
>>> Add R8A7795 SDHI clocks.
>
> Thanks for your patch!
>
>>> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
>>> ---
>>> Changes in v2: Add the missing *H clocks and correct the dividers.
>>>
>>> This replaces v1
>>>
>>> http://www.spinics.net/lists/linux-sh/msg47464.html
>>>
>>>    drivers/clk/shmobile/r8a7795-cpg-mssr.c | 13 ++++++++++++-
>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>> b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>> index 05479e6..f30ed32 100644
>>> --- a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>> +++ b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>> @@ -100,8 +100,15 @@ 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_FIXED("cl",         R8A7795_CLK_CL,    CLK_PLL1_DIV2, 48, 1),
>>> +       DEF_FIXED("sd0h",       R8A7795_CLK_SD0H,  CLK_PLL1_DIV2,  2, 1),
>>> +       DEF_FIXED("sd0",        R8A7795_CLK_SD0,   CLK_PLL1_DIV2,  8, 1),
>>> +       DEF_FIXED("sd1h",       R8A7795_CLK_SD1H,  CLK_PLL1_DIV2,  2, 1),
>>> +       DEF_FIXED("sd1",        R8A7795_CLK_SD1,   CLK_PLL1_DIV2,  8, 1),
>>> +       DEF_FIXED("sd2h",       R8A7795_CLK_SD2H,  CLK_PLL1_DIV2,  2, 1),
>>> +       DEF_FIXED("sd2",        R8A7795_CLK_SD2,   CLK_PLL1_DIV2,  8, 1),
>>> +       DEF_FIXED("sd3h",       R8A7795_CLK_SD3H,  CLK_PLL1_DIV2,  2, 1),
>>> +       DEF_FIXED("sd3",        R8A7795_CLK_SD3,   CLK_PLL1_DIV2,  8, 1),
>
> The dividers for these clocks are not fixed, they are controlled by the
> SDnCKCR registers.
>
> Unfortunately the register layout is more complicated than on R-Car Gen2, so
> you can no longer use clk_register_divider_table(), but have to write a custom
> clock driver.
>
> For an initial version, a simple "read-only" version that just calls
> clk_register_fixed_factor() with divider values read from the hardware
> registers may be good enough. But for full support, you need a driver that
> can program the registers, too.
>
> Just using fixed dividers like you did won't work, as the boot loader/reset
> state may be different.
> E.g. on my board, which has a 16.66666 MHz crystal, the initial state according
> to the register values is:
>    - sd[0-2]h are stopped,
>    - sd[0-2] run at 25 MHz,
>    - sd3h runs at 400 MHz,
>    - sd3 runs at 100 MHz.


Hmm, why do I get

sh_mobile_sdhi ee140000.mmc: mmc0 base at 0xee140000 clock rate 99 MHz

(or 199MHz considering the clock workaround) and for the other SDx 
with this patch, then?

Best regards

Dirk


>>> @@ -120,6 +127,10 @@ static const struct mssr_mod_clk r8a7795_mod_clks[]
>>> __initconst = {
>>>          DEF_MOD("sys-dmac1",             218,   R8A7795_CLK_S3D1),
>>>          DEF_MOD("sys-dmac0",             219,   R8A7795_CLK_S3D1),
>>>          DEF_MOD("scif2",                 310,   R8A7795_CLK_S3D4),
>>> +       DEF_MOD("sdif3",                 311,   R8A7795_CLK_SD3),
>>> +       DEF_MOD("sdif2",                 312,   R8A7795_CLK_SD2),
>>> +       DEF_MOD("sdif1",                 313,   R8A7795_CLK_SD1),
>>> +       DEF_MOD("sdif0",                 314,   R8A7795_CLK_SD0),
>
> This part is OK.
>
> Gr{oetje,eeting}s,
>
>                          Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                  -- Linus Torvalds
>

Re: [PATCH v2] clk: shmobile: r8a7795: Add SDHI clocks

[Geert Uytterhoeven]

Hi Dirk,

On Thu, Jan 14, 2016 at 8:02 PM, Dirk Behme <dirk.behme@gmail.com> wrote:
> On 14.01.2016 19:24, Geert Uytterhoeven wrote:
>> On Sat, Jan 9, 2016 at 7:42 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
>>> On 24.12.2015 11:09, Dirk Behme wrote:
>>>> Add R8A7795 SDHI clocks.
>>>> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
>>>> ---
>>>> Changes in v2: Add the missing *H clocks and correct the dividers.
>>>>
>>>> This replaces v1
>>>>
>>>> http://www.spinics.net/lists/linux-sh/msg47464.html
>>>>
>>>>    drivers/clk/shmobile/r8a7795-cpg-mssr.c | 13 ++++++++++++-
>>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>>> b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>>> index 05479e6..f30ed32 100644
>>>> --- a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>>> +++ b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>>> @@ -100,8 +100,15 @@ 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_FIXED("cl",         R8A7795_CLK_CL,    CLK_PLL1_DIV2, 48,
>>>> 1),
>>>> +       DEF_FIXED("sd0h",       R8A7795_CLK_SD0H,  CLK_PLL1_DIV2,  2,
>>>> 1),
>>>> +       DEF_FIXED("sd0",        R8A7795_CLK_SD0,   CLK_PLL1_DIV2,  8,
>>>> 1),
>>>> +       DEF_FIXED("sd1h",       R8A7795_CLK_SD1H,  CLK_PLL1_DIV2,  2,
>>>> 1),
>>>> +       DEF_FIXED("sd1",        R8A7795_CLK_SD1,   CLK_PLL1_DIV2,  8,
>>>> 1),
>>>> +       DEF_FIXED("sd2h",       R8A7795_CLK_SD2H,  CLK_PLL1_DIV2,  2,
>>>> 1),
>>>> +       DEF_FIXED("sd2",        R8A7795_CLK_SD2,   CLK_PLL1_DIV2,  8,
>>>> 1),
>>>> +       DEF_FIXED("sd3h",       R8A7795_CLK_SD3H,  CLK_PLL1_DIV2,  2,
>>>> 1),
>>>> +       DEF_FIXED("sd3",        R8A7795_CLK_SD3,   CLK_PLL1_DIV2,  8,
>>>> 1),
>>
>>
>> The dividers for these clocks are not fixed, they are controlled by the
>> SDnCKCR registers.
>>
>> Unfortunately the register layout is more complicated than on R-Car Gen2,
>> so
>> you can no longer use clk_register_divider_table(), but have to write a
>> custom
>> clock driver.
>>
>> For an initial version, a simple "read-only" version that just calls
>> clk_register_fixed_factor() with divider values read from the hardware
>> registers may be good enough. But for full support, you need a driver that
>> can program the registers, too.
>>
>> Just using fixed dividers like you did won't work, as the boot
>> loader/reset
>> state may be different.
>> E.g. on my board, which has a 16.66666 MHz crystal, the initial state
>> according
>> to the register values is:
>>    - sd[0-2]h are stopped,
>>    - sd[0-2] run at 25 MHz,
>>    - sd3h runs at 400 MHz,
>>    - sd3 runs at 100 MHz.
>
> Hmm, why do I get
>
> sh_mobile_sdhi ee140000.mmc: mmc0 base at 0xee140000 clock rate 99 MHz
>
> (or 199MHz considering the clock workaround) and for the other SDx with this
> patch, then?

Because your patch hardcodes them to PLL1_DIV2 / 8, and thus the driver will
report the hardcoded values.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2] clk: shmobile: r8a7795: Add SDHI clocks

[Dirk Behme]

On 14.01.2016 19:24, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> On Sat, Jan 9, 2016 at 7:42 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
>> On 24.12.2015 11:09, Dirk Behme wrote:
>>> Add R8A7795 SDHI clocks.
>
> Thanks for your patch!
>
>>> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
>>> ---
>>> Changes in v2: Add the missing *H clocks and correct the dividers.
>>>
>>> This replaces v1
>>>
>>> http://www.spinics.net/lists/linux-sh/msg47464.html
>>>
>>>    drivers/clk/shmobile/r8a7795-cpg-mssr.c | 13 ++++++++++++-
>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>> b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>> index 05479e6..f30ed32 100644
>>> --- a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>> +++ b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>> @@ -100,8 +100,15 @@ 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_FIXED("cl",         R8A7795_CLK_CL,    CLK_PLL1_DIV2, 48, 1),
>>> +       DEF_FIXED("sd0h",       R8A7795_CLK_SD0H,  CLK_PLL1_DIV2,  2, 1),
>>> +       DEF_FIXED("sd0",        R8A7795_CLK_SD0,   CLK_PLL1_DIV2,  8, 1),
>>> +       DEF_FIXED("sd1h",       R8A7795_CLK_SD1H,  CLK_PLL1_DIV2,  2, 1),
>>> +       DEF_FIXED("sd1",        R8A7795_CLK_SD1,   CLK_PLL1_DIV2,  8, 1),
>>> +       DEF_FIXED("sd2h",       R8A7795_CLK_SD2H,  CLK_PLL1_DIV2,  2, 1),
>>> +       DEF_FIXED("sd2",        R8A7795_CLK_SD2,   CLK_PLL1_DIV2,  8, 1),
>>> +       DEF_FIXED("sd3h",       R8A7795_CLK_SD3H,  CLK_PLL1_DIV2,  2, 1),
>>> +       DEF_FIXED("sd3",        R8A7795_CLK_SD3,   CLK_PLL1_DIV2,  8, 1),
>
> The dividers for these clocks are not fixed, they are controlled by the
> SDnCKCR registers.
>
> Unfortunately the register layout is more complicated than on R-Car Gen2, so
> you can no longer use clk_register_divider_table(), but have to write a custom
> clock driver.
>
> For an initial version, a simple "read-only" version that just calls
> clk_register_fixed_factor() with divider values read from the hardware
> registers may be good enough. But for full support, you need a driver that
> can program the registers, too.


Anything like

https://git.kernel.org/cgit/linux/kernel/git/horms/renesas-bsp.git/commit/drivers/clk/shmobile/clk-rcar-gen3.c?h=v4.2/rcar-3.0.x&idÍ10385afc15cef6bfbaea4aa5da41193b24fe82

?

Best regards

Dirk

Re: [PATCH v2] clk: shmobile: r8a7795: Add SDHI clocks

[Geert Uytterhoeven]

Hi Dirk,

On Fri, Jan 15, 2016 at 11:11 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
> On 14.01.2016 19:24, Geert Uytterhoeven wrote:
>> On Sat, Jan 9, 2016 at 7:42 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
>>> On 24.12.2015 11:09, Dirk Behme wrote:
>>>> Add R8A7795 SDHI clocks.
>>>> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
>>>> ---
>>>> Changes in v2: Add the missing *H clocks and correct the dividers.
>>>>
>>>> This replaces v1
>>>>
>>>> http://www.spinics.net/lists/linux-sh/msg47464.html
>>>>
>>>>    drivers/clk/shmobile/r8a7795-cpg-mssr.c | 13 ++++++++++++-
>>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>>> b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>>> index 05479e6..f30ed32 100644
>>>> --- a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>>> +++ b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>>> @@ -100,8 +100,15 @@ 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_FIXED("cl",         R8A7795_CLK_CL,    CLK_PLL1_DIV2, 48,
>>>> 1),
>>>> +       DEF_FIXED("sd0h",       R8A7795_CLK_SD0H,  CLK_PLL1_DIV2,  2,
>>>> 1),
>>>> +       DEF_FIXED("sd0",        R8A7795_CLK_SD0,   CLK_PLL1_DIV2,  8,
>>>> 1),
>>>> +       DEF_FIXED("sd1h",       R8A7795_CLK_SD1H,  CLK_PLL1_DIV2,  2,
>>>> 1),
>>>> +       DEF_FIXED("sd1",        R8A7795_CLK_SD1,   CLK_PLL1_DIV2,  8,
>>>> 1),
>>>> +       DEF_FIXED("sd2h",       R8A7795_CLK_SD2H,  CLK_PLL1_DIV2,  2,
>>>> 1),
>>>> +       DEF_FIXED("sd2",        R8A7795_CLK_SD2,   CLK_PLL1_DIV2,  8,
>>>> 1),
>>>> +       DEF_FIXED("sd3h",       R8A7795_CLK_SD3H,  CLK_PLL1_DIV2,  2,
>>>> 1),
>>>> +       DEF_FIXED("sd3",        R8A7795_CLK_SD3,   CLK_PLL1_DIV2,  8,
>>>> 1),
>>
>>
>> The dividers for these clocks are not fixed, they are controlled by the
>> SDnCKCR registers.
>>
>> Unfortunately the register layout is more complicated than on R-Car Gen2,
>> so
>> you can no longer use clk_register_divider_table(), but have to write a
>> custom
>> clock driver.
>>
>> For an initial version, a simple "read-only" version that just calls
>> clk_register_fixed_factor() with divider values read from the hardware
>> registers may be good enough. But for full support, you need a driver that
>> can program the registers, too.
>
>
>
> Anything like
>
> https://git.kernel.org/cgit/linux/kernel/git/horms/renesas-bsp.git/commit/drivers/clk/shmobile/clk-rcar-gen3.c?h=v4.2/rcar-3.0.x&idÍ10385afc15cef6bfbaea4aa5da41193b24fe82
>
> ?

Yes.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2] clk: shmobile: r8a7795: Add SDHI clocks

[Dirk Behme]

Hi Geert,

On 15.01.2016 11:20, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> On Fri, Jan 15, 2016 at 11:11 AM, Dirk Behme <dirk.behme@de.bosch.com> wrote:
>> On 14.01.2016 19:24, Geert Uytterhoeven wrote:
>>> On Sat, Jan 9, 2016 at 7:42 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
>>>> On 24.12.2015 11:09, Dirk Behme wrote:
>>>>> Add R8A7795 SDHI clocks.
>>>>> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
>>>>> ---
>>>>> Changes in v2: Add the missing *H clocks and correct the dividers.
>>>>>
>>>>> This replaces v1
>>>>>
>>>>> http://www.spinics.net/lists/linux-sh/msg47464.html
>>>>>
>>>>>     drivers/clk/shmobile/r8a7795-cpg-mssr.c | 13 ++++++++++++-
>>>>>     1 file changed, 12 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>>>> b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>>>> index 05479e6..f30ed32 100644
>>>>> --- a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>>>> +++ b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
>>>>> @@ -100,8 +100,15 @@ 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_FIXED("cl",         R8A7795_CLK_CL,    CLK_PLL1_DIV2, 48,
>>>>> 1),
>>>>> +       DEF_FIXED("sd0h",       R8A7795_CLK_SD0H,  CLK_PLL1_DIV2,  2,
>>>>> 1),
>>>>> +       DEF_FIXED("sd0",        R8A7795_CLK_SD0,   CLK_PLL1_DIV2,  8,
>>>>> 1),
>>>>> +       DEF_FIXED("sd1h",       R8A7795_CLK_SD1H,  CLK_PLL1_DIV2,  2,
>>>>> 1),
>>>>> +       DEF_FIXED("sd1",        R8A7795_CLK_SD1,   CLK_PLL1_DIV2,  8,
>>>>> 1),
>>>>> +       DEF_FIXED("sd2h",       R8A7795_CLK_SD2H,  CLK_PLL1_DIV2,  2,
>>>>> 1),
>>>>> +       DEF_FIXED("sd2",        R8A7795_CLK_SD2,   CLK_PLL1_DIV2,  8,
>>>>> 1),
>>>>> +       DEF_FIXED("sd3h",       R8A7795_CLK_SD3H,  CLK_PLL1_DIV2,  2,
>>>>> 1),
>>>>> +       DEF_FIXED("sd3",        R8A7795_CLK_SD3,   CLK_PLL1_DIV2,  8,
>>>>> 1),
>>>
>>>
>>> The dividers for these clocks are not fixed, they are controlled by the
>>> SDnCKCR registers.
>>>
>>> Unfortunately the register layout is more complicated than on R-Car Gen2,
>>> so
>>> you can no longer use clk_register_divider_table(), but have to write a
>>> custom
>>> clock driver.
>>>
>>> For an initial version, a simple "read-only" version that just calls
>>> clk_register_fixed_factor() with divider values read from the hardware
>>> registers may be good enough. But for full support, you need a driver that
>>> can program the registers, too.
>>
>>
>>
>> Anything like
>>
>> https://git.kernel.org/cgit/linux/kernel/git/horms/renesas-bsp.git/commit/drivers/clk/shmobile/clk-rcar-gen3.c?h=v4.2/rcar-3.0.x&idÍ10385afc15cef6bfbaea4aa5da41193b24fe82


I've had a look to that.

I think we can pick all the functions

+static const struct clk_ops cpg_sd_clock_ops = {
+	.enable = cpg_sd_clock_enable,
+	.disable = cpg_sd_clock_disable,
+	.is_enabled = cpg_sd_clock_is_enabled,
+	.recalc_rate = cpg_sd_clock_recalc_rate,
+	.round_rate = cpg_sd_clock_round_rate,
+	.set_rate = cpg_sd_clock_set_rate,
+};

unchanged from that patch.

However, I'm not sure how to interface this to cpg_mssr_probe()? It's 
similar to cpg_mssr_register_mod_clk(), but not identical so that this 
could be reused.

We could extend r8a7795_cpg_mssr_info by anything like an additional

/* Dynamic clocks */
.dyn_clks = /* Add an array allowing to pass various clk_ops structs */
...

?

Or we could hard code it like

https://git.kernel.org/cgit/linux/kernel/git/horms/renesas-bsp.git/commit/drivers/clk/shmobile/clk-rcar-gen3.c?h=v4.2/rcar-3.0.x&idÍ10385afc15cef6bfbaea4aa5da41193b24fe82

is doing it with

+	} else if (!strcmp(name, "sd0")) {
+		return cpg_sd_clk_register(name, cpg->reg + CPG_SD0CKCR, np);
+	} else if (!strcmp(name, "sd1")) {
+		return cpg_sd_clk_register(name, cpg->reg + CPG_SD1CKCR, np);
+	} else if (!strcmp(name, "sd2")) {
+		return cpg_sd_clk_register(name, cpg->reg + CPG_SD2CKCR, np);
+	} else if (!strcmp(name, "sd3")) {
+		return cpg_sd_clk_register(name, cpg->reg + CPG_SD3CKCR, np);

and add this to e.g. r8a7795_cpg_clk_register(). But, hmm.


What do you think? Opinions? Examples?


Best regards

Dirk

Re: [PATCH v2] clk: shmobile: r8a7795: Add SDHI clocks

[Geert Uytterhoeven]

Hi Dirk,

On Sun, Jan 17, 2016 at 10:47 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
> On 15.01.2016 11:20, Geert Uytterhoeven wrote:
>> On Fri, Jan 15, 2016 at 11:11 AM, Dirk Behme <dirk.behme@de.bosch.com>
>> wrote:
>>> On 14.01.2016 19:24, Geert Uytterhoeven wrote:
>>>> On Sat, Jan 9, 2016 at 7:42 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
>>>>> On 24.12.2015 11:09, Dirk Behme wrote:
>>>>>> Add R8A7795 SDHI clocks.
>>>>>> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
>>>>>> ---
>>>>>> Changes in v2: Add the missing *H clocks and correct the dividers.

>>>> The dividers for these clocks are not fixed, they are controlled by the
>>>> SDnCKCR registers.
>>>>
>>>> Unfortunately the register layout is more complicated than on R-Car
>>>> Gen2,
>>>> so
>>>> you can no longer use clk_register_divider_table(), but have to write a
>>>> custom
>>>> clock driver.
>>>>
>>>> For an initial version, a simple "read-only" version that just calls
>>>> clk_register_fixed_factor() with divider values read from the hardware
>>>> registers may be good enough. But for full support, you need a driver
>>>> that
>>>> can program the registers, too.
>>>
>>> Anything like
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/horms/renesas-bsp.git/commit/drivers/clk/shmobile/clk-rcar-gen3.c?h=v4.2/rcar-3.0.x&idÍ10385afc15cef6bfbaea4aa5da41193b24fe82
>
> I've had a look to that.
>
> I think we can pick all the functions
>
> +static const struct clk_ops cpg_sd_clock_ops = {
> +       .enable = cpg_sd_clock_enable,
> +       .disable = cpg_sd_clock_disable,
> +       .is_enabled = cpg_sd_clock_is_enabled,
> +       .recalc_rate = cpg_sd_clock_recalc_rate,
> +       .round_rate = cpg_sd_clock_round_rate,
> +       .set_rate = cpg_sd_clock_set_rate,
> +};
>
> unchanged from that patch.
>
> However, I'm not sure how to interface this to cpg_mssr_probe()? It's
> similar to cpg_mssr_register_mod_clk(), but not identical so that this could
> be reused.
>
> We could extend r8a7795_cpg_mssr_info by anything like an additional
>
> /* Dynamic clocks */
> .dyn_clks = /* Add an array allowing to pass various clk_ops structs */
> ...
>
> ?
>
> Or we could hard code it like
>
> https://git.kernel.org/cgit/linux/kernel/git/horms/renesas-bsp.git/commit/drivers/clk/shmobile/clk-rcar-gen3.c?h=v4.2/rcar-3.0.x&idÍ10385afc15cef6bfbaea4aa5da41193b24fe82
>
> is doing it with
>
> +       } else if (!strcmp(name, "sd0")) {
> +               return cpg_sd_clk_register(name, cpg->reg + CPG_SD0CKCR,
> np);
> +       } else if (!strcmp(name, "sd1")) {
> +               return cpg_sd_clk_register(name, cpg->reg + CPG_SD1CKCR,
> np);
> +       } else if (!strcmp(name, "sd2")) {
> +               return cpg_sd_clk_register(name, cpg->reg + CPG_SD2CKCR,
> np);
> +       } else if (!strcmp(name, "sd3")) {
> +               return cpg_sd_clk_register(name, cpg->reg + CPG_SD3CKCR,
> np);
>
> and add this to e.g. r8a7795_cpg_clk_register(). But, hmm.
>
>
> What do you think? Opinions? Examples?

Please extend enum r8a7795_clk_types, and handle them in
r8a7795_cpg_clk_register() based on the enum values.

You can find an (old) example in the prototype I did for r8a7791:
https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/tree/drivers/clk/shmobile/clk-r8a7791-cpg-mssr.c?h=topic/cpg-mssr-v4

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2] clk: shmobile: r8a7795: Add SDHI clocks

[Dirk Behme]

On 20.01.2016 09:16, Geert Uytterhoeven wrote:
> Hi Dirk,
>
> On Sun, Jan 17, 2016 at 10:47 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
>> On 15.01.2016 11:20, Geert Uytterhoeven wrote:
>>> On Fri, Jan 15, 2016 at 11:11 AM, Dirk Behme <dirk.behme@de.bosch.com>
>>> wrote:
>>>> On 14.01.2016 19:24, Geert Uytterhoeven wrote:
>>>>> On Sat, Jan 9, 2016 at 7:42 AM, Dirk Behme <dirk.behme@gmail.com> wrote:
>>>>>> On 24.12.2015 11:09, Dirk Behme wrote:
>>>>>>> Add R8A7795 SDHI clocks.
>>>>>>> Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
>>>>>>> ---
>>>>>>> Changes in v2: Add the missing *H clocks and correct the dividers.
>
>>>>> The dividers for these clocks are not fixed, they are controlled by the
>>>>> SDnCKCR registers.
>>>>>
>>>>> Unfortunately the register layout is more complicated than on R-Car
>>>>> Gen2,
>>>>> so
>>>>> you can no longer use clk_register_divider_table(), but have to write a
>>>>> custom
>>>>> clock driver.
>>>>>
>>>>> For an initial version, a simple "read-only" version that just calls
>>>>> clk_register_fixed_factor() with divider values read from the hardware
>>>>> registers may be good enough. But for full support, you need a driver
>>>>> that
>>>>> can program the registers, too.
>>>>
>>>> Anything like
>>>>
>>>> https://git.kernel.org/cgit/linux/kernel/git/horms/renesas-bsp.git/commit/drivers/clk/shmobile/clk-rcar-gen3.c?h=v4.2/rcar-3.0.x&idÍ10385afc15cef6bfbaea4aa5da41193b24fe82
>>
>> I've had a look to that.
>>
>> I think we can pick all the functions
>>
>> +static const struct clk_ops cpg_sd_clock_ops = {
>> +       .enable = cpg_sd_clock_enable,
>> +       .disable = cpg_sd_clock_disable,
>> +       .is_enabled = cpg_sd_clock_is_enabled,
>> +       .recalc_rate = cpg_sd_clock_recalc_rate,
>> +       .round_rate = cpg_sd_clock_round_rate,
>> +       .set_rate = cpg_sd_clock_set_rate,
>> +};
>>
>> unchanged from that patch.
>>
>> However, I'm not sure how to interface this to cpg_mssr_probe()? It's
>> similar to cpg_mssr_register_mod_clk(), but not identical so that this could
>> be reused.
>>
>> We could extend r8a7795_cpg_mssr_info by anything like an additional
>>
>> /* Dynamic clocks */
>> .dyn_clks = /* Add an array allowing to pass various clk_ops structs */
>> ...
>>
>> ?
>>
>> Or we could hard code it like
>>
>> https://git.kernel.org/cgit/linux/kernel/git/horms/renesas-bsp.git/commit/drivers/clk/shmobile/clk-rcar-gen3.c?h=v4.2/rcar-3.0.x&idÍ10385afc15cef6bfbaea4aa5da41193b24fe82
>>
>> is doing it with
>>
>> +       } else if (!strcmp(name, "sd0")) {
>> +               return cpg_sd_clk_register(name, cpg->reg + CPG_SD0CKCR,
>> np);
>> +       } else if (!strcmp(name, "sd1")) {
>> +               return cpg_sd_clk_register(name, cpg->reg + CPG_SD1CKCR,
>> np);
>> +       } else if (!strcmp(name, "sd2")) {
>> +               return cpg_sd_clk_register(name, cpg->reg + CPG_SD2CKCR,
>> np);
>> +       } else if (!strcmp(name, "sd3")) {
>> +               return cpg_sd_clk_register(name, cpg->reg + CPG_SD3CKCR,
>> np);
>>
>> and add this to e.g. r8a7795_cpg_clk_register(). But, hmm.
>>
>>
>> What do you think? Opinions? Examples?
>
> Please extend enum r8a7795_clk_types, and handle them in
> r8a7795_cpg_clk_register() based on the enum values.
>
> You can find an (old) example in the prototype I did for r8a7791:
> https://git.kernel.org/cgit/linux/kernel/git/geert/renesas-drivers.git/tree/drivers/clk/shmobile/clk-r8a7791-cpg-mssr.c?h=topic/cpg-mssr-v4


Whats about anything like

http://marc.info/?l=linux-sh&m\x145346559429931

?

Best regards

Dirk

[PATCH v2] clk: shmobile: r8a7795: Add SD divider support

[Dirk Behme]

This patch adds SD[0..3] clock divider support for R-Car Gen3 SoC.

Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---
Changes in v2: Incorporate review comments and merge with
               "clk: shmobile: r8a7795: Add SDHI clocks"

 drivers/clk/shmobile/r8a7795-cpg-mssr.c | 230 ++++++++++++++++++++++++++++++++
 drivers/clk/shmobile/renesas-cpg-mssr.h |   2 +
 2 files changed, 232 insertions(+)

diff --git a/drivers/clk/shmobile/r8a7795-cpg-mssr.c b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
index 23e98fe..9bf7035 100644
--- a/drivers/clk/shmobile/r8a7795-cpg-mssr.c
+++ b/drivers/clk/shmobile/r8a7795-cpg-mssr.c
@@ -20,6 +20,7 @@
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/of.h>
+#include <linux/slab.h>
 
 #include <dt-bindings/clock/r8a7795-cpg-mssr.h>
 
@@ -61,6 +62,7 @@ enum r8a7795_clk_types {
 	CLK_TYPE_GEN3_PLL2,
 	CLK_TYPE_GEN3_PLL3,
 	CLK_TYPE_GEN3_PLL4,
+	CLK_TYPE_GEN3_SD,
 };
 
 static const struct cpg_core_clk r8a7795_core_clks[] __initconst = {
@@ -99,6 +101,12 @@ static const struct cpg_core_clk r8a7795_core_clks[] __initconst = {
 	DEF_FIXED("s3d1",       R8A7795_CLK_S3D1,  CLK_S3,         1, 1),
 	DEF_FIXED("s3d2",       R8A7795_CLK_S3D2,  CLK_S3,         2, 1),
 	DEF_FIXED("s3d4",       R8A7795_CLK_S3D4,  CLK_S3,         4, 1),
+
+	DEF_SD("sd0",           R8A7795_CLK_SD0,   CLK_PLL1_DIV2, 0x0074),
+	DEF_SD("sd1",           R8A7795_CLK_SD1,   CLK_PLL1_DIV2, 0x0078),
+	DEF_SD("sd2",           R8A7795_CLK_SD2,   CLK_PLL1_DIV2, 0x0268),
+	DEF_SD("sd3",           R8A7795_CLK_SD3,   CLK_PLL1_DIV2, 0x026c),
+
 	DEF_FIXED("cl",         R8A7795_CLK_CL,    CLK_PLL1_DIV2, 48, 1),
 	DEF_FIXED("cp",         R8A7795_CLK_CP,    CLK_EXTAL,      2, 1),
 
@@ -120,6 +128,10 @@ static const struct mssr_mod_clk r8a7795_mod_clks[] __initconst = {
 	DEF_MOD("sys-dmac1",		 218,	R8A7795_CLK_S3D1),
 	DEF_MOD("sys-dmac0",		 219,	R8A7795_CLK_S3D1),
 	DEF_MOD("scif2",		 310,	R8A7795_CLK_S3D4),
+	DEF_MOD("sdif3",	       	 311,	R8A7795_CLK_SD3),
+	DEF_MOD("sdif2",	       	 312,	R8A7795_CLK_SD2),
+	DEF_MOD("sdif1",	      	 313,	R8A7795_CLK_SD1),
+	DEF_MOD("sdif0",	     	 314,	R8A7795_CLK_SD0),
 	DEF_MOD("pcie1",		 318,	R8A7795_CLK_S3D1),
 	DEF_MOD("pcie0",		 319,	R8A7795_CLK_S3D1),
 	DEF_MOD("intc-ap",		 408,	R8A7795_CLK_S3D1),
@@ -199,6 +211,221 @@ static const unsigned int r8a7795_crit_mod_clks[] __initconst = {
 	MOD_CLK_ID(408),	/* INTC-AP (GIC) */
 };
 
+/* -----------------------------------------------------------------------------
+ * SDn Clock
+ *
+ */
+#define CPG_SD_STP_HCK		BIT(9)
+#define CPG_SD_STP_CK		BIT(8)
+
+#define CPG_SD_STP_MASK		(CPG_SD_STP_HCK | CPG_SD_STP_CK)
+#define CPG_SD_FC_MASK		(0x7 << 2 | 0x3 << 0)
+
+#define CPG_SD_DIV_TABLE_DATA(stp_hck, stp_ck, sd_srcfc, sd_fc, sd_div) \
+{ \
+	.val = ((stp_hck) ? CPG_SD_STP_HCK : 0) | \
+	       ((stp_ck) ? CPG_SD_STP_CK : 0) | \
+	       ((sd_srcfc) << 2) | \
+	       ((sd_fc) << 0), \
+	.div = (sd_div), \
+}
+
+struct sd_div_table {
+	u32 val;
+	unsigned int div;
+};
+
+struct sd_clock {
+	struct clk_hw hw;
+	void __iomem *reg;
+	const struct sd_div_table *div_table;
+	unsigned int div_num;
+	unsigned int div_min;
+	unsigned int div_max;
+};
+
+/* SDn divider
+ *                     sd_srcfc   sd_fc   div
+ * stp_hck   stp_ck    (div)      (div)     = sd_srcfc x sd_fc
+ *-------------------------------------------------------------------
+ *  0         0         0 (1)      1 (4)      4
+ *  0         0         1 (2)      1 (4)      8
+ *  1         0         2 (4)      1 (4)     16
+ *  1         0         3 (8)      1 (4)     32
+ *  1         0         4 (16)     1 (4)     64
+ *  0         0         0 (1)      0 (2)      2
+ *  0         0         1 (2)      0 (2)      4
+ *  1         0         2 (4)      0 (2)      8
+ *  1         0         3 (8)      0 (2)     16
+ *  1         0         4 (16)     0 (2)     32
+ */
+static const struct sd_div_table cpg_sd_div_table[] = {
+/*	CPG_SD_DIV_TABLE_DATA(stp_hck,  stp_ck,   sd_srcfc,   sd_fc,  sd_div) */
+	CPG_SD_DIV_TABLE_DATA(0,        0,        0,          1,        4),
+	CPG_SD_DIV_TABLE_DATA(0,        0,        1,          1,        8),
+	CPG_SD_DIV_TABLE_DATA(1,        0,        2,          1,       16),
+	CPG_SD_DIV_TABLE_DATA(1,        0,        3,          1,       32),
+	CPG_SD_DIV_TABLE_DATA(1,        0,        4,          1,       64),
+	CPG_SD_DIV_TABLE_DATA(0,        0,        0,          0,        2),
+	CPG_SD_DIV_TABLE_DATA(0,        0,        1,          0,        4),
+	CPG_SD_DIV_TABLE_DATA(1,        0,        2,          0,        8),
+	CPG_SD_DIV_TABLE_DATA(1,        0,        3,          0,       16),
+	CPG_SD_DIV_TABLE_DATA(1,        0,        4,          0,       32),
+};
+
+#define to_sd_clock(_hw) container_of(_hw, struct sd_clock, hw)
+
+static int cpg_sd_clock_enable(struct clk_hw *hw)
+{
+	struct sd_clock *clock = to_sd_clock(hw);
+	u32 val, sd_fc;
+	unsigned int i;
+
+	val = clk_readl(clock->reg);
+
+	sd_fc = val & CPG_SD_FC_MASK;
+	for (i = 0; i < clock->div_num; i++)
+		if (sd_fc = (clock->div_table[i].val & CPG_SD_FC_MASK))
+			break;
+
+	if (i >= clock->div_num)
+		return -EINVAL;
+
+	val &= ~(CPG_SD_STP_MASK);
+	val |= clock->div_table[i].val & CPG_SD_STP_MASK;
+
+	clk_writel(val, clock->reg);
+
+	return 0;
+}
+
+static void cpg_sd_clock_disable(struct clk_hw *hw)
+{
+	struct sd_clock *clock = to_sd_clock(hw);
+
+	clk_writel(clk_readl(clock->reg) | CPG_SD_STP_MASK, clock->reg);
+}
+
+static int cpg_sd_clock_is_enabled(struct clk_hw *hw)
+{
+	struct sd_clock *clock = to_sd_clock(hw);
+
+	return !(clk_readl(clock->reg) & CPG_SD_STP_MASK);
+}
+
+static unsigned long cpg_sd_clock_recalc_rate(struct clk_hw *hw,
+						unsigned long parent_rate)
+{
+	struct sd_clock *clock = to_sd_clock(hw);
+	unsigned long rate = parent_rate;
+	u32 val, sd_fc;
+	unsigned int i;
+
+	val = clk_readl(clock->reg);
+
+	sd_fc = val & CPG_SD_FC_MASK;
+	for (i = 0; i < clock->div_num; i++)
+		if (sd_fc = (clock->div_table[i].val & CPG_SD_FC_MASK))
+			break;
+
+	if (i >= clock->div_num)
+		return -EINVAL;
+
+	return DIV_ROUND_CLOSEST(rate, clock->div_table[i].div);
+}
+
+static unsigned int cpg_sd_clock_calc_div(struct sd_clock *clock,
+					  unsigned long rate,
+					  unsigned long parent_rate)
+{
+	unsigned int div;
+
+	if (!rate)
+		rate = 1;
+
+	div = DIV_ROUND_CLOSEST(parent_rate, rate);
+
+	return clamp_t(unsigned int, div, clock->div_min, clock->div_max);
+}
+
+static long cpg_sd_clock_round_rate(struct clk_hw *hw, unsigned long rate,
+				      unsigned long *parent_rate)
+{
+	struct sd_clock *clock = to_sd_clock(hw);
+	unsigned int div = cpg_sd_clock_calc_div(clock, rate, *parent_rate);
+
+	return DIV_ROUND_CLOSEST(*parent_rate, div);
+}
+
+static int cpg_sd_clock_set_rate(struct clk_hw *hw, unsigned long rate,
+				   unsigned long parent_rate)
+{
+	struct sd_clock *clock = to_sd_clock(hw);
+	unsigned int div = cpg_sd_clock_calc_div(clock, rate, parent_rate);
+	u32 val;
+	unsigned int i;
+
+	for (i = 0; i < clock->div_num; i++)
+		if (div = clock->div_table[i].div)
+			break;
+
+	if (i >= clock->div_num)
+		return -EINVAL;
+
+	val = clk_readl(clock->reg);
+	val &= ~(CPG_SD_STP_MASK | CPG_SD_FC_MASK);
+	val |= clock->div_table[i].val & (CPG_SD_STP_MASK | CPG_SD_FC_MASK);
+	clk_writel(val, clock->reg);
+
+	return 0;
+}
+
+static const struct clk_ops cpg_sd_clock_ops = {
+	.enable = cpg_sd_clock_enable,
+	.disable = cpg_sd_clock_disable,
+	.is_enabled = cpg_sd_clock_is_enabled,
+	.recalc_rate = cpg_sd_clock_recalc_rate,
+	.round_rate = cpg_sd_clock_round_rate,
+	.set_rate = cpg_sd_clock_set_rate,
+};
+
+static struct clk * __init cpg_sd_clk_register(const struct cpg_core_clk *core,
+					       void __iomem *base,
+					       const char *parent_name)
+{
+	struct clk_init_data init;
+	struct sd_clock *clock;
+	struct clk *clk;
+	unsigned int i;
+
+	clock = kzalloc(sizeof(*clock), GFP_KERNEL);
+	if (!clock)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = core->name;
+	init.ops = &cpg_sd_clock_ops;
+	init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	clock->reg = base + core->offset;
+	clock->hw.init = &init;
+	clock->div_table = cpg_sd_div_table;
+	clock->div_num = ARRAY_SIZE(cpg_sd_div_table);
+
+	clock->div_max = clock->div_table[0].div;
+	clock->div_min = clock->div_max;
+	for (i = 1; i < clock->div_num; i++) {
+		clock->div_max = max(clock->div_max, clock->div_table[i].div);
+		clock->div_min = min(clock->div_min, clock->div_table[i].div);
+	}
+
+	clk = clk_register(NULL, &clock->hw);
+	if (IS_ERR(clk))
+		kfree(clock);
+
+	return clk;
+}
 
 #define CPG_PLL0CR	0x00d8
 #define CPG_PLL2CR	0x002c
@@ -324,6 +551,9 @@ struct clk * __init r8a7795_cpg_clk_register(struct device *dev,
 		mult = (((value >> 24) & 0x7f) + 1) * 2;
 		break;
 
+	case CLK_TYPE_GEN3_SD:
+		return cpg_sd_clk_register(core, base, __clk_get_name(parent));
+
 	default:
 		return ERR_PTR(-EINVAL);
 	}
diff --git a/drivers/clk/shmobile/renesas-cpg-mssr.h b/drivers/clk/shmobile/renesas-cpg-mssr.h
index e09f03c..952b695 100644
--- a/drivers/clk/shmobile/renesas-cpg-mssr.h
+++ b/drivers/clk/shmobile/renesas-cpg-mssr.h
@@ -53,6 +53,8 @@ enum clk_types {
 	DEF_BASE(_name, _id, CLK_TYPE_FF, _parent, .div = _div, .mult = _mult)
 #define DEF_DIV6P1(_name, _id, _parent, _offset)	\
 	DEF_BASE(_name, _id, CLK_TYPE_DIV6P1, _parent, .offset = _offset)
+#define DEF_SD(_name, _id, _parent, _offset)	\
+	DEF_BASE(_name, _id, CLK_TYPE_GEN3_SD, _parent, .offset = _offset)
 
 
     /*
-- 
2.7.0

Re: [PATCH v2] clk: shmobile: r8a7795: Add SD divider support

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]

On Sat, Jan 30, 2016 at 07:33:59AM +0100, Dirk Behme wrote:
> This patch adds SD[0..3] clock divider support for R-Car Gen3 SoC.
> 
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>

Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Since we don't have UHS support currently, changing the clock is not
really exercised; but the code looks good and if it needs fixing, we can
do it incrementally IMO. What is currently needed works fine on my
Salvator-X for the exposed SD interfaces.

Thanks,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]