[PATCH v2 1/2] clk: qcom: common: Add API to register board clocks backwards compatibly

[PATCH v2 1/2] clk: qcom: common: Add API to register board clocks backwards compatibly

[Stephen Boyd]

We want to put the XO board clocks into the dt files, but we also
need to be backwards compatible with an older dtb. Add an API to
the common code to do this. This also makes a place for us to
handle the case when the RPM clock driver is enabled and we don't
want to register the fixed factor clock.

Cc: Georgi Djakov <georgi.djakov@linaro.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

Changes from v1:
 * New sleep_clk specific API
 * Added in the RPM enabled config check
 * Always register the board clk if it's missing

 drivers/clk/qcom/common.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/qcom/common.h |  4 +++
 2 files changed, 80 insertions(+)

diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index 8fa477293ae0..09b3606e12be 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -88,6 +88,82 @@ static void qcom_cc_gdsc_unregister(void *data)
 	gdsc_unregister(data);
 }
 
+/*
+ * Backwards compatibility with old DTs. Register a pass-through factor 1/1
+ * clock to translate 'path' clk into 'name' clk and regsiter the 'path'
+ * clk as a fixed rate clock if it isn't present.
+ */
+static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
+				       const char *name, unsigned long rate,
+				       bool add_factor)
+{
+	struct device_node *node = NULL;
+	struct device_node *clocks_node;
+	struct clk_fixed_factor *factor;
+	struct clk_fixed_rate *fixed;
+	struct clk *clk;
+	struct clk_init_data init_data = { };
+
+	clocks_node = of_find_node_by_path("/clocks");
+	if (clocks_node)
+		node = of_find_node_by_name(clocks_node, path);
+	of_node_put(clocks_node);
+
+	if (!node) {
+		fixed = devm_kzalloc(dev, sizeof(*fixed), GFP_KERNEL);
+		if (!fixed)
+			return -EINVAL;
+
+		fixed->fixed_rate = rate;
+		fixed->hw.init = &init_data;
+
+		init_data.name = path;
+		init_data.flags = CLK_IS_ROOT;
+		init_data.ops = &clk_fixed_rate_ops;
+
+		clk = devm_clk_register(dev, &fixed->hw);
+		if (IS_ERR(clk))
+			return PTR_ERR(clk);
+	}
+	of_node_put(node);
+
+	if (add_factor) {
+		factor = devm_kzalloc(dev, sizeof(*factor), GFP_KERNEL);
+		if (!factor)
+			return -EINVAL;
+
+		factor->mult = factor->div = 1;
+		factor->hw.init = &init_data;
+
+		init_data.name = name;
+		init_data.parent_names = &path;
+		init_data.num_parents = 1;
+		init_data.flags = 0;
+		init_data.ops = &clk_fixed_factor_ops;
+
+		clk = devm_clk_register(dev, &factor->hw);
+		if (IS_ERR(clk))
+			return PTR_ERR(clk);
+	}
+
+	return 0;
+}
+
+int qcom_cc_register_board_clk(struct device *dev, const char *path,
+			       const char *name, unsigned long rate)
+{
+	return _qcom_cc_register_board_clk(dev, path, name, rate,
+					   !IS_ENABLED(CONFIG_QCOM_RPMCC));
+}
+EXPORT_SYMBOL_GPL(qcom_cc_register_board_clk);
+
+int qcom_cc_register_sleep_clk(struct device *dev)
+{
+	return _qcom_cc_register_board_clk(dev, "sleep_clk", "sleep_clk_src",
+					   32768, true);
+}
+EXPORT_SYMBOL_GPL(qcom_cc_register_sleep_clk);
+
 int qcom_cc_really_probe(struct platform_device *pdev,
 			 const struct qcom_cc_desc *desc, struct regmap *regmap)
 {
diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
index 7c1fba3ebc03..ae9bdeb21f29 100644
--- a/drivers/clk/qcom/common.h
+++ b/drivers/clk/qcom/common.h
@@ -37,6 +37,10 @@ extern const struct freq_tbl *qcom_find_freq(const struct freq_tbl *f,
 extern int qcom_find_src_index(struct clk_hw *hw, const struct parent_map *map,
 			       u8 src);
 
+extern int qcom_cc_register_board_clk(struct device *dev, const char *path,
+				      const char *name, unsigned long rate);
+extern int qcom_cc_register_sleep_clk(struct device *dev);
+
 extern struct regmap *qcom_cc_map(struct platform_device *pdev,
 				  const struct qcom_cc_desc *desc);
 extern int qcom_cc_really_probe(struct platform_device *pdev,
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 2/2] clk: qcom: Move cxo/pxo/xo into dt files

[Stephen Boyd]

Put these clocks into the dt files instead of registering them
from C code. This provides a few benefits. It allows us to
specify the frequency of these clocks at the board level instead
of hard-coding them in the driver. It allows us to insert an RPM
clock in between the consumers of the crystals and the actual
clock. And finally, it helps us transition the GCC driver to use
RPM clocks when that configuration is enabled.

Cc: Georgi Djakov <georgi.djakov@linaro.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

Changes from v1:
 * Convert all armv7 platforms instead of just msm8960

 drivers/clk/qcom/gcc-apq8084.c | 16 +++++++---------
 drivers/clk/qcom/gcc-ipq806x.c | 14 ++++++--------
 drivers/clk/qcom/gcc-msm8660.c | 15 +++++++--------
 drivers/clk/qcom/gcc-msm8960.c | 14 ++++++--------
 drivers/clk/qcom/gcc-msm8974.c | 17 +++++++----------
 5 files changed, 33 insertions(+), 43 deletions(-)

diff --git a/drivers/clk/qcom/gcc-apq8084.c b/drivers/clk/qcom/gcc-apq8084.c
index 1567c3a79534..070037a29ea5 100644
--- a/drivers/clk/qcom/gcc-apq8084.c
+++ b/drivers/clk/qcom/gcc-apq8084.c
@@ -3607,18 +3607,16 @@ MODULE_DEVICE_TABLE(of, gcc_apq8084_match_table);
 
 static int gcc_apq8084_probe(struct platform_device *pdev)
 {
-	struct clk *clk;
+	int ret;
 	struct device *dev = &pdev->dev;
 
-	/* Temporary until RPM clocks supported */
-	clk = clk_register_fixed_rate(dev, "xo", NULL, CLK_IS_ROOT, 19200000);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	ret = qcom_cc_register_board_clk(dev, "xo_board", "xo", 19200000);
+	if (ret)
+		return ret;
 
-	clk = clk_register_fixed_rate(dev, "sleep_clk_src", NULL,
-				      CLK_IS_ROOT, 32768);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	ret = qcom_cc_register_sleep_clk(dev);
+	if (ret)
+		return ret;
 
 	return qcom_cc_probe(pdev, &gcc_apq8084_desc);
 }
diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
index 16fc64c082a5..dd5402bac620 100644
--- a/drivers/clk/qcom/gcc-ipq806x.c
+++ b/drivers/clk/qcom/gcc-ipq806x.c
@@ -3023,19 +3023,17 @@ MODULE_DEVICE_TABLE(of, gcc_ipq806x_match_table);
 
 static int gcc_ipq806x_probe(struct platform_device *pdev)
 {
-	struct clk *clk;
 	struct device *dev = &pdev->dev;
 	struct regmap *regmap;
 	int ret;
 
-	/* Temporary until RPM clocks supported */
-	clk = clk_register_fixed_rate(dev, "cxo", NULL, CLK_IS_ROOT, 25000000);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	ret = qcom_cc_register_board_clk(dev, "cxo_board", "cxo", 19200000);
+	if (ret)
+		return ret;
 
-	clk = clk_register_fixed_rate(dev, "pxo", NULL, CLK_IS_ROOT, 25000000);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	ret = qcom_cc_register_board_clk(dev, "pxo_board", "pxo", 27000000);
+	if (ret)
+		return ret;
 
 	ret = qcom_cc_probe(pdev, &gcc_ipq806x_desc);
 	if (ret)
diff --git a/drivers/clk/qcom/gcc-msm8660.c b/drivers/clk/qcom/gcc-msm8660.c
index f110bb5a1df3..ad413036f7c7 100644
--- a/drivers/clk/qcom/gcc-msm8660.c
+++ b/drivers/clk/qcom/gcc-msm8660.c
@@ -2720,17 +2720,16 @@ MODULE_DEVICE_TABLE(of, gcc_msm8660_match_table);
 
 static int gcc_msm8660_probe(struct platform_device *pdev)
 {
-	struct clk *clk;
+	int ret;
 	struct device *dev = &pdev->dev;
 
-	/* Temporary until RPM clocks supported */
-	clk = clk_register_fixed_rate(dev, "cxo", NULL, CLK_IS_ROOT, 19200000);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	ret = qcom_cc_register_board_clk(dev, "cxo_board", "cxo", 19200000);
+	if (ret)
+		return ret;
 
-	clk = clk_register_fixed_rate(dev, "pxo", NULL, CLK_IS_ROOT, 27000000);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	ret = qcom_cc_register_board_clk(dev, "pxo_board", "pxo", 27000000);
+	if (ret)
+		return ret;
 
 	return qcom_cc_probe(pdev, &gcc_msm8660_desc);
 }
diff --git a/drivers/clk/qcom/gcc-msm8960.c b/drivers/clk/qcom/gcc-msm8960.c
index 66c18bc97857..983dd7dc89a7 100644
--- a/drivers/clk/qcom/gcc-msm8960.c
+++ b/drivers/clk/qcom/gcc-msm8960.c
@@ -3503,7 +3503,6 @@ MODULE_DEVICE_TABLE(of, gcc_msm8960_match_table);
 
 static int gcc_msm8960_probe(struct platform_device *pdev)
 {
-	struct clk *clk;
 	struct device *dev = &pdev->dev;
 	const struct of_device_id *match;
 	struct platform_device *tsens;
@@ -3513,14 +3512,13 @@ static int gcc_msm8960_probe(struct platform_device *pdev)
 	if (!match)
 		return -EINVAL;
 
-	/* Temporary until RPM clocks supported */
-	clk = clk_register_fixed_rate(dev, "cxo", NULL, CLK_IS_ROOT, 19200000);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	ret = qcom_cc_register_board_clk(dev, "cxo_board", "cxo", 19200000);
+	if (ret)
+		return ret;
 
-	clk = clk_register_fixed_rate(dev, "pxo", NULL, CLK_IS_ROOT, 27000000);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	ret = qcom_cc_register_board_clk(dev, "pxo_board", "pxo", 27000000);
+	if (ret)
+		return ret;
 
 	ret = qcom_cc_probe(pdev, match->data);
 	if (ret)
diff --git a/drivers/clk/qcom/gcc-msm8974.c b/drivers/clk/qcom/gcc-msm8974.c
index 28abb8f8f293..335952db309b 100644
--- a/drivers/clk/qcom/gcc-msm8974.c
+++ b/drivers/clk/qcom/gcc-msm8974.c
@@ -2717,7 +2717,7 @@ static void msm8974_pro_clock_override(void)
 
 static int gcc_msm8974_probe(struct platform_device *pdev)
 {
-	struct clk *clk;
+	int ret;
 	struct device *dev = &pdev->dev;
 	bool pro;
 	const struct of_device_id *id;
@@ -2730,16 +2730,13 @@ static int gcc_msm8974_probe(struct platform_device *pdev)
 	if (pro)
 		msm8974_pro_clock_override();
 
-	/* Temporary until RPM clocks supported */
-	clk = clk_register_fixed_rate(dev, "xo", NULL, CLK_IS_ROOT, 19200000);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	ret = qcom_cc_register_board_clk(dev, "xo_board", "xo", 19200000);
+	if (ret)
+		return ret;
 
-	/* Should move to DT node? */
-	clk = clk_register_fixed_rate(dev, "sleep_clk_src", NULL,
-				      CLK_IS_ROOT, 32768);
-	if (IS_ERR(clk))
-		return PTR_ERR(clk);
+	ret = qcom_cc_register_sleep_clk(dev);
+	if (ret)
+		return ret;
 
 	return qcom_cc_probe(pdev, &gcc_msm8974_desc);
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 1/2] clk: qcom: common: Add API to register board clocks backwards compatibly

[Stephen Boyd]

On 10/27, Stephen Boyd wrote:
> +
> +int qcom_cc_register_board_clk(struct device *dev, const char *path,
> +			       const char *name, unsigned long rate)
> +{
> +	return _qcom_cc_register_board_clk(dev, path, name, rate,
> +					   !IS_ENABLED(CONFIG_QCOM_RPMCC));

I just realized that we could have this config enabled but the
driver never probes on a certain platform. So we'll need to do
some sort of search to see if the rpmcc node exists and also
check to see if the driver is enabled. I guess we can do that all
here in this function as long as we have a compatible =
"qcom,rpmcc" as a more generic property for the rpm clock
controller node. So if the config is enabled, search the dt tree
for a qcom,rpmcc node. If the rpmcc node is there, the last
argument is false, otherwise it's true.

---8<----
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index 572851b5bc16..215a41ba0135 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -17,6 +17,7 @@
 #include <linux/platform_device.h>
 #include <linux/clk-provider.h>
 #include <linux/reset-controller.h>
+#include <linux/of.h>
 
 #include "common.h"
 #include "clk-rcg.h"
@@ -152,7 +153,18 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
 int qcom_cc_register_board_clk(struct device *dev, const char *path,
 			       const char *name, unsigned long rate)
 {
-	return _qcom_cc_register_board_clk(dev, path, name, rate, true);
+	bool add_factor = true;
+	struct device_node *node;
+
+	/* The RPM clock driver will add the factor clock if present */
+	if (IS_ENABLED(CONFIG_QCOM_RPMCC)) {
+		node = of_find_compatible_node(NULL, NULL, "qcom,rpmcc");
+		if (node)
+			add_factor = false;
+		of_node_put(node);
+	}
+
+	return _qcom_cc_register_board_clk(dev, path, name, rate, add_factor);
 }
 EXPORT_SYMBOL_GPL(qcom_cc_register_board_clk);
 
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 1/2] clk: qcom: common: Add API to register board clocks backwards compatibly

[Georgi Djakov]

On 10/28/2015 03:06 AM, Stephen Boyd wrote:
> On 10/27, Stephen Boyd wrote:
>> +
>> +int qcom_cc_register_board_clk(struct device *dev, const char *path,
>> +			       const char *name, unsigned long rate)
>> +{
>> +	return _qcom_cc_register_board_clk(dev, path, name, rate,
>> +					   !IS_ENABLED(CONFIG_QCOM_RPMCC));
> 
> I just realized that we could have this config enabled but the
> driver never probes on a certain platform. So we'll need to do
> some sort of search to see if the rpmcc node exists and also
> check to see if the driver is enabled. I guess we can do that all
> here in this function as long as we have a compatible =
> "qcom,rpmcc" as a more generic property for the rpm clock
> controller node. So if the config is enabled, search the dt tree
> for a qcom,rpmcc node. If the rpmcc node is there, the last
> argument is false, otherwise it's true.

I like the approach.  We could have both compatible strings like:
compatible = "qcom,rpmcc-msm8916", "qcom,rpmcc";

The only possible issue i could imagine is the case when rpmcc
driver probe fails - does not enable scaling for example. Then we
will be left without xo. But maybe we should not handle this
scenario anyway.

> 
> ---8<----
> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> index 572851b5bc16..215a41ba0135 100644
> --- a/drivers/clk/qcom/common.c
> +++ b/drivers/clk/qcom/common.c
> @@ -17,6 +17,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/clk-provider.h>
>  #include <linux/reset-controller.h>
> +#include <linux/of.h>
>  
>  #include "common.h"
>  #include "clk-rcg.h"
> @@ -152,7 +153,18 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
>  int qcom_cc_register_board_clk(struct device *dev, const char *path,
>  			       const char *name, unsigned long rate)
>  {
> -	return _qcom_cc_register_board_clk(dev, path, name, rate, true);
> +	bool add_factor = true;
> +	struct device_node *node;
> +
> +	/* The RPM clock driver will add the factor clock if present */
> +	if (IS_ENABLED(CONFIG_QCOM_RPMCC)) {
> +		node = of_find_compatible_node(NULL, NULL, "qcom,rpmcc");
> +		if (node)

if (node && of_device_is_available(node))
to handle also the case with status="disabled"

> +			add_factor = false;
> +		of_node_put(node);
> +	}
> +
> +	return _qcom_cc_register_board_clk(dev, path, name, rate, add_factor);
>  }
>  EXPORT_SYMBOL_GPL(qcom_cc_register_board_clk);
>  
> 

Re: [PATCH v2 1/2] clk: qcom: common: Add API to register board clocks backwards compatibly

[Stephen Boyd]

On 10/28/2015 11:03 AM, Georgi Djakov wrote:
> On 10/28/2015 03:06 AM, Stephen Boyd wrote:
>> On 10/27, Stephen Boyd wrote:
>>> +
>>> +int qcom_cc_register_board_clk(struct device *dev, const char *path,
>>> +			       const char *name, unsigned long rate)
>>> +{
>>> +	return _qcom_cc_register_board_clk(dev, path, name, rate,
>>> +					   !IS_ENABLED(CONFIG_QCOM_RPMCC));
>> I just realized that we could have this config enabled but the
>> driver never probes on a certain platform. So we'll need to do
>> some sort of search to see if the rpmcc node exists and also
>> check to see if the driver is enabled. I guess we can do that all
>> here in this function as long as we have a compatible =
>> "qcom,rpmcc" as a more generic property for the rpm clock
>> controller node. So if the config is enabled, search the dt tree
>> for a qcom,rpmcc node. If the rpmcc node is there, the last
>> argument is false, otherwise it's true.
> I like the approach.  We could have both compatible strings like:
> compatible = "qcom,rpmcc-msm8916", "qcom,rpmcc";

Exactly.

>
> The only possible issue i could imagine is the case when rpmcc
> driver probe fails - does not enable scaling for example. Then we
> will be left without xo. But maybe we should not handle this
> scenario anyway.

Yeah let's not care until that problem happens.

>
>> ---8<----
>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
>> index 572851b5bc16..215a41ba0135 100644
>> --- a/drivers/clk/qcom/common.c
>> +++ b/drivers/clk/qcom/common.c
>> @@ -17,6 +17,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/clk-provider.h>
>>  #include <linux/reset-controller.h>
>> +#include <linux/of.h>
>>  
>>  #include "common.h"
>>  #include "clk-rcg.h"
>> @@ -152,7 +153,18 @@ static int _qcom_cc_register_board_clk(struct device *dev, const char *path,
>>  int qcom_cc_register_board_clk(struct device *dev, const char *path,
>>  			       const char *name, unsigned long rate)
>>  {
>> -	return _qcom_cc_register_board_clk(dev, path, name, rate, true);
>> +	bool add_factor = true;
>> +	struct device_node *node;
>> +
>> +	/* The RPM clock driver will add the factor clock if present */
>> +	if (IS_ENABLED(CONFIG_QCOM_RPMCC)) {
>> +		node = of_find_compatible_node(NULL, NULL, "qcom,rpmcc");
>> +		if (node)
> if (node && of_device_is_available(node))
> to handle also the case with status="disabled"

Good catch! Looks like we can pass NULL there though so it can be
simplified slightly.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project