linux-kernel - Re: [PATCH] clk: qcom: gcc-msm8953: fix stuck gcc_usb30_master

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 22 Nov 2023 11:39:05 +0530
From:   Krishna Kurapati PSSNV <quic_kriskura@...cinc.com>
To:     Konrad Dybcio <konrad.dybcio@...aro.org>,
        Stephen Boyd <sboyd@...nel.org>,
        Barnabás Czémán <trabarni@...il.com>,
        Bjorn Andersson <andersson@...nel.org>
CC:     <linux-arm-msm@...r.kernel.org>, <linux-clk@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, Andy Gross <agross@...nel.org>,
        Michael Turquette <mturquette@...libre.com>
Subject: Re: [PATCH] clk: qcom: gcc-msm8953: fix stuck gcc_usb30_master_clk



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@...il.com>
>>>> ---
>>> "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@...d {};
> }
> 
> 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,