Hi Sean,
On Wed, 11 Oct 2023 at 18:56, Sean Anderson <sean...@gmail.com> wrote:
>
> Sandbox unit tests in U-Boot proper load a test device tree to have some
> devices to work with. In order to do the same in SPL, we must enable
> SPL_OF_REAL. However, we already have SPL_OF_PLATDATA enabled. When
> generating platdata from a devicetree, it is expected that we will not need
> devicetree access functions (even though SPL_OF_CONTROL is enabled). This
> expectation does not hold for sandbox, so allow user control of
> SPL_OF_REAL.
>
> There are several places in the tree where conditions involving OF_PLATDATA
> or OF_REAL no longer function correctly when both of these options can be
> selected at the same time. Adjust these conditions accordingly.
>
> Signed-off-by: Sean Anderson <sean...@gmail.com>
> ---
>
> drivers/core/Makefile | 1 +
> drivers/i2c/i2c-emul-uclass.c | 2 +-
> drivers/serial/sandbox.c | 2 +-
> drivers/sysreset/sysreset_sandbox.c | 2 +-
> dts/Kconfig | 8 +++++---
> test/test-main.c | 2 +-
> 6 files changed, 10 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass <s...@chromium.org>
with a due sense of foreboding
I wonder whether this might create confusion?
>
> diff --git a/drivers/core/Makefile b/drivers/core/Makefile
> index bce0a3f65cb..acbd2bf2cef 100644
> --- a/drivers/core/Makefile
> +++ b/drivers/core/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_$(SPL_)OF_LIVE) += of_access.o of_addr.o
> ifndef CONFIG_DM_DEV_READ_INLINE
> obj-$(CONFIG_OF_CONTROL) += read.o
> endif
> +obj-$(CONFIG_$(SPL_)OF_PLATDATA) += read.o
> obj-$(CONFIG_OF_CONTROL) += of_extra.o ofnode.o read_extra.o
>
> ccflags-$(CONFIG_DM_DEBUG) += -DDEBUG
> diff --git a/drivers/i2c/i2c-emul-uclass.c b/drivers/i2c/i2c-emul-uclass.c
> index 1107cf309fc..d421ddfcbe2 100644
> --- a/drivers/i2c/i2c-emul-uclass.c
> +++ b/drivers/i2c/i2c-emul-uclass.c
> @@ -46,7 +46,7 @@ int i2c_emul_find(struct udevice *dev, struct udevice
> **emulp)
> struct udevice *emul;
> int ret;
>
> - if (!CONFIG_IS_ENABLED(OF_PLATDATA)) {
> + if (CONFIG_IS_ENABLED(OF_REAL)) {
> ret = uclass_find_device_by_phandle(UCLASS_I2C_EMUL, dev,
> "sandbox,emul", &emul);
> } else {
> diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c
> index f4003811ee7..f6ac3d22852 100644
> --- a/drivers/serial/sandbox.c
> +++ b/drivers/serial/sandbox.c
> @@ -280,7 +280,7 @@ U_BOOT_DRIVER(sandbox_serial) = {
> .flags = DM_FLAG_PRE_RELOC,
> };
>
> -#if CONFIG_IS_ENABLED(OF_REAL)
> +#if CONFIG_IS_ENABLED(OF_REAL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
> static const struct sandbox_serial_plat platdata_non_fdt = {
> .colour = -1,
> };
> diff --git a/drivers/sysreset/sysreset_sandbox.c
> b/drivers/sysreset/sysreset_sandbox.c
> index 3750c60b9b9..f485a135299 100644
> --- a/drivers/sysreset/sysreset_sandbox.c
> +++ b/drivers/sysreset/sysreset_sandbox.c
> @@ -132,7 +132,7 @@ U_BOOT_DRIVER(warm_sysreset_sandbox) = {
> .ops = &sandbox_warm_sysreset_ops,
> };
>
> -#if CONFIG_IS_ENABLED(OF_REAL)
> +#if CONFIG_IS_ENABLED(OF_REAL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
> /* This is here in case we don't have a device tree */
> U_BOOT_DRVINFO(sysreset_sandbox_non_fdt) = {
> .name = "sysreset_sandbox",
> diff --git a/dts/Kconfig b/dts/Kconfig
> index 9152f5885e9..c6fb193ca89 100644
> --- a/dts/Kconfig
> +++ b/dts/Kconfig
> @@ -410,12 +410,14 @@ config SPL_OF_PLATDATA
> declarations for each node. See of-plat.txt for more information.
>
> config SPL_OF_REAL
> - bool
> + bool "Support a real devicetree in SPL"
To avoid the user doing something silly, I wonder if it would be
better to keep this option hidden, but enable it for sandbox_spl via
Kconfig?
> + depends on SPL_OF_CONTROL
> + select SPL_OF_LIBFDT
> help
> Indicates that a real devicetree is available which can be accessed
> at runtime. This means that dev_read_...() functions can be used to
> - read data from the devicetree for each device. This is true if
> - SPL_OF_CONTROL is enabled and not SPL_OF_PLATDATA
> + read data from the devicetree for each device. You do not need to
> + enable this option if you have enabled SPL_OF_PLATDATA.
>
> if SPL_OF_PLATDATA
>
> diff --git a/test/test-main.c b/test/test-main.c
> index edb20bc4b9c..b7015d9f38d 100644
> --- a/test/test-main.c
> +++ b/test/test-main.c
> @@ -303,7 +303,7 @@ static int test_pre_run(struct unit_test_state *uts,
> struct unit_test *test)
> if (test->flags & UT_TESTF_PROBE_TEST)
> ut_assertok(do_autoprobe(uts));
>
> - if (!CONFIG_IS_ENABLED(OF_PLATDATA) &&
> + if (CONFIG_IS_ENABLED(OF_REAL) &&
> (test->flags & UT_TESTF_SCAN_FDT)) {
> /*
> * only set this if we know the ethernet uclass will be
> created
> --
> 2.37.1
>
Regards,
Simon
