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.n...@ti.com>
> Signed-off-by: Tom Rini <tr...@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 at 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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to