Re: [PATCH] omap3: Add basic support for 720MHz part

Varadarajan, Charulatha
2011-01-18T09:15:06Z

Couple of minor comments.

On Tue, Jan 18, 2011 at 13:19, Sanjeev Premi <premi@ti.com> wrote:
> This patch adds support for new speed enhanced parts with ARM
> and IVA running at 720MHz and 520MHz respectively. These parts
> can be probed at run-time by reading PRODID.SKUID[3:0] at
> 0x4830A20C [1].
>
> This patch specifically does following:
>  * Detect devices capable of 720MHz.
>  * Add new OPP
>  * Ensure that OPP is conditionally enabled.
>  * Check for presence of IVA before attempting to enable
>   the corresponding OPP.
>
>  [1] http://focus.ti.com/lit/ug/spruff1d/spruff1d.pdf
>
> Signed-off-by: Sanjeev Premi <premi@ti.com>
> ---
> Since v2:
> 1) pr_xxx() -> dev_xxx() functions - suggested by
>   Manjunath (manjugk@ti.com)
> 2) Add check for presense of IVA - earlier planned to
>   be in a separate patch; but we multiple discussions
>   on optimizations.
> 3) Do look-up for hwmod corresponding for iva only if
>   iva is present. Should save multiple strcmp()s in
>   _lookup().
>
> Since v1:
> 1) Using opp_enable() to enable the OPP after the OPP
>   table has been initialized.
> 2) Starting at 3 levels of indent, the statements had
>   be broken into multiple lines for most of the code.
>   So, opted to create a new static that enables the
>   OPPs corresponding to 720MHz.
> 3) I have only build tested this patch - will be able
>   to confirm working tomorrow. With any further change,
>   if needed. (However, functionally nothing has changed.)
>
>
>  arch/arm/mach-omap2/control.h         |    7 ++++
>  arch/arm/mach-omap2/id.c              |   10 +++++
>  arch/arm/mach-omap2/opp3xxx_data.c    |   63 ++++++++++++++++++++++++++++++++-
>  arch/arm/plat-omap/include/plat/cpu.h |    2 +
>  4 files changed, 81 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
> index f0629ae..eebc045 100644
> --- a/arch/arm/mach-omap2/control.h
> +++ b/arch/arm/mach-omap2/control.h
> @@ -365,6 +365,13 @@
>  #define                FEAT_NEON               0
>  #define                FEAT_NEON_NONE          1
>
> +/*
> + * Product ID register
> + */

Do not use multiline comment style for single line comments

> +#define OMAP3_PRODID                   0x020C
> +
> +#define OMAP3_SKUID_MASK               0x0f
> +#define                OMAP3_SKUID_720MHZ      0x08
>
>  #ifndef __ASSEMBLY__
>  #ifdef CONFIG_ARCH_OMAP2PLUS
> diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
> index 5f9086c..53fbe01 100644
> --- a/arch/arm/mach-omap2/id.c
> +++ b/arch/arm/mach-omap2/id.c
> @@ -195,6 +195,15 @@ static void __init omap3_check_features(void)
>         * TODO: Get additional info (where applicable)
>         *       e.g. Size of L2 cache.
>         */
> +
> +       /*
> +        * Does it support 720MHz?
> +        */

Ditto

> +       status = (OMAP3_SKUID_MASK & read_tap_reg(OMAP3_PRODID));
> +
> +       if (status & OMAP3_SKUID_720MHZ) {
> +               omap3_features |= OMAP3_HAS_720MHZ;
> +       }

No need of { } as there is only one line statement inside the if condition.

>  }
>
>  static void __init omap3_check_revision(void)
> @@ -445,6 +454,7 @@ static void __init omap3_cpuinfo(void)
>        OMAP3_SHOW_FEATURE(neon);
>        OMAP3_SHOW_FEATURE(isp);
>        OMAP3_SHOW_FEATURE(192mhz_clk);
> +       OMAP3_SHOW_FEATURE(720mhz);
>
>        printk(")\n");
>  }

<<snip>>
--
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