[U-Boot-Users] (Try 2) Please pull branch fdt-cmd from u-boot-fdt.git

[U-Boot-Users] (Try 2) Please pull branch fdt-cmd from u-boot-fdt.git

[Jerry Van Baren]

Dear Wolfgang,

Please pull from the "fdt-cmd" branch at
   git://cideas.us/pub/scm/u-boot/u-boot-fdt.git fdt-cmd
currently broken, someday fixed:
   git://denx.de/git/u-boot-fdt.git fdt-cmd

I've resplit the patches and emailed them to the u-boot list.

This change is only the starting point. :-) I've only done the
mpc8360/mpc8360emds board.  Some fairly small changes need to be done
to the other mpc8xxx family CPU and board subdirectories to allow them
to use the libfdt/fdt command.  To see what needs adapting, see:
  cpu/mpc83xx/cpu.c               |  102 +++++-
  board/mpc8360emds/pci.c         |   20 +

Note that I've created a new CONFIG_OF_LIBFDT that is intended to
ultimately supplant CONFIG_OF_FLAT_TREE.  Use one or the other: new way
CONFIG_OF_LIBFDT or old way CONFIG_OF_FLAT_TREE.  Obviously, the only 
way to get the new fdt command is to use CONFIG_OF_LIBFDT.

Best regards,
gvb

[U-Boot-Users] (Try 2) Please pull branch fdt-cmd from u-boot-fdt.git

[Wolfgang Denk]

In message <20070331174341.GA26946@dellserver.lan> you wrote:
> 
> Please pull from the "fdt-cmd" branch at
>    git://cideas.us/pub/scm/u-boot/u-boot-fdt.git fdt-cmd
> currently broken, someday fixed:

Not broken, just using a separate branch...

>    git://denx.de/git/u-boot-fdt.git fdt-cmd



Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A woman should have compassion.
	-- Kirk, "Catspaw", stardate 3018.2

[U-Boot-Users] (Try 2) Please pull branch fdt-cmd from u-boot-fdt.git

[Jerry Van Baren]

Wolfgang Denk wrote:
> In message <20070331174341.GA26946@dellserver.lan> you wrote:
>> Please pull from the "fdt-cmd" branch at
>>    git://cideas.us/pub/scm/u-boot/u-boot-fdt.git fdt-cmd
>> currently broken, someday fixed:
> 
> Not broken, just using a separate branch...
> 
>>    git://denx.de/git/u-boot-fdt.git fdt-cmd
> 
> Best regards,
> Wolfgang Denk

FYI, I needed to use the --all option on the git-push.
$ git push --all git+ssh://gu-fdt at www.denx.de/u-boot-fdt

gvb

[U-Boot-Users] (Try 2) Please pull branch fdt-cmd from u-boot-fdt.git

[Wolfgang Denk]

In message <460EAD1B.2080703@gmail.com> you wrote:
>
> > Not broken, just using a separate branch...
> > 
> >>    git://denx.de/git/u-boot-fdt.git fdt-cmd
> > 
> > Best regards,
> > Wolfgang Denk
> 
> FYI, I needed to use the --all option on the git-push.
> $ git push --all git+ssh://gu-fdt at www.denx.de/u-boot-fdt

Pulled, and cleaned up. Please be a bit more careful about the coding
style.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Good morning. This is the telephone company. Due  to  repairs,  we're
giving  you  advance notice that your service will be cut off indefi-
nitely at ten o'clock. That's two minutes from now.

[U-Boot-Users] (Try 2) Please pull branch fdt-cmd from u-boot-fdt.git

[Jerry Van Baren]

Wolfgang Denk wrote:
> In message <460EAD1B.2080703@gmail.com> you wrote:
>>> Not broken, just using a separate branch...
>>>
>>>>    git://denx.de/git/u-boot-fdt.git fdt-cmd
>>> Best regards,
>>> Wolfgang Denk
>> FYI, I needed to use the --all option on the git-push.
>> $ git push --all git+ssh://gu-fdt at www.denx.de/u-boot-fdt
> 
> Pulled, and cleaned up. Please be a bit more careful about the coding
> style.
> 
> Best regards,
> 
> Wolfgang Denk

Looking good.  Thanks for the quick pull, git is GREAT!

Sorry about the coding style violations.  :-(

Best regards,
gvb

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Jerry Van Baren]

Jerry Van Baren wrote:
>> Wolfgang Denk wrote:
[snip]
>> Pulled, and cleaned up. Please be a bit more careful about the coding
>> style.
>>
>> Best regards,
>>
>> Wolfgang Denk
> 
> Looking good.  Thanks for the quick pull, git is GREAT!
> 
> Sorry about the coding style violations.  :-(
> 
> Best regards,
> gvb

Note that my patch enables CONFIG_OF_LIBFDT in
   include/configs/MPC8360EMDS.h

As a result, if you pull an update from the master u-boot repository, 
you will get the new fdt command, libfdt support, *and modified bootm* 
command.  While this shouldn't be a bad thing, it *is not* backward 
compatible with CONFIG_OF_FLAT_TREE.

This was sloppy on my part and I apologize in advance for any confusion 
this may cause.  Please take this opportunity to generate improvement 
patches, rather than invectives toward myself. ;-)

Best regards,
gvb

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Timur Tabi]

Jerry Van Baren wrote:

> As a result, if you pull an update from the master u-boot repository, 
> you will get the new fdt command, libfdt support, *and modified bootm* 
> command.  While this shouldn't be a bad thing, it *is not* backward 
> compatible with CONFIG_OF_FLAT_TREE.

I'm a little confused.  CONFIG_OF_FLAT_TREE is needed to boot any powerpc kernel.  Are you 
saying that the two options are mutually exclusive?  Shouldn't CONFIG_OF_LIBFDT be a 
subset (instead of an alternative) to CONFIG_OF_FLAT_TREE?  That is, you need to define 
CONFIG_OF_FLAT_TREE in order for CONFIG_OF_LIBFDT to be recognized?

Maybe I should have been paying more attention to your libfdt work, but I was assuming you 
were just going to alter the back-end handling of OF trees, not break existing code.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Jerry Van Baren]

Timur Tabi wrote:
> Jerry Van Baren wrote:
> 
>> As a result, if you pull an update from the master u-boot repository, 
>> you will get the new fdt command, libfdt support, *and modified bootm* 
>> command.  While this shouldn't be a bad thing, it *is not* backward 
>> compatible with CONFIG_OF_FLAT_TREE.
> 
> I'm a little confused.  CONFIG_OF_FLAT_TREE is needed to boot any 
> powerpc kernel.  Are you saying that the two options are mutually 
> exclusive?  Shouldn't CONFIG_OF_LIBFDT be a subset (instead of an 
> alternative) to CONFIG_OF_FLAT_TREE?  That is, you need to define 
> CONFIG_OF_FLAT_TREE in order for CONFIG_OF_LIBFDT to be recognized?
> 
> Maybe I should have been paying more attention to your libfdt work, but 
> I was assuming you were just going to alter the back-end handling of OF 
> trees, not break existing code.

