Re: [PATCH v2 1/3] arm64: dts: qcom: msm8998: correct xo clock name

From: Jeffrey Hugo <jhugo@codeaurora.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Marc Gonzalez <marc.w.gonzalez@free.fr>,
	Stephen Boyd <sboyd@kernel.org>,
	MSM <linux-arm-msm@vger.kernel.org>,
	Andy Gross <andy.gross@linaro.org>,
	Georgi Djakov <georgi.djakov@linaro.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2 1/3] arm64: dts: qcom: msm8998: correct xo clock name
Date: Wed, 5 Dec 2018 10:03:54 -0700	[thread overview]
Message-ID: <05b9ee40-d51e-3f80-8ca4-217fa6fbd51c@codeaurora.org> (raw)
In-Reply-To: <20181205164843.GB1492@tuxbook-pro>

On 12/5/2018 9:48 AM, Bjorn Andersson wrote:
> On Wed 05 Dec 08:38 PST 2018, Jeffrey Hugo wrote:
> 
>> On 12/5/2018 9:12 AM, Marc Gonzalez wrote:
>>> On 15/11/2018 21:44, Jeffrey Hugo wrote:
>>>
>>>> The root parent clock of most msm8998 clock is the "xo" clock.  The DT node
>>>> is incorrectly named "xo_board", which prevents Linux from correctly
>>>> parsing the clock tree, resulting in most clocks being unparented and
>>>> unable to be manipulated.  The end result is that we can't turn on clocks
>>>> for peripherals like SD, so init usually fails.
>>>>
>>>> Fixes: 4807c71cc688 (arm64: dts: Add msm8998 SoC and MTP board support)
>>>> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>>> Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org>
>>>> ---
>>>>    arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
>>>> index 78227cc..a948d4b 100644
>>>> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
>>>> @@ -53,7 +53,7 @@
>>>>    	};
>>>>    	clocks {
>>>> -		xo_board {
>>>> +		xo {
>>>>    			compatible = "fixed-clock";
>>>>    			#clock-cells = <0>;
>>>>    			clock-frequency = <19200000>;
>>>>
>>>
>>> Isn't there going to be a problem for msm8998 in drivers/clk/qcom/clk-smd-rpm.c
>>> which uses "xo_board" for parent_names?
>>
>> Looks like you are right.  This doesn't seem to be the correct way to
>> address the issue then.  I'll have to dig in and take another look.
>>
> 
> The appropriate solution is to describe references between clock drivers
> explicitly in DeviceTree and by that not rely on a global namespace.
> That will also sort out any initialization ordering issues that we might
> have.
> 
> But this task has not yet made it off the todo lists where it lives...

Ok, that sounds great, but doesn't seem like it helps anyone today.

Looks like 8916 and 8974 explicitly register an xo clock off the 
xo_board clock in the respective gcc drivers.  8996 does not, but I 
don't know how functional 8996 really is on mainline.  845 seems to 
register the bi_tcxo clock (the 845 version of xo) in the rpmh driver, 
off of xo_board.

Since both Marc and I are trying to get 8998 to work, it seems like one 
of those two solutions would be workable, short term.

How do you suggest we proceed?

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel