From: David Gibson <david@gibson.dropbear.id.au>
To: Timur Tabi <timur@freescale.com>
Cc: linuxppc-dev@ozlabs.org
Subject: Re: [PATCH 10/17] bootwrapper: Add dt_set_mac_addresses().
Date: Wed, 21 Mar 2007 13:54:47 +1100 [thread overview]
Message-ID: <20070321025447.GG27969@localhost.localdomain> (raw)
In-Reply-To: <45FFE8FD.9020902@freescale.com>
On Tue, Mar 20, 2007 at 09:00:29AM -0500, Timur Tabi wrote:
> David Gibson wrote:
>
> > local-mac-address isn't a node, it's a property. We never deal in
> > pointers (or handles) to properties.
>
> Sorry, I sometimes use those terms interchangeably.
>
> > Since mac-address is by definition a runtime property, it should
> > *never* exist in a static dts.
>
> But it does today. I think we need to be considerate of existing broken
> installations. Someone could be running an older U-Boot with an older
> DTS, and if they boot a future kernel with it, then
>
> >
> >> Obviously, the DTS files shouldn't have mac-address in them. But I
> >> haven't gotten around to cleaning that up, because I'm still waiting
> >> for the U-Boot maintainers to apply my pre-requisite patches.
> >
> > I don't see why fixing the dts files relies on u-boot changes.
>
> Because today's U-Boot will, on some platforms, write to mac-address and
> only mac-address. If you remove that property from the DTS without
> fixing U-Boot first, then U-Boot will never be able to pass the MAC
> address to the kernel.
Ah, and it can't add the property if it's not there at all. Ok, I
think I understand now. But.. does u-boot directly use the dts files
from the kernel tree, or does it have its own copies?
> > u-boot
> > *can* know what's going on and might wish to set the mac-address
> > property, but that's its business. The dts files which are used for
> > inclusion into a zImage should never have this property.
>
> Bugs need to be fixed in the right order. You can't fix the DTS files
> until you fix all the software that uses them first.
>
> >> Perhaps mac-address should be deleted instead of just ignored? I
> >> don't know if that breaks kexec.
> >
> > Hrm, maybe.
>
> Would the cuImage bootwrapper run in a kexec'd kernel? I hope not. I
> don't know anything about kexec.
The maybe was more in relation to should we delete the mac-address
property. But that would have the same brokenness as removing
mac-address from the dts on old, broken u-boot's that set mac-address
instead of local-mac-address.
> > It's just that every insert could require copying most of the blob,
> > which is a bit sucky.
>
> What if U-Boot were smart enough to insert all of the required items in
> one shot, and then filled them in?
I don't see how that's reasonably possible when the items in question
are properties in different nodes.
> > I was thinking of making an extension to dtc for these properties that
> > are expected to be replaced in-place by the bootloader: a special
> > token representing a value to be filled in later, so something like:
> > clock-frequency = < _ >;
> > or
> > local-mac-address = [??????];
> > The blob produced would just replace the blanks with either all 0s or
> > all 1s, so it doesn't actually do anything new, but it would provide a
> > strong visual clue in the source as to which properties are supposed
> > to be filled in later. Would that overcome your reluctance to include
> > bootloader-replaced properties in the dts?
>
> No, because it would require DTC to be constantly in sync with U-Boot,
> and that sounds like a lose-lose proposition to me. It takes months for
> any U-Boot patch that I write to be accepted and applied.
Uh.. how does it require synchronization with u-boot? This is really
just a form of internal documentation in the dts which shows which
properties are expected to be present, but overwrriten by the
bootloader.
> For instance, last month, I posted a fix for an old bug in U-Boot that
> prevents initrd from working with any OF kernel. U-Boot has had this
> bug ever since it added OF support. Despite repeated emails to WD
> himself, I haven't even gotten an acknowledgment from him as to whether
> he'll apply it, let alone when.
>
> So as you can imagine, I am very wary about any design that requires
> U-Boot to be updated in concert with any other software.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
next prev parent reply other threads:[~2007-03-21 2:54 UTC|newest]
Thread overview: 92+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-03-16 17:26 [PATCH 00/17] bootwrapper/cuImage patches Scott Wood
2007-03-16 17:27 ` [PATCH 01/17] bootwrapper: Make ft_create_node() pay attention to the parent parameter Scott Wood
2007-03-17 1:23 ` David Gibson
2007-03-16 17:27 ` [PATCH 02/17] bootwrapper: Add dt_ops methods Scott Wood
2007-03-17 1:24 ` David Gibson
2007-03-16 17:27 ` [PATCH 03/17] bootwrapper: Add xlate_reg(), and use it to find serial registers Scott Wood
2007-03-20 3:50 ` David Gibson
2007-03-20 16:34 ` Scott Wood
2007-03-21 3:12 ` David Gibson
2007-03-21 13:49 ` Segher Boessenkool
2007-03-21 16:01 ` Scott Wood
2007-03-21 19:11 ` Segher Boessenkool
2007-03-16 17:27 ` [PATCH 04/17] bootwrapper: Make compression of the kernel image optional Scott Wood
2007-03-17 1:25 ` David Gibson
2007-03-21 15:03 ` Patch: " Milton Miller
2007-03-16 17:28 ` [PATCH 05/17] bootwrapper: misc device tree fixes Scott Wood
2007-03-21 1:51 ` David Gibson
2007-03-22 16:22 ` Scott Wood
2007-03-23 3:26 ` David Gibson
2007-03-21 13:22 ` Segher Boessenkool
2007-03-16 17:28 ` [PATCH 06/17] Document the linux,network-index property Scott Wood
2007-03-17 1:26 ` David Gibson
2007-03-21 13:30 ` Segher Boessenkool
2007-03-21 14:48 ` Yoder Stuart-B08248
2007-03-21 18:57 ` Segher Boessenkool
2007-03-22 0:29 ` David Gibson
2007-03-22 11:11 ` Segher Boessenkool
2007-03-23 3:19 ` David Gibson
2007-03-23 11:36 ` Segher Boessenkool
2007-03-23 23:59 ` David Gibson
2007-03-24 0:23 ` Segher Boessenkool
2007-03-23 15:00 ` Scott Wood
2007-03-23 16:42 ` Segher Boessenkool
2007-03-16 17:28 ` [PATCH 07/17] bootwrapper: Add dt_set_memory(), to fill in the /memory node Scott Wood
2007-03-17 1:26 ` David Gibson
2007-03-21 13:32 ` Segher Boessenkool
2007-03-21 23:42 ` David Gibson
2007-03-22 11:02 ` Segher Boessenkool
2007-03-23 3:24 ` David Gibson
2007-03-23 6:49 ` Stefan Roese
2007-03-23 7:40 ` David Gibson
2007-03-23 12:34 ` Segher Boessenkool
2007-03-16 17:28 ` [PATCH 08/17] bootwrapper: Make setprop accept a const buffer Scott Wood
2007-03-17 1:27 ` David Gibson
2007-03-16 17:28 ` [PATCH 09/17] bootwrapper: Add dt_set_cpu_clocks() Scott Wood
2007-03-17 1:28 ` David Gibson
2007-03-16 17:28 ` [PATCH 10/17] bootwrapper: Add dt_set_mac_addresses() Scott Wood
2007-03-17 1:31 ` David Gibson
2007-03-18 0:22 ` Timur Tabi
2007-03-18 11:56 ` David Gibson
2007-03-19 15:09 ` Timur Tabi
2007-03-20 3:59 ` David Gibson
2007-03-20 14:00 ` Timur Tabi
2007-03-21 2:54 ` David Gibson [this message]
2007-03-21 15:01 ` Timur Tabi
2007-03-21 15:25 ` Jerry Van Baren
2007-03-21 15:55 ` Timur Tabi
2007-03-22 0:06 ` David Gibson
2007-03-22 15:13 ` Timur Tabi
2007-03-22 15:15 ` Jon Loeliger
2007-03-23 3:22 ` David Gibson
2007-03-23 14:38 ` Timur Tabi
2007-03-23 16:37 ` Segher Boessenkool
2007-03-23 16:42 ` Timur Tabi
2007-03-23 16:51 ` Segher Boessenkool
2007-03-23 16:54 ` Timur Tabi
2007-03-23 23:17 ` David Gibson
2007-03-20 18:17 ` Jon Loeliger
2007-03-21 13:45 ` Segher Boessenkool
2007-03-21 15:15 ` Timur Tabi
2007-03-21 19:07 ` Segher Boessenkool
2007-03-21 19:10 ` Timur Tabi
2007-03-21 19:34 ` Segher Boessenkool
2007-03-21 19:39 ` Timur Tabi
2007-03-21 19:48 ` Segher Boessenkool
2007-03-21 20:03 ` Timur Tabi
2007-03-21 20:22 ` Olof Johansson
2007-03-21 21:58 ` Segher Boessenkool
2007-03-21 21:54 ` Segher Boessenkool
2007-03-21 20:34 ` Doug Maxey
2007-03-21 22:01 ` Segher Boessenkool
2007-03-21 13:38 ` Segher Boessenkool
2007-03-21 15:09 ` Timur Tabi
2007-03-16 17:28 ` [PATCH 11/17] bootwrapper: Make set_cmdline non-static, and accept a const buffer Scott Wood
2007-03-17 1:36 ` David Gibson
2007-03-16 17:29 ` [PATCH 12/17] bootwrapper: Make set_cmdline() create /chosen if it doesn't exist Scott Wood
2007-03-21 13:32 ` Segher Boessenkool
2007-03-16 17:29 ` [PATCH 13/17] bootwrapper: Add ppcboot.h Scott Wood
2007-03-16 17:29 ` [PATCH 14/17] bootwrapper: Add support for cuboot platforms Scott Wood
2007-03-16 17:29 ` [PATCH 15/17] bootwrapper: Add cmd_wrap_dt Scott Wood
2007-03-16 17:29 ` [PATCH 16/17] bootwrapper: Add a cuImage target Scott Wood
2007-03-16 17:29 ` [PATCH 17/17] bootwrapper: cuboot for 83xx Scott Wood
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20070321025447.GG27969@localhost.localdomain \
--to=david@gibson.dropbear.id.au \
--cc=linuxppc-dev@ozlabs.org \
--cc=timur@freescale.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).