| Message ID | 5011FDF8.5060500@samsung.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Andy Fleming |
| Headers | show |
On Thu, Jul 26, 2012 at 7:33 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote: > > Useless code is removed, and get buswidth value. > > Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> > --- > 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(-) > > diff --git a/arch/arm/include/asm/arch-exynos/mmc.h b/arch/arm/include/asm/arch-exynos/mmc.h > > 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); > } > diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c > index 12b28dd..d7e92a4 100644 > --- a/drivers/mmc/s5p_sdhci.c > +++ b/drivers/mmc/s5p_sdhci.c > -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; 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". > > - 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
Hi Andy, >> @@ -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. > > >> >> - 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. Right, we can remove that. i will fix. Best Regards, Jaehoon Chung > > > Andy > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
diff --git a/arch/arm/include/asm/arch-exynos/mmc.h b/arch/arm/include/asm/arch-exynos/mmc.h index 0f701c9..afdfcf0 100644 --- a/arch/arm/include/asm/arch-exynos/mmc.h +++ b/arch/arm/include/asm/arch-exynos/mmc.h @@ -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 diff --git a/arch/arm/include/asm/arch-s5pc1xx/mmc.h b/arch/arm/include/asm/arch-s5pc1xx/mmc.h index 0f701c9..afdfcf0 100644 --- a/arch/arm/include/asm/arch-s5pc1xx/mmc.h +++ b/arch/arm/include/asm/arch-s5pc1xx/mmc.h @@ -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 diff --git a/drivers/mmc/s5p_sdhci.c b/drivers/mmc/s5p_sdhci.c index 12b28dd..d7e92a4 100644 --- a/drivers/mmc/s5p_sdhci.c +++ b/drivers/mmc/s5p_sdhci.c @@ -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; }
Useless code is removed, and get buswidth value. Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com> --- 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(-)