On 8/4/25 4:21 PM, Taniya Das wrote: > > > On 8/4/2025 6:40 PM, Konrad Dybcio wrote: >> On 8/4/25 11:00 AM, Taniya Das wrote: >>> >>> >>> On 8/1/2025 5:24 PM, Konrad Dybcio wrote: >>>> On 8/1/25 7:31 AM, Abel Vesa wrote: >>>>> On 25-08-01 10:02:15, Taniya Das wrote: >>>>>> >>>>>> >>>>>> On 7/30/2025 4:55 PM, Abel Vesa wrote: >>>>>>> On 25-07-29 11:12:37, Taniya Das wrote: >>>>>>>> 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 <taniya.das@xxxxxxxxxxxxxxxx> >>>>>>>> --- >>>>>>>> drivers/clk/qcom/Kconfig | 8 ++ >>>>>>>> drivers/clk/qcom/Makefile | 1 + >>>>>>>> drivers/clk/qcom/tcsrcc-glymur.c | 257 +++++++++++++++++++++++++++++++++++++++ >>>>>>>> 3 files changed, 266 insertions(+) >>>>>>>> >>>>>>> >>>>>>> [...] >>>>>>> >>>>>>>> + >>>>>>>> +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. >>> Abel, we earlier also had some PCIe, USB use cases where we had to leave >>> the ref clocks ON and APSS could collapse. >> >> XO votes will prevent CX collapse, not APSS collapse. CX also powers >> USB and PCIe so that only makes sense. >> >> I think it's fair to just stick XO as the parent of every refclock >> today and think about the what-ifs (such as the mdss case I mentioned >> above) later - especially since we have no infra to take full advantage >> of it today (non-APSS RSCs etc.) >> > > When ref clock have been part of GCC, then also they didn't have any xo > as the parent, similar design we kept when it was moved to TCSR as well. Perhaps we've been running on luck (i.e. XO votes being cast through another device / clock as a second order effect) all this time.. I'd happily move towards formal correctness. Konrad