Skip to contentSkip to conversation

pxe: implement fdtdir extlinux.conf tag

PATCH New Stephen Warren 3 participants 4 messages Jan 23, 2014 – Jan 24, 2014

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 pxe: implement fdtdir extlinux.conf tag

From: Stephen Warren <[email protected]>

People who write (or scripts that auto-generate) extlinux.conf don't want to know about HW-specific information such as FDT filenames. Create a new extlinux.conf tag "fdtdir" that specifies only the directory where FDT files are located, and defer all knowledge of the filename to U-Boot. The algorithm implemented is:

==========
if $fdt_addr_r is set:
if "fdt" tag was specified in extlinux.conf:
load the FDT from the filename in the tag
else if "fdtdir" tag was specified in extlinux.conf:
if "fdtfile" is set in the environment:
load the FDT from filename in "$fdtfile"
else:
load the FDT from some automatically generated filename

if no FDT file was loaded, and $fdtaddr is set:
# This indicates an FDT packaged with firmware
use the FDT at $fdtaddr
==========

A small part of an example /boot/extlinux.conf might be:

==========
LABEL primary
LINUX zImage
FDTDIR ./

LABEL failsafe
LINUX bkp/zImage
FDTDIR bkp/
==========

... with /boot/tegra20-seaboard.dtb or /boot/bkp/tegra20-seaboard.dtb being loaded by the sysboot/pxe code.

---
v1 (relative to RFC): no change; just reposting as non-RFC

This patch depends on the following from Dennis Gilmore:
- cmd_pxe.c add any option for filesystem with sysboot uses generic load
- config: add config_distro_defaults.h

---
 common/cmd_pxe.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 71 insertions(+), 6 deletions(-)
common/cmd_pxe.c +71 -6
index 20569bb04201..5d356295e48e 100644
@@ -459,6 +459,7 @@ struct pxe_label {
 	char *append;
 	char *initrd;
 	char *fdt;
+	char *fdtdir;
 	int ipappend;
 	int attempted;
 	int localboot;
@@ -531,6 +532,9 @@ static void label_destroy(struct pxe_label *label)
 	if (label->fdt)
 		free(label->fdt);
 
+	if (label->fdtdir)
+		free(label->fdtdir);
+
 	free(label);
 }
 
@@ -689,13 +693,67 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)
 	bootm_argv[3] = getenv("fdt_addr_r");
 
 	/* if fdt label is defined then get fdt from server */
-	if (bootm_argv[3] && label->fdt) {
-		if (get_relfile_envaddr(cmdtp, label->fdt, "fdt_addr_r") < 0) {
-			printf("Skipping %s for failure retrieving fdt\n",
-					label->name);
-			return 1;
+	if (bootm_argv[3]) {
+		char *fdtfile = NULL;
+		char *fdtfilefree = NULL;
+
+		if (label->fdt) {
+			fdtfile = label->fdt;
+		} else if (label->fdtdir) {
+			fdtfile = getenv("fdtfile");
+			/*
+			 * For complex cases, it might be worth calling a
+			 * board- or SoC-provided function here to provide a
+			 * better default:
+			 *
+			 * if (!fdtfile)
+			 *     fdtfile = gen_fdtfile();
+			 *
+			 * If this is added, be sure to keep the default below,
+			 * or move it to the default weak implementation of
+			 * gen_fdtfile().
+			 */
+			if (!fdtfile) {
+				char *soc = getenv("soc");
+				char *board = getenv("board");
+				char *slash;
+
+				len = strlen(label->fdtdir);
+				if (!len)
+					slash = "./";
+				else if (label->fdtdir[len - 1] != '/')
+					slash = "/";
+				else
+					slash = "";
+
+				len = strlen(label->fdtdir) + strlen(slash) +
+					strlen(soc) + 1 + strlen(board) + 5;
+				fdtfilefree = malloc(len);
+				if (!fdtfilefree) {
+					printf("malloc fail (FDT filename)\n");
+					return 1;
+				}
+
+				snprintf(fdtfilefree, len, "%s%s%s-%s.dtb",
+					label->fdtdir, slash, soc, board);
+				fdtfile = fdtfilefree;
+			}
 		}
-	} else
+
+		if (fdtfile) {
+			int err = get_relfile_envaddr(cmdtp, fdtfile, "fdt_addr_r");
+			free(fdtfilefree);
+			if (err < 0) {
+				printf("Skipping %s for failure retrieving fdt\n",
+						label->name);
+				return 1;
+			}
+		} else {
+			bootm_argv[3] = NULL;
+		}
+	}
+
+	if (!bootm_argv[3])
 		bootm_argv[3] = getenv("fdt_addr");
 
 	if (bootm_argv[3])
@@ -730,6 +788,7 @@ enum token_type {
 	T_PROMPT,
 	T_INCLUDE,
 	T_FDT,
+	T_FDTDIR,
 	T_ONTIMEOUT,
 	T_IPAPPEND,
 	T_INVALID
@@ -760,6 +819,7 @@ static const struct token keywords[] = {
 	{"initrd", T_INITRD},
 	{"include", T_INCLUDE},
 	{"fdt", T_FDT},
+	{"fdtdir", T_FDTDIR},
 	{"ontimeout", T_ONTIMEOUT,},
 	{"ipappend", T_IPAPPEND,},
 	{NULL, T_INVALID}
@@ -1148,6 +1208,11 @@ static int parse_label(char **c, struct pxe_menu *cfg)
 				err = parse_sliteral(c, &label->fdt);
 			break;
 
+		case T_FDTDIR:
+			if (!label->fdtdir)
+				err = parse_sliteral(c, &label->fdtdir);
+			break;
+
 		case T_LOCALBOOT:
 			label->localboot = 1;
 			err = parse_integer(c, &label->localboot_val);
Signature · 1 line
1.8.1.5
#1 lore raw

PATCH 1/1 pxe: implement fdtdir extlinux.conf tag

Quoted 35 lines · Stephen Warren

From: Stephen Warren <[email protected]>

People who write (or scripts that auto-generate) extlinux.conf don't want to know about HW-specific information such as FDT filenames. Create a new extlinux.conf tag "fdtdir" that specifies only the directory where FDT files are located, and defer all knowledge of the filename to U-Boot. The algorithm implemented is:

==========
if $fdt_addr_r is set:
if "fdt" tag was specified in extlinux.conf:
load the FDT from the filename in the tag
else if "fdtdir" tag was specified in extlinux.conf:
if "fdtfile" is set in the environment:
load the FDT from filename in "$fdtfile"
else:
load the FDT from some automatically generated filename

if no FDT file was loaded, and $fdtaddr is set:
# This indicates an FDT packaged with firmware
use the FDT at $fdtaddr
==========

A small part of an example /boot/extlinux.conf might be:

==========
LABEL primary
LINUX zImage
FDTDIR ./

LABEL failsafe
LINUX bkp/zImage
FDTDIR bkp/
==========

Quoted 4 lines

... with /boot/tegra20-seaboard.dtb or /boot/bkp/tegra20-seaboard.dtb being loaded by the sysboot/pxe code.

My only real concern is that
http://www.freedesktop.org/wiki/Specifications/BootLoaderSpec/ is as best I can see the spec for extlinux.conf and it doesn't talk about this tag. So while it sounds like a good idea, if we implement it and it's not documented outside of U-Boot, will anyone use it?

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140124/a7ac2f1b/attachment.pgp>
#2 lore raw

PATCH 1/1 pxe: implement fdtdir extlinux.conf tag

Quoted 8 lines · Tom Rini
Quoted 7 lines · Stephen Warren

From: Stephen Warren <[email protected]>

People who write (or scripts that auto-generate) extlinux.conf don't want to know about HW-specific information such as FDT filenames. Create a new extlinux.conf tag "fdtdir" that specifies only the directory where FDT files are located, and defer all knowledge of the filename to U-Boot. The algorithm implemented is:

...

Quoted 7 lines
Quoted 1 line

My only real concern is that
http://www.freedesktop.org/wiki/Specifications/BootLoaderSpec/ is as best I can see the spec for extlinux.conf and it doesn't talk about this tag. So while it sounds like a good idea, if we implement it and it's not documented outside of U-Boot, will anyone use it?

Well, Dennis is actively working on making Fedora support extlinux.conf for at least ARM, and said on IRC that he was going to patch the Fedora generator to use it, so yes:-)

Dennis, should you or I simply edit that wiki, or is there some other process for changing it?

Related, I notice that it documents a "devicetree" tag, whereas U-Boot (prior to my patches) actually implements an "fdt" tag for the same purpose...

#3 lore raw

PATCH 1/1 pxe: implement fdtdir extlinux.conf tag

El Fri, 24 Jan 2014 10:20:16 -0700
Stephen Warren <[email protected]> escribi?:

Quoted 18 lines · Tom Rini
Quoted 8 lines · Stephen Warren
Quoted 7 lines

From: Stephen Warren <[email protected]>

People who write (or scripts that auto-generate) extlinux.conf don't want to know about HW-specific information such as FDT filenames. Create a new extlinux.conf tag "fdtdir" that specifies only the directory where FDT files are located, and defer all knowledge of the filename to U-Boot. The algorithm implemented is:

...

Quoted 7 lines
Quoted 1 line

My only real concern is that
http://www.freedesktop.org/wiki/Specifications/BootLoaderSpec/ is as best I can see the spec for extlinux.conf and it doesn't talk about this tag. So while it sounds like a good idea, if we implement it and it's not documented outside of U-Boot, will anyone use it?

Quoted 3 lines

Well, Dennis is actively working on making Fedora support extlinux.conf for at least ARM, and said on IRC that he was going to patch the Fedora generator to use it, so yes:-)

the tag is going to be added to the spec. we still have a little more work to do to fully support the spec. At the least version and machine-id tags and being able to dynamically find the .conf files. It is also safe to add additional tags as needed.

Quoted 2 lines

Dennis, should you or I simply edit that wiki, or is there some other process for changing it?

I spoke with Harald Hoyer just now and he said he would edit the spec

Quoted 3 lines

Related, I notice that it documents a "devicetree" tag, whereas U-Boot (prior to my patches) actually implements an "fdt" tag for the same purpose...

he also said
18:12 < haraldh> dgilmore, go with "fdt" and "fdtdir"

since u-boot has existing support out there for fdt for the same purpose.

Dennis