Appended DTB files for multi-machine kernels

Appended DTB files for multi-machine kernels

[Daniel Mack]

Hi,

I'm facing a problem with a transition from legacy board-file driven ARM
machines to DTB, and I'm under the impression that a solution for it
could be of broader interest.

In short, devices that have been deployed in quantities come in three
hardware variants, which all boot with a unique machine-id. We ship
kernel images that have board support for all three machine types, and
do minor fixups to platform data of some drivers at runtime, depending
on the board revision number (passed in via ATAGs).

The built-in support for attaching a DTB to the zImage does not suffice
here, because we have one image for all models, and also, we couldn't do
a 'per-board-revision' selection that way either.

Unless I missed some recent discussion, this case is not easy to handle.
Yes, I know that these kind of things should be handled by a
next-generation bootloader, but in our case, we want to avoid a loader
update of already shipped hardware by all means.

As a solution, I'm thinking of a small framework that could for example
work as follows.

a) A small mechanism allows storing multiple DTB binary files inside the
kernel binary at compile time, and a simple function can extract them
again by name at runtime (something like what the firmware framework
does, but I don't know if that one can be used at such an early stage in
the boot process).

b) A DT_MACHINE_START-like macro takes both the machine ID and the name
of a DTB file that is compiled in. When matched, generic functions would
load the given file, populate the device tree and then conduct a generic
DT boot for the specified platform.

c) Allow users to open-code the DTB lookup depending on whatever kind of
runtime information (be it the board_revision or anything else).


Of course, everything has to be an opt-in that stubs itself out at zero
costs if not needed.


I'm open to opinion and sugesstions :)


Thanks,
Daniel

Re: Appended DTB files for multi-machine kernels

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1237 bytes --]

On Thu, Jul 04, 2013 at 06:56:24PM +0200, Daniel Mack wrote:

> Unless I missed some recent discussion, this case is not easy to handle.
> Yes, I know that these kind of things should be handled by a
> next-generation bootloader, but in our case, we want to avoid a loader
> update of already shipped hardware by all means.

There was some discussion about appending multiple DTBs recently.  I
can't actually recall anything about it though so that's not an entirely
helpful thing...

> As a solution, I'm thinking of a small framework that could for example
> work as follows.

> a) A small mechanism allows storing multiple DTB binary files inside the
> kernel binary at compile time, and a simple function can extract them
> again by name at runtime (something like what the firmware framework
> does, but I don't know if that one can be used at such an early stage in
> the boot process).

Another way of skinning this would be for either the kernel to contain
a set of machine ID to compatible string mappings or for the device
trees for the boards to have an additional properties giving the machine
IDs that the boards match.  The kernel could then look for multiple DTBs
appended to the image and try to pick one based on ATAGs.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: Appended DTB files for multi-machine kernels

[Nicolas Pitre]

On Thu, 4 Jul 2013, Daniel Mack wrote:

> Hi,
> 
> I'm facing a problem with a transition from legacy board-file driven ARM
> machines to DTB, and I'm under the impression that a solution for it
> could be of broader interest.
> 
> In short, devices that have been deployed in quantities come in three
> hardware variants, which all boot with a unique machine-id. We ship
> kernel images that have board support for all three machine types, and
> do minor fixups to platform data of some drivers at runtime, depending
> on the board revision number (passed in via ATAGs).
> 
> The built-in support for attaching a DTB to the zImage does not suffice
> here, because we have one image for all models, and also, we couldn't do
> a 'per-board-revision' selection that way either.
> 
> Unless I missed some recent discussion, this case is not easy to handle.
> Yes, I know that these kind of things should be handled by a
> next-generation bootloader, but in our case, we want to avoid a loader
> update of already shipped hardware by all means.
> 
> As a solution, I'm thinking of a small framework that could for example
> work as follows.
> 
> a) A small mechanism allows storing multiple DTB binary files inside the
> kernel binary at compile time, and a simple function can extract them
> again by name at runtime (something like what the firmware framework
> does, but I don't know if that one can be used at such an early stage in
> the boot process).
> 
> b) A DT_MACHINE_START-like macro takes both the machine ID and the name
> of a DTB file that is compiled in. When matched, generic functions would
> load the given file, populate the device tree and then conduct a generic
> DT boot for the specified platform.
> 
> c) Allow users to open-code the DTB lookup depending on whatever kind of
> runtime information (be it the board_revision or anything else).
> 
> 
> Of course, everything has to be an opt-in that stubs itself out at zero
> costs if not needed.
> 
> 
> I'm open to opinion and sugesstions :)

What you describe above more or less fits the definition of what I 
called the "impedance matcher".  However it doesn't need to be part of 
the kernel at all.  But you should make it into a separate binary.

Please have a look at the bottom of this post for a more comprehensive 
description: http://article.gmane.org/gmane.linux.ports.arm.kernel/242929


Nicolas

Re: Appended DTB files for multi-machine kernels

[Daniel Mack]

Hi Nicolas,

On 04.07.2013 19:28, Nicolas Pitre wrote:
> On Thu, 4 Jul 2013, Daniel Mack wrote:
>> I'm open to opinion and sugesstions :)
> 
> What you describe above more or less fits the definition of what I 
> called the "impedance matcher".  However it doesn't need to be part of 
> the kernel at all.  But you should make it into a separate binary.
> 
> Please have a look at the bottom of this post for a more comprehensive 
> description: http://article.gmane.org/gmane.linux.ports.arm.kernel/242929

Thanks for the link. Interesting read indeed.

