Re: [PATCH v2, RESEND] clk: uniphier: add clock drivers for UniPhier SoCs

2016-05-02 Thread Masahiro Yamada
Hi Michael,

I am back after long pause.

2016-01-04 17:36 GMT+09:00 Michael Turquette :
> Quoting Masahiro Yamada (2016-01-01 17:26:05)
>> Hi Michael,
>>
>>
>> 2015-12-31 10:35 GMT+09:00 Michael Turquette :
>> > Hello Yamada-san,
>> >
>> > Quoting Masahiro Yamada (2015-12-28 02:20:58)
>> >> diff --git a/drivers/clk/uniphier/Kconfig b/drivers/clk/uniphier/Kconfig
>> >> new file mode 100644
>> >> index 000..7606f27
>> >> --- /dev/null
>> >> +++ b/drivers/clk/uniphier/Kconfig
>> >> @@ -0,0 +1,35 @@
>> >> +menuconfig CLK_UNIPHIER
>> >> +   bool "Clock drivers for UniPhier SoCs"
>> >> +   depends on ARCH_UNIPHIER
>> >
>> > Might want to make the above line:
>> >
>> > depends on ARCH_UNIPHIER || COMPILE_TEST
>> >
>> >> +   depends on OF
>> >> +   default y
>> >> +   help
>> >> + Supports clock drivers for UniPhier SoCs.
>> >
>> > Why menuconfig? Can we make this a non-visible config option selected by
>> > the UniPhier platform?
>>
>> Yes, I can do that if you like.
>
> Thanks, and the other questions I asked in my previous reply are
> relevant here: can you make these into platform_drivers and compile them
> as loadable modules? That would be ideal.


I noticed a conflict between two of your requests.

If I have CLK_UNIPHIER selected by ARCH_UNIPHIER,
it is impossible to add "depends on COMPILE_TEST".


I see some drivers that have "depends on ARCH_ || COMPILE_TEST"
as follows:


config COMMON_CLK_HI6220
bool "Hi6220 Clock Driver"
depends on ARCH_HISI || COMPILE_TEST

config COMMON_CLK_QCOM
tristate "Support for Qualcomm's clock controllers"
depends on OF
depends on ARCH_QCOM || COMPILE_TEST


I think the style above works fine with COMPILE_TEST.


But, if we make it a non-visible config option selected by ARCH,
how can COMPILE_TEST select it?


config COMMON_CLK_HI6220
bool
prompt "Hi6220 Clock Driver" if COMPILE_TEST

should work, but looks a bit tricky.   Hmm...






>> >
>> > Where can I find the DT binding description and DTS for this clock
>> > controller?
>
> Any update on the above question? I wasn't able to find the DT binding
> description anywhere.


OK, will add a binding document in the next version.



>>
>> So, I implemented uniphier_clk_update_parent_name()
>> to convert the clock-names into the parent clock name.
>> In this case, this function takes "input" and converts it into "for-bar123".
>>
>>
>> Is there any good idea to solve this problem?
>
> Well, using the clock-output-names DT property in conjunction with
> of_clk_get_parent_name() solves this for you in a common way. However I
> would like to get rid of clock-output-names in the future. In general
> I'm working slowy to reduce the reliance on clock string names, but it
> is a long-term side project.


I do not intend to add clock-output-names in my device trees.

of_clk_get_parent_name() tries best to take the parent name via
of_clk_get_from_provider() if clock-output-names is missing.
I wanted to use this feature.



> How bad is it for you to hard-code the string names in your driver? Are
> the foo, bar and baz clock providers all on the same chip?

No problem, but not 100% happy.

If we allow to hardcode the parent name strings,
"clock-names" and "clocks" properties are useless in clock-cascading.

Like IRQ controllers can be cascaded by "interrupts" chain,
I want to describe the hierarchy of clock-providers
without hard-coding the detail of parent.



-- 
Best Regards
Masahiro Yamada


Re: [PATCH v2, RESEND] clk: uniphier: add clock drivers for UniPhier SoCs

2016-05-02 Thread Masahiro Yamada
Hi Michael,

I am back after long pause.

2016-01-04 17:36 GMT+09:00 Michael Turquette :
> Quoting Masahiro Yamada (2016-01-01 17:26:05)
>> Hi Michael,
>>
>>
>> 2015-12-31 10:35 GMT+09:00 Michael Turquette :
>> > Hello Yamada-san,
>> >
>> > Quoting Masahiro Yamada (2015-12-28 02:20:58)
>> >> diff --git a/drivers/clk/uniphier/Kconfig b/drivers/clk/uniphier/Kconfig
>> >> new file mode 100644
>> >> index 000..7606f27
>> >> --- /dev/null
>> >> +++ b/drivers/clk/uniphier/Kconfig
>> >> @@ -0,0 +1,35 @@
>> >> +menuconfig CLK_UNIPHIER
>> >> +   bool "Clock drivers for UniPhier SoCs"
>> >> +   depends on ARCH_UNIPHIER
>> >
>> > Might want to make the above line:
>> >
>> > depends on ARCH_UNIPHIER || COMPILE_TEST
>> >
>> >> +   depends on OF
>> >> +   default y
>> >> +   help
>> >> + Supports clock drivers for UniPhier SoCs.
>> >
>> > Why menuconfig? Can we make this a non-visible config option selected by
>> > the UniPhier platform?
>>
>> Yes, I can do that if you like.
>
> Thanks, and the other questions I asked in my previous reply are
> relevant here: can you make these into platform_drivers and compile them
> as loadable modules? That would be ideal.


I noticed a conflict between two of your requests.

If I have CLK_UNIPHIER selected by ARCH_UNIPHIER,
it is impossible to add "depends on COMPILE_TEST".


I see some drivers that have "depends on ARCH_ || COMPILE_TEST"
as follows:


config COMMON_CLK_HI6220
bool "Hi6220 Clock Driver"
depends on ARCH_HISI || COMPILE_TEST

config COMMON_CLK_QCOM
tristate "Support for Qualcomm's clock controllers"
depends on OF
depends on ARCH_QCOM || COMPILE_TEST


I think the style above works fine with COMPILE_TEST.


But, if we make it a non-visible config option selected by ARCH,
how can COMPILE_TEST select it?


config COMMON_CLK_HI6220
bool
prompt "Hi6220 Clock Driver" if COMPILE_TEST

should work, but looks a bit tricky.   Hmm...






>> >
>> > Where can I find the DT binding description and DTS for this clock
>> > controller?
>
> Any update on the above question? I wasn't able to find the DT binding
> description anywhere.


OK, will add a binding document in the next version.



>>
>> So, I implemented uniphier_clk_update_parent_name()
>> to convert the clock-names into the parent clock name.
>> In this case, this function takes "input" and converts it into "for-bar123".
>>
>>
>> Is there any good idea to solve this problem?
>
> Well, using the clock-output-names DT property in conjunction with
> of_clk_get_parent_name() solves this for you in a common way. However I
> would like to get rid of clock-output-names in the future. In general
> I'm working slowy to reduce the reliance on clock string names, but it
> is a long-term side project.


I do not intend to add clock-output-names in my device trees.

of_clk_get_parent_name() tries best to take the parent name via
of_clk_get_from_provider() if clock-output-names is missing.
I wanted to use this feature.



> How bad is it for you to hard-code the string names in your driver? Are
> the foo, bar and baz clock providers all on the same chip?

No problem, but not 100% happy.

If we allow to hardcode the parent name strings,
"clock-names" and "clocks" properties are useless in clock-cascading.

Like IRQ controllers can be cascaded by "interrupts" chain,
I want to describe the hierarchy of clock-providers
without hard-coding the detail of parent.



-- 
Best Regards
Masahiro Yamada


Re: [PATCH v2,RESEND] clk: uniphier: add clock drivers for UniPhier SoCs

2016-01-01 Thread Masahiro Yamada
Hi Michael,


2015-12-31 10:35 GMT+09:00 Michael Turquette :
> Hello Yamada-san,
>
> Quoting Masahiro Yamada (2015-12-28 02:20:58)
>> diff --git a/drivers/clk/uniphier/Kconfig b/drivers/clk/uniphier/Kconfig
>> new file mode 100644
>> index 000..7606f27
>> --- /dev/null
>> +++ b/drivers/clk/uniphier/Kconfig
>> @@ -0,0 +1,35 @@
>> +menuconfig CLK_UNIPHIER
>> +   bool "Clock drivers for UniPhier SoCs"
>> +   depends on ARCH_UNIPHIER
>
> Might want to make the above line:
>
> depends on ARCH_UNIPHIER || COMPILE_TEST
>
>> +   depends on OF
>> +   default y
>> +   help
>> + Supports clock drivers for UniPhier SoCs.
>
> Why menuconfig? Can we make this a non-visible config option selected by
> the UniPhier platform?

Yes, I can do that if you like.

Do you prefer to making it flat, like all clocks are shown in the
"Common Clock Framework" menu?

Or, is it better to categorize SoC clocks with "menu" for each SoC family?



>> +
>> +if CLK_UNIPHIER
>
> Please drop the if statement here and have the below options 'depends'
> on CLK_UNIPHIER.

Why is it bad?

I think surrounding all the options with "if CLK_UNIPHIER" ... "endif"
is easier than adding "depends on CLK_UNIPHIER" to each of them.



>> +
>> +config CLK_UNIPHIER_PH1_SLD3
>> +   bool "Clock driver for UniPhier PH1-sLD3 SoC"
>> +   default y
>
> Can you make these drivers into loadable modules? If so you might
> consider tristate.

For some, I can.
For some, I can't (because they must provide an input clock for the timer.)


>> +
>> +static void __init ph1_ld4_clk_init(struct device_node *np)
>> +{
>> +   uniphier_clk_init(np, ph1_ld4_clk_idata);
>> +}
>> +CLK_OF_DECLARE(ph1_ld4_clk, "socionext,ph1-ld4-sysctrl", ph1_ld4_clk_init);
>
> Can you avoid using CLK_OF_DECLARE and instead make this into a
> platform_driver? For examples please see drivers/clk/qcom*.c


For clk-uniphier-peri.c and clk-uniphier-mio.c, yes.
They provide clocks for peripheral devices like USB, MMC, etc., so I
can delay them.


