public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Peter Tyser <ptyser@xes-inc.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v3] Adding support for DevKit8000
Date: Thu, 20 Aug 2009 12:02:16 -0500	[thread overview]
Message-ID: <1250787736.2789.23.camel@localhost.localdomain> (raw)
In-Reply-To: <1250786842-9134-1-git-send-email-frederik@kriewitz.eu>

Hi Frederik,
I had some minor aesthetic nitpicks.  I'd change the title to "Add
support for the DevKit8000 board".

<snip>

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 620604c..03b2d10 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -706,6 +706,10 @@ Alex Z
>  	lart		SA1100
>  	dnp1110		SA1110
>  
> +Frederik Kriewitz <frederik@kriewitz.eu>
> +
> +	devkit8000	ARM CORTEX-A8 (OMAP3530 SoC)
> +

You should maintain the alphabetical order of MAINTAINERS when adding
yourself.

>  -------------------------------------------------------------------------
>  
>  Unknown / orphaned boards:
> diff --git a/MAKEALL b/MAKEALL
> index edebaea..34235b7 100755
> --- a/MAKEALL
> +++ b/MAKEALL
> @@ -581,6 +581,7 @@ LIST_ARM_CORTEX_A8="		\
>  	omap3_pandora		\
>  	omap3_zoom1		\
>  	omap3_zoom2		\
> +	devkit8000	\
>  "

You should maintain the alphabetical order of LIST_ARM_CORTEX_A8.

<snip>

> +/*-----------------------------------------------------------------------
> + * Stack sizes
> + *
> + * The stack sizes are set up in start.S using the settings below
> + */

Other's might disagree, but I think the "----" in the comments above are
not necessary/non-standard.  I'd personally use:

/*
 * Stack sizes
 *
 * The stack sizes are set up in start.S using the settings below
 */

Or just:
/* The stack sizes are set up in start.S using the settings below */

> +#define CONFIG_STACKSIZE	SZ_128K	/* regular stack */
> +#ifdef CONFIG_USE_IRQ
> +#define CONFIG_STACKSIZE_IRQ	SZ_4K	/* IRQ stack */
> +#define CONFIG_STACKSIZE_FIQ	SZ_4K	/* FIQ stack */
> +#endif
> +
> +/*-----------------------------------------------------------------------
> + * Physical Memory Map
> + */
> +#define CONFIG_NR_DRAM_BANKS	2	/* CS1 may or may not be populated */
> +#define PHYS_SDRAM_1		OMAP34XX_SDRC_CS0
> +#define PHYS_SDRAM_1_SIZE	SZ_128M	/* at least 128 meg */
> +#define PHYS_SDRAM_2		OMAP34XX_SDRC_CS1
> +
> +/* SDRAM Bank Allocation method */
> +#define SDRC_R_B_C		1
> +
> +/*-----------------------------------------------------------------------
> + * FLASH and environment organization
> + */
> +
> +/* **** PISMO SUPPORT *** */

You should use a standard comment style for "PISMO SUPPORT", eg less *'s
and standard capitalization.

> +
> +/* Configure the PISMO */

Maybe get rid of the above comment too - its pretty clear that you're
configuring the PISMO based on the "PISMO SUPPORT" comment above and the
define name.

> +#define PISMO1_NAND_SIZE		GPMC_SIZE_128M

Best,
Peter

  reply	other threads:[~2009-08-20 17:02 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-20 16:47 [U-Boot] [PATCH v3] Adding support for DevKit8000 Frederik Kriewitz
2009-08-20 17:02 ` Peter Tyser [this message]
2009-08-20 17:28   ` Dirk Behme
2009-08-20 18:37     ` Frederik Kriewitz
2009-08-20 20:20       ` Jean-Christophe PLAGNIOL-VILLARD
2009-08-21 13:00       ` Dirk Behme
2009-08-21 14:34         ` Peter Tyser
2009-08-21 15:08           ` [U-Boot] Rules for board/* directory, was: " Dirk Behme
2009-08-21 15:22             ` Detlev Zundel
2009-08-21 15:41               ` Dirk Behme
2009-08-21 16:04                 ` Detlev Zundel
2009-08-21 18:07                 ` Wolfgang Denk
2009-08-21 17:59               ` Wolfgang Denk
2009-08-21 23:27                 ` Frederik Kriewitz
2009-08-22  8:14                   ` Wolfgang Denk
2009-08-21 15:28             ` Peter Tyser

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1250787736.2789.23.camel@localhost.localdomain \
    --to=ptyser@xes-inc.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox