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

["G, Manjunath Kondaiah" <manjugk@ti.com>]

On Thu, Jan 20, 2011 at 04:28:11PM +0530, Premi, Sanjeev wrote:
> > -----Original Message-----
> > From: Varadarajan, Charulatha 
> > Sent: Tuesday, January 18, 2011 2:44 PM
> > To: Premi, Sanjeev
> > Cc: linux-omap@vger.kernel.org
> > Subject: Re: [PATCH] omap3: Add basic support for 720MHz part
> > 
> > Couple of minor comments.
> 
> [snip]...[snip]
> 
> > > --- 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
> 
> [sp] Was only following the multi-line comment stye few lines
>      above...
> 
> > 
> > > +#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
> 
> [sp] Same here.
>      But since the code is quite evident, I will remove the comment
>      altogether.
> 
> > 
> > > +       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.
> 
> [sp] missed this.

Looks fine. Feel free to add after the above changes:

Reviewed-by: G, Manjunath Kondaiah <manjugk@ti.com>

-Manjunath

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