IMX8MM SD UHS support

IMX8MM SD UHS support

[Tim Harvey]

Greetings,

In 50b1a69cee0d ("ARM: dts: imx8m: add UHS or HS400/HS400ES
properties") u-boot dt props were added to enable UHS and HS400 on a
couple of IMX8MM boards including the imx8mm-evk and in the subsequent
patch enabled the config items.

While I see this making a huge difference for eMMC performance in
U-Boot I find it doesn't do anything for microSD performance.

The issue appears to be that sd_get_capabilities() is not
appropriately detecting UHS speeds on cards that appropriately detect
as SDR104/DDR50 in Linux:

u-boot=> mmc info
Device: FSL_SDHC
Manufacturer ID: 1b
OEM: 534d
Name: 00000
Bus Speed: 50000000
Mode: SD High Speed (50MHz)
card capabilities: widths [4, 1] modes [MMC legacy, SD High Speed (50MHz)]
^^^^ no SDR104 detected for this card
host capabilities: widths [4, 1] modes [MMC legacy, MMC High Speed
(26MHz), SD High Speed (50MHz), MMC High Speed (52MHz), UHS DDR50
(50MHz), UHS SDR104 (208MHz)]
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 14.9 GiB
Bus Width: 4-bit
Erase Group Size: 512 Bytes

The same card in Linux shows the following upon insertion and
performance tests show that it is operating at SDR104 speeds:
mmc1: new ultra high speed SDR104 SDHC card at address 0001

I haven't found very good documentation on the SD switch settings to
understand if something is wrong in the U-Boot implementation of of
sd_get_capabilities() and I suppose it also could be an issue in
sdhci-esdhc-imx.c.

Any ideas where to look?

Anyone see SDR104 cards being detected properly for other platforms?

Best regards,

Tim

IMX8MM SD UHS support

[Jaehoon Chung]

Hi,

On 12/30/20 8:21 AM, Tim Harvey wrote:
> Greetings,
> 
> In 50b1a69cee0d ("ARM: dts: imx8m: add UHS or HS400/HS400ES
> properties") u-boot dt props were added to enable UHS and HS400 on a
> couple of IMX8MM boards including the imx8mm-evk and in the subsequent
> patch enabled the config items.
> 
> While I see this making a huge difference for eMMC performance in
> U-Boot I find it doesn't do anything for microSD performance.
> 
> The issue appears to be that sd_get_capabilities() is not
> appropriately detecting UHS speeds on cards that appropriately detect
> as SDR104/DDR50 in Linux:
> 
> u-boot=> mmc info
> Device: FSL_SDHC
> Manufacturer ID: 1b
> OEM: 534d
> Name: 00000
> Bus Speed: 50000000
> Mode: SD High Speed (50MHz)
> card capabilities: widths [4, 1] modes [MMC legacy, SD High Speed (50MHz)]
> ^^^^ no SDR104 detected for this card

Did you enable MMC_UHS_SUPPORT?

> host capabilities: widths [4, 1] modes [MMC legacy, MMC High Speed
> (26MHz), SD High Speed (50MHz), MMC High Speed (52MHz), UHS DDR50
> (50MHz), UHS SDR104 (208MHz)]
> Rd Block Len: 512
> SD version 3.0
> High Capacity: Yes
> Capacity: 14.9 GiB
> Bus Width: 4-bit
> Erase Group Size: 512 Bytes
> 
> The same card in Linux shows the following upon insertion and
> performance tests show that it is operating at SDR104 speeds:
> mmc1: new ultra high speed SDR104 SDHC card at address 0001
> 
> I haven't found very good documentation on the SD switch settings to
> understand if something is wrong in the U-Boot implementation of of
> sd_get_capabilities() and I suppose it also could be an issue in
> sdhci-esdhc-imx.c.
> 
> Any ideas where to look?
> 
> Anyone see SDR104 cards being detected properly for other platforms?

I remembered that SDR104 was working fine before. 
Will check whether it's working or not with my other targets.

Best Regards,
Jaehoon Chung

> 
> Best regards,
> 
> Tim
> 

IMX8MM SD UHS support

[Tim Harvey]

On Tue, Dec 29, 2020 at 3:43 PM Jaehoon Chung <jh80.chung@samsung.com> wrote:
>
> Hi,
>
> On 12/30/20 8:21 AM, Tim Harvey wrote:
> > Greetings,
> >
> > In 50b1a69cee0d ("ARM: dts: imx8m: add UHS or HS400/HS400ES
> > properties") u-boot dt props were added to enable UHS and HS400 on a
> > couple of IMX8MM boards including the imx8mm-evk and in the subsequent
> > patch enabled the config items.
> >
> > While I see this making a huge difference for eMMC performance in
> > U-Boot I find it doesn't do anything for microSD performance.
> >
> > The issue appears to be that sd_get_capabilities() is not
> > appropriately detecting UHS speeds on cards that appropriately detect
> > as SDR104/DDR50 in Linux:
> >
> > u-boot=> mmc info
> > Device: FSL_SDHC
> > Manufacturer ID: 1b
> > OEM: 534d
> > Name: 00000
> > Bus Speed: 50000000
> > Mode: SD High Speed (50MHz)
> > card capabilities: widths [4, 1] modes [MMC legacy, SD High Speed (50MHz)]
> > ^^^^ no SDR104 detected for this card
>
> Did you enable MMC_UHS_SUPPORT?

yes, I'm using master so I also have your patch that enables that:
e601f0f9c966 ("configs: imx8m: enable eMMC HS400ES and SD UHS mode on EVK")

>
> > host capabilities: widths [4, 1] modes [MMC legacy, MMC High Speed
> > (26MHz), SD High Speed (50MHz), MMC High Speed (52MHz), UHS DDR50
> > (50MHz), UHS SDR104 (208MHz)]
> > Rd Block Len: 512
> > SD version 3.0
> > High Capacity: Yes
> > Capacity: 14.9 GiB
> > Bus Width: 4-bit
> > Erase Group Size: 512 Bytes
> >
> > The same card in Linux shows the following upon insertion and
> > performance tests show that it is operating at SDR104 speeds:
> > mmc1: new ultra high speed SDR104 SDHC card at address 0001
> >
> > I haven't found very good documentation on the SD switch settings to
> > understand if something is wrong in the U-Boot implementation of of
> > sd_get_capabilities() and I suppose it also could be an issue in
> > sdhci-esdhc-imx.c.
> >
> > Any ideas where to look?
> >
> > Anyone see SDR104 cards being detected properly for other platforms?
>
> I remembered that SDR104 was working fine before.
> Will check whether it's working or not with my other targets.
>

The way to check is simply do an 'mmc info' and see if the card
capabilities show SDR104 or DDR50 (depending on the capabilities of
your card)

Let me know what you find. Again, I'm seeing this with master on an
imx8mm-evk and the host shows those capabilities, its the card that
does not. The code in sd_get_capabilities() is ancient so I'm guessing
it may be out of date.

Thanks,

Tim

IMX8MM SD UHS support

[ZHIZHIKIN Andrey]

Hello Tim,

> -----Original Message-----
> From: Tim Harvey <tharvey@gateworks.com>
> Sent: Wednesday, December 30, 2020 12:22 AM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; Peng Fan
> <Peng.Fan@nxp.com>; u-boot <u-boot@lists.denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>; Stefano Babic <sbabic@denx.de>
> Subject: IMX8MM SD UHS support
> 
> 
> Greetings,
> 
> In 50b1a69cee0d ("ARM: dts: imx8m: add UHS or HS400/HS400ES
> properties") u-boot dt props were added to enable UHS and HS400 on a couple of
> IMX8MM boards including the imx8mm-evk and in the subsequent patch
> enabled the config items.
> 
> While I see this making a huge difference for eMMC performance in U-Boot I find
> it doesn't do anything for microSD performance.
> 
> The issue appears to be that sd_get_capabilities() is not appropriately detecting
> UHS speeds on cards that appropriately detect as SDR104/DDR50 in Linux:
> 
> u-boot=> mmc info
> Device: FSL_SDHC
> Manufacturer ID: 1b
> OEM: 534d
> Name: 00000
> Bus Speed: 50000000
> Mode: SD High Speed (50MHz)
> card capabilities: widths [4, 1] modes [MMC legacy, SD High Speed (50MHz)] ^^^^
> no SDR104 detected for this card host capabilities: widths [4, 1] modes [MMC
> legacy, MMC High Speed (26MHz), SD High Speed (50MHz), MMC High Speed
> (52MHz), UHS DDR50 (50MHz), UHS SDR104 (208MHz)] Rd Block Len: 512 SD
> version 3.0 High Capacity: Yes
> Capacity: 14.9 GiB
> Bus Width: 4-bit
> Erase Group Size: 512 Bytes
> 
> The same card in Linux shows the following upon insertion and performance tests
> show that it is operating at SDR104 speeds:
> mmc1: new ultra high speed SDR104 SDHC card at address 0001
> 
> I haven't found very good documentation on the SD switch settings to
> understand if something is wrong in the U-Boot implementation of of
> sd_get_capabilities() and I suppose it also could be an issue in sdhci-esdhc-imx.c.

In order to SD Card to operate in High Speed modes, the signaling voltage should
be switched from 3v3 to 1v8. Otherwise, higher clock rates cannot be applied and
card continues to be operating with lower speed modes.

Have you verified that the NVCC_SD2 is switched to 1v8 in your case?

Since you did not indicate which board you're using - it is hard for me to suggest
on how measure this voltages. If you're using i.MX8M Mini EVK, then you can
verify this by measuring the voltage at the test point called "NVCC_SD2" located
on the Core Module in the PMIC area.

> 
> Any ideas where to look?
> 
> Anyone see SDR104 cards being detected properly for other platforms?

Yes, in fact I do see that SD Card is recognized for me as UHS SD104:

u-boot=> mmc dev 1
Run CMD11 1.8V switch
switch to partitions #0, OK
mmc1 is current device
u-boot=> mmc info
Device: FSL_SDHC
Manufacturer ID: 9f
OEM: 5449
Name: 00000
Bus Speed: 200000000
Mode: UHS SDR104 (208MHz)
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 29.8 GiB
Bus Width: 4-bit
Erase Group Size: 512 Bytes

Please pay attention to the output, which says "Run CMD11 1.8V switch".

Do you also observe the same output?

> 
> Best regards,
> 
> Tim

-- andrey

IMX8MM SD UHS support

[Tim Harvey]

On Wed, Dec 30, 2020 at 1:30 AM ZHIZHIKIN Andrey
<andrey.zhizhikin@leica-geosystems.com> wrote:
>
> Hello Tim,
>
> > -----Original Message-----
> > From: Tim Harvey <tharvey@gateworks.com>
> > Sent: Wednesday, December 30, 2020 12:22 AM
> > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>; Peng Fan
> > <Peng.Fan@nxp.com>; u-boot <u-boot@lists.denx.de>
> > Cc: Fabio Estevam <festevam@gmail.com>; Stefano Babic <sbabic@denx.de>
> > Subject: IMX8MM SD UHS support
> >
> >
> > Greetings,
> >
> > In 50b1a69cee0d ("ARM: dts: imx8m: add UHS or HS400/HS400ES
> > properties") u-boot dt props were added to enable UHS and HS400 on a couple of
> > IMX8MM boards including the imx8mm-evk and in the subsequent patch
> > enabled the config items.
> >
> > While I see this making a huge difference for eMMC performance in U-Boot I find
> > it doesn't do anything for microSD performance.
> >
> > The issue appears to be that sd_get_capabilities() is not appropriately detecting
> > UHS speeds on cards that appropriately detect as SDR104/DDR50 in Linux:
> >
> > u-boot=> mmc info
> > Device: FSL_SDHC
> > Manufacturer ID: 1b
> > OEM: 534d
> > Name: 00000
> > Bus Speed: 50000000
> > Mode: SD High Speed (50MHz)
> > card capabilities: widths [4, 1] modes [MMC legacy, SD High Speed (50MHz)] ^^^^
> > no SDR104 detected for this card host capabilities: widths [4, 1] modes [MMC
> > legacy, MMC High Speed (26MHz), SD High Speed (50MHz), MMC High Speed
> > (52MHz), UHS DDR50 (50MHz), UHS SDR104 (208MHz)] Rd Block Len: 512 SD
> > version 3.0 High Capacity: Yes
> > Capacity: 14.9 GiB
> > Bus Width: 4-bit
> > Erase Group Size: 512 Bytes
> >
> > The same card in Linux shows the following upon insertion and performance tests
> > show that it is operating at SDR104 speeds:
> > mmc1: new ultra high speed SDR104 SDHC card at address 0001
> >
> > I haven't found very good documentation on the SD switch settings to
> > understand if something is wrong in the U-Boot implementation of of
> > sd_get_capabilities() and I suppose it also could be an issue in sdhci-esdhc-imx.c.
>
> In order to SD Card to operate in High Speed modes, the signaling voltage should
> be switched from 3v3 to 1v8. Otherwise, higher clock rates cannot be applied and
> card continues to be operating with lower speed modes.
>
> Have you verified that the NVCC_SD2 is switched to 1v8 in your case?
>
> Since you did not indicate which board you're using - it is hard for me to suggest
> on how measure this voltages. If you're using i.MX8M Mini EVK, then you can
> verify this by measuring the voltage at the test point called "NVCC_SD2" located
> on the Core Module in the PMIC area.
>
> >
> > Any ideas where to look?
> >
> > Anyone see SDR104 cards being detected properly for other platforms?
>
> Yes, in fact I do see that SD Card is recognized for me as UHS SD104:
>
> u-boot=> mmc dev 1
> Run CMD11 1.8V switch
> switch to partitions #0, OK
> mmc1 is current device
> u-boot=> mmc info
> Device: FSL_SDHC
> Manufacturer ID: 9f
> OEM: 5449
> Name: 00000
> Bus Speed: 200000000
> Mode: UHS SDR104 (208MHz)
> Rd Block Len: 512
> SD version 3.0
> High Capacity: Yes
> Capacity: 29.8 GiB
> Bus Width: 4-bit
> Erase Group Size: 512 Bytes
>
> Please pay attention to the output, which says "Run CMD11 1.8V switch".
>
> Do you also observe the same output?
>

Andrey,

I did mention that I am using the imx8mm-evk. When I saw that my
custom board was having issues with sd_get_capabilities() I switched
to the imx8mm-evk and confirmed my findings there.

I'm using master (ab865a8ee5c1) with imx8mm_evk_defconfig running on
an imx8mm-evk board configured via dip switches to boot from eMMC. I
have a SDR104 microSD which detects and operates as such in Linux and
this is what I see in U-Boot:

U-Boot SPL 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24 -0800)
Normal Boot
WDT:   Started with servicing (60s timeout)
Trying to boot from MMC2


U-Boot 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24 -0800)

CPU:   Freescale i.MX8MMQ rev1.0 at 1200 MHz
Reset cause: POR
Model: FSL i.MX8MM EVK board
DRAM:  2 GiB
WDT:   Started with servicing (60s timeout)
MMC:   FSL_SDHC: 1, FSL_SDHC: 2
Loading Environment from MMC... OK
In:    serial
Out:   serial
Err:   serial
Net:   eth0: ethernet at 30be0000
Hit any key to stop autoboot:  0
u-boot=> mmc dev 1
Run CMD11 1.8V switch
switch to partitions #0, OK
mmc1 is current device
u-boot=> mmc info
Device: FSL_SDHC
Manufacturer ID: 1b
OEM: 534d
Name: 00000
Bus Speed: 50000000
Mode: SD High Speed (50MHz)
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 14.9 GiB
Bus Width: 4-bit
Erase Group Size: 512 Bytes

You can see that the 1.8V switch succeeds and the card is recognized
as high-speed but does not show the SDR104 capability.

Adding the following debugging:
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index a6394bc..663bf3d 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -1365,6 +1365,10 @@ static int sd_get_capabilities(struct mmc *mmc)
                return 0;

        sd3_bus_mode = __be32_to_cpu(switch_status[3]) >> 16 & 0x1f;
+printf("%s %s sd3_bus_mode=0x%x\n", __func__, mmc->dev->name, sd3_bus_mode);
+for (int i = 0; i < 16; i++) {
+       printf("switch_status[%d]=0x%08x\n", i, switch_status[i]);
+}
        if (sd3_bus_mode & SD_MODE_UHS_SDR104)
                mmc->card_caps |= MMC_CAP(UHS_SDR104);
        if (sd3_bus_mode & SD_MODE_UHS_SDR50)

Shows:
u-boot=> mmc dev 1
Run CMD11 1.8V switch
sd_get_capabilities mmc at 30b50000 sd3_bus_mode=0x3
switch_status[0]=0x0180c800
switch_status[1]=0x01800180
switch_status[2]=0x01800180
switch_status[3]=0x00000380
switch_status[4]=0x00000001
switch_status[5]=0x00000000
switch_status[6]=0x00000000
switch_status[7]=0x00000000
switch_status[8]=0x00000000
switch_status[9]=0x00000000
switch_status[10]=0x00000000
switch_status[11]=0x00000000
switch_status[12]=0x00000000
switch_status[13]=0x00000000
switch_status[14]=0x00000000
switch_status[15]=0x00000000
switch to partitions #0, OK
mmc1 is current device

What board and U-Boot sha/config are you using? Do you have the
ability to confirm my findings on an imx8mm-evk?

The code in sd_get_capabilities differs greatly from what Linux does.
I don't have documentation on the MMC switch settings so I can't
understand if what is being done is correct but it is interesting that
your card is detected as SDR104.

The card I'm using above is a Samsung PRO 16GB but I've repeated the
findings on a dozen cards that support either SD104 or DDR50 (again,
proven in Linux).

Best regard,

Tim

IMX8MM SD UHS support

[Fabio Estevam]

Hi Tim,

On Wed, Dec 30, 2020 at 1:54 PM Tim Harvey <tharvey@gateworks.com> wrote:

> Andrey,
>
> I did mention that I am using the imx8mm-evk. When I saw that my
> custom board was having issues with sd_get_capabilities() I switched
> to the imx8mm-evk and confirmed my findings there.
>
> I'm using master (ab865a8ee5c1) with imx8mm_evk_defconfig running on
> an imx8mm-evk board configured via dip switches to boot from eMMC. I
> have a SDR104 microSD which detects and operates as such in Linux and
> this is what I see in U-Boot:
>
> U-Boot SPL 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24 -0800)
> Normal Boot
> WDT:   Started with servicing (60s timeout)
> Trying to boot from MMC2
>
>
> U-Boot 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24 -0800)
>
> CPU:   Freescale i.MX8MMQ rev1.0 at 1200 MHz
> Reset cause: POR
> Model: FSL i.MX8MM EVK board
> DRAM:  2 GiB
> WDT:   Started with servicing (60s timeout)
> MMC:   FSL_SDHC: 1, FSL_SDHC: 2
> Loading Environment from MMC... OK
> In:    serial
> Out:   serial
> Err:   serial
> Net:   eth0: ethernet at 30be0000
> Hit any key to stop autoboot:  0
> u-boot=> mmc dev 1
> Run CMD11 1.8V switch
> switch to partitions #0, OK
> mmc1 is current device
> u-boot=> mmc info
> Device: FSL_SDHC
> Manufacturer ID: 1b
> OEM: 534d
> Name: 00000
> Bus Speed: 50000000
> Mode: SD High Speed (50MHz)
> Rd Block Len: 512
> SD version 3.0
> High Capacity: Yes
> Capacity: 14.9 GiB
> Bus Width: 4-bit
> Erase Group Size: 512 Bytes
>
> You can see that the 1.8V switch succeeds and the card is recognized
> as high-speed but does not show the SDR104 capability.

Could you please test this patch from Adam?
https://patchwork.ozlabs.org/project/uboot/patch/20201230173907.2891555-1-aford173 at gmail.com/

IMX8MM SD UHS support

[Adam Ford]

On Wed, Dec 30, 2020 at 11:50 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Tim,
>
> On Wed, Dec 30, 2020 at 1:54 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> > Andrey,
> >
> > I did mention that I am using the imx8mm-evk. When I saw that my
> > custom board was having issues with sd_get_capabilities() I switched
> > to the imx8mm-evk and confirmed my findings there.
> >
> > I'm using master (ab865a8ee5c1) with imx8mm_evk_defconfig running on
> > an imx8mm-evk board configured via dip switches to boot from eMMC. I
> > have a SDR104 microSD which detects and operates as such in Linux and
> > this is what I see in U-Boot:
> >
> > U-Boot SPL 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24 -0800)
> > Normal Boot
> > WDT:   Started with servicing (60s timeout)
> > Trying to boot from MMC2
> >
> >
> > U-Boot 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24 -0800)
> >
> > CPU:   Freescale i.MX8MMQ rev1.0 at 1200 MHz
> > Reset cause: POR
> > Model: FSL i.MX8MM EVK board
> > DRAM:  2 GiB
> > WDT:   Started with servicing (60s timeout)
> > MMC:   FSL_SDHC: 1, FSL_SDHC: 2
> > Loading Environment from MMC... OK
> > In:    serial
> > Out:   serial
> > Err:   serial
> > Net:   eth0: ethernet at 30be0000
> > Hit any key to stop autoboot:  0
> > u-boot=> mmc dev 1
> > Run CMD11 1.8V switch
> > switch to partitions #0, OK
> > mmc1 is current device
> > u-boot=> mmc info
> > Device: FSL_SDHC
> > Manufacturer ID: 1b
> > OEM: 534d
> > Name: 00000
> > Bus Speed: 50000000
> > Mode: SD High Speed (50MHz)
> > Rd Block Len: 512
> > SD version 3.0
> > High Capacity: Yes
> > Capacity: 14.9 GiB
> > Bus Width: 4-bit
> > Erase Group Size: 512 Bytes
> >
> > You can see that the 1.8V switch succeeds and the card is recognized
> > as high-speed but does not show the SDR104 capability.
>
> Could you please test this patch from Adam?
> https://patchwork.ozlabs.org/project/uboot/patch/20201230173907.2891555-1-aford173 at gmail.com/

