Merge dtc

Merge dtc

[David Gibson]

This very large patch incorporates a copy of dtc into the kernel
source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
longer an external dependency to build kernels with configurations
which need a dtb file.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Too big for the list, full patch at
http://ozlabs.org/~dgibson/home/merge-dtc.patch

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

Re: Merge dtc

[Kumar Gala]

On Oct 16, 2007, at 12:02 AM, David Gibson wrote:

> This very large patch incorporates a copy of dtc into the kernel
> source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> longer an external dependency to build kernels with configurations
> which need a dtb file.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>
> Too big for the list, full patch at
> http://ozlabs.org/~dgibson/home/merge-dtc.patch

Dare I ask why we are including dtc in the kernel source tree?  We  
don't really have precedence for this and there are users outside of  
linux for dtc.

- k

Re: Merge dtc

[David Gibson]

On Tue, Oct 16, 2007 at 12:08:06AM -0500, Kumar Gala wrote:
> 
> On Oct 16, 2007, at 12:02 AM, David Gibson wrote:
> 
> > This very large patch incorporates a copy of dtc into the kernel
> > source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> > longer an external dependency to build kernels with configurations
> > which need a dtb file.
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >
> > Too big for the list, full patch at
> > http://ozlabs.org/~dgibson/home/merge-dtc.patch
> 
> Dare I ask why we are including dtc in the kernel source tree?  We  

Well, we aren't unless Paul merges it..

> don't really have precedence for this and there are users outside of  
> linux for dtc.

Because having dtc as an external dependency is inconvenient.  This is
supposed to remain an embedded copy of dtc, not become the main dtc
repository.

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

Re: Merge dtc

[Paul Mackerras]

Kumar Gala writes:

> Dare I ask why we are including dtc in the kernel source tree?  We  
> don't really have precedence for this and there are users outside of  
> linux for dtc.

You must have missed the thread where various people where complaining
about how powerpc is the only architecture where they can't build
kernels without some external tool that they don't have, etc., etc.

We thought about shipping compiled DTBs for various platforms, but the
problem there is that they can't be updated with a patch, so whoever
commits a patch to the relevant .dts would have to remember to run dtc
and commit the updated .dtb as well, which seems a bit error-prone.

In the end, dtc isn't all that much code.  We already have several
other programs that run on the host in the process of making a zImage,
such as wrapper, hack-coff, and addnote, not to mention the various C
programs that are part of Kbuild, and unifdef, kallsyms, etc.

So I think there definitely is a precedent for including dtc.

Paul.

Re: Merge dtc

[Kumar Gala]

On Oct 16, 2007, at 12:39 AM, Paul Mackerras wrote:

> Kumar Gala writes:
>
>> Dare I ask why we are including dtc in the kernel source tree?  We
>> don't really have precedence for this and there are users outside of
>> linux for dtc.
>
> You must have missed the thread where various people where complaining
> about how powerpc is the only architecture where they can't build
> kernels without some external tool that they don't have, etc., etc.

I must have missed this thread.

> We thought about shipping compiled DTBs for various platforms, but the
> problem there is that they can't be updated with a patch, so whoever
> commits a patch to the relevant .dts would have to remember to run dtc
> and commit the updated .dtb as well, which seems a bit error-prone.

agreed, would seem .S would have been a better choice than .dtb, but  
I agree the keeping a .dts and .S form insync would be a bit of a pain.

> In the end, dtc isn't all that much code.  We already have several
> other programs that run on the host in the process of making a zImage,
> such as wrapper, hack-coff, and addnote, not to mention the various C
> programs that are part of Kbuild, and unifdef, kallsyms, etc.
>
> So I think there definitely is a precedent for including dtc.

Just out of interest who's complaining?  We don't include mkimage for  
u-boot related builds and I haven't seen any gripes related to that.

- k

Re: Merge dtc

[Benjamin Herrenschmidt]

On Tue, 2007-10-16 at 00:50 -0500, Kumar Gala wrote:
> Just out of interest who's complaining?  We don't include mkimage
> for  
> u-boot related builds and I haven't seen any gripes related to that.

It's a pain in the neck since those are built even for things that don't
need them (such as 4xx where I use the treeboot images).

Ben.

Re: Merge dtc

[Kumar Gala]

On Oct 16, 2007, at 1:01 AM, Benjamin Herrenschmidt wrote:

>
> On Tue, 2007-10-16 at 00:50 -0500, Kumar Gala wrote:
>> Just out of interest who's complaining?  We don't include mkimage
>> for
>> u-boot related builds and I haven't seen any gripes related to that.
>
> It's a pain in the neck since those are built even for things that  
> don't
> need them (such as 4xx where I use the treeboot images).

That was Paul's decision :)

- k

Re: Merge dtc

[Josh Boyer]

On Tue, 2007-10-16 at 16:01 +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2007-10-16 at 00:50 -0500, Kumar Gala wrote:
> > Just out of interest who's complaining?  We don't include mkimage
> > for  
> > u-boot related builds and I haven't seen any gripes related to that.
> 
> It's a pain in the neck since those are built even for things that don't
> need them (such as 4xx where I use the treeboot images).

You might not need them, but others with 4xx boards do.  Boards like
Sequoia, etc don't use OpenBIOS.  And your compile didn't fail.  It just
didn't create the cuImage.

Anyway, I said a couple months ago we should include mkimage in the
kernel and I never got around to doing it.  I'll add it to the list.

josh

Re: Merge dtc

[Timur Tabi]

Kumar Gala wrote:

> Just out of interest who's complaining?  We don't include mkimage for  
> u-boot related builds and I haven't seen any gripes related to that.

I think we should include mkimage *and* dtc.  But then, I'm not sure how much 
weight my opinion has. :-)

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: Merge dtc

[Josh Boyer]

On Wed, 17 Oct 2007 14:59:04 -0500
Timur Tabi <timur@freescale.com> wrote:

> Kumar Gala wrote:
> 
> > Just out of interest who's complaining?  We don't include mkimage for  
> > u-boot related builds and I haven't seen any gripes related to that.
> 
> I think we should include mkimage *and* dtc.  But then, I'm not sure how much 
> weight my opinion has. :-)

Either way, if we're going to include DTC we need to do it soon.
mkimage I'm less worried about because it should be a much smaller
patch.

josh

Re: Merge dtc

[Grant Likely]

