dm: core: Fix regression caused by c1d6f91

PATCH New Joe Hershberger 4 participants 7 messages Apr 29, 2015 – May 3, 2015

lore

Share & tools

lore

Joe Hershberger

author

2015-04-29 04:14 UTC #0 lore raw

PATCH 1/1 dm: core: Fix regression caused by c1d6f91

The change to refactor these functions created a regression.

commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788
Author: Przemyslaw Marczak <[email protected]>
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 <[email protected]>

---

 drivers/core/uclass.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/core/uclass.c +6 -2

index 04e939d..898c1fc 100644
@@ -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);
 }

Signature · 1 line

1.7.11.5

Simon Glass

2015-04-29 13:30 UTC #1 lore raw

PATCH 1/1 dm: core: Fix regression caused by c1d6f91

Hi Joe,

Quoted 13 lines · Joe Hershberger

The change to refactor these functions created a regression.
commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788
Author: Przemyslaw Marczak <[email protected]>
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

Quoted 8 lines

This restores the original behavior which would avoid a probe on invalid device pointers.
Signed-off-by Joe Hershberger <[email protected]>
---
 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

Joe Hershberger

2015-04-29 16:17 UTC #2 lore raw

PATCH 1/1 dm: core: Fix regression caused by c1d6f91

Hi Simon,

Quoted 22 lines · Simon Glass

Hi Joe,
Quoted 19 lines · Joe Hershberger
The change to refactor these functions created a regression.
commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788
Author: Przemyslaw Marczak <[email protected]>
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 <[email protected]>
---

Quoted 6 lines

Quoted 2 lines
 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

Simon Glass

2015-05-01 01:54 UTC #3 lore raw

PATCH 1/1 dm: core: Fix regression caused by c1d6f91

Hi Joe,

Quoted 30 lines · Joe Hershberger

Hi Simon,
Quoted 27 lines · Simon Glass
Hi Joe,
Quoted 13 lines · Joe Hershberger
The change to refactor these functions created a regression.
commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788
Author: Przemyslaw Marczak <[email protected]>
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
Quoted 8 lines
This restores the original behavior which would avoid a probe on invalid device pointers.
Signed-off-by Joe Hershberger <[email protected]>
---
 drivers/core/uclass.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
Can you please check this - it is very similar to yours:

Quoted 8 lines

Quoted 1 line
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

Tom Rini

2015-05-01 13:38 UTC #4 lore raw

PATCH 1/1 dm: core: Fix regression caused by c1d6f91

Quoted 35 lines · Simon Glass

Hi Joe,
Quoted 32 lines · Joe Hershberger
Hi Simon,
Quoted 28 lines · Simon Glass
Hi Joe,
Quoted 13 lines · Joe Hershberger
The change to refactor these functions created a regression.
commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788
Author: Przemyslaw Marczak <[email protected]>
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
Quoted 8 lines
This restores the original behavior which would avoid a probe on invalid device pointers.
Signed-off-by Joe Hershberger <[email protected]>
---
 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/

Quoted 8 lines

Quoted 6 lines
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.

Quoted 5 lines

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.

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150501/d8475932/attachment.sig>

Simon Glass

2015-05-01 14:23 UTC #5 lore raw

PATCH 1/1 dm: core: Fix regression caused by c1d6f91

Hi Tom,

Quoted 44 lines · Tom Rini

Quoted 42 lines · Simon Glass
Hi Joe,
Quoted 38 lines · Joe Hershberger
Hi Simon,
Quoted 22 lines · Simon Glass
Hi Joe,
Quoted 19 lines · Joe Hershberger
The change to refactor these functions created a regression.
commit c1d6f91952d0761f61b0f0f96e4c7aa32eee2788
Author: Przemyslaw Marczak <[email protected]>
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 <[email protected]>
---
Quoted 6 lines
Quoted 2 lines
 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?

Quoted 8 lines

Quoted 1 line
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.

Quoted 9 lines

Quoted 6 lines
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

Joe Hershberger

2015-05-03 20:19 UTC #6 lore raw

PATCH 1/1 dm: core: Fix regression caused by c1d6f91

Hi Simon and Tom,

Quoted 5 lines · Simon Glass

Hi Tom,
Quoted 2 lines · Tom Rini
On Thu, Apr 30, 2015 at 07:54:21PM -0600, Simon Glass wrote:

<--snip-->

Quoted 10 lines

Quoted 8 lines
Quoted 5 lines
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