[U-Boot] [RFC] armv8: layerscape: Add support of u-boot device tree fix-up

[Prabhakar Kushwaha]

> -----Original Message-----
> From: Scott Wood [mailto:oss at buserror.net]
> Sent: Tuesday, February 23, 2016 12:14 PM
> To: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>; u-
> boot at lists.denx.de
> Cc: york sun <york.sun at nxp.com>; Priyanka Jain <priyanka.jain at nxp.com>
> Subject: Re: [RFC] armv8: layerscape: Add support of u-boot device tree fix-
> up
> 
> On Tue, 2016-02-23 at 04:09 +0000, Prabhakar Kushwaha wrote:
> > > -----Original Message-----
> > > From: Scott Wood [mailto:oss at buserror.net]
> > > Sent: Tuesday, February 23, 2016 6:52 AM
> > > To: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>; u-
> > > boot at lists.denx.de
> > > Cc: york sun <york.sun at nxp.com>; Priyanka Jain
> > > <priyanka.jain at nxp.com>
> > > Subject: Re: [RFC] armv8: layerscape: Add support of u-boot device
> > > tree
> > > fix-
> > > up
> > >
> > > On Mon, 2016-02-22 at 16:05 +0530, Prabhakar Kushwaha wrote:
> > > > There is a requirement of u-boot dts fix-up before it is being
> > > > consumed by u-boot.
> > >
> > > You might want to explain the reason *why* we have this requirement
> > > -- that the board takes a socketed SoC, and we don't want to have to
> > > reflash the board if one SoC is unplugged and another plugged in.
> > >
> > > >
> > > > NXP's SoC LS2085A, LS2080A and LS2088A are almost same except
> > > > variation in ARM core where LS2085A/LS2080A has A57 and LS2088A
> > > > has
> > > A72.
> > > > These SoCs will be placed on common LS2085ARDB platform.
> > > >
> > > > So instead of maintaining defferent device tree per SoC, fix-up
> > > > dts before being used by u-boot.
> > > >
> > > > Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>
> > >
> > > And what happens when the next socketed board can support chips with
> > > device trees that are significantly more different?
> >
> > Usually next revision of board should not have major change in terms
> > of interface.
> > If it is **different** new device tree, defconfig, <board>_config.h
> > needs to be used.
> 
> This has nothing to do with different revisions of a board.  It has to do with
> the set of chips that can be plugged into the *same* board.
> 
> > > There should be a  mechanism for having multiple device trees
> > > present, and choosing one based on platform code.
> > >  That would also avoid any problems of trying to modify a device
> > > tree before relocation.
> > >
> >
> > I agree. But it may have following problems
> >  - Increasing bootloader size (u-boot + dtb(s)).
> 
> Not by much.  If some boot sources are so close to the limit that this presents
> a problem, then we can deal with it, but this should not be a problem with
> NOR flash at least.
> 

I agree. But we may not be looking to increase u-boot size too much.  

> >  - Need to maintain different dts per combination like LS2080 + LS2085ARDB
> ,
> >    LS2088 + LS2085ARDB, LS2085A + LS2085ARDB
> 
> So?  Includes make it easy to maintain variants.

I agree.  It will provide better dts understanding. 

Same can be achieved by  "fdt print" to get u-boot fixed dts. 

> 
> >  Here board is common and SoC getting change
> >
> > Assumption: SoC does not have major change.
> 
> I don't think that's a good assumption -- and what constitutes a major
> change?
>  A different core could be considered a major change.  In any case, you need
> specialized fixup code for every type of difference between the SoCs.
> 

Ideally dts fix-up approach should only be applied to new Revision of SoCs. 
And one should be very careful of using this approach for new SoC.

> Choosing from multiple device trees is a cleaner solution that does not place
> limits on the type of change from SoC to SoC, and does not consume OCRAM
> (among other benefits, this makes the mechanism transferable to other
> types of devices that may not have enough pre-relocation RAM to hold a
> device tree).

Choosing from multiple device tree is cleaner approach.  I agree with you. 

But same should be applied for Linux also.  We should not have different approach for u-boot dts and Linux dts. 

I will suggest to have both    a) dts fix-up                  b) multiple dts 

SoC with small change can follow "a"

SoC with major change can follow "b" assuming same approach has been accepted in Linux. 


> 
> > > > diff --git a/include/configs/ls2080a_common.h
> > > > b/include/configs/ls2080a_common.h
> > > > index def0a6f..aa5ace9 100644
> > > > --- a/include/configs/ls2080a_common.h
> > > > +++ b/include/configs/ls2080a_common.h
> > > > @@ -24,6 +24,7 @@
> > > >
> > > >  /* Link Definitions */
> > > >  #define CONFIG_SYS_INIT_SP_ADDR
> > > 	(CONFIG_SYS_FSL_OCRAM_BASE +
> > > > 0xfff0)
> > > > +#define CONFIG_SYS_DTS_ADDR
> > > 	(CONFIG_SYS_FSL_OCRAM_BASE +
> > > > 0xfff0)
> > >
> > > Why 0xfff0, and not, say, 0x10000 (or rather, why is INIT_SP_ADDR
> > > 0xfff0 and not 0x10000 if there is no need to reserve some space
> > > above the initial SP addr (e.g. to indicate the end of a
> > > traceback))?
> >
> > I used the 0xfff0 thinking STACK moving from 0xfff0 to 0x0000.
> > So better put dts starting from 0xfff0 to high address.
> 
> My point is that there is likely a reason that the stack ends at an odd address
> like 0xfff0 rather than 0x10000, and most likely that reason is to reserve a
> location at the end of the stack that contains zeroes, to terminate a stack
> traceback.  So please don't put something else there.
> 

Dtb can be put 128K - 32K location 17FF0.  I hope dts will not be more than 32K. Current Linux dts is ~27K
It is platform dependent define. 

> > > Where do you check that the device tree fits in OCRAM?  What about
> > > when SPL is occupying OCRAM?  Does the device tree get used with SPL
> > > (I don't think we were using FDT control at all the last time I
> > > worked with SPL on these chips)?
> > >
> >
> > I checked ls2085ardb dtb size in Linux. It is ~30K. So from 0xfff0 to
> > 20000 there is enough space.
> 
> What about when we have PPA or similar?  How much space will that take
> up?
> 

PPA will not be using OCRAM. 
If my understanding is correct PPA will be loaded directly in DDR.  There is 2 MB secure DDR for the same. 

> 
> > > > diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 1b1ca02..fc200cf
> > > > 100644
> > > > --- a/lib/fdtdec.c
> > > > +++ b/lib/fdtdec.c
> > > > @@ -76,6 +76,14 @@ static const char * const
> > > compat_names[COMPAT_COUNT] = {
> > > >  	COMPAT(COMPAT_INTEL_IVYBRIDGE_FSP, "intel,ivybridge-fsp"),  };
> > > >
> > > > +void __ft_early_cpu_setup(void **blob) {
> > > > +	return;
> > > > +}
> > > > +void ft_early_cpu_setup(void **blob)
> > > > +	__attribute__((weak, alias("__ft_early_cpu_setup")));
> > > > +
> > > > +
> > >
> > > Why is common code getting infrastructure that assumes CPU is some
> > > special consideration?  If we allow fixups for this, why not for other
> things?
> > >
> >
> > fl_early_cpu_setup has been creating similar to ft_cpu_setup which
> > calls all necessary fix-up including cpu.
> 
> I see ft_cpu_setup() doing a bunch of things that aren't related to a cpu, so
> that's not exactly a good pattern to follow.
> 

I will update it.

--prabhakar