[U-Boot] [PATCH v2 2/4] mmc: s5p_mmc: fixed wrong function argument

[Andy Fleming]

On Thu, Jul 26, 2012 at 7:33 PM, Jaehoon Chung <jh80.chung at samsung.com> wrote:
>
> Useless code is removed, and get buswidth value.
>
> Signed-off-by: Jaehoon Chung <jh80.chung at 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