Re: [PATCH 3/3] clk: remove redundant clock parents lookup table allocations

2016-01-07 Thread Masahiro Yamada
Hi Vladimir,


2016-01-07 12:16 GMT+09:00 Vladimir Zapolskiy <v...@mleia.com>:
> Since clock parents lookup table for clocks with multiple parent
> clocks is always allocated during clock registration, remove similar
> allocations from __clk_init_parent() and clk_fetch_parent_index().
>
> The change also corrects a pointer type of a single lookup table
> entry on calculation of the lookup table size.
>
> Signed-off-by: Vladimir Zapolskiy <v...@mleia.com>
> ---
>  drivers/clk/clk.c | 20 +++-
>  1 file changed, 3 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 2fb0dae..f8872f9 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1067,13 +1067,6 @@ static int clk_fetch_parent_index(struct clk_core 
> *core,
>  {
> int i;
>
> -   if (!core->parents) {
> -   core->parents = kcalloc(core->num_parents,
> -   sizeof(struct clk *), GFP_KERNEL);
> -   if (!core->parents)
> -   return -ENOMEM;
> -   }
> -
> /*
>  * find index of new parent clock using cached parent ptrs,
>  * or if not yet cached, use string name comparison and cache
> @@ -1711,18 +1704,11 @@ static struct clk_core *__clk_init_parent(struct 
> clk_core *core)
> }
>
> /*
> -* Do our best to cache parent clocks in core->parents.  This prevents
> -* unnecessary and expensive lookups.  We don't set core->parent here;
> -* that is done by the calling function.
> +* We don't set core->parent here; that is done by the calling 
> function.
>  */
>
> index = core->ops->get_parent(core->hw);
>
> -   if (!core->parents)
> -   core->parents =
> -   kcalloc(core->num_parents, sizeof(struct clk *),
> -   GFP_KERNEL);
> -
> ret = clk_core_get_parent_by_index(core, index);
>
>  out:
> @@ -2374,8 +2360,8 @@ static int __clk_init(struct device *dev, struct clk 
> *clk_user)
>  * look-ups of clk's possible parents.
>  */
> if (core->num_parents > 1) {
> -   core->parents = kcalloc(core->num_parents, sizeof(struct clk 
> *),
> -   GFP_KERNEL);
> +   core->parents = kcalloc(core->num_parents,
> +   sizeof(struct clk_core *), 
> GFP_KERNEL);
> if (!core->parents) {
> ret = -ENOMEM;
> goto out;


You are doing the similar thing as mine.

See
https://patchwork.kernel.org/patch/7925571/


This series gives a big conflict with my clean-up series,
which Michael said he would apply after v4.5-rc1.

See
http://www.serverphorums.com/read.php?12,1376630


-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] clk: add clk_unregister_fixed_factor()

2016-01-05 Thread Masahiro Yamada
Allow to unregister fixed factor clock.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk-fixed-factor.c | 13 +
 include/linux/clk-provider.h   |  1 +
 2 files changed, 14 insertions(+)

diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 83de57a..f9c80e3 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -102,6 +102,19 @@ struct clk *clk_register_fixed_factor(struct device *dev, 
const char *name,
 }
 EXPORT_SYMBOL_GPL(clk_register_fixed_factor);
 
+void clk_unregister_fixed_factor(struct clk *clk)
+{
+   struct clk_hw *hw;
+
+   hw = __clk_get_hw(clk);
+   if (!hw)
+   return;
+
+   clk_unregister(clk);
+   kfree(to_clk_fixed_factor(hw));
+}
+EXPORT_SYMBOL_GPL(clk_unregister_fixed_factor);
+
 #ifdef CONFIG_OF
 /**
  * of_fixed_factor_clk_setup() - Setup function for simple fixed factor clock
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index e7bd710..39edfd7 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -486,6 +486,7 @@ extern const struct clk_ops clk_fixed_factor_ops;
 struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,
unsigned int mult, unsigned int div);
+void clk_unregister_fixed_factor(struct clk *clk);
 
 /**
  * struct clk_fractional_divider - adjustable fractional divider clock
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] clk: add clk_unregister_fixed_rate()

2016-01-05 Thread Masahiro Yamada
Allow to unregister fixed rate clock.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk-fixed-rate.c | 13 +
 include/linux/clk-provider.h |  2 +-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index f85ec8d..a69a6db 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -106,6 +106,19 @@ struct clk *clk_register_fixed_rate(struct device *dev, 
const char *name,
 }
 EXPORT_SYMBOL_GPL(clk_register_fixed_rate);
 
+void clk_unregister_fixed_rate(struct clk *clk)
+{
+   struct clk_hw *hw;
+
+   hw = __clk_get_hw(clk);
+   if (!hw)
+   return;
+
+   clk_unregister(clk);
+   kfree(to_clk_fixed_rate(hw));
+}
+EXPORT_SYMBOL_GPL(clk_unregister_fixed_rate);
+
 #ifdef CONFIG_OF
 /**
  * of_fixed_clk_setup() - Setup function for simple fixed rate clock
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 39edfd7..b6c6bfb 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -282,7 +282,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, 
const char *name,
 struct clk *clk_register_fixed_rate_with_accuracy(struct device *dev,
const char *name, const char *parent_name, unsigned long flags,
unsigned long fixed_rate, unsigned long fixed_accuracy);
-
+void clk_unregister_fixed_rate(struct clk *clk);
 void of_fixed_clk_setup(struct device_node *np);
 
 /**
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] clk: optimize the divider walk in clk_divider_bestdiv()

2016-01-04 Thread Masahiro Yamada
Because _next_div() returns a valid divider, there is no need to
consult _is_valid_div() for the validity of the divider in every
iteration.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk-divider.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 358b9f9..8ced09e 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -304,9 +304,8 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned 
long rate,
 */
maxdiv = min(ULONG_MAX / rate, maxdiv);
 
-   for (i = 1; i <= maxdiv; i = _next_div(table, i, flags)) {
-   if (!_is_valid_div(table, i, flags))
-   continue;
+   for (i = _next_div(table, 0, flags); i <= maxdiv;
+i = _next_div(table, i, flags)) {
if (rate * i == parent_rate_saved) {
/*
 * It's the most ideal case if the requested rate can be
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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 <mturque...@baylibre.com>:
> 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.
(L

[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 <yamada.masah...@socionext.com>
---

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

[PATCH v2 03/16] clk: rename __clk_init() into __clk_core_init()

2015-12-28 Thread Masahiro Yamada
Now this function takes clk_core as its argument.  __clk_core_init()
would be more suitable for the name of this function.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 623838f..41179f3 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2285,13 +2285,13 @@ static inline void clk_debug_unregister(struct clk_core 
*core)
 #endif
 
 /**
- * __clk_init - initialize the data structures in a struct clk_core
+ * __clk_core_init - initialize the data structures in a struct clk_core
  * @core:  clk_core being initialized
  *
  * Initializes the lists in struct clk_core, queries the hardware for the
  * parent and rate and sets them both.
  */
-static int __clk_init(struct clk_core *core)
+static int __clk_core_init(struct clk_core *core)
 {
int i, ret = 0;
struct clk_core *orphan;
@@ -2571,7 +2571,7 @@ struct clk *clk_register(struct device *dev, struct 
clk_hw *hw)
goto fail_parent_names_copy;
}
 
-   ret = __clk_init(core);
+   ret = __clk_core_init(core);
if (!ret)
return hw->clk;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 15/16] clk: simplify clk_fetch_parent_index() function

2015-12-28 Thread Masahiro Yamada
The clk_core_get_parent_by_index can be used as a helper function
to simplify the implementation of clk_fetch_parent_index().

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 151cbe8..5b9aec0 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1069,23 +1069,9 @@ static int clk_fetch_parent_index(struct clk_core *core,
if (!parent)
return -EINVAL;
 
-   /*
-* find index of new parent clock using cached parent ptrs,
-* or if not yet cached, use string name comparison and cache
-* them now to avoid future calls to clk_core_lookup.
-*/
-   for (i = 0; i < core->num_parents; i++) {
-   if (core->parents[i] == parent)
-   return i;
-
-   if (core->parents[i])
-   continue;
-
-   if (!strcmp(core->parent_names[i], parent->name)) {
-   core->parents[i] = clk_core_lookup(parent->name);
+   for (i = 0; i < core->num_parents; i++)
+   if (clk_core_get_parent_by_index(core, i) == parent)
return i;
-   }
-   }
 
return -EINVAL;
 }
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 16/16] clk: slightly optimize clk_core_set_parent()

2015-12-28 Thread Masahiro Yamada
If clk_fetch_parent_index() fails, p_rate is unused.  Move the
assignment after the error checking.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 5b9aec0..e81301b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1730,13 +1730,13 @@ static int clk_core_set_parent(struct clk_core *core, 
struct clk_core *parent)
/* try finding the new parent index */
if (parent) {
p_index = clk_fetch_parent_index(core, parent);
-   p_rate = parent->rate;
if (p_index < 0) {
pr_debug("%s: clk %s can not be parent of clk %s\n",
__func__, parent->name, core->name);
ret = p_index;
goto out;
}
+   p_rate = parent->rate;
}
 
/* propagate PRE_RATE_CHANGE notifications */
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 14/16] clk: make sure parent is not NULL in clk_fetch_parent_index()

2015-12-28 Thread Masahiro Yamada
If parent is given with NULL, clk_fetch_parent_index() could return
a positive index value.

Currently, parent is checked by the callers of this function, but
it would be safer to do it in this function.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2:
  - Fix a bug.  Return -EINVAL when parent _is_ NULL.

 drivers/clk/clk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index efd093a..151cbe8 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1066,6 +1066,9 @@ static int clk_fetch_parent_index(struct clk_core *core,
 {
int i;
 
+   if (!parent)
+   return -EINVAL;
+
/*
 * find index of new parent clock using cached parent ptrs,
 * or if not yet cached, use string name comparison and cache
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 07/16] clk: simplify clk_core_get_parent_by_index()

2015-12-28 Thread Masahiro Yamada
Drop the "if (!core->parents)" case and refactor the function a bit
because core->parents is always allocated.  (Strictly speaking, it is
ZERO_SIZE_PTR if core->num_parents == 0, but such a case is omitted
by the if-conditional above.)

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index b5be02a..ca7849a 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -350,13 +350,12 @@ static struct clk_core 
*clk_core_get_parent_by_index(struct clk_core *core,
 {
if (!core || index >= core->num_parents)
return NULL;
-   else if (!core->parents)
-   return clk_core_lookup(core->parent_names[index]);
-   else if (!core->parents[index])
-   return core->parents[index] =
-   clk_core_lookup(core->parent_names[index]);
-   else
-   return core->parents[index];
+
+   if (!core->parents[index])
+   core->parents[index] =
+   clk_core_lookup(core->parent_names[index]);
+
+   return core->parents[index];
 }
 
 struct clk_hw *
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RESEND PATCH v2 09/16] clk: replace pr_warn() with pr_err() for fatal cases

2015-12-28 Thread Masahiro Yamada
These three cases let clk_register() fail.  They should be considered
as error messages.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 3215b2b..e6e10f5 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2302,22 +2302,22 @@ static int __clk_core_init(struct clk_core *core)
if (core->ops->set_rate &&
!((core->ops->round_rate || core->ops->determine_rate) &&
  core->ops->recalc_rate)) {
-   pr_warning("%s: %s must implement .round_rate or 
.determine_rate in addition to .recalc_rate\n",
-   __func__, core->name);
+   pr_err("%s: %s must implement .round_rate or .determine_rate in 
addition to .recalc_rate\n",
+  __func__, core->name);
ret = -EINVAL;
goto out;
}
 
if (core->ops->set_parent && !core->ops->get_parent) {
-   pr_warning("%s: %s must implement .get_parent & .set_parent\n",
-   __func__, core->name);
+   pr_err("%s: %s must implement .get_parent & .set_parent\n",
+  __func__, core->name);
ret = -EINVAL;
goto out;
}
 
if (core->ops->set_rate_and_parent &&
!(core->ops->set_parent && core->ops->set_rate)) {
-   pr_warn("%s: %s must implement .set_parent & .set_rate\n",
+   pr_err("%s: %s must implement .set_parent & .set_rate\n",
__func__, core->name);
ret = -EINVAL;
goto out;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RESEND PATCH v2 11/16] clk: simplify __clk_init_parent()

2015-12-28 Thread Masahiro Yamada
The translation from the index into clk_core is done by
clk_core_get_parent_by_index().  The if-block for num_parents == 1
case is duplicating the code in the clk_core_get_parent_by_index().

Drop the "if (num_parents == 1)" from the special case.  Instead,
set the index to zero if .get_parent() is missing.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 38 --
 1 file changed, 4 insertions(+), 34 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 4ad0a36..03b87d8 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1651,44 +1651,14 @@ struct clk *clk_get_parent(struct clk *clk)
 }
 EXPORT_SYMBOL_GPL(clk_get_parent);
 
-/*
- * .get_parent is mandatory for clocks with multiple possible parents.  It is
- * optional for single-parent clocks.  Always call .get_parent if it is
- * available and WARN if it is missing for multi-parent clocks.
- *
- * For single-parent clocks without .get_parent, first check to see if the
- * .parents array exists, and if so use it to avoid an expensive tree
- * traversal.  If .parents does not exist then walk the tree.
- */
 static struct clk_core *__clk_init_parent(struct clk_core *core)
 {
-   struct clk_core *ret = NULL;
-   u8 index;
-
-   /* handle the trivial cases */
+   u8 index = 0;
 
-   if (!core->num_parents)
-   goto out;
-
-   if (core->num_parents == 1) {
-   if (IS_ERR_OR_NULL(core->parent))
-   core->parent = clk_core_lookup(core->parent_names[0]);
-   ret = core->parent;
-   goto out;
-   }
+   if (core->ops->get_parent)
+   index = core->ops->get_parent(core->hw);
 
-   /*
-* Do our best to cache parent clocks in core->parents.  This prevents
-* unnecessary and expensive lookups.  We don't set core->parent here;
-* that is done by the calling function.
-*/
-
-   index = core->ops->get_parent(core->hw);
-
-   ret = clk_core_get_parent_by_index(core, index);
-
-out:
-   return ret;
+   return clk_core_get_parent_by_index(core, index);
 }
 
 static void clk_core_reparent(struct clk_core *core,
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RESEND PATCH v2 02/16] clk: change the argument of __clk_init() into pointer to clk_core

2015-12-28 Thread Masahiro Yamada
The argument clk_user is used only for the clk_user->core.  The rest
of this function only takes care of clk_core.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 897e5ae..623838f 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2285,25 +2285,22 @@ static inline void clk_debug_unregister(struct clk_core 
*core)
 #endif
 
 /**
- * __clk_init - initialize the data structures in a struct clk
- * @clk:   clk being initialized
+ * __clk_init - initialize the data structures in a struct clk_core
+ * @core:  clk_core being initialized
  *
  * Initializes the lists in struct clk_core, queries the hardware for the
  * parent and rate and sets them both.
  */
-static int __clk_init(struct clk *clk_user)
+static int __clk_init(struct clk_core *core)
 {
int i, ret = 0;
struct clk_core *orphan;
struct hlist_node *tmp2;
-   struct clk_core *core;
unsigned long rate;
 
-   if (!clk_user)
+   if (!core)
return -EINVAL;
 
-   core = clk_user->core;
-
clk_prepare_lock();
 
/* check to see if a clock with this name is already registered */
@@ -2574,7 +2571,7 @@ struct clk *clk_register(struct device *dev, struct 
clk_hw *hw)
goto fail_parent_names_copy;
}
 
-   ret = __clk_init(hw->clk);
+   ret = __clk_init(core);
if (!ret)
return hw->clk;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 08/16] clk: drop the initial core->parents look-ups from __clk_core_init()

2015-12-28 Thread Masahiro Yamada
The core->parents is a cache to save expensive clock parent look-ups.
It will be filled as needed later.  We do not have to do it here.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ca7849a..3215b2b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2329,17 +2329,6 @@ static int __clk_core_init(struct clk_core *core)
"%s: invalid NULL in %s's .parent_names\n",
__func__, core->name);
 
-   /*
-* clk_core_lookup returns NULL for parents that have not been
-* clk_init'd; thus any access to clk->parents[] must check
-* for a NULL pointer.  We can always perform lazy lookups for
-* missing parents later on.
-*/
-   if (core->parents)
-   for (i = 0; i < core->num_parents; i++)
-   core->parents[i] =
-   clk_core_lookup(core->parent_names[i]);
-
core->parent = __clk_init_parent(core);
 
/*
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 09/16] clk: replace pr_warn() with pr_err() for fatal cases

2015-12-28 Thread Masahiro Yamada
These three cases let clk_register() fail.  They should be considered
as error messages.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 3215b2b..e6e10f5 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2302,22 +2302,22 @@ static int __clk_core_init(struct clk_core *core)
if (core->ops->set_rate &&
!((core->ops->round_rate || core->ops->determine_rate) &&
  core->ops->recalc_rate)) {
-   pr_warning("%s: %s must implement .round_rate or 
.determine_rate in addition to .recalc_rate\n",
-   __func__, core->name);
+   pr_err("%s: %s must implement .round_rate or .determine_rate in 
addition to .recalc_rate\n",
+  __func__, core->name);
ret = -EINVAL;
goto out;
}
 
if (core->ops->set_parent && !core->ops->get_parent) {
-   pr_warning("%s: %s must implement .get_parent & .set_parent\n",
-   __func__, core->name);
+   pr_err("%s: %s must implement .get_parent & .set_parent\n",
+  __func__, core->name);
ret = -EINVAL;
goto out;
}
 
if (core->ops->set_rate_and_parent &&
!(core->ops->set_parent && core->ops->set_rate)) {
-   pr_warn("%s: %s must implement .set_parent & .set_rate\n",
+   pr_err("%s: %s must implement .set_parent & .set_rate\n",
__func__, core->name);
ret = -EINVAL;
goto out;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 01/16] clk: remove unused first argument of __clk_init()

2015-12-28 Thread Masahiro Yamada
The "struct device *dev" is not used at all in this function.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 3fba5ac..897e5ae 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2286,13 +2286,12 @@ static inline void clk_debug_unregister(struct clk_core 
*core)
 
 /**
  * __clk_init - initialize the data structures in a struct clk
- * @dev:   device initializing this clk, placeholder for now
  * @clk:   clk being initialized
  *
  * Initializes the lists in struct clk_core, queries the hardware for the
  * parent and rate and sets them both.
  */
-static int __clk_init(struct device *dev, struct clk *clk_user)
+static int __clk_init(struct clk *clk_user)
 {
int i, ret = 0;
struct clk_core *orphan;
@@ -2575,7 +2574,7 @@ struct clk *clk_register(struct device *dev, struct 
clk_hw *hw)
goto fail_parent_names_copy;
}
 
-   ret = __clk_init(dev, hw->clk);
+   ret = __clk_init(hw->clk);
if (!ret)
return hw->clk;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 10/16] clk: move checking .git_parent to __clk_core_init()

2015-12-28 Thread Masahiro Yamada
The .git_parent is mandatory for multi-parent clocks.  Move the check
to __clk_core_init(), like other callback checkings.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index e6e10f5..4ad0a36 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1677,13 +1677,6 @@ static struct clk_core *__clk_init_parent(struct 
clk_core *core)
goto out;
}
 
-   if (!core->ops->get_parent) {
-   WARN(!core->ops->get_parent,
-   "%s: multi-parent clocks must implement .get_parent\n",
-   __func__);
-   goto out;
-   }
-
/*
 * Do our best to cache parent clocks in core->parents.  This prevents
 * unnecessary and expensive lookups.  We don't set core->parent here;
@@ -2315,6 +2308,11 @@ static int __clk_core_init(struct clk_core *core)
goto out;
}
 
+   if (core->num_parents > 1 && !core->ops->get_parent) {
+   pr_err("%s: %s must implement .get_parent as it has multi 
parents\n",
+  __func__, core->name);
+   }
+
if (core->ops->set_rate_and_parent &&
!(core->ops->set_parent && core->ops->set_rate)) {
pr_err("%s: %s must implement .set_parent & .set_rate\n",
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 13/16] clk: walk the orphan clock list more simply

2015-12-28 Thread Masahiro Yamada
This loop can be much simpler.  If a new parent is available for
orphan clocks, __clk_init_parent(orphan) can detect it.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 21 ++---
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index a4f66fe..efd093a 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2402,24 +2402,15 @@ static int __clk_core_init(struct clk_core *core)
core->rate = core->req_rate = rate;
 
/*
-* walk the list of orphan clocks and reparent any that are children of
-* this clock
+* walk the list of orphan clocks and reparent any that newly finds a
+* parent.
 */
hlist_for_each_entry_safe(orphan, tmp2, _orphan_list, child_node) {
-   if (orphan->num_parents && orphan->ops->get_parent) {
-   i = orphan->ops->get_parent(orphan->hw);
-   if (i >= 0 && i < orphan->num_parents &&
-   !strcmp(core->name, orphan->parent_names[i]))
-   clk_core_reparent(orphan, core);
-   continue;
-   }
+   struct clk_core *parent = __clk_init_parent(orphan);
 
-   for (i = 0; i < orphan->num_parents; i++)
-   if (!strcmp(core->name, orphan->parent_names[i])) {
-   clk_core_reparent(orphan, core);
-   break;
-   }
-}
+   if (parent)
+   clk_core_reparent(orphan, parent);
+   }
 
/*
 * optional platform-specific magic
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 12/16] clk: avoid circular clock topology

2015-12-28 Thread Masahiro Yamada
Currently, clk_register() never checks a circular parent looping,
but clock providers could register such an insane clock topology.
For example, "clk_a" could have "clk_b" as a parent, and vice versa.
In this case, clk_core_reparent() creates a circular parent list
and __clk_recalc_accuracies() calls itself recursively forever.

The core infrastructure should be kind enough to bail out, showing
an appropriate error message in such a case.  This helps to easily
find a bug in clock providers.  (uh, I made such a silly mistake
when I was implementing my clock providers first.  I was upset
because the kernel did not respond, without any error message.)

This commit adds a new helper function, __clk_is_ancestor().  It
returns true if the second argument is a possible ancestor of the
first one.  If a clock core is a possible ancestor of itself, it
would make a loop when it were registered.  That should be detected
as an error.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 40 
 1 file changed, 40 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 03b87d8..a4f66fe 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2235,6 +2235,38 @@ static inline void clk_debug_unregister(struct clk_core 
*core)
 #endif
 
 /**
+ * __clk_is_ancestor - check if a clk_core is a possible ancestor of another
+ * @core: clock core
+ * @ancestor: ancestor clock core
+ *
+ * Returns true if there is a possibility that @ancestor can be an ancestor
+ * of @core, false otherwise.
+ *
+ * This function can be used against @core or @ancestor that has not been
+ * registered yet.
+ */
+static bool __clk_is_ancestor(struct clk_core *core, struct clk_core *ancestor)
+{
+   struct clk_core *parent;
+   int i;
+
+   for (i = 0; i < core->num_parents; i++) {
+   parent = clk_core_get_parent_by_index(core, i);
+   /*
+* If ancestor has not been added to clk_{root,orphan}_list
+* yet, clk_core_lookup() cannot find it.  If parent is NULL,
+* compare the name strings, too.
+*/
+   if ((parent && (parent == ancestor ||
+   __clk_is_ancestor(parent, ancestor))) ||
+   (!parent && !strcmp(core->parent_names[i], ancestor->name)))
+   return true;
+   }
+
+   return false;
+}
+
+/**
  * __clk_core_init - initialize the data structures in a struct clk_core
  * @core:  clk_core being initialized
  *
@@ -2297,6 +2329,14 @@ static int __clk_core_init(struct clk_core *core)
"%s: invalid NULL in %s's .parent_names\n",
__func__, core->name);
 
+   /* If core is an ancestor of itself, it would make a loop. */
+   if (__clk_is_ancestor(core, core)) {
+   pr_err("%s: %s would create circular parent\n", __func__,
+  core->name);
+   ret = -EINVAL;
+   goto out;
+   }
+
core->parent = __clk_init_parent(core);
 
/*
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 04/16] clk: remove unnecessary !core->parents conditional

2015-12-28 Thread Masahiro Yamada
This if-block has been here since the introduction of the common
clock framework.  Now no clock drivers are statically initialized.
core->parent is always NULL at this point.  Drop the redundant
check and the confusing comment.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 41179f3..30c0bbc 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2348,11 +2348,8 @@ static int __clk_core_init(struct clk_core *core)
 * in to clk_init during early boot; thus any access to core->parents[]
 * must always check for a NULL pointer and try to populate it if
 * necessary.
-*
-* If core->parents is not NULL we skip this entire block.  This allows
-* for clock drivers to statically initialize core->parents.
 */
-   if (core->num_parents > 1 && !core->parents) {
+   if (core->num_parents > 1) {
core->parents = kcalloc(core->num_parents, sizeof(struct clk *),
GFP_KERNEL);
/*
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RESEND PATCH v2 14/16] clk: make sure parent is not NULL in clk_fetch_parent_index()

2015-12-28 Thread Masahiro Yamada
If parent is given with NULL, clk_fetch_parent_index() could return
a positive index value.

Currently, parent is checked by the callers of this function, but
it would be safer to do it in this function.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2:
  - Fix a bug.  Return -EINVAL when parent _is_ NULL.

 drivers/clk/clk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index efd093a..151cbe8 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1066,6 +1066,9 @@ static int clk_fetch_parent_index(struct clk_core *core,
 {
int i;
 
+   if (!parent)
+   return -EINVAL;
+
/*
 * find index of new parent clock using cached parent ptrs,
 * or if not yet cached, use string name comparison and cache
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RESEND PATCH v2 13/16] clk: walk the orphan clock list more simply

2015-12-28 Thread Masahiro Yamada
This loop can be much simpler.  If a new parent is available for
orphan clocks, __clk_init_parent(orphan) can detect it.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 21 ++---
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index a4f66fe..efd093a 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2402,24 +2402,15 @@ static int __clk_core_init(struct clk_core *core)
core->rate = core->req_rate = rate;
 
/*
-* walk the list of orphan clocks and reparent any that are children of
-* this clock
+* walk the list of orphan clocks and reparent any that newly finds a
+* parent.
 */
hlist_for_each_entry_safe(orphan, tmp2, _orphan_list, child_node) {
-   if (orphan->num_parents && orphan->ops->get_parent) {
-   i = orphan->ops->get_parent(orphan->hw);
-   if (i >= 0 && i < orphan->num_parents &&
-   !strcmp(core->name, orphan->parent_names[i]))
-   clk_core_reparent(orphan, core);
-   continue;
-   }
+   struct clk_core *parent = __clk_init_parent(orphan);
 
-   for (i = 0; i < orphan->num_parents; i++)
-   if (!strcmp(core->name, orphan->parent_names[i])) {
-   clk_core_reparent(orphan, core);
-   break;
-   }
-}
+   if (parent)
+   clk_core_reparent(orphan, parent);
+   }
 
/*
 * optional platform-specific magic
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RESEND PATCH v2 15/16] clk: simplify clk_fetch_parent_index() function

2015-12-28 Thread Masahiro Yamada
The clk_core_get_parent_by_index can be used as a helper function
to simplify the implementation of clk_fetch_parent_index().

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 151cbe8..5b9aec0 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1069,23 +1069,9 @@ static int clk_fetch_parent_index(struct clk_core *core,
if (!parent)
return -EINVAL;
 
-   /*
-* find index of new parent clock using cached parent ptrs,
-* or if not yet cached, use string name comparison and cache
-* them now to avoid future calls to clk_core_lookup.
-*/
-   for (i = 0; i < core->num_parents; i++) {
-   if (core->parents[i] == parent)
-   return i;
-
-   if (core->parents[i])
-   continue;
-
-   if (!strcmp(core->parent_names[i], parent->name)) {
-   core->parents[i] = clk_core_lookup(parent->name);
+   for (i = 0; i < core->num_parents; i++)
+   if (clk_core_get_parent_by_index(core, i) == parent)
return i;
-   }
-   }
 
return -EINVAL;
 }
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RESEND PATCH v2 10/16] clk: move checking .git_parent to __clk_core_init()

