| Message ID | 1430280873-20561-1-git-send-email-joe.hershberger@ni.com |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Joe, On 28 April 2015 at 22:14, Joe Hershberger <joe.hershberger@ni.com> wrote: > The change to refactor these functions created a regression. > > commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788 > Author: Przemyslaw Marczak <p.marczak@samsung.com> > Date: Wed Apr 15 13:07:17 2015 +0200 > dm: core: add internal functions for getting the device without probe > > With this change, the dm unit tests started failing with a probe error > -22 in the dm_test_children test. > > Test: dm_test_children > test/dm/core.c:544, dm_test_children(): 0 == ret: Expected 0, got -22 > > This restores the original behavior which would avoid a probe on invalid > device pointers. > > Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> > --- > > drivers/core/uclass.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) Can you please check this - it is very similar to yours: http://patchwork.ozlabs.org/patch/464459/ Regards, Simon
Hi Simon, On Wed, Apr 29, 2015 at 8:30 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Joe, > > On 28 April 2015 at 22:14, Joe Hershberger <joe.hershberger@ni.com> wrote: >> The change to refactor these functions created a regression. >> >> commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788 >> Author: Przemyslaw Marczak <p.marczak@samsung.com> >> Date: Wed Apr 15 13:07:17 2015 +0200 >> dm: core: add internal functions for getting the device without probe >> >> With this change, the dm unit tests started failing with a probe error >> -22 in the dm_test_children test. >> >> Test: dm_test_children >> test/dm/core.c:544, dm_test_children(): 0 == ret: Expected 0, got -22 >> >> This restores the original behavior which would avoid a probe on invalid >> device pointers. >> >> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> >> --- >> >> drivers/core/uclass.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) > > Can you please check this - it is very similar to yours: > > http://patchwork.ozlabs.org/patch/464459/ Yes, it looks like it solves the same problem. I don't care which way it gets solved. Looks like yours is already on the way in. Hopefully sooner than later. At what point will we make the tests be a gating factor for pulling patches, kinda like checkpatch.pl? Thanks, -Joe
Hi Joe, On 29 April 2015 at 10:17, Joe Hershberger <joe.hershberger@gmail.com> wrote: > Hi Simon, > > On Wed, Apr 29, 2015 at 8:30 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Joe, >> >> On 28 April 2015 at 22:14, Joe Hershberger <joe.hershberger@ni.com> wrote: >>> The change to refactor these functions created a regression. >>> >>> commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788 >>> Author: Przemyslaw Marczak <p.marczak@samsung.com> >>> Date: Wed Apr 15 13:07:17 2015 +0200 >>> dm: core: add internal functions for getting the device without probe >>> >>> With this change, the dm unit tests started failing with a probe error >>> -22 in the dm_test_children test. >>> >>> Test: dm_test_children >>> test/dm/core.c:544, dm_test_children(): 0 == ret: Expected 0, got -22 >>> >>> This restores the original behavior which would avoid a probe on invalid >>> device pointers. >>> >>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> >>> --- >>> >>> drivers/core/uclass.c | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> Can you please check this - it is very similar to yours: >> >> http://patchwork.ozlabs.org/patch/464459/ > > Yes, it looks like it solves the same problem. I don't care which way > it gets solved. Looks like yours is already on the way in. Hopefully > sooner than later. > > At what point will we make the tests be a gating factor for pulling > patches, kinda like checkpatch.pl? Not sure, we don't even check that things build at present.... Now that the tests are running again, I'll resume checking driver model things before pulling things in. But we could use a way to run all tests, and some unification of them (e.g. all run with the same U-Boot build). Regards, Simon
On Thu, Apr 30, 2015 at 07:54:21PM -0600, Simon Glass wrote: > Hi Joe, > > On 29 April 2015 at 10:17, Joe Hershberger <joe.hershberger@gmail.com> wrote: > > Hi Simon, > > > > On Wed, Apr 29, 2015 at 8:30 AM, Simon Glass <sjg@chromium.org> wrote: > >> Hi Joe, > >> > >> On 28 April 2015 at 22:14, Joe Hershberger <joe.hershberger@ni.com> wrote: > >>> The change to refactor these functions created a regression. > >>> > >>> commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788 > >>> Author: Przemyslaw Marczak <p.marczak@samsung.com> > >>> Date: Wed Apr 15 13:07:17 2015 +0200 > >>> dm: core: add internal functions for getting the device without probe > >>> > >>> With this change, the dm unit tests started failing with a probe error > >>> -22 in the dm_test_children test. > >>> > >>> Test: dm_test_children > >>> test/dm/core.c:544, dm_test_children(): 0 == ret: Expected 0, got -22 > >>> > >>> This restores the original behavior which would avoid a probe on invalid > >>> device pointers. > >>> > >>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> > >>> --- > >>> > >>> drivers/core/uclass.c | 8 ++++++-- > >>> 1 file changed, 6 insertions(+), 2 deletions(-) > >> > >> Can you please check this - it is very similar to yours: > >> > >> http://patchwork.ozlabs.org/patch/464459/ > > > > Yes, it looks like it solves the same problem. I don't care which way > > it gets solved. Looks like yours is already on the way in. Hopefully > > sooner than later. > > > > At what point will we make the tests be a gating factor for pulling > > patches, kinda like checkpatch.pl? > > Not sure, we don't even check that things build at present.... That's not quite true. For every PR I do: ./tools/buildman/buildman -b master --force-build --step 0 -dvel 'blackfin|microblaze|m68k|nds32|x86|aarch64|sandbox|mips|avr32|arm|powerpc|sh4' And check that things haven't changed or at least not changed in a breaking way. > Now that the tests are running again, I'll resume checking driver > model things before pulling things in. > > But we could use a way to run all tests, and some unification of them > (e.g. all run with the same U-Boot build). Yes, if it was easy to kick off most of the tests (ie anything we can do with sandbox) I'd integrate that into my process.
Hi Tom, On 1 May 2015 at 07:38, Tom Rini <trini@konsulko.com> wrote: > > On Thu, Apr 30, 2015 at 07:54:21PM -0600, Simon Glass wrote: > > Hi Joe, > > > > On 29 April 2015 at 10:17, Joe Hershberger <joe.hershberger@gmail.com> wrote: > > > Hi Simon, > > > > > > On Wed, Apr 29, 2015 at 8:30 AM, Simon Glass <sjg@chromium.org> wrote: > > >> Hi Joe, > > >> > > >> On 28 April 2015 at 22:14, Joe Hershberger <joe.hershberger@ni.com> wrote: > > >>> The change to refactor these functions created a regression. > > >>> > > >>> commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788 > > >>> Author: Przemyslaw Marczak <p.marczak@samsung.com> > > >>> Date: Wed Apr 15 13:07:17 2015 +0200 > > >>> dm: core: add internal functions for getting the device without probe > > >>> > > >>> With this change, the dm unit tests started failing with a probe error > > >>> -22 in the dm_test_children test. > > >>> > > >>> Test: dm_test_children > > >>> test/dm/core.c:544, dm_test_children(): 0 == ret: Expected 0, got -22 > > >>> > > >>> This restores the original behavior which would avoid a probe on invalid > > >>> device pointers. > > >>> > > >>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> > > >>> --- > > >>> > > >>> drivers/core/uclass.c | 8 ++++++-- > > >>> 1 file changed, 6 insertions(+), 2 deletions(-) > > >> > > >> Can you please check this - it is very similar to yours: > > >> > > >> http://patchwork.ozlabs.org/patch/464459/ > > > > > > Yes, it looks like it solves the same problem. I don't care which way > > > it gets solved. Looks like yours is already on the way in. Hopefully > > > sooner than later. > > > > > > At what point will we make the tests be a gating factor for pulling > > > patches, kinda like checkpatch.pl? > > > > Not sure, we don't even check that things build at present.... > > That's not quite true. For every PR I do: > ./tools/buildman/buildman -b master --force-build --step 0 -dvel > 'blackfin|microblaze|m68k|nds32|x86|aarch64|sandbox|mips|avr32|arm|powerpc|sh4' > > And check that things haven't changed or at least not changed in a > breaking way. Oops I completely got the wrong end of the stick there. I was thinking of a something like checkpatch that checks the build and tests before people send patches. It's an idea that has come up before but I'm don't think anyone has a good idea on how to do it. Of course PRs are built! I do the same before sending them, although I actually don't --force-build which might leave me open to problems. Sorry Tom. > > > Now that the tests are running again, I'll resume checking driver > > model things before pulling things in. > > > > But we could use a way to run all tests, and some unification of them > > (e.g. all run with the same U-Boot build). > > Yes, if it was easy to kick off most of the tests (ie anything we can do > with sandbox) I'd integrate that into my process. Me too. Something I vaguely think about tidying up one day. Regards, Simon
Hi Simon and Tom, On Fri, May 1, 2015 at 9:23 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Tom, > > On 1 May 2015 at 07:38, Tom Rini <trini@konsulko.com> wrote: >> >> On Thu, Apr 30, 2015 at 07:54:21PM -0600, Simon Glass wrote: <--snip--> >> > Now that the tests are running again, I'll resume checking driver >> > model things before pulling things in. >> > >> > But we could use a way to run all tests, and some unification of them >> > (e.g. all run with the same U-Boot build). >> >> Yes, if it was easy to kick off most of the tests (ie anything we can do >> with sandbox) I'd integrate that into my process. > > Me too. Something I vaguely think about tidying up one day. In my version 3 of the net env series I made a move in the direction of making it easy to run all of the tests. I haven't ported all existing tests over, but it should be pretty easy now. I may get to it or others might. Cheers, -Joe
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c index 04e939d..898c1fc 100644 --- a/drivers/core/uclass.c +++ b/drivers/core/uclass.c @@ -174,7 +174,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); @@ -187,7 +187,7 @@ int uclass_find_next_device(struct udevice **devp) *devp = NULL; if (list_is_last(&dev->uclass_node, &dev->uclass->dev_head)) - return 0; + return -ENODEV; *devp = list_entry(dev->uclass_node.next, struct udevice, uclass_node); @@ -342,6 +342,8 @@ int uclass_first_device(enum uclass_id id, struct udevice **devp) *devp = NULL; ret = uclass_find_first_device(id, &dev); + if (ret == -ENODEV) + return 0; return uclass_get_device_tail(dev, ret, devp); } @@ -352,6 +354,8 @@ int uclass_next_device(struct udevice **devp) *devp = NULL; ret = uclass_find_next_device(&dev); + if (ret == -ENODEV) + return 0; return uclass_get_device_tail(dev, ret, devp); }
The change to refactor these functions created a regression. commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788 Author: Przemyslaw Marczak <p.marczak@samsung.com> Date: Wed Apr 15 13:07:17 2015 +0200 dm: core: add internal functions for getting the device without probe With this change, the dm unit tests started failing with a probe error -22 in the dm_test_children test. Test: dm_test_children test/dm/core.c:544, dm_test_children(): 0 == ret: Expected 0, got -22 This restores the original behavior which would avoid a probe on invalid device pointers. Signed-off-by: Joe Hershberger <joe.hershberger@ni.com> --- drivers/core/uclass.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)