[PATCH 0/3] Fix-up series for x86/master
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
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); +}
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));
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); }
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
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
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
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
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
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
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
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
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
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
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
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
participants (2)
-
Bin Meng -
Simon Glass