[U-Boot] [PATCH v2] socfpga/dwmmc: Adding DesignWare MMC driver support for SOCFPGA
To add the DesignWare MMC driver support for Altera SOCFPGA. It required information such as clocks and bus width from platform specific files (SOCFPGA handoff files)
Signed-off-by: Chin Liang See clsee@altera.com Cc: Rajeshwari Shinde rajeshwari.s@samsung.com Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Andy Fleming afleming@freescale.com Cc: Pantelis Antoniou panto@antoniou-consulting.com --- Changes for v2 - Adding u-boot-mmc maintainer --- arch/arm/include/asm/arch-socfpga/dwmmc.h | 12 +++++ drivers/mmc/Makefile | 1 + drivers/mmc/socfpga_dw_mmc.c | 72 +++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100755 arch/arm/include/asm/arch-socfpga/dwmmc.h create mode 100755 drivers/mmc/socfpga_dw_mmc.c
diff --git a/arch/arm/include/asm/arch-socfpga/dwmmc.h b/arch/arm/include/asm/arch-socfpga/dwmmc.h new file mode 100755 index 0000000..945eb64 --- /dev/null +++ b/arch/arm/include/asm/arch-socfpga/dwmmc.h @@ -0,0 +1,12 @@ +/* + * (C) Copyright 2013 Altera Corporation <www.altera.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef _SOCFPGA_DWMMC_H_ +#define _SOCFPGA_DWMMC_H_ + +extern int socfpga_dwmmc_init(u32 regbase, int bus_width, int index); + +#endif /* _SOCFPGA_SDMMC_H_ */ diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile index 1ed26ca..e793ed9 100644 --- a/drivers/mmc/Makefile +++ b/drivers/mmc/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o obj-$(CONFIG_DWMMC) += dw_mmc.o obj-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o +obj-$(CONFIG_SOCFPGA_DWMMC) += socfpga_dw_mmc.o ifdef CONFIG_SPL_BUILD obj-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o else diff --git a/drivers/mmc/socfpga_dw_mmc.c b/drivers/mmc/socfpga_dw_mmc.c new file mode 100755 index 0000000..554f51b --- /dev/null +++ b/drivers/mmc/socfpga_dw_mmc.c @@ -0,0 +1,72 @@ +/* + * (C) Copyright 2013 Altera Corporation <www.altera.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <malloc.h> +#include <dwmmc.h> +#include <asm/arch/dwmmc.h> + +#define CLKMGR_PERPLLGRP_EN_REG (SOCFPGA_CLKMGR_ADDRESS + 0xA0) +#define CLKMGR_SDMMC_CLK_ENABLE (1 << 8) +#define SYSMGR_SDMMCGRP_CTRL_REG (SOCFPGA_SYSMGR_ADDRESS + 0x108) +#define SYSMGR_SDMMC_CTRL_GET_DRVSEL(x) (((x) >> 0) & 0x7) +#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \ + ((((drvsel) << 0) & 0x7) | (((smplsel) << 3) & 0x38)) + +static char *SOCFPGA_NAME = "SOCFPGA DWMMC"; + +static void socfpga_dwmci_clksel(struct dwmci_host *host) +{ + unsigned int en; + unsigned int drvsel; + unsigned int smplsel; + + /* Disable SDMMC clock. */ + en = readl(CLKMGR_PERPLLGRP_EN_REG); + en &= ~CLKMGR_SDMMC_CLK_ENABLE; + writel(en, CLKMGR_PERPLLGRP_EN_REG); + + /* Configures drv_sel and smpl_sel */ + drvsel = 3; + smplsel = 0; + + debug("%s: drvsel %d smplsel %d\n", __FUNCTION__, drvsel, smplsel); + writel(SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel), + SYSMGR_SDMMCGRP_CTRL_REG); + + debug("%s: SYSMGR_SDMMCGRP_CTRL_REG = 0x%x\n", __FUNCTION__, + readl(SYSMGR_SDMMCGRP_CTRL_REG)); + /* Enable SDMMC clock */ + en = readl(CLKMGR_PERPLLGRP_EN_REG); + en |= CLKMGR_SDMMC_CLK_ENABLE; + writel(en, CLKMGR_PERPLLGRP_EN_REG); +} + +int socfpga_dwmmc_init(u32 regbase, int bus_width, int index) +{ + struct dwmci_host *host = NULL; + host = calloc(sizeof(struct dwmci_host), 1); + if (!host) { + printf("dwmci_host calloc fail!\n"); + return 1; + } + + host->name = SOCFPGA_NAME; + host->ioaddr = (void *)regbase; + host->buswidth = bus_width; + host->clksel = socfpga_dwmci_clksel; + host->dev_index = index; + /* fixed clock divide by 4 which due to the SDMMC wrapper */ + host->bus_hz = CONFIG_DWMMC_BUS_HZ; + host->fifoth_val = MSIZE(0x2) | + RX_WMARK(CONFIG_DWMMC_FIFO_DEPTH / 2 - 1) | + TX_WMARK(CONFIG_DWMMC_FIFO_DEPTH / 2); + + add_dwmci(host, host->bus_hz, 400000); + + return 0; +} +
Hi, Chin.
On 12/19/2013 02:16 AM, Chin Liang See wrote:
To add the DesignWare MMC driver support for Altera SOCFPGA. It required information such as clocks and bus width from platform specific files (SOCFPGA handoff files)
Signed-off-by: Chin Liang See clsee@altera.com Cc: Rajeshwari Shinde rajeshwari.s@samsung.com Cc: Jaehoon Chung jh80.chung@samsung.com Cc: Andy Fleming afleming@freescale.com Cc: Pantelis Antoniou panto@antoniou-consulting.com
Changes for v2
- Adding u-boot-mmc maintainer
arch/arm/include/asm/arch-socfpga/dwmmc.h | 12 +++++ drivers/mmc/Makefile | 1 + drivers/mmc/socfpga_dw_mmc.c | 72 +++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) create mode 100755 arch/arm/include/asm/arch-socfpga/dwmmc.h create mode 100755 drivers/mmc/socfpga_dw_mmc.c
diff --git a/arch/arm/include/asm/arch-socfpga/dwmmc.h b/arch/arm/include/asm/arch-socfpga/dwmmc.h new file mode 100755 index 0000000..945eb64 --- /dev/null +++ b/arch/arm/include/asm/arch-socfpga/dwmmc.h @@ -0,0 +1,12 @@ +/*
- (C) Copyright 2013 Altera Corporation <www.altera.com>
- SPDX-License-Identifier: GPL-2.0+
- */
+#ifndef _SOCFPGA_DWMMC_H_ +#define _SOCFPGA_DWMMC_H_
+extern int socfpga_dwmmc_init(u32 regbase, int bus_width, int index);
+#endif /* _SOCFPGA_SDMMC_H_ */ diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile index 1ed26ca..e793ed9 100644 --- a/drivers/mmc/Makefile +++ b/drivers/mmc/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o obj-$(CONFIG_DWMMC) += dw_mmc.o obj-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o +obj-$(CONFIG_SOCFPGA_DWMMC) += socfpga_dw_mmc.o ifdef CONFIG_SPL_BUILD obj-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o else diff --git a/drivers/mmc/socfpga_dw_mmc.c b/drivers/mmc/socfpga_dw_mmc.c new file mode 100755 index 0000000..554f51b --- /dev/null +++ b/drivers/mmc/socfpga_dw_mmc.c @@ -0,0 +1,72 @@ +/*
- (C) Copyright 2013 Altera Corporation <www.altera.com>
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <malloc.h> +#include <dwmmc.h> +#include <asm/arch/dwmmc.h>
+#define CLKMGR_PERPLLGRP_EN_REG (SOCFPGA_CLKMGR_ADDRESS + 0xA0) +#define CLKMGR_SDMMC_CLK_ENABLE (1 << 8) +#define SYSMGR_SDMMCGRP_CTRL_REG (SOCFPGA_SYSMGR_ADDRESS + 0x108)
Where is SOCFPGA_CLKMGR_ADDRESS defined?
+#define SYSMGR_SDMMC_CTRL_GET_DRVSEL(x) (((x) >> 0) & 0x7)
((x) & 0x7) is more readable?
+#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
- ((((drvsel) << 0) & 0x7) | (((smplsel) << 3) & 0x38))
+static char *SOCFPGA_NAME = "SOCFPGA DWMMC";
+static void socfpga_dwmci_clksel(struct dwmci_host *host) +{
- unsigned int en;
- unsigned int drvsel;
- unsigned int smplsel;
- /* Disable SDMMC clock. */
- en = readl(CLKMGR_PERPLLGRP_EN_REG);
- en &= ~CLKMGR_SDMMC_CLK_ENABLE;
- writel(en, CLKMGR_PERPLLGRP_EN_REG);
- /* Configures drv_sel and smpl_sel */
- drvsel = 3;
- smplsel = 0;
Is this value static? then why is value assigned drvsel and smpsel at here? I didn't know that SOCFPGA is only used with drvsel = 3, smplsel = 0. But if you need to change this value for other SoC version in future, I think that hard coding is not good.
- debug("%s: drvsel %d smplsel %d\n", __FUNCTION__, drvsel, smplsel);
- writel(SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel),
SYSMGR_SDMMCGRP_CTRL_REG);- debug("%s: SYSMGR_SDMMCGRP_CTRL_REG = 0x%x\n", __FUNCTION__,
readl(SYSMGR_SDMMCGRP_CTRL_REG));- /* Enable SDMMC clock */
- en = readl(CLKMGR_PERPLLGRP_EN_REG);
- en |= CLKMGR_SDMMC_CLK_ENABLE;
- writel(en, CLKMGR_PERPLLGRP_EN_REG);
+}
+int socfpga_dwmmc_init(u32 regbase, int bus_width, int index) +{
- struct dwmci_host *host = NULL;
- host = calloc(sizeof(struct dwmci_host), 1);
- if (!host) {
printf("dwmci_host calloc fail!\n");return 1;- }
- host->name = SOCFPGA_NAME;
- host->ioaddr = (void *)regbase;
- host->buswidth = bus_width;
- host->clksel = socfpga_dwmci_clksel;
- host->dev_index = index;
- /* fixed clock divide by 4 which due to the SDMMC wrapper */
- host->bus_hz = CONFIG_DWMMC_BUS_HZ;
I didn't want to use the CONFIG_DWMMC_BUS_HZ.
- host->fifoth_val = MSIZE(0x2) |
RX_WMARK(CONFIG_DWMMC_FIFO_DEPTH / 2 - 1) |TX_WMARK(CONFIG_DWMMC_FIFO_DEPTH / 2);- add_dwmci(host, host->bus_hz, 400000);
add_dwmci() has the return value.
Best Regards, Jaehoon Chung
- return 0;
+}
Dear Jaehoon,
On Thu, 2013-12-26 at 14:05 +0900, Jaehoon Chung wrote:
Hi, Chin.
On 12/19/2013 02:16 AM, Chin Liang See wrote:
+#define CLKMGR_PERPLLGRP_EN_REG (SOCFPGA_CLKMGR_ADDRESS + 0xA0) +#define CLKMGR_SDMMC_CLK_ENABLE (1 << 8) +#define SYSMGR_SDMMCGRP_CTRL_REG (SOCFPGA_SYSMGR_ADDRESS + 0x108)
Where is SOCFPGA_CLKMGR_ADDRESS defined?
This is located at platform specific declaration file.
+#define SYSMGR_SDMMC_CTRL_GET_DRVSEL(x) (((x) >> 0) & 0x7)
((x) & 0x7) is more readable?
+#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
- ((((drvsel) << 0) & 0x7) | (((smplsel) << 3) & 0x38))
+static char *SOCFPGA_NAME = "SOCFPGA DWMMC";
+static void socfpga_dwmci_clksel(struct dwmci_host *host) +{
- unsigned int en;
- unsigned int drvsel;
- unsigned int smplsel;
- /* Disable SDMMC clock. */
- en = readl(CLKMGR_PERPLLGRP_EN_REG);
- en &= ~CLKMGR_SDMMC_CLK_ENABLE;
- writel(en, CLKMGR_PERPLLGRP_EN_REG);
- /* Configures drv_sel and smpl_sel */
- drvsel = 3;
- smplsel = 0;
Is this value static? then why is value assigned drvsel and smpsel at here? I didn't know that SOCFPGA is only used with drvsel = 3, smplsel = 0. But if you need to change this value for other SoC version in future, I think that hard coding is not good.
Good suggestion as I put them as macro now for v3
- debug("%s: drvsel %d smplsel %d\n", __FUNCTION__, drvsel, smplsel);
- writel(SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel),
SYSMGR_SDMMCGRP_CTRL_REG);- debug("%s: SYSMGR_SDMMCGRP_CTRL_REG = 0x%x\n", __FUNCTION__,
readl(SYSMGR_SDMMCGRP_CTRL_REG));- /* Enable SDMMC clock */
- en = readl(CLKMGR_PERPLLGRP_EN_REG);
- en |= CLKMGR_SDMMC_CLK_ENABLE;
- writel(en, CLKMGR_PERPLLGRP_EN_REG);
+}
+int socfpga_dwmmc_init(u32 regbase, int bus_width, int index) +{
- struct dwmci_host *host = NULL;
- host = calloc(sizeof(struct dwmci_host), 1);
- if (!host) {
printf("dwmci_host calloc fail!\n");return 1;- }
- host->name = SOCFPGA_NAME;
- host->ioaddr = (void *)regbase;
- host->buswidth = bus_width;
- host->clksel = socfpga_dwmci_clksel;
- host->dev_index = index;
- /* fixed clock divide by 4 which due to the SDMMC wrapper */
- host->bus_hz = CONFIG_DWMMC_BUS_HZ;
I didn't want to use the CONFIG_DWMMC_BUS_HZ.
Yup, this is SOCFPGA specific and I am using CONFIG_SOCFPGA_DWMMC_BUS_HZ for v3
Thanks
Chin Liang
- host->fifoth_val = MSIZE(0x2) |
RX_WMARK(CONFIG_DWMMC_FIFO_DEPTH / 2 - 1) |TX_WMARK(CONFIG_DWMMC_FIFO_DEPTH / 2);- add_dwmci(host, host->bus_hz, 400000);
add_dwmci() has the return value.
Best Regards, Jaehoon Chung
- return 0;
+}
Dear Chin Liang See,
PLease fix your address list. There is no such address as "Andy@denx.de".
In message 1387386987-3581-1-git-send-email-clsee@altera.com you wrote:
To add the DesignWare MMC driver support for Altera SOCFPGA. It required information such as clocks and bus width from platform specific files (SOCFPGA handoff files)
...
+#define CLKMGR_PERPLLGRP_EN_REG (SOCFPGA_CLKMGR_ADDRESS + 0xA0)
...
+#define SYSMGR_SDMMCGRP_CTRL_REG (SOCFPGA_SYSMGR_ADDRESS + 0x108)
This looks as if you were trying to access device rtegisters through a base address plus offset notation?
We do not allow this in U-Boot, as the compiler then has no chance to check if you are using the correct data types, i. e. it cannot warn you for example when you access a 32 bit register for a 16 bit data type.
Please use C structs and proper I/O accessors instead.
- /* Disable SDMMC clock. */
- en = readl(CLKMGR_PERPLLGRP_EN_REG);
- en &= ~CLKMGR_SDMMC_CLK_ENABLE;
- writel(en, CLKMGR_PERPLLGRP_EN_REG);
Please fix such code. Use proper I/O accessors. This could (and should) be written as:
clrbits_le32(clkmgr->perpllgrp_en, CLKMGR_SDMMC_CLK_ENABLE);
[or similar struct member name].
- /* Enable SDMMC clock */
- en = readl(CLKMGR_PERPLLGRP_EN_REG);
- en |= CLKMGR_SDMMC_CLK_ENABLE;
- writel(en, CLKMGR_PERPLLGRP_EN_REG);
Ditto here, etc.
Please check all your device accesses.
Best regards,
Wolfgang Denk
Dear Wolfgang,
On Thu, 2013-12-26 at 09:38 +0100, ZY - wd wrote:
Dear Chin Liang See,
PLease fix your address list. There is no such address as "Andy@denx.de".
Oh... I presume you are referring to Andy Fleming. I am getting delivery failure to afleming@freescale.com and removing it from CC list.
In message 1387386987-3581-1-git-send-email-clsee@altera.com you wrote:
To add the DesignWare MMC driver support for Altera SOCFPGA. It required information such as clocks and bus width from platform specific files (SOCFPGA handoff files)
...
+#define CLKMGR_PERPLLGRP_EN_REG (SOCFPGA_CLKMGR_ADDRESS + 0xA0)
...
+#define SYSMGR_SDMMCGRP_CTRL_REG (SOCFPGA_SYSMGR_ADDRESS + 0x108)
This looks as if you were trying to access device rtegisters through a base address plus offset notation?
We do not allow this in U-Boot, as the compiler then has no chance to check if you are using the correct data types, i. e. it cannot warn you for example when you access a 32 bit register for a 16 bit data type.
Noted and I believe I miss out this. I will be using structure for v3
Please use C structs and proper I/O accessors instead.
- /* Disable SDMMC clock. */
- en = readl(CLKMGR_PERPLLGRP_EN_REG);
- en &= ~CLKMGR_SDMMC_CLK_ENABLE;
- writel(en, CLKMGR_PERPLLGRP_EN_REG);
Please fix such code. Use proper I/O accessors. This could (and should) be written as:
clrbits_le32(clkmgr->perpllgrp_en, CLKMGR_SDMMC_CLK_ENABLE);
[or similar struct member name].
- /* Enable SDMMC clock */
- en = readl(CLKMGR_PERPLLGRP_EN_REG);
- en |= CLKMGR_SDMMC_CLK_ENABLE;
- writel(en, CLKMGR_PERPLLGRP_EN_REG);
Ditto here, etc.
Noted.
Thanks
Chin Liang
Please check all your device accesses.
Best regards,
Wolfgang Denk
participants (3)
-
Chin Liang See -
Jaehoon Chung -
Wolfgang Denk