[PATCH/RFC] Add cmd_wrap_dts rules to arch/powerpc/boot/Makefile

[PATCH/RFC] Add cmd_wrap_dts rules to arch/powerpc/boot/Makefile

[Mark A. Greer]

Paul, et. al.,

Currently, platforms that don't have a dt-aware firmware need to
manually run the wrapper script to wrap a dtb into the zImage that
was just built.  This is a PITA and is guaranteed to mess up people
who just want to build a zImage and have it work.

To address this, I added a couple new rules to arch/powerpc/boot/Makefile
so that the dtb is automatically wrapped for "reference" platforms.
"reference" meaning a platform that has its dts in
arch/powerpc/boot/dts/<platform>.dts.  It won't work for non-reference
platforms.

The patch is attached below with a header in case it is acceptable, as
is.

This would be used by the sandpoint, say, by adding the following rules
to the Makefile in the appropriate places:

	$(obj)/zImage.sandpoint: vmlinux $(wrapperbits)
		$(call cmd,wrap_dts,sandpoint)

	$(obj)/zImage.initrd.sandpoint: vmlinux $(wrapperbits)
		$(call cmd,wrap_dts_initrd,sandpoint)

and:

	image-$(CONFIG_SANDPOINT)               += zImage.sandpoint

Comments?

Mark
---

[PATCH] Add cmd_wrap_dts rules to arch/powerpc/boot/Makefile

Add cmd_wrap_dts rules to bootwrapper Makefile so "make zImage" and
"make zImage.initrd" will wrap the platform's dtb in the zImage.
This eliminates the need to manually run the wrapper script a
second time just to add the dtb to the zImage.

This only works for reference platforms that have their dts files
in arch/powerpc/boot/dts/<platform>.dts.

Depends-on: http://ozlabs.org/pipermail/linuxppc-dev/2006-October/027003.html

Signed-off-by: Mark A. Greer <mgreer@mvista.com>
---

 Makefile |    7 +++++++
 1 files changed, 7 insertions(+)
---

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 4b2be61..3dc4e55 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -117,6 +117,13 @@ quiet_cmd_wrap	= WRAP    $@
 quiet_cmd_wrap_initrd = WRAP    $@
       cmd_wrap_initrd =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) \
 				-i $(obj)/ramdisk.image.gz vmlinux
+quiet_cmd_wrap_dts = WRAP_DTS    $@
+      cmd_wrap_dts =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) \
+				-s $(obj)/dts/$2.dts vmlinux
+quiet_cmd_wrap_dts_initrd = WRAP_DTS    $@
+      cmd_wrap_dts_initrd =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 \
+				$(CROSSWRAP) -s $(obj)/dts/$2.dts \
+				-i $(obj)/ramdisk.image.gz vmlinux
 
 $(obj)/zImage.chrp: vmlinux $(wrapperbits)
 	$(call cmd,wrap,chrp)

Re: [PATCH/RFC] Add cmd_wrap_dts rules to arch/powerpc/boot/Makefile

[Josh Boyer]

On Wed, 2006-10-18 at 17:26 -0700, Mark A. Greer wrote:
> Paul, et. al.,
> 
> Currently, platforms that don't have a dt-aware firmware need to
> manually run the wrapper script to wrap a dtb into the zImage that
> was just built.  This is a PITA and is guaranteed to mess up people
> who just want to build a zImage and have it work.
> 
> To address this, I added a couple new rules to arch/powerpc/boot/Makefile
> so that the dtb is automatically wrapped for "reference" platforms.
> "reference" meaning a platform that has its dts in
> arch/powerpc/boot/dts/<platform>.dts.  It won't work for non-reference
> platforms.
> 
> The patch is attached below with a header in case it is acceptable, as
> is.
> 
> This would be used by the sandpoint, say, by adding the following rules
> to the Makefile in the appropriate places:
> 
> 	$(obj)/zImage.sandpoint: vmlinux $(wrapperbits)
> 		$(call cmd,wrap_dts,sandpoint)
> 
> 	$(obj)/zImage.initrd.sandpoint: vmlinux $(wrapperbits)
> 		$(call cmd,wrap_dts_initrd,sandpoint)
> 
> and:
> 
> 	image-$(CONFIG_SANDPOINT)               += zImage.sandpoint
> 
> Comments?

I've been using a patch much like this while I work on getting my
platform up and running with the new bootwrapper patches.  It's very
useful.

josh

