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

Premi, Sanjeev
2011-01-20T10:58:20Z

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

~sanjeev

[snip]...[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