The only thing I'm pondering about is that we already have generic
loader code in Linux and a build system that supports a wide range of
platforms. I don't know whether it's worth opening that can of worms
again and re-implement all details about load addresses, compiler flags,
device-tree parsing code yet once again. At least in terms of LOCs, we
would certainly be better off doing it inside the kernel.

But I generally second your opinion about pushing vendors to do it
right, so I might give that approach a shot soon; there is no code yet
anywhere I take it?


Thanks,
Daniel

Re: Appended DTB files for multi-machine kernels

[Nicolas Pitre]

On Thu, 4 Jul 2013, Daniel Mack wrote:

> Hi Nicolas,
> 
> On 04.07.2013 19:28, Nicolas Pitre wrote:
> > On Thu, 4 Jul 2013, Daniel Mack wrote:
> >> I'm open to opinion and sugesstions :)
> > 
> > What you describe above more or less fits the definition of what I 
> > called the "impedance matcher".  However it doesn't need to be part of 
> > the kernel at all.  But you should make it into a separate binary.
> > 
> > Please have a look at the bottom of this post for a more comprehensive 
> > description: http://article.gmane.org/gmane.linux.ports.arm.kernel/242929
> 
> Thanks for the link. Interesting read indeed.
> 
> The only thing I'm pondering about is that we already have generic
> loader code in Linux and a build system that supports a wide range of
> platforms. I don't know whether it's worth opening that can of worms
> again and re-implement all details about load addresses, compiler flags,
> device-tree parsing code yet once again. At least in terms of LOCs, we
> would certainly be better off doing it inside the kernel.

That is still code with relatively low value that has to be carried and 
maintained.

And if you do it for your own device outside of the kernel then you can 
literally hack it the way you want quickly and forget about it once it 
works.  Since this is useful for legacy setups only, you do not have to 
care about maintaining it in the future.

> But I generally second your opinion about pushing vendors to do it
> right, so I might give that approach a shot soon; there is no code yet
> anywhere I take it?

Not that I'm aware of, besides parts of libfdt and atags_to_fdt.c from 
the kernel tree that can be lifted and modified according to your needs.  
I don't imagine you'd need much more than that anyway.


Nicolas

Re: Appended DTB files for multi-machine kernels

[Dirk Behme]

Am 04.07.2013 18:56, schrieb Daniel Mack:
> Hi,
>
> I'm facing a problem with a transition from legacy board-file driven ARM
> machines to DTB, and I'm under the impression that a solution for it
> could be of broader interest.
>
> In short, devices that have been deployed in quantities come in three
> hardware variants, which all boot with a unique machine-id. We ship
> kernel images that have board support for all three machine types, and
> do minor fixups to platform data of some drivers at runtime, depending
> on the board revision number (passed in via ATAGs).
>
> The built-in support for attaching a DTB to the zImage does not suffice
> here, because we have one image for all models, and also, we couldn't do
> a 'per-board-revision' selection that way either.
>
> Unless I missed some recent discussion, this case is not easy to handle.
> Yes, I know that these kind of things should be handled by a
> next-generation bootloader, but in our case, we want to avoid a loader
> update of already shipped hardware by all means.
>
> As a solution, I'm thinking of a small framework that could for example
> work as follows.
>
> a) A small mechanism allows storing multiple DTB binary files inside the
> kernel binary at compile time, and a simple function can extract them
> again by name at runtime (something like what the firmware framework
> does, but I don't know if that one can be used at such an early stage in
> the boot process).
>
> b) A DT_MACHINE_START-like macro takes both the machine ID and the name
> of a DTB file that is compiled in. When matched, generic functions would
> load the given file, populate the device tree and then conduct a generic
> DT boot for the specified platform.
>
> c) Allow users to open-code the DTB lookup depending on whatever kind of
> runtime information (be it the board_revision or anything else).
>
>
> Of course, everything has to be an opt-in that stubs itself out at zero
> costs if not needed.
>
>
> I'm open to opinion and sugesstions :)

Depending on how different the board support for all your three 
machine types is, there might be an (of course unimplemented) option

d) have only one DTB file which contains all the information for your 
three machine types and then have a mechanism which selects 
("activates") the entries valid for the board you boot based on some 
external information, e.g. the machine ID. I.e. you have one DTB file 
which contains alternative entries for the different boards. And which 
entry is used/activated is selected at (early) kernel runtime by the 
machine ID.

What I'm thinking about here is an extended version of the device tree 
overlays used (invented?) by the Beagle board community. They have a 
base device tree file and for their add on boards (capes) they load 
device tree overlay files based on an EEPROM ID read.

http://elinux.org/BeagleBone_and_the_3.8_Kernel#Cape_Manager_and_Device_Tree_Overlays

With this, they extend the device tree at runtime based on the 
attached extension boards.

Going one step further, having the alternative plugin/overlay 
definitions already in the initial DTB file and being able to 
select/activate single plugin/overlay definitions already in the early 
boot stage should give a general mechanism to support several, similar 
boards with slightly different configurations with one DTB.

Best regards

Dirk

Re: Appended DTB files for multi-machine kernels

[Arnd Bergmann]

On Thursday 04 July 2013, Mark Brown wrote:
> On Thu, Jul 04, 2013 at 06:56:24PM +0200, Daniel Mack wrote:
> 
> > Unless I missed some recent discussion, this case is not easy to handle.
> > Yes, I know that these kind of things should be handled by a
> > next-generation bootloader, but in our case, we want to avoid a loader
> > update of already shipped hardware by all means.
> 
> There was some discussion about appending multiple DTBs recently.  I
> can't actually recall anything about it though so that's not an entirely
> helpful thing...

