[PATCH] bootstd: Replicate the dtb-filename quirks of distroboot

[PATCH] bootstd: Replicate the dtb-filename quirks of distroboot

[Simon Glass]

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 <sjg@chromium.org>
Suggested-by: Mark Kettenis <kettenis@openbsd.org>
---

 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

Re: [PATCH] bootstd: Replicate the dtb-filename quirks of distroboot

[Heinrich Schuchardt]

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 <sjg@chromium.org>
> Suggested-by: Mark Kettenis <kettenis@openbsd.org>
> ---
> 
>   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);

Re: [PATCH] bootstd: Replicate the dtb-filename quirks of distroboot

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4177 bytes --]

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 <sjg@chromium.org>
> > Suggested-by: Mark Kettenis <kettenis@openbsd.org>
> > ---
> > 
> >   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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] bootstd: Replicate the dtb-filename quirks of distroboot

[Heinrich Schuchardt]

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 <sjg@chromium.org>
>>> Suggested-by: Mark Kettenis <kettenis@openbsd.org>
>>> ---
>>>
>>>    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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 5078 bytes --]

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 <sjg@chromium.org>
> > > > Suggested-by: Mark Kettenis <kettenis@openbsd.org>
> > > > ---
> > > > 
> > > >    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.

Yes, I'm saying we need a positive message when using the in memory
device tree.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]