Hi Masahiro, On Thu, 27 Feb 2020 at 09:57, Masahiro Yamada <[email protected]> wrote: > > 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.
I think that is fine, since we don't support any other error than -ENODEV. I did intend to add checks that the structs are correct (by adding type information into each struct conditional on a DM_DEBUG option) and returning a special error when something goes wrong. But even then I think we could just return NULL and -ENODEV in the caller would be good enough. Regards, Simon
