linux-kernel - Re: [PATCH] mmc: meson-mx-sdhc: Fix initialization frozen issue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <TYZPR01MB555645E1C1100ED41B3D02D5C93EA@TYZPR01MB5556.apcprd01.prod.exchangelabs.com>
Date:   Fri, 21 Jul 2023 01:45:01 +0800
From:   Ziyang Huang <hzyitc@...look.com>
To:     Martin Blumenstingl <martin.blumenstingl@...glemail.com>
Cc:     jbrunet@...libre.com, khilman@...libre.com,
        linux-amlogic@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-mmc@...r.kernel.org, neil.armstrong@...aro.org,
        ulf.hansson@...aro.org
Subject: Re: [PATCH] mmc: meson-mx-sdhc: Fix initialization frozen issue

在 2023/6/20 3:51, Martin Blumenstingl 写道:
> Hello,
> 
> first of all: thank you for this patch!
> 
> On Mon, Jun 19, 2023 at 7:36 PM Ziyang Huang <hzyitc@...look.com> wrote:
>>
>> 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).

Sorry for the later reply. I'm so busy these day.

After checking the code, I found the following flow, so I think 
tuningshould work for all cards.

   mmc_start_request()
     -> __mmc_start_request()
       -> mmc_retune()
         -> mmc_execute_tuning()          -> host->ops->execute_tuning()

And yes, 1 may be not a good choice. I consider 3 values:
   - 1
   - div_val / 2
   - div_val - 1

Maybe, (div_val / 2) is a good choice. How do you think?

> 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...).

With some modifications, I found that the mmc controller of S805 can 
work with all frequency, not only those mentioned in commit 
e4bf1b0970ef("mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson 
SDHC host"). It seem like that every things work fine. I will use a 
oscilloscope to confirm the hardware clock is correct when I have time.

For this future commit, I don't want to add a special case.

> Best regards,
> Martin
> 
> 
> [0] https://lore.kernel.org/linux-amlogic/CAFBinCD0RT0p-jk86W0JuMT3ufohRh1RqWCcM35DKZJpuc10HQ@mail.gmail.com/
> [1] https://lore.kernel.org/linux-amlogic/CAPDyKFpS-UwiaRPMqSpX0mNPrS5p=yJzu3g0=pGyCkWHSYyqWg@mail.gmail.com/