[U-Boot] [PATCH v2 0/4] ARM: aspeed: Add SD host controller driver

[U-Boot] [PATCH v2 0/4] ARM: aspeed: Add SD host controller driver

[Eddie James]

This series adds support for the SD host controller on Aspeed ast2XXX SocS.
It also enables MMC and the SD controller on the ast2500 EVB.

Changes since v1:
 - split the patch up
 - Add defconfig and dts changes

Eddie James (4):
  clk: aspeed: Add support for SD clock
  mmc: Add Aspeed SD controller driver
  aspeed: Support SD controller on the ast2500 board
  ARM: dts: ast2500: Add SDHCI nodes

 arch/arm/dts/ast2500-evb.dts                   | 14 +++++
 arch/arm/dts/ast2500-u-boot.dtsi               | 16 ++++++
 arch/arm/include/asm/arch-aspeed/scu_ast2500.h |  3 +
 arch/arm/include/asm/gpio.h                    |  3 +-
 arch/arm/mach-aspeed/ast2500-board.c           |  3 +
 configs/evb-ast2500_defconfig                  |  6 ++
 drivers/clk/aspeed/clk_ast2500.c               | 27 +++++++++
 drivers/mmc/Kconfig                            | 11 ++++
 drivers/mmc/Makefile                           |  1 +
 drivers/mmc/aspeed_sdhci.c                     | 78 ++++++++++++++++++++++++++
 drivers/pinctrl/aspeed/pinctrl_ast2500.c       |  2 +
 11 files changed, 163 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mmc/aspeed_sdhci.c

-- 
1.8.3.1

[U-Boot] [PATCH v2 1/4] clk: aspeed: Add support for SD clock

[Eddie James]

Add code to enable the SD clock on the ast2500 SoC.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 arch/arm/include/asm/arch-aspeed/scu_ast2500.h |  3 +++
 drivers/clk/aspeed/clk_ast2500.c               | 27 ++++++++++++++++++++++++++
 drivers/pinctrl/aspeed/pinctrl_ast2500.c       |  2 ++
 3 files changed, 32 insertions(+)

diff --git a/arch/arm/include/asm/arch-aspeed/scu_ast2500.h b/arch/arm/include/asm/arch-aspeed/scu_ast2500.h
index 4988ced..8db4901 100644
--- a/arch/arm/include/asm/arch-aspeed/scu_ast2500.h
+++ b/arch/arm/include/asm/arch-aspeed/scu_ast2500.h
@@ -22,6 +22,8 @@
 #define SCU_MPLL_POST_MASK		(0x3f << SCU_MPLL_POST_SHIFT)
 #define SCU_PCLK_DIV_SHIFT		23
 #define SCU_PCLK_DIV_MASK		(7 << SCU_PCLK_DIV_SHIFT)
+#define SCU_SDCLK_DIV_SHIFT		12
+#define SCU_SDCLK_DIV_MASK		(7 << SCU_SDCLK_DIV_SHIFT)
 #define SCU_HPLL_DENUM_SHIFT		0
 #define SCU_HPLL_DENUM_MASK		0x1f
 #define SCU_HPLL_NUM_SHIFT		5
@@ -107,6 +109,7 @@
 
 #define SCU_CLKSTOP_MAC1		(1 << 20)
 #define SCU_CLKSTOP_MAC2		(1 << 21)
+#define SCU_CLKSTOP_SDCLK		(1 << 27)
 
 #define SCU_D2PLL_EXT1_OFF		(1 << 0)
 #define SCU_D2PLL_EXT1_BYPASS		(1 << 1)
diff --git a/drivers/clk/aspeed/clk_ast2500.c b/drivers/clk/aspeed/clk_ast2500.c
index dbee13a..9249cf9 100644
--- a/drivers/clk/aspeed/clk_ast2500.c
+++ b/drivers/clk/aspeed/clk_ast2500.c
@@ -143,6 +143,17 @@ static ulong ast2500_clk_get_rate(struct clk *clk)
 			rate = rate / apb_div;
 		}
 		break;
+	case BCLK_SDCLK:
+		{
+			ulong apb_div = 4 + 4 * ((readl(&priv->scu->clk_sel1)
+						  & SCU_SDCLK_DIV_MASK)
+						 >> SCU_SDCLK_DIV_SHIFT);
+			rate = ast2500_get_hpll_rate(clkin,
+						     readl(&priv->
+							   scu->h_pll_param));
+			rate = rate / apb_div;
+		}
+		break;
 	case PCLK_UART1:
 		rate = ast2500_get_uart_clk_rate(priv->scu, 1);
 		break;
@@ -436,6 +447,22 @@ static int ast2500_clk_enable(struct clk *clk)
 	struct ast2500_clk_priv *priv = dev_get_priv(clk->dev);
 
 	switch (clk->id) {
+	case BCLK_SDCLK:
+		if (readl(&priv->scu->clk_stop_ctrl1) & SCU_CLKSTOP_SDCLK) {
+			ast_scu_unlock(priv->scu);
+
+			setbits_le32(&priv->scu->sysreset_ctrl1,
+				     SCU_SYSRESET_SDIO);
+			udelay(100);
+			clrbits_le32(&priv->scu->clk_stop_ctrl1,
+				     SCU_CLKSTOP_SDCLK);
+			mdelay(10);
+			clrbits_le32(&priv->scu->sysreset_ctrl1,
+				     SCU_SYSRESET_SDIO);
+
+			ast_scu_lock(priv->scu);
+		}
+		break;
 	/*
 	 * For MAC clocks the clock rate is
 	 * configured based on whether RGMII or RMII mode has been selected
diff --git a/drivers/pinctrl/aspeed/pinctrl_ast2500.c b/drivers/pinctrl/aspeed/pinctrl_ast2500.c
index ed333b9..a6e9c0d 100644
--- a/drivers/pinctrl/aspeed/pinctrl_ast2500.c
+++ b/drivers/pinctrl/aspeed/pinctrl_ast2500.c
@@ -58,6 +58,8 @@ static const struct ast2500_group_config ast2500_groups[] = {
 	{ "MDIO1", 3, (1 << 31) | (1 << 30) },
 	{ "MAC2LINK", 1, (1 << 1) },
 	{ "MDIO2", 5, (1 << 2) },
+	{ "SD1", 5, (1 << 0) },
+	{ "SD2", 5, (1 << 1) },
 };
 
 static int ast2500_pinctrl_get_groups_count(struct udevice *dev)
-- 
1.8.3.1

[U-Boot] [PATCH v2 2/4] mmc: Add Aspeed SD controller driver

[Eddie James]

Add support for the Aspeed SD host controller engine.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/mmc/Kconfig        | 11 +++++++
 drivers/mmc/Makefile       |  1 +
 drivers/mmc/aspeed_sdhci.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+)
 create mode 100644 drivers/mmc/aspeed_sdhci.c

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index c6812f6..536f66a 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -421,6 +421,17 @@ config SPL_MMC_SDHCI_ADMA
 	  This enables support for the ADMA (Advanced DMA) defined
 	  in the SD Host Controller Standard Specification Version 3.00 in SPL.
 
+config MMC_SDHCI_ASPEED
+	bool "Aspeed SDHCI controller"
+	depends on ARCH_ASPEED
+	depends on DM_MMC
+	depends on MMC_SDHCI
+	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
+	  specifications. On the AST2600, the device is also compatible with
+	  MMC 5.1 and eMMC 3.0.
+
 config MMC_SDHCI_ATMEL
 	bool "Atmel SDHCI controller support"
 	depends on ARCH_AT91
diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
index 6cc018b..5594195 100644
--- a/drivers/mmc/Makefile
+++ b/drivers/mmc/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_JZ47XX_MMC) += jz_mmc.o
 
 # SDHCI
 obj-$(CONFIG_MMC_SDHCI)			+= sdhci.o
+obj-$(CONFIG_MMC_SDHCI_ASPEED)		+= aspeed_sdhci.o
 obj-$(CONFIG_MMC_SDHCI_ATMEL)		+= atmel_sdhci.o
 obj-$(CONFIG_MMC_SDHCI_BCM2835)		+= bcm2835_sdhci.o
 obj-$(CONFIG_MMC_SDHCI_BCMSTB)		+= bcmstb_sdhci.o
diff --git a/drivers/mmc/aspeed_sdhci.c b/drivers/mmc/aspeed_sdhci.c
new file mode 100644
index 0000000..c292c42
--- /dev/null
+++ b/drivers/mmc/aspeed_sdhci.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019 IBM Corp.
+ * Eddie James <eajames@linux.ibm.com>
+ */
+
+#include <common.h>
+#include <clk.h>
+#include <dm.h>
+#include <malloc.h>
+#include <sdhci.h>
+
+struct aspeed_sdhci_plat {
+	struct mmc_config cfg;
+	struct mmc mmc;
+};
+
+static int aspeed_sdhci_probe(struct udevice *dev)
+{
+	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
+	struct aspeed_sdhci_plat *plat = dev_get_platdata(dev);
+	struct sdhci_host *host = dev_get_priv(dev);
+	u32 max_clk;
+	struct clk clk;
+	int ret;
+
+	ret = clk_get_by_index(dev, 0, &clk);
+	if (ret)
+		return ret;
+
+	ret = clk_enable(&clk);
+	if (ret)
+		return ret;
+
+	host->name = dev->name;
+	host->ioaddr = (void *)devfdt_get_addr(dev);
+
+	max_clk = clk_get_rate(&clk);
+	if (!max_clk)
+		return -EINVAL;
+
+	host->max_clk = max_clk;
+	host->mmc = &plat->mmc;
+	host->mmc->dev = dev;
+	host->mmc->priv = host;
+	upriv->mmc = host->mmc;
+
+	ret = sdhci_setup_cfg(&plat->cfg, host, 0, 0);
+	if (ret)
+		return ret;
+
+	return sdhci_probe(dev);
+}
+
+static int aspeed_sdhci_bind(struct udevice *dev)
+{
+	struct aspeed_sdhci_plat *plat = dev_get_platdata(dev);
+
+	return sdhci_bind(dev, &plat->mmc, &plat->cfg);
+}
+
+static const struct udevice_id aspeed_sdhci_ids[] = {
+	{ .compatible = "aspeed,ast2400-sdhci" },
+	{ .compatible = "aspeed,ast2500-sdhci" },
+	{ .compatible = "aspeed,ast2600-sdhci" },
+	{ }
+};
+
+U_BOOT_DRIVER(aspeed_sdhci_drv) = {
+	.name		= "aspeed_sdhci",
+	.id		= UCLASS_MMC,
+	.of_match	= aspeed_sdhci_ids,
+	.ops		= &sdhci_ops,
+	.bind		= aspeed_sdhci_bind,
+	.probe		= aspeed_sdhci_probe,
+	.priv_auto_alloc_size = sizeof(struct sdhci_host),
+	.platdata_auto_alloc_size = sizeof(struct aspeed_sdhci_plat),
+};
-- 
1.8.3.1

[U-Boot] [PATCH v2 3/4] aspeed: Support SD controller on the ast2500 board

[Eddie James]

Initialize the MMC subsystem on the ast2500 board. Compile MMC and the
Aspeed SD controller on the ast2500 EVB.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 arch/arm/include/asm/gpio.h          | 3 ++-
 arch/arm/mach-aspeed/ast2500-board.c | 3 +++
 configs/evb-ast2500_defconfig        | 6 ++++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/gpio.h b/arch/arm/include/asm/gpio.h
index 370031f..38a5922 100644
--- a/arch/arm/include/asm/gpio.h
+++ b/arch/arm/include/asm/gpio.h
@@ -1,6 +1,7 @@
 #if !defined(CONFIG_ARCH_UNIPHIER) && !defined(CONFIG_ARCH_STI) && \
 	!defined(CONFIG_ARCH_K3) && !defined(CONFIG_ARCH_BCM6858) && \
-	!defined(CONFIG_ARCH_BCM63158) && !defined(CONFIG_ARCH_ROCKCHIP)
+	!defined(CONFIG_ARCH_BCM63158) && !defined(CONFIG_ARCH_ROCKCHIP) && \
+	!defined(CONFIG_ARCH_ASPEED)
 #include <asm/arch/gpio.h>
 #endif
 #include <asm-generic/gpio.h>
diff --git a/arch/arm/mach-aspeed/ast2500-board.c b/arch/arm/mach-aspeed/ast2500-board.c
index e7edd54..52a4e05 100644
--- a/arch/arm/mach-aspeed/ast2500-board.c
+++ b/arch/arm/mach-aspeed/ast2500-board.c
@@ -4,6 +4,7 @@
  */
 #include <common.h>
 #include <dm.h>
+#include <mmc.h>
 #include <ram.h>
 #include <timer.h>
 #include <asm/io.h>
@@ -55,6 +56,8 @@ int board_init(void)
 {
 	gd->bd->bi_boot_params = CONFIG_SYS_SDRAM_BASE + 0x100;
 
+	mmc_initialize(gd->bd);
+
 	return 0;
 }
 
diff --git a/configs/evb-ast2500_defconfig b/configs/evb-ast2500_defconfig
index 59d41cb..bfa6b32 100644
--- a/configs/evb-ast2500_defconfig
+++ b/configs/evb-ast2500_defconfig
@@ -16,6 +16,7 @@ CONFIG_HUSH_PARSER=y
 CONFIG_CMD_I2C=y
 CONFIG_CMD_DHCP=y
 CONFIG_CMD_MII=y
+CONFIG_CMD_MMC=y
 CONFIG_CMD_PING=y
 CONFIG_DEFAULT_DEVICE_TREE="ast2500-evb"
 CONFIG_NET_RANDOM_ETHADDR=y
@@ -36,3 +37,8 @@ CONFIG_SYS_NS16550=y
 CONFIG_SYSRESET=y
 CONFIG_TIMER=y
 CONFIG_WDT=y
+CONFIG_MMC=y
+CONFIG_DM_MMC=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_ASPEED=y
+CONFIG_MMC_VERBOSE=y
-- 
1.8.3.1

[U-Boot] [PATCH v2 4/4] ARM: dts: ast2500: Add SDHCI nodes

[Eddie James]

Add nodes for the Aspeed SD controllers with their necessary properties.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 arch/arm/dts/ast2500-evb.dts     | 14 ++++++++++++++
 arch/arm/dts/ast2500-u-boot.dtsi | 16 ++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/arch/arm/dts/ast2500-evb.dts b/arch/arm/dts/ast2500-evb.dts
index ebf44fd..4796ed4 100644
--- a/arch/arm/dts/ast2500-evb.dts
+++ b/arch/arm/dts/ast2500-evb.dts
@@ -59,3 +59,17 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_mac2link_default &pinctrl_mdio2_default>;
 };
+
+&sdhci0 {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_sd1_default>;
+};
+
+&sdhci1 {
+	status = "okay";
+
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_sd2_default>;
+};
diff --git a/arch/arm/dts/ast2500-u-boot.dtsi b/arch/arm/dts/ast2500-u-boot.dtsi
index 7f80bad..8ac4215 100644
--- a/arch/arm/dts/ast2500-u-boot.dtsi
+++ b/arch/arm/dts/ast2500-u-boot.dtsi
@@ -34,6 +34,22 @@
 
 		apb {
 			u-boot,dm-pre-reloc;
+
+			sdhci0: sdhci at 1e740100 {
+				compatible = "aspeed,ast2500-sdhci";
+				reg = <0x1e740100>;
+				#reset-cells = <1>;
+				clocks = <&scu BCLK_SDCLK>;
+				resets = <&rst AST_RESET_SDIO>;
+			};
+
+			sdhci1: sdhci at 1e740200 {
+				compatible = "aspeed,ast2500-sdhci";
+				reg = <0x1e740200>;
+				#reset-cells = <1>;
+				clocks = <&scu BCLK_SDCLK>;
+				resets = <&rst AST_RESET_SDIO>;
+			};
 		};
 
 	};
-- 
1.8.3.1

[U-Boot] [PATCH v2 1/4] clk: aspeed: Add support for SD clock

