socfpga: initialize MMC

U-Boot Superseded Pavel Machek 5 participants 20 messages Jul 16, 2014 – Nov 10, 2014

Subsystems (2)

Arm (32-bit) MMC, SD and SDIO

lore

Share & tools

lore

Pavel Machek

author

2014-07-16 12:34 UTC #0 lore raw

Hi!

Once ethernet changes I merged, I can rediff mmc changes for application. It will look something like this:

Albert, can you apply the ethernet patch? I'd like to continue on top of it...
Pavel

arch/arm/cpu/armv7/socfpga/misc.c +11 -0

index 2f1c716..7af384e 100644
@@ -14,3 +16,27 @@ int dram_init(void)
+
+#ifdef CONFIG_DWMMC
+/*
+ * Initializes MMC controllers.
+ * to override, implement board_mmc_init()
+ */
+int cpu_mmc_init(bd_t *bis)
+{
+       return altera_dwmmc_init(SOCFPGA_SDMMC_ADDRESS, 4, 0);
+}
+#endif

arch/arm/include/asm/arch-socfpga/socfpga_base_addrs.h +1 -0

index f564046..9b488d9 100644
@@ -14,5 +14,8 @@
 #define SOCFPGA_CLKMGR_ADDRESS 0xffd04000
 #define SOCFPGA_RSTMGR_ADDRESS 0xffd05000
 #define SOCFPGA_SYSMGR_ADDRESS 0xffd08000
+#define SOCFPGA_SDMMC_ADDRESS 0xff704000
 
 #endif /* _SOCFPGA_BASE_ADDRS_H_ */

drivers/mmc/Makefile +1 -0

index 34febf5..5902105 100644
@@ -28,6 +28,7 @@ obj-$(CONFIG_SPEAR_SDHCI) += spear_sdhci.o
 obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o
 obj-$(CONFIG_DWMMC) += dw_mmc.o
 obj-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o
+obj-$(CONFIG_ALTERA_DWMMC) += altera_dw_mmc.o
 obj-$(CONFIG_MMC_SUNXI) += sunxi_mmc.o
 obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o
 obj-$(CONFIG_SOCFPGA_DWMMC) += socfpga_dw_mmc.o

drivers/mmc/altera_dw_mmc.c new file +74 -0

74 lines not on this page · raw message

include/configs/socfpga_cyclone5.h +17 -1

index 0254249..189dcde 100644
@@ -180,13 +183,30 @@
 /*
  * FLASH
  */
 #define CONFIG_SYS_NO_FLASH
 
+
+/*
+ * MMC support
+ */
+#define CONFIG_MMC
+#ifdef CONFIG_MMC
+#define CONFIG_CMD_MMC
+
+#define CONFIG_BOUNCE_BUFFER
+#define CONFIG_GENERIC_MMC             1
+#define CONFIG_DWMMC                   1
+#define CONFIG_ALTERA_DWMMC            1
+#define CONFIG_DWMMC_FIFO_DEPTH		1024
+/* using smaller max blk cnt to avoid flooding the limited stack we have */
+#define CONFIG_SYS_MMC_MAX_BLK_COUNT     256
+#endif /* CONFIG_MMC */
+
 /*
  * L4 OSC1 Timer 0
  */
 
--

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Wolfgang Denk

2014-07-16 12:52 UTC #1 lore raw

Dear Pavel Machek,

Quoted 6 lines · In message <[email protected]> you wrote:

Once ethernet changes I merged, I can rediff mmc changes for application. It will look something like this:
Albert, can you apply the ethernet patch? I'd like to continue on top of it...

Please mark such stuff for discussion explicitly as RFC so they can easily be spottet in PW.

arch/arm/include/asm/arch-socfpga/socfpga_base_addrs.h line 17

Quoted 6 lines

+++ b/arch/arm/include/asm/arch-socfpga/socfpga_base_addrs.h
arch/arm/include/asm/arch-socfpga/socfpga_base_addrs.h +1 -0
@@ -14,5 +14,8 @@
 #define SOCFPGA_CLKMGR_ADDRESS 0xffd04000
 #define SOCFPGA_RSTMGR_ADDRESS 0xffd05000
 #define SOCFPGA_SYSMGR_ADDRESS 0xffd08000
+#define SOCFPGA_SDMMC_ADDRESS 0xff704000

Please keep list sorted.

drivers/mmc/Makefile obj-$(CONFIG_SPEAR_SDHCI) += spear_sdhci.o · line 34

Quoted 10 lines

drivers/mmc/Makefile +1 -0

@@ -28,6 +28,7 @@ obj-$(CONFIG_SPEAR_SDHCI) += spear_sdhci.o
 obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o
 obj-$(CONFIG_DWMMC) += dw_mmc.o
 obj-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o
+obj-$(CONFIG_ALTERA_DWMMC) += altera_dw_mmc.o
 obj-$(CONFIG_MMC_SUNXI) += sunxi_mmc.o
 obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o
 obj-$(CONFIG_SOCFPGA_DWMMC) += socfpga_dw_mmc.o

Ditto. This should be a sorted list.

Quoted 9 lines

drivers/mmc/altera_dw_mmc.c new file +6 -0

@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2013 Altera Corporation <www.altera.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of

drivers/mmc/altera_dw_mmc.c line 16

Quoted 10 lines

drivers/mmc/altera_dw_mmc.c new file +10 -0

+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,  MA 02111-1307 USA

NAK. Please use SPDX ID tags instead.

drivers/mmc/altera_dw_mmc.c line 30

Quoted 1 line

drivers/mmc/altera_dw_mmc.c new file +1 -0
+static char *ALTERA_NAME = "ALTERA DWMMC";

Please use lower case variable names only.

drivers/mmc/altera_dw_mmc.c line 54

Quoted 4 lines

drivers/mmc/altera_dw_mmc.c new file +4 -0

+int altera_dwmmc_init(u32 regbase, int bus_width, int index)
+{
+	struct dwmci_host *host = NULL;
+	host = malloc(sizeof(struct dwmci_host));

Please separate declartions and code by a blank line.

Hm, which sense does it make to initialize host as NULL, just to assign a different value in the next step?

Please fix.

drivers/mmc/altera_dw_mmc.c line 70

Quoted 11 lines

drivers/mmc/altera_dw_mmc.c new file +11 -0

+	host->name = ALTERA_NAME;
+	host->ioaddr = (void *)regbase;
+	host->buswidth = bus_width;
+	host->clksel = altera_dwmci_clksel;
+	host->dev_index = index;
+	host->bus_hz = 400000;
+	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, host->bus_hz);

Is there a free(host) anywhere?

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
While money can't buy happiness, it certainly lets  you  choose  your
own form of misery.

Wolfgang Denk

2014-07-16 13:13 UTC #2 lore raw

Dear Pavel,

Quoted 4 lines · In message <[email protected]> I wrote:

drivers/mmc/altera_dw_mmc.c line 70
Quoted 2 lines
drivers/mmc/altera_dw_mmc.c new file +1 -0
+	add_dwmci(host, host->bus_hz, host->bus_hz);
Is there a free(host) anywhere?

I think there is none, which raises the question why you use malloc() in the first place. Just use a struct on the stack.

Also, please make sure to check the return code of add_dwmci() for errors, and handle these as needed.

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The light at the end of the tunnel is usually a "No Exit" sign.

Chin Liang See

2014-07-17 11:00 UTC #3 lore raw

Hi Wolfgang,

Quoted 7 lines · ZY - wd

Dear Pavel,
Quoted 4 lines · In message <[email protected]> I wrote:
drivers/mmc/altera_dw_mmc.c line 70
Quoted 2 lines
drivers/mmc/altera_dw_mmc.c new file +1 -0
+	add_dwmci(host, host->bus_hz, host->bus_hz);
Is there a free(host) anywhere?

Actually the host will be referred throughout the code execution when SDMMC access is required. Hence, we can only call the free when exit which deems not needed then.

Thanks
Chin Liang

Quoted 11 lines

I think there is none, which raises the question why you use malloc() in the first place. Just use a struct on the stack.
Also, please make sure to check the return code of add_dwmci() for errors, and handle these as needed.
Best regards,
Wolfgang Denk

Wolfgang Denk

2014-07-17 12:01 UTC #4 lore raw

Dear Chin Liang,

Quoted 8 lines · In message <[email protected]> you wrote:

Quoted 4 lines
Quoted 4 lines
drivers/mmc/altera_dw_mmc.c line 70
Quoted 2 lines
drivers/mmc/altera_dw_mmc.c new file +1 -0
+	add_dwmci(host, host->bus_hz, host->bus_hz);
Is there a free(host) anywhere?
Actually the host will be referred throughout the code execution when SDMMC access is required. Hence, we can only call the free when exit which deems not needed then.

I have to admit that I have hard times trying to understand the concept of behind this code.

1) The new patches add altera_dwmmc_init() - but is this not basically a verbatim copy of socfpga_dwmmc_init() which is alreday in mainline?

Except that the new patches use hardcoded constants (which is BAD) where the current mainline code uses #defines:

altera_dwmmc_init():

	host->bus_hz = 400000;

socfpga_dwmmc_init():

	host->bus_hz = CONFIG_SOCFPGA_DWMMC_BUS_HZ;

2) Is it really necessary to always and unconditionally initialyze the MMC subsystem, even in nobody will ever use any MMC commands in U-Boot? The rule is that we only initialize hardwar when we actually use them inside of U-Boot, i. e. if any commands are executed that access such hardware?

[Ok, this seems to be a generic problem, not specific you your patch set.]

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The average woman would rather have beauty than brains,  because  the
average man can see better than he can think.

Pavel Machek

author

2014-07-19 12:03 UTC #5 lore raw

Quoted 12 lines · Wolfgang Denk

Dear Chin Liang,
Quoted 8 lines · In message <[email protected]> you wrote:
Quoted 4 lines
Quoted 4 lines
drivers/mmc/altera_dw_mmc.c line 70
Quoted 2 lines
drivers/mmc/altera_dw_mmc.c new file +1 -0
+	add_dwmci(host, host->bus_hz, host->bus_hz);
Is there a free(host) anywhere?
Actually the host will be referred throughout the code execution when SDMMC access is required. Hence, we can only call the free when exit which deems not needed then.

Quoted 6 lines

I have to admit that I have hard times trying to understand the concept of behind this code.
1) The new patches add altera_dwmmc_init() - but is this not basically a verbatim copy of socfpga_dwmmc_init() which is alreday in mainline?

Hmm. altera_dwmmc_init(), already in the mainline but not referenced anywhere... which is why I failed to notice it.

And most of your review comments apply even to that version. I'll see if that version works for me and will fix it up according to review.

Quoted 8 lines

2) Is it really necessary to always and unconditionally initialyze the MMC subsystem, even in nobody will ever use any MMC commands in U-Boot? The rule is that we only initialize hardwar when we actually use them inside of U-Boot, i. e. if any commands are executed that access such hardware?
[Ok, this seems to be a generic problem, not specific you your patch set.]

I seem to have great trouble getting even the most trivial patches to apply. I can do more extensive changes, but would like to get the simple things merged, first...

Best regards,
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Pavel Machek

author

2014-07-21 10:20 UTC #6 lore raw

In reply to Wolfgang Denk

Hi!

Quoted 8 lines

arch/arm/include/asm/arch-socfpga/socfpga_base_addrs.h line 17
Quoted 6 lines
+++ b/arch/arm/include/asm/arch-socfpga/socfpga_base_addrs.h
arch/arm/include/asm/arch-socfpga/socfpga_base_addrs.h +1 -0
@@ -14,5 +14,8 @@
 #define SOCFPGA_CLKMGR_ADDRESS 0xffd04000
 #define SOCFPGA_RSTMGR_ADDRESS 0xffd05000
 #define SOCFPGA_SYSMGR_ADDRESS 0xffd08000
+#define SOCFPGA_SDMMC_ADDRESS 0xff704000
Please keep list sorted.

It was sorted by address before, so I'll keep it like that.

Quoted 4 lines

drivers/mmc/Makefile +0 -0

@@ -28,6 +28,7 @@ obj-$(CONFIG_SPEAR_SDHCI) += spear_sdhci.o
 obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o

Quoted 8 lines

drivers/mmc/Makefile obj-$(CONFIG_SPEAR_SDHCI) += spear_sdhci.o · line 34
Quoted 6 lines
drivers/mmc/Makefile +1 -0
 obj-$(CONFIG_DWMMC) += dw_mmc.o
 obj-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o
+obj-$(CONFIG_ALTERA_DWMMC) += altera_dw_mmc.o
 obj-$(CONFIG_MMC_SUNXI) += sunxi_mmc.o
 obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o
 obj-$(CONFIG_SOCFPGA_DWMMC) += socfpga_dw_mmc.o
Ditto. This should be a sorted list.

This was not sorted and I will not need to modify it in next patch version, so I'll leave this unfixed.

Quoted 3 lines

Quoted 1 line
+static char *ALTERA_NAME = "ALTERA DWMMC";
Please use lower case variable names only.

Actually, it turns out we don't need the variable :-).

Quoted 6 lines

drivers/mmc/Makefile
Quoted 4 lines
drivers/mmc/Makefile +4 -0
+int altera_dwmmc_init(u32 regbase, int bus_width, int index)
+{
+	struct dwmci_host *host = NULL;
+	host = malloc(sizeof(struct dwmci_host));
Please separate declartions and code by a blank line.

Ok.

Quoted 4 lines

Hm, which sense does it make to initialize host as NULL, just to assign a different value in the next step?
Please fix.

Fixed.

Quoted 5 lines

drivers/mmc/Makefile +5 -0

+	host->name = ALTERA_NAME;
+	host->ioaddr = (void *)regbase;
+	host->buswidth = bus_width;
+	host->clksel = altera_dwmci_clksel;
+	host->dev_index = index;

Quoted 8 lines

drivers/mmc/Makefile

Quoted 6 lines

drivers/mmc/Makefile +6 -0

+	host->bus_hz = 400000;
+	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, host->bus_hz);

Is there a free(host) anywhere?

Dynamic allocation does not make much sense here, agreed. But as it is existing code, and there are bigger issues around, I'd prefer to leave it to someone else to clean it up...

Thanks,
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Wolfgang Denk

2014-07-21 10:50 UTC #7 lore raw

Dear Pavel,

Quoted 9 lines · In message <[email protected]> you wrote:

Quoted 7 lines
Quoted 5 lines
#define SOCFPGA_CLKMGR_ADDRESS 0xffd04000
#define SOCFPGA_RSTMGR_ADDRESS 0xffd05000
#define SOCFPGA_SYSMGR_ADDRESS 0xffd08000
+#define SOCFPGA_SDMMC_ADDRESS 0xff704000
Please keep list sorted.
It was sorted by address before, so I'll keep it like that.

