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

[Simon Glass]

Hi Andreas,

On 23 April 2017 at 04:48, Andreas Färber <afaerber at suse.de> wrote:
> Am 23.04.2017 um 01:58 schrieb Simon Glass:
>> Hi,
>>
>> On 19 April 2017 at 15:34, Simon Glass <sjg at chromium.org> wrote:
>>> On 19 April 2017 at 15:06, Andreas Färber <afaerber at 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 at suse.de>
>>>>> Cc: Simon Glass <sjg at chromium.org>
>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at 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 at suse.de>
>>>> Tested-by: Andreas Färber <afaerber at 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/
>
> You have taken my idea with s/available/ok/ and Heinrich's v2 loop
> implementation and posted it without Suggested-by or Signed-off-by. :(

Please send me what you would like and I'll add it.

I've had second (third?) thoughts on this so will be sending another version.

Regards,
Simon