[PATCH 0/3] Fix-up series for x86/master

[PATCH 0/3] Fix-up series for x86/master

[Simon Glass]

Note that this includes two patches which are in mainline but not in
x86/master


Simon Glass (3):
  test: Add the beginnings of some string tests
  lib: Add a function to convert a string to upper case
  acpi: Fix-up patch to correct sandbox test errors

 arch/x86/lib/acpi_table.c |   5 --
 include/test/suites.h     |   1 +
 include/vsprintf.h        |  12 ++++
 lib/acpi/acpi_table.c     |   7 ++-
 lib/strto.c               |   8 +++
 test/Makefile             |   1 +
 test/cmd_ut.c             |   5 ++
 test/str_ut.c             | 115 ++++++++++++++++++++++++++++++++++++++
 8 files changed, 148 insertions(+), 6 deletions(-)
 create mode 100644 test/str_ut.c

-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH 1/3] test: Add the beginnings of some string tests

[Simon Glass]

There are quite a few string functions in U-Boot with no tests. Make a
start by adding a test for strtoul().

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/test/suites.h |  1 +
 test/Makefile         |  1 +
 test/cmd_ut.c         |  5 ++++
 test/str_ut.c         | 67 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 74 insertions(+)
 create mode 100644 test/str_ut.c

