> -----Original Message-----
> From: Wolfgang Denk [mailto:w...@denx.de]
> Sent: Monday, May 31, 2010 3:11 PM
> To: Hiremath, Vaibhav
> Cc: u-boot@lists.denx.de; t...@bumblecow.com; Paulraj, Sandeep; Premi,
> Sanjeev
> Subject: Re: [PATCH-V3 1/2] AM35x: Add support for AM3517EVM
>
> Dear hvaib...@ti.com,
>
> In message <1273166585-26101-1-git-send-email-hvaib...@ti.com> you wrote:
> > From: Vaibhav Hiremath <hvaib...@ti.com>
> >
> > This patch adds basic support for the AM3517EVM.
> > It includes:
> > - Board int file (.c and .h)
> > - Default configuration file
> > - Updates for Makefile
> >
> > Changes from V2:
> > - Removed trailing spaces
> > - Updated MAINTAINERS & MAKEALL for am3517_evm
>
> Such comments do not belong into the commit message. Please place thes
> ebelow the "---" line:
>
[Hiremath, Vaibhav] Ok will incorporate in next post.
> > Signed-off-by: Vaibhav Hiremath <hvaib...@ti.com>
> > Signed-off-by: Sanjeev Premi <pr...@ti.com>
> > ---
>
> ==> Comments should go here.
>
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 5cbc845..0bc65e1 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -798,6 +798,10 @@ Alex Z
> > lart SA1100
> > dnp1110 SA1110
> >
> > +Vaibhav Hiremath <hvaib...@ti.com>
> > +
> > + am3517_evm ARM CORTEX-A8 (AM35x SoC)
> > +
>
> Please keep list sorted.
>
[Hiremath, Vaibhav] How does this sorted, I could not see any relation between
the entries there.
> ...
> > diff --git a/arch/arm/include/asm/arch-omap3/mux.h
> b/arch/arm/include/asm/arch-omap3/mux.h
> > index 0c01c73..ffeb982 100644
> > --- a/arch/arm/include/asm/arch-omap3/mux.h
> > +++ b/arch/arm/include/asm/arch-omap3/mux.h
> ...
> > +/* AM3517 specific */
> > +#define CONTROL_PADCONF_CCDC_PCLK 0x01E4
> > +#define CONTROL_PADCONF_CCDC_FIELD 0x01E6
>
> Board specific defoinitions should not be added to global header
> files. Please use a board specific header instead.
>
[Hiremath, Vaibhav] I think this has been placed at the right place. This is
mux definition and got added to mux.h file. You could see all mux definition
are present in this file.
Do you see any issues with this?
> > --- /dev/null
> > +++ b/board/logicpd/am3517evm/am3517evm.c
> > @@ -0,0 +1,76 @@
> ...
> > +int board_init(void)
> > +{
> > + DECLARE_GLOBAL_DATA_PTR;
>
> This is bound to break. DECLARE_GLOBAL_DATA_PTR must always be used on
> file scope only; never use this on function scope.
[Hiremath, Vaibhav] Agreed. Actually this code is derived form board/ti/evm/,
so followed the same here.
I will change it in next post.
> Please check all
> your code.
>
[Hiremath, Vaibhav] Do you see any other issues? I don't get this statement.
Thanks,
Vaibhav
>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
> Genius doesn't work on an assembly line basis. You can't simply say,
> "Today I will be brilliant."
> -- Kirk, "The Ultimate Computer", stardate 4731.3
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