On 10/17/07, Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> On Wed, 17 Oct 2007 14:59:04 -0500
> Timur Tabi <timur@freescale.com> wrote:
>
> > Kumar Gala wrote:
> >
> > > Just out of interest who's complaining?  We don't include mkimage for
> > > u-boot related builds and I haven't seen any gripes related to that.
> >
> > I think we should include mkimage *and* dtc.  But then, I'm not sure how much
> > weight my opinion has. :-)
>
> Either way, if we're going to include DTC we need to do it soon.
> mkimage I'm less worried about because it should be a much smaller
> patch.

Cast my vote on the 'no' side.  I don't like this approach.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: Merge dtc

[Linas Vepstas]

On Wed, Oct 17, 2007 at 02:59:04PM -0500, Timur Tabi wrote:
> Kumar Gala wrote:
> 
> > Just out of interest who's complaining?  We don't include mkimage for  
> > u-boot related builds and I haven't seen any gripes related to that.
> 
> I think we should include mkimage *and* dtc.  But then, I'm not sure how much 
> weight my opinion has. :-)

Isn't anyone concerned about the defacto fork-of-source-code that 
this causes? Which will be the official version? How will the code
baes be kept in sync?

--linas

Re: Merge dtc

[Jon Loeliger]

So, like, the other day Linas Vepstas mumbled:
> 
> Isn't anyone concerned about the defacto fork-of-source-code that 
> this causes?

You betcha.  That's why I'm negative on the notion,
but I won't stand in front of _that_ train either.

> Which will be the official version?

The one I am maintaining here on jdl.com.

> How will the code baes be kept in sync?

I heard Gibson volunteer. :-)

jdl

Re: Merge dtc

[Benjamin Herrenschmidt]

> You must have missed the thread where various people where complaining
> about how powerpc is the only architecture where they can't build
> kernels without some external tool that they don't have, etc., etc.
> 
> We thought about shipping compiled DTBs for various platforms, but the
> problem there is that they can't be updated with a patch, so whoever
> commits a patch to the relevant .dts would have to remember to run dtc
> and commit the updated .dtb as well, which seems a bit error-prone.
> 
> In the end, dtc isn't all that much code.  We already have several
> other programs that run on the host in the process of making a zImage,
> such as wrapper, hack-coff, and addnote, not to mention the various C
> programs that are part of Kbuild, and unifdef, kallsyms, etc.
> 
> So I think there definitely is a precedent for including dtc.

Note that I just tried to build a 4xx kernel today ... and failed due to
the lack of mkimage ..

Another one that we need to either add to the kernel or make the
building of cuImage's optional.

Ben.

Re: Merge dtc

[Kumar Gala]

On Oct 16, 2007, at 1:00 AM, Benjamin Herrenschmidt wrote:

>
>> You must have missed the thread where various people where  
>> complaining
>> about how powerpc is the only architecture where they can't build
>> kernels without some external tool that they don't have, etc., etc.
>>
>> We thought about shipping compiled DTBs for various platforms, but  
>> the
>> problem there is that they can't be updated with a patch, so whoever
>> commits a patch to the relevant .dts would have to remember to run  
>> dtc
>> and commit the updated .dtb as well, which seems a bit error-prone.
>>
>> In the end, dtc isn't all that much code.  We already have several
>> other programs that run on the host in the process of making a  
>> zImage,
>> such as wrapper, hack-coff, and addnote, not to mention the various C
>> programs that are part of Kbuild, and unifdef, kallsyms, etc.
>>
>> So I think there definitely is a precedent for including dtc.
>
> Note that I just tried to build a 4xx kernel today ... and failed  
> due to
> the lack of mkimage ..
>
> Another one that we need to either add to the kernel or make the
> building of cuImage's optional.

I think if we add dtc we add mkimage since its even smaller.

- k

Re: Merge dtc

[Grant Likely]

On 10/15/07, David Gibson <david@gibson.dropbear.id.au> wrote:
> This very large patch incorporates a copy of dtc into the kernel
> source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> longer an external dependency to build kernels with configurations
> which need a dtb file.

Powerpc is probably not going to be the only user of dtc.  Microblaze
will be using it too.  Can it be put somewhere more common?

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: Merge dtc

[Kumar Gala]

On Oct 16, 2007, at 8:17 AM, Grant Likely wrote:

> On 10/15/07, David Gibson <david@gibson.dropbear.id.au> wrote:
>> This very large patch incorporates a copy of dtc into the kernel
>> source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
>> longer an external dependency to build kernels with configurations
>> which need a dtb file.
>
> Powerpc is probably not going to be the only user of dtc.  Microblaze
> will be using it too.  Can it be put somewhere more common?

mkimage should also be put somewhere common since other archs use it.

- k

Re: Merge dtc

[David Gibson]

On Tue, Oct 16, 2007 at 07:17:01AM -0600, Grant Likely wrote:
> On 10/15/07, David Gibson <david@gibson.dropbear.id.au> wrote:
> > This very large patch incorporates a copy of dtc into the kernel
> > source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> > longer an external dependency to build kernels with configurations
> > which need a dtb file.
> 
> Powerpc is probably not going to be the only user of dtc.  Microblaze
> will be using it too.  Can it be put somewhere more common?

Well, I guess we can move it to scripts/ when microblaze starts using
it.

Also, tell me more about this microblaze, I'm certainly interested in
new users of dtc...

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

Re: Merge dtc

[Grant Likely]

On 10/16/07, David Gibson <dwg@au1.ibm.com> wrote:
> On Tue, Oct 16, 2007 at 07:17:01AM -0600, Grant Likely wrote:
> > On 10/15/07, David Gibson <david@gibson.dropbear.id.au> wrote:
> > > This very large patch incorporates a copy of dtc into the kernel
> > > source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> > > longer an external dependency to build kernels with configurations
> > > which need a dtb file.
> >
> > Powerpc is probably not going to be the only user of dtc.  Microblaze
> > will be using it too.  Can it be put somewhere more common?
>
> Well, I guess we can move it to scripts/ when microblaze starts using
> it.
>
> Also, tell me more about this microblaze, I'm certainly interested in
> new users of dtc...

It's a 'soft processor'.  Implemented in the fabric of an FPGA.  It's
a ucLinux target.  Xilinx Virtex and Xilinx MicroBlaze targets share a
lot of common devices.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

RE: Merge dtc

[Stephen Neuendorffer]

