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

Simon Glass
2017-04-19T15:14:07Z

Hi Andreas,

On 19 April 2017 at 08:43, Andreas Färber <afaerber@suse.de> wrote:
> Hi Simon,
>
> Am 19.04.2017 um 16:28 schrieb Simon Glass:
>> On 19 April 2017 at 05:26, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>> 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
>>> 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.
>>
>> I would like to avoid changing the API that much. Can you please
>> change it to stop calling the tail function and always set the device,
>> like you did in v1?
>
> I fear you're missing the key point I made in my lengthy explanation:

That's not entirely impossible.

>
> Our caller (EFI) wants to iterate over the available devices. SDIO is
> not available. If we return a non-NULL device it will try to scan the
> disk. Therefore I think v2 is more correct (not yet tested).
>
> So really the question is what your desired semantics of this function
> are and how callers here and elsewhere are handling it. If we want to
> return non-probed devices to the caller, as you now suggest, then we
> would need to audit and amend all callers of the API with some "if
> !is_probed() then continue" check. If we simply skip them internally, as
> done here IIUC, we require no changes on the caller side, thus much less
> invasive.

Well the value of 'ret' gives you that information if you want it. But
yes it is a change and on second thoughts I'm not entirely comfortable
with it.

>
> Maybe we need a new API uclass_{first,next}_available_device() to make
> this clear? The change would then only affect callers of the new API,
> and EFI and possibly others would again need to be audited and updated.

Actually how about we go back to my foreach idea, and have a manual
probe. So I mean let's just change the EFI code.

See for example print_cpu_list() for how this is done.

Regards,
Simon