For clk-ph1-*.c, I am afraid I can't.
They provide an input clock for the timer (ARM global timer).
I think they should be initialized very early.


Is platform_driver better than CLK_OF_DECLARE() where it is possible?

I just chose CLK_OF_DECLARE() for consistency
and it seems the majority.



>> +
>> +static void __init uniphier_clk_update_parent_name(struct device_node *np,
>> +  const char **parent_name)
>> +{
>> +   const char *new_name;
>> +   int index;
>> +
>> +   if (!parent_name || !*parent_name)
>> +   return;
>> +
>> +   if (strncmp(*parent_name, UNIPHIER_CLK_EXT, 
>> strlen(UNIPHIER_CLK_EXT)))
>> +   return;
>> +
>> +   index = of_property_match_string(np, "clock-names",
>> +   *parent_name + strlen(UNIPHIER_CLK_EXT));
>> +   new_name = of_clk_get_parent_name(np, index);
>> +   if (new_name)
>> +   *parent_name = new_name;
>
> Where can I find the DT binding description and DTS for this clock
> controller? I'm confused why the parent name function is necessary and
> I'd like to see the data.


clk_register() needs the name strings of parent clocks,
not pointers to parent clocks.
(It looks a bit odd to me, although it allows us to register clocks in
any order.
In stead, we have to think about orphan-walking.)
So, we need to exactly know the names of parent clocks.

Let's assume the clock system that consists of some hardware blocks.


  root_clk: root_clk {
compatible = "foo-clk";
reg = <0x1000 0x1000>;
#clock-cells = <1>;
  };

  bar_clk {
compatible = "bar-clk";
reg = <0x2000 0x1000>;
#clock-cells = <1>;
clock-names = "input"
clocks = <_clk 0>;
   };

  baz_clk {
compatible = "baz-clk";
reg = <0x3000 0x1000>;
#clock-cells = <1>;
clock-names = "input"
clocks = <_clk 1>;
   };



The "bar_clk" is a clock provider for other peripherals,
and also a consumer of "root_clk".

The "bar_clk" wants to know the name of index 0 of root_clk
because it is needed when calling clk_register().


Let's say the names of clocks provided root_clk are, "for-bar123", "for-baz456".

The "bar_clk" needs the string "for-bar123", but it does not know.
(Likewise, the "baz_clk" needs the string "for-baz456", but it does not know.)

Of course, I can hard-code "for-bar123" in the driver of "bar_clk",
but I think it is annoying to keep the name synced between the
provider and the consumer.

So, I implemented uniphier_clk_update_parent_name()
to convert the clock-names into the parent clock name.
In this case, this function takes "input" and converts it into "for-bar123".


Is there any good idea to solve this problem?


-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to 

Re: [PATCH v2,RESEND] clk: uniphier: add clock drivers for UniPhier SoCs

2016-01-01 Thread Masahiro Yamada
Hi Michael,


2015-12-31 10:35 GMT+09:00 Michael Turquette :
> Hello Yamada-san,
>
> Quoting Masahiro Yamada (2015-12-28 02:20:58)
>> diff --git a/drivers/clk/uniphier/Kconfig b/drivers/clk/uniphier/Kconfig
>> new file mode 100644
>> index 000..7606f27
>> --- /dev/null
>> +++ b/drivers/clk/uniphier/Kconfig
>> @@ -0,0 +1,35 @@
>> +menuconfig CLK_UNIPHIER
>> +   bool "Clock drivers for UniPhier SoCs"
>> +   depends on ARCH_UNIPHIER
>
> Might want to make the above line:
>
> depends on ARCH_UNIPHIER || COMPILE_TEST
>
>> +   depends on OF
>> +   default y
>> +   help
>> + Supports clock drivers for UniPhier SoCs.
>
> Why menuconfig? Can we make this a non-visible config option selected by
> the UniPhier platform?

Yes, I can do that if you like.

Do you prefer to making it flat, like all clocks are shown in the
"Common Clock Framework" menu?

Or, is it better to categorize SoC clocks with "menu" for each SoC family?



>> +
>> +if CLK_UNIPHIER
>
> Please drop the if statement here and have the below options 'depends'
> on CLK_UNIPHIER.

Why is it bad?

I think surrounding all the options with "if CLK_UNIPHIER" ... "endif"
is easier than adding "depends on CLK_UNIPHIER" to each of them.



>> +
>> +config CLK_UNIPHIER_PH1_SLD3
>> +   bool "Clock driver for UniPhier PH1-sLD3 SoC"
>> +   default y
>
> Can you make these drivers into loadable modules? If so you might
> consider tristate.

For some, I can.
For some, I can't (because they must provide an input clock for the timer.)


>> +
>> +static void __init ph1_ld4_clk_init(struct device_node *np)
>> +{
>> +   uniphier_clk_init(np, ph1_ld4_clk_idata);
>> +}
>> +CLK_OF_DECLARE(ph1_ld4_clk, "socionext,ph1-ld4-sysctrl", ph1_ld4_clk_init);
>
> Can you avoid using CLK_OF_DECLARE and instead make this into a
> platform_driver? For examples please see drivers/clk/qcom*.c


For clk-uniphier-peri.c and clk-uniphier-mio.c, yes.
They provide clocks for peripheral devices like USB, MMC, etc., so I
can delay them.


For clk-ph1-*.c, I am afraid I can't.
They provide an input clock for the timer (ARM global timer).
I think they should be initialized very early.


Is platform_driver better than CLK_OF_DECLARE() where it is possible?

I just chose CLK_OF_DECLARE() for consistency
and it seems the majority.



>> +
>> +static void __init uniphier_clk_update_parent_name(struct device_node *np,
>> +  const char **parent_name)
>> +{
>> +   const char *new_name;
>> +   int index;
>> +
>> +   if (!parent_name || !*parent_name)
>> +   return;
>> +
>> +   if (strncmp(*parent_name, UNIPHIER_CLK_EXT, 
>> strlen(UNIPHIER_CLK_EXT)))
>> +   return;
>> +
>> +   index = of_property_match_string(np, "clock-names",
>> +   *parent_name + strlen(UNIPHIER_CLK_EXT));
>> +   new_name = of_clk_get_parent_name(np, index);
>> +   if (new_name)
>> +   *parent_name = new_name;
>
> Where can I find the DT binding description and DTS for this clock
> controller? I'm confused why the parent name function is necessary and
> I'd like to see the data.


clk_register() needs the name strings of parent clocks,
not pointers to parent clocks.
(It looks a bit odd to me, although it allows us to register clocks in
any order.
In stead, we have to think about orphan-walking.)
So, we need to exactly know the names of parent clocks.

Let's assume the clock system that consists of some hardware blocks.


  root_clk: root_clk {
compatible = "foo-clk";
reg = <0x1000 0x1000>;
#clock-cells = <1>;
  };

  bar_clk {
compatible = "bar-clk";
reg = <0x2000 0x1000>;
#clock-cells = <1>;
clock-names = "input"
clocks = <_clk 0>;
   };

  baz_clk {
compatible = "baz-clk";
reg = <0x3000 0x1000>;
#clock-cells = <1>;
clock-names = "input"
clocks = <_clk 1>;
   };



The "bar_clk" is a clock provider for other peripherals,
and also a consumer of "root_clk".

The "bar_clk" wants to know the name of index 0 of root_clk
because it is needed when calling clk_register().


Let's say the names of clocks provided root_clk are, "for-bar123", "for-baz456".

The "bar_clk" needs the string "for-bar123", but it does not know.
(Likewise, the "baz_clk" needs the string "for-baz456", but it does not know.)

Of course, I can hard-code "for-bar123" in the driver of "bar_clk",
but I think it is annoying to keep the name synced between the
provider and the consumer.

So, I implemented uniphier_clk_update_parent_name()
to convert the clock-names into the parent clock name.
In this case, this function takes "input" and converts it into "for-bar123".


Is there any good idea to solve this problem?


-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to 

Re: [PATCH v2,RESEND] clk: uniphier: add clock drivers for UniPhier SoCs

2015-12-30 Thread Michael Turquette
Hello Yamada-san,

Quoting Masahiro Yamada (2015-12-28 02:20:58)
> diff --git a/drivers/clk/uniphier/Kconfig b/drivers/clk/uniphier/Kconfig
> new file mode 100644
> index 000..7606f27
> --- /dev/null
> +++ b/drivers/clk/uniphier/Kconfig
> @@ -0,0 +1,35 @@
> +menuconfig CLK_UNIPHIER
> +   bool "Clock drivers for UniPhier SoCs"
> +   depends on ARCH_UNIPHIER

Might want to make the above line:

depends on ARCH_UNIPHIER || COMPILE_TEST

> +   depends on OF
> +   default y
> +   help
> + Supports clock drivers for UniPhier SoCs.

Why menuconfig? Can we make this a non-visible config option selected by
the UniPhier platform?

> +
> +if CLK_UNIPHIER

Please drop the if statement here and have the below options 'depends'
on CLK_UNIPHIER.

> +
> +config CLK_UNIPHIER_PH1_SLD3
> +   bool "Clock driver for UniPhier PH1-sLD3 SoC"
> +   default y

Can you make these drivers into loadable modules? If so you might
consider tristate.

> +
> +config CLK_UNIPHIER_PH1_LD4
> +   bool "Clock driver for UniPhier PH1-LD4 SoC"
> +   default y
> +
> +config CLK_UNIPHIER_PH1_PRO4
> +   bool "Clock driver for UniPhier PH1-Pro4 SoC"
> +   default y
> +
> +config CLK_UNIPHIER_PH1_SLD8
> +   bool "Clock driver for UniPhier PH1-sLD8 SoC"
> +   default y
> +
> +config CLK_UNIPHIER_PH1_PRO5
> +   bool "Clock driver for UniPhier PH1-Pro5 SoC"
> +   default y
> +
> +config CLK_UNIPHIER_PROXSTREAM2
> +   bool "Clock driver for UniPhier ProXstream2/PH1-LD6b SoC"
> +   default y
> +
> +endif
> diff --git a/drivers/clk/uniphier/Makefile b/drivers/clk/uniphier/Makefile
> new file mode 100644
> index 000..3be1a17
> --- /dev/null
> +++ b/drivers/clk/uniphier/Makefile
> @@ -0,0 +1,10 @@
> +obj-y += clk-uniphier-core.o
> +obj-y += clk-uniphier-peri.o
> +obj-y += clk-uniphier-mio.o
> +
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_SLD3)+= clk-ph1-sld3.o
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_LD4) += clk-ph1-ld4.o
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_PRO4)+= clk-ph1-pro4.o
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_SLD8)+= clk-ph1-sld8.o
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_PRO5)+= clk-ph1-pro5.o
> +obj-$(CONFIG_CLK_UNIPHIER_PROXSTREAM2) += clk-proxstream2.o
> diff --git a/drivers/clk/uniphier/clk-ph1-ld4.c 
> b/drivers/clk/uniphier/clk-ph1-ld4.c
> new file mode 100644
> index 000..48d342f
> --- /dev/null
> +++ b/drivers/clk/uniphier/clk-ph1-ld4.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright (C) 2015 Masahiro Yamada 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +
> +#include "clk-uniphier.h"
> +
> +static struct uniphier_clk_init_data ph1_ld4_clk_idata[] __initdata = {
> +   {
> +   .name = "spll",
> +   .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +   .output_index = -1,
> +   .data.factor = {
> +   .parent_name = UNIPHIER_CLK_EXT "ref",
> +   .mult = 65,
> +   .div = 1,
> +   },
> +   },
> +   {
> +   .name = "upll",
> +   .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +   .output_index = -1,
> +   .data.factor = {
> +   .parent_name = UNIPHIER_CLK_EXT "ref",
> +   .mult = 288000,
> +   .div = 24576,
> +   },
> +   },
> +   {
> +   .name = "a2pll",
> +   .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +   .output_index = -1,
> +   .data.factor = {
> +   .parent_name = UNIPHIER_CLK_EXT "ref",
> +   .mult = 24,
> +   .div = 1,
> +   },
> +   },
> +   {
> +   .name = "uart",
> +   .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +   .output_index = 3,
> +   .data.factor = {
> +   .parent_name = "a2pll",
> +   .mult = 1,
> +   .div = 16,
> +   },
> +   },
> +   {
> +   .name = "i2c",
> +   .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +   .output_index = 4,
> +   .data.factor = {
> +   .parent_name = "spll",
> +   .mult = 1,
> +   .div = 16,
> +   },
> +   },
> +   {
> +   .name = "arm-scu",
> +   .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +   

Re: [PATCH v2,RESEND] clk: uniphier: add clock drivers for UniPhier SoCs

2015-12-30 Thread Michael Turquette
Hello Yamada-san,

Quoting Masahiro Yamada (2015-12-28 02:20:58)
> diff --git a/drivers/clk/uniphier/Kconfig b/drivers/clk/uniphier/Kconfig
> new file mode 100644
> index 000..7606f27
> --- /dev/null
> +++ b/drivers/clk/uniphier/Kconfig
> @@ -0,0 +1,35 @@
> +menuconfig CLK_UNIPHIER
> +   bool "Clock drivers for UniPhier SoCs"
> +   depends on ARCH_UNIPHIER

Might want to make the above line:

depends on ARCH_UNIPHIER || COMPILE_TEST

> +   depends on OF
> +   default y
> +   help
> + Supports clock drivers for UniPhier SoCs.

Why menuconfig? Can we make this a non-visible config option selected by
the UniPhier platform?

> +
> +if CLK_UNIPHIER

Please drop the if statement here and have the below options 'depends'
on CLK_UNIPHIER.

> +
> +config CLK_UNIPHIER_PH1_SLD3
> +   bool "Clock driver for UniPhier PH1-sLD3 SoC"
> +   default y

Can you make these drivers into loadable modules? If so you might
consider tristate.

> +
> +config CLK_UNIPHIER_PH1_LD4
> +   bool "Clock driver for UniPhier PH1-LD4 SoC"
> +   default y
> +
> +config CLK_UNIPHIER_PH1_PRO4
> +   bool "Clock driver for UniPhier PH1-Pro4 SoC"
> +   default y
> +
> +config CLK_UNIPHIER_PH1_SLD8
> +   bool "Clock driver for UniPhier PH1-sLD8 SoC"
> +   default y
> +
> +config CLK_UNIPHIER_PH1_PRO5
> +   bool "Clock driver for UniPhier PH1-Pro5 SoC"
> +   default y
> +
> +config CLK_UNIPHIER_PROXSTREAM2
> +   bool "Clock driver for UniPhier ProXstream2/PH1-LD6b SoC"
> +   default y
> +
> +endif
> diff --git a/drivers/clk/uniphier/Makefile b/drivers/clk/uniphier/Makefile
> new file mode 100644
> index 000..3be1a17
> --- /dev/null
> +++ b/drivers/clk/uniphier/Makefile
> @@ -0,0 +1,10 @@
> +obj-y += clk-uniphier-core.o
> +obj-y += clk-uniphier-peri.o
> +obj-y += clk-uniphier-mio.o
> +
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_SLD3)+= clk-ph1-sld3.o
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_LD4) += clk-ph1-ld4.o
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_PRO4)+= clk-ph1-pro4.o
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_SLD8)+= clk-ph1-sld8.o
> +obj-$(CONFIG_CLK_UNIPHIER_PH1_PRO5)+= clk-ph1-pro5.o
> +obj-$(CONFIG_CLK_UNIPHIER_PROXSTREAM2) += clk-proxstream2.o
> diff --git a/drivers/clk/uniphier/clk-ph1-ld4.c 
> b/drivers/clk/uniphier/clk-ph1-ld4.c
> new file mode 100644
> index 000..48d342f
> --- /dev/null
> +++ b/drivers/clk/uniphier/clk-ph1-ld4.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright (C) 2015 Masahiro Yamada 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +
> +#include "clk-uniphier.h"
> +
> +static struct uniphier_clk_init_data ph1_ld4_clk_idata[] __initdata = {
> +   {
> +   .name = "spll",
> +   .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +   .output_index = -1,
> +   .data.factor = {
> +   .parent_name = UNIPHIER_CLK_EXT "ref",
> +   .mult = 65,
> +   .div = 1,
> +   },
> +   },
> +   {
> +   .name = "upll",
> +   .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +   .output_index = -1,
> +   .data.factor = {
> +   .parent_name = UNIPHIER_CLK_EXT "ref",
> +   .mult = 288000,
> +   .div = 24576,
> +   },
> +   },
> +   {
> +   .name = "a2pll",
> +   .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +   .output_index = -1,
> +   .data.factor = {
> +   .parent_name = UNIPHIER_CLK_EXT "ref",
> +   .mult = 24,
> +   .div = 1,
> +   },
> +   },
> +   {
> +   .name = "uart",
> +   .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +   .output_index = 3,
> +   .data.factor = {
> +   .parent_name = "a2pll",
> +   .mult = 1,
> +   .div = 16,
> +   },
> +   },
> +   {
> +   .name = "i2c",
> +   .type = UNIPHIER_CLK_TYPE_FIXED_FACTOR,
> +   .output_index = 4,
> +   .data.factor = {
> +   .parent_name = "spll",
> +   .mult = 1,
> +   .div = 16,
> +   },
> +   },
> +   {
> +   .name = "arm-scu",
> +   .type = 

[PATCH v2,RESEND] clk: uniphier: add clock drivers for UniPhier SoCs

2015-12-28 Thread Masahiro Yamada
This is the initial commit for the UniPhier clock drivers, including
support for PH1-sLD3, PH1-LD4, PH1-Pro4, PH1-sLD8, PH1-Pro5, and
ProXstream2/PH1-LD6b.

To improve the code maintainability, the driver consists of common
functions (clk-uniphier-core.c) and clock data arrays needed to
support each SoC.

Signed-off-by: Masahiro Yamada 
---

Changes in v2:
  - split emmc_hw_reset
  - make SD clock rate-controllable
  - add CLK_SET_RATE_PARENT flag to mux, gate, fixed-factor clocks

 MAINTAINERS  |   1 +
 drivers/clk/Kconfig  |   1 +
 drivers/clk/Makefile |   1 +
 drivers/clk/uniphier/Kconfig |  35 
 drivers/clk/uniphier/Makefile|  10 +
 drivers/clk/uniphier/clk-ph1-ld4.c   | 117 
 drivers/clk/uniphier/clk-ph1-pro4.c  | 117 
 drivers/clk/uniphier/clk-ph1-pro5.c  | 107 +++
 drivers/clk/uniphier/clk-ph1-sld3.c  | 117 
 drivers/clk/uniphier/clk-ph1-sld8.c  | 107 +++
 drivers/clk/uniphier/clk-proxstream2.c   |  88 +
 drivers/clk/uniphier/clk-uniphier-core.c | 151 +++
 drivers/clk/uniphier/clk-uniphier-mio.c  | 315 +++
 drivers/clk/uniphier/clk-uniphier-peri.c | 175 +
 drivers/clk/uniphier/clk-uniphier.h  |  68 +++
 15 files changed, 1410 insertions(+)
 create mode 100644 drivers/clk/uniphier/Kconfig
 create mode 100644 drivers/clk/uniphier/Makefile
 create mode 100644 drivers/clk/uniphier/clk-ph1-ld4.c
 create mode 100644 drivers/clk/uniphier/clk-ph1-pro4.c
 create mode 100644 drivers/clk/uniphier/clk-ph1-pro5.c
 create mode 100644 drivers/clk/uniphier/clk-ph1-sld3.c
 create mode 100644 drivers/clk/uniphier/clk-ph1-sld8.c
 create mode 100644 drivers/clk/uniphier/clk-proxstream2.c
 create mode 100644 drivers/clk/uniphier/clk-uniphier-core.c
 create mode 100644 drivers/clk/uniphier/clk-uniphier-mio.c
 create mode 100644 drivers/clk/uniphier/clk-uniphier-peri.c
 create mode 100644 drivers/clk/uniphier/clk-uniphier.h

diff --git a/MAINTAINERS b/MAINTAINERS
index fc08493..6c10e9b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1644,6 +1644,7 @@ F:arch/arm/mach-uniphier/
 F: arch/arm/mm/cache-uniphier.c
 F: arch/arm64/boot/dts/socionext/
 F: drivers/bus/uniphier-system-bus.c
+F: drivers/clk/uniphier/
 F: drivers/i2c/busses/i2c-uniphier*
 F: drivers/pinctrl/uniphier/
 F: drivers/tty/serial/8250/8250_uniphier.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index c3e3a02..7580323 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -191,6 +191,7 @@ config COMMON_CLK_CDCE706
 source "drivers/clk/bcm/Kconfig"
 source "drivers/clk/hisilicon/Kconfig"
 source "drivers/clk/qcom/Kconfig"
+source "drivers/clk/uniphier/Kconfig"
 
 endmenu
 
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 820714c..ab9d1bd 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_ARCH_STI)+= st/
 obj-$(CONFIG_ARCH_SUNXI)   += sunxi/
 obj-$(CONFIG_ARCH_TEGRA)   += tegra/
 obj-$(CONFIG_ARCH_OMAP2PLUS)   += ti/
+obj-$(CONFIG_CLK_UNIPHIER) += uniphier/
 obj-$(CONFIG_ARCH_U8500)   += ux500/
 obj-$(CONFIG_COMMON_CLK_VERSATILE) += versatile/
 obj-$(CONFIG_X86)  += x86/
diff --git a/drivers/clk/uniphier/Kconfig b/drivers/clk/uniphier/Kconfig
new file mode 100644
index 000..7606f27
--- /dev/null
+++ b/drivers/clk/uniphier/Kconfig
@@ -0,0 +1,35 @@
+menuconfig CLK_UNIPHIER
+   bool "Clock drivers for UniPhier SoCs"
+   depends on ARCH_UNIPHIER
+   depends on OF
+   default y
+   help
+ Supports clock drivers for UniPhier SoCs.
+
+if CLK_UNIPHIER
+
+config CLK_UNIPHIER_PH1_SLD3
+   bool "Clock driver for UniPhier PH1-sLD3 SoC"
+   default y
+
+config CLK_UNIPHIER_PH1_LD4
+   bool "Clock driver for UniPhier PH1-LD4 SoC"
+   default y
+
+config CLK_UNIPHIER_PH1_PRO4
+   bool "Clock driver for UniPhier PH1-Pro4 SoC"
+   default y
+
+config CLK_UNIPHIER_PH1_SLD8
+   bool "Clock driver for UniPhier PH1-sLD8 SoC"
+   default y
+
+config CLK_UNIPHIER_PH1_PRO5
+   bool "Clock driver for UniPhier PH1-Pro5 SoC"
+   default y
+
+config CLK_UNIPHIER_PROXSTREAM2
+   bool "Clock driver for UniPhier ProXstream2/PH1-LD6b SoC"
+   default y
+
+endif
diff --git a/drivers/clk/uniphier/Makefile b/drivers/clk/uniphier/Makefile
new file mode 100644
index 000..3be1a17
--- /dev/null
+++ b/drivers/clk/uniphier/Makefile
@@ -0,0 +1,10 @@
+obj-y += clk-uniphier-core.o
+obj-y += clk-uniphier-peri.o
+obj-y += clk-uniphier-mio.o
+
+obj-$(CONFIG_CLK_UNIPHIER_PH1_SLD3)+= clk-ph1-sld3.o
+obj-$(CONFIG_CLK_UNIPHIER_PH1_LD4) += clk-ph1-ld4.o
+obj-$(CONFIG_CLK_UNIPHIER_PH1_PRO4)+= clk-ph1-pro4.o
+obj-$(CONFIG_CLK_UNIPHIER_PH1_SLD8)+= clk-ph1-sld8.o

[PATCH v2,RESEND] clk: uniphier: add clock drivers for UniPhier SoCs

2015-12-28 Thread Masahiro Yamada
This is the initial commit for the UniPhier clock drivers, including
support for PH1-sLD3, PH1-LD4, PH1-Pro4, PH1-sLD8, PH1-Pro5, and
ProXstream2/PH1-LD6b.

To improve the code maintainability, the driver consists of common
functions (clk-uniphier-core.c) and clock data arrays needed to
support each SoC.

Signed-off-by: Masahiro Yamada 
---

Changes in v2:
  - split emmc_hw_reset
  - make SD clock rate-controllable
  - add CLK_SET_RATE_PARENT flag to mux, gate, fixed-factor clocks

 MAINTAINERS  |   1 +
 drivers/clk/Kconfig  |   1 +
 drivers/clk/Makefile |   1 +
 drivers/clk/uniphier/Kconfig |  35 
 drivers/clk/uniphier/Makefile|  10 +
 drivers/clk/uniphier/clk-ph1-ld4.c   | 117 
 drivers/clk/uniphier/clk-ph1-pro4.c  | 117 
 drivers/clk/uniphier/clk-ph1-pro5.c  | 107 +++
 drivers/clk/uniphier/clk-ph1-sld3.c  | 117 
 drivers/clk/uniphier/clk-ph1-sld8.c  | 107 +++
 drivers/clk/uniphier/clk-proxstream2.c   |  88 +
 drivers/clk/uniphier/clk-uniphier-core.c | 151 +++
 drivers/clk/uniphier/clk-uniphier-mio.c  | 315 +++
 drivers/clk/uniphier/clk-uniphier-peri.c | 175 +
 drivers/clk/uniphier/clk-uniphier.h  |  68 +++
 15 files changed, 1410 insertions(+)
 create mode 100644 drivers/clk/uniphier/Kconfig
 create mode 100644 drivers/clk/uniphier/Makefile
 create mode 100644 drivers/clk/uniphier/clk-ph1-ld4.c
 create mode 100644 drivers/clk/uniphier/clk-ph1-pro4.c
 create mode 100644 drivers/clk/uniphier/clk-ph1-pro5.c
 create mode 100644 drivers/clk/uniphier/clk-ph1-sld3.c
 create mode 100644 drivers/clk/uniphier/clk-ph1-sld8.c
 create mode 100644 drivers/clk/uniphier/clk-proxstream2.c
 create mode 100644 drivers/clk/uniphier/clk-uniphier-core.c
 create mode 100644 drivers/clk/uniphier/clk-uniphier-mio.c
 create mode 100644 drivers/clk/uniphier/clk-uniphier-peri.c
 create mode 100644 drivers/clk/uniphier/clk-uniphier.h

diff --git a/MAINTAINERS b/MAINTAINERS
index fc08493..6c10e9b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1644,6 +1644,7 @@ F:arch/arm/mach-uniphier/
 F: arch/arm/mm/cache-uniphier.c
 F: arch/arm64/boot/dts/socionext/
 F: drivers/bus/uniphier-system-bus.c
+F: drivers/clk/uniphier/
 F: drivers/i2c/busses/i2c-uniphier*
 F: drivers/pinctrl/uniphier/
 F: drivers/tty/serial/8250/8250_uniphier.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index c3e3a02..7580323 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -191,6 +191,7 @@ config COMMON_CLK_CDCE706
 source "drivers/clk/bcm/Kconfig"
 source "drivers/clk/hisilicon/Kconfig"
 source "drivers/clk/qcom/Kconfig"
+source "drivers/clk/uniphier/Kconfig"
 
 endmenu
 
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 820714c..ab9d1bd 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_ARCH_STI)+= st/
 obj-$(CONFIG_ARCH_SUNXI)   += sunxi/
 obj-$(CONFIG_ARCH_TEGRA)   += tegra/
 obj-$(CONFIG_ARCH_OMAP2PLUS)   += ti/