=20
> -----Original Message-----
> From:=20
> linuxppc-dev-bounces+stephen.neuendorffer=3Dxilinx.com@ozlabs.or
> g=20
> [mailto:linuxppc-dev-bounces+stephen.neuendorffer=3Dxilinx.com@o
zlabs.org] On Behalf Of Grant Likely
> Sent: Wednesday, October 17, 2007 6:15 AM
> To: Grant Likely; Paul Mackerras; Josh Boyer; linuxppc-dev@ozlabs.org
> Subject: Re: Merge dtc
>=20
> On 10/16/07, David Gibson <dwg@au1.ibm.com> wrote:
> > On Tue, Oct 16, 2007 at 07:17:01AM -0600, Grant Likely wrote:
> > > On 10/15/07, David Gibson <david@gibson.dropbear.id.au> wrote:
> > > > This very large patch incorporates a copy of dtc into the kernel
> > > > source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> > > > longer an external dependency to build kernels with=20
> configurations
> > > > which need a dtb file.
> > >
> > > Powerpc is probably not going to be the only user of dtc.=20
>  Microblaze
> > > will be using it too.  Can it be put somewhere more common?
> >
> > Well, I guess we can move it to scripts/ when microblaze=20
> starts using
> > it.
> >
> > Also, tell me more about this microblaze, I'm certainly=20
> interested in
> > new users of dtc...
>=20
> It's a 'soft processor'.  Implemented in the fabric of an FPGA.  It's
> a ucLinux target.  Xilinx Virtex and Xilinx MicroBlaze targets share a
> lot of common devices.
>=20
> Cheers,
> g.

Basically, this partially works today.   Most of the device information
can come out of a flat device tree, with most of the kernel code copied
straight from arch/powerpc.  I'm still trying to sort out how some of
the architecture specific stuff can be extracted in a smart way.  For
instance, the microblaze has configurable cache sizes, and may or may
not be available.  Currently, this is #defined in the kernel, but I'd
like to get to the point where the same information is pulled out of a
device tree.  My main quandry at the moment is that I'm not wild about
self-modifying code and I'm not sure if the extra overhead of
implementing it with conditionals is significant.

In any event, my plan is to get this into a reasonable state and then
post it here for review.  The main reason why we went with flat device
trees for this was to get as much symmettry as possible between the soft
microblaze and the embedded ppc405 hard core in some FPGAs.

Steve

Re: Merge dtc

[David Woodhouse]

On Tue, 2007-10-16 at 15:02 +1000, David Gibson wrote:
> This very large patch incorporates a copy of dtc into the kernel
> source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> longer an external dependency to build kernels with configurations
> which need a dtb file.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> Too big for the list, full patch at
> http://ozlabs.org/~dgibson/home/merge-dtc.patch

I think this is a bad idea -- it's hardly a difficult for those people
who _do_ need dts to obtain it separately.

It's bad enough that I have to separate out the bootwrapper code, which
probably ought to live outside the kernel. We shouldn't be merging
_more_ stuff in.

-- 
dwmw2

Re: Merge dtc

[David Gibson]

On Tue, Dec 04, 2007 at 01:59:04AM +0000, David Woodhouse wrote:
> 
> On Tue, 2007-10-16 at 15:02 +1000, David Gibson wrote:
> > This very large patch incorporates a copy of dtc into the kernel
> > source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> > longer an external dependency to build kernels with configurations
> > which need a dtb file.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > 
> > Too big for the list, full patch at
> > http://ozlabs.org/~dgibson/home/merge-dtc.patch
> 
> I think this is a bad idea -- it's hardly a difficult for those people
> who _do_ need dts to obtain it separately.
> 
> It's bad enough that I have to separate out the bootwrapper code, which
> probably ought to live outside the kernel. We shouldn't be merging
> _more_ stuff in.

We've been back and forth on this several times, Paul and I finally
concluded this was the better option.

It means we can feel free to use dtc for whatever new platforms we
wish to without people whinging about having to install a new tool.
And it means as dtc evolves to have new useful features, we just need
to ensure that the dts files in the kernel are buildable with the dtc
in the kernel, rather than requireing people to keep updating their
dtc to match what the kernel build needs.

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

Re: Merge dtc

[David Woodhouse]

On Tue, 2007-12-04 at 14:10 +1100, David Gibson wrote:
> We've been back and forth on this several times, Paul and I finally
> concluded this was the better option.

As long as I can just ignore it and use the separately-shipped dtc, I
suppose it doesn't have to bother me too much.

> It means we can feel free to use dtc for whatever new platforms we
> wish to without people whinging about having to install a new tool.

I think we're overestimating this 'problem'; really.

But anyway, if we have to go ahead with it, can we make sure we keep in
sync with the 'real' dtc?  One way of doing that is as follows...

It's not hard to make a git repository which is a simple 'transform' of
another tree -- I have a couple, at
http://git.infradead.org/?p=users/dwmw2/jffs2-ecos-core.git and 
http://git.kernel.org/?p=linux/kernel/git/dwmw2/kernel-headers.git

The scripts to generate those are at 
	http://david.woodhou.se/extract-jffs2-git.sh
	http://david.woodhou.se/extract-khdrs-git.sh 
	http://david.woodhou.se/extract-khdrs-stage2.sh

(The kernel headers one is in two stages because it has to mangle the
files through unifdef too).

I'm sure there are better ways of doing it, but the scripts I have work,
and should hopefully serve as a vaguely useful example.

I'd recommend making such a 'transform' which tracks the upstream dtc
tree but with the files you want moved into the correct places. Then all
we have to do to update the kernel's copy is pull from that transformed
tree.

Of course, it's possible that git has matured to the point where it can
handle the 'renames' and you can just pull from the upstream dtc
repository directly. I don't think that's the case though.

-- 
dwmw2

Re: Merge dtc

[Jon Loeliger]

So, like, the other day David Woodhouse mumbled:
> 
> I think this is a bad idea -- it's hardly a difficult for those people
> who _do_ need dts to obtain it separately.
> 
> We shouldn't be merging _more_ stuff in.

Thanks for chiming in here, David W.  As far as I can tell
so far, the only two people who have voiced an opinion on
this issue are Dave G, submitting patches, and me disagreeing
with the approach.  :-)

Anyone else?

jdl

Re: Merge dtc

[Josh Boyer]

On Tue, 04 Dec 2007 07:25:57 -0600
Jon Loeliger <jdl@jdl.com> wrote:

> So, like, the other day David Woodhouse mumbled:
> > 
> > I think this is a bad idea -- it's hardly a difficult for those people
> > who _do_ need dts to obtain it separately.
> > 
> > We shouldn't be merging _more_ stuff in.
> 
> Thanks for chiming in here, David W.  As far as I can tell
> so far, the only two people who have voiced an opinion on
> this issue are Dave G, submitting patches, and me disagreeing
> with the approach.  :-)
> 
> Anyone else?

I don't see an overwhelmingly great reason to merge it.  It might help
test people who do automated rebuilds, etc and aren't used to dealing
with powerpc and it's requirements.  Outside of that, I see it as
dual-maintenance.

