Re: [PATCH v3 3/7] clk: qcom: Add TCSR clock driver for Glymur

On 25-08-04 14:30:00, Taniya Das wrote:

On 8/1/2025 5:24 PM, Konrad Dybcio wrote:

quoted

On 8/1/25 7:31 AM, Abel Vesa wrote:

quoted

On 25-08-01 10:02:15, Taniya Das wrote:

quoted

On 7/30/2025 4:55 PM, Abel Vesa wrote:

quoted

On 25-07-29 11:12:37, Taniya Das wrote:

quoted

Add a clock driver for the TCSR clock controller found on Glymur, which
provides refclks for PCIE, USB, and UFS.

Signed-off-by: Taniya Das <redacted>
---
 drivers/clk/qcom/Kconfig         |   8 ++
 drivers/clk/qcom/Makefile        |   1 +
 drivers/clk/qcom/tcsrcc-glymur.c | 257 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 266 insertions(+)

[...]

quoted

+
+static struct clk_branch tcsr_edp_clkref_en = {
+	.halt_reg = 0x1c,
+	.halt_check = BRANCH_HALT_DELAY,
+	.clkr = {
+		.enable_reg = 0x1c,
+		.enable_mask = BIT(0),
+		.hw.init = &(const struct clk_init_data) {
+			.name = "tcsr_edp_clkref_en",
+			.ops = &clk_branch2_ops,

As discussed off-list, these clocks need to have the bi_tcxo as parent.

Otherwise, as far as the CCF is concerned these clocks will have rate 0,
which is obviously not the case.

Bringing this here since there is a disconnect between X Elite and
Glymur w.r.t this now.

The ref clocks are not required to be have a parent of bi_tcxo as these
ideally can be left enabled(as a subsystem requirement) even if HLOS
(APSS) goes to suspend. With the bi_tcxo parent the ARC vote from
HLOS/APSS will not allow APSS to collapse.

Is there a scenario where the APSS is collapsed and still the ref clock
needs to stay enabled ? Sorry, this doesn't make sense to me.

MDSS is capable of displaying things from a buffer when the CPU is off,
AFAICU

We can do CXO_AO instead to have it auto-collapse if it's just Linux
requesting it to stay on, I think.

Thanks Konrad for adding the display use case.

OK, that usecase makes sense then.

Abel, we earlier also had some PCIe, USB use cases where we had to leave
the ref clocks ON and APSS could collapse.

Then we need to model that in a different way. Leaving those clocks
without parents should not be the option. Maybe the CXO_AO is the best
option then.

-- 
Thanks,
Taniya Das