[PATCH RESEND 0/9] mmc: Fixes for Aspeed boards

These changes get the SDHCI hardware on the AST2600 and AST2500 working using the same device tree layout as upstream Linux.
The series has been tested on the Qemu models of the ast2500-evb and ast2600-evb, and tested on the ast2600a3-evb hardware.
(this is a resend as I had a stray comma in the To list that broke sending the first attempt)
Joel Stanley (9): ARM: dts: ast2600: Update SDHCI nodes ARM: dts: ast2500: Update SDHCI nodes clk/aspeed: Add debug message when clock fails clk/ast2600: Adjust eMMC clock names clk/ast2500: Add SD clock mmc/aspeed: Add debuging for clock probe failures mmc/aspeed: Probe from controller mmc/aspeed: Enable controller clocks config/ast2600: Enable eMMC related boot options
drivers/clk/aspeed/clk_ast2500.c | 26 +++++++++++++++ drivers/clk/aspeed/clk_ast2600.c | 8 ++--- drivers/mmc/aspeed_sdhci.c | 45 +++++++++++++++++++++++-- arch/arm/dts/ast2500-evb.dts | 4 +++ arch/arm/dts/ast2500-u-boot.dtsi | 25 -------------- arch/arm/dts/ast2500.dtsi | 28 ++++++++++++++++ arch/arm/dts/ast2600-evb.dts | 24 ++++++-------- arch/arm/dts/ast2600.dtsi | 57 ++++++++++++++------------------ configs/evb-ast2600_defconfig | 13 ++++++++ drivers/mmc/Kconfig | 1 + 10 files changed, 154 insertions(+), 77 deletions(-)

