Re: Odroid-C1 regression with commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending HS CMD13")

Re: Odroid-C1 regression with commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending HS CMD13")

[Linux regression tracking (Thorsten Leemhuis)]

[TLDR: I'm adding this report to the list of tracked Linux kernel
regressions; the text you find below is based on a few templates
paragraphs you might have encountered already in similar form.
See link in footer if these mails annoy you.]

[CCing the regression list, as it should be in the loop for regressions:
https://docs.kernel.org/admin-guide/reporting-regressions.html]

On 11.04.23 01:13, Martin Blumenstingl wrote:
> 
> today I found that upstream commit 4bc31edebde5 ("mmc: core: Set HS
> clock speed before sending HS CMD13") breaks eMMC support on my
> Odroid-C1.
> (yes, I know that I am very late to the party...)
> 
> The .dts for this board is upstream using
> arch/arm/boot/dts/meson8b-odroidc1.dts (&sdhc node)
> The MMC host driver used is drivers/mmc/host/meson-mx-sdhc*.
> 
> The problem I'm seeing is that the eMMC module is simply not detected.
> There are no errors in the kernel log (note that mmc1 is the eMMC
> while mmc0 is the SD card which is attached to another controller on
> that SoC):
> # dmesg | grep mmc
> [    2.742136] meson-mx-sdhc c1108e00.mmc: allocated mmc-pwrseq
> [    4.315905] platform c1108c20.mmc:slot@1: Got CD GPIO
> [    4.450921] mmc0: new high speed SDHC card at address 0001
> [    4.456985] mmcblk0: mmc0:0001 EB1QT 29.8 GiB
> [    4.466108]  mmcblk0: p1
> 
> In this state I get:
> # cat /sys/kernel/debug/mmc1/ios
> clock:          52000000 Hz
> actual clock:   51000000 Hz
> vdd:            21 (3.3 ~ 3.4 V)
> bus mode:       2 (push-pull)
> chip select:    0 (don't care)
> power mode:     2 (on)
> bus width:      3 (8 bits)
> timing spec:    9 (mmc HS200)
> signal voltage: 1 (1.80 V)
> driver type:    0 (driver type B)
> 
> After reverting the mentioned commit I get:
> # dmesg | grep mmc
> [    2.744226] meson-mx-sdhc c1108e00.mmc: allocated mmc-pwrseq
> [    2.970944] mmc1: new HS200 MMC card at address 0001
> [    2.974492] mmcblk1: mmc1:0001 8GND3R 7.28 GiB
> [    2.985126]  mmcblk1: p1
> [    2.987810] mmcblk1boot0: mmc1:0001 8GND3R 4.00 MiB
> [    3.003007] mmcblk1boot1: mmc1:0001 8GND3R 4.00 MiB
> [    4.311754] platform c1108c20.mmc:slot@1: Got CD GPIO
> [    4.374732] mmc0: new high speed SDHC card at address 0001
> [    4.377228] mmcblk0: mmc0:0001 EB1QT 29.8 GiB
> [    4.399152]  mmcblk0: p1
> # cat /sys/kernel/debug/mmc1/ios
> clock:          100000000 Hz
> actual clock:   94444445 Hz
> vdd:            21 (3.3 ~ 3.4 V)
> bus mode:       2 (push-pull)
> chip select:    0 (don't care)
> power mode:     2 (on)
> bus width:      3 (8 bits)
> timing spec:    9 (mmc HS200)
> signal voltage: 1 (1.80 V)
> driver type:    0 (driver type B)
> 
> Please let me know which additional information would be helpful for
> debugging this further.
> Also I'd like to highlight that I'm not blaming the above commit
> (unless I know better). It's entirely possible that the meson-mx-sdhc
> driver (which I wrote) can be at fault and that I was previously just
> lucky.
> 
> [...]
> # first bad commit: [4bc31edebde51fcf8ad0794763b8679a7ecb5ec0] mmc:
> core: Set HS clock speed before sending HS CMD13

Thanks for the report. To be sure the issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
tracking bot:

#regzbot ^introduced 4bc31edebde51fcf8ad079
#regzbot title mmc: core: eMMC support broken on Odroid-C1
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply and tell me -- ideally
while also telling regzbot about it, as explained by the page listed in
the footer of this mail.

Developers: When fixing the issue, remember to add 'Link:' tags pointing
to the report (the parent of this mail). See page linked in footer for
details.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

Re: Odroid-C1 regression with commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending HS CMD13")

[Linux regression tracking (Thorsten Leemhuis)]

Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
for once, to make this easily accessible to everyone.

Ulf, Martin, what happened to this? It looks like we didn't get any
closer to fixing this regression in the last two weeks. Did it fall
through the cracks? Or was progress made and I just missed it?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

On 16.05.23 22:45, Martin Blumenstingl wrote:
> On Mon, May 15, 2023 at 11:44 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> [...]
>>>> 3) If 2) seems to work above, we need to figure out why
>>>> mmc_switch_status() is hanging. If there is a problem with the eMMC
>>>> card responding in-correctly, the host driver should return with an
>>>> error code, right?
>>> You're right: it's indeed hanging in mmc_switch_status()
>>> I don't get any interrupts (timeout, CRC error, ...) for it.
>>> Do you have any suggestions what to check next?
>>
>> So the mmc_switch_status() is sending a CMD13. Even if the card
>> doesn't reply, I would expect that the meson mmc controller would
>> raise some kind of error condition, probably via a timeout-irq.
>>
>> Did you verify that the driver is actually waiting for an IRQ to
>> happen, which also means waiting for a CMD13 response?
> register 0x24 is ICTL (interrupt control) and 0x28 is ISTAT (interrupt status)
> ISTAT is all zeros and ICTL is 0x3067 which translates so:
> - BIT(0): RESP_OK
> - BIT(1): RESP_TIMEOUT
> - BIT(2): RESP_ERR_CRC
> - BIT(5): DATA_TIMEOUT
> - BIT(6): DATA_ERR_CRC
> - BIT(12): RXFIFO_FULL
> - BIT(13): TXFIFO_EMPTY
> 
> I guess in this case BIT(1) RESP_TIMEOUT is the relevant one.
> 
> register 0x04 is SEND and reads 0x4d which translates to:
> - CMD13
> - MMC_RSP_PRESENT (HAS_RESP, BIT(6))
> - no other flags (STOP, R1B, ...) are set
> 
> Full register dump:
> # cat /sys/kernel/debug/regmap/c1108e00.mmc/registers
> 00: 00000900
> 04: 0000004d
> 08: e7ffe002
> 0c: 02f0003f
> 10: 0003f009
> 14: 03b81c00
> 18: 2c43bcf0
> 1c: e0000150
> 20: 00000000
> 24: 00003067
> 28: 00000000
> 2c: 00000000
> 30: 00000000
> 34: 00fe0cff
> 38: 0000100b
> 
> In case you are curious, the driver is: drivers/mmc/host/meson-mx-sdhc-mmc.c
> 
> 
> Best regards,
> Martin

Re: Odroid-C1 regression with commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending HS CMD13")

[Martin Blumenstingl]

Hi Thorsten,

On Fri, Jun 2, 2023 at 3:17 PM Linux regression tracking (Thorsten
Leemhuis) <regressions@leemhuis.info> wrote:
>
> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
> for once, to make this easily accessible to everyone.
>
> Ulf, Martin, what happened to this? It looks like we didn't get any
> closer to fixing this regression in the last two weeks. Did it fall
> through the cracks? Or was progress made and I just missed it?
Life got in the way so there's no progress on my end.
I'm hoping that Ulf has some more ideas that I can work with though.


Best regards,
Martin

Re: Odroid-C1 regression with commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending HS CMD13")

[Linux regression tracking (Thorsten Leemhuis)]

Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
for once, to make this easily accessible to everyone.

I still have this regression (caused by a commit from Brian, applied by
Ulf) on my list of tracked issues. Was there any progress to finally get
this fixed[1]? Doesn't look like it from here, but I might have easily
missed something, that's why I'm asking.

[1] this patch from Ziyang Huang afaics was supposed to help, but it
went nowhere afaics
https://lore.kernel.org/linux-amlogic/TYZPR01MB5556B56D834E02F41C44D81DC95FA@TYZPR01MB5556.apcprd01.prod.exchangelabs.com/

Or does anybody stop caring?

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

#regzbot poke

On 20.06.23 11:53, Ulf Hansson wrote:
> On Mon, 19 Jun 2023 at 21:54, Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
>>
>> On Wed, Jun 14, 2023 at 5:49 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> [...]
>>>> Full register dump:
>>>> # cat /sys/kernel/debug/regmap/c1108e00.mmc/registers
>>>> 00: 00000900
>>>> 04: 0000004d
>>>> 08: e7ffe002
>>>> 0c: 02f0003f
>>>> 10: 0003f009
>>>> 14: 03b81c00
>>>> 18: 2c43bcf0
>>>> 1c: e0000150
>>>> 20: 00000000
>>>> 24: 00003067
>>>> 28: 00000000
>>>> 2c: 00000000
>>>> 30: 00000000
>>>> 34: 00fe0cff
>>>> 38: 0000100b
>>>>
>>>> In case you are curious, the driver is: drivers/mmc/host/meson-mx-sdhc-mmc.c
>>>
>>> Thanks for sharing this data!
>>>
>>> I assume the above registers indicate that we have sent the command
>>> and are now waiting for an IRQ for a response/error, but we never
>>> receive one.
>>>
>>> To really figure out what is going on, I think we need to do some
>>> additional low level debugging/testing.
>>>
>>> I was looking at the commit message from e4bf1b0970ef ("mmc: host:
>>> meson-mx-sdhc: new driver for the Amlogic Meson SDHC host"), which
>>> indicates that the clock management is quite limited for this HW. For
>>> example, the 51000000Hz isn't one of the supported frequencies. Could
>>> that be the reason for the problem? Perhaps if we play with changing
>>> the frequency to something that is considered supported - then can we
>>> make this work?
>> You seem to be more familiar with this Amlogic MMC controller than I am ;-)
> 
> Sometimes a well written commit message actually helps. :-)
> 
>> Today I finally had some time for testing and when I started Ziyang
>> Huang provided a patch [0] (admittedly: I think it needs to be
>> improved, but finally we know that it's a MMC controller driver
>> limitation and not an MMC core bug)
>
> Great news!
> 
> I will continue to monitor the thread and defer to apply anything
> until you have given it your blessing, of course.
> 
> Kind regards
> Uffe

Re: Odroid-C1 regression with commit 4bc31edebde5 ("mmc: core: Set HS clock speed before sending HS CMD13")

[Thorsten Leemhuis]

On 29.08.23 11:59, Linux regression tracking (Thorsten Leemhuis) wrote:
> Hi, Thorsten here, the Linux kernel's regression tracker. Top-posting
> for once, to make this easily accessible to everyone.
> 
> I still have this regression (caused by a commit from Brian, applied by
> Ulf) on my list of tracked issues. Was there any progress to finally get
> this fixed[1]? Doesn't look like it from here, but I might have easily
> missed something, that's why I'm asking.
> 
> [1] this patch from Ziyang Huang afaics was supposed to help, but it
> went nowhere afaics
> https://lore.kernel.org/linux-amlogic/TYZPR01MB5556B56D834E02F41C44D81DC95FA@TYZPR01MB5556.apcprd01.prod.exchangelabs.com/
> 
> Or does anybody stop caring?

No reply, then I assume that's the case. In that case I won't waste any
cycles tracking this:

#regzbot inconclusive: seem nobody cares anymore

Martin, if this is wrong and you want to see this fixed, please speak up.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

> On 20.06.23 11:53, Ulf Hansson wrote:
>> On Mon, 19 Jun 2023 at 21:54, Martin Blumenstingl
>> <martin.blumenstingl@googlemail.com> wrote:
>>>
>>> On Wed, Jun 14, 2023 at 5:49 PM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>> [...]
>>>>> Full register dump:
>>>>> # cat /sys/kernel/debug/regmap/c1108e00.mmc/registers
>>>>> 00: 00000900
>>>>> 04: 0000004d
>>>>> 08: e7ffe002
>>>>> 0c: 02f0003f
>>>>> 10: 0003f009
>>>>> 14: 03b81c00
>>>>> 18: 2c43bcf0
>>>>> 1c: e0000150
>>>>> 20: 00000000
>>>>> 24: 00003067
>>>>> 28: 00000000
>>>>> 2c: 00000000
>>>>> 30: 00000000
>>>>> 34: 00fe0cff
>>>>> 38: 0000100b
>>>>>
>>>>> In case you are curious, the driver is: drivers/mmc/host/meson-mx-sdhc-mmc.c
>>>>
>>>> Thanks for sharing this data!
>>>>
>>>> I assume the above registers indicate that we have sent the command
>>>> and are now waiting for an IRQ for a response/error, but we never
>>>> receive one.
>>>>
>>>> To really figure out what is going on, I think we need to do some
>>>> additional low level debugging/testing.
>>>>
>>>> I was looking at the commit message from e4bf1b0970ef ("mmc: host:
>>>> meson-mx-sdhc: new driver for the Amlogic Meson SDHC host"), which
>>>> indicates that the clock management is quite limited for this HW. For
>>>> example, the 51000000Hz isn't one of the supported frequencies. Could
>>>> that be the reason for the problem? Perhaps if we play with changing
>>>> the frequency to something that is considered supported - then can we
>>>> make this work?
>>> You seem to be more familiar with this Amlogic MMC controller than I am ;-)
>>
>> Sometimes a well written commit message actually helps. :-)
>>
>>> Today I finally had some time for testing and when I started Ziyang
>>> Huang provided a patch [0] (admittedly: I think it needs to be
>>> improved, but finally we know that it's a MMC controller driver
>>> limitation and not an MMC core bug)
>>
>> Great news!
>>
>> I will continue to monitor the thread and defer to apply anything
>> until you have given it your blessing, of course.
>>
>> Kind regards
>> Uffe