My patch probably won't do much more than the one from Andrey.  From
what I could gather, the generic mmc driver uses those flags to enable
the host caps.  My enables the same host caps by checking the
structure so the device tree flags are not needed.

The UHS and HS200/HS400 config options need to be enabled in Kconfig
for them to make a difference.  Part of me wonders if they should be
implied-on if USHDC is set, but that's a different issue.

If Tim is not seeing the SDR104 negotiated from Andrey's patch, it
probably won't change with mine.

adam

IMX8MM SD UHS support

[Tim Harvey]

On Wed, Dec 30, 2020 at 10:22 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Wed, Dec 30, 2020 at 11:50 AM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > Hi Tim,
> >
> > On Wed, Dec 30, 2020 at 1:54 PM Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > > Andrey,
> > >
> > > I did mention that I am using the imx8mm-evk. When I saw that my
> > > custom board was having issues with sd_get_capabilities() I switched
> > > to the imx8mm-evk and confirmed my findings there.
> > >
> > > I'm using master (ab865a8ee5c1) with imx8mm_evk_defconfig running on
> > > an imx8mm-evk board configured via dip switches to boot from eMMC. I
> > > have a SDR104 microSD which detects and operates as such in Linux and
> > > this is what I see in U-Boot:
> > >
> > > U-Boot SPL 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24 -0800)
> > > Normal Boot
> > > WDT:   Started with servicing (60s timeout)
> > > Trying to boot from MMC2
> > >
> > >
> > > U-Boot 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24 -0800)
> > >
> > > CPU:   Freescale i.MX8MMQ rev1.0 at 1200 MHz
> > > Reset cause: POR
> > > Model: FSL i.MX8MM EVK board
> > > DRAM:  2 GiB
> > > WDT:   Started with servicing (60s timeout)
> > > MMC:   FSL_SDHC: 1, FSL_SDHC: 2
> > > Loading Environment from MMC... OK
> > > In:    serial
> > > Out:   serial
> > > Err:   serial
> > > Net:   eth0: ethernet at 30be0000
> > > Hit any key to stop autoboot:  0
> > > u-boot=> mmc dev 1
> > > Run CMD11 1.8V switch
> > > switch to partitions #0, OK
> > > mmc1 is current device
> > > u-boot=> mmc info
> > > Device: FSL_SDHC
> > > Manufacturer ID: 1b
> > > OEM: 534d
> > > Name: 00000
> > > Bus Speed: 50000000
> > > Mode: SD High Speed (50MHz)
> > > Rd Block Len: 512
> > > SD version 3.0
> > > High Capacity: Yes
> > > Capacity: 14.9 GiB
> > > Bus Width: 4-bit
> > > Erase Group Size: 512 Bytes
> > >
> > > You can see that the 1.8V switch succeeds and the card is recognized
> > > as high-speed but does not show the SDR104 capability.
> >
> > Could you please test this patch from Adam?
> > https://patchwork.ozlabs.org/project/uboot/patch/20201230173907.2891555-1-aford173 at gmail.com/
>
> My patch probably won't do much more than the one from Andrey.  From
> what I could gather, the generic mmc driver uses those flags to enable
> the host caps.  My enables the same host caps by checking the
> structure so the device tree flags are not needed.
>
> The UHS and HS200/HS400 config options need to be enabled in Kconfig
> for them to make a difference.  Part of me wonders if they should be
> implied-on if USHDC is set, but that's a different issue.
>
> If Tim is not seeing the SDR104 negotiated from Andrey's patch, it
> probably won't change with mine.
>

Right, the issue is not the host caps.

