This patch set unflattens device trees when running DT's unittest. It is
a follow up to
https://www.spinics.net/lists/linux-kselftest/msg05454.html part of the
KUnit RFC v3 thread
(https://www.spinics.net/lists/linux-kselftest/msg05431.html). Rob asked
me to submit this patch separately along with some modifications.
--
2.20.1.791.gb4d0f1c61a-goog
UML supports enabling OF, and is useful for running the device tree
tests, so add support for unflattening device tree blobs so we can
actually use it.
Signed-off-by: Brendan Higgins <[email protected]>
---
drivers/of/unittest.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 84427384654d5..effa4e2b9d992 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -2527,6 +2527,9 @@ static int __init of_unittest(void)
}
of_node_put(np);
+ if (IS_ENABLED(CONFIG_UML))
+ unflatten_device_tree();
+
pr_info("start of unittest - you will see error messages\n");
of_unittest_check_tree_linkage();
of_unittest_check_phandles();
--
2.20.1.791.gb4d0f1c61a-goog
On Tue, Feb 12, 2019 at 10:53:05AM -0800, Brendan Higgins wrote:
> UML supports enabling OF, and is useful for running the device tree
> tests, so add support for unflattening device tree blobs so we can
> actually use it.
>
> Signed-off-by: Brendan Higgins <[email protected]>
> ---
For a single patch, no need for a cover letter. Just add any commentary
here.
> drivers/of/unittest.c | 3 +++
> 1 file changed, 3 insertions(+)
Applied, thanks.
Rob
On 2/12/19 10:53 AM, Brendan Higgins wrote:
> UML supports enabling OF, and is useful for running the device tree
> tests, so add support for unflattening device tree blobs so we can
> actually use it.
>
> Signed-off-by: Brendan Higgins <[email protected]>
> ---
> drivers/of/unittest.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index 84427384654d5..effa4e2b9d992 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -2527,6 +2527,9 @@ static int __init of_unittest(void)
> }
> of_node_put(np);
>
> + if (IS_ENABLED(CONFIG_UML))
> + unflatten_device_tree();
> +
> pr_info("start of unittest - you will see error messages\n");
> of_unittest_check_tree_linkage();
> of_unittest_check_phandles();
>
(Insert my usual disclaimer that I am clueless about UML, I still need to learn
about it...)
This does not look correct to me.
A few lines earlier in of_unittest(), the live devicetree needs to exist for
unittest_data_data() and a few of_*() functions to succeed. So it seems
that the unflatten_device_tree() for uml should be at the beginning of
of_unittest().
Rob, if I am correct please revert this patch.
-Frank
On Thu, Feb 14, 2019 at 4:10 PM Frank Rowand <[email protected]> wrote:
>
> On 2/12/19 10:53 AM, Brendan Higgins wrote:
> > UML supports enabling OF, and is useful for running the device tree
> > tests, so add support for unflattening device tree blobs so we can
> > actually use it.
> >
> > Signed-off-by: Brendan Higgins <[email protected]>
> > ---
> > drivers/of/unittest.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> > index 84427384654d5..effa4e2b9d992 100644
> > --- a/drivers/of/unittest.c
> > +++ b/drivers/of/unittest.c
> > @@ -2527,6 +2527,9 @@ static int __init of_unittest(void)
> > }
> > of_node_put(np);
> >
> > + if (IS_ENABLED(CONFIG_UML))
> > + unflatten_device_tree();
> > +
> > pr_info("start of unittest - you will see error messages\n");
> > of_unittest_check_tree_linkage();
> > of_unittest_check_phandles();
> >
>
> (Insert my usual disclaimer that I am clueless about UML, I still need to learn
> about it...)
>
> This does not look correct to me.
>
> A few lines earlier in of_unittest(), the live devicetree needs to exist for
> unittest_data_data() and a few of_*() functions to succeed. So it seems
> that the unflatten_device_tree() for uml should be at the beginning of
> of_unittest().
It is true that other functions ahead of it depend on the presence of
a device tree, but an unflattened tree does get linked in somewhere
else. Despite that, this is needed for overlay_base_root. I got
similar behavior if you don't link in a flattened device tree on x86.
Thus, the order in which you call them doesn't actually seem to
matter. I found no difference from changing the order in UML myself.
Without my patch we get the following error,
### dt-test ### FAIL of_unittest_overlay_high_level():2372
overlay_base_root not initialized
### dt-test ### end of unittest - 219 passed, 1 failed
With my patch we get:
### dt-test ### end of unittest - 224 passed, 0 failed
I used the following .config for these results:
CONFIG_OF=y
CONFIG_OF_UNITTEST=y
CONFIG_OF_OVERLAY=y
CONFIG_I2C=y
CONFIG_I2C_MUX=y
>
> Rob, if I am correct please revert this patch.
>
Cheers
On 2/14/19 5:26 PM, Brendan Higgins wrote:
> On Thu, Feb 14, 2019 at 4:10 PM Frank Rowand <[email protected]> wrote:
>>
>> On 2/12/19 10:53 AM, Brendan Higgins wrote:
>>> UML supports enabling OF, and is useful for running the device tree
>>> tests, so add support for unflattening device tree blobs so we can
>>> actually use it.
>>>
>>> Signed-off-by: Brendan Higgins <[email protected]>
>>> ---
>>> drivers/of/unittest.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
>>> index 84427384654d5..effa4e2b9d992 100644
>>> --- a/drivers/of/unittest.c
>>> +++ b/drivers/of/unittest.c
>>> @@ -2527,6 +2527,9 @@ static int __init of_unittest(void)
>>> }
>>> of_node_put(np);
>>>
>>> + if (IS_ENABLED(CONFIG_UML))
>>> + unflatten_device_tree();
>>> +
>>> pr_info("start of unittest - you will see error messages\n");
>>> of_unittest_check_tree_linkage();
>>> of_unittest_check_phandles();
>>>
>>
>> (Insert my usual disclaimer that I am clueless about UML, I still need to learn
>> about it...)
>>
>> This does not look correct to me.
>>
>> A few lines earlier in of_unittest(), the live devicetree needs to exist for
>> unittest_data_data() and a few of_*() functions to succeed. So it seems
>> that the unflatten_device_tree() for uml should be at the beginning of
>> of_unittest().
>
> It is true that other functions ahead of it depend on the presence of
> a device tree, but an unflattened tree does get linked in somewhere
> else. Despite that, this is needed for overlay_base_root. I got
> similar behavior if you don't link in a flattened device tree on x86.
> Thus, the order in which you call them doesn't actually seem to
> matter. I found no difference from changing the order in UML myself.
>
> Without my patch we get the following error,
> ### dt-test ### FAIL of_unittest_overlay_high_level():2372
> overlay_base_root not initialized
> ### dt-test ### end of unittest - 219 passed, 1 failed
>
> With my patch we get:
> ### dt-test ### end of unittest - 224 passed, 0 failed
Thanks for reporting both the failure and the success cases,
that helps me understand a little bit better.
If instead of the above patch, if you add the following (untested,
not even compile tested) to the beginning of of_unittest():
if (IS_ENABLED(CONFIG_UML))
unittest_unflatten_overlay_base();
does that also result in a good test result of:
### dt-test ### end of unittest - 224 passed, 0 failed
I think I need to find some time to build and boot a UML kernel soon.
My current _guess_ is that the original problem was not a failure to
unflatten any present devicetree in UML but instead that the UML
kernel does not call unflatten_device_tree() and thus fails to
indirectly call unittest_unflatten_overlay_base(), which is
called by unflatten_device_tree().
unittest_unflatten_overlay_base() is an unfortunate wart that I
added, but I don't have a better alternative yet.
-Frank
>
> I used the following .config for these results:
> CONFIG_OF=y
> CONFIG_OF_UNITTEST=y
> CONFIG_OF_OVERLAY=y
> CONFIG_I2C=y
> CONFIG_I2C_MUX=y
>
>>
>> Rob, if I am correct please revert this patch.
>>
>
> Cheers
>
On Thu, Feb 14, 2019 at 6:48 PM Frank Rowand <[email protected]> wrote:
>
> On 2/14/19 5:26 PM, Brendan Higgins wrote:
> > On Thu, Feb 14, 2019 at 4:10 PM Frank Rowand <[email protected]> wrote:
> >>
> >> On 2/12/19 10:53 AM, Brendan Higgins wrote:
> >>> UML supports enabling OF, and is useful for running the device tree
> >>> tests, so add support for unflattening device tree blobs so we can
> >>> actually use it.
> >>>
> >>> Signed-off-by: Brendan Higgins <[email protected]>
> >>> ---
> >>> drivers/of/unittest.c | 3 +++
> >>> 1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> >>> index 84427384654d5..effa4e2b9d992 100644
> >>> --- a/drivers/of/unittest.c
> >>> +++ b/drivers/of/unittest.c
> >>> @@ -2527,6 +2527,9 @@ static int __init of_unittest(void)
> >>> }
> >>> of_node_put(np);
> >>>
> >>> + if (IS_ENABLED(CONFIG_UML))
> >>> + unflatten_device_tree();
> >>> +
> >>> pr_info("start of unittest - you will see error messages\n");
> >>> of_unittest_check_tree_linkage();
> >>> of_unittest_check_phandles();
> >>>
> >>
> >> (Insert my usual disclaimer that I am clueless about UML, I still need to learn
> >> about it...)
> >>
> >> This does not look correct to me.
> >>
> >> A few lines earlier in of_unittest(), the live devicetree needs to exist for
> >> unittest_data_data() and a few of_*() functions to succeed. So it seems
> >> that the unflatten_device_tree() for uml should be at the beginning of
> >> of_unittest().
> >
> > It is true that other functions ahead of it depend on the presence of
> > a device tree, but an unflattened tree does get linked in somewhere
> > else. Despite that, this is needed for overlay_base_root. I got
> > similar behavior if you don't link in a flattened device tree on x86.
> > Thus, the order in which you call them doesn't actually seem to
> > matter. I found no difference from changing the order in UML myself.
> >
> > Without my patch we get the following error,
> > ### dt-test ### FAIL of_unittest_overlay_high_level():2372
> > overlay_base_root not initialized
> > ### dt-test ### end of unittest - 219 passed, 1 failed
> >
> > With my patch we get:
> > ### dt-test ### end of unittest - 224 passed, 0 failed
>
> Thanks for reporting both the failure and the success cases,
> that helps me understand a little bit better.
>
> If instead of the above patch, if you add the following (untested,
> not even compile tested) to the beginning of of_unittest():
>
> if (IS_ENABLED(CONFIG_UML))
> unittest_unflatten_overlay_base();
>
> does that also result in a good test result of:
>
> ### dt-test ### end of unittest - 224 passed, 0 failed
Yep, I just tried it. It works as you suspected.
>
> I think I need to find some time to build and boot a UML kernel soon.
It's really no big deal, just copy the .config I sent and build with
`make ARCH=um` then you "boot" the kernel with `./linux` (note this
will mess up your terminal settings); that's it! (Shameless plug: you
can also try it out with the KUnit patchset with
`./tools/testing/kunit/kunit.py --timeout=30 --jobs=12 --defconfig`,
which builds the kernel with a pretty similar config, boots the
kernel, and then parses the output for you. ;-) )
>
> My current _guess_ is that the original problem was not a failure to
> unflatten any present devicetree in UML but instead that the UML
> kernel does not call unflatten_device_tree() and thus fails to
> indirectly call unittest_unflatten_overlay_base(), which is
> called by unflatten_device_tree().
I think you are right. Sorry for not noticing this before making my
change. Since it was pretty much the only architecture (the only one I
care about) that does not unflatten DT, I assumed that was the
problem. I didn't put too much thought into it after that point beyond
making sure that it did what I wanted.
>
> unittest_unflatten_overlay_base() is an unfortunate wart that I
> added, but I don't have a better alternative yet.
Hey, I get it. No worries.
In any case, it seems like unittest_unflatten_overlay_base() is the
right function to call there. I will send out patch. Do you want me to
send a patch on top of this one, or do you want to revert this one and
for me to send a v2 follow up to this patch? I don't care either way,
whatever you guys prefer.
Cheers
On Fri, Feb 15, 2019 at 3:49 AM Brendan Higgins
<[email protected]> wrote:
>
> On Thu, Feb 14, 2019 at 6:48 PM Frank Rowand <[email protected]> wrote:
> >
> > On 2/14/19 5:26 PM, Brendan Higgins wrote:
> > > On Thu, Feb 14, 2019 at 4:10 PM Frank Rowand <[email protected]> wrote:
> > >>
> > >> On 2/12/19 10:53 AM, Brendan Higgins wrote:
> > >>> UML supports enabling OF, and is useful for running the device tree
> > >>> tests, so add support for unflattening device tree blobs so we can
> > >>> actually use it.
> > >>>
> > >>> Signed-off-by: Brendan Higgins <[email protected]>
> > >>> ---
> > >>> drivers/of/unittest.c | 3 +++
> > >>> 1 file changed, 3 insertions(+)
> > >>>
> > >>> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> > >>> index 84427384654d5..effa4e2b9d992 100644
> > >>> --- a/drivers/of/unittest.c
> > >>> +++ b/drivers/of/unittest.c
> > >>> @@ -2527,6 +2527,9 @@ static int __init of_unittest(void)
> > >>> }
> > >>> of_node_put(np);
> > >>>
> > >>> + if (IS_ENABLED(CONFIG_UML))
> > >>> + unflatten_device_tree();
> > >>> +
> > >>> pr_info("start of unittest - you will see error messages\n");
> > >>> of_unittest_check_tree_linkage();
> > >>> of_unittest_check_phandles();
> > >>>
> > >>
> > >> (Insert my usual disclaimer that I am clueless about UML, I still need to learn
> > >> about it...)
> > >>
> > >> This does not look correct to me.
> > >>
> > >> A few lines earlier in of_unittest(), the live devicetree needs to exist for
> > >> unittest_data_data() and a few of_*() functions to succeed. So it seems
> > >> that the unflatten_device_tree() for uml should be at the beginning of
> > >> of_unittest().
> > >
> > > It is true that other functions ahead of it depend on the presence of
> > > a device tree, but an unflattened tree does get linked in somewhere
> > > else. Despite that, this is needed for overlay_base_root. I got
> > > similar behavior if you don't link in a flattened device tree on x86.
> > > Thus, the order in which you call them doesn't actually seem to
> > > matter. I found no difference from changing the order in UML myself.
> > >
> > > Without my patch we get the following error,
> > > ### dt-test ### FAIL of_unittest_overlay_high_level():2372
> > > overlay_base_root not initialized
> > > ### dt-test ### end of unittest - 219 passed, 1 failed
> > >
> > > With my patch we get:
> > > ### dt-test ### end of unittest - 224 passed, 0 failed
> >
> > Thanks for reporting both the failure and the success cases,
> > that helps me understand a little bit better.
> >
> > If instead of the above patch, if you add the following (untested,
> > not even compile tested) to the beginning of of_unittest():
> >
> > if (IS_ENABLED(CONFIG_UML))
> > unittest_unflatten_overlay_base();
> >
> > does that also result in a good test result of:
> >
> > ### dt-test ### end of unittest - 224 passed, 0 failed
>
> Yep, I just tried it. It works as you suspected.
>
> >
> > I think I need to find some time to build and boot a UML kernel soon.
>
> It's really no big deal, just copy the .config I sent and build with
> `make ARCH=um` then you "boot" the kernel with `./linux` (note this
> will mess up your terminal settings); that's it! (Shameless plug: you
> can also try it out with the KUnit patchset with
> `./tools/testing/kunit/kunit.py --timeout=30 --jobs=12 --defconfig`,
> which builds the kernel with a pretty similar config, boots the
> kernel, and then parses the output for you. ;-) )
>
> >
> > My current _guess_ is that the original problem was not a failure to
> > unflatten any present devicetree in UML but instead that the UML
> > kernel does not call unflatten_device_tree() and thus fails to
> > indirectly call unittest_unflatten_overlay_base(), which is
> > called by unflatten_device_tree().
>
> I think you are right. Sorry for not noticing this before making my
> change. Since it was pretty much the only architecture (the only one I
> care about) that does not unflatten DT, I assumed that was the
> problem. I didn't put too much thought into it after that point beyond
> making sure that it did what I wanted.
>
> >
> > unittest_unflatten_overlay_base() is an unfortunate wart that I
> > added, but I don't have a better alternative yet.
>
> Hey, I get it. No worries.
>
> In any case, it seems like unittest_unflatten_overlay_base() is the
> right function to call there. I will send out patch. Do you want me to
> send a patch on top of this one, or do you want to revert this one and
> for me to send a v2 follow up to this patch? I don't care either way,
> whatever you guys prefer.
I'll drop or revert the existing one, so against mainline is good.
Rob
On 2/15/19 1:49 AM, Brendan Higgins wrote:
> On Thu, Feb 14, 2019 at 6:48 PM Frank Rowand <[email protected]> wrote:
>>
>> On 2/14/19 5:26 PM, Brendan Higgins wrote:
>>> On Thu, Feb 14, 2019 at 4:10 PM Frank Rowand <[email protected]> wrote:
>>>>
>>>> On 2/12/19 10:53 AM, Brendan Higgins wrote:
>>>>> UML supports enabling OF, and is useful for running the device tree
>>>>> tests, so add support for unflattening device tree blobs so we can
>>>>> actually use it.
>>>>>
>>>>> Signed-off-by: Brendan Higgins <[email protected]>
>>>>> ---
>>>>> drivers/of/unittest.c | 3 +++
>>>>> 1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
>>>>> index 84427384654d5..effa4e2b9d992 100644
>>>>> --- a/drivers/of/unittest.c
>>>>> +++ b/drivers/of/unittest.c
>>>>> @@ -2527,6 +2527,9 @@ static int __init of_unittest(void)
>>>>> }
>>>>> of_node_put(np);
>>>>>
>>>>> + if (IS_ENABLED(CONFIG_UML))
>>>>> + unflatten_device_tree();
>>>>> +
>>>>> pr_info("start of unittest - you will see error messages\n");
>>>>> of_unittest_check_tree_linkage();
>>>>> of_unittest_check_phandles();
>>>>>
>>>>
>>>> (Insert my usual disclaimer that I am clueless about UML, I still need to learn
>>>> about it...)
>>>>
>>>> This does not look correct to me.
>>>>
>>>> A few lines earlier in of_unittest(), the live devicetree needs to exist for
>>>> unittest_data_data() and a few of_*() functions to succeed. So it seems
>>>> that the unflatten_device_tree() for uml should be at the beginning of
>>>> of_unittest().
>>>
>>> It is true that other functions ahead of it depend on the presence of
>>> a device tree, but an unflattened tree does get linked in somewhere
>>> else. Despite that, this is needed for overlay_base_root. I got
>>> similar behavior if you don't link in a flattened device tree on x86.
>>> Thus, the order in which you call them doesn't actually seem to
>>> matter. I found no difference from changing the order in UML myself.
>>>
>>> Without my patch we get the following error,
>>> ### dt-test ### FAIL of_unittest_overlay_high_level():2372
>>> overlay_base_root not initialized
>>> ### dt-test ### end of unittest - 219 passed, 1 failed
>>>
>>> With my patch we get:
>>> ### dt-test ### end of unittest - 224 passed, 0 failed
>>
>> Thanks for reporting both the failure and the success cases,
>> that helps me understand a little bit better.
>>
>> If instead of the above patch, if you add the following (untested,
>> not even compile tested) to the beginning of of_unittest():
>>
>> if (IS_ENABLED(CONFIG_UML))
>> unittest_unflatten_overlay_base();
>>
>> does that also result in a good test result of:
>>
>> ### dt-test ### end of unittest - 224 passed, 0 failed
>
> Yep, I just tried it. It works as you suspected.
>
>>
>> I think I need to find some time to build and boot a UML kernel soon.
>
> It's really no big deal, just copy the .config I sent and build with
> `make ARCH=um` then you "boot" the kernel with `./linux` (note this
> will mess up your terminal settings); that's it! (Shameless plug: you
> can also try it out with the KUnit patchset with
> `./tools/testing/kunit/kunit.py --timeout=30 --jobs=12 --defconfig`,
> which builds the kernel with a pretty similar config, boots the
> kernel, and then parses the output for you. ;-) )
Thanks, that was enough info to prod me into building and "booting"
a uml kernel. I have another framework that I use, so I did not
try kunit.py, but reading that filled in any missing details that
I needed.
As I mentioned, I used my own framework, but the commands that it
emitted essentially boil down to a rather simple recipe:
export ARCH=um
make kunit_defconfig
make olddefconfig
make linux
# KBUILD_OUTPUT is my build directory
${KBUILD_OUTPUT}/linux mem=256m
>
>>
>> My current _guess_ is that the original problem was not a failure to
>> unflatten any present devicetree in UML but instead that the UML
>> kernel does not call unflatten_device_tree() and thus fails to
>> indirectly call unittest_unflatten_overlay_base(), which is
>> called by unflatten_device_tree().
>
> I think you are right. Sorry for not noticing this before making my
> change. Since it was pretty much the only architecture (the only one I
> care about) that does not unflatten DT, I assumed that was the
> problem. I didn't put too much thought into it after that point beyond
> making sure that it did what I wanted.
>
>>
>> unittest_unflatten_overlay_base() is an unfortunate wart that I
>> added, but I don't have a better alternative yet.
>
> Hey, I get it. No worries.
>
> In any case, it seems like unittest_unflatten_overlay_base() is the
> right function to call there. I will send out patch. Do you want me to
Thanks for the updated patch.
> send a patch on top of this one, or do you want to revert this one and
> for me to send a v2 follow up to this patch? I don't care either way,
> whatever you guys prefer.
>
> Cheers
>