Peter Tyser said the following on 09/19/2009 09:34 AM: thanks for your review
Hi Nishanth,
On Fri, 2009-09-18 at 21:21 -0500, Nishanth Menon wrote:
From: David Brownell david-b@pacbell.net
Start of SDP3430 support in "mainline" u-boot mainline code
Original Patch written by David Brownell
I don't think the above comments are necessary. David will be credited with authorship already, and the subject line and text below make it clear what this patch does.
Ack..
create mode 100644 board/ti/sdp3430/sdp.h create mode 100644 board/ti/sdp3430/u-boot.lds create mode 100644 include/configs/omap3_sdp.h
The board config header file should be renamed to sdp.h from omap3_sdp.h. There was a recent thread discussing this convention "ARM Pull Request" around Sept 6.
sdp3430 - there are many software development platforms -> omap2420 based, omap2430 based etc.. Thanks for pointing this chain out.. a specific link describing the thought will help and prevent me misunderstanding the intention here.
diff --git a/MAINTAINERS b/MAINTAINERS index e9db278..adc8a63 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -619,6 +619,7 @@ Guennadi Liakhovetski g.liakhovetski@gmx.de Nishanth Menon nm@ti.com
omap3_zoom1 ARM CORTEX-A8 (OMAP3xx SoC)
- omap3_sdp ARM CORTEX-A8 (OMAP3xx SoC)
You may as well keep the boards ordered alphabetically (and remove the omap_ prefix from sdp).
ack to alphabetical sort.
+/******************************************************************************
- Routine: board_init
- Description: Early hardware init.
- *****************************************************************************/
+int board_init(void) +{
- DECLARE_GLOBAL_DATA_PTR;
I'd use the preferred multi-line comment style: /*
*/
There are lots of other non-preferred multi-line comments throughout the patch as well.
ack.
I personally don't think its necessary to put "Routine: <name>" stuff for each function either. It doesn't add any benefit, adds cruft to grep output, and might get out of sync with the real function name at some point. If it were me, I would get rid of "Description: " text too. Its pretty obvious that the text "Early hardware init" is a description of the function.
not to all.. I dont like it either, I would rather go doxygen style.. will convert.
Regards, Nishanth Menon