Re: [PATCH 1/1 RFC] treewide: Deprecate OF_PRIOR_STAGE

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

On Fri, Sep 24, 2021 at 04:46:58PM +0200, Heinrich Schuchardt wrote:
> 
> 
> On 9/24/21 3:10 PM, Ilias Apalodimas wrote:
> > At some point back in 2018 prior_stage_fdt_address and OF_PRIOR_STAGE got
> > introduced,  in order to support a DTB handed over by an earlier stage boot
> > loader.  However we have another option in the Kconfig (OF_BOARD) which has
> > identical semantics.
> > 
> > A good example of this is RISC-V boards which during their startup,
> > pick up the DTB from a1 and copy it in their private gd_t.  Apart from that
> > they also copy it to prior_stage_fdt_address,  if the Kconfig option is
> > selected,  which seems unnecessary(??).
> > 
> > This is mostly an RFC,  trying to figure out if I am missing some subtle
> > functionality,  which would justify having 2 Kconfig options doing similar
> > things present.
> > 
> > - Should we do this?
> > - Doesn't OF_BOARD and OF_PRIOR_STAGE practically mean "Someone else is
> >    going to pass me my DTB".  Why should we care if that someone is a prior
> >    bootloader or runtime memory generated on the fly by U-Boot?  It all
> >    boils down to having a *board* specific callback for that.
> > - RISC-V binman should get rid of the option as well if we decide to go
> >    though with this (but I have no idea what RISC-V expects there).
> 
> Just replace CONFIG_OF_PRIOR_STAGE by CONFIG_OF_BOARD.

Ah thanks!

> 
> > -			return (ulong *)gd->arch.firmware_fdt_addr;

[...]

> > -		else
> > -			return (ulong *)&_end;
> > -	}
> > +	if (gd->arch.firmware_fdt_addr)
> > +		return (ulong *)gd->arch.firmware_fdt_addr;
> 
> (ulong *) makes no sense here. (void *) would be more adequate.
> 

Yea I preserved what was already in there,  since I thought that was gonna
require a different patch to fix.  But Since I'll be moving these lines
away I might as well fix it.

> >   # elif defined(CONFIG_OF_PRIOR_STAGE)
[...]
> > +	/*
> > +	 * obsolete don't use this on newer boards.  Prefer CONFIG_OF_BOARD
> > +	 * instead
> > +	 */
> 
> This comment should be in Kconfig.

I'll add it on both.  The point is prevent people from doing a similar
thing again,  even if I miss the mail on the list!


Cheers
/Ilias
> 
> Best regards
> 
> Heinrich
> 
> >   	gd->fdt_blob = (void *)(uintptr_t)prior_stage_fdt_address;
> >   # endif
> >   # ifndef CONFIG_SPL_BUILD
> >