If I enable the calls to mmc_dump_capabilities() I see:
u-boot=> mmc info
Device: FSL_SDHC
Manufacturer ID: 3
OEM: 5344
Name: SL16G
Bus Speed: 50000000
Mode: SD High Speed (50MHz)
card capabilities: widths [4, 1] modes [MMC legacy, SD High Speed
(50MHz), UHS SDR12 (25MHz), UHS SDR25 (50MHz)]
host capabilities: widths [4, 1] modes [MMC legacy, MMC High Speed
(26MHz), SD High Speed (50MHz), MMC High Speed (52MHz), UHS DDR50
(50MHz), UHS SDR104 (208MHz)]
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 14.8 GiB
Bus Width: 4-bit
Erase Group Size: 512 Bytes

You can see above the host shows DDR50/SDR104 capability but the card
does not. Again, the issue is in sd_get_capabilities()

Adam / Fabio, what results do you see on your board(s)?

Tim

IMX8MM SD UHS support

[Adam Ford]

On Wed, Dec 30, 2020 at 12:47 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Wed, Dec 30, 2020 at 10:22 AM Adam Ford <aford173@gmail.com> wrote:
> >
> > On Wed, Dec 30, 2020 at 11:50 AM Fabio Estevam <festevam@gmail.com> wrote:
> > >
> > > Hi Tim,
> > >
> > > On Wed, Dec 30, 2020 at 1:54 PM Tim Harvey <tharvey@gateworks.com> wrote:
> > >
> > > > Andrey,
> > > >
> > > > I did mention that I am using the imx8mm-evk. When I saw that my
> > > > custom board was having issues with sd_get_capabilities() I switched
> > > > to the imx8mm-evk and confirmed my findings there.
> > > >
> > > > I'm using master (ab865a8ee5c1) with imx8mm_evk_defconfig running on
> > > > an imx8mm-evk board configured via dip switches to boot from eMMC. I
> > > > have a SDR104 microSD which detects and operates as such in Linux and
> > > > this is what I see in U-Boot:
> > > >
> > > > U-Boot SPL 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24 -0800)
> > > > Normal Boot
> > > > WDT:   Started with servicing (60s timeout)
> > > > Trying to boot from MMC2
> > > >
> > > >
> > > > U-Boot 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24 -0800)
> > > >
> > > > CPU:   Freescale i.MX8MMQ rev1.0 at 1200 MHz
> > > > Reset cause: POR
> > > > Model: FSL i.MX8MM EVK board
> > > > DRAM:  2 GiB
> > > > WDT:   Started with servicing (60s timeout)
> > > > MMC:   FSL_SDHC: 1, FSL_SDHC: 2
> > > > Loading Environment from MMC... OK
> > > > In:    serial
> > > > Out:   serial
> > > > Err:   serial
> > > > Net:   eth0: ethernet at 30be0000
> > > > Hit any key to stop autoboot:  0
> > > > u-boot=> mmc dev 1
> > > > Run CMD11 1.8V switch
> > > > switch to partitions #0, OK
> > > > mmc1 is current device
> > > > u-boot=> mmc info
> > > > Device: FSL_SDHC
> > > > Manufacturer ID: 1b
> > > > OEM: 534d
> > > > Name: 00000
> > > > Bus Speed: 50000000
> > > > Mode: SD High Speed (50MHz)
> > > > Rd Block Len: 512
> > > > SD version 3.0
> > > > High Capacity: Yes
> > > > Capacity: 14.9 GiB
> > > > Bus Width: 4-bit
> > > > Erase Group Size: 512 Bytes
> > > >
> > > > You can see that the 1.8V switch succeeds and the card is recognized
> > > > as high-speed but does not show the SDR104 capability.
> > >
> > > Could you please test this patch from Adam?
> > > https://patchwork.ozlabs.org/project/uboot/patch/20201230173907.2891555-1-aford173 at gmail.com/
> >
> > My patch probably won't do much more than the one from Andrey.  From
> > what I could gather, the generic mmc driver uses those flags to enable
> > the host caps.  My enables the same host caps by checking the
> > structure so the device tree flags are not needed.
> >
> > The UHS and HS200/HS400 config options need to be enabled in Kconfig
> > for them to make a difference.  Part of me wonders if they should be
> > implied-on if USHDC is set, but that's a different issue.
> >
> > If Tim is not seeing the SDR104 negotiated from Andrey's patch, it
> > probably won't change with mine.
> >
>
> Right, the issue is not the host caps.
>
> If I enable the calls to mmc_dump_capabilities() I see:
> u-boot=> mmc info
> Device: FSL_SDHC
> Manufacturer ID: 3
> OEM: 5344
> Name: SL16G
> Bus Speed: 50000000
> Mode: SD High Speed (50MHz)
> card capabilities: widths [4, 1] modes [MMC legacy, SD High Speed
> (50MHz), UHS SDR12 (25MHz), UHS SDR25 (50MHz)]
> host capabilities: widths [4, 1] modes [MMC legacy, MMC High Speed
> (26MHz), SD High Speed (50MHz), MMC High Speed (52MHz), UHS DDR50
> (50MHz), UHS SDR104 (208MHz)]
> Rd Block Len: 512
> SD version 3.0
> High Capacity: Yes
> Capacity: 14.8 GiB
> Bus Width: 4-bit
> Erase Group Size: 512 Bytes
>
> You can see above the host shows DDR50/SDR104 capability but the card
> does not. Again, the issue is in sd_get_capabilities()
>
> Adam / Fabio, what results do you see on your board(s)?

My eMMC appears to work properly at HS400ES, and that was my focus,
but I am seeing similar results on the SD card as you are.

u-boot=> mmc info
Device: FSL_SDHC
Manufacturer ID: 27
OEM: 5048
Name: SD32G
Bus Speed: 50000000
Mode: SD High Speed (50MHz)
card capabilities: widths [4, 1] modes [MMC legacy, SD High Speed
(50MHz), UHS SDR12 (25MHz), UHS SDR25 (50MHz)]
host capabilities: widths [4, 1] modes [MMC legacy, MMC High Speed
(26MHz), SD High Speed (50MHz), MMC High Speed (52MHz), UHS SDR12
(25MHz), UHS SDR25 (50MHz), UHS SDR50 (100MHz), UHS DDR50 (50MHz), UHS
SDR104 (208MHz), HS200 (200MHz), HS400 (200MHz), HS400ES (200MHz)]
Rd Block Len: 512
SD version 3.0
High Capacity: Yes
Capacity: 29 GiB
Bus Width: 4-bit
Erase Group Size: 512 Bytes
u-boot=>

It doesn't appear to be able to properly negotiate the SDR104 from the
SD card, but the host is showing the proper host capabilities.  I'm
looking into the sd card query now.  I don't have the full spec, but
I'll try to compare it against the Linux driver.

adam
>
> Tim

IMX8MM SD UHS support

[ZHIZHIKIN Andrey]

Hello Tim,

> -----Original Message-----
> From: Tim Harvey <tharvey@gateworks.com>
> Sent: Wednesday, December 30, 2020 7:48 PM
> To: Adam Ford <aford173@gmail.com>
> Cc: Fabio Estevam <festevam@gmail.com>; ZHIZHIKIN Andrey
> <andrey.zhizhikin@leica-geosystems.com>; Peng Fan <Peng.Fan@nxp.com>; u-
> boot <u-boot@lists.denx.de>; Stefano Babic <sbabic@denx.de>
> Subject: Re: IMX8MM SD UHS support
> 
> 
> On Wed, Dec 30, 2020 at 10:22 AM Adam Ford <aford173@gmail.com> wrote:
> >
> > On Wed, Dec 30, 2020 at 11:50 AM Fabio Estevam <festevam@gmail.com>
> wrote:
> > >
> > > Hi Tim,
> > >
> > > On Wed, Dec 30, 2020 at 1:54 PM Tim Harvey <tharvey@gateworks.com>
> wrote:
> > >
> > > > Andrey,
> > > >
> > > > I did mention that I am using the imx8mm-evk. When I saw that my
> > > > custom board was having issues with sd_get_capabilities() I
> > > > switched to the imx8mm-evk and confirmed my findings there.

I've missed the part that you've tested the same behavior on i.MX8M Mini EVK in your original message, sorry for that.

> > > >
> > > > I'm using master (ab865a8ee5c1) with imx8mm_evk_defconfig running
> > > > on an imx8mm-evk board configured via dip switches to boot from
> > > > eMMC. I have a SDR104 microSD which detects and operates as such
> > > > in Linux and this is what I see in U-Boot:
> > > >
> > > > U-Boot SPL 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24
> > > > -0800) Normal Boot
> > > > WDT:   Started with servicing (60s timeout)
> > > > Trying to boot from MMC2
> > > >
> > > >
> > > > U-Boot 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24 -0800)
> > > >
> > > > CPU:   Freescale i.MX8MMQ rev1.0 at 1200 MHz
> > > > Reset cause: POR
> > > > Model: FSL i.MX8MM EVK board
> > > > DRAM:  2 GiB
> > > > WDT:   Started with servicing (60s timeout)
> > > > MMC:   FSL_SDHC: 1, FSL_SDHC: 2
> > > > Loading Environment from MMC... OK
> > > > In:    serial
> > > > Out:   serial
> > > > Err:   serial
> > > > Net:   eth0: ethernet at 30be0000
> > > > Hit any key to stop autoboot:  0
> > > > u-boot=> mmc dev 1
> > > > Run CMD11 1.8V switch
> > > > switch to partitions #0, OK
> > > > mmc1 is current device
> > > > u-boot=> mmc info
> > > > Device: FSL_SDHC
> > > > Manufacturer ID: 1b
> > > > OEM: 534d
> > > > Name: 00000
> > > > Bus Speed: 50000000
> > > > Mode: SD High Speed (50MHz)
> > > > Rd Block Len: 512
> > > > SD version 3.0
> > > > High Capacity: Yes
> > > > Capacity: 14.9 GiB
> > > > Bus Width: 4-bit
> > > > Erase Group Size: 512 Bytes
> > > >
> > > > You can see that the 1.8V switch succeeds and the card is
> > > > recognized as high-speed but does not show the SDR104 capability.

Did you actually measured that the signaling voltages are switched to 1v8
on the test point I've mentioned in my mail?

> > >
> > > Could you please test this patch from Adam?
> > > https://patchwork.ozlabs.org/project/uboot/patch/20201230173907.2891555-1-aford173 at gmail.com/
> >

If voltages are not actually switched - then probably another patch from Adam
might help: http://patchwork.ozlabs.org/project/uboot/patch/20201230141421.2860212-1-aford173 at gmail.com/

This is my pure guess, as neither I tested it myself nor I have a fixed regulator option enabled in my
build for SPL, but still I see that SD Card I have is switched into SDR104 mode.

> > My patch probably won't do much more than the one from Andrey.  From
> > what I could gather, the generic mmc driver uses those flags to enable
> > the host caps.  My enables the same host caps by checking the
> > structure so the device tree flags are not needed.
> >
> > The UHS and HS200/HS400 config options need to be enabled in Kconfig
> > for them to make a difference.  Part of me wonders if they should be
> > implied-on if USHDC is set, but that's a different issue.
> >
> > If Tim is not seeing the SDR104 negotiated from Andrey's patch, it
> > probably won't change with mine.
> >
> 
> Right, the issue is not the host caps.
> 
> If I enable the calls to mmc_dump_capabilities() I see:
> u-boot=> mmc info
> Device: FSL_SDHC
> Manufacturer ID: 3
> OEM: 5344
> Name: SL16G
> Bus Speed: 50000000
> Mode: SD High Speed (50MHz)
> card capabilities: widths [4, 1] modes [MMC legacy, SD High Speed (50MHz), UHS
> SDR12 (25MHz), UHS SDR25 (50MHz)]

I actually don't see that the card reports an SDR104 mode support here, and clocks can only be
configured as high as 50 MHz which we do see in the "Bus speed" output.

> host capabilities: widths [4, 1] modes [MMC
> legacy, MMC High Speed (26MHz), SD High Speed (50MHz), MMC High Speed
> (52MHz), UHS DDR50 (50MHz), UHS SDR104 (208MHz)]

Since the only matching mode from host to card is " SD High Speed (50MHz)" - it is the one that has been configured.

> Rd Block Len: 512 SD
> version 3.0 High Capacity: Yes
> Capacity: 14.8 GiB
> Bus Width: 4-bit
> Erase Group Size: 512 Bytes
> 
> You can see above the host shows DDR50/SDR104 capability but the card does
> not. Again, the issue is in sd_get_capabilities()

Here is what puzzles me the most: Kernel *can* operate with this card configured to SDR104,
but U-Boot simply does not see that the mode is available since it is not reported by the card... 

Looking at the other email report from you, I can see that the sd3_bus_mode=0x3, which suggests
that the card was not setup for high-speed mode, and this is for example what Linux driver checks
in mmc_sd_card_using_v18 function [drivers/mmc/core/sd.c].

For the record: I do have U-Boot 2020.10 with my patches applied on top, and I'm building image using Yocto.

My setup is: i.MX8M Mini EVK with Intenso 32GB SD Card. As I've already indicated, it is indeed switched to
SDR104 mode and working proper. I've also successfully tried Sandisk 32 GB SD Card with the same EVK.

