Thread (11 messages) 11 messages, 6 authors, 2021-12-17

Re: [PATCH v2 1/2] clk: qcom: Add display clock controller driver for QCM2290

From: Dmitry Baryshkov <hidden>
Date: 2021-12-17 01:59:09
Also in: linux-arm-msm, linux-clk

On Fri, 17 Dec 2021 at 04:37, Stephen Boyd [off-list ref] wrote:
Quoting Loic Poulain (2021-12-16 11:21:51)
quoted
Hi Stephen,


On Thu, 16 Dec 2021 at 04:49, Stephen Boyd [off-list ref] wrote:
quoted
Quoting Loic Poulain (2021-12-09 06:09:10)
quoted
diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
new file mode 100644
index 00000000..8aa5d31
--- /dev/null
+++ b/drivers/clk/qcom/dispcc-qcm2290.c
@@ -0,0 +1,602 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2021, Linaro Ltd.
+ */
+
[...]
quoted
quoted
+static struct clk_rcg2 disp_cc_mdss_pclk0_clk_src = {
+       .cmd_rcgr = 0x205c,
+       .mnd_width = 8,
+       .hid_width = 5,
+       .parent_map = disp_cc_parent_map_4,
+       .clkr.hw.init = &(struct clk_init_data){
+               .name = "disp_cc_mdss_pclk0_clk_src",
+               .parent_data = disp_cc_parent_data_4,
+               .num_parents = ARRAY_SIZE(disp_cc_parent_data_4),
+               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE | CLK_OPS_PARENT_ENABLE,
These last two flags are needed for what?
NOCACHE is probably useless with mainline.
Ok then please remove it.
quoted
I've added OPS_PARENT_ENABLE because AFAIU changing clock rate can
lead to parent switch, and parent switch can only be done if parent
clocks are enabled for rcg2 clocks. Otherwise the update fails and we
get the following:
    disp_cc_mdss_pclk0_clk_src: rcg didn't update its configuration.
    WARNING: CPU: 2 PID: 77 at drivers/clk/qcom/clk-rcg2.c:122
update_config+0xe0/0xf0

I'm a bit surprised other similar dispcc drivers don't use the same
flags though.
That's quite odd. We migrate the prepare and enable count to the new
parent in the core framework so is the rcg on, but doesn't look like it
is on to the core and set_rate is being called?
It's a part of a typical problem for some clocks (that we were
discussing in a thread regarding Bjorn's ASSUME_ENABLED and my 'park
clocks' proposal). For set_rate and set_parent to succeed, both old
and new parrents must be running at the time of the operation/



-- 
With best wishes
Dmitry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help