On Tue, Nov 29, 2016 at 01:11:43PM +0200, Pantelis Antoniou wrote:
> Hi David,
>
> > On Nov 29, 2016, at 05:08 , David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > On Mon, Nov 28, 2016 at 06:05:38PM +0200, Pantelis Antoniou wrote:
> >> Add a number of tests for dynamic objects/overlays.
> >>
> >> Re-use the original test by moving the contents to a .dtsi include
> >>
> >> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@xxxxxxxxxxxx>
> >> ---
> >> tests/overlay_overlay_dtc.dts | 76 +----------------------------------
> >> tests/overlay_overlay_dtc.dtsi | 83 +++++++++++++++++++++++++++++++++++++++
> >> tests/overlay_overlay_new_dtc.dts | 11 ++++++
> >> tests/overlay_overlay_simple.dts | 12 ++++++
> >> tests/run_tests.sh | 41 +++++++++++++++++++
> >> 5 files changed, 148 insertions(+), 75 deletions(-)
> >> create mode 100644 tests/overlay_overlay_dtc.dtsi
> >> create mode 100644 tests/overlay_overlay_new_dtc.dts
> >> create mode 100644 tests/overlay_overlay_simple.dts
> >>
> >> diff --git a/tests/overlay_overlay_dtc.dts b/tests/overlay_overlay_dtc.dts
> >> index 30d2362..ca943ea 100644
> >> --- a/tests/overlay_overlay_dtc.dts
> >> +++ b/tests/overlay_overlay_dtc.dts
> >> @@ -8,78 +8,4 @@
> >> /dts-v1/;
> >> /plugin/;
> >>
> >> -/ {
> >> - /* Test that we can change an int by another */
> >> - fragment@0 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - test-int-property = <43>;
> >> - };
> >> - };
> >> -
> >> - /* Test that we can replace a string by a longer one */
> >> - fragment@1 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - test-str-property = "foobar";
> >> - };
> >> - };
> >> -
> >> - /* Test that we add a new property */
> >> - fragment@2 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - test-str-property-2 = "foobar2";
> >> - };
> >> - };
> >> -
> >> - /* Test that we add a new node (by phandle) */
> >> - fragment@3 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - new-node {
> >> - new-property;
> >> - };
> >> - };
> >> - };
> >> -
> >> - fragment@5 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - local: new-local-node {
> >> - new-property;
> >> - };
> >> - };
> >> - };
> >> -
> >> - fragment@6 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - test-phandle = <&test>, <&local>;
> >> - };
> >> - };
> >> -
> >> - fragment@7 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - test-several-phandle = <&local>, <&local>;
> >> - };
> >> - };
> >> -
> >> - fragment@8 {
> >> - target = <&test>;
> >> -
> >> - __overlay__ {
> >> - sub-test-node {
> >> - new-sub-test-property;
> >> - };
> >> - };
> >> - };
> >> -};
> >> +/include/ "overlay_overlay_dtc.dtsi"
> >
> > Don't duplicate this, just replace it with the new style. This only
> > existed as essentially documentation for the libfdt overlay
> > application stuff. Since the new dtc won't support the old tag
> > format, there's no point having a test for it.
>
> The parser now handles both tag formats just fine. I could remove support
> for it if you’re willing to tackle the flak.
Oh, sorry, I missed that.
I'd suggest having that controlled by the same "backwards compat"
option to control magic number and other things. Although changing
parser behaviour based on flags can get fiddly.
>
> >> diff --git a/tests/overlay_overlay_dtc.dtsi b/tests/overlay_overlay_dtc.dtsi
> >> new file mode 100644
> >> index 0000000..8ea8d5d
> >> --- /dev/null
> >> +++ b/tests/overlay_overlay_dtc.dtsi
> >> @@ -0,0 +1,83 @@
> >> +/*
> >> + * Copyright (c) 2016 NextThing Co
> >> + * Copyright (c) 2016 Free Electrons
> >> + * Copyright (c) 2016 Konsulko Inc.
> >> + *
> >> + * SPDX-License-Identifier: GPL-2.0+
> >> + */
> >> +
> >> +/ {
> >> + /* Test that we can change an int by another */
> >> + fragment@0 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + test-int-property = <43>;
> >> + };
> >> + };
> >> +
> >> + /* Test that we can replace a string by a longer one */
> >> + fragment@1 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + test-str-property = "foobar";
> >> + };
> >> + };
> >> +
> >> + /* Test that we add a new property */
> >> + fragment@2 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + test-str-property-2 = "foobar2";
> >> + };
> >> + };
> >> +
> >> + /* Test that we add a new node (by phandle) */
> >> + fragment@3 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + new-node {
> >> + new-property;
> >> + };
> >> + };
> >> + };
> >> +
> >> + fragment@5 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + local: new-local-node {
> >> + new-property;
> >> + };
> >> + };
> >> + };
> >> +
> >> + fragment@6 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + test-phandle = <&test>, <&local>;
> >> + };
> >> + };
> >> +
> >> + fragment@7 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + test-several-phandle = <&local>, <&local>;
> >> + };
> >> + };
> >> +
> >> + fragment@8 {
> >> + target = <&test>;
> >> +
> >> + __overlay__ {
> >> + sub-test-node {
> >> + new-sub-test-property;
> >> + };
> >> + };
> >> + };
> >> +};
> >> diff --git a/tests/overlay_overlay_new_dtc.dts b/tests/overlay_overlay_new_dtc.dts
> >> new file mode 100644
> >> index 0000000..14d3f54
> >> --- /dev/null
> >> +++ b/tests/overlay_overlay_new_dtc.dts
> >> @@ -0,0 +1,11 @@
> >> +/*
> >> + * Copyright (c) 2016 NextThing Co
> >> + * Copyright (c) 2016 Free Electrons
> >> + * Copyright (c) 2016 Konsulko Inc.
> >> + *
> >> + * SPDX-License-Identifier: GPL-2.0+
> >> + */
> >> +
> >> +/dts-v1/ /plugin/;
> >> +
> >> +/include/ "overlay_overlay_dtc.dtsi"
> >> diff --git a/tests/overlay_overlay_simple.dts b/tests/overlay_overlay_simple.dts
> >> new file mode 100644
> >> index 0000000..8657e1e
> >> --- /dev/null
> >> +++ b/tests/overlay_overlay_simple.dts
> >> @@ -0,0 +1,12 @@
> >> +/*
> >> + * Copyright (c) 2016 Konsulko Inc.
> >> + *
> >> + * SPDX-License-Identifier: GPL-2.0+
> >> + */
> >> +
> >> +/dts-v1/;
> >> +/plugin/;
> >> +
> >> +&test {
> >> + test-int-property = <43>;
> >> +};
> >> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> >> index e4139dd..74af0ff 100755
> >> --- a/tests/run_tests.sh
> >> +++ b/tests/run_tests.sh
> >> @@ -181,6 +181,47 @@ overlay_tests () {
> >> run_dtc_test -@ -I dts -O dtb -o overlay_base_with_symbols.test.dtb overlay_base.dts
> >> run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.dtb overlay_overlay_dtc.dts
> >> run_test overlay overlay_base_with_symbols.test.dtb overlay_overlay_with_symbols.test.dtb
> >> +
> >> + # new /plugin/ format
> >> + run_dtc_test -@ -I dts -O dtb -o overlay_overlay_new_with_symbols.test.dtb overlay_overlay_new_dtc.dts
> >> + run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__symbols__"
> >> + run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__fixups__"
> >> + run_test check_path overlay_overlay_new_with_symbols.test.dtb exists "/__local_fixups__"
> >
> > Looks like you're mixing tabs and spaces here. I don't really mind
> > which, but keep it consistent at least at the same indentation level.
> >
>
> Oh, sorry, I use tabs but this sections has spaces… Will fix.
Well, it's more tha the first line in the block seems to be using
spaces, then the rest using tabs.
>
> >> + # test new magic option
> >> + run_dtc_test -M@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_dtc.dts
> >> + run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__symbols__"
> >> + run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__fixups__"
> >> + run_test check_path overlay_overlay_with_symbols_new_magic.test.dtb exists "/__local_fixups__"
> >> +
> >> + # test plugin source to dtb and back
> >> + run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc.test.dts overlay_overlay_with_symbols.test.dtb
> >> + run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols.test.test.dtb overlay_overlay_dtc.test.dts
> >> + run_test dtbs_equal_ordered overlay_overlay_with_symbols.test.dtb overlay_overlay_with_symbols.test.test.dtb
> >> +
> >> + # test plugin source to dtb and back (with new magic)
> >> + run_dtc_test -@ -I dtb -O dts -o overlay_overlay_dtc_new_magic.test.dts overlay_overlay_with_symbols_new_magic.test.dtb
> >> + run_dtc_test -@ -I dts -O dtb -o overlay_overlay_with_symbols_new_magic.test.test.dtb overlay_overlay_dtc_new_magic.test.dts
> >> + run_test dtbs_equal_ordered overlay_overlay_with_symbols_new_magic.test.dtb overlay_overlay_with_symbols_new_magic.test.test.dtb
> >> +
> >> + # test plugin auto-generation without using -@
> >> + run_dtc_test -I dts -O dtb -o overlay_overlay_new_with_symbols_auto.test.dtb overlay_overlay_dtc.dts
> >> + run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__symbols__"
> >> + run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__fixups__"
> >> + run_test check_path overlay_overlay_new_with_symbols_auto.test.dtb exists "/__local_fixups__"
> >> +
> >> + # Test suppression of fixups
> >> + run_dtc_test -F -@ -I dts -O dtb -o overlay_base_with_symbols_no_fixups.test.dtb overlay_base.dts
> >> + run_test check_path overlay_base_with_symbols_no_fixups.test.dtb exists "/__symbols__"
> >> + run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__fixups__"
> >> + run_test check_path overlay_base_with_symbols_no_fixups.test.dtb not-exists "/__local_fixups__"
> >> +
> >> + # Test generation of aliases insted of symbols
> >> + run_dtc_test -A -I dts -O dtb -o overlay_overlay_with_aliases.dtb overlay_overlay_dtc.dts
> >> + run_test check_path overlay_overlay_with_aliases.dtb exists "/aliases"
> >> + run_test check_path overlay_overlay_with_aliases.dtb exists "/__symbols__"
> >> + run_test check_path overlay_overlay_with_aliases.dtb exists "/__fixups__"
> >> + run_test check_path overlay_overlay_with_aliases.dtb exists "/__local_fixups__"
> >> fi
> >>
> >> # Bad fixup tests
> >
>
> Regards
>
> — Pantelis
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