But I'm not doing the maintenance, and it doesn't effect me too much.
I only ask that a decision is made and executed on soon so we can move
on.

josh

Re: Merge dtc

[Kumar Gala]

On Dec 4, 2007, at 9:26 AM, Josh Boyer wrote:

> On Tue, 04 Dec 2007 07:25:57 -0600
> Jon Loeliger <jdl@jdl.com> wrote:
>
>> So, like, the other day David Woodhouse mumbled:
>>>
>>> I think this is a bad idea -- it's hardly a difficult for those  
>>> people
>>> who _do_ need dts to obtain it separately.
>>>
>>> We shouldn't be merging _more_ stuff in.
>>
>> Thanks for chiming in here, David W.  As far as I can tell
>> so far, the only two people who have voiced an opinion on
>> this issue are Dave G, submitting patches, and me disagreeing
>> with the approach.  :-)
>>
>> Anyone else?
>
> I don't see an overwhelmingly great reason to merge it.  It might help
> test people who do automated rebuilds, etc and aren't used to dealing
> with powerpc and it's requirements.  Outside of that, I see it as
> dual-maintenance.
>
> But I'm not doing the maintenance, and it doesn't effect me too much.
> I only ask that a decision is made and executed on soon so we can move
> on.

I'm also in disagreement of duplicating dtc in the kernel.

However, if we are going to do this we should make the path expansion  
for labels work before we do it.

- k

Re: Merge dtc

[David Gibson]

On Tue, Dec 04, 2007 at 10:04:53AM -0600, Kumar Gala wrote:
> 
> On Dec 4, 2007, at 9:26 AM, Josh Boyer wrote:
> 
> > On Tue, 04 Dec 2007 07:25:57 -0600
> > Jon Loeliger <jdl@jdl.com> wrote:
> >
> >> So, like, the other day David Woodhouse mumbled:
> >>>
> >>> I think this is a bad idea -- it's hardly a difficult for those  
> >>> people
> >>> who _do_ need dts to obtain it separately.
> >>>
> >>> We shouldn't be merging _more_ stuff in.
> >>
> >> Thanks for chiming in here, David W.  As far as I can tell
> >> so far, the only two people who have voiced an opinion on
> >> this issue are Dave G, submitting patches, and me disagreeing
> >> with the approach.  :-)
> >>
> >> Anyone else?
> >
> > I don't see an overwhelmingly great reason to merge it.  It might help
> > test people who do automated rebuilds, etc and aren't used to dealing
> > with powerpc and it's requirements.  Outside of that, I see it as
> > dual-maintenance.
> >
> > But I'm not doing the maintenance, and it doesn't effect me too much.
> > I only ask that a decision is made and executed on soon so we can move
> > on.
> 
> I'm also in disagreement of duplicating dtc in the kernel.
> 
> However, if we are going to do this we should make the path expansion  
> for labels work before we do it.

Since we're going to have to update the in-kernel copy reasonably
frequently anyway, I don't see that there's much point in waiting for
particular features to be implemented.

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

Re: Merge dtc

[Scott Wood]

On Tue, Dec 04, 2007 at 07:25:57AM -0600, Jon Loeliger wrote:
> So, like, the other day David Woodhouse mumbled:
> > 
> > I think this is a bad idea -- it's hardly a difficult for those people
> > who _do_ need dts to obtain it separately.
> > 
> > We shouldn't be merging _more_ stuff in.
> 
> Thanks for chiming in here, David W.  As far as I can tell
> so far, the only two people who have voiced an opinion on
> this issue are Dave G, submitting patches, and me disagreeing
> with the approach.  :-)
> 
> Anyone else?

I vote for keeping it separate.

-Scott

Re: Merge dtc

[Paul Mackerras]

David Woodhouse writes:

> I think this is a bad idea -- it's hardly a difficult for those people
> who _do_ need dts to obtain it separately.

The trouble is that it's not just people who are making a kernel for a
specific embedded board that need dtc.  These days anyone who wants to
try cross-compiling a powerpc kernel and does a make allyesconfig, or
who picks cell_defconfig or ps3_defconfig to try, needs dtc if their
kernel build is to go all the way through and give them an exit status
of 0.  I can see that for people who are trying to do the right thing
and compile-test their patch across architectures, it's annoying that
powerpc has an extra external requirement when no other architecture
does, and it usually just means they don't compile-test on powerpc.

Of the various options for solving this, including dtc in the kernel
sources seems best to me.

Paul.

Re: Merge dtc

[David Woodhouse]

On Wed, 2007-12-05 at 09:21 +1100, Paul Mackerras wrote:
> David Woodhouse writes:
> 
> > I think this is a bad idea -- it's hardly a difficult for those people
> > who _do_ need dts to obtain it separately.
> 
> The trouble is that it's not just people who are making a kernel for a
> specific embedded board that need dtc.  These days anyone who wants to
> try cross-compiling a powerpc kernel and does a make allyesconfig, or
> who picks cell_defconfig or ps3_defconfig to try, needs dtc if their
> kernel build is to go all the way through and give them an exit status
> of 0.  I can see that for people who are trying to do the right thing
> and compile-test their patch across architectures, it's annoying that
> powerpc has an extra external requirement when no other architecture
> does, and it usually just means they don't compile-test on powerpc.
> 
> Of the various options for solving this, including dtc in the kernel
> sources seems best to me.

Make vmlinux the default target instead of zImage would seem like a
better answer. I'm surprised that it isn't like that already, in fact --
and I'm only inferring that it isn't from your message. I always thought
that if I typed 'make' I got the vmlinux and not a zImage.

-- 
dwmw2

Re: Merge dtc

[David Woodhouse]

On Tue, 2007-12-04 at 22:33 +0000, David Woodhouse wrote:
> Make vmlinux the default target instead of zImage would seem like a
> better answer. I'm surprised that it isn't like that already, in fact --
> and I'm only inferring that it isn't from your message. I always thought
> that if I typed 'make' I got the vmlinux and not a zImage.

Ooh, no -- I don't. I get an error, and I never even noticed...

  WRAP    arch/powerpc/boot/zImage.chrp
  WRAP    arch/powerpc/boot/zImage.pmac
  WRAP    arch/powerpc/boot/cuImage.52xx
/pmac/git/libertas-2.6/arch/powerpc/boot/wrapper: line 257: mkimage: command not found
make[1]: *** [arch/powerpc/boot/cuImage.52xx] Error 127


