Am 19.04.2017 um 18:56 schrieb Simon Glass: > 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.
You misunderstand: That ret handling should be hidden inside _available_device, not exposed to its caller! :) Note that v2 adds a for loop inside both functions! So the caller only sees the devices that probed and would never see a non-zero ret, i.e. they could become void (or return the dev, which would change the calling convention). Andreas -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