Re: [PATCH/RFC] Add cmd_wrap_dts rules to arch/powerpc/boot/Makefile

[Matthew McClintock]

On Wed, 2006-10-18 at 17:26 -0700, Mark A. Greer wrote:
> 
>         $(obj)/zImage.sandpoint: vmlinux $(wrapperbits)
>                 $(call cmd,wrap_dts,sandpoint)
> 
>         $(obj)/zImage.initrd.sandpoint: vmlinux $(wrapperbits)
>                 $(call cmd,wrap_dts_initrd,sandpoint) 


Can you do something like this to make it even more automatic:

        $(obj)/$(CONFIG_SANDPOINT): vmlinux $(wrapperbits)
                $(call cmd,wrap_dts,`echo $CONFIG_SANDPOINT | awk '{print substr($1, 8)}'`)

I'm not sure if this will work exactly as is. But, I do think adding an
entry in the Makefile for every possible bootwrapper target is needed. I
say this because I intend to add targets for all variants of Freescale
boards at some point in the future.

Let me clarify my needs here some more, I want to be able to type 'make
cuImage' and have the scripts build the wrapper target and attached the
correct dtb for all variants of Freescale boards. Of course, the dtb
will hopefully be specified in the defconfig. Does this make sense?

-Matthew

Re: [PATCH/RFC] Add cmd_wrap_dts rules to arch/powerpc/boot/Makefile

[Matthew McClintock]

On Thu, 2006-10-19 at 00:50 -0500, Matthew McClintock wrote:
>         $(obj)/$(CONFIG_SANDPOINT): vmlinux $(wrapperbits)
>                 $(call cmd,wrap_dts,`echo $CONFIG_SANDPOINT | awk
> '{print substr($1, 8)}'`) 

The $(CONFIG_SANDPOINT) bits should ideally be renamed to
$(CONFIG_TARGET) or something....

-Matthew

Re: [PATCH/RFC] Add cmd_wrap_dts rules to arch/powerpc/boot/Makefile

[Jon Loeliger]

So, like, the other day "Mark A. Greer" mumbled:
> 
> This would be used by the sandpoint, say, by adding the following rules
> to the Makefile in the appropriate places:
> 
> 	$(obj)/zImage.sandpoint: vmlinux $(wrapperbits)
> 		$(call cmd,wrap_dts,sandpoint)
> 
> 	$(obj)/zImage.initrd.sandpoint: vmlinux $(wrapperbits)
> 		$(call cmd,wrap_dts_initrd,sandpoint)
> 
> and:
> 
> 	image-$(CONFIG_SANDPOINT)               += zImage.sandpoint
> 
> Comments?

Isn't there a way to do this without hardcoding the
platform name in the makefiles directly?  I fear for
the one-per-platform effect that is yet to come with
the conversion of many of the legacy boards.


> +++ b/arch/powerpc/boot/Makefile
> @@ -117,6 +117,13 @@ quiet_cmd_wrap	= WRAP    $@
>  quiet_cmd_wrap_initrd = WRAP    $@
>        cmd_wrap_initrd =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) \
>  				-i $(obj)/ramdisk.image.gz vmlinux
> +quiet_cmd_wrap_dts = WRAP_DTS    $@
> +      cmd_wrap_dts =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) \
> +				-s $(obj)/dts/$2.dts vmlinux
> +quiet_cmd_wrap_dts_initrd = WRAP_DTS    $@
> +      cmd_wrap_dts_initrd =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 \
> +				$(CROSSWRAP) -s $(obj)/dts/$2.dts \
> +				-i $(obj)/ramdisk.image.gz vmlinux
>  
>  $(obj)/zImage.chrp: vmlinux $(wrapperbits)
>  	$(call cmd,wrap,chrp)

Which I guess means this patch is OK, but the zImage.chrp use
around it is suspect.... :-)

jdl

Re: [PATCH/RFC] Add cmd_wrap_dts rules to arch/powerpc/boot/Makefile

[Mark A. Greer]