I'd be perfectly happy with 'make vmlinux then' as a response to anyone
who complains. And in fact since it'll correctly make the vmlinux and
_then_ fail to create the zImage, I would have thought that anyone with
even a modicum of common sense will _notice_ that, and start using 
'make vmlinux' all by themselves without prompting.

-- 
dwmw2

Re: Merge dtc

[Josh Boyer]

On Wed, 05 Dec 2007 00:54:38 +0000
David Woodhouse <dwmw2@infradead.org> wrote:

> 
> On Tue, 2007-12-04 at 22:33 +0000, David Woodhouse wrote:
> > Make vmlinux the default target instead of zImage would seem like a
> > better answer. I'm surprised that it isn't like that already, in fact --
> > and I'm only inferring that it isn't from your message. I always thought
> > that if I typed 'make' I got the vmlinux and not a zImage.
> 
> Ooh, no -- I don't. I get an error, and I never even noticed...
> 
>   WRAP    arch/powerpc/boot/zImage.chrp
>   WRAP    arch/powerpc/boot/zImage.pmac
>   WRAP    arch/powerpc/boot/cuImage.52xx
> /pmac/git/libertas-2.6/arch/powerpc/boot/wrapper: line 257: mkimage: command not found
> make[1]: *** [arch/powerpc/boot/cuImage.52xx] Error 127
> 
> 
> I'd be perfectly happy with 'make vmlinux then' as a response to anyone
> who complains. And in fact since it'll correctly make the vmlinux and
> _then_ fail to create the zImage, I would have thought that anyone with
> even a modicum of common sense will _notice_ that, and start using 
> 'make vmlinux' all by themselves without prompting.

People build what the default is.  You don't boot a vmlinux, you boot a
zImage (in most cases).

(Nevermind the fact that for the 'build patch on all arches' part Paul
mentioned earlier it doesn't really matter since they probably aren't
going to actually boot it anyway.)

josh 

Re: Merge dtc

[Paul Mackerras]

David Woodhouse writes:

> Make vmlinux the default target instead of zImage would seem like a
> better answer. I'm surprised that it isn't like that already, in fact --
> and I'm only inferring that it isn't from your message. I always thought
> that if I typed 'make' I got the vmlinux and not a zImage.

You're obviously an old-timer. :)  Plain "make" has made the zImage
since at least 2002 in 32-bit and since January 2004 in 64-bit.

The alternative to including dtc is to include compiled versions of
all the .dts files.  The difficulty with that is that .dtb files are
binary blobs which can't be updated with a patch.  The shipped
versions could possibly be shipped as .S versions, or I (and everyone
else who has a tree that I pull from) could have something in my/their
patch-applying scripts that updates the .dtbs if necessary, but in
both cases things could get out of sync for one reason or another
without it being obvious.

In contrast, if the version of dtc in the kernel tree gets out of
date, it will become obvious pretty quickly because compiles will
start failing.  And anyway, the kernel dtc only needs to be recent
enough to compile the .dts files in the kernel tree.

Paul.

Re: Merge dtc

[Josh Boyer]

On Wed, 5 Dec 2007 09:21:21 +1100
Paul Mackerras <paulus@samba.org> wrote:

> David Woodhouse writes:
> 
> > I think this is a bad idea -- it's hardly a difficult for those people
> > who _do_ need dts to obtain it separately.
> 
> The trouble is that it's not just people who are making a kernel for a
> specific embedded board that need dtc.  These days anyone who wants to
> try cross-compiling a powerpc kernel and does a make allyesconfig, or
> who picks cell_defconfig or ps3_defconfig to try, needs dtc if their
> kernel build is to go all the way through and give them an exit status
> of 0.  I can see that for people who are trying to do the right thing
> and compile-test their patch across architectures, it's annoying that
> powerpc has an extra external requirement when no other architecture
> does, and it usually just means they don't compile-test on powerpc.
> 
> Of the various options for solving this, including dtc in the kernel
> sources seems best to me.

Using that same reasoning, should we merge a mkimage patch for the
boards that use U-Boot?