I'll apply your patch with debug output on top of my setup to compare which values I do get from my setup.

> 
> Adam / Fabio, what results do you see on your board(s)?
> 
> Tim

-- Andrey

IMX8MM SD UHS support

[Tim Harvey]

On Wed, Dec 30, 2020 at 1:00 PM ZHIZHIKIN Andrey
<andrey.zhizhikin@leica-geosystems.com> wrote:
>
> Hello Tim,
>
> > -----Original Message-----
> > From: Tim Harvey <tharvey@gateworks.com>
> > Sent: Wednesday, December 30, 2020 7:48 PM
> > To: Adam Ford <aford173@gmail.com>
> > Cc: Fabio Estevam <festevam@gmail.com>; ZHIZHIKIN Andrey
> > <andrey.zhizhikin@leica-geosystems.com>; Peng Fan <Peng.Fan@nxp.com>; u-
> > boot <u-boot@lists.denx.de>; Stefano Babic <sbabic@denx.de>
> > Subject: Re: IMX8MM SD UHS support
> >
> >
> > On Wed, Dec 30, 2020 at 10:22 AM Adam Ford <aford173@gmail.com> wrote:
> > >
> > > On Wed, Dec 30, 2020 at 11:50 AM Fabio Estevam <festevam@gmail.com>
> > wrote:
> > > >
> > > > Hi Tim,
> > > >
> > > > On Wed, Dec 30, 2020 at 1:54 PM Tim Harvey <tharvey@gateworks.com>
> > wrote:
> > > >
> > > > > Andrey,
> > > > >
> > > > > I did mention that I am using the imx8mm-evk. When I saw that my
> > > > > custom board was having issues with sd_get_capabilities() I
> > > > > switched to the imx8mm-evk and confirmed my findings there.
>
> I've missed the part that you've tested the same behavior on i.MX8M Mini EVK in your original message, sorry for that.
>
> > > > >
> > > > > I'm using master (ab865a8ee5c1) with imx8mm_evk_defconfig running
> > > > > on an imx8mm-evk board configured via dip switches to boot from
> > > > > eMMC. I have a SDR104 microSD which detects and operates as such
> > > > > in Linux and this is what I see in U-Boot:
> > > > >
> > > > > U-Boot SPL 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24
> > > > > -0800) Normal Boot
> > > > > WDT:   Started with servicing (60s timeout)
> > > > > Trying to boot from MMC2
> > > > >
> > > > >
> > > > > U-Boot 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24 -0800)
> > > > >
> > > > > CPU:   Freescale i.MX8MMQ rev1.0 at 1200 MHz
> > > > > Reset cause: POR
> > > > > Model: FSL i.MX8MM EVK board
> > > > > DRAM:  2 GiB
> > > > > WDT:   Started with servicing (60s timeout)
> > > > > MMC:   FSL_SDHC: 1, FSL_SDHC: 2
> > > > > Loading Environment from MMC... OK
> > > > > In:    serial
> > > > > Out:   serial
> > > > > Err:   serial
> > > > > Net:   eth0: ethernet at 30be0000
> > > > > Hit any key to stop autoboot:  0
> > > > > u-boot=> mmc dev 1
> > > > > Run CMD11 1.8V switch
> > > > > switch to partitions #0, OK
> > > > > mmc1 is current device
> > > > > u-boot=> mmc info
> > > > > Device: FSL_SDHC
> > > > > Manufacturer ID: 1b
> > > > > OEM: 534d
> > > > > Name: 00000
> > > > > Bus Speed: 50000000
> > > > > Mode: SD High Speed (50MHz)
> > > > > Rd Block Len: 512
> > > > > SD version 3.0
> > > > > High Capacity: Yes
> > > > > Capacity: 14.9 GiB
> > > > > Bus Width: 4-bit
> > > > > Erase Group Size: 512 Bytes
> > > > >
> > > > > You can see that the 1.8V switch succeeds and the card is
> > > > > recognized as high-speed but does not show the SDR104 capability.
>
> Did you actually measured that the signaling voltages are switched to 1v8
> on the test point I've mentioned in my mail?
>
> > > >
> > > > Could you please test this patch from Adam?
> > > > https://patchwork.ozlabs.org/project/uboot/patch/20201230173907.2891555-1-aford173 at gmail.com/
> > >
>
> If voltages are not actually switched - then probably another patch from Adam
> might help: http://patchwork.ozlabs.org/project/uboot/patch/20201230141421.2860212-1-aford173 at gmail.com/
>

That patch wouldn't help in this case... that's an SPL thing. This
failure is in U-Boot itself.

The NVCC_SD2 rail does indeed switch to 1.8V but for some reason the
card indicates a failure to switch itself to 1.8V I/O as the
mmc_wait_dat0() call at the end of mmc_switch_voltage() fails. It
looks like U-Boot is doing the same steps in the same order as the
Linux driver so I'm not sure what's wrong here. I suspect it is a
timing thing as it works for your card(s) but fails for mine as well
as Adam's card(s). Because Adam verified his card(s) work with another
MMC host in UHS mode that would indicate it is probably something in
fsl_esdhc_imx.c.

I've added all kinds of delays and it doesn't make a difference. I've
also commented out the following as it doesn't seem right to switch
the host to 1.8V within the CMD11 (supposed to be done after the CMD11
and it does get done by esdhc_set_voltage):
diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index e5409ad..9e9d4c6 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -514,6 +514,7 @@ static int esdhc_send_cmd_common(struct
fsl_esdhc_priv *priv, struct mmc *mmc,
                goto out;
        }

