Hi Simon,
On Tue, May 22, 2018 at 05:30:40PM -0600, Simon Glass wrote:
Hi Eugeniu,
On 19 May 2018 at 06:13, Eugeniu Rosca roscaeugeniu@gmail.com wrote:
--snip--
v2->v3:
- Fixed an issue in the test code (test/dm/test-fdt.c) generated by the DTS update (arch/sandbox/dts/test.dts) in [PATCH v2].
- Changed commit summary line, to cover test/dm/test-fdt.c.
- Added: Reported-by: Petr Vorel pvorel@suse.cz
- [Due to update] Dropped: Reviewed-by: Simon Glass sjg@chromium.org
v1->v2:
- Newly pushed
arch/sandbox/dts/test.dts | 8 ++++---- test/dm/test-fdt.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
See below
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 683b1970e0af..3e87c5c0f3fd 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -27,10 +27,10 @@ testfdt3 = "/b-test"; testfdt5 = "/some-bus/c-test@5"; testfdt8 = "/a-test";
fdt_dummy0 = "/translation-test@8000/dev@0,0";fdt_dummy1 = "/translation-test@8000/dev@1,100";fdt_dummy2 = "/translation-test@8000/dev@2,200";fdt_dummy3 = "/translation-test@8000/noxlatebus@3,300/dev@42";
fdt-dummy0 = "/translation-test@8000/dev@0,0";fdt-dummy1 = "/translation-test@8000/dev@1,100";fdt-dummy2 = "/translation-test@8000/dev@2,200";fdt-dummy3 = "/translation-test@8000/noxlatebus@3,300/dev@42"; usb0 = &usb_0; usb1 = &usb_1; usb2 = &usb_2;diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 8196844e89a7..66d0df5629a2 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -425,7 +425,7 @@ static const struct udevice_id fdt_dummy_ids[] = { };
UCLASS_DRIVER(fdt_dummy) = {
.name = "fdt_dummy",
.name = "fdt-dummy",You should not need to change this one, and I worry that it is confusing since this is the driver name, not a compatible string.
I am not familiar with the in-tree U-boot unit tests, but doing a small experiment I get clear evidence that there is a tight relationship/dependency between the name of the entries in the aliases DTS node and the "name" field of UCLASS_DRIVER using that DTS.
The experiment is running 'ut dm' in sandbox before and after below patch (I also had to fix two null pointer dereferences in test/dm/bus.c to avoid segmentation faults with the patch applied):
diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 8196844e89a7..5b715e965269 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -94,7 +94,7 @@ int testfdt_ping(struct udevice *dev, int pingval, int *pingret) }
UCLASS_DRIVER(testfdt) = { - .name = "testfdt", + .name = "test_fdt", .id = UCLASS_TEST_FDT, .flags = DM_UC_FLAG_SEQ_ALIAS, };
Before the patch, I get: Failures: 9 After the patch, I get: Failures: 25
So, while the aliases entries are certainly not compatible strings, not keeping them aligned to UCLASS_DRIVER->name values leads to test failures. I am not sure if this is something specific to architecture of the tests or applies generically to any driver which fetches its configuration from DTS. I was hoping to get an assessment from somebody with more experience in this area.
Besides the above, it is not clear to me if your Reviewed-by applies to to this patch partially (since you expressed some concerns) or applies globally, in which case the concerns are not major?
.id = UCLASS_TEST_DUMMY, .flags = DM_UC_FLAG_SEQ_ALIAS,};
2.17.0
Regards, Simon
Best regards, Eugeniu.