Re: [PATCH V6 5/6] of: unittest: Create overlay_common.dtsi and testcases_common.dtsi

[Frank Rowand <frowand.list@gmail.com>]

Hi Viresh,

On 1/26/21 11:56 PM, Viresh Kumar wrote:
> On 22-01-21, 16:20, Viresh Kumar wrote:
>> In order to build-test the same unit-test files using fdtoverlay tool,
>> move the device nodes from the existing overlay_base.dts and
>> testcases_common.dts files to .dtsi files. The .dts files now include
>> the new .dtsi files, resulting in exactly the same behavior as earlier.
>>
>> The .dtsi files can now be reused for compile time tests using
>> fdtoverlay (will be done in a later patch).
>>
>> This is required because the base files passed to fdtoverlay tool
>> shouldn't be overlays themselves (i.e. shouldn't have the /plugin/;
>> tag).
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>>  drivers/of/unittest-data/overlay_base.dts     | 90 +-----------------
>>  drivers/of/unittest-data/overlay_common.dtsi  | 91 +++++++++++++++++++
>>  drivers/of/unittest-data/testcases.dts        | 17 +---
>>  .../of/unittest-data/testcases_common.dtsi    | 18 ++++
>>  4 files changed, 111 insertions(+), 105 deletions(-)
>>  create mode 100644 drivers/of/unittest-data/overlay_common.dtsi
>>  create mode 100644 drivers/of/unittest-data/testcases_common.dtsi
> 
> Frank,
> 
> As I mentioned in the cover-letter, I get a build warning right now:
> 
> drivers/of/unittest-data/tests-interrupts.dtsi:20.5-28: Warning (interrupts_property): /testcase-data/testcase-device2:#interrupt-cells: size is (4), expected multiple of 8

Thanks for catching that.

> 
> I think I need to add below diff to this patch to fix this warning, will that
> be okay ?

Nope, the change below won't work because it removes the node testcase-device2 from the tests
that unittest.c does (if I am thinking correctly).  I will double check my thinking, but I
know you are spinning the patch, so I didn't want to delay this reply.

Note that this node has a deliberate error in it "/* invalid specifier - too short */".

I'm not sure why the dtc warning triggers on line 20 instead of line 68.  I'll have to go
look at the dtc source to better understand the warning.

-Frank

> 
> diff --git a/drivers/of/unittest-data/testcases.dts b/drivers/of/unittest-data/testcases.dts
> index 185125085784..04b9e7bb30d9 100644
> --- a/drivers/of/unittest-data/testcases.dts
> +++ b/drivers/of/unittest-data/testcases.dts
> @@ -3,3 +3,14 @@
>  /plugin/;
>  
>  #include "testcases_common.dtsi"
> +
> +/ {
> +       testcase-data {
> +               testcase-device2 {
> +                       compatible = "testcase-device";
> +                       interrupt-parent = <&test_intc2>;
> +                       interrupts = <1>; /* invalid specifier - too short */
> +               };
> +       };
> +
> +};
> diff --git a/drivers/of/unittest-data/tests-interrupts.dtsi b/drivers/of/unittest-data/tests-interrupts.dtsi
> index ec175e800725..0e5914611107 100644
> --- a/drivers/of/unittest-data/tests-interrupts.dtsi
> +++ b/drivers/of/unittest-data/tests-interrupts.dtsi
> @@ -61,12 +61,5 @@ testcase-device1 {
>                         interrupt-parent = <&test_intc0>;
>                         interrupts = <1>;
>                 };
> -
> -               testcase-device2 {
> -                       compatible = "testcase-device";
> -                       interrupt-parent = <&test_intc2>;
> -                       interrupts = <1>; /* invalid specifier - too short */
> -               };
>         };
> -
>  };
>