Match the description used by the Linux kernel, except use scu instead of syscon as the phandle.
Signed-off-by: Joel Stanley joel@jms.id.au --- arch/arm/dts/ast2600-evb.dts | 24 +++++++-------- arch/arm/dts/ast2600.dtsi | 57 +++++++++++++++--------------------- 2 files changed, 35 insertions(+), 46 deletions(-)
diff --git a/arch/arm/dts/ast2600-evb.dts b/arch/arm/dts/ast2600-evb.dts index 0d650543134a..47a0daa6dfbf 100644 --- a/arch/arm/dts/ast2600-evb.dts +++ b/arch/arm/dts/ast2600-evb.dts @@ -15,9 +15,9 @@ };
aliases { - mmc0 = &emmc_slot0; - mmc1 = &sdhci_slot0; - mmc2 = &sdhci_slot1; + mmc0 = &emmc; + mmc1 = &sdhci0; + mmc2 = &sdhci1; spi0 = &fmc; spi1 = &spi1; spi2 = &spi2; @@ -134,18 +134,16 @@ }; };
-&emmc { - u-boot,dm-pre-reloc; - timing-phase = <0x700ff>; + +&emmc_controller { + status = "okay"; };
-&emmc_slot0 { - u-boot,dm-pre-reloc; - status = "okay"; - bus-width = <4>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_emmc_default>; - sdhci-drive-type = <1>; +&emmc { + non-removable; + bus-width = <4>; + max-frequency = <100000000>; + clk-phase-mmc-hs200 = <9>, <225>; };
&i2c4 { diff --git a/arch/arm/dts/ast2600.dtsi b/arch/arm/dts/ast2600.dtsi index 64074309b7b2..3161e76941fd 100644 --- a/arch/arm/dts/ast2600.dtsi +++ b/arch/arm/dts/ast2600.dtsi @@ -416,60 +416,51 @@ status = "disabled"; };
- sdhci: sdhci@1e740000 { - #interrupt-cells = <1>; - compatible = "aspeed,aspeed-sdhci-irq", "simple-mfd"; - reg = <0x1e740000 0x1000>; - interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; - interrupt-controller; - clocks = <&scu ASPEED_CLK_GATE_SDCLK>, - <&scu ASPEED_CLK_GATE_SDEXTCLK>; - clock-names = "ctrlclk", "extclk"; + sdc: sdc@1e740000 { + compatible = "aspeed,ast2600-sd-controller"; + reg = <0x1e740000 0x100>; #address-cells = <1>; #size-cells = <1>; - ranges = <0x0 0x1e740000 0x1000>; + ranges = <0 0x1e740000 0x10000>; + clocks = <&scu ASPEED_CLK_GATE_SDCLK>; + status = "disabled";
- sdhci_slot0: sdhci_slot0@100 { - compatible = "aspeed,sdhci-ast2600"; + sdhci0: sdhci@1e740100 { + compatible = "aspeed,ast2600-sdhci", "sdhci"; reg = <0x100 0x100>; - interrupts = <0>; - interrupt-parent = <&sdhci>; + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; sdhci,auto-cmd12; clocks = <&scu ASPEED_CLK_SDIO>; status = "disabled"; };
- sdhci_slot1: sdhci_slot1@200 { - compatible = "aspeed,sdhci-ast2600"; + sdhci1: sdhci@1e740200 { + compatible = "aspeed,ast2600-sdhci", "sdhci"; reg = <0x200 0x100>; - interrupts = <1>; - interrupt-parent = <&sdhci>; + interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>; sdhci,auto-cmd12; clocks = <&scu ASPEED_CLK_SDIO>; status = "disabled"; }; };
- emmc: emmc@1e750000 { - #interrupt-cells = <1>; - compatible = "aspeed,aspeed-emmc-irq", "simple-mfd"; - reg = <0x1e750000 0x1000>; - interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>; - interrupt-controller; - clocks = <&scu ASPEED_CLK_GATE_EMMCCLK>, - <&scu ASPEED_CLK_GATE_EMMCEXTCLK>; - clock-names = "ctrlclk", "extclk"; + emmc_controller: sdc@1e750000 { + compatible = "aspeed,ast2600-sd-controller"; + reg = <0x1e750000 0x100>; #address-cells = <1>; #size-cells = <1>; - ranges = <0x0 0x1e750000 0x1000>; + ranges = <0 0x1e750000 0x10000>; + clocks = <&scu ASPEED_CLK_GATE_EMMCCLK>; + status = "disabled";
- emmc_slot0: emmc_slot0@100 { - compatible = "aspeed,emmc-ast2600"; + emmc: sdhci@1e750100 { + compatible = "aspeed,ast2600-sdhci"; reg = <0x100 0x100>; - interrupts = <0>; - interrupt-parent = <&emmc>; + sdhci,auto-cmd12; + interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>; clocks = <&scu ASPEED_CLK_EMMC>; - status = "disabled"; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_emmc_default>; }; };

On Thu, Jun 23, 2022 at 06:35:28PM +0930, Joel Stanley wrote:
Match the description used by the Linux kernel, except use scu instead of syscon as the phandle.
Signed-off-by: Joel Stanley joel@jms.id.au
For the series, applied to u-boot/next, thanks!

Match the description used by the Linux kernel, except use scu instead of syscon as the phandle.
Signed-off-by: Joel Stanley joel@jms.id.au --- arch/arm/dts/ast2500-evb.dts | 4 ++++ arch/arm/dts/ast2500-u-boot.dtsi | 25 ------------------------- arch/arm/dts/ast2500.dtsi | 28 ++++++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 25 deletions(-)
diff --git a/arch/arm/dts/ast2500-evb.dts b/arch/arm/dts/ast2500-evb.dts index 4796ed445f57..2f1f246dc460 100644 --- a/arch/arm/dts/ast2500-evb.dts +++ b/arch/arm/dts/ast2500-evb.dts @@ -60,6 +60,10 @@ pinctrl-0 = <&pinctrl_mac2link_default &pinctrl_mdio2_default>; };
+&sdmmc { + status = "okay"; +}; + &sdhci0 { status = "okay";
diff --git a/arch/arm/dts/ast2500-u-boot.dtsi b/arch/arm/dts/ast2500-u-boot.dtsi index ea60e4c8db92..057390fe707e 100644 --- a/arch/arm/dts/ast2500-u-boot.dtsi +++ b/arch/arm/dts/ast2500-u-boot.dtsi @@ -28,31 +28,6 @@ clocks = <&scu ASPEED_CLK_MPLL>; resets = <&rst ASPEED_RESET_SDRAM>; }; - - ahb { - u-boot,dm-pre-reloc; - - apb { - u-boot,dm-pre-reloc; - - sdhci0: sdhci@1e740100 { - compatible = "aspeed,ast2500-sdhci"; - reg = <0x1e740100>; - #reset-cells = <1>; - clocks = <&scu ASPEED_CLK_SDIO>; - resets = <&rst ASPEED_RESET_SDIO>; - }; - - sdhci1: sdhci@1e740200 { - compatible = "aspeed,ast2500-sdhci"; - reg = <0x1e740200>; - #reset-cells = <1>; - clocks = <&scu ASPEED_CLK_SDIO>; - resets = <&rst ASPEED_RESET_SDIO>; - }; - }; - - }; };
&uart1 { diff --git a/arch/arm/dts/ast2500.dtsi b/arch/arm/dts/ast2500.dtsi index ee66ef67042b..cea08e6f08df 100644 --- a/arch/arm/dts/ast2500.dtsi +++ b/arch/arm/dts/ast2500.dtsi @@ -207,6 +207,34 @@ reg = <0x1e720000 0x9000>; // 36K };
+ sdmmc: sd-controller@1e740000 { + compatible = "aspeed,ast2500-sd-controller"; + reg = <0x1e740000 0x100>; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0x1e740000 0x10000>; + clocks = <&scu ASPEED_CLK_GATE_SDCLK>; + status = "disabled"; + + sdhci0: sdhci@100 { + compatible = "aspeed,ast2500-sdhci"; + reg = <0x100 0x100>; + interrupts = <26>; + sdhci,auto-cmd12; + clocks = <&scu ASPEED_CLK_SDIO>; + status = "disabled"; + }; + + sdhci1: sdhci@200 { + compatible = "aspeed,ast2500-sdhci"; + reg = <0x200 0x100>; + interrupts = <26>; + sdhci,auto-cmd12; + clocks = <&scu ASPEED_CLK_SDIO>; + status = "disabled"; + }; + }; + gpio: gpio@1e780000 { #gpio-cells = <2>; gpio-controller;

A common message across platforms that prints the clock number.
Signed-off-by: Joel Stanley joel@jms.id.au --- drivers/clk/aspeed/clk_ast2500.c | 3 +++ drivers/clk/aspeed/clk_ast2600.c | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/aspeed/clk_ast2500.c b/drivers/clk/aspeed/clk_ast2500.c index a1b4496ca2c6..dcf299548de1 100644 --- a/drivers/clk/aspeed/clk_ast2500.c +++ b/drivers/clk/aspeed/clk_ast2500.c @@ -173,6 +173,7 @@ static ulong ast2500_clk_get_rate(struct clk *clk) rate = ast2500_get_uart_clk_rate(priv->scu, 5); break; default: + debug("%s: unknown clk %ld\n", __func__, clk->id); return -ENOENT; }
@@ -438,6 +439,7 @@ static ulong ast2500_clk_set_rate(struct clk *clk, ulong rate) new_rate = ast2500_configure_d2pll(priv->scu, rate); break; default: + debug("%s: unknown clk %ld\n", __func__, clk->id); return -ENOENT; }
@@ -480,6 +482,7 @@ static int ast2500_clk_enable(struct clk *clk) ast2500_configure_d2pll(priv->scu, D2PLL_DEFAULT_RATE); break; default: + debug("%s: unknown clk %ld\n", __func__, clk->id); return -ENOENT; }
diff --git a/drivers/clk/aspeed/clk_ast2600.c b/drivers/clk/aspeed/clk_ast2600.c index f191b0f31707..7d85c7f09823 100644 --- a/drivers/clk/aspeed/clk_ast2600.c +++ b/drivers/clk/aspeed/clk_ast2600.c @@ -471,7 +471,7 @@ static ulong ast2600_clk_get_rate(struct clk *clk) rate = ast2600_get_uart_huxclk_rate(priv->scu); break; default: - debug("can't get clk rate\n"); + debug("%s: unknown clk %ld\n", __func__, clk->id); return -ENOENT; }
@@ -1098,7 +1098,7 @@ static int ast2600_clk_enable(struct clk *clk) ast2600_enable_rsaclk(priv->scu); break; default: - pr_err("can't enable clk\n"); + debug("%s: unknown clk %ld\n", __func__, clk->id); return -ENOENT; }

Adjust clock to stay compatible with those used by the Linux kernel device tree.
Signed-off-by: Joel Stanley joel@jms.id.au --- drivers/clk/aspeed/clk_ast2600.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/aspeed/clk_ast2600.c b/drivers/clk/aspeed/clk_ast2600.c index 7d85c7f09823..0df1dc3718d3 100644 --- a/drivers/clk/aspeed/clk_ast2600.c +++ b/drivers/clk/aspeed/clk_ast2600.c @@ -1073,13 +1073,13 @@ static int ast2600_clk_enable(struct clk *clk) case ASPEED_CLK_GATE_SDCLK: ast2600_enable_sdclk(priv->scu); break; - case ASPEED_CLK_GATE_SDEXTCLK: + case ASPEED_CLK_SDIO: ast2600_enable_extsdclk(priv->scu); break; case ASPEED_CLK_GATE_EMMCCLK: ast2600_enable_emmcclk(priv->scu); break; - case ASPEED_CLK_GATE_EMMCEXTCLK: + case ASPEED_CLK_EMMC: ast2600_enable_extemmcclk(priv->scu); break; case ASPEED_CLK_GATE_FSICLK:

In order to use the clock from the sdhci driver, add the SD clock.
Signed-off-by: Joel Stanley joel@jms.id.au --- drivers/clk/aspeed/clk_ast2500.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
diff --git a/drivers/clk/aspeed/clk_ast2500.c b/drivers/clk/aspeed/clk_ast2500.c index dcf299548de1..623c6915b81f 100644 --- a/drivers/clk/aspeed/clk_ast2500.c +++ b/drivers/clk/aspeed/clk_ast2500.c @@ -12,6 +12,7 @@ #include <asm/arch/scu_ast2500.h> #include <dm/lists.h> #include <dt-bindings/clock/aspeed-clock.h> +#include <dt-bindings/reset/ast2500-reset.h> #include <linux/delay.h> #include <linux/err.h>
@@ -426,6 +427,25 @@ static ulong ast2500_configure_d2pll(struct ast2500_scu *scu, ulong rate) return new_rate; }
+#define SCU_CLKSTOP_SDIO 27 +static ulong ast2500_enable_sdclk(struct ast2500_scu *scu) +{ + u32 reset_bit; + u32 clkstop_bit; + + reset_bit = BIT(ASPEED_RESET_SDIO); + clkstop_bit = BIT(SCU_CLKSTOP_SDIO); + + setbits_le32(&scu->sysreset_ctrl1, reset_bit); + udelay(100); + //enable clk + clrbits_le32(&scu->clk_stop_ctrl1, clkstop_bit); + mdelay(10); + clrbits_le32(&scu->sysreset_ctrl1, reset_bit); + + return 0; +} + static ulong ast2500_clk_set_rate(struct clk *clk, ulong rate) { struct ast2500_clk_priv *priv = dev_get_priv(clk->dev); @@ -481,6 +501,9 @@ static int ast2500_clk_enable(struct clk *clk) case ASPEED_CLK_D2PLL: ast2500_configure_d2pll(priv->scu, D2PLL_DEFAULT_RATE); break; + case ASPEED_CLK_GATE_SDCLK: + ast2500_enable_sdclk(priv->scu); + break; default: debug("%s: unknown clk %ld\n", __func__, clk->id); return -ENOENT;

Signed-off-by: Joel Stanley joel@jms.id.au --- drivers/mmc/aspeed_sdhci.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/aspeed_sdhci.c b/drivers/mmc/aspeed_sdhci.c index 453731571987..c71daae97584 100644 --- a/drivers/mmc/aspeed_sdhci.c +++ b/drivers/mmc/aspeed_sdhci.c @@ -26,12 +26,16 @@ static int aspeed_sdhci_probe(struct udevice *dev) int ret;
ret = clk_get_by_index(dev, 0, &clk); - if (ret) + if (ret) { + debug("%s: clock get failed %d\n", __func__, ret); return ret; + }
ret = clk_enable(&clk); - if (ret) + if (ret) { + debug("%s: clock enable failed %d\n", __func__, ret); goto free; + }
host->name = dev->name; host->ioaddr = dev_read_addr_ptr(dev); @@ -39,6 +43,7 @@ static int aspeed_sdhci_probe(struct udevice *dev) max_clk = clk_get_rate(&clk); if (IS_ERR_VALUE(max_clk)) { ret = max_clk; + debug("%s: clock rate get failed %d\n", __func__, ret); goto err; }

The Aspeed SDHCI controller is arranged with some shared control registers, followed by one or two sets of actual SDHCI registers.
Adjust the driver to probe this controller device first. The driver then wants to iterate over the child nodes to probe the SDHCI proper:
ofnode node;
dev_for_each_subnode(node, parent) { struct udevice *dev; int ret;
ret = device_bind_driver_to_node(parent, "aspeed_sdhci", ofnode_get_name(node), node, &dev); if (ret) return ret; }
However if we did this the sdhci driver would probe twice; once "naturally" from the device tree and a second time due to this code.
Instead of doing this we can rely on the probe order, where the controller will be set up before the sdhci devices. A better solution is preferred.
Select MISC as the controller driver is implemented as a misc device.
Signed-off-by: Joel Stanley joel@jms.id.au --- drivers/mmc/aspeed_sdhci.c | 21 +++++++++++++++++++++ drivers/mmc/Kconfig | 1 + 2 files changed, 22 insertions(+)
diff --git a/drivers/mmc/aspeed_sdhci.c b/drivers/mmc/aspeed_sdhci.c index c71daae97584..5591fa2b0891 100644 --- a/drivers/mmc/aspeed_sdhci.c +++ b/drivers/mmc/aspeed_sdhci.c @@ -10,6 +10,7 @@ #include <malloc.h> #include <sdhci.h> #include <linux/err.h> +#include <dm/lists.h>
struct aspeed_sdhci_plat { struct mmc_config cfg; @@ -94,3 +95,23 @@ U_BOOT_DRIVER(aspeed_sdhci_drv) = { .priv_auto = sizeof(struct sdhci_host), .plat_auto = sizeof(struct aspeed_sdhci_plat), }; + + +static int aspeed_sdc_probe(struct udevice *parent) +{ + return 0; +} + +static const struct udevice_id aspeed_sdc_ids[] = { + { .compatible = "aspeed,ast2400-sd-controller" }, + { .compatible = "aspeed,ast2500-sd-controller" }, + { .compatible = "aspeed,ast2600-sd-controller" }, + { } +}; + +U_BOOT_DRIVER(aspeed_sdc_drv) = { + .name = "aspeed_sdc", + .id = UCLASS_MISC, + .of_match = aspeed_sdc_ids, + .probe = aspeed_sdc_probe, +}; diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index 5e2921ce41a7..07ff69afea69 100644 --- a/drivers/mmc/Kconfig +++ b/drivers/mmc/Kconfig @@ -489,6 +489,7 @@ config MMC_SDHCI_ASPEED depends on ARCH_ASPEED depends on DM_MMC depends on MMC_SDHCI + select MISC help Enables support for the Aspeed SDHCI 2.0 controller present on Aspeed SoCs. This device is compatible with SD 3.0 and/or MMC 4.3

Request and enable the controller level clocks.
Signed-off-by: Joel Stanley joel@jms.id.au --- drivers/mmc/aspeed_sdhci.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/drivers/mmc/aspeed_sdhci.c b/drivers/mmc/aspeed_sdhci.c index 5591fa2b0891..9d79bf58cc70 100644 --- a/drivers/mmc/aspeed_sdhci.c +++ b/drivers/mmc/aspeed_sdhci.c @@ -99,6 +99,21 @@ U_BOOT_DRIVER(aspeed_sdhci_drv) = {
static int aspeed_sdc_probe(struct udevice *parent) { + struct clk clk; + int ret; + + ret = clk_get_by_index(parent, 0, &clk); + if (ret) { + debug("%s: clock get failed %d\n", __func__, ret); + return ret; + } + + ret = clk_enable(&clk); + if (ret) { + debug("%s: clock enable failed %d\n", __func__, ret); + return ret; + } + return 0; }

Allow booting zImage from ext4 devices with DOS or UEFI partition layouts.
Signed-off-by: Joel Stanley joel@jms.id.au --- configs/evb-ast2600_defconfig | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig index 160bccff48e2..69f6f30c6543 100644 --- a/configs/evb-ast2600_defconfig +++ b/configs/evb-ast2600_defconfig @@ -42,6 +42,14 @@ CONFIG_SPL_DM_RESET=y CONFIG_SPL_RAM_SUPPORT=y CONFIG_SPL_RAM_DEVICE=y CONFIG_HUSH_PARSER=y +CONFIG_CMD_BOOTZ=y +# CONFIG_BOOTM_NETBSD is not set +# CONFIG_BOOTM_PLAN9 is not set +# CONFIG_BOOTM_RTEMS is not set +# CONFIG_BOOTM_VXWORKS is not set +# CONFIG_CMD_IMI is not set +# CONFIG_CMD_XIMG is not set +CONFIG_CMD_EEPROM=y CONFIG_CMD_GPIO=y CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y @@ -49,6 +57,11 @@ CONFIG_CMD_DHCP=y CONFIG_BOOTP_BOOTFILESIZE=y CONFIG_CMD_MII=y CONFIG_CMD_PING=y +CONFIG_CMD_EXT4=y +CONFIG_DOS_PARTITION=y +# CONFIG_SPL_DOS_PARTITION is not set +CONFIG_EFI_PARTITION=y +# CONFIG_SPL_EFI_PARTITION is not set CONFIG_SPL_OF_CONTROL=y CONFIG_ENV_OVERWRITE=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y
participants (2)
-
Joel Stanley
-
Tom Rini