[PATCH 0/3] mmc: meson-gx: fix mmc & scard failure on SM1 SoCs

Amlogic SM1 SoCs doesn't handle very well high clocks from the DIV2 input Thus we limit the max freq to 26MHz on SM1 SoCs until we handle higher frequencies via the first input from a composite clock.
Here 26MHz corresponds to MMC_HS clock speed.
We also add a u-boot only sm1 compatible to distinguish the controller in a new meson-sm1-u-boot.dtsi and reworks the other -u-boot.dtsi to use this for SM1 based boards.
Finally a TOFIX is added to precise the clock management should use the clock controller instead of local management with fixed clock rates.
Neil Armstrong (3): mmc: meson-gx: move arch header to local header mmc: meson-gx: limit max frequency on SM1 SoCs ARM: dts: meson-sm1: add u-boot specific MMC controller compatible
.../meson-g12b-a311d-khadas-vim3-u-boot.dtsi | 1 + arch/arm/dts/meson-khadas-vim3-u-boot.dtsi | 2 -- .../dts/meson-sm1-khadas-vim3l-u-boot.dtsi | 1 + arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi | 2 +- arch/arm/dts/meson-sm1-sei610-u-boot.dtsi | 2 +- arch/arm/dts/meson-sm1-u-boot.dtsi | 20 +++++++++++++ drivers/mmc/meson_gx_mmc.c | 28 ++++++++++++++++--- .../sd_emmc.h => drivers/mmc/meson_gx_mmc.h | 10 ++++--- 8 files changed, 54 insertions(+), 12 deletions(-) create mode 100644 arch/arm/dts/meson-sm1-u-boot.dtsi rename arch/arm/include/asm/arch-meson/sd_emmc.h => drivers/mmc/meson_gx_mmc.h (95%)

Move the asm/arch-meson/sd_emmc.h to a local meson_gx_mmc.h, remove the useless if/then and fix the meson_gx_mmc.c include.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com --- drivers/mmc/meson_gx_mmc.c | 4 ++-- .../asm/arch-meson/sd_emmc.h => drivers/mmc/meson_gx_mmc.h | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) rename arch/arm/include/asm/arch-meson/sd_emmc.h => drivers/mmc/meson_gx_mmc.h (97%)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e5..9350edf3fa 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -13,9 +13,9 @@ #include <mmc.h> #include <asm/io.h> #include <asm/gpio.h> -#include <asm/arch/sd_emmc.h> #include <linux/delay.h> #include <linux/log2.h> +#include "meson_gx_mmc.h"
static inline void *get_regbase(const struct mmc *mmc) { @@ -265,7 +265,7 @@ static int meson_mmc_probe(struct udevice *dev) cfg->host_caps = MMC_MODE_8BIT | MMC_MODE_4BIT | 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->f_max = 100000000; /* 100 MHz */; cfg->b_max = 511; /* max 512 - 1 blocks */ cfg->name = dev->name;
diff --git a/arch/arm/include/asm/arch-meson/sd_emmc.h b/drivers/mmc/meson_gx_mmc.h similarity index 97% rename from arch/arm/include/asm/arch-meson/sd_emmc.h rename to drivers/mmc/meson_gx_mmc.h index 1e9f8cf498..b4544b5562 100644 --- a/arch/arm/include/asm/arch-meson/sd_emmc.h +++ b/drivers/mmc/meson_gx_mmc.h @@ -3,14 +3,11 @@ * (C) Copyright 2016 Carlo Caione carlo@caione.org */
-#ifndef __SD_EMMC_H__ -#define __SD_EMMC_H__ +#ifndef __MESON_GX_MMC_H__ +#define __MESON_GX_MMC_H__
#include <mmc.h> -#ifndef __ASSEMBLY__ #include <linux/bitops.h> -#endif -
#define SDIO_PORT_A 0 #define SDIO_PORT_B 1

On 11/6/20 6:27 PM, Neil Armstrong wrote:
Move the asm/arch-meson/sd_emmc.h to a local meson_gx_mmc.h, remove the useless if/then and fix the meson_gx_mmc.c include.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com
drivers/mmc/meson_gx_mmc.c | 4 ++-- .../asm/arch-meson/sd_emmc.h => drivers/mmc/meson_gx_mmc.h | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) rename arch/arm/include/asm/arch-meson/sd_emmc.h => drivers/mmc/meson_gx_mmc.h (97%)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e5..9350edf3fa 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -13,9 +13,9 @@ #include <mmc.h> #include <asm/io.h> #include <asm/gpio.h> -#include <asm/arch/sd_emmc.h> #include <linux/delay.h> #include <linux/log2.h> +#include "meson_gx_mmc.h"
static inline void *get_regbase(const struct mmc *mmc) { @@ -265,7 +265,7 @@ static int meson_mmc_probe(struct udevice *dev) cfg->host_caps = MMC_MODE_8BIT | MMC_MODE_4BIT | 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->f_max = 100000000; /* 100 MHz */;
Don't need to touch. Remove ";"
cfg->b_max = 511; /* max 512 - 1 blocks */ cfg->name = dev->name;
diff --git a/arch/arm/include/asm/arch-meson/sd_emmc.h b/drivers/mmc/meson_gx_mmc.h similarity index 97% rename from arch/arm/include/asm/arch-meson/sd_emmc.h rename to drivers/mmc/meson_gx_mmc.h index 1e9f8cf498..b4544b5562 100644 --- a/arch/arm/include/asm/arch-meson/sd_emmc.h +++ b/drivers/mmc/meson_gx_mmc.h @@ -3,14 +3,11 @@
- (C) Copyright 2016 Carlo Caione carlo@caione.org
*/
-#ifndef __SD_EMMC_H__ -#define __SD_EMMC_H__ +#ifndef __MESON_GX_MMC_H__ +#define __MESON_GX_MMC_H__
#include <mmc.h> -#ifndef __ASSEMBLY__ #include <linux/bitops.h> -#endif
#define SDIO_PORT_A 0 #define SDIO_PORT_B 1

On 06/11/2020 11:27, Jaehoon Chung wrote:
On 11/6/20 6:27 PM, Neil Armstrong wrote:
Move the asm/arch-meson/sd_emmc.h to a local meson_gx_mmc.h, remove the useless if/then and fix the meson_gx_mmc.c include.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com
drivers/mmc/meson_gx_mmc.c | 4 ++-- .../asm/arch-meson/sd_emmc.h => drivers/mmc/meson_gx_mmc.h | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) rename arch/arm/include/asm/arch-meson/sd_emmc.h => drivers/mmc/meson_gx_mmc.h (97%)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 719dd1e5e5..9350edf3fa 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -13,9 +13,9 @@ #include <mmc.h> #include <asm/io.h> #include <asm/gpio.h> -#include <asm/arch/sd_emmc.h> #include <linux/delay.h> #include <linux/log2.h> +#include "meson_gx_mmc.h"
static inline void *get_regbase(const struct mmc *mmc) { @@ -265,7 +265,7 @@ static int meson_mmc_probe(struct udevice *dev) cfg->host_caps = MMC_MODE_8BIT | MMC_MODE_4BIT | 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->f_max = 100000000; /* 100 MHz */;
Don't need to touch. Remove ";"
Oops it's a spurious change, thanks for noticing.
Neil
cfg->b_max = 511; /* max 512 - 1 blocks */ cfg->name = dev->name;
diff --git a/arch/arm/include/asm/arch-meson/sd_emmc.h b/drivers/mmc/meson_gx_mmc.h similarity index 97% rename from arch/arm/include/asm/arch-meson/sd_emmc.h rename to drivers/mmc/meson_gx_mmc.h index 1e9f8cf498..b4544b5562 100644 --- a/arch/arm/include/asm/arch-meson/sd_emmc.h +++ b/drivers/mmc/meson_gx_mmc.h @@ -3,14 +3,11 @@
- (C) Copyright 2016 Carlo Caione carlo@caione.org
*/
-#ifndef __SD_EMMC_H__ -#define __SD_EMMC_H__ +#ifndef __MESON_GX_MMC_H__ +#define __MESON_GX_MMC_H__
#include <mmc.h> -#ifndef __ASSEMBLY__ #include <linux/bitops.h> -#endif
#define SDIO_PORT_A 0 #define SDIO_PORT_B 1

Amlogic SM1 SoCs doesn't handle very well high clocks from the DIV2 input Thus we limit the max freq to 26MHz on SM1 SoCs until we handle higher frequencies via the first input from a composite clock.
Here 26MHz corresponds to MMC_HS clock speed.
We also add a u-boot only sm1 compatible to distinguish the controller.
Finally a TOFIX is added to precise the clock management should use the clock controller instead of local management with fixed clock rates.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com --- drivers/mmc/meson_gx_mmc.c | 26 +++++++++++++++++++++++--- drivers/mmc/meson_gx_mmc.h | 5 +++++ 2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 9350edf3fa..2c6b6cd15b 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -17,6 +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; +} + static inline void *get_regbase(const struct mmc *mmc) { struct meson_mmc_platdata *pdata = mmc->priv; @@ -42,6 +50,8 @@ static void meson_mmc_config_clock(struct mmc *mmc) if (!mmc->clock) return;
+ /* TOFIX This should use the proper clock taken from DT */ + /* 1GHz / CLK_MAX_DIV = 15,9 MHz */ if (mmc->clock > 16000000) { clk = SD_EMMC_CLKSRC_DIV2; @@ -265,7 +275,16 @@ static int meson_mmc_probe(struct udevice *dev) cfg->host_caps = MMC_MODE_8BIT | MMC_MODE_4BIT | 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 */; + /* + * TOFIX SM1 SoCs doesn't handle very well high clocks from the DIV2 input + * Thus we limit the max freq to 26MHz on SM1 SoCs until we handle higher + * frequencies via the first input from a composite clock + * 26MHz corresponds to MMC_HS clock speed + */ + if (meson_gx_mmc_is_compatible(dev, MMC_COMPATIBLE_SM1)) + cfg->f_max = 26000000; /* 26 MHz */ + else + cfg->f_max = 100000000; /* 100 MHz */ cfg->b_max = 511; /* max 512 - 1 blocks */ cfg->name = dev->name;
@@ -308,8 +327,9 @@ int meson_mmc_bind(struct udevice *dev) }
static const struct udevice_id meson_mmc_match[] = { - { .compatible = "amlogic,meson-gx-mmc" }, - { .compatible = "amlogic,meson-axg-mmc" }, + { .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 }, { /* sentinel */ } };
diff --git a/drivers/mmc/meson_gx_mmc.h b/drivers/mmc/meson_gx_mmc.h index b4544b5562..92aec5329f 100644 --- a/drivers/mmc/meson_gx_mmc.h +++ b/drivers/mmc/meson_gx_mmc.h @@ -9,6 +9,11 @@ #include <mmc.h> #include <linux/bitops.h>
+enum meson_gx_mmc_compatible { + MMC_COMPATIBLE_GX, + MMC_COMPATIBLE_SM1, +}; + #define SDIO_PORT_A 0 #define SDIO_PORT_B 1 #define SDIO_PORT_C 2

On 11/6/20 6:27 PM, Neil Armstrong wrote:
Amlogic SM1 SoCs doesn't handle very well high clocks from the DIV2 input Thus we limit the max freq to 26MHz on SM1 SoCs until we handle higher frequencies via the first input from a composite clock.
Here 26MHz corresponds to MMC_HS clock speed.
We also add a u-boot only sm1 compatible to distinguish the controller.
Well, it's workaround for using eMMC. I didn't check all SM1 SoCs..but odroid-c4 is supported hs200 mode (200MHz) on Kernel side. It means that bootloader can also support its mode.
I'm checking this problem with odroid-c4 target. If i find how to fix, i will share.
Best Regards, Jaehoon Chung
Finally a TOFIX is added to precise the clock management should use the clock controller instead of local management with fixed clock rates.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com
drivers/mmc/meson_gx_mmc.c | 26 +++++++++++++++++++++++--- drivers/mmc/meson_gx_mmc.h | 5 +++++ 2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 9350edf3fa..2c6b6cd15b 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -17,6 +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;
+}
static inline void *get_regbase(const struct mmc *mmc) { struct meson_mmc_platdata *pdata = mmc->priv; @@ -42,6 +50,8 @@ static void meson_mmc_config_clock(struct mmc *mmc) if (!mmc->clock) return;
- /* TOFIX This should use the proper clock taken from DT */
- /* 1GHz / CLK_MAX_DIV = 15,9 MHz */ if (mmc->clock > 16000000) { clk = SD_EMMC_CLKSRC_DIV2;
@@ -265,7 +275,16 @@ static int meson_mmc_probe(struct udevice *dev) cfg->host_caps = MMC_MODE_8BIT | MMC_MODE_4BIT | 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 */;
- /*
* TOFIX SM1 SoCs doesn't handle very well high clocks from the DIV2 input
* Thus we limit the max freq to 26MHz on SM1 SoCs until we handle higher
* frequencies via the first input from a composite clock
* 26MHz corresponds to MMC_HS clock speed
*/
- if (meson_gx_mmc_is_compatible(dev, MMC_COMPATIBLE_SM1))
cfg->f_max = 26000000; /* 26 MHz */
- else
cfg->b_max = 511; /* max 512 - 1 blocks */ cfg->name = dev->name;cfg->f_max = 100000000; /* 100 MHz */
@@ -308,8 +327,9 @@ int meson_mmc_bind(struct udevice *dev) }
static const struct udevice_id meson_mmc_match[] = {
- { .compatible = "amlogic,meson-gx-mmc" },
- { .compatible = "amlogic,meson-axg-mmc" },
- { .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 }, { /* sentinel */ }
};
diff --git a/drivers/mmc/meson_gx_mmc.h b/drivers/mmc/meson_gx_mmc.h index b4544b5562..92aec5329f 100644 --- a/drivers/mmc/meson_gx_mmc.h +++ b/drivers/mmc/meson_gx_mmc.h @@ -9,6 +9,11 @@ #include <mmc.h> #include <linux/bitops.h>
+enum meson_gx_mmc_compatible {
- MMC_COMPATIBLE_GX,
- MMC_COMPATIBLE_SM1,
+};
#define SDIO_PORT_A 0 #define SDIO_PORT_B 1 #define SDIO_PORT_C 2

On 06/11/2020 11:32, Jaehoon Chung wrote:
On 11/6/20 6:27 PM, Neil Armstrong wrote:
Amlogic SM1 SoCs doesn't handle very well high clocks from the DIV2 input Thus we limit the max freq to 26MHz on SM1 SoCs until we handle higher frequencies via the first input from a composite clock.
Here 26MHz corresponds to MMC_HS clock speed.
We also add a u-boot only sm1 compatible to distinguish the controller.
Well, it's workaround for using eMMC.
It's for the next release, to have a temporary fix. The goal is to not have such workaround....
I didn't check all SM1 SoCs..but odroid-c4 is supported hs200 mode (200MHz) on Kernel side. It means that bootloader can also support its mode.
I'm checking this problem with odroid-c4 target. If i find how to fix, i will share.
H200 will need a big work including tuning and proper clock management.
If you manage to got that far, I'll be great !
Neil
Best Regards, Jaehoon Chung
Finally a TOFIX is added to precise the clock management should use the clock controller instead of local management with fixed clock rates.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com
drivers/mmc/meson_gx_mmc.c | 26 +++++++++++++++++++++++--- drivers/mmc/meson_gx_mmc.h | 5 +++++ 2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 9350edf3fa..2c6b6cd15b 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -17,6 +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;
+}
static inline void *get_regbase(const struct mmc *mmc) { struct meson_mmc_platdata *pdata = mmc->priv; @@ -42,6 +50,8 @@ static void meson_mmc_config_clock(struct mmc *mmc) if (!mmc->clock) return;
- /* TOFIX This should use the proper clock taken from DT */
- /* 1GHz / CLK_MAX_DIV = 15,9 MHz */ if (mmc->clock > 16000000) { clk = SD_EMMC_CLKSRC_DIV2;
@@ -265,7 +275,16 @@ static int meson_mmc_probe(struct udevice *dev) cfg->host_caps = MMC_MODE_8BIT | MMC_MODE_4BIT | 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 */;
- /*
* TOFIX SM1 SoCs doesn't handle very well high clocks from the DIV2 input
* Thus we limit the max freq to 26MHz on SM1 SoCs until we handle higher
* frequencies via the first input from a composite clock
* 26MHz corresponds to MMC_HS clock speed
*/
- if (meson_gx_mmc_is_compatible(dev, MMC_COMPATIBLE_SM1))
cfg->f_max = 26000000; /* 26 MHz */
- else
cfg->b_max = 511; /* max 512 - 1 blocks */ cfg->name = dev->name;cfg->f_max = 100000000; /* 100 MHz */
@@ -308,8 +327,9 @@ int meson_mmc_bind(struct udevice *dev) }
static const struct udevice_id meson_mmc_match[] = {
- { .compatible = "amlogic,meson-gx-mmc" },
- { .compatible = "amlogic,meson-axg-mmc" },
- { .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 }, { /* sentinel */ }
};
diff --git a/drivers/mmc/meson_gx_mmc.h b/drivers/mmc/meson_gx_mmc.h index b4544b5562..92aec5329f 100644 --- a/drivers/mmc/meson_gx_mmc.h +++ b/drivers/mmc/meson_gx_mmc.h @@ -9,6 +9,11 @@ #include <mmc.h> #include <linux/bitops.h>
+enum meson_gx_mmc_compatible {
- MMC_COMPATIBLE_GX,
- MMC_COMPATIBLE_SM1,
+};
#define SDIO_PORT_A 0 #define SDIO_PORT_B 1 #define SDIO_PORT_C 2

From: Neil Armstrong narmstrong@baylibre.com Date: Fri, 6 Nov 2020 11:35:30 +0100
On 06/11/2020 11:32, Jaehoon Chung wrote:
On 11/6/20 6:27 PM, Neil Armstrong wrote:
Amlogic SM1 SoCs doesn't handle very well high clocks from the DIV2 input Thus we limit the max freq to 26MHz on SM1 SoCs until we handle higher frequencies via the first input from a composite clock.
Here 26MHz corresponds to MMC_HS clock speed.
We also add a u-boot only sm1 compatible to distinguish the controller.
Well, it's workaround for using eMMC.
It's for the next release, to have a temporary fix. The goal is to not have such workaround....
I didn't check all SM1 SoCs..but odroid-c4 is supported hs200 mode (200MHz) on Kernel side. It means that bootloader can also support its mode.
I'm checking this problem with odroid-c4 target. If i find how to fix, i will share.
H200 will need a big work including tuning and proper clock management.
I worry a bit that this would make things more fragile. I have some trouble getting HS200 to work properly on the Odroid-N2 in OpenBSD (might be a clock management issue). And the fact that on both the Odroid-N2 and Odroid-C4 the eMMC isn't soldered on means that it has to work reliably on a wider variety of eMMC modules.
I believe Linux automatically switches back to a lowe-speed mode if HS200 doesn't work isn't it? Should U-Boot do something similar?
If you manage to got that far, I'll be great !
Neil
Best Regards, Jaehoon Chung
Finally a TOFIX is added to precise the clock management should use the clock controller instead of local management with fixed clock rates.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com
drivers/mmc/meson_gx_mmc.c | 26 +++++++++++++++++++++++--- drivers/mmc/meson_gx_mmc.h | 5 +++++ 2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 9350edf3fa..2c6b6cd15b 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -17,6 +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;
+}
static inline void *get_regbase(const struct mmc *mmc) { struct meson_mmc_platdata *pdata = mmc->priv; @@ -42,6 +50,8 @@ static void meson_mmc_config_clock(struct mmc *mmc) if (!mmc->clock) return;
- /* TOFIX This should use the proper clock taken from DT */
- /* 1GHz / CLK_MAX_DIV = 15,9 MHz */ if (mmc->clock > 16000000) { clk = SD_EMMC_CLKSRC_DIV2;
@@ -265,7 +275,16 @@ static int meson_mmc_probe(struct udevice *dev) cfg->host_caps = MMC_MODE_8BIT | MMC_MODE_4BIT | 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 */;
- /*
* TOFIX SM1 SoCs doesn't handle very well high clocks from the DIV2 input
* Thus we limit the max freq to 26MHz on SM1 SoCs until we handle higher
* frequencies via the first input from a composite clock
* 26MHz corresponds to MMC_HS clock speed
*/
- if (meson_gx_mmc_is_compatible(dev, MMC_COMPATIBLE_SM1))
cfg->f_max = 26000000; /* 26 MHz */
- else
cfg->b_max = 511; /* max 512 - 1 blocks */ cfg->name = dev->name;cfg->f_max = 100000000; /* 100 MHz */
@@ -308,8 +327,9 @@ int meson_mmc_bind(struct udevice *dev) }
static const struct udevice_id meson_mmc_match[] = {
- { .compatible = "amlogic,meson-gx-mmc" },
- { .compatible = "amlogic,meson-axg-mmc" },
- { .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 }, { /* sentinel */ }
};
diff --git a/drivers/mmc/meson_gx_mmc.h b/drivers/mmc/meson_gx_mmc.h index b4544b5562..92aec5329f 100644 --- a/drivers/mmc/meson_gx_mmc.h +++ b/drivers/mmc/meson_gx_mmc.h @@ -9,6 +9,11 @@ #include <mmc.h> #include <linux/bitops.h>
+enum meson_gx_mmc_compatible {
- MMC_COMPATIBLE_GX,
- MMC_COMPATIBLE_SM1,
+};
#define SDIO_PORT_A 0 #define SDIO_PORT_B 1 #define SDIO_PORT_C 2

Hi Mark,
On 11/6/20 8:53 PM, Mark Kettenis wrote:
From: Neil Armstrong narmstrong@baylibre.com Date: Fri, 6 Nov 2020 11:35:30 +0100
On 06/11/2020 11:32, Jaehoon Chung wrote:
On 11/6/20 6:27 PM, Neil Armstrong wrote:
Amlogic SM1 SoCs doesn't handle very well high clocks from the DIV2 input Thus we limit the max freq to 26MHz on SM1 SoCs until we handle higher frequencies via the first input from a composite clock.
Here 26MHz corresponds to MMC_HS clock speed.
We also add a u-boot only sm1 compatible to distinguish the controller.
Well, it's workaround for using eMMC.
It's for the next release, to have a temporary fix. The goal is to not have such workaround....
I didn't check all SM1 SoCs..but odroid-c4 is supported hs200 mode (200MHz) on Kernel side. It means that bootloader can also support its mode.
I'm checking this problem with odroid-c4 target. If i find how to fix, i will share.
H200 will need a big work including tuning and proper clock management.
I worry a bit that this would make things more fragile. I have some trouble getting HS200 to work properly on the Odroid-N2 in OpenBSD (might be a clock management issue). And the fact that on both the Odroid-N2 and Odroid-C4 the eMMC isn't soldered on means that it has to work reliably on a wider variety of eMMC modules.
I believe Linux automatically switches back to a lowe-speed mode if HS200 doesn't work isn't it? Should U-Boot do something similar?
As i know, It's provided on U-boot side, likes Linux. If HS200 or other modes are failed, then it's trying to set lower mode. But I don't know why SM1 SoC doesn't set to it. It seems that doesn't recovery to previous clock or some configs.
Best Regards, Jaehoon Chung
If you manage to got that far, I'll be great !
Neil
Best Regards, Jaehoon Chung
Finally a TOFIX is added to precise the clock management should use the clock controller instead of local management with fixed clock rates.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com
drivers/mmc/meson_gx_mmc.c | 26 +++++++++++++++++++++++--- drivers/mmc/meson_gx_mmc.h | 5 +++++ 2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c index 9350edf3fa..2c6b6cd15b 100644 --- a/drivers/mmc/meson_gx_mmc.c +++ b/drivers/mmc/meson_gx_mmc.c @@ -17,6 +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;
+}
static inline void *get_regbase(const struct mmc *mmc) { struct meson_mmc_platdata *pdata = mmc->priv; @@ -42,6 +50,8 @@ static void meson_mmc_config_clock(struct mmc *mmc) if (!mmc->clock) return;
- /* TOFIX This should use the proper clock taken from DT */
- /* 1GHz / CLK_MAX_DIV = 15,9 MHz */ if (mmc->clock > 16000000) { clk = SD_EMMC_CLKSRC_DIV2;
@@ -265,7 +275,16 @@ static int meson_mmc_probe(struct udevice *dev) cfg->host_caps = MMC_MODE_8BIT | MMC_MODE_4BIT | 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 */;
- /*
* TOFIX SM1 SoCs doesn't handle very well high clocks from the DIV2 input
* Thus we limit the max freq to 26MHz on SM1 SoCs until we handle higher
* frequencies via the first input from a composite clock
* 26MHz corresponds to MMC_HS clock speed
*/
- if (meson_gx_mmc_is_compatible(dev, MMC_COMPATIBLE_SM1))
cfg->f_max = 26000000; /* 26 MHz */
- else
cfg->b_max = 511; /* max 512 - 1 blocks */ cfg->name = dev->name;cfg->f_max = 100000000; /* 100 MHz */
@@ -308,8 +327,9 @@ int meson_mmc_bind(struct udevice *dev) }
static const struct udevice_id meson_mmc_match[] = {
- { .compatible = "amlogic,meson-gx-mmc" },
- { .compatible = "amlogic,meson-axg-mmc" },
- { .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 }, { /* sentinel */ }
};
diff --git a/drivers/mmc/meson_gx_mmc.h b/drivers/mmc/meson_gx_mmc.h index b4544b5562..92aec5329f 100644 --- a/drivers/mmc/meson_gx_mmc.h +++ b/drivers/mmc/meson_gx_mmc.h @@ -9,6 +9,11 @@ #include <mmc.h> #include <linux/bitops.h>
+enum meson_gx_mmc_compatible {
- MMC_COMPATIBLE_GX,
- MMC_COMPATIBLE_SM1,
+};
#define SDIO_PORT_A 0 #define SDIO_PORT_B 1 #define SDIO_PORT_C 2

In order to enable the Amlogic SM1 MMC controller fix, we need to add a u-boot specific MMC controller compatible.
This adds a new meson-sm1-u-boot.dtsi and reworks the other -u-boot.dtsi to use this for SM1 based boards.
Signed-off-by: Neil Armstrong narmstrong@baylibre.com --- .../meson-g12b-a311d-khadas-vim3-u-boot.dtsi | 1 + arch/arm/dts/meson-khadas-vim3-u-boot.dtsi | 2 -- .../dts/meson-sm1-khadas-vim3l-u-boot.dtsi | 1 + arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi | 2 +- arch/arm/dts/meson-sm1-sei610-u-boot.dtsi | 2 +- arch/arm/dts/meson-sm1-u-boot.dtsi | 20 +++++++++++++++++++ 6 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 arch/arm/dts/meson-sm1-u-boot.dtsi
diff --git a/arch/arm/dts/meson-g12b-a311d-khadas-vim3-u-boot.dtsi b/arch/arm/dts/meson-g12b-a311d-khadas-vim3-u-boot.dtsi index f66eca14b1..489efa150a 100644 --- a/arch/arm/dts/meson-g12b-a311d-khadas-vim3-u-boot.dtsi +++ b/arch/arm/dts/meson-g12b-a311d-khadas-vim3-u-boot.dtsi @@ -4,4 +4,5 @@ * Author: Neil Armstrong narmstrong@baylibre.com */
+#include "meson-g12-common-u-boot.dtsi" #include "meson-khadas-vim3-u-boot.dtsi" diff --git a/arch/arm/dts/meson-khadas-vim3-u-boot.dtsi b/arch/arm/dts/meson-khadas-vim3-u-boot.dtsi index b5da4fdfc3..81fd5be378 100644 --- a/arch/arm/dts/meson-khadas-vim3-u-boot.dtsi +++ b/arch/arm/dts/meson-khadas-vim3-u-boot.dtsi @@ -4,8 +4,6 @@ * Author: Neil Armstrong narmstrong@baylibre.com */
-#include "meson-g12-common-u-boot.dtsi" - / { aliases { spi0 = &spifc; diff --git a/arch/arm/dts/meson-sm1-khadas-vim3l-u-boot.dtsi b/arch/arm/dts/meson-sm1-khadas-vim3l-u-boot.dtsi index f66eca14b1..a591c0c9f2 100644 --- a/arch/arm/dts/meson-sm1-khadas-vim3l-u-boot.dtsi +++ b/arch/arm/dts/meson-sm1-khadas-vim3l-u-boot.dtsi @@ -4,4 +4,5 @@ * Author: Neil Armstrong narmstrong@baylibre.com */
+#include "meson-sm1-u-boot.dtsi" #include "meson-khadas-vim3-u-boot.dtsi" diff --git a/arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi b/arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi index 2a8f0545b1..c431988075 100644 --- a/arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi +++ b/arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi @@ -4,7 +4,7 @@ * Author: Neil Armstrong narmstrong@baylibre.com */
-#include "meson-g12-common-u-boot.dtsi" +#include "meson-sm1-u-boot.dtsi"
ðmac { snps,reset-gpio = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>; diff --git a/arch/arm/dts/meson-sm1-sei610-u-boot.dtsi b/arch/arm/dts/meson-sm1-sei610-u-boot.dtsi index 236f2468dc..8ebc1caa4a 100644 --- a/arch/arm/dts/meson-sm1-sei610-u-boot.dtsi +++ b/arch/arm/dts/meson-sm1-sei610-u-boot.dtsi @@ -4,4 +4,4 @@ * Author: Neil Armstrong narmstrong@baylibre.com */
-#include "meson-g12-common-u-boot.dtsi" +#include "meson-sm1-u-boot.dtsi" diff --git a/arch/arm/dts/meson-sm1-u-boot.dtsi b/arch/arm/dts/meson-sm1-u-boot.dtsi new file mode 100644 index 0000000000..e05d4c369a --- /dev/null +++ b/arch/arm/dts/meson-sm1-u-boot.dtsi @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright (c) 2020 BayLibre, SAS. + * Author: Neil Armstrong narmstrong@baylibre.com + */ + +#include "meson-g12-common-u-boot.dtsi" + +&sd_emmc_a { + compatible = "amlogic,meson-sm1-mmc"; +}; + +&sd_emmc_b { + compatible = "amlogic,meson-sm1-mmc"; +}; + +&sd_emmc_c { + compatible = "amlogic,meson-sm1-mmc"; +}; +

Dear Neil,
On 11/6/20 6:27 PM, Neil Armstrong wrote:
Amlogic SM1 SoCs doesn't handle very well high clocks from the DIV2 input Thus we limit the max freq to 26MHz on SM1 SoCs until we handle higher frequencies via the first input from a composite clock.
Here 26MHz corresponds to MMC_HS clock speed.
When i have checked, it's working with 52MHz.
mmc1(part 0) is current device Odroid N2> mmcinfo Device: mmc@ffe07000 Manufacturer ID: 15 OEM: 100 Name: BJTD4 Bus Speed: 52000000 Mode: MMC High Speed (52MHz) Rd Block Len: 512 MMC version 5.1 High Capacity: Yes Capacity: 29.1 GiB Bus Width: 8-bit Erase Group Size: 512 KiB HC WP Group Size: 8 MiB User Capacity: 29.1 GiB WRREL Boot Capacity: 4 MiB ENH RPMB Capacity: 4 MiB ENH
Device: sd@ffe05000 Manufacturer ID: 3 OEM: 5344 Name: SB16G Bus Speed: 50000000 Mode: SD High Speed (50MHz) Rd Block Len: 512 SD version 3.0 High Capacity: Yes Capacity: 14.8 GiB Bus Width: 4-bit Erase Group Size: 512 Bytes Odroid N2> ums 0 mmc 0
If you can wait for more time, i can fix it. I think that it's better than applying this patch. But i don't know which boards are SM1 SoC..(Odroid-c4 and VIM3L?)
Best Regards, Jaehoon Chung
We also add a u-boot only sm1 compatible to distinguish the controller in a new meson-sm1-u-boot.dtsi and reworks the other -u-boot.dtsi to use this for SM1 based boards.
Finally a TOFIX is added to precise the clock management should use the clock controller instead of local management with fixed clock rates.
Neil Armstrong (3): mmc: meson-gx: move arch header to local header mmc: meson-gx: limit max frequency on SM1 SoCs ARM: dts: meson-sm1: add u-boot specific MMC controller compatible
.../meson-g12b-a311d-khadas-vim3-u-boot.dtsi | 1 + arch/arm/dts/meson-khadas-vim3-u-boot.dtsi | 2 -- .../dts/meson-sm1-khadas-vim3l-u-boot.dtsi | 1 + arch/arm/dts/meson-sm1-odroid-c4-u-boot.dtsi | 2 +- arch/arm/dts/meson-sm1-sei610-u-boot.dtsi | 2 +- arch/arm/dts/meson-sm1-u-boot.dtsi | 20 +++++++++++++ drivers/mmc/meson_gx_mmc.c | 28 ++++++++++++++++--- .../sd_emmc.h => drivers/mmc/meson_gx_mmc.h | 10 ++++--- 8 files changed, 54 insertions(+), 12 deletions(-) create mode 100644 arch/arm/dts/meson-sm1-u-boot.dtsi rename arch/arm/include/asm/arch-meson/sd_emmc.h => drivers/mmc/meson_gx_mmc.h (95%)
participants (3)
-
Jaehoon Chung
-
Mark Kettenis
-
Neil Armstrong