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

[Igor Grinberg <grinberg@compulab.co.il>]

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.