Hi Andreas, On 19 April 2017 at 10:37, Andreas Färber <afaer...@suse.de> wrote: > Am 19.04.2017 um 17:52 schrieb Simon Glass: >> 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? > > No. I think that should be the semantics of uclass_first_device(), i.e. > Heinrich's v1, possibly moved out of _tail() as you requested. > > The idea behind a separate ..._available_device() implementation was to > not have to do that ret check and to simply replace the function name to > change the semantics, i.e. Heinrich's v2 implementation, but not inside > uclass_{first,next}_device() but in copies with different name.
How do you know what the error was? If you return a 'dev' then how do you know whether it is OK to process it? I'd prefer that this is provided via the return value rather than checking device_active(), etc. > > What use case is there for an incomplete enumeration as done by > uclass_first_device() today? In order to continue enumeration devp needs > to be set IIUC. The existing API returns a NULL device when it gets an error, meaning that the device cannot be used. The use case is for devices which don't fail on probe. While I understand that removeable devices can fail to probe today and then succeed later, many devices are not like that, and failing to probe is an error. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
