Re: [PATCH v2 3/3] renesas: rcar3: Load the correct device tree

[Detlev Casanova <detlev.casanova@collabora.com>]

On Wednesday, June 14, 2023 11:32:31 A.M. EDT Marek Vasut wrote:
> On 6/14/23 17:10, Detlev Casanova wrote:
> > On Wednesday, June 14, 2023 9:53:14 A.M. EDT Marek Vasut wrote:
> >> On 6/12/23 21:51, Detlev Casanova wrote:
> >>> The Renesas R-Car Gen3 boards use different device trees than
> >>> the default one.
> >>> 
> >>> This commit uses the sysinfo's board id and revision
> >>> 
> >>> to determine which linux device tree to load:
> >>>    * H3 (Starter Kit Premier v2.0): renesas/r8a77951-ulcb.dtb
> >>>    * H3e (Starter Kit Premier v2.1): renesas/r8a779m1-ulcb.dtb
> >> 
> >> This is not about loading DTs (as the subject would suggest), this is
> >> about setting the correct default DT name in environment.
> >> 
> >>> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> >>> ---
> >>> 
> >>>    board/renesas/ulcb/ulcb.c    | 59
> >>>    ++++++++++++++++++++++++++++++++++++
> >>>    configs/rcar3_ulcb_defconfig |  1 +
> >>>    2 files changed, 60 insertions(+)
> >>> 
> >>> diff --git a/board/renesas/ulcb/ulcb.c b/board/renesas/ulcb/ulcb.c
> >>> index 1477750f921..cc78e0952b6 100644
> >>> --- a/board/renesas/ulcb/ulcb.c
> >>> +++ b/board/renesas/ulcb/ulcb.c
> >>> @@ -27,6 +27,7 @@
> >>> 
> >>>    #include <asm/arch/sh_sdhi.h>
> >>>    #include <i2c.h>
> >>>    #include <mmc.h>
> >>> 
> >>> +#include <sysinfo.h>
> >>> 
> >>>    DECLARE_GLOBAL_DATA_PTR;
> >>> 
> >>> @@ -65,6 +66,64 @@ int board_init(void)
> >>> 
> >>>    	return 0;
> >>>    
> >>>    }
> >>> 
> >>> +int misc_init_r(void)
> >>> +{
> >>> +	struct udevice *dev;
> >>> +	int board_id;
> >>> +	int rev_major, rev_minor;
> >>> +	int ret = sysinfo_get(&dev);
> >>> +
> >>> +	if (ret) {
> >>> +		debug("Cannot get sysinfo: %d\n", ret);
> >>> +		return 0;
> >> 
> >> Why do we ignore errors here ?
> >> 
> >>> +	}
> >>> +
> >>> +	ret = sysinfo_detect(dev);
> >>> +	if (ret) {
> >>> +		debug("Cannot detect sysinfo: %d\n", ret);
> >>> +		return 0;
> >>> +	}
> >> 
> >> Looking at all this, I really have to wonder, wouldn't it be nicer to
> >> introduce a 'sysinfo' command which provides interface to obtain the
> >> different properties (like board name, id, revision ...) from U-Boot
> >> command line, and then script the DT selection in U-Boot shell ?
> > 
> > Yes, that could be a good option. This is more based on how raspberry pis
> > are selecting the correct devicetree in `board/raspberrypi/rpi/rpi.c`. It
> > is either about having simple shell scripts that are similar between
> > devices and the implementation is "hidden" in C for each platform (maybe
> > easier to use but less flexible). Or more complex shell scripts with
> > simpler C implementation (more flexible but having to modify a boot
> > script can become complicated for users)
> > 
> > Has this direction choice been discussed in the past already ?
> 
> The less hard-coded board code (which cannot be updated by the user
> easily), the better. Scripts can be updated in deployment far easier
> than the bootloader itself. Hence the push for scripts over custom C code.

That makes sense. I'll create a new command for this then and use it to select 
the dtb in the ulcb boards script.