[U-Boot] [PATCH0/6] patchset to support TPL and P1021MDS board

[Scott Wood]

On Mon, 31 Jan 2011 22:34:34 +0100
Wolfgang Denk <wd at denx.de> wrote:

> Dear Scott Wood,
> 
> In message <20110131151506.700ddcd7 at udp111988uds.am.freescale.net> you wrote:
> > 
> > > For example, why must we add the Makefile changes in the first step,
> > > when all the code it references is still missing?  Should this not be
> > > the last step?
> > 
> > If you make it the last step, then the board will exist but not be
> > buildable in the previous step (unless you combine them, but you said
> > that's not what you're asking for).  How is that better?  And is this
> > really worth bickering about?
> 
> Yes, this is better, and this is how we always do it: add the featurs,
> but not enable them unless we have all together, then add the needed
> #defines and make rules to actually use the code.

Those two "this"es don't match.

The latter is what we did do.  We added TPL, but it wasn't enabled
until a board actually turns on CONFIG_HAS_TPL.

The former, what I was asking above if it was what you meant, would be
to have the board be added, enabling CONFIG_HAS_TPL because that's the
only way this board can be built, with a commit that is broken until
the subsequent commit adding TPL to the toplevel makefile is added.  Or
to have the toplevel makefile changes squashed into the board patch.

It's not as if this is a make rule pointing at a specific file (with no
$(BOARDDIR)) that is absent.

> > Because it's not specific to 85xx or p1021mds.  The generic
> > infrastructure for TPL consists of the makefile changes and
> > documentation.  It seems useful to me to separate that for review, but
> 
> A dead / broken make rule and dead documentation is what the generic
> infrastructure for TPL consists of?

What is broken about it?

Yes, the makefile change and documentation are what the generic
infrastructure for TPL consists of.  Yes, it's inactive until a board
enables the feature ("when we have all together"), at which point the
board is required to provide tpl/board/$(BOARDDIR)/Makefile.  Code
which is not board-specific is pulled from nand_spl and main U-Boot via
this board-specific makefile.

BTW, CONFIG_HAS_TPL is actually used in the toplevel makefile changes.

> > if you want it squashed into a board-specific patch instead, fine.
> > Just tell us what you want to see.
> 
> I already did, but here we go:

No, you made some vague statements of general principle, of which your
interpretation apparently differs from mine.  I was hoping for
specifics about this patch set.

> First, please do not add make rules before you have code they apply
> to.

So squash the makefile changes into the board patch?

Which seems to be how nand_spl got added a while back (patch title
"Add support for AMCC Sequoia PPC440EPx eval board").  Maybe the
makefile construct you recently objected to (possibly-empty variable
rule target) would have been more visible if it had been separated out. :-)

What about the division between the mpc85xx portion and the p1021mds
portion?

> After doing this, there is this rudimentary patch to the README.
> From a strictly technical point of view it should be split nto two
> parts: the first one (documenting the existing NAND_SPL variables) is
> independent of the TPL stuff and could be handles separately. The
> second part should be mergeed into the patch that first uses these
> variables.  Note that I do not insist on splitting the README changes.
> It's OK with me to keep this together.

Yes, the NAND_SPL bits were lumped in there for convenience, and to
demonstrate the correspondence.

Do you want the README changes to be a separate patch from the
board/makefile changes?

-Scott