[U-Boot] [PATCH v2 1/2] mmc : Add SDIO driver for Marvell SoCs (Kirkwood)

[Prafulla Wadaskar]

> -----Original Message-----
> From: Gérald Kerma [mailto:geraker at gmail.com]
> Sent: Friday, November 19, 2010 9:41 PM
> To: Prafulla Wadaskar
> Cc: u-boot at lists.denx.de; Clint Adams
> Subject: [PATCH v2 1/2] mmc : Add SDIO driver for Marvell SoCs (Kirkwood)
> 

There should be some patch explanation here and 

> ---
Changelog info here..
You can refer some other patches for this, because rest other make it pure driver add patch and this file is pre-requisite requirement by kirkwood SoC to support it, also the patches may be applied by different custodians

(these comments are applicable for all your patches in this patch series)

>  arch/arm/include/asm/arch-kirkwood/kirkwood.h     |    1 +
This should be separate patch 

>  arch/arm/include/asm/arch-kirkwood/mmc_host_def.h |  292 +++++++++
Please move this file to drivers/mmc/ and rename it as mv_sdio.h

>  drivers/mmc/Makefile                              |    1 +
>  drivers/mmc/mv_sdio.c                             |  663
> +++++++++++++++++++++
>  4 files changed, 957 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-kirkwood/kirkwood.h
> b/arch/arm/include/asm/arch-kirkwood/kirkwood.h
> index 0104418..4f9fe7e 100644
> --- a/arch/arm/include/asm/arch-kirkwood/kirkwood.h
> +++ b/arch/arm/include/asm/arch-kirkwood/kirkwood.h
> @@ -60,6 +60,7 @@
>  #define KW_EGIGA0_BASE			(KW_REGISTER(0x72000))
>  #define KW_EGIGA1_BASE			(KW_REGISTER(0x76000))
>  #define KW_SATA_BASE			(KW_REGISTER(0x80000))
> +#define KW_SDIO_BASE			(KW_REGISTER(0x90000))
> 
>  /* Kirkwood Sata controller has two ports */
>  #define KW_SATA_PORT0_OFFSET		0x2000
> diff --git a/arch/arm/include/asm/arch-kirkwood/mmc_host_def.h
> b/arch/arm/include/asm/arch-kirkwood/mmc_host_def.h
> new file mode 100644
> index 0000000..4327c93
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-kirkwood/mmc_host_def.h
> @@ -0,0 +1,292 @@
> +/*
> + * Copyright (C) 2008 Marvell Semiconductors, All Rights Reserved.
> + * Copyright (C) 2010 Gérald Kerma <gerald.kerma at gk2.net>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.

Software license is wrong here, it should be v2 or latter, please refer some recent files added in some other commits Or other Kirkwood files

> + *
> + */
> +
> +#ifndef _MVSDIO_INCLUDE
> +#define _MVSDIO_INCLUDE
> +
> +#define SDIO_REG(x) (KW_SDIO_BASE + (x))
> +
> +#define MVSDMMC_DMA_SIZE			65536
> +#define MVSDMMC_CMD_TIMEOUT			2 /* 100 usec*/
> +
> +/*
> + * Clock rates
> + */
> +
> +#define MVSD_CLOCKRATE_MAX			50000000
> +#define MVSD_BASE_DIV_MAX			0x7ff
> +
> +#define CONFIG_SYS_MMC_CLK_PP			25000000
> +
> +/*
> + * The base MMC clock rate
> + */
> +
> +#define MVSDMMC_CLOCKRATE_MIN			100000
> +#define MVSDMMC_CLOCKRATE_MAX			MVSD_CLOCKRATE_MAX
> +#define MVSDMMC_BASE_FAST_CLOCK			CONFIG_SYS_TCLK
> +
> +
> +/*
> + * SDIO register
> + */
> +
> +#define SDIO_SYS_ADDR				0x000
> +#define SDIO_SYS_ADDR_LOW			0x000
> +#define SDIO_SYS_ADDR_HI			0x004

NAK, these are not acceptable, please use c-struct for register definitions.

Regards..
Prafulla . .