[PATCH 00/14] testb: Various tweaks and fixes for Labgrid

[PATCH 00/14] testb: Various tweaks and fixes for Labgrid

[Simon Glass]

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(-)

-- 
2.34.1

[PATCH 01/14] trace: Update test to tolerate different trace-cmd version

[Simon Glass]

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):
-- 
2.34.1

[PATCH 02/14] dm: core: Enhance comments on bind_drivers_pass()

[Simon Glass]

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 =
-- 
2.34.1

[PATCH 03/14] initcall: Correct use of relocation offset

[Simon Glass]

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
-- 
2.34.1

[PATCH 04/14] am33xx: Provide a function to set up the debug UART

[Simon Glass]

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
-- 
2.34.1

[PATCH 05/14] sunxi: Mark scp as optional

[Simon Glass]

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
-- 
2.34.1

Re: [PATCH 05/14] sunxi: Mark scp as optional

[Andre Przywara]

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

Re: [PATCH 05/14] sunxi: Mark scp as optional

[Dragan Simic]

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

[PATCH 06/14] google: Disable TPMv2 on most Chromebooks

[Simon Glass]

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
-- 
2.34.1

[PATCH 07/14] meson: Correct driver declaration for meson_axg_gpio

[Simon Glass]

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,
 };
 
-- 
2.34.1

[PATCH 08/14] test: Make bootstd init run only on sandbox

[Simon Glass]

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"""
 
-- 
2.34.1

[PATCH 09/14] log: Allow tests to pass with CONFIG_LOGF_FUNC_PAD set

[Simon Glass]

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')
 
-- 
2.34.1

[PATCH 10/14] test: dm: Show failing driver name

[Simon Glass]

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
-- 
2.34.1

[PATCH 11/14] test: Decode exceptions only with sandbox

[Simon Glass]

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
-- 
2.34.1

[PATCH 12/14] test: Check help output

[Simon Glass]

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):
-- 
2.34.1

[PATCH 13/14] Update u-boot.cfg to include CFG also

[Simon Glass]

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;						\
 	} || {								\
-- 
2.34.1

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1301 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Simon Glass]

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

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2065 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Simon Glass]

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

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2687 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Simon Glass]

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

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3601 bytes --]

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).

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Simon Glass]

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

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Simon Glass]

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

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4954 bytes --]

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".

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Simon Glass]

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

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 6009 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Simon Glass]

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

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 7573 bytes --]

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 13/14] Update u-boot.cfg to include CFG also

[Simon Glass]

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

[PATCH 14/14] smbios: Correct error handling when writing tables

[Simon Glass]

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;
-- 
2.34.1

Re: [PATCH 14/14] smbios: Correct error handling when writing tables

[Heinrich Schuchardt]

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;

Re: [PATCH 14/14] smbios: Correct error handling when writing tables

[Simon Glass]

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

Re: (subset) [PATCH 00/14] testb: Various tweaks and fixes for Labgrid

[Neil Armstrong]

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/96e1a156e90a0674b21c56e7121b17a9211176ec

-- 
Neil

Re: (subset) [PATCH 00/14] testb: Various tweaks and fixes for Labgrid

[Tom Rini]

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!

-- 
Tom