Hi Andreas,
On 19 April 2017 at 09:14, Simon Glass <s...@chromium.org> wrote:
> Hi Andreas,
>
> On 19 April 2017 at 08:43, Andreas Färber <afaer...@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.g...@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.
If you think this is generally useful then you could add this. I think
it would be something like:
for (ret = uclass_first_avail_device(UCLASS_..., &dev; dev; ret =
uclass_next_avail_device(&dev)) {
if (!ret) {
// do something
}
}
Does that sounds right?
>
> 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
