All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.