[U-Boot] [PATCH v2 1/1] core/uclass: iterate over all devices of a uclass

Simon Glass
2017-04-22T23:58:33Z

Hi,

On 19 April 2017 at 15:34, Simon Glass <sjg@chromium.org> wrote:
> On 19 April 2017 at 15:06, Andreas Färber <afaerber@suse.de> wrote:
>> Am 19.04.2017 um 13:26 schrieb Heinrich Schuchardt:
>>> When iterating over the devices of an uclass the iteration stops
>>> at the first device that cannot be probed.
>>> When calling booefi this will result in no block device being
>>
>> "bootefi"
>>
>>> passed to the EFI executable if the first device cannot be probed.
>>>
>>> The problem was reported by Andreas Färber in
>>> https://lists.denx.de/pipermail/u-boot/2017-April/287432.html
>>>
>>> For testing I used an odroid-c2 with a dts including
>>> &sd_emmc_a {
>>> status = "okay";
>>> }
>>> This device does not exist on the board and cannot be initialized.
>>>
>>> With the patch uclass_first_device and uclass_next_device
>>> iterate internally until they find the first device that can be
>>> probed or the end of the device list is reached.
>>>
>>> Debug output is provided for the two functions.
>>>
>>> Reported-by: Andreas Färber <afaerber@suse.de>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> ---
>>> v2:
>>> As suggested by Simon Glass correct uclass_first_device() and
>>> uclass_next_device() instead of uclass_get_device_tail() to
>>> avoid side effects.
>>> v1:
>>> The original patch was posted as
>>> core/uclass: uclass_get_device_tail: always set devp
>>> https://lists.denx.de/pipermail/u-boot/2017-April/288068.html
>>> ---
>>> drivers/core/uclass.c | 44 +++++++++++++++++++++++++++++++++-----------
>>> 1 file changed, 33 insertions(+), 11 deletions(-)
>>
>> Reviewed-by: Andreas Färber <afaerber@suse.de>
>> Tested-by: Andreas Färber <afaerber@suse.de>
>>
>> Confirming that on my Vega S95 Telos this results in a full GRUB menu,
>> and GRUB sees two disks.
>>
>> Many thanks,
>>
>> Andreas
>
> Just to be clear, I am NAKing this patch. I do not want to change the
> existing semantics as it requires existing code to check the function
> return value. Instead I would like to:
>
> - Fix the bug found by Alvaro
> https://lists.denx.de/pipermail/u-boot/2017-April/288091.html This
> involves setting *devp to NULL when returning an error.
> - Add two new functions which return the device whether it probes or
> not, allowing iteration to continue. Since we have two potential users
> for these functions, it seems worth it.
>
> I'll take a look at this soon unless someone else does :-)

Here is my attempt:

http://patchwork.ozlabs.org/patch/752578/
http://patchwork.ozlabs.org/patch/752576/
http://patchwork.ozlabs.org/patch/752577/

Regards,
Simon …