* [PATCH] dm: make uclass_find_first_device() return error when no defice is found
@ 2020-02-25 6:58 Masahiro Yamada
2020-02-26 15:33 ` Simon Glass
0 siblings, 1 reply; 4+ messages in thread
From: Masahiro Yamada @ 2020-02-25 6:58 UTC (permalink / raw)
To: u-boot
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 <yamada.masahiro@socionext.com>
---
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(-)
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] dm: make uclass_find_first_device() return error when no defice is found
2020-02-25 6:58 [PATCH] dm: make uclass_find_first_device() return error when no defice is found Masahiro Yamada
@ 2020-02-26 15:33 ` Simon Glass
2020-02-27 17:56 ` Masahiro Yamada
0 siblings, 1 reply; 4+ messages in thread
From: Simon Glass @ 2020-02-26 15:33 UTC (permalink / raw)
To: u-boot
Hi Masahiro,
On Mon, 24 Feb 2020 at 23:58, Masahiro Yamada
<yamada.masahiro@socionext.com> 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 <yamada.masahiro@socionext.com>
> ---
>
> 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?
>
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] dm: make uclass_find_first_device() return error when no defice is found
2020-02-26 15:33 ` Simon Glass
@ 2020-02-27 17:56 ` Masahiro Yamada
2020-02-27 23:40 ` Simon Glass
0 siblings, 1 reply; 4+ messages in thread
From: Masahiro Yamada @ 2020-02-27 17:56 UTC (permalink / raw)
To: u-boot
Hi Simon,
On Thu, Feb 27, 2020 at 12:33 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Masahiro,
>
> On Mon, 24 Feb 2020 at 23:58, Masahiro Yamada
> <yamada.masahiro@socionext.com> 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 <yamada.masahiro@socionext.com>
> > ---
> >
> > 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] dm: make uclass_find_first_device() return error when no defice is found
2020-02-27 17:56 ` Masahiro Yamada
@ 2020-02-27 23:40 ` Simon Glass
0 siblings, 0 replies; 4+ messages in thread
From: Simon Glass @ 2020-02-27 23:40 UTC (permalink / raw)
To: u-boot
Hi Masahiro,
On Thu, 27 Feb 2020 at 09:57, Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Hi Simon,
>
> On Thu, Feb 27, 2020 at 12:33 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Masahiro,
> >
> > On Mon, 24 Feb 2020 at 23:58, Masahiro Yamada
> > <yamada.masahiro@socionext.com> 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 <yamada.masahiro@socionext.com>
> > > ---
> > >
> > > 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-02-27 23:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-25 6:58 [PATCH] dm: make uclass_find_first_device() return error when no defice is found Masahiro Yamada
2020-02-26 15:33 ` Simon Glass
2020-02-27 17:56 ` Masahiro Yamada
2020-02-27 23:40 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.