Received-SPF: pass (google.com: domain of linux-kernel+bounces-164738-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <6ba2967c6c9d24e3f1c9b76496176010.sboyd@kernel.org>
Content-Type: text/plain; charset="utf-8"
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <11bd7146-30cd-4b71-b2ca-d76875763731@linaro.org>
References: <20240427-topic-8450sdc2-v1-1-631cbb59e0e5@linaro.org> <2337ba58adb3fb127710bead9b8665a9.sboyd@kernel.org> <11bd7146-30cd-4b71-b2ca-d76875763731@linaro.org>
Subject: Re: [PATCH] clk: qcom: gcc-sm8450: set OPS_PARENT_ENABLE on gcc_sdcc2_apps_clk_src
From: Stephen Boyd <sboyd@kernel.org>
Cc: Marijn Suijten <marijn.suijten@somainline.org>, linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Bjorn Andersson <andersson@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, Michael Turquette <mturquette@baylibre.com>, Vinod Koul <vkoul@kernel.org>
Date: Tue, 30 Apr 2024 14:26:53 -0700
User-Agent: alot/0.10

Quoting Konrad Dybcio (2024-04-30 03:46:52)
> On 30.04.2024 2:21 AM, Stephen Boyd wrote:
> > Quoting Konrad Dybcio (2024-04-27 05:01:07)
> >> Similar to how it works on other SoCs, the top frequency of the SDHCI2
> >> core clock is generated by a separate PLL (peculiar design choice) that
> >> is not guaranteed to be enabled (why does the clock framework not hand=
le
> >> this by default?).
> >>
> >> Add the CLK_OPS_PARENT_ENABLE flag to make sure we're not muxing the
> >> RCG input to a dormant source.
> >=20
> > The RCG2 hardware hasn't required the parent to be enabled for clk
> > operations besides for the glitch-free source switch. What scenario is
> > happening here that's requiring this flag? Is the RCG forcibly enabled
> > perhaps because the bootloader has left the root enable bit set
> > (CMD_ROOT_EN)? Or are we changing the parent while the clk framework
> > thinks the clk is off when it is actually on?
> >=20
> > TL;DR: This is papering over a bigger bug.
>=20
> Definitely.
>=20
>=20
> Take a look at:
>=20
> static const struct freq_tbl ftbl_gcc_sdcc2_apps_clk_src[] =3D {
>         F(400000, P_BI_TCXO, 12, 1, 4),
>         F(25000000, P_GCC_GPLL0_OUT_EVEN, 12, 0, 0),
>         F(50000000, P_GCC_GPLL0_OUT_EVEN, 6, 0, 0),
>         F(100000000, P_GCC_GPLL0_OUT_EVEN, 3, 0, 0),
>         F(202000000, P_GCC_GPLL9_OUT_MAIN, 4, 0, 0),
>         { }
> };
>=20
> XO and GPLL0 are more or less always on, but GPLL9 is described to only
> be used for this specific clock for this specific frequency (perhaps it
> feeds something else on the soc but that's besides the point).
>=20
> Then, the parent input is changed during set_rate, but GPLL9 seems to
> never be enabled:

Is the sdcc2 RCG enabled during the set_rate?