linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
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

  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).