Hi Timur,

Using CONFIG_OF_FLAT_TREE results in an unchanged u-boot image, no 
libfdt, no "fdt" command, backwards compatibility.

The incompatibility with CONFIG_OF_LIBFDT is that "bootm" does *not* 
auto-generate nodes ("chosen", "u-boot-env", and "bd_t") when it runs. 
With CONFIG_OF_LIBFDT, I expect the boot script or the user to use the 
sequence:
* "fdt addr" command to set the blob address
* "fdt chosen" to generate/augment the chosen node
* "fdt env" to generate the u-boot-env node (optional)
* "fdt bd_t" to generate the bd_t node (optional)

I view autogenerating fdt entries inside "bootm" as being evil. 
Obviously, if there are enough people on the Dark Side to overwhelm me, 
that can be changed.

NOTE: I've probably screwed up the multi-image.  Note to self, fix it - 
need to find the fdt blob in a multi-image and implicitly set the "fdt 
addr" to it.

Best regards,
gvb

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Wolfgang Denk]

In message <4613CF9F.4040501@smiths-aerospace.com> you wrote:
>
> I view autogenerating fdt entries inside "bootm" as being evil. 
> Obviously, if there are enough people on the Dark Side to overwhelm me, 
> that can be changed.

"bootm" is intended to boot an  image  located  somewhere  in  system
memory.  It  will  pass arguments like the booargs and the FDT to the
kernel. It evaluates bootargs and substitured env vars for this. It
might not be insane to expect that it also does reasonable things to
the FDT.

I'm not in a position to determine what's reasonable there, though.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The combination of a number of things to make existence worthwhile."
"Yes, the philosophy of 'none,' meaning 'all.'"
	-- Spock and Lincoln, "The Savage Curtain", stardate 5906.4

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Jerry Van Baren]

Wolfgang Denk wrote:
> In message <4613CF9F.4040501@smiths-aerospace.com> you wrote:
>> I view autogenerating fdt entries inside "bootm" as being evil. 
>> Obviously, if there are enough people on the Dark Side to overwhelm me, 
>> that can be changed.
> 
> "bootm" is intended to boot an  image  located  somewhere  in  system
> memory.  It  will  pass arguments like the booargs and the FDT to the
> kernel. It evaluates bootargs and substitured env vars for this. It
> might not be insane to expect that it also does reasonable things to
> the FDT.
> 
> I'm not in a position to determine what's reasonable there, though.
> 
> Best regards,
> 
> Wolfgang Denk

Well, I'm feeling my way around here too. :-/

If bootm edits/augments the FDT, the boot scripts/user has no chance to 
change the items it edits/augments (biggie: the chosen node), or even 
print it before linux is launched.  This defeats 90% of the purpose of 
the fdt command - allowing the user/script customize the blob before 
linux is launched.

My reasoning is that you can string together (or script) the proper 
"fdt" commands to do what previously was done by "bootm" in one step, so 
we are losing a small bit of convenience and gaining a whole lot of 
flexibility.  As a for instance, currently/previously you had to 
uncomment the blob dump call and recompile u-boot to get a dump of what 
is fed to linux - fugly.  With the New Improved[tm] behavior, you simply 
add "fdt print" to the super-bootm script and it Just Works[tm].  This 
is actually the itch that started me down this path. :-/

Much, perhaps most, possibly even all of the current stuff that is 
wedged into bootm is or should be passed through the blob.

Best regards,
gvb

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Wolfgang Denk]

In message <46146842.8060509@gmail.com> you wrote:
>
> If bootm edits/augments the FDT, the boot scripts/user has no chance to 
> change the items it edits/augments (biggie: the chosen node), or even 
> print it before linux is launched.  This defeats 90% of the purpose of 
> the fdt command - allowing the user/script customize the blob before 
> linux is launched.

I agree that it should be *possible* to do this, if wanted.Similar
like we can set up our own contents of thebootargs variable.

On the other hand, bootm should do everything that is necessary to
start a kernel without such interaction, if needed.

Remember for example that bootm gets called automatically and without
user interaction after download commands when "autostart" is  set  to
"yes".

Now assume we set "autostart" to "yes" nd  use  "dhcp"  to  load  and
start a multifile image containing the FDT. This is required to work,
to.

> My reasoning is that you can string together (or script) the proper 
> "fdt" commands to do what previously was done by "bootm" in one step, so 
> we are losing a small bit of convenience and gaining a whole lot of 
> flexibility.  As a for instance, currently/previously you had to 

Agreed, but as an option, and without breaking compatibility of
exiting methods.

> Much, perhaps most, possibly even all of the current stuff that is 
> wedged into bootm is or should be passed through the blob.

I cannot parse this. Please explain what you have in mind.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Is the glass half empty, half full, or twice as large as it needs to

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Jerry Van Baren]

Wolfgang Denk wrote:
> In message <46146842.8060509@gmail.com> you wrote:
>> If bootm edits/augments the FDT, the boot scripts/user has no chance to 
>> change the items it edits/augments (biggie: the chosen node), or even 
>> print it before linux is launched.  This defeats 90% of the purpose of 
>> the fdt command - allowing the user/script customize the blob before 
>> linux is launched.
> 
> I agree that it should be *possible* to do this, if wanted.Similar
> like we can set up our own contents of thebootargs variable.
> 
> On the other hand, bootm should do everything that is necessary to
> start a kernel without such interaction, if needed.
> 
> Remember for example that bootm gets called automatically and without
> user interaction after download commands when "autostart" is  set  to
> "yes".
> 
> Now assume we set "autostart" to "yes" nd  use  "dhcp"  to  load  and
> start a multifile image containing the FDT. This is required to work,
> to.
> 
>> My reasoning is that you can string together (or script) the proper 
>> "fdt" commands to do what previously was done by "bootm" in one step, so 
>> we are losing a small bit of convenience and gaining a whole lot of 
>> flexibility.  As a for instance, currently/previously you had to 
> 
> Agreed, but as an option, and without breaking compatibility of
> exiting methods.
> 
>> Much, perhaps most, possibly even all of the current stuff that is 
>> wedged into bootm is or should be passed through the blob.
> 
> I cannot parse this. Please explain what you have in mind.

Sorry, not my best example of English composition. :-/

What I was trying to say was that the linux bootargs (command line - in 
the chosen node), env variables (optional), and bd_t stuff (optional) is 
passed through the fdt blob now.  I was advocating that, by moving the 
creation of that stuff into the "fdt" command, we could simplify bootm. 
  Quite likely unrealistic, but it sounded good at the time.  Since then 
