Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandre Belloni <abelloni@adeneo-embedded.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 1/2] lpc32xx: Add CDL to the available bootloaders
Date: Wed, 04 Jan 2012 12:19:57 +0100	[thread overview]
Message-ID: <1325675997.2442.25.camel@emblysd024.adetel.com> (raw)
In-Reply-To: <20120103210841.7a5ff77a@skate>

On Tue, 2012-01-03 at 21:08 +0100, Thomas Petazzoni wrote:
> Le Tue,  3 Jan 2012 19:49:27 +0100,
> Alexandre Belloni <abelloni@adeneo-embedded.com> a ?crit :
> 
> > +config BR2_TARGET_LPC32XXCDL
> > +    depends on BR2_arm
> > +    bool "LPC32XX CDL (kickstart and S1L)"
> 
> Indentation must be one tab.
> 

OK


> > +++ b/boot/lpc32xxcdl/lpc32xxcdl-2.11-delete_redundant_files.patch
> 
> Is it really needed to carry this patch in Buildroot? It's probably a
> nice cleanup to be submitted upstream, but from a buildability point of
> view, it has no impact I guess.
> 

It won't compile without it because the files are duplicate, the linker
will complain about symbols being defined twice. I definitely tried to
keep the patch as small as possible.

> > diff --git a/boot/lpc32xxcdl/lpc32xxcdl-2.11-libnosys_gnu.patch b/boot/lpc32xxcdl/lpc32xxcdl-2.11-libnosys_gnu.patch
> > new file mode 100644
> > index 0000000..fc8af0a
> > --- /dev/null
> > +++ b/boot/lpc32xxcdl/lpc32xxcdl-2.11-libnosys_gnu.patch
> > @@ -0,0 +1,183 @@
> > +Fix compilation and eabi issues
> 
> > +
> > +Signed-off-by: Alexandre Belloni <abelloni@adeneo-embedded.com>
> > +---
> > + csps/lpc32xx/bsps/ea3250/source/libnosys_gnu.c  |   41 +++++++++++++++++++++++
> > + csps/lpc32xx/bsps/fdi3250/source/libnosys_gnu.c |   41 +++++++++++++++++++++++
> > + csps/lpc32xx/bsps/phy3250/source/libnosys_gnu.c |   41 +++++++++++++++++++++++
> 
> Is there a way to somehow share those instead of duplicating them
> per board?
> 

Th fact is that they are already duplicated in the original source. Is
it worth patching the makefiles to unify those ?

Upstream is trying to unify everything. I hope t will be done at some
point in time.

> > diff --git a/boot/lpc32xxcdl/lpc32xxcdl.mk b/boot/lpc32xxcdl/lpc32xxcdl.mk
> > new file mode 100644
> > index 0000000..cd8e76d
> > --- /dev/null
> > +++ b/boot/lpc32xxcdl/lpc32xxcdl.mk
> > @@ -0,0 +1,54 @@
> > +#############################################################
> > +#
> > +# DataFlashBoot
> 
> Please fix this.
> 

Right.

> > +#
> > +#############################################################
> > +
> > +#LPC32XXCDL_VERSION = 2.01
> > +#LPC32XXCDL_SOURCE = lpc32xx_cdl.zip
> > +#LPC32XXCDL_SITE = http://ics.nxp.com/support/software/lpc32xx.cdl.drivers/zip/
> 
> Please remove those comments.
> 

OK

> > +LPC32XXCDL_VERSION = lpc32xx_cdl_v2.11
> > +LPC32XXCDL_SITE = http://git.lpcware.com/lpc3xxx_cdl.git
> > +LPC32XXCDL_SITE_METHOD = git
> > +
> > +LPC32XXCDL_INSTALL_TARGET = NO
> > +LPC32XXCDL_INSTALL_IMAGES = YES
> > +
> > +ifeq ($(BR2_TARGET_LPC32XXCDL_BOARDNAME),"ea3250")
> > +LPC32XXCDL_KICKSTART = kickstart/nand
> > +LPC32XXCDL_KICKSTART_BURNER = nand/kickstart
> > +LPC32XXCDL_S1L = s1l
> > +LPC32XXCDL_S1L_BURNER = nand/s1lapp
> > +endif
> > +
> > +ifeq ($(BR2_TARGET_LPC32XXCDL_BOARDNAME),"phy3250")
> > +LPC32XXCDL_KICKSTART = kickstart/kickstart_nand
> > +LPC32XXCDL_KICKSTART_BURNER = nand/kickstart
> > +LPC32XXCDL_S1L = s1l/s1l_nand_boot
> > +LPC32XXCDL_S1L_BURNER = nand/s1lapp
> > +endif
> > +
> > +ifeq ($(BR2_TARGET_LPC32XXCDL_BOARDNAME),"fdi3250")
> > +LPC32XXCDL_KICKSTART = kickstart/nand
> > +LPC32XXCDL_KICKSTART_BURNER = nand/kickstart_jtag
> > +LPC32XXCDL_S1L = s1l
> > +LPC32XXCDL_S1L_BURNER = nand/s1lapp_jtag
> > +endif
> > +
> > +define LPC32XXCDL_BUILD_CMDS
> > +	make -C $(@D) CROSS_COMPILE=$(TARGET_CROSS) NXPMCU_WINBASE=$(@D) NXPMCU_SOFTWARE=$(@D) BSP=$(BR2_TARGET_LPC32XXCDL_BOARDNAME) CSP=lpc32xx TOOL=gnu GEN=lpc
> > +    make -C $(@D)/csps/lpc32xx/bsps/$(BR2_TARGET_LPC32XXCDL_BOARDNAME)/startup/examples/Burners/$(LPC32XXCDL_KICKSTART_BURNER) CROSS_COMPILE=$(TARGET_CROSS) NXPMCU_WINBASE=$(@D) NXPMCU_SOFTWARE=$(@D) BSP=$(BR2_TARGET_LPC32XXCDL_BOARDNAME) CSP=lpc32xx TOOL=gnu GEN=lpc
> > +    make -C $(@D)/csps/lpc32xx/bsps/$(BR2_TARGET_LPC32XXCDL_BOARDNAME)/startup/examples/$(LPC32XXCDL_KICKSTART) CROSS_COMPILE=$(TARGET_CROSS) NXPMCU_WINBASE=$(@D) NXPMCU_SOFTWARE=$(@D) BSP=$(BR2_TARGET_LPC32XXCDL_BOARDNAME) CSP=lpc32xx TOOL=gnu GEN=lpc
> > +    make -C $(@D)/csps/lpc32xx/bsps/$(BR2_TARGET_LPC32XXCDL_BOARDNAME)/startup/examples/Burners/$(LPC32XXCDL_S1L_BURNER) CROSS_COMPILE=$(TARGET_CROSS) NXPMCU_WINBASE=$(@D) NXPMCU_SOFTWARE=$(@D) BSP=$(BR2_TARGET_LPC32XXCDL_BOARDNAME) CSP=lpc32xx TOOL=gnu GEN=lpc
> > +    make -C $(@D)/csps/lpc32xx/bsps/$(BR2_TARGET_LPC32XXCDL_BOARDNAME)/startup/examples/$(LPC32XXCDL_S1L) CROSS_COMPILE=$(TARGET_CROSS) NXPMCU_WINBASE=$(@D) NXPMCU_SOFTWARE=$(@D) BSP=$(BR2_TARGET_LPC32XXCDL_BOARDNAME) CSP=lpc32xx TOOL=gnu GEN=lpc
> > +endef
> 
> Hum, that's ugly.
> 
> What about something like:
> 
> LPC32XXCDL_BUILD_FLAGS = \
> 	CROSS_COMPILE=$(TARGET_CROSS) 		\
> 	NXPMCU_WINBASE=$(@D) 			\
> 	NXPMCU_SOFTWARE=$(@D)			\
> 	BSP=$(BR2_TARGET_LPC32XXCDL_BOARDNAME)	\
> 	CSP=lpc32xx TOOL=gnu GEN=lpc
> 
> LPC32XXCDL_BOARD_STARTUP_DIR = \
> 	csps/lpc32xx/bsps/$(BR2_TARGET_LPC32XXCDL_BOARDNAME)/startup/examples/
> 
> And then:
> 
> define LPC32XXCDL_BUILD_CMDS
> 	$(MAKE) $(LPC32XXCDL_BUILD_FLAGS) -C $(@D) 
> 	$(MAKE) $(LPC32XXCDL_BUILD_FLAGS) -C $(@D)/$(LPC32XXCDL_BOARD_STARTUP_DIR)/$(LPC32XXCDL_KICKSTART)
> 	$(MAKE) $(LPC32XXCDL_BUILD_FLAGS) -C $(@D)/$(LPC32XXCDL_BOARD_STARTUP_DIR)/Burners/$(LPC32XXCDL_S1L_BURNER)
> 	$(MAKE) $(LPC32XXCDL_BUILD_FLAGS) -C $(@D)/$(LPC32XXCDL_BOARD_STARTUP_DIR)/$(LPC32XXCDL_S1L)
> endef
> 
> would probably be a bit more readable (still not perfectly nice, though).
> 

Will be done but $(MAKE) won't work as it isn't possible to compile
using multiple jobs. Is it worth fixing the makefiles for a so small
piece of code ?

-- 
Alexandre Belloni

Adeneo Embedded
Adetel Group

2, ch. du Ruisseau - 69134 Ecully, France

www.adeneo-embedded.com
 

  reply	other threads:[~2012-01-04 11:19 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-03 18:49 [Buildroot] [PATCH 0/2] V2: Add defconfigs for LPC3250 boards Alexandre Belloni
2012-01-03 18:49 ` [Buildroot] [PATCH 1/2] lpc32xx: Add CDL to the available bootloaders Alexandre Belloni
2012-01-03 20:08   ` Thomas Petazzoni
2012-01-04 11:19     ` Alexandre Belloni [this message]
2012-01-04 12:39       ` Thomas Petazzoni
2012-01-03 18:49 ` [Buildroot] [PATCH 2/2] Add defconfigs for LPC3250 boards Alexandre Belloni
2012-01-03 20:09 ` [Buildroot] [PATCH 0/2] V2: " Thomas Petazzoni
2012-01-04 11:12   ` Alexandre Belloni

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=1325675997.2442.25.camel@emblysd024.adetel.com \
    --to=abelloni@adeneo-embedded.com \
    --cc=buildroot@busybox.net \
    /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