diff --git a/include/test/suites.h b/include/test/suites.h
index 39ad81a90f..213e3cee77 100644
--- a/include/test/suites.h
+++ b/include/test/suites.h
@@ -33,6 +33,7 @@ int do_ut_lib(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 int do_ut_log(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 int do_ut_optee(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 int do_ut_overlay(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
+int do_ut_str(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_time(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 int do_ut_unicode(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 
diff --git a/test/Makefile b/test/Makefile
index 2971d0d87f..bab8f1a5c2 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_UNIT_TEST) += ut.o
 obj-$(CONFIG_SANDBOX) += command_ut.o
 obj-$(CONFIG_SANDBOX) += compression.o
 obj-$(CONFIG_SANDBOX) += print_ut.o
+obj-$(CONFIG_SANDBOX) += str_ut.o
 obj-$(CONFIG_UT_TIME) += time_ut.o
 obj-$(CONFIG_UT_UNICODE) += unicode_ut.o
 obj-y += log/
diff --git a/test/cmd_ut.c b/test/cmd_ut.c
index 7fdcdbb1a6..bd20a69c55 100644
--- a/test/cmd_ut.c
+++ b/test/cmd_ut.c
@@ -74,6 +74,8 @@ static cmd_tbl_t cmd_ut_sub[] = {
 			 "", ""),
 	U_BOOT_CMD_MKENT(bloblist, CONFIG_SYS_MAXARGS, 1, do_ut_bloblist,
 			 "", ""),
+	U_BOOT_CMD_MKENT(str, CONFIG_SYS_MAXARGS, 1, do_ut_str,
+			 "", ""),
 #endif
 };
 
@@ -137,6 +139,9 @@ static char ut_help_text[] =
 #ifdef CONFIG_UT_OVERLAY
 	"ut overlay [test-name]\n"
 #endif
+#ifdef CONFIG_SANDBOX
+	"ut str - Basic test of string functions\n"
+#endif
 #ifdef CONFIG_UT_TIME
 	"ut time - Very basic test of time functions\n"
 #endif
diff --git a/test/str_ut.c b/test/str_ut.c
new file mode 100644
index 0000000000..fab8de595c
--- /dev/null
+++ b/test/str_ut.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2020 Google LLC
+ */
+
+#include <common.h>
+#include <vsprintf.h>
+#include <test/suites.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+/* This is large enough for any of the test strings */
+#define TEST_STR_SIZE	200
+
+static const char str1[] = "I'm sorry I'm late.";
+static const char str2[] = "1099abNo, don't bother apologising.";
+static const char str3[] = "0xbI'm sorry you're alive.";
+
+/* Declare a new str test */
+#define STR_TEST(_name, _flags)		UNIT_TEST(_name, _flags, str_test)
+
+static int run_strtoul(struct unit_test_state *uts, const char *str, int base,
+		       ulong expect_val, int expect_endp_offset)
+{
+	char *endp;
+	ulong val;
+
+	val = simple_strtoul(str, &endp, base);
+	ut_asserteq(expect_val, val);
+	ut_asserteq(expect_endp_offset, endp - str);
+
+	return 0;
+}
+
+static int str_simple_strtoul(struct unit_test_state *uts)
+{
+	/* Base 10 and base 16 */
+	ut_assertok(run_strtoul(uts, str2, 10, 1099, 4));
+	ut_assertok(run_strtoul(uts, str2, 16, 0x1099ab, 6));
+
+	/* Invalid string */
+	ut_assertok(run_strtoul(uts, str1, 10, 0, 0));
+
+	/* Base 0 */
+	ut_assertok(run_strtoul(uts, str1, 0, 0, 0));
+	ut_assertok(run_strtoul(uts, str2, 0, 1099, 4));
+	ut_assertok(run_strtoul(uts, str3, 0, 0xb, 3));
+
+	/* Base 2 */
+	ut_assertok(run_strtoul(uts, str1, 2, 0, 0));
+	ut_assertok(run_strtoul(uts, str2, 2, 2, 2));
+
+	/* Check endp being NULL */
+	ut_asserteq(1099, simple_strtoul(str2, NULL, 0));
+
+	return 0;
+}
+STR_TEST(str_simple_strtoul, 0);
+
+int do_ut_str(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
+{
+	struct unit_test *tests = ll_entry_start(struct unit_test,
+						 str_test);
+	const int n_ents = ll_entry_count(struct unit_test, str_test);
+
+	return cmd_ut_category("str", "str_", tests, n_ents, argc, argv);
+}
-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH 2/3] lib: Add a function to convert a string to upper case

[Simon Glass]

Add a helper function for this operation. Update the strtoul() tests to
check upper case as well.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---

 include/vsprintf.h | 12 +++++++
 lib/strto.c        |  8 +++++
 test/str_ut.c      | 78 +++++++++++++++++++++++++++++++++++++---------
 3 files changed, 83 insertions(+), 15 deletions(-)

diff --git a/include/vsprintf.h b/include/vsprintf.h
index 56844dd2de..d9fb68add0 100644
--- a/include/vsprintf.h
+++ b/include/vsprintf.h
@@ -222,4 +222,16 @@ bool str2long(const char *p, ulong *num);
  * @hz: Value to convert
  */
 char *strmhz(char *buf, unsigned long hz);
+
+/**
+ * str_to_upper() - Convert a string to upper case
+ *
+ * This simply uses toupper() on each character of the string.
+ *
+ * @in: String to convert (must be large enough to hold the output string)
+ * @out: Buffer to put converted string
+ * @len: Number of bytes available in @out (SIZE_MAX for all)
+ */
+void str_to_upper(const char *in, char *out, size_t len);
+
 #endif
diff --git a/lib/strto.c b/lib/strto.c
index 1ac2b09c72..37e1fbe63f 100644
--- a/lib/strto.c
+++ b/lib/strto.c
@@ -176,3 +176,11 @@ long trailing_strtol(const char *str)
 {
 	return trailing_strtoln(str, NULL);
 }
+
+void str_to_upper(const char *in, char *out, size_t len)
+{
+	for (; len > 0 && *in; len--)
+		*out++ = toupper(*in++);
+	if (len)
+		*out = '\0';
+}
diff --git a/test/str_ut.c b/test/str_ut.c
index fab8de595c..7c8015050a 100644
--- a/test/str_ut.c
+++ b/test/str_ut.c
@@ -19,36 +19,84 @@ static const char str3[] = "0xbI'm sorry you're alive.";
 /* Declare a new str test */
 #define STR_TEST(_name, _flags)		UNIT_TEST(_name, _flags, str_test)
 
+static int str_test_upper(struct unit_test_state *uts)
+{
+	char out[TEST_STR_SIZE];
+
+	/* Make sure it adds a terminator */
+	out[strlen(str1)] = 'a';
+	str_to_upper(str1, out, SIZE_MAX);
+	ut_asserteq_str("I'M SORRY I'M LATE.", out);
+
+	/* In-place operation */
+	strcpy(out, str2);
+	str_to_upper(out, out, SIZE_MAX);
+	ut_asserteq_str("1099ABNO, DON'T BOTHER APOLOGISING.", out);
+
+	/* Limited length */
+	str_to_upper(str1, out, 7);
+	ut_asserteq_str("I'M SORO, DON'T BOTHER APOLOGISING.", out);
+
+	/* In-place with limited length */
+	strcpy(out, str2);
+	str_to_upper(out, out, 7);
+	ut_asserteq_str("1099ABNo, don't bother apologising.", out);
+
+	/* Copy an empty string to a buffer with space*/
+	out[1] = 0x7f;
+	str_to_upper("", out, SIZE_MAX);
+	ut_asserteq('\0', *out);
+	ut_asserteq(0x7f, out[1]);
+
+	/* Copy an empty string to a buffer with no space*/
+	out[0] = 0x7f;
+	str_to_upper("", out, 0);
+	ut_asserteq(0x7f, out[0]);
+
+	return 0;
+}
+STR_TEST(str_test_upper, 0);
+
 static int run_strtoul(struct unit_test_state *uts, const char *str, int base,
-		       ulong expect_val, int expect_endp_offset)
+		       ulong expect_val, int expect_endp_offset, bool upper)
 {
+	char out[TEST_STR_SIZE];
 	char *endp;
 	ulong val;
 
-	val = simple_strtoul(str, &endp, base);
+	strcpy(out, str);
+	if (upper)
+		str_to_upper(out, out, -1);
+
+	val = simple_strtoul(out, &endp, base);
 	ut_asserteq(expect_val, val);
-	ut_asserteq(expect_endp_offset, endp - str);
+	ut_asserteq(expect_endp_offset, endp - out);
 
 	return 0;
 }
 
 static int str_simple_strtoul(struct unit_test_state *uts)
 {
-	/* Base 10 and base 16 */
-	ut_assertok(run_strtoul(uts, str2, 10, 1099, 4));
-	ut_assertok(run_strtoul(uts, str2, 16, 0x1099ab, 6));
+	int upper;
+
+	/* Check that it is case-insentive */
+	for (upper = 0; upper < 2; upper++) {
+		/* Base 10 and base 16 */
+		ut_assertok(run_strtoul(uts, str2, 10, 1099, 4, upper));
+		ut_assertok(run_strtoul(uts, str2, 16, 0x1099ab, 6, upper));
 
-	/* Invalid string */
-	ut_assertok(run_strtoul(uts, str1, 10, 0, 0));
+		/* Invalid string */
+		ut_assertok(run_strtoul(uts, str1, 10, 0, 0, upper));
 
-	/* Base 0 */
-	ut_assertok(run_strtoul(uts, str1, 0, 0, 0));
-	ut_assertok(run_strtoul(uts, str2, 0, 1099, 4));
-	ut_assertok(run_strtoul(uts, str3, 0, 0xb, 3));
+		/* Base 0 */
+		ut_assertok(run_strtoul(uts, str1, 0, 0, 0, upper));
+		ut_assertok(run_strtoul(uts, str2, 0, 1099, 4, upper));
+		ut_assertok(run_strtoul(uts, str3, 0, 0xb, 3, upper));
 
-	/* Base 2 */
-	ut_assertok(run_strtoul(uts, str1, 2, 0, 0));
-	ut_assertok(run_strtoul(uts, str2, 2, 2, 2));
+		/* Base 2 */
+		ut_assertok(run_strtoul(uts, str1, 2, 0, 0, upper));
+		ut_assertok(run_strtoul(uts, str2, 2, 2, 2, upper));
+	}
 
 	/* Check endp being NULL */
 	ut_asserteq(1099, simple_strtoul(str2, NULL, 0));
-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH 3/3] acpi: Fix-up patch to correct sandbox test errors

[Simon Glass]

Move the alignment code into acpi_setup_base_tables() so that test and
production code are in alignment.

This brings x86/master into line with patch series v8.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/x86/lib/acpi_table.c | 5 -----
 lib/acpi/acpi_table.c     | 7 ++++++-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
index 600bde2f5f..13f1409de8 100644
--- a/arch/x86/lib/acpi_table.c
+++ b/arch/x86/lib/acpi_table.c
@@ -375,11 +375,6 @@ ulong write_acpi_tables(ulong start_addr)
 	debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
 
 	acpi_setup_base_tables(ctx, start);
-	/*
-	 * Per ACPI spec, the FACS table address must be aligned to a 64 byte
-	 * boundary (Windows checks this, but Linux does not).
-	 */
-	acpi_align64(ctx);
 
 	debug("ACPI:    * FACS\n");
 	facs = ctx->current;
diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
index 5abf1cad50..1c253af3bf 100644
--- a/lib/acpi/acpi_table.c
+++ b/lib/acpi/acpi_table.c
@@ -145,7 +145,7 @@ int acpi_add_table(struct acpi_ctx *ctx, void *table)
 	}
 
 	if (i >= entries_num) {
-		debug("ACPI: Error: too many tables\n");
+		log_err("ACPI: Error: too many tables\n");
 		return -E2BIG;
 	}
 
@@ -256,4 +256,9 @@ void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
 	acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
 	acpi_write_rsdt(ctx->rsdt);
 	acpi_write_xsdt(ctx->xsdt);
+	/*
+	 * Per ACPI spec, the FACS table address must be aligned to a 64 byte
+	 * boundary (Windows checks this, but Linux does not).
+	 */
+	acpi_align64(ctx);
 }
-- 
2.26.2.303.gf8c07b1a785-goog

[PATCH 3/3] acpi: Fix-up patch to correct sandbox test errors

[Bin Meng]

Hi Simon,

On Tue, Apr 28, 2020 at 1:02 AM Simon Glass <sjg@chromium.org> wrote:
>
> Move the alignment code into acpi_setup_base_tables() so that test and
> production code are in alignment.
>
> This brings x86/master into line with patch series v8.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  arch/x86/lib/acpi_table.c | 5 -----
>  lib/acpi/acpi_table.c     | 7 ++++++-
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> index 600bde2f5f..13f1409de8 100644
> --- a/arch/x86/lib/acpi_table.c
> +++ b/arch/x86/lib/acpi_table.c
> @@ -375,11 +375,6 @@ ulong write_acpi_tables(ulong start_addr)
>         debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
>
>         acpi_setup_base_tables(ctx, start);
> -       /*
> -        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> -        * boundary (Windows checks this, but Linux does not).
> -        */
> -       acpi_align64(ctx);
>
>         debug("ACPI:    * FACS\n");
>         facs = ctx->current;
> diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> index 5abf1cad50..1c253af3bf 100644
> --- a/lib/acpi/acpi_table.c
> +++ b/lib/acpi/acpi_table.c
> @@ -145,7 +145,7 @@ int acpi_add_table(struct acpi_ctx *ctx, void *table)
>         }
>
>         if (i >= entries_num) {
> -               debug("ACPI: Error: too many tables\n");
> +               log_err("ACPI: Error: too many tables\n");
>                 return -E2BIG;
>         }
>
> @@ -256,4 +256,9 @@ void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
>         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
>         acpi_write_rsdt(ctx->rsdt);
>         acpi_write_xsdt(ctx->xsdt);
> +       /*
> +        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> +        * boundary (Windows checks this, but Linux does not).
> +        */
> +       acpi_align64(ctx);
>  }
> --

Could you please point out which commit in u-boot-x86/master should
squash in this patch to fix the build error on sandbox?

Regards,
Bin

[PATCH 3/3] acpi: Fix-up patch to correct sandbox test errors

[Simon Glass]

Hi Bin,