+#if 0 // esdhc_set_voltage does this and technically it shouldn't be
done until the CMD11 has completed
        /* Switch voltage to 1.8V if CMD11 succeeded */
        if (cmd->cmdidx == SD_CMD_SWITCH_UHS18V) {
                esdhc_setbits32(&regs->vendorspec, ESDHC_VENDORSPEC_VSELECT);
@@ -522,6 +523,7 @@ static int esdhc_send_cmd_common(struct
fsl_esdhc_priv *priv, struct mmc *mmc,
                /* Sleep for 5 ms - max time for card to switch to 1.8V */
                udelay(5000);
        }
+#endif

I'm wondering if all the stuff done in esdhc_send_cmd_common() breaks
the ability to check card failure via mmc_wait_dat0?

Also note that if I comment out the error check from the mmc_wait_dat0
at the end of mmc_switch_voltage() SDR104 mode appears to work just
fine (verified read performance and data integrity).

So at least I've verified that card capability detection is not to blame here.

Tim



>
> > host capabilities: widths [4, 1] modes [MMC
> > legacy, MMC High Speed (26MHz), SD High Speed (50MHz), MMC High Speed
> > (52MHz), UHS DDR50 (50MHz), UHS SDR104 (208MHz)]
>
> Since the only matching mode from host to card is " SD High Speed (50MHz)" - it is the one that has been configured.
>
> > Rd Block Len: 512 SD
> > version 3.0 High Capacity: Yes
> > Capacity: 14.8 GiB
> > Bus Width: 4-bit
> > Erase Group Size: 512 Bytes
> >
> > You can see above the host shows DDR50/SDR104 capability but the card does
> > not. Again, the issue is in sd_get_capabilities()
>
> Here is what puzzles me the most: Kernel *can* operate with this card configured to SDR104,
> but U-Boot simply does not see that the mode is available since it is not reported by the card...
>
> Looking at the other email report from you, I can see that the sd3_bus_mode=0x3, which suggests
> that the card was not setup for high-speed mode, and this is for example what Linux driver checks
> in mmc_sd_card_using_v18 function [drivers/mmc/core/sd.c].
>
> For the record: I do have U-Boot 2020.10 with my patches applied on top, and I'm building image using Yocto.
>
> My setup is: i.MX8M Mini EVK with Intenso 32GB SD Card. As I've already indicated, it is indeed switched to
> SDR104 mode and working proper. I've also successfully tried Sandisk 32 GB SD Card with the same EVK.
>
> I'll apply your patch with debug output on top of my setup to compare which values I do get from my setup.
>
> >
> > Adam / Fabio, what results do you see on your board(s)?
> >
> > Tim
>
> -- Andrey

IMX8MM SD UHS support

[ZHIZHIKIN Andrey]

Hello Tim,

Sorry it took me quite some time to get this sorted out, but I believe I was able to identify an offending commit that is preventing the USDHC to switch to higher speed modes.

It is in fact b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0() support"), reverting it makes SD Card to properly report capabilities and switch to high speed modes.

Can you try to revert this on your end to see if the SD Card would start to operate in high speed mode?

I'm still investigating why this addition of wait_dat0() caused this, I believe this is due to the fact that the same wait is already performed while voltage switch to handle the errata, thus this addition wait might erroneously timeout.

++ Haibo Chen <haibo.chen@nxp.com>

Haibo,

Can you please explain the purpose of adding wait_dat0() Introduced with commit b5874b552f? It is not clear from the commit message what was the purpose of adding it. Have you tested the USDHC switch to higher modes with this change?

> -----Original Message-----
> From: Tim Harvey <tharvey@gateworks.com>
> Sent: Wednesday, January 6, 2021 12:10 AM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> Cc: Adam Ford <aford173@gmail.com>; Fabio Estevam <festevam@gmail.com>;
> Peng Fan <Peng.Fan@nxp.com>; u-boot <u-boot@lists.denx.de>; Stefano Babic
> <sbabic@denx.de>
> Subject: Re: IMX8MM SD UHS support
> 
> This email is not from Hexagon?s Office 365 instance. Please be careful while
> clicking links, opening attachments, or replying to this email.
> 
> 
> On Wed, Dec 30, 2020 at 1:00 PM ZHIZHIKIN Andrey <andrey.zhizhikin@leica-
> geosystems.com> wrote:
> >
> > Hello Tim,
> >
> > > -----Original Message-----
> > > From: Tim Harvey <tharvey@gateworks.com>
> > > Sent: Wednesday, December 30, 2020 7:48 PM
> > > To: Adam Ford <aford173@gmail.com>
> > > Cc: Fabio Estevam <festevam@gmail.com>; ZHIZHIKIN Andrey
> > > <andrey.zhizhikin@leica-geosystems.com>; Peng Fan
> > > <Peng.Fan@nxp.com>; u- boot <u-boot@lists.denx.de>; Stefano Babic
> > > <sbabic@denx.de>
> > > Subject: Re: IMX8MM SD UHS support
> > >
> > >
> > > On Wed, Dec 30, 2020 at 10:22 AM Adam Ford <aford173@gmail.com> wrote:
> > > >
> > > > On Wed, Dec 30, 2020 at 11:50 AM Fabio Estevam
> > > > <festevam@gmail.com>
> > > wrote:
> > > > >
> > > > > Hi Tim,
> > > > >
> > > > > On Wed, Dec 30, 2020 at 1:54 PM Tim Harvey
> > > > > <tharvey@gateworks.com>
> > > wrote:
> > > > >
> > > > > > Andrey,
> > > > > >
> > > > > > I did mention that I am using the imx8mm-evk. When I saw that
> > > > > > my custom board was having issues with sd_get_capabilities() I
> > > > > > switched to the imx8mm-evk and confirmed my findings there.
> >
> > I've missed the part that you've tested the same behavior on i.MX8M Mini EVK
> in your original message, sorry for that.
> >
> > > > > >
> > > > > > I'm using master (ab865a8ee5c1) with imx8mm_evk_defconfig
> > > > > > running on an imx8mm-evk board configured via dip switches to
> > > > > > boot from eMMC. I have a SDR104 microSD which detects and
> > > > > > operates as such in Linux and this is what I see in U-Boot:
> > > > > >
> > > > > > U-Boot SPL 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24
> > > > > > -0800) Normal Boot
> > > > > > WDT:   Started with servicing (60s timeout)
> > > > > > Trying to boot from MMC2
> > > > > >
> > > > > >
> > > > > > U-Boot 2021.01-rc4-00029-gab865a8 (Dec 30 2020 - 08:29:24
> > > > > > -0800)
> > > > > >
> > > > > > CPU:   Freescale i.MX8MMQ rev1.0 at 1200 MHz
> > > > > > Reset cause: POR
> > > > > > Model: FSL i.MX8MM EVK board
> > > > > > DRAM:  2 GiB
> > > > > > WDT:   Started with servicing (60s timeout)
> > > > > > MMC:   FSL_SDHC: 1, FSL_SDHC: 2
> > > > > > Loading Environment from MMC... OK
> > > > > > In:    serial
> > > > > > Out:   serial
> > > > > > Err:   serial
> > > > > > Net:   eth0: ethernet at 30be0000
> > > > > > Hit any key to stop autoboot:  0 u-boot=> mmc dev 1 Run CMD11
> > > > > > 1.8V switch switch to partitions #0, OK
> > > > > > mmc1 is current device
> > > > > > u-boot=> mmc info
> > > > > > Device: FSL_SDHC
> > > > > > Manufacturer ID: 1b
> > > > > > OEM: 534d
> > > > > > Name: 00000
> > > > > > Bus Speed: 50000000
> > > > > > Mode: SD High Speed (50MHz)
> > > > > > Rd Block Len: 512
> > > > > > SD version 3.0
> > > > > > High Capacity: Yes
> > > > > > Capacity: 14.9 GiB
> > > > > > Bus Width: 4-bit
> > > > > > Erase Group Size: 512 Bytes
> > > > > >
> > > > > > You can see that the 1.8V switch succeeds and the card is
> > > > > > recognized as high-speed but does not show the SDR104 capability.
> >
> > Did you actually measured that the signaling voltages are switched to
> > 1v8 on the test point I've mentioned in my mail?
> >
> > > > >
> > > > > Could you please test this patch from Adam?
> > > > > https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > > 2Fpatchwork.ozlabs.org%2Fproject%2Fuboot%2Fpatch%2F2020123017390
> > > > > 7.2891555-1-
> aford173%40gmail.com%2F&amp;data=04%7C01%7C%7C12ecf6
> > > > >
> da5d4d4b8ea07708d8b1cf01ee%7C1b16ab3eb8f64fe39f3e2db7fe549f6a%7C
> > > > >
> 0%7C0%7C637454849942704702%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
> L
> > > > >
> jAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp
> > > > >
> ;sdata=libCAoYJAg%2Brm2V%2BkrFBK%2FovUaWV0u1kbwHhj7pfues%3D&amp;
> > > > > reserved=0
> > > >
> >
> > If voltages are not actually switched - then probably another patch
> > from Adam might help:
> > https://eur02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
> > work.ozlabs.org%2Fproject%2Fuboot%2Fpatch%2F20201230141421.2860212-1-
> a
> >
> ford173%40gmail.com%2F&amp;data=04%7C01%7C%7C12ecf6da5d4d4b8ea0770
> 8d8b
> >
> 1cf01ee%7C1b16ab3eb8f64fe39f3e2db7fe549f6a%7C0%7C0%7C637454849942704
> 70
> >
> 2%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJ
> BTiI6
> >
> Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=oBQ%2BiOr%2B%2BYVmI47EAs
> J%2FIs
> > cet48wzpcUY%2FY%2BcJm2eaY%3D&amp;reserved=0
> >
> 
> That patch wouldn't help in this case... that's an SPL thing. This failure is in U-Boot
> itself.
> 
> The NVCC_SD2 rail does indeed switch to 1.8V but for some reason the card
> indicates a failure to switch itself to 1.8V I/O as the
> mmc_wait_dat0() call at the end of mmc_switch_voltage() fails. It looks like U-
> Boot is doing the same steps in the same order as the Linux driver so I'm not sure
> what's wrong here. I suspect it is a timing thing as it works for your card(s) but
> fails for mine as well as Adam's card(s). Because Adam verified his card(s) work
> with another MMC host in UHS mode that would indicate it is probably something
> in fsl_esdhc_imx.c.
> 
> I've added all kinds of delays and it doesn't make a difference. I've also
> commented out the following as it doesn't seem right to switch the host to 1.8V
> within the CMD11 (supposed to be done after the CMD11 and it does get done
> by esdhc_set_voltage):
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c index
> e5409ad..9e9d4c6 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -514,6 +514,7 @@ static int esdhc_send_cmd_common(struct
> fsl_esdhc_priv *priv, struct mmc *mmc,
>                 goto out;
>         }
> 
> +#if 0 // esdhc_set_voltage does this and technically it shouldn't be
> done until the CMD11 has completed
>         /* Switch voltage to 1.8V if CMD11 succeeded */
>         if (cmd->cmdidx == SD_CMD_SWITCH_UHS18V) {
>                 esdhc_setbits32(&regs->vendorspec, ESDHC_VENDORSPEC_VSELECT);
> @@ -522,6 +523,7 @@ static int esdhc_send_cmd_common(struct
> fsl_esdhc_priv *priv, struct mmc *mmc,
>                 /* Sleep for 5 ms - max time for card to switch to 1.8V */
>                 udelay(5000);
>         }
> +#endif
> 
> I'm wondering if all the stuff done in esdhc_send_cmd_common() breaks the
> ability to check card failure via mmc_wait_dat0?
> 
> Also note that if I comment out the error check from the mmc_wait_dat0 at the
> end of mmc_switch_voltage() SDR104 mode appears to work just fine (verified
> read performance and data integrity).
> 
> So at least I've verified that card capability detection is not to blame here.
> 
> Tim
> 
> 
> 
> >
> > > host capabilities: widths [4, 1] modes [MMC legacy, MMC High Speed
> > > (26MHz), SD High Speed (50MHz), MMC High Speed (52MHz), UHS DDR50
> > > (50MHz), UHS SDR104 (208MHz)]
> >
> > Since the only matching mode from host to card is " SD High Speed (50MHz)" - it
> is the one that has been configured.
> >
> > > Rd Block Len: 512 SD
> > > version 3.0 High Capacity: Yes
> > > Capacity: 14.8 GiB
> > > Bus Width: 4-bit
> > > Erase Group Size: 512 Bytes
> > >
> > > You can see above the host shows DDR50/SDR104 capability but the
> > > card does not. Again, the issue is in sd_get_capabilities()
> >
> > Here is what puzzles me the most: Kernel *can* operate with this card
> > configured to SDR104, but U-Boot simply does not see that the mode is
> available since it is not reported by the card...
> >
> > Looking at the other email report from you, I can see that the
> > sd3_bus_mode=0x3, which suggests that the card was not setup for
> > high-speed mode, and this is for example what Linux driver checks in
> mmc_sd_card_using_v18 function [drivers/mmc/core/sd.c].
> >
> > For the record: I do have U-Boot 2020.10 with my patches applied on top, and
> I'm building image using Yocto.
> >
> > My setup is: i.MX8M Mini EVK with Intenso 32GB SD Card. As I've
> > already indicated, it is indeed switched to
> > SDR104 mode and working proper. I've also successfully tried Sandisk 32 GB SD
> Card with the same EVK.
> >
> > I'll apply your patch with debug output on top of my setup to compare which
> values I do get from my setup.
> >
> > >
> > > Adam / Fabio, what results do you see on your board(s)?
> > >
> > > Tim
> >
> > -- Andrey

-- Andrey

IMX8MM SD UHS support

[Tim Harvey]

On Mon, Jan 18, 2021 at 11:38 AM ZHIZHIKIN Andrey
<andrey.zhizhikin@leica-geosystems.com> wrote:
>
> Hello Tim,
>
> Sorry it took me quite some time to get this sorted out, but I believe I was able to identify an offending commit that is preventing the USDHC to switch to higher speed modes.
>
> It is in fact b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0() support"), reverting it makes SD Card to properly report capabilities and switch to high speed modes.
>
> Can you try to revert this on your end to see if the SD Card would start to operate in high speed mode?
>
> I'm still investigating why this addition of wait_dat0() caused this, I believe this is due to the fact that the same wait is already performed while voltage switch to handle the errata, thus this addition wait might erroneously timeout.
>
> ++ Haibo Chen <haibo.chen@nxp.com>
>
> Haibo,
>
> Can you please explain the purpose of adding wait_dat0() Introduced with commit b5874b552f? It is not clear from the commit message what was the purpose of adding it. Have you tested the USDHC switch to higher modes with this change?

Andrey,

Reverting b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0() support")
does not resolve the issue. That function waits for a specified
timeout for the card to assert DAT[0] either high or low depending on
arg and is used to check for command busy or failure. The patch in
question adds that function so that the common mmc code can utilize
it. If the function does not exist in the host controller driver any
call to mmc_wait_dat0 returns -ENOSYS so it must be there to support
UHS anyway.

What is not clear to me is why the card would hold DAT[0] low on the
voltage switch indicating a failure as it does not in the Linux driver
which appears to me to be doing the same thing. Again, if we ignore
the return code UHS works fine and I'm not at all clear why you
wouldn't run into this issue:
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index a6394bc..5ea3cd2 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -579,10 +579,12 @@ static int mmc_switch_voltage(struct mmc *mmc,
int signal_voltage)
         * dat[0:3] low. Wait for at least 1 ms according to spec
         */
        err = mmc_wait_dat0(mmc, 1, 1000);
+#if 0 // hack: not clear why card always holds DAT[0] high here on
fsl_esdhc_imx
        if (err == -ENOSYS)
                udelay(1000);
        else if (err)
                return -ETIMEDOUT;
+#endif

        return 0;
 }

Honestly I don't have the time to keep delving into this. I don't have
any reason to believe that UHS has ever worked with fsl_esdhc_imx and
because my boards boot from eMMC (and HS200/HS400 works) I'm not
missing out on much by having a slow microSD.

Best regards,

Tim

IMX8MM SD UHS support

[ZHIZHIKIN Andrey]

Hello Tim,

> -----Original Message-----
> From: Tim Harvey <tharvey@gateworks.com>
> Sent: Tuesday, January 19, 2021 6:32 PM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> Cc: haibo.chen at nxp.com; Adam Ford <aford173@gmail.com>; Fabio Estevam
> <festevam@gmail.com>; Peng Fan <Peng.Fan@nxp.com>; u-boot <u-
> boot at lists.denx.de>; Stefano Babic <sbabic@denx.de>; Jaehoon Chung
> <jh80.chung@samsung.com>
> Subject: Re: IMX8MM SD UHS support
> 
> On Mon, Jan 18, 2021 at 11:38 AM ZHIZHIKIN Andrey
> <andrey.zhizhikin@leica-geosystems.com> wrote:
> >
> > Hello Tim,
> >
> > Sorry it took me quite some time to get this sorted out, but I believe I was able
> to identify an offending commit that is preventing the USDHC to switch to higher
> speed modes.
> >
> > It is in fact b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0() support"),
> reverting it makes SD Card to properly report capabilities and switch to high
> speed modes.
> >
> > Can you try to revert this on your end to see if the SD Card would start to
> operate in high speed mode?
> >
> > I'm still investigating why this addition of wait_dat0() caused this, I believe this
> is due to the fact that the same wait is already performed while voltage switch to
> handle the errata, thus this addition wait might erroneously timeout.
> >
> > ++ Haibo Chen <haibo.chen@nxp.com>
> >
> > Haibo,
> >
> > Can you please explain the purpose of adding wait_dat0() Introduced with
> commit b5874b552f? It is not clear from the commit message what was the
> purpose of adding it. Have you tested the USDHC switch to higher modes with
> this change?
> 
> Andrey,
> 
> Reverting b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0() support")
> does not resolve the issue. That function waits for a specified
> timeout for the card to assert DAT[0] either high or low depending on
> arg and is used to check for command busy or failure. The patch in
> question adds that function so that the common mmc code can utilize
> it. If the function does not exist in the host controller driver any
> call to mmc_wait_dat0 returns -ENOSYS so it must be there to support
> UHS anyway.

Perhaps, this is due to the fact that the same wait cycle is already
executed during the invocation of mmc_send_cmd above the
mmc_wait_dat0() in drivers/mmc/mmc.c.

mmc_send_cmd calls esdhc_send_cmd_common in
drivers/mmc/fsl_esdhc_imx.c, which already has a wait loop
implemented to cover the "Workaround for ESDHC errata ENGcm03648"
(as seen from the comment), which might explain why consecutive
wait fails to return within timeout value.

> 
> What is not clear to me is why the card would hold DAT[0] low on the
> voltage switch indicating a failure as it does not in the Linux driver
> which appears to me to be doing the same thing.

This behavior might be explained by the implementation I traced above.

> Again, if we ignore the return code UHS works fine and I'm not at all clear why you
> wouldn't run into this issue:
> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> index a6394bc..5ea3cd2 100644
> --- a/drivers/mmc/mmc.c
> +++ b/drivers/mmc/mmc.c
> @@ -579,10 +579,12 @@ static int mmc_switch_voltage(struct mmc *mmc,
> int signal_voltage)
>          * dat[0:3] low. Wait for at least 1 ms according to spec
>          */
>         err = mmc_wait_dat0(mmc, 1, 1000);
> +#if 0 // hack: not clear why card always holds DAT[0] high here on
> fsl_esdhc_imx
>         if (err == -ENOSYS)
>                 udelay(1000);
>         else if (err)
>                 return -ETIMEDOUT;
> +#endif
> 
>         return 0;
>  }

This is expected. When the wait_dat0 callback is undefined in dm_mmc_ops
structure - it defaults to return -ENOSYS, which effectively just inhibit a delay
in mmc_switch_voltage and returns 0 indicating that call was successful. This
all can be seen from the code snippet you've posted above and had commented
out under #if0 block.

Looking at the change you've posted, it seems to me that you haven't attempted
to revert the patch I mentioned, as by reverting it - the "if (err == -ENOSYS)"
path would've been taken and the desired return 0; would've been called.

> 
> Honestly I don't have the time to keep delving into this. I don't have
> any reason to believe that UHS has ever worked with fsl_esdhc_imx and
> because my boards boot from eMMC (and HS200/HS400 works) I'm not
> missing out on much by having a slow microSD.

I still believe (and witnessed it myself) that the original implementation was
indeed capable of successfully switching the USDHC to high speed modes.

At this point in time it might not be relevant for your implementation,
but could help those who has a severe impact from the microSD RW
performance in U-Boot.

Anyways, thanks a lot for writing back on the findings you have!

As for me, it would still be beneficial if the patch author (Haibo) could
comment on the intention of its introduction, because I clearly see
that reverting it on the current master branch does improve the behavior.

> 
> Best regards,
> 
> Tim

Cheers,
Andrey

IMX8MM SD UHS support

[Adam Ford]

On Tue, Jan 19, 2021 at 2:47 PM ZHIZHIKIN Andrey
<andrey.zhizhikin@leica-geosystems.com> wrote:
>
> Hello Tim,
>
> > -----Original Message-----
> > From: Tim Harvey <tharvey@gateworks.com>
> > Sent: Tuesday, January 19, 2021 6:32 PM
> > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> > Cc: haibo.chen at nxp.com; Adam Ford <aford173@gmail.com>; Fabio Estevam
> > <festevam@gmail.com>; Peng Fan <Peng.Fan@nxp.com>; u-boot <u-
> > boot at lists.denx.de>; Stefano Babic <sbabic@denx.de>; Jaehoon Chung
> > <jh80.chung@samsung.com>
> > Subject: Re: IMX8MM SD UHS support
> >
> > On Mon, Jan 18, 2021 at 11:38 AM ZHIZHIKIN Andrey
> > <andrey.zhizhikin@leica-geosystems.com> wrote:
> > >
> > > Hello Tim,
> > >
> > > Sorry it took me quite some time to get this sorted out, but I believe I was able
> > to identify an offending commit that is preventing the USDHC to switch to higher
> > speed modes.
> > >
> > > It is in fact b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0() support"),
> > reverting it makes SD Card to properly report capabilities and switch to high
> > speed modes.
> > >
> > > Can you try to revert this on your end to see if the SD Card would start to
> > operate in high speed mode?
> > >
> > > I'm still investigating why this addition of wait_dat0() caused this, I believe this
> > is due to the fact that the same wait is already performed while voltage switch to
> > handle the errata, thus this addition wait might erroneously timeout.
> > >
> > > ++ Haibo Chen <haibo.chen@nxp.com>
> > >
> > > Haibo,
> > >
> > > Can you please explain the purpose of adding wait_dat0() Introduced with
> > commit b5874b552f? It is not clear from the commit message what was the
> > purpose of adding it. Have you tested the USDHC switch to higher modes with
> > this change?
> >
> > Andrey,
> >
> > Reverting b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0() support")
> > does not resolve the issue. That function waits for a specified
> > timeout for the card to assert DAT[0] either high or low depending on
> > arg and is used to check for command busy or failure. The patch in
> > question adds that function so that the common mmc code can utilize
> > it. If the function does not exist in the host controller driver any
> > call to mmc_wait_dat0 returns -ENOSYS so it must be there to support
> > UHS anyway.
>
> Perhaps, this is due to the fact that the same wait cycle is already
> executed during the invocation of mmc_send_cmd above the
> mmc_wait_dat0() in drivers/mmc/mmc.c.
>
> mmc_send_cmd calls esdhc_send_cmd_common in
> drivers/mmc/fsl_esdhc_imx.c, which already has a wait loop
> implemented to cover the "Workaround for ESDHC errata ENGcm03648"
> (as seen from the comment), which might explain why consecutive
> wait fails to return within timeout value.
>
> >
> > What is not clear to me is why the card would hold DAT[0] low on the
> > voltage switch indicating a failure as it does not in the Linux driver
> > which appears to me to be doing the same thing.
>
> This behavior might be explained by the implementation I traced above.
>
> > Again, if we ignore the return code UHS works fine and I'm not at all clear why you
> > wouldn't run into this issue:
> > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> > index a6394bc..5ea3cd2 100644
> > --- a/drivers/mmc/mmc.c
> > +++ b/drivers/mmc/mmc.c
> > @@ -579,10 +579,12 @@ static int mmc_switch_voltage(struct mmc *mmc,
> > int signal_voltage)
> >          * dat[0:3] low. Wait for at least 1 ms according to spec
> >          */
> >         err = mmc_wait_dat0(mmc, 1, 1000);
> > +#if 0 // hack: not clear why card always holds DAT[0] high here on
> > fsl_esdhc_imx
> >         if (err == -ENOSYS)
> >                 udelay(1000);
> >         else if (err)
> >                 return -ETIMEDOUT;
> > +#endif
> >
> >         return 0;
> >  }
>
> This is expected. When the wait_dat0 callback is undefined in dm_mmc_ops
> structure - it defaults to return -ENOSYS, which effectively just inhibit a delay
> in mmc_switch_voltage and returns 0 indicating that call was successful. This
> all can be seen from the code snippet you've posted above and had commented
> out under #if0 block.
>
> Looking at the change you've posted, it seems to me that you haven't attempted
> to revert the patch I mentioned, as by reverting it - the "if (err == -ENOSYS)"
> path would've been taken and the desired return 0; would've been called.
>
> >
> > Honestly I don't have the time to keep delving into this. I don't have
> > any reason to believe that UHS has ever worked with fsl_esdhc_imx and
> > because my boards boot from eMMC (and HS200/HS400 works) I'm not
> > missing out on much by having a slow microSD.
>
> I still believe (and witnessed it myself) that the original implementation was
> indeed capable of successfully switching the USDHC to high speed modes.
>
> At this point in time it might not be relevant for your implementation,
> but could help those who has a severe impact from the microSD RW
> performance in U-Boot.
>
> Anyways, thanks a lot for writing back on the findings you have!
>
> As for me, it would still be beneficial if the patch author (Haibo) could
> comment on the intention of its introduction, because I clearly see
> that reverting it on the current master branch does improve the behavior.

I am planning on testing the revert and some other stuff related to
this issue, but I probably won't be able to get to it until tomorrow.

adam

>
> >
> > Best regards,
> >
> > Tim
>
> Cheers,
> Andrey

IMX8MM SD UHS support

[Bough Chen]

Hi Tim and Andrey

I can reproduce this issue on my side, after revert my patch which you point out, SD can work on SDR104 mode.
Till now, I do not know why wait for data0 status will cause this issue. Give me more time, I will try to dig that out.

Again, thanks for report this issue.

Haibo 

> -----Original Message-----
> From: ZHIZHIKIN Andrey [mailto:andrey.zhizhikin at leica-geosystems.com]
> Sent: 2021?1?20? 4:48
> To: tharvey at gateworks.com
> Cc: Bough Chen <haibo.chen@nxp.com>; Adam Ford
> <aford173@gmail.com>; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; u-boot <u-boot@lists.denx.de>; Stefano Babic
> <sbabic@denx.de>; Jaehoon Chung <jh80.chung@samsung.com>
> Subject: RE: IMX8MM SD UHS support
> 
> Hello Tim,
> 
> > -----Original Message-----
> > From: Tim Harvey <tharvey@gateworks.com>
> > Sent: Tuesday, January 19, 2021 6:32 PM
> > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> > Cc: haibo.chen at nxp.com; Adam Ford <aford173@gmail.com>; Fabio
> Estevam
> > <festevam@gmail.com>; Peng Fan <Peng.Fan@nxp.com>; u-boot <u-
> > boot at lists.denx.de>; Stefano Babic <sbabic@denx.de>; Jaehoon Chung
> > <jh80.chung@samsung.com>
> > Subject: Re: IMX8MM SD UHS support
> >
> > On Mon, Jan 18, 2021 at 11:38 AM ZHIZHIKIN Andrey
> > <andrey.zhizhikin@leica-geosystems.com> wrote:
> > >
> > > Hello Tim,
> > >
> > > Sorry it took me quite some time to get this sorted out, but I
> > > believe I was able
> > to identify an offending commit that is preventing the USDHC to switch
> > to higher speed modes.
> > >
> > > It is in fact b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0()
> > > support"),
> > reverting it makes SD Card to properly report capabilities and switch
> > to high speed modes.
> > >
> > > Can you try to revert this on your end to see if the SD Card would
> > > start to
> > operate in high speed mode?
> > >
> > > I'm still investigating why this addition of wait_dat0() caused
> > > this, I believe this
> > is due to the fact that the same wait is already performed while
> > voltage switch to handle the errata, thus this addition wait might
> erroneously timeout.
> > >
> > > ++ Haibo Chen <haibo.chen@nxp.com>
> > >
> > > Haibo,
> > >
> > > Can you please explain the purpose of adding wait_dat0() Introduced
> > > with
> > commit b5874b552f? It is not clear from the commit message what was
> > the purpose of adding it. Have you tested the USDHC switch to higher
> > modes with this change?
> >
> > Andrey,
> >
> > Reverting b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0() support")
> > does not resolve the issue. That function waits for a specified
> > timeout for the card to assert DAT[0] either high or low depending on
> > arg and is used to check for command busy or failure. The patch in
> > question adds that function so that the common mmc code can utilize
> > it. If the function does not exist in the host controller driver any
> > call to mmc_wait_dat0 returns -ENOSYS so it must be there to support
> > UHS anyway.
> 
> Perhaps, this is due to the fact that the same wait cycle is already executed
> during the invocation of mmc_send_cmd above the
> mmc_wait_dat0() in drivers/mmc/mmc.c.
> 
> mmc_send_cmd calls esdhc_send_cmd_common in
> drivers/mmc/fsl_esdhc_imx.c, which already has a wait loop implemented to
> cover the "Workaround for ESDHC errata ENGcm03648"
> (as seen from the comment), which might explain why consecutive wait fails
> to return within timeout value.
> 
> >
> > What is not clear to me is why the card would hold DAT[0] low on the
> > voltage switch indicating a failure as it does not in the Linux driver
> > which appears to me to be doing the same thing.
> 
> This behavior might be explained by the implementation I traced above.
> 
> > Again, if we ignore the return code UHS works fine and I'm not at all
> > clear why you wouldn't run into this issue:
> > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> > a6394bc..5ea3cd2 100644
> > --- a/drivers/mmc/mmc.c
> > +++ b/drivers/mmc/mmc.c
> > @@ -579,10 +579,12 @@ static int mmc_switch_voltage(struct mmc
> *mmc,
> > int signal_voltage)
> >          * dat[0:3] low. Wait for at least 1 ms according to spec
> >          */
> >         err = mmc_wait_dat0(mmc, 1, 1000);
> > +#if 0 // hack: not clear why card always holds DAT[0] high here on
> > fsl_esdhc_imx
> >         if (err == -ENOSYS)
> >                 udelay(1000);
> >         else if (err)
> >                 return -ETIMEDOUT;
> > +#endif
> >
> >         return 0;
> >  }
> 
> This is expected. When the wait_dat0 callback is undefined in dm_mmc_ops
> structure - it defaults to return -ENOSYS, which effectively just inhibit a delay
> in mmc_switch_voltage and returns 0 indicating that call was successful. This
> all can be seen from the code snippet you've posted above and had
> commented out under #if0 block.
> 
> Looking at the change you've posted, it seems to me that you haven't
> attempted to revert the patch I mentioned, as by reverting it - the "if (err ==
> -ENOSYS)"
> path would've been taken and the desired return 0; would've been called.
> 
> >
> > Honestly I don't have the time to keep delving into this. I don't have
> > any reason to believe that UHS has ever worked with fsl_esdhc_imx and
> > because my boards boot from eMMC (and HS200/HS400 works) I'm not
> > missing out on much by having a slow microSD.
> 
> I still believe (and witnessed it myself) that the original implementation was
> indeed capable of successfully switching the USDHC to high speed modes.
> 
> At this point in time it might not be relevant for your implementation, but
> could help those who has a severe impact from the microSD RW
> performance in U-Boot.
> 
> Anyways, thanks a lot for writing back on the findings you have!
> 
> As for me, it would still be beneficial if the patch author (Haibo) could
> comment on the intention of its introduction, because I clearly see that
> reverting it on the current master branch does improve the behavior.
> 
> >
> > Best regards,
> >
> > Tim
> 
> Cheers,
> Andrey

IMX8MM SD UHS support

[Bough Chen]

Hi Tim and Andrey,

I find the root cause, it is because during the 1.8v voltage switch process, SD clock do not gate off/ on according to the SD spec.
For i.MX usdhc, if want to gate off the sd card clock, need to clear the bit 8 of VEND_SPEC, and set this bit to gate on the sd card clock.
For the bit[14:11] of VEND_SPEC, our IP do not implement the function, just mark as reserved bits. 
So for current logic, we'll see the second mmc_wait_dat0() in mmc_switch_voltage() always return timeout.

Let me double confirm with our IC team, and will send a formal fix patch tomorrow.


Haibo   

