Hi Simon, On Thu, Feb 27, 2020 at 12:33 AM Simon Glass <[email protected]> wrote: > > Hi Masahiro, > > On Mon, 24 Feb 2020 at 23:58, Masahiro Yamada > <[email protected]> wrote: > > > > uclass_find_first_device() succeeds even if it cannot find any device. > > So, the caller must check the return code and also *devp is not NULL. > > > > Returning -ENODEV will be sensible in this case. > > > > Signed-off-by: Masahiro Yamada <[email protected]> > > --- > > > > If this patch is acceptable, I want to fold this > > into my pull request because it need it > > for my another patch: > > http://patchwork.ozlabs.org/patch/1238000/ > > > > drivers/core/uclass.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > I sort-of agree and have thought about this a lot. > > But what are you doing in the case of uclass_find_next_device()? > > Also what about the tests and other code that expects the current behaviour?
OK, I will change my caller side. I would probably design these functions as follows: struct udevice *uclass_find_first_device(enum uclass_id id); struct udevice *uclass_find_next_device(struct udevice *dev); Return the pointer to the device, or NULL if not found. > > > > diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c > > index 58b19a421091..3580974f3b85 100644 > > --- a/drivers/core/uclass.c > > +++ b/drivers/core/uclass.c > > @@ -227,7 +227,7 @@ int uclass_find_first_device(enum uclass_id id, struct > > udevice **devp) > > if (ret) > > return ret; > > if (list_empty(&uc->dev_head)) > > - return 0; > > + return -ENODEV; > > > > *devp = list_first_entry(&uc->dev_head, struct udevice, > > uclass_node); > > > > -- > > 2.17.1 > > > > Regards, > Simon -- Best Regards Masahiro Yamada
