[RFT PATCH 0/2] mmc: meson-gx: improve MMC reliabilty
Amlogic MMC on the GX (and later) SoCs has been problematic for years, especially with u-boot.
Linux has been fairly stable for a few years. It is using a fixed phase setting with Core = 180, Tx = 0 and Rx = 0 (the latter cannot be set starting from the v3 MMC IPs)
Still the results were not good with those settings with u-boot, on some sm1 based platforms. U-boot then started using a 270 core phase for sm1 only. This worked for most sm1 platforms but problems persist on others.
The proposal with this patchset is to use 270 for the ID phase, 180 otherwise. This works well on the platforms I have tested (Libretech's boards and VIM3L)
It would be great if others could test this and report whether this work for them or not.
If the results are good, this might be ported to Linux as well (... but the situation is less critical there)
Jerome Brunet (2): mmc: meson-gx: clean up and align on Linux settings mmc: meson-gx: set 270 core phase during the identification
drivers/mmc/meson_gx_mmc.c | 50 ++++++++++++++++++-------------------- drivers/mmc/meson_gx_mmc.h | 9 +++++-- 2 files changed, 31 insertions(+), 28 deletions(-)
* Remove obsolete comments * Set core phase to 180 regardless of the SoC like Linux * Enable always-on clock
AML mmc driver has been working okay(ish) for a few years The purpose of this patch is to bring u-boot closer to what Linux is doing
Signed-off-by: Jerome Brunet jbrunet@baylibre.com --- drivers/mmc/meson_gx_mmc.c | 45 ++++++++++++++++---------------------- drivers/mmc/meson_gx_mmc.h | 9 ++++++-- 2 files changed, 26 insertions(+), 28 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index fcf4f03d1e24..c6168792cbae 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -17,13 +17,14 @@ #include <linux/log2.h> #include "meson_gx_mmc.h"
-bool meson_gx_mmc_is_compatible(struct udevice *dev, - enum meson_gx_mmc_compatible family) -{ - enum meson_gx_mmc_compatible compat = dev_get_driver_data(dev); - - return compat == family; -} +struct meson_gx_mmc_version_data meson_gx_mmc_version[] = { + [MMC_COMPATIBLE_V2] = { + .clk_always_on = BIT(24), + }, + [MMC_COMPATIBLE_V3] = { + .clk_always_on = BIT(28), + }, +};
static inline void *get_regbase(const struct mmc *mmc) { @@ -44,13 +45,17 @@ static inline void meson_write(struct mmc *mmc, uint32_t val, int offset)
static void meson_mmc_config_clock(struct mmc *mmc) { + struct meson_mmc_plat *pdata = mmc->priv; uint32_t meson_mmc_clk = 0; unsigned int clk, clk_src, clk_div;
if (!mmc->clock) return;
- /* TOFIX This should use the proper clock taken from DT */ + /* Clk always on */ + meson_mmc_clk |= pdata->version->clk_always_on; + meson_mmc_clk |= CLK_CO_PHASE_180; + meson_mmc_clk |= CLK_TX_PHASE_000;
/* 1GHz / CLK_MAX_DIV = 15,9 MHz */ if (mmc->clock > 16000000) { @@ -62,20 +67,6 @@ static void meson_mmc_config_clock(struct mmc *mmc) } clk_div = DIV_ROUND_UP(clk, mmc->clock);
- /* - * SM1 SoCs doesn't work fine over 50MHz with CLK_CO_PHASE_180 - * If CLK_CO_PHASE_270 is used, it's more stable than other. - * Other SoCs use CLK_CO_PHASE_180 by default. - * It needs to find what is a proper value about each SoCs. - */ - if (meson_gx_mmc_is_compatible(mmc->dev, MMC_COMPATIBLE_SM1)) - meson_mmc_clk |= CLK_CO_PHASE_270; - else - meson_mmc_clk |= CLK_CO_PHASE_180; - - /* 180 phase tx clock */ - meson_mmc_clk |= CLK_TX_PHASE_000; - /* clock settings */ meson_mmc_clk |= clk_src; meson_mmc_clk |= clk_div; @@ -243,6 +234,7 @@ static const struct dm_mmc_ops meson_dm_mmc_ops = {
static int meson_mmc_of_to_plat(struct udevice *dev) { + enum meson_gx_mmc_compatible compat = dev_get_driver_data(dev); struct meson_mmc_plat *pdata = dev_get_plat(dev); fdt_addr_t addr;
@@ -251,6 +243,7 @@ static int meson_mmc_of_to_plat(struct udevice *dev) return -EINVAL;
pdata->regbase = (void *)addr; + pdata->version = &meson_gx_mmc_version[compat];
return 0; } @@ -277,7 +270,7 @@ static int meson_mmc_probe(struct udevice *dev) cfg->voltages = MMC_VDD_33_34 | MMC_VDD_32_33 | MMC_VDD_31_32 | MMC_VDD_165_195; cfg->host_caps = MMC_MODE_8BIT | MMC_MODE_4BIT | - MMC_MODE_HS_52MHz | MMC_MODE_HS; + SD_HS | MMC_MODE_HS_52MHz | MMC_MODE_HS; cfg->f_min = DIV_ROUND_UP(SD_EMMC_CLKSRC_24M, CLK_MAX_DIV); cfg->f_max = 100000000; /* 100 MHz */ cfg->b_max = 511; /* max 512 - 1 blocks */ @@ -321,9 +314,9 @@ int meson_mmc_bind(struct udevice *dev) }
static const struct udevice_id meson_mmc_match[] = { - { .compatible = "amlogic,meson-gx-mmc", .data = MMC_COMPATIBLE_GX }, - { .compatible = "amlogic,meson-axg-mmc", .data = MMC_COMPATIBLE_GX }, - { .compatible = "amlogic,meson-sm1-mmc", .data = MMC_COMPATIBLE_SM1 }, + { .compatible = "amlogic,meson-gx-mmc", .data = MMC_COMPATIBLE_V2 }, + { .compatible = "amlogic,meson-axg-mmc", .data = MMC_COMPATIBLE_V3 }, + { .compatible = "amlogic,meson-sm1-mmc", .data = MMC_COMPATIBLE_V3 }, { /* sentinel */ } };
diff --git a/drivers/mmc/meson_gx_mmc.h b/drivers/mmc/meson_gx_mmc.h index 8974b78f559d..3ec913d1b5ef 100644 --- a/drivers/mmc/meson_gx_mmc.h +++ b/drivers/mmc/meson_gx_mmc.h @@ -10,8 +10,8 @@ #include <linux/bitops.h>
enum meson_gx_mmc_compatible { - MMC_COMPATIBLE_GX, - MMC_COMPATIBLE_SM1, + MMC_COMPATIBLE_V2, + MMC_COMPATIBLE_V3, };
#define SDIO_PORT_A 0 @@ -84,7 +84,12 @@ enum meson_gx_mmc_compatible { #define MESON_SD_EMMC_CMD_RSP2 0x64 #define MESON_SD_EMMC_CMD_RSP3 0x68
+struct meson_gx_mmc_version_data { + uint32_t clk_always_on; +}; + struct meson_mmc_plat { + struct meson_gx_mmc_version_data *version; struct mmc_config cfg; struct mmc mmc; void *regbase;
It has been reported that some devices have problems with a 180 degree core phase. Setting 270 helped some of these devices. Other continue to struggle (while it works fine with 180 in Linux ... :sigh:)
Poking around the HW, it seems that setting a 270 core phase during the identification, then using 180 for the rest of the operations, helps the device operate correctly.
Signed-off-by: Jerome Brunet jbrunet@baylibre.com --- drivers/mmc/meson_gx_mmc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index c6168792cbae..284be2b9dca4 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -54,9 +54,14 @@ static void meson_mmc_config_clock(struct mmc *mmc)
/* Clk always on */ meson_mmc_clk |= pdata->version->clk_always_on; - meson_mmc_clk |= CLK_CO_PHASE_180; meson_mmc_clk |= CLK_TX_PHASE_000;
+ /* Core phase according to mode */ + if (mmc->selected_mode == MMC_LEGACY) + meson_mmc_clk |= CLK_CO_PHASE_270; + else + meson_mmc_clk |= CLK_CO_PHASE_180; + /* 1GHz / CLK_MAX_DIV = 15,9 MHz */ if (mmc->clock > 16000000) { clk = SD_EMMC_CLKSRC_DIV2;
After running some tests, I've noticed inconsistent behavior with the eMMC: On a cold boot (following a shutdown), the eMMC operates as expected within U-Boot. On a hot reboot (via the kernel's reboot command), the eMMC sometimes (not always, but if fit in error, next need to do poweroff) does not work: U-Boot is able to read the dtb file but won't boot kernel completely, or U-Boot might fail to read from the eMMC altogether.
This behavior had not occurred before (when the phase was set to 270 on axg).
On Fri, 2023-09-15 at 18:01 +0200, Jerome Brunet wrote:
It has been reported that some devices have problems with a 180 degree core phase. Setting 270 helped some of these devices. Other continue to struggle (while it works fine with 180 in Linux ... :sigh:)
Poking around the HW, it seems that setting a 270 core phase during the identification, then using 180 for the rest of the operations, helps the device operate correctly.
Signed-off-by: Jerome Brunet jbrunet@baylibre.com
drivers/mmc/meson_gx_mmc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index c6168792cbae..284be2b9dca4 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -54,9 +54,14 @@ static void meson_mmc_config_clock(struct mmc *mmc)
/* Clk always on */ meson_mmc_clk |= pdata->version->clk_always_on;
- meson_mmc_clk |= CLK_CO_PHASE_180; meson_mmc_clk |= CLK_TX_PHASE_000;
- /* Core phase according to mode */
- if (mmc->selected_mode == MMC_LEGACY)
meson_mmc_clk |= CLK_CO_PHASE_270;- else
meson_mmc_clk |= CLK_CO_PHASE_180;- /* 1GHz / CLK_MAX_DIV = 15,9 MHz */ if (mmc->clock > 16000000) { clk = SD_EMMC_CLKSRC_DIV2;
On Thu 09 Nov 2023 at 16:49, Viacheslav adeep@lexina.in wrote:
After running some tests, I've noticed inconsistent behavior with the eMMC: On a cold boot (following a shutdown), the eMMC operates as expected within U-Boot. On a hot reboot (via the kernel's reboot command), the eMMC sometimes (not always, but if fit in error, next need to do poweroff) does not work:
No sure I understand. MMC appears to work (passes the ID phases - which was the problem before) but fitImage (or dtb) you get is not correct out of eMMC read, is not correct ?
Have you checked the content ? some checksum maybe ?
What does 'mmc info' reports on boot and reboot (when it fails or not) ?
U-Boot is able to read the dtb file but won't boot kernel completely, or U-Boot might fail to read from the eMMC altogether.This behavior had not occurred before (when the phase was set to 270 on axg).
On Fri, 2023-09-15 at 18:01 +0200, Jerome Brunet wrote:
It has been reported that some devices have problems with a 180 degree core phase. Setting 270 helped some of these devices. Other continue to struggle (while it works fine with 180 in Linux ... :sigh:)
Poking around the HW, it seems that setting a 270 core phase during the identification, then using 180 for the rest of the operations, helps the device operate correctly.
Signed-off-by: Jerome Brunet jbrunet@baylibre.com
drivers/mmc/meson_gx_mmc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index c6168792cbae..284be2b9dca4 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -54,9 +54,14 @@ static void meson_mmc_config_clock(struct mmc *mmc)
/* Clk always on */ meson_mmc_clk |= pdata->version->clk_always_on;
- meson_mmc_clk |= CLK_CO_PHASE_180; meson_mmc_clk |= CLK_TX_PHASE_000;
- /* Core phase according to mode */
- if (mmc->selected_mode == MMC_LEGACY)
meson_mmc_clk |= CLK_CO_PHASE_270;- else
meson_mmc_clk |= CLK_CO_PHASE_180;- /* 1GHz / CLK_MAX_DIV = 15,9 MHz */ if (mmc->clock > 16000000) { clk = SD_EMMC_CLKSRC_DIV2;
Hi,
On 15/09/2023 18:01, Jerome Brunet wrote:
Amlogic MMC on the GX (and later) SoCs has been problematic for years, especially with u-boot.
Linux has been fairly stable for a few years. It is using a fixed phase setting with Core = 180, Tx = 0 and Rx = 0 (the latter cannot be set starting from the v3 MMC IPs)
Still the results were not good with those settings with u-boot, on some sm1 based platforms. U-boot then started using a 270 core phase for sm1 only. This worked for most sm1 platforms but problems persist on others.
The proposal with this patchset is to use 270 for the ID phase, 180 otherwise. This works well on the platforms I have tested (Libretech's boards and VIM3L)
It would be great if others could test this and report whether this work for them or not.
If the results are good, this might be ported to Linux as well (... but the situation is less critical there)
Jerome Brunet (2): mmc: meson-gx: clean up and align on Linux settings mmc: meson-gx: set 270 core phase during the identification
drivers/mmc/meson_gx_mmc.c | 50 ++++++++++++++++++-------------------- drivers/mmc/meson_gx_mmc.h | 9 +++++-- 2 files changed, 31 insertions(+), 28 deletions(-)
I'll apply on my test branch which triggers my autobuilder providing ready-to-boot binaries for most of the supported platforms: https://gitlab.com/superna9999/amlogic-u-boot-autotest/-/pipelines
Neil
From: Neil Armstrong neil.armstrong@linaro.org
Hi,
On Fri, 15 Sep 2023 18:01:28 +0200, Jerome Brunet wrote:
Amlogic MMC on the GX (and later) SoCs has been problematic for years, especially with u-boot.
Linux has been fairly stable for a few years. It is using a fixed phase setting with Core = 180, Tx = 0 and Rx = 0 (the latter cannot be set starting from the v3 MMC IPs)
[...]
Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-amlogic (u-boot-amlogic-test)
[1/2] mmc: meson-gx: clean up and align on Linux settings https://source.denx.de/u-boot/custodians/u-boot-amlogic/-/commit/1f505e80e20... [2/2] mmc: meson-gx: set 270 core phase during the identification https://source.denx.de/u-boot/custodians/u-boot-amlogic/-/commit/100f9e63762...
This triggered the following build https://gitlab.com/superna9999/amlogic-u-boot-autotest/-/pipelines/101939757... which will produce ready-to-boot binaries on most of the supported platforms in https://github.com/LibreELEC/amlogic-boot-fip FIP repo.
Hi, Jerome!
Seems works for me. Tested on axg (A113X) and gxl (S905W).
Tested-by: Viacheslav Bocharov adeep@lexina.in
On 15/09/2023 19.01, Jerome Brunet wrote:
Amlogic MMC on the GX (and later) SoCs has been problematic for years, especially with u-boot.
Linux has been fairly stable for a few years. It is using a fixed phase setting with Core = 180, Tx = 0 and Rx = 0 (the latter cannot be set starting from the v3 MMC IPs)
Still the results were not good with those settings with u-boot, on some sm1 based platforms. U-boot then started using a 270 core phase for sm1 only. This worked for most sm1 platforms but problems persist on others.
The proposal with this patchset is to use 270 for the ID phase, 180 otherwise. This works well on the platforms I have tested (Libretech's boards and VIM3L)
It would be great if others could test this and report whether this work for them or not.
If the results are good, this might be ported to Linux as well (... but the situation is less critical there)
Jerome Brunet (2): mmc: meson-gx: clean up and align on Linux settings mmc: meson-gx: set 270 core phase during the identification
drivers/mmc/meson_gx_mmc.c | 50 ++++++++++++++++++-------------------- drivers/mmc/meson_gx_mmc.h | 9 +++++-- 2 files changed, 31 insertions(+), 28 deletions(-)
On 15/09/2023 18:01, Jerome Brunet wrote:
Amlogic MMC on the GX (and later) SoCs has been problematic for years, especially with u-boot.
Linux has been fairly stable for a few years. It is using a fixed phase setting with Core = 180, Tx = 0 and Rx = 0 (the latter cannot be set starting from the v3 MMC IPs)
Still the results were not good with those settings with u-boot, on some sm1 based platforms. U-boot then started using a 270 core phase for sm1 only. This worked for most sm1 platforms but problems persist on others.
The proposal with this patchset is to use 270 for the ID phase, 180 otherwise. This works well on the platforms I have tested (Libretech's boards and VIM3L)
It would be great if others could test this and report whether this work for them or not.
If the results are good, this might be ported to Linux as well (... but the situation is less critical there)
Jerome Brunet (2): mmc: meson-gx: clean up and align on Linux settings mmc: meson-gx: set 270 core phase during the identification
drivers/mmc/meson_gx_mmc.c | 50 ++++++++++++++++++-------------------- drivers/mmc/meson_gx_mmc.h | 9 +++++-- 2 files changed, 31 insertions(+), 28 deletions(-)
I got a regression with this patchset on the BPI M5 (SM1) SDCard, u-boot pytest results: - https://gitlab.com/amlogic-foss/amlogic-u-boot-autotest/-/jobs/5603936884 - full HTML log: https://amlogic-foss.gitlab.io/-/amlogic-u-boot-autotest/-/jobs/5603936884/a... Same device but without the patch: - https://gitlab.com/amlogic-foss/amlogic-u-boot-autotest/-/jobs/5601899556 - full HTML log: https://amlogic-foss.gitlab.io/-/amlogic-u-boot-autotest/-/jobs/5601899556/a... Same branch but on A311D gives expected results: - https://gitlab.com/amlogic-foss/amlogic-u-boot-autotest/-/jobs/5603932746 - full HTML log: https://amlogic-foss.gitlab.io/-/amlogic-u-boot-autotest/-/jobs/5603932746/a...
eMMC devices gets 26MHz speed instead of 52MHz, and SD reads and writes gives random errors:
=> mmc info Device: mmc@ffe07000 Manufacturer ID: 15 OEM: 0 Name: AJTD4R Bus Speed: 26000000 Mode: MMC High Speed (26MHz) Rd Block Len: 512 MMC version 5.1 High Capacity: Yes Capacity: 4 MiB Bus Width: 8-bit Erase Group Size: 512 KiB HC WP Group Size: 8 MiB User Capacity: 14.6 GiB WRREL Boot Capacity: 4 MiB ENH RPMB Capacity: 4 MiB ENH Boot area 0 is not write protected Boot area 1 is not write protected
=> mmc dev 0 ** fs_devread read error - block switch to partitions #0, OK mmc0 is current device => mmc read 0x00200000 0 1000 MMC read: dev # 0, block # 0, count 4096 ... 0 blocks read: ERROR => mmc write 0x00200000 200000 3e8 MMC write: dev # 0, block # 2097152, count 1000 ... 0 blocks written: ERROR
I ran the test twice to be sure.
Neil
On Thu, Nov 23, 2023 at 8:54 AM Neil Armstrong neil.armstrong@linaro.org wrote:
On 15/09/2023 18:01, Jerome Brunet wrote:
Amlogic MMC on the GX (and later) SoCs has been problematic for years, especially with u-boot.
Linux has been fairly stable for a few years. It is using a fixed phase setting with Core = 180, Tx = 0 and Rx = 0 (the latter cannot be set starting from the v3 MMC IPs)
Still the results were not good with those settings with u-boot, on some sm1 based platforms. U-boot then started using a 270 core phase for sm1 only. This worked for most sm1 platforms but problems persist on others.
The proposal with this patchset is to use 270 for the ID phase, 180 otherwise. This works well on the platforms I have tested (Libretech's boards and VIM3L)
It would be great if others could test this and report whether this work for them or not.
If the results are good, this might be ported to Linux as well (... but the situation is less critical there)
Jerome Brunet (2): mmc: meson-gx: clean up and align on Linux settings mmc: meson-gx: set 270 core phase during the identification
drivers/mmc/meson_gx_mmc.c | 50 ++++++++++++++++++-------------------- drivers/mmc/meson_gx_mmc.h | 9 +++++-- 2 files changed, 31 insertions(+), 28 deletions(-)
I got a regression with this patchset on the BPI M5 (SM1) SDCard, u-boot pytest results:
- https://gitlab.com/amlogic-foss/amlogic-u-boot-autotest/-/jobs/5603936884
- full HTML log: https://amlogic-foss.gitlab.io/-/amlogic-u-boot-autotest/-/jobs/5603936884/a...
Same device but without the patch:
- https://gitlab.com/amlogic-foss/amlogic-u-boot-autotest/-/jobs/5601899556
- full HTML log: https://amlogic-foss.gitlab.io/-/amlogic-u-boot-autotest/-/jobs/5601899556/a...
Same branch but on A311D gives expected results:
- https://gitlab.com/amlogic-foss/amlogic-u-boot-autotest/-/jobs/5603932746
- full HTML log: https://amlogic-foss.gitlab.io/-/amlogic-u-boot-autotest/-/jobs/5603932746/a...
eMMC devices gets 26MHz speed instead of 52MHz, and SD reads and writes gives random errors:
=> mmc info Device: mmc@ffe07000 Manufacturer ID: 15 OEM: 0 Name: AJTD4R Bus Speed: 26000000 Mode: MMC High Speed (26MHz) Rd Block Len: 512 MMC version 5.1 High Capacity: Yes Capacity: 4 MiB Bus Width: 8-bit Erase Group Size: 512 KiB HC WP Group Size: 8 MiB User Capacity: 14.6 GiB WRREL Boot Capacity: 4 MiB ENH RPMB Capacity: 4 MiB ENH Boot area 0 is not write protected Boot area 1 is not write protected
=> mmc dev 0 ** fs_devread read error - block switch to partitions #0, OK mmc0 is current device => mmc read 0x00200000 0 1000 MMC read: dev # 0, block # 0, count 4096 ... 0 blocks read: ERROR => mmc write 0x00200000 200000 3e8 MMC write: dev # 0, block # 2097152, count 1000 ... 0 blocks written: ERROR
I ran the test twice to be sure.
I experienced some issues with certain SD cards as well on AML-S905D3-CC-V02. I have a bunch of SD cards and will go through testing them later this week.
=> mmc dev 1 switch to partitions #0, OK mmc1 is current device => mmc info Device: mmc@ffe05000 Manufacturer ID: df OEM: 2104 Name: SDBus Speed: 25000000 Mode: MMC legacy Rd Block Len: 512 SD version 3.0 High Capacity: Yes Capacity: 58.2 GiB Bus Width: 1-bit Erase Group Size: 512 Bytes => mmc read $kernel_addr_r 0 10000
MMC read: dev # 1, block # 0, count 65536 ... 0 blocks read: ERROR
Neil
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#1889): https://groups.io/g/u-boot-amlogic/message/1889 Mute This Topic: https://groups.io/mt/102766907/1922544 Group Owner: u-boot-amlogic+owner@groups.io Unsubscribe: https://groups.io/g/u-boot-amlogic/leave/4137477/1922544/1183329822/xyzzy [da@lessconfused.com] -=-=-=-=-=-=-=-=-=-=-=-
participants (5)
-
Da Xue -
Jerome Brunet -
Neil Armstrong -
neil.armstrong＠linaro.org
-
Viacheslav