[PATCH 0/3] VideoCore FDT interop for Raspberry Pi

[PATCH 0/3] VideoCore FDT interop for Raspberry Pi

[Tomaz Solc]

Dear all,

these patches make Barebox aware of the device tree and boot arguments that are
constructed by the VideoCore firmware on Raspberry Pi. This fixes many problems
with hardware that is initialized from VideoCore when booting a kernel shipped
with Raspbian (e.g. when using dtoverlay directives in config.txt, bcm2708_fb
arguments, etc.) They are based on previous work done by Pascal Vizeli [1].

Passing of FDT between PBL and rpi_devices_init is somewhat ugly, but as far as
I can see, the patch mentioned in [2] has not been merged yet.

Overview of the changes:

 - PBL saves the VideoCore FDT into a scrap RAM area just above Barebox memory
   (this was an alternative approach suggested in [2]).
 - rpi_devices_init copies the FDT from scrap RAM into a file (/vd.dtb)
 - I had to add arm_mem_endmem_get() so that code in rpi_devices_init can get
   the pointer to the end of Barebox memory.
 - The new of_bootargs command makes it possible for an environment to include
   the kernel command-line from the VideoCore FDT.

[1] http://lists.infradead.org/pipermail/barebox/2018-June/033460.html
[2] http://lists.infradead.org/pipermail/barebox/2018-June/033469.html

Best regards
Tomaz

*** BLURB HERE ***

Tomaz Solc (3):
  ARM: start: save end of memory passed to start.
  ARM: rpi: save fdt that was passed from VideoCore
  commands: add of_bootargs command.

 Documentation/boards/bcm2835.rst          |  8 +++
 arch/arm/boards/raspberry-pi/lowlevel.c   | 68 +++++++++++++++------
 arch/arm/boards/raspberry-pi/lowlevel.h   |  9 +++
 arch/arm/boards/raspberry-pi/rpi-common.c | 34 +++++++++++
 arch/arm/cpu/start.c                      |  8 +++
 arch/arm/include/asm/barebox-arm.h        |  1 +
 commands/Kconfig                          | 13 ++++
 commands/Makefile                         |  1 +
 commands/of_bootargs.c                    | 99 +++++++++++++++++++++++++++++++
 9 files changed, 222 insertions(+), 19 deletions(-)
 create mode 100644 arch/arm/boards/raspberry-pi/lowlevel.h
 create mode 100644 commands/of_bootargs.c

-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/3] ARM: start: save end of memory passed to start.

[Tomaz Solc]

Knowing the address of the end of the memory area used by Barebox is
useful if PBL stores some extra data after it, so that board init code
can later retrieve it from there.
---
 arch/arm/cpu/start.c               | 8 ++++++++
 arch/arm/include/asm/barebox-arm.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/arch/arm/cpu/start.c b/arch/arm/cpu/start.c
index 768fa9e1b..6573c2ef7 100644
--- a/arch/arm/cpu/start.c
+++ b/arch/arm/cpu/start.c
@@ -38,6 +38,7 @@
 
 unsigned long arm_stack_top;
 static unsigned long arm_barebox_size;
+static unsigned long arm_endmem;
 static void *barebox_boarddata;
 static unsigned long barebox_boarddata_size;
 
@@ -131,6 +132,12 @@ unsigned long arm_mem_ramoops_get(void)
 }
 EXPORT_SYMBOL_GPL(arm_mem_ramoops_get);
 
+unsigned long arm_mem_endmem_get(void)
+{
+	return arm_endmem;
+}
+EXPORT_SYMBOL_GPL(arm_mem_endmem_get);
+
 static int barebox_memory_areas_init(void)
 {
 	if(barebox_boarddata)
@@ -163,6 +170,7 @@ __noreturn void barebox_non_pbl_start(unsigned long membase,
 
 	pr_debug("memory at 0x%08lx, size 0x%08lx\n", membase, memsize);
 
+	arm_endmem = endmem;
 	arm_stack_top = arm_mem_stack_top(membase, endmem);
 	arm_barebox_size = barebox_size;
 	malloc_end = barebox_base;
diff --git a/arch/arm/include/asm/barebox-arm.h b/arch/arm/include/asm/barebox-arm.h
index e065b479e..a11d34923 100644
--- a/arch/arm/include/asm/barebox-arm.h
+++ b/arch/arm/include/asm/barebox-arm.h
@@ -86,6 +86,7 @@ static inline void boarddata_create(void *adr, u32 machine)
 u32 barebox_arm_machine(void);
 
 unsigned long arm_mem_ramoops_get(void);
+unsigned long arm_mem_endmem_get(void);
 
 struct barebox_arm_boarddata_compressed_dtb {
 #define BAREBOX_ARM_BOARDDATA_COMPRESSED_DTB_MAGIC 0x7b66bcbd
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] ARM: rpi: save fdt that was passed from VideoCore

[Tomaz Solc]

On Raspberry Pi, VideoCore firmware creates a device tree that contains
information about peripherals that were initialized by VideoCore based
on settings in config.txt. Normally this device tree is passed to the
Linux kernel via a pointer in the r2 register. A bootloader needs to
pass this device tree to the kernel, or some peripherals will not work
correctly.

Since the VideoCore device tree is not compatible with barebox, we can't
just pass it to barebox_arm_entry() as the internal barebox device tree.

This commit makes the prebootloader code copy the device tree from
VideoCore into a scrap RAM area just above the area reserved for the
bootloader. Board initialization code in the bootloader proper then
copies it into a file /vc.dtb. The bootloader environment is then free
to pass this file to the kernel at boot (e.g. via bootm -o).
---
 Documentation/boards/bcm2835.rst          |  4 ++
 arch/arm/boards/raspberry-pi/lowlevel.c   | 68 ++++++++++++++++++++++---------
 arch/arm/boards/raspberry-pi/lowlevel.h   |  9 ++++
 arch/arm/boards/raspberry-pi/rpi-common.c | 34 ++++++++++++++++
 4 files changed, 96 insertions(+), 19 deletions(-)
 create mode 100644 arch/arm/boards/raspberry-pi/lowlevel.h

diff --git a/Documentation/boards/bcm2835.rst b/Documentation/boards/bcm2835.rst
index ea80d5834..95e910896 100644
--- a/Documentation/boards/bcm2835.rst
+++ b/Documentation/boards/bcm2835.rst
@@ -30,5 +30,9 @@ Raspberry Pi
 
   6. Turn board's power on.
 
+VideoCore firmware creates a device tree based on the entries in ``config.txt``. This file is available to the Barebox environment in the file ``/vc.dtb``. For example, to boot a kernel shipped with Raspbian::
+
+    bootm -o /vc.dtb /boot/kernel7.img
+
 .. _Raspberry Pi firmware: https://codeload.github.com/raspberrypi/firmware/zip/80e1fbeb78f9df06701d28c0ed3a3060a3f557ef
 .. _documentation for config.txt: https://www.raspberrypi.org/documentation/configuration/config-txt/
diff --git a/arch/arm/boards/raspberry-pi/lowlevel.c b/arch/arm/boards/raspberry-pi/lowlevel.c
index 1a3d39421..4b64f5d1d 100644
--- a/arch/arm/boards/raspberry-pi/lowlevel.c
+++ b/arch/arm/boards/raspberry-pi/lowlevel.c
@@ -3,43 +3,73 @@
 #include <common.h>
 #include <linux/sizes.h>
 #include <mach/platform.h>
+#include <of.h>
 
-extern char __dtb_bcm2835_rpi_start[];
-ENTRY_FUNCTION(start_raspberry_pi1, r0, r1, r2)
+#include "lowlevel.h"
+
+static void copy_vc_fdt(void *dest, void *src, unsigned long max_size)
 {
-	void *fdt = __dtb_bcm2835_rpi_start + get_runtime_offset();
+	struct fdt_header *oftree_src = src;
+	struct fdt_header *oftree_dest = dest;
 
-	arm_cpu_lowlevel_init();
+	unsigned long size = be32_to_cpu(oftree_src->totalsize);
+	if (size > max_size) {
+		oftree_dest->magic = cpu_to_be32(VIDEOCORE_FDT_ERROR);
+		/* Save an error code after the magic value for easier
+		 * debugging. We can't print out anything this early */
+		oftree_dest->totalsize = cpu_to_be32(ENOMEM);
+		return;
+	}
 
-	barebox_arm_entry(BCM2835_SDRAM_BASE, SZ_128M, fdt);
+	memmove(dest, src, size);
 }
 
-extern char __dtb_bcm2836_rpi_2_start[];
-ENTRY_FUNCTION(start_raspberry_pi2, r0, r1, r2)
+/* Must be inline since stack isn't setup yet. */
+static inline void start_raspberry_pi(unsigned long memsize, void *fdt,
+								void *vc_fdt)
 {
-	void *fdt = __dtb_bcm2836_rpi_2_start + get_runtime_offset();
+	void *saved_vc_fdt;
+	unsigned long membase = BCM2835_SDRAM_BASE;
+
+	/* A pointer to the FDT created by VideoCore was passed to us in r2. We
+	 * reserve some memory just above the region used for Basebox and copy
+	 * this FDT there. We fetch it from there later in rpi_devices_init().*/
+	memsize -= VIDEOCORE_FDT_SZ;
 
 	arm_cpu_lowlevel_init();
 
-	barebox_arm_entry(BCM2835_SDRAM_BASE, SZ_512M, fdt);
+	/* Copied from barebox_arm_entry(). We need stack here early
+	 * for normal function calls to work. */
+	arm_setup_stack(arm_mem_stack_top(membase, membase + memsize) - 16);
+
+	fdt += get_runtime_offset();
+
+	saved_vc_fdt = (void *)(membase + memsize);
+	copy_vc_fdt(saved_vc_fdt, vc_fdt, VIDEOCORE_FDT_SZ);
+
+	barebox_arm_entry(membase, memsize, fdt);
 }
 
-extern char __dtb_bcm2837_rpi_3_start[];
-ENTRY_FUNCTION(start_raspberry_pi3, r0, r1, r2)
+extern char __dtb_bcm2835_rpi_start[];
+ENTRY_FUNCTION(start_raspberry_pi1, r0, r1, r2)
 {
-	void *fdt = __dtb_bcm2837_rpi_3_start + get_runtime_offset();
+	start_raspberry_pi(SZ_128M, __dtb_bcm2835_rpi_start, (void *)r2);
+}
 
-	arm_cpu_lowlevel_init();
+extern char __dtb_bcm2836_rpi_2_start[];
+ENTRY_FUNCTION(start_raspberry_pi2, r0, r1, r2)
+{
+	start_raspberry_pi(SZ_512M, __dtb_bcm2836_rpi_2_start, (void *)r2);
+}
 
-	barebox_arm_entry(BCM2835_SDRAM_BASE, SZ_512M, fdt);
+extern char __dtb_bcm2837_rpi_3_start[];
+ENTRY_FUNCTION(start_raspberry_pi3, r0, r1, r2)
+{
+	start_raspberry_pi(SZ_512M, __dtb_bcm2837_rpi_3_start, (void *)r2);
 }
 
 extern char __dtb_bcm2837_rpi_cm3_start[];
 ENTRY_FUNCTION(start_raspberry_pi_cm3, r0, r1, r2)
 {
-	void *fdt = __dtb_bcm2837_rpi_cm3_start + get_runtime_offset();
-
-	arm_cpu_lowlevel_init();
-
-	barebox_arm_entry(BCM2835_SDRAM_BASE, SZ_512M, fdt);
+	start_raspberry_pi(SZ_512M, __dtb_bcm2837_rpi_cm3_start, (void *)r2);
 }
diff --git a/arch/arm/boards/raspberry-pi/lowlevel.h b/arch/arm/boards/raspberry-pi/lowlevel.h
new file mode 100644
index 000000000..9ef9135b2
--- /dev/null
+++ b/arch/arm/boards/raspberry-pi/lowlevel.h
@@ -0,0 +1,9 @@
+#ifndef __ARCH_ARM_BOARDS_LOWLEVEL_H__
+#define __ARCH_ARM_BOARDS_LOWLEVEL_H__
+
+#include <linux/sizes.h>
+
+#define VIDEOCORE_FDT_SZ SZ_1M
+#define VIDEOCORE_FDT_ERROR 0xdeadfeed
+
+#endif /* __ARCH_ARM_BOARDS_LOWLEVEL_H__ */
diff --git a/arch/arm/boards/raspberry-pi/rpi-common.c b/arch/arm/boards/raspberry-pi/rpi-common.c
index b5d16a15c..f17384857 100644
--- a/arch/arm/boards/raspberry-pi/rpi-common.c
+++ b/arch/arm/boards/raspberry-pi/rpi-common.c
@@ -16,21 +16,26 @@
 #include <common.h>
 #include <init.h>
 #include <fs.h>
+#include <of.h>
 #include <linux/stat.h>
 #include <linux/clk.h>
 #include <linux/clkdev.h>
 #include <envfs.h>
 #include <malloc.h>
+#include <libfile.h>
 #include <gpio.h>
 #include <net.h>
 #include <led.h>
 #include <asm/armlinux.h>
+#include <asm/barebox-arm.h>
 #include <generated/mach-types.h>
+#include <linux/sizes.h>
 
 #include <mach/core.h>
 #include <mach/mbox.h>
 
 #include "rpi.h"
+#include "lowlevel.h"
 
 struct msg_get_arm_mem {
 	struct bcm2835_mbox_hdr hdr;
@@ -370,12 +375,41 @@ static int rpi_env_init(void)
 	return 0;
 }
 
+static void rpi_vc_fdt(void)
+{
+	void *saved_vc_fdt;
+	struct fdt_header *oftree;
+	unsigned long magic, size;
+
+	/* VideoCore FDT was copied in PBL just above Barebox memory */
+	saved_vc_fdt = (void *)(arm_mem_endmem_get());
+
+	oftree = saved_vc_fdt;
+	magic = be32_to_cpu(oftree->magic);
+	if (magic != FDT_MAGIC) {
+		printf("videocore fdt saved in pbl has invalid magic\n");
+
+		if (magic == VIDEOCORE_FDT_ERROR) {
+			printf("there was an error copying fdt in pbl: %d\n",
+					be32_to_cpu(oftree->totalsize));
+		}
+		return;
+	}
+
+	size = be32_to_cpu(oftree->totalsize);
+	if (write_file("/vc.dtb", saved_vc_fdt, size)) {
+		printf("failed to save videocore fdt to a file\n");
+		return;
+	}
+}
+
 static int rpi_devices_init(void)
 {
 	rpi_model_init();
 	bcm2835_register_fb();
 	armlinux_set_architecture(MACH_TYPE_BCM2708);
 	rpi_env_init();
+	rpi_vc_fdt();
 	return 0;
 }
 late_initcall(rpi_devices_init);
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] commands: add of_bootargs command.

[Tomaz Solc]

When booting a Raspberry Pi, it is useful to extract bootargs from the
device tree that was created by the VideoCore firmware. These bootargs
contain for example settings for the framebuffer that the kernel needs
to properly set the video output.

This commit adds an of_bootargs command that extracts a bootargs
property from a device tree and saves it to a global variable.

For example, a bootloader environment can use this command to extract
the bootargs to linux.bootargs.vc, which then gets included into the
final bootargs for the kernel using CONFIG_FLEXIBLE_BOOTARGS.
---
 Documentation/boards/bcm2835.rst |  4 ++
 commands/Kconfig                 | 13 ++++++
 commands/Makefile                |  1 +
 commands/of_bootargs.c           | 99 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 117 insertions(+)
 create mode 100644 commands/of_bootargs.c

diff --git a/Documentation/boards/bcm2835.rst b/Documentation/boards/bcm2835.rst
index 95e910896..b8a6c18ea 100644
--- a/Documentation/boards/bcm2835.rst
+++ b/Documentation/boards/bcm2835.rst
@@ -34,5 +34,9 @@ VideoCore firmware creates a device tree based on the entries in ``config.txt``.
 
     bootm -o /vc.dtb /boot/kernel7.img
 
+VideoCore device tree also contains the kernel command-line that is constructed from ``cmdline.txt`` and other parameters internally determined by the VideoCore firmware. Normally in Barebox this command-line gets overwritten by the Linux bootargs (see :ref:`booting_linux`). You can extract the VideoCore command-line from the device tree using the following command::
+
+    of_bootargs /vc.dtb linux.bootargs.vc
+
 .. _Raspberry Pi firmware: https://codeload.github.com/raspberrypi/firmware/zip/80e1fbeb78f9df06701d28c0ed3a3060a3f557ef
 .. _documentation for config.txt: https://www.raspberrypi.org/documentation/configuration/config-txt/
diff --git a/commands/Kconfig b/commands/Kconfig
index c14332c9d..0ae6d526f 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -2015,6 +2015,19 @@ config CMD_OF_DUMP
 	  Options:
 		-f <dtb>	work on <dtb> instead of internal devicetree
 
+config CMD_OF_BOOTARGS
+	tristate
+	select OFTREE
+	prompt "of_bootargs"
+	help
+	  Extract bootargs from a device tree into a global variable
+
+	  Usage: of_bootargs [DTB] [VAR]
+
+	  DTB is path to a device tree file (e.g. /vc.dtb).
+	  VAR is a name of the global variable to set (e.g. linux.bootargs.vc)
+
+
 config CMD_OF_NODE
 	tristate
 	select OFTREE
diff --git a/commands/Makefile b/commands/Makefile
index 358671bb5..f554f6041 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_CMD_OFTREE)	+= oftree.o
 obj-$(CONFIG_CMD_OF_PROPERTY)	+= of_property.o
 obj-$(CONFIG_CMD_OF_NODE)	+= of_node.o
 obj-$(CONFIG_CMD_OF_DUMP)	+= of_dump.o