+obj-$(CONFIG_CLK_UNIPHIER) += uniphier/
 obj-$(CONFIG_ARCH_U8500)   += ux500/
 obj-$(CONFIG_COMMON_CLK_VERSATILE) += versatile/
 obj-$(CONFIG_X86)  += x86/
diff --git a/drivers/clk/uniphier/Kconfig b/drivers/clk/uniphier/Kconfig
new file mode 100644
index 000..7606f27
--- /dev/null
+++ b/drivers/clk/uniphier/Kconfig
@@ -0,0 +1,35 @@
+menuconfig CLK_UNIPHIER
+   bool "Clock drivers for UniPhier SoCs"
+   depends on ARCH_UNIPHIER
+   depends on OF
+   default y
+   help
+ Supports clock drivers for UniPhier SoCs.
+
+if CLK_UNIPHIER
+
+config CLK_UNIPHIER_PH1_SLD3
+   bool "Clock driver for UniPhier PH1-sLD3 SoC"
+   default y
+
+config CLK_UNIPHIER_PH1_LD4
+   bool "Clock driver for UniPhier PH1-LD4 SoC"
+   default y
+
+config CLK_UNIPHIER_PH1_PRO4
+   bool "Clock driver for UniPhier PH1-Pro4 SoC"
+   default y
+
+config CLK_UNIPHIER_PH1_SLD8
+   bool "Clock driver for UniPhier PH1-sLD8 SoC"
+   default y
+
+config CLK_UNIPHIER_PH1_PRO5
+   bool "Clock driver for UniPhier PH1-Pro5 SoC"
+   default y
+
+config CLK_UNIPHIER_PROXSTREAM2
+   bool "Clock driver for UniPhier ProXstream2/PH1-LD6b SoC"
+   default y
+
+endif
diff --git a/drivers/clk/uniphier/Makefile b/drivers/clk/uniphier/Makefile
new file mode 100644
index 000..3be1a17
--- /dev/null
+++ b/drivers/clk/uniphier/Makefile
@@ -0,0 +1,10 @@
+obj-y += clk-uniphier-core.o
+obj-y += clk-uniphier-peri.o
+obj-y += clk-uniphier-mio.o
+
+obj-$(CONFIG_CLK_UNIPHIER_PH1_SLD3)+= clk-ph1-sld3.o
+obj-$(CONFIG_CLK_UNIPHIER_PH1_LD4) += clk-ph1-ld4.o
+obj-$(CONFIG_CLK_UNIPHIER_PH1_PRO4)+= clk-ph1-pro4.o
+obj-$(CONFIG_CLK_UNIPHIER_PH1_SLD8)