From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jason <u-boot@lakedaemon.net>
Date: Tue, 13 Sep 2011 09:53:37 -0400
Subject: [U-Boot] [PATCH v7] dreamplug: initial board support.
In-Reply-To: <4E6F0719.5050500@compulab.co.il>
References: <1315779002-22788-1-git-send-email-u-boot@lakedaemon.net>
	<1315827911-9634-1-git-send-email-u-boot@lakedaemon.net>
	<4E6F0719.5050500@compulab.co.il>
Message-ID: <20110913135337.GA24172@titan.lakedaemon.net>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Tue, Sep 13, 2011 at 10:32:41AM +0300, Igor Grinberg wrote:
> On 09/12/11 14:45, Jason Cooper wrote:
> > Copied wholeheartedly from board/Marvell/guruplug and modified to add support
> > for SPI NOR flash.
> > 
> > CONFIG_MACH_DREAMPLUG defined in include/configs/dreamplug.h until Linus's
> > kernel.org tree adds it to mach-types.h.  Once it trickles down, the definition
> > can be removed from dreamplug.h.
> > 
> > Signed-off-by: Jason Cooper <u-boot@lakedaemon.net>
> > ---
> 
> [...]
> 
> > Changes from v6 to v7:
> > 
> > 	- Use CONFIG_MACH_TYPE in dreamplug.c and define it to
> > 	  MACH_TYPE_DREAMPLUG in include/configs/dreamplug.h
> 
> Sorry, but you've got me wrong...
> All you need to do is define the CONFIG_MACH_TYPE in your config file.
> The rest will be taken care of by the common code.
> See below for details.

Okay, thanks for the assist, v8 on it's way.

> 
> [...]
> 
> > diff --git a/board/Marvell/dreamplug/dreamplug.c b/board/Marvell/dreamplug/dreamplug.c
> > new file mode 100644
> > index 0000000..05098fa
> > --- /dev/null
> > +++ b/board/Marvell/dreamplug/dreamplug.c
> 
> [...]
> 
> > +
> > +int board_init(void)
> > +{
> > +	/* arch number of board */
> > +	gd->bd->bi_arch_number = CONFIG_MACH_TYPE;
> 
> the above two line should be removed.
> 
> [...]
> 
> > diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h
> > new file mode 100644
> > index 0000000..a7168bd
> > --- /dev/null
> > +++ b/include/configs/dreamplug.h
> 
> [...]
> 
> > +
> > +/*
> > + * FIXME: This belongs in mach-types.h.  However, we only pull mach-types
> > + * from Linus' kernel.org tree.  This hasn't been updated primarily due to
> > + * the recent arch/arm reshuffling.  So, in the meantime, we'll place it
> > + * here.
> > + */
> > +#define MACH_TYPE_DREAMPLUG            3550
> > +
> > +#ifdef CONFIG_MACH_DREAMPLUG
> > +# ifdef machine_arch_type
> > +#  undef machine_arch_type
> > +#  define machine_arch_type	__machine_arch_type
> > +# else
> > +#  define machine_arch_type	MACH_TYPE_DREAMPLUG
> > +# endif
> > +# define machine_is_dreamplug()	(machine_arch_type == MACH_TYPE_DREAMPLUG)
> > +#else
> > +# define machine_is_dreamplug()	(0)
> > +#endif
> 
> Do you actually use the machine_is_dreamplug() macro in your code?
> If you don't, then you don't need the above block at all.
> 
> > +
> > +/*
> > + * Version number information
> > + */
> > +#define CONFIG_IDENT_STRING	"\nMarvell-DreamPlug"
> > +
> > +/*
> > + * High Level Configuration Options (easy to change)
> > + */
> > +#define CONFIG_SHEEVA_88SV131	1	/* CPU Core subversion */
> > +#define CONFIG_KIRKWOOD		1	/* SOC Family Name */
> > +#define CONFIG_KW88F6281	1	/* SOC Name */
> > +#define CONFIG_MACH_TYPE	MACH_TYPE_DREAMPLUG
> 
> The above line is all what you need to setup the machine id
> (along with MACH_TYPE_DREAMPLUG definition of course,
> unless it is in mach_type file).
> 
> > +#define CONFIG_MACH_DREAMPLUG		/* Machine type */
> 
> shouldn't the above be defined before the machine_is_... block?
> If you don't need the machine_is_dreamplug() macro, then you also
> don't need the above line.
> 
> [...]
> 
> 
> -- 
> Regards,
> Igor.