From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eugeniu Rosca Date: Fri, 25 May 2018 00:04:38 +0200 Subject: [U-Boot] [PATCH v3 6/6] test: dm: Fix wrong aliases property names In-Reply-To: References: <20180519121355.18377-1-erosca@de.adit-jv.com> <20180519121355.18377-6-erosca@de.adit-jv.com> Message-ID: <20180524220438.GA14935@example.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 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 > > * [Due to update] Dropped: Reviewed-by: Simon Glass > > > > 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 > > 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 at 5"; > > testfdt8 = "/a-test"; > > - fdt_dummy0 = "/translation-test at 8000/dev at 0,0"; > > - fdt_dummy1 = "/translation-test at 8000/dev at 1,100"; > > - fdt_dummy2 = "/translation-test at 8000/dev at 2,200"; > > - fdt_dummy3 = "/translation-test at 8000/noxlatebus at 3,300/dev at 42"; > > + fdt-dummy0 = "/translation-test at 8000/dev at 0,0"; > > + fdt-dummy1 = "/translation-test at 8000/dev at 1,100"; > > + fdt-dummy2 = "/translation-test at 8000/dev at 2,200"; > > + fdt-dummy3 = "/translation-test at 8000/noxlatebus at 3,300/dev at 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.