All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.