Re: [PATCH v2 5/5] OMAP: devices: Modify HSMMC device to adapt to hwmod framework

["Varadarajan, Charulatha" <charu@ti.com>]

Kishore,

On Wed, Feb 9, 2011 at 01:23, Kishore Kadiyala <kishore.kadiyala@ti.com> wrote:
> Changes involves:
> 1) Remove controller reset in devices.c which is taken care
>   by hwmod framework.
> 2) Removing all base address macro defines.
> 3) Using omap-device layer to register device and utilizing data from
>   hwmod data file for base address, dma channel number, Irq_number,
>   device attribute.
> 4) Update the driver to use dev_attr to find whether controller
>   supports dual volt cards.
> 5) Moving "omap_mmc_add" api from plat-omap/devices.c to mach-omap1/devices.c
>
> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
> Signed-off-by: Kishore Kadiyala <kishore.kadiyala@ti.com>
> ---
>  arch/arm/mach-omap1/devices.c         |   41 +++++++
>  arch/arm/mach-omap2/board-n8x0.c      |    6 +-
>  arch/arm/mach-omap2/devices.c         |  207 +++++++--------------------------
>  arch/arm/mach-omap2/hsmmc.c           |    6 +-
>  arch/arm/plat-omap/devices.c          |   50 --------
>  arch/arm/plat-omap/include/plat/mmc.h |   36 ++-----
>  drivers/mmc/host/omap_hsmmc.c         |    4 +-
>  7 files changed, 100 insertions(+), 250 deletions(-)
>

<<snip>>

> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index e0f0ef9..80e46b3 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -610,112 +610,6 @@ static inline void omap_init_aes(void) { }
>
>  /*-------------------------------------------------------------------------*/
>
<<snip>>