On Thu, Oct 19, 2006 at 12:50:01AM -0500, Matthew McClintock wrote:
> On Wed, 2006-10-18 at 17:26 -0700, Mark A. Greer wrote:
> > 
> >         $(obj)/zImage.sandpoint: vmlinux $(wrapperbits)
> >                 $(call cmd,wrap_dts,sandpoint)
> > 
> >         $(obj)/zImage.initrd.sandpoint: vmlinux $(wrapperbits)
> >                 $(call cmd,wrap_dts_initrd,sandpoint) 
> 
> 
> Can you do something like this to make it even more automatic:
> 
>         $(obj)/$(CONFIG_SANDPOINT): vmlinux $(wrapperbits)
>                 $(call cmd,wrap_dts,`echo $CONFIG_SANDPOINT | awk '{print substr($1, 8)}'`)
> 
> I'm not sure if this will work exactly as is. But, I do think adding an
> entry in the Makefile for every possible bootwrapper target is needed. I
> say this because I intend to add targets for all variants of Freescale
> boards at some point in the future.
> 
> Let me clarify my needs here some more, I want to be able to type 'make
> cuImage' and have the scripts build the wrapper target and attached the
> correct dtb for all variants of Freescale boards. Of course, the dtb
> will hopefully be specified in the defconfig. Does this make sense?

Yes, something like that would reduce the number of rules that would
have to be added.  I'll think about it.  It'll be low on my priority
list though so feel free to submit your own patch.

Mark

Re: [PATCH/RFC] Add cmd_wrap_dts rules to arch/powerpc/boot/Makefile

[Mark A. Greer]

On Thu, Oct 19, 2006 at 09:03:18AM -0500, Jon Loeliger wrote:
> So, like, the other day "Mark A. Greer" mumbled:
> > 
> > This would be used by the sandpoint, say, by adding the following rules
> > to the Makefile in the appropriate places:
> > 
> > 	$(obj)/zImage.sandpoint: vmlinux $(wrapperbits)
> > 		$(call cmd,wrap_dts,sandpoint)
> > 
> > 	$(obj)/zImage.initrd.sandpoint: vmlinux $(wrapperbits)
> > 		$(call cmd,wrap_dts_initrd,sandpoint)
> > 
> > and:
> > 
> > 	image-$(CONFIG_SANDPOINT)               += zImage.sandpoint
> > 
> > Comments?
> 
> Isn't there a way to do this without hardcoding the
> platform name in the makefiles directly?  I fear for
> the one-per-platform effect that is yet to come with
> the conversion of many of the legacy boards.

I share your apprehension but somewhere we have to map a
CONFIG_<platform> into a name so we can make/use the correct
zImage.<platform> and dts/<platform>.dts.

We could do that in the Kconfig--define CONFIG_<platform> and
CONFIG_TARGET to "<platform>".  Don't know if that's acceptable.
Other ideas?

> > +++ b/arch/powerpc/boot/Makefile
> > @@ -117,6 +117,13 @@ quiet_cmd_wrap	= WRAP    $@
> >  quiet_cmd_wrap_initrd = WRAP    $@
> >        cmd_wrap_initrd =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) \
> >  				-i $(obj)/ramdisk.image.gz vmlinux
> > +quiet_cmd_wrap_dts = WRAP_DTS    $@
> > +      cmd_wrap_dts =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) \
> > +				-s $(obj)/dts/$2.dts vmlinux
> > +quiet_cmd_wrap_dts_initrd = WRAP_DTS    $@
> > +      cmd_wrap_dts_initrd =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 \
> > +				$(CROSSWRAP) -s $(obj)/dts/$2.dts \
> > +				-i $(obj)/ramdisk.image.gz vmlinux
> >  
> >  $(obj)/zImage.chrp: vmlinux $(wrapperbits)
> >  	$(call cmd,wrap,chrp)
> 
> Which I guess means this patch is OK, but the zImage.chrp use
> around it is suspect.... :-)

I didn't follow you here.  zImage.chrp was already there; I didn't
change it.  It should probably stay the as it is because "chrp" could
be any number of platforms and have several different dt's attached
once the zImage is built.

Mark

Re: [PATCH/RFC] Add cmd_wrap_dts rules to arch/powerpc/boot/Makefile

[Mark A. Greer]

On Thu, Oct 19, 2006 at 12:51:56PM -0700, Mark A. Greer wrote:
> On Thu, Oct 19, 2006 at 09:03:18AM -0500, Jon Loeliger wrote:
> > So, like, the other day "Mark A. Greer" mumbled:
> > > 
> > > This would be used by the sandpoint, say, by adding the following rules
> > > to the Makefile in the appropriate places:
> > > 
> > > 	$(obj)/zImage.sandpoint: vmlinux $(wrapperbits)
> > > 		$(call cmd,wrap_dts,sandpoint)
> > > 
> > > 	$(obj)/zImage.initrd.sandpoint: vmlinux $(wrapperbits)
> > > 		$(call cmd,wrap_dts_initrd,sandpoint)
> > > 
> > > and:
> > > 
> > > 	image-$(CONFIG_SANDPOINT)               += zImage.sandpoint
> > > 
> > > Comments?
> > 
> > Isn't there a way to do this without hardcoding the
> > platform name in the makefiles directly?  I fear for
> > the one-per-platform effect that is yet to come with
> > the conversion of many of the legacy boards.

