Add support for MSM's mmio clocks

Export this symbol so that modules can register fixed rate clocks.

Signed-off-by Stephen Boyd <[email protected]>

---
 drivers/clk/clk-fixed-rate.c | 1 +
 1 file changed, 1 insertion(+)

drivers/clk/clk-fixed-rate.c +1 -0

index dc58fbd..1ed591a 100644
@@ -80,6 +80,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
 
        return clk;
 }
+EXPORT_SYMBOL_GPL(clk_register_fixed_rate);
 
 #ifdef CONFIG_OF
 /**

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Consolidate DT parsing for the common bits of a clock binding in one place to simplify clock drivers. This also has the added benefit of standardizing how the clock names used by the common clock framework are generated from the DT bindings. We always use the first clock-output-names string if it exists, otherwise we fall back to the node name.

To be slightly more efficient and make the caller's life easier, we introduce a shallow copy flag so that the clock core knows to just copy the pointers to the strings and not the string contents. Otherwise the callers of this function would have to free the strings allocated here which could be cumbersome.

Cc Rob Herring <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---
 drivers/clk/clk.c            | 59
+++++++++++++++++++++++++++++++++++++++++++-
include/linux/clk-provider.h |  7 ++++++
 2 files changed, 65 insertions(+), 1 deletion(-)

drivers/clk/clk.c +7 -1

index 1ed9bdd..ea8e951b 100644
@@ -1809,6 +1809,10 @@ static int _clk_register(struct device *dev,
 struct clk_hw *hw, struct clk *clk) {
 	int i, ret;
 
+	hw->clk = clk;
+	if (hw->init->flags & CLK_SHALLOW_COPY)
+		return PTR_RET(__clk_register(dev, hw));
+
 	clk->name = kstrdup(hw->init->name, GFP_KERNEL);
 	if (!clk->name) {
 		pr_err("%s: could not allocate clk->name\n", __func__);

@@ -1819,7 +1823,6 @@ static int _clk_register(struct device *dev,
 struct clk_hw *hw, struct clk *clk) clk->hw = hw;
 	clk->flags = hw->init->flags;
 	clk->num_parents = hw->init->num_parents;
-	hw->clk = clk;
 
 	/* allocate local copy in case parent_names is __initdata */
 	clk->parent_names = kzalloc((sizeof(char*) * clk->num_parents),

@@ -2232,4 +2235,58 @@ void __init of_clk_init(const struct of_device_id
 *matches) clk_init_cb(np);
 	}
 }
+
+/**
+ * of_init_clk_data() - Initialize a clk_init_data struct from a DT

node + * @np: node to initialize struct from
+ * @init: struct to initialize
+ *
+ * Populates the clk_init_data struct by parsing the device node for
+ * properties matching the common clock binding. Returns 0 on success
+ * and a negative error code on failure.
+ */
+int of_init_clk_data(struct device_node *np, struct clk_init_data *init) +{

drivers/clk/clk.c +2 -0

+	struct of_phandle_args s;
+	const char **names = NULL, **p;

Quoted 62 lines · On Wednesday 24 of July 2013 17:43:30 Stephen Boyd wrote:

Consolidate DT parsing for the common bits of a clock binding in one place to simplify clock drivers. This also has the added benefit of standardizing how the clock names used by the common clock framework are generated from the DT bindings. We always use the first clock-output-names string if it exists, otherwise we fall back to the node name.

To be slightly more efficient and make the caller's life easier, we introduce a shallow copy flag so that the clock core knows to just copy the pointers to the strings and not the string contents. Otherwise the callers of this function would have to free the strings allocated here which could be cumbersome.

Cc Rob Herring <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---
 drivers/clk/clk.c            | 59
+++++++++++++++++++++++++++++++++++++++++++-
include/linux/clk-provider.h |  7 ++++++
 2 files changed, 65 insertions(+), 1 deletion(-)

drivers/clk/clk.c +7 -1

index 1ed9bdd..ea8e951b 100644
@@ -1809,6 +1809,10 @@ static int _clk_register(struct device *dev,
 struct clk_hw *hw, struct clk *clk) {
 	int i, ret;
 
+	hw->clk = clk;
+	if (hw->init->flags & CLK_SHALLOW_COPY)
+		return PTR_RET(__clk_register(dev, hw));
+
 	clk->name = kstrdup(hw->init->name, GFP_KERNEL);
 	if (!clk->name) {
 		pr_err("%s: could not allocate clk->name\n", __func__);

@@ -1819,7 +1823,6 @@ static int _clk_register(struct device *dev,
 struct clk_hw *hw, struct clk *clk) clk->hw = hw;
 	clk->flags = hw->init->flags;
 	clk->num_parents = hw->init->num_parents;
-	hw->clk = clk;
 
 	/* allocate local copy in case parent_names is __initdata */
 	clk->parent_names = kzalloc((sizeof(char*) * clk->num_parents),

@@ -2232,4 +2235,58 @@ void __init of_clk_init(const struct of_device_id
 *matches) clk_init_cb(np);
 	}
 }
+
+/**
+ * of_init_clk_data() - Initialize a clk_init_data struct from a DT

node + * @np: node to initialize struct from
+ * @init: struct to initialize
+ *
+ * Populates the clk_init_data struct by parsing the device node for
+ * properties matching the common clock binding. Returns 0 on success
+ * and a negative error code on failure.
+ */
+int of_init_clk_data(struct device_node *np, struct clk_init_data *init) +{

drivers/clk/clk.c +2 -0

+	struct of_phandle_args s;
+	const char **names = NULL, **p;

drivers/clk/clk.c void __init of_clk_init(const struct of_device_id *matches)

Quoted 10 lines

drivers/clk/clk.c +10 -0

+	const char *name;
+	int i;
+
+	if (of_property_read_string(np, "clock-output-names", &name) < 0)
+		name = np->name;
+	init->name = kstrdup(name, GFP_KERNEL);
+	if (!init->name)
+		return -ENOMEM;
+
+	for (i = 0; of_parse_phandle_with_args(np, "clocks", "#clock-

cells",

drivers/clk/clk.c void __init of_clk_init(const struct of_device_id *matches) · line 2265

Quoted 4 lines

drivers/clk/clk.c +4 -0

+				i, &s) == 0; i++) {
+		p = krealloc(names, sizeof(*names) * (i + 1), GFP_KERNEL);
+		if (!p)
+			goto err;

What about using of_count_phandle_with_args() first to get the parent count?

drivers/clk/clk.c void __init of_clk_init(const struct of_device_id *matches) · line 2270

Quoted 6 lines

drivers/clk/clk.c +5 -0

+		names = p;
+
+		if (of_property_read_string(s.np, "clock-output-names",
+					&name) < 0)
+			name = s.np->name;

You should be able to use of_clk_get_parent_name() here, instead of parsing the properties directly.

Quoted 79 lines · On Wednesday 24 of July 2013 17:43:30 Stephen Boyd wrote:

Quoted 62 lines

Consolidate DT parsing for the common bits of a clock binding in one place to simplify clock drivers. This also has the added benefit of standardizing how the clock names used by the common clock framework are generated from the DT bindings. We always use the first clock-output-names string if it exists, otherwise we fall back to the node name.

To be slightly more efficient and make the caller's life easier, we introduce a shallow copy flag so that the clock core knows to just copy the pointers to the strings and not the string contents. Otherwise the callers of this function would have to free the strings allocated here which could be cumbersome.

Cc Rob Herring <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---
 drivers/clk/clk.c            | 59
+++++++++++++++++++++++++++++++++++++++++++-
include/linux/clk-provider.h |  7 ++++++
 2 files changed, 65 insertions(+), 1 deletion(-)

drivers/clk/clk.c +7 -1

index 1ed9bdd..ea8e951b 100644
@@ -1809,6 +1809,10 @@ static int _clk_register(struct device *dev,
 struct clk_hw *hw, struct clk *clk) {
    int i, ret;
 
+   hw->clk = clk;
+   if (hw->init->flags & CLK_SHALLOW_COPY)
+           return PTR_RET(__clk_register(dev, hw));
+
    clk->name = kstrdup(hw->init->name, GFP_KERNEL);
    if (!clk->name) {
            pr_err("%s: could not allocate clk->name\n", __func__);

@@ -1819,7 +1823,6 @@ static int _clk_register(struct device *dev,
 struct clk_hw *hw, struct clk *clk) clk->hw = hw;
    clk->flags = hw->init->flags;
    clk->num_parents = hw->init->num_parents;
-   hw->clk = clk;
 
    /* allocate local copy in case parent_names is __initdata */
    clk->parent_names = kzalloc((sizeof(char*) * clk->num_parents),

@@ -2232,4 +2235,58 @@ void __init of_clk_init(const struct of_device_id
 *matches) clk_init_cb(np);
    }
 }
+
+/**
+ * of_init_clk_data() - Initialize a clk_init_data struct from a DT

node + * @np: node to initialize struct from
+ * @init: struct to initialize
+ *
+ * Populates the clk_init_data struct by parsing the device node for
+ * properties matching the common clock binding. Returns 0 on success
+ * and a negative error code on failure.
+ */
+int of_init_clk_data(struct device_node *np, struct clk_init_data *init) +{

drivers/clk/clk.c +2 -0

+   struct of_phandle_args s;
+   const char **names = NULL, **p;

drivers/clk/clk.c void __init of_clk_init(const struct of_device_id *matches)

Quoted 10 lines

drivers/clk/clk.c +10 -0

+   const char *name;
+   int i;
+
+   if (of_property_read_string(np, "clock-output-names", &name) < 0)
+           name = np->name;
+   init->name = kstrdup(name, GFP_KERNEL);
+   if (!init->name)
+           return -ENOMEM;
+
+   for (i = 0; of_parse_phandle_with_args(np, "clocks", "#clock-

cells",

Quoted 4 lines

drivers/clk/clk.c +4 -0

+                           i, &s) == 0; i++) {
+           p = krealloc(names, sizeof(*names) * (i + 1), GFP_KERNEL);
+           if (!p)
+                   goto err;

Quoted 2 lines

What about using of_count_phandle_with_args() first to get the parent count?

Sure that simplifies things.

Quoted 10 lines

drivers/clk/clk.c void __init of_clk_init(const struct of_device_id *matches) · line 2270

Quoted 7 lines

drivers/clk/clk.c +5 -0

+           names = p;
+
+           if (of_property_read_string(s.np, "clock-output-names",
+                                   &name) < 0)
+                   name = s.np->name;

You should be able to use of_clk_get_parent_name() here, instead of parsing the properties directly.

This must be new too. Will do in v2.

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

In similar fashion as of_regulator_match() add an of_clk_match() function that finds an initializes clock init_data structs from devicetree. Drivers should use this API to find clocks that their device is providing and then iterate over their match table registering the clocks with the init data parsed.

Signed-off-by Stephen Boyd <[email protected]>

---
 drivers/clk/clk.c            | 64
++++++++++++++++++++++++++++++++++++++++++++
include/linux/clk-provider.h | 16 +++++++++++
 2 files changed, 80 insertions(+)

drivers/clk/clk.c +3 -0

index ea8e951b..1e3e0db 100644
@@ -2289,4 +2289,68 @@ err:
 	return -ENOMEM;
 }
 EXPORT_SYMBOL_GPL(of_init_clk_data);
+
+/**
+ * of_clk_match() - Scan and match clock providers from the DT node of

a device + * @dev: device initializing clock providers
+ * @matches: match table of clocks
+ * @num_matches: number of entries in @matches
+ *
+ * This function uses a match table specified by the clock driver to parse + * clock init data from the device tree. @dev is expected to be a device with + * a 'clocks' child node containing a set of child nodes, each providing the + * init data for one clock. The data parsed from a child node will be matched + * to a clock based on the child node's name. Note that the match table is + * modified in place.
+ *
+ * Returns the number of matches found or a negative error code on failure. + */

drivers/clk/clk.c +18 -0

+int of_clk_match(struct device *dev, struct of_clk_match *matches,
+		 int num_matches)
+{
+	int count = 0;
+	int err, i;
+	struct device_node *np, *clocks;
+	struct clk_init_data *init;
+
+	clocks = of_find_node_by_name(dev->of_node, "clocks");
+	if (!clocks)
+		return -EINVAL;
+
+	for (i = 0; i < num_matches; i++) {
+		struct of_clk_match *match = &matches[i];
+		match->init_data = NULL;
+		match->of_node = NULL;
+	}
+

drivers/clk/clk.c +13 -0

+					np->name);
+				return err;
+			}
+			match->of_node = np;
+			match->init_data = init;
+			count++;
+			break;
+		}
+	}
+
+	return count;
+}
+EXPORT_SYMBOL_GPL(of_clk_match);
 #endif

include/linux/clk-provider.h +0 -0

index 18d6362..484f8ad 100644
@@ -461,6 +461,13 @@ struct clk_onecell_data {
 		__used __section(__clk_of_table)		\
 		= { .compatible = compat, .data = fn };
 

include/linux/clk-provider.h +0 -0

@@ -475,6 +482,9 @@ const char *of_clk_get_parent_name(struct

device_node *np, int index); void of_clk_init(const struct of_device_id *matches);
int of_init_clk_data(struct device_node *np, struct clk_init_data
*init);

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

+int of_clk_match(struct device *dev, struct of_clk_match *matches,
+		 int num_matches);
+
 #else /* !CONFIG_OF */
 
 static inline int of_clk_add_provider(struct device_node *np,

@@ -508,6 +518,12 @@ of_init_clk_data(struct device_node *np, struct
 clk_init_data *init) {
 	return 0;
 }
+static inline int
+of_clk_match(struct device *dev, struct of_clk_match *matches, int
 num_matches) +{
+
+	return 0;
+}
 #endif /* CONFIG_OF */
 #endif /* CONFIG_COMMON_CLK */
 #endif /* CLK_PROVIDER_H */

Hi Stephen,

Quoted 5 lines · On Wednesday 24 of July 2013 17:43:31 Stephen Boyd wrote:

In similar fashion as of_regulator_match() add an of_clk_match() function that finds an initializes clock init_data structs from devicetree. Drivers should use this API to find clocks that their device is providing and then iterate over their match table registering the clocks with the init data parsed.

I think all you need here is declaring all your clock drivers using CLK_OF_DECLARE() macro. Then they will be automatically probed by of_clk_init(). See last lines of drivers/clk/clk-fixed-rate.c for an example.

In similar fashion as of_regulator_match() add an of_clk_match() function that finds an initializes clock init_data structs from devicetree. Drivers should use this API to find clocks that their device is providing and then iterate over their match table registering the clocks with the init data parsed.

Signed-off-by Stephen Boyd <[email protected]>

---
 drivers/clk/clk.c            | 64 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/clk-provider.h | 16 +++++++++++
 2 files changed, 80 insertions(+)

drivers/clk/clk.c +64 -0

index ea8e951b..1e3e0db 100644
@@ -2289,4 +2289,68 @@ err:
        return -ENOMEM;
 }
 EXPORT_SYMBOL_GPL(of_init_clk_data);
+
+/**
+ * of_clk_match() - Scan and match clock providers from the DT node of a device
+ * @dev: device initializing clock providers
+ * @matches: match table of clocks
+ * @num_matches: number of entries in @matches
+ *
+ * This function uses a match table specified by the clock driver to parse
+ * clock init data from the device tree. @dev is expected to be a device with
+ * a 'clocks' child node containing a set of child nodes, each providing the
+ * init data for one clock. The data parsed from a child node will be matched
+ * to a clock based on the child node's name. Note that the match table is
+ * modified in place.
+ *
+ * Returns the number of matches found or a negative error code on failure.
+ */
+int of_clk_match(struct device *dev, struct of_clk_match *matches,
+                int num_matches)
+{
+       int count = 0;
+       int err, i;
+       struct device_node *np, *clocks;
+       struct clk_init_data *init;
+
+       clocks = of_find_node_by_name(dev->of_node, "clocks");
+       if (!clocks)
+               return -EINVAL;
+
+       for (i = 0; i < num_matches; i++) {
+               struct of_clk_match *match = &matches[i];
+               match->init_data = NULL;
+               match->of_node = NULL;
+       }
+
+       for_each_available_child_of_node(clocks, np) {
+               for (i = 0; i < num_matches; i++) {
+                       struct of_clk_match *match = &matches[i];
+                       if (match->of_node)
+                               continue;
+
+                       if (strcmp(match->name, np->name))
+                               continue;
+
+                       init = devm_kzalloc(dev, sizeof(*init), GFP_KERNEL);
+                       if (!init)
+                               return -ENOMEM;
+
+                       err = of_init_clk_data(np, init);
+                       if (err) {
+                               dev_err(dev,
+                                       "failed to parse DT for clock %s\n",
+                                       np->name);
+                               return err;
+                       }
+                       match->of_node = np;
+                       match->init_data = init;
+                       count++;
+                       break;
+               }
+       }
+
+       return count;
+}
+EXPORT_SYMBOL_GPL(of_clk_match);
 #endif

include/linux/clk-provider.h +16 -0

index 18d6362..484f8ad 100644
@@ -461,6 +461,13 @@ struct clk_onecell_data {
                __used __section(__clk_of_table)                \
                = { .compatible = compat, .data = fn };
 
+struct of_clk_match {
+       const char *name;
+       void *driver_data;
+       struct clk_init_data *init_data;
+       struct device_node *of_node;
+};
+
 #ifdef CONFIG_OF
 int of_clk_add_provider(struct device_node *np,
                        struct clk *(*clk_src_get)(struct of_phandle_args *args,

@@ -475,6 +482,9 @@ const char *of_clk_get_parent_name(struct device_node *np, int index);
 void of_clk_init(const struct of_device_id *matches);
 int of_init_clk_data(struct device_node *np, struct clk_init_data *init);
 
+int of_clk_match(struct device *dev, struct of_clk_match *matches,
+                int num_matches);
+
 #else /* !CONFIG_OF */
 
 static inline int of_clk_add_provider(struct device_node *np,

@@ -508,6 +518,12 @@ of_init_clk_data(struct device_node *np, struct clk_init_data *init)
 {
        return 0;
 }
+static inline int
+of_clk_match(struct device *dev, struct of_clk_match *matches, int num_matches)
+{
+
+       return 0;
+}
 #endif /* CONFIG_OF */
 #endif /* CONFIG_COMMON_CLK */
 #endif /* CLK_PROVIDER_H */

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Quoting Stephen Boyd (2013-07-24 17:43:31)

Quoted 7 lines

In similar fashion as of_regulator_match() add an of_clk_match() function that finds an initializes clock init_data structs from devicetree. Drivers should use this API to find clocks that their device is providing and then iterate over their match table registering the clocks with the init data parsed.

Signed-off-by Stephen Boyd <[email protected]>

Stephen,

In general I like this approach. Writing real device drivers for clock controllers is The Right Way and of_clk_match helps.

Am I reading this correctly that the base register addresses/offsets for the clock nodes still come from C files? For example I still see pll8_desc defining reg stuff in drivers/clk/msm/gcc-8960.c.

What do you think about fetching this data from DT? My thinking here is that the definition of that PLL structure would be in C, as would all of the control logic. But any per-clock data whatsoever should live in DTS. This means the clock data you supply in the DTS files in patches #9 and #10 would have base addresses or offsets per-clock. I think this echoes Mark R's concerns as well.

In the future if new chips have more of that type of PLL it would not require changes to your clock driver, only new DTS data for the new chip.

I could have that wrong though, there is a fair amount of indirection in this series...

Quoted 14 lines

Quoting Stephen Boyd (2013-07-24 17:43:31)

Quoted 7 lines

In similar fashion as of_regulator_match() add an of_clk_match() function that finds an initializes clock init_data structs from devicetree. Drivers should use this API to find clocks that their device is providing and then iterate over their match table registering the clocks with the init data parsed.

Signed-off-by Stephen Boyd <[email protected]>

Stephen,

In general I like this approach. Writing real device drivers for clock controllers is The Right Way and of_clk_match helps.

Quoted 3 lines

Am I reading this correctly that the base register addresses/offsets for the clock nodes still come from C files? For example I still see pll8_desc defining reg stuff in drivers/clk/msm/gcc-8960.c.

I think we may be able to put the registers in DT but I don't know why we need to do that if we're already matching up nodes with C structs.

It also made me want to introduce devm_of_iomap() which just seemed wrong (if you have a dev struct why can't you use devm_ioremap()).

Quoted 8 lines

What do you think about fetching this data from DT? My thinking here is that the definition of that PLL structure would be in C, as would all of the control logic. But any per-clock data whatsoever should live in DTS. This means the clock data you supply in the DTS files in patches #9 and #10 would have base addresses or offsets per-clock. I think this echoes Mark R's concerns as well.

Quoted 6 lines

In the future if new chips have more of that type of PLL it would not require changes to your clock driver, only new DTS data for the new chip.

I could have that wrong though, there is a fair amount of indirection in this series...

Let's take the PLL example and see if I follow what would be in DT and what would be in C.

Right now we have

        pll8: pll8 {
                #clock-cells = <0>;
                compatible = "qcom,pll";
                clocks = <&pxo>;
        };

in DT and

        static struct pll_desc pll8_desc = {
                .l_reg = 0x3144,
                .m_reg = 0x3148,
                .n_reg = 0x314c,
                .config_reg = 0x3154,
                .mode_reg = 0x3140,
                .status_reg = 0x3158,
                .status_bit = 16,
        };

in C. Do you want everything to be in DT? Something like:

        pll8: pll8 at 3140 {
                #clock-cells = <0>;
                compatible = "qcom,pll";
                clocks = <&pxo>;
                reg = <0x3140 0x20>;
        };

and then assume that all those registers are offset from the base register and that the status bit is 16 (it usually is but not always)?

The problem I see is this quickly breaks down with more complicated clocks like the RCGs.

We have

        gsbi5_uart_rcg: gsbi5_uart_rcg {
                #clock-cells = <0>;
                compatible = "qcom,p2-mn16-clock";
                clocks = <&pxo>, <&vpll8>;
        };

in DT and

        static struct freq_tbl clk_tbl_gsbi_uart[] = {
                {  1843200, PLL8, 2,  6, 625 },
                {  3686400, PLL8, 2, 12, 625 },
                {  7372800, PLL8, 2, 24, 625 },
                { 14745600, PLL8, 2, 48, 625 },
                { 16000000, PLL8, 4,  1,   6 },
                { 24000000, PLL8, 4,  1,   4 },
                { 32000000, PLL8, 4,  1,   3 },
                { 40000000, PLL8, 1,  5,  48 },
                { 46400000, PLL8, 1, 29, 240 },
                { 48000000, PLL8, 4,  1,   2 },
                { 51200000, PLL8, 1,  2,  15 },
                { 56000000, PLL8, 1,  7,  48 },
                { 58982400, PLL8, 1, 96, 625 },
                { 64000000, PLL8, 2,  1,   3 },
                { }
        };

        static struct rcg_desc gsbi5_uart_rcg = {
                .ctl_reg = 0x2a54,
                .ns_reg = 0x2a54,
                .md_reg = 0x2a50,
                .ctl_bit = 11,
                .mnctr_en_bit = 8,
                .mnctr_reset_bit = 7,
                .mnctr_mode_shift = 5,
                .pre_div_shift = 3,
                .src_sel_shift = 0,
                .n_val_shift = 16,
                .m_val_shift = 16,
                .parent_map = gcc_pxo_pll8_map,
                .freq_tbl = clk_tbl_gsbi_uart,
        };

in C. It starts to get pretty unwieldy when you put this all in DT, plus you'll notice that the ns_reg and ctl_reg are the same here because we've generalized the code to work with different types of software interfaces (technically this clock has no ctl register, just an NS and MD register). Our multimedia clock controllers don't follow any standard base/offset pair and so the ctl_reg can be a different offset from the md_reg depending on which clock we're talking about. My initial try at translating this into DT pretty much just made every struct member into a property, including the duplicate register, expect for the frequency table, which could probably also be DT-ified with some work.

        gsbi5_uart_rcg: gsbi5_uart_rcg at 2a54 {
                #clock-cells = <0>;
                compatible = "qcom,p2-mn16-clock";
                clocks = <&pxo>, <&vpll8>;
                reg = <0x2a54 0x4>,
                      <0x2a54 0x4>,
                      <0x2a50 0x4>;
                ctl_bit = <11>;
                mnctr_en_bit = <8>;
                mnctr_reset_bit = <7>;
                mnctr_mode_shift = <5>;
                pre_div_shift = <3>;
                src_sel_shift = <0>;
                n_val_shift = <16>;
                m_val_shift = <16>;
        };

This is great for making the kernel DT-data-driven, but I couldn't find any other driver that was describing register level details in DT.

The good news is that newer clock controllers follow a standard and so we can specify one or two register properties and the type of clock and we're pretty much done. The software interface hasn't been randomized like on earlier controllers and bits within registers are always the same.

We still have some clocks
that are just on/off switches though and so we'll have to put register level details like which bit turns that clock on in DT (which I believe is not preferred/allowed?). I don't see any way to avoid that if we want it to be entirely DT driven.

Signature · 3 lines

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Quoting Stephen Boyd (2013-08-12 22:48:39)

Quoted 19 lines · Mike Turquette

Quoted 17 lines

Quoting Stephen Boyd (2013-07-24 17:43:31)

Quoted 7 lines

In similar fashion as of_regulator_match() add an of_clk_match() function that finds an initializes clock init_data structs from devicetree. Drivers should use this API to find clocks that their device is providing and then iterate over their match table registering the clocks with the init data parsed.

Signed-off-by Stephen Boyd <[email protected]>

Stephen,

In general I like this approach. Writing real device drivers for clock controllers is The Right Way and of_clk_match helps.

Am I reading this correctly that the base register addresses/offsets for the clock nodes still come from C files? For example I still see pll8_desc defining reg stuff in drivers/clk/msm/gcc-8960.c.

Quoted 3 lines

I think we may be able to put the registers in DT but I don't know why we need to do that if we're already matching up nodes with C structs.

The reason to do so is to remove the per-clock data from the kernel sources entirely. Separating logic and data.

Quoted 5 lines

It also made me want to introduce devm_of_iomap() which just seemed wrong (if you have a dev struct why can't you use devm_ioremap()).

Quoted 4 lines

This is great for making the kernel DT-data-driven, but I couldn't find any other driver that was describing register level details in DT.

Yeah, this sucks. Building a binding from a C struct is a bad idea. How many permutations are there? Hopefully some clocks use the same bit shifts and have reliable register offsets, with the only difference being base address. If this is the case then a compatible string could be done for each permutation assuming that number is low and manageable.

Quoted 7 lines

The good news is that newer clock controllers follow a standard and so we can specify one or two register properties and the type of clock and we're pretty much done. The software interface hasn't been randomized like on earlier controllers and bits within registers are always the same.

This does not suck. Just for the sake of argument let's say that you only had to deal with this new and improved register layout and not the old (current) stuff. Do you still see a reason to match DT data up with C struct data objects in the kernel?

Quoted 6 lines

We still have some clocks
that are just on/off switches though and so we'll have to put register level details like which bit turns that clock on in DT (which I believe is not preferred/allowed?). I don't see any way to avoid that if we want it to be entirely DT driven.

This is what I'm doing for the generic clock bindings. No one has screamed over that stuff so I guess rules were meant to be broken.

Quoted 20 lines

Quoting Stephen Boyd (2013-08-12 22:48:39)

Quoted 19 lines · Mike Turquette

Quoted 17 lines

Quoting Stephen Boyd (2013-07-24 17:43:31)

Quoted 7 lines

In similar fashion as of_regulator_match() add an of_clk_match() function that finds an initializes clock init_data structs from devicetree. Drivers should use this API to find clocks that their device is providing and then iterate over their match table registering the clocks with the init data parsed.

Signed-off-by Stephen Boyd <[email protected]>

Stephen,

In general I like this approach. Writing real device drivers for clock controllers is The Right Way and of_clk_match helps.

Am I reading this correctly that the base register addresses/offsets for the clock nodes still come from C files? For example I still see pll8_desc defining reg stuff in drivers/clk/msm/gcc-8960.c.

Quoted 6 lines

Quoted 3 lines

I think we may be able to put the registers in DT but I don't know why we need to do that if we're already matching up nodes with C structs.

The reason to do so is to remove the per-clock data from the kernel sources entirely. Separating logic and data.

Ok.

Quoted 5 lines

Quoted 5 lines

It also made me want to introduce devm_of_iomap() which just seemed wrong (if you have a dev struct why can't you use devm_ioremap()).

[snip]

Quoted 10 lines

Quoted 4 lines

This is great for making the kernel DT-data-driven, but I couldn't find any other driver that was describing register level details in DT.

Yeah, this sucks. Building a binding from a C struct is a bad idea. How many permutations are there? Hopefully some clocks use the same bit shifts and have reliable register offsets, with the only difference being base address. If this is the case then a compatible string could be done for each permutation assuming that number is low and manageable.

In the multimedia controller almost every clock has a different register layout. Making a compatible string for each clock is pretty much what I've done if you consider that I match up the name of the node to a struct instead of matching a compatible string to a struct.

In the global controller it's more sane,
following a single register layout per compatible string. Remember though, all the single bit clocks (gates or what we call branches) have a completely random location for their on/off bit and status bit.

Quoted 9 lines

Quoted 8 lines

The good news is that newer clock controllers follow a standard and so we can specify one or two register properties and the type of clock and we're pretty much done. The software interface hasn't been randomized like on earlier controllers and bits within registers are always the same.

Quoted 4 lines

This does not suck. Just for the sake of argument let's say that you only had to deal with this new and improved register layout and not the old (current) stuff. Do you still see a reason to match DT data up with C struct data objects in the kernel?

I would still need to match up frequency tables unless I figure out a way to put that in DT too.

Quoted 10 lines

Quoted 6 lines

We still have some clocks
that are just on/off switches though and so we'll have to put register level details like which bit turns that clock on in DT (which I believe is not preferred/allowed?). I don't see any way to avoid that if we want it to be entirely DT driven.

This is what I'm doing for the generic clock bindings. No one has screamed over that stuff so I guess rules were meant to be broken.

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Quoted 29 lines

Right now we have

       pll8: pll8 {
               #clock-cells = <0>;
               compatible = "qcom,pll";
               clocks = <&pxo>;
       };

in DT and

       static struct pll_desc pll8_desc = {
               .l_reg = 0x3144,
               .m_reg = 0x3148,
               .n_reg = 0x314c,
               .config_reg = 0x3154,
               .mode_reg = 0x3140,
               .status_reg = 0x3158,
               .status_bit = 16,
       };

in C. Do you want everything to be in DT? Something like:

       pll8: pll8 at 3140 {
               #clock-cells = <0>;
               compatible = "qcom,pll";
               clocks = <&pxo>;
               reg = <0x3140 0x20>;
       };

Quoted 3 lines

and then assume that all those registers are offset from the base register and that the status bit is 16 (it usually is but not always)?

Quoted 29 lines · Mike Turquette

Quoted 29 lines

Right now we have

      pll8: pll8 {
              #clock-cells = <0>;
              compatible = "qcom,pll";
              clocks = <&pxo>;
      };

in DT and

      static struct pll_desc pll8_desc = {
              .l_reg = 0x3144,
              .m_reg = 0x3148,
              .n_reg = 0x314c,
              .config_reg = 0x3154,
              .mode_reg = 0x3140,
              .status_reg = 0x3158,
              .status_bit = 16,
      };

in C. Do you want everything to be in DT? Something like:

      pll8: pll8 at 3140 {
              #clock-cells = <0>;
              compatible = "qcom,pll";
              clocks = <&pxo>;
              reg = <0x3140 0x20>;
      };

Quoted 3 lines

Quoted 3 lines

and then assume that all those registers are offset from the base register and that the status bit is 16 (it usually is but not always)?

I think its reasonable to put the various regs associated with a clock in the .dts like the example you show, but we should be going down to bit level details. If we think of each clock as its own device its reasonable that the clock would have some set of registers associated with it.

Some of Qualcomm's clocks can change their parent and rate at the same time with a single register write. Add support for this hardware to the common clock framework by adding a new set_rate_and_parent() op. When the clock framework determines that both the parent and the rate are going to change during clk_set_rate() it will call the .set_rate_and_parent() op if available and fall back to calling .set_parent() followed by .set_rate() otherwise.

Cc James Hogan <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

---
 Documentation/clk.txt        |  3 ++
 drivers/clk/clk.c            | 78
+++++++++++++++++++++++++++++++++-----------
include/linux/clk-provider.h | 15 +++++++++
 3 files changed, 77 insertions(+), 19 deletions(-)

Documentation/clk.txt +0 -0

index 3aeb5c4..79700ea 100644

Documentation/clk.txt +0 -0

 		void		(*init)(struct clk_hw *hw);
 	};
 

drivers/clk/clk.c +43 -19

index 1e3e0db..73de07c 100644
@@ -1121,10 +1121,9 @@ static void clk_reparent(struct clk *clk, struct
 clk *new_parent) clk->parent = new_parent;
 }
 
-static int __clk_set_parent(struct clk *clk, struct clk *parent, u8
 p_index) +static struct clk *__clk_set_parent_before(struct clk *clk,
 struct clk *parent) {
 	unsigned long flags;
-	int ret = 0;
 	struct clk *old_parent = clk->parent;
 
 	/*

@@ -1155,6 +1154,34 @@ static int __clk_set_parent(struct clk *clk,
 struct clk *parent, u8 p_index) clk_reparent(clk, parent);
 	clk_enable_unlock(flags);
 
+	return old_parent;
+}
+
+static void __clk_set_parent_after(struct clk *clk, struct clk *parent,
+		struct clk *old_parent)
+{
+	/*
+	 * Finish the migration of prepare state and undo the changes done
+	 * for preventing a race with clk_enable().
+	 */
+	if (clk->prepare_count) {
+		clk_disable(clk);
+		clk_disable(old_parent);
+		__clk_unprepare(old_parent);
+	}
+
+	/* update debugfs with new clk tree topology */
+	clk_debug_reparent(clk, parent);
+}
+
+static int __clk_set_parent(struct clk *clk, struct clk *parent, u8
 p_index) +{
+	unsigned long flags;
+	int ret = 0;
+	struct clk *old_parent;
+
+	old_parent = __clk_set_parent_before(clk, parent);
+
 	/* change clock input source */
 	if (parent && clk->ops->set_parent)
 		ret = clk->ops->set_parent(clk->hw, p_index);

@@ -1172,18 +1199,8 @@ static int __clk_set_parent(struct clk *clk,
 struct clk *parent, u8 p_index) return ret;
 	}
 
-	/*
-	 * Finish the migration of prepare state and undo the changes done
-	 * for preventing a race with clk_enable().
-	 */
-	if (clk->prepare_count) {
-		clk_disable(clk);
-		clk_disable(old_parent);
-		__clk_unprepare(old_parent);
-	}
+	__clk_set_parent_after(clk, parent, old_parent);
 
-	/* update debugfs with new clk tree topology */
-	clk_debug_reparent(clk, parent);
 	return 0;
 }
 

@@ -1368,17 +1385,32 @@ static void clk_change_rate(struct clk *clk)
 	struct clk *child;
 	unsigned long old_rate;
 	unsigned long best_parent_rate = 0;
+	bool skip_set_rate = false;
+	struct clk *old_parent;
 
 	old_rate = clk->rate;
 
-	/* set parent */
-	if (clk->new_parent && clk->new_parent != clk->parent)
-		__clk_set_parent(clk, clk->new_parent, clk-
 new_parent_index);
-
-	if (clk->parent)
+	if (clk->new_parent)
+		best_parent_rate = clk->new_parent->rate;
+	else if (clk->parent)
 		best_parent_rate = clk->parent->rate;
 
-	if (clk->ops->set_rate)
+	if (clk->new_parent && clk->new_parent != clk->parent) {
+		old_parent = __clk_set_parent_before(clk, clk-
 new_parent);
+
+		if (clk->ops->set_rate_and_parent) {
+			skip_set_rate = true;
+			clk->ops->set_rate_and_parent(clk->hw, clk-
 new_rate,
+					best_parent_rate,
+					clk->new_parent_index);
+		} else if (clk->ops->set_parent) {
+			clk->ops->set_parent(clk->hw, clk-

new_parent_index);

drivers/clk/clk.c +5 -0

+				__func__, clk->name);
+		ret = -EINVAL;
+		goto out;
+	}
+
 	/* throw a WARN if any entries in parent_names are NULL */
 	for (i = 0; i < clk->num_parents; i++)
 		WARN(!clk->parent_names[i],

include/linux/clk-provider.h +1 -0

index 484f8ad..1f7eabb 100644
@@ -108,6 +108,18 @@ struct clk_hw;
  *		which is likely helpful for most .set_rate implementation.
  *		Returns 0 on success, -EERROR otherwise.
  *
+ * @set_rate_and_parent: Change the rate and the parent of this clock.

The + * requested rate is specified by the second

* should typically be the return of .round_rate call. The
+ * third argument gives the parent rate which is likely

+ * for most .set_rate_and_parent implementation. The fourth
+ * argument gives the parent index. It is optional (and
+ * unnecessary) for clocks with 0 or 1 parents as well as
+ * for clocks that can tolerate switching the rate and the

+ * separately via calls to .set_parent and .set_rate.
+ * Returns 0 on success, -EERROR otherwise.
+ *
+ *
* The clk_enable/clk_disable and clk_prepare/clk_unprepare pairs allow
* implementations to split any work between atomic (enable) and sleepable * (prepare) contexts. If enabling a clock requires code that might sleep, @@ -139,6 +151,9 @@ struct clk_ops {

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

 	u8		(*get_parent)(struct clk_hw *hw);
 	int		(*set_rate)(struct clk_hw *hw, unsigned long,
 				    unsigned long);
+	int		(*set_rate_and_parent)(struct clk_hw *hw,
+				    unsigned long rate,
+				    unsigned long parent_rate, u8 index);
 	void		(*init)(struct clk_hw *hw);
 };

Quoted 8 lines · On Wednesday 24 of July 2013 17:43:32 Stephen Boyd wrote:

Some of Qualcomm's clocks can change their parent and rate at the same time with a single register write. Add support for this hardware to the common clock framework by adding a new set_rate_and_parent() op. When the clock framework determines that both the parent and the rate are going to change during clk_set_rate() it will call the .set_rate_and_parent() op if available and fall back to calling .set_parent() followed by .set_rate() otherwise.

This is strange. Does you hardware support switching parent and rate separately or you always need to set both and so all the fuss here?

If the latter is the case, then maybe you can simply keep parent index and rate cached inside driver data of your clock driver and use them on any .set_rate() or .set_parent() calls?

Quoted 10 lines · On Wednesday 24 of July 2013 17:43:32 Stephen Boyd wrote:

Quoted 8 lines

Some of Qualcomm's clocks can change their parent and rate at the same time with a single register write. Add support for this hardware to the common clock framework by adding a new set_rate_and_parent() op. When the clock framework determines that both the parent and the rate are going to change during clk_set_rate() it will call the .set_rate_and_parent() op if available and fall back to calling .set_parent() followed by .set_rate() otherwise.

Quoted 2 lines

This is strange. Does you hardware support switching parent and rate separately or you always need to set both and so all the fuss here?

It supports setting the parent or setting the rate, or setting both at the same time.

Quoted 5 lines

If the latter is the case, then maybe you can simply keep parent index and rate cached inside driver data of your clock driver and use them on any .set_rate() or .set_parent() calls?

This will not work. In fact, doing that would cause us to overclock hardware for a short time between switching the parent and the rate.

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Some of Qualcomm's clocks can change their parent and rate at the same time with a single register write. Add support for this hardware to the common clock framework by adding a new set_rate_and_parent() op. When the clock framework determines that both the parent and the rate are going to change during clk_set_rate() it will call the .set_rate_and_parent() op if available and fall back to calling .set_parent() followed by .set_rate() otherwise.

Cc James Hogan <[email protected]>

Signed-off-by Stephen Boyd <[email protected]>

include/linux/clk-provider.h +0 -0

index 484f8ad..1f7eabb 100644

Add support for MSM's PLLs (phase locked loops). This is sufficient enough to be able to determine the rate the PLL is running at. We can add rate setting support later when it's needed.

Cc devicetree at vger.kernel.org

Signed-off-by Stephen Boyd <[email protected]>

---
 Documentation/devicetree/bindings/clock/msm.txt |  40 ++++
 drivers/clk/Kconfig                             |   2 +
 drivers/clk/Makefile                            |   1 +
 drivers/clk/msm/Kconfig                         |   4 +
 drivers/clk/msm/Makefile                        |   3 +
 drivers/clk/msm/clk-pll.c                       | 233
++++++++++++++++++++++++ drivers/clk/msm/clk-pll.h                     
 |  43 +++++
 7 files changed, 326 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/msm.txt
 create mode 100644 drivers/clk/msm/Kconfig
 create mode 100644 drivers/clk/msm/Makefile
 create mode 100644 drivers/clk/msm/clk-pll.c
 create mode 100644 drivers/clk/msm/clk-pll.h

drivers/clk/msm/clk-pll.h new file +37 -0

new file mode 100644
index 0000000..4e63a5e
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms. + *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __MSM_CLK_PLL_H__
+#define __MSM_CLK_PLL_H__
+
+struct device;
+struct clk;
+struct clk_init_data;
+
+struct pll_desc {
+	void __iomem *base;
+	u16 l_reg;
+	u16 m_reg;
+	u16 n_reg;
+	u16 config_reg;
+	u16 mode_reg;
+	u16 status_reg;
+	u8 status_bit;
+};
+
+struct pll_vote_desc {
+	void __iomem *base;
+	u16 vote_reg;
+	u8 vote_bit;
+};
+
+extern struct clk *pll_clk_register(struct device *dev, struct pll_desc

*desc, + struct clk_init_data *init);

drivers/clk/msm/clk-pll.h line 41

Quoted 4 lines · On Wednesday 24 of July 2013 17:43:33 Stephen Boyd wrote:

+extern struct clk *pll_clk_register(struct device *dev, struct pll_desc
*desc, + struct clk_init_data *init);

drivers/clk/msm/clk-pll.h new file +2 -0

+extern struct clk *pll_vote_clk_register(struct device *dev,
+		struct pll_vote_desc *desc, struct clk_init_data *init);

I don't think such generic name is good here. What about prefixing them with msm_ or qcom_ string?

Add a clock driver that registers clocks from a DT node's 'clocks' child. Each new SoC will add a file describing the software interface and frequency plan to drivers/clk/msm/ and then hook that into the msm_cc_match_table by means of a compatible string and an msm_clk_match table.

Signed-off-by Stephen Boyd <[email protected]>

---
 drivers/clk/msm/Makefile   |   2 +
 drivers/clk/msm/core.c     | 265
+++++++++++++++++++++++++++++++++++++++++++++
drivers/clk/msm/internal.h |  24 ++++
 3 files changed, 291 insertions(+)
 create mode 100644 drivers/clk/msm/core.c
 create mode 100644 drivers/clk/msm/internal.h

drivers/clk/msm/Makefile +2 -0

index e1cee29..9cfd0d7 100644
@@ -4,3 +4,5 @@ clk-msm-$(CONFIG_COMMON_CLK_MSM) += clk-pll.o
 clk-msm-$(CONFIG_COMMON_CLK_MSM) += clk-rcg.o
 clk-msm-$(CONFIG_COMMON_CLK_MSM) += clk-rcg2.o
 clk-msm-$(CONFIG_COMMON_CLK_MSM) += clk-branch.o
+
+clk-msm-$(CONFIG_COMMON_CLK_MSM) += core.o

drivers/clk/msm/core.c new file +31 -0

new file mode 100644
index 0000000..b1904c0
@@ -0,0 +1,265 @@
+/*
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms. + *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/spinlock.h>
+
+#include "internal.h"
+#include "clk-pll.h"
+#include "clk-rcg.h"
+#include "clk-branch.h"
+
+struct cc_data {
+	void __iomem *base;
+	spinlock_t lock;
+};
+
+static struct clk *

drivers/clk/msm/core.c new file +33 -0

+}
+
+static struct clk *
+dispatch_pll_clk(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc)
+{
+	struct pll_desc *desc = m->driver_data;
+
+	desc->base = cc->base;
+
+	return pll_clk_register(dev, desc, m->init_data);
+}
+
+static struct clk *
+dispatch_pll_vote_clk(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc)
+{
+	struct pll_vote_desc *desc = m->driver_data;
+
+	desc->base = cc->base;
+
+	return pll_vote_clk_register(dev, desc, m->init_data);
+}
+
+static struct clk *
+dispatch_rcg_p2mn16_clk(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc)
+{
+	struct rcg_desc *desc = m->driver_data;
+
+	desc->base = cc->base;
+
+	return rcg_p2mn16_clk_register(dev, desc, &cc->lock, m-

init_data);

drivers/clk/msm/core.c new file +15 -0

+}
+
+static struct clk *
+dispatch_rcg_p2mn8_clk(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc)
+{
+	struct rcg_desc *desc = m->driver_data;
+
+	desc->base = cc->base;
+
+	return rcg_p2mn8_clk_register(dev, desc, &cc->lock, m->init_data);
+}
+
+static struct clk *
+dispatch_rcg_mn8_dyn_clk(struct of_clk_match *match, struct device

*dev, + struct cc_data *cc)

drivers/clk/msm/core.c new file +30 -0

+{
+	struct rcg_dyn_desc *desc = match->driver_data;
+
+	desc->base = cc->base;
+
+	return rcg_mn8_dyn_clk_register(dev, desc, &cc->lock,
+			match->init_data);
+}
+
+static struct clk *
+dispatch_rcg_p4_dyn_clk(struct of_clk_match *match, struct device *dev,
+		struct cc_data *cc)
+{
+	struct rcg_dyn_desc *desc = match->driver_data;
+
+	desc->base = cc->base;
+
+	return rcg_p4_dyn_clk_register(dev, desc, &cc->lock,
+			match->init_data);
+}
+
+static struct clk *
+dispatch_rcg_h5mn8_clk(struct of_clk_match *match, struct device *dev,
+		struct cc_data *cc)
+{
+	struct rcg2_desc *desc = match->driver_data;
+
+	desc->base = cc->base;
+
+	return rcg_h5mn8_clk_register(dev, desc, &cc->lock, match-

init_data);

drivers/clk/msm/core.c new file +11 -0

+}
+
+static struct clk *
+dispatch_rcg_h5mn16_clk(struct of_clk_match *match, struct device *dev,
+		struct cc_data *cc)
+{
+	struct rcg2_desc *desc = match->driver_data;
+
+	desc->base = cc->base;
+
+	return rcg_h5mn16_clk_register(dev, desc, &cc->lock,

match->init_data); +}

drivers/clk/msm/core.c new file +15 -0

+
+static struct clk *
+dispatch_branch_clk(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc)
+{
+	struct branch_desc *desc = m->driver_data;
+
+	desc->base = cc->base;
+
+	return branch_clk_register(dev, desc, &cc->lock, m->init_data);
+}
+
+static struct clk *
+dispatch_branch_hg_clk(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc)

}, + { .compatible = "qcom,mn8-dyn-clock", .data =
dispatch_rcg_mn8_dyn_clk }, + { .compatible = "qcom,p4-dyn-clock",
.data = dispatch_rcg_p4_dyn_clk }, + { .compatible =
"qcom,h5-mn8-clock", .data = dispatch_rcg_h5mn8_clk }, + {

= "qcom,h5-mn16-clock", .data = dispatch_rcg_h5mn16_clk }, + {
.compatible = "qcom,cxc-clock", .data = dispatch_branch_clk }, + {
.compatible = "qcom,cxc-hg-clock", .data = dispatch_branch_hg_clk }, +};

drivers/clk/msm/core.c new file +75 -0

+typedef struct clk *
+(*clk_dispatch_fn)(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc);
+
+static const struct of_device_id msm_cc_match_table[] = {
+	{ }
+};
+MODULE_DEVICE_TABLE(of, msm_cc_match_table);
+
+static int msm_cc_probe(struct platform_device *pdev)
+{
+	struct cc_data *cc;
+	void __iomem *base;
+	struct resource *res;
+	int count, i;
+	const struct of_device_id *id_entry;
+	const struct msm_clk_match *m;
+	struct of_clk_match *matches;
+	size_t size;
+
+	id_entry = of_match_device(msm_cc_match_table, &pdev->dev);
+	m = id_entry->data;
+	matches = m->matches;
+	size = m->size;
+
+	cc = devm_kzalloc(&pdev->dev, sizeof(*cc), GFP_KERNEL);
+	if (!cc)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	cc->base = base;
+	spin_lock_init(&cc->lock);
+
+	count = of_clk_match(&pdev->dev, matches, size);
+	if (count < 0)
+		return count;
+
+	for (i = 0; i < size; i++) {
+		struct of_clk_match *match;
+		const struct of_device_id *id;
+		struct device_node *np;
+		clk_dispatch_fn fn;
+		struct clk *clk;
+		int err;
+
+		match = &matches[i];
+		np = match->of_node;
+		if (!np)
+			continue;
+
+		id = of_match_node(dispatch_table, match->of_node);
+		if (!id)
+			continue;
+
+		fn = id->data;
+		clk = fn(match, &pdev->dev, cc);
+		if (IS_ERR(clk))
+			return PTR_ERR(clk);
+
+		err = of_clk_add_provider(np, of_clk_src_simple_get, clk);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static struct platform_driver msm_cc_driver = {
+	.probe		= msm_cc_probe,
+	.driver		= {
+		.name	= "msm-clock",

Quoted 63 lines · On Wednesday 24 of July 2013 17:43:36 Stephen Boyd wrote:

Add a clock driver that registers clocks from a DT node's 'clocks' child. Each new SoC will add a file describing the software interface and frequency plan to drivers/clk/msm/ and then hook that into the msm_cc_match_table by means of a compatible string and an msm_clk_match table.

Signed-off-by Stephen Boyd <[email protected]>

---
 drivers/clk/msm/Makefile   |   2 +
 drivers/clk/msm/core.c     | 265
+++++++++++++++++++++++++++++++++++++++++++++
drivers/clk/msm/internal.h |  24 ++++
 3 files changed, 291 insertions(+)
 create mode 100644 drivers/clk/msm/core.c
 create mode 100644 drivers/clk/msm/internal.h

drivers/clk/msm/Makefile +2 -0

index e1cee29..9cfd0d7 100644
@@ -4,3 +4,5 @@ clk-msm-$(CONFIG_COMMON_CLK_MSM) += clk-pll.o
 clk-msm-$(CONFIG_COMMON_CLK_MSM) += clk-rcg.o
 clk-msm-$(CONFIG_COMMON_CLK_MSM) += clk-rcg2.o
 clk-msm-$(CONFIG_COMMON_CLK_MSM) += clk-branch.o
+
+clk-msm-$(CONFIG_COMMON_CLK_MSM) += core.o

drivers/clk/msm/core.c new file +31 -0

new file mode 100644
index 0000000..b1904c0
@@ -0,0 +1,265 @@
+/*
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms. + *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/spinlock.h>
+
+#include "internal.h"
+#include "clk-pll.h"
+#include "clk-rcg.h"
+#include "clk-branch.h"
+
+struct cc_data {
+	void __iomem *base;
+	spinlock_t lock;
+};
+
+static struct clk *

drivers/clk/msm/core.c line 41

Quoted 10 lines

drivers/clk/msm/core.c new file +10 -0

+dispatch_fixed_clk(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc)
+{
+	u32 rate;
+	const char *name = m->init_data->name;
+
+	if (of_property_read_u32(m->of_node, "clock-frequency", &rate))
+		return ERR_PTR(-EINVAL);
+
+	return clk_register_fixed_rate(dev, name, NULL, CLK_IS_ROOT, 

rate);

Quoted 112 lines

drivers/clk/msm/core.c new file +33 -0

+}
+
+static struct clk *
+dispatch_pll_clk(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc)
+{
+	struct pll_desc *desc = m->driver_data;
+
+	desc->base = cc->base;
+
+	return pll_clk_register(dev, desc, m->init_data);
+}
+
+static struct clk *
+dispatch_pll_vote_clk(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc)
+{
+	struct pll_vote_desc *desc = m->driver_data;
+
+	desc->base = cc->base;
+
+	return pll_vote_clk_register(dev, desc, m->init_data);
+}
+
+static struct clk *
+dispatch_rcg_p2mn16_clk(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc)
+{
+	struct rcg_desc *desc = m->driver_data;
+
+	desc->base = cc->base;
+
+	return rcg_p2mn16_clk_register(dev, desc, &cc->lock, m-

init_data);

drivers/clk/msm/core.c new file +15 -0

+}
+
+static struct clk *
+dispatch_rcg_p2mn8_clk(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc)
+{
+	struct rcg_desc *desc = m->driver_data;
+
+	desc->base = cc->base;
+
+	return rcg_p2mn8_clk_register(dev, desc, &cc->lock, m->init_data);
+}
+
+static struct clk *
+dispatch_rcg_mn8_dyn_clk(struct of_clk_match *match, struct device

*dev, + struct cc_data *cc)

drivers/clk/msm/core.c new file +30 -0

+{
+	struct rcg_dyn_desc *desc = match->driver_data;
+
+	desc->base = cc->base;
+
+	return rcg_mn8_dyn_clk_register(dev, desc, &cc->lock,
+			match->init_data);
+}
+
+static struct clk *
+dispatch_rcg_p4_dyn_clk(struct of_clk_match *match, struct device *dev,
+		struct cc_data *cc)
+{
+	struct rcg_dyn_desc *desc = match->driver_data;
+
+	desc->base = cc->base;
+
+	return rcg_p4_dyn_clk_register(dev, desc, &cc->lock,
+			match->init_data);
+}
+
+static struct clk *
+dispatch_rcg_h5mn8_clk(struct of_clk_match *match, struct device *dev,
+		struct cc_data *cc)
+{
+	struct rcg2_desc *desc = match->driver_data;
+
+	desc->base = cc->base;
+
+	return rcg_h5mn8_clk_register(dev, desc, &cc->lock, match-

init_data);

drivers/clk/msm/core.c new file +11 -0

+}
+
+static struct clk *
+dispatch_rcg_h5mn16_clk(struct of_clk_match *match, struct device *dev,
+		struct cc_data *cc)
+{
+	struct rcg2_desc *desc = match->driver_data;
+
+	desc->base = cc->base;
+
+	return rcg_h5mn16_clk_register(dev, desc, &cc->lock,

match->init_data); +}

drivers/clk/msm/core.c new file +19 -0

+
+static struct clk *
+dispatch_branch_clk(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc)
+{
+	struct branch_desc *desc = m->driver_data;
+
+	desc->base = cc->base;
+
+	return branch_clk_register(dev, desc, &cc->lock, m->init_data);
+}
+
+static struct clk *
+dispatch_branch_hg_clk(struct of_clk_match *m, struct device *dev,
+		struct cc_data *cc)
+{
+	struct branch_desc *desc = m->driver_data;
+
+	desc->base = cc->base;

drivers/clk/msm/core.c line 158

Quoted 6 lines

drivers/clk/msm/core.c new file +6 -0

+
+	return branch_hg_clk_register(dev, desc, &cc->lock, m->init_data);
+}
+
+static const struct of_device_id dispatch_table[] = {
+	{ .compatible = "fixed-clock", .data = dispatch_fixed_clk },

This is already handled by the fixed-rate-clock driver and of_clk_init().

drivers/clk/msm/core.c

Quoted 4 lines

drivers/clk/msm/core.c new file +3 -0

+	{ .compatible = "qcom,pll", .data = dispatch_pll_clk },
+	{ .compatible = "qcom,pll-vote", .data = dispatch_pll_vote_clk },
+	{ .compatible = "qcom,p2-mn8-clock", .data = 

dispatch_rcg_p2mn8_clk },

drivers/clk/msm/core.c

Quoted 1 line

drivers/clk/msm/core.c new file +1 -0

+	{ .compatible = "qcom,p2-mn16-clock", .data = 

dispatch_rcg_p2mn16_clk

Quoted 4 lines

}, + { .compatible = "qcom,mn8-dyn-clock", .data =
dispatch_rcg_mn8_dyn_clk }, + { .compatible = "qcom,p4-dyn-clock",
.data = dispatch_rcg_p4_dyn_clk }, + { .compatible =
"qcom,h5-mn8-clock", .data = dispatch_rcg_h5mn8_clk }, + {

.compatible

Quoted 4 lines

= "qcom,h5-mn16-clock", .data = dispatch_rcg_h5mn16_clk }, + {
.compatible = "qcom,cxc-clock", .data = dispatch_branch_clk }, + {
.compatible = "qcom,cxc-hg-clock", .data = dispatch_branch_hg_clk }, +};

You can avoid all the code above by using OF_CLK_DECLARE() and of_clk_init().

drivers/clk/msm/core.c line 255

Quoted 6 lines

drivers/clk/msm/core.c new file +6 -0

+
+static int __init msm_cc_init(void)
+{
+	return platform_driver_register(&msm_cc_driver);
+}
+core_initcall(msm_cc_init);

I might be saying something stupid, but is it okay for a removable module to have a core_initcall?

Fill in the data and wire up the global clock controller to the MSM clock driver. This should allow most non-multimedia device drivers to control their clocks on 8960 based platforms.

Cc devicetree at vger.kernel.org

Signed-off-by Stephen Boyd <[email protected]>

---
 .../devicetree/bindings/clock/qcom,gcc.txt         |  55 +++++++
 drivers/clk/msm/Kconfig                            |  10 ++
 drivers/clk/msm/Makefile                           |   2 +
 drivers/clk/msm/core.c                             |   3 +
 drivers/clk/msm/gcc-8960.c                         | 174 +++++++++++++++++++++
 drivers/clk/msm/internal.h                         |   2 +
 6 files changed, 246 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc.txt
 create mode 100644 drivers/clk/msm/gcc-8960.c

Documentation/devicetree/bindings/clock/qcom,gcc.txt new file +45 -0

new file mode 100644
index 0000000..2311e1a
@@ -0,0 +1,55 @@
+MSM Global Clock Controller Binding
+-----------------------------------
+
+Required properties :
+- compatible : shall contain at least "qcom,gcc" and only one of the
+              following:
+
+                       "qcom,gcc-8660"
+                       "qcom,gcc-8960"
+
+- reg : shall contain base register location and length
+- clocks : shall contain clocks supplied by the clock controller
+
+Example:
+       clock-controller at 900000 {
+               compatible = "qcom,gcc-8960", "qcom,gcc";
+               reg = <0x900000 0x4000>;
+
+               clocks {
+                       pxo: pxo {
+                               #clock-cells = <0>;
+                               compatible = "fixed-clock";
+                               clock-frequency = <27000000>;
+                       };
+
+                       pll8: pll8 {
+                               #clock-cells = <0>;
+                               compatible = "qcom,pll";
+                               clocks = <&pxo>;
+                       };
+
+                       vpll8: vpll8 {
+                               #clock-cells = <0>;
+                               compatible = "qcom,pll-vote";
+                               clocks = <&pll8>;
+                       };
+
+                       gsbi5_uart_rcg: gsbi5_uart_rcg {
+                               #clock-cells = <0>;
+                               compatible = "qcom,p2-mn16-clock";
+                               clocks = <&pxo>, <&vpll8>;
+                       };
+
+                       gsbi5_uart_clk: gsbi5_uart_cxc {
+                               #clock-cells = <0>;

drivers/clk/msm/Kconfig +10 -0

index bf7e3d2..3eaffb6 100644
@@ -2,3 +2,13 @@ menuconfig COMMON_CLK_MSM
        tristate "Support for Qualcomm's MSM designs"
        depends on OF
 
+if COMMON_CLK_MSM
+
+config MSM_GCC_8960
+       bool "MSM8960 Global Clock Controller"
+       help
+         Support for the global clock controller on msm8960 devices.
+         Say Y if you want to use peripheral devices such as UART, SPI,
+         i2c, USB, SD/eMMC, SATA, PCIe, etc.
+
+endif

drivers/clk/msm/Makefile +2 -0

index 9cfd0d7..c785943 100644
@@ -6,3 +6,5 @@ clk-msm-$(CONFIG_COMMON_CLK_MSM) += clk-rcg2.o
 clk-msm-$(CONFIG_COMMON_CLK_MSM) += clk-branch.o
 
 clk-msm-$(CONFIG_COMMON_CLK_MSM) += core.o
+
+clk-msm-$(CONFIG_MSM_GCC_8960) += gcc-8960.o

drivers/clk/msm/core.c +3 -0

index b1904c0..b8e702b 100644
@@ -173,6 +173,9 @@ typedef struct clk *
                struct cc_data *cc);
 
 static const struct of_device_id msm_cc_match_table[] = {
+#ifdef CONFIG_MSM_GCC_8960
+       { .compatible = "qcom,gcc-8960", .data = &msm_gcc_8960_matches },
+#endif
        { }
 };
 MODULE_DEVICE_TABLE(of, msm_cc_match_table);

drivers/clk/msm/gcc-8960.c new file +174 -0

new file mode 100644
index 0000000..b57d2dd
@@ -0,0 +1,174 @@
+/*
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+
+#include "internal.h"
+#include "clk-pll.h"
+#include "clk-rcg.h"
+#include "clk-branch.h"
+
+static struct pll_desc pll8_desc = {
+       .l_reg = 0x3144,
+       .m_reg = 0x3148,
+       .n_reg = 0x314c,
+       .config_reg = 0x3154,
+       .mode_reg = 0x3140,
+       .status_reg = 0x3158,
+       .status_bit = 16,
+};
+
+static struct pll_vote_desc pll8_vote_desc = {
+       .vote_reg = 0x34c0,
+       .vote_bit = 8,
+};
+
+#define PXO    0
+#define PLL8   1
+
+static u8 gcc_pxo_pll8_map[] = {
+       [PXO]   = 0,
+       [PLL8]  = 3,
+};
+
+static struct freq_tbl clk_tbl_gsbi_uart[] = {
+       {  1843200, PLL8, 2,  6, 625 },
+       {  3686400, PLL8, 2, 12, 625 },
+       {  7372800, PLL8, 2, 24, 625 },
+       { 14745600, PLL8, 2, 48, 625 },
+       { 16000000, PLL8, 4,  1,   6 },
+       { 24000000, PLL8, 4,  1,   4 },
+       { 32000000, PLL8, 4,  1,   3 },
+       { 40000000, PLL8, 1,  5,  48 },
+       { 46400000, PLL8, 1, 29, 240 },
+       { 48000000, PLL8, 4,  1,   2 },
+       { 51200000, PLL8, 1,  2,  15 },
+       { 56000000, PLL8, 1,  7,  48 },
+       { 58982400, PLL8, 1, 96, 625 },
+       { 64000000, PLL8, 2,  1,   3 },
+       { }
+};
+
+static struct rcg_desc gsbi5_uart_rcg = {
+       .ctl_reg = 0x2a54,
+       .ns_reg = 0x2a54,
+       .md_reg = 0x2a50,
+       .ctl_bit = 11,
+       .mnctr_en_bit = 8,
+       .mnctr_reset_bit = 7,
+       .mnctr_mode_shift = 5,
+       .pre_div_shift = 3,
+       .src_sel_shift = 0,
+       .n_val_shift = 16,
+       .m_val_shift = 16,
+       .parent_map = gcc_pxo_pll8_map,
+       .freq_tbl = clk_tbl_gsbi_uart,
+};
+
+static struct branch_desc gsbi5_uart_cxc = {
+       .ctl_reg = 0x2a54,
+       .halt_reg = 0x2fd0,
+       .ctl_bit = 9,
+       .halt_bit = 22,
+       .halt_check = BRANCH_HALT,
+};
+
+static struct rcg_desc gsbi6_uart_rcg = {
+       .ctl_reg = 0x2a74,
+       .ns_reg = 0x2a74,
+       .md_reg = 0x2a70,
+       .ctl_bit = 11,
+       .mnctr_en_bit = 8,
+       .mnctr_reset_bit = 7,
+       .mnctr_mode_shift = 5,
+       .pre_div_shift = 3,
+       .src_sel_shift = 0,
+       .n_val_shift = 16,
+       .m_val_shift = 16,
+       .parent_map = gcc_pxo_pll8_map,
+       .freq_tbl = clk_tbl_gsbi_uart,
+};
+
+static struct branch_desc gsbi6_uart_cxc = {
+       .ctl_reg = 0x2a74,
+       .halt_reg = 0x2fd0,
+       .ctl_bit = 9,
+       .halt_bit = 18,
+       .halt_check = BRANCH_HALT,
+};
+
+static struct branch_desc gsbi5_uart_ahb = {
+       .ctl_reg = 0x2a40,
+       .halt_reg = 0x2fd0,
+       .hwcg_reg = 0x2a40,
+       .ctl_bit = 4,
+       .hwcg_bit = 6,
+       .halt_bit = 23,
+       .halt_check = BRANCH_HALT,
+};
+
+static struct freq_tbl clk_tbl_gsbi_qup[] = {
+       {  1100000, PXO,  1, 2, 49 },
+       {  5400000, PXO,  1, 1,  5 },
+       { 10800000, PXO,  1, 2,  5 },
+       { 15060000, PLL8, 1, 2, 51 },
+       { 24000000, PLL8, 4, 1,  4 },
+       { 25600000, PLL8, 1, 1, 15 },
+       { 27000000, PXO,  1, 0,  0 },
+       { 48000000, PLL8, 4, 1,  2 },
+       { 51200000, PLL8, 1, 2, 15 },
+       { }
+};
+
+static struct rcg_desc gsbi5_qup_rcg = {
+       .ctl_reg = 0x2a4c,
+       .ns_reg = 0x2a4c,
+       .md_reg = 0x2a48,
+       .ctl_bit = 11,
+       .mnctr_en_bit = 8,
+       .mnctr_reset_bit = 7,
+       .mnctr_mode_shift = 5,
+       .pre_div_shift = 3,
+       .src_sel_shift = 0,
+       .n_val_shift = 16,
+       .m_val_shift = 16,
+       .parent_map = gcc_pxo_pll8_map,
+       .freq_tbl = clk_tbl_gsbi_qup,
+};
+
+static struct branch_desc gsbi5_qup_cxc = {
+       .ctl_reg = 0x2a4c,
+       .halt_reg = 0x2fd0,
+       .ctl_bit = 9,
+       .halt_bit = 20,
+       .halt_check = BRANCH_HALT,
+};
+
+static struct of_clk_match msm_gcc_clk_match[] = {
+       { .name = "cxo" },
+       { .name = "pxo" },
+       { .name = "pll8", .driver_data = &pll8_desc },
+       { .name = "vpll8", .driver_data = &pll8_vote_desc },
+       { .name = "gsbi5_uart_rcg", .driver_data = &gsbi5_uart_rcg },
+       { .name = "gsbi5_uart_cxc", .driver_data = &gsbi5_uart_cxc },
+       { .name = "gsbi6_uart_rcg", .driver_data = &gsbi6_uart_rcg },
+       { .name = "gsbi6_uart_cxc", .driver_data = &gsbi6_uart_cxc },
+       { .name = "gsbi5_uart_ahb", .driver_data = &gsbi5_uart_ahb },
+       { .name = "gsbi5_qup_rcg", .driver_data = &gsbi5_qup_rcg },
+       { .name = "gsbi5_qup_cxc", .driver_data = &gsbi5_qup_cxc },
+};
+
+const struct msm_clk_match msm_gcc_8960_matches = {
+       .matches = msm_gcc_clk_match,
+       .size = ARRAY_SIZE(msm_gcc_clk_match)
+};

drivers/clk/msm/internal.h +2 -1

index 177bd3b..b0ffda7 100644
@@ -21,4 +21,6 @@ struct msm_clk_match {
        size_t size;
 };
 
+extern const struct msm_clk_match msm_gcc_8960_matches;
+
 #endif
--

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Hi Stephen,

Quoted 73 lines · Stephen Boyd

Fill in the data and wire up the global clock controller to the MSM clock driver. This should allow most non-multimedia device drivers to control their clocks on 8960 based platforms.

Cc devicetree at vger.kernel.org

Signed-off-by Stephen Boyd <[email protected]>

---
 .../devicetree/bindings/clock/qcom,gcc.txt         |  55 +++++++
 drivers/clk/msm/Kconfig                            |  10 ++
 drivers/clk/msm/Makefile                           |   2 +
 drivers/clk/msm/core.c                             |   3 +
 drivers/clk/msm/gcc-8960.c                         | 174 +++++++++++++++++++++
 drivers/clk/msm/internal.h                         |   2 +
 6 files changed, 246 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc.txt
 create mode 100644 drivers/clk/msm/gcc-8960.c

Documentation/devicetree/bindings/clock/qcom,gcc.txt new file +50 -0

new file mode 100644
index 0000000..2311e1a
@@ -0,0 +1,55 @@
+MSM Global Clock Controller Binding
+-----------------------------------
+
+Required properties :
+- compatible : shall contain at least "qcom,gcc" and only one of the
+              following:
+
+                       "qcom,gcc-8660"
+                       "qcom,gcc-8960"
+
+- reg : shall contain base register location and length
+- clocks : shall contain clocks supplied by the clock controller
+
+Example:
+       clock-controller at 900000 {
+               compatible = "qcom,gcc-8960", "qcom,gcc";
+               reg = <0x900000 0x4000>;
+
+               clocks {
+                       pxo: pxo {
+                               #clock-cells = <0>;
+                               compatible = "fixed-clock";
+                               clock-frequency = <27000000>;
+                       };
+
+                       pll8: pll8 {
+                               #clock-cells = <0>;
+                               compatible = "qcom,pll";
+                               clocks = <&pxo>;
+                       };
+
+                       vpll8: vpll8 {
+                               #clock-cells = <0>;
+                               compatible = "qcom,pll-vote";
+                               clocks = <&pll8>;
+                       };
+
+                       gsbi5_uart_rcg: gsbi5_uart_rcg {
+                               #clock-cells = <0>;
+                               compatible = "qcom,p2-mn16-clock";
+                               clocks = <&pxo>, <&vpll8>;
+                       };
+
+                       gsbi5_uart_clk: gsbi5_uart_cxc {
+                               #clock-cells = <0>;
+                               compatible = "qcom,cxc-clock";
+                               clocks = <&gsbi5_uart_rcg>;
+                       };
+
+                       gsbi5_uart_ahb: gsbi5_uart_ahb {

Documentation/devicetree/bindings/clock/qcom,gcc.txt line 55

Quoted 5 lines

Documentation/devicetree/bindings/clock/qcom,gcc.txt new file +5 -0

+                               #clock-cells = <0>;
+                               compatible = "qcom,cxc-hg-clock";
+                       };
+               };
+       };

I'm slightly confused by this. How is each of the clocks described in the clocks node related to a portion of the register set?

If the set of clocks is fixed, surely the gcc node gives you enough information alone, and the whole block can be modelled as a single provider of multiple clock outputs, or it's not fixed, and some linkage needs to be defined?

The code seems to imply the former, unless only a subset of clocks may be present? In that case, the set of clocks which might be present should be described in the binding.

Quoted 76 lines

Hi Stephen,

Quoted 73 lines · Stephen Boyd

Fill in the data and wire up the global clock controller to the MSM clock driver. This should allow most non-multimedia device drivers to control their clocks on 8960 based platforms.

Cc devicetree at vger.kernel.org

Signed-off-by Stephen Boyd <[email protected]>

---
 .../devicetree/bindings/clock/qcom,gcc.txt         |  55 +++++++
 drivers/clk/msm/Kconfig                            |  10 ++
 drivers/clk/msm/Makefile                           |   2 +
 drivers/clk/msm/core.c                             |   3 +
 drivers/clk/msm/gcc-8960.c                         | 174 +++++++++++++++++++++
 drivers/clk/msm/internal.h                         |   2 +
 6 files changed, 246 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc.txt
 create mode 100644 drivers/clk/msm/gcc-8960.c

Documentation/devicetree/bindings/clock/qcom,gcc.txt new file +50 -0

new file mode 100644
index 0000000..2311e1a
@@ -0,0 +1,55 @@
+MSM Global Clock Controller Binding
+-----------------------------------
+
+Required properties :
+- compatible : shall contain at least "qcom,gcc" and only one of the
+              following:
+
+                       "qcom,gcc-8660"
+                       "qcom,gcc-8960"
+
+- reg : shall contain base register location and length
+- clocks : shall contain clocks supplied by the clock controller
+
+Example:
+       clock-controller at 900000 {
+               compatible = "qcom,gcc-8960", "qcom,gcc";
+               reg = <0x900000 0x4000>;
+
+               clocks {
+                       pxo: pxo {
+                               #clock-cells = <0>;
+                               compatible = "fixed-clock";
+                               clock-frequency = <27000000>;
+                       };
+
+                       pll8: pll8 {
+                               #clock-cells = <0>;
+                               compatible = "qcom,pll";
+                               clocks = <&pxo>;
+                       };
+
+                       vpll8: vpll8 {
+                               #clock-cells = <0>;
+                               compatible = "qcom,pll-vote";
+                               clocks = <&pll8>;
+                       };
+
+                       gsbi5_uart_rcg: gsbi5_uart_rcg {
+                               #clock-cells = <0>;
+                               compatible = "qcom,p2-mn16-clock";
+                               clocks = <&pxo>, <&vpll8>;
+                       };
+
+                       gsbi5_uart_clk: gsbi5_uart_cxc {
+                               #clock-cells = <0>;
+                               compatible = "qcom,cxc-clock";
+                               clocks = <&gsbi5_uart_rcg>;
+                       };
+
+                       gsbi5_uart_ahb: gsbi5_uart_ahb {

Quoted 8 lines

Documentation/devicetree/bindings/clock/qcom,gcc.txt line 55

Quoted 5 lines

Documentation/devicetree/bindings/clock/qcom,gcc.txt new file +5 -0

+                               #clock-cells = <0>;
+                               compatible = "qcom,cxc-hg-clock";
+                       };
+               };
+       };

I'm slightly confused by this. How is each of the clocks described in the clocks node related to a portion of the register set?

The registers to control clocks and determine their state are scattered throughout the registers in the gcc (in this example from 0x900000 to 0x903fff). If you match up gsbi5_uart_rcg with its C struct counterpart you'll notice that there are multiple registers used to configure the clock. It isn't as simple as one reg property per clock even for the case where we're just toggling a bit to turn a clock on and off either. And it isn't as simple as saying the clock has a base register that we can offset from because offsets are almost always different (we've tried to correct this in future chip versions).

Quoted 9 lines

If the set of clocks is fixed, surely the gcc node gives you enough information alone, and the whole block can be modelled as a single provider of multiple clock outputs, or it's not fixed, and some linkage needs to be defined?

The code seems to imply the former, unless only a subset of clocks may be present? In that case, the set of clocks which might be present should be described in the binding.

The clock controller is hardware and the number of clock outputs is fixed. Isn't all hardware fixed until you start talking about FPGAs? The next minor revision of the clock controller may add more clocks or remove clocks from that base design, but otherwise the two are 90% the same and generally software compatible. It isn't until we start a new generation of chips that we make major changes to the design. Is that loose enough to qualify?

These bindings attempt to follow the regulator bindings. With regulators there is a node for each regulator and we describe physical characteristics of those regulators within the nodes but we don't describe the software interface (bits, masks, shifts, etc). I imagine we could extend these clock nodes to describe physical characteristics such as min/max frequency or if the bootloader has left the clocks on. Right now we're using the nodes to describe what types of clocks there are and how the clock tree is layed out.

Or perhaps you're talking about clock sharing? We share the clock controller with multiple masters (processors running other OSes) and the partitioning of the clocks is mostly predefined. We just won't use some clocks because they're reserved for other processors. They're still part of the same clock controller hardware block but we don't want to control them on Linux because we'll trample over other processors and most likely hang the system. I wonder how this would work for hexagon and krait both running linux on the same SoC. If all DT says is that there is a gcc here at this address how are we supposed to know that we shouldn't use some clock?

In fact we have some clocks that are
"voteable" in the sense that each master has its own register to vote for a clock to be on or off. The registers are all ORed together by hardware to determine if the clock should be on or not. I should probably rename those clocks to have a _krait or _apps at the end so that it's clear we want to instantiate the krait version of the clock and not the hexagon version. I suppose the other solution there is to say we have gcc-8960-krait and gcc-8960-hexagon so we know which voting registers to use or put an ifdef ARCH_HEXAGON/ARCH_ARM. Is that the right solution?

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Quoting Stephen Boyd (2013-08-12 22:03:34)

Quoted 18 lines

The clock controller is hardware and the number of clock outputs is fixed. Isn't all hardware fixed until you start talking about FPGAs? The next minor revision of the clock controller may add more clocks or remove clocks from that base design, but otherwise the two are 90% the same and generally software compatible. It isn't until we start a new generation of chips that we make major changes to the design. Is that loose enough to qualify?

These bindings attempt to follow the regulator bindings. With regulators there is a node for each regulator and we describe physical characteristics of those regulators within the nodes but we don't describe the software interface (bits, masks, shifts, etc). I imagine we could extend these clock nodes to describe physical characteristics such as min/max frequency or if the bootloader has left the clocks on. Right now we're using the nodes to describe what types of clocks there are and how the clock tree is layed out.

Quoted 11 lines

Or perhaps you're talking about clock sharing? We share the clock controller with multiple masters (processors running other OSes) and the partitioning of the clocks is mostly predefined. We just won't use some clocks because they're reserved for other processors. They're still part of the same clock controller hardware block but we don't want to control them on Linux because we'll trample over other processors and most likely hang the system. I wonder how this would work for hexagon and krait both running linux on the same SoC. If all DT says is that there is a gcc here at this address how are we supposed to know that we shouldn't use some clock?

Do Krait and Hexagon have the same register map? On the ARM SoCs I am familiar with the masters have differing views of register addresses for the same peripherals and hardware blocks. So you couldn't use the same DTS in a straightforward way if this is true for your system.

Fill in the data and wire up the multimedia clock controller to the MSM clock driver. This should allow multimedia device drivers to control their clocks on 8960 based platforms.

Cc devicetree at vger.kernel.org

Signed-off-by Stephen Boyd <[email protected]>

---
 .../devicetree/bindings/clock/qcom,mmcc.txt        |  38 ++++++
 drivers/clk/msm/Kconfig                            |   8 ++
 drivers/clk/msm/Makefile                           |   1 +
 drivers/clk/msm/core.c                             |   3 +
 drivers/clk/msm/internal.h                         |   1 +
 drivers/clk/msm/mmcc-8960.c                        | 142 +++++++++++++++++++++
 6 files changed, 193 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,mmcc.txt
 create mode 100644 drivers/clk/msm/mmcc-8960.c

Documentation/devicetree/bindings/clock/qcom,mmcc.txt new file +28 -0

new file mode 100644
index 0000000..e06577e
@@ -0,0 +1,38 @@
+MSM Multimedia Clock Controller Binding
+-----------------------------------------
+
+Required properties :
+- compatible : shall contain at least "qcom,mmcc" and only one of the
+	       following:
+
+			"qcom,mmcc-8660"
+			"qcom,mmcc-8960"
+
+- reg : shall contain base register location and length
+- clocks : shall contain clocks supplied by the clock controller
+
+Example:
+	clock-controller at 4000000 {
+		compatible = "qcom,mmcc-8960", "qcom,mmcc";
+		reg = <0x4000000 0x1000>;
+
+		clocks {
+			pll2: pll2 {
+				#clock-cells = <0>;
+				compatible = "qcom,pll";
+				clocks = <&pxo>;
+			};
+
+			mdp_rcg: mdp_rcg {
+				#clock-cells = <0>;
+				compatible = "qcom,mn8-dyn-clock";

drivers/clk/msm/Kconfig +8 -0

index 3eaffb6..6147380 100644
@@ -11,4 +11,12 @@ config MSM_GCC_8960
 	  Say Y if you want to use peripheral devices such as UART, SPI,
 	  i2c, USB, SD/eMMC, SATA, PCIe, etc.
 
+config MSM_MMCC_8960
+	bool "MSM8960 Multimedia Clock Controller"
+	select MSM_GCC_8960
+	help
+	  Support for the multimedia clock controller on msm8960 devices.
+	  Say Y if you want to support multimedia devices such as display,
+	  graphics, video encode/decode, camera, etc.
+
 endif

drivers/clk/msm/Makefile +1 -0

index c785943..ae199f5 100644
@@ -8,3 +8,4 @@ clk-msm-$(CONFIG_COMMON_CLK_MSM) += clk-branch.o
 clk-msm-$(CONFIG_COMMON_CLK_MSM) += core.o
 
 clk-msm-$(CONFIG_MSM_GCC_8960) += gcc-8960.o
+clk-msm-$(CONFIG_MSM_MMCC_8960) += mmcc-8960.o

drivers/clk/msm/core.c +3 -0

index b8e702b..4e8b8d0 100644
@@ -176,6 +176,9 @@ static const struct of_device_id msm_cc_match_table[] = {
 #ifdef CONFIG_MSM_GCC_8960
 	{ .compatible = "qcom,gcc-8960", .data = &msm_gcc_8960_matches },
 #endif
+#ifdef CONFIG_MSM_MMCC_8960
+	{ .compatible = "qcom,mmcc-8960", .data = &msm_mmcc_8960_matches },
+#endif
 	{ }
 };
 MODULE_DEVICE_TABLE(of, msm_cc_match_table);

drivers/clk/msm/internal.h +1 -0

index b0ffda7..45435a8 100644
@@ -22,5 +22,6 @@ struct msm_clk_match {
 };
 
 extern const struct msm_clk_match msm_gcc_8960_matches;
+extern const struct msm_clk_match msm_mmcc_8960_matches;
 
 #endif

drivers/clk/msm/mmcc-8960.c new file +142 -0

new file mode 100644
index 0000000..e7b7867
@@ -0,0 +1,142 @@
+/*
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+
+#include "internal.h"
+#include "clk-pll.h"
+#include "clk-rcg.h"
+#include "clk-branch.h"
+
+static struct pll_desc pll2_desc = {
+	.l_reg = 0x320,
+	.m_reg = 0x324,
+	.n_reg = 0x328,
+	.config_reg = 0x32c,
+	.mode_reg = 0x31c,
+	.status_reg = 0x334,
+	.status_bit = 16,
+};
+
+#define PXO	0
+#define PLL2	1
+#define PLL8	2
+
+static u8 mmcc_pxo_pll2_pll8_map[] = {
+	[PXO]	= 0,
+	[PLL2]	= 1,
+	[PLL8]	= 2,
+};
+
+static struct freq_tbl clk_tbl_mdp[] = {
+	{   9600000, PLL8, 0, 1, 40 },
+	{  13710000, PLL8, 0, 1, 28 },
+	{  27000000, PXO,  0, 0,  0 },
+	{  29540000, PLL8, 0, 1, 13 },
+	{  34910000, PLL8, 0, 1, 11 },
+	{  38400000, PLL8, 0, 1, 10 },
+	{  59080000, PLL8, 0, 2, 13 },
+	{  76800000, PLL8, 0, 1,  5 },
+	{  85330000, PLL8, 0, 2,  9 },
+	{  96000000, PLL8, 0, 1,  4 },
+	{ 128000000, PLL8, 0, 1,  3 },
+	{ 160000000, PLL2, 0, 1,  5 },
+	{ 177780000, PLL2, 0, 2,  9 },
+	{ 200000000, PLL2, 0, 1,  4 },
+	{ 228571000, PLL2, 0, 2,  7 },
+	{ 266667000, PLL2, 0, 1,  3 },
+	{ }
+};
+
+static struct rcg_dyn_desc mdp_rcg = {
+	.ctl_reg = 0xc0,
+	.ns_reg = 0xd0,
+	.md0_reg = 0xc4,
+	.md1_reg = 0xc8,
+	.ctl_bit = 2,
+	.mnctr0_en_bit = 8,
+	.mnctr1_en_bit = 5,
+	.mnctr0_reset_bit = 31,
+	.mnctr1_reset_bit = 30,
+	.mnctr0_mode_shift = 9,
+	.mnctr1_mode_shift = 6,
+	.src0_sel_shift = 3,
+	.src1_sel_shift = 0,
+	.n0_val_shift = 22,
+	.n1_val_shift = 14,
+	.m0_val_shift = 8,
+	.m1_val_shift = 8,
+	.mux_sel_bit = 11,
+	.parent_map = mmcc_pxo_pll2_pll8_map,
+	.freq_tbl = clk_tbl_mdp,
+};
+
+static struct branch_desc mdp_cxc = {
+	.ctl_reg = 0xc0,
+	.halt_reg = 0x1d0,
+	.ctl_bit = 0,
+	.halt_bit = 10,
+	.halt_check = BRANCH_HALT,
+};
+
+static struct freq_tbl clk_tbl_rot[] = {
+	{  27000000, PXO,   1 },
+	{  29540000, PLL8, 13 },
+	{  32000000, PLL8, 12 },
+	{  38400000, PLL8, 10 },
+	{  48000000, PLL8,  8 },
+	{  54860000, PLL8,  7 },
+	{  64000000, PLL8,  6 },
+	{  76800000, PLL8,  5 },
+	{  96000000, PLL8,  4 },
+	{ 100000000, PLL2,  8 },
+	{ 114290000, PLL2,  7 },
+	{ 133330000, PLL2,  6 },
+	{ 160000000, PLL2,  5 },
+	{ 200000000, PLL2,  4 },
+	{ }
+};
+
+static struct rcg_dyn_desc rot_rcg = {
+	.ctl_reg = 0xe0,
+	.ns_reg = 0xe8,
+	.ctl_bit = 2,
+	.pre_div0_shift = 22,
+	.pre_div1_shift = 26,
+	.src0_sel_shift = 16,
+	.src1_sel_shift = 19,
+	.mux_sel_bit = 30,
+	.parent_map = mmcc_pxo_pll2_pll8_map,
+	.freq_tbl = clk_tbl_rot,
+};
+
+static struct branch_desc rot_cxc = {
+	.ctl_reg = 0xe0,
+	.halt_reg = 0x1d0,
+	.ctl_bit = 0,
+	.halt_bit = 15,
+	.halt_check = BRANCH_HALT,
+};
+
+static struct of_clk_match msm_mmcc_clk_match[] = {
+	{ .name = "pll2", .driver_data = &pll2_desc },
+	{ .name = "mdp_rcg", .driver_data = &mdp_rcg },
+	{ .name = "mdp_cxc", .driver_data = &mdp_cxc },
+	{ .name = "rot_rcg", .driver_data = &rot_rcg },
+	{ .name = "rot_cxc", .driver_data = &rot_cxc },
+};
+
+const struct msm_clk_match msm_mmcc_8960_matches = {
+	.matches = msm_mmcc_clk_match,
+	.size = ARRAY_SIZE(msm_mmcc_clk_match)
+};

Signature · 8 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel