[U-Boot] [PATCH-V3 1/2] AM35x: Add support for AM3517EVM

[Hiremath, Vaibhav]

> -----Original Message-----
> From: Hiremath, Vaibhav
> Sent: Thursday, June 03, 2010 10:57 PM
> To: 'Wolfgang Denk'
> Cc: u-boot at lists.denx.de; tom at bumblecow.com; Paulraj, Sandeep; Premi,
> Sanjeev
> Subject: RE: [PATCH-V3 1/2] AM35x: Add support for AM3517EVM
> 
[Hiremath, Vaibhav] Denk,

Can you please reply to below queries of mine, so that I can incorporate the changes and submit it again?

Thanks,
Vaibhav

> 
> > -----Original Message-----
> > From: Wolfgang Denk [mailto:wd at denx.de]
> > Sent: Monday, May 31, 2010 3:11 PM
> > To: Hiremath, Vaibhav
> > Cc: u-boot at lists.denx.de; tom at bumblecow.com; Paulraj, Sandeep; Premi,
> > Sanjeev
> > Subject: Re: [PATCH-V3 1/2] AM35x: Add support for AM3517EVM
> >
> > Dear hvaibhav at ti.com,
> >
> > In message <1273166585-26101-1-git-send-email-hvaibhav at ti.com> you wrote:
> > > From: Vaibhav Hiremath <hvaibhav at 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 <hvaibhav at ti.com>
> > > Signed-off-by: Sanjeev Premi <premi at 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 <hvaibhav at 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.
> 
[Hiremath, Vaibhav] Denk,
> > ...
> > > 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: wd at 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