(That's a serious question, not a smart-ass response)

josh

Re: Merge dtc

[Paul Mackerras]

Josh Boyer writes:

> Using that same reasoning, should we merge a mkimage patch for the
> boards that use U-Boot?

I think so.  It's fairly small and it would mean that people could
cross-compile all the defconfigs easily.

Paul.

Re: Merge dtc

[Josh Boyer]

On Wed, 5 Dec 2007 13:22:46 +1100
Paul Mackerras <paulus@samba.org> wrote:

> Josh Boyer writes:
> 
> > Using that same reasoning, should we merge a mkimage patch for the
> > boards that use U-Boot?
> 
> I think so.  It's fairly small and it would mean that people could
> cross-compile all the defconfigs easily.

I'll try to work up a patch tonight.

josh

Re: Merge dtc

[Josh Boyer]

On Tue, 4 Dec 2007 20:26:25 -0600
Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:

> On Wed, 5 Dec 2007 13:22:46 +1100
> Paul Mackerras <paulus@samba.org> wrote:
> 
> > Josh Boyer writes:
> > 
> > > Using that same reasoning, should we merge a mkimage patch for the
> > > boards that use U-Boot?
> > 
> > I think so.  It's fairly small and it would mean that people could
> > cross-compile all the defconfigs easily.
> 
> I'll try to work up a patch tonight.

OK so it won't be tonight.

The problem we have with mkimage, unlike DTC at the moment, is that it
_is_ used on other architectures.  There's already a mkuboot.sh script,
which calls mkimage, in scripts/ right now.  Adding mkimage to
arch/powerpc only seems pretty wrong.

So if mkimage is going to be put in-kernel, I'd rather it be in a more
generic place.  Arguably, dtc should go there as well seeing as how
microblaze could probably use it too.

josh

Re: Merge dtc

[Olof Johansson]

On Tue, Dec 04, 2007 at 10:00:12PM -0600, Josh Boyer wrote:

> So if mkimage is going to be put in-kernel, I'd rather it be in a more
> generic place.  Arguably, dtc should go there as well seeing as how
> microblaze could probably use it too.

Well, kconfig is in scripts/ in spite of not being a script. Maybe
dtc and mkimage should go there too?


-Olof

Re: Merge dtc

[Milton Miller]

On Tue Oct 16 15:02:17 EST 2007, David Gibson wrote:

> This very large patch incorporates a copy of dtc into the kernel
> source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> longer an external dependency to build kernels with configurations
> which need a dtb file.
>
> Signed-off-by: David Gibson <david at gibson.dropbear.id.au>
>
> Too big for the list, full patch at
> http://ozlabs.org/~dgibson/home/merge-dtc.patch+

So split it up.   The obvious one is "here is the unique content, then 
copy these files from dtc git" would have been better.

I finally went and looked at the url.  The Kbuild integration is wrong. 
  It should build dtc in dtc-src and run the binary from there, and the 
rules should be in a Kconfig as a normal host-target in that directory. 
   (I don't have a problem with that Kbuild including Makefie.dtc).

things like the dependancy on scripts_basic in the top level Makefile.

milton

Re: Merge dtc

[Sam Ravnborg]

On Thu, Oct 18, 2007 at 12:49:54PM -0500, Milton Miller wrote:
> On Tue Oct 16 15:02:17 EST 2007, David Gibson wrote:
> 
> >This very large patch incorporates a copy of dtc into the kernel
> >source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> >longer an external dependency to build kernels with configurations
> >which need a dtb file.
> >
> >Signed-off-by: David Gibson <david at gibson.dropbear.id.au>
> >
> >Too big for the list, full patch at
> >http://ozlabs.org/~dgibson/home/merge-dtc.patch+
> 
> So split it up.   The obvious one is "here is the unique content, then 
> copy these files from dtc git" would have been better.
One obvious split is a patch solely containing the _shipped files.
And next patch the rest.

As Milton already pointed out you should build dtc in the
dtc directory (why the -src prefix??).
And the dtc specific Makefile looks like something from
the late 80'. Please drop all these ALLUPPERCASE variables
and accept a little bit of redundancy.
Then mere humans may be able to read the Makefile.

When you have done the above I would be happy to review the
kbuild bits.

	Sam

Re: Merge dtc

[David Gibson]

On Thu, Oct 18, 2007 at 09:59:26PM +0200, Sam Ravnborg wrote:
> On Thu, Oct 18, 2007 at 12:49:54PM -0500, Milton Miller wrote:
> > On Tue Oct 16 15:02:17 EST 2007, David Gibson wrote:
> > 
> > >This very large patch incorporates a copy of dtc into the kernel
> > >source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> > >longer an external dependency to build kernels with configurations
> > >which need a dtb file.
> > >
> > >Signed-off-by: David Gibson <david at gibson.dropbear.id.au>
> > >
> > >Too big for the list, full patch at
> > >http://ozlabs.org/~dgibson/home/merge-dtc.patch+
> > 
> > So split it up.   The obvious one is "here is the unique content, then 
> > copy these files from dtc git" would have been better.
> One obvious split is a patch solely containing the _shipped files.
> And next patch the rest.

Um.. why?  No way is that a logical split.

> As Milton already pointed out you should build dtc in the
> dtc directory (why the -src prefix??).

The -src suffix is only there because I'm not building in the
directory - we can't have both a dtc binary and a dtc directory in
arch/powerpc/boot.  

Ok, so how do I build in the subdirectory?  I was going to do that,
but couldn't for the life of me figure out how.

> And the dtc specific Makefile looks like something from
> the late 80'. Please drop all these ALLUPPERCASE variables
> and accept a little bit of redundancy.

Hrm... I'm pretty dubious about this.  Practically every Makefile in
the universe, *except* Kbuild uses uppercase for most variables.
Makefile.dtc is imported verbatim from the standalone dtc package, and
is supposed to have the minimal information about what needs to be
built to import into Makefiles that actually know how to build things.

> Then mere humans may be able to read the Makefile.

Says a maintainer of Kbuild, about my tiny and not-very-complex
Makefile fragment... um, ok...

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

Re: Merge dtc

[Milton Miller]

On Oct 18, 2007, at 8:45 PM, David Gibson wrote:
> On Thu, Oct 18, 2007 at 09:59:26PM +0200, Sam Ravnborg wrote:
>> On Thu, Oct 18, 2007 at 12:49:54PM -0500, Milton Miller wrote:
>>> On Tue Oct 16 15:02:17 EST 2007, David Gibson wrote:
>>>
>>>> This very large patch incorporates a copy of dtc into the kernel
>>>> source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
>>>> longer an external dependency to build kernels with configurations
>>>> which need a dtb file.
>
>> As Milton already pointed out you should build dtc in the
>> dtc directory (why the -src prefix??).
>
> The -src suffix is only there because I'm not building in the
> directory - we can't have both a dtc binary and a dtc directory in
> arch/powerpc/boot.

So run the dtc binary stored in the sub directory.  Thats what we do 
elsewhere.

> Ok, so how do I build in the subdirectory?  I was going to do that,
> but couldn't for the life of me figure out how.

Documentation/kbuild/makefiles.txt  6.4 boot images:

         "$(Q)$(MAKE) $(build)=<dir>" is the recommended way to invoke
         make in a subdirectory.

Section 4 Host Program Support is also relavent, and mentions $(always).


>> And the dtc specific Makefile looks like something from
>> the late 80'. Please drop all these ALLUPPERCASE variables
>> and accept a little bit of redundancy.
>
> Hrm... I'm pretty dubious about this.  Practically every Makefile in
> the universe, *except* Kbuild uses uppercase for most variables.
> Makefile.dtc is imported verbatim from the standalone dtc package, and
> is supposed to have the minimal information about what needs to be
> built to import into Makefiles that actually know how to build things.
>
>> Then mere humans may be able to read the Makefile.
>
> Says a maintainer of Kbuild, about my tiny and not-very-complex
> Makefile fragment... um, ok...

overley complex calls to override source, conditional rules based on 
shipped files?  Its not a trivial fragment.

milton

Re: Merge dtc

[David Gibson]

On Fri, Oct 19, 2007 at 12:56:41AM -0500, Milton Miller wrote:
> On Oct 18, 2007, at 8:45 PM, David Gibson wrote:
> > On Thu, Oct 18, 2007 at 09:59:26PM +0200, Sam Ravnborg wrote:
> >> On Thu, Oct 18, 2007 at 12:49:54PM -0500, Milton Miller wrote:
> >>> On Tue Oct 16 15:02:17 EST 2007, David Gibson wrote:
> >>>
> >>>> This very large patch incorporates a copy of dtc into the kernel
> >>>> source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> >>>> longer an external dependency to build kernels with configurations
> >>>> which need a dtb file.
> >
> >> As Milton already pointed out you should build dtc in the
> >> dtc directory (why the -src prefix??).
> >
> > The -src suffix is only there because I'm not building in the
> > directory - we can't have both a dtc binary and a dtc directory in
> > arch/powerpc/boot.
> 
> So run the dtc binary stored in the sub directory.  Thats what we do 
> elsewhere.

Yes, yes, which boils down to your other point of building in the
subdirectory.  It's not a separate issue.

> > Ok, so how do I build in the subdirectory?  I was going to do that,
> > but couldn't for the life of me figure out how.
> 
> Documentation/kbuild/makefiles.txt  6.4 boot images:
> 
>          "$(Q)$(MAKE) $(build)=<dir>" is the recommended way to invoke
>          make in a subdirectory.

But where does it go?  To have somewhere to put that rule, we'd still
need a target in arch/powerpc/boot itself.

> Section 4 Host Program Support is also relavent, and mentions $(always).

I know I tried using $(always), but I couldn't figure out how to make
it do something useful.

> >> And the dtc specific Makefile looks like something from
> >> the late 80'. Please drop all these ALLUPPERCASE variables
> >> and accept a little bit of redundancy.
> >
> > Hrm... I'm pretty dubious about this.  Practically every Makefile in
> > the universe, *except* Kbuild uses uppercase for most variables.
> > Makefile.dtc is imported verbatim from the standalone dtc package, and
> > is supposed to have the minimal information about what needs to be
> > built to import into Makefiles that actually know how to build things.
> >
> >> Then mere humans may be able to read the Makefile.
> >
> > Says a maintainer of Kbuild, about my tiny and not-very-complex
> > Makefile fragment... um, ok...
> 
> overley complex calls to override source, 

I'm not sure what you're referring to...  even if you mean in
Makefile.dtc or in the diff to arch/powerpc/boot/Makefile.

>conditional rules based on 
> shipped files?  

Or here, for sure.  The shipped/lex/yacc rules are just based on those
from Kconfig and genksyms.

> Its not a trivial fragment.

Compared to the behemoth that is Kbuild...

I'm happy to improve the Makefile integration, but you seem to be
rather vague on how, and the Kbuild documentation makes my brain hurt.

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

Re: Merge dtc

[Sam Ravnborg]

> Compared to the behemoth that is Kbuild...
> 
> I'm happy to improve the Makefile integration, but you seem to be
> rather vague on how, and the Kbuild documentation makes my brain hurt.

I can make an ALL UPPERCASE VERSION if that makes it easier for you to read ;-)
Give me a day or two then I shall give it a try and see what I can do about it.
I will use the previsous posted URL as basis if you do not tell me otherwise.

	Sam

Re: Merge dtc

[David Gibson]

On Fri, Oct 19, 2007 at 09:07:18AM +0200, Sam Ravnborg wrote:
> > Compared to the behemoth that is Kbuild...
> > 
> > I'm happy to improve the Makefile integration, but you seem to be
> > rather vague on how, and the Kbuild documentation makes my brain hurt.
> 
> I can make an ALL UPPERCASE VERSION if that makes it easier for you
> to read ;-)

Hardy har har.  Since this is supposed to build both within the kernel
and standalone there will necessarily be a few style conflicts at the
boundary.  Kbuild is not in the majority here; in fact it's close to
unique.

> Give me a day or two then I shall give it a try and see what I can
> do about it.  I will use the previsous posted URL as basis if you do
> not tell me otherwise.

Thank you.  The previous URL should be fine, I've made no changes
since then.

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

Re: Merge dtc

[Sam Ravnborg]

Hi David.

> > Give me a day or two then I shall give it a try and see what I can
> > do about it.  I will use the previsous posted URL as basis if you do
> > not tell me otherwise.
> 
> Thank you.  The previous URL should be fine, I've made no changes
> since then.

I decided to go for a kbuild specific version integrated
in boot/Makefile.
This is much more readable because this syntax is explicit.
We do not favour 3 levels of variabls to avoid rewriting the same
filename two times.
I have tested the change only with a O=.. crosscompile build.

I have tested the patch with and without DTC_GENPARSER=1.
It does not rebuild if not needed and is OK with -j10 builds.

Please consider this version in favour of your old version.

Take this as review feedback.

You can add:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org> [kbuild integration only]

Thanks,
	Sam

 arch/powerpc/boot/Makefile |   40 ++++++++++++++++++++++++++++++++++++++--
 1 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 18e3271..064dc07 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -108,17 +108,53 @@ $(patsubst %.S,%.o, $(filter %.S, $(src-boot))): %.o: %.S FORCE
 $(obj)/wrapper.a: $(obj-wlib) FORCE
 	$(call if_changed,bootar)
 
-hostprogs-y	:= addnote addRamDisk hack-coff mktree
+hostprogs-y	:= addnote addRamDisk hack-coff mktree dtc
 
 targets		+= $(patsubst $(obj)/%,%,$(obj-boot) wrapper.a)
 extra-y		:= $(obj)/wrapper.a $(obj-plat) $(obj)/empty.o \
 		   $(obj)/zImage.lds $(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds
 
 wrapper		:=$(srctree)/$(src)/wrapper
-wrapperbits	:= $(extra-y) $(addprefix $(obj)/,addnote hack-coff mktree) \
+wrapperbits	:= $(extra-y) $(addprefix $(obj)/,addnote hack-coff mktree dtc) \
 			$(wrapper) FORCE
 
 #############
+# Bits for building dtc
+# DTC_GENPARSER	:= 1	# Uncomment to rebuild flex/bison output
+
+dtc-objs := dtc.o flattree.o fstree.o data.o livetree.o treesource.o srcpos.o
+dtc-objs += dtc-lexer.lex.o dtc-parser.tab.o
+dtc-objs := $(addprefix dtc-src/, $(dtc-objs))
+
+# prerequisites on generated files needs to be explicit
+$(obj)/dtc-src/dtc-parser.tab.o: $(obj)/dtc-src/dtc-parser.tab.c $(obj)/dtc-src/dtc-parser.tab.h
+$(obj)/dtc-src/dtc-lexer.lex.o:  $(obj)/dtc-src/dtc-lexer.lex.c
+$(obj)/dtc-src/data.o:           $(obj)/dtc-src/dtc-parser.tab.h
+
+HOSTCFLAGS += -I$(src)/dtc-src/
+
+targets += dtc-src/dtc-parser.tab.c
+targets += dtc-src/dtc-lexer.lex.c
+
+ifdef DTC_GENPARSER
+BISON = bison
+FLEX = flex
+
+quiet_cmd_bison = BISON   $@
+      cmd_bison = $(BISON) -o$@ -d $<; cp $@ $@_shipped
+quiet_cmd_flex = FLEX    $@
+      cmd_flex = $(FLEX) -o$@ $<; cp $@ $@_shipped
+
+$(obj)/dtc-src/dtc-parser.tab.c: $(src)/dtc-src/dtc-parser.y FORCE
+	$(call if_changed,bison)
+
+$(obj)/dtc-src/dtc-parser.tab.h: $(obj)/dtc-src/dtc-parser.tab.c
+
+$(obj)/dtc-src/dtc-lexer.lex.c: $(src)/dtc-src/dtc-lexer.l FORCE
+	$(call if_changed,flex)
+endif
+
+#############
 # Bits for building various flavours of zImage
 
 ifneq ($(CROSS32_COMPILE),)

Re: Merge dtc

[David Gibson]

On Fri, Oct 19, 2007 at 08:42:49PM +0200, Sam Ravnborg wrote:
> Hi David.
> 
> > > Give me a day or two then I shall give it a try and see what I can
> > > do about it.  I will use the previsous posted URL as basis if you do
> > > not tell me otherwise.
> > 
> > Thank you.  The previous URL should be fine, I've made no changes
> > since then.
> 
> I decided to go for a kbuild specific version integrated
> in boot/Makefile.
> This is much more readable because this syntax is explicit.
> We do not favour 3 levels of variabls to avoid rewriting the same
> filename two times.

Well, yes, on consideration the substitutions in Makefile.dtc are a
bit over-involved.  Although I still find describing its dozen or so
lines as unreadable when set next to the intricate wonders of Kbuild a
little...  bemusing.

> I have tested the change only with a O=.. crosscompile build.
> 
> I have tested the patch with and without DTC_GENPARSER=1.
> It does not rebuild if not needed and is OK with -j10 builds.
> 
> Please consider this version in favour of your old version.

Ok.  I'll use this version in my next spin; except for adding one
dependency you missed, and removing one which should never have been
there (unneccessary #include, which I've already fixed in dtc
upstream).

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

Re: Merge dtc

[Jon Loeliger]

David Gibson wrote:
>
> Ok.  I'll use this version in my next spin; except for adding one
> dependency you missed, and removing one which should never have been
> there (unneccessary #include, which I've already fixed in dtc
> upstream).
> 


I think if we embed the DTC in the kernel tree, we should
wait until it supports /dts-v1/ files first.

jdl

Re: Merge dtc

[David Gibson]

On Thu, Oct 18, 2007 at 12:49:54PM -0500, Milton Miller wrote:
> On Tue Oct 16 15:02:17 EST 2007, David Gibson wrote:
> 
> > This very large patch incorporates a copy of dtc into the kernel
> > source, in arch/powerpc/boot/dtc-src.  This means that dtc is no
> > longer an external dependency to build kernels with configurations
> > which need a dtb file.
> >
> > Signed-off-by: David Gibson <david at gibson.dropbear.id.au>
> >
> > Too big for the list, full patch at
> > http://ozlabs.org/~dgibson/home/merge-dtc.patch+
> 
> So split it up.   The obvious one is "here is the unique content, then 
> copy these files from dtc git" would have been better.

*facepalm*

The point of the small patches thing is not to glom together logically
separate patches.  It's *not* to split patches up simply for the sake
of making them smaller.

You've suggested the closest thing to a logical split here, but even
then - the dtc files are dead without the Makefile changes, and the
Makefile changes won't build without the dtc files (so the patches
would have to go in reversed order to what you suggest).

Not to mention that the Makefile patch will be tiny and the raw import
will still be way too big for the list.

> I finally went and looked at the url.  The Kbuild integration is wrong. 

Wrong?  Not how you'd prefer them perhaps...

>   It should build dtc in dtc-src and run the binary from there, and the 
> rules should be in a Kconfig as a normal host-target in that directory. 

Why?  This approach has the advantage that the subdirectory contains
*only* verbatim imported files, which could well make updates simpler.

>    (I don't have a problem with that Kbuild including Makefie.dtc).
> 
> things like the dependancy on scripts_basic in the top level
> Makefile.

I'm not even sure what you mean by this.

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

Re: Merge dtc

[Milton Miller]

On Oct 18, 2007, at 8:30 PM, David Gibson wrote:

> On Thu, Oct 18, 2007 at 12:49:54PM -0500, Milton Miller wrote:
>> On Tue Oct 16 15:02:17 EST 2007, David Gibson wrote:
>>> Too big for the list, full patch at
>>> http://ozlabs.org/~dgibson/home/merge-dtc.patch+
>>
>> So split it up.   The obvious one is "here is the unique content, then
>> copy these files from dtc git" would have been better.
>
> *facepalm*
>
> The point of the small patches thing is not to glom together logically
> separate patches.  It's *not* to split patches up simply for the sake
> of making them smaller.

The point of posting on the list is to encourage review before merging. 
  Hiding them behind a download means a lot fewer people are looking at 
the code.

> You've suggested the closest thing to a logical split here, but even
> then - the dtc files are dead without the Makefile changes, and the
> Makefile changes won't build without the dtc files (so the patches
> would have to go in reversed order to what you suggest).
>
> Not to mention that the Makefile patch will be tiny and the raw import
> will still be way too big for the list.

I'm talking about split for review, not necessarily merge.

We split by function for review so that different people pay closer 
attention to their areas of intrest and speciality.  This helps prevent 
people being distracted by the bulk.

The files that are verbatim from the other project have been reviewed 
in that project (at least in theory), and therefore are not likely to 
draw comments.   Especially since they are (1) shadows from another 
project and (2) host files, there is more flexibility and less review 
required, eg relaxed coding standards, correctness already tested, and 
in this case multiple platform testing.

Sam's suggestion to split the generated files was because they require 
no review other than for damage.

In contrast, the files such as the make rules are original and unique 
to this import, and therefore draw comments during review.  If you 
weren't asking for a review, why did you post to the mailing list?

Oh, and the files being in the tree but dead is fine from a bisect 
standpoint.  The patch can be reverted if the makefile doesn't go in 
(not that a maintainer would commit them separately).

>> I finally went and looked at the url.  The Kbuild integration is 
>> wrong.
>
> Wrong?  Not how you'd prefer them perhaps...

Wrong in that its unlike every other program that Kbuild makes.

>>   It should build dtc in dtc-src and run the binary from there, and 
>> the
>> rules should be in a Kconfig as a normal host-target in that 
>> directory.
>
> Why?  This approach has the advantage that the subdirectory contains
> *only* verbatim imported files, which could well make updates simpler.

The file name Kbuild is unlikely to conflict with a file from that 
other repository.  It doesn't contain all the files in that repository 
(or even all in a subdirectory) so there is some filtering going on 
anyways.

>    (I don't have a problem with that Kbuild including Makefie.dtc).
>>
>> things like the dependancy on scripts_basic in the top level
>> Makefile.
>
> I'm not even sure what you mean by this.

I was trying to give a hint where you could find "compile programs in 
that directory so I can run them here" for you to draw upon.