Patchwork [U-Boot] Fix device detection for API consumers

login
register
mail settings
Submitter Ilya Bakulin
Date May 19, 2013, 10:09 a.m.
Message ID <20130519100919.GA47317@olymp.kibab.com>
Download mbox | patch
Permalink /patch/244803/
State Changes Requested
Delegated to: Tom Rini
Headers show

Comments

Ilya Bakulin - May 19, 2013, 10:09 a.m.
Hi list,
I use U-Boot for starting FreeBSD/arm on Globalscale DreamPlug.
On this platform FreeBSD uses "ubldr" second-stage bootloader, which is
an U-Boot API consumer and uses U-Boot API to access block devices, network, etc.
Dreamplug has several block devices accessible: internal SD card, SD card reader,
and any number of USB-attached mass storage devices.

But when I boot ubldr, I can see only one block device.

I have traced down the problem and it seems to be in U-Boot API. When doing
block device enumeration, the "more" flag is not set properly on first access.

This patch fixes the problem for me. After applying the patch, FreeBSD ubldr
is able to see and access all block devices that U-Boot self knows.
Jeroen Hofstee - May 21, 2013, 6:14 p.m.
Hello Ilya,

On 05/19/2013 12:09 PM, Ilya Bakulin wrote:
> Hi list,
> I use U-Boot for starting FreeBSD/arm on Globalscale DreamPlug.
> On this platform FreeBSD uses "ubldr" second-stage bootloader, which is
> an U-Boot API consumer and uses U-Boot API to access block devices, network, etc.
> Dreamplug has several block devices accessible: internal SD card, SD card reader,
> and any number of USB-attached mass storage devices.
>
> But when I boot ubldr, I can see only one block device.
>
> I have traced down the problem and it seems to be in U-Boot API. When doing
> block device enumeration, the "more" flag is not set properly on first access.
>
> This patch fixes the problem for me. After applying the patch, FreeBSD ubldr
> is able to see and access all block devices that U-Boot self knows.
>
> diff --git a/api/api_storage.c b/api/api_storage.c
> index c535712..1147e79 100644
> --- a/api/api_storage.c
> +++ b/api/api_storage.c
> @@ -129,6 +129,9 @@ static int dev_stor_get(int type, int first, int *more, struct device_info *di)
>   		else
>   			found = 1;
>   
> +		if (specs[type].max_dev > 1)
> +			*more = 1;
> +
>   	} else {
>   		for (i = 0; i < specs[type].max_dev; i++)
>   			if (di->cookie == (void *)get_dev(specs[type].name, i)) {
>

I would personally prefer to set i = 0 in the "first" block and move the 
/* provide
  hint if.. */ out of the else and into the if (found)  block. So there 
is only one
place for checking the next device, but haven't tested that, just 
looking at the
code. Since *more is already set to 0 initially any early problems which 
return 0
will already have more = 0. When a device is found, as an exit routine it is
checked if more devices are present, and might set it to 1.

But more important, read http://www.denx.de/wiki/U-Boot/Patches.
This patch misses a signed-off-by (and please make the commit message 
reflect
why the api is broken and optionally the cover letter how you found it, 
that is a bit
more to the point in my mind).

Regards,
Jeroen

Patch

diff --git a/api/api_storage.c b/api/api_storage.c
index c535712..1147e79 100644
--- a/api/api_storage.c
+++ b/api/api_storage.c
@@ -129,6 +129,9 @@  static int dev_stor_get(int type, int first, int *more, struct device_info *di)
 		else
 			found = 1;
 
+		if (specs[type].max_dev > 1)
+			*more = 1;
+
 	} else {
 		for (i = 0; i < specs[type].max_dev; i++)
 			if (di->cookie == (void *)get_dev(specs[type].name, i)) {