Re: [PATCH] bootstd: Replicate the dtb-filename quirks of distroboot
On Wed, Jan 25, 2023 at 06:04:56PM +0100, Heinrich Schuchardt wrote:
> On 1/25/23 17:15, Tom Rini wrote:
> > On Wed, Jan 25, 2023 at 04:38:53PM +0100, Heinrich Schuchardt wrote:
> > > On 1/25/23 04:11, Simon Glass wrote:
> > > > For EFI, the distro boot scripts search in three different directories
> > > > for the .dtb file. The SOC-based filename fallback is supported only for
> > > > 32-bit ARM.
> > > >
> > > > Adjust the code to mirror this behaviour.
> > > >
> > > > Signed-off-by: Simon Glass
> > > > Suggested-by: Mark Kettenis
> > > > ---
> > > >
> > > >boot/bootmeth_efi.c | 63
> > > > ++---
> > > >1 file changed, 54 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
> > > > index 67c972e3fe4..f9544846e68 100644
> > > > --- a/boot/bootmeth_efi.c
> > > > +++ b/boot/bootmeth_efi.c
> > > > @@ -147,25 +147,57 @@ static int distro_efi_check(struct udevice *dev,
> > > > struct bootflow_iter *iter)
> > > > return 0;
> > > >}
> > > > -static void distro_efi_get_fdt_name(char *fname, int size)
> > > > +/**
> > > > + * distro_efi_get_fdt_name() - Get the filename for reading the .dtb
> > > > file
> > > > + *
> > > > + * @fname: Place to put filename
> > > > + * @size: Max size of filename
> > > > + * @seq: Sequence number, to cycle through options (0=first)
> > > > + * Returns: 0 on success, -ENOENT if the "fdtfile" env var does not
> > > > exist,
> > > > + * -EINVAL if there are no more options
> > > > + */
> > > > +static int distro_efi_get_fdt_name(char *fname, int size, int seq)
> > > >{
> > > > const char *fdt_fname;
> > > > + const char *prefix;
> > > > +
> > > > + /* select the prefix */
> > > > + switch (seq) {
> > > > + case 0:
> > > > + /* this is the default */
> > > > + prefix = "/dtb";
> > > > + break;
> > > > + case 1:
> > > > + prefix = "";
> > > > + break;
> > > > + case 2:
> > > > + prefix = "/dtb/current";
> > > > + break;
> > > > + default:
> > > > + return log_msg_ret("pref", -EINVAL);
> > > > + }
> > > > fdt_fname = env_get("fdtfile");
> > > > if (fdt_fname) {
> > > > - snprintf(fname, size, "dtb/%s", fdt_fname);
> > > > + snprintf(fname, size, "%s/%s", prefix, fdt_fname);
> > > > log_debug("Using device tree: %s\n", fname);
> > > > - } else {
> > > > +
> > > > + /* Use this fallback only for 32-bit ARM */
> > > > + } else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64))
> > > > {
> > > > const char *soc = env_get("soc");
> > > > const char *board = env_get("board");
> > > > const char *boardver = env_get("boardver");
> > > > /* cf the code in label_boot() which seems very complex
> > > > */
> > > > - snprintf(fname, size, "dtb/%s%s%s%s.dtb",
> > > > + snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix,
> > > > soc ? soc : "", soc ? "-" : "", board ? board
> > > > : "",
> > > > boardver ? boardver : "");
> > > > log_debug("Using default device tree: %s\n", fname);
> > > > + } else {
> > > > + return log_msg_ret("env", -ENOENT);
> > > > }
> > > > +
> > > > + return 0;
> > > >}
> > > >static int distro_efi_read_bootflow_file(struct udevice *dev,
> > > > @@ -174,7 +206,7 @@ static int distro_efi_read_bootflow_file(struct
> > > > udevice *dev,
> > > > struct blk_desc *desc = NULL;
> > > > ulong fdt_addr, size;
> > > > char fname[256];
> > > > - int ret;
> > > > + int ret, seq;
> > > > /* We require a partition table */
> > > > if (!bflow->part)
> > > > @@ -196,13 +228,22 @@ static int distro_efi_read_bootflow_file(struct
> > > > udevice *dev,
> > > > if (ret)
> > > > return log_msg_ret("read", -EINVAL);
> > > > - distro_efi_get_fdt_name(fname, sizeof(fname));
> > > > + fdt_addr = env_get_hex("fdt_addr_r", 0);
> > > > +
> > > > + /* try the various available names */
> > > > + ret = -ENOENT;
> > > > + for (seq = 0; ret; seq++)
> > >
> > > There are boards that don't have a dtb file available and that is ok.
> > > Don't
> > > loop past seq = 2.
> > >
> > > {
> > > > + ret = distro_efi_get_fdt_name(fname, sizeof(fname),
> > > > seq);
> > > > + if (ret)
> > > > + return log_msg_ret("nam", ret);
> > > > + ret = bootmeth_common_read_file(dev, bflow, fname,
> > > > fdt_addr,
> > > > + &size);
> > > > + }
> > > > +
> > > > bflow->fdt_fname = strdup(fname);
> > > > if (!bflow->fdt_fname)
> > > >
Re: [PATCH] bootstd: Replicate the dtb-filename quirks of distroboot
On 1/25/23 17:15, Tom Rini wrote:
On Wed, Jan 25, 2023 at 04:38:53PM +0100, Heinrich Schuchardt wrote:
On 1/25/23 04:11, Simon Glass wrote:
For EFI, the distro boot scripts search in three different directories
for the .dtb file. The SOC-based filename fallback is supported only for
32-bit ARM.
Adjust the code to mirror this behaviour.
Signed-off-by: Simon Glass
Suggested-by: Mark Kettenis
---
boot/bootmeth_efi.c | 63 ++---
1 file changed, 54 insertions(+), 9 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index 67c972e3fe4..f9544846e68 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -147,25 +147,57 @@ static int distro_efi_check(struct udevice *dev, struct
bootflow_iter *iter)
return 0;
}
-static void distro_efi_get_fdt_name(char *fname, int size)
+/**
+ * distro_efi_get_fdt_name() - Get the filename for reading the .dtb file
+ *
+ * @fname: Place to put filename
+ * @size: Max size of filename
+ * @seq: Sequence number, to cycle through options (0=first)
+ * Returns: 0 on success, -ENOENT if the "fdtfile" env var does not exist,
+ * -EINVAL if there are no more options
+ */
+static int distro_efi_get_fdt_name(char *fname, int size, int seq)
{
const char *fdt_fname;
+ const char *prefix;
+
+ /* select the prefix */
+ switch (seq) {
+ case 0:
+ /* this is the default */
+ prefix = "/dtb";
+ break;
+ case 1:
+ prefix = "";
+ break;
+ case 2:
+ prefix = "/dtb/current";
+ break;
+ default:
+ return log_msg_ret("pref", -EINVAL);
+ }
fdt_fname = env_get("fdtfile");
if (fdt_fname) {
- snprintf(fname, size, "dtb/%s", fdt_fname);
+ snprintf(fname, size, "%s/%s", prefix, fdt_fname);
log_debug("Using device tree: %s\n", fname);
- } else {
+
+ /* Use this fallback only for 32-bit ARM */
+ } else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) {
const char *soc = env_get("soc");
const char *board = env_get("board");
const char *boardver = env_get("boardver");
/* cf the code in label_boot() which seems very complex */
- snprintf(fname, size, "dtb/%s%s%s%s.dtb",
+ snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix,
soc ? soc : "", soc ? "-" : "", board ? board : "",
boardver ? boardver : "");
log_debug("Using default device tree: %s\n", fname);
+ } else {
+ return log_msg_ret("env", -ENOENT);
}
+
+ return 0;
}
static int distro_efi_read_bootflow_file(struct udevice *dev,
@@ -174,7 +206,7 @@ static int distro_efi_read_bootflow_file(struct udevice
*dev,
struct blk_desc *desc = NULL;
ulong fdt_addr, size;
char fname[256];
- int ret;
+ int ret, seq;
/* We require a partition table */
if (!bflow->part)
@@ -196,13 +228,22 @@ static int distro_efi_read_bootflow_file(struct udevice
*dev,
if (ret)
return log_msg_ret("read", -EINVAL);
- distro_efi_get_fdt_name(fname, sizeof(fname));
+ fdt_addr = env_get_hex("fdt_addr_r", 0);
+
+ /* try the various available names */
+ ret = -ENOENT;
+ for (seq = 0; ret; seq++)
There are boards that don't have a dtb file available and that is ok. Don't
loop past seq = 2.
{
+ ret = distro_efi_get_fdt_name(fname, sizeof(fname), seq);
+ if (ret)
+ return log_msg_ret("nam", ret);
+ ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr,
+ &size);
+ }
+
bflow->fdt_fname = strdup(fname);
if (!bflow->fdt_fname)
return log_msg_ret("fil", -ENOMEM);
This should not be an error. The Distroboot fallback is the device-tree at
$fdtcontroladdr.
But it should note that it's using that because that's still quite often
an unexpected feature to people.
On QEMU it is just what is needed. Other boards supply the device-tree
via TF-A or OpenSBI. We should not start breaking boards.
A message "fil: returning err= -12" signals not caring about users.
Best regards
Heinrich
Re: [PATCH] bootstd: Replicate the dtb-filename quirks of distroboot
On Wed, Jan 25, 2023 at 04:38:53PM +0100, Heinrich Schuchardt wrote:
> On 1/25/23 04:11, Simon Glass wrote:
> > For EFI, the distro boot scripts search in three different directories
> > for the .dtb file. The SOC-based filename fallback is supported only for
> > 32-bit ARM.
> >
> > Adjust the code to mirror this behaviour.
> >
> > Signed-off-by: Simon Glass
> > Suggested-by: Mark Kettenis
> > ---
> >
> > boot/bootmeth_efi.c | 63 ++---
> > 1 file changed, 54 insertions(+), 9 deletions(-)
> >
> > diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
> > index 67c972e3fe4..f9544846e68 100644
> > --- a/boot/bootmeth_efi.c
> > +++ b/boot/bootmeth_efi.c
> > @@ -147,25 +147,57 @@ static int distro_efi_check(struct udevice *dev,
> > struct bootflow_iter *iter)
> > return 0;
> > }
> > -static void distro_efi_get_fdt_name(char *fname, int size)
> > +/**
> > + * distro_efi_get_fdt_name() - Get the filename for reading the .dtb file
> > + *
> > + * @fname: Place to put filename
> > + * @size: Max size of filename
> > + * @seq: Sequence number, to cycle through options (0=first)
> > + * Returns: 0 on success, -ENOENT if the "fdtfile" env var does not exist,
> > + * -EINVAL if there are no more options
> > + */
> > +static int distro_efi_get_fdt_name(char *fname, int size, int seq)
> > {
> > const char *fdt_fname;
> > + const char *prefix;
> > +
> > + /* select the prefix */
> > + switch (seq) {
> > + case 0:
> > + /* this is the default */
> > + prefix = "/dtb";
> > + break;
> > + case 1:
> > + prefix = "";
> > + break;
> > + case 2:
> > + prefix = "/dtb/current";
> > + break;
> > + default:
> > + return log_msg_ret("pref", -EINVAL);
> > + }
> > fdt_fname = env_get("fdtfile");
> > if (fdt_fname) {
> > - snprintf(fname, size, "dtb/%s", fdt_fname);
> > + snprintf(fname, size, "%s/%s", prefix, fdt_fname);
> > log_debug("Using device tree: %s\n", fname);
> > - } else {
> > +
> > + /* Use this fallback only for 32-bit ARM */
> > + } else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) {
> > const char *soc = env_get("soc");
> > const char *board = env_get("board");
> > const char *boardver = env_get("boardver");
> > /* cf the code in label_boot() which seems very complex */
> > - snprintf(fname, size, "dtb/%s%s%s%s.dtb",
> > + snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix,
> > soc ? soc : "", soc ? "-" : "", board ? board : "",
> > boardver ? boardver : "");
> > log_debug("Using default device tree: %s\n", fname);
> > + } else {
> > + return log_msg_ret("env", -ENOENT);
> > }
> > +
> > + return 0;
> > }
> > static int distro_efi_read_bootflow_file(struct udevice *dev,
> > @@ -174,7 +206,7 @@ static int distro_efi_read_bootflow_file(struct udevice
> > *dev,
> > struct blk_desc *desc = NULL;
> > ulong fdt_addr, size;
> > char fname[256];
> > - int ret;
> > + int ret, seq;
> > /* We require a partition table */
> > if (!bflow->part)
> > @@ -196,13 +228,22 @@ static int distro_efi_read_bootflow_file(struct
> > udevice *dev,
> > if (ret)
> > return log_msg_ret("read", -EINVAL);
> > - distro_efi_get_fdt_name(fname, sizeof(fname));
> > + fdt_addr = env_get_hex("fdt_addr_r", 0);
> > +
> > + /* try the various available names */
> > + ret = -ENOENT;
> > + for (seq = 0; ret; seq++)
>
> There are boards that don't have a dtb file available and that is ok. Don't
> loop past seq = 2.
>
> {
> > + ret = distro_efi_get_fdt_name(fname, sizeof(fname), seq);
> > + if (ret)
> > + return log_msg_ret("nam", ret);
> > + ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr,
> > + &size);
> > + }
> > +
> > bflow->fdt_fname = strdup(fname);
> > if (!bflow->fdt_fname)
> > return log_msg_ret("fil", -ENOMEM);
>
> This should not be an error. The Distroboot fallback is the device-tree at
> $fdtcontroladdr.
But it should note that it's using that because that's still quite often
an unexpected feature to people.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] bootstd: Replicate the dtb-filename quirks of distroboot
On 1/25/23 04:11, Simon Glass wrote:
For EFI, the distro boot scripts search in three different directories
for the .dtb file. The SOC-based filename fallback is supported only for
32-bit ARM.
Adjust the code to mirror this behaviour.
Signed-off-by: Simon Glass
Suggested-by: Mark Kettenis
---
boot/bootmeth_efi.c | 63 ++---
1 file changed, 54 insertions(+), 9 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index 67c972e3fe4..f9544846e68 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -147,25 +147,57 @@ static int distro_efi_check(struct udevice *dev, struct
bootflow_iter *iter)
return 0;
}
-static void distro_efi_get_fdt_name(char *fname, int size)
+/**
+ * distro_efi_get_fdt_name() - Get the filename for reading the .dtb file
+ *
+ * @fname: Place to put filename
+ * @size: Max size of filename
+ * @seq: Sequence number, to cycle through options (0=first)
+ * Returns: 0 on success, -ENOENT if the "fdtfile" env var does not exist,
+ * -EINVAL if there are no more options
+ */
+static int distro_efi_get_fdt_name(char *fname, int size, int seq)
{
const char *fdt_fname;
+ const char *prefix;
+
+ /* select the prefix */
+ switch (seq) {
+ case 0:
+ /* this is the default */
+ prefix = "/dtb";
+ break;
+ case 1:
+ prefix = "";
+ break;
+ case 2:
+ prefix = "/dtb/current";
+ break;
+ default:
+ return log_msg_ret("pref", -EINVAL);
+ }
fdt_fname = env_get("fdtfile");
if (fdt_fname) {
- snprintf(fname, size, "dtb/%s", fdt_fname);
+ snprintf(fname, size, "%s/%s", prefix, fdt_fname);
log_debug("Using device tree: %s\n", fname);
- } else {
+
+ /* Use this fallback only for 32-bit ARM */
+ } else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) {
const char *soc = env_get("soc");
const char *board = env_get("board");
const char *boardver = env_get("boardver");
/* cf the code in label_boot() which seems very complex */
- snprintf(fname, size, "dtb/%s%s%s%s.dtb",
+ snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix,
soc ? soc : "", soc ? "-" : "", board ? board : "",
boardver ? boardver : "");
log_debug("Using default device tree: %s\n", fname);
+ } else {
+ return log_msg_ret("env", -ENOENT);
}
+
+ return 0;
}
static int distro_efi_read_bootflow_file(struct udevice *dev,
@@ -174,7 +206,7 @@ static int distro_efi_read_bootflow_file(struct udevice
*dev,
struct blk_desc *desc = NULL;
ulong fdt_addr, size;
char fname[256];
- int ret;
+ int ret, seq;
/* We require a partition table */
if (!bflow->part)
@@ -196,13 +228,22 @@ static int distro_efi_read_bootflow_file(struct udevice
*dev,
if (ret)
return log_msg_ret("read", -EINVAL);
- distro_efi_get_fdt_name(fname, sizeof(fname));
+ fdt_addr = env_get_hex("fdt_addr_r", 0);
+
+ /* try the various available names */
+ ret = -ENOENT;
+ for (seq = 0; ret; seq++)
There are boards that don't have a dtb file available and that is ok.
Don't loop past seq = 2.
{
+ ret = distro_efi_get_fdt_name(fname, sizeof(fname), seq);
+ if (ret)
+ return log_msg_ret("nam", ret);
+ ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr,
+ &size);
+ }
+
bflow->fdt_fname = strdup(fname);
if (!bflow->fdt_fname)
return log_msg_ret("fil", -ENOMEM);
This should not be an error. The Distroboot fallback is the device-tree
at $fdtcontroladdr.
Best regards
Heinrich
- fdt_addr = env_get_hex("fdt_addr_r", 0);
- ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr, &size);
if (!ret) {
bflow->fdt_size = size;
bflow->fdt_addr = fdt_addr;
@@ -277,7 +318,11 @@ static int distro_efi_read_bootflow_net(struct bootflow
*bflow)
fdt_addr = hextoul(fdt_addr_str, NULL);
sprintf(file_addr, "%lx", fdt_addr);
- distro_efi_get_fdt_name(fname, sizeof(fname));
+ /* We only allow the first prefix with PXE */
+ ret = distro_efi_get_fdt_name(fname, sizeof(fname), 0);
+ if (ret)
+ return log_msg_ret("nam", ret);
+
bflow->fdt_fname = strdup(fname);
if (!bflow->fdt_fname)
return log_msg_ret("fil", -ENOMEM);
[PATCH] bootstd: Replicate the dtb-filename quirks of distroboot
For EFI, the distro boot scripts search in three different directories
for the .dtb file. The SOC-based filename fallback is supported only for
32-bit ARM.
Adjust the code to mirror this behaviour.
Signed-off-by: Simon Glass
Suggested-by: Mark Kettenis
---
boot/bootmeth_efi.c | 63 ++---
1 file changed, 54 insertions(+), 9 deletions(-)
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index 67c972e3fe4..f9544846e68 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -147,25 +147,57 @@ static int distro_efi_check(struct udevice *dev, struct
bootflow_iter *iter)
return 0;
}
-static void distro_efi_get_fdt_name(char *fname, int size)
+/**
+ * distro_efi_get_fdt_name() - Get the filename for reading the .dtb file
+ *
+ * @fname: Place to put filename
+ * @size: Max size of filename
+ * @seq: Sequence number, to cycle through options (0=first)
+ * Returns: 0 on success, -ENOENT if the "fdtfile" env var does not exist,
+ * -EINVAL if there are no more options
+ */
+static int distro_efi_get_fdt_name(char *fname, int size, int seq)
{
const char *fdt_fname;
+ const char *prefix;
+
+ /* select the prefix */
+ switch (seq) {
+ case 0:
+ /* this is the default */
+ prefix = "/dtb";
+ break;
+ case 1:
+ prefix = "";
+ break;
+ case 2:
+ prefix = "/dtb/current";
+ break;
+ default:
+ return log_msg_ret("pref", -EINVAL);
+ }
fdt_fname = env_get("fdtfile");
if (fdt_fname) {
- snprintf(fname, size, "dtb/%s", fdt_fname);
+ snprintf(fname, size, "%s/%s", prefix, fdt_fname);
log_debug("Using device tree: %s\n", fname);
- } else {
+
+ /* Use this fallback only for 32-bit ARM */
+ } else if (IS_ENABLED(CONFIG_ARM) && !IS_ENABLED(CONFIG_ARM64)) {
const char *soc = env_get("soc");
const char *board = env_get("board");
const char *boardver = env_get("boardver");
/* cf the code in label_boot() which seems very complex */
- snprintf(fname, size, "dtb/%s%s%s%s.dtb",
+ snprintf(fname, size, "%s/%s%s%s%s.dtb", prefix,
soc ? soc : "", soc ? "-" : "", board ? board : "",
boardver ? boardver : "");
log_debug("Using default device tree: %s\n", fname);
+ } else {
+ return log_msg_ret("env", -ENOENT);
}
+
+ return 0;
}
static int distro_efi_read_bootflow_file(struct udevice *dev,
@@ -174,7 +206,7 @@ static int distro_efi_read_bootflow_file(struct udevice
*dev,
struct blk_desc *desc = NULL;
ulong fdt_addr, size;
char fname[256];
- int ret;
+ int ret, seq;
/* We require a partition table */
if (!bflow->part)
@@ -196,13 +228,22 @@ static int distro_efi_read_bootflow_file(struct udevice
*dev,
if (ret)
return log_msg_ret("read", -EINVAL);
- distro_efi_get_fdt_name(fname, sizeof(fname));
+ fdt_addr = env_get_hex("fdt_addr_r", 0);
+
+ /* try the various available names */
+ ret = -ENOENT;
+ for (seq = 0; ret; seq++) {
+ ret = distro_efi_get_fdt_name(fname, sizeof(fname), seq);
+ if (ret)
+ return log_msg_ret("nam", ret);
+ ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr,
+ &size);
+ }
+
bflow->fdt_fname = strdup(fname);
if (!bflow->fdt_fname)
return log_msg_ret("fil", -ENOMEM);
- fdt_addr = env_get_hex("fdt_addr_r", 0);
- ret = bootmeth_common_read_file(dev, bflow, fname, fdt_addr, &size);
if (!ret) {
bflow->fdt_size = size;
bflow->fdt_addr = fdt_addr;
@@ -277,7 +318,11 @@ static int distro_efi_read_bootflow_net(struct bootflow
*bflow)
fdt_addr = hextoul(fdt_addr_str, NULL);
sprintf(file_addr, "%lx", fdt_addr);
- distro_efi_get_fdt_name(fname, sizeof(fname));
+ /* We only allow the first prefix with PXE */
+ ret = distro_efi_get_fdt_name(fname, sizeof(fname), 0);
+ if (ret)
+ return log_msg_ret("nam", ret);
+
bflow->fdt_fname = strdup(fname);
if (!bflow->fdt_fname)
return log_msg_ret("fil", -ENOMEM);
--
2.39.1.405.gd4c25cc71f-goog
