[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-omap
Subject:    Re: [Resending: PATCH 3/6]Device file update to support MMC/SD on OMAP2430/3430
From:       "Eduardo Valentin" <edubezval () gmail ! com>
Date:       2007-12-24 3:46:18
Message-ID: a0580c510712231946o2616e3f2p67d48de7da4f0288 () mail ! gmail ! com
[Download RAW message or body]

Hi Madhusudhan,

Thanks for your good patch. I have some suggestions:

On Dec 20, 2007 6:04 AM, Madhusudhan Chikkature Rajashekar
<madhu.cr@ti.com> wrote:
> This patch registers the MMC device for OMAP2430/3430.
>
> Signed-off-by: Madhusudhan Chikkature<madhu.cr@ti.com>
>
> ---
>  arch/arm/plat-omap/devices.c |   18 ++++++++++++------
>  1 files changed, 12 insertions(+), 6 deletions(-)
>
> Index: linux-omap-2.6/arch/arm/plat-omap/devices.c
> ===================================================================
> --- linux-omap-2.6.orig/arch/arm/plat-omap/devices.c    2007-12-18 17:35:45.578743000 +0530
> +++ linux-omap-2.6/arch/arm/plat-omap/devices.c 2007-12-20 11:19:42.970456249 +0530
> @@ -19,6 +19,7 @@
>  #include <asm/mach-types.h>
>  #include <asm/mach/map.h>
>
> +#include <asm/arch/mmc.h>
Do not duplicate #include.
>  #include <asm/arch/tc.h>
>  #include <asm/arch/board.h>
>  #include <asm/arch/mmc.h>

See? It is already there! :)

> @@ -154,9 +155,10 @@
>
>  /*-------------------------------------------------------------------------*/
>
> -#if    defined(CONFIG_MMC_OMAP) || defined(CONFIG_MMC_OMAP_MODULE)
> +#if    defined(CONFIG_MMC_OMAP) || defined(CONFIG_MMC_OMAP_MODULE) \
> +       || defined(CONFIG_MMC_OMAP_HS) || defined(CONFIG_MMC_OMAP_HS_MODULE)
>
> -#ifdef CONFIG_ARCH_OMAP24XX
> +#if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
>  #define        OMAP_MMC1_BASE          0x4809c000
>  #define OMAP_MMC1_INT          INT_24XX_MMC_IRQ
>  #else
> @@ -228,10 +230,6 @@
>         const struct omap_mmc_config    *mmc_conf;
>         const struct omap_mmc_conf      *mmc;
>
> -       /* REVISIT: 2430 has HS MMC */
> -       if (cpu_is_omap2430() || cpu_is_omap34xx())
> -               return;
> -
>         /* NOTE:  assumes MMC was never (wrongly) enabled */
>         mmc_conf = omap_get_config(OMAP_TAG_MMC, struct omap_mmc_config);
>         if (!mmc_conf)
> @@ -239,6 +237,13 @@
>
>         /* block 1 is always available and has just one pinout option */
>         mmc = &mmc_conf->mmc[0];
> +
> +       if (cpu_is_omap2430() || cpu_is_omap34xx()) {
> +               if (mmc->enabled)
> +                       (void) platform_device_register(&mmc_omap_device1);
> +               return;
> +       }
> +
>         if (mmc->enabled) {
>                 if (cpu_is_omap24xx()) {
>                         omap_cfg_reg(H18_24XX_MMC_CMD);
> @@ -339,6 +344,7 @@
>  }
>
>  #else
I believe it is better to change this function to be static inline.
> +void omap_set_mmc_info(int host, const struct omap_mmc_platform_data *info) {}


>  static inline void omap_init_mmc(void) {}
>  #endif
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Cheers,

-- 
Eduardo Bezerra Valentin
-
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]