Yes, it keeps coming up, I think by now everybody agreed it's a good
idea to extend the ATAGS compat mode, but so far nobody has implemented
it. Magnus Damm was very interested in this feature last year and
was planning to work on it, but I don't know how far he got.

> > As a solution, I'm thinking of a small framework that could for example
> > work as follows.
> 
> > a) A small mechanism allows storing multiple DTB binary files inside the
> > kernel binary at compile time, and a simple function can extract them
> > again by name at runtime (something like what the firmware framework
> > does, but I don't know if that one can be used at such an early stage in
> > the boot process).
> 
> Another way of skinning this would be for either the kernel to contain
> a set of machine ID to compatible string mappings or for the device
> trees for the boards to have an additional properties giving the machine
> IDs that the boards match.  The kernel could then look for multiple DTBs
> appended to the image and try to pick one based on ATAGs.

IIRC there is actually an unused field in the dtb header that could
be used to match a board ID.

	Arnd

Re: Appended DTB files for multi-machine kernels

[Daniel Mack]

Hi Arnd,

On 04.07.2013 23:34, Arnd Bergmann wrote:
> On Thursday 04 July 2013, Mark Brown wrote:

>> Another way of skinning this would be for either the kernel to contain
>> a set of machine ID to compatible string mappings or for the device
>> trees for the boards to have an additional properties giving the machine
>> IDs that the boards match.  The kernel could then look for multiple DTBs
>> appended to the image and try to pick one based on ATAGs.
> 
> IIRC there is actually an unused field in the dtb header that could
> be used to match a board ID.

While that would help many users, it doesn't cover our case where
different board revisions require different DTBs to be loaded, depending
on the supplied ATAG.

I think I'll go for Nicolas Pitre's suggestion as first step. He's right
that's it's a hack anyway, and we're just dealing with legacy systems
here, and hence it might not even be worth caring too much about a
proper solution.


Thanks for all the answers,
Daniel

Re: Appended DTB files for multi-machine kernels

[Magnus Damm]

Hi Arnd,

On Fri, Jul 5, 2013 at 6:34 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Thursday 04 July 2013, Mark Brown wrote:
>> On Thu, Jul 04, 2013 at 06:56:24PM +0200, Daniel Mack wrote:
>>
>> > Unless I missed some recent discussion, this case is not easy to handle.
>> > Yes, I know that these kind of things should be handled by a
>> > next-generation bootloader, but in our case, we want to avoid a loader
>> > update of already shipped hardware by all means.
>>
>> There was some discussion about appending multiple DTBs recently.  I
>> can't actually recall anything about it though so that's not an entirely
>> helpful thing...
>
> Yes, it keeps coming up, I think by now everybody agreed it's a good
> idea to extend the ATAGS compat mode, but so far nobody has implemented
> it. Magnus Damm was very interested in this feature last year and
> was planning to work on it, but I don't know how far he got.

Correct, I was and still am a bit interested in it. I did however
never get around to write any code. My apologies if that stalled
anyone.

With mach-shmobile we're now getting to a point where most of our
boards are using DT_MACHINE and appended DTB instead of the MACHINE
and mach-type. Because of that I feel that the moment has sort of
passed for me.

Regardless of this nearly all our boot loaders pass mach-type so it
would be nice to match on that instead of the appended DTB.

Thanks,

/ magnus

Re: Appended DTB files for multi-machine kernels

[Stephen Warren]

On 07/04/2013 10:56 AM, Daniel Mack wrote:
...
> The built-in support for attaching a DTB to the zImage does not suffice
> here, because we have one image for all models, and also, we couldn't do
> a 'per-board-revision' selection that way either.
...
> As a solution, I'm thinking of a small framework that could for example
> work as follows.
> 
> a) A small mechanism allows storing multiple DTB binary files inside the
> kernel binary at compile time...
...
> c) Allow users to open-code the DTB lookup depending on whatever kind of
> runtime information (be it the board_revision or anything else).

The problem with (c) is that in general it probably requires a lot of
infra-structure to be working to implement it. On Tegra, the board
revision is in an EEPROM attached to an I2C bus. You'd need to have
parsed DT and started instantiating devices already before you could
access the ID EEPROM to know which board revision you're running on.
That's a bit of a chicked-and-egg problem. Even with a much simpler
scheme such as board ID encoded into GPIOs, you'll still likely run into
similar problems; pinmux and GPIO controller drivers are needed just to
read the GPIOs, which then requires DT to instantiate those drivers.

So, the suggestion of Nicolas's that you mentioned does solve that
rather neatly.

Re: Appended DTB files for multi-machine kernels

[Daniel Mack]

On 04.07.2013 19:28, Nicolas Pitre wrote:
> On Thu, 4 Jul 2013, Daniel Mack wrote:
>> In short, devices that have been deployed in quantities come in three
>> hardware variants, which all boot with a unique machine-id. We ship
>> kernel images that have board support for all three machine types, and
>> do minor fixups to platform data of some drivers at runtime, depending
>> on the board revision number (passed in via ATAGs).
>>

> What you describe above more or less fits the definition of what I 
> called the "impedance matcher".  However it doesn't need to be part of 
> the kernel at all.  But you should make it into a separate binary.
> 
> Please have a look at the bottom of this post for a more comprehensive 
> description: http://article.gmane.org/gmane.linux.ports.arm.kernel/242929

FWIW, I gave that approach a try today for a PXA based hardware
platform, and in fact it seems to work very well. Thanks again for
sharing the idea.

In case anyone is interested, the repository is here:

  https://github.com/zonque/pxa-impedance-matcher

As the README says, it's completely specific to my use case, but maybe
anyone can use the code base to spin a different version for a different
board.


Daniel

Re: Appended DTB files for multi-machine kernels

[Nicolas Pitre]

On Fri, 26 Jul 2013, Daniel Mack wrote:

> On 04.07.2013 19:28, Nicolas Pitre wrote:
> > On Thu, 4 Jul 2013, Daniel Mack wrote:
> >> In short, devices that have been deployed in quantities come in three
> >> hardware variants, which all boot with a unique machine-id. We ship
> >> kernel images that have board support for all three machine types, and
> >> do minor fixups to platform data of some drivers at runtime, depending
> >> on the board revision number (passed in via ATAGs).
> >>
> 
> > What you describe above more or less fits the definition of what I 
> > called the "impedance matcher".  However it doesn't need to be part of 
> > the kernel at all.  But you should make it into a separate binary.
> > 
> > Please have a look at the bottom of this post for a more comprehensive 
> > description: http://article.gmane.org/gmane.linux.ports.arm.kernel/242929
> 
> FWIW, I gave that approach a try today for a PXA based hardware
> platform, and in fact it seems to work very well. Thanks again for
> sharing the idea.
> 
> In case anyone is interested, the repository is here:
> 
>   https://github.com/zonque/pxa-impedance-matcher
> 
> As the README says, it's completely specific to my use case, but maybe
> anyone can use the code base to spin a different version for a different
> board.

That looks nice, but I'd suggest going a step further if possible.

If you could load the kernel zImage independently with a separate load 
command in u-boot, then people could update their kernel only without 
having to relink with this shim.  The load address for that zImage can 
be totally arbitrary as longas it is known to be the same from within 
this shim and in the u-boot script.

Also, more related to your code, you apparently link and load it to run 
at 0xa0008000.  This is also where the zImage code is going to 
decompress the final kernel image after it has relocated itself.  I'm 
afraid the DTBs you point to might be overwritten at that point since 
zImage is not aware of them (in fact I'm surprized this works at all as 
you say).  If I were you I'd use 0xa1000000 instead (and make sure the 
initrd is out of the way as well if you have one).


Nicolas

[RFC PATCH 0/2] impedance-matcher generic improvements

[Jason Cooper]

Daniel,

Here's a patch (plus a cscope target) to attempt to make the impedance-matcher
generic code.  Hopefully I'll have time to test tomorrow.  All I can say atm is
that it compiles cleanly with and without specifying an appended zImage.

Warning: the code in string.c is little better than psuedo-code, maybe not even
that good.  I'm posting this so if anyone else is inclined to work on this,
there's no need to duplicate effort.  And, I have a firm belief in the
motivating effects af posting half-assed code.  ;-)

Basic idea is to replace a device's stock kernel with this impedance-matcher.
Load the kernel and/or the devicetree blob to desired addresses.  Add
'loadaddrs=0xXXXXXXXX,0xYYYYYYYY' to the kernel's command line.  Or, if you are
appending, 'loadaddrs=appended,0xYYYYYYYY'.  First address is the location of
the kernel, second is the location of the dtb.

Then, for u-boot, 'bootm 0xZZZZZZZZ' where 0xZZZZZZZZ is the address of the
impedance-matcher.

For those interested, a public branch can be found at (prone to rebase!):

  git://git.infradead.org/users/jcooper/impedance-matcher.git master

Thomas,

You weren't in the original Cc: of this thread, I added you because this may
help with your shifting register base address issues.

thx,

Jason.

Jason Cooper (2):
  add cscope Makefile target
  WIP: Get kernel and dtb addresses from command line

 .gitignore |  1 +
 Makefile   | 31 ++++++++++------------
 atags.h    |  6 +++++
 board.c    | 88 --------------------------------------------------------------
 board.h    | 13 ----------
 led.c      | 54 --------------------------------------
 led.h      |  6 -----
 main.c     | 52 +++++++++++++++++++++----------------
 string.c   | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 string.h   |  8 ++++++
 10 files changed, 141 insertions(+), 201 deletions(-)
 delete mode 100644 board.c
 delete mode 100644 board.h
 delete mode 100644 led.c
 delete mode 100644 led.h
 create mode 100644 string.c
 create mode 100644 string.h

-- 
1.8.3.2

[PATCH 1/2] add cscope Makefile target

[Jason Cooper]

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
 .gitignore | 1 +
 Makefile   | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/.gitignore b/.gitignore
index c3b59bf..ac4198c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,6 +1,7 @@
 *.sh
 *.o
 *.bin
+cscope.*
 matcher
 uImage
 input/zImage
diff --git a/Makefile b/Makefile
index 5ad5fc0..daeafa4 100644
--- a/Makefile
+++ b/Makefile
@@ -43,3 +43,9 @@ uImage: matcher.bin
 clean:
 	rm -fr matcher.bin matcher *.o uImage
 
+distclean:
+	rm -fr cscope.*
+
+cscope:
+	-find . -name "*.[chS]" >cscope.files
+	-cscope -b -R
-- 
1.8.3.2

[RFC PATCH 2/2] WIP: Get kernel and dtb addresses from command line

[Jason Cooper]

Allow for detached kernel and dtb images.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
 Makefile | 25 ++++++------------
 atags.h  |  6 +++++
 board.c  | 88 ----------------------------------------------------------------
 board.h  | 13 ----------
 led.c    | 54 ---------------------------------------
 led.h    |  6 -----
 main.c   | 52 +++++++++++++++++++++-----------------
 string.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 string.h |  8 ++++++
 9 files changed, 134 insertions(+), 201 deletions(-)
 delete mode 100644 board.c
 delete mode 100644 board.h
 delete mode 100644 led.c
 delete mode 100644 led.h
 create mode 100644 string.c
 create mode 100644 string.h

diff --git a/Makefile b/Makefile
index daeafa4..deb76b9 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,10 @@
-CFLAGS=-Wall -ffreestanding
+ifeq ($(origin APPEND_IMAGE), undefined)
+DOAPPEND=
+else
+DOAPPEND=-DAPPENDED_IMAGE=1
+endif
+
+CFLAGS=-Wall -ffreestanding $(DOAPPEND)
 LDFLAGS=-static -nostdlib
 GCC=$(CROSS_COMPILE)gcc
 OBJCOPY=$(CROSS_COMPILE)objcopy
@@ -6,30 +12,15 @@ LD=$(CROSS_COMPILE)ld
 LOADADDR=0xa0008000
 BINFMT=elf32-littlearm
 
-INPUT_OBJS = \
-	zimage.o		\
-	dtb-raumfeld-controller-0.o	\
-	dtb-raumfeld-controller-1.o	\
-	dtb-raumfeld-controller-2.o	\
-	dtb-raumfeld-connector-0.o	\
-	dtb-raumfeld-connector-1.o	\
-	dtb-raumfeld-connector-2.o	\
-	dtb-raumfeld-speaker-0.o	\
-	dtb-raumfeld-speaker-1.o	\
-	dtb-raumfeld-speaker-2.o
-
 all: uImage
 
-dtb-%.o: input/%.dtb
-	$(OBJCOPY) -I binary -O $(BINFMT) -B arm $^ $@
-
 zimage.o: input/zImage
 	$(OBJCOPY) -I binary -O $(BINFMT) -B arm $^ $@
 
 %.o: %.c
 	$(GCC) $(CFLAGS) -c $^
 
-matcher: main.o print.o board.o led.o $(INPUT_OBJS)
+matcher: main.o print.o string.o $(APPEND_IMAGE)
 	$(LD) $(LDFLAGS) -T matcher.lds -o $@ $^
 
 matcher.bin: matcher
diff --git a/atags.h b/atags.h
index 30e6ee7..67d3d01 100644
--- a/atags.h
+++ b/atags.h
@@ -10,15 +10,21 @@ struct tag_header {
 
 /* board revision */
 #define ATAG_REVISION   0x54410007
+#define ATAG_CMDLINE	0x54410009
 
 struct tag_revision {
 	__u32 rev;
 };
 
+struct tag_cmdline {
+	char cmdline[1];     /* this is the minimum size */
+};
+
 struct tag {
 	struct tag_header hdr;
 	union {
 		struct tag_revision	rev;
+		struct tag_cmdline	cmdline;
 	} u;
 };
 
diff --git a/board.c b/board.c
deleted file mode 100644
index f9c2425..0000000
--- a/board.c
+++ /dev/null
@@ -1,88 +0,0 @@
-#include "types.h"
-#include "board.h"
-
-extern __u32 _binary_input_zImage_start;
-extern __u32 _binary_input_raumfeld_controller_0_dtb_start;
-extern __u32 _binary_input_raumfeld_controller_1_dtb_start;
-extern __u32 _binary_input_raumfeld_controller_2_dtb_start;
-extern __u32 _binary_input_raumfeld_connector_0_dtb_start;
-extern __u32 _binary_input_raumfeld_connector_1_dtb_start;
-extern __u32 _binary_input_raumfeld_connector_2_dtb_start;
-extern __u32 _binary_input_raumfeld_speaker_0_dtb_start;
-extern __u32 _binary_input_raumfeld_speaker_1_dtb_start;
-extern __u32 _binary_input_raumfeld_speaker_2_dtb_start;
-
-static struct board boards[] = {
-	/* Controller */
-	{
-		.machid		= 2413,
-		.system_rev	= 0,
-		.dtb		= &_binary_input_raumfeld_controller_0_dtb_start,
-		.name		= "Raumfeld Controller, revision 0",
-	},
-	{
-		.machid		= 2413,
-		.system_rev	= 1,
-		.dtb		= &_binary_input_raumfeld_controller_1_dtb_start,
-		.name		= "Raumfeld Controller, revision 1",
-	},
-	{
-		.machid		= 2413,
-		.system_rev	= 2,
-		.dtb		= &_binary_input_raumfeld_controller_2_dtb_start,
-		.name		= "Raumfeld Controller, revision 2",
-	},
-
-	/* Controller */
-	{
-		.machid		= 2414,
-		.system_rev	= 0,
-		.dtb		= &_binary_input_raumfeld_connector_0_dtb_start,
-		.name		= "Raumfeld Connector, revision 0",
-	},
-	{
-		.machid		= 2414,
-		.system_rev	= 1,
-		.dtb		= &_binary_input_raumfeld_connector_1_dtb_start,
-		.name		= "Raumfeld Connector, revision 1",
-	},
-	{
-		.machid		= 2414,
-		.system_rev	= 2,
-		.dtb		= &_binary_input_raumfeld_connector_2_dtb_start,
-		.name		= "Raumfeld Connector, revision 2",
-	},
-
-	/* Speaker */
-	{
-		.machid		= 2415,
-		.system_rev	= 0,
-		.dtb		= &_binary_input_raumfeld_speaker_0_dtb_start,
-		.name		= "Raumfeld Speaker, revision 0",
-	},
-	{
-		.machid		= 2415,
-		.system_rev	= 1,
-		.dtb		= &_binary_input_raumfeld_speaker_1_dtb_start,
-		.name		= "Raumfeld Speaker, revision 1",
-	},
-	{
-		.machid		= 2415,
-		.system_rev	= 2,
-		.dtb		= &_binary_input_raumfeld_speaker_2_dtb_start,
-		.name		= "Raumfeld Speaker, revision 2",
-	},
-	{ 0, 0, NULL, NULL }	/* sentinel */
-};
-
-struct board *match_board(__u32 machid, __u32 revision)
-{
-	struct board *board;
-
-	for (board = boards; board->machid; board++)
-		if (board->machid == machid && board->system_rev == revision)
-			return board;
-
-	return NULL;
-}
-
diff --git a/board.h b/board.h
deleted file mode 100644
index b256a6c..0000000
--- a/board.h
+++ /dev/null
@@ -1,13 +0,0 @@
-#ifndef _BOARD_H
-#define _BOARD_H
-
-struct board {
-	__u32		machid;
-	__u32		system_rev;
-	void		*dtb;
-	const char	*name;
-};
-
-struct board *match_board(__u32 machid, __u32 revision);
-
-#endif
diff --git a/led.c b/led.c
deleted file mode 100644
index 837d961..0000000
--- a/led.c
+++ /dev/null
@@ -1,54 +0,0 @@
-#include "types.h"
-#include "led.h"
-
-static inline void writel(__u32 val, __u32 addr)
-{
-	*(volatile __u32 *) addr = val;
-}
-
-static inline __u32 readl(__u32 addr)
-{
-	return *(volatile __u32 *) addr;
-}
-
-static void wait(__u32 ticks)
-{
-	__u32 v;
-
-	/* OSCR */
-	writel(0, 0x40A00010);
-
-	do {
-		v = readl(0x40A00010);
-	} while (ticks > v);
-}
-
-static void led_init(void)
-{
-	writel(0, 0x40e10420);		/* GPIO35 */
-	writel(0, 0x40e10424);		/* GPIO36 */
-	writel(0x18, 0x40e00010);	/* GPDR1 */
-}
-
-static void led_set(__u32 index, __u32 state)
-{
-	__u32 v = 1 << (index ? 3 : 4);
-
-	if (state)
-		writel(v, 0x40e0001c);
-	else
-		writel(v, 0x40e00028);
-}
-
-void led_panic(void)
-{
-	int i;
-
-	led_init();
-
-	for (i = 0;; i++) {
-		led_set(0, i & 1);
-		led_set(1, ~i & 1);
-		wait(500000);
-	}
-}
diff --git a/led.h b/led.h
deleted file mode 100644
index 0cdcf31..0000000
--- a/led.h
+++ /dev/null
@@ -1,6 +0,0 @@
-#ifndef _LED_H_
-#define _LED_H_
-
-void led_panic(void);
-
-#endif
diff --git a/main.c b/main.c
index 3dd6636..af2b6c8 100644
--- a/main.c
+++ b/main.c
@@ -1,45 +1,51 @@
 #include "types.h"
 #include "atags.h"
 #include "print.h"
-#include "board.h"
-#include "led.h"
+#include "string.h"
 
+#ifdef DOAPPENDED
 extern __u32 _binary_input_zImage_start;
+#endif
 
 void main(__u32 dummy, __u32 machid, const struct tag *tags)
 {
 	const struct tag *t;
-	struct board *board;
-	__u32 system_rev = 0;
-	void (*start_kernel)(__u32 dummy, __u32 machid, void *dtb) =
-		(void *) &_binary_input_zImage_start;
+	void *kernel=NULL;
+	void *dtb=NULL;
+	void (*start_kernel)(__u32 dummy, __u32 machid, void *dtb);
+
+#ifdef DOAPPENDED
+	start_kernel = (void *) &_binary_input_zImage_start;
+#endif
 
 	putstr("++ Impedance Matcher (3rd stage loader) ++\n");
 
 	/* walk the atags to determine the system revision */
 	for_each_tag(t, tags)
 		switch (t->hdr.tag) {
-			case ATAG_REVISION:
-				system_rev = t->u.rev.rev;
+			case ATAG_CMDLINE:
+				/*
+				 * look for load addresses
+				 * eg: loadaddrs=0x0800000,0x0700000
+				 * for           kernel    dtb
+				 */
+				getaddrs(&kernel, &dtb, t->u.cmdline.cmdline);
 				break;
 		}
 
-	board = match_board(machid, system_rev & 0xff);
-	if (!board) {
-		putstr("ERROR MATCHING BOARD!\n");
-		putstr("MACHID: 0x");
-		printhex(machid);
-		putstr("\n");
-		putstr("SYSTEM_REV: 0x");
-		printhex(system_rev);
-		putstr("\n");
-		led_panic();
-	}
-
-	putstr("Detected board: ");
-	putstr(board->name);
+	if (kernel) {
+		putstr("Kernel: 0x");
+		printhex((__u32)kernel);
+		start_kernel = kernel;
+	} else
+		putstr("Kernel: Appended");
+	putstr("\n");
+
+	putstr("DTB:    0x");
+	printhex((__u32)dtb);
 	putstr("\n");
 
 	putstr("Booting into Linux kernel ...\n");
-	start_kernel(0, 0xffffffff, board->dtb);
+	start_kernel(0, 0xffffffff, dtb);
+
 }
diff --git a/string.c b/string.c
new file mode 100644
index 0000000..8eb459f
--- /dev/null
+++ b/string.c
@@ -0,0 +1,83 @@
+#include "types.h"
+#include "string.h"
+
+int hexlut[1 + 'F' - '0'] = {
+	0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, /* 0 - 9 */
+	0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+	0xa, 0xb, 0xc, 0xd, 0xe, 0xf                      /* A - F */
+};
+
+int strncmp(const char *stra, const char *strb, int len)
+{
+	int diff=0;
+	const char *a = stra;
+	const char *b = strb;
+
+	while ((a - stra) <= len)
+		diff += *(a++) - *(b++);
+
+	return diff;
+}
+
+void *gethexaddr(const char *str, const char **end)
+{
+	const char *s = str;
+	void *addr = NULL;
+	int shift = 0;
+
+	/* set the end */
+	while (*s) {
+		if (*s != ' ' && *s != '\t' && *s != ',')
+			break;
+		s++;
+	}
+
+	if (!*s)
+		return NULL;
+
+	*end = s;
+	s = *end - 1;
+
+	while (s >= str && *s != 'x') {
+		/* we assume base16! */
+		int off = (*s > 'F') ? 0x20 : 0x0;
+		addr += hexlut[*s - ('0' + off)] << shift;
+		shift += 4;
+		s--;
+	}
+
+	return addr;
+}
+
+int getaddrs(void **k, void **d, const char *str)
+{
+	const char *s = str;
+	const char *end;
+
+	while (*(s++)) {
+		if (*s == 'l')
+			if (strncmp(s, "loadaddrs=", 10) == 0)
+				break;
+	}
+
+	if (!*s)
+		return -1;
+
+	s += 10; /* skip over 'loadaddrs=' */
+
+	/* allow address to be 'appended' for some use cases */
+	if (strncmp(s, "appended", 8) == 0) {
+		*k = NULL;
+		end = s + 8;
+	} else
+		*k = gethexaddr(s, &end);
+
+	if (*end == ',')
+		s = end + 1;
+	else
+		return -1;
+
+	*d = gethexaddr(s, NULL);
+
+	return 0;
+}
diff --git a/string.h b/string.h
new file mode 100644
index 0000000..14d6764
--- /dev/null
+++ b/string.h
@@ -0,0 +1,8 @@
+#ifndef _STRING_H_
+
+#include "types.h"
+
+int strncmp(const char *, const char *, int);
+void *gethexaddr(const char *, const char **);
+int getaddrs(void **, void **, const char *);
+#endif
-- 
1.8.3.2

Re: [RFC PATCH 0/2] impedance-matcher generic improvements

[Daniel Mack]

Hi Jason,

On 29.07.2013 23:23, Jason Cooper wrote:
> Basic idea is to replace a device's stock kernel with this impedance-matcher.
> Load the kernel and/or the devicetree blob to desired addresses.  Add
> 'loadaddrs=0xXXXXXXXX,0xYYYYYYYY' to the kernel's command line.  Or, if you are
> appending, 'loadaddrs=appended,0xYYYYYYYY'.  First address is the location of
> the kernel, second is the location of the dtb.

Nice to see that idea implemented! For me though, changing any of the
U-Boot variables is out of the question, and I need the matcher just the
way I implemented it, including the LED blink pattern to let the user
know if things go wrong.

Hence, I won't change to your generic version on my boards, and so I
think it might be best if you maintain your own tree for your approach.


Best regards,
Daniel

Re: [RFC PATCH 0/2] impedance-matcher generic improvements

[Jason Cooper]

On Tue, Jul 30, 2013 at 09:48:51AM +0200, Daniel Mack wrote:
> Hi Jason,
> 
> On 29.07.2013 23:23, Jason Cooper wrote:
> > Basic idea is to replace a device's stock kernel with this impedance-matcher.
> > Load the kernel and/or the devicetree blob to desired addresses.  Add
> > 'loadaddrs=0xXXXXXXXX,0xYYYYYYYY' to the kernel's command line.  Or, if you are
> > appending, 'loadaddrs=appended,0xYYYYYYYY'.  First address is the location of
> > the kernel, second is the location of the dtb.
> 
> Nice to see that idea implemented! For me though, changing any of the
> U-Boot variables is out of the question, and I need the matcher just the
> way I implemented it, including the LED blink pattern to let the user
> know if things go wrong.

Understandable.  After thinking about it a bit, I think I'm going to
rework this patch to allow appending both kernels and dtbs (as you had),
and abstract out the led blink so that your code would become a compile
time option.

> Hence, I won't change to your generic version on my boards, and so I
> think it might be best if you maintain your own tree for your approach.

hrmmm, I don't like forking on general principle.  Let me do the above
rework and see if that works for you, while adding the capabilities I
need.

When I'm done, you should just be able to do:

  $ make APPEND_KERNEL=input/zImage APPEND_DTBS=input/*.dtb uImage

And get the same result.

In the meantime, would you mind adding a license to your code?  If you
have no preference, I like GPLv2-only, but it's your code, so whatever
you like.

thx,

Jason.

Re: [RFC PATCH 0/2] impedance-matcher generic improvements

[Daniel Mack]

Hi Jason,

On 30.07.2013 15:17, Jason Cooper wrote:
> On Tue, Jul 30, 2013 at 09:48:51AM +0200, Daniel Mack wrote:
>> On 29.07.2013 23:23, Jason Cooper wrote:
>>> Basic idea is to replace a device's stock kernel with this impedance-matcher.
>>> Load the kernel and/or the devicetree blob to desired addresses.  Add
>>> 'loadaddrs=0xXXXXXXXX,0xYYYYYYYY' to the kernel's command line.  Or, if you are
>>> appending, 'loadaddrs=appended,0xYYYYYYYY'.  First address is the location of
>>> the kernel, second is the location of the dtb.
>>
>> Nice to see that idea implemented! For me though, changing any of the
>> U-Boot variables is out of the question, and I need the matcher just the
>> way I implemented it, including the LED blink pattern to let the user
>> know if things go wrong.
> 
> Understandable.  After thinking about it a bit, I think I'm going to
> rework this patch to allow appending both kernels and dtbs (as you had),
> and abstract out the led blink so that your code would become a compile
> time option.

And how would you match machids and system_rev values to the dtb files?
This is how I identify my boards at least. But as Stephen Warren said in
the thread that started this discussion, there might be totally
different ways of detection. For instance, other boards could read GPIO
lines, or even have a pseudo driver that initializes the I2C controller
in order to talk to an EEPROM or such. Reading from an ADC is another
option.

And the more I think about it, I believe Nicolas was right in saying
that such a thing does not need to be very generic (as much as I like
generic solutions in principle).

If at all, I think we should start thinking of that code base as
something like a framework, consisting of a merely empty main() function
body, a lds linker script and some objcopy magic in the Makefile. And
then have a menuconfig which allows to select one out of many hardware
types, which then uses its specific way of board detection, LED
signaling and so on. The two lines of vectoring to the actual kernel can
be generic then. But again, I really don't know if it's worth the hassle.

> hrmmm, I don't like forking on general principle.  Let me do the above
> rework and see if that works for you, while adding the capabilities I
> need.
> 
> When I'm done, you should just be able to do:
> 
>   $ make APPEND_KERNEL=input/zImage APPEND_DTBS=input/*.dtb uImage

Out of curiosity, I'd like to see how you dynamically create the symbol
definitions for the binary blobs :)

> In the meantime, would you mind adding a license to your code?  If you
> have no preference, I like GPLv2-only, but it's your code, so whatever
> you like.

Done, GPLv2 it is.


Thanks,
Daniel

Re: [RFC PATCH 0/2] impedance-matcher generic improvements

[Jason Cooper]

On Tue, Jul 30, 2013 at 03:37:38PM +0200, Daniel Mack wrote:
> Hi Jason,
> 
> On 30.07.2013 15:17, Jason Cooper wrote:
> > On Tue, Jul 30, 2013 at 09:48:51AM +0200, Daniel Mack wrote:
> >> On 29.07.2013 23:23, Jason Cooper wrote:
> >>> Basic idea is to replace a device's stock kernel with this impedance-matcher.
> >>> Load the kernel and/or the devicetree blob to desired addresses.  Add
> >>> 'loadaddrs=0xXXXXXXXX,0xYYYYYYYY' to the kernel's command line.  Or, if you are
> >>> appending, 'loadaddrs=appended,0xYYYYYYYY'.  First address is the location of
> >>> the kernel, second is the location of the dtb.
> >>
> >> Nice to see that idea implemented! For me though, changing any of the
> >> U-Boot variables is out of the question, and I need the matcher just the
> >> way I implemented it, including the LED blink pattern to let the user
> >> know if things go wrong.
> > 
> > Understandable.  After thinking about it a bit, I think I'm going to
> > rework this patch to allow appending both kernels and dtbs (as you had),
> > and abstract out the led blink so that your code would become a compile
> > time option.
> 
> And how would you match machids and system_rev values to the dtb files?

I'm thinking board-specific match functions.  For your use case, you
would match machid/system_rev to a compatible string.  Then, generic
code would scan the appended dtbs for a matching string.

> This is how I identify my boards at least. But as Stephen Warren said in
> the thread that started this discussion, there might be totally
> different ways of detection. For instance, other boards could read GPIO
> lines, or even have a pseudo driver that initializes the I2C controller
> in order to talk to an EEPROM or such. Reading from an ADC is another
> option.

As long as it resolves to a compatible string.

> And the more I think about it, I believe Nicolas was right in saying
> that such a thing does not need to be very generic (as much as I like
> generic solutions in principle).
> 
> If at all, I think we should start thinking of that code base as
> something like a framework, consisting of a merely empty main() function
> body, a lds linker script and some objcopy magic in the Makefile. And
> then have a menuconfig which allows to select one out of many hardware
> types, which then uses its specific way of board detection, LED
> signaling and so on. The two lines of vectoring to the actual kernel can
> be generic then. But again, I really don't know if it's worth the hassle.
> 
> > hrmmm, I don't like forking on general principle.  Let me do the above
> > rework and see if that works for you, while adding the capabilities I
> > need.
> > 
> > When I'm done, you should just be able to do:
> > 
> >   $ make APPEND_KERNEL=input/zImage APPEND_DTBS=input/*.dtb uImage
> 
> Out of curiosity, I'd like to see how you dynamically create the symbol
> definitions for the binary blobs :)

Me too.  ;-)

> > In the meantime, would you mind adding a license to your code?  If you
> > have no preference, I like GPLv2-only, but it's your code, so whatever
> > you like.
> 
> Done, GPLv2 it is.

Thanks!

thx,

Jason.