2015-12-28 Thread Masahiro Yamada
The .git_parent is mandatory for multi-parent clocks.  Move the check
to __clk_core_init(), like other callback checkings.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index e6e10f5..4ad0a36 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1677,13 +1677,6 @@ static struct clk_core *__clk_init_parent(struct 
clk_core *core)
goto out;
}
 
-   if (!core->ops->get_parent) {
-   WARN(!core->ops->get_parent,
-   "%s: multi-parent clocks must implement .get_parent\n",
-   __func__);
-   goto out;
-   }
-
/*
 * Do our best to cache parent clocks in core->parents.  This prevents
 * unnecessary and expensive lookups.  We don't set core->parent here;
@@ -2315,6 +2308,11 @@ static int __clk_core_init(struct clk_core *core)
goto out;
}
 
+   if (core->num_parents > 1 && !core->ops->get_parent) {
+   pr_err("%s: %s must implement .get_parent as it has multi 
parents\n",
+  __func__, core->name);
+   }
+
if (core->ops->set_rate_and_parent &&
!(core->ops->set_parent && core->ops->set_rate)) {
pr_err("%s: %s must implement .set_parent & .set_rate\n",
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RESEND PATCH v2 08/16] clk: drop the initial core->parents look-ups from __clk_core_init()

2015-12-28 Thread Masahiro Yamada
The core->parents is a cache to save expensive clock parent look-ups.
It will be filled as needed later.  We do not have to do it here.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2: None

 drivers/clk/clk.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ca7849a..3215b2b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2329,17 +2329,6 @@ static int __clk_core_init(struct clk_core *core)
"%s: invalid NULL in %s's .parent_names\n",
__func__, core->name);
 
-   /*
-* clk_core_lookup returns NULL for parents that have not been
-* clk_init'd; thus any access to clk->parents[] must check
-* for a NULL pointer.  We can always perform lazy lookups for
-* missing parents later on.
-*/
-   if (core->parents)
-   for (i = 0; i < core->num_parents; i++)
-   core->parents[i] =
-   clk_core_lookup(core->parent_names[i]);
-
core->parent = __clk_init_parent(core);
 
/*
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-12-02 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 <yamada.masah...@socionext.com>
---

 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 | 150 ++
 drivers/clk/uniphier/clk-uniphier-mio.c  | 206 +++
 drivers/clk/uniphier/clk-uniphier-peri.c | 175 ++
 drivers/clk/uniphier/clk-uniphier.h  |  68 ++
 15 files changed, 1300 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 cba790b..1c6edc8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1642,6 +1642,7 @@ F:arch/arm/boot/dts/uniphier*
 F: arch/arm/include/asm/hardware/cache-uniphier.h
 F: arch/arm/mach-uniphier/
 F: arch/arm/mm/cache-uniphier.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

[PATCH v3] clk: let of_clk_get_parent_name() fail for invalid clock-indices

2015-12-02 Thread Masahiro Yamada
Currently, of_clk_get_parent_name() returns a wrong parent clock name
when "clock-indices" property exists and the target index is not
found in the property.  In this case, NULL should be returned.

For example,

oscillator {
compatible = "myclocktype";
#clock-cells = <1>;
clock-indices = <1>, <3>;
clock-output-names = "clka", "clkb";
};

consumer {
compatible = "myclockconsumer";
clocks = < 0>, < 1>;
};

Currently, of_clk_get_parent_name(consumer_np, 0) returns "clka"
(and of_clk_get_parent_name(consumer_np, 1) also returns "clka",
this is correct).   Because the "clock-indices" in the clock parent
does not contain <0>, of_clk_get_parent_name(consumer_np, 0) should
return NULL.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v3:
  - Choose the least invasive way

Changes in v2:
  - Rephrase the git-log

 drivers/clk/clk.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 88a723b..1d95033 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3079,6 +3079,9 @@ const char *of_clk_get_parent_name(struct device_node 
*np, int index)
}
count++;
}
+   /* We went off the end of 'clock-indices' without finding it */
+   if (prop && !vp)
+   return NULL;
 
