[PATCH 00/14] testb: Various tweaks and fixes for Labgrid
This series includes a number of mostly unrelated changes which are in service of running U-Boot on a lab using Labgrid.
Changes in v2: - Add new patch to update u-boot.cfg with CFG_... options
Simon Glass (14): trace: Update test to tolerate different trace-cmd version dm: core: Enhance comments on bind_drivers_pass() initcall: Correct use of relocation offset am33xx: Provide a function to set up the debug UART sunxi: Mark scp as optional google: Disable TPMv2 on most Chromebooks meson: Correct driver declaration for meson_axg_gpio test: Make bootstd init run only on sandbox log: Allow tests to pass with CONFIG_LOGF_FUNC_PAD set test: dm: Show failing driver name test: Decode exceptions only with sandbox test: Check help output Update u-boot.cfg to include CFG also smbios: Correct error handling when writing tables
arch/arm/dts/sunxi-u-boot.dtsi | 1 + arch/arm/mach-omap2/am33xx/board.c | 18 +++++++++++++++--- configs/chromebook_link64_defconfig | 1 + configs/chromebook_link_defconfig | 1 + configs/chromebook_samus_defconfig | 1 + configs/chromebook_samus_tpl_defconfig | 1 + configs/nyan-big_defconfig | 4 +--- configs/snow_defconfig | 1 + drivers/core/lists.c | 16 ++++++++++++++++ drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c | 2 +- drivers/pinctrl/meson/pinctrl-meson-axg.c | 4 ++-- drivers/pinctrl/meson/pinctrl-meson-axg.h | 2 +- drivers/pinctrl/meson/pinctrl-meson-g12a.c | 4 ++-- lib/initcall.c | 6 ++++-- lib/smbios.c | 10 ++++++++-- scripts/Makefile.autoconf | 2 +- test/py/tests/test_dm.py | 5 ++++- test/py/tests/test_help.py | 6 +++++- test/py/tests/test_log.py | 11 +++++++---- test/py/tests/test_trace.py | 6 +++--- test/py/tests/test_ut.py | 1 + test/py/u_boot_console_sandbox.py | 2 +- test/py/u_boot_spawn.py | 10 ++++++---- 23 files changed, 84 insertions(+), 31 deletions(-)
Some versions of trace-cmd (or some machines?) show one less dot in the CPU list.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/tests/test_trace.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/py/tests/test_trace.py b/test/py/tests/test_trace.py index 7c5696ce747..f41d4cf71f0 100644 --- a/test/py/tests/test_trace.py +++ b/test/py/tests/test_trace.py @@ -12,7 +12,7 @@ import u_boot_utils as util TMPDIR = '/tmp/test_trace'
# Decode a function-graph line -RE_LINE = re.compile(r'.*0..... \s*([0-9.]*): func.*[|](\s*)(\S.*)?([{};])$') +RE_LINE = re.compile(r'.*0.....? \s*([0-9.]*): func.*[|](\s*)(\S.*)?([{};])$')
def collect_trace(cons):
This part of driver model is a little subtle, so add some more comments to promote better understanding.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
drivers/core/lists.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/drivers/core/lists.c b/drivers/core/lists.c index 2839a9b7371..a84e6a98cb1 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -8,6 +8,7 @@
#define LOG_CATEGORY LOGC_DM
+#include <debug_uart.h> #include <errno.h> #include <log.h> #include <dm/device.h> @@ -50,6 +51,21 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id) return NULL; }
+/** + * bind_drivers_pass() - Perform a pass of driver binding + * + * Work through the driver_info records binding a driver for each one. If the + * binding fails, continue binding others, but return the error. + * + * For OF_PLATDATA we must bind parent devices before their children. So only + * children of bound parents are bound on each call to this function. When a + * child is left unbound, -EAGAIN is returned, indicating that this function + * should be called again + * + * @parent: Parent device to use when binding each child device + * Return: 0 if OK, -EAGAIN if unbound children exist, -ENOENT if there is no + * driver for one of the devices, other -ve on other error + */ static int bind_drivers_pass(struct udevice *parent, bool pre_reloc_only) { struct driver_info *info =
The relocation offset can change in some initcall sequences. Handle this and make sure it is used for all debugging statements in init_run_list()
Update the trace test to match.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Caleb Connolly caleb.connolly@linaro.org ---
(no changes since v1)
lib/initcall.c | 6 ++++-- test/py/tests/test_trace.py | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/initcall.c b/lib/initcall.c index c8e2b0f6a38..2686b9aed5c 100644 --- a/lib/initcall.c +++ b/lib/initcall.c @@ -49,13 +49,14 @@ static int initcall_is_event(init_fnc_t func) */ int initcall_run_list(const init_fnc_t init_sequence[]) { - ulong reloc_ofs = calc_reloc_ofs(); + ulong reloc_ofs; const init_fnc_t *ptr; enum event_t type; init_fnc_t func; int ret = 0;
for (ptr = init_sequence; func = *ptr, func; ptr++) { + reloc_ofs = calc_reloc_ofs(); type = initcall_is_event(func);
if (type) { @@ -84,7 +85,8 @@ int initcall_run_list(const init_fnc_t init_sequence[]) sprintf(buf, "event %d/%s", type, event_type_name(type)); } else { - sprintf(buf, "call %p", func); + sprintf(buf, "call %p", + (char *)func - reloc_ofs); }
printf("initcall failed at %s (err=%dE)\n", buf, ret); diff --git a/test/py/tests/test_trace.py b/test/py/tests/test_trace.py index f41d4cf71f0..ec1e624722c 100644 --- a/test/py/tests/test_trace.py +++ b/test/py/tests/test_trace.py @@ -175,7 +175,7 @@ def check_funcgraph(cons, fname, proftool, map_fname, trace_dat): # Then look for this: # u-boot-1 0..... 282.101375: funcgraph_exit: 0.006 us | } # Then check for this: - # u-boot-1 0..... 282.101375: funcgraph_entry: 0.000 us | initcall_is_event(); + # u-boot-1 0..... 282.101375: funcgraph_entry: 0.000 us | calc_reloc_ofs();
expected_indent = None found_start = False @@ -199,7 +199,7 @@ def check_funcgraph(cons, fname, proftool, map_fname, trace_dat):
# The next function after initf_bootstage() exits should be # initcall_is_event() - assert upto == 'initcall_is_event()' + assert upto == 'calc_reloc_ofs()'
# Now look for initf_dm() and dm_timer_init() so we can check the bootstage # time
Since commit 0dba45864b2a ("arm: Init the debug UART") the debug UART is set up in _main() before early_system_init() is called.
Add a suitable board_debug_uart_init() function to set up the UART in SPL.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/arm/mach-omap2/am33xx/board.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/am33xx/board.c b/arch/arm/mach-omap2/am33xx/board.c index 78c1e965c9f..84a60dedd72 100644 --- a/arch/arm/mach-omap2/am33xx/board.c +++ b/arch/arm/mach-omap2/am33xx/board.c @@ -490,9 +490,6 @@ void early_system_init(void) */ save_omap_boot_params(); #endif -#ifdef CONFIG_DEBUG_UART_OMAP - debug_uart_init(); -#endif
#ifdef CONFIG_SPL_BUILD spl_early_init(); @@ -533,3 +530,18 @@ static int am33xx_dm_post_init(void) return 0; } EVENT_SPY_SIMPLE(EVT_DM_POST_INIT_F, am33xx_dm_post_init); + +#ifdef CONFIG_DEBUG_UART_BOARD_INIT +void board_debug_uart_init(void) +{ + if (u_boot_first_phase()) { + hw_data_init(); + set_uart_mux_conf(); + setup_early_clocks(); + uart_soft_reset(); + + /* avoid uart gibberish by allowing the clocks to settle */ + mdelay(50); + } +} +#endif
This binary does not prevent the system from booting. Mark it optional so that U-Boot can be built without it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
arch/arm/dts/sunxi-u-boot.dtsi | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi index 0909a67883e..e1a9a7f5d4c 100644 --- a/arch/arm/dts/sunxi-u-boot.dtsi +++ b/arch/arm/dts/sunxi-u-boot.dtsi @@ -90,6 +90,7 @@ scp { filename = "scp.bin"; missing-msg = "scp-sunxi"; + optional; }; }; #endif
On Sun, 23 Jun 2024 14:30:24 -0600 Simon Glass sjg@chromium.org wrote:
This binary does not prevent the system from booting. Mark it optional so that U-Boot can be built without it.
Signed-off-by: Simon Glass sjg@chromium.org
Reviewed-by: Andre Przywara andre.przywara@arm.com
Cheers, Andre
(no changes since v1)
arch/arm/dts/sunxi-u-boot.dtsi | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi index 0909a67883e..e1a9a7f5d4c 100644 --- a/arch/arm/dts/sunxi-u-boot.dtsi +++ b/arch/arm/dts/sunxi-u-boot.dtsi @@ -90,6 +90,7 @@ scp { filename = "scp.bin"; missing-msg = "scp-sunxi";
optional; }; };#endif
Hello Simon,
On 2024-06-23 22:30, Simon Glass wrote:
This binary does not prevent the system from booting. Mark it optional so that U-Boot can be built without it.
It should read "Lack of this binary..." instead, to properly describe the background of this patch.
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v1)
arch/arm/dts/sunxi-u-boot.dtsi | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi index 0909a67883e..e1a9a7f5d4c 100644 --- a/arch/arm/dts/sunxi-u-boot.dtsi +++ b/arch/arm/dts/sunxi-u-boot.dtsi @@ -90,6 +90,7 @@ scp { filename = "scp.bin"; missing-msg = "scp-sunxi";
optional; }; };#endif
This feature is not present on older Chromebooks, so disable the setting.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org ---
(no changes since v1)
configs/chromebook_link64_defconfig | 1 + configs/chromebook_link_defconfig | 1 + configs/chromebook_samus_defconfig | 1 + configs/chromebook_samus_tpl_defconfig | 1 + configs/nyan-big_defconfig | 4 +--- configs/snow_defconfig | 1 + 6 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/configs/chromebook_link64_defconfig b/configs/chromebook_link64_defconfig index 7cf23b29e46..9583f87bf0f 100644 --- a/configs/chromebook_link64_defconfig +++ b/configs/chromebook_link64_defconfig @@ -80,6 +80,7 @@ CONFIG_SYS_NS16550=y CONFIG_SYS_NS16550_PORT_MAPPED=y CONFIG_SPI=y CONFIG_TPM_TIS_LPC=y +# CONFIG_TPM_V2 is not set CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y CONFIG_FRAMEBUFFER_SET_VESA_MODE=y diff --git a/configs/chromebook_link_defconfig b/configs/chromebook_link_defconfig index 1a72fd178a8..e10fabd92d8 100644 --- a/configs/chromebook_link_defconfig +++ b/configs/chromebook_link_defconfig @@ -72,6 +72,7 @@ CONFIG_SYS_NS16550_PORT_MAPPED=y CONFIG_SOUND=y CONFIG_SPI=y CONFIG_TPM_TIS_LPC=y +# CONFIG_TPM_V2 is not set CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y CONFIG_VIDEO_COPY=y diff --git a/configs/chromebook_samus_defconfig b/configs/chromebook_samus_defconfig index 40cc449b9b3..8cdad8d2344 100644 --- a/configs/chromebook_samus_defconfig +++ b/configs/chromebook_samus_defconfig @@ -74,6 +74,7 @@ CONFIG_SOUND_I8254=y CONFIG_SOUND_RT5677=y CONFIG_SPI=y CONFIG_TPM_TIS_LPC=y +# CONFIG_TPM_V2 is not set CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y CONFIG_VIDEO_COPY=y diff --git a/configs/chromebook_samus_tpl_defconfig b/configs/chromebook_samus_tpl_defconfig index 3e7298f16af..1be57560f89 100644 --- a/configs/chromebook_samus_tpl_defconfig +++ b/configs/chromebook_samus_tpl_defconfig @@ -96,6 +96,7 @@ CONFIG_SOUND_RT5677=y CONFIG_SPI=y CONFIG_TPL_SYSRESET=y CONFIG_TPM_TIS_LPC=y +# CONFIG_TPM_V2 is not set CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y CONFIG_FRAMEBUFFER_SET_VESA_MODE=y diff --git a/configs/nyan-big_defconfig b/configs/nyan-big_defconfig index 4dec710cf8d..78fb7580da7 100644 --- a/configs/nyan-big_defconfig +++ b/configs/nyan-big_defconfig @@ -11,8 +11,6 @@ CONFIG_DEFAULT_DEVICE_TREE="tegra124-nyan-big" CONFIG_SPL_TEXT_BASE=0x80108000 CONFIG_SPL_STACK=0x800ffffc CONFIG_BOOTSTAGE_STASH_ADDR=0x83000000 -CONFIG_DEBUG_UART_BASE=0x70006000 -CONFIG_DEBUG_UART_CLOCK=408000000 CONFIG_TEGRA124=y CONFIG_TARGET_NYAN_BIG=y CONFIG_TEGRA_GPU=y @@ -75,7 +73,6 @@ CONFIG_DM_REGULATOR=y CONFIG_REGULATOR_AS3722=y CONFIG_DM_REGULATOR_FIXED=y CONFIG_PWM_TEGRA=y -CONFIG_DEBUG_UART_SHIFT=2 CONFIG_SYS_NS16550=y CONFIG_SOUND=y CONFIG_I2S=y @@ -83,6 +80,7 @@ CONFIG_I2S_TEGRA=y CONFIG_SOUND_MAX98090=y CONFIG_TEGRA114_SPI=y CONFIG_TPM_TIS_INFINEON=y +# CONFIG_TPM_V2 is not set CONFIG_USB=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_TEGRA=y diff --git a/configs/snow_defconfig b/configs/snow_defconfig index 3a617c6cf40..2c0757194bd 100644 --- a/configs/snow_defconfig +++ b/configs/snow_defconfig @@ -88,6 +88,7 @@ CONFIG_SOUND_MAX98095=y CONFIG_SOUND_WM8994=y CONFIG_EXYNOS_SPI=y CONFIG_TPM_TIS_INFINEON=y +# CONFIG_TPM_V2 is not set CONFIG_USB=y CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_DWC3=y
This should use the driver macros so that the driver appears in the linker list. Fix this.
Fixes: 8587839f19d ("pinctrl: meson: add axg support") Reviewed-by: Neil Armstrong neil.armstrong@linaro.org
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c | 2 +- drivers/pinctrl/meson/pinctrl-meson-axg.c | 4 ++-- drivers/pinctrl/meson/pinctrl-meson-axg.h | 2 +- drivers/pinctrl/meson/pinctrl-meson-g12a.c | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c b/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c index 52c726cf038..15ebd574ef1 100644 --- a/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c +++ b/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c @@ -179,7 +179,7 @@ static const struct dm_gpio_ops meson_axg_gpio_ops = { .direction_output = meson_gpio_direction_output, };
-const struct driver meson_axg_gpio_driver = { +U_BOOT_DRIVER(meson_axg_gpio) = { .name = "meson-axg-gpio", .id = UCLASS_GPIO, .probe = meson_gpio_probe, diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg.c b/drivers/pinctrl/meson/pinctrl-meson-axg.c index 94e09cd3f8a..ed3f92b2d75 100644 --- a/drivers/pinctrl/meson/pinctrl-meson-axg.c +++ b/drivers/pinctrl/meson/pinctrl-meson-axg.c @@ -939,7 +939,7 @@ struct meson_pinctrl_data meson_axg_periphs_pinctrl_data = { .num_groups = ARRAY_SIZE(meson_axg_periphs_groups), .num_funcs = ARRAY_SIZE(meson_axg_periphs_functions), .num_banks = ARRAY_SIZE(meson_axg_periphs_banks), - .gpio_driver = &meson_axg_gpio_driver, + .gpio_driver = DM_DRIVER_REF(meson_axg_gpio), .pmx_data = &meson_axg_periphs_pmx_banks_data, };
@@ -953,7 +953,7 @@ struct meson_pinctrl_data meson_axg_aobus_pinctrl_data = { .num_groups = ARRAY_SIZE(meson_axg_aobus_groups), .num_funcs = ARRAY_SIZE(meson_axg_aobus_functions), .num_banks = ARRAY_SIZE(meson_axg_aobus_banks), - .gpio_driver = &meson_axg_gpio_driver, + .gpio_driver = DM_DRIVER_REF(meson_axg_gpio), .pmx_data = &meson_axg_aobus_pmx_banks_data, };
diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg.h b/drivers/pinctrl/meson/pinctrl-meson-axg.h index c8d2b3af036..a6581bab500 100644 --- a/drivers/pinctrl/meson/pinctrl-meson-axg.h +++ b/drivers/pinctrl/meson/pinctrl-meson-axg.h @@ -61,6 +61,6 @@ struct meson_pmx_axg_data { }
extern const struct pinctrl_ops meson_axg_pinctrl_ops; -extern const struct driver meson_axg_gpio_driver; +extern U_BOOT_DRIVER(meson_axg_gpio);
#endif /* __PINCTRL_MESON_AXG_H__ */ diff --git a/drivers/pinctrl/meson/pinctrl-meson-g12a.c b/drivers/pinctrl/meson/pinctrl-meson-g12a.c index 24f47f82558..67114df6824 100644 --- a/drivers/pinctrl/meson/pinctrl-meson-g12a.c +++ b/drivers/pinctrl/meson/pinctrl-meson-g12a.c @@ -1253,7 +1253,7 @@ static struct meson_pinctrl_data meson_g12a_periphs_pinctrl_data = { .num_groups = ARRAY_SIZE(meson_g12a_periphs_groups), .num_funcs = ARRAY_SIZE(meson_g12a_periphs_functions), .num_banks = ARRAY_SIZE(meson_g12a_periphs_banks), - .gpio_driver = &meson_axg_gpio_driver, + .gpio_driver = DM_DRIVER_REF(meson_axg_gpio), .pmx_data = &meson_g12a_periphs_pmx_banks_data, };
@@ -1267,7 +1267,7 @@ static struct meson_pinctrl_data meson_g12a_aobus_pinctrl_data = { .num_groups = ARRAY_SIZE(meson_g12a_aobus_groups), .num_funcs = ARRAY_SIZE(meson_g12a_aobus_functions), .num_banks = ARRAY_SIZE(meson_g12a_aobus_banks), - .gpio_driver = &meson_axg_gpio_driver, + .gpio_driver = DM_DRIVER_REF(meson_axg_gpio), .pmx_data = &meson_g12a_aobus_pmx_banks_data, };
Tests for standard boot need disks to be set up, which can only be done on sandbox, since adjusting disks on real hardware is not currently supported. Mark the init function as sandbox-only.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/tests/test_ut.py | 1 + 1 file changed, 1 insertion(+)
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py index c169c835e38..58205066ec8 100644 --- a/test/py/tests/test_ut.py +++ b/test/py/tests/test_ut.py @@ -470,6 +470,7 @@ def test_ut_dm_init(u_boot_console): fh.write(data)
@pytest.mark.buildconfigspec('cmd_bootflow') +@pytest.mark.buildconfigspec('sandbox') def test_ut_dm_init_bootstd(u_boot_console): """Initialise data for bootflow tests"""
This setting pads out the function names. Adjust the test to handle this, since some boards use it.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/tests/test_log.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/test/py/tests/test_log.py b/test/py/tests/test_log.py index 140dcb9aa2b..79808674bbe 100644 --- a/test/py/tests/test_log.py +++ b/test/py/tests/test_log.py @@ -27,13 +27,16 @@ def test_log_format(u_boot_console):
cons = u_boot_console with cons.log.section('format'): - run_with_format('all', 'NOTICE.arch,file.c:123-func() msg') + pad = int(u_boot_console.config.buildconfig.get('config_logf_func_pad')) + padding = ' ' * (pad - len('func')) + + run_with_format('all', f'NOTICE.arch,file.c:123-{padding}func() msg') output = cons.run_command('log format') assert output == 'Log format: clFLfm'
- run_with_format('fm', 'func() msg') - run_with_format('clfm', 'NOTICE.arch,func() msg') - run_with_format('FLfm', 'file.c:123-func() msg') + run_with_format('fm', f'{padding}func() msg') + run_with_format('clfm', f'NOTICE.arch,{padding}func() msg') + run_with_format('FLfm', f'file.c:123-{padding}func() msg') run_with_format('lm', 'NOTICE. msg') run_with_format('m', 'msg')
When a driver is not registered properly it is not clear which one it is. Adjust test_dm_compat() to show this.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/tests/test_dm.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/test/py/tests/test_dm.py b/test/py/tests/test_dm.py index 68d4ea12235..be94971e455 100644 --- a/test/py/tests/test_dm.py +++ b/test/py/tests/test_dm.py @@ -13,8 +13,11 @@ def test_dm_compat(u_boot_console): for line in response[:-1].split('\n')[2:])
response = u_boot_console.run_command('dm compat') + bad_drivers = set() for driver in drivers: - assert driver in response + if not driver in response: + bad_drivers.add(driver) + assert not bad_drivers
# check sorting - output looks something like this: # testacpi 0 [ ] testacpi_drv |-- acpi-test
When a real board fails we don't want to decode the exception. Reserve that behaviour for sandbox. Also avoid raising a new exception on failure - just re-raise the existing one.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/u_boot_console_sandbox.py | 2 +- test/py/u_boot_spawn.py | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/test/py/u_boot_console_sandbox.py b/test/py/u_boot_console_sandbox.py index 27c6db8d719..7bc44c78b8b 100644 --- a/test/py/u_boot_console_sandbox.py +++ b/test/py/u_boot_console_sandbox.py @@ -58,7 +58,7 @@ class ConsoleSandbox(ConsoleBase): if self.use_dtb: cmd += ['-d', self.config.dtb] cmd += self.sandbox_flags - return Spawn(cmd, cwd=self.config.source_dir) + return Spawn(cmd, cwd=self.config.source_dir, decode_signal=True)
def restart_uboot_with_flags(self, flags, expect_reset=False, use_dtb=True): """Run U-Boot with the given command-line flags diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index 7c48d96210e..97e95e07c80 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -24,18 +24,20 @@ class Spawn: output: accumulated output from expect() """
- def __init__(self, args, cwd=None): + def __init__(self, args, cwd=None, decode_signal=False): """Spawn (fork/exec) the sub-process.
Args: args: array of processs arguments. argv[0] is the command to execute. cwd: the directory to run the process in, or None for no change. + decode_signal (bool): True to indicate the exception number when + something goes wrong
Returns: Nothing. """ - + self.decode_signal = decode_signal self.waited = False self.exit_code = 0 self.exit_info = '' @@ -197,12 +199,12 @@ class Spawn: # With sandbox, try to detect when U-Boot exits when it # shouldn't and explain why. This is much more friendly than # just dying with an I/O error - if err.errno == 5: # Input/output error + if self.decode_signal and err.errno == 5: # I/O error alive, _, info = self.checkalive() if alive: raise err raise ValueError('U-Boot exited with %s' % info) - raise err + raise if self.logfile_read: self.logfile_read.write(c) self.buf += c
The current test doesn't check anything about the output. If a bug results in junk before the output, this is not currently detected.
Add a check for the first line being the one expected.
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
test/py/tests/test_help.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/test/py/tests/test_help.py b/test/py/tests/test_help.py index 153133cf28f..2325ff69229 100644 --- a/test/py/tests/test_help.py +++ b/test/py/tests/test_help.py @@ -7,7 +7,11 @@ import pytest def test_help(u_boot_console): """Test that the "help" command can be executed."""
- u_boot_console.run_command('help') + lines = u_boot_console.run_command('help') + if u_boot_console.config.buildconfig.get('config_cmd_2048', 'n') == 'y': + assert lines.splitlines()[0] == "2048 - The 2048 game" + else: + assert lines.splitlines()[0] == "? - alias for 'help'"
@pytest.mark.boardspec('sandbox') def test_help_no_devicetree(u_boot_console):
Some configuration is now in variables with a CFG_ prefix. Add these to the .cfg file so that we can see everything in one place. Sort the options so they are easier to find and compare.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v2: - Add new patch to update u-boot.cfg with CFG_... options
scripts/Makefile.autoconf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf index b42f9b525fe..65ff11ea508 100644 --- a/scripts/Makefile.autoconf +++ b/scripts/Makefile.autoconf @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ quiet_cmd_u_boot_cfg = CFG $@ cmd_u_boot_cfg = \ $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \ - grep 'define CONFIG_' $@.tmp | \ + egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ rm $@.tmp; \ } || { \
On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
Some configuration is now in variables with a CFG_ prefix. Add these to the .cfg file so that we can see everything in one place. Sort the options so they are easier to find and compare.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Add new patch to update u-boot.cfg with CFG_... options
scripts/Makefile.autoconf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf index b42f9b525fe..65ff11ea508 100644 --- a/scripts/Makefile.autoconf +++ b/scripts/Makefile.autoconf @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ quiet_cmd_u_boot_cfg = CFG $@ cmd_u_boot_cfg = \ $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
grep 'define CONFIG_' $@.tmp | \
egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \
I don't like this because whereas "CONFIG_" is enforced to be set only by Kconfig and so always all reliably set and found via a single header, CFG_ stuff is not.
Hi Tom,
On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote:
On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
Some configuration is now in variables with a CFG_ prefix. Add these to the .cfg file so that we can see everything in one place. Sort the options so they are easier to find and compare.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Add new patch to update u-boot.cfg with CFG_... options
scripts/Makefile.autoconf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf index b42f9b525fe..65ff11ea508 100644 --- a/scripts/Makefile.autoconf +++ b/scripts/Makefile.autoconf @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ quiet_cmd_u_boot_cfg = CFG $@ cmd_u_boot_cfg = \ $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
grep 'define CONFIG_' $@.tmp | \
egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ rm $@.tmp; \ } || { \I don't like this because whereas "CONFIG_" is enforced to be set only by Kconfig and so always all reliably set and found via a single header, CFG_ stuff is not.
OK, so how are CFG_ options found? I hit this when trying to find the SDRAM size on rockchip 3399 and I could not find any way of figuring it out.
Regards, Simon
On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
Hi Tom,
On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote:
On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
Some configuration is now in variables with a CFG_ prefix. Add these to the .cfg file so that we can see everything in one place. Sort the options so they are easier to find and compare.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Add new patch to update u-boot.cfg with CFG_... options
scripts/Makefile.autoconf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf index b42f9b525fe..65ff11ea508 100644 --- a/scripts/Makefile.autoconf +++ b/scripts/Makefile.autoconf @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ quiet_cmd_u_boot_cfg = CFG $@ cmd_u_boot_cfg = \ $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
grep 'define CONFIG_' $@.tmp | \
egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ rm $@.tmp; \ } || { \I don't like this because whereas "CONFIG_" is enforced to be set only by Kconfig and so always all reliably set and found via a single header, CFG_ stuff is not.
OK, so how are CFG_ options found? I hit this when trying to find the SDRAM size on rockchip 3399 and I could not find any way of figuring it out.
It's just another define, there's no uniformity to it. For some of the SDRAM values really we need some build time way to grab some information out of the default device tree.
Hi Tom,
On Tue, 25 Jun 2024 at 15:14, Tom Rini trini@konsulko.com wrote:
On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
Hi Tom,
On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote:
On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
Some configuration is now in variables with a CFG_ prefix. Add these to the .cfg file so that we can see everything in one place. Sort the options so they are easier to find and compare.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Add new patch to update u-boot.cfg with CFG_... options
scripts/Makefile.autoconf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf index b42f9b525fe..65ff11ea508 100644 --- a/scripts/Makefile.autoconf +++ b/scripts/Makefile.autoconf @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ quiet_cmd_u_boot_cfg = CFG $@ cmd_u_boot_cfg = \ $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
grep 'define CONFIG_' $@.tmp | \
egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ rm $@.tmp; \ } || { \I don't like this because whereas "CONFIG_" is enforced to be set only by Kconfig and so always all reliably set and found via a single header, CFG_ stuff is not.
OK, so how are CFG_ options found? I hit this when trying to find the SDRAM size on rockchip 3399 and I could not find any way of figuring it out.
It's just another define, there's no uniformity to it. For some of the SDRAM values really we need some build time way to grab some information out of the default device tree.
Can you give an example of a board that could use this? I looked at the devicetree for chromebook_kevin and don't see a memory range in ther.
Regards, Simon
On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
Hi Tom,
On Tue, 25 Jun 2024 at 15:14, Tom Rini trini@konsulko.com wrote:
On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
Hi Tom,
On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote:
On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
Some configuration is now in variables with a CFG_ prefix. Add these to the .cfg file so that we can see everything in one place. Sort the options so they are easier to find and compare.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Add new patch to update u-boot.cfg with CFG_... options
scripts/Makefile.autoconf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf index b42f9b525fe..65ff11ea508 100644 --- a/scripts/Makefile.autoconf +++ b/scripts/Makefile.autoconf @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ quiet_cmd_u_boot_cfg = CFG $@ cmd_u_boot_cfg = \ $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
grep 'define CONFIG_' $@.tmp | \
egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ rm $@.tmp; \ } || { \I don't like this because whereas "CONFIG_" is enforced to be set only by Kconfig and so always all reliably set and found via a single header, CFG_ stuff is not.
OK, so how are CFG_ options found? I hit this when trying to find the SDRAM size on rockchip 3399 and I could not find any way of figuring it out.
It's just another define, there's no uniformity to it. For some of the SDRAM values really we need some build time way to grab some information out of the default device tree.
Can you give an example of a board that could use this? I looked at the devicetree for chromebook_kevin and don't see a memory range in ther.
OK, wow, I didn't realize /memory was optional now. But indeed, I don't see it in the dtb file. That removes that option then, sadly.
Hi Tom,
On Wed, 26 Jun 2024 at 15:07, Tom Rini trini@konsulko.com wrote:
On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
Hi Tom,
On Tue, 25 Jun 2024 at 15:14, Tom Rini trini@konsulko.com wrote:
On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
Hi Tom,
On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote:
On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
Some configuration is now in variables with a CFG_ prefix. Add these to the .cfg file so that we can see everything in one place. Sort the options so they are easier to find and compare.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Add new patch to update u-boot.cfg with CFG_... options
scripts/Makefile.autoconf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf index b42f9b525fe..65ff11ea508 100644 --- a/scripts/Makefile.autoconf +++ b/scripts/Makefile.autoconf @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ quiet_cmd_u_boot_cfg = CFG $@ cmd_u_boot_cfg = \ $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \
grep 'define CONFIG_' $@.tmp | \
egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ rm $@.tmp; \ } || { \I don't like this because whereas "CONFIG_" is enforced to be set only by Kconfig and so always all reliably set and found via a single header, CFG_ stuff is not.
OK, so how are CFG_ options found? I hit this when trying to find the SDRAM size on rockchip 3399 and I could not find any way of figuring it out.
It's just another define, there's no uniformity to it. For some of the SDRAM values really we need some build time way to grab some information out of the default device tree.
Can you give an example of a board that could use this? I looked at the devicetree for chromebook_kevin and don't see a memory range in ther.
OK, wow, I didn't realize /memory was optional now. But indeed, I don't see it in the dtb file. That removes that option then, sadly.
Well, we can still require it, so long as an error is produced if the property is needed but does not exist.
Regards, Simon
On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote:
Hi Tom,
On Wed, 26 Jun 2024 at 15:07, Tom Rini trini@konsulko.com wrote:
On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
Hi Tom,
On Tue, 25 Jun 2024 at 15:14, Tom Rini trini@konsulko.com wrote:
On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
Hi Tom,
On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote:
On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote:
> Some configuration is now in variables with a CFG_ prefix. Add these to > the .cfg file so that we can see everything in one place. Sort the > options so they are easier to find and compare. > > Signed-off-by: Simon Glass sjg@chromium.org > --- > > Changes in v2: > - Add new patch to update u-boot.cfg with CFG_... options > > scripts/Makefile.autoconf | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf > index b42f9b525fe..65ff11ea508 100644 > --- a/scripts/Makefile.autoconf > +++ b/scripts/Makefile.autoconf > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ > quiet_cmd_u_boot_cfg = CFG $@ > cmd_u_boot_cfg = \ > $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \ > - grep 'define CONFIG_' $@.tmp | \ > + egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ > sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ > rm $@.tmp; \ > } || { \
I don't like this because whereas "CONFIG_" is enforced to be set only by Kconfig and so always all reliably set and found via a single header, CFG_ stuff is not.
OK, so how are CFG_ options found? I hit this when trying to find the SDRAM size on rockchip 3399 and I could not find any way of figuring it out.
It's just another define, there's no uniformity to it. For some of the SDRAM values really we need some build time way to grab some information out of the default device tree.
Can you give an example of a board that could use this? I looked at the devicetree for chromebook_kevin and don't see a memory range in ther.
OK, wow, I didn't realize /memory was optional now. But indeed, I don't see it in the dtb file. That removes that option then, sadly.
Well, we can still require it, so long as an error is produced if the property is needed but does not exist.
"We" who? I don't feel like we'll have a lot of traction with linux kernel folks in requiring /memory to be added to the dts files on however many platforms don't have it today because I'm going to guess it's added at run time, possibly by us, with the correct size and we'd be asking for statically adding things half-wrong like a lot of platforms used to do (and in turn rely on U-Boot to correct the size).
Hi Tom,
On Thu, 27 Jun 2024 at 15:42, Tom Rini trini@konsulko.com wrote:
On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote:
Hi Tom,
On Wed, 26 Jun 2024 at 15:07, Tom Rini trini@konsulko.com wrote:
On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
Hi Tom,
On Tue, 25 Jun 2024 at 15:14, Tom Rini trini@konsulko.com wrote:
On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote:
Hi Tom,
On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote: > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote: > > > Some configuration is now in variables with a CFG_ prefix. Add these to > > the .cfg file so that we can see everything in one place. Sort the > > options so they are easier to find and compare. > > > > Signed-off-by: Simon Glass sjg@chromium.org > > --- > > > > Changes in v2: > > - Add new patch to update u-boot.cfg with CFG_... options > > > > scripts/Makefile.autoconf | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf > > index b42f9b525fe..65ff11ea508 100644 > > --- a/scripts/Makefile.autoconf > > +++ b/scripts/Makefile.autoconf > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ > > quiet_cmd_u_boot_cfg = CFG $@ > > cmd_u_boot_cfg = \ > > $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \ > > - grep 'define CONFIG_' $@.tmp | \ > > + egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ > > sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ > > rm $@.tmp; \ > > } || { \ > > I don't like this because whereas "CONFIG_" is enforced to be set only > by Kconfig and so always all reliably set and found via a single header, > CFG_ stuff is not.
OK, so how are CFG_ options found? I hit this when trying to find the SDRAM size on rockchip 3399 and I could not find any way of figuring it out.
It's just another define, there's no uniformity to it. For some of the SDRAM values really we need some build time way to grab some information out of the default device tree.
Can you give an example of a board that could use this? I looked at the devicetree for chromebook_kevin and don't see a memory range in ther.
OK, wow, I didn't realize /memory was optional now. But indeed, I don't see it in the dtb file. That removes that option then, sadly.
Well, we can still require it, so long as an error is produced if the property is needed but does not exist.
"We" who? I don't feel like we'll have a lot of traction with linux kernel folks in requiring /memory to be added to the dts files on however many platforms don't have it today because I'm going to guess it's added at run time, possibly by us, with the correct size and we'd be asking for statically adding things half-wrong like a lot of platforms used to do (and in turn rely on U-Boot to correct the size).
Hmm yes of course, the firmware is supposed to add these properties...that's how it gets in there. So we need to stick with CFG (and perhaps the RAM-size prober) for now.
Regards, Simon
Hi Tom,
On Fri, 28 Jun 2024 at 01:33, Simon Glass sjg@chromium.org wrote:
Hi Tom,
On Thu, 27 Jun 2024 at 15:42, Tom Rini trini@konsulko.com wrote:
On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote:
Hi Tom,
On Wed, 26 Jun 2024 at 15:07, Tom Rini trini@konsulko.com wrote:
On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
Hi Tom,
On Tue, 25 Jun 2024 at 15:14, Tom Rini trini@konsulko.com wrote:
On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote: > Hi Tom, > > On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote: > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote: > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to > > > the .cfg file so that we can see everything in one place. Sort the > > > options so they are easier to find and compare. > > > > > > Signed-off-by: Simon Glass sjg@chromium.org > > > --- > > > > > > Changes in v2: > > > - Add new patch to update u-boot.cfg with CFG_... options > > > > > > scripts/Makefile.autoconf | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf > > > index b42f9b525fe..65ff11ea508 100644 > > > --- a/scripts/Makefile.autoconf > > > +++ b/scripts/Makefile.autoconf > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ > > > quiet_cmd_u_boot_cfg = CFG $@ > > > cmd_u_boot_cfg = \ > > > $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \ > > > - grep 'define CONFIG_' $@.tmp | \ > > > + egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ > > > sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ > > > rm $@.tmp; \ > > > } || { \ > > > > I don't like this because whereas "CONFIG_" is enforced to be set only > > by Kconfig and so always all reliably set and found via a single header, > > CFG_ stuff is not. > > OK, so how are CFG_ options found? I hit this when trying to find the > SDRAM size on rockchip 3399 and I could not find any way of figuring > it out.
It's just another define, there's no uniformity to it. For some of the SDRAM values really we need some build time way to grab some information out of the default device tree.
Can you give an example of a board that could use this? I looked at the devicetree for chromebook_kevin and don't see a memory range in ther.
OK, wow, I didn't realize /memory was optional now. But indeed, I don't see it in the dtb file. That removes that option then, sadly.
Well, we can still require it, so long as an error is produced if the property is needed but does not exist.
"We" who? I don't feel like we'll have a lot of traction with linux kernel folks in requiring /memory to be added to the dts files on however many platforms don't have it today because I'm going to guess it's added at run time, possibly by us, with the correct size and we'd be asking for statically adding things half-wrong like a lot of platforms used to do (and in turn rely on U-Boot to correct the size).
Hmm yes of course, the firmware is supposed to add these properties...that's how it gets in there. So we need to stick with CFG (and perhaps the RAM-size prober) for now.
Coming back to this patch, can we apply it? It provides a way to find out the value of these CFG options, which otherwise involves chasing around header files.
Regards, SImon
On Sun, Jul 28, 2024 at 01:36:09PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 28 Jun 2024 at 01:33, Simon Glass sjg@chromium.org wrote:
Hi Tom,
On Thu, 27 Jun 2024 at 15:42, Tom Rini trini@konsulko.com wrote:
On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote:
Hi Tom,
On Wed, 26 Jun 2024 at 15:07, Tom Rini trini@konsulko.com wrote:
On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote:
Hi Tom,
On Tue, 25 Jun 2024 at 15:14, Tom Rini trini@konsulko.com wrote: > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote: > > > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote: > > > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to > > > > the .cfg file so that we can see everything in one place. Sort the > > > > options so they are easier to find and compare. > > > > > > > > Signed-off-by: Simon Glass sjg@chromium.org > > > > --- > > > > > > > > Changes in v2: > > > > - Add new patch to update u-boot.cfg with CFG_... options > > > > > > > > scripts/Makefile.autoconf | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf > > > > index b42f9b525fe..65ff11ea508 100644 > > > > --- a/scripts/Makefile.autoconf > > > > +++ b/scripts/Makefile.autoconf > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ > > > > quiet_cmd_u_boot_cfg = CFG $@ > > > > cmd_u_boot_cfg = \ > > > > $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \ > > > > - grep 'define CONFIG_' $@.tmp | \ > > > > + egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ > > > > sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ > > > > rm $@.tmp; \ > > > > } || { \ > > > > > > I don't like this because whereas "CONFIG_" is enforced to be set only > > > by Kconfig and so always all reliably set and found via a single header, > > > CFG_ stuff is not. > > > > OK, so how are CFG_ options found? I hit this when trying to find the > > SDRAM size on rockchip 3399 and I could not find any way of figuring > > it out. > > It's just another define, there's no uniformity to it. For some of the > SDRAM values really we need some build time way to grab some information > out of the default device tree.
Can you give an example of a board that could use this? I looked at the devicetree for chromebook_kevin and don't see a memory range in ther.
OK, wow, I didn't realize /memory was optional now. But indeed, I don't see it in the dtb file. That removes that option then, sadly.
Well, we can still require it, so long as an error is produced if the property is needed but does not exist.
"We" who? I don't feel like we'll have a lot of traction with linux kernel folks in requiring /memory to be added to the dts files on however many platforms don't have it today because I'm going to guess it's added at run time, possibly by us, with the correct size and we'd be asking for statically adding things half-wrong like a lot of platforms used to do (and in turn rely on U-Boot to correct the size).
Hmm yes of course, the firmware is supposed to add these properties...that's how it gets in there. So we need to stick with CFG (and perhaps the RAM-size prober) for now.
Coming back to this patch, can we apply it? It provides a way to find out the value of these CFG options, which otherwise involves chasing around header files.
No, because it implies that there's a consistent way to know what a given CFG value will be when there is not. There is no equivalent header to include like for CONFIG symbols to know that you got them. You're likely better off trying out "ripgrep" which I have found to be much faster than "git grep".
Hi Tom,
On Mon, 29 Jul 2024 at 12:17, Tom Rini trini@konsulko.com wrote:
On Sun, Jul 28, 2024 at 01:36:09PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 28 Jun 2024 at 01:33, Simon Glass sjg@chromium.org wrote:
Hi Tom,
On Thu, 27 Jun 2024 at 15:42, Tom Rini trini@konsulko.com wrote:
On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote:
Hi Tom,
On Wed, 26 Jun 2024 at 15:07, Tom Rini trini@konsulko.com wrote:
On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote: > Hi Tom, > > On Tue, 25 Jun 2024 at 15:14, Tom Rini trini@konsulko.com wrote: > > > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote: > > > Hi Tom, > > > > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote: > > > > > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote: > > > > > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to > > > > > the .cfg file so that we can see everything in one place. Sort the > > > > > options so they are easier to find and compare. > > > > > > > > > > Signed-off-by: Simon Glass sjg@chromium.org > > > > > --- > > > > > > > > > > Changes in v2: > > > > > - Add new patch to update u-boot.cfg with CFG_... options > > > > > > > > > > scripts/Makefile.autoconf | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf > > > > > index b42f9b525fe..65ff11ea508 100644 > > > > > --- a/scripts/Makefile.autoconf > > > > > +++ b/scripts/Makefile.autoconf > > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ > > > > > quiet_cmd_u_boot_cfg = CFG $@ > > > > > cmd_u_boot_cfg = \ > > > > > $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \ > > > > > - grep 'define CONFIG_' $@.tmp | \ > > > > > + egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ > > > > > sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ > > > > > rm $@.tmp; \ > > > > > } || { \ > > > > > > > > I don't like this because whereas "CONFIG_" is enforced to be set only > > > > by Kconfig and so always all reliably set and found via a single header, > > > > CFG_ stuff is not. > > > > > > OK, so how are CFG_ options found? I hit this when trying to find the > > > SDRAM size on rockchip 3399 and I could not find any way of figuring > > > it out. > > > > It's just another define, there's no uniformity to it. For some of the > > SDRAM values really we need some build time way to grab some information > > out of the default device tree. > > Can you give an example of a board that could use this? I looked at > the devicetree for chromebook_kevin and don't see a memory range in > ther.
OK, wow, I didn't realize /memory was optional now. But indeed, I don't see it in the dtb file. That removes that option then, sadly.
Well, we can still require it, so long as an error is produced if the property is needed but does not exist.
"We" who? I don't feel like we'll have a lot of traction with linux kernel folks in requiring /memory to be added to the dts files on however many platforms don't have it today because I'm going to guess it's added at run time, possibly by us, with the correct size and we'd be asking for statically adding things half-wrong like a lot of platforms used to do (and in turn rely on U-Boot to correct the size).
Hmm yes of course, the firmware is supposed to add these properties...that's how it gets in there. So we need to stick with CFG (and perhaps the RAM-size prober) for now.
Coming back to this patch, can we apply it? It provides a way to find out the value of these CFG options, which otherwise involves chasing around header files.
No, because it implies that there's a consistent way to know what a given CFG value will be when there is not. There is no equivalent header to include like for CONFIG symbols to know that you got them. You're likely better off trying out "ripgrep" which I have found to be much faster than "git grep".
Hmm it is really hard to know which files to grep!
Could we require that config.h includes all the CFG values? I'm just trying to find a way to bring a bit more order to this area.
Regards, Simon
On Wed, Jul 31, 2024 at 08:39:30AM -0600, Simon Glass wrote:
Hi Tom,
On Mon, 29 Jul 2024 at 12:17, Tom Rini trini@konsulko.com wrote:
On Sun, Jul 28, 2024 at 01:36:09PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 28 Jun 2024 at 01:33, Simon Glass sjg@chromium.org wrote:
Hi Tom,
On Thu, 27 Jun 2024 at 15:42, Tom Rini trini@konsulko.com wrote:
On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote:
Hi Tom,
On Wed, 26 Jun 2024 at 15:07, Tom Rini trini@konsulko.com wrote: > > On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote: > > Hi Tom, > > > > On Tue, 25 Jun 2024 at 15:14, Tom Rini trini@konsulko.com wrote: > > > > > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote: > > > > > > > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to > > > > > > the .cfg file so that we can see everything in one place. Sort the > > > > > > options so they are easier to find and compare. > > > > > > > > > > > > Signed-off-by: Simon Glass sjg@chromium.org > > > > > > --- > > > > > > > > > > > > Changes in v2: > > > > > > - Add new patch to update u-boot.cfg with CFG_... options > > > > > > > > > > > > scripts/Makefile.autoconf | 2 +- > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf > > > > > > index b42f9b525fe..65ff11ea508 100644 > > > > > > --- a/scripts/Makefile.autoconf > > > > > > +++ b/scripts/Makefile.autoconf > > > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ > > > > > > quiet_cmd_u_boot_cfg = CFG $@ > > > > > > cmd_u_boot_cfg = \ > > > > > > $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \ > > > > > > - grep 'define CONFIG_' $@.tmp | \ > > > > > > + egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ > > > > > > sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ > > > > > > rm $@.tmp; \ > > > > > > } || { \ > > > > > > > > > > I don't like this because whereas "CONFIG_" is enforced to be set only > > > > > by Kconfig and so always all reliably set and found via a single header, > > > > > CFG_ stuff is not. > > > > > > > > OK, so how are CFG_ options found? I hit this when trying to find the > > > > SDRAM size on rockchip 3399 and I could not find any way of figuring > > > > it out. > > > > > > It's just another define, there's no uniformity to it. For some of the > > > SDRAM values really we need some build time way to grab some information > > > out of the default device tree. > > > > Can you give an example of a board that could use this? I looked at > > the devicetree for chromebook_kevin and don't see a memory range in > > ther. > > OK, wow, I didn't realize /memory was optional now. But indeed, I don't > see it in the dtb file. That removes that option then, sadly.
Well, we can still require it, so long as an error is produced if the property is needed but does not exist.
"We" who? I don't feel like we'll have a lot of traction with linux kernel folks in requiring /memory to be added to the dts files on however many platforms don't have it today because I'm going to guess it's added at run time, possibly by us, with the correct size and we'd be asking for statically adding things half-wrong like a lot of platforms used to do (and in turn rely on U-Boot to correct the size).
Hmm yes of course, the firmware is supposed to add these properties...that's how it gets in there. So we need to stick with CFG (and perhaps the RAM-size prober) for now.
Coming back to this patch, can we apply it? It provides a way to find out the value of these CFG options, which otherwise involves chasing around header files.
No, because it implies that there's a consistent way to know what a given CFG value will be when there is not. There is no equivalent header to include like for CONFIG symbols to know that you got them. You're likely better off trying out "ripgrep" which I have found to be much faster than "git grep".
Hmm it is really hard to know which files to grep!
It's super easy with ripgrep: $ rg -g *.h CFG_SYS_SDRAM_SIZE
Could we require that config.h includes all the CFG values? I'm just trying to find a way to bring a bit more order to this area.
Well, some of them could perhaps be Kconfig symbols instead, it just got too tedious to untangle some of them. For others (not CFG_SYS_SDRAM_SIZE/BASE, sadly) it goes back to my idea about seeing what can be pulled at build time from the default device tree.
Hi Tom,
On Wed, 31 Jul 2024 at 19:17, Tom Rini trini@konsulko.com wrote:
On Wed, Jul 31, 2024 at 08:39:30AM -0600, Simon Glass wrote:
Hi Tom,
On Mon, 29 Jul 2024 at 12:17, Tom Rini trini@konsulko.com wrote:
On Sun, Jul 28, 2024 at 01:36:09PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 28 Jun 2024 at 01:33, Simon Glass sjg@chromium.org wrote:
Hi Tom,
On Thu, 27 Jun 2024 at 15:42, Tom Rini trini@konsulko.com wrote:
On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote: > Hi Tom, > > On Wed, 26 Jun 2024 at 15:07, Tom Rini trini@konsulko.com wrote: > > > > On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote: > > > Hi Tom, > > > > > > On Tue, 25 Jun 2024 at 15:14, Tom Rini trini@konsulko.com wrote: > > > > > > > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote: > > > > > > > > > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to > > > > > > > the .cfg file so that we can see everything in one place. Sort the > > > > > > > options so they are easier to find and compare. > > > > > > > > > > > > > > Signed-off-by: Simon Glass sjg@chromium.org > > > > > > > --- > > > > > > > > > > > > > > Changes in v2: > > > > > > > - Add new patch to update u-boot.cfg with CFG_... options > > > > > > > > > > > > > > scripts/Makefile.autoconf | 2 +- > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf > > > > > > > index b42f9b525fe..65ff11ea508 100644 > > > > > > > --- a/scripts/Makefile.autoconf > > > > > > > +++ b/scripts/Makefile.autoconf > > > > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ > > > > > > > quiet_cmd_u_boot_cfg = CFG $@ > > > > > > > cmd_u_boot_cfg = \ > > > > > > > $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \ > > > > > > > - grep 'define CONFIG_' $@.tmp | \ > > > > > > > + egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ > > > > > > > sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ > > > > > > > rm $@.tmp; \ > > > > > > > } || { \ > > > > > > > > > > > > I don't like this because whereas "CONFIG_" is enforced to be set only > > > > > > by Kconfig and so always all reliably set and found via a single header, > > > > > > CFG_ stuff is not. > > > > > > > > > > OK, so how are CFG_ options found? I hit this when trying to find the > > > > > SDRAM size on rockchip 3399 and I could not find any way of figuring > > > > > it out. > > > > > > > > It's just another define, there's no uniformity to it. For some of the > > > > SDRAM values really we need some build time way to grab some information > > > > out of the default device tree. > > > > > > Can you give an example of a board that could use this? I looked at > > > the devicetree for chromebook_kevin and don't see a memory range in > > > ther. > > > > OK, wow, I didn't realize /memory was optional now. But indeed, I don't > > see it in the dtb file. That removes that option then, sadly. > > Well, we can still require it, so long as an error is produced if the > property is needed but does not exist.
"We" who? I don't feel like we'll have a lot of traction with linux kernel folks in requiring /memory to be added to the dts files on however many platforms don't have it today because I'm going to guess it's added at run time, possibly by us, with the correct size and we'd be asking for statically adding things half-wrong like a lot of platforms used to do (and in turn rely on U-Boot to correct the size).
Hmm yes of course, the firmware is supposed to add these properties...that's how it gets in there. So we need to stick with CFG (and perhaps the RAM-size prober) for now.
Coming back to this patch, can we apply it? It provides a way to find out the value of these CFG options, which otherwise involves chasing around header files.
No, because it implies that there's a consistent way to know what a given CFG value will be when there is not. There is no equivalent header to include like for CONFIG symbols to know that you got them. You're likely better off trying out "ripgrep" which I have found to be much faster than "git grep".
Hmm it is really hard to know which files to grep!
It's super easy with ripgrep: $ rg -g *.h CFG_SYS_SDRAM_SIZE
All that does is show me lots of matches. I'm not sure which board they relate to. Some of them are obvious, but quite a few have header files common to many boards.
Could we require that config.h includes all the CFG values? I'm just trying to find a way to bring a bit more order to this area.
Well, some of them could perhaps be Kconfig symbols instead, it just got too tedious to untangle some of them. For others (not CFG_SYS_SDRAM_SIZE/BASE, sadly) it goes back to my idea about seeing what can be pulled at build time from the default device tree.
Hmm, yes. I am not sure how many are in that category. Linux often seems to use tables of data in the code, since it doesn't care so much about code size. So finding bindings for some of this data may be tricky.
I suppose the SDRAM base/size could go in the DT if it had its own binding, e.g. in /options/u-boot - but still many boards would want to determine the size at runtime.
With text environment and a solution to SDRAM, perhaps we can require that new boards not use CFG_...?
Regards, Simon
On Thu, Sep 19, 2024 at 04:13:57PM +0200, Simon Glass wrote:
Hi Tom,
On Wed, 31 Jul 2024 at 19:17, Tom Rini trini@konsulko.com wrote:
On Wed, Jul 31, 2024 at 08:39:30AM -0600, Simon Glass wrote:
Hi Tom,
On Mon, 29 Jul 2024 at 12:17, Tom Rini trini@konsulko.com wrote:
On Sun, Jul 28, 2024 at 01:36:09PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 28 Jun 2024 at 01:33, Simon Glass sjg@chromium.org wrote:
Hi Tom,
On Thu, 27 Jun 2024 at 15:42, Tom Rini trini@konsulko.com wrote: > > On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote: > > Hi Tom, > > > > On Wed, 26 Jun 2024 at 15:07, Tom Rini trini@konsulko.com wrote: > > > > > > On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Tue, 25 Jun 2024 at 15:14, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote: > > > > > > > > > > > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to > > > > > > > > the .cfg file so that we can see everything in one place. Sort the > > > > > > > > options so they are easier to find and compare. > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass sjg@chromium.org > > > > > > > > --- > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > - Add new patch to update u-boot.cfg with CFG_... options > > > > > > > > > > > > > > > > scripts/Makefile.autoconf | 2 +- > > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf > > > > > > > > index b42f9b525fe..65ff11ea508 100644 > > > > > > > > --- a/scripts/Makefile.autoconf > > > > > > > > +++ b/scripts/Makefile.autoconf > > > > > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ > > > > > > > > quiet_cmd_u_boot_cfg = CFG $@ > > > > > > > > cmd_u_boot_cfg = \ > > > > > > > > $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \ > > > > > > > > - grep 'define CONFIG_' $@.tmp | \ > > > > > > > > + egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ > > > > > > > > sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ > > > > > > > > rm $@.tmp; \ > > > > > > > > } || { \ > > > > > > > > > > > > > > I don't like this because whereas "CONFIG_" is enforced to be set only > > > > > > > by Kconfig and so always all reliably set and found via a single header, > > > > > > > CFG_ stuff is not. > > > > > > > > > > > > OK, so how are CFG_ options found? I hit this when trying to find the > > > > > > SDRAM size on rockchip 3399 and I could not find any way of figuring > > > > > > it out. > > > > > > > > > > It's just another define, there's no uniformity to it. For some of the > > > > > SDRAM values really we need some build time way to grab some information > > > > > out of the default device tree. > > > > > > > > Can you give an example of a board that could use this? I looked at > > > > the devicetree for chromebook_kevin and don't see a memory range in > > > > ther. > > > > > > OK, wow, I didn't realize /memory was optional now. But indeed, I don't > > > see it in the dtb file. That removes that option then, sadly. > > > > Well, we can still require it, so long as an error is produced if the > > property is needed but does not exist. > > "We" who? I don't feel like we'll have a lot of traction with linux > kernel folks in requiring /memory to be added to the dts files on > however many platforms don't have it today because I'm going to guess > it's added at run time, possibly by us, with the correct size and we'd > be asking for statically adding things half-wrong like a lot of > platforms used to do (and in turn rely on U-Boot to correct the size).
Hmm yes of course, the firmware is supposed to add these properties...that's how it gets in there. So we need to stick with CFG (and perhaps the RAM-size prober) for now.
Coming back to this patch, can we apply it? It provides a way to find out the value of these CFG options, which otherwise involves chasing around header files.
No, because it implies that there's a consistent way to know what a given CFG value will be when there is not. There is no equivalent header to include like for CONFIG symbols to know that you got them. You're likely better off trying out "ripgrep" which I have found to be much faster than "git grep".
Hmm it is really hard to know which files to grep!
It's super easy with ripgrep: $ rg -g *.h CFG_SYS_SDRAM_SIZE
All that does is show me lots of matches. I'm not sure which board they relate to. Some of them are obvious, but quite a few have header files common to many boards.
Could we require that config.h includes all the CFG values? I'm just trying to find a way to bring a bit more order to this area.
Well, some of them could perhaps be Kconfig symbols instead, it just got too tedious to untangle some of them. For others (not CFG_SYS_SDRAM_SIZE/BASE, sadly) it goes back to my idea about seeing what can be pulled at build time from the default device tree.
Hmm, yes. I am not sure how many are in that category. Linux often seems to use tables of data in the code, since it doesn't care so much about code size. So finding bindings for some of this data may be tricky.
I suppose the SDRAM base/size could go in the DT if it had its own binding, e.g. in /options/u-boot - but still many boards would want to determine the size at runtime.
With text environment and a solution to SDRAM, perhaps we can require that new boards not use CFG_...?
I don't see doing anything further to CFG_ as particularly important right now, honestly. The majority of symbols are in legacy things and then it's just a few symbols that no, really, there's not a good way to get this information other than a define.
Hi Tom,
On Mon, 23 Sept 2024 at 22:35, Tom Rini trini@konsulko.com wrote:
On Thu, Sep 19, 2024 at 04:13:57PM +0200, Simon Glass wrote:
Hi Tom,
On Wed, 31 Jul 2024 at 19:17, Tom Rini trini@konsulko.com wrote:
On Wed, Jul 31, 2024 at 08:39:30AM -0600, Simon Glass wrote:
Hi Tom,
On Mon, 29 Jul 2024 at 12:17, Tom Rini trini@konsulko.com wrote:
On Sun, Jul 28, 2024 at 01:36:09PM -0600, Simon Glass wrote:
Hi Tom,
On Fri, 28 Jun 2024 at 01:33, Simon Glass sjg@chromium.org wrote: > > Hi Tom, > > On Thu, 27 Jun 2024 at 15:42, Tom Rini trini@konsulko.com wrote: > > > > On Thu, Jun 27, 2024 at 09:37:15AM +0100, Simon Glass wrote: > > > Hi Tom, > > > > > > On Wed, 26 Jun 2024 at 15:07, Tom Rini trini@konsulko.com wrote: > > > > > > > > On Wed, Jun 26, 2024 at 09:00:41AM +0100, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Tue, 25 Jun 2024 at 15:14, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > On Tue, Jun 25, 2024 at 01:38:04PM +0100, Simon Glass wrote: > > > > > > > Hi Tom, > > > > > > > > > > > > > > On Mon, 24 Jun 2024 at 19:29, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > > > > > On Sun, Jun 23, 2024 at 02:30:32PM -0600, Simon Glass wrote: > > > > > > > > > > > > > > > > > Some configuration is now in variables with a CFG_ prefix. Add these to > > > > > > > > > the .cfg file so that we can see everything in one place. Sort the > > > > > > > > > options so they are easier to find and compare. > > > > > > > > > > > > > > > > > > Signed-off-by: Simon Glass sjg@chromium.org > > > > > > > > > --- > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > - Add new patch to update u-boot.cfg with CFG_... options > > > > > > > > > > > > > > > > > > scripts/Makefile.autoconf | 2 +- > > > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > > > > > > diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf > > > > > > > > > index b42f9b525fe..65ff11ea508 100644 > > > > > > > > > --- a/scripts/Makefile.autoconf > > > > > > > > > +++ b/scripts/Makefile.autoconf > > > > > > > > > @@ -71,7 +71,7 @@ quiet_cmd_autoconf = GEN $@ > > > > > > > > > quiet_cmd_u_boot_cfg = CFG $@ > > > > > > > > > cmd_u_boot_cfg = \ > > > > > > > > > $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM include/config.h > $@.tmp && { \ > > > > > > > > > - grep 'define CONFIG_' $@.tmp | \ > > > > > > > > > + egrep 'define (CONFIG_|CFG_)' $@.tmp | sort | \ > > > > > > > > > sed '/define CONFIG_IS_ENABLED(/d;/define CONFIG_IF_ENABLED_INT(/d;/define CONFIG_VAL(/d;' > $@; \ > > > > > > > > > rm $@.tmp; \ > > > > > > > > > } || { \ > > > > > > > > > > > > > > > > I don't like this because whereas "CONFIG_" is enforced to be set only > > > > > > > > by Kconfig and so always all reliably set and found via a single header, > > > > > > > > CFG_ stuff is not. > > > > > > > > > > > > > > OK, so how are CFG_ options found? I hit this when trying to find the > > > > > > > SDRAM size on rockchip 3399 and I could not find any way of figuring > > > > > > > it out. > > > > > > > > > > > > It's just another define, there's no uniformity to it. For some of the > > > > > > SDRAM values really we need some build time way to grab some information > > > > > > out of the default device tree. > > > > > > > > > > Can you give an example of a board that could use this? I looked at > > > > > the devicetree for chromebook_kevin and don't see a memory range in > > > > > ther. > > > > > > > > OK, wow, I didn't realize /memory was optional now. But indeed, I don't > > > > see it in the dtb file. That removes that option then, sadly. > > > > > > Well, we can still require it, so long as an error is produced if the > > > property is needed but does not exist. > > > > "We" who? I don't feel like we'll have a lot of traction with linux > > kernel folks in requiring /memory to be added to the dts files on > > however many platforms don't have it today because I'm going to guess > > it's added at run time, possibly by us, with the correct size and we'd > > be asking for statically adding things half-wrong like a lot of > > platforms used to do (and in turn rely on U-Boot to correct the size). > > Hmm yes of course, the firmware is supposed to add these > properties...that's how it gets in there. So we need to stick with CFG > (and perhaps the RAM-size prober) for now.
Coming back to this patch, can we apply it? It provides a way to find out the value of these CFG options, which otherwise involves chasing around header files.
No, because it implies that there's a consistent way to know what a given CFG value will be when there is not. There is no equivalent header to include like for CONFIG symbols to know that you got them. You're likely better off trying out "ripgrep" which I have found to be much faster than "git grep".
Hmm it is really hard to know which files to grep!
It's super easy with ripgrep: $ rg -g *.h CFG_SYS_SDRAM_SIZE
All that does is show me lots of matches. I'm not sure which board they relate to. Some of them are obvious, but quite a few have header files common to many boards.
Could we require that config.h includes all the CFG values? I'm just trying to find a way to bring a bit more order to this area.
Well, some of them could perhaps be Kconfig symbols instead, it just got too tedious to untangle some of them. For others (not CFG_SYS_SDRAM_SIZE/BASE, sadly) it goes back to my idea about seeing what can be pulled at build time from the default device tree.
Hmm, yes. I am not sure how many are in that category. Linux often seems to use tables of data in the code, since it doesn't care so much about code size. So finding bindings for some of this data may be tricky.
I suppose the SDRAM base/size could go in the DT if it had its own binding, e.g. in /options/u-boot - but still many boards would want to determine the size at runtime.
With text environment and a solution to SDRAM, perhaps we can require that new boards not use CFG_...?
I don't see doing anything further to CFG_ as particularly important right now, honestly. The majority of symbols are in legacy things and then it's just a few symbols that no, really, there's not a good way to get this information other than a define.
OK, I give up on this. I will try to ignore CFG from now on. Most things are in Kconfig so that is good.
Regards, Simon
Since write_smbios_table() returns an address, we cannot use it to return and error number. Also, failing on sysinfo_detect() breaks existing boards, e.g. chromebook_link
Correct this by logging and swallowing the error.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: a5a57562856 ("lib: smbios: Detect system properties via...") ---
(no changes since v1)
lib/smbios.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/smbios.c b/lib/smbios.c index fb6eaf1d5ca..4126466e34a 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -5,6 +5,8 @@ * Adapted from coreboot src/arch/x86/smbios.c */
+#define LOG_CATEGORY LOGC_BOARD + #include <dm.h> #include <env.h> #include <linux/stringify.h> @@ -596,8 +598,12 @@ ulong write_smbios_table(ulong addr)
parent_node = dev_read_subnode(ctx.dev, "smbios"); ret = sysinfo_detect(ctx.dev); - if (ret) - return ret; + + /* + * ignore the error since many boards don't implement + * this and we can still use the info in the devicetree + */ + ret = log_msg_ret("sys", ret); } } else { ctx.dev = NULL;
Am 23. Juni 2024 22:30:33 MESZ schrieb Simon Glass sjg@chromium.org:
Since write_smbios_table() returns an address, we cannot use it to return and error number. Also, failing on sysinfo_detect() breaks
IS_ERR_VALUE() could serve as template for conveying errors in addresses.
existing boards, e.g. chromebook_link
Correct this by logging and swallowing the error.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: a5a57562856 ("lib: smbios: Detect system properties via...")
(no changes since v1)
lib/smbios.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/smbios.c b/lib/smbios.c index fb6eaf1d5ca..4126466e34a 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -5,6 +5,8 @@
- Adapted from coreboot src/arch/x86/smbios.c
*/
+#define LOG_CATEGORY LOGC_BOARD
#include <dm.h> #include <env.h> #include <linux/stringify.h> @@ -596,8 +598,12 @@ ulong write_smbios_table(ulong addr)
parent_node = dev_read_subnode(ctx.dev, "smbios"); ret = sysinfo_detect(ctx.dev);
if (ret)return ret;
/** ignore the error since many boards don't implement* this and we can still use the info in the devicetree*/ret = log_msg_ret("sys", ret);
Can we make this a debug message? It is nothing an end user should worry about.
Best regards
Heinrich
}} else { ctx.dev = NULL;
Hi Heinrich,
On Sun, 23 Jun 2024 at 22:41, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 23. Juni 2024 22:30:33 MESZ schrieb Simon Glass sjg@chromium.org:
Since write_smbios_table() returns an address, we cannot use it to return and error number. Also, failing on sysinfo_detect() breaks
IS_ERR_VALUE() could serve as template for conveying errors in addresses.
existing boards, e.g. chromebook_link
Correct this by logging and swallowing the error.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: a5a57562856 ("lib: smbios: Detect system properties via...")
(no changes since v1)
lib/smbios.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/smbios.c b/lib/smbios.c index fb6eaf1d5ca..4126466e34a 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -5,6 +5,8 @@
- Adapted from coreboot src/arch/x86/smbios.c
*/
+#define LOG_CATEGORY LOGC_BOARD
#include <dm.h> #include <env.h> #include <linux/stringify.h> @@ -596,8 +598,12 @@ ulong write_smbios_table(ulong addr)
parent_node = dev_read_subnode(ctx.dev, "smbios"); ret = sysinfo_detect(ctx.dev);
if (ret)return ret;
/** ignore the error since many boards don't implement* this and we can still use the info in the devicetree*/ret = log_msg_ret("sys", ret);Can we make this a debug message? It is nothing an end user should worry about.
Yes, this is a debug message...it will only print if you have CONFIG_LOG_ERROR_RETURN enabled.
Regards, Simon
Hi,
On Sun, 23 Jun 2024 14:30:19 -0600, Simon Glass wrote:
This series includes a number of mostly unrelated changes which are in service of running U-Boot on a lab using Labgrid.
Changes in v2:
- Add new patch to update u-boot.cfg with CFG_... options
Simon Glass (14): trace: Update test to tolerate different trace-cmd version dm: core: Enhance comments on bind_drivers_pass() initcall: Correct use of relocation offset am33xx: Provide a function to set up the debug UART sunxi: Mark scp as optional google: Disable TPMv2 on most Chromebooks meson: Correct driver declaration for meson_axg_gpio test: Make bootstd init run only on sandbox log: Allow tests to pass with CONFIG_LOGF_FUNC_PAD set test: dm: Show failing driver name test: Decode exceptions only with sandbox test: Check help output Update u-boot.cfg to include CFG also smbios: Correct error handling when writing tables
[...]
Thanks, Applied to https://source.denx.de/u-boot/custodians/u-boot-amlogic (u-boot-amlogic-next)
[07/14] meson: Correct driver declaration for meson_axg_gpio https://source.denx.de/u-boot/custodians/u-boot-amlogic/-/commit/96e1a156e90...
On Sun, 23 Jun 2024 14:30:19 -0600, Simon Glass wrote:
This series includes a number of mostly unrelated changes which are in service of running U-Boot on a lab using Labgrid.
Changes in v2:
- Add new patch to update u-boot.cfg with CFG_... options
Simon Glass (14): trace: Update test to tolerate different trace-cmd version dm: core: Enhance comments on bind_drivers_pass() initcall: Correct use of relocation offset am33xx: Provide a function to set up the debug UART sunxi: Mark scp as optional google: Disable TPMv2 on most Chromebooks meson: Correct driver declaration for meson_axg_gpio test: Make bootstd init run only on sandbox log: Allow tests to pass with CONFIG_LOGF_FUNC_PAD set test: dm: Show failing driver name test: Decode exceptions only with sandbox test: Check help output Update u-boot.cfg to include CFG also smbios: Correct error handling when writing tables
[...]
Applied to u-boot/master, thanks!
participants (6)
-
Andre Przywara -
Dragan Simic -
Heinrich Schuchardt -
Neil Armstrong -
Simon Glass -
Tom Rini