clk: qcom: common: Add API to register board clocks backwards compatibly
Subsystems (3)
PATCH v2 1/2 clk: qcom: common: Add API to register board clocks backwards compatibly
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.
---
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(+)
drivers/clk/qcom/common.c +76 -0
@@ -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) {
drivers/clk/qcom/common.h +4 -0
@@ -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,
Signature · 2 lines
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
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.
---
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(-)
drivers/clk/qcom/gcc-apq8084.c +7 -9
@@ -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); }
drivers/clk/qcom/gcc-ipq806x.c +6 -8
@@ -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)
drivers/clk/qcom/gcc-msm8660.c +7 -8
@@ -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); }
drivers/clk/qcom/gcc-msm8960.c +6 -8
@@ -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)
drivers/clk/qcom/gcc-msm8974.c +7 -10
@@ -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); }
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Review
Quoted 6 lines · Stephen Boyd
(quoted diff) +6
-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));
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<----
drivers/clk/qcom/common.c +13 -1
@@ -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);
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Quoted 17 lines · Stephen Boyd
Review
Quoted 6 lines · Stephen Boyd
(quoted diff) +6
-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));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.
Quoted 16 lines
---8<----
drivers/clk/qcom/common.c +1
-0@@ -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,
drivers/clk/qcom/common.c static int _qcom_cc_register_board_clk(struct device *dev, const char *path, · line 162
Quoted 10 lines
drivers/clk/qcom/common.c +7
-1const 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"
Quoted 9 lines
drivers/clk/qcom/common.c +5
-0+ 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);
Quoted 17 lines · Georgi Djakov
Quoted 16 lines · Stephen Boyd
Review
Quoted 6 lines · Stephen Boyd
(quoted diff) +6
-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));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.
Quoted 2 lines
I like the approach. We could have both compatible strings like:
compatible = "qcom,rpmcc-msm8916", "qcom,rpmcc";
Exactly.
Quoted 5 lines
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.
Quoted 12 lines
Quoted 12 lines
---8<----
drivers/clk/qcom/common.c +1
-0@@ -17,6 +17,7 @@#include <linux/platform_device.h> #include <linux/clk-provider.h> #include <linux/reset-controller.h> +#include <linux/of.h>
Quoted 16 lines
Quoted 4 lines
drivers/clk/qcom/common.c +0
-0#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,drivers/clk/qcom/common.c static int _qcom_cc_register_board_clk(struct device *dev, const char *path, · line 162
Quoted 10 lines
drivers/clk/qcom/common.c +7
-1const 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.
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project