[Cédric Le Goater]

On 13/08/2019 21:31, Eddie James wrote:
> Add code to enable the SD clock on the ast2500 SoC.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  arch/arm/include/asm/arch-aspeed/scu_ast2500.h |  3 +++
>  drivers/clk/aspeed/clk_ast2500.c               | 27 ++++++++++++++++++++++++++
>  drivers/pinctrl/aspeed/pinctrl_ast2500.c       |  2 ++
>  3 files changed, 32 insertions(+)
> 
> diff --git a/arch/arm/include/asm/arch-aspeed/scu_ast2500.h b/arch/arm/include/asm/arch-aspeed/scu_ast2500.h
> index 4988ced..8db4901 100644
> --- a/arch/arm/include/asm/arch-aspeed/scu_ast2500.h
> +++ b/arch/arm/include/asm/arch-aspeed/scu_ast2500.h
> @@ -22,6 +22,8 @@
>  #define SCU_MPLL_POST_MASK		(0x3f << SCU_MPLL_POST_SHIFT)
>  #define SCU_PCLK_DIV_SHIFT		23
>  #define SCU_PCLK_DIV_MASK		(7 << SCU_PCLK_DIV_SHIFT)
> +#define SCU_SDCLK_DIV_SHIFT		12
> +#define SCU_SDCLK_DIV_MASK		(7 << SCU_SDCLK_DIV_SHIFT)
>  #define SCU_HPLL_DENUM_SHIFT		0
>  #define SCU_HPLL_DENUM_MASK		0x1f
>  #define SCU_HPLL_NUM_SHIFT		5
> @@ -107,6 +109,7 @@
>  
>  #define SCU_CLKSTOP_MAC1		(1 << 20)
>  #define SCU_CLKSTOP_MAC2		(1 << 21)
> +#define SCU_CLKSTOP_SDCLK		(1 << 27)
>  
>  #define SCU_D2PLL_EXT1_OFF		(1 << 0)
>  #define SCU_D2PLL_EXT1_BYPASS		(1 << 1)
> diff --git a/drivers/clk/aspeed/clk_ast2500.c b/drivers/clk/aspeed/clk_ast2500.c
> index dbee13a..9249cf9 100644
> --- a/drivers/clk/aspeed/clk_ast2500.c
> +++ b/drivers/clk/aspeed/clk_ast2500.c
> @@ -143,6 +143,17 @@ static ulong ast2500_clk_get_rate(struct clk *clk)
>  			rate = rate / apb_div;
>  		}
>  		break;
> +	case BCLK_SDCLK:
> +		{
> +			ulong apb_div = 4 + 4 * ((readl(&priv->scu->clk_sel1)
> +						  & SCU_SDCLK_DIV_MASK)
> +						 >> SCU_SDCLK_DIV_SHIFT);
> +			rate = ast2500_get_hpll_rate(clkin,
> +						     readl(&priv->
> +							   scu->h_pll_param));
> +			rate = rate / apb_div;
> +		}
> +		break;
>  	case PCLK_UART1:
>  		rate = ast2500_get_uart_clk_rate(priv->scu, 1);
>  		break;
> @@ -436,6 +447,22 @@ static int ast2500_clk_enable(struct clk *clk)
>  	struct ast2500_clk_priv *priv = dev_get_priv(clk->dev);
>  
>  	switch (clk->id) {
> +	case BCLK_SDCLK:
> +		if (readl(&priv->scu->clk_stop_ctrl1) & SCU_CLKSTOP_SDCLK) {
> +			ast_scu_unlock(priv->scu);
> +
> +			setbits_le32(&priv->scu->sysreset_ctrl1,
> +				     SCU_SYSRESET_SDIO);
> +			udelay(100);
> +			clrbits_le32(&priv->scu->clk_stop_ctrl1,
> +				     SCU_CLKSTOP_SDCLK);
> +			mdelay(10);
> +			clrbits_le32(&priv->scu->sysreset_ctrl1,
> +				     SCU_SYSRESET_SDIO);
> +
> +			ast_scu_lock(priv->scu);
> +		}
> +		break;
>  	/*
>  	 * For MAC clocks the clock rate is
>  	 * configured based on whether RGMII or RMII mode has been selected
> diff --git a/drivers/pinctrl/aspeed/pinctrl_ast2500.c b/drivers/pinctrl/aspeed/pinctrl_ast2500.c
> index ed333b9..a6e9c0d 100644
> --- a/drivers/pinctrl/aspeed/pinctrl_ast2500.c
> +++ b/drivers/pinctrl/aspeed/pinctrl_ast2500.c
> @@ -58,6 +58,8 @@ static const struct ast2500_group_config ast2500_groups[] = {
>  	{ "MDIO1", 3, (1 << 31) | (1 << 30) },
>  	{ "MAC2LINK", 1, (1 << 1) },
>  	{ "MDIO2", 5, (1 << 2) },
> +	{ "SD1", 5, (1 << 0) },
> +	{ "SD2", 5, (1 << 1) },
>  };
>  
>  static int ast2500_pinctrl_get_groups_count(struct udevice *dev)
> 

[U-Boot] [PATCH v2 2/4] mmc: Add Aspeed SD controller driver

[Cédric Le Goater]

On 13/08/2019 21:31, Eddie James wrote:
> Add support for the Aspeed SD host controller engine.

It looks correct and simple enough. Some comments below.

> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/mmc/Kconfig        | 11 +++++++
>  drivers/mmc/Makefile       |  1 +
>  drivers/mmc/aspeed_sdhci.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 90 insertions(+)
>  create mode 100644 drivers/mmc/aspeed_sdhci.c
> 
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index c6812f6..536f66a 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -421,6 +421,17 @@ config SPL_MMC_SDHCI_ADMA
>  	  This enables support for the ADMA (Advanced DMA) defined
>  	  in the SD Host Controller Standard Specification Version 3.00 in SPL.
>  
> +config MMC_SDHCI_ASPEED
> +	bool "Aspeed SDHCI controller"
> +	depends on ARCH_ASPEED
> +	depends on DM_MMC
> +	depends on MMC_SDHCI
> +	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
> +	  specifications. On the AST2600, the device is also compatible with
> +	  MMC 5.1 and eMMC 3.0.
> +
>  config MMC_SDHCI_ATMEL
>  	bool "Atmel SDHCI controller support"
>  	depends on ARCH_AT91
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index 6cc018b..5594195 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -46,6 +46,7 @@ obj-$(CONFIG_JZ47XX_MMC) += jz_mmc.o
>  
>  # SDHCI
>  obj-$(CONFIG_MMC_SDHCI)			+= sdhci.o
> +obj-$(CONFIG_MMC_SDHCI_ASPEED)		+= aspeed_sdhci.o
>  obj-$(CONFIG_MMC_SDHCI_ATMEL)		+= atmel_sdhci.o
>  obj-$(CONFIG_MMC_SDHCI_BCM2835)		+= bcm2835_sdhci.o
>  obj-$(CONFIG_MMC_SDHCI_BCMSTB)		+= bcmstb_sdhci.o
> diff --git a/drivers/mmc/aspeed_sdhci.c b/drivers/mmc/aspeed_sdhci.c
> new file mode 100644
> index 0000000..c292c42
> --- /dev/null
> +++ b/drivers/mmc/aspeed_sdhci.c
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2019 IBM Corp.
> + * Eddie James <eajames@linux.ibm.com>
> + */
> +
> +#include <common.h>
> +#include <clk.h>
> +#include <dm.h>
> +#include <malloc.h>
> +#include <sdhci.h>
> +
> +struct aspeed_sdhci_plat {
> +	struct mmc_config cfg;
> +	struct mmc mmc;
> +};
> +
> +static int aspeed_sdhci_probe(struct udevice *dev)
> +{
> +	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> +	struct aspeed_sdhci_plat *plat = dev_get_platdata(dev);
> +	struct sdhci_host *host = dev_get_priv(dev);
> +	u32 max_clk;
> +	struct clk clk;
> +	int ret;
> +
> +	ret = clk_get_by_index(dev, 0, &clk);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_enable(&clk);
> +	if (ret)
> +		return ret;

Don't we need to release the clock below in case of error ? 
> +	host->name = dev->name;
> +	host->ioaddr = (void *)devfdt_get_addr(dev);
> +
> +	max_clk = clk_get_rate(&clk);
> +	if (!max_clk)

may be use IS_ERR_VALUE(max_clk) and return max_clk

> +		return -EINVAL;
> +
> +	host->max_clk = max_clk;
> +	host->mmc = &plat->mmc;
> +	host->mmc->dev = dev;
> +	host->mmc->priv = host;
> +	upriv->mmc = host->mmc;
> +
> +	ret = sdhci_setup_cfg(&plat->cfg, host, 0, 0);
> +	if (ret)
> +		return ret;
> +
> +	return sdhci_probe(dev);
> +}
> +
> +static int aspeed_sdhci_bind(struct udevice *dev)
> +{
> +	struct aspeed_sdhci_plat *plat = dev_get_platdata(dev);
> +
> +	return sdhci_bind(dev, &plat->mmc, &plat->cfg);
> +}
> +
> +static const struct udevice_id aspeed_sdhci_ids[] = {
> +	{ .compatible = "aspeed,ast2400-sdhci" },
> +	{ .compatible = "aspeed,ast2500-sdhci" },
> +	{ .compatible = "aspeed,ast2600-sdhci" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(aspeed_sdhci_drv) = {
> +	.name		= "aspeed_sdhci",
> +	.id		= UCLASS_MMC,
> +	.of_match	= aspeed_sdhci_ids,
> +	.ops		= &sdhci_ops,
> +	.bind		= aspeed_sdhci_bind,
> +	.probe		= aspeed_sdhci_probe,
> +	.priv_auto_alloc_size = sizeof(struct sdhci_host),
> +	.platdata_auto_alloc_size = sizeof(struct aspeed_sdhci_plat),
> +};
> 

[U-Boot] [PATCH v2 3/4] aspeed: Support SD controller on the ast2500 board

[Cédric Le Goater]

On 13/08/2019 21:31, Eddie James wrote:
> Initialize the MMC subsystem on the ast2500 board. Compile MMC and the
> Aspeed SD controller on the ast2500 EVB.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  arch/arm/include/asm/gpio.h          | 3 ++-
>  arch/arm/mach-aspeed/ast2500-board.c | 3 +++
>  configs/evb-ast2500_defconfig        | 6 ++++++
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/gpio.h b/arch/arm/include/asm/gpio.h
> index 370031f..38a5922 100644
> --- a/arch/arm/include/asm/gpio.h
> +++ b/arch/arm/include/asm/gpio.h
> @@ -1,6 +1,7 @@
>  #if !defined(CONFIG_ARCH_UNIPHIER) && !defined(CONFIG_ARCH_STI) && \
>  	!defined(CONFIG_ARCH_K3) && !defined(CONFIG_ARCH_BCM6858) && \
> -	!defined(CONFIG_ARCH_BCM63158) && !defined(CONFIG_ARCH_ROCKCHIP)
> +	!defined(CONFIG_ARCH_BCM63158) && !defined(CONFIG_ARCH_ROCKCHIP) && \
> +	!defined(CONFIG_ARCH_ASPEED)
>  #include <asm/arch/gpio.h>
>  #endif
>  #include <asm-generic/gpio.h>
> diff --git a/arch/arm/mach-aspeed/ast2500-board.c b/arch/arm/mach-aspeed/ast2500-board.c
> index e7edd54..52a4e05 100644
> --- a/arch/arm/mach-aspeed/ast2500-board.c
> +++ b/arch/arm/mach-aspeed/ast2500-board.c
> @@ -4,6 +4,7 @@
>   */
>  #include <common.h>
>  #include <dm.h>
> +#include <mmc.h>
>  #include <ram.h>
>  #include <timer.h>
>  #include <asm/io.h>
> @@ -55,6 +56,8 @@ int board_init(void)
>  {
>  	gd->bd->bi_boot_params = CONFIG_SYS_SDRAM_BASE + 0x100;
>  
> +	mmc_initialize(gd->bd);
> +

isn't that called automatically by board_init_r() already ?

C. 

>  	return 0;
>  }
>  
> diff --git a/configs/evb-ast2500_defconfig b/configs/evb-ast2500_defconfig
> index 59d41cb..bfa6b32 100644
> --- a/configs/evb-ast2500_defconfig
> +++ b/configs/evb-ast2500_defconfig
> @@ -16,6 +16,7 @@ CONFIG_HUSH_PARSER=y
>  CONFIG_CMD_I2C=y
>  CONFIG_CMD_DHCP=y
>  CONFIG_CMD_MII=y
> +CONFIG_CMD_MMC=y
>  CONFIG_CMD_PING=y
>  CONFIG_DEFAULT_DEVICE_TREE="ast2500-evb"
>  CONFIG_NET_RANDOM_ETHADDR=y
> @@ -36,3 +37,8 @@ CONFIG_SYS_NS16550=y
>  CONFIG_SYSRESET=y
>  CONFIG_TIMER=y
>  CONFIG_WDT=y
> +CONFIG_MMC=y
> +CONFIG_DM_MMC=y
> +CONFIG_MMC_SDHCI=y
> +CONFIG_MMC_SDHCI_ASPEED=y
> +CONFIG_MMC_VERBOSE=y
> 

[U-Boot] [PATCH v2 4/4] ARM: dts: ast2500: Add SDHCI nodes

[Cédric Le Goater]

On 13/08/2019 21:31, Eddie James wrote:
> Add nodes for the Aspeed SD controllers with their necessary properties.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>



Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>  arch/arm/dts/ast2500-evb.dts     | 14 ++++++++++++++
>  arch/arm/dts/ast2500-u-boot.dtsi | 16 ++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/arch/arm/dts/ast2500-evb.dts b/arch/arm/dts/ast2500-evb.dts
> index ebf44fd..4796ed4 100644
> --- a/arch/arm/dts/ast2500-evb.dts
> +++ b/arch/arm/dts/ast2500-evb.dts
> @@ -59,3 +59,17 @@
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_mac2link_default &pinctrl_mdio2_default>;
>  };
> +
> +&sdhci0 {
> +	status = "okay";
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_sd1_default>;
> +};
> +
> +&sdhci1 {
> +	status = "okay";
> +
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_sd2_default>;
> +};
> diff --git a/arch/arm/dts/ast2500-u-boot.dtsi b/arch/arm/dts/ast2500-u-boot.dtsi
> index 7f80bad..8ac4215 100644
> --- a/arch/arm/dts/ast2500-u-boot.dtsi
> +++ b/arch/arm/dts/ast2500-u-boot.dtsi
> @@ -34,6 +34,22 @@
>  
>  		apb {
>  			u-boot,dm-pre-reloc;
> +
> +			sdhci0: sdhci at 1e740100 {
> +				compatible = "aspeed,ast2500-sdhci";
> +				reg = <0x1e740100>;
> +				#reset-cells = <1>;
> +				clocks = <&scu BCLK_SDCLK>;
> +				resets = <&rst AST_RESET_SDIO>;
> +			};
> +
> +			sdhci1: sdhci at 1e740200 {
> +				compatible = "aspeed,ast2500-sdhci";
> +				reg = <0x1e740200>;
> +				#reset-cells = <1>;
> +				clocks = <&scu BCLK_SDCLK>;
> +				resets = <&rst AST_RESET_SDIO>;
> +			};
>  		};
>  
>  	};
> 

[U-Boot] [PATCH v2 2/4] mmc: Add Aspeed SD controller driver

[Eddie James]

On 8/14/19 10:18 AM, Cédric Le Goater wrote:
> On 13/08/2019 21:31, Eddie James wrote:
>> Add support for the Aspeed SD host controller engine.
> It looks correct and simple enough. Some comments below.
>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>>   drivers/mmc/Kconfig        | 11 +++++++
>>   drivers/mmc/Makefile       |  1 +
>>   drivers/mmc/aspeed_sdhci.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 90 insertions(+)
>>   create mode 100644 drivers/mmc/aspeed_sdhci.c
>>
>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>> index c6812f6..536f66a 100644
>> --- a/drivers/mmc/Kconfig
>> +++ b/drivers/mmc/Kconfig
>> @@ -421,6 +421,17 @@ config SPL_MMC_SDHCI_ADMA
>>   	  This enables support for the ADMA (Advanced DMA) defined
>>   	  in the SD Host Controller Standard Specification Version 3.00 in SPL.
>>   
>> +config MMC_SDHCI_ASPEED
>> +	bool "Aspeed SDHCI controller"
>> +	depends on ARCH_ASPEED
>> +	depends on DM_MMC
>> +	depends on MMC_SDHCI
>> +	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
>> +	  specifications. On the AST2600, the device is also compatible with
>> +	  MMC 5.1 and eMMC 3.0.
>> +
>>   config MMC_SDHCI_ATMEL
>>   	bool "Atmel SDHCI controller support"
>>   	depends on ARCH_AT91
>> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
>> index 6cc018b..5594195 100644
>> --- a/drivers/mmc/Makefile
>> +++ b/drivers/mmc/Makefile
>> @@ -46,6 +46,7 @@ obj-$(CONFIG_JZ47XX_MMC) += jz_mmc.o
>>   
>>   # SDHCI
>>   obj-$(CONFIG_MMC_SDHCI)			+= sdhci.o
>> +obj-$(CONFIG_MMC_SDHCI_ASPEED)		+= aspeed_sdhci.o
>>   obj-$(CONFIG_MMC_SDHCI_ATMEL)		+= atmel_sdhci.o
>>   obj-$(CONFIG_MMC_SDHCI_BCM2835)		+= bcm2835_sdhci.o
>>   obj-$(CONFIG_MMC_SDHCI_BCMSTB)		+= bcmstb_sdhci.o
>> diff --git a/drivers/mmc/aspeed_sdhci.c b/drivers/mmc/aspeed_sdhci.c
>> new file mode 100644
>> index 0000000..c292c42
>> --- /dev/null
>> +++ b/drivers/mmc/aspeed_sdhci.c
>> @@ -0,0 +1,78 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright 2019 IBM Corp.
>> + * Eddie James <eajames@linux.ibm.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <clk.h>
>> +#include <dm.h>
>> +#include <malloc.h>
>> +#include <sdhci.h>
>> +
>> +struct aspeed_sdhci_plat {
>> +	struct mmc_config cfg;
>> +	struct mmc mmc;
>> +};
>> +
>> +static int aspeed_sdhci_probe(struct udevice *dev)
>> +{
>> +	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>> +	struct aspeed_sdhci_plat *plat = dev_get_platdata(dev);
>> +	struct sdhci_host *host = dev_get_priv(dev);
>> +	u32 max_clk;
>> +	struct clk clk;
>> +	int ret;
>> +
>> +	ret = clk_get_by_index(dev, 0, &clk);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = clk_enable(&clk);
>> +	if (ret)
>> +		return ret;
> Don't we need to release the clock below in case of error ?


Yes.


>> +	host->name = dev->name;
>> +	host->ioaddr = (void *)devfdt_get_addr(dev);
>> +
>> +	max_clk = clk_get_rate(&clk);
>> +	if (!max_clk)
> may be use IS_ERR_VALUE(max_clk) and return max_clk


Yep, good idea.


Thanks for the quick review!

Eddie


>
>> +		return -EINVAL;
>> +
>> +	host->max_clk = max_clk;
>> +	host->mmc = &plat->mmc;
>> +	host->mmc->dev = dev;
>> +	host->mmc->priv = host;
>> +	upriv->mmc = host->mmc;
>> +
>> +	ret = sdhci_setup_cfg(&plat->cfg, host, 0, 0);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return sdhci_probe(dev);
>> +}
>> +
>> +static int aspeed_sdhci_bind(struct udevice *dev)
>> +{
>> +	struct aspeed_sdhci_plat *plat = dev_get_platdata(dev);
>> +
>> +	return sdhci_bind(dev, &plat->mmc, &plat->cfg);
>> +}
>> +
>> +static const struct udevice_id aspeed_sdhci_ids[] = {
>> +	{ .compatible = "aspeed,ast2400-sdhci" },
>> +	{ .compatible = "aspeed,ast2500-sdhci" },
>> +	{ .compatible = "aspeed,ast2600-sdhci" },
>> +	{ }
>> +};
>> +
>> +U_BOOT_DRIVER(aspeed_sdhci_drv) = {
>> +	.name		= "aspeed_sdhci",
>> +	.id		= UCLASS_MMC,
>> +	.of_match	= aspeed_sdhci_ids,
>> +	.ops		= &sdhci_ops,
>> +	.bind		= aspeed_sdhci_bind,
>> +	.probe		= aspeed_sdhci_probe,
>> +	.priv_auto_alloc_size = sizeof(struct sdhci_host),
>> +	.platdata_auto_alloc_size = sizeof(struct aspeed_sdhci_plat),
>> +};
>>

[U-Boot] [PATCH v2 3/4] aspeed: Support SD controller on the ast2500 board

[Eddie James]

On 8/14/19 10:23 AM, Cédric Le Goater wrote:
> On 13/08/2019 21:31, Eddie James wrote:
>> Initialize the MMC subsystem on the ast2500 board. Compile MMC and the
>> Aspeed SD controller on the ast2500 EVB.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>>   arch/arm/include/asm/gpio.h          | 3 ++-
>>   arch/arm/mach-aspeed/ast2500-board.c | 3 +++
>>   configs/evb-ast2500_defconfig        | 6 ++++++
>>   3 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/include/asm/gpio.h b/arch/arm/include/asm/gpio.h
>> index 370031f..38a5922 100644
>> --- a/arch/arm/include/asm/gpio.h
>> +++ b/arch/arm/include/asm/gpio.h
>> @@ -1,6 +1,7 @@
>>   #if !defined(CONFIG_ARCH_UNIPHIER) && !defined(CONFIG_ARCH_STI) && \
>>   	!defined(CONFIG_ARCH_K3) && !defined(CONFIG_ARCH_BCM6858) && \
>> -	!defined(CONFIG_ARCH_BCM63158) && !defined(CONFIG_ARCH_ROCKCHIP)
>> +	!defined(CONFIG_ARCH_BCM63158) && !defined(CONFIG_ARCH_ROCKCHIP) && \
>> +	!defined(CONFIG_ARCH_ASPEED)
>>   #include <asm/arch/gpio.h>
>>   #endif
>>   #include <asm-generic/gpio.h>
>> diff --git a/arch/arm/mach-aspeed/ast2500-board.c b/arch/arm/mach-aspeed/ast2500-board.c
>> index e7edd54..52a4e05 100644
>> --- a/arch/arm/mach-aspeed/ast2500-board.c
>> +++ b/arch/arm/mach-aspeed/ast2500-board.c
>> @@ -4,6 +4,7 @@
>>    */
>>   #include <common.h>
>>   #include <dm.h>
>> +#include <mmc.h>
>>   #include <ram.h>
>>   #include <timer.h>
>>   #include <asm/io.h>
>> @@ -55,6 +56,8 @@ int board_init(void)
>>   {
>>   	gd->bd->bi_boot_params = CONFIG_SYS_SDRAM_BASE + 0x100;
>>   
>> +	mmc_initialize(gd->bd);
>> +
> isn't that called automatically by board_init_r() already ?


You are correct, I will drop this.

Thanks,

Eddie


>
> C.
>
>>   	return 0;
>>   }
>>   
>> diff --git a/configs/evb-ast2500_defconfig b/configs/evb-ast2500_defconfig
>> index 59d41cb..bfa6b32 100644
>> --- a/configs/evb-ast2500_defconfig
>> +++ b/configs/evb-ast2500_defconfig
>> @@ -16,6 +16,7 @@ CONFIG_HUSH_PARSER=y
>>   CONFIG_CMD_I2C=y
>>   CONFIG_CMD_DHCP=y
>>   CONFIG_CMD_MII=y
>> +CONFIG_CMD_MMC=y
>>   CONFIG_CMD_PING=y
>>   CONFIG_DEFAULT_DEVICE_TREE="ast2500-evb"
>>   CONFIG_NET_RANDOM_ETHADDR=y
>> @@ -36,3 +37,8 @@ CONFIG_SYS_NS16550=y
>>   CONFIG_SYSRESET=y
>>   CONFIG_TIMER=y
>>   CONFIG_WDT=y
>> +CONFIG_MMC=y
>> +CONFIG_DM_MMC=y
>> +CONFIG_MMC_SDHCI=y
>> +CONFIG_MMC_SDHCI_ASPEED=y
>> +CONFIG_MMC_VERBOSE=y
>>