[U-Boot] [PATCH v3 2/3] ARM:AM33XX: Add mmc/sd support

Igor Grinberg
2012-01-03T08:00:33Z

Hi Chandan, Tom,

On 01/03/12 08:42, Chandan Nath wrote:
> This patch add supports for mmc/sd driver on AM335X platform.
> PLL and pinmux configurations for mmc/sd are configured in this
> patch.
>
> Signed-off-by: Chandan Nath <chandan.nath@ti.com>
> Signed-off-by: Tom Rini <trini@ti.com>
> ---
> Changes since v2:
> - No change
>
> Changes since v1:
> - Removed unwanted code from omap_hsmmc.c file
> - Rebased to master branch
>
> arch/arm/cpu/armv7/am33xx/board.c | 7 +
> arch/arm/cpu/armv7/am33xx/clock.c | 5 +
> arch/arm/include/asm/arch-am33xx/mmc_host_def.h | 164 +++++++++++++++++++++++
> board/ti/am335x/mux.c | 20 +++
> include/configs/am335x_evm.h | 9 ++
> 5 files changed, 205 insertions(+), 0 deletions(-)
> create mode 100644 arch/arm/include/asm/arch-am33xx/mmc_host_def.h
>
> diff --git a/arch/arm/cpu/armv7/am33xx/board.c b/arch/arm/cpu/armv7/am33xx/board.c
> index 78db3a5..312643c 100644
> --- a/arch/arm/cpu/armv7/am33xx/board.c
> +++ b/arch/arm/cpu/armv7/am33xx/board.c
> @@ -64,3 +64,10 @@ void init_timer(void)
> /* Start the Timer */
> writel(0x1, (&timer_base->tclr));
> }
> +
> +#if defined(CONFIG_OMAP_HSMMC) && !defined(CONFIG_SPL_BUILD)
> +int board_mmc_init(bd_t *bis)
> +{
> + return omap_mmc_init(0);
> +}

I would suggest to make this definition "weak",
so it can be overridden if needed by the board code.

> +#endif
> diff --git a/arch/arm/cpu/armv7/am33xx/clock.c b/arch/arm/cpu/armv7/am33xx/clock.c
> index 7070e7d..98cfd93 100644
> --- a/arch/arm/cpu/armv7/am33xx/clock.c
> +++ b/arch/arm/cpu/armv7/am33xx/clock.c
> @@ -108,6 +108,11 @@ static void enable_per_clocks(void)
> writel(PRCM_MOD_EN, &cmwkup->wkup_uart0ctrl);
> while (readl(&cmwkup->wkup_uart0ctrl) != PRCM_MOD_EN)
> ;
> +
> + /* MMC0*/
> + writel(PRCM_MOD_EN, &cmper->mmc0clkctrl);
> + while (readl(&cmper->mmc0clkctrl) != PRCM_MOD_EN)
> + ;
> }
>
> static void mpu_pll_config(void)
> diff --git a/arch/arm/include/asm/arch-am33xx/mmc_host_def.h b/arch/arm/include/asm/arch-am33xx/mmc_host_def.h
> new file mode 100644
> index 0000000..e56c018
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-am33xx/mmc_host_def.h
> @@ -0,0 +1,164 @@
> +/*
> + * mmc_host_def.h
> + *
> + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.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 version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef MMC_HOST_DEF_H
> +#define MMC_HOST_DEF_H
> +
> +/*
> + * OMAP HSMMC register definitions
> + */
> +#define OMAP_HSMMC1_BASE 0x48060100
> +#define OMAP_HSMMC2_BASE 0x481D8000
> +#define OMAP_HSMMC3_BASE 0x47C24000
> +
> +typedef struct hsmmc {
> + unsigned char res1[0x10];
> + unsigned int sysconfig; /* 0x10 */
> + unsigned int sysstatus; /* 0x14 */
> + unsigned char res2[0x14];
> + unsigned int con; /* 0x2C */
> + unsigned char res3[0xD4];
> + unsigned int blk; /* 0x104 */
> + unsigned int arg; /* 0x108 */
> + unsigned int cmd; /* 0x10C */
> + unsigned int rsp10; /* 0x110 */
> + unsigned int rsp32; /* 0x114 */
> + unsigned int rsp54; /* 0x118 */
> + unsigned int rsp76; /* 0x11C */
> + unsigned int data; /* 0x120 */
> + unsigned int pstate; /* 0x124 */
> + unsigned int hctl; /* 0x128 */
> + unsigned int sysctl; /* 0x12C */
> + unsigned int stat; /* 0x130 */
> + unsigned int ie; /* 0x134 */
> + unsigned char res4[0x8];
> + unsigned int capa; /* 0x140 */
> +} hsmmc_t;
> +
> +/*
> + * OMAP HS MMC Bit definitions
> + */
> +#define MMC_SOFTRESET (0x1 << 1)
> +#define RESETDONE (0x1 << 0)
> +#define NOOPENDRAIN (0x0 << 0)
> +#define OPENDRAIN (0x1 << 0)
> +#define OD (0x1 << 0)

Most of the names here are too generic,
but the above one is just...
OD - what does it stand for? Open Drain?
Then why do you have two of them (with the same value)?

> +#define INIT_NOINIT (0x0 << 1)
> +#define INIT_INITSTREAM (0x1 << 1)
> +#define HR_NOHOSTRESP (0x0 << 2)
> +#define STR_BLOCK (0x0 << 3)
> +#define MODE_FUNC (0x0 << 4)
> +#define DW8_1_4BITMODE (0x0 << 5)
> +#define MIT_CTO (0x0 << 6)
> +#define CDP_ACTIVEHIGH (0x0 << 7)
> +#define WPP_ACTIVEHIGH (0x0 << 8)
> +#define RESERVED_MASK (0x3 << 9)
> +#define CTPL_MMC_SD (0x0 << 11)
> +#define BLEN_512BYTESLEN (0x200 << 0)
> +#define NBLK_STPCNT (0x0 << 16)
> +#define DE_DISABLE (0x0 << 0)
> +#define BCE_DISABLE (0x0 << 1)
> +#define BCE_ENABLE (0x1 << 1)
> +#define ACEN_DISABLE (0x0 << 2)
> +#define DDIR_OFFSET (4)
> +#define DDIR_MASK (0x1 << 4)
> +#define DDIR_WRITE (0x0 << 4)
> +#define DDIR_READ (0x1 << 4)
> +#define MSBS_SGLEBLK (0x0 << 5)
> +#define MSBS_MULTIBLK (0x1 << 5)
> +#define RSP_TYPE_OFFSET (16)
> +#define RSP_TYPE_MASK (0x3 << 16)
> +#define RSP_TYPE_NORSP (0x0 << 16)
> +#define RSP_TYPE_LGHT136 (0x1 << 16)
> +#define RSP_TYPE_LGHT48 (0x2 << 16)
> +#define RSP_TYPE_LGHT48B (0x3 << 16)
> +#define CCCE_NOCHECK (0x0 << 19)
> +#define CCCE_CHECK (0x1 << 19)
> +#define CICE_NOCHECK (0x0 << 20)
> +#define CICE_CHECK (0x1 << 20)
> +#define DP_OFFSET (21)
> +#define DP_MASK (0x1 << 21)
> +#define DP_NO_DATA (0x0 << 21)
> +#define DP_DATA (0x1 << 21)
> +#define CMD_TYPE_NORMAL (0x0 << 22)
> +#define INDEX_OFFSET (24)
> +#define INDEX_MASK (0x3f << 24)
> +#define INDEX(i) (i << 24)
> +#define DATI_MASK (0x1 << 1)
> +#define DATI_CMDDIS (0x1 << 1)
> +#define DTW_1_BITMODE (0x0 << 1)
> +#define DTW_4_BITMODE (0x1 << 1)
> +#define DTW_8_BITMODE (0x1 << 5) /* CON[DW8]*/
> +#define SDBP_PWROFF (0x0 << 8)
> +#define SDBP_PWRON (0x1 << 8)
> +#define SDVS_1V8 (0x5 << 9)
> +#define SDVS_3V0 (0x6 << 9)
> +#define ICE_MASK (0x1 << 0)
> +#define ICE_STOP (0x0 << 0)
> +#define ICS_MASK (0x1 << 1)
> +#define ICS_NOTREADY (0x0 << 1)
> +#define ICE_OSCILLATE (0x1 << 0)
> +#define CEN_MASK (0x1 << 2)
> +#define CEN_DISABLE (0x0 << 2)
> +#define CEN_ENABLE (0x1 << 2)
> +#define CLKD_OFFSET (6)
> +#define CLKD_MASK (0x3FF << 6)
> +#define DTO_MASK (0xF << 16)
> +#define DTO_15THDTO (0xE << 16)
> +#define SOFTRESETALL (0x1 << 24)
> +#define CC_MASK (0x1 << 0)
> +#define TC_MASK (0x1 << 1)
> +#define BWR_MASK (0x1 << 4)
> +#define BRR_MASK (0x1 << 5)
> +#define ERRI_MASK (0x1 << 15)
> +#define IE_CC (0x01 << 0)
> +#define IE_TC (0x01 << 1)
> +#define IE_BWR (0x01 << 4)
> +#define IE_BRR (0x01 << 5)
> +#define IE_CTO (0x01 << 16)
> +#define IE_CCRC (0x01 << 17)
> +#define IE_CEB (0x01 << 18)
> +#define IE_CIE (0x01 << 19)
> +#define IE_DTO (0x01 << 20)
> +#define IE_DCRC (0x01 << 21)
> +#define IE_DEB (0x01 << 22)
> +#define IE_CERR (0x01 << 28)
> +#define IE_BADA (0x01 << 29)