I got a solid 6 hrs sleep. ;-)

> Best regards,
> Wolfgang Denk
> 

Andy Fleming sent a suggestion that is really growing on me:
> What if we made it so if there isn't a chosen node in the blob when
> bootm is called, it fills in a default one.  This prevents some odd
> failures, and allows people to continue using device trees in the
> current manner, while still enabling the extra flexibility. 

<foghorn> That boy, I say, that boy has a /point/. </foghorn>

This means that, if you use "fdt chosen" to create the chosen node, you 
will be able to look at it and edit it and bootm will do the Right 
Thing[tm].  If you don't bootm will still do the Right Thing[tm] and 
autogenerate the necessary nodes.

Thanks,
gvb
<http://en.wikipedia.org/wiki/Foghorn_Leghorn>

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Bruce_Leonard at selinc.com]

Hi all

u-boot-users-bounces at lists.sourceforge.net wrote on 04/05/2007 04:00:15 
AM:

> Wolfgang Denk wrote:
> > In message <46146842.8060509@gmail.com> you wrote:
> >> If bootm edits/augments the FDT, the boot scripts/user has no chance 
to 
> >> change the items it edits/augments (biggie: the chosen node), or even 

> >> print it before linux is launched.  This defeats 90% of the purpose 
of 
> >> the fdt command - allowing the user/script customize the blob before 
> >> linux is launched.
> > 
> > I agree that it should be *possible* to do this, if wanted.Similar
> > like we can set up our own contents of thebootargs variable.
> > 
> > On the other hand, bootm should do everything that is necessary to
> > start a kernel without such interaction, if needed.
> > 
<snip snip>

My two cents worth (for what it's worth :-/): from the standpoint of 
someone using uboot for the first time and having to learn from the ground 
up without the benefit of having used this stuff for many years, I agree 
with Wolfgang.  It took me two weeks just to figure out what a device tree 
is and that I even needed one.  It took me two more days to figure out how 
to create the blob and how to use it.  I still don't know the details of 
DTS files, what needs to be in them or what the different fields mean. 
Adding another step/level of obscurity with REQUIRING the use of fdt 
commands and/or scripts is just another barrier to new users.  And I have 
to tell you, this thing is a bear to learn.  For folks who have been 
digging around in the guts of it, I'm sure it's trivial.  But I at least 
am pretty overwhelmed by it.

I think it would be great to have the option of using the ftd commands if 
it suited your purpose, but still be able to use things as they currently 
are.  That would be the most flexible, give the expert users something 
they want, and not add yet another thing that new people HAVE to learn 
just to get an OS to boot.

Again just my 2 cents.  Flames welcome.

Bruce

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Jerry Van Baren]

Bruce_Leonard at selinc.com wrote:
> Hi all
> 
> u-boot-users-bounces at lists.sourceforge.net wrote on 04/05/2007 04:00:15 
> AM:
> 
>> Wolfgang Denk wrote:
>>> In message <46146842.8060509@gmail.com> you wrote:
>>>> If bootm edits/augments the FDT, the boot scripts/user has no chance 
> to 
>>>> change the items it edits/augments (biggie: the chosen node), or even 
> 
>>>> print it before linux is launched.  This defeats 90% of the purpose 
> of 
>>>> the fdt command - allowing the user/script customize the blob before 
>>>> linux is launched.
>>> I agree that it should be *possible* to do this, if wanted.Similar
>>> like we can set up our own contents of thebootargs variable.
>>>
>>> On the other hand, bootm should do everything that is necessary to
>>> start a kernel without such interaction, if needed.
>>>
> <snip snip>
> 
> My two cents worth (for what it's worth :-/): from the standpoint of 
> someone using uboot for the first time and having to learn from the ground 
> up without the benefit of having used this stuff for many years, I agree 
> with Wolfgang.  It took me two weeks just to figure out what a device tree 
> is and that I even needed one.  It took me two more days to figure out how 
> to create the blob and how to use it.  I still don't know the details of 
> DTS files, what needs to be in them or what the different fields mean. 
> Adding another step/level of obscurity with REQUIRING the use of fdt 
> commands and/or scripts is just another barrier to new users.  And I have 
> to tell you, this thing is a bear to learn.  For folks who have been 
> digging around in the guts of it, I'm sure it's trivial.  But I at least 
> am pretty overwhelmed by it.
> 
> I think it would be great to have the option of using the ftd commands if 
> it suited your purpose, but still be able to use things as they currently 
> are.  That would be the most flexible, give the expert users something 
> they want, and not add yet another thing that new people HAVE to learn 
> just to get an OS to boot.
> 
> Again just my 2 cents.  Flames welcome.
> 
> Bruce

Yes, but now you are worth 5 figures more: expect a $00,000 raise as 
part of your next performance appraisal.

I plan to implement Andy Fleming's suggestion which will resolve the issue:
 > > What if we made it so if there isn't a chosen node in the blob when
 > > bootm is called, it fills in a default one.  This prevents some odd
 > > failures, and allows people to continue using device trees in the
 > > current manner, while still enabling the extra flexibility.

gvb
Pedantic Script: device trees and blobs are kernel things, they ain't 
our fault. :-P Yet. ;-)

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Bruce_Leonard at selinc.com]

> 
> I plan to implement Andy Fleming's suggestion which will resolve the 
issue:
>  > > What if we made it so if there isn't a chosen node in the blob when
>  > > bootm is called, it fills in a default one.  This prevents some odd
>  > > failures, and allows people to continue using device trees in the
>  > > current manner, while still enabling the extra flexibility.

Which (if I had paid attention to your earlier post) I would have known. 
:)

> 
> gvb
> Pedantic Script: device trees and blobs are kernel things, they ain't 
> our fault. :-P Yet. ;-)

It's never 'our' fault, it's always 'their' fault.  Or is the the 
processes' fault?  I can never keep up with who we're supposed to balme. 
:)

Thanks

Bruce

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Timur Tabi]

Jerry Van Baren wrote:

> This was sloppy on my part and I apologize in advance for any confusion 
> this may cause.  Please take this opportunity to generate improvement 
> patches, rather than invectives toward myself. ;-)

I finally had a chance to look at this code, and, well, I'm not crazy about it.

In short, I have a real problem with this:

#define	FT_BUSFREQ	0x00000002		/* source is bd->bi_busfreq */
#define	FT_ENETADDR	0x00000004		/* source is bd->bi_enetaddr */

Using flags to determine the SOURCE of the property data is bad, IMHO.  It's just not 
flexible enough.  Not only that, but you already have a bug in the definition of 
fixup_props[]:

#ifdef CONFIG_MPC83XX_TSEC2
	{	FT_UPDATE | FT_ENETADDR,
		"/" OF_SOC "/ethernet at 25000,
		"mac-address",
	},
	{	FT_UPDATE | FT_ENETADDR,
		"/" OF_SOC "/ethernet at 25000,
		"local-mac-address",
	},
#endif

FT_ENETADDR is the MAC address of eth0, but here it's being programmed into eth1!  The 
obvious solution is to introduce:

#define	FT_ENETADDR1	0x00000008		/* source is bd->bi_enetaddr1 */

But then you need definitions for eth2 and eth3

#define	FT_ENETADDR2	0x00000010		/* source is bd->bi_enetaddr2 */
#define	FT_ENETADDR3	0x00000020		/* source is bd->bi_enetaddr3 */

As you can see, you're already bloating the code.

A better solution would be to provide a function pointer for a function that can be called 
to fill in the property.  Something like:

int ft_set_eth0(void *fdt, int nodeoffset, const char *name, bd_t *bd)
{
	return fdt_setprop(fdt, nodeoffset, name, bd->bi_enetaddr, 6);
}

or something like that.  Then define fixup_props[] like this:

static const struct {
	int  createflags;
	char *node;
	char *prop;
	int (*set_function)(void *fdt, int nodeoffset, const char *name, bd_t *bd);
} fixup_props[] = {

What do you think about that?  If you like it, I can make the change to mpc83xx/cpu.c

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Jerry Van Baren]

Timur Tabi wrote:
> Jerry Van Baren wrote:
> 
>> This was sloppy on my part and I apologize in advance for any confusion 
>> this may cause.  Please take this opportunity to generate improvement 
>> patches, rather than invectives toward myself. ;-)
> 
> I finally had a chance to look at this code, and, well, I'm not crazy about it.
> 
> In short, I have a real problem with this:
> 
> #define	FT_BUSFREQ	0x00000002		/* source is bd->bi_busfreq */
> #define	FT_ENETADDR	0x00000004		/* source is bd->bi_enetaddr */
> 
> Using flags to determine the SOURCE of the property data is bad, IMHO.  It's just not 
> flexible enough.  Not only that, but you already have a bug in the definition of 
> fixup_props[]:
> 
> #ifdef CONFIG_MPC83XX_TSEC2
> 	{	FT_UPDATE | FT_ENETADDR,
> 		"/" OF_SOC "/ethernet at 25000,
> 		"mac-address",
> 	},
> 	{	FT_UPDATE | FT_ENETADDR,
> 		"/" OF_SOC "/ethernet at 25000,
> 		"local-mac-address",
> 	},
> #endif
> 
> FT_ENETADDR is the MAC address of eth0, but here it's being programmed into eth1!  The 
> obvious solution is to introduce:
> 
> #define	FT_ENETADDR1	0x00000008		/* source is bd->bi_enetaddr1 */
> 
> But then you need definitions for eth2 and eth3
> 
> #define	FT_ENETADDR2	0x00000010		/* source is bd->bi_enetaddr2 */
> #define	FT_ENETADDR3	0x00000020		/* source is bd->bi_enetaddr3 */
> 
> As you can see, you're already bloating the code.
> 
> A better solution would be to provide a function pointer for a function that can be called 
> to fill in the property.  Something like:
> 
> int ft_set_eth0(void *fdt, int nodeoffset, const char *name, bd_t *bd)
> {
> 	return fdt_setprop(fdt, nodeoffset, name, bd->bi_enetaddr, 6);
> }
> 
> or something like that.  Then define fixup_props[] like this:
> 
> static const struct {
> 	int  createflags;
> 	char *node;
> 	char *prop;
> 	int (*set_function)(void *fdt, int nodeoffset, const char *name, bd_t *bd);
> } fixup_props[] = {
> 
> What do you think about that?  If you like it, I can make the change to mpc83xx/cpu.c

Hi Timur,

Thanks for the code review.

Oops, yes, the MAC address programming was bad. :-(  I thought I was 
being clever but all I was was very incorrect.

I'm not all that wild about having a bunch of one-liner functions with a 
table of fn pointers, but I cannot think of a better way (the original 
code is/was a sequence of hardcoded calls).

I'll take you up on your offer with a proposed tweak: remove the 
"createflags" too - the only remaining purpose is to indicate "create" 
vs. "update only" (i.e. must previously exist).  This logic can go into 
each "setter" function.  Half the properties are "create or force set" 
so the fdt_get_property() call isn't needed, just set it.  The MAC 
setters that care can do the fdt_get_property() in the "setter" function.

static const struct {
	char *node;
	char *prop;
	int (*set_function)(void *fdt, int nodeoffset, const char *name, bd_t *bd);
} fixup_props[] = {

Does that make sense?

Thanks,
gvb

P.S. I've pushed a new fdt-cmd branch to the u-boot-fdt repo that does 
some clean up and separates out some fdt_support.c functions in 
preparation for improving the bootm automagic logic.  There should be no 
collisions with Timur's proposed changes (if there are, I'll bear the 
responsibility of merging them :-/).

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Wolfgang Denk]

In message <4616CC07.9050400@gmail.com> you wrote:
>
> P.S. I've pushed a new fdt-cmd branch to the u-boot-fdt repo that does 
> some clean up and separates out some fdt_support.c functions in 
> preparation for improving the bootm automagic logic.  There should be no 
> collisions with Timur's proposed changes (if there are, I'll bear the 
> responsibility of merging them :-/).

I understand this is a call for review, not a pull request yet?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The trouble with our times is that the future is not what it used  to
be.                                                     - Paul Valery

[U-Boot-Users] Warning for mpc8360emds users: fdt-cmd from u-boot-fdt.git

[Jerry Van Baren]

Wolfgang Denk wrote:
> In message <4616CC07.9050400@gmail.com> you wrote:
>> P.S. I've pushed a new fdt-cmd branch to the u-boot-fdt repo that does 
>> some clean up and separates out some fdt_support.c functions in 
>> preparation for improving the bootm automagic logic.  There should be no 
>> collisions with Timur's proposed changes (if there are, I'll bear the 
>> responsibility of merging them :-/).
> 
> I understand this is a call for review, not a pull request yet?
> 
> Best regards,
> 
> Wolfgang Denk

Correct.  I am fighting sf.net and mutt to get my current changes 
published on the list (why do spammers get through and not honest 
engineers???).  I also intend to do the next step of improving the 
handling in bootm (restore the automagic) before calling for a pull.

Best regards,
gvb

[U-Boot-Users] (Try 2) Please pull branch fdt-cmd from u-boot-fdt.git

[Jerry Van Baren]

Jerry Van Baren wrote:
> Dear Wolfgang,
> 
> Please pull from the "fdt-cmd" branch at
>    git://cideas.us/pub/scm/u-boot/u-boot-fdt.git fdt-cmd
> currently broken, someday fixed:
>    git://denx.de/git/u-boot-fdt.git fdt-cmd

I figured out what I was doing wrong, so the u-boot-fdt repository now 
has my fdt-cmd branch with the changes in it.  Life is good!
<http://www.denx.de/cgi-bin/gitweb.cgi?p=u-boot/u-boot-fdt.git;a=shortlog;h=fdt-cmd>

Best regards,
gvb

[U-Boot-Users] (Try 2) Please pull branch fdt-cmd from u-boot-fdt.git

[Wolfgang Denk]

In message <460EA801.8020803@gmail.com> you wrote:
>
> I figured out what I was doing wrong, so the u-boot-fdt repository now 
> has my fdt-cmd branch with the changes in it.  Life is good!
> <http://www.denx.de/cgi-bin/gitweb.cgi?p=u-boot/u-boot-fdt.git;a=shortlog;h=fdt-cmd>

I think I got these, please verify.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The universe contains any amount of horrible ways  to  be  woken  up,
such as the noise of the mob breaking down the front door, the scream
of fire engines, or the realization that today is the Monday which on
Friday night was a comfortably long way off.
                                 - Terry Pratchett, _Moving Pictures_

[U-Boot-Users] (Try 2) Please pull branch fdt-cmd from u-boot-fdt.git

[Joakim Tjernlund]

On Sat, 2007-03-31 at 13:43 -0400, Jerry Van Baren wrote:
> Dear Wolfgang,
> 
> Please pull from the "fdt-cmd" branch at
>    git://cideas.us/pub/scm/u-boot/u-boot-fdt.git fdt-cmd
> currently broken, someday fixed:
>    git://denx.de/git/u-boot-fdt.git fdt-cmd
> 
> I've resplit the patches and emailed them to the u-boot list.
> 
> This change is only the starting point. :-) I've only done the
> mpc8360/mpc8360emds board.  Some fairly small changes need to be done
> to the other mpc8xxx family CPU and board subdirectories to allow them
> to use the libfdt/fdt command.  To see what needs adapting, see:
>   cpu/mpc83xx/cpu.c               |  102 +++++-
>   board/mpc8360emds/pci.c         |   20 +
> 
> Note that I've created a new CONFIG_OF_LIBFDT that is intended to
> ultimately supplant CONFIG_OF_FLAT_TREE.  Use one or the other: new way
> CONFIG_OF_LIBFDT or old way CONFIG_OF_FLAT_TREE.  Obviously, the only 
> way to get the new fdt command is to use CONFIG_OF_LIBFDT.

This is probably a little off but I am thinking about were to
put my dtb on the flash and it occured to me that it could fit into the
environment sector as the environment doesn't use the whole sector.
But one can't just write it into that sector without take measures to
preserve the environment.
Is this a bad idea?

 Jocke

[U-Boot-Users] (Try 2) Please pull branch fdt-cmd from u-boot-fdt.git

[Jerry Van Baren]

Joakim Tjernlund wrote:
> On Sat, 2007-03-31 at 13:43 -0400, Jerry Van Baren wrote:
>> Dear Wolfgang,
>>
>> Please pull from the "fdt-cmd" branch at
>>    git://cideas.us/pub/scm/u-boot/u-boot-fdt.git fdt-cmd
>> currently broken, someday fixed:
>>    git://denx.de/git/u-boot-fdt.git fdt-cmd
>>
>> I've resplit the patches and emailed them to the u-boot list.
>>
>> This change is only the starting point. :-) I've only done the
>> mpc8360/mpc8360emds board.  Some fairly small changes need to be done
>> to the other mpc8xxx family CPU and board subdirectories to allow them
>> to use the libfdt/fdt command.  To see what needs adapting, see:
>>   cpu/mpc83xx/cpu.c               |  102 +++++-
>>   board/mpc8360emds/pci.c         |   20 +
>>
>> Note that I've created a new CONFIG_OF_LIBFDT that is intended to
>> ultimately supplant CONFIG_OF_FLAT_TREE.  Use one or the other: new way
>> CONFIG_OF_LIBFDT or old way CONFIG_OF_FLAT_TREE.  Obviously, the only 
>> way to get the new fdt command is to use CONFIG_OF_LIBFDT.
> 
> This is probably a little off but I am thinking about were to
> put my dtb on the flash and it occured to me that it could fit into the
> environment sector as the environment doesn't use the whole sector.
> But one can't just write it into that sector without take measures to
> preserve the environment.
> Is this a bad idea?
> 
>  Jocke

That is an excellent idea and should be one of our goals: to better 
coordinate the dft and env variables.  As the penultimate goal, I 
believe we could move the env variables _into_ the dft and replace the 
get/set env routines with dft get/set routines.  I'm still thinking 
about this... env variables are used very early in power up (before RAM 
is initialized), but I haven't seen any reason that it /can't/ be done.

The downside of dfts is that the code is bigger.  My preliminary size 
delta with the code as it stands is about +4.5K.  I don't know how much 
we would save if we removed the env handling routines, but I'm sure 
nowhere near that (0.5K would be my guess).  For size sensitive u-boot 
images, we could use it read-only which would probably cut it down to 
1-2K, but then we would not be able to replace the env handling.

gvb

[U-Boot-Users] (Try 2) Please pull branch fdt-cmd from u-boot-fdt.git

[Joakim Tjernlund]

On Tue, 2007-04-03 at 06:34 -0400, Jerry Van Baren wrote:
> Joakim Tjernlund wrote:
> > On Sat, 2007-03-31 at 13:43 -0400, Jerry Van Baren wrote:
> >> Dear Wolfgang,
> >>
> >> Please pull from the "fdt-cmd" branch at
> >>    git://cideas.us/pub/scm/u-boot/u-boot-fdt.git fdt-cmd
> >> currently broken, someday fixed:
> >>    git://denx.de/git/u-boot-fdt.git fdt-cmd
> >>
> >> I've resplit the patches and emailed them to the u-boot list.
> >>
> >> This change is only the starting point. :-) I've only done the
> >> mpc8360/mpc8360emds board.  Some fairly small changes need to be done
> >> to the other mpc8xxx family CPU and board subdirectories to allow them
> >> to use the libfdt/fdt command.  To see what needs adapting, see:
> >>   cpu/mpc83xx/cpu.c               |  102 +++++-
> >>   board/mpc8360emds/pci.c         |   20 +
> >>
> >> Note that I've created a new CONFIG_OF_LIBFDT that is intended to
> >> ultimately supplant CONFIG_OF_FLAT_TREE.  Use one or the other: new way
> >> CONFIG_OF_LIBFDT or old way CONFIG_OF_FLAT_TREE.  Obviously, the only 
> >> way to get the new fdt command is to use CONFIG_OF_LIBFDT.
> > 
> > This is probably a little off but I am thinking about were to
> > put my dtb on the flash and it occured to me that it could fit into the
> > environment sector as the environment doesn't use the whole sector.
> > But one can't just write it into that sector without take measures to
> > preserve the environment.
> > Is this a bad idea?
> > 
> >  Jocke
> 
> That is an excellent idea and should be one of our goals: to better 
> coordinate the dft and env variables.  As the penultimate goal, I 
> believe we could move the env variables _into_ the dft and replace the 
> get/set env routines with dft get/set routines.  I'm still thinking 
> about this... env variables are used very early in power up (before RAM 
> is initialized), but I haven't seen any reason that it /can't/ be done.

Good, I am not totally off then :)
I think to start with one would just need an install dtb command that
places the dtb at the end of the env. sector to allow either one
to grow without trashing eachother. Also rendundant env needs to be
handled by this new command.
Secondly one needs someway of telling bootm where to find the dtb.

I have just started a board port here and there will be some time
before I get to to the dtb part.

  Jocke
> 
> The downside of dfts is that the code is bigger.  My preliminary size 
> delta with the code as it stands is about +4.5K.  I don't know how much 
> we would save if we removed the env handling routines, but I'm sure 
> nowhere near that (0.5K would be my guess).  For size sensitive u-boot 
> images, we could use it read-only which would probably cut it down to 
> 1-2K, but then we would not be able to replace the env handling.
> 
> gvb
> 
> 

[U-Boot-Users] (Try 2) Please pull branch fdt-cmd from u-boot-fdt.git

[Jerry Van Baren]

Joakim Tjernlund wrote:
> On Tue, 2007-04-03 at 06:34 -0400, Jerry Van Baren wrote:
>> Joakim Tjernlund wrote:
>>> On Sat, 2007-03-31 at 13:43 -0400, Jerry Van Baren wrote:
>>>> Dear Wolfgang,
>>>>
>>>> Please pull from the "fdt-cmd" branch at
>>>>    git://cideas.us/pub/scm/u-boot/u-boot-fdt.git fdt-cmd
>>>> currently broken, someday fixed:
>>>>    git://denx.de/git/u-boot-fdt.git fdt-cmd
>>>>
>>>> I've resplit the patches and emailed them to the u-boot list.
>>>>
>>>> This change is only the starting point. :-) I've only done the
>>>> mpc8360/mpc8360emds board.  Some fairly small changes need to be done
>>>> to the other mpc8xxx family CPU and board subdirectories to allow them
>>>> to use the libfdt/fdt command.  To see what needs adapting, see:
>>>>   cpu/mpc83xx/cpu.c               |  102 +++++-
>>>>   board/mpc8360emds/pci.c         |   20 +
>>>>
>>>> Note that I've created a new CONFIG_OF_LIBFDT that is intended to
>>>> ultimately supplant CONFIG_OF_FLAT_TREE.  Use one or the other: new way
>>>> CONFIG_OF_LIBFDT or old way CONFIG_OF_FLAT_TREE.  Obviously, the only 
>>>> way to get the new fdt command is to use CONFIG_OF_LIBFDT.
>>> This is probably a little off but I am thinking about were to
>>> put my dtb on the flash and it occured to me that it could fit into the
>>> environment sector as the environment doesn't use the whole sector.
>>> But one can't just write it into that sector without take measures to
>>> preserve the environment.
>>> Is this a bad idea?
>>>
>>>  Jocke
>> That is an excellent idea and should be one of our goals: to better 
>> coordinate the dft and env variables.  As the penultimate goal, I 
>> believe we could move the env variables _into_ the dft and replace the 
>> get/set env routines with dft get/set routines.  I'm still thinking 
>> about this... env variables are used very early in power up (before RAM 
>> is initialized), but I haven't seen any reason that it /can't/ be done.
> 
> Good, I am not totally off then :)
> I think to start with one would just need an install dtb command that
> places the dtb at the end of the env. sector to allow either one
> to grow without trashing eachother. Also rendundant env needs to be
> handled by this new command.
> Secondly one needs someway of telling bootm where to find the dtb.
> 
> I have just started a board port here and there will be some time
> before I get to to the dtb part.
> 
>   Jocke

Hi Jocke,

* I've removed the fdt munging from bootm.  For the "fdt" flavor, you
     need to munge the blob explicitly (see "fdt chosen", "fdt env",
     "fdt bd_t").
* The "fdt address" subcommand tells u-boot where the blob is.
* The 3rd parameter to bootm is the blob address (unchanged).  We could
     use the address set by the "fdt address" command automatically, but
     I'm not wild about doing things automatically.

If and when we save blobs with/instead of the env...
* We will undoubtedly want redundancy and CRC protection.
* We will probably want to add a "merge" command that can take a u-boot
     blob (from the env area, for instance) and merge it with a loaded
     (kernel-related) blob.  This is easy to say, but again will take
     some thought because we will need to Do The Right Thing[tm] when a
     given property exists in both (need to establish which one wins).

gvb
P.S. s/dft/fdt/ in my previous post, s/sleep/caffine/ on me.  :-O

[U-Boot-Users] dtb in env sector - was: (Try 2) Please pull ...

[Wolfgang Denk]

In message <1175600261.12080.24.camel@gentoo-jocke.transmode.se> you wrote:
>
> I think to start with one would just need an install dtb command that
> places the dtb at the end of the env. sector to allow either one
> to grow without trashing eachother. Also rendundant env needs to be

Umm... The environment will not grow beyond it's defined  size.  Then
just use "cp" to copy the dtb to the next free address.

> Secondly one needs someway of telling bootm where to find the dtb.

You pass the address as an argument...


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
IMPORTANT NOTICE TO PURCHASERS: The Entire Physical Universe,  Inclu-
ding  This Product, May One Day Collapse Back into an Infinitesimally
Small Space. Should  Another  Universe  Subsequently  Re-emerge,  the
Existence of This Product in That Universe Cannot Be Guaranteed.

[U-Boot-Users] dtb in env sector - was: (Try 2) Please pull ...

[Joakim Tjernlund]

On Tue, 2007-04-03 at 14:59 +0200, Wolfgang Denk wrote:
> In message <1175600261.12080.24.camel@gentoo-jocke.transmode.se> you wrote:
> >
> > I think to start with one would just need an install dtb command that
> > places the dtb at the end of the env. sector to allow either one
> > to grow without trashing eachother. Also rendundant env needs to be
> 
> Umm... The environment will not grow beyond it's defined  size.  Then
> just use "cp" to copy the dtb to the next free address.

And later you need some more space for env. so you increase CFG_ENV_SIZE
a little and now you have destroyed your dtb as well.

> 
> > Secondly one needs someway of telling bootm where to find the dtb.
> 
> You pass the address as an argument...

yes, but it would be handy to just say "use the dtb in my spare
env space", especially if you use rendundant env. so the right
one is selected.

 Regards
          Jocke

[U-Boot-Users] dtb in env sector - was: (Try 2) Please pull ...

[Jerry Van Baren]

Joakim Tjernlund wrote:
> On Tue, 2007-04-03 at 14:59 +0200, Wolfgang Denk wrote:
>> In message <1175600261.12080.24.camel@gentoo-jocke.transmode.se> you wrote:
>>> I think to start with one would just need an install dtb command that
>>> places the dtb at the end of the env. sector to allow either one
>>> to grow without trashing eachother. Also rendundant env needs to be
>> Umm... The environment will not grow beyond it's defined  size.  Then
>> just use "cp" to copy the dtb to the next free address.
> 
> And later you need some more space for env. so you increase CFG_ENV_SIZE
> a little and now you have destroyed your dtb as well.

Actually, it is much worse than that: currently when you do a "saveenv" 
it will wipe out the fdt blob because it doesn't know that the blob is 
in the same sector.

I'm thinking that we want to store the blob immediately after the env 
variable storage _reserved area_ as a #define option (as a new #define 
option? part of the CONFIG_OF_LIBFDT define?) and enhance the env 
save/restore to do the blob too.  This would get us the redundancy and 
we can cover it with a new (preferred?) or existing env CRC (not good?).

>>> Secondly one needs someway of telling bootm where to find the dtb.
>> You pass the address as an argument...
> 
> yes, but it would be handy to just say "use the dtb in my spare
> env space", especially if you use rendundant env. so the right
> one is selected.
> 
>  Regards
>           Jocke

Best regards,
gvb

[U-Boot-Users] dtb in env sector - was: (Try 2) Please pull ...

[Martin Krause]

u-boot-users-bounces at lists.sourceforge.net wrote on :
> Actually, it is much worse than that: currently when you do a
> "saveenv" 
> it will wipe out the fdt blob because it doesn't know that the blob is
> in the same sector.

IMO this is not correct. The "saveenv" command preserves the content
of the environment sector(s) outside the environment area, if 
CFG_ENV_SECT_SIZE > CFG_ENV_SIZE. The content outside the environment
is copied to RAM before erasing the sector(s) and written back to 
flash afterwards (see common/env_flash.c).

Regards,
Martin

[U-Boot-Users] dtb in env sector - was: (Try 2) Please pull ...

[Joakim Tjernlund]

On Tue, 2007-04-03 at 10:21 -0400, Jerry Van Baren wrote:
> Joakim Tjernlund wrote:
> > On Tue, 2007-04-03 at 14:59 +0200, Wolfgang Denk wrote:
> >> In message <1175600261.12080.24.camel@gentoo-jocke.transmode.se> you wrote:
> >>> I think to start with one would just need an install dtb command that
> >>> places the dtb at the end of the env. sector to allow either one
> >>> to grow without trashing eachother. Also rendundant env needs to be
> >> Umm... The environment will not grow beyond it's defined  size.  Then
> >> just use "cp" to copy the dtb to the next free address.
> > 
> > And later you need some more space for env. so you increase CFG_ENV_SIZE
> > a little and now you have destroyed your dtb as well.
> 
> Actually, it is much worse than that: currently when you do a "saveenv" 
> it will wipe out the fdt blob because it doesn't know that the blob is 
> in the same sector.

Oh, my memory from the good old days(ppcboot) was that saveenv copied 
the whole sector to RAM, modified env. parts and wrote the whole
block back. Probably I remember incorrectly.

> 
> I'm thinking that we want to store the blob immediately after the env 
> variable storage _reserved area_ as a #define option (as a new #define 
> option? part of the CONFIG_OF_LIBFDT define?) and enhance the env 
> save/restore to do the blob too.  This would get us the redundancy and 
> we can cover it with a new (preferred?) or existing env CRC (not good?).

Not sure, havn't had time to think about it yet.

> 
> >>> Secondly one needs someway of telling bootm where to find the dtb.
> >> You pass the address as an argument...
> > 
> > yes, but it would be handy to just say "use the dtb in my spare
> > env space", especially if you use rendundant env. so the right
> > one is selected.
> > 
> >  Regards
> >           Jocke
> 
> Best regards,
> gvb
> 
> 

[U-Boot-Users] dtb in env sector - was: (Try 2) Please pull ...

[Jerry Van Baren]

Joakim Tjernlund wrote:
> On Tue, 2007-04-03 at 10:21 -0400, Jerry Van Baren wrote:
>> Joakim Tjernlund wrote:
>>> On Tue, 2007-04-03 at 14:59 +0200, Wolfgang Denk wrote:
>>>> In message <1175600261.12080.24.camel@gentoo-jocke.transmode.se> you wrote:
>>>>> I think to start with one would just need an install dtb command that
>>>>> places the dtb at the end of the env. sector to allow either one
>>>>> to grow without trashing eachother. Also rendundant env needs to be
>>>> Umm... The environment will not grow beyond it's defined  size.  Then
>>>> just use "cp" to copy the dtb to the next free address.
>>> And later you need some more space for env. so you increase CFG_ENV_SIZE
>>> a little and now you have destroyed your dtb as well.
>> Actually, it is much worse than that: currently when you do a "saveenv" 
>> it will wipe out the fdt blob because it doesn't know that the blob is 
>> in the same sector.
> 
> Oh, my memory from the good old days(ppcboot) was that saveenv copied 
> the whole sector to RAM, modified env. parts and wrote the whole
> block back. Probably I remember incorrectly.

Nope, looks like my bad (per email from Martin Krause).  That is good.

Best regards,
gvb

[U-Boot-Users] dtb in env sector - was: (Try 2) Please pull ...

[Wolfgang Denk]

In message <461262CD.7000309@smiths-aerospace.com> you wrote:
>
> Actually, it is much worse than that: currently when you do a "saveenv" 
> it will wipe out the fdt blob because it doesn't know that the blob is 
> in the same sector.

No, it doesn't; see "common/env_flash.c":

#if defined(CFG_ENV_SECT_SIZE) && (CFG_ENV_SECT_SIZE > CFG_ENV_SIZE)
	...
	/* copy old contents to temporary buffer */
	...
	/* copy current environment to temporary buffer */
	...
	flash_sect_protect(OFF,...);
	...
	flash_sect_erase();
	...
	flash_write();
	...

> I'm thinking that we want to store the blob immediately after the env 
> variable storage _reserved area_ as a #define option (as a new #define 
> option? part of the CONFIG_OF_LIBFDT define?) and enhance the env 

You mean at CFG_ENV_ADDR+CFG_ENV_SIZE ?

> save/restore to do the blob too.  This would get us the redundancy and 
> we can cover it with a new (preferred?) or existing env CRC (not good?).

Not good.

> > yes, but it would be handy to just say "use the dtb in my spare
> > env space", especially if you use rendundant env. so the right
> > one is selected.

I consider this bad design. The environment is one thing, and the dtb
is a different thing. You are mixing two unrelated things here, which
causes interdependencies that can cause a lot of trouble.

I agree it can be done, but I don't recommend to do it.

If you do it, it will remain board-specific. ATM I  don't  intend  to
accept this for the common code.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Out of register space (ugh)"
- vi

[U-Boot-Users] dtb in env sector - was: (Try 2) Please pull ...

[Joakim Tjernlund]

> -----Original Message-----
> From: wd at denx.de [mailto:wd at denx.de] 
> Sent: den 3 april 2007 17:25
> To: Jerry Van Baren
> Cc: joakim.tjernlund at transmode.se; u-boot-users at lists.sourceforge.net
> Subject: Re: [U-Boot-Users] dtb in env sector - was: (Try 2) 
> Please pull ... 
> 
> In message <461262CD.7000309@smiths-aerospace.com> you wrote:
> >
> > Actually, it is much worse than that: currently when you do 
> a "saveenv" 
> > it will wipe out the fdt blob because it doesn't know that 
> the blob is 
> > in the same sector.
> 
> No, it doesn't; see "common/env_flash.c":
> 
> #if defined(CFG_ENV_SECT_SIZE) && (CFG_ENV_SECT_SIZE > CFG_ENV_SIZE)
> 	...
> 	/* copy old contents to temporary buffer */
> 	...
> 	/* copy current environment to temporary buffer */
> 	...
> 	flash_sect_protect(OFF,...);
> 	...
> 	flash_sect_erase();
> 	...
> 	flash_write();
> 	...
> 
> > I'm thinking that we want to store the blob immediately 
> after the env 
> > variable storage _reserved area_ as a #define option (as a 
> new #define 
> > option? part of the CONFIG_OF_LIBFDT define?) and enhance the env 
> 
> You mean at CFG_ENV_ADDR+CFG_ENV_SIZE ?
> 
> > save/restore to do the blob too.  This would get us the 
> redundancy and 
> > we can cover it with a new (preferred?) or existing env CRC 
> (not good?).
> 
> Not good.
> 
> > > yes, but it would be handy to just say "use the dtb in my spare
> > > env space", especially if you use rendundant env. so the right
> > > one is selected.
> 
> I consider this bad design. The environment is one thing, and the dtb
> is a different thing. You are mixing two unrelated things here, which
> causes interdependencies that can cause a lot of trouble.

They are not so different or unrelated, but I don't propose to integrate
them. I think of it as 2 things that needs storage and they should be able
to share an erase block. Redundant env. aside, these two items don't
need to know about oneother. Adding redundant env. makes it a little
bit harder but should be solvable.

I see 2 benefits with this shared storage:
1) less flash space is wasted, 1 (or 2 if you want redundancy)
   erase blocks can be saved.

2) existing boards in can add a dtb without repartioning its
   flash space, you can even upgrade eq. in the field.


 Regards
         Jocke

[U-Boot-Users] dtb in env sector - was: (Try 2) Please pull ...

[Wolfgang Denk]

Hi,

in message <46122D99.3080408@gmail.com> you wrote:
> 
> That is an excellent idea and should be one of our goals: to better 
> coordinate the dft and env variables.  As the penultimate goal, I 
> believe we could move the env variables _into_ the dft and replace the 
> get/set env routines with dft get/set routines.  I'm still thinking 
> about this... env variables are used very early in power up (before RAM 
> is initialized), but I haven't seen any reason that it /can't/ be done.

This depends on how much resources your code needs. Remember that  we
have  to  read  the  environment  before  relocation,  i.  e. without
writable data segment, without BSS, and with just minimal  stack  and
global storage (assume 128 bytes or so :-).

> The downside of dfts is that the code is bigger.  My preliminary size 

We will  probably  also  have  to  keep  the  old  code  for  special
requirements  - for example, some systems store the environment in an
EEPROM.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
This is now.  Later is later.

[U-Boot-Users] dtb in env sector - was: (Try 2) Please pull ...

[Wolfgang Denk]

Hi,

in message <1175593151.12080.17.camel@gentoo-jocke.transmode.se> you wrote:
> 
> This is probably a little off but I am thinking about were to
> put my dtb on the flash and it occured to me that it could fit into the
> environment sector as the environment doesn't use the whole sector.

It is you who defines the usage of yoru flash.

> But one can't just write it into that sector without take measures to
> preserve the environment.

What measures? No matter where you store it in flash  you  must  make
sure   it's  an  otherwise  unused  area.  The  unused  part  of  the
environment sector is not much different.

> Is this a bad idea?

I think it is not a good idea. When something  goes  wrong  during  a
"saveenv"  you lose your dtb. THe environment is still secured if you
use redundant environment, but your dtb is gone.

I would use a dedicated flash sector, but that's just my $ 0.02

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The IQ of the group is the lowest IQ of a member of the group divided
by the number of people in the group.

[U-Boot-Users] dtb in env sector - was: (Try 2) Please pull ...

[Joakim Tjernlund]

On Tue, 2007-04-03 at 14:49 +0200, Wolfgang Denk wrote:
> Hi,
> 
> in message <1175593151.12080.17.camel@gentoo-jocke.transmode.se> you wrote:
> > 
> > This is probably a little off but I am thinking about were to
> > put my dtb on the flash and it occured to me that it could fit into the
> > environment sector as the environment doesn't use the whole sector.
> 
> It is you who defines the usage of yoru flash.

yes and I want to waste as little as possible

> 
> > But one can't just write it into that sector without take measures to
> > preserve the environment.
> 
> What measures? No matter where you store it in flash  you  must  make
> sure   it's  an  otherwise  unused  area.  The  unused  part  of  the
> environment sector is not much different.

You need some help from u-boot to make this practical to use:
Add an command like dtbinstall that given an address saves
the dtb into the free area avaliable in the env. sector (and into
the redundant env sector if enabled)

Then you can download your dtb with tftp into RAM and install
it in a space efficient way. 

> 
> > Is this a bad idea?
> 
> I think it is not a good idea. When something  goes  wrong  during  a
> "saveenv"  you lose your dtb. THe environment is still secured if you
> use redundant environment, but your dtb is gone.
> 
> I would use a dedicated flash sector, but that's just my $ 0.02
> 
> Best regards,
> 
> Wolfgang Denk
> 

[U-Boot-Users] dtb in env sector - was: (Try 2) Please pull ...

[Wolfgang Denk]

In message <1175608728.12080.34.camel@gentoo-jocke.transmode.se> you wrote:
>
> You need some help from u-boot to make this practical to use:
> Add an command like dtbinstall that given an address saves
> the dtb into the free area avaliable in the env. sector (and into
> the redundant env sector if enabled)

Hm... remember that you may have to erase the environment to do that
:-(

Also - what does redundancy give you here? It is not supported by the
bootm command...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"At least they're __________EXPERIENCED incompetents"