>
> @@ -828,66 +722,54 @@ static inline void omap2_mmc_mux(struct omap_mmc_platform_data *mmc_controller,
>        }
>  }
>
> -void __init omap2_init_mmc(struct omap_mmc_platform_data **mmc_data,
> -                       int nr_controllers)
> +struct omap_mmc_platform_data *hsmmc_data[OMAP44XX_NR_MMC];
> +EXPORT_SYMBOL(hsmmc_data);
> +
> +static struct omap_device_pm_latency omap_hsmmc_latency[] = {
> +       [0] = {
> +               .deactivate_func = omap_device_idle_hwmods,
> +               .activate_func   = omap_device_enable_hwmods,
> +               .flags           = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
> +       },
> +       /*
> +        * XXX There should also be an entry here to power off/on the
> +        * MMC regulators/PBIAS cells, etc.
> +        */
> +};
> +
> +int omap2_init_mmc(struct omap_hwmod *oh, void *nop)
>  {
> -       int i;
> +       struct omap_device *od;
> +       struct omap_device_pm_latency *ohl;
>        char *name;
> +       int ohl_cnt = 0;
> +       static int mmc_num;
>
> -       for (i = 0; i < nr_controllers; i++) {
> -               unsigned long base, size;
> -               unsigned int irq = 0;
> -
> -               if (!mmc_data[i])
> -                       continue;
> -
> -               omap2_mmc_mux(mmc_data[i], i);
> -
> -               switch (i) {
> -               case 0:
> -                       base = OMAP2_MMC1_BASE;
> -                       irq = INT_24XX_MMC_IRQ;
> -                       break;
> -               case 1:
> -                       base = OMAP2_MMC2_BASE;
> -                       irq = INT_24XX_MMC2_IRQ;
> -                       break;
> -               case 2:
> -                       if (!cpu_is_omap44xx() && !cpu_is_omap34xx())
> -                               return;
> -                       base = OMAP3_MMC3_BASE;
> -                       irq = INT_34XX_MMC3_IRQ;
> -                       break;
> -               case 3:
> -                       if (!cpu_is_omap44xx())
> -                               return;
> -                       base = OMAP4_MMC4_BASE;
> -                       irq = OMAP44XX_IRQ_MMC4;
> -                       break;
> -               case 4:
> -                       if (!cpu_is_omap44xx())
> -                               return;
> -                       base = OMAP4_MMC5_BASE;
> -                       irq = OMAP44XX_IRQ_MMC5;
> -                       break;
> -               default:
> -                       continue;
> -               }
> +       if (!hsmmc_data[mmc_num]) {
> +               mmc_num++;
> +               return 0;
> +       }
> +       if (cpu_is_omap2420()) {
> +               name = "mmci-omap";
> +       } else {
> +               name = "mmci-omap-hs";
> +               ohl = omap_hsmmc_latency;
> +               ohl_cnt = ARRAY_SIZE(omap_hsmmc_latency);
> +       }
>
> -               if (cpu_is_omap2420()) {
> -                       size = OMAP2420_MMC_SIZE;
> -                       name = "mmci-omap";
> -               } else if (cpu_is_omap44xx()) {
> -                       if (i < 3)
> -                               irq += OMAP44XX_IRQ_GIC_START;
> -                       size = OMAP4_HSMMC_SIZE;
> -                       name = "mmci-omap-hs";
> -               } else {
> -                       size = OMAP3_HSMMC_SIZE;
> -                       name = "mmci-omap-hs";
> -               }
> -               omap_mmc_add(name, i, base, size, irq, mmc_data[i]);
> -       };
> +       hsmmc_data[mmc_num]->dev_attr = oh->dev_attr;

Copy the data from oh->dev_attr instead of relying on oh pointer
(don't rely on internals of hwmod data structures in the driver).

> +       omap2_mmc_mux(hsmmc_data[mmc_num], mmc_num);
> +       od = omap_device_build(name, mmc_num, oh, hsmmc_data[mmc_num],
> +               sizeof(struct omap_mmc_platform_data), ohl, ohl_cnt, false);
> +       WARN(IS_ERR(od), "Could not build omap_device for %s %s\n",
> +                                                       name, oh->name);
> +       /*
> +        * return device handle to board setup code
> +        * required to populate for regulator framework structure
> +        */
> +       hsmmc_data[mmc_num]->dev = &od->pdev.dev;

If od is incorrect, do a WARN() and return error. Don't access od
in case of a failure.

> +       mmc_num++;
> +       return 0;
>  }
>

<<snip>>

> diff --git a/arch/arm/plat-omap/include/plat/mmc.h b/arch/arm/plat-omap/include/plat/mmc.h
> index 7821344..08b7503 100644
> --- a/arch/arm/plat-omap/include/plat/mmc.h
> +++ b/arch/arm/plat-omap/include/plat/mmc.h
> @@ -16,6 +16,7 @@
>  #include <linux/mmc/host.h>
>
>  #include <plat/board.h>
> +#include <plat/omap_hwmod.h>
>
>  #define OMAP15XX_NR_MMC                1
>  #define OMAP16XX_NR_MMC                2
> @@ -23,23 +24,8 @@
>  #define OMAP1_MMC1_BASE                0xfffb7800
>  #define OMAP1_MMC2_BASE                0xfffb7c00      /* omap16xx only */

These need not be part of plat/mmc.h and
can be moved to mach-omap1/*

>
> -#define OMAP24XX_NR_MMC                2
> -#define OMAP34XX_NR_MMC                3
>  #define OMAP44XX_NR_MMC                5

Shouldn't OMAP44XX_NR_MMC  should also be
removed? I think this info should be populated
dynamically during omap_hwmod_for_each_by_class().

> -#define OMAP2420_MMC_SIZE      OMAP1_MMC_SIZE
> -#define OMAP3_HSMMC_SIZE       0x200
> -#define OMAP4_HSMMC_SIZE       0x1000
> -#define OMAP2_MMC1_BASE                0x4809c000
> -#define OMAP2_MMC2_BASE                0x480b4000
> -#define OMAP3_MMC3_BASE                0x480ad000
> -#define OMAP4_MMC4_BASE                0x480d1000
> -#define OMAP4_MMC5_BASE                0x480d5000
>  #define OMAP4_MMC_REG_OFFSET   0x100
> -#define HSMMC5                 (1 << 4)
> -#define HSMMC4                 (1 << 3)
> -#define HSMMC3                 (1 << 2)
> -#define HSMMC2                 (1 << 1)
> -#define HSMMC1                 (1 << 0)
>
>  #define OMAP_MMC_MAX_SLOTS     2
>
> @@ -78,6 +64,9 @@ struct omap_mmc_platform_data {
>
>        u64 dma_mask;
>
> +       /* Integrating attributes from the omap_hwmod layer */
> +       struct mmc_dev_attr *dev_attr;
> +
>        /* Register offset deviation */
>        u16 reg_offset;
>
> @@ -164,25 +153,18 @@ extern void omap_mmc_notify_cover_event(struct device *dev, int slot,
>
>  #if    defined(CONFIG_MMC_OMAP) || defined(CONFIG_MMC_OMAP_MODULE) || \
>        defined(CONFIG_MMC_OMAP_HS) || defined(CONFIG_MMC_OMAP_HS_MODULE)
> +
> +extern struct omap_mmc_platform_data *hsmmc_data[OMAP44XX_NR_MMC];

Same again. OMAP44XX_NR_MMC shall be removed and
instead be populated dynamically.

> +
>  void omap1_init_mmc(struct omap_mmc_platform_data **mmc_data,
>                                int nr_controllers);

<<snip>>