ARM: TI SoC clock DT conversion

Current clock wrappers assume simple and direct mapped hardware register access. Improve this support by adding functionality for registering platform specific clock I/O wrappers, which can be used to support various features needed like endianess conversions, indexed regmap support, etc. Default I/O wrapper provided also which uses the existing direct I/O mapped behavior.

Signed-off-by Tero Kristo <[email protected]>

---
 drivers/clk/clk.c            |   68 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/clk-provider.h |   15 +++++-----
 2 files changed, 75 insertions(+), 8 deletions(-)

drivers/clk/clk.c +68 -0

index 2cf2ea6..c331386 100644
@@ -34,6 +34,74 @@ static HLIST_HEAD(clk_root_list);
 static HLIST_HEAD(clk_orphan_list);
 static LIST_HEAD(clk_notifier_list);
 
+/**
+ * clk_readl_default - default clock register read support function
+ * @reg: register to read
+ *
+ * Default implementation for reading a clock register.
+ */
+static u32 clk_readl_default(u32 __iomem *reg)
+{
+       return readl(reg);
+}
+
+/**
+ * clk_writel_default - default clock register write support function
+ * @val: value to write
+ * @reg: register to write to
+ *
+ * Default implementation for writing a clock register.
+ */
+static void clk_writel_default(u32 val, u32 __iomem *reg)
+{
+       writel(val, reg);
+}
+
+struct clk_reg_ops clk_reg_ops_default = {
+       .clk_readl = clk_readl_default,
+       .clk_writel = clk_writel_default
+};
+
+static struct clk_reg_ops *clk_reg_ops = &clk_reg_ops_default;
+
+/**
+ * clk_register_reg_ops - register access functions for clock registers
+ * @ops: register level ops
+ *
+ * Registers platform or SoC specific operations for reading / writing
+ * clock registers.
+ */
+int clk_register_reg_ops(struct clk_reg_ops *ops)
+{
+       if (!ops)
+               return -EINVAL;
+       clk_reg_ops = ops;
+       return 0;
+}
+
+/**
+ * clk_readl - read a clock register value from hardware
+ * @reg: register to read
+ *
+ * Uses the registered clk_reg_ops to read a hardware clock register value.
+ */
+u32 clk_readl(u32 __iomem *reg)
+{
+       return clk_reg_ops->clk_readl(reg);
+}
+
+/**
+ * clk_writel - write a clock register value to hardware
+ * @val: value to write
+ * @reg: register to write
+ *
+ * Uses the registered clk_reg_ops to write a hardware clock register value.
+ */
+void clk_writel(u32 val, u32 __iomem *reg)
+{
+       clk_reg_ops->clk_writel(val, reg);
+}
+
 /***           locking             ***/
 static void clk_prepare_lock(void)
 {

include/linux/clk-provider.h +7 -8

index 7e59253..16e4df2 100644
@@ -512,15 +512,14 @@ static inline const char *of_clk_get_parent_name(struct device_node *np,
  * for improved portability across platforms
  */
 
-static inline u32 clk_readl(u32 __iomem *reg)
-{
-       return readl(reg);
-}
+struct clk_reg_ops {
+       u32 (*clk_readl)(u32 __iomem *reg);
+       void (*clk_writel)(u32 val, u32 __iomem *reg);
+};
 
-static inline void clk_writel(u32 val, u32 __iomem *reg)
-{
-       writel(val, reg);
-}
+u32 clk_readl(u32 __iomem *reg);
+void clk_writel(u32 val, u32 __iomem *reg);
+int clk_register_reg_ops(struct clk_reg_ops *ops);
 
 #endif /* CONFIG_COMMON_CLK */
 #endif /* CLK_PROVIDER_H */

Signature · 1 line

1.7.9.5

Quoting Tero Kristo (2013-11-26 00:05:42)

Quoted 7 lines

Current clock wrappers assume simple and direct mapped hardware register access. Improve this support by adding functionality for registering platform specific clock I/O wrappers, which can be used to support various features needed like endianess conversions, indexed regmap support, etc. Default I/O wrapper provided also which uses the existing direct I/O mapped behavior.

Quoted 1 line

Signed-off-by Tero Kristo <[email protected]>

There is a separate discussion on removing regmap for reading omap clk registers. I guess this patch is not needed in that case?

Regards,
Mike

Quoted 114 lines

---
  drivers/clk/clk.c            |   68 ++++++++++++++++++++++++++++++++++++++++++
  include/linux/clk-provider.h |   15 +++++-----
  2 files changed, 75 insertions(+), 8 deletions(-)

drivers/clk/clk.c +68 -0

index 2cf2ea6..c331386 100644
@@ -34,6 +34,74 @@ static HLIST_HEAD(clk_root_list);
  static HLIST_HEAD(clk_orphan_list);
  static LIST_HEAD(clk_notifier_list);
 
+/**
+ * clk_readl_default - default clock register read support function
+ * @reg: register to read
+ *
+ * Default implementation for reading a clock register.
+ */
+static u32 clk_readl_default(u32 __iomem *reg)
+{
+       return readl(reg);
+}
+
+/**
+ * clk_writel_default - default clock register write support function
+ * @val: value to write
+ * @reg: register to write to
+ *
+ * Default implementation for writing a clock register.
+ */
+static void clk_writel_default(u32 val, u32 __iomem *reg)
+{
+       writel(val, reg);
+}
+
+struct clk_reg_ops clk_reg_ops_default = {
+       .clk_readl = clk_readl_default,
+       .clk_writel = clk_writel_default
+};
+
+static struct clk_reg_ops *clk_reg_ops = &clk_reg_ops_default;
+
+/**
+ * clk_register_reg_ops - register access functions for clock registers
+ * @ops: register level ops
+ *
+ * Registers platform or SoC specific operations for reading / writing
+ * clock registers.
+ */
+int clk_register_reg_ops(struct clk_reg_ops *ops)
+{
+       if (!ops)
+               return -EINVAL;
+       clk_reg_ops = ops;
+       return 0;
+}
+
+/**
+ * clk_readl - read a clock register value from hardware
+ * @reg: register to read
+ *
+ * Uses the registered clk_reg_ops to read a hardware clock register value.
+ */
+u32 clk_readl(u32 __iomem *reg)
+{
+       return clk_reg_ops->clk_readl(reg);
+}
+
+/**
+ * clk_writel - write a clock register value to hardware
+ * @val: value to write
+ * @reg: register to write
+ *
+ * Uses the registered clk_reg_ops to write a hardware clock register value.
+ */
+void clk_writel(u32 val, u32 __iomem *reg)
+{
+       clk_reg_ops->clk_writel(val, reg);
+}
+
  /***           locking             ***/
  static void clk_prepare_lock(void)
  {

include/linux/clk-provider.h +7 -9

index 7e59253..16e4df2 100644
@@ -512,15 +512,14 @@ static inline const char *of_clk_get_parent_name(struct device_node *np,
   * for improved portability across platforms
   */
 
-static inline u32 clk_readl(u32 __iomem *reg)
-{
-       return readl(reg);
-}
+struct clk_reg_ops {
+       u32 (*clk_readl)(u32 __iomem *reg);
+       void (*clk_writel)(u32 val, u32 __iomem *reg);
+};
 
-static inline void clk_writel(u32 val, u32 __iomem *reg)
-{
-       writel(val, reg);
-}
+u32 clk_readl(u32 __iomem *reg);
+void clk_writel(u32 val, u32 __iomem *reg);
+int clk_register_reg_ops(struct clk_reg_ops *ops);
 
  #endif /* CONFIG_COMMON_CLK */
  #endif /* CLK_PROVIDER_H */
--

1.7.9.5

Current clock wrappers assume simple and direct mapped hardware register access. Improve this support by adding functionality for registering platform specific clock I/O wrappers, which can be used to support various features needed like endianess conversions, indexed regmap support, etc. Default I/O wrapper provided also which uses the existing direct I/O mapped behavior.

Quoted 6 lines · Tero Kristo

Current clock wrappers assume simple and direct mapped hardware register access. Improve this support by adding functionality for registering platform specific clock I/O wrappers, which can be used to support various features needed like endianess conversions, indexed regmap support, etc. Default I/O wrapper provided also which uses the existing direct I/O mapped behavior.

I think it makes more sense to define per-SoC register read and write functions,

for the same reason that I think it makes more sense to define clock data as part of the IP blocks that contain the clocks' registers. The register read and write functions should be implemented by the drivers for the IP blocks that the clocks are contained in. That way the register read and write functions can take whatever steps are necessary to ensure that the IP block registers are accessible before performing the read/write; can implement non-MMIO register accesses if needed; and can operate on register offsets, rather than absolute addresses.

Something like the following draft implementation. Also needed would be a way for clock providers that rely on the common clock providers (e.g., divider, mux, etc.) to pass in the struct clk_ll_ops pointer. I guess the simplest way to implement this would be to add another set of registration functions - e.g., clk_register_divider_ipb() or clk_register_divider2() or something similar. These functions would be similar to clk_register_divider(), but would take a pointer to a struct clk_ll_ops and pass that along to the CCF core.

ti_dt_clk_init_provider() can now be used to initialize the contents of a single clock IP block. This parses all the clocks under the IP block and calls the corresponding init function for them.

This patch also introduces a helper function for the TI clock drivers to get register info from DT and append the master IP info to this.

Signed-off-by Tero Kristo <[email protected]>

drivers/clk/ti/clk.c +0 -0

index ef1a7cd..63f85e9 100644

drivers/clk/ti/clk.c +72 -0

+
+struct clk_init_item {
+	int index;
+	struct device_node *np;
+	ti_of_clk_init_cb_t init_cb;
+	struct list_head node;
+};
+
+static LIST_HEAD(retry_list);
+
+/**
+ * ti_clk_get_reg_addr - get register address for a clock register
+ * @node: device node for the clock
+ * @index: register index from the clock node
+ *
+ * Builds clock register address from device tree information. This
+ * is a struct of type clk_omap_reg.
+ */
+void __iomem *ti_clk_get_reg_addr(struct device_node *node, int index)
+{
+	struct clk_omap_reg *reg;
+	u32 val;
+	u32 tmp;
+
+	reg = (struct clk_omap_reg *)&tmp;
+	reg->index = ti_dt_clk_regmap_index;
+
+	if (of_property_read_u32_index(node, "reg", index, &val)) {
+		pr_err("%s must have reg[%d]!\n", node->name, index);
+		return NULL;
+	}
+
+	reg->offset = val;
+
+	return (void __iomem *)tmp;
+}
+
+/**
+ * ti_dt_clk_init_provider - init master clock provider
+ * @parent: master node
+ * @index: internal index for clk_reg_ops
+ *
+ * Initializes a master clock IP block and its child clock nodes.
+ * Regmap is provided for accessing the register space for the
+ * IP block and all the clocks under it.
+ */
+void ti_dt_clk_init_provider(struct device_node *parent, int index)
+{
+	const struct of_device_id *match;
+	struct device_node *np;
+	ti_of_clk_init_cb_t clk_init_cb;
+	struct clk_init_item *retry;
+	struct clk_init_item *tmp;
+	int ret;
+
+	ti_dt_clk_regmap_index = index;
+
+	for_each_child_of_node(parent, np) {
+		match = of_match_node(__clk_of_table, np);
+		if (!match)
+			continue;
+		clk_init_cb = (ti_of_clk_init_cb_t)match->data;
+		pr_debug("%s: initializing: %s\n", __func__, np->name);
+		ret = clk_init_cb(np);
+		if (ret == -EAGAIN) {
+			pr_debug("%s: adding to again list...\n", np->name);
+			retry = kzalloc(sizeof(*retry), GFP_KERNEL);
+			retry->np = np;
+			retry->init_cb = clk_init_cb;
+			list_add(&retry->node, &retry_list);
+		} else if (ret) {
+			pr_err("%s: clock init failed for %s (%d)!\n", __func__,

drivers/clk/ti/clk.c +9 -0

+		} else {
+			if (ret)
+				pr_err("%s: clock init failed for %s (%d)!\n",
+				       __func__, retry->np->name, ret);
+			list_del(&retry->node);
+			kfree(retry);
+		}
+	}
+}

include/linux/clk/ti.h +14 -0

index df94c24..d6bf530 100644
@@ -36,7 +36,21 @@ struct ti_dt_clk {
 		.node_name = name,	\
 	}
 
+/* Maximum number of clock regmaps */
+#define CLK_MAX_REGMAPS			4
 
+/**
+ * struct clk_omap_reg - OMAP register declaration
+ * @offset: offset from the master IP module base address
+ * @index: index of the master IP module
+ */
+struct clk_omap_reg {
+	u16 offset;
+	u16 index;
+};
+
+void __iomem *ti_clk_get_reg_addr(struct device_node *node, int index);
 void ti_dt_clocks_register(struct ti_dt_clk *oclks);
+void ti_dt_clk_init_provider(struct device_node *np, int index);
 
 #endif

Signature · 1 line

1.7.9.5

Quoted 8 lines · Tero Kristo

ti_dt_clk_init_provider() can now be used to initialize the contents of a single clock IP block. This parses all the clocks under the IP block and calls the corresponding init function for them.

This patch also introduces a helper function for the TI clock drivers to get register info from DT and append the master IP info to this.

Signed-off-by Tero Kristo <[email protected]>

...

Quoted 14 lines

drivers/clk/ti/clk.c +2 -0

index ef1a7cd..63f85e9 100644
@@ -19,10 +19,15 @@
  #include <linux/clkdev.h>
  #include <linux/clk/ti.h>
  #include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/list.h>
 
  #undef pr_fmt
  #define pr_fmt(fmt) "%s: " fmt, __func__
 

drivers/clk/ti/clk.c line 28

Quoted 1 line

drivers/clk/ti/clk.c +1 -0

+extern struct of_device_id __clk_of_table[];

This results in a checkpatch.pl warning:

WARNING: externs should be avoided in .c files
#33: FILE: drivers/clk/ti/clk.c:28:
+extern struct of_device_id __clk_of_table[];

Please make sure your patches are checkpatch.pl-clean, with the exception of the 80-column warnings and any const-related warnings related to the clock framework.

drivers/clk/ti/clk.c void __init ti_dt_clocks_register(struct ti_dt_clk oclks[]) · line 62

Quoted 11 lines

drivers/clk/ti/clk.c +4 -0

+static int ti_dt_clk_regmap_index;
+
  /**
   * ti_dt_clocks_register - register DT alias clocks during boot
   * @oclks: list of clocks to register

@@ -53,3 +58,96 @@ void __init ti_dt_clocks_register(struct ti_dt_clk oclks[])
  		}
  	}
  }
+
+typedef int (*ti_of_clk_init_cb_t)(struct device_node *);

Normally typedefs should be a red flag to reviewers due to Documentation/CodingStyle chapter 5. Still it seems this patch is just duplicating the way that the CCF does it, so I'm not too worried about it.

Quoted 78 lines

drivers/clk/ti/clk.c +77 -0

+
+struct clk_init_item {
+	int index;
+	struct device_node *np;
+	ti_of_clk_init_cb_t init_cb;
+	struct list_head node;
+};
+
+static LIST_HEAD(retry_list);
+
+/**
+ * ti_clk_get_reg_addr - get register address for a clock register
+ * @node: device node for the clock
+ * @index: register index from the clock node
+ *
+ * Builds clock register address from device tree information. This
+ * is a struct of type clk_omap_reg.
+ */
+void __iomem *ti_clk_get_reg_addr(struct device_node *node, int index)
+{
+	struct clk_omap_reg *reg;
+	u32 val;
+	u32 tmp;
+
+	reg = (struct clk_omap_reg *)&tmp;
+	reg->index = ti_dt_clk_regmap_index;
+
+	if (of_property_read_u32_index(node, "reg", index, &val)) {
+		pr_err("%s must have reg[%d]!\n", node->name, index);
+		return NULL;
+	}
+
+	reg->offset = val;
+
+	return (void __iomem *)tmp;
+}
+
+/**
+ * ti_dt_clk_init_provider - init master clock provider
+ * @parent: master node
+ * @index: internal index for clk_reg_ops
+ *
+ * Initializes a master clock IP block and its child clock nodes.
+ * Regmap is provided for accessing the register space for the
+ * IP block and all the clocks under it.
+ */
+void ti_dt_clk_init_provider(struct device_node *parent, int index)
+{
+	const struct of_device_id *match;
+	struct device_node *np;
+	ti_of_clk_init_cb_t clk_init_cb;
+	struct clk_init_item *retry;
+	struct clk_init_item *tmp;
+	int ret;
+
+	ti_dt_clk_regmap_index = index;
+
+	for_each_child_of_node(parent, np) {
+		match = of_match_node(__clk_of_table, np);
+		if (!match)
+			continue;
+		clk_init_cb = (ti_of_clk_init_cb_t)match->data;
+		pr_debug("%s: initializing: %s\n", __func__, np->name);
+		ret = clk_init_cb(np);
+		if (ret == -EAGAIN) {
+			pr_debug("%s: adding to again list...\n", np->name);
+			retry = kzalloc(sizeof(*retry), GFP_KERNEL);
+			retry->np = np;
+			retry->init_cb = clk_init_cb;
+			list_add(&retry->node, &retry_list);
+		} else if (ret) {
+			pr_err("%s: clock init failed for %s (%d)!\n", __func__,
+			       np->name, ret);
+		}
+	}
+
+	list_for_each_entry_safe(retry, tmp, &retry_list, node) {

drivers/clk/ti/clk.c void __init ti_dt_clocks_register(struct ti_dt_clk oclks[]) · line 144

Quoted 5 lines

drivers/clk/ti/clk.c +5 -0

+		pr_debug("%s: retry-init: %s\n", __func__, retry->np->name);
+		ti_dt_clk_regmap_index = retry->index;
+		ret = retry->init_cb(retry->np);
+		if (ret == -EAGAIN) {
+			pr_debug("%s failed again?\n", retry->np->name);

This is presumably a serious error condition and should be a pr_warn() or pr_err(), right?

If retry_list won't be walked again, then it seems best to delete and free the list_entry no matter what the return value is from retry->init_cb(), since it's not like it will be retried. Otherwise the code will leak memory.

Quoted 37 lines

drivers/clk/ti/clk.c +9 -0

+		} else {
+			if (ret)
+				pr_err("%s: clock init failed for %s (%d)!\n",
+				       __func__, retry->np->name, ret);
+			list_del(&retry->node);
+			kfree(retry);
+		}
+	}
+}

include/linux/clk/ti.h +14 -1

index df94c24..d6bf530 100644
@@ -36,7 +36,21 @@ struct ti_dt_clk {
  		.node_name = name,	\
  	}
 
+/* Maximum number of clock regmaps */
+#define CLK_MAX_REGMAPS			4
 
+/**
+ * struct clk_omap_reg - OMAP register declaration
+ * @offset: offset from the master IP module base address
+ * @index: index of the master IP module
+ */
+struct clk_omap_reg {
+	u16 offset;
+	u16 index;
+};
+
+void __iomem *ti_clk_get_reg_addr(struct device_node *node, int index);
  void ti_dt_clocks_register(struct ti_dt_clk *oclks);
+void ti_dt_clk_init_provider(struct device_node *np, int index);
 
  #endif
--

Quoted 2 lines

1.7.9.5

- Paul

Quoted 31 lines · Paul Walmsley

Quoted 8 lines · Tero Kristo

ti_dt_clk_init_provider() can now be used to initialize the contents of a single clock IP block. This parses all the clocks under the IP block and calls the corresponding init function for them.

This patch also introduces a helper function for the TI clock drivers to get register info from DT and append the master IP info to this.

Signed-off-by Tero Kristo <[email protected]>

...

Quoted 5 lines

drivers/clk/ti/clk.c +0 -0

index ef1a7cd..63f85e9 100644

drivers/clk/ti/clk.c line 28

Quoted 10 lines

drivers/clk/ti/clk.c +3 -0

@@ -19,10 +19,15 @@
  #include <linux/clkdev.h>
  #include <linux/clk/ti.h>
  #include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/list.h>
 
  #undef pr_fmt
  #define pr_fmt(fmt) "%s: " fmt, __func__
 
+extern struct of_device_id __clk_of_table[];

This results in a checkpatch.pl warning:

Quoted 3 lines

WARNING: externs should be avoided in .c files
#33: FILE: drivers/clk/ti/clk.c:28:
+extern struct of_device_id __clk_of_table[];

This extern is only needed from this single file, and this code is duplicated from drivers/clk/clk.c.

The OMAP clock driver now supports DPLL clock type. This patch also adds support for DT DPLL nodes.

Signed-off-by Tero Kristo <[email protected]>

+/*
+ * struct clk_hw_omap.flags possibilities
+ *
+ * XXX document the rest of the clock flags here
+ *
+ * ENABLE_REG_32BIT: (OMAP1 only) clock control register must be accessed
+ * with 32bit ops, by default OMAP1 uses 16bit ops.
+ * CLOCK_IDLE_CONTROL: (OMAP1 only) clock has autoidle support.
+ * CLOCK_NO_IDLE_PARENT: (OMAP1 only) when clock is enabled, its parent
+ * clock is put to no-idle mode.
+ * ENABLE_ON_INIT: Clock is enabled on init.
+ * INVERT_ENABLE: By default, clock enable bit behavior is '1' enable, '0'
+ * disable. This inverts the behavior making '0' enable and '1' disable.
+ * CLOCK_CLKOUTX2: (OMAP4 only) DPLL CLKOUT and CLKOUTX2 GATE_CTRL

+ *     bits share the same register.  This flag allows the
+ *     omap4_dpllmx*() code to determine which GATE_CTRL bit field
+ *     should be used.  This is a temporary solution - a better approach
+ *     would be to associate clock type-specific data with the clock,
+ *     similar to the struct dpll_data approach.

+ * REGMAP_ADDRESSING: Use regmap addressing to access clock registers.
+ */

The OMAP clock driver now supports DPLL clock type. This patch also adds support for DT DPLL nodes.

Signed-off-by Tero Kristo <[email protected]>

drivers/clk/ti/dpll.c new file +0 -0

new file mode 100644
index 0000000..921a409
@@ -0,0 +1,597 @@

+/**
+ * of_ti_dpll_setup - Setup function for OMAP DPLL clocks
+ *
+ * @node: device node containing the DPLL info
+ * @ops: ops for the DPLL
+ * @ddt: DPLL data template to use
+ * @init_flags: flags for controlling init types
+ */

Some OMAP clocks require knowledge about their parent clockdomain for book keeping purposes. This patch creates a new DT binding for TI clockdomains, which act as a collection of device clocks.

Signed-off-by Tero Kristo <[email protected]>

---
 .../devicetree/bindings/clock/ti/clockdomain.txt   |   21 ++++++
 arch/arm/mach-omap2/clock.h                        |    1 -
 drivers/clk/ti/Makefile                            |    3 +-
 drivers/clk/ti/clockdomain.c                       |   70 ++++++++++++++++++++
 include/linux/clk/ti.h                             |    3 +
 5 files changed, 96 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/ti/clockdomain.txt
 create mode 100644 drivers/clk/ti/clockdomain.c

Documentation/devicetree/bindings/clock/ti/clockdomain.txt new file +0 -0

new file mode 100644
index 0000000..45e6f7c

Documentation/devicetree/bindings/clock/ti/clockdomain.txt new file +16 -0

+TI SoC belongs to one clockdomain, but software only needs this
+information for specific clocks which require their parent
+clockdomain to be controlled when the clock is enabled/disabled.
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be "ti,clockdomain"
+- #clock-cells : from common clock binding; shall be set to 0.
+- clocks : link phandles of clocks within this domain
+
+Examples:
+       dss_clkdm: dss_clkdm {
+               compatible = "ti,clockdomain";
+               clocks = <&dss1_alwon_fck_3430es2>, <&dss_ick_3430es2>;
+       };

arch/arm/mach-omap2/clock.h +0 -1

index bc0f9fc..6bd72b5 100644
@@ -38,7 +38,6 @@ struct omap_clk {
        }
 
 struct clockdomain;
-#define to_clk_hw_omap(_hw) container_of(_hw, struct clk_hw_omap, hw)
 
 #define DEFINE_STRUCT_CLK(_name, _parent_array_name, _clkops_name)     \
        static struct clk _name = {                             \

drivers/clk/ti/Makefile +2 -1

index 7cba389..67056fb 100644
@@ -1,4 +1,5 @@
 ifneq ($(CONFIG_OF),)
 obj-y                                  += clk.o dpll.o autoidle.o divider.o \
-                                          fixed-factor.o gate.o composite.o
+                                          fixed-factor.o gate.o clockdomain.o \
+                                          composite.o
 endif

drivers/clk/ti/clockdomain.c new file +70 -0

new file mode 100644
index 0000000..f1e0038
@@ -0,0 +1,70 @@
+/*
+ * OMAP clockdomain support
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * Tero Kristo <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/clk/ti.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+static void __init of_ti_clockdomain_setup(struct device_node *node)
+{
+       struct clk *clk;
+       struct clk_hw *clk_hw;
+       const char *clkdm_name = node->name;
+       int i;
+       int num_clks;
+
+       num_clks = of_count_phandle_with_args(node, "clocks", "#clock-cells");
+
+       for (i = 0; i < num_clks; i++) {
+               clk = of_clk_get(node, i);
+               if (__clk_get_flags(clk) & CLK_IS_BASIC) {
+                       pr_warn("can't setup clkdm for basic clk %s\n",
+                               __clk_get_name(clk));
+                       continue;
+               }
+               clk_hw = __clk_get_hw(clk);
+               to_clk_hw_omap(clk_hw)->clkdm_name = clkdm_name;
+               omap2_init_clk_clkdm(clk_hw);
+       }
+}
+
+static struct of_device_id ti_clkdm_match_table[] __initdata = {
+       { .compatible = "ti,clockdomain" },
+       { }
+};
+
+/**
+ * ti_dt_clockdomains_setup - setup device tree clockdomains
+ *
+ * Initializes clockdomain nodes for a SoC. This parses through all the
+ * nodes with compatible = "ti,clockdomain", and add the clockdomain
+ * info for all the clocks listed under these. This function shall be
+ * called after rest of the DT clock init has completed and all
+ * clock nodes have been registered.
+ */
+void __init ti_dt_clockdomains_setup(void)
+{
+       struct device_node *np;
+       for_each_matching_node(np, ti_clkdm_match_table) {
+               of_ti_clockdomain_setup(np);
+       }
+}

include/linux/clk/ti.h +3 -0

index 872ff2a..231b071 100644
@@ -204,6 +204,8 @@ struct clk_omap_reg {
        u16 index;
 };
 
+#define to_clk_hw_omap(_hw) container_of(_hw, struct clk_hw_omap, hw)
+
 void omap2_init_clk_hw_omap_clocks(struct clk *clk);
 int omap3_noncore_dpll_enable(struct clk_hw *hw);
 void omap3_noncore_dpll_disable(struct clk_hw *hw);

@@ -232,6 +234,7 @@ int omap2_dflt_clk_is_enabled(struct clk_hw *hw);
 void __iomem *ti_clk_get_reg_addr(struct device_node *node, int index);
 void ti_dt_clocks_register(struct ti_dt_clk *oclks);
 void ti_dt_clk_init_provider(struct device_node *np, int index);
+void ti_dt_clockdomains_setup(void);
 int of_ti_clk_autoidle_setup(struct device_node *node);
 int ti_clk_add_component(struct device_node *node, struct clk_hw *hw, int type);
 

Signature · 1 line

1.7.9.5

Quoting Tero Kristo (2013-11-26 00:05:51)

Quoted 21 lines

Some OMAP clocks require knowledge about their parent clockdomain for book keeping purposes. This patch creates a new DT binding for TI clockdomains, which act as a collection of device clocks.

Signed-off-by Tero Kristo <[email protected]>

---
  .../devicetree/bindings/clock/ti/clockdomain.txt   |   21 ++++++
  arch/arm/mach-omap2/clock.h                        |    1 -
  drivers/clk/ti/Makefile                            |    3 +-
  drivers/clk/ti/clockdomain.c                       |   70 ++++++++++++++++++++
  include/linux/clk/ti.h                             |    3 +
  5 files changed, 96 insertions(+), 2 deletions(-)

create mode 100644 Documentation/devicetree/bindings/clock/ti/clockdomain.txt create mode 100644 drivers/clk/ti/clockdomain.c

Documentation/devicetree/bindings/clock/ti/clockdomain.txt new file +0 -0

new file mode 100644
index 0000000..45e6f7c

Documentation/devicetree/bindings/clock/ti/clockdomain.txt line 5

Quoted 5 lines

Documentation/devicetree/bindings/clock/ti/clockdomain.txt new file +5 -0

@@ -0,0 +1,21 @@
+Binding for Texas Instruments clockdomain.
+
+Binding status: Unstable - ABI compatibility may be broken in the future
+
+This binding uses the common clock binding[1]. Every clock on

The patch looks fine to me but I think that the binding description should capture the fact that you are re-using the common clock binding but that this binding definition does not define any new clocks or clock controllers in the way that a typical clock binding would.

This code uses the 'clocks' property the same way that any other consumer binding definition would, such as an MMC controller or UART. Those bindings do not say that they are based on the common clock binding AFAIK.

Regards,
Mike

Quoted 139 lines

Documentation/devicetree/bindings/clock/ti/clockdomain.txt new file +16 -0

+TI SoC belongs to one clockdomain, but software only needs this
+information for specific clocks which require their parent
+clockdomain to be controlled when the clock is enabled/disabled.
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be "ti,clockdomain"
+- #clock-cells : from common clock binding; shall be set to 0.
+- clocks : link phandles of clocks within this domain
+
+Examples:
+       dss_clkdm: dss_clkdm {
+               compatible = "ti,clockdomain";
+               clocks = <&dss1_alwon_fck_3430es2>, <&dss_ick_3430es2>;
+       };

arch/arm/mach-omap2/clock.h +0 -1

index bc0f9fc..6bd72b5 100644
@@ -38,7 +38,6 @@ struct omap_clk {
         }
 
  struct clockdomain;
-#define to_clk_hw_omap(_hw) container_of(_hw, struct clk_hw_omap, hw)
 
  #define DEFINE_STRUCT_CLK(_name, _parent_array_name, _clkops_name)     \
         static struct clk _name = {                             \

drivers/clk/ti/Makefile +2 -1

index 7cba389..67056fb 100644
@@ -1,4 +1,5 @@
  ifneq ($(CONFIG_OF),)
  obj-y                                  += clk.o dpll.o autoidle.o divider.o \
-                                          fixed-factor.o gate.o composite.o
+                                          fixed-factor.o gate.o clockdomain.o \
+                                          composite.o
  endif

drivers/clk/ti/clockdomain.c new file +70 -0

new file mode 100644
index 0000000..f1e0038
@@ -0,0 +1,70 @@
+/*
+ * OMAP clockdomain support
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * Tero Kristo <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/clk/ti.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+static void __init of_ti_clockdomain_setup(struct device_node *node)
+{
+       struct clk *clk;
+       struct clk_hw *clk_hw;
+       const char *clkdm_name = node->name;
+       int i;
+       int num_clks;
+
+       num_clks = of_count_phandle_with_args(node, "clocks", "#clock-cells");
+
+       for (i = 0; i < num_clks; i++) {
+               clk = of_clk_get(node, i);
+               if (__clk_get_flags(clk) & CLK_IS_BASIC) {
+                       pr_warn("can't setup clkdm for basic clk %s\n",
+                               __clk_get_name(clk));
+                       continue;
+               }
+               clk_hw = __clk_get_hw(clk);
+               to_clk_hw_omap(clk_hw)->clkdm_name = clkdm_name;
+               omap2_init_clk_clkdm(clk_hw);
+       }
+}
+
+static struct of_device_id ti_clkdm_match_table[] __initdata = {
+       { .compatible = "ti,clockdomain" },
+       { }
+};
+
+/**
+ * ti_dt_clockdomains_setup - setup device tree clockdomains
+ *
+ * Initializes clockdomain nodes for a SoC. This parses through all the
+ * nodes with compatible = "ti,clockdomain", and add the clockdomain
+ * info for all the clocks listed under these. This function shall be
+ * called after rest of the DT clock init has completed and all
+ * clock nodes have been registered.
+ */
+void __init ti_dt_clockdomains_setup(void)
+{
+       struct device_node *np;
+       for_each_matching_node(np, ti_clkdm_match_table) {
+               of_ti_clockdomain_setup(np);
+       }
+}

include/linux/clk/ti.h +3 -1

index 872ff2a..231b071 100644
@@ -204,6 +204,8 @@ struct clk_omap_reg {
         u16 index;
  };
 
+#define to_clk_hw_omap(_hw) container_of(_hw, struct clk_hw_omap, hw)
+
  void omap2_init_clk_hw_omap_clocks(struct clk *clk);
  int omap3_noncore_dpll_enable(struct clk_hw *hw);
  void omap3_noncore_dpll_disable(struct clk_hw *hw);

@@ -232,6 +234,7 @@ int omap2_dflt_clk_is_enabled(struct clk_hw *hw);
  void __iomem *ti_clk_get_reg_addr(struct device_node *node, int index);
  void ti_dt_clocks_register(struct ti_dt_clk *oclks);
  void ti_dt_clk_init_provider(struct device_node *np, int index);
+void ti_dt_clockdomains_setup(void);
  int of_ti_clk_autoidle_setup(struct device_node *node);
  int ti_clk_add_component(struct device_node *node, struct clk_hw *hw, int type);
 
--

1.7.9.5

Quoted 33 lines

Quoting Tero Kristo (2013-11-26 00:05:51)

Quoted 21 lines

Some OMAP clocks require knowledge about their parent clockdomain for book keeping purposes. This patch creates a new DT binding for TI clockdomains, which act as a collection of device clocks.

Signed-off-by Tero Kristo <[email protected]>

---
  .../devicetree/bindings/clock/ti/clockdomain.txt   |   21 ++++++
  arch/arm/mach-omap2/clock.h                        |    1 -
  drivers/clk/ti/Makefile                            |    3 +-
  drivers/clk/ti/clockdomain.c                       |   70 ++++++++++++++++++++
  include/linux/clk/ti.h                             |    3 +
  5 files changed, 96 insertions(+), 2 deletions(-)

create mode 100644 Documentation/devicetree/bindings/clock/ti/clockdomain.txt create mode 100644 drivers/clk/ti/clockdomain.c

Documentation/devicetree/bindings/clock/ti/clockdomain.txt new file +0 -0

new file mode 100644
index 0000000..45e6f7c

Documentation/devicetree/bindings/clock/ti/clockdomain.txt line 5

Quoted 5 lines

Documentation/devicetree/bindings/clock/ti/clockdomain.txt new file +5 -0

@@ -0,0 +1,21 @@
+Binding for Texas Instruments clockdomain.
+
+Binding status: Unstable - ABI compatibility may be broken in the future
+
+This binding uses the common clock binding[1]. Every clock on

The patch looks fine to me but I think that the binding description should capture the fact that you are re-using the common clock binding but that this binding definition does not define any new clocks or clock controllers in the way that a typical clock binding would.

Quoted 5 lines

This code uses the 'clocks' property the same way that any other consumer binding definition would, such as an MMC controller or UART. Those bindings do not say that they are based on the common clock binding AFAIK.

Ok, will modify the doc accordingly.

-Tero

Quoted 142 lines

Regards,
Mike

Quoted 139 lines

Documentation/devicetree/bindings/clock/ti/clockdomain.txt new file +16 -0

+TI SoC belongs to one clockdomain, but software only needs this
+information for specific clocks which require their parent
+clockdomain to be controlled when the clock is enabled/disabled.
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be "ti,clockdomain"
+- #clock-cells : from common clock binding; shall be set to 0.
+- clocks : link phandles of clocks within this domain
+
+Examples:
+       dss_clkdm: dss_clkdm {
+               compatible = "ti,clockdomain";
+               clocks = <&dss1_alwon_fck_3430es2>, <&dss_ick_3430es2>;
+       };

arch/arm/mach-omap2/clock.h +0 -1

index bc0f9fc..6bd72b5 100644
@@ -38,7 +38,6 @@ struct omap_clk {
         }
 
  struct clockdomain;
-#define to_clk_hw_omap(_hw) container_of(_hw, struct clk_hw_omap, hw)
 
  #define DEFINE_STRUCT_CLK(_name, _parent_array_name, _clkops_name)     \
         static struct clk _name = {                             \

drivers/clk/ti/Makefile +2 -1

index 7cba389..67056fb 100644
@@ -1,4 +1,5 @@
  ifneq ($(CONFIG_OF),)
  obj-y                                  += clk.o dpll.o autoidle.o divider.o \
-                                          fixed-factor.o gate.o composite.o
+                                          fixed-factor.o gate.o clockdomain.o \
+                                          composite.o
  endif

drivers/clk/ti/clockdomain.c new file +70 -0

new file mode 100644
index 0000000..f1e0038
@@ -0,0 +1,70 @@
+/*
+ * OMAP clockdomain support
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * Tero Kristo <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/clk/ti.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
+static void __init of_ti_clockdomain_setup(struct device_node *node)
+{
+       struct clk *clk;
+       struct clk_hw *clk_hw;
+       const char *clkdm_name = node->name;
+       int i;
+       int num_clks;
+
+       num_clks = of_count_phandle_with_args(node, "clocks", "#clock-cells");
+
+       for (i = 0; i < num_clks; i++) {
+               clk = of_clk_get(node, i);
+               if (__clk_get_flags(clk) & CLK_IS_BASIC) {
+                       pr_warn("can't setup clkdm for basic clk %s\n",
+                               __clk_get_name(clk));
+                       continue;
+               }
+               clk_hw = __clk_get_hw(clk);
+               to_clk_hw_omap(clk_hw)->clkdm_name = clkdm_name;
+               omap2_init_clk_clkdm(clk_hw);
+       }
+}
+
+static struct of_device_id ti_clkdm_match_table[] __initdata = {
+       { .compatible = "ti,clockdomain" },
+       { }
+};
+
+/**
+ * ti_dt_clockdomains_setup - setup device tree clockdomains
+ *
+ * Initializes clockdomain nodes for a SoC. This parses through all the
+ * nodes with compatible = "ti,clockdomain", and add the clockdomain
+ * info for all the clocks listed under these. This function shall be
+ * called after rest of the DT clock init has completed and all
+ * clock nodes have been registered.
+ */
+void __init ti_dt_clockdomains_setup(void)
+{
+       struct device_node *np;
+       for_each_matching_node(np, ti_clkdm_match_table) {
+               of_ti_clockdomain_setup(np);
+       }
+}

include/linux/clk/ti.h +3 -1

index 872ff2a..231b071 100644
@@ -204,6 +204,8 @@ struct clk_omap_reg {
         u16 index;
  };
 
+#define to_clk_hw_omap(_hw) container_of(_hw, struct clk_hw_omap, hw)
+
  void omap2_init_clk_hw_omap_clocks(struct clk *clk);
  int omap3_noncore_dpll_enable(struct clk_hw *hw);
  void omap3_noncore_dpll_disable(struct clk_hw *hw);

@@ -232,6 +234,7 @@ int omap2_dflt_clk_is_enabled(struct clk_hw *hw);
  void __iomem *ti_clk_get_reg_addr(struct device_node *node, int index);
  void ti_dt_clocks_register(struct ti_dt_clk *oclks);
  void ti_dt_clk_init_provider(struct device_node *np, int index);
+void ti_dt_clockdomains_setup(void);
  int of_ti_clk_autoidle_setup(struct device_node *node);
  int ti_clk_add_component(struct device_node *node, struct clk_hw *hw, int type);
 
--

1.7.9.5

clk-44xx.c now contains the clock init functionality for omap4, including DT clock registration and adding of static clkdev entries.

From: J Keerthy <[email protected]>

The patch adds support for DRA7 PCIe APLL. The APLL sources the optional functional clocks for PCIe module.

APLL stands for Analog PLL. This is different when comapred with DPLL meaning Digital PLL, the phase detection is done using an analog circuit.

Signed-off-by J Keerthy <[email protected]>

Signed-off-by Tero Kristo <[email protected]>

---
 .../devicetree/bindings/clock/ti/apll.txt          |   31 +++
 drivers/clk/ti/Makefile                            |    2 +-
 drivers/clk/ti/apll.c                              |  239 ++++++++++++++++++++
 3 files changed, 271 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/clock/ti/apll.txt
 create mode 100644 drivers/clk/ti/apll.c

Documentation/devicetree/bindings/clock/ti/apll.txt new file +0 -0

new file mode 100644
index 0000000..7faf5a6

Documentation/devicetree/bindings/clock/ti/apll.txt new file +59 -0

+
+static int __init of_dra7_apll_setup(struct device_node *node)
+{
+	const struct clk_ops *ops;
+	struct clk *clk;
+	const char *clk_name = node->name;
+	int num_parents;
+	const char **parent_names = NULL;
+	u8 apll_flags = 0;
+	struct dpll_data *ad;
+	u32 idlest_mask = 0x1;
+	u32 autoidle_mask = 0x3;
+	int i;
+	int ret;
+
+	ops = &apll_ck_ops;
+	ad = kzalloc(sizeof(*ad), GFP_KERNEL);
+	if (!ad)
+		return -ENOMEM;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+
+	num_parents = of_clk_get_parent_count(node);
+	if (num_parents < 1) {
+		pr_err("dra7 apll %s must have parent(s)\n", node->name);
+		ret = -EINVAL;
+		goto cleanup;
+	}
+
+	parent_names = kzalloc(sizeof(char *) * num_parents, GFP_KERNEL);
+
+	for (i = 0; i < num_parents; i++)
+		parent_names[i] = of_clk_get_parent_name(node, i);
+
+	ad->clk_ref = of_clk_get(node, 0);
+	ad->clk_bypass = of_clk_get(node, 1);
+
+	if (IS_ERR(ad->clk_ref)) {
+		pr_debug("ti,clk-ref for %s not found\n", clk_name);
+		ret = -EAGAIN;
+		goto cleanup;
+	}
+
+	if (IS_ERR(ad->clk_bypass)) {
+		pr_debug("ti,clk-bypass for %s not found\n", clk_name);
+		ret = -EAGAIN;
+		goto cleanup;
+	}
+
+	ad->control_reg = ti_clk_get_reg_addr(node, 0);
+	ad->idlest_reg = ti_clk_get_reg_addr(node, 1);
+
+	if (!ad->control_reg || !ad->idlest_reg) {
+		ret = -EINVAL;
+		goto cleanup;
+	}
+
+	ad->idlest_mask = idlest_mask;
+	ad->enable_mask = autoidle_mask;

Hi,

Quoted 22 lines · Tero Kristo

From: J Keerthy <[email protected]>

The patch adds support for DRA7 PCIe APLL. The APLL sources the optional functional clocks for PCIe module.

APLL stands for Analog PLL. This is different when comapred with DPLL meaning Digital PLL, the phase detection is done using an analog circuit.

Signed-off-by J Keerthy <[email protected]>

Signed-off-by Tero Kristo <[email protected]>

---
  .../devicetree/bindings/clock/ti/apll.txt          |   31 +++
  drivers/clk/ti/Makefile                            |    2 +-
  drivers/clk/ti/apll.c                              |  239 ++++++++++++++++++++
  3 files changed, 271 insertions(+), 1 deletion(-)

create mode 100644 Documentation/devicetree/bindings/clock/ti/apll.txt create mode 100644 drivers/clk/ti/apll.c

Documentation/devicetree/bindings/clock/ti/apll.txt new file +0 -0

new file mode 100644
index 0000000..7faf5a6

...

Quoted 59 lines

Documentation/devicetree/bindings/clock/ti/apll.txt new file +59 -0

+
+static int __init of_dra7_apll_setup(struct device_node *node)
+{
+	const struct clk_ops *ops;
+	struct clk *clk;
+	const char *clk_name = node->name;
+	int num_parents;
+	const char **parent_names = NULL;
+	u8 apll_flags = 0;
+	struct dpll_data *ad;
+	u32 idlest_mask = 0x1;
+	u32 autoidle_mask = 0x3;
+	int i;
+	int ret;
+
+	ops = &apll_ck_ops;
+	ad = kzalloc(sizeof(*ad), GFP_KERNEL);
+	if (!ad)
+		return -ENOMEM;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+
+	num_parents = of_clk_get_parent_count(node);
+	if (num_parents < 1) {
+		pr_err("dra7 apll %s must have parent(s)\n", node->name);
+		ret = -EINVAL;
+		goto cleanup;
+	}
+
+	parent_names = kzalloc(sizeof(char *) * num_parents, GFP_KERNEL);
+
+	for (i = 0; i < num_parents; i++)
+		parent_names[i] = of_clk_get_parent_name(node, i);
+
+	ad->clk_ref = of_clk_get(node, 0);
+	ad->clk_bypass = of_clk_get(node, 1);
+
+	if (IS_ERR(ad->clk_ref)) {
+		pr_debug("ti,clk-ref for %s not found\n", clk_name);
+		ret = -EAGAIN;
+		goto cleanup;
+	}
+
+	if (IS_ERR(ad->clk_bypass)) {
+		pr_debug("ti,clk-bypass for %s not found\n", clk_name);
+		ret = -EAGAIN;
+		goto cleanup;
+	}
+
+	ad->control_reg = ti_clk_get_reg_addr(node, 0);
+	ad->idlest_reg = ti_clk_get_reg_addr(node, 1);
+
+	if (!ad->control_reg || !ad->idlest_reg) {
+		ret = -EINVAL;
+		goto cleanup;
+	}
+
+	ad->idlest_mask = idlest_mask;
+	ad->enable_mask = autoidle_mask;

Documentation/devicetree/bindings/clock/ti/apll.txt

Quoted 10 lines

Documentation/devicetree/bindings/clock/ti/apll.txt new file +10 -0

+
+	clk = omap_clk_register_apll(NULL, clk_name, parent_names,
+				num_parents, apll_flags, ad,
+				NULL, ops);
+
+	if (!IS_ERR(clk)) {
+		of_clk_add_provider(node, of_clk_src_simple_get, clk);
+		return 0;
+	}
+

Should we not also here do a cleanup for allocated memory?

Documentation/devicetree/bindings/clock/ti/apll.txt

Quoted 1 line

Documentation/devicetree/bindings/clock/ti/apll.txt new file +1 -0

+	return PTR_ERR(clk);

Documentation/devicetree/bindings/clock/ti/apll.txt

Quoted 7 lines

Documentation/devicetree/bindings/clock/ti/apll.txt new file +7 -0

+
+cleanup:
+	kfree(parent_names);
+	kfree(ad);
+	return ret;
+}
+CLK_OF_DECLARE(dra7_apll_clock, "ti,dra7-apll-clock", of_dra7_apll_setup);

- Alex

Quoted 98 lines · Alexander Aring

Hi,

Quoted 22 lines · Tero Kristo

From: J Keerthy <[email protected]>

The patch adds support for DRA7 PCIe APLL. The APLL sources the optional functional clocks for PCIe module.

APLL stands for Analog PLL. This is different when comapred with DPLL meaning Digital PLL, the phase detection is done using an analog circuit.

Signed-off-by J Keerthy <[email protected]>

Signed-off-by Tero Kristo <[email protected]>

---
 .../devicetree/bindings/clock/ti/apll.txt          |   31 +++
 drivers/clk/ti/Makefile                            |    2 +-
 drivers/clk/ti/apll.c                              |  239 ++++++++++++++++++++
 3 files changed, 271 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/clock/ti/apll.txt
 create mode 100644 drivers/clk/ti/apll.c

Documentation/devicetree/bindings/clock/ti/apll.txt new file +0 -0

new file mode 100644
index 0000000..7faf5a6

...

Quoted 59 lines

Documentation/devicetree/bindings/clock/ti/apll.txt new file +59 -0

+
+static int __init of_dra7_apll_setup(struct device_node *node)
+{
+	const struct clk_ops *ops;
+	struct clk *clk;
+	const char *clk_name = node->name;
+	int num_parents;
+	const char **parent_names = NULL;
+	u8 apll_flags = 0;
+	struct dpll_data *ad;
+	u32 idlest_mask = 0x1;
+	u32 autoidle_mask = 0x3;
+	int i;
+	int ret;
+
+	ops = &apll_ck_ops;
+	ad = kzalloc(sizeof(*ad), GFP_KERNEL);
+	if (!ad)
+		return -ENOMEM;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+
+	num_parents = of_clk_get_parent_count(node);
+	if (num_parents < 1) {
+		pr_err("dra7 apll %s must have parent(s)\n", node->name);
+		ret = -EINVAL;
+		goto cleanup;
+	}
+
+	parent_names = kzalloc(sizeof(char *) * num_parents, GFP_KERNEL);
+
+	for (i = 0; i < num_parents; i++)
+		parent_names[i] = of_clk_get_parent_name(node, i);
+
+	ad->clk_ref = of_clk_get(node, 0);
+	ad->clk_bypass = of_clk_get(node, 1);
+
+	if (IS_ERR(ad->clk_ref)) {
+		pr_debug("ti,clk-ref for %s not found\n", clk_name);
+		ret = -EAGAIN;
+		goto cleanup;
+	}
+
+	if (IS_ERR(ad->clk_bypass)) {
+		pr_debug("ti,clk-bypass for %s not found\n", clk_name);
+		ret = -EAGAIN;
+		goto cleanup;
+	}
+
+	ad->control_reg = ti_clk_get_reg_addr(node, 0);
+	ad->idlest_reg = ti_clk_get_reg_addr(node, 1);
+
+	if (!ad->control_reg || !ad->idlest_reg) {
+		ret = -EINVAL;
+		goto cleanup;
+	}
+
+	ad->idlest_mask = idlest_mask;
+	ad->enable_mask = autoidle_mask;

Documentation/devicetree/bindings/clock/ti/apll.txt

Quoted 10 lines

Documentation/devicetree/bindings/clock/ti/apll.txt new file +10 -0

+
+	clk = omap_clk_register_apll(NULL, clk_name, parent_names,
+				num_parents, apll_flags, ad,
+				NULL, ops);
+
+	if (!IS_ERR(clk)) {
+		of_clk_add_provider(node, of_clk_src_simple_get, clk);
+		return 0;
+	}
+

Should we not also here do a cleanup for allocated memory?

Quoted 1 line

Documentation/devicetree/bindings/clock/ti/apll.txt

Quoted 1 line

Documentation/devicetree/bindings/clock/ti/apll.txt new file +1 -0

+	return PTR_ERR(clk);

Yes, this should be changed to be ret = PTR_ERR(clk);

This patch creates a unique node for each clock in the OMAP4 power, reset and clock manager (PRCM). OMAP443x and OMAP446x have slightly different clock tree which is taken into account in the data.

Signed-off-by Tero Kristo <[email protected]>

arch/arm/boot/dts/omap443x-clocks.dtsi new file +7 -0

new file mode 100644
index 0000000..643755b
@@ -0,0 +1,18 @@
+/*
+ * Device Tree Source for OMAP4 clock data
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as

arch/arm/boot/dts/omap44xx-clocks.dtsi new file +0 -0

new file mode 100644
index 0000000..2b59d54

Quoted 5 lines · Tero Kristo

This patch creates a unique node for each clock in the OMAP4 power, reset and clock manager (PRCM). OMAP443x and OMAP446x have slightly different clock tree which is taken into account in the data.

Signed-off-by Tero Kristo <[email protected]>

...

arch/arm/boot/dts/omap4.dtsi line 111

Quoted 10 lines

arch/arm/boot/dts/omap4.dtsi +2 -0

index a1e0585..c2e3993 100644
@@ -107,6 +107,34 @@
  		interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>,
  			     <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
 
+		cm1: cm1 at 4a004000 {
+			compatible = "ti,clock-master";

These devices are low-level IP blocks, and should have accurate compatible strings like any other low-level IP block. At some point in the future, these IP blocks will have device driver code that matches up with these DT nodes, and is probed via these compatible strings. These should be corrected now, so unnecessary DT data synchronization problems don't appear with later kernels.

So this should be something like:

compatible = "ti,omap4-cm1";

arch/arm/boot/dts/omap4.dtsi line 118

Quoted 7 lines

arch/arm/boot/dts/omap4.dtsi +7 -0

+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x4a004000 0x2000>;
+		};
+
+		prm: prm at 4a306000 {
+			compatible = "ti,clock-master";

Similarly this should be

compatible = "ti,omap4-prm";

arch/arm/boot/dts/omap4.dtsi line 125

Quoted 8 lines

arch/arm/boot/dts/omap4.dtsi +7 -0

+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x4a306000 0x3000>;
+		};
+
+		cm2: cm2 at 4a008000 {
+			compatible = "ti,clock-master";

compatible = "ti,omap4-cm2";

arch/arm/boot/dts/omap4.dtsi line 132

Quoted 7 lines

arch/arm/boot/dts/omap4.dtsi +7 -0

+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x4a008000 0x3000>;
+		};
+
+		scrm: scrm at 4a30a000 {
+			compatible = "ti,clock-master";

compatible = "ti,omap4-scrm";

...

Quoted 18 lines

arch/arm/boot/dts/omap443x-clocks.dtsi new file +12 -0

new file mode 100644
index 0000000..643755b
@@ -0,0 +1,18 @@
+/*
+ * Device Tree Source for OMAP4 clock data
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+&prm {
+	bandgap_fclk: bandgap_fclk {
+		#clock-cells = <0>;

arch/arm/boot/dts/omap443x-clocks.dtsi line 17

Quoted 5 lines

arch/arm/boot/dts/omap443x-clocks.dtsi new file +5 -0

+		compatible = "ti,gate-clock";
+		clocks = <&sys_32k_ck>;
+		ti,bit-shift = <8>;
+		reg = <0x1888>;
+	};

So we've already discussed that clocks should be moved underneath separate "clocks {" node in the IP block data. And similarly...