if (of_property_read_string_index(clkspec.np, "clock-output-names",
  index,
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] clk: let of_clk_get_parent_name() fail for invalid clock-indices

2015-11-30 Thread Masahiro Yamada
Hi Stephen,


2015-12-01 9:58 GMT+09:00 Stephen Boyd <sb...@codeaurora.org>:
> On 11/30, Masahiro Yamada wrote:
>> Currently, of_clk_get_parent_name() returns a wrong parent clock name
>> when "clock-indices" property exists and the target index is not
>> found in the property.  In this case, NULL should be returned.
>>
>> For example,
>>
>> oscillator {
>> compatible = "myclocktype";
>> #clock-cells = <1>;
>> clock-indices = <1>, <3>;
>> clock-output-names = "clka", "clkb";
>> };
>>
>> consumer {
>> compatible = "myclockconsumer";
>> clocks = < 0>, < 1>;
>> };
>>
>> Currently, of_clk_get_parent_name(consumer_np, 0) returns "clka"
>> (and of_clk_get_parent_name(consumer_np, 1) also returns "clka",
>> this is correct).   Because the "clock-indices" in the clock parent
>> does not contain <0>, of_clk_get_parent_name(consumer_np, 0) should
>> return NULL.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
>
> Here's the proposed alternative, if you agree I will merge it
> with the above commit text and attribution to you.
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index a66efc9d8bfc..f54583a9835a 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3079,6 +3079,9 @@ const char *of_clk_get_parent_name(struct device_node 
> *np, int index)
> }
> count++;
> }
> +   /* We went off the end of 'clock-indices' without finding it */
> +   if (!vp && count)
> +   return NULL;
>
> if (of_property_read_string_index(clkspec.np, "clock-output-names",
>   index,
>

No, again.
The existence of "clock-indices" should be checked
in order to omit the zero-length "clock-indices".


OK, let me guess the next alternative from you.


> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3079,6 +3079,9 @@ const char *of_clk_get_parent_name(struct device_node 
> *np, int index)
> }
> count++;
> }
> +   /* We went off the end of 'clock-indices' without finding it */
> +   if (prop && !vp)
> +   return NULL;
>
> if (of_property_read_string_index(clkspec.np, "clock-output-names",
>   index,
>

This works, but clumsy things are:

[1] If the "clock-indices" is missing, we can know it
 before looping around the of_property_for_each_u32().
 Checking the "prop" after the loop seems odd.

[2] "prop" and "vp" seem to be temporary storage that we should not
 know what they exactly are, like the auxiliary pointer in
list_for_each_safe().


Why do you insist on of_property_for_each_u32()?
It no longer fits in here.


-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] clk: split of_clk_get_parent_name() into two functions

2015-11-30 Thread Masahiro Yamada
2015-12-01 9:49 GMT+09:00 Stephen Boyd <sb...@codeaurora.org>:
> On 11/24, Masahiro Yamada wrote:
>> Hi Stephen,
>>
>>
>> 2015-11-22 14:44 GMT+09:00 Masahiro Yamada <yamada.masah...@socionext.com>:
>> > Hi Stephen,
>> >
>> >
>> > 2015-11-21 9:37 GMT+09:00 Stephen Boyd <sb...@codeaurora.org>:
>> >> On 11/20, Masahiro Yamada wrote:
>> >>> Currently, there is no function to get the clock name of the given
>> >>> node.  Create a new helper function, of_clk_get_name().  This is
>> >>> useful to get the clock name where "clock-indices" property is used.
>> >>>
>> >>>   of_clk_get_name(): get the clock name in the given node
>> >>>   of_clk_get_parent_name(): get the name of the parent clock
>> >>>
>> >>> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
>> >>> ---
>> >>>
>> >>> I want to use of_clk_get_name() for my clk drivers for my SoCs,
>> >>> which I will submit later.
>> >>>
>> >>> I found this helper function is useful.
>> >>
>> >> I don't see how this is useful. Is the new driver so generic it
>> >> doesn't know what clocks it's outputting? We've been trying to
>> >> move people away from using clock-output-names, so most likely
>> >> this sort of information should be conveyed from DT via the
>> >> compatible string and a table in the driver that matches up the
>> >> compatible string with the list of clock names.
>> >
>> >
>> > What I implemented in my driver was:
>> >
>> > [1] Use the clock names built in the driver, if "clock-output-names"
>> > is missing in the DT.
>> >
>> > [2] If "clock-output-names" is specified, the driver overrides
>> > the clock names, respecting the "clock-output-names".
>> >
>> >
>> > The following is a snippet from my driver code.
>> >
>> >
>> > /*
>> >  * update the clock name with the one provided by
>> >  * clock-output-names property, if exists
>> >  */
>> > new_name = of_clk_get_name(np, index);
>> > if (new_name)
>> > init_data[i].name = new_name;
>> > else
>> > pr_info("DT does not specify output name for %s[%d]\n",
>> > np->name, index);
>> >
>> >
>> > I read the Documentation/devicetree/bindings/clock-bindings.txt
>> > before I stared my driver development.
>> >
>> > I did not know we are deprecating the "clock-output-names".
>> > (Should it be mentioned in the clock-bindings.txt?)
>>
>>
>>
>>
>> Please let me clarify the driver implementation guideline.
>>
>> [1] Do not add "clock-output-names" in new device trees.
>> [2] New clock drivers should not rely on "clock-output-names" at all.
>>
>>
>> Is this correct?
>>
>
> Seems a little extreme, but yes, we would like to see clock
> provider drivers stop using clock-output-names. The binding isn't
> deprecated, so we shouldn't say that in the binding document, but
> perhaps we could add a comment that it's strongly suggested that
> another way be found.
>
> The only use I can think of is for something like fixed rate
> clocks or other pure DT described clocks that want to tell us
> their name. But the end goal is to make that irrelevant by using
> something besides strings (or at least, user definded strings) to
> describe the linkages between clocks in the clock tree. When this
> is done, the clock-output-names property will be unused.

Right.

What is weird about clk_register() is that
it takes the parent name(s), not pointers of the parent(s).

The current way allows us to add clock providers in any order,
but I am not sure if this flexibility is really needed.




-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] clk: let of_clk_get_parent_name() fail for invalid clock-indices

2015-11-30 Thread Masahiro Yamada
Hi Stephen,



>>
>> Of course we can, although we have to mention "clock-indices" twice.
>>
>> A good thing for of_get_property() is that we can get both the value
>> and the length
>> at the same time.
>>
>
> Ok. Well if we don't want to count them again, perhaps a goto
> jump over an unconditional return NULL would be better?
>
> of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) {
> if (index == pv) {
> index = count;
> goto found;
> }
> count++;
> }
>
> return NULL;
> found:
>
> Or since the macro for of_property_for_each_u32() tests the vp
> poitner for NULL, we can check that pointer too...
>
> of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) {
> if (index == pv) {
> index = count;
> break;
> }
> count++;
> }
>
> /* We didn't find anything */
> if (!vp)
> return NULL;
>
> I guess I prefer the latter approach here.
>

No.

Neither of your two suggestions works because they are false positive.

With your way, if "clock-indices" does not exist, of_clk_get_parent_name()
would return NULL;  in this case it should just parse "clock-output-names",
assuming that the clock names are simply indexed from zero.


-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] clk: let of_clk_get_parent_name() fail for invalid clock-indices

2015-11-30 Thread Masahiro Yamada
Currently, of_clk_get_parent_name() returns a wrong parent clock name
when "clock-indices" property exists and the target index is not
found in the property.  In this case, NULL should be returned.

For example,

oscillator {
compatible = "myclocktype";
#clock-cells = <1>;
clock-indices = <1>, <3>;
clock-output-names = "clka", "clkb";
};

consumer {
compatible = "myclockconsumer";
clocks = < 0>, < 1>;
};

Currently, of_clk_get_parent_name(consumer_np, 0) returns "clka"
(and of_clk_get_parent_name(consumer_np, 1) also returns "clka",
this is correct).   Because the "clock-indices" in the clock parent
does not contain <0>, of_clk_get_parent_name(consumer_np, 0) should
return NULL.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

Changes in v2:
  - Rephrase the git-log

 drivers/clk/clk.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 20d8e07..8698074 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3054,12 +3054,9 @@ EXPORT_SYMBOL_GPL(of_clk_get_parent_count);
 const char *of_clk_get_parent_name(struct device_node *np, int index)
 {
struct of_phandle_args clkspec;
-   struct property *prop;
const char *clk_name;
-   const __be32 *vp;
-   u32 pv;
-   int rc;
-   int count;
+   const __be32 *list;
+   int rc, len, i;
struct clk *clk;
 
rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index,
@@ -3068,17 +3065,20 @@ const char *of_clk_get_parent_name(struct device_node 
*np, int index)
return NULL;
 
index = clkspec.args_count ? clkspec.args[0] : 0;
-   count = 0;
 
/* if there is an indices property, use it to transfer the index
 * specified into an array offset for the clock-output-names property.
 */
-   of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) {
-   if (index == pv) {
-   index = count;
-   break;
-   }
-   count++;
+   list = of_get_property(clkspec.np, "clock-indices", );
+   if (list) {
+   len /= sizeof(*list);
+   for (i = 0; i < len; i++)
+   if (index == be32_to_cpup(list++)) {
+   index = i;
+   break;
+   }
+   if (i == len)
+   return NULL;
}
 
if (of_property_read_string_index(clkspec.np, "clock-output-names",
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 16/16] clk: slightly optimize clk_core_set_parent()

2015-11-29 Thread Masahiro Yamada
If clk_fetch_parent_index() fails, p_rate is unused.  Move the
assignment after the error checking.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8a6a33b..479a754 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1730,13 +1730,13 @@ static int clk_core_set_parent(struct clk_core *core, 
struct clk_core *parent)
/* try finding the new parent index */
if (parent) {
p_index = clk_fetch_parent_index(core, parent);
-   p_rate = parent->rate;
if (p_index < 0) {
pr_debug("%s: clk %s can not be parent of clk %s\n",
__func__, parent->name, core->name);
ret = p_index;
goto out;
}
+   p_rate = parent->rate;
}
 
/* propagate PRE_RATE_CHANGE notifications */
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 08/16] clk: drop the initial core->parents look-ups from __clk_core_init()

2015-11-29 Thread Masahiro Yamada
The core->parents is a cache to save expensive clock parent look-ups.
It will be filled as needed later.  We do not have to do it here.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f2758c4..43fb329 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2329,17 +2329,6 @@ static int __clk_core_init(struct clk_core *core)
"%s: invalid NULL in %s's .parent_names\n",
__func__, core->name);
 
-   /*
-* clk_core_lookup returns NULL for parents that have not been
-* clk_init'd; thus any access to clk->parents[] must check
-* for a NULL pointer.  We can always perform lazy lookups for
-* missing parents later on.
-*/
-   if (core->parents)
-   for (i = 0; i < core->num_parents; i++)
-   core->parents[i] =
-   clk_core_lookup(core->parent_names[i]);
-
core->parent = __clk_init_parent(core);
 
/*
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 09/16] clk: replace pr_warn() with pr_err() for fatal cases

2015-11-29 Thread Masahiro Yamada
These three cases let clk_register() fail.  They should be considered
as error messages.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 43fb329..cd96442 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2302,22 +2302,22 @@ static int __clk_core_init(struct clk_core *core)
if (core->ops->set_rate &&
!((core->ops->round_rate || core->ops->determine_rate) &&
  core->ops->recalc_rate)) {
-   pr_warning("%s: %s must implement .round_rate or 
.determine_rate in addition to .recalc_rate\n",
-   __func__, core->name);
+   pr_err("%s: %s must implement .round_rate or .determine_rate in 
addition to .recalc_rate\n",
+  __func__, core->name);
ret = -EINVAL;
goto out;
}
 
if (core->ops->set_parent && !core->ops->get_parent) {
-   pr_warning("%s: %s must implement .get_parent & .set_parent\n",
-   __func__, core->name);
+   pr_err("%s: %s must implement .get_parent & .set_parent\n",
+  __func__, core->name);
ret = -EINVAL;
goto out;
}
 
if (core->ops->set_rate_and_parent &&
!(core->ops->set_parent && core->ops->set_rate)) {
-   pr_warn("%s: %s must implement .set_parent & .set_rate\n",
+   pr_err("%s: %s must implement .set_parent & .set_rate\n",
__func__, core->name);
ret = -EINVAL;
goto out;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 02/16] clk: change the argument of __clk_init() into pointer to clk_core

2015-11-29 Thread Masahiro Yamada
The argument clk_user is used only for the clk_user->core.  The rest
of this function only takes care of clk_core.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 65530e9..8c8ba91 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2285,25 +2285,22 @@ static inline void clk_debug_unregister(struct clk_core 
*core)
 #endif
 
 /**
- * __clk_init - initialize the data structures in a struct clk
- * @clk:   clk being initialized
+ * __clk_init - initialize the data structures in a struct clk_core
+ * @core:  clk_core being initialized
  *
  * Initializes the lists in struct clk_core, queries the hardware for the
  * parent and rate and sets them both.
  */
-static int __clk_init(struct clk *clk_user)
+static int __clk_init(struct clk_core *core)
 {
int i, ret = 0;
struct clk_core *orphan;
struct hlist_node *tmp2;
-   struct clk_core *core;
unsigned long rate;
 
-   if (!clk_user)
+   if (!core)
return -EINVAL;
 
-   core = clk_user->core;
-
clk_prepare_lock();
 
/* check to see if a clock with this name is already registered */
@@ -2574,7 +2571,7 @@ struct clk *clk_register(struct device *dev, struct 
clk_hw *hw)
goto fail_parent_names_copy;
}
 
-   ret = __clk_init(hw->clk);
+   ret = __clk_init(core);
if (!ret)
return hw->clk;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 10/16] clk: move checking .git_parent to __clk_core_init()

2015-11-29 Thread Masahiro Yamada
The .git_parent is mandatory for multi-parent clocks.  Move the check
to __clk_core_init(), like other callback checkings.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index cd96442..486f6d4 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1677,13 +1677,6 @@ static struct clk_core *__clk_init_parent(struct 
clk_core *core)
goto out;
}
 
-   if (!core->ops->get_parent) {
-   WARN(!core->ops->get_parent,
-   "%s: multi-parent clocks must implement .get_parent\n",
-   __func__);
-   goto out;
-   }
-
/*
 * Do our best to cache parent clocks in core->parents.  This prevents
 * unnecessary and expensive lookups.  We don't set core->parent here;
@@ -2315,6 +2308,11 @@ static int __clk_core_init(struct clk_core *core)
goto out;
}
 
+   if (core->num_parents > 1 && !core->ops->get_parent) {
+   pr_err("%s: %s must implement .get_parent as it has multi 
parents\n",
+  __func__, core->name);
+   }
+
if (core->ops->set_rate_and_parent &&
!(core->ops->set_parent && core->ops->set_rate)) {
pr_err("%s: %s must implement .set_parent & .set_rate\n",
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 03/16] clk: rename __clk_init() into __clk_core_init()

2015-11-29 Thread Masahiro Yamada
Now this function takes clk_core as its argument.  __clk_core_init()
would be more suitable for the name of this function.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8c8ba91..36373d3 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2285,13 +2285,13 @@ static inline void clk_debug_unregister(struct clk_core 
*core)
 #endif
 
 /**
- * __clk_init - initialize the data structures in a struct clk_core
+ * __clk_core_init - initialize the data structures in a struct clk_core
  * @core:  clk_core being initialized
  *
  * Initializes the lists in struct clk_core, queries the hardware for the
  * parent and rate and sets them both.
  */
-static int __clk_init(struct clk_core *core)
+static int __clk_core_init(struct clk_core *core)
 {
int i, ret = 0;
struct clk_core *orphan;
@@ -2571,7 +2571,7 @@ struct clk *clk_register(struct device *dev, struct 
clk_hw *hw)
goto fail_parent_names_copy;
}
 
-   ret = __clk_init(core);
+   ret = __clk_core_init(core);
if (!ret)
return hw->clk;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 11/16] clk: simplify __clk_init_parent()

2015-11-29 Thread Masahiro Yamada
The translation from the index into clk_core is done by
clk_core_get_parent_by_index().  The if-block for num_parents == 1
case is duplicating the code in the clk_core_get_parent_by_index().

Drop the "if (num_parents == 1)" from the special case.  Instead,
set the index to zero if .get_parent() is missing.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk.c | 38 --
 1 file changed, 4 insertions(+), 34 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 486f6d4..ef6fedb 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1651,44 +1651,14 @@ struct clk *clk_get_parent(struct clk *clk)
 }
 EXPORT_SYMBOL_GPL(clk_get_parent);
 
-/*
- * .get_parent is mandatory for clocks with multiple possible parents.  It is
- * optional for single-parent clocks.  Always call .get_parent if it is
- * available and WARN if it is missing for multi-parent clocks.
- *
- * For single-parent clocks without .get_parent, first check to see if the
- * .parents array exists, and if so use it to avoid an expensive tree
- * traversal.  If .parents does not exist then walk the tree.
- */
 static struct clk_core *__clk_init_parent(struct clk_core *core)
 {
-   struct clk_core *ret = NULL;
-   u8 index;
-
-   /* handle the trivial cases */
+   u8 index = 0;
 
-   if (!core->num_parents)
-   goto out;
-
-   if (core->num_parents == 1) {
-   if (IS_ERR_OR_NULL(core->parent))
-   core->parent = clk_core_lookup(core->parent_names[0]);
-   ret = core->parent;
-   goto out;
-   }
+   if (core->ops->get_parent)
+   index = core->ops->get_parent(core->hw);
 
-   /*
-* Do our best to cache parent clocks in core->parents.  This prevents
-* unnecessary and expensive lookups.  We don't set core->parent here;
-* that is done by the calling function.
-*/
-
-   index = core->ops->get_parent(core->hw);
-
-   ret = clk_core_get_parent_by_index(core, index);
-
-out:
-   return ret;
+   return clk_core_get_parent_by_index(core, index);
 }
 
 static void clk_core_reparent(struct clk_core *core,
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 06/16] clk: move core->parents allocation to clk_register()

2015-11-29 Thread Masahiro Yamada
Currently, __clk_core_init() allows failure of the kcalloc() for the
core->parents.  So, clk_fetch_parent_index() and __clk_init_parent()
also try to allocate core->parents in case it has not been allocated
yet.  Scattering memory allocation here and there makes things
complicated.

Like other clk_core members, allocate core->parents in clk_register()
and let it fail in case of memory shortage.  If we cannot allocate
such a small piece of memory, the system is already insane.  There is
no point to postpone the memory allocation.

Also, allocate core->parents regardless of core->num_parents.  We want
it even if core->num_parents == 1 because clk_fetch_parent_index()
might be called against the clk_core with a single parent.

If core->num_parents == 0, core->parents is set to ZERO_SIZE_PTR. It
is harmless because no access happens to core->parents in such a case.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk.c | 51 +++
 1 file changed, 19 insertions(+), 32 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 0f80c69..e05084e 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1067,13 +1067,6 @@ static int clk_fetch_parent_index(struct clk_core *core,
 {
int i;
 
-   if (!core->parents) {
-   core->parents = kcalloc(core->num_parents,
-   sizeof(*core->parents), GFP_KERNEL);
-   if (!core->parents)
-   return -ENOMEM;
-   }
-
/*
 * find index of new parent clock using cached parent ptrs,
 * or if not yet cached, use string name comparison and cache
@@ -1700,11 +1693,6 @@ static struct clk_core *__clk_init_parent(struct 
clk_core *core)
 
index = core->ops->get_parent(core->hw);
 
-   if (!core->parents)
-   core->parents =
-   kcalloc(core->num_parents, sizeof(*core->parents),
-   GFP_KERNEL);
-
ret = clk_core_get_parent_by_index(core, index);
 
 out:
@@ -2343,26 +2331,15 @@ static int __clk_core_init(struct clk_core *core)
__func__, core->name);
 
/*
-* Allocate an array of struct clk *'s to avoid unnecessary string
-* look-ups of clk's possible parents.  This can fail for clocks passed
-* in to clk_init during early boot; thus any access to core->parents[]
-* must always check for a NULL pointer and try to populate it if
-* necessary.
+* clk_core_lookup returns NULL for parents that have not been
+* clk_init'd; thus any access to clk->parents[] must check
+* for a NULL pointer.  We can always perform lazy lookups for
+* missing parents later on.
 */
-   if (core->num_parents > 1) {
-   core->parents = kcalloc(core->num_parents,
-   sizeof(*core->parents), GFP_KERNEL);
-   /*
-* clk_core_lookup returns NULL for parents that have not been
-* clk_init'd; thus any access to clk->parents[] must check
-* for a NULL pointer.  We can always perform lazy lookups for
-* missing parents later on.
-*/
-   if (core->parents)
-   for (i = 0; i < core->num_parents; i++)
-   core->parents[i] =
-   clk_core_lookup(core->parent_names[i]);
-   }
+   if (core->parents)
+   for (i = 0; i < core->num_parents; i++)
+   core->parents[i] =
+   clk_core_lookup(core->parent_names[i]);
 
core->parent = __clk_init_parent(core);
 
@@ -2560,12 +2537,20 @@ struct clk *clk_register(struct device *dev, struct 
clk_hw *hw)
}
}
 
+   /* avoid unnecessary string look-ups of clk_core's possible parents. */
+   core->parents = kcalloc(core->num_parents, sizeof(*core->parents),
+   GFP_KERNEL);
+   if (!core->parents) {
+   ret = -ENOMEM;
+   goto fail_parents;
+   };
+
INIT_HLIST_HEAD(>clks);
 
hw->clk = __clk_create_clk(hw, NULL, NULL);
if (IS_ERR(hw->clk)) {
ret = PTR_ERR(hw->clk);
-   goto fail_parent_names_copy;
+   goto fail_parents;
}
 
ret = __clk_core_init(core);
@@ -2575,6 +2560,8 @@ struct clk *clk_register(struct device *dev, struct 
clk_hw *hw)
__clk_free_clk(hw->clk);
hw->clk = NULL;
 
+fail_parents:
+   kfree(core->parents);
 fail_parent_names_copy:
while (--i >= 0)
kfree_const(core->parent_names[i]

Re: [PATCH 2/3] clk: let of_clk_get_parent_name() fail for invalid clock-indices

2015-11-21 Thread Masahiro Yamada
Hi Stephen,


2015-11-21 2:45 GMT+09:00 Stephen Boyd <sb...@codeaurora.org>:
> On 11/20, Masahiro Yamada wrote:
>> Currently, of_clk_get_parent_name() returns a wrong parent clock name
>> when "clock-indices" property exists and the given index is not found
>> in the property.  In this case, NULL should be returned.
>>
>> For example,
>>
>> oscillator {
>> compatible = "myclocktype";
>> #clock-cells = <1>;
>> clock-indices = <1>, <3>;
>> clock-output-names = "clka", "clkb";
>> };
>>
>> Currently, of_clk_get_parent_name(np, 0) returns "clka", but should
>> return NULL because "clock-indices" does not contain <0>.
>
> What is np pointing at? Something like:
>
> consumer {
> clocks = < 0>;
> };
>
> Which would be invalid DT because oscillator doesn't have an
> output for index 0?


You are right.  My example was confusing.



 oscillator: oscillator {
 compatible = "myclocktype";
 #clock-cells = <1>;
 clock-indices = <1>, <3>;
 clock-output-names = "clka", "clkb";
 };

 consumer {
 compatible = "myclockconsumer";
 clocks = < 0>;
 };

Currently, of_clk_get_parent_name(consumer_np, 0) returns "clks", but
should return NULL;

I will rephrase the git-log in v2.




>>
>> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
>> @@ -3068,17 +3065,20 @@ const char *of_clk_get_parent_name(struct 
>> device_node *np, int index)
>>   return NULL;
>>
>>   index = clkspec.args_count ? clkspec.args[0] : 0;
>> - count = 0;
>>
>>   /* if there is an indices property, use it to transfer the index
>>* specified into an array offset for the clock-output-names property.
>>*/
>> - of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) {
>> - if (index == pv) {
>> - index = count;
>> - break;
>> - }
>> - count++;
>> + list = of_get_property(clkspec.np, "clock-indices", );
>> + if (list) {
>> + len /= sizeof(*list);
>> + for (i = 0; i < len; i++)
>> + if (index == be32_to_cpup(list++)) {
>> + index = i;
>> + break;
>> + }
>> + if (i == len)
>> + return NULL;
>>   }
>
> Why can't we leave everything in place and check count == len at
> the end? i.e.
>
> of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) {
> if (index == pv) {
> index = count;
> break;
> }
> count++;
> }
>
> if (count == of_property_count_u32_elems(clkspec.np, "clock-indices"))
> return NULL
>


Of course we can, although we have to mention "clock-indices" twice.

A good thing for of_get_property() is that we can get both the value
and the length
at the same time.



-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] clk: split of_clk_get_parent_name() into two functions

2015-11-21 Thread Masahiro Yamada
Hi Stephen,


2015-11-21 9:37 GMT+09:00 Stephen Boyd <sb...@codeaurora.org>:
> On 11/20, Masahiro Yamada wrote:
>> Currently, there is no function to get the clock name of the given
>> node.  Create a new helper function, of_clk_get_name().  This is
>> useful to get the clock name where "clock-indices" property is used.
>>
>>   of_clk_get_name(): get the clock name in the given node
>>   of_clk_get_parent_name(): get the name of the parent clock
>>
>> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
>> ---
>>
>> I want to use of_clk_get_name() for my clk drivers for my SoCs,
>> which I will submit later.
>>
>> I found this helper function is useful.
>
> I don't see how this is useful. Is the new driver so generic it
> doesn't know what clocks it's outputting? We've been trying to
> move people away from using clock-output-names, so most likely
> this sort of information should be conveyed from DT via the
> compatible string and a table in the driver that matches up the
> compatible string with the list of clock names.


What I implemented in my driver was:

[1] Use the clock names built in the driver, if "clock-output-names"
is missing in the DT.

[2] If "clock-output-names" is specified, the driver overrides
the clock names, respecting the "clock-output-names".


The following is a snippet from my driver code.


/*
 * update the clock name with the one provided by
 * clock-output-names property, if exists
 */
new_name = of_clk_get_name(np, index);
if (new_name)
init_data[i].name = new_name;
else
pr_info("DT does not specify output name for %s[%d]\n",
np->name, index);


I read the Documentation/devicetree/bindings/clock-bindings.txt
before I stared my driver development.

I did not know we are deprecating the "clock-output-names".
(Should it be mentioned in the clock-bindings.txt?)


-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] clk: use IS_ERR_OR_NULL(hw) instead of !hw || IS_ERR(hw)

2015-11-19 Thread Masahiro Yamada
This minor refactoring does not change the function behavior.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f13c3f4..764aca2 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2482,7 +2482,7 @@ struct clk *__clk_create_clk(struct clk_hw *hw, const 
char *dev_id,
struct clk *clk;
 
/* This is to allow this function to be chained to others */
-   if (!hw || IS_ERR(hw))
+   if (IS_ERR_OR_NULL(hw))
return (struct clk *) hw;
 
clk = kzalloc(sizeof(*clk), GFP_KERNEL);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3] clk: split of_clk_get_parent_name() into two functions

