[U-Boot] [PATCH v2 0/1] Pass board revision and serial number to the

[U-Boot] [PATCH v2 0/1] Pass board revision and serial number to the

[Anton Gerasimov]

Raspberry Pi proprietary bootloader adds this information to the device
tree to finally land in /proc/cpuinfo.

It is then used by some userspace tools. In particular `gpio` tool from
WiringPi suite would use the wrong register mapping when this
information is missing from /proc/cpuinfo.

Anton Gerasimov (1):
  board: raspberrypi: add serial and revision to the device tree

 board/raspberrypi/rpi/rpi.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

-- 
2.19.1

[U-Boot] [PATCH v2 1/1] board: raspberrypi: add serial and revision to the device tree

[Anton Gerasimov]

Raspberry Pi bootloader adds this node to fdt, but if u-boot script
doesn't reuse the tree provided by it, this information is lost.

Revision and serial are displayed in /proc/cpuinfo after boot.

Suggested-By: Ricardo Salveti <rsalveti@rsalveti.net>
Reported-by: Karl Eaves <karleeaves@gmail.com>
Signed-off-by: Anton Gerasimov <tossel@gmail.com>
---
 board/raspberrypi/rpi/rpi.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index 153a1fdcb7..f6a2cdfbfd 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -238,6 +238,8 @@ static uint32_t rev_scheme;
 static uint32_t rev_type;
 static const struct rpi_model *model;
 
+static uint64_t serial;
+
 #ifdef CONFIG_ARM64
 static struct mm_region bcm2837_mem_map[] = {
 	{
@@ -381,8 +383,8 @@ static void set_serial_number(void)
 		return;
 	}
 
-	snprintf(serial_string, sizeof(serial_string), "%016llx",
-		 msg->get_board_serial.body.resp.serial);
+	serial = msg->get_board_serial.body.resp.serial;
+	snprintf(serial_string, sizeof(serial_string), "%016llx", serial);
 	env_set("serial#", serial_string);
 }
 
@@ -475,6 +477,33 @@ void *board_fdt_blob_setup(void)
 	return (void *)fw_dtb_pointer;
 }
 
+static int ft_add_revision_info(void *blob) {
+	int off;
+	int ret;
+
+	off = fdt_subnode_offset(blob, 0, "system");
+
+	if (off < 0) {
+		off = fdt_add_subnode(blob, 0, "system");
+		if (off < 0)
+			return -1;
+	}
+
+	if (!fdt_getprop(blob, off, "linux,serial", NULL))  {
+		ret = fdt_setprop_u64(blob, off, "linux,serial", serial);
+		if (ret < 0)
+			return -1;
+	}
+
+	if (!fdt_getprop(blob, off, "linux,revision", NULL))  {
+		ret = fdt_setprop_u32(blob, off, "linux,revision", revision);
+		if (ret < 0)
+			return -1;
+	}
+
+	return 0;
+}
+
 int ft_board_setup(void *blob, bd_t *bd)
 {
 	/*
@@ -484,6 +513,8 @@ int ft_board_setup(void *blob, bd_t *bd)
 	 */
 	lcd_dt_simplefb_add_node(blob);
 
+	ft_add_revision_info(blob);
+
 #ifdef CONFIG_EFI_LOADER
 	/* Reserve the spin table */
 	efi_add_memory_map(0, 1, EFI_RESERVED_MEMORY_TYPE, 0);
-- 
2.19.1

[U-Boot] [PATCH v2 1/1] board: raspberrypi: add serial and revision to the device tree

[Alexander Graf]

On 02/20/2019 01:09 PM, Anton Gerasimov wrote:
> Raspberry Pi bootloader adds this node to fdt, but if u-boot script
> doesn't reuse the tree provided by it, this information is lost.
>
> Revision and serial are displayed in /proc/cpuinfo after boot.
>
> Suggested-By: Ricardo Salveti <rsalveti@rsalveti.net>
> Reported-by: Karl Eaves <karleeaves@gmail.com>
> Signed-off-by: Anton Gerasimov <tossel@gmail.com>

WiringPi really shouldn't rely on this information, as it is not present 
on AArch64 for example, but I don't see any harm in providing it. I'll 
leave it to Matthias to apply.

Reviewed-by: Alexander Graf <agraf@suse.de>


Alex

[U-Boot] [PATCH v2 1/1] board: raspberrypi: add serial and revision to the device tree

[Stephen Warren]

On 2/20/19 5:09 AM, Anton Gerasimov wrote:
> Raspberry Pi bootloader adds this node to fdt, but if u-boot script
> doesn't reuse the tree provided by it, this information is lost.
> 
> Revision and serial are displayed in /proc/cpuinfo after boot.

Are these properties documented in the upstream Linux kernel in 
Documentation/devicetree/bindings/? If so, this patch is fine. If not, 
we should not merge it, since we don't want to pollute the DT with 
non-standard properties (or: add this to the upstream kernel DT 
documentation, and then apply this patch once the doc update is approved 
upstream).

[U-Boot] [PATCH v2 1/1] board: raspberrypi: add serial and revision to the device tree

[Alexander Graf]

On 02/20/2019 05:59 PM, Stephen Warren wrote:
> On 2/20/19 5:09 AM, Anton Gerasimov wrote:
>> Raspberry Pi bootloader adds this node to fdt, but if u-boot script
>> doesn't reuse the tree provided by it, this information is lost.
>>
>> Revision and serial are displayed in /proc/cpuinfo after boot.
>
> Are these properties documented in the upstream Linux kernel in 
> Documentation/devicetree/bindings/? If so, this patch is fine. If not, 
> we should not merge it, since we don't want to pollute the DT with 
> non-standard properties (or: add this to the upstream kernel DT 
> documentation, and then apply this patch once the doc update is 
> approved upstream).


Hm, it almost looks like it's a downstream hack. Unfortunately as U-Boot 
we're in this really weird place where people may want to use it to load 
downstream kernels.

I personally would be perfectly fine to just enable CONFIG_OF_BOARD by 
default and defer people to $fdtcontroladdr. That way, they get the 
firmware patched device tree automatically.


Alex

[U-Boot] [PATCH v2 1/1] board: raspberrypi: add serial and revision to the device tree

[Stephen Warren]

On 2/20/19 10:04 AM, Alexander Graf wrote:
> On 02/20/2019 05:59 PM, Stephen Warren wrote:
>> On 2/20/19 5:09 AM, Anton Gerasimov wrote:
>>> Raspberry Pi bootloader adds this node to fdt, but if u-boot script
>>> doesn't reuse the tree provided by it, this information is lost.
>>>
>>> Revision and serial are displayed in /proc/cpuinfo after boot.
>>
>> Are these properties documented in the upstream Linux kernel in 
>> Documentation/devicetree/bindings/? If so, this patch is fine. If not, 
>> we should not merge it, since we don't want to pollute the DT with 
>> non-standard properties (or: add this to the upstream kernel DT 
>> documentation, and then apply this patch once the doc update is 
>> approved upstream).
> 
> Hm, it almost looks like it's a downstream hack. Unfortunately as U-Boot 
> we're in this really weird place where people may want to use it to load 
> downstream kernels.

For similar things in L4T's[1] downstream fork of U-Boot, what I've done 
is this:

a) U-Boot implements the code to set various DT properties, or copy them 
from the DT provided by whatever-runs-before-U-Boot to the DT U-Boot 
provides to whatever-runs-after-U-Boot.

b) Whether U-Boot actually does this set/copy operation is determined by 
the value of an environment variable.

c) The default U-Boot environment enables all set/copy operations 
required by L4T.

d) If someone wants a "pure upstream" DT passed to the kernel, they can 
edit the environment to disable those operations, and save the environment.

This allows users to select which kind of DT they want passed to the kernel.

For example, see dt-edit.* at:
> http://nv-tegra.nvidia.com/gitweb/?p=3rdparty/u-boot.git;a=tree;f=arch/arm/mach-tegra;h=d9a17eec4f0271cdc3932f0cee8c39fb17197a0b;hb=l4t/l4t-r27.1
... and MEM_LAYOUT_ENV_SETTINGS in:
> http://nv-tegra.nvidia.com/gitweb/?p=3rdparty/u-boot.git;a=blob;f=include/configs/tegra210-common.h;h=156b280a00f37f811c96d0006fe2b87bdeb07e74;hb=l4t/l4t-r27.1

I'm pretty sure the value of fdt_copy_node_paths was longer in some 
release, but I don't remember which one, so can't quickly find it. And 
yes, the links above are about copying nodes between DTs, but you can 
imagine a similar flag env. var. for the feature in this current patch too.

[1] NVIDIA Linux for Tegra.

[U-Boot] [PATCH v2 1/1] board: raspberrypi: add serial and revision to the device tree

[Anton Gerasimov]

On 2/20/19 6:15 PM, Stephen Warren wrote:
> On 2/20/19 10:04 AM, Alexander Graf wrote:
>> On 02/20/2019 05:59 PM, Stephen Warren wrote:
>>> On 2/20/19 5:09 AM, Anton Gerasimov wrote:
>>>> Raspberry Pi bootloader adds this node to fdt, but if u-boot script
>>>> doesn't reuse the tree provided by it, this information is lost.
>>>>
>>>> Revision and serial are displayed in /proc/cpuinfo after boot.
>>>
>>> Are these properties documented in the upstream Linux kernel in 
>>> Documentation/devicetree/bindings/? If so, this patch is fine. If 
>>> not, we should not merge it, since we don't want to pollute the DT 
>>> with non-standard properties (or: add this to the upstream kernel DT 
>>> documentation, and then apply this patch once the doc update is 
>>> approved upstream).
>>
>> Hm, it almost looks like it's a downstream hack. Unfortunately as 
>> U-Boot we're in this really weird place where people may want to use 
>> it to load downstream kernels.
>
> For similar things in L4T's[1] downstream fork of U-Boot, what I've 
> done is this:
>
> a) U-Boot implements the code to set various DT properties, or copy 
> them from the DT provided by whatever-runs-before-U-Boot to the DT 
> U-Boot provides to whatever-runs-after-U-Boot.
>
> b) Whether U-Boot actually does this set/copy operation is determined 
> by the value of an environment variable.
>
> c) The default U-Boot environment enables all set/copy operations 
> required by L4T.
>
> d) If someone wants a "pure upstream" DT passed to the kernel, they 
> can edit the environment to disable those operations, and save the 
> environment.
>
> This allows users to select which kind of DT they want passed to the 
> kernel.
>
> For example, see dt-edit.* at:
>> http://nv-tegra.nvidia.com/gitweb/?p=3rdparty/u-boot.git;a=tree;f=arch/arm/mach-tegra;h=d9a17eec4f0271cdc3932f0cee8c39fb17197a0b;hb=l4t/l4t-r27.1 
>>
> ... and MEM_LAYOUT_ENV_SETTINGS in:
>> http://nv-tegra.nvidia.com/gitweb/?p=3rdparty/u-boot.git;a=blob;f=include/configs/tegra210-common.h;h=156b280a00f37f811c96d0006fe2b87bdeb07e74;hb=l4t/l4t-r27.1 
>>
>
> I'm pretty sure the value of fdt_copy_node_paths was longer in some 
> release, but I don't remember which one, so can't quickly find it. And 
> yes, the links above are about copying nodes between DTs, but you can 
> imagine a similar flag env. var. for the feature in this current patch 
> too.
>
> [1] NVIDIA Linux for Tegra.


Yes, that's something implemented in linux-raspberrypi [1] only. My use 
case ([2], [3], or more specifically [4]) for u-boot on Raspberry Pi 
includes letting the user update the device tree as a part of their FIT 
image, so using the device tree provided by the primary bootloader would 
not quite work here.

But I understand that it might not quite belong to the u-boot code base, 
so this functionality might continue its existence as a patch applied in 
meta-updater. As for polluting the device tree, it just imitates the 
pollution already done by the proprietary Raspberry Pi bootloader, so I 
don't think it should be a problem.

[1] 
https://github.com/raspberrypi/linux/blob/rpi-4.19.y/arch/arm/mach-bcm/board_bcm2835.c#L29
[2] https://github.com/advancedtelematic/meta-updater/
[3] https://github.com/advancedtelematic/meta-updater-raspberrypi/
[4] 
https://github.com/advancedtelematic/meta-updater-raspberrypi/blob/master/recipes-bsp/u-boot-otascript/u-boot-otascript/uEnv.txt