I would@least separate all the above crappy defines
into register related sets and give a prefix which will make it
clear what register is this bit related to...

Also, why all this defines are in the header file?
Are they used in the driver?

Ahh, I see... most of it is just a copy/paste of the same data in:
arch/arm/include/asm/arch-omap3/mmc_host_def.h

So, may be just create a common file in a common location,
instead of copying? And use it with all supported platforms?

(Also, would be nice to clean it up, but it is another story)

> +
> +#define VS30_3V0SUP (1 << 25)
> +#define VS18_1V8SUP (1 << 26)
> +
> +/* Driver definitions */
> +#define MMCSD_SECTOR_SIZE 512
> +#define MMC_CARD 0
> +#define SD_CARD 1
> +#define BYTE_MODE 0
> +#define SECTOR_MODE 1
> +#define CLK_INITSEQ 0
> +#define CLK_400KHZ 1
> +#define CLK_MISC 2
> +
> +#define RSP_TYPE_NONE (RSP_TYPE_NORSP | CCCE_NOCHECK | CICE_NOCHECK)
> +#define MMC_CMD0 (INDEX(0) | RSP_TYPE_NONE | DP_NO_DATA | DDIR_WRITE)

Multiple spaces inside the mask?
If you want to align, align with tabs.

> +
> +/* Clock Configurations and Macros */
> +#define MMC_CLOCK_REFERENCE 96 /* MHz */
> +
> +#define mmc_reg_out(addr, mask, val)\
> + writel((readl(addr) & (~(mask))) | ((val) & (mask)), (addr))
> +
> +int omap_mmc_init(int dev_index);
> +
> +#endif /* MMC_HOST_DEF_H */
> diff --git a/board/ti/am335x/mux.c b/board/ti/am335x/mux.c
> index 8f27409..df11752 100644
> --- a/board/ti/am335x/mux.c
> +++ b/board/ti/am335x/mux.c
> @@ -258,6 +258,20 @@ static struct module_pin_mux uart0_pin_mux[] = {
> {-1},
> };
>
> +#ifdef CONFIG_MMC
> +static struct module_pin_mux mmc0_pin_mux[] = {
> + {OFFSET(mmc0_dat3), (MODE(0) | RXACTIVE | PULLUP_EN)}, /* MMC0_DAT3 */
> + {OFFSET(mmc0_dat2), (MODE(0) | RXACTIVE | PULLUP_EN)}, /* MMC0_DAT2 */
> + {OFFSET(mmc0_dat1), (MODE(0) | RXACTIVE | PULLUP_EN)}, /* MMC0_DAT1 */
> + {OFFSET(mmc0_dat0), (MODE(0) | RXACTIVE | PULLUP_EN)}, /* MMC0_DAT0 */
> + {OFFSET(mmc0_clk), (MODE(0) | RXACTIVE | PULLUP_EN)}, /* MMC0_CLK */
> + {OFFSET(mmc0_cmd), (MODE(0) | RXACTIVE | PULLUP_EN)}, /* MMC0_CMD */
> + {OFFSET(mcasp0_aclkr), (MODE(4) | RXACTIVE)}, /* MMC0_WP */
> + {OFFSET(spi0_cs1), (MODE(5) | RXACTIVE | PULLUP_EN)}, /* MMC0_CD */
> + {-1},
> +};
> +#endif
> +
> /*
> * Configure the pin mux for the module
> */
> @@ -276,3 +290,9 @@ void enable_uart0_pin_mux(void)
> {
> configure_module_pin_mux(uart0_pin_mux);
> }
> +
> +

Why two empty lines?

> +void enable_mmc0_pin_mux(void)
> +{
> + configure_module_pin_mux(mmc0_pin_mux);
> +}
> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
> index c189031..bd68e9a 100644
> --- a/include/configs/am335x_evm.h
> +++ b/include/configs/am335x_evm.h
> @@ -76,6 +76,15 @@
> #define CONFIG_SYS_LOAD_ADDR 0x81000000 /* Default load address */
> #define CONFIG_SYS_HZ 1000 /* 1ms clock */
>
> +#define CONFIG_MMC
> +#define CONFIG_AM335X_HSMMC_BASE 0x48060100
> +#define CONFIG_GENERIC_MMC
> +#define CONFIG_OMAP_HSMMC
> +#define CONFIG_CMD_MMC
> +#define CONFIG_DOS_PARTITION
> +#define CONFIG_CMD_FAT
> +#define CONFIG_CMD_EXT2
> +
> /* Physical Memory Map */
> #define CONFIG_NR_DRAM_BANKS 1 /* 1 bank of DRAM */
> #define PHYS_DRAM_1 0x80000000 /* DRAM Bank #1 */

--
Regards,
Igor.