Re: [PATCH 23/32] test: cmd: fdt: Test fdt set

2023-03-06 Thread Simon Glass
On Wed, 1 Mar 2023 at 20:07, Marek Vasut  wrote:
>
> On 3/1/23 16:02, Simon Glass wrote:
> > Hi Marek,
> >
> > On Mon, 27 Feb 2023 at 12:55, Marek Vasut
> >  wrote:
> >>
> >> Add 'fdt set' test which works as follows:
> >> - Create fuller FDT, map it to sysmem
> >> - Set either existing property to overwrite it, or new property
> >> - Test setting both single properties as well as string and integer arrays
> >> - Test setting to non-existent nodes and aliases
> >> - Verify set values using 'fdt get value'
> >>
> >> The test case can be triggered using:
> >> "
> >> ./u-boot -Dc 'ut fdt'
> >> "
> >> To dump the full output from commands used during test, add '-v' flag.
> >>
> >> Signed-off-by: Marek Vasut 
> >> ---
> >> Cc: Heinrich Schuchardt 
> >> Cc: Simon Glass 
> >> Cc: Tom Rini 
> >> ---
> >>   test/cmd/fdt.c | 123 +
> >>   1 file changed, 123 insertions(+)
> >>
> >> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
> >> index ae67b468b71..42d067090aa 100644
> >> --- a/test/cmd/fdt.c
> >> +++ b/test/cmd/fdt.c
> >> @@ -777,6 +777,129 @@ static int fdt_test_get_size(struct unit_test_state 
> >> *uts)
> >>   }
> >>   FDT_TEST(fdt_test_get_size, UT_TESTF_CONSOLE_REC);
> >>
> >> +static int fdt_test_set_single(struct unit_test_state *uts,
> >> +  const char *path, const char *prop,
> >> +  const char *sval, int ival, bool integer)
> >
> > Please  add a comment for this function.
> >
> >> +{
> >> +   ut_assertok(console_record_reset_enable());
> >> +   if (sval) {
> >> +   ut_assertok(run_commandf("fdt set %s %s %s", path, prop, 
> >> sval));
> >> +   } else if (integer) {
> >> +   ut_assertok(run_commandf("fdt set %s %s <%d>", path, prop, 
> >> ival));
> >> +   } else {
> >> +   ut_assertok(run_commandf("fdt set %s %s", path, prop));
> >> +   }
> >
> > Should drop {} on single-line statements - please check patman
>
> This one isn't as simple as "drop the {}" in fact, I sent a separate
> series to address that:
>
> https://patchwork.ozlabs.org/project/uboot/list/?series=344329

Oh yes, I hit that a while back.

Reviewed-by: Simon Glass 


Re: [PATCH 23/32] test: cmd: fdt: Test fdt set

2023-03-01 Thread Marek Vasut

On 3/1/23 16:02, Simon Glass wrote:

Hi Marek,

On Mon, 27 Feb 2023 at 12:55, Marek Vasut
 wrote:


Add 'fdt set' test which works as follows:
- Create fuller FDT, map it to sysmem
- Set either existing property to overwrite it, or new property
- Test setting both single properties as well as string and integer arrays
- Test setting to non-existent nodes and aliases
- Verify set values using 'fdt get value'

The test case can be triggered using:
"
./u-boot -Dc 'ut fdt'
"
To dump the full output from commands used during test, add '-v' flag.

Signed-off-by: Marek Vasut 
---
Cc: Heinrich Schuchardt 
Cc: Simon Glass 
Cc: Tom Rini 
---
  test/cmd/fdt.c | 123 +
  1 file changed, 123 insertions(+)

diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
index ae67b468b71..42d067090aa 100644
--- a/test/cmd/fdt.c
+++ b/test/cmd/fdt.c
@@ -777,6 +777,129 @@ static int fdt_test_get_size(struct unit_test_state *uts)
  }
  FDT_TEST(fdt_test_get_size, UT_TESTF_CONSOLE_REC);

+static int fdt_test_set_single(struct unit_test_state *uts,
+  const char *path, const char *prop,
+  const char *sval, int ival, bool integer)


Please  add a comment for this function.


+{
+   ut_assertok(console_record_reset_enable());
+   if (sval) {
+   ut_assertok(run_commandf("fdt set %s %s %s", path, prop, sval));
+   } else if (integer) {
+   ut_assertok(run_commandf("fdt set %s %s <%d>", path, prop, 
ival));
+   } else {
+   ut_assertok(run_commandf("fdt set %s %s", path, prop));
+   }


Should drop {} on single-line statements - please check patman


This one isn't as simple as "drop the {}" in fact, I sent a separate 
series to address that:


https://patchwork.ozlabs.org/project/uboot/list/?series=344329


Re: [PATCH 23/32] test: cmd: fdt: Test fdt set

2023-03-01 Thread Simon Glass
Hi Marek,

On Mon, 27 Feb 2023 at 12:55, Marek Vasut
 wrote:
>
> Add 'fdt set' test which works as follows:
> - Create fuller FDT, map it to sysmem
> - Set either existing property to overwrite it, or new property
> - Test setting both single properties as well as string and integer arrays
> - Test setting to non-existent nodes and aliases
> - Verify set values using 'fdt get value'
>
> The test case can be triggered using:
> "
> ./u-boot -Dc 'ut fdt'
> "
> To dump the full output from commands used during test, add '-v' flag.
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: Heinrich Schuchardt 
> Cc: Simon Glass 
> Cc: Tom Rini 
> ---
>  test/cmd/fdt.c | 123 +
>  1 file changed, 123 insertions(+)
>
> diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
> index ae67b468b71..42d067090aa 100644
> --- a/test/cmd/fdt.c
> +++ b/test/cmd/fdt.c
> @@ -777,6 +777,129 @@ static int fdt_test_get_size(struct unit_test_state 
> *uts)
>  }
>  FDT_TEST(fdt_test_get_size, UT_TESTF_CONSOLE_REC);
>
> +static int fdt_test_set_single(struct unit_test_state *uts,
> +  const char *path, const char *prop,
> +  const char *sval, int ival, bool integer)

Please  add a comment for this function.

> +{
> +   ut_assertok(console_record_reset_enable());
> +   if (sval) {
> +   ut_assertok(run_commandf("fdt set %s %s %s", path, prop, 
> sval));
> +   } else if (integer) {
> +   ut_assertok(run_commandf("fdt set %s %s <%d>", path, prop, 
> ival));
> +   } else {
> +   ut_assertok(run_commandf("fdt set %s %s", path, prop));
> +   }

Should drop {} on single-line statements - please check patman

> +
> +   ut_assertok(run_commandf("fdt get value svar %s %s", path, prop));
> +   if (sval) {
> +   ut_asserteq_str(sval, env_get("svar"));
> +   } else if (integer) {
> +   ut_asserteq(ival, env_get_hex("svar", 0x1234));
> +   } else {
> +   ut_assertnull(env_get("svar"));
> +   }
> +   ut_assertok(ut_check_console_end(uts));
> +
> +   return 0;
> +}
> +
> +static int fdt_test_set_multi(struct unit_test_state *uts,
> + const char *path, const char *prop,
> + const char *sval1, const char *sval2,
> + int ival1, int ival2)

and this could use a comment too

> +{
> +   ut_assertok(console_record_reset_enable());
> +   if (sval1 && sval2) {
> +   ut_assertok(run_commandf("fdt set %s %s %s %s end", path, 
> prop, sval1, sval2));
> +   ut_assertok(run_commandf("fdt set %s %s %s %s", path, prop, 
> sval1, sval2));
> +   } else {
> +   ut_assertok(run_commandf("fdt set %s %s <%d %d 10>", path, 
> prop, ival1, ival2));
> +   ut_assertok(run_commandf("fdt set %s %s <%d %d>", path, prop, 
> ival1, ival2));
> +   }
> +

[..]

Regards,
Simon


[PATCH 23/32] test: cmd: fdt: Test fdt set

2023-02-27 Thread Marek Vasut
Add 'fdt set' test which works as follows:
- Create fuller FDT, map it to sysmem
- Set either existing property to overwrite it, or new property
- Test setting both single properties as well as string and integer arrays
- Test setting to non-existent nodes and aliases
- Verify set values using 'fdt get value'

The test case can be triggered using:
"
./u-boot -Dc 'ut fdt'
"
To dump the full output from commands used during test, add '-v' flag.

Signed-off-by: Marek Vasut 
---
Cc: Heinrich Schuchardt 
Cc: Simon Glass 
Cc: Tom Rini 
---
 test/cmd/fdt.c | 123 +
 1 file changed, 123 insertions(+)

diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
index ae67b468b71..42d067090aa 100644
--- a/test/cmd/fdt.c
+++ b/test/cmd/fdt.c
@@ -777,6 +777,129 @@ static int fdt_test_get_size(struct unit_test_state *uts)
 }
 FDT_TEST(fdt_test_get_size, UT_TESTF_CONSOLE_REC);
 
+static int fdt_test_set_single(struct unit_test_state *uts,
+  const char *path, const char *prop,
+  const char *sval, int ival, bool integer)
+{
+   ut_assertok(console_record_reset_enable());
+   if (sval) {
+   ut_assertok(run_commandf("fdt set %s %s %s", path, prop, sval));
+   } else if (integer) {
+   ut_assertok(run_commandf("fdt set %s %s <%d>", path, prop, 
ival));
+   } else {
+   ut_assertok(run_commandf("fdt set %s %s", path, prop));
+   }
+
+   ut_assertok(run_commandf("fdt get value svar %s %s", path, prop));
+   if (sval) {
+   ut_asserteq_str(sval, env_get("svar"));
+   } else if (integer) {
+   ut_asserteq(ival, env_get_hex("svar", 0x1234));
+   } else {
+   ut_assertnull(env_get("svar"));
+   }
+   ut_assertok(ut_check_console_end(uts));
+
+   return 0;
+}
+
+static int fdt_test_set_multi(struct unit_test_state *uts,
+ const char *path, const char *prop,
+ const char *sval1, const char *sval2,
+ int ival1, int ival2)
+{
+   ut_assertok(console_record_reset_enable());
+   if (sval1 && sval2) {
+   ut_assertok(run_commandf("fdt set %s %s %s %s end", path, prop, 
sval1, sval2));
+   ut_assertok(run_commandf("fdt set %s %s %s %s", path, prop, 
sval1, sval2));
+   } else {
+   ut_assertok(run_commandf("fdt set %s %s <%d %d 10>", path, 
prop, ival1, ival2));
+   ut_assertok(run_commandf("fdt set %s %s <%d %d>", path, prop, 
ival1, ival2));
+   }
+
+   /*
+* The "end/10" above and "svarn" below is used to validate that
+* previous 'fdt set' to longer array does not polute newly set
+* shorter array.
+*/
+   ut_assertok(run_commandf("fdt get value svar1 %s %s 0", path, prop));
+   ut_assertok(run_commandf("fdt get value svar2 %s %s 1", path, prop));
+   ut_asserteq(1, run_commandf("fdt get value svarn %s %s 2", path, prop));
+   if (sval1 && sval2) {
+   ut_asserteq_str(sval1, env_get("svar1"));
+   ut_asserteq_str(sval2, env_get("svar2"));
+   ut_assertnull(env_get("svarn"));
+   } else {
+   ut_asserteq(ival1, env_get_hex("svar1", 0x1234));
+   ut_asserteq(ival2, env_get_hex("svar2", 0x1234));
+   ut_assertnull(env_get("svarn"));
+   }
+   ut_assertok(ut_check_console_end(uts));
+
+   return 0;
+}
+
+static int fdt_test_set_node(struct unit_test_state *uts,
+const char *path, const char *prop)
+{
+   fdt_test_set_single(uts, path, prop, "new", 0, false);
+   fdt_test_set_single(uts, path, prop, "rewrite", 0, false);
+   fdt_test_set_single(uts, path, prop, NULL, 42, true);
+   fdt_test_set_single(uts, path, prop, NULL, 0, false);
+   fdt_test_set_multi(uts, path, prop, NULL, NULL, 42, 1701);
+   fdt_test_set_multi(uts, path, prop, NULL, NULL, 74656, 9);
+   fdt_test_set_multi(uts, path, prop, "42", "1701", 0, 0);
+   fdt_test_set_multi(uts, path, prop, "74656", "9", 0, 0);
+
+   return 0;
+}
+
+static int fdt_test_set(struct unit_test_state *uts)
+{
+   char fdt[8192];
+   ulong addr;
+
+   ut_assertok(make_fuller_fdt(uts, fdt, sizeof(fdt)));
+   fdt_shrink_to_minimum(fdt, 4096);   /* Resize with 4096 extra bytes 
*/
+   addr = map_to_sysmem(fdt);
+   set_working_fdt_addr(addr);
+
+   /* Test setting of root node / existing property "compatible" */
+   fdt_test_set_node(uts, "/", "compatible");
+
+   /* Test setting of root node / new property "newproperty" */
+   fdt_test_set_node(uts, "/", "newproperty");
+
+   /* Test setting of subnode existing property "compatible" */
+   fdt_test_set_node(uts, "/test-node@1234/subnode", "compatible");
+   fdt_test_set_node(uts, "subnodealias", "compatible");
+
+   /* Test setting of