On Mon, 27 Apr 2020 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Tue, Apr 28, 2020 at 1:02 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Move the alignment code into acpi_setup_base_tables() so that test and
> > production code are in alignment.
> >
> > This brings x86/master into line with patch series v8.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  arch/x86/lib/acpi_table.c | 5 -----
> >  lib/acpi/acpi_table.c     | 7 ++++++-
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> > index 600bde2f5f..13f1409de8 100644
> > --- a/arch/x86/lib/acpi_table.c
> > +++ b/arch/x86/lib/acpi_table.c
> > @@ -375,11 +375,6 @@ ulong write_acpi_tables(ulong start_addr)
> >         debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
> >
> >         acpi_setup_base_tables(ctx, start);
> > -       /*
> > -        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > -        * boundary (Windows checks this, but Linux does not).
> > -        */
> > -       acpi_align64(ctx);
> >
> >         debug("ACPI:    * FACS\n");
> >         facs = ctx->current;
> > diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> > index 5abf1cad50..1c253af3bf 100644
> > --- a/lib/acpi/acpi_table.c
> > +++ b/lib/acpi/acpi_table.c
> > @@ -145,7 +145,7 @@ int acpi_add_table(struct acpi_ctx *ctx, void *table)
> >         }
> >
> >         if (i >= entries_num) {
> > -               debug("ACPI: Error: too many tables\n");
> > +               log_err("ACPI: Error: too many tables\n");
> >                 return -E2BIG;
> >         }
> >
> > @@ -256,4 +256,9 @@ void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
> >         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
> >         acpi_write_rsdt(ctx->rsdt);
> >         acpi_write_xsdt(ctx->xsdt);
> > +       /*
> > +        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > +        * boundary (Windows checks this, but Linux does not).
> > +        */
> > +       acpi_align64(ctx);
> >  }
> > --
>
> Could you please point out which commit in u-boot-x86/master should
> squash in this patch to fix the build error on sandbox?

It might be easier to pick up v8 in that case. I think there are three
patches that need to change because of conflicts caused by the first
one.

So can you pick up the v8 patches? Also you do need to rebase on
master because of the str_to_upper patches.

Regards,
Simon

[PATCH 3/3] acpi: Fix-up patch to correct sandbox test errors

[Bin Meng]

Hi Simon,

On Tue, Apr 28, 2020 at 10:29 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Mon, 27 Apr 2020 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Tue, Apr 28, 2020 at 1:02 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Move the alignment code into acpi_setup_base_tables() so that test and
> > > production code are in alignment.
> > >
> > > This brings x86/master into line with patch series v8.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > >  arch/x86/lib/acpi_table.c | 5 -----
> > >  lib/acpi/acpi_table.c     | 7 ++++++-
> > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> > > index 600bde2f5f..13f1409de8 100644
> > > --- a/arch/x86/lib/acpi_table.c
> > > +++ b/arch/x86/lib/acpi_table.c
> > > @@ -375,11 +375,6 @@ ulong write_acpi_tables(ulong start_addr)
> > >         debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
> > >
> > >         acpi_setup_base_tables(ctx, start);
> > > -       /*
> > > -        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > -        * boundary (Windows checks this, but Linux does not).
> > > -        */
> > > -       acpi_align64(ctx);
> > >
> > >         debug("ACPI:    * FACS\n");
> > >         facs = ctx->current;
> > > diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> > > index 5abf1cad50..1c253af3bf 100644
> > > --- a/lib/acpi/acpi_table.c
> > > +++ b/lib/acpi/acpi_table.c
> > > @@ -145,7 +145,7 @@ int acpi_add_table(struct acpi_ctx *ctx, void *table)
> > >         }
> > >
> > >         if (i >= entries_num) {
> > > -               debug("ACPI: Error: too many tables\n");
> > > +               log_err("ACPI: Error: too many tables\n");
> > >                 return -E2BIG;
> > >         }
> > >
> > > @@ -256,4 +256,9 @@ void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
> > >         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
> > >         acpi_write_rsdt(ctx->rsdt);
> > >         acpi_write_xsdt(ctx->xsdt);
> > > +       /*
> > > +        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > +        * boundary (Windows checks this, but Linux does not).
> > > +        */
> > > +       acpi_align64(ctx);
> > >  }
> > > --
> >
> > Could you please point out which commit in u-boot-x86/master should
> > squash in this patch to fix the build error on sandbox?
>
> It might be easier to pick up v8 in that case. I think there are three
> patches that need to change because of conflicts caused by the first
> one.
>
> So can you pick up the v8 patches? Also you do need to rebase on
> master because of the str_to_upper patches.

But v8 is a complete new series for part B? I think we'd better re-tag
v8 as v1. It's quite confusing.

Regards,
Bin

[PATCH 3/3] acpi: Fix-up patch to correct sandbox test errors

[Simon Glass]

Hi Bin,

On Mon, 27 Apr 2020 at 20:42, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Tue, Apr 28, 2020 at 10:29 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Mon, 27 Apr 2020 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Tue, Apr 28, 2020 at 1:02 AM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Move the alignment code into acpi_setup_base_tables() so that test and
> > > > production code are in alignment.
> > > >
> > > > This brings x86/master into line with patch series v8.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > >
> > > >  arch/x86/lib/acpi_table.c | 5 -----
> > > >  lib/acpi/acpi_table.c     | 7 ++++++-
> > > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> > > > index 600bde2f5f..13f1409de8 100644
> > > > --- a/arch/x86/lib/acpi_table.c
> > > > +++ b/arch/x86/lib/acpi_table.c
> > > > @@ -375,11 +375,6 @@ ulong write_acpi_tables(ulong start_addr)
> > > >         debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
> > > >
> > > >         acpi_setup_base_tables(ctx, start);
> > > > -       /*
> > > > -        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > -        * boundary (Windows checks this, but Linux does not).
> > > > -        */
> > > > -       acpi_align64(ctx);
> > > >
> > > >         debug("ACPI:    * FACS\n");
> > > >         facs = ctx->current;
> > > > diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> > > > index 5abf1cad50..1c253af3bf 100644
> > > > --- a/lib/acpi/acpi_table.c
> > > > +++ b/lib/acpi/acpi_table.c
> > > > @@ -145,7 +145,7 @@ int acpi_add_table(struct acpi_ctx *ctx, void *table)
> > > >         }
> > > >
> > > >         if (i >= entries_num) {
> > > > -               debug("ACPI: Error: too many tables\n");
> > > > +               log_err("ACPI: Error: too many tables\n");
> > > >                 return -E2BIG;
> > > >         }
> > > >
> > > > @@ -256,4 +256,9 @@ void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
> > > >         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
> > > >         acpi_write_rsdt(ctx->rsdt);
> > > >         acpi_write_xsdt(ctx->xsdt);
> > > > +       /*
> > > > +        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > +        * boundary (Windows checks this, but Linux does not).
> > > > +        */
> > > > +       acpi_align64(ctx);
> > > >  }
> > > > --
> > >
> > > Could you please point out which commit in u-boot-x86/master should
> > > squash in this patch to fix the build error on sandbox?
> >
> > It might be easier to pick up v8 in that case. I think there are three
> > patches that need to change because of conflicts caused by the first
> > one.
> >
> > So can you pick up the v8 patches? Also you do need to rebase on
> > master because of the str_to_upper patches.
>
> But v8 is a complete new series for part B? I think we'd better re-tag
> v8 as v1. It's quite confusing.

No I mean the part A series. Let's get that applied and then we will
be in a brave new world.

I could resent part B as v1 if you like, but note that it has some
reviewed-by tags and some v3, etc. comments. The patches languished
for quite a while which is why I decided to try to split them up.

Regards,
Simon

[PATCH 3/3] acpi: Fix-up patch to correct sandbox test errors

[Bin Meng]

Hi Simon,

On Tue, Apr 28, 2020 at 11:06 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Mon, 27 Apr 2020 at 20:42, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Tue, Apr 28, 2020 at 10:29 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Mon, 27 Apr 2020 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Tue, Apr 28, 2020 at 1:02 AM Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Move the alignment code into acpi_setup_base_tables() so that test and
> > > > > production code are in alignment.
> > > > >
> > > > > This brings x86/master into line with patch series v8.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > >
> > > > >  arch/x86/lib/acpi_table.c | 5 -----
> > > > >  lib/acpi/acpi_table.c     | 7 ++++++-
> > > > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> > > > > index 600bde2f5f..13f1409de8 100644
> > > > > --- a/arch/x86/lib/acpi_table.c
> > > > > +++ b/arch/x86/lib/acpi_table.c
> > > > > @@ -375,11 +375,6 @@ ulong write_acpi_tables(ulong start_addr)
> > > > >         debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
> > > > >
> > > > >         acpi_setup_base_tables(ctx, start);
> > > > > -       /*
> > > > > -        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > -        * boundary (Windows checks this, but Linux does not).
> > > > > -        */
> > > > > -       acpi_align64(ctx);
> > > > >
> > > > >         debug("ACPI:    * FACS\n");
> > > > >         facs = ctx->current;
> > > > > diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> > > > > index 5abf1cad50..1c253af3bf 100644
> > > > > --- a/lib/acpi/acpi_table.c
> > > > > +++ b/lib/acpi/acpi_table.c
> > > > > @@ -145,7 +145,7 @@ int acpi_add_table(struct acpi_ctx *ctx, void *table)
> > > > >         }
> > > > >
> > > > >         if (i >= entries_num) {
> > > > > -               debug("ACPI: Error: too many tables\n");
> > > > > +               log_err("ACPI: Error: too many tables\n");
> > > > >                 return -E2BIG;
> > > > >         }
> > > > >
> > > > > @@ -256,4 +256,9 @@ void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
> > > > >         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
> > > > >         acpi_write_rsdt(ctx->rsdt);
> > > > >         acpi_write_xsdt(ctx->xsdt);
> > > > > +       /*
> > > > > +        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > +        * boundary (Windows checks this, but Linux does not).
> > > > > +        */
> > > > > +       acpi_align64(ctx);
> > > > >  }
> > > > > --
> > > >
> > > > Could you please point out which commit in u-boot-x86/master should
> > > > squash in this patch to fix the build error on sandbox?
> > >
> > > It might be easier to pick up v8 in that case. I think there are three
> > > patches that need to change because of conflicts caused by the first
> > > one.
> > >
> > > So can you pick up the v8 patches? Also you do need to rebase on
> > > master because of the str_to_upper patches.
> >
> > But v8 is a complete new series for part B? I think we'd better re-tag
> > v8 as v1. It's quite confusing.
>
> No I mean the part A series. Let's get that applied and then we will
> be in a brave new world.

Is the v8 this one?
http://patchwork.ozlabs.org/user/todo/uboot/?series=172777

The part A series are already applied, but it has a build error as I mentioned.

>
> I could resent part B as v1 if you like, but note that it has some
> reviewed-by tags and some v3, etc. comments. The patches languished
> for quite a while which is why I decided to try to split them up.


Regards,
Bin

[PATCH 3/3] acpi: Fix-up patch to correct sandbox test errors

[Simon Glass]

Hi Bin,

On Mon, 27 Apr 2020 at 21:25, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Tue, Apr 28, 2020 at 11:06 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Mon, 27 Apr 2020 at 20:42, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Tue, Apr 28, 2020 at 10:29 AM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Mon, 27 Apr 2020 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Tue, Apr 28, 2020 at 1:02 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Move the alignment code into acpi_setup_base_tables() so that test and
> > > > > > production code are in alignment.
> > > > > >
> > > > > > This brings x86/master into line with patch series v8.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > ---
> > > > > >
> > > > > >  arch/x86/lib/acpi_table.c | 5 -----
> > > > > >  lib/acpi/acpi_table.c     | 7 ++++++-
> > > > > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > > > >
> > > > > > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> > > > > > index 600bde2f5f..13f1409de8 100644
> > > > > > --- a/arch/x86/lib/acpi_table.c
> > > > > > +++ b/arch/x86/lib/acpi_table.c
> > > > > > @@ -375,11 +375,6 @@ ulong write_acpi_tables(ulong start_addr)
> > > > > >         debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
> > > > > >
> > > > > >         acpi_setup_base_tables(ctx, start);
> > > > > > -       /*
> > > > > > -        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > -        * boundary (Windows checks this, but Linux does not).
> > > > > > -        */
> > > > > > -       acpi_align64(ctx);
> > > > > >
> > > > > >         debug("ACPI:    * FACS\n");
> > > > > >         facs = ctx->current;
> > > > > > diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> > > > > > index 5abf1cad50..1c253af3bf 100644
> > > > > > --- a/lib/acpi/acpi_table.c
> > > > > > +++ b/lib/acpi/acpi_table.c
> > > > > > @@ -145,7 +145,7 @@ int acpi_add_table(struct acpi_ctx *ctx, void *table)
> > > > > >         }
> > > > > >
> > > > > >         if (i >= entries_num) {
> > > > > > -               debug("ACPI: Error: too many tables\n");
> > > > > > +               log_err("ACPI: Error: too many tables\n");
> > > > > >                 return -E2BIG;
> > > > > >         }
> > > > > >
> > > > > > @@ -256,4 +256,9 @@ void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
> > > > > >         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
> > > > > >         acpi_write_rsdt(ctx->rsdt);
> > > > > >         acpi_write_xsdt(ctx->xsdt);
> > > > > > +       /*
> > > > > > +        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > +        * boundary (Windows checks this, but Linux does not).
> > > > > > +        */
> > > > > > +       acpi_align64(ctx);
> > > > > >  }
> > > > > > --
> > > > >
> > > > > Could you please point out which commit in u-boot-x86/master should
> > > > > squash in this patch to fix the build error on sandbox?
> > > >
> > > > It might be easier to pick up v8 in that case. I think there are three
> > > > patches that need to change because of conflicts caused by the first
> > > > one.
> > > >
> > > > So can you pick up the v8 patches? Also you do need to rebase on
> > > > master because of the str_to_upper patches.
> > >
> > > But v8 is a complete new series for part B? I think we'd better re-tag
> > > v8 as v1. It's quite confusing.
> >
> > No I mean the part A series. Let's get that applied and then we will
> > be in a brave new world.
>
> Is the v8 this one?
> http://patchwork.ozlabs.org/user/todo/uboot/?series=172777

No it is this one:

http://patchwork.ozlabs.org/project/uboot/list/?series=172776

>
> The part A series are already applied, but it has a build error as I mentioned.

Right, but the options are to do a fixup patch or to pick up the v8
series instead. Unfortunately there are merge conflicts in patches so
if you just pick up one patch it won't apply cleanly.

>
> >
> > I could resent part B as v1 if you like, but note that it has some
> > reviewed-by tags and some v3, etc. comments. The patches languished
> > for quite a while which is why I decided to try to split them up.
>

Regards,
Simon

[PATCH 3/3] acpi: Fix-up patch to correct sandbox test errors

[Bin Meng]

Hi Simon,

On Tue, Apr 28, 2020 at 10:19 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Mon, 27 Apr 2020 at 21:25, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Tue, Apr 28, 2020 at 11:06 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Mon, 27 Apr 2020 at 20:42, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Tue, Apr 28, 2020 at 10:29 AM Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Mon, 27 Apr 2020 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Tue, Apr 28, 2020 at 1:02 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > >
> > > > > > > Move the alignment code into acpi_setup_base_tables() so that test and
> > > > > > > production code are in alignment.
> > > > > > >
> > > > > > > This brings x86/master into line with patch series v8.
> > > > > > >
> > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > ---
> > > > > > >
> > > > > > >  arch/x86/lib/acpi_table.c | 5 -----
> > > > > > >  lib/acpi/acpi_table.c     | 7 ++++++-
> > > > > > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > > > > >
> > > > > > > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> > > > > > > index 600bde2f5f..13f1409de8 100644
> > > > > > > --- a/arch/x86/lib/acpi_table.c
> > > > > > > +++ b/arch/x86/lib/acpi_table.c
> > > > > > > @@ -375,11 +375,6 @@ ulong write_acpi_tables(ulong start_addr)
> > > > > > >         debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
> > > > > > >
> > > > > > >         acpi_setup_base_tables(ctx, start);
> > > > > > > -       /*
> > > > > > > -        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > > -        * boundary (Windows checks this, but Linux does not).
> > > > > > > -        */
> > > > > > > -       acpi_align64(ctx);
> > > > > > >
> > > > > > >         debug("ACPI:    * FACS\n");
> > > > > > >         facs = ctx->current;
> > > > > > > diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> > > > > > > index 5abf1cad50..1c253af3bf 100644
> > > > > > > --- a/lib/acpi/acpi_table.c
> > > > > > > +++ b/lib/acpi/acpi_table.c
> > > > > > > @@ -145,7 +145,7 @@ int acpi_add_table(struct acpi_ctx *ctx, void *table)
> > > > > > >         }
> > > > > > >
> > > > > > >         if (i >= entries_num) {
> > > > > > > -               debug("ACPI: Error: too many tables\n");
> > > > > > > +               log_err("ACPI: Error: too many tables\n");
> > > > > > >                 return -E2BIG;
> > > > > > >         }
> > > > > > >
> > > > > > > @@ -256,4 +256,9 @@ void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
> > > > > > >         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
> > > > > > >         acpi_write_rsdt(ctx->rsdt);
> > > > > > >         acpi_write_xsdt(ctx->xsdt);
> > > > > > > +       /*
> > > > > > > +        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > > +        * boundary (Windows checks this, but Linux does not).
> > > > > > > +        */
> > > > > > > +       acpi_align64(ctx);
> > > > > > >  }
> > > > > > > --
> > > > > >
> > > > > > Could you please point out which commit in u-boot-x86/master should
> > > > > > squash in this patch to fix the build error on sandbox?
> > > > >
> > > > > It might be easier to pick up v8 in that case. I think there are three
> > > > > patches that need to change because of conflicts caused by the first
> > > > > one.
> > > > >
> > > > > So can you pick up the v8 patches? Also you do need to rebase on
> > > > > master because of the str_to_upper patches.
> > > >
> > > > But v8 is a complete new series for part B? I think we'd better re-tag
> > > > v8 as v1. It's quite confusing.
> > >
> > > No I mean the part A series. Let's get that applied and then we will
> > > be in a brave new world.
> >
> > Is the v8 this one?
> > http://patchwork.ozlabs.org/user/todo/uboot/?series=172777
>
> No it is this one:
>
> http://patchwork.ozlabs.org/project/uboot/list/?series=172776
>
> >
> > The part A series are already applied, but it has a build error as I mentioned.
>
> Right, but the options are to do a fixup patch or to pick up the v8
> series instead. Unfortunately there are merge conflicts in patches so
> if you just pick up one patch it won't apply cleanly.
>

But we still need maintain bisectablity right? To do that we need a
fix patch that can be squashed into the u-boot-x86/master. Picking up
the v8 series does not fix the bisectiablity I think.

> >
> > >
> > > I could resent part B as v1 if you like, but note that it has some
> > > reviewed-by tags and some v3, etc. comments. The patches languished
> > > for quite a while which is why I decided to try to split them up.
> >

Regards,
Bin

[PATCH 3/3] acpi: Fix-up patch to correct sandbox test errors

[Simon Glass]

Hi Bin,

On Tue, 28 Apr 2020 at 08:34, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Tue, Apr 28, 2020 at 10:19 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Mon, 27 Apr 2020 at 21:25, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Tue, Apr 28, 2020 at 11:06 AM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Mon, 27 Apr 2020 at 20:42, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Tue, Apr 28, 2020 at 10:29 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Bin,
> > > > > >
> > > > > > On Mon, 27 Apr 2020 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > >
> > > > > > > Hi Simon,
> > > > > > >
> > > > > > > On Tue, Apr 28, 2020 at 1:02 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > >
> > > > > > > > Move the alignment code into acpi_setup_base_tables() so that test and
> > > > > > > > production code are in alignment.
> > > > > > > >
> > > > > > > > This brings x86/master into line with patch series v8.
> > > > > > > >
> > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > ---
> > > > > > > >
> > > > > > > >  arch/x86/lib/acpi_table.c | 5 -----
> > > > > > > >  lib/acpi/acpi_table.c     | 7 ++++++-
> > > > > > > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> > > > > > > > index 600bde2f5f..13f1409de8 100644
> > > > > > > > --- a/arch/x86/lib/acpi_table.c
> > > > > > > > +++ b/arch/x86/lib/acpi_table.c
> > > > > > > > @@ -375,11 +375,6 @@ ulong write_acpi_tables(ulong start_addr)
> > > > > > > >         debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
> > > > > > > >
> > > > > > > >         acpi_setup_base_tables(ctx, start);
> > > > > > > > -       /*
> > > > > > > > -        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > > > -        * boundary (Windows checks this, but Linux does not).
> > > > > > > > -        */
> > > > > > > > -       acpi_align64(ctx);
> > > > > > > >
> > > > > > > >         debug("ACPI:    * FACS\n");
> > > > > > > >         facs = ctx->current;
> > > > > > > > diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> > > > > > > > index 5abf1cad50..1c253af3bf 100644
> > > > > > > > --- a/lib/acpi/acpi_table.c
> > > > > > > > +++ b/lib/acpi/acpi_table.c
> > > > > > > > @@ -145,7 +145,7 @@ int acpi_add_table(struct acpi_ctx *ctx, void *table)
> > > > > > > >         }
> > > > > > > >
> > > > > > > >         if (i >= entries_num) {
> > > > > > > > -               debug("ACPI: Error: too many tables\n");
> > > > > > > > +               log_err("ACPI: Error: too many tables\n");
> > > > > > > >                 return -E2BIG;
> > > > > > > >         }
> > > > > > > >
> > > > > > > > @@ -256,4 +256,9 @@ void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
> > > > > > > >         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
> > > > > > > >         acpi_write_rsdt(ctx->rsdt);
> > > > > > > >         acpi_write_xsdt(ctx->xsdt);
> > > > > > > > +       /*
> > > > > > > > +        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > > > +        * boundary (Windows checks this, but Linux does not).
> > > > > > > > +        */
> > > > > > > > +       acpi_align64(ctx);
> > > > > > > >  }
> > > > > > > > --
> > > > > > >
> > > > > > > Could you please point out which commit in u-boot-x86/master should
> > > > > > > squash in this patch to fix the build error on sandbox?
> > > > > >
> > > > > > It might be easier to pick up v8 in that case. I think there are three
> > > > > > patches that need to change because of conflicts caused by the first
> > > > > > one.
> > > > > >
> > > > > > So can you pick up the v8 patches? Also you do need to rebase on
> > > > > > master because of the str_to_upper patches.
> > > > >
> > > > > But v8 is a complete new series for part B? I think we'd better re-tag
> > > > > v8 as v1. It's quite confusing.
> > > >
> > > > No I mean the part A series. Let's get that applied and then we will
> > > > be in a brave new world.
> > >
> > > Is the v8 this one?
> > > http://patchwork.ozlabs.org/user/todo/uboot/?series=172777
> >
> > No it is this one:
> >
> > http://patchwork.ozlabs.org/project/uboot/list/?series=172776
> >
> > >
> > > The part A series are already applied, but it has a build error as I mentioned.
> >
> > Right, but the options are to do a fixup patch or to pick up the v8
> > series instead. Unfortunately there are merge conflicts in patches so
> > if you just pick up one patch it won't apply cleanly.
> >
>
> But we still need maintain bisectablity right? To do that we need a
> fix patch that can be squashed into the u-boot-x86/master. Picking up
> the v8 series does not fix the bisectiablity I think.

But my suggestion is to pick up the v8 patch *and drop what you
currently have in x86/master*. Otherwise, as you say, bisect might not
work.

I should have objected at the time to Andy refactoring patch, but I
wasn't sure when my stuff would land so it did not seem reasonable to
do so.

Regards,
SImon

[PATCH 3/3] acpi: Fix-up patch to correct sandbox test errors

[Bin Meng]

Hi Simon,

On Tue, Apr 28, 2020 at 10:39 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Tue, 28 Apr 2020 at 08:34, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Tue, Apr 28, 2020 at 10:19 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Mon, 27 Apr 2020 at 21:25, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Tue, Apr 28, 2020 at 11:06 AM Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Mon, 27 Apr 2020 at 20:42, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Tue, Apr 28, 2020 at 10:29 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Bin,
> > > > > > >
> > > > > > > On Mon, 27 Apr 2020 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Hi Simon,
> > > > > > > >
> > > > > > > > On Tue, Apr 28, 2020 at 1:02 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > > >
> > > > > > > > > Move the alignment code into acpi_setup_base_tables() so that test and
> > > > > > > > > production code are in alignment.
> > > > > > > > >
> > > > > > > > > This brings x86/master into line with patch series v8.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > >  arch/x86/lib/acpi_table.c | 5 -----
> > > > > > > > >  lib/acpi/acpi_table.c     | 7 ++++++-
> > > > > > > > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> > > > > > > > > index 600bde2f5f..13f1409de8 100644
> > > > > > > > > --- a/arch/x86/lib/acpi_table.c
> > > > > > > > > +++ b/arch/x86/lib/acpi_table.c
> > > > > > > > > @@ -375,11 +375,6 @@ ulong write_acpi_tables(ulong start_addr)
> > > > > > > > >         debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
> > > > > > > > >
> > > > > > > > >         acpi_setup_base_tables(ctx, start);
> > > > > > > > > -       /*
> > > > > > > > > -        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > > > > -        * boundary (Windows checks this, but Linux does not).
> > > > > > > > > -        */
> > > > > > > > > -       acpi_align64(ctx);
> > > > > > > > >
> > > > > > > > >         debug("ACPI:    * FACS\n");
> > > > > > > > >         facs = ctx->current;
> > > > > > > > > diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> > > > > > > > > index 5abf1cad50..1c253af3bf 100644
> > > > > > > > > --- a/lib/acpi/acpi_table.c
> > > > > > > > > +++ b/lib/acpi/acpi_table.c
> > > > > > > > > @@ -145,7 +145,7 @@ int acpi_add_table(struct acpi_ctx *ctx, void *table)
> > > > > > > > >         }
> > > > > > > > >
> > > > > > > > >         if (i >= entries_num) {
> > > > > > > > > -               debug("ACPI: Error: too many tables\n");
> > > > > > > > > +               log_err("ACPI: Error: too many tables\n");
> > > > > > > > >                 return -E2BIG;
> > > > > > > > >         }
> > > > > > > > >
> > > > > > > > > @@ -256,4 +256,9 @@ void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
> > > > > > > > >         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
> > > > > > > > >         acpi_write_rsdt(ctx->rsdt);
> > > > > > > > >         acpi_write_xsdt(ctx->xsdt);
> > > > > > > > > +       /*
> > > > > > > > > +        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > > > > +        * boundary (Windows checks this, but Linux does not).
> > > > > > > > > +        */
> > > > > > > > > +       acpi_align64(ctx);
> > > > > > > > >  }
> > > > > > > > > --
> > > > > > > >
> > > > > > > > Could you please point out which commit in u-boot-x86/master should
> > > > > > > > squash in this patch to fix the build error on sandbox?
> > > > > > >
> > > > > > > It might be easier to pick up v8 in that case. I think there are three
> > > > > > > patches that need to change because of conflicts caused by the first
> > > > > > > one.
> > > > > > >
> > > > > > > So can you pick up the v8 patches? Also you do need to rebase on
> > > > > > > master because of the str_to_upper patches.
> > > > > >
> > > > > > But v8 is a complete new series for part B? I think we'd better re-tag
> > > > > > v8 as v1. It's quite confusing.
> > > > >
> > > > > No I mean the part A series. Let's get that applied and then we will
> > > > > be in a brave new world.
> > > >
> > > > Is the v8 this one?
> > > > http://patchwork.ozlabs.org/user/todo/uboot/?series=172777
> > >
> > > No it is this one:
> > >
> > > http://patchwork.ozlabs.org/project/uboot/list/?series=172776
> > >
> > > >
> > > > The part A series are already applied, but it has a build error as I mentioned.
> > >
> > > Right, but the options are to do a fixup patch or to pick up the v8
> > > series instead. Unfortunately there are merge conflicts in patches so
> > > if you just pick up one patch it won't apply cleanly.
> > >
> >
> > But we still need maintain bisectablity right? To do that we need a
> > fix patch that can be squashed into the u-boot-x86/master. Picking up
> > the v8 series does not fix the bisectiablity I think.
>
> But my suggestion is to pick up the v8 patch *and drop what you
> currently have in x86/master*. Otherwise, as you say, bisect might not
> work.

Oops, I missed the v8 patch in patchwork (not assigned to me yet)

>
> I should have objected at the time to Andy refactoring patch, but I
> wasn't sure when my stuff would land so it did not seem reasonable to
> do so.

I will drop what is applied in x86/master, and apply the v8 part A
series. But it's better to re-tag v8 part B as v1 part B. I got quite
confused until now :)

Regards,
Bin

[PATCH 3/3] acpi: Fix-up patch to correct sandbox test errors

[Simon Glass]

Hi Bin,

On Tue, 28 Apr 2020 at 08:59, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Tue, Apr 28, 2020 at 10:39 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Tue, 28 Apr 2020 at 08:34, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Tue, Apr 28, 2020 at 10:19 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Mon, 27 Apr 2020 at 21:25, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Tue, Apr 28, 2020 at 11:06 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Bin,
> > > > > >
> > > > > > On Mon, 27 Apr 2020 at 20:42, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > >
> > > > > > > Hi Simon,
> > > > > > >
> > > > > > > On Tue, Apr 28, 2020 at 10:29 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > >
> > > > > > > > Hi Bin,
> > > > > > > >
> > > > > > > > On Mon, 27 Apr 2020 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > Hi Simon,
> > > > > > > > >
> > > > > > > > > On Tue, Apr 28, 2020 at 1:02 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > > > >
> > > > > > > > > > Move the alignment code into acpi_setup_base_tables() so that test and
> > > > > > > > > > production code are in alignment.
> > > > > > > > > >
> > > > > > > > > > This brings x86/master into line with patch series v8.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > > > ---
> > > > > > > > > >
> > > > > > > > > >  arch/x86/lib/acpi_table.c | 5 -----
> > > > > > > > > >  lib/acpi/acpi_table.c     | 7 ++++++-
> > > > > > > > > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> > > > > > > > > > index 600bde2f5f..13f1409de8 100644
> > > > > > > > > > --- a/arch/x86/lib/acpi_table.c
> > > > > > > > > > +++ b/arch/x86/lib/acpi_table.c
> > > > > > > > > > @@ -375,11 +375,6 @@ ulong write_acpi_tables(ulong start_addr)
> > > > > > > > > >         debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
> > > > > > > > > >
> > > > > > > > > >         acpi_setup_base_tables(ctx, start);
> > > > > > > > > > -       /*
> > > > > > > > > > -        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > > > > > -        * boundary (Windows checks this, but Linux does not).
> > > > > > > > > > -        */
> > > > > > > > > > -       acpi_align64(ctx);
> > > > > > > > > >
> > > > > > > > > >         debug("ACPI:    * FACS\n");
> > > > > > > > > >         facs = ctx->current;
> > > > > > > > > > diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> > > > > > > > > > index 5abf1cad50..1c253af3bf 100644
> > > > > > > > > > --- a/lib/acpi/acpi_table.c
> > > > > > > > > > +++ b/lib/acpi/acpi_table.c
> > > > > > > > > > @@ -145,7 +145,7 @@ int acpi_add_table(struct acpi_ctx *ctx, void *table)
> > > > > > > > > >         }
> > > > > > > > > >
> > > > > > > > > >         if (i >= entries_num) {
> > > > > > > > > > -               debug("ACPI: Error: too many tables\n");
> > > > > > > > > > +               log_err("ACPI: Error: too many tables\n");
> > > > > > > > > >                 return -E2BIG;
> > > > > > > > > >         }
> > > > > > > > > >
> > > > > > > > > > @@ -256,4 +256,9 @@ void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
> > > > > > > > > >         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
> > > > > > > > > >         acpi_write_rsdt(ctx->rsdt);
> > > > > > > > > >         acpi_write_xsdt(ctx->xsdt);
> > > > > > > > > > +       /*
> > > > > > > > > > +        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > > > > > +        * boundary (Windows checks this, but Linux does not).
> > > > > > > > > > +        */
> > > > > > > > > > +       acpi_align64(ctx);
> > > > > > > > > >  }
> > > > > > > > > > --
> > > > > > > > >
> > > > > > > > > Could you please point out which commit in u-boot-x86/master should
> > > > > > > > > squash in this patch to fix the build error on sandbox?
> > > > > > > >
> > > > > > > > It might be easier to pick up v8 in that case. I think there are three
> > > > > > > > patches that need to change because of conflicts caused by the first
> > > > > > > > one.
> > > > > > > >
> > > > > > > > So can you pick up the v8 patches? Also you do need to rebase on
> > > > > > > > master because of the str_to_upper patches.
> > > > > > >
> > > > > > > But v8 is a complete new series for part B? I think we'd better re-tag
> > > > > > > v8 as v1. It's quite confusing.
> > > > > >
> > > > > > No I mean the part A series. Let's get that applied and then we will
> > > > > > be in a brave new world.
> > > > >
> > > > > Is the v8 this one?
> > > > > http://patchwork.ozlabs.org/user/todo/uboot/?series=172777
> > > >
> > > > No it is this one:
> > > >
> > > > http://patchwork.ozlabs.org/project/uboot/list/?series=172776
> > > >
> > > > >
> > > > > The part A series are already applied, but it has a build error as I mentioned.
> > > >
> > > > Right, but the options are to do a fixup patch or to pick up the v8
> > > > series instead. Unfortunately there are merge conflicts in patches so
> > > > if you just pick up one patch it won't apply cleanly.
> > > >
> > >
> > > But we still need maintain bisectablity right? To do that we need a
> > > fix patch that can be squashed into the u-boot-x86/master. Picking up
> > > the v8 series does not fix the bisectiablity I think.
> >
> > But my suggestion is to pick up the v8 patch *and drop what you
> > currently have in x86/master*. Otherwise, as you say, bisect might not
> > work.
>
> Oops, I missed the v8 patch in patchwork (not assigned to me yet)
>
> >
> > I should have objected at the time to Andy refactoring patch, but I
> > wasn't sure when my stuff would land so it did not seem reasonable to
> > do so.
>
> I will drop what is applied in x86/master, and apply the v8 part A
> series. But it's better to re-tag v8 part B as v1 part B. I got quite
> confused until now :)

OK I see. Shall I send out part B as v1 now? Or wait until there are
comments and then send v2?

Regards,
Simon

[PATCH 3/3] acpi: Fix-up patch to correct sandbox test errors

[Bin Meng]

Hi Simon,

On Tue, Apr 28, 2020 at 11:41 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Tue, 28 Apr 2020 at 08:59, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Tue, Apr 28, 2020 at 10:39 PM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Tue, 28 Apr 2020 at 08:34, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Tue, Apr 28, 2020 at 10:19 PM Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Mon, 27 Apr 2020 at 21:25, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Tue, Apr 28, 2020 at 11:06 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Bin,
> > > > > > >
> > > > > > > On Mon, 27 Apr 2020 at 20:42, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Hi Simon,
> > > > > > > >
> > > > > > > > On Tue, Apr 28, 2020 at 10:29 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > > >
> > > > > > > > > Hi Bin,
> > > > > > > > >
> > > > > > > > > On Mon, 27 Apr 2020 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Simon,
> > > > > > > > > >
> > > > > > > > > > On Tue, Apr 28, 2020 at 1:02 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > > > > >
> > > > > > > > > > > Move the alignment code into acpi_setup_base_tables() so that test and
> > > > > > > > > > > production code are in alignment.
> > > > > > > > > > >
> > > > > > > > > > > This brings x86/master into line with patch series v8.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > > > > ---
> > > > > > > > > > >
> > > > > > > > > > >  arch/x86/lib/acpi_table.c | 5 -----
> > > > > > > > > > >  lib/acpi/acpi_table.c     | 7 ++++++-
> > > > > > > > > > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> > > > > > > > > > > index 600bde2f5f..13f1409de8 100644
> > > > > > > > > > > --- a/arch/x86/lib/acpi_table.c
> > > > > > > > > > > +++ b/arch/x86/lib/acpi_table.c
> > > > > > > > > > > @@ -375,11 +375,6 @@ ulong write_acpi_tables(ulong start_addr)
> > > > > > > > > > >         debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
> > > > > > > > > > >
> > > > > > > > > > >         acpi_setup_base_tables(ctx, start);
> > > > > > > > > > > -       /*
> > > > > > > > > > > -        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > > > > > > -        * boundary (Windows checks this, but Linux does not).
> > > > > > > > > > > -        */
> > > > > > > > > > > -       acpi_align64(ctx);
> > > > > > > > > > >
> > > > > > > > > > >         debug("ACPI:    * FACS\n");
> > > > > > > > > > >         facs = ctx->current;
> > > > > > > > > > > diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> > > > > > > > > > > index 5abf1cad50..1c253af3bf 100644
> > > > > > > > > > > --- a/lib/acpi/acpi_table.c
> > > > > > > > > > > +++ b/lib/acpi/acpi_table.c
> > > > > > > > > > > @@ -145,7 +145,7 @@ int acpi_add_table(struct acpi_ctx *ctx, void *table)
> > > > > > > > > > >         }
> > > > > > > > > > >
> > > > > > > > > > >         if (i >= entries_num) {
> > > > > > > > > > > -               debug("ACPI: Error: too many tables\n");
> > > > > > > > > > > +               log_err("ACPI: Error: too many tables\n");
> > > > > > > > > > >                 return -E2BIG;
> > > > > > > > > > >         }
> > > > > > > > > > >
> > > > > > > > > > > @@ -256,4 +256,9 @@ void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
> > > > > > > > > > >         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
> > > > > > > > > > >         acpi_write_rsdt(ctx->rsdt);
> > > > > > > > > > >         acpi_write_xsdt(ctx->xsdt);
> > > > > > > > > > > +       /*
> > > > > > > > > > > +        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > > > > > > +        * boundary (Windows checks this, but Linux does not).
> > > > > > > > > > > +        */
> > > > > > > > > > > +       acpi_align64(ctx);
> > > > > > > > > > >  }
> > > > > > > > > > > --
> > > > > > > > > >
> > > > > > > > > > Could you please point out which commit in u-boot-x86/master should
> > > > > > > > > > squash in this patch to fix the build error on sandbox?
> > > > > > > > >
> > > > > > > > > It might be easier to pick up v8 in that case. I think there are three
> > > > > > > > > patches that need to change because of conflicts caused by the first
> > > > > > > > > one.
> > > > > > > > >
> > > > > > > > > So can you pick up the v8 patches? Also you do need to rebase on
> > > > > > > > > master because of the str_to_upper patches.
> > > > > > > >
> > > > > > > > But v8 is a complete new series for part B? I think we'd better re-tag
> > > > > > > > v8 as v1. It's quite confusing.
> > > > > > >
> > > > > > > No I mean the part A series. Let's get that applied and then we will
> > > > > > > be in a brave new world.
> > > > > >
> > > > > > Is the v8 this one?
> > > > > > http://patchwork.ozlabs.org/user/todo/uboot/?series=172777
> > > > >
> > > > > No it is this one:
> > > > >
> > > > > http://patchwork.ozlabs.org/project/uboot/list/?series=172776
> > > > >
> > > > > >
> > > > > > The part A series are already applied, but it has a build error as I mentioned.
> > > > >
> > > > > Right, but the options are to do a fixup patch or to pick up the v8
> > > > > series instead. Unfortunately there are merge conflicts in patches so
> > > > > if you just pick up one patch it won't apply cleanly.
> > > > >
> > > >
> > > > But we still need maintain bisectablity right? To do that we need a
> > > > fix patch that can be squashed into the u-boot-x86/master. Picking up
> > > > the v8 series does not fix the bisectiablity I think.
> > >
> > > But my suggestion is to pick up the v8 patch *and drop what you
> > > currently have in x86/master*. Otherwise, as you say, bisect might not
> > > work.
> >
> > Oops, I missed the v8 patch in patchwork (not assigned to me yet)
> >
> > >
> > > I should have objected at the time to Andy refactoring patch, but I
> > > wasn't sure when my stuff would land so it did not seem reasonable to
> > > do so.
> >
> > I will drop what is applied in x86/master, and apply the v8 part A
> > series. But it's better to re-tag v8 part B as v1 part B. I got quite
> > confused until now :)
>
> OK I see. Shall I send out part B as v1 now? Or wait until there are
> comments and then send v2?

Please resend it as v1. I suspect people might get confused like me.

Regards,
Bin

[PATCH 3/3] acpi: Fix-up patch to correct sandbox test errors

[Simon Glass]

Hi Bin,

On Tue, 28 Apr 2020 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Tue, Apr 28, 2020 at 11:41 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Tue, 28 Apr 2020 at 08:59, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Tue, Apr 28, 2020 at 10:39 PM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Bin,
> > > >
> > > > On Tue, 28 Apr 2020 at 08:34, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Tue, Apr 28, 2020 at 10:19 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Bin,
> > > > > >
> > > > > > On Mon, 27 Apr 2020 at 21:25, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > >
> > > > > > > Hi Simon,
> > > > > > >
> > > > > > > On Tue, Apr 28, 2020 at 11:06 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > >
> > > > > > > > Hi Bin,
> > > > > > > >
> > > > > > > > On Mon, 27 Apr 2020 at 20:42, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > Hi Simon,
> > > > > > > > >
> > > > > > > > > On Tue, Apr 28, 2020 at 10:29 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Bin,
> > > > > > > > > >
> > > > > > > > > > On Mon, 27 Apr 2020 at 19:30, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Simon,
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Apr 28, 2020 at 1:02 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Move the alignment code into acpi_setup_base_tables() so that test and
> > > > > > > > > > > > production code are in alignment.
> > > > > > > > > > > >
> > > > > > > > > > > > This brings x86/master into line with patch series v8.
> > > > > > > > > > > >
> > > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > > > > > > > > ---
> > > > > > > > > > > >
> > > > > > > > > > > >  arch/x86/lib/acpi_table.c | 5 -----
> > > > > > > > > > > >  lib/acpi/acpi_table.c     | 7 ++++++-
> > > > > > > > > > > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > > > > > > > > > >
> > > > > > > > > > > > diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> > > > > > > > > > > > index 600bde2f5f..13f1409de8 100644
> > > > > > > > > > > > --- a/arch/x86/lib/acpi_table.c
> > > > > > > > > > > > +++ b/arch/x86/lib/acpi_table.c
> > > > > > > > > > > > @@ -375,11 +375,6 @@ ulong write_acpi_tables(ulong start_addr)
> > > > > > > > > > > >         debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
> > > > > > > > > > > >
> > > > > > > > > > > >         acpi_setup_base_tables(ctx, start);
> > > > > > > > > > > > -       /*
> > > > > > > > > > > > -        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > > > > > > > -        * boundary (Windows checks this, but Linux does not).
> > > > > > > > > > > > -        */
> > > > > > > > > > > > -       acpi_align64(ctx);
> > > > > > > > > > > >
> > > > > > > > > > > >         debug("ACPI:    * FACS\n");
> > > > > > > > > > > >         facs = ctx->current;
> > > > > > > > > > > > diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> > > > > > > > > > > > index 5abf1cad50..1c253af3bf 100644
> > > > > > > > > > > > --- a/lib/acpi/acpi_table.c
> > > > > > > > > > > > +++ b/lib/acpi/acpi_table.c
> > > > > > > > > > > > @@ -145,7 +145,7 @@ int acpi_add_table(struct acpi_ctx *ctx, void *table)
> > > > > > > > > > > >         }
> > > > > > > > > > > >
> > > > > > > > > > > >         if (i >= entries_num) {
> > > > > > > > > > > > -               debug("ACPI: Error: too many tables\n");
> > > > > > > > > > > > +               log_err("ACPI: Error: too many tables\n");
> > > > > > > > > > > >                 return -E2BIG;
> > > > > > > > > > > >         }
> > > > > > > > > > > >
> > > > > > > > > > > > @@ -256,4 +256,9 @@ void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
> > > > > > > > > > > >         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
> > > > > > > > > > > >         acpi_write_rsdt(ctx->rsdt);
> > > > > > > > > > > >         acpi_write_xsdt(ctx->xsdt);
> > > > > > > > > > > > +       /*
> > > > > > > > > > > > +        * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> > > > > > > > > > > > +        * boundary (Windows checks this, but Linux does not).
> > > > > > > > > > > > +        */
> > > > > > > > > > > > +       acpi_align64(ctx);
> > > > > > > > > > > >  }
> > > > > > > > > > > > --
> > > > > > > > > > >
> > > > > > > > > > > Could you please point out which commit in u-boot-x86/master should
> > > > > > > > > > > squash in this patch to fix the build error on sandbox?
> > > > > > > > > >
> > > > > > > > > > It might be easier to pick up v8 in that case. I think there are three
> > > > > > > > > > patches that need to change because of conflicts caused by the first
> > > > > > > > > > one.
> > > > > > > > > >
> > > > > > > > > > So can you pick up the v8 patches? Also you do need to rebase on
> > > > > > > > > > master because of the str_to_upper patches.
> > > > > > > > >
> > > > > > > > > But v8 is a complete new series for part B? I think we'd better re-tag
> > > > > > > > > v8 as v1. It's quite confusing.
> > > > > > > >
> > > > > > > > No I mean the part A series. Let's get that applied and then we will
> > > > > > > > be in a brave new world.
> > > > > > >
> > > > > > > Is the v8 this one?
> > > > > > > http://patchwork.ozlabs.org/user/todo/uboot/?series=172777
> > > > > >
> > > > > > No it is this one:
> > > > > >
> > > > > > http://patchwork.ozlabs.org/project/uboot/list/?series=172776
> > > > > >
> > > > > > >
> > > > > > > The part A series are already applied, but it has a build error as I mentioned.
> > > > > >
> > > > > > Right, but the options are to do a fixup patch or to pick up the v8
> > > > > > series instead. Unfortunately there are merge conflicts in patches so
> > > > > > if you just pick up one patch it won't apply cleanly.
> > > > > >
> > > > >
> > > > > But we still need maintain bisectablity right? To do that we need a
> > > > > fix patch that can be squashed into the u-boot-x86/master. Picking up
> > > > > the v8 series does not fix the bisectiablity I think.
> > > >
> > > > But my suggestion is to pick up the v8 patch *and drop what you
> > > > currently have in x86/master*. Otherwise, as you say, bisect might not
> > > > work.
> > >
> > > Oops, I missed the v8 patch in patchwork (not assigned to me yet)
> > >
> > > >
> > > > I should have objected at the time to Andy refactoring patch, but I
> > > > wasn't sure when my stuff would land so it did not seem reasonable to
> > > > do so.
> > >
> > > I will drop what is applied in x86/master, and apply the v8 part A
> > > series. But it's better to re-tag v8 part B as v1 part B. I got quite
> > > confused until now :)
> >
> > OK I see. Shall I send out part B as v1 now? Or wait until there are
> > comments and then send v2?
>
> Please resend it as v1. I suspect people might get confused like me.

OK this is done. Hoping people can find time to review soon! I first
posted all this in January.

Regards,
Simon