Bug#667681: flash-kernel: Please add support for Dreamplug / Marvell Kirkwood FDT
Hi Loïc,
I must confess that I hadn't properly read this mail yet when I produced
the attached patches etc, so there are probably some things which need
updating based upon your "Next steps". I figured it was worth getting
what I'd done out there as a starting point.
On Wed, 2012-04-11 at 18:46 +0200, Loïc Minier wrote:
> On Mon, Apr 09, 2012, Martin Michlmayr wrote:
> > * Ian Campbell <ijc@hellion.org.uk> [2012-04-06 09:49]:
> > > Good question. Some DT stuff gets exported in /proc/device-tree.
> > > $ echo $(cat /proc/device-tree/model)
> > > Globalscale Technologies Dreamplug
>
> But that doesn't say whether the DT was passed to the kernel or whether
> it was appended to the kernel image?
No, I'm not sure there is anyway to tell this, other than inferring
based on knowledge of the platform.
> It's good that we can identify the actual board though.
>
> > In that case, I believe you should modify your flash-kernel patch to
> > check this file so the code your proposed will only run on the
> > Dreamplug and not on other Kirkwood DT devices which might require
> > different behaviour.
>
> Right; IIUC we want to do these things:
> a) if /proc/device-tree/model exist, read machine from it rather than
> using Machine: in cpuinfo
The attached patch (flash-kernel-dt-model.patch) implements this.
> b) add support for either appending the DT to the kernel image, or for
> installing the DT (e.g. writing to flash or copying it to a specific
> SD partition etc.)
The attached patch (flash-kernel-install-dtb.patch) implements the copy
to SD partition bit. It depends on an update to my "include DTB in
linux-image files on armel" patch sent to debian-kernel@ last week (I'll
cc flash-kernel@packages.debian.org with the update).
> b) depends on the way the boards we care about boot; for instance if
> Debian provides SD card d-i images which contain u-boot with a certain
> behavior, this is typically the behavior we would rely on; however it
> might be important to align this with what u-boot does in devices
> coming out of the factory.
>
> I don't like requiring people to replace their bootloader if it's not
> on user-swappable media. If it's on user-swappable media, I prefer if
> it's included from the start in Debian images and updated regularly
> (with each u-boot package upgrade). But none of that exists sadly.
>
> Concerning appending the DT vs. copying it, it would be ideal if we
> could do the same thing for all boards; I guess appending the DT is
> universal and would work all the time, but it does require turning on a
> specific option in the kernel config. At the very least, we should
> attempt to check that the relevant config is turned on and break if we
> notice it's not.
My concern with the appending option is that the Kconfig option is
marked EXPERIMENTAL and the help says:
This is meant as a backward compatibility convenience for those
systems with a bootloader that can't be upgraded to accommodate
the documented boot protocol using a device tree.
Beware that there is very little in terms of protection against
this option being confused by leftover garbage in memory that might
look like a DTB header after a reboot if no actual DTB is appended
to zImage. Do not leave this option active in a production kernel
if you don't intend to always append a DTB. Proper passing of the
location into r2 of a bootloader provided DTB is always preferable
to this option.
My initial Dreamplug kernel patches don't turn this on, but there's no
fundamental reason why they couldn't, other than the concerns about the
help text.
It's hard to judge what the future will be like, upstream are strongly
preferring FDT based solutions but it's hard to know if/when this will
gain traction with suppliers (i.e. when they will start shipping it
turned on in u-boot by default).
I wonder if there is scope for writing a shim which can be prepended
onto the kernel to add DTB support in a safer way than the current
append support? I suppose that would be a thing to deal with upstream.
> Another final question is where the DT comes from; I expect it's ship
> pre-built in the linux-image package, just like vmlinuz, but per board.
> In Ubuntu, these are shipped as e.g.:
> /boot/dt-2.6.38-1002-linaro-omap/omap3-overo.dtb
I proposed a patch to do this on debian-kernel@ last week.
(http://lists.debian.org/debian-kernel/2012/04/msg00546.html). I've got
a followup which I'm just about to send which moves the image
to /usr/lib/linux-image-$kvers which matches the attached
flash-kernel-install-dtb.patch.
I wasn't aware of the Ubuntu solution, the path is easy to change
though. My reason for choosing /usr/lib was that /boot might be
relatively small and the dir needs to contain all the DTB files for the
flavour. They are pretty tiny though so this likely isn't a big deal.
> we need to be really careful that these are stable names (e.g. if linux
> upstream splits dreamplug.dtb into dreamplug-v1.dtb and
> dreamplug-v2.dtb, it will break flash-kernel).
I've no idea what upstream policy is, but I think we could convince them
to at least go to dreamplug.dtb and dreamplug-v2.dtb or to maintain a
compat symlink or something, or at the very least we could arrange this
in the kernel packaging.
> (There's another question in the back of my mind with DT: I think we
> can set things like cmdline args via DT; we could use that to set
> root= instead of using the initrd, but that's not supported in Debian
> right now and it's not obvious to me that we can post-process .dtb
> files easily to do this at the right time.)
This is one of the nodes under /chosen/, I think?
For a bootloader which supports dtb I think the bootloader does the
right thing itself and fills in that node from it's $bootcmd (my u-boot
does). Obviously for the APPEND mode it won't.
There is a second config option (CONFIG_ARM_ATAG_DTB_COMPAT) which
merges the ATAGS into the appended DTB, which includes translating the
ATAG_CMDLINE into a chosen node, I think. IMHO this stuff gets
increasingly fragile the more you try to do in it (it happens really
early in the boot).
> Next steps:
> * could we assume Debian-ish kernels for your plaform will provide the
> .dtb in a reliable location and use that as the .dtb file to install?
> what's the name of the .dtb file and is this available in Debian
> .debs?
I have patches to the kernel packaging which will make this be so.
> * could we assume Debian-ish kernels for your plaform turn on DTB
> append in the config?
My initial patchset currently does not, but it could if that turns out
to be preferred.
> * I see your patch adds an entry which requests generation of
> uImage/uInitrd, but the default upstream u-boot config doesn't
> include loading an initrd or a DT; what can we assume out of factory?
The factory supplied u-boot is somewhat broken -- in that it reuses the
guruplug machine ID, so nuked it fairly early on in favour of the Debian
u-boot since this was before the kernel patches using DT were available.
It doesn't support FDT, which would mean that append mode would be the
only one which works with the factory bootloader.
However there is another issue which is that it has the bug in #658904
which I think stops it from booting any new kernel at all, so it is
likely that an update will be required in any case.
The in-flash uboot env which my dreamplug was shipped with included
bootcmd runes to load a kernel (but not initrd) from the USB storage.
At least from the supplied I bought from I had the option of Debian
installed (one of those weird customised ones which people like to
ship..) to either the on-board micro-sd (sda) or an add-on card (sdb)
(I got mine from http://www.newit.co.uk/shop/products.php?cat=21 , FWIW)
Although the bootloader is not on user-swappable media the JTAG module
is reasonably cheaply available alongside the device and the s/w to
drive it is all in Debian. You also need the same module to get a serial
console out of the device so I'd imagine most people would want to have
at least one to hand.
> * can we ship u-boot in Debian images so that we can assume which
> features/config u-boot has?
u-boot (and the env) is in a separate SPI flash part, I think that means
the answer here is no to shipping it in images? However based on the
presence of #658904 I think it might unfortunately be required to update
u-boot to the Debian version as a pre-install step anyway.
The dreamplug u-boot in Debian will support FDT after #667680 is
applied.
> * you request installation of uImage/uInitrd into /boot; that's what
> other similar platforms do, but my preference would be to use a
> partition device name instead (Boot-Device:) just like for
> "OMAP4 Panda board"; I find this is a bit nicer for multiple reasons:
> + it allows /boot to be on any device to store possibly large and
> numerous vmlinuz files
> + it doesn't require support for e.g. symlinks in the filesystem used
> for firmware files such as uImage/uInitrd (if it's mounted as /boot
> and you request /boot/initrd.img and /boot/vmlinuz symlinks on a
> filesystem which doesn't support symlinks, things break)
> + this doesn't keep the filesystem mounted all the time
> - there is a drawback that fsck might not be run automatically for
> that filesystem, but since it's shared with the bootloader it's
> probably a good idea
> so I'd prefer if you could use Boot-Device: whatever,
> Boot-Kernel-Path: uImage and Boot-Initrd-Path: uInitrd
Depending on whether you install the OS to the on-board micro-sd or an
add on card this would be either sda or sdb, I think that rules this
approach out?
> Hope this helps,
It did, thanks. Sorry that I seem to have raised more questions than
answers!
Ian.
--
Ian Campbell
I've been on this lonely road so long,
Does anybody know where it goes,
I remember last time the signs pointed home,
A month ago.
-- Carpenters, "Road Ode"
diff --git a/db/all.db b/db/all.db
index 2ce4645..c59ec94 100644
--- a/db/all.db
+++ b/db/all.db
@@ -145,6 +145,15 @@ Boot-Initrd-Path: /boot/uInitrd
Required-Packages: u-boot-tools
Bootloader-sets-root: no
+Machine: Globalscale Technologies Dreamplug
+Kernel-Flavors: kirkwood
+U-Boot-Kernel-Address: 0x00008000
+U-Boot-Initrd-Address: 0x0
+Boot-Kernel-Path: /boot/uImage
+Boot-Initrd-Path: /boot/uInitrd
+Required-Packages: u-boot-tools
+Bootloader-sets-root: no
+
Machine: Marvell GuruPlug Reference Board
Kernel-Flavors: kirkwood
U-Boot-Kernel-Address: 0x00008000
diff --git a/debian/changelog b/debian/changelog
index 12b620a..2fe3fb4 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,10 +1,15 @@
flash-kernel (3.0~rc.5) UNRELEASED; urgency=low
+ [ Hector Oron ]
* Add support for FK_MACHINE and FK_PROC_CPUINFO.
- Makes it possible to pass machine name for use under chrooted setups.
Setting FK_MACHINE to none avoids flash-kernel run.
Thanks Johannes Schauer for patch.
+ [ Ian Campbell ]
+ * Add support for FDT based devices using /proc/device-tree/model for detection.
+ * Add support for DreamPlug.
+
-- Hector Oron <zumbi@debian.org> Sun, 11 Mar 2012 16:02:39 +0100
flash-kernel (3.0~rc.4) unstable; urgency=low
diff --git a/functions b/functions
index bbfa772..82bb09e 100644
--- a/functions
+++ b/functions
@@ -21,6 +21,7 @@
BOOTSCRIPTS_DIR="${FK_CHECKOUT:-$FK_DIR}/bootscript"
MACHINE_DB="$(cat "${FK_CHECKOUT:-$FK_DIR}/db/"*.db)"
PROC_CPUINFO="${FK_PROC_CPUINFO:-/proc/cpuinfo}"
+PROC_DTMODEL="${FK_PROC_DRMODEL:-/proc/device-tree/model}"
PROC_MTD="/proc/mtd"
@@ -94,6 +95,16 @@ check_supported() {
get_cpuinfo_hardware() {
grep "^Hardware" "$PROC_CPUINFO" | sed 's/Hardware\s*:\s*//'
}
+get_dt_model() {
+ cat "$PROC_DTMODEL"
+}
+get_machine() {
+ if [ -f "$PROC_DTMODEL" ] ; then
+ get_dt_model
+ else
+ get_cpuinfo_hardware
+ fi
+}
get_kfile_suffix() {
local kfile="$1"
@@ -302,7 +313,7 @@ elif [ -n "$FK_MACHINE" ]; then
machine="$FK_MACHINE"
[ "x$machine" = "xnone" ] && exit
else
- machine="$(get_cpuinfo_hardware)"
+ machine="$(get_machine)"
fi
if [ "x$1" = "x--supported" ]; then
diff --git a/README b/README
index 4eb4f35..7b74cd6 100644
--- a/README
+++ b/README
@@ -83,6 +83,8 @@ The supported fields are:
* Machine-Id: (optional) linux mach-type to set before starting vmlinuz;
will be set by a small piece of ARM code prepended to the kernel image
+* DTB-Id: (optional) specifies the name of the DTB file for this device
+
* U-Boot-Kernel-Address, U-Boot-Initrd-Address: (optional) address where
to load in (physical) RAM the kernel and initrd, respectively; this
also indicates that U-Boot images should be generated with mkimage
@@ -102,6 +104,10 @@ The supported fields are:
Boot-Initrd-Path but for an U-Boot boot script; see also
U-Boot-Script-Name and Boot-Device
+* Boot-DTB-Path: (optional) like Boot-Kernel-Path and Boot-Initrd-Path
+ but for a DTB file. The DTB file named by DTB-Id will be copied
+ here; see also DTB-Id
+
* Required-packages: (optional) list of packages which must be added
during installer phase for flash-kernel to work properly; failure to
add these packages aborts the installation
diff --git a/db/all.db b/db/all.db
index c59ec94..82451fe 100644
--- a/db/all.db
+++ b/db/all.db
@@ -147,10 +147,12 @@ Bootloader-sets-root: no
Machine: Globalscale Technologies Dreamplug
Kernel-Flavors: kirkwood
+DTB-Id: kirkwood-dreamplug.dtb
U-Boot-Kernel-Address: 0x00008000
U-Boot-Initrd-Address: 0x0
Boot-Kernel-Path: /boot/uImage
Boot-Initrd-Path: /boot/uInitrd
+Boot-DTB-Path: /boot/dtb
Required-Packages: u-boot-tools
Bootloader-sets-root: no
diff --git a/functions b/functions
index 82bb09e..a46bfed 100644
--- a/functions
+++ b/functions
@@ -382,6 +382,7 @@ machine_id="$(get_machine_field "$machine" "Machine-Id")" || :
method="$(get_machine_field "$machine" "Method")" || method="generic"
mtd_kernel="$(get_machine_field "$machine" "Mtd-Kernel")" || :
mtd_initrd="$(get_machine_field "$machine" "Mtd-Initrd")" || :
+dtb_name="$(get_machine_field "$machine" "DTB-Id")" || :
ukaddr="$(get_machine_field "$machine" "U-Boot-Kernel-Address")" || :
uiaddr="$(get_machine_field "$machine" "U-Boot-Initrd-Address")" || :
umaddr="$(get_machine_field "$machine" "U-Boot-Multi-Address")" || :
@@ -391,6 +392,7 @@ boot_device="$(get_machine_field "$machine" "Boot-Device")" || :
boot_kernel_path="$(get_machine_field "$machine" "Boot-Kernel-Path")" || :
boot_initrd_path="$(get_machine_field "$machine" "Boot-Initrd-Path")" || :
boot_script_path="$(get_machine_field "$machine" "Boot-Script-Path")" || :
+boot_dtb_path="$(get_machine_field "$machine" "Boot-DTB-Path")" || :
boot_multi_path="$(get_machine_field "$machine" "Boot-Multi-Path")" || :
android_boot_device="$(get_machine_field "$machine" "Android-Boot-Device")" || :
@@ -537,6 +539,13 @@ case "$method" in
boot_script="$tmpdir/boot.scr"
backup_and_install "$boot_script" "$boot_script_path"
fi
+ if [ -n "$boot_dtb_path" ] ; then
+ boot_dtb_path="$boot_mnt_dir/$boot_dtb_path"
+ boot_dtb="/usr/lib/linux-image-$kvers/$dtb_name"
+ dtb="$tmpdir/dtb"
+ cp "$boot_dtb" "$dtb"
+ backup_and_install "$dtb" "$boot_dtb_path"
+ fi
;;
"symlink")
rm -f /boot/initrd /boot/zImage
Reply to: