Hi Simon On 10/19/18 5:27 AM, Simon Glass wrote: > Hi Patrick, > > On 15 October 2018 at 03:01, Patrice CHOTARD <patrice.chot...@st.com> wrote: >> 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). > > I actually don't like this function. Let's use > uclass_first_device_err() when we care about errors. I am wondering if > we should delete uclass_first_device().
Ok i will use uclass_first_device_err() instead >> >> In any case uclass_first_device() returns a valid dev with an error. >> >> Simon, Bin do you confirm ? > > I don't see how, no. >> >>> >>>> 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(). I will add a new uclass_next_device_err() Thanks Patrice > > > Reviewed-by: Simon Glass <s...@chromium.org> > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
