Skip to contentSkip to conversation

mmc: s5p_mmc: fixed wrong function argument

PATCH Superseded Jaehoon Chung 2 participants 3 messages Jul 27, 2012 – Aug 31, 2012

Subsystems (3)
Arm (32-bit) Arm Samsung Exynos MMC, SD and SDIO

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Jaehoon Chung

author
#0 lore raw

PATCH v2 2/4 mmc: s5p_mmc: fixed wrong function argument

Useless code is removed, and get buswidth value.

Signed-off-by Jaehoon Chung <[email protected]>
---
 arch/arm/include/asm/arch-exynos/mmc.h  |    4 ++--
 arch/arm/include/asm/arch-s5pc1xx/mmc.h |    4 ++--
 drivers/mmc/s5p_sdhci.c                 |    9 ++++-----
 3 files changed, 8 insertions(+), 9 deletions(-)
arch/arm/include/asm/arch-exynos/mmc.h +2 -2
index 0f701c9..afdfcf0 100644
@@ -64,11 +64,11 @@
 #define SDHCI_CTRL4_DRIVE_MASK(_x)	((_x) << 16)
 #define SDHCI_CTRL4_DRIVE_SHIFT		(16)
 
-int s5p_sdhci_init(u32 regbase, u32 max_clk, u32 min_clk, u32 quirks);
+int s5p_sdhci_init(u32 regbase, int index, int bus_width);
 
 static inline unsigned int s5p_mmc_init(int index, int bus_width)
 {
 	unsigned int base = samsung_get_base_mmc() + (0x10000 * index);
-	return s5p_sdhci_init(base, 52000000, 400000, index);
+	return s5p_sdhci_init(base, index, bus_width);
 }
 #endif
arch/arm/include/asm/arch-s5pc1xx/mmc.h +2 -2
index 0f701c9..afdfcf0 100644
@@ -64,11 +64,11 @@
 #define SDHCI_CTRL4_DRIVE_MASK(_x)	((_x) << 16)
 #define SDHCI_CTRL4_DRIVE_SHIFT		(16)
 
-int s5p_sdhci_init(u32 regbase, u32 max_clk, u32 min_clk, u32 quirks);
+int s5p_sdhci_init(u32 regbase, int index, int bus_width);
 
 static inline unsigned int s5p_mmc_init(int index, int bus_width)
 {
 	unsigned int base = samsung_get_base_mmc() + (0x10000 * index);
-	return s5p_sdhci_init(base, 52000000, 400000, index);
+	return s5p_sdhci_init(base, index, bus_width);
 }
 #endif
drivers/mmc/s5p_sdhci.c +4 -5
index 12b28dd..d7e92a4 100644
@@ -69,7 +69,7 @@ static void s5p_sdhci_set_control_reg(struct sdhci_host *host)
 	sdhci_writel(host, ctrl, SDHCI_CONTROL2);
 }
 
-int s5p_sdhci_init(u32 regbase, u32 max_clk, u32 min_clk, u32 quirks)
+int s5p_sdhci_init(u32 regbase, int index, int bus_width)
 {
 	struct sdhci_host *host = NULL;
 	host = (struct sdhci_host *)malloc(sizeof(struct sdhci_host));

@@ -80,12 +80,11 @@ int s5p_sdhci_init(u32 regbase, u32 max_clk, u32 min_clk, u32 quirks)
 
 	host->name = S5P_NAME;
 	host->ioaddr = (void *)regbase;
-	host->quirks = quirks;
 
-	host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_BROKEN_VOLTAGE |
+	host->quirks = SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_BROKEN_VOLTAGE |
 		SDHCI_QUIRK_BROKEN_R1B | SDHCI_QUIRK_32BIT_DMA_ADDR;
 	host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
-	if (quirks & SDHCI_QUIRK_REG32_RW)
+	if (host->quirks & SDHCI_QUIRK_REG32_RW)
 		host->version = sdhci_readl(host, SDHCI_HOST_VERSION - 2) >> 16;
 	else
 		host->version = sdhci_readw(host, SDHCI_HOST_VERSION);

@@ -94,6 +93,6 @@ int s5p_sdhci_init(u32 regbase, u32 max_clk, u32 min_clk, u32 quirks)
 
 	host->host_caps = MMC_MODE_HC;
 
-	add_sdhci(host, max_clk, min_clk);
+	add_sdhci(host, 52000000, 400000);
 	return 0;
 }
Signature · 1 line
1.7.4.1
#1 lore raw

PATCH v2 2/4 mmc: s5p_mmc: fixed wrong function argument

Quoted 31 lines · Jaehoon Chung

Useless code is removed, and get buswidth value.

Signed-off-by Jaehoon Chung <[email protected]>
---
 arch/arm/include/asm/arch-exynos/mmc.h  |    4 ++--
 arch/arm/include/asm/arch-s5pc1xx/mmc.h |    4 ++--
 drivers/mmc/s5p_sdhci.c                 |    9 ++++-----
 3 files changed, 8 insertions(+), 9 deletions(-)
arch/arm/include/asm/arch-exynos/mmc.h +1 -1
 
 
 static inline unsigned int s5p_mmc_init(int index, int bus_width)
 {
        unsigned int base = samsung_get_base_mmc() + (0x10000 * index);
-       return s5p_sdhci_init(base, 52000000, 400000, index);
+       return s5p_sdhci_init(base, index, bus_width);
 }
drivers/mmc/s5p_sdhci.c +1 -1
index 12b28dd..d7e92a4 100644
 
-int s5p_sdhci_init(u32 regbase, u32 max_clk, u32 min_clk, u32 quirks)
+int s5p_sdhci_init(u32 regbase, int index, int bus_width)
 {
        struct sdhci_host *host = NULL;
        host = (struct sdhci_host *)malloc(sizeof(struct sdhci_host));

drivers/mmc/s5p_sdhci.c int s5p_sdhci_init(u32 regbase, u32 max_clk, u32 min_clk, u32 quirks) · line 83

Quoted 4 lines
drivers/mmc/s5p_sdhci.c +0 -1
@@ -80,12 +80,11 @@ int s5p_sdhci_init(u32 regbase, u32 max_clk, u32 min_clk, u32 quirks)
 
        host->name = S5P_NAME;
        host->ioaddr = (void *)regbase;
-       host->quirks = quirks;

I'm very confused by this line. "quirks" is the 4th argument to the original version of this function, but I see that the original *caller* of this function passed in "index". This is amazingly broken. Fortunately, your patch fixes it, but it calls into doubt how it ever worked with any index but "0".

drivers/mmc/s5p_sdhci.c int s5p_sdhci_init(u32 regbase, u32 max_clk, u32 min_clk, u32 quirks) · line 87

Quoted 7 lines
drivers/mmc/s5p_sdhci.c +2 -2
-       host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_BROKEN_VOLTAGE |
+       host->quirks = SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_BROKEN_VOLTAGE |
                SDHCI_QUIRK_BROKEN_R1B | SDHCI_QUIRK_32BIT_DMA_ADDR;
        host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
-       if (quirks & SDHCI_QUIRK_REG32_RW)
+       if (host->quirks & SDHCI_QUIRK_REG32_RW)

Now that host->quirks is initialized just above, SDHCI_QUIRK_REG32_RW will never be set, so there's no point in checking for it.

Andy

author
#2 lore raw

PATCH v2 2/4 mmc: s5p_mmc: fixed wrong function argument

Hi Andy,

Quoted 12 lines

drivers/mmc/s5p_sdhci.c int s5p_sdhci_init(u32 regbase, u32 max_clk, u32 min_clk, u32 quirks) · line 83

Quoted 5 lines
drivers/mmc/s5p_sdhci.c +0 -1
@@ -80,12 +80,11 @@ int s5p_sdhci_init(u32 regbase, u32 max_clk, u32 min_clk, u32 quirks)
 
        host->name = S5P_NAME;
        host->ioaddr = (void *)regbase;
-       host->quirks = quirks;

I'm very confused by this line. "quirks" is the 4th argument to the original version of this function, but I see that the original *caller* of this function passed in "index". This is amazingly broken. Fortunately, your patch fixes it, but it calls into doubt how it ever worked with any index but "0".

It's just lucky. Because We are using the index "0" for eMMC card. So i fixed this problem.

Quoted 11 lines

drivers/mmc/s5p_sdhci.c int s5p_sdhci_init(u32 regbase, u32 max_clk, u32 min_clk, u32 quirks) · line 87

Quoted 9 lines
drivers/mmc/s5p_sdhci.c +2 -2
-       host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_BROKEN_VOLTAGE |
+       host->quirks = SDHCI_QUIRK_NO_HISPD_BIT | SDHCI_QUIRK_BROKEN_VOLTAGE |
                SDHCI_QUIRK_BROKEN_R1B | SDHCI_QUIRK_32BIT_DMA_ADDR;
        host->voltages = MMC_VDD_32_33 | MMC_VDD_33_34 | MMC_VDD_165_195;
-       if (quirks & SDHCI_QUIRK_REG32_RW)
+       if (host->quirks & SDHCI_QUIRK_REG32_RW)
Quoted 2 lines

Now that host->quirks is initialized just above, SDHCI_QUIRK_REG32_RW will never be set, so there's no point in checking for it.

Right, we can remove that. i will fix.

Best Regards,
Jaehoon Chung

Quoted 8 lines

Andy
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot