On 30.08.2024 7:59 PM, Stephen Boyd wrote:
> Quoting Konrad Dybcio (2024-08-30 05:24:20)
>> On 27.08.2024 8:12 PM, Stephen Boyd wrote:
>>> Quoting Stephen Boyd (2024-08-19 16:36:27)
>>>> Amit Pundir reports that audio and USB-C host mode stops working if the
>>>> gcc_usb30_prim_master_clk_src clk is registered and
>>>> clk_rcg2_shared_init() parks it on XO. Skip parking this clk at
>>>> registration time to fix those issues.
>>>>
>>>> Partially revert commit 01a0a6cc8cfd ("clk: qcom: Park shared RCGs upon
>>>> registration") by skipping the parking bit for this clk, but keep the
>>>> part where we cache the config register. That's still necessary to
>>>> figure out the true parent of the clk at registration time.
>>>>
>>>> Fixes: 01a0a6cc8cfd ("clk: qcom: Park shared RCGs upon registration")
>>>> Fixes: 929c75d57566 ("clk: qcom: gcc-sm8550: Mark RCGs shared where applicable")
>>>> Cc: Konrad Dybcio <konradybcio@xxxxxxxxxx>
>>>> Cc: Bjorn Andersson <andersson@xxxxxxxxxx>
>>>> Cc: Taniya Das <quic_tdas@xxxxxxxxxxx>
>>>> Reported-by: Amit Pundir <amit.pundir@xxxxxxxxxx>
>>>> Closes: https://lore.kernel.org/CAMi1Hd1KQBE4kKUdAn8E5FV+BiKzuv+8FoyWQrrTHPDoYTuhgA@xxxxxxxxxxxxxx
>>>> Signed-off-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
>>>> ---
>>>> drivers/clk/qcom/clk-rcg.h | 1 +
>>>> drivers/clk/qcom/clk-rcg2.c | 30 ++++++++++++++++++++++++++++++
>>>> drivers/clk/qcom/gcc-sm8550.c | 2 +-
>>>> 3 files changed, 32 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
>>>> index d7414361e432..8e0f3372dc7a 100644
>>>> --- a/drivers/clk/qcom/clk-rcg.h
>>>> +++ b/drivers/clk/qcom/clk-rcg.h
>>>> @@ -198,6 +198,7 @@ extern const struct clk_ops clk_byte2_ops;
>>>> extern const struct clk_ops clk_pixel_ops;
>>>> extern const struct clk_ops clk_gfx3d_ops;
>>>> extern const struct clk_ops clk_rcg2_shared_ops;
>>>> +extern const struct clk_ops clk_rcg2_shared_no_init_park_ops;
>>>
>>> I'm considering inverting these two rcg2_shared clk_ops so that only a
>>> few clks are parked at clk registration time, to minimize the impact of
>>> commit 01a0a6cc8cfd ("clk: qcom: Park shared RCGs upon registration").
>>> We're up to three or four band-aids, that we can probably wait on
>>> applying if we make all the shared RCGs determine the correct parent at
>>> registration time but skip the parking, except for the display clks on
>>> sc7180 where that exposes another problem with shared parents getting
>>> turned off during probe. It's possible that other SoCs will want to park
>>> their display clks as well to avoid that secondary problem, but it can
>>> be an opt-in case instead of a change to all shared RCGs.
>>
>> Are all cases that need the parking obvious like it was the case on 7180,
>> i.e. some downstream branch is stuck and there's complaining in dmesg?
>>
>
> I'm under the impression that we need to park the clk when it is shared
> by a remoteproc/firmware or is associated with a GDSC. It seems that on
> older generations of hardware the GDSC would get unstuck eventually, but
> newer generations stay broken and cause all sorts of havoc.
I heard newer GDSCs are funky..
> Note that in my statement earlier in this thread I'm talking about
> parking the clk at registration time. That's done to avoid a problem
> where a shared RCG turns off their parent PLL and another shared RCG is
> also using that PLL but hasn't parked yet. The solution was to park at
> registration time to fix that. It's mostly a workaround for the fact
> that the clk framework doesn't have a good way to track dependencies for
> all the child clks that are enable at registration time which want to
> keep the parent PLL enabled. The problem is that it breaks things like
> USB that has strict frequency requirements for the link.
Should we just do something like .sync_state, where top-level parents
aren't turned off until all clocks have been registered?
Konrad