> -----Original Message-----
> From: Bough Chen
> Sent: 2021?1?22? 20:42
> To: 'ZHIZHIKIN Andrey' <andrey.zhizhikin@leica-geosystems.com>;
> tharvey at gateworks.com
> Cc: Adam Ford <aford173@gmail.com>; Fabio Estevam
> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; u-boot
> <u-boot@lists.denx.de>; Stefano Babic <sbabic@denx.de>; Jaehoon Chung
> <jh80.chung@samsung.com>
> Subject: RE: IMX8MM SD UHS support
> 
> Hi Tim and Andrey
> 
> I can reproduce this issue on my side, after revert my patch which you point out,
> SD can work on SDR104 mode.
> Till now, I do not know why wait for data0 status will cause this issue. Give me
> more time, I will try to dig that out.
> 
> Again, thanks for report this issue.
> 
> Haibo
> 
> > -----Original Message-----
> > From: ZHIZHIKIN Andrey [mailto:andrey.zhizhikin at leica-geosystems.com]
> > Sent: 2021?1?20? 4:48
> > To: tharvey at gateworks.com
> > Cc: Bough Chen <haibo.chen@nxp.com>; Adam Ford
> <aford173@gmail.com>;
> > Fabio Estevam <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>;
> > u-boot <u-boot@lists.denx.de>; Stefano Babic <sbabic@denx.de>; Jaehoon
> > Chung <jh80.chung@samsung.com>
> > Subject: RE: IMX8MM SD UHS support
> >
> > Hello Tim,
> >
> > > -----Original Message-----
> > > From: Tim Harvey <tharvey@gateworks.com>
> > > Sent: Tuesday, January 19, 2021 6:32 PM
> > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> > > Cc: haibo.chen at nxp.com; Adam Ford <aford173@gmail.com>; Fabio
> > Estevam
> > > <festevam@gmail.com>; Peng Fan <Peng.Fan@nxp.com>; u-boot <u-
> > > boot at lists.denx.de>; Stefano Babic <sbabic@denx.de>; Jaehoon Chung
> > > <jh80.chung@samsung.com>
> > > Subject: Re: IMX8MM SD UHS support
> > >
> > > On Mon, Jan 18, 2021 at 11:38 AM ZHIZHIKIN Andrey
> > > <andrey.zhizhikin@leica-geosystems.com> wrote:
> > > >
> > > > Hello Tim,
> > > >
> > > > Sorry it took me quite some time to get this sorted out, but I
> > > > believe I was able
> > > to identify an offending commit that is preventing the USDHC to
> > > switch to higher speed modes.
> > > >
> > > > It is in fact b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0()
> > > > support"),
> > > reverting it makes SD Card to properly report capabilities and
> > > switch to high speed modes.
> > > >
> > > > Can you try to revert this on your end to see if the SD Card would
> > > > start to
> > > operate in high speed mode?
> > > >
> > > > I'm still investigating why this addition of wait_dat0() caused
> > > > this, I believe this
> > > is due to the fact that the same wait is already performed while
> > > voltage switch to handle the errata, thus this addition wait might
> > erroneously timeout.
> > > >
> > > > ++ Haibo Chen <haibo.chen@nxp.com>
> > > >
> > > > Haibo,
> > > >
> > > > Can you please explain the purpose of adding wait_dat0()
> > > > Introduced with
> > > commit b5874b552f? It is not clear from the commit message what was
> > > the purpose of adding it. Have you tested the USDHC switch to higher
> > > modes with this change?
> > >
> > > Andrey,
> > >
> > > Reverting b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0() support")
> > > does not resolve the issue. That function waits for a specified
> > > timeout for the card to assert DAT[0] either high or low depending
> > > on arg and is used to check for command busy or failure. The patch
> > > in question adds that function so that the common mmc code can
> > > utilize it. If the function does not exist in the host controller
> > > driver any call to mmc_wait_dat0 returns -ENOSYS so it must be there
> > > to support UHS anyway.
> >
> > Perhaps, this is due to the fact that the same wait cycle is already
> > executed during the invocation of mmc_send_cmd above the
> > mmc_wait_dat0() in drivers/mmc/mmc.c.
> >
> > mmc_send_cmd calls esdhc_send_cmd_common in
> > drivers/mmc/fsl_esdhc_imx.c, which already has a wait loop implemented
> > to cover the "Workaround for ESDHC errata ENGcm03648"
> > (as seen from the comment), which might explain why consecutive wait
> > fails to return within timeout value.
> >
> > >
> > > What is not clear to me is why the card would hold DAT[0] low on the
> > > voltage switch indicating a failure as it does not in the Linux
> > > driver which appears to me to be doing the same thing.
> >
> > This behavior might be explained by the implementation I traced above.
> >
> > > Again, if we ignore the return code UHS works fine and I'm not at
> > > all clear why you wouldn't run into this issue:
> > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> > > a6394bc..5ea3cd2 100644
> > > --- a/drivers/mmc/mmc.c
> > > +++ b/drivers/mmc/mmc.c
> > > @@ -579,10 +579,12 @@ static int mmc_switch_voltage(struct mmc
> > *mmc,
> > > int signal_voltage)
> > >          * dat[0:3] low. Wait for at least 1 ms according to spec
> > >          */
> > >         err = mmc_wait_dat0(mmc, 1, 1000);
> > > +#if 0 // hack: not clear why card always holds DAT[0] high here on
> > > fsl_esdhc_imx
> > >         if (err == -ENOSYS)
> > >                 udelay(1000);
> > >         else if (err)
> > >                 return -ETIMEDOUT;
> > > +#endif
> > >
> > >         return 0;
> > >  }
> >
> > This is expected. When the wait_dat0 callback is undefined in
> > dm_mmc_ops structure - it defaults to return -ENOSYS, which
> > effectively just inhibit a delay in mmc_switch_voltage and returns 0
> > indicating that call was successful. This all can be seen from the
> > code snippet you've posted above and had commented out under #if0 block.
> >
> > Looking at the change you've posted, it seems to me that you haven't
> > attempted to revert the patch I mentioned, as by reverting it - the
> > "if (err == -ENOSYS)"
> > path would've been taken and the desired return 0; would've been called.
> >
> > >
> > > Honestly I don't have the time to keep delving into this. I don't
> > > have any reason to believe that UHS has ever worked with
> > > fsl_esdhc_imx and because my boards boot from eMMC (and HS200/HS400
> > > works) I'm not missing out on much by having a slow microSD.
> >
> > I still believe (and witnessed it myself) that the original
> > implementation was indeed capable of successfully switching the USDHC to
> high speed modes.
> >
> > At this point in time it might not be relevant for your
> > implementation, but could help those who has a severe impact from the
> > microSD RW performance in U-Boot.
> >
> > Anyways, thanks a lot for writing back on the findings you have!
> >
> > As for me, it would still be beneficial if the patch author (Haibo)
> > could comment on the intention of its introduction, because I clearly
> > see that reverting it on the current master branch does improve the
> behavior.
> >
> > >
> > > Best regards,
> > >
> > > Tim
> >
> > Cheers,
> > Andrey

Re: IMX8MM SD UHS support

[Adam Ford]

On Mon, Jan 25, 2021 at 4:43 AM Bough Chen <haibo.chen@nxp.com> wrote:
>
> Hi Tim and Andrey,
>
> I find the root cause, it is because during the 1.8v voltage switch process, SD clock do not gate off/ on according to the SD spec.
> For i.MX usdhc, if want to gate off the sd card clock, need to clear the bit 8 of VEND_SPEC, and set this bit to gate on the sd card clock.
> For the bit[14:11] of VEND_SPEC, our IP do not implement the function, just mark as reserved bits.
> So for current logic, we'll see the second mmc_wait_dat0() in mmc_switch_voltage() always return timeout.
>
> Let me double confirm with our IC team, and will send a formal fix patch tomorrow.

Haibo,

Sorry to resurrect an old thread, but it's been almost a year since we
last discussed this, and from what I can tell, the MMC switching to
SDR104 is still broken.

I was hoping you might have a suggestion as to how to move forward.

>
>
> Haibo
>
> > -----Original Message-----
> > From: Bough Chen
> > Sent: 2021年1月22日 20:42
> > To: 'ZHIZHIKIN Andrey' <andrey.zhizhikin@leica-geosystems.com>;
> > tharvey@gateworks.com
> > Cc: Adam Ford <aford173@gmail.com>; Fabio Estevam
> > <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; u-boot
> > <u-boot@lists.denx.de>; Stefano Babic <sbabic@denx.de>; Jaehoon Chung
> > <jh80.chung@samsung.com>
> > Subject: RE: IMX8MM SD UHS support
> >
> > Hi Tim and Andrey
> >
> > I can reproduce this issue on my side, after revert my patch which you point out,
> > SD can work on SDR104 mode.
> > Till now, I do not know why wait for data0 status will cause this issue. Give me
> > more time, I will try to dig that out.
> >
> > Again, thanks for report this issue.
> >
> > Haibo
> >
> > > -----Original Message-----
> > > From: ZHIZHIKIN Andrey [mailto:andrey.zhizhikin@leica-geosystems.com]
> > > Sent: 2021年1月20日 4:48
> > > To: tharvey@gateworks.com
> > > Cc: Bough Chen <haibo.chen@nxp.com>; Adam Ford
> > <aford173@gmail.com>;
> > > Fabio Estevam <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>;
> > > u-boot <u-boot@lists.denx.de>; Stefano Babic <sbabic@denx.de>; Jaehoon
> > > Chung <jh80.chung@samsung.com>
> > > Subject: RE: IMX8MM SD UHS support
> > >
> > > Hello Tim,
> > >
> > > > -----Original Message-----
> > > > From: Tim Harvey <tharvey@gateworks.com>
> > > > Sent: Tuesday, January 19, 2021 6:32 PM
> > > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> > > > Cc: haibo.chen@nxp.com; Adam Ford <aford173@gmail.com>; Fabio
> > > Estevam
> > > > <festevam@gmail.com>; Peng Fan <Peng.Fan@nxp.com>; u-boot <u-
> > > > boot@lists.denx.de>; Stefano Babic <sbabic@denx.de>; Jaehoon Chung
> > > > <jh80.chung@samsung.com>
> > > > Subject: Re: IMX8MM SD UHS support
> > > >
> > > > On Mon, Jan 18, 2021 at 11:38 AM ZHIZHIKIN Andrey
> > > > <andrey.zhizhikin@leica-geosystems.com> wrote:
> > > > >
> > > > > Hello Tim,
> > > > >
> > > > > Sorry it took me quite some time to get this sorted out, but I
> > > > > believe I was able
> > > > to identify an offending commit that is preventing the USDHC to
> > > > switch to higher speed modes.
> > > > >
> > > > > It is in fact b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0()
> > > > > support"),
> > > > reverting it makes SD Card to properly report capabilities and
> > > > switch to high speed modes.
> > > > >
> > > > > Can you try to revert this on your end to see if the SD Card would
> > > > > start to
> > > > operate in high speed mode?
> > > > >
> > > > > I'm still investigating why this addition of wait_dat0() caused
> > > > > this, I believe this
> > > > is due to the fact that the same wait is already performed while
> > > > voltage switch to handle the errata, thus this addition wait might
> > > erroneously timeout.
> > > > >
> > > > > ++ Haibo Chen <haibo.chen@nxp.com>
> > > > >
> > > > > Haibo,
> > > > >
> > > > > Can you please explain the purpose of adding wait_dat0()
> > > > > Introduced with
> > > > commit b5874b552f? It is not clear from the commit message what was
> > > > the purpose of adding it. Have you tested the USDHC switch to higher
> > > > modes with this change?
> > > >
> > > > Andrey,
> > > >
> > > > Reverting b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0() support")
> > > > does not resolve the issue. That function waits for a specified
> > > > timeout for the card to assert DAT[0] either high or low depending
> > > > on arg and is used to check for command busy or failure. The patch
> > > > in question adds that function so that the common mmc code can
> > > > utilize it. If the function does not exist in the host controller
> > > > driver any call to mmc_wait_dat0 returns -ENOSYS so it must be there
> > > > to support UHS anyway.
> > >
> > > Perhaps, this is due to the fact that the same wait cycle is already
> > > executed during the invocation of mmc_send_cmd above the
> > > mmc_wait_dat0() in drivers/mmc/mmc.c.
> > >
> > > mmc_send_cmd calls esdhc_send_cmd_common in
> > > drivers/mmc/fsl_esdhc_imx.c, which already has a wait loop implemented
> > > to cover the "Workaround for ESDHC errata ENGcm03648"
> > > (as seen from the comment), which might explain why consecutive wait
> > > fails to return within timeout value.
> > >
> > > >
> > > > What is not clear to me is why the card would hold DAT[0] low on the
> > > > voltage switch indicating a failure as it does not in the Linux
> > > > driver which appears to me to be doing the same thing.
> > >
> > > This behavior might be explained by the implementation I traced above.
> > >
> > > > Again, if we ignore the return code UHS works fine and I'm not at
> > > > all clear why you wouldn't run into this issue:
> > > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> > > > a6394bc..5ea3cd2 100644
> > > > --- a/drivers/mmc/mmc.c
> > > > +++ b/drivers/mmc/mmc.c
> > > > @@ -579,10 +579,12 @@ static int mmc_switch_voltage(struct mmc
> > > *mmc,
> > > > int signal_voltage)
> > > >          * dat[0:3] low. Wait for at least 1 ms according to spec
> > > >          */
> > > >         err = mmc_wait_dat0(mmc, 1, 1000);
> > > > +#if 0 // hack: not clear why card always holds DAT[0] high here on
> > > > fsl_esdhc_imx
> > > >         if (err == -ENOSYS)
> > > >                 udelay(1000);
> > > >         else if (err)
> > > >                 return -ETIMEDOUT;
> > > > +#endif
> > > >
> > > >         return 0;
> > > >  }

I implemented the above hack and I can get the card to operate in
sdr104 mode, but I doubt that would be an acceptable solution.  Does
anyone have any ideas how we might be able to work around this quirk?

thanks,

adam

> > >
> > > This is expected. When the wait_dat0 callback is undefined in
> > > dm_mmc_ops structure - it defaults to return -ENOSYS, which
> > > effectively just inhibit a delay in mmc_switch_voltage and returns 0
> > > indicating that call was successful. This all can be seen from the
> > > code snippet you've posted above and had commented out under #if0 block.
> > >
> > > Looking at the change you've posted, it seems to me that you haven't
> > > attempted to revert the patch I mentioned, as by reverting it - the
> > > "if (err == -ENOSYS)"
> > > path would've been taken and the desired return 0; would've been called.
> > >
> > > >
> > > > Honestly I don't have the time to keep delving into this. I don't
> > > > have any reason to believe that UHS has ever worked with
> > > > fsl_esdhc_imx and because my boards boot from eMMC (and HS200/HS400
> > > > works) I'm not missing out on much by having a slow microSD.
> > >
> > > I still believe (and witnessed it myself) that the original
> > > implementation was indeed capable of successfully switching the USDHC to
> > high speed modes.
> > >
> > > At this point in time it might not be relevant for your
> > > implementation, but could help those who has a severe impact from the
> > > microSD RW performance in U-Boot.
> > >
> > > Anyways, thanks a lot for writing back on the findings you have!
> > >
> > > As for me, it would still be beneficial if the patch author (Haibo)
> > > could comment on the intention of its introduction, because I clearly
> > > see that reverting it on the current master branch does improve the
> > behavior.
> > >
> > > >
> > > > Best regards,
> > > >
> > > > Tim
> > >
> > > Cheers,
> > > Andrey

RE: IMX8MM SD UHS support

[Bough Chen]

[-- Attachment #1: Type: text/plain, Size: 10179 bytes --]

> -----Original Message-----
> From: Adam Ford [mailto:aford173@gmail.com]
> Sent: 2022年1月12日 7:32
> To: Bough Chen <haibo.chen@nxp.com>
> Cc: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>;
> tharvey@gateworks.com; Fabio Estevam <festevam@gmail.com>; Peng Fan
> <peng.fan@nxp.com>; u-boot <u-boot@lists.denx.de>; Stefano Babic
> <sbabic@denx.de>; Jaehoon Chung <jh80.chung@samsung.com>
> Subject: Re: IMX8MM SD UHS support
> 
> On Mon, Jan 25, 2021 at 4:43 AM Bough Chen <haibo.chen@nxp.com> wrote:
> >
> > Hi Tim and Andrey,
> >
> > I find the root cause, it is because during the 1.8v voltage switch process, SD
> clock do not gate off/ on according to the SD spec.
> > For i.MX usdhc, if want to gate off the sd card clock, need to clear the bit 8 of
> VEND_SPEC, and set this bit to gate on the sd card clock.
> > For the bit[14:11] of VEND_SPEC, our IP do not implement the function, just
> mark as reserved bits.
> > So for current logic, we'll see the second mmc_wait_dat0() in
> mmc_switch_voltage() always return timeout.
> >
> > Let me double confirm with our IC team, and will send a formal fix patch
> tomorrow.
> 
> Haibo,
> 
> Sorry to resurrect an old thread, but it's been almost a year since we last
> discussed this, and from what I can tell, the MMC switching to
> SDR104 is still broken.
> 
> I was hoping you might have a suggestion as to how to move forward.
> 

HI Adam,

The original fix patch is reverted few month ago.
Please refer to commit f132aab403271 " Revert "mmc: fsl_esdhc_imx: use VENDORSPEC_FRC_SDCLK_ON to control card clock output""
According to my test, I find for some mmc card, we can't keep the clock always on, especially when mmc internal status get stuck, seems must need the clock gate off, then the card can recover back. Otherwise, the card can't response any command. I meet this issue on one eMMC card, after meet switch error, must gate off and gate on the clock rate, then this emmc Card can recover back. I think this would be the reason of why this patched is reverted.
I'm working on this issue these days, will try to send another patch to fix all these issue.

Best Regards
Haibo Chen
> >
> >
> > Haibo
> >
> > > -----Original Message-----
> > > From: Bough Chen
> > > Sent: 2021年1月22日 20:42
> > > To: 'ZHIZHIKIN Andrey' <andrey.zhizhikin@leica-geosystems.com>;
> > > tharvey@gateworks.com
> > > Cc: Adam Ford <aford173@gmail.com>; Fabio Estevam
> > > <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; u-boot
> > > <u-boot@lists.denx.de>; Stefano Babic <sbabic@denx.de>; Jaehoon
> > > Chung <jh80.chung@samsung.com>
> > > Subject: RE: IMX8MM SD UHS support
> > >
> > > Hi Tim and Andrey
> > >
> > > I can reproduce this issue on my side, after revert my patch which
> > > you point out, SD can work on SDR104 mode.
> > > Till now, I do not know why wait for data0 status will cause this
> > > issue. Give me more time, I will try to dig that out.
> > >
> > > Again, thanks for report this issue.
> > >
> > > Haibo
> > >
> > > > -----Original Message-----
> > > > From: ZHIZHIKIN Andrey
> > > > [mailto:andrey.zhizhikin@leica-geosystems.com]
> > > > Sent: 2021年1月20日 4:48
> > > > To: tharvey@gateworks.com
> > > > Cc: Bough Chen <haibo.chen@nxp.com>; Adam Ford
> > > <aford173@gmail.com>;
> > > > Fabio Estevam <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>;
> > > > u-boot <u-boot@lists.denx.de>; Stefano Babic <sbabic@denx.de>;
> > > > Jaehoon Chung <jh80.chung@samsung.com>
> > > > Subject: RE: IMX8MM SD UHS support
> > > >
> > > > Hello Tim,
> > > >
> > > > > -----Original Message-----
> > > > > From: Tim Harvey <tharvey@gateworks.com>
> > > > > Sent: Tuesday, January 19, 2021 6:32 PM
> > > > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>
> > > > > Cc: haibo.chen@nxp.com; Adam Ford <aford173@gmail.com>; Fabio
> > > > Estevam
> > > > > <festevam@gmail.com>; Peng Fan <Peng.Fan@nxp.com>; u-boot <u-
> > > > > boot@lists.denx.de>; Stefano Babic <sbabic@denx.de>; Jaehoon
> > > > > Chung <jh80.chung@samsung.com>
> > > > > Subject: Re: IMX8MM SD UHS support
> > > > >
> > > > > On Mon, Jan 18, 2021 at 11:38 AM ZHIZHIKIN Andrey
> > > > > <andrey.zhizhikin@leica-geosystems.com> wrote:
> > > > > >
> > > > > > Hello Tim,
> > > > > >
> > > > > > Sorry it took me quite some time to get this sorted out, but I
> > > > > > believe I was able
> > > > > to identify an offending commit that is preventing the USDHC to
> > > > > switch to higher speed modes.
> > > > > >
> > > > > > It is in fact b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0()
> > > > > > support"),
> > > > > reverting it makes SD Card to properly report capabilities and
> > > > > switch to high speed modes.
> > > > > >
> > > > > > Can you try to revert this on your end to see if the SD Card
> > > > > > would start to
> > > > > operate in high speed mode?
> > > > > >
> > > > > > I'm still investigating why this addition of wait_dat0()
> > > > > > caused this, I believe this
> > > > > is due to the fact that the same wait is already performed while
> > > > > voltage switch to handle the errata, thus this addition wait
> > > > > might
> > > > erroneously timeout.
> > > > > >
> > > > > > ++ Haibo Chen <haibo.chen@nxp.com>
> > > > > >
> > > > > > Haibo,
> > > > > >
> > > > > > Can you please explain the purpose of adding wait_dat0()
> > > > > > Introduced with
> > > > > commit b5874b552f? It is not clear from the commit message what
> > > > > was the purpose of adding it. Have you tested the USDHC switch
> > > > > to higher modes with this change?
> > > > >
> > > > > Andrey,
> > > > >
> > > > > Reverting b5874b552f ("mmc: fsl_esdhc_imx: add wait_dat0()
> > > > > support") does not resolve the issue. That function waits for a
> > > > > specified timeout for the card to assert DAT[0] either high or
> > > > > low depending on arg and is used to check for command busy or
> > > > > failure. The patch in question adds that function so that the
> > > > > common mmc code can utilize it. If the function does not exist
> > > > > in the host controller driver any call to mmc_wait_dat0 returns
> > > > > -ENOSYS so it must be there to support UHS anyway.
> > > >
> > > > Perhaps, this is due to the fact that the same wait cycle is
> > > > already executed during the invocation of mmc_send_cmd above the
> > > > mmc_wait_dat0() in drivers/mmc/mmc.c.
> > > >
> > > > mmc_send_cmd calls esdhc_send_cmd_common in
> > > > drivers/mmc/fsl_esdhc_imx.c, which already has a wait loop
> > > > implemented to cover the "Workaround for ESDHC errata ENGcm03648"
> > > > (as seen from the comment), which might explain why consecutive
> > > > wait fails to return within timeout value.
> > > >
> > > > >
> > > > > What is not clear to me is why the card would hold DAT[0] low on
> > > > > the voltage switch indicating a failure as it does not in the
> > > > > Linux driver which appears to me to be doing the same thing.
> > > >
> > > > This behavior might be explained by the implementation I traced above.
> > > >
> > > > > Again, if we ignore the return code UHS works fine and I'm not
> > > > > at all clear why you wouldn't run into this issue:
> > > > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index
> > > > > a6394bc..5ea3cd2 100644
> > > > > --- a/drivers/mmc/mmc.c
> > > > > +++ b/drivers/mmc/mmc.c
> > > > > @@ -579,10 +579,12 @@ static int mmc_switch_voltage(struct mmc
> > > > *mmc,
> > > > > int signal_voltage)
> > > > >          * dat[0:3] low. Wait for at least 1 ms according to spec
> > > > >          */
> > > > >         err = mmc_wait_dat0(mmc, 1, 1000);
> > > > > +#if 0 // hack: not clear why card always holds DAT[0] high here
> > > > > +on
> > > > > fsl_esdhc_imx
> > > > >         if (err == -ENOSYS)
> > > > >                 udelay(1000);
> > > > >         else if (err)
> > > > >                 return -ETIMEDOUT;
> > > > > +#endif
> > > > >
> > > > >         return 0;
> > > > >  }
> 
> I implemented the above hack and I can get the card to operate in
> sdr104 mode, but I doubt that would be an acceptable solution.  Does anyone
> have any ideas how we might be able to work around this quirk?
> 
> thanks,
> 
> adam
> 
> > > >
> > > > This is expected. When the wait_dat0 callback is undefined in
> > > > dm_mmc_ops structure - it defaults to return -ENOSYS, which
> > > > effectively just inhibit a delay in mmc_switch_voltage and returns
> > > > 0 indicating that call was successful. This all can be seen from
> > > > the code snippet you've posted above and had commented out under #if0
> block.
> > > >
> > > > Looking at the change you've posted, it seems to me that you
> > > > haven't attempted to revert the patch I mentioned, as by reverting
> > > > it - the "if (err == -ENOSYS)"
> > > > path would've been taken and the desired return 0; would've been called.
> > > >
> > > > >
> > > > > Honestly I don't have the time to keep delving into this. I
> > > > > don't have any reason to believe that UHS has ever worked with
> > > > > fsl_esdhc_imx and because my boards boot from eMMC (and
> > > > > HS200/HS400
> > > > > works) I'm not missing out on much by having a slow microSD.
> > > >
> > > > I still believe (and witnessed it myself) that the original
> > > > implementation was indeed capable of successfully switching the
> > > > USDHC to
> > > high speed modes.
> > > >
> > > > At this point in time it might not be relevant for your
> > > > implementation, but could help those who has a severe impact from
> > > > the microSD RW performance in U-Boot.
> > > >
> > > > Anyways, thanks a lot for writing back on the findings you have!
> > > >
> > > > As for me, it would still be beneficial if the patch author
> > > > (Haibo) could comment on the intention of its introduction,
> > > > because I clearly see that reverting it on the current master
> > > > branch does improve the
> > > behavior.
> > > >
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Tim
> > > >
> > > > Cheers,
> > > > Andrey

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 9551 bytes --]