diff mbox series

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

Message ID 20230612195107.171748-4-detlev.casanova@collabora.com
State Superseded
Delegated to: Marek Vasut
Headers show
Series rcar: Select the correct device tree | expand

Commit Message

Detlev Casanova June 12, 2023, 7:51 p.m. UTC
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

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(+)

Comments

Marek Vasut June 14, 2023, 1:53 p.m. UTC | #1
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 ?
Detlev Casanova June 14, 2023, 3:10 p.m. UTC | #2
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 ?
Marek Vasut June 14, 2023, 3:32 p.m. UTC | #3
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.
Detlev Casanova June 14, 2023, 3:40 p.m. UTC | #4
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.
Marek Vasut June 14, 2023, 4:32 p.m. UTC | #5
On 6/14/23 17:40, Detlev Casanova wrote:
> 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.

Thanks !
diff mbox series

Patch

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;
+	}
+
+	ret = sysinfo_detect(dev);
+	if (ret) {
+		debug("Cannot detect sysinfo: %d\n", ret);
+		return 0;
+	}
+
+	ret = sysinfo_get_int(dev,
+			      SYSINFO_ID_BOARD_MODEL,
+			      &board_id);
+
+	if (ret) {
+		debug("Cannot get sysinfo int: %d\n", ret);
+		return 0;
+	}
+
+	ret = sysinfo_get_int(dev,
+			      RCAR_SYSINFO_REV_MAJOR,
+			      &rev_major);
+
+	if (ret) {
+		debug("Cannot get sysinfo int: %d\n", ret);
+		return 0;
+	}
+
+	ret = sysinfo_get_int(dev,
+			      RCAR_SYSINFO_REV_MINOR,
+			      &rev_minor);
+
+	if (ret) {
+		debug("Cannot get sysinfo int: %d\n", ret);
+		return 0;
+	}
+
+	if (board_id == BOARD_STARTER_KIT_PRE && rev_major == '2') {
+		/*
+		 * H3 and H3e boards
+		 */
+		if (rev_minor == '0')
+			env_set("fdtfile", "renesas/r8a77951-ulcb.dtb");
+		else if (rev_minor == '1')
+			env_set("fdtfile", "renesas/r8a779m1-ulcb.dtb");
+	}
+
+	return 0;
+}
+
 #ifdef CONFIG_MULTI_DTB_FIT
 int board_fit_config_name_match(const char *name)
 {
diff --git a/configs/rcar3_ulcb_defconfig b/configs/rcar3_ulcb_defconfig
index b8fdb5e3826..752d33d77ec 100644
--- a/configs/rcar3_ulcb_defconfig
+++ b/configs/rcar3_ulcb_defconfig
@@ -111,3 +111,4 @@  CONFIG_USB_XHCI_HCD=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_GENERIC=y
 CONFIG_USB_STORAGE=y
+CONFIG_MISC_INIT_R=y