clk: qcom: clk-rpmh: Add QCOM RPMh clock driver
v4 cover clk: qcom: clk-rpmh: Add QCOM RPMh clock driver
[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 (2):
dt-bindings: clock: Introduce QCOM RPMh clock bindings
clk: qcom: clk-rpmh: Add QCOM RPMh clock driver
.../devicetree/bindings/clock/qcom,rpmh-clk.txt | 22 ++ drivers/clk/qcom/Kconfig | 9 + drivers/clk/qcom/Makefile | 1 + drivers/clk/qcom/clk-rpmh.c | 364 +++++++++++++++++++++ include/dt-bindings/clock/qcom,rpmh.h | 24 ++ 5 files changed, 420 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/qcom,rpmh-clk.txt create mode 100644 drivers/clk/qcom/clk-rpmh.c create mode 100644 include/dt-bindings/clock/qcom,rpmh.h
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
PATCH v4 1/2 dt-bindings: clock: Introduce QCOM RPMh clock bindings
Add RPMh clock device bindings for Qualcomm Technology Inc's SoCs. These devices would be used for communicating resource state requests to control the clocks managed by RPMh.
--- .../devicetree/bindings/clock/qcom,rpmh-clk.txt | 22 ++++++++++++++++++++ include/dt-bindings/clock/qcom,rpmh.h | 24 ++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/qcom,rpmh-clk.txt create mode 100644 include/dt-bindings/clock/qcom,rpmh.h
Documentation/devicetree/bindings/clock/qcom,rpmh-clk.txt new file +22 -0
@@ -0,0 +1,22 @@ +Qualcomm Technologies, Inc. RPMh Clocks +------------------------------------------------------- + +Resource Power Manager Hardened (RPMh) manages shared resources on +some Qualcomm Technologies Inc. SoCs. It accepts clock requests from +other hardware subsystems via RSC to control clocks. + +Required properties : +- compatible : shall contain "qcom,sdm845-rpmh-clk" + +- #clock-cells : must contain 1 + +Example : + +#include <dt-bindings/clock/qcom,rpmh.h> + + &apps_rsc { + rpmhcc: clock-controller { + compatible = "qcom,sdm845-rpmh-clk"; + #clock-cells = <1>; + }; + };
include/dt-bindings/clock/qcom,rpmh.h new file +24 -0
@@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (c) 2018, The Linux Foundation. All rights reserved. */ + + +#ifndef _DT_BINDINGS_CLK_MSM_RPMH_H +#define _DT_BINDINGS_CLK_MSM_RPMH_H + +/* RPMh controlled clocks */ +#define RPMH_CXO_CLK 0 +#define RPMH_CXO_CLK_A 1 +#define RPMH_LN_BB_CLK2 2 +#define RPMH_LN_BB_CLK2_A 3 +#define RPMH_LN_BB_CLK3 4 +#define RPMH_LN_BB_CLK3_A 5 +#define RPMH_RF_CLK1 6 +#define RPMH_RF_CLK1_A 7 +#define RPMH_RF_CLK2 8 +#define RPMH_RF_CLK2_A 9 +#define RPMH_RF_CLK3 10 +#define RPMH_RF_CLK3_A 11 +#define RPMH_RF_CLK4 12 +#define RPMH_RF_CLK4_A 13 + +#endif
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Re: [v4 1/2] dt-bindings: clock: Introduce QCOM RPMh clock bindings
Quoting Taniya Das (2018-04-24 05:23:18)
Quoted 8 lines
Add RPMh clock device bindings for Qualcomm Technology Inc's SoCs. These devices would be used for communicating resource state requests to control the clocks managed by RPMh.
Signed-off-by Amit Nischal <[email protected]>Signed-off-by Taniya Das <[email protected]>Reviewed-by Rob Herring <[email protected]>---
Applied to clk-next
PATCH v4 2/2 clk: qcom: clk-rpmh: Add QCOM RPMh clock driver
In reply to Taniya DasAdd 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 | 364 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 374 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
@@ -36,5 +36,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 +364 -0
364 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-04-24 05:23:19)
Quoted 23 lines
drivers/clk/qcom/clk-rpmh.c new file +17
-0@@ -0,0 +1,364 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2018, The Linux Foundation. All rights reserved. +*/ + +#include <linux/clk-provider.h> +#include <linux/err.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <soc/qcom/cmd-db.h> +#include <soc/qcom/rpmh.h> + +#include <dt-bindings/clock/qcom,rpmh.h> +
drivers/clk/qcom/clk-rpmh.c line 27
Quoted 10 lines
drivers/clk/qcom/clk-rpmh.c new file +10
-0+#define CLK_RPMH_ARC_EN_OFFSET 0 +#define CLK_RPMH_VRM_EN_OFFSET 4 + +/** +* struct clk_rpmh - individual rpmh clock data structure +* @hw: handle between common and hardware-specific interfaces +* @res_name: resource name for the rpmh clock +* @res_addr: base address of the rpmh resource within the RPMh +* @res_en_offset: clock resource enable offset corresponding to ARC or +* VRM type clock
Can these be combined still? Just do a += on res_addr after we get the base of it?
Quoted 27 lines
drivers/clk/qcom/clk-rpmh.c new file +27
-0+* @res_on_val: rpmh clock enable value +* @res_off_val: rpmh clock disable value +* @state: rpmh clock requested state +* @aggr_state: rpmh clock aggregated state +* @last_sent_aggr_state: rpmh clock last aggr state sent to RPMh +* @valid_state_mask: mask to determine the state of the rpmh clock +* @rpmh_client: handle used for rpmh communications +* @dev: device to which it is attached +* @peer: pointer to the clock rpmh sibling +* @rate: rate of the rpmh clock +*/ +struct clk_rpmh { +struct clk_hw hw; +const char *res_name; +u32 res_addr; +u32 res_en_offset; +u32 res_on_val; +u32 res_off_val; +u32 state; +u32 aggr_state; +u32 last_sent_aggr_state; +u32 valid_state_mask; +struct rpmh_client *rpmh_client; +struct device *dev; +struct clk_rpmh *peer; +unsigned long rate; +};
[...]
drivers/clk/qcom/clk-rpmh.c line 114
Quoted 5 lines
drivers/clk/qcom/clk-rpmh.c new file +5
-0+ +static inline bool has_state_changed (struct clk_rpmh *c,u32 state) +{ +return (c->last_sent_aggr_state &BIT (state)) + != (c->aggr_state &BIT (state));
Please drop useless parenthesis.
Quoted 11 lines
drivers/clk/qcom/clk-rpmh.c new file +11
-0+} + +static int clk_rpmh_send_aggregate_command (struct clk_rpmh *c) +{ +struct tcs_cmd cmd = {0 }; +u32 cmd_state, on_val; +enum rpmh_state state =RPMH_SLEEP_STATE ; +int ret; + + cmd.addr = c->res_addr + c->res_en_offset; + cmd_state = c->aggr_state;
drivers/clk/qcom/clk-rpmh.c line 135
Quoted 10 lines
drivers/clk/qcom/clk-rpmh.c new file +10
-0+ on_val = c->res_on_val; + +for (; state <=RPMH_ACTIVE_ONLY_STATE ; state++) { +if (has_state_changed (c, state)) { + cmd.data = (cmd_state &BIT (state)) ? on_val :0 ; + ret =rpmh_write_async (c->rpmh_client, state, + &cmd,1 ); +if (ret) { + dev_err(c->dev, "set %s state of %s failed: (%d)\n", + (!state) ? "sleep" :
Drop parenthesis please.
Quoted 1 line
+ (state ==
RPMH_WAKE_ONLY_STATE) ?
Drop parenthesis please.
Quoted 11 lines
drivers/clk/qcom/clk-rpmh.c new file +11
-0+ "wake" : "active", c->res_name, ret); +return ret; + } + } + } + + c->last_sent_aggr_state = c->aggr_state; + c->peer->last_sent_aggr_state = c->last_sent_aggr_state; + +return 0 ; +}
[...]
Quoted 4 lines
drivers/clk/qcom/clk-rpmh.c new file +4
-0+ +static const struct clk_ops clk_rpmh_ops = { + .prepare = clk_rpmh_prepare, + .unprepare = clk_rpmh_unprepare,
drivers/clk/qcom/clk-rpmh.c line 222
Quoted 10 lines
drivers/clk/qcom/clk-rpmh.c new file +10
-0+ .recalc_rate = clk_rpmh_recalc_rate, +}; + +/* Resource name must match resource id present in cmd-db. */ +DEFINE_CLK_RPMH_ARC (sdm845, bi_tcxo, bi_tcxo_ao,"xo.lvl" ,0x3 ,0x0 ,19200000 ); +DEFINE_CLK_RPMH_VRM (sdm845, ln_bb_clk2, ln_bb_clk2_ao,"lnbclka2" ,19200000 ); +DEFINE_CLK_RPMH_VRM (sdm845, ln_bb_clk3, ln_bb_clk3_ao,"lnbclka3" ,19200000 ); +DEFINE_CLK_RPMH_VRM (sdm845, rf_clk1, rf_clk1_ao,"rfclka1" ,38400000 ); +DEFINE_CLK_RPMH_VRM (sdm845, rf_clk2, rf_clk2_ao,"rfclka2" ,38400000 ); +DEFINE_CLK_RPMH_VRM (sdm845, rf_clk3, rf_clk3_ao,"rfclka3" ,38400000 );
These rates should come from DT and parents of these clks.
Hello Stephen,
Thanks for the review comments.
Quoted 29 lines · Stephen Boyd
Quoting Taniya Das (2018-04-24 05:23:19)
Quoted 28 lines
drivers/clk/qcom/clk-rpmh.c new file +22
-0@@ -0,0 +1,364 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (c) 2018, The Linux Foundation. All rights reserved. +*/ + +#include <linux/clk-provider.h> +#include <linux/err.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <soc/qcom/cmd-db.h> +#include <soc/qcom/rpmh.h> + +#include <dt-bindings/clock/qcom,rpmh.h> + +#define CLK_RPMH_ARC_EN_OFFSET 0 +#define CLK_RPMH_VRM_EN_OFFSET 4 + +/** +* struct clk_rpmh - individual rpmh clock data structure
Quoted 9 lines
drivers/clk/qcom/clk-rpmh.c line 27
Quoted 5 lines
drivers/clk/qcom/clk-rpmh.c new file +5
-0+* @hw: handle between common and hardware-specific interfaces +* @res_name: resource name for the rpmh clock +* @res_addr: base address of the rpmh resource within the RPMh +* @res_en_offset: clock resource enable offset corresponding to ARC or +* VRM type clock Can these be combined still? Just do a += on
res_addrafter we get the base of it?
I have taken care in the next patch.
Quoted 28 lines
Quoted 27 lines
drivers/clk/qcom/clk-rpmh.c new file +27
-0+* @res_on_val: rpmh clock enable value +* @res_off_val: rpmh clock disable value +* @state: rpmh clock requested state +* @aggr_state: rpmh clock aggregated state +* @last_sent_aggr_state: rpmh clock last aggr state sent to RPMh +* @valid_state_mask: mask to determine the state of the rpmh clock +* @rpmh_client: handle used for rpmh communications +* @dev: device to which it is attached +* @peer: pointer to the clock rpmh sibling +* @rate: rate of the rpmh clock +*/ +struct clk_rpmh { +struct clk_hw hw; +const char *res_name; +u32 res_addr; +u32 res_en_offset; +u32 res_on_val; +u32 res_off_val; +u32 state; +u32 aggr_state; +u32 last_sent_aggr_state; +u32 valid_state_mask; +struct rpmh_client *rpmh_client; +struct device *dev; +struct clk_rpmh *peer; +unsigned long rate; +};[...]
Quoted 8 lines
drivers/clk/qcom/clk-rpmh.c line 114
Quoted 5 lines
drivers/clk/qcom/clk-rpmh.c new file +5
-0+ +static inline bool has_state_changed (struct clk_rpmh *c,u32 state) +{ +return (c->last_sent_aggr_state &BIT (state)) + != (c->aggr_state &BIT (state));Please drop useless parenthesis.
I see a warning in case I remove the parenthesis.
warning: suggest parentheses around comparison in operand of ‘&’ [-Wparentheses]
!= c->aggr_state & BIT(state));
Quoted 15 lines
Quoted 15 lines
drivers/clk/qcom/clk-rpmh.c new file +15
-0+} + +static int clk_rpmh_send_aggregate_command (struct clk_rpmh *c) +{ +struct tcs_cmd cmd = {0 }; +u32 cmd_state, on_val; +enum rpmh_state state =RPMH_SLEEP_STATE ; +int ret; + + cmd.addr = c->res_addr + c->res_en_offset; + cmd_state = c->aggr_state; + on_val = c->res_on_val; + +for (; state <=RPMH_ACTIVE_ONLY_STATE ; state++) { +if (has_state_changed (c, state)) {
Quoted 9 lines
drivers/clk/qcom/clk-rpmh.c line 135
Quoted 6 lines
drivers/clk/qcom/clk-rpmh.c new file +6
-0+ cmd.data = (cmd_state &BIT (state)) ? on_val :0 ; + ret =rpmh_write_async (c->rpmh_client, state, + &cmd,1 ); +if (ret) { + dev_err(c->dev, "set %s state of %s failed: (%d)\n", + (!state) ? "sleep" :Drop parenthesis please.
Dropped.
Quoted 4 lines
Quoted 1 line
+ (state ==
RPMH_WAKE_ONLY_STATE) ?Drop parenthesis please.
Dropped.
Quoted 12 lines
Quoted 11 lines
drivers/clk/qcom/clk-rpmh.c new file +11
-0+ "wake" : "active", c->res_name, ret); +return ret; + } + } + } + + c->last_sent_aggr_state = c->aggr_state; + c->peer->last_sent_aggr_state = c->last_sent_aggr_state; + +return 0 ; +}[...]
Quoted 17 lines
Quoted 4 lines
drivers/clk/qcom/clk-rpmh.c new file +4
-0+ +static const struct clk_ops clk_rpmh_ops = { + .prepare = clk_rpmh_prepare, + .unprepare = clk_rpmh_unprepare,drivers/clk/qcom/clk-rpmh.c line 222
Quoted 10 lines
drivers/clk/qcom/clk-rpmh.c new file +10
-0+ .recalc_rate = clk_rpmh_recalc_rate, +}; + +/* Resource name must match resource id present in cmd-db. */ +DEFINE_CLK_RPMH_ARC (sdm845, bi_tcxo, bi_tcxo_ao,"xo.lvl" ,0x3 ,0x0 ,19200000 ); +DEFINE_CLK_RPMH_VRM (sdm845, ln_bb_clk2, ln_bb_clk2_ao,"lnbclka2" ,19200000 ); +DEFINE_CLK_RPMH_VRM (sdm845, ln_bb_clk3, ln_bb_clk3_ao,"lnbclka3" ,19200000 ); +DEFINE_CLK_RPMH_VRM (sdm845, rf_clk1, rf_clk1_ao,"rfclka1" ,38400000 ); +DEFINE_CLK_RPMH_VRM (sdm845, rf_clk2, rf_clk2_ao,"rfclka2" ,38400000 ); +DEFINE_CLK_RPMH_VRM (sdm845, rf_clk3, rf_clk3_ao,"rfclka3" ,38400000 );These rates should come from DT and parents of these clks.
Removed the rates from the above and instead I would take the clk-div values from the DT for the clocks and compute the rates and parents are fixed to "xo_board".
Signature · 4 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation. --