Re: [PATCH 3/4] mpc8349emitx: Add chosen node for default stdout path

[Timur Tabi]

Jerry Van Baren wrote:

> The "force" parameter was added to sort of emulate the previous bootm 
> command behavior (but behave better in the case where /chosen already 
> existed).

I don't think the previous bootm behavior was ever desirable.  In fact, I think 
most people didn't realize that bootm used to create an additional /chosen 
section if one was in the DTS, and the only reason it worked was because the 
kernel picked the right one.  IMHO, I think you can safely remove the 'force' 
parameter and change to code to match "force==true", and no one will care.

I believe all of the DTS files have already been scrubbed of their /chosen 
section, so "force" doesn't matter with any recent DTS.

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 3/4] mpc8349emitx: Add chosen node for default stdout path

[Jerry Van Baren]

Scott Wood wrote:
> Jerry Van Baren wrote:
>> Scott Wood wrote:
>>> Kim Phillips wrote:
 The LIBFDT implementation replaces any existing /chosen with its fixed
 up version.
>>
>> Sort of.  If /chosen doesn't exist, it creates it.
>>
>> If /chosen exists and "force" parameter is false, it doesn't touch it. 
>> If "force"  is true, it creates or fixes up properties.  The "bootm" 
>> command passes in force == false.  The "fdt" command passes in force == 
>> true.
>>
>> The "force" parameter was added to sort of emulate the previous bootm 
>> command behavior (but behave better in the case where /chosen already 
>> existed).
> 
> The problem is that "force" is node-granular, rather than 
> property-granular -- If I add a /chosen/linux,stdout-path in the 
> original dts (or via an fdt command), then bootm will decline to add 
> bootargs and initrd information to the /chosen node.
> 
> -Scott

Hi Scott,

Yes, making "force" property-granular makes more sense.  I'll add that 
to my u-boot-fdt repo.

FWIIW, my original proposal (and code) was to *REMOVE* the automagic 
modifications of the fdt blob from the bootm command.  My original 
proposal was to replace "bootm" in scripts (or in the user's fingers) 
with "fdt chosen && fdt env && fdt bd_t && bootm" (or an appropriate 
combination thereof).  I was shouted down.  ;-)

IMHO, having bootm modify the fdt blob is a poor practice.  Bootm's 
mandate is to boot an image from memory, it *shouldn't* be to rewrite 
the fdt blob.  Unfortunately, we had an existing practice (poor, IMHO) 
of having bootm rewrite the fdt blob and so the current implementation 
was done to meet the "user expectation" of bootm "just working" without 
needing to add "fdt xyz" before the bootm command.

gvb
(wipes the foamy spit off his face)
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 3/4] mpc8349emitx: Add chosen node for default stdout path

[Scott Wood]

Jerry Van Baren wrote:
> Scott Wood wrote:
>> Kim Phillips wrote:
>>> The LIBFDT implementation replaces any existing /chosen with its fixed
>>> up version.
> 
> 
> Sort of.  If /chosen doesn't exist, it creates it.
> 
> If /chosen exists and "force" parameter is false, it doesn't touch it. 
> If "force"  is true, it creates or fixes up properties.  The "bootm" 
> command passes in force == false.  The "fdt" command passes in force == 
> true.
> 
> The "force" parameter was added to sort of emulate the previous bootm 
> command behavior (but behave better in the case where /chosen already 
> existed).

The problem is that "force" is node-granular, rather than 
property-granular -- If I add a /chosen/linux,stdout-path in the 
original dts (or via an fdt command), then bootm will decline to add 
bootargs and initrd information to the /chosen node.

-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 3/4] mpc8349emitx: Add chosen node for default stdout path

[Jerry Van Baren]

Scott Wood wrote:
> Kim Phillips wrote:
>> the old FLAT_TREE u-boot fdt fixup code renames any existing chosen
>> node out of the way, and adds its fixed up version as /chosen.

Not in my experience.  My experience is that it blindly created a second 
/chosen node.

>> The LIBFDT implementation replaces any existing /chosen with its fixed
>> up version.

Sort of.  If /chosen doesn't exist, it creates it.

If /chosen exists and "force" parameter is false, it doesn't touch it. 
If "force"  is true, it creates or fixes up properties.  The "bootm" 
command passes in force == false.  The "fdt" command passes in force == 
true.

The "force" parameter was added to sort of emulate the previous bootm 
command behavior (but behave better in the case where /chosen already 
existed).

Such is the price for not totally breaking user expectations.  Hopefully 
we recalibrate user expectations in the future and improve this.

> Could you point out the code that does this?  I don't see it in either 
> the old code or the new.
> 
> -Scott

Old code - I don't think so.

New code...
u-boot-fdt repo:


u-boot repo:


Hope this makes sense,
gvb
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 3/4] mpc8349emitx: Add chosen node for default stdout path

[Kim Phillips]

On Thu, 19 Jul 2007 16:44:27 -0500
Scott Wood <[EMAIL PROTECTED]> wrote:

> Kim Phillips wrote:
> > the old FLAT_TREE u-boot fdt fixup code renames any existing chosen
> > node out of the way, and adds its fixed up version as /chosen.
> > 
> > The LIBFDT implementation replaces any existing /chosen with its fixed
> > up version.
> 
> Could you point out the code that does this?  I don't see it in either 
> the old code or the new.
> 

libfdt (new): do_fdt() calls fdt_chosen(..., force=1), where, in
fdt_chosen(), the check to not replace the existing node is governed by
the value of force.

[in gvb's fdt branch, where an 'fdt addr' command is no longer required
prior to bootm ing, do_bootm_linux() calls fdt_chosen(...,force=0),
so maybe it should be changed to force=1.]

about the old code, you're right, I don't see it either - I must have
been remembering running something else that did the rename.  Sorry for
the noise.

Kim



Kim
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 3/4] mpc8349emitx: Add chosen node for default stdout path

[Grant Likely]

On 7/19/07, Kim Phillips <[EMAIL PROTECTED]> wrote:
> On Thu, 19 Jul 2007 13:43:17 -0600
> "Grant Likely" <[EMAIL PROTECTED]> wrote:
>
> > On 7/19/07, Scott Wood <[EMAIL PROTECTED]> wrote:
> > > Grant Likely wrote:
> > > > From: Grant Likely <[EMAIL PROTECTED]>
> > > >
> > > > To boot from a cuImage requires the device tree to have a
> > > > linux,stdout-path property in the chosen node.  This patch adds it
> > > > to the .dts files.
> > >
> > > This will break many current u-boots, as they blindly add a /chosen node
> > > regardless of whether one already exists.
> >
> > That really should be fixed then.  In the meantime; I'll resubmit the
> > patch to add the chosen node; but with it commented out.
> >
>
> the old FLAT_TREE u-boot fdt fixup code renames any existing chosen
> node out of the way, and adds its fixed up version as /chosen.
>
> The LIBFDT implementation replaces any existing /chosen with its fixed
> up version.
>
> So it all works out.  Adding chosen/linux,stdout-path properties to the
> kernel dtses is fine.
>
> btw, why are these changes being limited to the ITX and GP?

Simply because I'm working on an itx-gp board.  If this is deemed to
be a good change, I can whip up a patch to fix all boards with my
'best guess' as to what stdout should be.

Cheers,
g.


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 3/4] mpc8349emitx: Add chosen node for default stdout path

[Scott Wood]

Kim Phillips wrote:
> the old FLAT_TREE u-boot fdt fixup code renames any existing chosen
> node out of the way, and adds its fixed up version as /chosen.
> 
> The LIBFDT implementation replaces any existing /chosen with its fixed
> up version.

Could you point out the code that does this?  I don't see it in either 
the old code or the new.

-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 3/4] mpc8349emitx: Add chosen node for default stdout path

[Kim Phillips]

On Thu, 19 Jul 2007 13:43:17 -0600
"Grant Likely" <[EMAIL PROTECTED]> wrote:

> On 7/19/07, Scott Wood <[EMAIL PROTECTED]> wrote:
> > Grant Likely wrote:
> > > From: Grant Likely <[EMAIL PROTECTED]>
> > >
> > > To boot from a cuImage requires the device tree to have a
> > > linux,stdout-path property in the chosen node.  This patch adds it
> > > to the .dts files.
> >
> > This will break many current u-boots, as they blindly add a /chosen node
> > regardless of whether one already exists.
> 
> That really should be fixed then.  In the meantime; I'll resubmit the
> patch to add the chosen node; but with it commented out.
> 

the old FLAT_TREE u-boot fdt fixup code renames any existing chosen
node out of the way, and adds its fixed up version as /chosen.

The LIBFDT implementation replaces any existing /chosen with its fixed
up version.

So it all works out.  Adding chosen/linux,stdout-path properties to the
kernel dtses is fine.

btw, why are these changes being limited to the ITX and GP?

Kim
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 3/4] mpc8349emitx: Add chosen node for default stdout path

[Jerry Van Baren]

Grant Likely wrote:
> On 7/19/07, Scott Wood <[EMAIL PROTECTED]> wrote:
>> Grant Likely wrote:
>>> From: Grant Likely <[EMAIL PROTECTED]>
>>>
>>> To boot from a cuImage requires the device tree to have a
>>> linux,stdout-path property in the chosen node.  This patch adds it
>>> to the .dts files.
>> This will break many current u-boots, as they blindly add a /chosen node
>> regardless of whether one already exists.
> 
> That really should be fixed then.  In the meantime; I'll resubmit the
> patch to add the chosen node; but with it commented out.
> 
> g.

We are working on fixing that with the "u-boot-fdt" subtree, but it is 
slow going and it will not magically fix already fielded boards. 
(Forget that Harry Potter crap magic, _that_ would be some useful magic!)

I think the "proper" solution is not to add the /chosen node in the dts 
but rather to generate a _correct_ one in u-boot.  Note that 
linux,stdout-path is something that _should_ be generated based on the 
u-boot configuration (currently it is hard #defined, IMHO it should be 
part of the u-boot environment).

Looks like it is at fdt_support.c line 152:


Best regards,
gvb
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 3/4] mpc8349emitx: Add chosen node for default stdout path

[Grant Likely]

On 7/19/07, Jerry Van Baren <[EMAIL PROTECTED]> wrote:
> I think the "proper" solution is not to add the /chosen node in the dts
> but rather to generate a _correct_ one in u-boot.  Note that
> linux,stdout-path is something that _should_ be generated based on the
> u-boot configuration (currently it is hard #defined, IMHO it should be
> part of the u-boot environment).

That works for u-boot; but it doesn't work for cuImage.  As it stands
now; you need different dts files for the two boot methods.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 3/4] mpc8349emitx: Add chosen node for default stdout path

[Grant Likely]

On 7/19/07, Scott Wood <[EMAIL PROTECTED]> wrote:
> Grant Likely wrote:
> > From: Grant Likely <[EMAIL PROTECTED]>
> >
> > To boot from a cuImage requires the device tree to have a
> > linux,stdout-path property in the chosen node.  This patch adds it
> > to the .dts files.
>
> This will break many current u-boots, as they blindly add a /chosen node
> regardless of whether one already exists.

That really should be fixed then.  In the meantime; I'll resubmit the
patch to add the chosen node; but with it commented out.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
[EMAIL PROTECTED]
(403) 399-0195
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 3/4] mpc8349emitx: Add chosen node for default stdout path

[Scott Wood]

Grant Likely wrote:
> From: Grant Likely <[EMAIL PROTECTED]>
> 
> To boot from a cuImage requires the device tree to have a
> linux,stdout-path property in the chosen node.  This patch adds it
> to the .dts files.

This will break many current u-boots, as they blindly add a /chosen node 
regardless of whether one already exists.

-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev