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

From: Andy Gross <andy.gross@linaro.org>
To: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Marc Gonzalez <marc.w.gonzalez@free.fr>,
	Stephen Boyd <sboyd@kernel.org>,
	MSM <linux-arm-msm@vger.kernel.org>,
	Bjorn Andersson <bjorn.andersson@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: Fri, 7 Dec 2018 12:23:24 -0600	[thread overview]
Message-ID: <20181207182324.GD5497@hector.attlocal.net> (raw)
In-Reply-To: <8a1ec3da-0c07-7c05-cf5b-f1fef001c972@codeaurora.org>

On Fri, Dec 07, 2018 at 07:58:52AM -0700, Jeffrey Hugo wrote:
> On 12/7/2018 2:03 AM, Marc Gonzalez wrote:
> >On 06/12/2018 19:34, Stephen Boyd wrote:
> >>Quoting Jeffrey Hugo (2018-12-05 15:04:01)
> >>>On 12/5/2018 2:42 PM, Stephen Boyd wrote:
> >>>>Quoting Jeffrey Hugo (2018-12-05 13:20:07)
> >>>>>On 12/5/2018 2:04 PM, Stephen Boyd wrote:
> >>>>>>Quoting Jeffrey Hugo (2018-12-05 09:03:54)
> >>>>>>
> >>>>>>I don't quite understand the patch in general. The xo_board clk should
> >>>>>>always exist in DT and the fixed factor clk in GCC is there until the
> >>>>>>rpm clk driver can control the XO clk state vote for the kernel.
> >>>>>
> >>>>>Sorry, this wasn't apparent.  It doesn't seem like this "requirement" is
> >>>>>captured anywhere.
> >>>>
> >>>>Agreed!
> >>>>
> >>>>>
> >>>>>As far as the SD clocks are concerned, they are defined in GCC, and
> >>>>>eventually have a root parent called "xo".  "xo" isn't defined anywhere,
> >>>>>so the SD clocks can't really be used, and the hardware doesn't come up.
> >>>>>    This patch "fixed" that, but I missed the link to the rpm driver that
> >>>>>Marc pointed out.
> >>>>
> >>>>Hmm ok. The SD DT node should just point to the xo_board clk for now and
> >>>>later on it can be changed to use the rpm clk when the rpm node is
> >>>>created.
> >>>>
> >>>>>
> >>>>>
> >>>>>>
> >>>>>>If anything, change the DT node to be named xo-board instead of xo_board
> >>>>>>because that matches DT naming schemes and then add a clock-output-names
> >>>>>>= "xo_board" property to it so that we keep the underscore.
> >>>>>
> >>>>>I see this now, and I agree with it, but then SD goes back to a broken
> >>>>>state because there is "xo" clock for GCC.  Its not quite clear to me
> >>>>>how to make GCC (and thus SD) happy again with this change reverted/fixed.
> >>>>>
> >>>>>Bjorn mentioned offline he is going to take a look, but he has a few
> >>>>>other things on his plate first.
> >>>>>
> >>>>
> >>>>There is an XO clk created in drivers/clk/qcom/gcc-msm8996.c, we should
> >>>>do the same here until rpm can handle this. I'll pack this patch up and
> >>>>merge it to clk-next soon.
> >>>
> >>>Thanks.  I pulled in the below change into my tree, and fixed up the DT
> >>>based on the discussion we had.  SD works, and things look sane to me
> >>>per clk_summary in debugfs.
> >>>
> >>>Feel free to throw my tested-by on if you want.
> >>
> >>Thanks. I did so and merged it up to clk-next.
> >
> >@Andy, don't you still need to revert 634da3307b083ee83eb9b377081fdfd6416a148a
> >("arm64: dts: qcom: msm8998: correct xo clock name") in for-next?

I'll send a fix for this.  I think we want to keep the xo label, and just change
it to xo: xo_board.  So instead of a revert, it'll be that + a Fixes line.

That ok?

Andy

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