[PATCH v3 03/36] mmc: msm_sdhci: use modern clock handling

[Sumit Garg]

On Thu, 1 Feb 2024 at 20:04, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>
>
>
> On 01/02/2024 08:19, Sumit Garg wrote:
> > On Tue, 30 Jan 2024 at 19:35, Caleb Connolly <caleb.connolly at linaro.org> wrote:
> >>
> >> Use the clk_* helper functions and the correct property name for clocks.
> >
> > This still doesn't handle fixed clocks like in case of QCS404, upstream DT says:
> >
> >                 sdcc1: mmc at 7804000 {
> >                         compatible = "qcom,qcs404-sdhci", "qcom,sdhci-msm-v5";
> >                         ...
> >                         clocks = <&gcc GCC_SDCC1_AHB_CLK>,
> >                                  <&gcc GCC_SDCC1_APPS_CLK>,
> >                                  <&xo_board>;
> >                         clock-names = "iface", "core", "xo";
> >                         ...
> >                 };
> >
> > Due to that "xo" fixed clock we get:
> >
> > MMC:   clk_set_rate(clk=00000000ffaf7348, rate=19200000)
> > Unknown clock id 0
> > clk_set_rate(clk=00000000ffaf7598, rate=19200000)
> > Unknown clock id 0
> > clk_set_rate(clk=00000000ffb08868, rate=400000)
> > mmc at 7804000: 0
>
> Does MMC work? This doesn't look like a probe failure to me.
>

Yeah it's not a breaking change for MMC but rather an annoying one.
MMC works with following DT hack (as per my comments on the other
patch) and defconfig change as follows:

diff --git a/arch/arm/dts/qcs404.dtsi b/arch/arm/dts/qcs404.dtsi
index 2721f32dfb71..5e6705e82837 100644
--- a/arch/arm/dts/qcs404.dtsi
+++ b/arch/arm/dts/qcs404.dtsi
@@ -730,9 +730,7 @@

                tlmm: pinctrl at 1000000 {
                        compatible = "qcom,qcs404-pinctrl";
-                       reg = <0x01000000 0x200000>,
-                             <0x01300000 0x200000>,
-                             <0x07b00000 0x200000>;
+                       reg = <0x01300000 0x200000>;
                        reg-names = "south", "north", "east";
                        interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
                        gpio-ranges = <&tlmm 0 0 120>;
diff --git a/configs/qcom_defconfig b/configs/qcom_defconfig
index 3c6bdc2071b2..3596bc650b41 100644
--- a/configs/qcom_defconfig
+++ b/configs/qcom_defconfig
@@ -43,6 +43,8 @@ CONFIG_BUTTON_KEYBOARD=y
 CONFIG_BUTTON_QCOM_PMIC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_MSM=y
+CONFIG_MMC_HS200_SUPPORT=y
+CONFIG_MMC_SDHCI_ADMA=y
 CONFIG_PHY=y
 CONFIG_PINCTRL=y
 CONFIG_PINCTRL_QCOM_QCS404=y

> But yes this does look odd, somehow qcs404_clk_set_rate() is being
> called with clk->id == 0. The xo_board clock uses a different driver so
> it can't be that...
>
> Could you try and add some more logging around here? I can't see what
> might be causing this.

I tried to dump the call chain as follows. It starts with clk_get_bulk().

clk_get_bulk()
clk_get_by_index()
clk_get_by_index_nodev()
clk_get_by_index_tail()
clk_set_defaults(clock-controller at 1800000)
clk_set_default_rates()
clk_get_by_indexed_prop(dev=00000000ffb05d70, index=0, clk=00000000ffaf7588)
clk_get_by_index_tail()
clk_set_defaults(clock-controller at 1800000)
clk_set_default_rates()
clk_get_by_indexed_prop(dev=00000000ffb05e50, index=0, clk=00000000ffaf7328)
clk_get_by_index_tail()
clk_set_defaults(clock-controller at 1800000)
clk_set_default_rates()
clk_get_by_indexed_prop(dev=00000000ffb05e50, index=0, clk=00000000ffaf7318)
clk_get_by_index_tail()
clk_set_rate(clk=00000000ffaf7318, rate=19200000)
Unknown clock id 0
clk_set_rate(clk=00000000ffaf7588, rate=19200000)
Unknown clock id 0

-Sumit