Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <TYZPR01MB5556B56D834E02F41C44D81DC95FA@TYZPR01MB5556.apcprd01.prod.exchangelabs.com>
In-Reply-To: <TYZPR01MB5556B56D834E02F41C44D81DC95FA@TYZPR01MB5556.apcprd01.prod.exchangelabs.com>
From:   Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date:   Mon, 19 Jun 2023 21:51:08 +0200
Message-ID: <CAFBinCAJ1E6JKmFTuaJwGpd_MBzHMZ0mMj-1AE3TNeB2_72nZA@mail.gmail.com>
Subject: Re: [PATCH] mmc: meson-mx-sdhc: Fix initialization frozen issue
To:     Ziyang Huang <hzyitc@outlook.com>
Cc:     ulf.hansson@linaro.org, neil.armstrong@linaro.org,
        khilman@baylibre.com, jbrunet@baylibre.com,
        linux-mmc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hello,

first of all: thank you for this patch!

On Mon, Jun 19, 2023 at 7:36=E2=80=AFPM Ziyang Huang <hzyitc@outlook.com> w=
rote:
>
> Commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending
> HS CMD13") set HS clock (52MHz) before switching to HS mode. For this
> freq, FCLK_DIV5 will be selected and div value is 10 (reg value is 9).
> Then we set rx_clk_phase to 11 or 15 which is out of range and make
> hardware frozen. After we send command request, no irq will be
> interrupted and the mmc driver will keep to wait for request finished,
> even durning rebooting.
I think this is the exact same problem I reported some days ago: [0]
Ulf is questioning whether we properly support 52MHz clocks correctly,
so I think you're onto something!

So this is an excellent finding! I can confirm that using rx_clk_phase
of 1 makes my Odroid-C1 eMMC work again :-)

> So let's set a common value - 1 just for initialization. Then let
> meson_mx_sdhc_execute_tuning() to find the accurate value for data
> transfer.
As far as I know unconditionally using value 1 can negatively affect
other devices.
I'm assuming that you're testing on an Odroid-C1 or similar board with
HS200 eMMC:
On those SoC + eMMC combinations we do support. But on other boards
(for example Meson8b EC-100 / Endless Mini) there's no HS200 support
because the eMMC is connected with 3.3V IO lines. So tuning is not
executed there (if I recall correctly).

What do you think about adding a special case for the 51MHz "actual
clock rate" and adding a comment that it was found by manual testing?
For some reason (that I don't understand) Amlogic's vendor driver
maxes out at 47.22MHz (presumably because they limit themselves to
using FCLK_DIV3 as input only - but I don't get why...).


Best regards,
Martin


[0] https://lore.kernel.org/linux-amlogic/CAFBinCD0RT0p-jk86W0JuMT3ufohRh1R=
qWCcM35DKZJpuc10HQ@mail.gmail.com/
[1] https://lore.kernel.org/linux-amlogic/CAPDyKFpS-UwiaRPMqSpX0mNPrS5p=3Dy=
Jzu3g0=3DpGyCkWHSYyqWg@mail.gmail.com/