Yes, it was sorted by address, and 0xff704000 goes _before_ 0xffd08000, not after.

Quoted 13 lines

Quoted 4 lines
Quoted 4 lines
drivers/mmc/Makefile +0 -0
@@ -28,6 +28,7 @@ obj-$(CONFIG_SPEAR_SDHCI) += spear_sdhci.o
 obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o
Quoted 8 lines
drivers/mmc/Makefile obj-$(CONFIG_SPEAR_SDHCI) += spear_sdhci.o · line 34
Quoted 6 lines
drivers/mmc/Makefile +1 -0
 obj-$(CONFIG_DWMMC) += dw_mmc.o
 obj-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o
+obj-$(CONFIG_ALTERA_DWMMC) += altera_dw_mmc.o
 obj-$(CONFIG_MMC_SUNXI) += sunxi_mmc.o
 obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o
 obj-$(CONFIG_SOCFPGA_DWMMC) += socfpga_dw_mmc.o
Ditto. This should be a sorted list.

Quoted 2 lines

This was not sorted and I will not need to modify it in next patch version, so I'll leave this unfixed.

No, please clean up and fix it.

Quoted 5 lines

Quoted 1 line
Is there a free(host) anywhere?
Dynamic allocation does not make much sense here, agreed. But as it is existing code, and there are bigger issues around, I'd prefer to leave it to someone else to clean it up...

I tend to disagree. If we do not fix the problems as we see them, and when we are working on the code anyway, they will never get fixed.

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
When the bosses talk about improving  productivity,  they  are  never
talking about themselves.

Pavel Machek

author

2014-07-21 12:05 UTC #8 lore raw

Hi!

Quoted 11 lines

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
#define SOCFPGA_CLKMGR_ADDRESS 0xffd04000
#define SOCFPGA_RSTMGR_ADDRESS 0xffd05000
#define SOCFPGA_SYSMGR_ADDRESS 0xffd08000
+#define SOCFPGA_SDMMC_ADDRESS 0xff704000
Please keep list sorted.
It was sorted by address before, so I'll keep it like that.
Yes, it was sorted by address, and 0xff704000 goes _before_ 0xffd08000, not after.

Sorry, I meant "will fix it so that it is sorted after my patches".

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
Is there a free(host) anywhere?
Dynamic allocation does not make much sense here, agreed. But as it is existing code, and there are bigger issues around, I'd prefer to leave it to someone else to clean it up...
I tend to disagree. If we do not fix the problems as we see them, and when we are working on the code anyway, they will never get fixed.

I'm working on the code, anyway, yes. Currently, u-boot is completely unusable on socfpga. I'd rather fix big problems first, so that code could be actually tested, and does not bitrot further as happened in clock and time areas. There are just too many small problems around.

Could we talk on jabber or phone?

Best regards,
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Chin Liang See

2014-07-22 09:33 UTC #9 lore raw

Hi Pavel,

Quoted 6 lines · ZY - pavel

Quoted 6 lines
Quoted 5 lines
Quoted 1 line
Is there a free(host) anywhere?
Dynamic allocation does not make much sense here, agreed. But as it is existing code, and there are bigger issues around, I'd prefer to leave it to someone else to clean it up...

Quoted 8 lines

Quoted 2 lines
I tend to disagree. If we do not fix the problems as we see them, and when we are working on the code anyway, they will never get fixed.
I'm working on the code, anyway, yes. Currently, u-boot is completely unusable on socfpga. I'd rather fix big problems first, so that code could be actually tested, and does not bitrot further as happened in clock and time areas. There are just too many small problems around.

I believe some of them break due to recent upstream. The bug escaped as we still build socfpga for Virtual Target instead dev kit. I already fix this at another patch and I can boot up the U-Boot successfully again. While SDMMC, I can try it tomorrow.

Thanks
Chin Liang

Quoted 4 lines

Could we talk on jabber or phone?
Best regards,
Pavel

Pavel Machek

author

2014-09-09 13:16 UTC #10 lore raw

PATCH 1/1 sort drivers/mmc/Makefile

In reply to Wolfgang Denk

Sort drivers/mmc makefile, as requested by wd.

Signed-off-by Pavel Machek <[email protected]>

drivers/mmc/Makefile +12 ~~-10~~

index 464cee1..461d7d8 100644
@@ -5,37 +5,39 @@
 # SPDX-License-Identifier:	GPL-2.0+
 #
 
+obj-$(CONFIG_ARM_PL180_MMCI) += arm_pl180_mmci.o
+obj-$(CONFIG_BCM2835_SDHCI) += bcm2835_sdhci.o
 obj-$(CONFIG_BFIN_SDH) += bfin_sdh.o
 obj-$(CONFIG_DAVINCI_MMC) += davinci_mmc.o
+obj-$(CONFIG_DWMMC) += dw_mmc.o
+obj-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o
 obj-$(CONFIG_FSL_ESDHC) += fsl_esdhc.o
 obj-$(CONFIG_FTSDC010) += ftsdc010_mci.o
 obj-$(CONFIG_FTSDC021) += ftsdc021_sdhci.o
 obj-$(CONFIG_GENERIC_MMC) += mmc.o
 obj-$(CONFIG_GENERIC_ATMEL_MCI) += gen_atmel_mci.o
+obj-$(CONFIG_KONA_SDHCI) += kona_sdhci.o
 obj-$(CONFIG_MMC_SPI) += mmc_spi.o
-obj-$(CONFIG_ARM_PL180_MMCI) += arm_pl180_mmci.o
+obj-$(CONFIG_MMC_SUNXI) += sunxi_mmc.o
 obj-$(CONFIG_MV_SDHCI) += mv_sdhci.o
+obj-$(CONFIG_MVEBU_MMC) += mvebu_mmc.o
 obj-$(CONFIG_MXC_MMC) += mxcmmc.o
 obj-$(CONFIG_MXS_MMC) += mxsmmc.o
 obj-$(CONFIG_OMAP_HSMMC) += omap_hsmmc.o
 obj-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
-obj-$(CONFIG_SDHCI) += sdhci.o
-obj-$(CONFIG_BCM2835_SDHCI) += bcm2835_sdhci.o
-obj-$(CONFIG_KONA_SDHCI) += kona_sdhci.o
+obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
 obj-$(CONFIG_S3C_SDI) += s3c_sdi.o
 obj-$(CONFIG_S5P_SDHCI) += s5p_sdhci.o
+obj-$(CONFIG_SDHCI) += sdhci.o
 obj-$(CONFIG_SH_MMCIF) += sh_mmcif.o
+obj-$(CONFIG_SOCFPGA_DWMMC) += socfpga_dw_mmc.o
 obj-$(CONFIG_SPEAR_SDHCI) += spear_sdhci.o
 obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o
-obj-$(CONFIG_DWMMC) += dw_mmc.o
-obj-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o
-obj-$(CONFIG_MMC_SUNXI) += sunxi_mmc.o
 obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o
-obj-$(CONFIG_SOCFPGA_DWMMC) += socfpga_dw_mmc.o
-obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
+
 ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o
 else
 obj-$(CONFIG_GENERIC_MMC) += mmc_write.o
 endif
-obj-$(CONFIG_MVEBU_MMC) += mvebu_mmc.o
+

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Chin Liang See

2014-09-12 07:25 UTC #11 lore raw

PATCH 1/1 sort drivers/mmc/Makefile

Quoted 49 lines · ZY - pavel

Sort drivers/mmc makefile, as requested by wd.

Signed-off-by Pavel Machek <[email protected]>

drivers/mmc/Makefile +10 -7

index 464cee1..461d7d8 100644
@@ -5,37 +5,39 @@
 # SPDX-License-Identifier:	GPL-2.0+
 #
 
+obj-$(CONFIG_ARM_PL180_MMCI) += arm_pl180_mmci.o
+obj-$(CONFIG_BCM2835_SDHCI) += bcm2835_sdhci.o
 obj-$(CONFIG_BFIN_SDH) += bfin_sdh.o
 obj-$(CONFIG_DAVINCI_MMC) += davinci_mmc.o
+obj-$(CONFIG_DWMMC) += dw_mmc.o
+obj-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o
 obj-$(CONFIG_FSL_ESDHC) += fsl_esdhc.o
 obj-$(CONFIG_FTSDC010) += ftsdc010_mci.o
 obj-$(CONFIG_FTSDC021) += ftsdc021_sdhci.o
 obj-$(CONFIG_GENERIC_MMC) += mmc.o
 obj-$(CONFIG_GENERIC_ATMEL_MCI) += gen_atmel_mci.o
+obj-$(CONFIG_KONA_SDHCI) += kona_sdhci.o
 obj-$(CONFIG_MMC_SPI) += mmc_spi.o
-obj-$(CONFIG_ARM_PL180_MMCI) += arm_pl180_mmci.o
+obj-$(CONFIG_MMC_SUNXI) += sunxi_mmc.o
 obj-$(CONFIG_MV_SDHCI) += mv_sdhci.o
+obj-$(CONFIG_MVEBU_MMC) += mvebu_mmc.o
 obj-$(CONFIG_MXC_MMC) += mxcmmc.o
 obj-$(CONFIG_MXS_MMC) += mxsmmc.o
 obj-$(CONFIG_OMAP_HSMMC) += omap_hsmmc.o
 obj-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
-obj-$(CONFIG_SDHCI) += sdhci.o
-obj-$(CONFIG_BCM2835_SDHCI) += bcm2835_sdhci.o
-obj-$(CONFIG_KONA_SDHCI) += kona_sdhci.o
+obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
 obj-$(CONFIG_S3C_SDI) += s3c_sdi.o
 obj-$(CONFIG_S5P_SDHCI) += s5p_sdhci.o
+obj-$(CONFIG_SDHCI) += sdhci.o
 obj-$(CONFIG_SH_MMCIF) += sh_mmcif.o
+obj-$(CONFIG_SOCFPGA_DWMMC) += socfpga_dw_mmc.o
 obj-$(CONFIG_SPEAR_SDHCI) += spear_sdhci.o
 obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o
-obj-$(CONFIG_DWMMC) += dw_mmc.o
-obj-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o
-obj-$(CONFIG_MMC_SUNXI) += sunxi_mmc.o
 obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o

drivers/mmc/Makefile line 43

Quoted 10 lines

drivers/mmc/Makefile +2 -3

-obj-$(CONFIG_SOCFPGA_DWMMC) += socfpga_dw_mmc.o
-obj-$(CONFIG_SUPPORT_EMMC_RPMB) += rpmb.o
+
 ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o
 else
 obj-$(CONFIG_GENERIC_MMC) += mmc_write.o
 endif
-obj-$(CONFIG_MVEBU_MMC) += mvebu_mmc.o
+

Acked-by Chin Liang See <[email protected]>

Tom Rini

2014-11-10 21:29 UTC #12 lore raw

[U-Boot] sort drivers/mmc/Makefile

In reply to Pavel Machek

drivers/mmc/Makefile

Quoted 7 lines · Pavel Machek

Sort drivers/mmc makefile, as requested by wd.
Signed-off-by Pavel Machek <[email protected]>
Acked-by Chin Liang See <[email protected]>
drivers/mmc/Makefile +0 -0
index 464cee1..461d7d8 100644

Applied to u-boot/master, thanks!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141110/fb6f6504/attachment.pgp>

Pavel Machek

author

2014-07-21 10:30 UTC #13 lore raw

PATCH 1/1 socfpga: cleanup socfpga_dw_mmc

In reply to Wolfgang Denk

Cleanups as suggested by wd on mailing list, plus I replaced calloc(...,1) with malloc().

Signed-off-by Pavel Machek <[email protected]>

drivers/mmc/socfpga_dw_mmc.c +4 -5

@@ -16,8 +16,6 @@ static const struct socfpga_clock_manager *clock_manager_base =
 static const struct socfpga_system_manager *system_manager_base =
 		(void *)SOCFPGA_SYSMGR_ADDRESS;
 
-static char *SOCFPGA_NAME = "SOCFPGA DWMMC";
-
 #define CLKMGR_PERPLLGRP_EN_SDMMCCLK_MASK (1 << 8)
 
 static void socfpga_dwmci_clksel(struct dwmci_host *host)

