[U-Boot] [PATCH 2/8] mmc: sdhci: rework Samsung specfic code

Minkyu Kang
2011-06-13T06:59:36Z

Dear Rob Herring,

On 12 June 2011 06:46, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> Move the register definitions into the sdhci.c file. Set the base
> address from the board init code.
>
> The Samsung SDHCI controller has extra registers. Make them conditional
> on CONFIG_MMC_S5P.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
> ?arch/arm/include/asm/arch-s5pc1xx/mmc.h ?| ? 73 --------------------------
> ?arch/arm/include/asm/arch-s5pc2xx/mmc.h ?| ? 73 --------------------------
> ?board/samsung/goni/goni.c ? ? ? ? ? ? ? ?| ? ?4 +-
> ?board/samsung/universal_c210/universal.c | ? ?8 ++-
> ?drivers/mmc/sdhci.c ? ? ? ? ? ? ? ? ? ? ?| ? 82 +++++++++++++++++++++++-------
> ?include/sdhci.h ? ? ? ? ? ? ? ? ? ? ? ? ?| ? 18 +++++++
> ?6 files changed, 89 insertions(+), 169 deletions(-)
> ?delete mode 100644 arch/arm/include/asm/arch-s5pc1xx/mmc.h
> ?delete mode 100644 arch/arm/include/asm/arch-s5pc2xx/mmc.h
> ?create mode 100644 include/sdhci.h
>
> diff --git a/board/samsung/universal_c210/universal.c b/board/samsung/universal_c210/universal.c
> index b65bc6e..b0e76e8 100644
> --- a/board/samsung/universal_c210/universal.c
> +++ b/board/samsung/universal_c210/universal.c
> @@ -23,10 +23,10 @@
> ?*/
>
> ?#include <common.h>
> +#include <sdhci.h>
> ?#include <asm/io.h>
> ?#include <asm/arch/adc.h>
> ?#include <asm/arch/gpio.h>
> -#include <asm/arch/mmc.h>
>
> ?DECLARE_GLOBAL_DATA_PTR;
>
> @@ -151,6 +151,7 @@ int checkboard(void)
> ?int board_mmc_init(bd_t *bis)
> ?{
> ? ? ? ?int i, err;
> + ? ? ? u32 base;
>
> ? ? ? ?switch (get_hwrev()) {
> ? ? ? ?case 0:
> @@ -217,7 +218,8 @@ int board_mmc_init(bd_t *bis)
> ? ? ? ? * mmc0 ?: eMMC (8-bit buswidth)
> ? ? ? ? * mmc2 ?: SD card (4-bit buswidth)
> ? ? ? ? */
> - ? ? ? err = s5p_mmc_init(0, 8);
> + ? ? ? base = samsung_get_base_mmc();
> + ? ? ? err = sdhci_mmc_init((void *)base, 8);
>
> ? ? ? ?/*
> ? ? ? ? * Check the T-flash ?detect pin
> @@ -241,7 +243,7 @@ int board_mmc_init(bd_t *bis)
> ? ? ? ? ? ? ? ? ? ? ? ?/* GPK2[0:6] drv 4x */
> ? ? ? ? ? ? ? ? ? ? ? ?gpio_set_drv(&gpio2->k2, i, GPIO_DRV_4X);
> ? ? ? ? ? ? ? ?}
> - ? ? ? ? ? ? ? err = s5p_mmc_init(2, 4);
> + ? ? ? ? ? ? ? err = sdhci_mmc_init((void *)(base + 0x20000), 4);

No, please use the macro instead of constant. (pass the device number to macro)

> ? ? ? ?}
>
> ? ? ? ?return err;
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 280738f..c82bde0 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -21,18 +21,56 @@
> ?#include <common.h>
> ?#include <mmc.h>
> ?#include <asm/io.h>
> -#include <asm/arch/mmc.h>
> ?#include <asm/arch/clk.h>
>
> +struct sdhci_mmc {
> + ? ? ? unsigned int ? ?sysad;
> + ? ? ? unsigned short ?blksize;
> + ? ? ? unsigned short ?blkcnt;
> + ? ? ? unsigned int ? ?argument;
> + ? ? ? unsigned short ?trnmod;
> + ? ? ? unsigned short ?cmdreg;
> + ? ? ? unsigned int ? ?rspreg0;
> + ? ? ? unsigned int ? ?rspreg1;
> + ? ? ? unsigned int ? ?rspreg2;
> + ? ? ? unsigned int ? ?rspreg3;
> + ? ? ? unsigned int ? ?bdata;
> + ? ? ? unsigned int ? ?prnsts;
> + ? ? ? unsigned char ? hostctl;
> + ? ? ? unsigned char ? pwrcon;
> + ? ? ? unsigned char ? blkgap;
> + ? ? ? unsigned char ? wakcon;
> + ? ? ? unsigned short ?clkcon;
> + ? ? ? unsigned char ? timeoutcon;
> + ? ? ? unsigned char ? swrst;
> + ? ? ? unsigned int ? ?norintsts; ? ? ?/* errintsts */
> + ? ? ? unsigned int ? ?norintstsen; ? ?/* errintstsen */
> + ? ? ? unsigned int ? ?norintsigen; ? ?/* errintsigen */
> + ? ? ? unsigned short ?acmd12errsts;
> + ? ? ? unsigned char ? res1[2];
> + ? ? ? unsigned int ? ?capareg;
> + ? ? ? unsigned char ? res2[4];
> + ? ? ? unsigned int ? ?maxcurr;
> + ? ? ? unsigned char ? res3[0x34];
> + ? ? ? unsigned int ? ?control2;
> + ? ? ? unsigned int ? ?control3;
> + ? ? ? unsigned char ? res4[4];
> + ? ? ? unsigned int ? ?control4;
> + ? ? ? unsigned char ? res5[0x6e];
> + ? ? ? unsigned short ?hcver;
> +};
> +
> +struct mmc_host {
> + ? ? ? struct sdhci_mmc *reg;
> + ? ? ? unsigned int version; ? /* SDHCI spec. version */
> + ? ? ? unsigned int clock; ? ? /* Current clock (MHz) */
> + ? ? ? int dev_index;
> +};

Please move this structures to header file.

> +
> ?/* support 4 mmc hosts */
> ?struct mmc mmc_dev[4];
> ?struct mmc_host mmc_host[4];
> -
> -static inline struct s5p_mmc *s5p_get_base_mmc(int dev_index)
> -{
> - ? ? ? unsigned long offset = dev_index * sizeof(struct s5p_mmc);
> - ? ? ? return (struct s5p_mmc *)(samsung_get_base_mmc() + offset);
> -}
> +int dev_count;
>
> ?static void mmc_prepare_data(struct mmc_host *host, struct mmc_data *data)
> ?{
> @@ -255,6 +293,7 @@ static void mmc_change_clock(struct mmc_host *host, uint clock)
> ? ? ? ?unsigned long timeout;
> ? ? ? ?unsigned long ctrl2;
>
> +#ifdef CONFIG_S5P_MMC
> ? ? ? ?/*
> ? ? ? ? * SELBASECLK[5:4]
> ? ? ? ? * 00/01 = HCLK
> @@ -265,7 +304,7 @@ static void mmc_change_clock(struct mmc_host *host, uint clock)
> ? ? ? ?ctrl2 &= ~(3 << 4);
> ? ? ? ?ctrl2 |= (2 << 4);
> ? ? ? ?writel(ctrl2, &host->reg->control2);
> -
> +#endif
> ? ? ? ?writew(0, &host->reg->clkcon);
>
> ? ? ? ?/* XXX: we assume that clock is between 40MHz and 50MHz */
> @@ -320,6 +359,7 @@ static void mmc_set_ios(struct mmc *mmc)
>
> ? ? ? ?debug("bus_width: %x, clock: %d\n", mmc->bus_width, mmc->clock);
>
> +#ifdef CONFIG_S5P_MMC
> ? ? ? ?/*
> ? ? ? ? * SELCLKPADDS[17:16]
> ? ? ? ? * 00 = 2mA
> @@ -349,7 +389,7 @@ static void mmc_set_ios(struct mmc *mmc)
> ? ? ? ? * ? ? ?10 = Delay4 (inverter delay + 2ns)
> ? ? ? ? */
> ? ? ? ?writel(0x8080, &host->reg->control3);
> -
> +#endif
> ? ? ? ?mmc_change_clock(host, mmc->clock);
>
> ? ? ? ?ctrl = readb(&host->reg->hostctl);
> @@ -445,14 +485,21 @@ static int mmc_core_init(struct mmc *mmc)
> ? ? ? ?return 0;
> ?}
>
> -static int s5p_mmc_initialize(int dev_index, int bus_width)
> +static int sdhci_mmc_initialize(void *base, int bus_width)
> ?{
> ? ? ? ?struct mmc *mmc;
> + ? ? ? struct mmc_host *host;
> +
> + ? ? ? if (dev_count >= 4)
> + ? ? ? ? ? ? ? return -1;
>
> - ? ? ? mmc = &mmc_dev[dev_index];
> + ? ? ? mmc = &mmc_dev[dev_count];
> + ? ? ? host = &mmc_host[dev_count];
> + ? ? ? host->dev_index = dev_count;
> + ? ? ? dev_count++;

NAK.
dev_index is not a count number, it's a h/w device number.
This change is wrong. (Maybe you misunderstood this concept)
Please keep the dev_index.

>
> - ? ? ? sprintf(mmc->name, "SAMSUNG SD/MMC");
> - ? ? ? mmc->priv = &mmc_host[dev_index];
> + ? ? ? sprintf(mmc->name, "SDHCI SD/MMC");
> + ? ? ? mmc->priv = host;
> ? ? ? ?mmc->send_cmd = mmc_send_cmd;
> ? ? ? ?mmc->set_ios = mmc_set_ios;
> ? ? ? ?mmc->init = mmc_core_init;
> @@ -467,16 +514,15 @@ static int s5p_mmc_initialize(int dev_index, int bus_width)
> ? ? ? ?mmc->f_min = 400000;
> ? ? ? ?mmc->f_max = 52000000;
>
> - ? ? ? mmc_host[dev_index].dev_index = dev_index;

Ditto. Please keep the dev_index.

> - ? ? ? mmc_host[dev_index].clock = 0;
> - ? ? ? mmc_host[dev_index].reg = s5p_get_base_mmc(dev_index);
> + ? ? ? host->clock = 0;
> + ? ? ? host->reg = base;
> ? ? ? ?mmc->b_max = 0;
> ? ? ? ?mmc_register(mmc);
>
> ? ? ? ?return 0;
> ?}
>
> -int s5p_mmc_init(int dev_index, int bus_width)
> +int sdhci_mmc_init(void *base, int bus_width)
> ?{
> - ? ? ? return s5p_mmc_initialize(dev_index, bus_width);
> + ? ? ? return sdhci_mmc_initialize(base, bus_width);
> ?}

Thanks
Minkyu Kang
--
from. prom.