+obj-$(CONFIG_CMD_OF_BOOTARGS)	+= of_bootargs.o
 obj-$(CONFIG_CMD_OF_DISPLAY_TIMINGS)	+= of_display_timings.o
 obj-$(CONFIG_CMD_OF_FIXUP_STATUS)	+= of_fixup_status.o
 obj-$(CONFIG_CMD_MAGICVAR)	+= magicvar.o
diff --git a/commands/of_bootargs.c b/commands/of_bootargs.c
new file mode 100644
index 000000000..93e7b74b1
--- /dev/null
+++ b/commands/of_bootargs.c
@@ -0,0 +1,99 @@
+/*
+ * of_dump.c - dump devicetrees to the console
+ *
+ * Copyright (c) 2014 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <common.h>
+#include <libfile.h>
+#include <of.h>
+#include <command.h>
+#include <malloc.h>
+#include <complete.h>
+#include <linux/ctype.h>
+#include <errno.h>
+#include <linux/err.h>
+#include <globalvar.h>
+
+static int do_of_bootargs(int argc, char *argv[])
+{
+	int ret = 0;
+	struct device_node *root = NULL, *node;
+	char *dtbfile, *varname;
+	void *fdt;
+
+	size_t size;
+	const char *cmdline;
+
+	if (argc != 3) {
+		return COMMAND_ERROR_USAGE;
+	}
+
+	dtbfile = argv[1];
+	varname = argv[2];
+
+	fdt = read_file(dtbfile, &size);
+	if (!fdt) {
+		printf("unable to read %s: %s\n", dtbfile, strerror(errno));
+		return -errno;
+	}
+
+	root = of_unflatten_dtb(fdt);
+
+	free(fdt);
+
+	if (IS_ERR(root)) {
+		ret = PTR_ERR(root);
+		root = NULL;
+		goto out;
+	}
+
+	node = of_find_node_by_path_from(root, "/chosen");
+	if (!node) {
+		printf("no /chosen node\n");
+		ret = -ENOENT;
+		goto out;
+	}
+
+	cmdline = of_get_property(node, "bootargs", NULL);
+	if (!cmdline) {
+		printf("no bootargs property in the /chosen node\n");
+		ret = -ENOENT;
+		goto out;
+	}
+
+	globalvar_add_simple(varname, cmdline);
+
+out:
+	if (root)
+		of_delete_node(root);
+
+	return ret;
+}
+
+BAREBOX_CMD_HELP_START(of_bootargs)
+BAREBOX_CMD_HELP_TEXT("DTB is path to a device tree file (e.g. /vc.dtb).")
+BAREBOX_CMD_HELP_TEXT("VAR is a name of the global variable to set (e.g. linux.bootargs.vc)")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(of_bootargs)
+	.cmd		= do_of_bootargs,
+	BAREBOX_CMD_DESC("extract bootargs from a device tree into a global variable")
+	BAREBOX_CMD_OPTS("[DTB] [VAR]")
+	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
+	BAREBOX_CMD_COMPLETE(devicetree_file_complete)
+	BAREBOX_CMD_HELP(cmd_of_bootargs_help)
+BAREBOX_CMD_END
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/3] commands: add of_bootargs command.

[Sascha Hauer]

On Thu, Feb 21, 2019 at 10:28:48AM +0100, Tomaz Solc wrote:
> When booting a Raspberry Pi, it is useful to extract bootargs from the
> device tree that was created by the VideoCore firmware. These bootargs
> contain for example settings for the framebuffer that the kernel needs
> to properly set the video output.
> 
> This commit adds an of_bootargs command that extracts a bootargs
> property from a device tree and saves it to a global variable.
> 
> For example, a bootloader environment can use this command to extract
> the bootargs to linux.bootargs.vc, which then gets included into the
> final bootargs for the kernel using CONFIG_FLEXIBLE_BOOTARGS.

Do we need an extra command for this? Can't you just unflatten the
VideoCore provided device tree in the board code you have added in 2/3
and set global.linux.bootargs.vc from there? It seems to be just the
right thing without further user intervention.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/3] commands: add of_bootargs command.

[Tomaž Šolc]

Hi Sascha,

On 22. 02. 19 08:49, Sascha Hauer wrote:
> On Thu, Feb 21, 2019 at 10:28:48AM +0100, Tomaz Solc wrote:
>> This commit adds an of_bootargs command that extracts a bootargs
>> property from a device tree and saves it to a global variable.
>>
>> For example, a bootloader environment can use this command to extract
>> the bootargs to linux.bootargs.vc, which then gets included into the
>> final bootargs for the kernel using CONFIG_FLEXIBLE_BOOTARGS.
> 
> Do we need an extra command for this? Can't you just unflatten the
> VideoCore provided device tree in the board code you have added in 2/3
> and set global.linux.bootargs.vc from there? It seems to be just the
> right thing without further user intervention.

I wanted to keep things flexible. I thought having an explicit command 
to import the args into Barebox is better than doing this automatically 
in rpi-common.c.

As far as I know [1], upstream kernels use their own device tree and 
don't need bootargs and fdt from VideoCore. On the other hand, I'm using 
the Raspbian-supplied kernels that do depend on these two things. That's 
also why I liked the approach where the VideoCore fdt is saved into a 
file, where it can be passed to boot or not, depending on the environment.

If you think of_bootargs is too specific for a command, I can make it 
more general and have it save an arbitrary property from a fdt into a 
global variable (something similar to readf). That might be useful for 
something else as well.

[1] 
https://elinux.org/RPi_Upstream_Kernel_Compilation#Building_your_bootloader

Best regards
Tomaž

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/3] commands: add of_bootargs command.

[Sascha Hauer]

On Fri, Feb 22, 2019 at 11:38:58AM +0100, Tomaž Šolc wrote:
> Hi Sascha,
> 
> On 22. 02. 19 08:49, Sascha Hauer wrote:
> > On Thu, Feb 21, 2019 at 10:28:48AM +0100, Tomaz Solc wrote:
> > > This commit adds an of_bootargs command that extracts a bootargs
> > > property from a device tree and saves it to a global variable.
> > > 
> > > For example, a bootloader environment can use this command to extract
> > > the bootargs to linux.bootargs.vc, which then gets included into the
> > > final bootargs for the kernel using CONFIG_FLEXIBLE_BOOTARGS.
> > 
> > Do we need an extra command for this? Can't you just unflatten the
> > VideoCore provided device tree in the board code you have added in 2/3
> > and set global.linux.bootargs.vc from there? It seems to be just the
> > right thing without further user intervention.
> 
> I wanted to keep things flexible. I thought having an explicit command to
> import the args into Barebox is better than doing this automatically in
> rpi-common.c.

You can still set nv.linux.bootargs.vc="" if you want to drop the
vc provided bootargs later. Or other way round, set some other
variable, say global.vc.bootargs, to the vc provided bootargs, and
in some initscript set global.linux.bootargs.vc to $global.vc.bootargs.

> 
> As far as I know [1], upstream kernels use their own device tree and don't
> need bootargs and fdt from VideoCore. On the other hand, I'm using the
> Raspbian-supplied kernels that do depend on these two things. That's also
> why I liked the approach where the VideoCore fdt is saved into a file, where
> it can be passed to boot or not, depending on the environment.

Yes, that's fine. Having the devicetree available is a good thing.

> 
> If you think of_bootargs is too specific for a command, I can make it more
> general and have it save an arbitrary property from a fdt into a global
> variable (something similar to readf). That might be useful for something
> else as well.

Could be integrated into of_property maybe.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] ARM: rpi: save fdt that was passed from VideoCore

[Roland Hieber]

Hi,

On Thu, Feb 21, 2019 at 10:28:47AM +0100, Tomaz Solc wrote:
> On Raspberry Pi, VideoCore firmware creates a device tree that contains
> information about peripherals that were initialized by VideoCore based
> on settings in config.txt. Normally this device tree is passed to the
> Linux kernel via a pointer in the r2 register. A bootloader needs to
> pass this device tree to the kernel, or some peripherals will not work
> correctly.
> 
> Since the VideoCore device tree is not compatible with barebox, we can't
> just pass it to barebox_arm_entry() as the internal barebox device tree.
> 
> This commit makes the prebootloader code copy the device tree from
> VideoCore into a scrap RAM area just above the area reserved for the
> bootloader. Board initialization code in the bootloader proper then
> copies it into a file /vc.dtb. The bootloader environment is then free
> to pass this file to the kernel at boot (e.g. via bootm -o).
> ---
>  Documentation/boards/bcm2835.rst          |  4 ++
>  arch/arm/boards/raspberry-pi/lowlevel.c   | 68 ++++++++++++++++++++++---------
>  arch/arm/boards/raspberry-pi/lowlevel.h   |  9 ++++
>  arch/arm/boards/raspberry-pi/rpi-common.c | 34 ++++++++++++++++
>  4 files changed, 96 insertions(+), 19 deletions(-)
>  create mode 100644 arch/arm/boards/raspberry-pi/lowlevel.h
> 
[...]
> diff --git a/arch/arm/boards/raspberry-pi/rpi-common.c b/arch/arm/boards/raspberry-pi/rpi-common.c
> index b5d16a15c..f17384857 100644
> --- a/arch/arm/boards/raspberry-pi/rpi-common.c
> +++ b/arch/arm/boards/raspberry-pi/rpi-common.c
> @@ -370,12 +375,41 @@ static int rpi_env_init(void)
>  	return 0;
>  }
>  
> +static void rpi_vc_fdt(void)
> +{
> +	void *saved_vc_fdt;
> +	struct fdt_header *oftree;
> +	unsigned long magic, size;
> +
> +	/* VideoCore FDT was copied in PBL just above Barebox memory */
> +	saved_vc_fdt = (void *)(arm_mem_endmem_get());
> +
> +	oftree = saved_vc_fdt;
> +	magic = be32_to_cpu(oftree->magic);
> +	if (magic != FDT_MAGIC) {
> +		printf("videocore fdt saved in pbl has invalid magic\n");

It's more idiomatic to use pr_err() here

> +
> +		if (magic == VIDEOCORE_FDT_ERROR) {
> +			printf("there was an error copying fdt in pbl: %d\n",
> +					be32_to_cpu(oftree->totalsize));

and here

> +		}
> +		return;
> +	}
> +
> +	size = be32_to_cpu(oftree->totalsize);
> +	if (write_file("/vc.dtb", saved_vc_fdt, size)) {
> +		printf("failed to save videocore fdt to a file\n");

and here.

 - Roland

> +		return;
> +	}
> +}
> +
>  static int rpi_devices_init(void)
>  {
>  	rpi_model_init();
>  	bcm2835_register_fb();
>  	armlinux_set_architecture(MACH_TYPE_BCM2708);
>  	rpi_env_init();
> +	rpi_vc_fdt();
>  	return 0;
>  }
>  late_initcall(rpi_devices_init);
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Roland Hieber                     | r.hieber@pengutronix.de     |
Pengutronix e.K.                  | https://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim | Phone: +49-5121-206917-5086 |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 3/3] commands: add of_bootargs command.

