Message ID | 20231025-topic-sm8650-upstream-clocks-v1-0-c89b59594caf@linaro.org (mailing list archive) |
---|---|
Headers | show |
Series | clk: qcom: Introduce clocks drivers for SM8650 | expand |
On 10/25/23 09:32, Neil Armstrong wrote: > Add Global Clock Controller (GCC) support for SM8650 platform. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- Just a couple remarks 1. looks like there's no usage of shared ops (corresponding to enable_safe_parent or something along these lines downstream) 2. none of the GDSCs have interesting flags.. I have this little cheat sheet that you may find handy: qcom,retain-regs -> RETAIN_FF_ENABLE qcom,support-hw-trigger + set_mode in driver -> HW_CONTROL qcom,no-status-check-on-disable -> VOTABLE qcom,reset-aon-logic -> AON_RESET domain-addr = clamp_io_ctrl 3. gcc_cpuss_ubwcp_clk_src uses the XO_A clock as parent, but it's not there in the ftbl.. Could you confirm whether this clock should even be accessed from HLOS? [...] > +static int gcc_sm8650_probe(struct platform_device *pdev) > +{ > + struct regmap *regmap; > + int ret; > + > + regmap = qcom_cc_map(pdev, &gcc_sm8650_desc); > + if (IS_ERR(regmap)) > + return PTR_ERR(regmap); > + > + ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks, > + ARRAY_SIZE(gcc_dfs_clocks)); > + if (ret) > + return ret; > + > + /* > + * Keep the critical clock always-On > + * gcc_camera_ahb_clk, gcc_camera_xo_clk, gcc_disp_ahb_clk, > + * gcc_disp_xo_clk, gcc_gpu_cfg_ahb_clk, gcc_video_ahb_clk, > + * gcc_video_xo_clk > + */ Could you make these comments inline, i.e. regmap_update_bits(regmap, 0x26004, BIT(0), BIT(0)); /* gcc_camera_ahb_clk */ ? Konrad
On 25/10/2023 10:41, Konrad Dybcio wrote: > > > On 10/25/23 09:32, Neil Armstrong wrote: >> Add Global Clock Controller (GCC) support for SM8650 platform. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- > Just a couple remarks > > 1. looks like there's no usage of shared ops (corresponding > to enable_safe_parent or something along these lines > downstream) Indeed, it was missing, I'll give a test before posting a v2. > > 2. none of the GDSCs have interesting flags.. I have this > little cheat sheet that you may find handy: > > qcom,retain-regs -> RETAIN_FF_ENABLE > qcom,support-hw-trigger + set_mode in driver -> HW_CONTROL > qcom,no-status-check-on-disable -> VOTABLE > qcom,reset-aon-logic -> AON_RESET > domain-addr = clamp_io_ctrl Thx, I updated the GDSCs. > > 3. gcc_cpuss_ubwcp_clk_src uses the XO_A clock as parent, but > it's not there in the ftbl.. Could you confirm whether this > clock should even be accessed from HLOS? Downstream this clock is only used by gem_noc, since we don't use such clock upstream I think it's safer to remove it until we have the usage. > > [...] > >> +static int gcc_sm8650_probe(struct platform_device *pdev) >> +{ >> + struct regmap *regmap; >> + int ret; >> + >> + regmap = qcom_cc_map(pdev, &gcc_sm8650_desc); >> + if (IS_ERR(regmap)) >> + return PTR_ERR(regmap); >> + >> + ret = qcom_cc_register_rcg_dfs(regmap, gcc_dfs_clocks, >> + ARRAY_SIZE(gcc_dfs_clocks)); >> + if (ret) >> + return ret; >> + >> + /* >> + * Keep the critical clock always-On >> + * gcc_camera_ahb_clk, gcc_camera_xo_clk, gcc_disp_ahb_clk, >> + * gcc_disp_xo_clk, gcc_gpu_cfg_ahb_clk, gcc_video_ahb_clk, >> + * gcc_video_xo_clk >> + */ > Could you make these comments inline, i.e. > > regmap_update_bits(regmap, 0x26004, BIT(0), BIT(0)); /* gcc_camera_ahb_clk */ > > ? Done > > Konrad Thanks, Neil
This patchset introduces the following SM8650 Clock drivers: - GCC: Global Clock Controller - DISPCC: Display Clock Controller - TCSR Clock Controller - GPUCC: GPU Clock Controller driver - rpmh clocks Dependencies: None For convenience, a regularly refreshed linux-next based git tree containing all the SM8650 related work is available at: https://git.codelinaro.org/neil.armstrong/linux/-/tree/topic/sm85650/upstream/integ Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- Neil Armstrong (10): dt-bindings: clock: qcom: document the SM8650 TCSR Clock Controller dt-bindings: clock: qcom: document the SM8650 General Clock Controller dt-bindings: clock: qcom: document the SM8650 Display Clock Controller dt-bindings: clock: qcom: document the SM8650 GPU Clock Controller dt-bindings: clock: qcom-rpmhcc: document the SM8650 RPMH Clock Controller clk: qcom: add the SM8650 Global Clock Controller driver clk: qcom: add the SM8650 TCSR Clock Controller driver clk: qcom: add the SM8650 Display Clock Controller driver clk: qcom: add the SM8650 GPU Clock Controller driver clk: qcom: rpmh: add clocks for SM8650 .../devicetree/bindings/clock/qcom,rpmhcc.yaml | 1 + .../bindings/clock/qcom,sm8450-gpucc.yaml | 2 + .../bindings/clock/qcom,sm8650-dispcc.yaml | 106 + .../devicetree/bindings/clock/qcom,sm8650-gcc.yaml | 65 + .../bindings/clock/qcom,sm8650-tcsr.yaml | 55 + drivers/clk/qcom/Kconfig | 32 + drivers/clk/qcom/Makefile | 4 + drivers/clk/qcom/clk-rpmh.c | 29 + drivers/clk/qcom/dispcc-sm8650.c | 1806 +++++++++ drivers/clk/qcom/gcc-sm8650.c | 3931 ++++++++++++++++++++ drivers/clk/qcom/gpucc-sm8650.c | 660 ++++ drivers/clk/qcom/tcsrcc-sm8650.c | 192 + include/dt-bindings/clock/qcom,sm8650-dispcc.h | 101 + include/dt-bindings/clock/qcom,sm8650-gcc.h | 257 ++ include/dt-bindings/clock/qcom,sm8650-gpucc.h | 43 + include/dt-bindings/clock/qcom,sm8650-tcsr.h | 18 + include/dt-bindings/reset/qcom,sm8650-gpucc.h | 20 + 17 files changed, 7322 insertions(+) --- base-commit: fe1998aa935b44ef873193c0772c43bce74f17dc change-id: 20231016-topic-sm8650-upstream-clocks-3c09f464b7d4 Best regards,