2015-11-19 Thread Masahiro Yamada
Currently, there is no function to get the clock name of the given
node.  Create a new helper function, of_clk_get_name().  This is
useful to get the clock name where "clock-indices" property is used.

  of_clk_get_name(): get the clock name in the given node
  of_clk_get_parent_name(): get the name of the parent clock

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

I want to use of_clk_get_name() for my clk drivers for my SoCs,
which I will submit later.

I found this helper function is useful.


 drivers/clk/clk.c| 45 +++-
 include/linux/clk-provider.h |  1 +
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8698074..1788ec7 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3051,25 +3051,17 @@ int of_clk_get_parent_count(struct device_node *np)
 }
 EXPORT_SYMBOL_GPL(of_clk_get_parent_count);
 
-const char *of_clk_get_parent_name(struct device_node *np, int index)
+const char *of_clk_get_name(struct device_node *np, int index)
 {
-   struct of_phandle_args clkspec;
const char *clk_name;
const __be32 *list;
-   int rc, len, i;
-   struct clk *clk;
-
-   rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index,
-   );
-   if (rc)
-   return NULL;
-
-   index = clkspec.args_count ? clkspec.args[0] : 0;
+   int len, i, rc;
 
-   /* if there is an indices property, use it to transfer the index
+   /*
+* if there is an indices property, use it to transfer the index
 * specified into an array offset for the clock-output-names property.
 */
-   list = of_get_property(clkspec.np, "clock-indices", );
+   list = of_get_property(np, "clock-indices", );
if (list) {
len /= sizeof(*list);
for (i = 0; i < len; i++)
@@ -3081,9 +3073,29 @@ const char *of_clk_get_parent_name(struct device_node 
*np, int index)
return NULL;
}
 
-   if (of_property_read_string_index(clkspec.np, "clock-output-names",
- index,
- _name) < 0) {
+   rc = of_property_read_string_index(np, "clock-output-names", index,
+  _name);
+
+   return rc ? NULL : clk_name;
+}
+EXPORT_SYMBOL_GPL(of_clk_get_name);
+
+const char *of_clk_get_parent_name(struct device_node *np, int index)
+{
+   struct of_phandle_args clkspec;
+   const char *clk_name;
+   struct clk *clk;
+   int rc;
+
+   rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index,
+   );
+   if (rc)
+   return NULL;
+
+   index = clkspec.args_count ? clkspec.args[0] : 0;
+
+   clk_name = of_clk_get_name(clkspec.np, index);
+   if (!clk_name) {
/*
 * Best effort to get the name if the clock has been
 * registered with the framework. If the clock isn't
@@ -3102,7 +3114,6 @@ const char *of_clk_get_parent_name(struct device_node 
*np, int index)
}
}
 
-
of_node_put(clkspec.np);
return clk_name;
 }
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index e7bd710..c6ff498 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -702,6 +702,7 @@ struct clk *of_clk_src_onecell_get(struct of_phandle_args 
*clkspec, void *data);
 int of_clk_get_parent_count(struct device_node *np);
 int of_clk_parent_fill(struct device_node *np, const char **parents,
   unsigned int size);
+const char *of_clk_get_name(struct device_node *np, int index);
 const char *of_clk_get_parent_name(struct device_node *np, int index);
 
 void of_clk_init(const struct of_device_id *matches);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/3] clk: let of_clk_get_parent_name() fail for invalid clock-indices

2015-11-19 Thread Masahiro Yamada
Currently, of_clk_get_parent_name() returns a wrong parent clock name
when "clock-indices" property exists and the given index is not found
in the property.  In this case, NULL should be returned.

For example,

oscillator {
compatible = "myclocktype";
#clock-cells = <1>;
clock-indices = <1>, <3>;
clock-output-names = "clka", "clkb";
};

Currently, of_clk_get_parent_name(np, 0) returns "clka", but should
return NULL because "clock-indices" does not contain <0>.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 20d8e07..8698074 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3054,12 +3054,9 @@ EXPORT_SYMBOL_GPL(of_clk_get_parent_count);
 const char *of_clk_get_parent_name(struct device_node *np, int index)
 {
struct of_phandle_args clkspec;
-   struct property *prop;
const char *clk_name;
-   const __be32 *vp;
-   u32 pv;
-   int rc;
-   int count;
+   const __be32 *list;
+   int rc, len, i;
struct clk *clk;
 
rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index,
@@ -3068,17 +3065,20 @@ const char *of_clk_get_parent_name(struct device_node 
*np, int index)
return NULL;
 
index = clkspec.args_count ? clkspec.args[0] : 0;
-   count = 0;
 
/* if there is an indices property, use it to transfer the index
 * specified into an array offset for the clock-output-names property.
 */
-   of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) {
-   if (index == pv) {
-   index = count;
-   break;
-   }
-   count++;
+   list = of_get_property(clkspec.np, "clock-indices", );
+   if (list) {
+   len /= sizeof(*list);
+   for (i = 0; i < len; i++)
+   if (index == be32_to_cpup(list++)) {
+   index = i;
+   break;
+   }
+   if (i == len)
+   return NULL;
}
 
if (of_property_read_string_index(clkspec.np, "clock-output-names",
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] clk: remove redundant negative index check in of_clk_get_parent_name()

2015-11-19 Thread Masahiro Yamada
This if-block can be dropped because the of_parse_phandle_with_args()
in the following line returns -EINVAL for negative index.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 764aca2..20d8e07 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3062,9 +3062,6 @@ const char *of_clk_get_parent_name(struct device_node 
*np, int index)
int count;
struct clk *clk;
 
-   if (index < 0)
-   return NULL;
-
rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index,
);
if (rc)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


How to implement common clk in multi function controller?

2015-11-10 Thread Masahiro Yamada
Hi.

I am implementing clk and reset drivers for my SoCs.
(drivers/clk/uniphier/* and drivers/reset/uniphier/*)


In my SoCs, one hardware block contains various
registers for both clock and reset controlling.
(so, it is like a MFD system controller device).
I think it is a common case.


I am guessing my device tree would be like follows:
(one syscon device contains clk and rst devices under it)

syscon {
 compatible = "socionext,uniphier-syscon",
 "syscon", "simple-mfd";
 reg = <...   ...>;

 clk_ctrl {
.compatible = "socionext,uniphier-clkctrl";
#clock-cells = <1>;
 };

 rst_ctrl {
 .compatible = "socionext,uniphier-rstctrl";
 #reset-cells = <1>;
 };
};



One problem I noticed was,
we are supposed to use regmap for register access
if we use syscon.

OTOH, common clk APIs such as clk-gate, clk-divider
expect simple register access via writel()/readl().

Is it a good idea to expand such APIs to regmap?


Of course, I could my own uniphier/clk-gate.c
to use regmap as other SoCs do.

But, I think regmap is general demand, so
I am wondering if it could be supported in common parts.

Or, any other good solution exists?



-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] clk: fix codying style of if ... else blocks

2015-11-05 Thread Masahiro Yamada
This code is unreadable due to the blank line between if and else
blocks.

Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
---

 drivers/clk/clk-mux.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 7129c86..5ed03c8 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -71,10 +71,9 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
u32 val;
unsigned long flags = 0;
 
-   if (mux->table)
+   if (mux->table) {
index = mux->table[index];
-
-   else {
+   } else {
if (mux->flags & CLK_MUX_INDEX_BIT)
index = 1 << index;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: ML for reset controller sub-system?

2015-11-02 Thread Masahiro Yamada
Hi Philipp,

2015-11-02 22:24 GMT+09:00 Philipp Zabel <p.za...@pengutronix.de>:
> Hi Masahiro,
>
> Am Montag, den 02.11.2015, 22:05 +0900 schrieb Masahiro Yamada:
>> Hi Philipp,
>>
>>
>> I searched for the reset controller entry in MAINTAINERS.
>>
>>
>> RESET CONTROLLER FRAMEWORK
>> M:  Philipp Zabel <p.za...@pengutronix.de>
>> S:  Maintained
>> F:  drivers/reset/
>> F:  Documentation/devicetree/bindings/reset/
>> F:  include/dt-bindings/reset/
>> F:  include/linux/reset.h
>> F:  include/linux/reset-controller.h
>>
>>
>> Nothing is mentioned which mailing list
>> reset controller patches should be sent to.
>>
>> Please advise me.
>
> There is no special reset-controller mailing list. Due to the low amount
> of patches until now, I'm just using linux-ker...@vger.kernel.org.
> scripts/get_maintainer.pl should give you the right list for existing
> drivers.
>
> For new drivers consider adding the SoC/architecture specific mailing
> list to --cc, especially when coordination between reset and other
> drivers is needed during bringup. If new device tree bindings are added,
> always include devicet...@vger.kernel.org.
>

This helped me a lot.

Thanks!



-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html