Re: [PATCH v3] arm64: dts: qcom: sc8280xp: correct ref_aux clock for ufs_mem_phy

[Johan Hovold]

On Tue, Aug 30, 2022 at 02:01:20PM -0400, Brian Masney wrote:
> The first UFS host controller fails to start on the SA8540P automotive
> board (QDrive3) due to the following errors:
> 
>     ufshcd-qcom 1d84000.ufs: ufshcd_query_flag: Sending flag query for idn 18 failed, err = 253
>     ufshcd-qcom 1d84000.ufs: ufshcd_query_flag: Sending flag query for idn 18 failed, err = 253
>     ufshcd-qcom 1d84000.ufs: ufshcd_query_flag: Sending flag query for idn 18 failed, err = 253
>     ufshcd-qcom 1d84000.ufs: ufshcd_query_flag_retry: query attribute, opcode 5, idn 18, failed
>         with error 253 after 3 retries
> 
> The system eventually fails to boot with the warning:
> 
>     gcc_ufs_phy_axi_clk status stuck at 'off'
> 
> This issue can be worked around by adding clk_ignore_unused to the
> kernel command line since the system firmware sets up this clock for us.
> 
> Let's fix this issue by updating the ref clock on ufs_mem_phy. Note
> that the downstream MSM 5.4 sources list this as ref_clk_parent. With
> this patch, the SA8540P is able to be booted without clk_ignore_unused.

You should fix the Subject which still refers to the "ref_aux" clock.

> Signed-off-by: Brian Masney <bmasney@xxxxxxxxxx>
> Fixes: 152d1faf1e2f3 ("arm64: dts: qcom: add SC8280XP platform")

I can confirm that this is needed also for sc8280xp-crd and sa8295p-adp,
so with Subject fixed:

Tested-by: Johan Hovold <johan+linaro@xxxxxxxxxx>
Reviewed-by: Johan Hovold <johan+linaro@xxxxxxxxxx>

> ---
> v2 of this patch can be found at
> https://lore.kernel.org/lkml/20220825163755.683843-1-bmasney@xxxxxxxxxx/T/#u
> 
> v1 of this patch can be found at
> https://lore.kernel.org/lkml/20220623142837.3140680-1-bmasney@xxxxxxxxxx/T/#u
> 
> Note that there's also a similar issue with the second UFS controller
> (ufs_card_hc) since it separately fails with:
> 
>     ufshcd-qcom 1da4000.ufs: Controller enable failed
>     ufshcd-qcom 1da4000.ufs: link startup failed 1
>     ...
>     gcc_ufs_card_axi_clk status stuck at 'off'
> 
> We are currently disabling the second UFS host controller (ufs_card_hc) in
> our DTS at the moment. I haven't had any luck so far tracking this one
> down and it's particularly tough without docs access.

Yeah, the lack of documentation is a pain. I took a closer look at this
today, and the CRD ACPI tables only appear to enable
GCC_UFS_REF_CLKREF_CLK even if GCC_UFS_1_CARD_CLKREF_CLK is also left
enabled by the boot firmware (note that that's "UFS_1_CARD" and not
"UFS_CARD" as the vendor dts uses for the first controller).

Neither card reference clock appears to be needed for the controllers on
the CRD and ADP so I think we should go with this approach also for the
second controller for now. If it turns out some platform actually needs
the card refclocks we should describe the relationship to
GCC_UFS_REF_CLKREF_CLK in the clock driver instead.

Input from Qualcomm on this would be great too.

>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index 49ea8b5612fc..2bdde4b8f72b 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> @@ -891,7 +891,7 @@ ufs_mem_phy: phy@1d87000 {
>  			ranges;
>  			clock-names = "ref",
>  				      "ref_aux";
> -			clocks = <&rpmhcc RPMH_CXO_CLK>,
> +			clocks = <&gcc GCC_UFS_REF_CLKREF_CLK>,
>  				 <&gcc GCC_UFS_PHY_PHY_AUX_CLK>;
>  
>  			resets = <&ufs_mem_hc 0>;

Johan