Hi Bin On 10/12/2018 12:13 PM, Bin Meng wrote: > Hi Patrice, > > On Fri, Oct 12, 2018 at 3:51 PM Patrice CHOTARD <patrice.chot...@st.com> > wrote: >> >> Hi Bin >> >> On 10/11/2018 11:06 AM, Bin Meng wrote: >>> Hi Patrice, >>> >>> On Tue, Oct 9, 2018 at 9:41 PM Patrice Chotard <patrice.chot...@st.com> >>> wrote: >>>> >>>> Add uclass_foreach_dev_probe() which iterates through >>>> devices of a given uclass. Devices are probed if necessary >>>> and are ready to use. >>>> >>>> Signed-off-by: Patrice Chotard <patrice.chot...@st.com> >>>> Reviewed-by: Simon Glass <s...@chromium.org> >>>> --- >>>> >>>> Changes in v3: None >>>> Changes in v2: None >>>> >>>> include/dm/uclass.h | 16 ++++++++++++++++ >>>> 1 file changed, 16 insertions(+) >>>> >>>> diff --git a/include/dm/uclass.h b/include/dm/uclass.h >>>> index 6e7c1cd3e8bc..10ccfdce951e 100644 >>>> --- a/include/dm/uclass.h >>>> +++ b/include/dm/uclass.h >>>> @@ -376,4 +376,20 @@ int uclass_resolve_seq(struct udevice *dev); >>>> #define uclass_foreach_dev_safe(pos, next, uc) \ >>>> list_for_each_entry_safe(pos, next, &uc->dev_head, uclass_node) >>>> >>>> +/** >>>> + * uclass_foreach_dev_probe() - Helper function to iteration through >>>> devices >>>> + * of given uclass >>>> + * >>>> + * This creates a for() loop which works through the available devices in >>>> + * a uclass in order from start to end. Devices are probed if necessary, >>>> + * and ready for use. >>>> + * >>>> + * @id: Uclass ID >>>> + * @dev: struct udevice * to hold the current device. Set to NULL when >>>> there >>>> + * are no more devices. >>>> + */ >>>> +#define uclass_foreach_dev_probe(id, dev) \ >>>> + for (uclass_first_device(id, &dev); dev; \ >>>> + uclass_next_device(&dev)) >>> >>> Shouldn't we check the return value of uclass_first_device() and >>> uclass_next_device()? >> >> It's not necessary to check the return value of uclass_first_device(id, >> &dev) because in any error case, dev is NULL, which is the loop output >> condition. This is only for the first iteration. >> > > Please see the comment of uclass_first_device(). > > * @devp: Returns pointer to the first device in that uclass if no error > * occurred, or NULL if there is no first device, or an error occurred with > * that device. > * @return 0 if OK (found or not found), other -ve on error > > So it can return error with a valid dev.
I checked the uclass_first_device() implementation and the comment seems not aligned with what is coded. When uclass_first_device() returns a valid dev, return value is always 0. (see uclass_get_device_tail() return value). In any case uclass_first_device() returns a valid dev with an error. Simon, Bin do you confirm ? > >> For the other iteration, dev comes from uclass_next_device(&dev), >> similarly to uclass_first_device(), in any error case dev is NULL. >> > > Similar please see the comment of uclass_next_device(). I have the same remark regarding uclass_next_device(). Thanks Patrice > > Regards, > Bin > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
