Re: [PATCH 2/2] of: Add unit tests for applying overlays.

[Rob Herring]

On Mon, Apr 24, 2017 at 12:47 PM, Frank Rowand <frowand.list@xxxxxxxxx> wrote:
> On 04/24/17 10:16, Rob Herring wrote:
>> On Mon, Apr 24, 2017 at 12:43 AM,  <frowand.list@xxxxxxxxx> wrote:
>>> From: Frank Rowand <frank.rowand@xxxxxxxx>
>>>
>>> Existing overlay unit tests examine individual pieces of the overlay
>>> code.  The new tests target the entire process of applying an overlay.
>>>
>>> Signed-off-by: Frank Rowand <frank.rowand@xxxxxxxx>

[...]

>>> @@ -1256,11 +1258,54 @@ bool __init early_init_dt_scan(void *params)
>>>   */
>>>  void __init unflatten_device_tree(void)
>>>  {
>>> +#ifdef CONFIG_OF_UNITTEST
>>> +       extern uint8_t __dtb_ot_base_begin[];
>>> +       extern uint8_t __dtb_ot_base_end[];
>>> +       struct device_node *ot_base_root;
>>> +       void *ot_base;
>>> +       u32 data_size;
>>> +       u32 size;
>>> +#endif
>>> +
>>>         __unflatten_device_tree(initial_boot_params, NULL, &of_root,
>>>                                 early_init_dt_alloc_memory_arch, false);
>>>
>>>         /* Get pointer to "/chosen" and "/aliases" nodes for use everywhere */
>>>         of_alias_scan(early_init_dt_alloc_memory_arch);
>>
>> Just make __unflatten_device_tree accessible to the unit test code and
>> move all this to it. Then you don't need the ifdefery.
>
> Good idea.  I'll do that.
>
>
>> Does this need to be immediately after unflattening the base tree?
>
> My goal is to make the creation of the test data in the tree follow
> the normal process as much as possible, so that real code is tested
> instead of testing test code.
>
> This flattened device tree contains the base information that the
> test overlays are applied against.

Okay. If you need it here, then you can put this all into a unittest
function and call it from here.


>>> +#ifdef CONFIG_OF_OVERLAY
>>> +/*
>>> + * The purpose of of_unittest_overlay_test_data_add is to add an
>>> + * overlay in the normal fashion.  This is a test of the whole
>>> + * picture, instead of testing individual elements.
>>> + *
>>> + * A secondary purpose is to be able to verify that the contents of
>>> + * /proc/device-tree/ contains the updated structure and values from
>>> + * the overlay.  That must be verified separately in user space.
>>> + *
>>> + * Return 0 on unexpected error.
>>> + */
>>> +static int __init overlay_test_data_add(int onum)
>>
>> There's a need for a general function to apply built-in overlays
>> beyond just unittests. See
>> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c. It's pretty close to the
>> same set of calls.
>
> Yes, agreed.
>
> My plan in the next release cycle is to first clean up drivers/of/overlay.c.
> No functional changes, just cosmetic such as aligning function names with
> what they actually do.
>
> Then make some (hopefully) minor correctness changes, such as locking
> correctly around phandle adjustments.
>
> Then create the general function to apply built-in overlays and convert
> all (two) separate implementations to use the common function.  I did
> not want to delay adding the unit tests to wait for this step.

Okay. Whatever order you want to do it is fine.

Rob