Okay, its been a while so let's see if anyone remembers and/or cares
anymore.  :)

How about something like the patch below?

Basically, if you want a dtb wrapped up in your zImage, set
CONFIG_DEFAULT_DTB=y and also define CONFIG_DTS to point where your dts
file is.

Does that meet everyones requirements?

Mark
---

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 98392fb..848d1e4 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -118,6 +118,13 @@ quiet_cmd_wrap	= WRAP    $@
 quiet_cmd_wrap_initrd = WRAP    $@
       cmd_wrap_initrd =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) \
 				-i $(obj)/ramdisk.image.gz vmlinux
+quiet_cmd_wrap_dts = WRAP_DTS    $@
+      cmd_wrap_dts =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 $(CROSSWRAP) \
+				-s $(CONFIG_DTS) vmlinux
+quiet_cmd_wrap_dts_initrd = WRAP_DTS    $@
+      cmd_wrap_dts_initrd =$(CONFIG_SHELL) $(wrapper) -c -o $@ -p $2 \
+				$(CROSSWRAP) -s $(CONFIG_DTS) \
+				-i $(obj)/ramdisk.image.gz vmlinux
 
 $(obj)/zImage.chrp: vmlinux $(wrapperbits)
 	$(call cmd,wrap,chrp)
@@ -155,6 +162,12 @@ $(obj)/zImage.ps3: vmlinux
 $(obj)/zImage.initrd.ps3: vmlinux
 	@echo "  WARNING zImage.initrd.ps3 not supported (yet)"
 
+$(obj)/zImage.dtb: vmlinux $(wrapperbits)
+	$(call cmd,wrap_dts,dtb)
+
+$(obj)/zImage.initrd.dtb: vmlinux $(wrapperbits)
+	$(call cmd,wrap_dts_initrd,dtb)
+
 $(obj)/uImage: vmlinux $(wrapperbits)
 	$(call cmd,wrap,uboot)
 
@@ -165,6 +178,7 @@ image-$(CONFIG_PPC_PS3)			+= zImage.ps3
 image-$(CONFIG_PPC_CHRP)		+= zImage.chrp
 image-$(CONFIG_PPC_EFIKA)		+= zImage.chrp
 image-$(CONFIG_PPC_PMAC)		+= zImage.pmac
+image-$(CONFIG_DEFAULT_DTB)		+= zImage.dtb
 image-$(CONFIG_DEFAULT_UIMAGE)		+= uImage
 
 # For 32-bit powermacs, build the COFF and miboot images

Re: [PATCH/RFC] Add cmd_wrap_dts rules to arch/powerpc/boot/Makefile

[Jon Loeliger]

On Wed, 2007-01-10 at 18:40, Mark A. Greer wrote:
> On Thu, Oct 19, 2006 at 12:51:56PM -0700, Mark A. Greer wrote:
> > On Thu, Oct 19, 2006 at 09:03:18AM -0500, Jon Loeliger wrote:
> > > So, like, the other day "Mark A. Greer" mumbled:
> > > > 
> > > > This would be used by the sandpoint, say, by adding the following rules
> > > > to the Makefile in the appropriate places:
> > > > 
> > > > 	$(obj)/zImage.sandpoint: vmlinux $(wrapperbits)
> > > > 		$(call cmd,wrap_dts,sandpoint)
> > > > 
> > > > 	$(obj)/zImage.initrd.sandpoint: vmlinux $(wrapperbits)
> > > > 		$(call cmd,wrap_dts_initrd,sandpoint)
> > > > 
> > > > and:
> > > > 
> > > > 	image-$(CONFIG_SANDPOINT)               += zImage.sandpoint
> > > > 
> > > > Comments?
> > > 
> > > Isn't there a way to do this without hardcoding the
> > > platform name in the makefiles directly?  I fear for
> > > the one-per-platform effect that is yet to come with
> > > the conversion of many of the legacy boards.
> 
> Okay, its been a while so let's see if anyone remembers and/or cares
> anymore.  :)