@@ -47,14 +45,15 @@ static void socfpga_dwmci_clksel(struct dwmci_host *host)
 
 int socfpga_dwmmc_init(u32 regbase, int bus_width, int index)
 {
-	struct dwmci_host *host = NULL;
-	host = calloc(sizeof(struct dwmci_host), 1);
+	struct dwmci_host *host;
+
+	host = malloc(sizeof(struct dwmci_host));
 	if (!host) {
 		printf("dwmci_host calloc fail!\n");
 		return -1;
 	}
 
-	host->name = SOCFPGA_NAME;
+	host->name = "SOCFPGA DWMMC";
 	host->ioaddr = (void *)regbase;
 	host->buswidth = bus_width;
 	host->clksel = socfpga_dwmci_clksel;

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Chin Liang See

2014-07-21 10:49 UTC #14 lore raw

PATCH 1/1 socfpga: cleanup socfpga_dw_mmc

Hi Pavel,

Quoted 17 lines · ZY - pavel

Cleanups as suggested by wd on mailing list, plus I replaced calloc(...,1) with malloc().

Signed-off-by Pavel Machek <[email protected]>

drivers/mmc/socfpga_dw_mmc.c +0 -2

@@ -16,8 +16,6 @@ static const struct socfpga_clock_manager *clock_manager_base =
 static const struct socfpga_system_manager *system_manager_base =
 		(void *)SOCFPGA_SYSMGR_ADDRESS;
 
-static char *SOCFPGA_NAME = "SOCFPGA DWMMC";
-
 #define CLKMGR_PERPLLGRP_EN_SDMMCCLK_MASK (1 << 8)
 
 static void socfpga_dwmci_clksel(struct dwmci_host *host)

drivers/mmc/socfpga_dw_mmc.c static void socfpga_dwmci_clksel(struct dwmci_host *host) · line 50

Quoted 8 lines

drivers/mmc/socfpga_dw_mmc.c +3 -2

@@ -47,14 +45,15 @@ static void socfpga_dwmci_clksel(struct dwmci_host *host)
 
 int socfpga_dwmmc_init(u32 regbase, int bus_width, int index)
 {
-	struct dwmci_host *host = NULL;
-	host = calloc(sizeof(struct dwmci_host), 1);
+	struct dwmci_host *host;
+
+	host = malloc(sizeof(struct dwmci_host));

We need calloc as it will initialize the allocated region. It will prevent errors due to NULL comparison against uninitialized structure member.

Thanks
Chin Liang

Quoted 12 lines

drivers/mmc/socfpga_dw_mmc.c +1 -1

 	if (!host) {
 		printf("dwmci_host calloc fail!\n");
 		return -1;
 	}
 
-	host->name = SOCFPGA_NAME;
+	host->name = "SOCFPGA DWMMC";
 	host->ioaddr = (void *)regbase;
 	host->buswidth = bus_width;
 	host->clksel = socfpga_dwmci_clksel;

Pavel Machek

author

2014-07-21 11:30 UTC #15 lore raw

PATCH v2 1/1 socfpga: cleanup socfpga_dw_mmc

In reply to Pavel Machek

Cleanups as suggested by wd on mailing list.

Signed-off-by Pavel Machek <[email protected]>

---

v2: calloc is actually needed, document it.

drivers/mmc/socfpga_dw_mmc.c +4 -4

@@ -16,8 +16,6 @@ static const struct socfpga_clock_manager *clock_manager_base =
 static const struct socfpga_system_manager *system_manager_base =
 		(void *)SOCFPGA_SYSMGR_ADDRESS;
 
-static char *SOCFPGA_NAME = "SOCFPGA DWMMC";
-
 #define CLKMGR_PERPLLGRP_EN_SDMMCCLK_MASK (1 << 8)
 
 static void socfpga_dwmci_clksel(struct dwmci_host *host)

@@ -47,14 +45,16 @@ static void socfpga_dwmci_clksel(struct dwmci_host *host)
 
 int socfpga_dwmmc_init(u32 regbase, int bus_width, int index)
 {
-	struct dwmci_host *host = NULL;
+	struct dwmci_host *host;
+
+	/* calloc for zero init */
 	host = calloc(sizeof(struct dwmci_host), 1);
 	if (!host) {
 		printf("dwmci_host calloc fail!\n");
 		return -1;
 	}
 
-	host->name = SOCFPGA_NAME;
+	host->name = "SOCFPGA DWMMC";
 	host->ioaddr = (void *)regbase;
 	host->buswidth = bus_width;
 	host->clksel = socfpga_dwmci_clksel;

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Chin Liang See

2014-07-22 04:55 UTC #16 lore raw

PATCH v2 1/1 socfpga: cleanup socfpga_dw_mmc

Quoted 30 lines · ZY - pavel

Cleanups as suggested by wd on mailing list.

Signed-off-by Pavel Machek <[email protected]>

---

v2: calloc is actually needed, document it.

drivers/mmc/socfpga_dw_mmc.c +3 -3

@@ -16,8 +16,6 @@ static const struct socfpga_clock_manager *clock_manager_base =
 static const struct socfpga_system_manager *system_manager_base =
 		(void *)SOCFPGA_SYSMGR_ADDRESS;
 
-static char *SOCFPGA_NAME = "SOCFPGA DWMMC";
-
 #define CLKMGR_PERPLLGRP_EN_SDMMCCLK_MASK (1 << 8)
 
 static void socfpga_dwmci_clksel(struct dwmci_host *host)

@@ -47,14 +45,16 @@ static void socfpga_dwmci_clksel(struct dwmci_host *host)
 
 int socfpga_dwmmc_init(u32 regbase, int bus_width, int index)
 {
-	struct dwmci_host *host = NULL;
+	struct dwmci_host *host;
+
+	/* calloc for zero init */
 	host = calloc(sizeof(struct dwmci_host), 1);

drivers/mmc/socfpga_dw_mmc.c static void socfpga_dwmci_clksel(struct dwmci_host *host) · line 60

Quoted 10 lines

drivers/mmc/socfpga_dw_mmc.c +1 -1

 	if (!host) {
 		printf("dwmci_host calloc fail!\n");
 		return -1;
 	}
 
-	host->name = SOCFPGA_NAME;
+	host->name = "SOCFPGA DWMMC";
 	host->ioaddr = (void *)regbase;
 	host->buswidth = bus_width;
 	host->clksel = socfpga_dwmci_clksel;

Acked-by Chin Liang See <[email protected]>

Thanks
Chin Liang

Tom Rini

2014-08-30 15:13 UTC #17 lore raw

PATCH v2 1/1 socfpga: cleanup socfpga_dw_mmc

In reply to Pavel Machek

Quoted 4 lines · Pavel Machek

Cleanups as suggested by wd on mailing list.
Signed-off-by Pavel Machek <[email protected]>
Acked-by Chin Liang See <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140830/8155c996/attachment.pgp>

Jaehoon Chung

2014-07-22 03:22 UTC #18 lore raw

In reply to Pavel Machek

Hi,

I don't know what differ with socfpga_dw_mmc.c.
I think you can use it.

Best Regards,
Jaehoon Chung

Quoted 159 lines · Pavel Machek

Hi!

Once ethernet changes I merged, I can rediff mmc changes for application. It will look something like this:

Albert, can you apply the ethernet patch? I'd like to continue on top of it...
Pavel

arch/arm/cpu/armv7/socfpga/misc.c +11 -0

index 2f1c716..7af384e 100644
@@ -14,3 +16,27 @@ int dram_init(void)
+
+#ifdef CONFIG_DWMMC
+/*
+ * Initializes MMC controllers.
+ * to override, implement board_mmc_init()
+ */
+int cpu_mmc_init(bd_t *bis)
+{
+       return altera_dwmmc_init(SOCFPGA_SDMMC_ADDRESS, 4, 0);
+}
+#endif

arch/arm/include/asm/arch-socfpga/socfpga_base_addrs.h +1 -0

index f564046..9b488d9 100644
@@ -14,5 +14,8 @@
 #define SOCFPGA_CLKMGR_ADDRESS 0xffd04000
 #define SOCFPGA_RSTMGR_ADDRESS 0xffd05000
 #define SOCFPGA_SYSMGR_ADDRESS 0xffd08000
+#define SOCFPGA_SDMMC_ADDRESS 0xff704000
 
 #endif /* _SOCFPGA_BASE_ADDRS_H_ */

drivers/mmc/Makefile +1 -0

index 34febf5..5902105 100644
@@ -28,6 +28,7 @@ obj-$(CONFIG_SPEAR_SDHCI) += spear_sdhci.o
 obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o
 obj-$(CONFIG_DWMMC) += dw_mmc.o
 obj-$(CONFIG_EXYNOS_DWMMC) += exynos_dw_mmc.o
+obj-$(CONFIG_ALTERA_DWMMC) += altera_dw_mmc.o
 obj-$(CONFIG_MMC_SUNXI) += sunxi_mmc.o
 obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o
 obj-$(CONFIG_SOCFPGA_DWMMC) += socfpga_dw_mmc.o

drivers/mmc/altera_dw_mmc.c new file +74 -0

new file mode 100644
index 0000000..b22dc45
@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2013 Altera Corporation <www.altera.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,  MA 02111-1307 USA
+ *
+ */
+
+#include <common.h>
+#include <malloc.h>
+#include <dwmmc.h>
+#include <asm/arch/dwmmc.h>
+#include <errno.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)
+
+static char *ALTERA_NAME = "ALTERA DWMMC";
+
+static void altera_dwmci_clksel(struct dwmci_host *host)
+{
+	unsigned int drvsel;
+	unsigned int smplsel;
+
+	/* Disable SDMMC clock. */
+	clrbits_le32(CLKMGR_PERPLLGRP_EN_REG, CLKMGR_SDMMC_CLK_ENABLE);
+
+	/* Configures drv_sel and smpl_sel */
+	drvsel = 3;
+	smplsel = 0;
+
+	debug("%s: drvsel %d smplsel %d\n", __FUNCTION__, drvsel, smplsel);
+	writel((smplsel << 3) | drvsel, SYSMGR_SDMMCGRP_CTRL_REG);
+
+	/* Enable SDMMC clock */
+	setbits_le32(CLKMGR_PERPLLGRP_EN_REG, CLKMGR_SDMMC_CLK_ENABLE);
+}
+
+int altera_dwmmc_init(u32 regbase, int bus_width, int index)
+{
+	struct dwmci_host *host = NULL;
+	host = malloc(sizeof(struct dwmci_host));
+	if (!host) {
+		printf("dwmci_host malloc failed!\n");
+		return -ENOMEM;
+	}
+
+	host->name = ALTERA_NAME;
+	host->ioaddr = (void *)regbase;
+	host->buswidth = bus_width;
+	host->clksel = altera_dwmci_clksel;
+	host->dev_index = index;
+	host->bus_hz = 400000;
+	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, host->bus_hz);
+
+	return 0;
+}
+

include/configs/socfpga_cyclone5.h +17 -0

index 0254249..189dcde 100644
@@ -180,13 +183,30 @@
 /*
  * FLASH
  */
 #define CONFIG_SYS_NO_FLASH
 
+
+/*
+ * MMC support
+ */
+#define CONFIG_MMC
+#ifdef CONFIG_MMC
+#define CONFIG_CMD_MMC
+
+#define CONFIG_BOUNCE_BUFFER
+#define CONFIG_GENERIC_MMC             1
+#define CONFIG_DWMMC                   1
+#define CONFIG_ALTERA_DWMMC            1
+#define CONFIG_DWMMC_FIFO_DEPTH		1024
+/* using smaller max blk cnt to avoid flooding the limited stack we have */
+#define CONFIG_SYS_MMC_MAX_BLK_COUNT     256
+#endif /* CONFIG_MMC */
+
 /*
  * L4 OSC1 Timer 0
  */

Pavel Machek

author

2014-07-22 08:49 UTC #19 lore raw

Hi!

Quoted 2 lines

I don't know what differ with socfpga_dw_mmc.c.
I think you can use it.

Yes, I think I can. I missed it because it has different name than in altera sources, and because it was not hooked up with rest of system.

Thanks,
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html