clk: qcom: clk-rpmh: Add QCOM RPMh clock driver
Subsystems (3)
PATCH v8 1/1 clk: qcom: clk-rpmh: Add QCOM RPMh clock driver
[v8]
* cleanup code to remove of rpmh_client handle.
* pass the hw_clks array to the devm_of_clk_add_hw_provider getter function.
[v7]
* Addressed comments from Stephen
- Keep only xo_board as clock parent.
- Associate divs from SoC specific clock listing to calculate the clock rate.
- Add back the recalc_rate clock ops to return the clock rate based on the parent rate and div associated with clock.
- void cast kept to avoid compiler warnings.
[v6]
* Addressed comments from Rob
* Addressed comments from Stephen
[v5]
* Addressed comments from Stephen
* Introduced a new DT property to take clock divs
[v4]
* Addressed comments from Stephen
[v3]
* Addressed documentation & code review comments from Bjorn
* Addressed bindings comments from Rob
* Updated the patch series order for bindings
[v2]
* Addressed comments from Stephen
* Addressed comments from Evan
This patch series adds a driver and device tree documentation binding for the clock control via Resource Power Manager-hardened (RPMh) on some Qualcomm Technologies, Inc, SoCs such as SDM845. The clock RPMh driver would send requests for the RPMh managed clock resources.
The RPMh clock driver depends upon the RPMh driver [1] and command DB driver [2] which are both still undergoing review.
Thanks,
Taniya
[1]: https://lkml.org/lkml/2018/3/9/979
[2]: https://lkml.org/lkml/2018/3/14/787
Taniya Das (1):
clk: qcom: clk-rpmh: Add QCOM RPMh clock driver
drivers/clk/qcom/Kconfig | 9 ++ drivers/clk/qcom/Makefile | 1 + drivers/clk/qcom/clk-rpmh.c | 334 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 344 insertions(+) create mode 100644 drivers/clk/qcom/clk-rpmh.c
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
PATCH v8 1/1 clk: qcom: clk-rpmh: Add QCOM RPMh clock driver
Add the RPMh clock driver to control the RPMh managed clock resources on some of the Qualcomm Technologies, Inc. SoCs.
--- drivers/clk/qcom/Kconfig | 9 ++ drivers/clk/qcom/Makefile | 1 + drivers/clk/qcom/clk-rpmh.c | 334 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 344 insertions(+) create mode 100644 drivers/clk/qcom/clk-rpmh.c
drivers/clk/qcom/Kconfig +9 -0
@@ -59,6 +59,15 @@ config QCOM_CLK_SMD_RPM Say Y if you want to support the clocks exposed by the RPM on platforms such as apq8016, apq8084, msm8974 etc. +config QCOM_CLK_RPMH + tristate "RPMh Clock Driver" + depends on COMMON_CLK_QCOM && QCOM_RPMH + help + RPMh manages shared resources on some Qualcomm Technologies, Inc. + SoCs. It accepts requests from other hardware subsystems via RSC. + Say Y if you want to support the clocks exposed by RPMh on + platforms such as SDM845. + config APQ_GCC_8084 tristate "APQ8084 Global Clock Controller" select QCOM_GDSC
drivers/clk/qcom/Makefile +1 -0
@@ -37,5 +37,6 @@ obj-$(CONFIG_MSM_MMCC_8996) += mmcc-msm8996.o obj-$(CONFIG_QCOM_A53PLL) += a53-pll.o obj-$(CONFIG_QCOM_CLK_APCS_MSM8916) += apcs-msm8916.o obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o +obj-$(CONFIG_QCOM_CLK_RPMH) += clk-rpmh.o obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
drivers/clk/qcom/clk-rpmh.c new file +334 -0
334 lines not on this page · raw message
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Stephen Boyd
maintainerQuoting Taniya Das (2018-05-04 03:02:38)
Quoted 7 lines
drivers/clk/qcom/clk-rpmh.c new file +1
-0@@ -0,0 +1,334 @@ +// SPDX-License-Identifier: GPL-2.0
drivers/clk/qcom/clk-rpmh.c line 11
Quoted 10 lines
drivers/clk/qcom/clk-rpmh.c new file +10
-0+/* +* Copyright (c) 2018, The Linux Foundation. All rights reserved. +*/ + +}; + +struct clk_rpmh_desc { +struct clk_hw **clks; +size_t num_clks; +};
This could be replaced with the clk_hw_onecell_data struct and then the only problem becomes the const part which seems pretty impossible to fix at this point. One "workaround" is to memdup the structure. Ugh.
Quoted 7 lines
drivers/clk/qcom/clk-rpmh.c new file +7
-0+ +static DEFINE_MUTEX (rpmh_clk_lock); + +#define __DEFINE_CLK_RPMH (_platform, _name, _name_active, _res_name, \ + _res_en_offset, _res_on, _div) \ + static struct clk_rpmh _platform##_##_name_active; \ + static struct clk_rpmh _platform##_##_name = { \
[..]
drivers/clk/qcom/clk-rpmh.c line 204
Quoted 11 lines
drivers/clk/qcom/clk-rpmh.c new file +11
-0+ +static unsigned long clk_rpmh_recalc_rate (struct clk_hw *hw, +unsigned long prate) +{ +struct clk_rpmh *r =to_clk_rpmh (hw); +unsigned long rate = prate; + +/* +* RPMh clocks have a fixed rate. Return static rate. +*/ +do_div (rate, r->div);
Integer division won't suffice?
Quoted 2 lines
drivers/clk/qcom/clk-rpmh.c new file +2
-0+return rate; +}
[...]
drivers/clk/qcom/clk-rpmh.c line 248
Quoted 8 lines
drivers/clk/qcom/clk-rpmh.c new file +8
-0+ +static struct clk_hw *of_clk_rpmh_hw_get (struct of_phandle_args *clkspec, +void *data) +{ +struct clk_hw *hw_clks = data; +unsigned int idx = clkspec->args[0 ]; + +if (idx <0 ) {
It can't be less than 0 though because it's unsigned.
Quoted 6 lines
drivers/clk/qcom/clk-rpmh.c new file +6
-0+pr_err ("%s: invalid index %u\n" , __func__, idx); +return ERR_PTR (-EINVAL ); + } + +return &hw_clks[idx]; +}
Hello Stephen,
Could you please let me know your comments on the below.
Quoted 14 lines · Stephen Boyd
Quoting Taniya Das (2018-05-04 03:02:38)
Quoted 13 lines
drivers/clk/qcom/clk-rpmh.c new file +7
-0@@ -0,0 +1,334 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2018, The Linux Foundation. All rights reserved. +*/ + +}; +
Quoted 9 lines
drivers/clk/qcom/clk-rpmh.c line 52
Quoted 4 lines
drivers/clk/qcom/clk-rpmh.c new file +4
-0+struct clk_rpmh_desc { +struct clk_hw **clks; +size_t num_clks; +};This could be replaced with the
clk_hw_onecell_datastruct and then the only problem becomes the const part which seems pretty impossible to fix at this point. One "workaround" is to memdup the structure. Ugh.
Will be okay, if I can the following?
_probe...
{
struct clk_rpmh_desc *hw_desc_data;
....
hw_desc_data = kmemdup(desc, sizeof(*desc), GFP_KERNEL);
...
ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_rpmh_hw_get,
hw_desc_data);
....}
And also I fix the "getter" function.
Quoted 42 lines
Quoted 7 lines
drivers/clk/qcom/clk-rpmh.c new file +7
-0+ +static DEFINE_MUTEX (rpmh_clk_lock); + +#define __DEFINE_CLK_RPMH (_platform, _name, _name_active, _res_name, \ + _res_en_offset, _res_on, _div) \ + static struct clk_rpmh _platform##_##_name_active; \ + static struct clk_rpmh _platform##_##_name = { \[..]
drivers/clk/qcom/clk-rpmh.c line 204
Quoted 11 lines
drivers/clk/qcom/clk-rpmh.c new file +11
-0+ +static unsigned long clk_rpmh_recalc_rate (struct clk_hw *hw, +unsigned long prate) +{ +struct clk_rpmh *r =to_clk_rpmh (hw); +unsigned long rate = prate; + +/* +* RPMh clocks have a fixed rate. Return static rate. +*/ +do_div (rate, r->div);Integer division won't suffice?
Quoted 2 lines
drivers/clk/qcom/clk-rpmh.c new file +2
-0+return rate; +}[...]
drivers/clk/qcom/clk-rpmh.c line 248
Quoted 8 lines
drivers/clk/qcom/clk-rpmh.c new file +8
-0+ +static struct clk_hw *of_clk_rpmh_hw_get (struct of_phandle_args *clkspec, +void *data) +{ +struct clk_hw *hw_clks = data; +unsigned int idx = clkspec->args[0 ]; + +if (idx <0 ) {It can't be less than 0 though because it's unsigned.
Quoted 6 lines
drivers/clk/qcom/clk-rpmh.c new file +6
-0+pr_err ("%s: invalid index %u\n" , __func__, idx); +return ERR_PTR (-EINVAL ); + } + +return &hw_clks[idx]; +}
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Stephen Boyd
maintainerQuoting Taniya Das (2018-05-07 03:48:06)
Quoted 43 lines
Hello Stephen,
Could you please let me know your comments on the below.
Quoted 14 lines · Stephen Boyd
Quoting Taniya Das (2018-05-04 03:02:38)
Quoted 13 lines
drivers/clk/qcom/clk-rpmh.c new file +7
-0@@ -0,0 +1,334 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2018, The Linux Foundation. All rights reserved. +*/ + +}; +Quoted 9 lines
drivers/clk/qcom/clk-rpmh.c line 52
Quoted 4 lines
drivers/clk/qcom/clk-rpmh.c new file +4
-0+struct clk_rpmh_desc { +struct clk_hw **clks; +size_t num_clks; +};This could be replaced with the
clk_hw_onecell_datastruct and then the only problem becomes the const part which seems pretty impossible to fix at this point. One "workaround" is to memdup the structure. Ugh.Will be okay, if I can the following?
_probe...{ struct clk_rpmh_desc *hw_desc_data; .... hw_desc_data = kmemdup(desc, sizeof(*desc), GFP_KERNEL); ... ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_rpmh_hw_get, hw_desc_data); ....
Quoted 3 lines
}
And also I fix the "getter" function.
I'd rather see the check for out of bounds number just go away, unless that's helping something. The kmemdup() doesn't look good.
Hello Stephen,
Thanks for your review comments, please check my comments below, so that I could submit the next patch series.
Quoted 44 lines · Stephen Boyd
Quoting Taniya Das (2018-05-07 03:48:06)
Quoted 43 lines
Hello Stephen,
Could you please let me know your comments on the below.
Quoted 14 lines · Stephen Boyd
Quoting Taniya Das (2018-05-04 03:02:38)
Quoted 13 lines
drivers/clk/qcom/clk-rpmh.c new file +7
-0@@ -0,0 +1,334 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2018, The Linux Foundation. All rights reserved. +*/ + +}; +Quoted 9 lines
drivers/clk/qcom/clk-rpmh.c line 52
Quoted 4 lines
drivers/clk/qcom/clk-rpmh.c new file +4
-0+struct clk_rpmh_desc { +struct clk_hw **clks; +size_t num_clks; +};This could be replaced with the
clk_hw_onecell_datastruct and then the only problem becomes the const part which seems pretty impossible to fix at this point. One "workaround" is to memdup the structure. Ugh.Will be okay, if I can the following?
_probe...{ struct clk_rpmh_desc *hw_desc_data; .... hw_desc_data = kmemdup(desc, sizeof(*desc), GFP_KERNEL); ... ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_rpmh_hw_get, hw_desc_data); ....
Quoted 7 lines
Quoted 3 lines
}
And also I fix the "getter" function.
I'd rather see the check for out of bounds number just go away, unless that's helping something. The
kmemdup()doesn't look good.
I think it is better to copy the desc data to "clk_hw_onecell_data", as I would also have to fix the "getter" function to check for out of bound numbers (idx >= num_clks).
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --
Stephen Boyd
maintainerQuoting Taniya Das (2018-05-08 09:57:36)
Quoted 20 lines · Stephen Boyd
Quoted 19 lines
Quoting Taniya Das (2018-05-07 03:48:06)
Quoted 17 lines
Will be okay, if I can the following?
_probe...{ struct clk_rpmh_desc *hw_desc_data; .... hw_desc_data = kmemdup(desc, sizeof(*desc), GFP_KERNEL); ... ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_rpmh_hw_get, hw_desc_data); ....}
And also I fix the "getter" function.
Quoted 8 lines
Quoted 3 lines
I'd rather see the check for out of bounds number just go away, unless that's helping something. The
kmemdup()doesn't look good.I think it is better to copy the desc data to "
clk_hw_onecell_data", as I would also have to fix the "getter" function to check for out of bound numbers (idx >=num_clks).
Hmm ok. It would be nicer if we didn't have to duplicate the pointers just because of const though. I'd rather have the cast away of const and a comment indicating we know what we're doing. Maybe in the future we can make the getter function take a const void pointer.