We care.  We care deeply.  We've just all been on vacation. :-)

Scott Wood (here) has recently pushed around a patch
internally that is very similar and much more comprehensive,
though I confess I've not really studied it.  He mumbled
something about prepping it to send out when he gets back.
And I think that will be Monday.

jdl

Re: [PATCH/RFC] Add cmd_wrap_dts rules to arch/powerpc/boot/Makefile

[Mark A. Greer]

On Fri, Jan 12, 2007 at 01:50:59PM -0600, Jon Loeliger wrote:
> On Wed, 2007-01-10 at 18:40, Mark A. Greer wrote:
> > On Thu, Oct 19, 2006 at 12:51:56PM -0700, Mark A. Greer wrote:
> > > On Thu, Oct 19, 2006 at 09:03:18AM -0500, Jon Loeliger wrote:
> > > > So, like, the other day "Mark A. Greer" mumbled:
> > > > > 
> > > > > This would be used by the sandpoint, say, by adding the following rules
> > > > > to the Makefile in the appropriate places:
> > > > > 
> > > > > 	$(obj)/zImage.sandpoint: vmlinux $(wrapperbits)
> > > > > 		$(call cmd,wrap_dts,sandpoint)
> > > > > 
> > > > > 	$(obj)/zImage.initrd.sandpoint: vmlinux $(wrapperbits)
> > > > > 		$(call cmd,wrap_dts_initrd,sandpoint)
> > > > > 
> > > > > and:
> > > > > 
> > > > > 	image-$(CONFIG_SANDPOINT)               += zImage.sandpoint
> > > > > 
> > > > > Comments?
> > > > 
> > > > Isn't there a way to do this without hardcoding the
> > > > platform name in the makefiles directly?  I fear for
> > > > the one-per-platform effect that is yet to come with
> > > > the conversion of many of the legacy boards.
> > 
> > Okay, its been a while so let's see if anyone remembers and/or cares
> > anymore.  :)
> 
> We care.  We care deeply.  We've just all been on vacation. :-)
> 
> Scott Wood (here) has recently pushed around a patch
> internally that is very similar and much more comprehensive,
> though I confess I've not really studied it.  He mumbled
> something about prepping it to send out when he gets back.
> And I think that will be Monday.

Okay, that would be great.  Thanks for the heads up.

Mark

Re: [PATCH/RFC] Add cmd_wrap_dts rules to arch/powerpc/boot/Makefile

[David Gibson]

On Wed, Jan 10, 2007 at 05:40:48PM -0700, Mark A. Greer wrote:
> On Thu, Oct 19, 2006 at 12:51:56PM -0700, Mark A. Greer wrote:
> > On Thu, Oct 19, 2006 at 09:03:18AM -0500, Jon Loeliger wrote:
> > > So, like, the other day "Mark A. Greer" mumbled:
> > > > 
> > > > This would be used by the sandpoint, say, by adding the following rules
> > > > to the Makefile in the appropriate places:
> > > > 
> > > > 	$(obj)/zImage.sandpoint: vmlinux $(wrapperbits)
> > > > 		$(call cmd,wrap_dts,sandpoint)
> > > > 
> > > > 	$(obj)/zImage.initrd.sandpoint: vmlinux $(wrapperbits)
> > > > 		$(call cmd,wrap_dts_initrd,sandpoint)
> > > > 
> > > > and:
> > > > 
> > > > 	image-$(CONFIG_SANDPOINT)               += zImage.sandpoint
> > > > 
> > > > Comments?
> > > 
> > > Isn't there a way to do this without hardcoding the
> > > platform name in the makefiles directly?  I fear for
> > > the one-per-platform effect that is yet to come with
> > > the conversion of many of the legacy boards.
> 
> Okay, its been a while so let's see if anyone remembers and/or cares
> anymore.  :)
> 
> How about something like the patch below?
> 
> Basically, if you want a dtb wrapped up in your zImage, set
> CONFIG_DEFAULT_DTB=y and also define CONFIG_DTS to point where your dts
> file is.
> 
> Does that meet everyones requirements?

Erm.. actually from what I could tell, the way the wrapper script
invokes dtc will make a mess if you're building with a different
object tree from the source tree.  I think the other suggested patches
which have a cmd_wrap_dtb and a spearate make rule to invoke dtc are a
better shot, and it might be a good idea to rip the dtc invocation out
of wrapper entirely.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson