Re: [PATCH] clk: qcom: gcc-msm8953: fix stuck gcc_usb30_master_clk

[Krishna Kurapati PSSNV]

On 11/18/2023 6:18 AM, Konrad Dybcio wrote:
> On 24.10.2023 04:59, Stephen Boyd wrote:
> > Quoting Konrad Dybcio (2023-10-06 16:50:18)
> > > On 2.10.2023 19:00, Barnabás Czémán wrote:
> > > > According to downstream dwc3-msm source this clock has FSM dependency on
> > > > gcc_pcnoc_usb30_clk so enabling it would fail if latter isn't enabled.
> > > > This patch add works around this issue by changing parent of
> > > > gcc_usb30_master_clk to gcc_pcnoc_usb30_clk. This is acceptable because
> > > > both clocks have same parent and are branches/gates.
> > > >
> > > > Signed-off-by: Barnabás Czémán <trabarni@xxxxxxxxx>
> > > > ---
> > > "meh"
> > >
> > > There are multiple cases, especially with qcom, where there are some
> > > magic "dependencies" without parent-child relationship. The common
> > > clock framework doesn't currently have any good way to handle this,
> > > other than some mind gymnastics like you had to do here with matching
> > > them against a common parent/ancestor..
> > >
> > > Stephen, what do you say?
> >
> > You can't change the parent to be not the actual parent. The consumer of
> > the branch probably wants to call clk_set_rate() on the branch and have
> > it propagate up to the parent to set the actual rate. Can the axi clk
> > simply be left enabled all the time? That seems simpler. Otherwise we
> > probably need to leave the axi clk control to the interconnect driver
> > and make sure drivers enable interconnects before enabling this branch.
> Yeah I'm almost inclined to think adding even more ifs to the icc driver
> will consume more power than just leaving the AXI hanging..
>
> > When things start to get this tangled I tend to think that we need to
> > remove control of the clk from the general drivers and put the logic to
> > control interconnects and clks into some SoC glue driver and expose a
> > single interface, like genpd power_on/power_off so that general drivers
> > can't get the sequence of steps wrong. Instead all they can do is "power
> > on" their device, and the SoC glue driver can do the proper sequence of
> > framework calls to power up the device.
> That too, given the structure of qcom SoCs, it should almost look like:
>
> xyznoc-bus {
> 	compatible = "simple-pm-bus";
> 	clocks = <&gcc xyznoc_ahb>,
> 		 <&gcc xyznoc_axi>;
> 	...
>
> 	xyznoc-node@abcd {};
> }
>
> etc.
>
> I've actually discussed this with Bjorn, but we came to a conclusion
> that it's not trivial to determine which peripheral lives on which NoC
> + many of them seem to sorta overlap more than one..

Are we seeing the clk getting stuck during suspend/resume or during 
clk_prepare_enable in probe ?

Regards,
Krishna,