On 17 November 2014 at 19:23, Simon Glass <[email protected]> wrote: > Hi Masahiro, > > On 17 November 2014 11:19, Masahiro Yamada <[email protected]> wrote: >> Hi Simon, >> >> >> >> On Mon, 17 Nov 2014 09:14:15 +0000 >> Simon Glass <[email protected]> wrote: >> >>> Hi Masahiro, >>> >>> On 17 November 2014 08:19, Masahiro Yamada <[email protected]> >>> wrote: >>> > If the variable "ret" is equal to "-ENOENT", it is trapped at [1] and >>> > never reaches [2]. At [3], the condition "ret != -ENOENT" is always >>> > true. >>> > >>> > if (ret == -ENOENT) { <------------------ [1] >>> > continue; >>> > } else if (ret == -ENODEV) { >>> > dm_dbg("Device '%s' has no compatible string\n", name); >>> > break; >>> > } else if (ret) { <------------------ [2] >>> > dm_warn("Device tree error at offset %d\n", offset); >>> > if (!result || ret != -ENOENT) <------------------ [3] >>> > result = ret; >>> > break; >>> > } >>> > >>> > Signed-off-by: Masahiro Yamada <[email protected]> >>> > --- >>> > >>> > drivers/core/lists.c | 3 +-- >>> > 1 file changed, 1 insertion(+), 2 deletions(-) >>> > >>> > diff --git a/drivers/core/lists.c b/drivers/core/lists.c >>> > index 3a1ea85..0aad56d 100644 >>> > --- a/drivers/core/lists.c >>> > +++ b/drivers/core/lists.c >>> > @@ -136,8 +136,7 @@ int lists_bind_fdt(struct udevice *parent, const void >>> > *blob, int offset, >>> > break; >>> > } else if (ret) { >>> > dm_warn("Device tree error at offset %d\n", >>> > offset); >>> > - if (!result || ret != -ENOENT) >>> > - result = ret; >>> > + result = ret; >>> >>> Thanks for the clear explanation. >>> It looks good, except my intent was to return the first error, hence >>> the 'if (!result ...'. >>> >>> Your code will return the last error. Can we preserve the current bahaviour? >> >> >> Do you mean, like this? >> >> dm_warn("Device tree error at offset %d\n", offset); >> if (!result) >> result = ret; >> break; >> >> >> I think there is no difference in the behaviour, because >> this "for" loop is escaped out at the first encounter with an error >> except -ENOENT. >> >> Here is the only line to set "result" and it is followed by "break" >> statement, >> so the last error is also the first error, I think. > > Ah yes, OK. The intent here is to provide a useful error message > without adding a lot of noise. It's a bit of a pain but we should keep > it I think. Your patch looks good. > > Acked-by: Simon Glass <[email protected]>
Applied to u-boot-dm. _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
