| Message ID | alpine.LFD.2.00.1106140300500.2142@xanadu.home (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 14, 2011 at 03:09:02AM -0400, Nicolas Pitre wrote: > On Mon, 13 Jun 2011, Nicolas Pitre wrote: > > > > Unless I'm missing something, I don't see a clean way of supporting this > > > that doesn't involve the kernel being able to parse the ATAGS as well. > > > > FYI: I've dug up the patch from John Bonesio doing just that. While the > > patch doesn't apply anymore, it looks trivial enough. I should have it > > working by tomorrow. > > Well, here it is. It compiles, but otherwise completely untested. > Just tested the patch on mx51 babbage, and it's working. Great work! I have one comment below though. > This applies on top of the 3 other patches I posted when this thread was > started. > > While this could be cleaned up further, the functionality should all be > there and usable. > > From: Nicolas Pitre <nicolas.pitre@linaro.org> > Date: Tue, 14 Jun 2011 02:40:33 -0400 > Subject: [PATCH] ARM: zImage: allow supplementing appended DTB with traditional ATAG data > > This is based on an older patch from John Bonesio <bones@secretlab.ca>. > > Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 66b7d1e..166bd2a 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -1735,6 +1735,17 @@ config ARM_APPENDED_DTB > (dtb) appended to zImage > (e.g. cat zImage <filename>.dtb > zImage_w_dtb). > > +config ARM_ATAG_DTB_COMPAT > + bool "Supplement the appended DTB with traditional ATAG information" > + depends on ARM_APPENDED_DTB > + help > + Some old bootloaders can't be updated to a DTB capable one, yet > + they provide ATAGs with memory configuration, the ramdisk address, > + the kernel cmdline string, etc. To allow a device tree enabled > + kernel to be used with such bootloaders, this option allows > + zImage to extract the information from the ATAG list and store it > + at run time into the appended DTB. > + > config CMDLINE > string "Default kernel command string" > default "" > diff --git a/arch/arm/boot/compressed/.gitignore b/arch/arm/boot/compressed/.gitignore > index c602896..e0936a1 100644 > --- a/arch/arm/boot/compressed/.gitignore > +++ b/arch/arm/boot/compressed/.gitignore > @@ -5,3 +5,12 @@ piggy.lzo > piggy.lzma > vmlinux > vmlinux.lds > + > +# borrowed libfdt files > +fdt.c > +fdt.h > +fdt_ro.c > +fdt_rw.c > +fdt_wip.c > +libfdt.h > +libfdt_internal.h [Note: I copied these files from scripts/dtc/libfdt] > diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile > index 48bead9..4b94995 100644 > --- a/arch/arm/boot/compressed/Makefile > +++ b/arch/arm/boot/compressed/Makefile > @@ -83,19 +83,36 @@ suffix_$(CONFIG_KERNEL_GZIP) = gzip > suffix_$(CONFIG_KERNEL_LZO) = lzo > suffix_$(CONFIG_KERNEL_LZMA) = lzma > > +# libfdt files for the ATAG compatibility mode > + > +libfdt := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c > +libfdt_hdrs := fdt.h libfdt.h libfdt_internal.h > + > +libfdt_objs := $(addsuffix .o, $(basename $(libfdt))) > + > +$(addprefix $(obj)/,$(libfdt) $(libfdt_hdrs)): $(obj)/%: $(srctree)/scripts/dtc/libfdt/% > + $(call if_changed,shipped) > + > +$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o): \ > + $(addprefix $(obj)/,$(libfdt_hdrs)) > + > +ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y) > +OBJS += $(libfdt_objs) atags_to_fdt.o > +endif > + > targets := vmlinux vmlinux.lds \ > piggy.$(suffix_y) piggy.$(suffix_y).o \ > font.o font.c head.o misc.o $(OBJS) > > # Make sure files are removed during clean > -extra-y += piggy.gzip piggy.lzo piggy.lzma lib1funcs.S > +extra-y += piggy.gzip piggy.lzo piggy.lzma lib1funcs.S $(libfdt) $(libfdt_hdrs) > > ifeq ($(CONFIG_FUNCTION_TRACER),y) > ORIG_CFLAGS := $(KBUILD_CFLAGS) > KBUILD_CFLAGS = $(subst -pg, , $(ORIG_CFLAGS)) > endif > > -ccflags-y := -fpic -fno-builtin > +ccflags-y := -fpic -fno-builtin -I$(src) > asflags-y := -Wa,-march=all > > # Supply kernel BSS size to the decompressor via a linker symbol. > @@ -118,7 +135,7 @@ LDFLAGS_vmlinux += -X > LDFLAGS_vmlinux += -T > > # For __aeabi_uidivmod > -lib1funcs = $(obj)/lib1funcs.o > +lib1funcs = $(obj)/lib1funcs.o $(obj)/../../lib/lib.a > > $(obj)/lib1funcs.S: $(srctree)/arch/$(SRCARCH)/lib/lib1funcs.S FORCE > $(call cmd,shipped) > diff --git a/arch/arm/boot/compressed/atags_to_fdt.c b/arch/arm/boot/compressed/atags_to_fdt.c > new file mode 100644 > index 0000000..11c1a88 > --- /dev/null > +++ b/arch/arm/boot/compressed/atags_to_fdt.c > @@ -0,0 +1,71 @@ > +#include <asm/setup.h> > +#include <libfdt.h> > + > +static int setprop(void *fdt, const char *node_path, const char *property, > + uint32_t *val_array, int size) > +{ > + int offset = fdt_path_offset(fdt, node_path); > + if (offset < 0) > + return offset; > + return fdt_setprop(fdt, offset, property, val_array, size); > +} > + > +static int setprop_string(void *fdt, const char *node_path, > + const char *property, const char *string) > +{ > + int offset = fdt_path_offset(fdt, node_path); > + if (offset < 0) > + return offset; > + return fdt_setprop_string(fdt, offset, property, string); > +} > + > +static int setprop_cell(void *fdt, const char *node_path, > + const char *property, uint32_t val) > +{ > + int offset = fdt_path_offset(fdt, node_path); > + if (offset < 0) > + return offset; > + return fdt_setprop_cell(fdt, offset, property, val); > +} > + > +int atags_to_fdt(void *dt, void *atag_list) > +{ > + struct tag *atag = atag_list; > + > + /* make sure we've got an aligned pointer */ > + if ((u32)atag_list & 0x3) > + return -1; > + > + /* if we get a DTB here we're done already */ > + if (*(u32 *)atag_list == fdt32_to_cpu(FDT_MAGIC)) > + return 0; > + > + /* validate the ATAG */ > + if (atag->hdr.tag != ATAG_CORE || > + (atag->hdr.size != tag_size(tag_core) && > + atag->hdr.size != 2)) > + return -1; > + > + for_each_tag(atag, atag_list) { > + if (atag->hdr.tag == ATAG_CMDLINE) { > + setprop_string(dt, "/chosen", "bootargs", > + atag->u.cmdline.cmdline); > + } else if (atag->hdr.tag == ATAG_MEM) { > + uint32_t mem_reg_property[2]; > + mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start); > + mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size); > + setprop(dt, "/memory", "reg", mem_reg_property, > + sizeof(mem_reg_property)); > + } else if (atag->hdr.tag == ATAG_INITRD2) { > + uint32_t initrd_start, initrd_size; > + initrd_start = atag->u.initrd.start; > + initrd_size = atag->u.initrd.size; > + setprop_cell(dt, "/chosen", "linux,initrd-start", > + initrd_start); > + setprop_cell(dt, "/chosen", "linux,initrd-end", > + initrd_start + initrd_size); > + } > + } > + > + return 0; > +} This works only when there are corresponding nodes already in dtb. Otherwise, the atag arguments are not going to be updated into dtb. I suppose these nodes should be created to accommodate the arguments from atags if the appended dtb does not have them, no?
* Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]: > + > + for_each_tag(atag, atag_list) { > + if (atag->hdr.tag == ATAG_CMDLINE) { > + setprop_string(dt, "/chosen", "bootargs", > + atag->u.cmdline.cmdline); > + } else if (atag->hdr.tag == ATAG_MEM) { > + uint32_t mem_reg_property[2]; > + mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start); > + mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size); > + setprop(dt, "/memory", "reg", mem_reg_property, > + sizeof(mem_reg_property)); > + } else if (atag->hdr.tag == ATAG_INITRD2) { > + uint32_t initrd_start, initrd_size; > + initrd_start = atag->u.initrd.start; > + initrd_size = atag->u.initrd.size; > + setprop_cell(dt, "/chosen", "linux,initrd-start", > + initrd_start); > + setprop_cell(dt, "/chosen", "linux,initrd-end", > + initrd_start + initrd_size); > + } > + } I think Russell posted a complete list of the ATAGs to translate somewhere, but at least ATAG_REVISION is missing here. That's being used quite a bit as system_rev to set things dynamically. Regards, Tony
On Tue, 14 Jun 2011, Tony Lindgren wrote: > * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]: > > + > > + for_each_tag(atag, atag_list) { > > + if (atag->hdr.tag == ATAG_CMDLINE) { > > + setprop_string(dt, "/chosen", "bootargs", > > + atag->u.cmdline.cmdline); > > + } else if (atag->hdr.tag == ATAG_MEM) { > > + uint32_t mem_reg_property[2]; > > + mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start); > > + mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size); > > + setprop(dt, "/memory", "reg", mem_reg_property, > > + sizeof(mem_reg_property)); > > + } else if (atag->hdr.tag == ATAG_INITRD2) { > > + uint32_t initrd_start, initrd_size; > > + initrd_start = atag->u.initrd.start; > > + initrd_size = atag->u.initrd.size; > > + setprop_cell(dt, "/chosen", "linux,initrd-start", > > + initrd_start); > > + setprop_cell(dt, "/chosen", "linux,initrd-end", > > + initrd_start + initrd_size); > > + } > > + } > > I think Russell posted a complete list of the ATAGs to translate > somewhere, but at least ATAG_REVISION is missing here. That's being > used quite a bit as system_rev to set things dynamically. No problem. This is a work in progress. We still can test the concept and fine tune the actual set of ATAGs being translated. Nicolas
On Tuesday 14 June 2011 19:28:49 Nicolas Pitre wrote: > On Tue, 14 Jun 2011, Tony Lindgren wrote: > > > * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]: > > > + > > > + for_each_tag(atag, atag_list) { > > > + if (atag->hdr.tag == ATAG_CMDLINE) { > > > + setprop_string(dt, "/chosen", "bootargs", > > > + atag->u.cmdline.cmdline); > > > + } else if (atag->hdr.tag == ATAG_MEM) { > > > + uint32_t mem_reg_property[2]; > > > + mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start); > > > + mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size); > > > + setprop(dt, "/memory", "reg", mem_reg_property, > > > + sizeof(mem_reg_property)); > > > + } else if (atag->hdr.tag == ATAG_INITRD2) { > > > + uint32_t initrd_start, initrd_size; > > > + initrd_start = atag->u.initrd.start; > > > + initrd_size = atag->u.initrd.size; > > > + setprop_cell(dt, "/chosen", "linux,initrd-start", > > > + initrd_start); > > > + setprop_cell(dt, "/chosen", "linux,initrd-end", > > > + initrd_start + initrd_size); > > > + } > > > + } > > > > I think Russell posted a complete list of the ATAGs to translate > > somewhere, but at least ATAG_REVISION is missing here. That's being > > used quite a bit as system_rev to set things dynamically. > > No problem. This is a work in progress. We still can test the concept > and fine tune the actual set of ATAGs being translated. Let's talk about the revision field now, because it doesn't have a direct correspondence in existing attributes. Functionality-wise, this would probably be the "compatible" property of the root node, with its most specific name to match, but that's not trivial to generate. In some cases, the system revisions have significant differences in their devices (why else would you care about the revision), which means that you actually need a different device tree per revision anyway. If that's the common case, we could just ignore it completely or instead have a way to choose a specific device tree from a list based on the revision. Another option would be to merge both the ATAG_REVISION and ATAG_SERIAL into a string and put them into the root "serial-number" property that is fairly easy to do, but would be harder to parse if you actually rely on specific values. Arnd
On Tue, 14 Jun 2011, Arnd Bergmann wrote: > On Tuesday 14 June 2011 19:28:49 Nicolas Pitre wrote: > > On Tue, 14 Jun 2011, Tony Lindgren wrote: > > > > > * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]: > > > > + > > > > + for_each_tag(atag, atag_list) { > > > > + if (atag->hdr.tag == ATAG_CMDLINE) { > > > > + setprop_string(dt, "/chosen", "bootargs", > > > > + atag->u.cmdline.cmdline); > > > > + } else if (atag->hdr.tag == ATAG_MEM) { > > > > + uint32_t mem_reg_property[2]; > > > > + mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start); > > > > + mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size); > > > > + setprop(dt, "/memory", "reg", mem_reg_property, > > > > + sizeof(mem_reg_property)); > > > > + } else if (atag->hdr.tag == ATAG_INITRD2) { > > > > + uint32_t initrd_start, initrd_size; > > > > + initrd_start = atag->u.initrd.start; > > > > + initrd_size = atag->u.initrd.size; > > > > + setprop_cell(dt, "/chosen", "linux,initrd-start", > > > > + initrd_start); > > > > + setprop_cell(dt, "/chosen", "linux,initrd-end", > > > > + initrd_start + initrd_size); > > > > + } > > > > + } > > > > > > I think Russell posted a complete list of the ATAGs to translate > > > somewhere, but at least ATAG_REVISION is missing here. That's being > > > used quite a bit as system_rev to set things dynamically. > > > > No problem. This is a work in progress. We still can test the concept > > and fine tune the actual set of ATAGs being translated. > > Let's talk about the revision field now, because it doesn't have a > direct correspondence in existing attributes. > > Functionality-wise, this would probably be the "compatible" property > of the root node, with its most specific name to match, but that's not > trivial to generate. > > In some cases, the system revisions have significant differences in their > devices (why else would you care about the revision), which means that you > actually need a different device tree per revision anyway. If that's the > common case, we could just ignore it completely or instead have a way > to choose a specific device tree from a list based on the revision. I don't think this is all that common. Almost half of the cases, the system_rev variable is assigned a specific value from direct probing, not from the ATAG_REVISION tag. So there are only a few cases to consider. Yet it is used only for a few things, which doesn't indicate the system differences are that significant in practice. If we were to use this revision number, which appears to be completely arbitrary from one board to another, in order to select the appropriate DTB amongst many possibilities, then we'll need something in the DTB that can be correlated to this revision number in the first place. And let's remember that this is only about some compatibility convenience for legacy systems. I really don't want this to be over engineered. > Another option would be to merge both the ATAG_REVISION and ATAG_SERIAL > into a string and put them into the root "serial-number" property that > is fairly easy to do, but would be harder to parse if you actually rely > on specific values. At some point, if some boards do use the ATAG to let the bootloader specify the board revision, that must be because this revision number can't be probed by software. If this can be probed by software, then let's do it with kernel code instead. Otherwise, if the revision number is effectively non probable, then I would guess it is the device tree's purpose to carry that information somehow, right? Maybe this can be appended to the board name string? Nicolas
On Tue, Jun 14 2011, Nicolas Pitre wrote: > + } else if (atag->hdr.tag == ATAG_MEM) { > + uint32_t mem_reg_property[2]; > + mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start); > + mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size); > + setprop(dt, "/memory", "reg", mem_reg_property, > + sizeof(mem_reg_property)); Hopefully, I'll be able to get zImages working on MSM8x60 today, and I can test this. I think the above might need some work to handle multiple ATAG_MEM attributes (our memory isn't contiguous). I can look at this once I get it working. Does this mean that merging appended DTB and atags requires using a compressed kernel? It seems that several years ago, someone tested the decompressor on a slow MSM and decided to boot from the uncompressed image, and nobody has bothered with the compressed image. I don't think it is a problem to require zImage for this case, though. David
On Tuesday 14 June 2011 23:21:52 Nicolas Pitre wrote: > Otherwise, if the revision number is effectively non probable, then I > would guess it is the device tree's purpose to carry that information > somehow, right? Maybe this can be appended to the board name string? That's what I meant with adding it to "compatible", which is a list of strings with varying degrees of detailed information, e.g. ti,omap7 ti,omap7-squirrelboard ti,omap7-squirrelboard-v3 ti,omap7-squirrelboard-v3.17b You can match the list against a specific revision or a less specific identifier if you just want to know whether you are on a squirrelboard or a hamsterboard. Arnd
On Tue, Jun 14, 2011 at 3:42 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Tuesday 14 June 2011 23:21:52 Nicolas Pitre wrote: >> Otherwise, if the revision number is effectively non probable, then I >> would guess it is the device tree's purpose to carry that information >> somehow, right? Maybe this can be appended to the board name string? > > That's what I meant with adding it to "compatible", which is a list of > strings with varying degrees of detailed information, e.g. > > ti,omap7 > ti,omap7-squirrelboard > ti,omap7-squirrelboard-v3 > ti,omap7-squirrelboard-v3.17b > > You can match the list against a specific revision or a less specific > identifier if you just want to know whether you are on a squirrelboard > or a hamsterboard. Yes, that's generally the right way to handle it. In practice I've not seen many cases where it is really required, but it doesn't hurt if somebody wants to include it in the DT for their board. g.
On 06/14/2011 03:32 PM, Arnd Bergmann wrote: > On Tuesday 14 June 2011 19:28:49 Nicolas Pitre wrote: >> On Tue, 14 Jun 2011, Tony Lindgren wrote: >> >>> * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]: >>>> + >>>> + for_each_tag(atag, atag_list) { >>>> + if (atag->hdr.tag == ATAG_CMDLINE) { >>>> + setprop_string(dt, "/chosen", "bootargs", >>>> + atag->u.cmdline.cmdline); >>>> + } else if (atag->hdr.tag == ATAG_MEM) { >>>> + uint32_t mem_reg_property[2]; >>>> + mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start); >>>> + mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size); >>>> + setprop(dt, "/memory", "reg", mem_reg_property, >>>> + sizeof(mem_reg_property)); >>>> + } else if (atag->hdr.tag == ATAG_INITRD2) { >>>> + uint32_t initrd_start, initrd_size; >>>> + initrd_start = atag->u.initrd.start; >>>> + initrd_size = atag->u.initrd.size; >>>> + setprop_cell(dt, "/chosen", "linux,initrd-start", >>>> + initrd_start); >>>> + setprop_cell(dt, "/chosen", "linux,initrd-end", >>>> + initrd_start + initrd_size); >>>> + } >>>> + } >>> >>> I think Russell posted a complete list of the ATAGs to translate >>> somewhere, but at least ATAG_REVISION is missing here. That's being >>> used quite a bit as system_rev to set things dynamically. >> >> No problem. This is a work in progress. We still can test the concept >> and fine tune the actual set of ATAGs being translated. > > Let's talk about the revision field now, because it doesn't have a > direct correspondence in existing attributes. > > Functionality-wise, this would probably be the "compatible" property > of the root node, with its most specific name to match, but that's not > trivial to generate. > > In some cases, the system revisions have significant differences in their > devices (why else would you care about the revision), which means that you > actually need a different device tree per revision anyway. If that's the > common case, we could just ignore it completely or instead have a way > to choose a specific device tree from a list based on the revision. > > Another option would be to merge both the ATAG_REVISION and ATAG_SERIAL > into a string and put them into the root "serial-number" property that > is fairly easy to do, but would be harder to parse if you actually rely > on specific values. > IIRC, system_rev at least can be specified on the kernel command line, so you could just append the cmd line. However, if you force the built-in command line to be used that would not work, Rob
On Tue, 14 Jun 2011, Rob Herring wrote: > On 06/14/2011 03:32 PM, Arnd Bergmann wrote: > > On Tuesday 14 June 2011 19:28:49 Nicolas Pitre wrote: > >> On Tue, 14 Jun 2011, Tony Lindgren wrote: > >> > >>> * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]: > >>>> + > >>>> + for_each_tag(atag, atag_list) { > >>>> + if (atag->hdr.tag == ATAG_CMDLINE) { > >>>> + setprop_string(dt, "/chosen", "bootargs", > >>>> + atag->u.cmdline.cmdline); > >>>> + } else if (atag->hdr.tag == ATAG_MEM) { > >>>> + uint32_t mem_reg_property[2]; > >>>> + mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start); > >>>> + mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size); > >>>> + setprop(dt, "/memory", "reg", mem_reg_property, > >>>> + sizeof(mem_reg_property)); > >>>> + } else if (atag->hdr.tag == ATAG_INITRD2) { > >>>> + uint32_t initrd_start, initrd_size; > >>>> + initrd_start = atag->u.initrd.start; > >>>> + initrd_size = atag->u.initrd.size; > >>>> + setprop_cell(dt, "/chosen", "linux,initrd-start", > >>>> + initrd_start); > >>>> + setprop_cell(dt, "/chosen", "linux,initrd-end", > >>>> + initrd_start + initrd_size); > >>>> + } > >>>> + } > >>> > >>> I think Russell posted a complete list of the ATAGs to translate > >>> somewhere, but at least ATAG_REVISION is missing here. That's being > >>> used quite a bit as system_rev to set things dynamically. > >> > >> No problem. This is a work in progress. We still can test the concept > >> and fine tune the actual set of ATAGs being translated. > > > > Let's talk about the revision field now, because it doesn't have a > > direct correspondence in existing attributes. > > > > Functionality-wise, this would probably be the "compatible" property > > of the root node, with its most specific name to match, but that's not > > trivial to generate. > > > > In some cases, the system revisions have significant differences in their > > devices (why else would you care about the revision), which means that you > > actually need a different device tree per revision anyway. If that's the > > common case, we could just ignore it completely or instead have a way > > to choose a specific device tree from a list based on the revision. > > > > Another option would be to merge both the ATAG_REVISION and ATAG_SERIAL > > into a string and put them into the root "serial-number" property that > > is fairly easy to do, but would be harder to parse if you actually rely > > on specific values. > > > > IIRC, system_rev at least can be specified on the kernel command line, > so you could just append the cmd line. However, if you force the > built-in command line to be used that would not work, If you do that then you probably don't care as much about translating existing ATAGs from the bootloader and could as well just live with the hardcoded values in the appended DTB. Nicolas
On 06/14/2011 06:50 PM, Nicolas Pitre wrote: > On Tue, 14 Jun 2011, Rob Herring wrote: > >> On 06/14/2011 03:32 PM, Arnd Bergmann wrote: >>> On Tuesday 14 June 2011 19:28:49 Nicolas Pitre wrote: >>>> On Tue, 14 Jun 2011, Tony Lindgren wrote: >>>> >>>>> * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]: >>>>>> + >>>>>> + for_each_tag(atag, atag_list) { >>>>>> + if (atag->hdr.tag == ATAG_CMDLINE) { >>>>>> + setprop_string(dt, "/chosen", "bootargs", >>>>>> + atag->u.cmdline.cmdline); >>>>>> + } else if (atag->hdr.tag == ATAG_MEM) { >>>>>> + uint32_t mem_reg_property[2]; >>>>>> + mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start); >>>>>> + mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size); >>>>>> + setprop(dt, "/memory", "reg", mem_reg_property, >>>>>> + sizeof(mem_reg_property)); >>>>>> + } else if (atag->hdr.tag == ATAG_INITRD2) { >>>>>> + uint32_t initrd_start, initrd_size; >>>>>> + initrd_start = atag->u.initrd.start; >>>>>> + initrd_size = atag->u.initrd.size; >>>>>> + setprop_cell(dt, "/chosen", "linux,initrd-start", >>>>>> + initrd_start); >>>>>> + setprop_cell(dt, "/chosen", "linux,initrd-end", >>>>>> + initrd_start + initrd_size); >>>>>> + } >>>>>> + } >>>>> >>>>> I think Russell posted a complete list of the ATAGs to translate >>>>> somewhere, but at least ATAG_REVISION is missing here. That's being >>>>> used quite a bit as system_rev to set things dynamically. >>>> >>>> No problem. This is a work in progress. We still can test the concept >>>> and fine tune the actual set of ATAGs being translated. >>> >>> Let's talk about the revision field now, because it doesn't have a >>> direct correspondence in existing attributes. >>> >>> Functionality-wise, this would probably be the "compatible" property >>> of the root node, with its most specific name to match, but that's not >>> trivial to generate. >>> >>> In some cases, the system revisions have significant differences in their >>> devices (why else would you care about the revision), which means that you >>> actually need a different device tree per revision anyway. If that's the >>> common case, we could just ignore it completely or instead have a way >>> to choose a specific device tree from a list based on the revision. >>> >>> Another option would be to merge both the ATAG_REVISION and ATAG_SERIAL >>> into a string and put them into the root "serial-number" property that >>> is fairly easy to do, but would be harder to parse if you actually rely >>> on specific values. >>> >> >> IIRC, system_rev at least can be specified on the kernel command line, >> so you could just append the cmd line. However, if you force the >> built-in command line to be used that would not work, > > If you do that then you probably don't care as much about translating > existing ATAGs from the bootloader and could as well just live with the > hardcoded values in the appended DTB. > What I meant was you could translate these ATAGs into the DTB kernel command-line options rather than come-up with something new. You already having to mess with the command line. Rob
On Tue, 14 Jun 2011, Rob Herring wrote: > On 06/14/2011 06:50 PM, Nicolas Pitre wrote: > > On Tue, 14 Jun 2011, Rob Herring wrote: > > > >> On 06/14/2011 03:32 PM, Arnd Bergmann wrote: > >>> On Tuesday 14 June 2011 19:28:49 Nicolas Pitre wrote: > >>>> On Tue, 14 Jun 2011, Tony Lindgren wrote: > >>>> > >>>>> * Nicolas Pitre <nicolas.pitre@linaro.org> [110614 00:04]: > >>>>>> + > >>>>>> + for_each_tag(atag, atag_list) { > >>>>>> + if (atag->hdr.tag == ATAG_CMDLINE) { > >>>>>> + setprop_string(dt, "/chosen", "bootargs", > >>>>>> + atag->u.cmdline.cmdline); > >>>>>> + } else if (atag->hdr.tag == ATAG_MEM) { > >>>>>> + uint32_t mem_reg_property[2]; > >>>>>> + mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start); > >>>>>> + mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size); > >>>>>> + setprop(dt, "/memory", "reg", mem_reg_property, > >>>>>> + sizeof(mem_reg_property)); > >>>>>> + } else if (atag->hdr.tag == ATAG_INITRD2) { > >>>>>> + uint32_t initrd_start, initrd_size; > >>>>>> + initrd_start = atag->u.initrd.start; > >>>>>> + initrd_size = atag->u.initrd.size; > >>>>>> + setprop_cell(dt, "/chosen", "linux,initrd-start", > >>>>>> + initrd_start); > >>>>>> + setprop_cell(dt, "/chosen", "linux,initrd-end", > >>>>>> + initrd_start + initrd_size); > >>>>>> + } > >>>>>> + } > >>>>> > >>>>> I think Russell posted a complete list of the ATAGs to translate > >>>>> somewhere, but at least ATAG_REVISION is missing here. That's being > >>>>> used quite a bit as system_rev to set things dynamically. > >>>> > >>>> No problem. This is a work in progress. We still can test the concept > >>>> and fine tune the actual set of ATAGs being translated. > >>> > >>> Let's talk about the revision field now, because it doesn't have a > >>> direct correspondence in existing attributes. > >>> > >>> Functionality-wise, this would probably be the "compatible" property > >>> of the root node, with its most specific name to match, but that's not > >>> trivial to generate. > >>> > >>> In some cases, the system revisions have significant differences in their > >>> devices (why else would you care about the revision), which means that you > >>> actually need a different device tree per revision anyway. If that's the > >>> common case, we could just ignore it completely or instead have a way > >>> to choose a specific device tree from a list based on the revision. > >>> > >>> Another option would be to merge both the ATAG_REVISION and ATAG_SERIAL > >>> into a string and put them into the root "serial-number" property that > >>> is fairly easy to do, but would be harder to parse if you actually rely > >>> on specific values. > >>> > >> > >> IIRC, system_rev at least can be specified on the kernel command line, > >> so you could just append the cmd line. However, if you force the > >> built-in command line to be used that would not work, > > > > If you do that then you probably don't care as much about translating > > existing ATAGs from the bootloader and could as well just live with the > > hardcoded values in the appended DTB. > > > > What I meant was you could translate these ATAGs into the DTB kernel > command-line options rather than come-up with something new. You already > having to mess with the command line. Agreed. What I meant is that you're unlikely to use the kernel built-in command line if you care about this. Nicolas
* Grant Likely <grant.likely@secretlab.ca> [110614 15:02]: > On Tue, Jun 14, 2011 at 3:42 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Tuesday 14 June 2011 23:21:52 Nicolas Pitre wrote: > >> Otherwise, if the revision number is effectively non probable, then I > >> would guess it is the device tree's purpose to carry that information > >> somehow, right? Maybe this can be appended to the board name string? > > > > That's what I meant with adding it to "compatible", which is a list of > > strings with varying degrees of detailed information, e.g. > > > > ti,omap7 > > ti,omap7-squirrelboard > > ti,omap7-squirrelboard-v3 > > ti,omap7-squirrelboard-v3.17b > > > > You can match the list against a specific revision or a less specific > > identifier if you just want to know whether you are on a squirrelboard > > or a hamsterboard. > > Yes, that's generally the right way to handle it. In practice I've > not seen many cases where it is really required, but it doesn't hurt > if somebody wants to include it in the DT for their board. Appending the ATAG_REVISION to "compatible" and also setting system_rev to ATAG_REVISION like we already do should work. Just to clarify things abit: We just can't generate .dts files for all existing hardware. It would require dumping out system_rev and other ATAGs on tens or hundeds of pieces of hardware. Regards, Tony
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 66b7d1e..166bd2a 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1735,6 +1735,17 @@ config ARM_APPENDED_DTB (dtb) appended to zImage (e.g. cat zImage <filename>.dtb > zImage_w_dtb). +config ARM_ATAG_DTB_COMPAT + bool "Supplement the appended DTB with traditional ATAG information" + depends on ARM_APPENDED_DTB + help + Some old bootloaders can't be updated to a DTB capable one, yet + they provide ATAGs with memory configuration, the ramdisk address, + the kernel cmdline string, etc. To allow a device tree enabled + kernel to be used with such bootloaders, this option allows + zImage to extract the information from the ATAG list and store it + at run time into the appended DTB. + config CMDLINE string "Default kernel command string" default "" diff --git a/arch/arm/boot/compressed/.gitignore b/arch/arm/boot/compressed/.gitignore index c602896..e0936a1 100644 --- a/arch/arm/boot/compressed/.gitignore +++ b/arch/arm/boot/compressed/.gitignore @@ -5,3 +5,12 @@ piggy.lzo piggy.lzma vmlinux vmlinux.lds + +# borrowed libfdt files +fdt.c +fdt.h +fdt_ro.c +fdt_rw.c +fdt_wip.c +libfdt.h +libfdt_internal.h diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile index 48bead9..4b94995 100644 --- a/arch/arm/boot/compressed/Makefile +++ b/arch/arm/boot/compressed/Makefile @@ -83,19 +83,36 @@ suffix_$(CONFIG_KERNEL_GZIP) = gzip suffix_$(CONFIG_KERNEL_LZO) = lzo suffix_$(CONFIG_KERNEL_LZMA) = lzma +# libfdt files for the ATAG compatibility mode + +libfdt := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c +libfdt_hdrs := fdt.h libfdt.h libfdt_internal.h + +libfdt_objs := $(addsuffix .o, $(basename $(libfdt))) + +$(addprefix $(obj)/,$(libfdt) $(libfdt_hdrs)): $(obj)/%: $(srctree)/scripts/dtc/libfdt/% + $(call if_changed,shipped) + +$(addprefix $(obj)/,$(libfdt_objs) atags_to_fdt.o): \ + $(addprefix $(obj)/,$(libfdt_hdrs)) + +ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y) +OBJS += $(libfdt_objs) atags_to_fdt.o +endif + targets := vmlinux vmlinux.lds \ piggy.$(suffix_y) piggy.$(suffix_y).o \ font.o font.c head.o misc.o $(OBJS) # Make sure files are removed during clean -extra-y += piggy.gzip piggy.lzo piggy.lzma lib1funcs.S +extra-y += piggy.gzip piggy.lzo piggy.lzma lib1funcs.S $(libfdt) $(libfdt_hdrs) ifeq ($(CONFIG_FUNCTION_TRACER),y) ORIG_CFLAGS := $(KBUILD_CFLAGS) KBUILD_CFLAGS = $(subst -pg, , $(ORIG_CFLAGS)) endif -ccflags-y := -fpic -fno-builtin +ccflags-y := -fpic -fno-builtin -I$(src) asflags-y := -Wa,-march=all # Supply kernel BSS size to the decompressor via a linker symbol. @@ -118,7 +135,7 @@ LDFLAGS_vmlinux += -X LDFLAGS_vmlinux += -T # For __aeabi_uidivmod -lib1funcs = $(obj)/lib1funcs.o +lib1funcs = $(obj)/lib1funcs.o $(obj)/../../lib/lib.a $(obj)/lib1funcs.S: $(srctree)/arch/$(SRCARCH)/lib/lib1funcs.S FORCE $(call cmd,shipped) diff --git a/arch/arm/boot/compressed/atags_to_fdt.c b/arch/arm/boot/compressed/atags_to_fdt.c new file mode 100644 index 0000000..11c1a88 --- /dev/null +++ b/arch/arm/boot/compressed/atags_to_fdt.c @@ -0,0 +1,71 @@ +#include <asm/setup.h> +#include <libfdt.h> + +static int setprop(void *fdt, const char *node_path, const char *property, + uint32_t *val_array, int size) +{ + int offset = fdt_path_offset(fdt, node_path); + if (offset < 0) + return offset; + return fdt_setprop(fdt, offset, property, val_array, size); +} + +static int setprop_string(void *fdt, const char *node_path, + const char *property, const char *string) +{ + int offset = fdt_path_offset(fdt, node_path); + if (offset < 0) + return offset; + return fdt_setprop_string(fdt, offset, property, string); +} + +static int setprop_cell(void *fdt, const char *node_path, + const char *property, uint32_t val) +{ + int offset = fdt_path_offset(fdt, node_path); + if (offset < 0) + return offset; + return fdt_setprop_cell(fdt, offset, property, val); +} + +int atags_to_fdt(void *dt, void *atag_list) +{ + struct tag *atag = atag_list; + + /* make sure we've got an aligned pointer */ + if ((u32)atag_list & 0x3) + return -1; + + /* if we get a DTB here we're done already */ + if (*(u32 *)atag_list == fdt32_to_cpu(FDT_MAGIC)) + return 0; + + /* validate the ATAG */ + if (atag->hdr.tag != ATAG_CORE || + (atag->hdr.size != tag_size(tag_core) && + atag->hdr.size != 2)) + return -1; + + for_each_tag(atag, atag_list) { + if (atag->hdr.tag == ATAG_CMDLINE) { + setprop_string(dt, "/chosen", "bootargs", + atag->u.cmdline.cmdline); + } else if (atag->hdr.tag == ATAG_MEM) { + uint32_t mem_reg_property[2]; + mem_reg_property[0] = cpu_to_fdt32(atag->u.mem.start); + mem_reg_property[1] = cpu_to_fdt32(atag->u.mem.size); + setprop(dt, "/memory", "reg", mem_reg_property, + sizeof(mem_reg_property)); + } else if (atag->hdr.tag == ATAG_INITRD2) { + uint32_t initrd_start, initrd_size; + initrd_start = atag->u.initrd.start; + initrd_size = atag->u.initrd.size; + setprop_cell(dt, "/chosen", "linux,initrd-start", + initrd_start); + setprop_cell(dt, "/chosen", "linux,initrd-end", + initrd_start + initrd_size); + } + } + + return 0; +} diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S index d4f8db2..5a0a2409 100644 --- a/arch/arm/boot/compressed/head.S +++ b/arch/arm/boot/compressed/head.S @@ -246,6 +246,36 @@ restart: adr r0, LC0 cmp lr, r1 bne dtb_check_done @ not found +#ifdef CONFIG_ARM_ATAG_DTB_COMPAT + /* + * OK... Let's do some funky business here. + * If we do have a DTB appended to zImage, and we do have + * an ATAG list around, we want the later to be translated + * and folded into the former here. To be on the safe side, + * let's temporarily move the stack away into the malloc + * area. No GOT fixup has occurred yet, but none of the + * code we're about to call uses any global variable. + */ + add sp, sp, #0x10000 + stmfd sp!, {r0-r3, ip, lr} + mov r0, r6 + mov r1, r8 + bl atags_to_fdt + + /* + * If that didn't work (non-zero return), there is no ATAG + * at the location pointed by r8. Try the typical 0x100 + * offset from start of RAM. + */ + cmp r0, #0 + mov r0, r6 + sub r1, r4, #(TEXT_OFFSET - 0x100) + blne atags_to_fdt + + ldmfd sp!, {r0-r3, ip, lr} + sub sp, sp, #0x10000 +#endif + mov r8, r6 @ use the appended device tree /* diff --git a/arch/arm/boot/compressed/libfdt_env.h b/arch/arm/boot/compressed/libfdt_env.h new file mode 100644 index 0000000..1f4e718 --- /dev/null +++ b/arch/arm/boot/compressed/libfdt_env.h @@ -0,0 +1,15 @@ +#ifndef _ARM_LIBFDT_ENV_H +#define _ARM_LIBFDT_ENV_H + +#include <linux/types.h> +#include <linux/string.h> +#include <asm/byteorder.h> + +#define fdt16_to_cpu(x) be16_to_cpu(x) +#define cpu_to_fdt16(x) cpu_to_be16(x) +#define fdt32_to_cpu(x) be32_to_cpu(x) +#define cpu_to_fdt32(x) cpu_to_be32(x) +#define fdt64_to_cpu(x) be64_to_cpu(x) +#define cpu_to_fdt64(x) cpu_to_be64(x) + +#endif diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c index 832d372..747de6b 100644 --- a/arch/arm/boot/compressed/misc.c +++ b/arch/arm/boot/compressed/misc.c @@ -136,6 +136,55 @@ void *memcpy(void *__dest, __const void *__src, size_t __n) return __dest; } +void *memmove(void *__dest, __const void *__src, size_t __n) +{ + unsigned char *d = __dest; + const unsigned char *s = __src; + + if (__dest <= __src) + return memcpy(__dest, __src, __n); + + while (--__n >= 0) + d[__n] = s[__n]; + + return __dest; +} + +size_t strlen(const char *s) +{ + const char *sc; + + for (sc = s; *sc != '\0'; ++sc) + /* nothing */; + return sc - s; +} + +int memcmp(const void *cs, const void *ct, size_t count) +{ + const unsigned char *su1, *su2; + int res = 0; + + for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--) + if ((res = *su1 - *su2) != 0) + break; + return res; +} + +int strcmp(const char *cs, const char *ct) +{ + unsigned char c1, c2; + + while (1) { + c1 = *cs++; + c2 = *ct++; + if (c1 != c2) + return c1 < c2 ? -1 : 1; + if (!c1) + break; + } + return 0; +} + /* * gzip declarations */