[Roland Hieber]

On Thu, Feb 21, 2019 at 10:28:48AM +0100, Tomaz Solc wrote:
> When booting a Raspberry Pi, it is useful to extract bootargs from the
> device tree that was created by the VideoCore firmware. These bootargs
> contain for example settings for the framebuffer that the kernel needs
> to properly set the video output.
> 
> This commit adds an of_bootargs command that extracts a bootargs
> property from a device tree and saves it to a global variable.
> 
> For example, a bootloader environment can use this command to extract
> the bootargs to linux.bootargs.vc, which then gets included into the
> final bootargs for the kernel using CONFIG_FLEXIBLE_BOOTARGS.
> ---
>  Documentation/boards/bcm2835.rst |  4 ++
>  commands/Kconfig                 | 13 ++++++
>  commands/Makefile                |  1 +
>  commands/of_bootargs.c           | 99 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 117 insertions(+)
>  create mode 100644 commands/of_bootargs.c
> 
> diff --git a/Documentation/boards/bcm2835.rst b/Documentation/boards/bcm2835.rst
> index 95e910896..b8a6c18ea 100644
> --- a/Documentation/boards/bcm2835.rst
> +++ b/Documentation/boards/bcm2835.rst
> @@ -34,5 +34,9 @@ VideoCore firmware creates a device tree based on the entries in ``config.txt``.
>  
>      bootm -o /vc.dtb /boot/kernel7.img
>  
> +VideoCore device tree also contains the kernel command-line that is constructed from ``cmdline.txt`` and other parameters internally determined by the VideoCore firmware. Normally in Barebox this command-line gets overwritten by the Linux bootargs (see :ref:`booting_linux`). You can extract the VideoCore command-line from the device tree using the following command::
> +
> +    of_bootargs /vc.dtb linux.bootargs.vc
> +
>  .. _Raspberry Pi firmware: https://codeload.github.com/raspberrypi/firmware/zip/80e1fbeb78f9df06701d28c0ed3a3060a3f557ef
>  .. _documentation for config.txt: https://www.raspberrypi.org/documentation/configuration/config-txt/
> diff --git a/commands/Kconfig b/commands/Kconfig
> index c14332c9d..0ae6d526f 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -2015,6 +2015,19 @@ config CMD_OF_DUMP
>  	  Options:
>  		-f <dtb>	work on <dtb> instead of internal devicetree
>  
> +config CMD_OF_BOOTARGS
> +	tristate
> +	select OFTREE
> +	prompt "of_bootargs"
> +	help
> +	  Extract bootargs from a device tree into a global variable
> +
> +	  Usage: of_bootargs [DTB] [VAR]
> +
> +	  DTB is path to a device tree file (e.g. /vc.dtb).
> +	  VAR is a name of the global variable to set (e.g. linux.bootargs.vc)
> +
> +
>  config CMD_OF_NODE
>  	tristate
>  	select OFTREE
> diff --git a/commands/Makefile b/commands/Makefile
> index 358671bb5..f554f6041 100644
> --- a/commands/Makefile
> +++ b/commands/Makefile
> @@ -76,6 +76,7 @@ obj-$(CONFIG_CMD_OFTREE)	+= oftree.o
>  obj-$(CONFIG_CMD_OF_PROPERTY)	+= of_property.o
>  obj-$(CONFIG_CMD_OF_NODE)	+= of_node.o
>  obj-$(CONFIG_CMD_OF_DUMP)	+= of_dump.o
> +obj-$(CONFIG_CMD_OF_BOOTARGS)	+= of_bootargs.o
>  obj-$(CONFIG_CMD_OF_DISPLAY_TIMINGS)	+= of_display_timings.o
>  obj-$(CONFIG_CMD_OF_FIXUP_STATUS)	+= of_fixup_status.o
>  obj-$(CONFIG_CMD_MAGICVAR)	+= magicvar.o
> diff --git a/commands/of_bootargs.c b/commands/of_bootargs.c
> new file mode 100644
> index 000000000..93e7b74b1
> --- /dev/null
> +++ b/commands/of_bootargs.c
> @@ -0,0 +1,99 @@
> +/*
> + * of_dump.c - dump devicetrees to the console
> + *
> + * Copyright (c) 2014 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix

Is this true? :-)

And please add a SPDX-License-Identifier to the new files. See [9] for
an explanation (and replace "Linux" by "barebox" in your head when
reading it :))

[9]: https://www.kernel.org/doc/html/latest/process/license-rules.html

> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.

Leave out this paragraph, that file does not exist.

 - Roland

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <common.h>
> +#include <libfile.h>
> +#include <of.h>
> +#include <command.h>
> +#include <malloc.h>
> +#include <complete.h>
> +#include <linux/ctype.h>
> +#include <errno.h>
> +#include <linux/err.h>
> +#include <globalvar.h>
> +
> +static int do_of_bootargs(int argc, char *argv[])
> +{
> +	int ret = 0;
> +	struct device_node *root = NULL, *node;
> +	char *dtbfile, *varname;
> +	void *fdt;
> +
> +	size_t size;
> +	const char *cmdline;
> +
> +	if (argc != 3) {
> +		return COMMAND_ERROR_USAGE;
> +	}
> +
> +	dtbfile = argv[1];
> +	varname = argv[2];
> +
> +	fdt = read_file(dtbfile, &size);
> +	if (!fdt) {
> +		printf("unable to read %s: %s\n", dtbfile, strerror(errno));
> +		return -errno;
> +	}
> +
> +	root = of_unflatten_dtb(fdt);
> +
> +	free(fdt);
> +
> +	if (IS_ERR(root)) {
> +		ret = PTR_ERR(root);
> +		root = NULL;
> +		goto out;
> +	}
> +
> +	node = of_find_node_by_path_from(root, "/chosen");
> +	if (!node) {
> +		printf("no /chosen node\n");
> +		ret = -ENOENT;
> +		goto out;
> +	}
> +
> +	cmdline = of_get_property(node, "bootargs", NULL);
> +	if (!cmdline) {
> +		printf("no bootargs property in the /chosen node\n");
> +		ret = -ENOENT;
> +		goto out;
> +	}
> +
> +	globalvar_add_simple(varname, cmdline);
> +
> +out:
> +	if (root)
> +		of_delete_node(root);
> +
> +	return ret;
> +}
> +
> +BAREBOX_CMD_HELP_START(of_bootargs)
> +BAREBOX_CMD_HELP_TEXT("DTB is path to a device tree file (e.g. /vc.dtb).")
> +BAREBOX_CMD_HELP_TEXT("VAR is a name of the global variable to set (e.g. linux.bootargs.vc)")
> +BAREBOX_CMD_HELP_END
> +
> +BAREBOX_CMD_START(of_bootargs)
> +	.cmd		= do_of_bootargs,
> +	BAREBOX_CMD_DESC("extract bootargs from a device tree into a global variable")
> +	BAREBOX_CMD_OPTS("[DTB] [VAR]")
> +	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
> +	BAREBOX_CMD_COMPLETE(devicetree_file_complete)
> +	BAREBOX_CMD_HELP(cmd_of_bootargs_help)
> +BAREBOX_CMD_END
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Roland Hieber                     | r.hieber@pengutronix.de     |
Pengutronix e.K.                  | https://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim | Phone: +49-5121-206917-5086 |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox