Re: [PATCH v3 0/4] malloc: Reduce size by initializing data at runtime
On 10/1/23 21:16, Simon Glass wrote:
Hi Sean,
On Thu, 28 Sept 2023 at 08:45, Sean Anderson wrote:
In my efforts to get SPL to fit into flash after some changes I made, I
noticed that av_ is one of the largest variables in SPL. As it turns
out, we can generate it at runtime, and the code is already there. This
has the potential to save 1-2k across the board, for some (very) minor
boot time increase.
This series is based on [1], since this makes checking for SYS_MALLOC_F
easier. Passing CI at [2].
To measure the boot time difference, I applied the following patch:
---
common/board_r.c | 5 +
common/spl/spl.c | 4
2 files changed, 9 insertions(+)
diff --git a/common/board_r.c b/common/board_r.c
index 58a5986aa54..ca624b20d46 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -194,6 +194,7 @@ static int initr_barrier(void)
return 0;
}
+static ulong malloc_begin, malloc_end;
static int initr_malloc(void)
{
ulong malloc_start;
@@ -208,8 +209,10 @@ static int initr_malloc(void)
* reserve_noncached().
*/
malloc_start = gd->relocaddr - TOTAL_MALLOC_LEN;
+ malloc_begin = timer_get_boot_us();
Perhaps this would be better done with bootstage, since then the
timing can be enabled / disabled, and reported along with other
timings.
I'll try that out next time.
mem_malloc_init((ulong)map_sysmem(malloc_start, TOTAL_MALLOC_LEN),
TOTAL_MALLOC_LEN);
+ malloc_end = timer_get_boot_us();
gd->flags |= GD_FLG_FULL_MALLOC_INIT;
return 0;
}
@@ -570,6 +573,8 @@ static int dm_announce(void)
static int run_main_loop(void)
{
+ printf("malloc_init took %luus (%lu %lu)\n", malloc_end - malloc_begin,
+ malloc_begin, malloc_end);
#ifdef CONFIG_SANDBOX
sandbox_main_loop_init();
#endif
diff --git a/common/spl/spl.c b/common/spl/spl.c
index d74acec10b5..b34d1f4b4e6 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -755,7 +755,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
spl_set_bd();
#if defined(CONFIG_SYS_SPL_MALLOC)
+ ulong malloc_begin = timer_get_boot_us();
mem_malloc_init(SYS_SPL_MALLOC_START, CONFIG_SYS_SPL_MALLOC_SIZE);
+ ulong malloc_end = timer_get_boot_us();
gd->flags |= GD_FLG_FULL_MALLOC_INIT;
#endif
if (!(gd->flags & GD_FLG_SPL_INIT)) {
@@ -817,6 +819,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
spl_image.boot_device = BOOT_DEVICE_NONE;
board_boot_order(spl_boot_list);
+ printf("malloc_init took %luus (%lu %lu)\n", malloc_end - malloc_begin,
+ malloc_begin, malloc_end);
debug() ?
Well, this is not going to be applied, so I used the easiest thing :)
--Sean
ret = boot_from_devices(_image, spl_boot_list,
ARRAY_SIZE(spl_boot_list));
if (ret) {
--
2.25.1
I found that MALLOC_CLEAR_ON_INIT dominated the mem_malloc_init time
(taking around 150 ms in SPL on my board). After disabling it, I found
that MALLOC_RUNTIME_INIT took around 5 us on average.
[1] https://lore.kernel.org/u-boot/20230926141514.2101787-1-...@chromium.org/
[2] https://source.denx.de/u-boot/custodians/u-boot-clk/-/pipelines/17900
Changes in v3:
- Use CONFIG_IS_ENABLED in conditionals
- Don't enable SPL_SYS_MALLOC_RUNTIME_INIT if we are short on BSS
Changes in v2:
- Only mark malloc initialized after mem_malloc_init
- Fix cALLOc condition
Sean Anderson (4):
common: Only mark malloc initialized after mem_malloc_init
malloc: Don't use ifdefs for SYS_MALLOC_DEFAULT_TO_INIT
malloc: Don't statically initialize av_ if using malloc_init
malloc: Enable SYS_MALLOC_RUNTIME_INIT by default in SPL
Kconfig | 27 +--
common/board_r.c | 3 ++-
common/dlmalloc.c | 16
3 files changed, 27 insertions(+), 19 deletions(-)
--
2.35.1.1320.gc452695387.dirty
REgards,
SImon
Re: [PATCH] linker_list: Fix ll_entry_get alignment
On 10/1/23 21:16, Simon Glass wrote:
Hi Sean,
On Sat, 30 Sept 2023 at 09:23, Sean Anderson wrote:
On 9/30/23 10:36, Sean Anderson wrote:
When ll_entry_get is used on a list entry ll_entry_declare'd in the same
file, the lack of alignment on the access will override the
ll_entry_declare alignment. This causes GCC to use the default section
alignment of 32 bytes. As list entries are not necessarily 32-byte aligned,
this will cause a gap in the linker list, corrupting further entries.
As a specific example, get_fs_loader uses DM_DRIVER_GET(fs_loader) in the
same file where U_BOOT_DRIVER(fs_loader) is present. This causes a crash
when walking the driver list.
Fix this by adding appropriate alignment to all accesses.
Fixes: 42ebaae3a33 ("common: Implement support for linker-generated arrays")
Signed-off-by: Sean Anderson
---
include/linker_lists.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linker_lists.h b/include/linker_lists.h
index f9a2ee0c762..e0c8a01b9ba 100644
--- a/include/linker_lists.h
+++ b/include/linker_lists.h
@@ -209,7 +209,8 @@
*/
#define ll_entry_get(_type, _name, _list) \
({ \
- extern _type _u_boot_list_2_##_list##_2_##_name;\
+ extern _type __aligned(4) \
+ _u_boot_list_2_##_list##_2_##_name; \
_type *_ll_result = \
&_u_boot_list_2_##_list##_2_##_name;\
_ll_result; \
@@ -229,7 +230,7 @@
* @_list: name of the list
*/
#define ll_entry_ref(_type, _name, _list) \
- ((_type *)&_u_boot_list_2_##_list##_2_##_name)
+ ((_type __aligned(4) *)&_u_boot_list_2_##_list##_2_##_name)
OK, so this causes an error in clang. And it isn't really necessary
because the entry is already declared at this point.
So I guess the right fix is to replace DM_DRIVER_GET with DM_DRIVER_REF in
get_fs_loader. But this seems like a really big footgun. You can use the
wrong one and there are no errors except at runtime. I wonder if we can add
a warning of some kind?
I can imagine having a runtime check, something like:
ll_check(sizeof(struct something))
which checks that the linker list (end - start) is a multiple of the
struct size. Do you think that would find the problem?
Most of the time, yes.
If so, then it could be perhaps be turned into a link-time check. This
produces a list of the linker lists along with their individual
members:
or ll in $(nm /tmp/b/coreboot/u-boot |grep u_boot_list_2 |sed
's/.*_u_boot_list_2_\(.*\)_2_.*/\1/' |uniq); do echo; echo "linker
list: %ll"; nm /tmp/b/coreboot/u-boot |grep $ll; done
...
linker list: ut_str_test
011a9a20 D _u_boot_list_2_ut_str_test_2_str_dectoul
011a9a30 D _u_boot_list_2_ut_str_test_2_str_hextoul
011a9a40 D _u_boot_list_2_ut_str_test_2_str_itoa
011a9a50 D _u_boot_list_2_ut_str_test_2_str_simple_strtoul
011a9a60 D _u_boot_list_2_ut_str_test_2_str_simple_strtoull
011a9a70 D _u_boot_list_2_ut_str_test_2_str_trailing
011a9a80 D _u_boot_list_2_ut_str_test_2_str_upper
011a9a90 D _u_boot_list_2_ut_str_test_2_str_xtoa
011a9aa0 D _u_boot_list_2_ut_str_test_2_test_str_to_list
...
Then you can check that the address of each one increments by the same amount.
Maybe.
Yeah, this would be the best way to find errors in the current system.
But maybe ll_entry_get should look like
#define ll_entry_get(_type, _name, _list) \
({ \
ll_entry_declare(_type, _name, _list); \
_type *_ll_result = \
&_u_boot_list_2_##_list##_2_##_name;\
_ll_result; \
})
(untested)
Regardless, I think a link-time check would be a good sanity check.
--Sean
Re: [RFC PATCH v2 4/8] binman: k3: add k3-security.h and include it in k3-binman.dtsi
Hi Manorit,
On Tue, 26 Sept 2023 at 01:59, Manorit Chawdhry wrote:
>
> For readability during configuring firewalls, adding k3-security.h file
> and including it in k3-binman.dtsi to be accessible across K3 SoCs
>
> Signed-off-by: Manorit Chawdhry
> ---
> arch/arm/dts/k3-binman.dtsi | 2 ++
> arch/arm/dts/k3-security.h | 58
> +
> 2 files changed, 60 insertions(+)
>
Reviewed-by: Simon Glass
nits below
> diff --git a/arch/arm/dts/k3-binman.dtsi b/arch/arm/dts/k3-binman.dtsi
> index 2ea2dd18a12b..71ffa998a59f 100644
> --- a/arch/arm/dts/k3-binman.dtsi
> +++ b/arch/arm/dts/k3-binman.dtsi
> @@ -3,6 +3,8 @@
> * Copyright (C) 2022-2023 Texas Instruments Incorporated -
> https://www.ti.com/
> */
>
> +#include "k3-security.h"
> +
> / {
> binman: binman {
> multiple-images;
> diff --git a/arch/arm/dts/k3-security.h b/arch/arm/dts/k3-security.h
> new file mode 100644
> index ..e012b7afaf94
> --- /dev/null
> +++ b/arch/arm/dts/k3-security.h
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
> + */
> +
> +#ifndef DTS_ARM64_TI_K3_FIREWALL_H
> +#define DTS_ARM64_TI_K3_FIREWALL_H
> +
> +#define FWPRIVID_ALL(0xc3)
> +#define FWPRIVID_ARMV8 (1)
> +#define FWPRIVID_SHIFT (16)
drop () on those three and the next one
> +
> +#define FWCTRL_EN (0xA)
> +#define FWCTRL_LOCK (1 << 4)
> +#define FWCTRL_BG (1 << 8)
> +#define FWCTRL_CACHE (1 << 9)
> +
> +#define FWPERM_SECURE_PRIV_WRITE (1 << 0)
> +#define FWPERM_SECURE_PRIV_READ (1 << 1)
> +#define FWPERM_SECURE_PRIV_CACHEABLE (1 << 2)
> +#define FWPERM_SECURE_PRIV_DEBUG (1 << 3)
> +
> +#define FWPERM_SECURE_PRIV_RWCD (FWPERM_SECURE_PRIV_READ | \
> + FWPERM_SECURE_PRIV_WRITE | \
> + FWPERM_SECURE_PRIV_CACHEABLE | \
> + FWPERM_SECURE_PRIV_DEBUG)
> +
> +#define FWPERM_SECURE_USER_WRITE (1 << 4)
> +#define FWPERM_SECURE_USER_READ (1 << 5)
> +#define FWPERM_SECURE_USER_CACHEABLE (1 << 6)
> +#define FWPERM_SECURE_USER_DEBUG (1 << 7)
> +
> +#define FWPERM_SECURE_USER_RWCD (FWPERM_SECURE_USER_READ | \
> + FWPERM_SECURE_USER_WRITE | \
> + FWPERM_SECURE_USER_CACHEABLE | \
> + FWPERM_SECURE_USER_DEBUG)
> +
> +#define FWPERM_NON_SECURE_PRIV_WRITE (1 << 8)
> +#define FWPERM_NON_SECURE_PRIV_READ (1 << 9)
> +#define FWPERM_NON_SECURE_PRIV_CACHEABLE (1 << 10)
> +#define FWPERM_NON_SECURE_PRIV_DEBUG (1 << 11)
> +
> +#define FWPERM_NON_SECURE_PRIV_RWCD (FWPERM_NON_SECURE_PRIV_READ | \
> + FWPERM_NON_SECURE_PRIV_WRITE | \
> + FWPERM_NON_SECURE_PRIV_CACHEABLE
> | \
> + FWPERM_NON_SECURE_PRIV_DEBUG)
> +
> +#define FWPERM_NON_SECURE_USER_WRITE (1 << 12)
> +#define FWPERM_NON_SECURE_USER_READ (1 << 13)
> +#define FWPERM_NON_SECURE_USER_CACHEABLE (1 << 14)
> +#define FWPERM_NON_SECURE_USER_DEBUG (1 << 15)
> +
> +#define FWPERM_NON_SECURE_USER_RWCD (FWPERM_NON_SECURE_USER_READ | \
> + FWPERM_NON_SECURE_USER_WRITE | \
> + FWPERM_NON_SECURE_USER_CACHEABLE
> | \
> + FWPERM_NON_SECURE_USER_DEBUG)
> +
> +#endif
>
> --
> 2.41.0
>
Regards,
Simon
Re: [PATCH] scripts/Makefile.lib: also consider $(CONFIG_SYS_BOARD)-u-boot.dtsi
Hi Tom, On Fri, 29 Sept 2023 at 10:02, Tom Rini wrote: > > On Fri, Sep 29, 2023 at 09:15:00AM -0600, Simon Glass wrote: > > Hi Rasmus, > > > > On Mon, 25 Sept 2023 at 13:05, Rasmus Villemoes > > wrote: > > > > > > On 25/09/2023 20.19, Tom Rini wrote: > > > > On Mon, Sep 25, 2023 at 10:27:43AM +0200, Rasmus Villemoes wrote: > > > >> On 04/05/2023 14.35, Rasmus Villemoes wrote: > > > >>> On 03/05/2023 16.54, Tom Rini wrote: > > > >> > > > The one last problem now is on stm32mp15_dhcor_basic which is a > > > defconfig missing one from OF_LIST but including it in the its file, > > > so > > > the above is the patch we need. > > > > > > >> > > > >> Hi Tom > > > >> > > > >> Can I persuade you to try something like > > > >> https://source.denx.de/u-boot/u-boot/-/commit/a05e0d0e6b9103542a1076f9cab0005f400fa072 > > > >> again, but leaving the .dtbo targets in there? > > > >> > > > >> I could send a patch, but it's entirely mechanical, and not really > > > >> meant > > > >> for being applied until we know if there's more to be cleaned up. > > > > > > > > So what ended up being the problem I think is the case Simon pointed out > > > > where we do take the output from "make all" and concatenate one of the > > > > dtbs that was generated with u-boot.img or so, and it works. But maybe > > > > that should just list all of the valid DTBs that it needs in the > > > > defconfig to start with? I don't quite know, it was a case I hadn't > > > > considered at the time. > > > > > > > > > > Re-reading the thread, I can't see where that was mentioned. > > > > > > But yes, if some boards (still) need that, and have more than one > > > possible .dtb, the board can't set an OF_LIST different from the default > > > consisting of DEFAULT_DEVICE_TREE because changing OF_LIST requires > > > SPL_LOAD_FIT || MULTI_DTB_FIT. > > > > > > How do we figure out if such boards even exist? > > > > Honestly at this point I've forgotten what this is all about. > > > > Perhaps the easiest approach is to create a new Kconfig to control > > whether a board-level .dtsi is included in the list of wildcard > > searches. Then you can enable it for your board without affecting > > others. > > That's getting things backwards, from what this cleanup does. Today we > have messy lists of "build these device trees" and then don't use most > of them, and some of the list is just Wrong (listing dts files as an > output). With the series to handle dtbo files, we could remove > virtually all of that, and the only use cases that don't Just Work still > are the ones I forget which board you mentioned (I think it was Samsung > tho?) where the defconfig doesn't list all of the device trees, just one > of them, and the other 5 that we build can also be easily used. Does > that ring a bell? Yes it does...but what is the problem here? The DT files for an SoC are supposed to be buildable without needing to have the context of a particular board. I don't know what the point of the dtbo files is...is that for using overlays somehow at runtime? The defconfig does not need to mention all the DTs...we just need to make sure they get built. Is this another instance of something that needs cleaning up, like the config fragments? I am find with making the boards list the DTs that they can run with, if there is an easy way of doing that. CONFIG_SPL_OF_LIST is just for SPL, I think. Regards, Simon
Re: [PATCH 1/2] image-host: add a check of the return value of snprintf.
Hi Hugo,
On Wed, 27 Sept 2023 at 06:24, Hugo Cornelis
wrote:
>
Please add a commit message
> ---
> tools/image-host.c | 27 ++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/tools/image-host.c b/tools/image-host.c
> index a6b0a94420..0c92a2ddeb 100644
> --- a/tools/image-host.c
> +++ b/tools/image-host.c
> @@ -363,6 +363,7 @@ static int fit_image_setup_cipher(struct
> image_cipher_info *info,
> char *algo_name;
> char filename[128];
> int ret = -1;
> + int snprintf_return;
>
> if (fit_image_cipher_get_algo(fit, noffset, _name)) {
> printf("Can't get algo name for cipher in image '%s'\n",
> @@ -399,8 +400,20 @@ static int fit_image_setup_cipher(struct
> image_cipher_info *info,
> }
>
> /* Read the key in the file */
> - snprintf(filename, sizeof(filename), "%s/%s%s",
> + snprintf_return = snprintf(filename, sizeof(filename), "%s/%s%s",
> info->keydir, info->keyname, ".bin");
> + if (snprintf_return >= sizeof(filename))
> + {
> + printf("Can't format the key filename when setting up the
> cipher: insufficient buffer space\n");
> + ret = -1;
> + goto out;
> + }
> + if (snprintf_return < 0)
> + {
> + printf("Can't format the key filename when setting up the
> cipher: snprintf error\n");
> + ret = -1;
> + goto out;
> + }
> info->key = malloc(info->cipher->key_len);
> if (!info->key) {
> printf("Can't allocate memory for key\n");
> @@ -423,6 +436,18 @@ static int fit_image_setup_cipher(struct
> image_cipher_info *info,
> /* Read the IV in the file */
> snprintf(filename, sizeof(filename), "%s/%s%s",
> info->keydir, info->ivname, ".bin");
> + if (snprintf_return >= sizeof(filename))
> + {
> + printf("Can't format the IV filename when setting up
> the cipher: insufficient buffer space\n");
> + ret = -1;
> + goto out;
> + }
> + if (snprintf_return < 0)
Please check code style...the { should go at the end of the 'if' line.
You can run patman to do it for you.
> + {
> + printf("Can't format the IV filename when setting up
> the cipher: snprintf error\n");
> + ret = -1;
> + goto out;
> + }
> ret = fit_image_read_data(filename, (unsigned char *)info->iv,
> info->cipher->iv_len);
> } else {
> --
> 2.34.1
>
REgards,
Simon
Re: [PATCH v2 30/32] fdt: Allow the devicetree to come from a bloblist
Hi Ilias,
On Tue, 26 Sept 2023 at 07:13, Ilias Apalodimas
wrote:
>
> Hi Simon,
>
> [...]
>
> > > > >
> > > > > So, instead of adding OF_BLOBLIST, just move this code under OF_BOARD,
> > > > > inside an IS_ENABLED(BLOBLIST) check. If a bloblist is required and
> > > > > the previous stage loader is supposed to provide a DT we can just
> > > > > throw an error and stop booting
> > > >
> > > > This is the bit I don't get.
> > > >
> > > > The OF_BOARD thing is a hack, in that the board can do what it likes.
> > > > It is our way of handling board-specific mechanisms.
> > > >
> > > > But I am wanting a standard mechanism, i.e. like 'standard passage', a
> > > > way of passing the DT through the phases.
> > > >
> > > > If I put this under OF_BOARD, then the board gets to override the
> > > > mechanism, so which is it?
> > >
> > > No, it's the other way around in my head. OF_BOARD description is 'a
> > > previous stage loader hands me over the DT', which is a superset of
> > > the bloblist.
> > > Whether it comes in a firmware handoff format, or a hacky register the
> > > previous bootloader filled in is a detail we have to deal with and we
> > > need to keep backwards compatibility.
> > >
> > > Maybe adding a coding snip would help
> > > if (IS_ENABLED(CONFIG_OF_BOARD)) {
> > > if (CONFIG_IS_ENABLED(BLOBLIST)) { <- This instead of OF_BLOBLIST
> > > ret = bloblist_maybe_init();
> > > if (ret)
> > > return ret;
> > > /* Dynamically scan for a DT in the bloblist. */
> > > gd->fdt_blob = bloblist_find(BLOBLISTT_CONTROL_FDT, 0);
> > > if (!gd->fdt_blob) {
> > > printf("Not FDT found in bloblist\n");
> > > bloblist_show_list();
> > >// We can choose to not return an error here and keep
> > > scanning in case the DT is in a register, but I am fine with both
> > > return -ENOENT;
> > > }
> > >gd->fdt_src = FDTSRC_BLOBLIST;
> > >bloblist_show_list();
> > >log_debug("Devicetree is in bloblist at %p\n", gd->fdt_blob);
> > > // We can also bail out of this entirely if we do find a DT via
> > > a bloblist.
> > > } else {
> > > gd->fdt_blob = board_fdt_blob_setup();
> > > if (ret)
> > > return ret;
> > > gd->fdt_src = FDTSRC_BOARD;
> > > }
> > > }
> > >
> > > I haven't even compiled the code above, but it should give you a
> > > better idea of what I am suggesting
> >
> > OK I see...yes that is along the lines of what I thought you meant.
> >
> > But OF_BOARD does not mean 'previous stage loader hands me over the
> > DT'. I means call board_fdt_blob_setup() which could do anything. If
> > some boards use that function to implement getting a DT from the prior
> > stage, that's fine, but it isn't limited to that.
>
> I think it is limited. The help message says 'Provider of DTB for DT
> control' and OF_BOARD help message is 'Provided by the board (e.g a
> previous loader) at runtime '.
> In fact, that's exactly what I tried to clean up with commit
> e7fb789612e39. Sandbox had its special OF_HOSTFILE for the DT. We
> cleaned that up and then reintroduced it with a different name in
> commit 275b4832f6bf91c.
That commit adds HAS_PRIOR_STATE, though, not anything to do with sandbox.
OK. Perhaps it is just the OF_BOARD name that I object to?
>
> In any case, if people want to do 'more' we have
> CONFIG_OF_BOARD_SETUP/FIXUP which should be used instead for other
> platform-specific stuff. While at it OF_PRIOR_STAGE is a much better
> name that OF_BOARD. So we could get rid of OF_PRIOR_STAGE and rename
> OF_BOARD to that.
To do that, we have to work out what to do with board-specific setup.
>
> The function name that is called in the setup phase is
> board_fdt_blob_setup(), so it's explicitly targeting the fdt setup, or
> at least that's what the name suggets. Grepping for CONFIG_OF_BOARD
> matches in
> board/AndesTech/ae350/ae350.c
> board/armltd/vexpress64/vexpress64.c
> board/raspberrypi/rpi/rpi.c
> board/sifive/unleashed/unleashed.c
> board/sifive/unmatched/unmatched.c
> board/starfive/visionfive2/starfive_visionfive2.c
> board/xilinx/common/board.c:
> All these just use it to setup the DT, apart from the rpi which
> additionally sets up some memory for the DT.
>
> Apart from that it's used on a few platform drivers to setup so DM
> flags (most of them set DM_FLAG_PRE_RELOC), but we can easily change
> that and use The only affected files are
> drivers/pinctrl/broadcom/pinctrl-bcm283x.c
> drivers/serial/serial_bcm283x_mu.c
> drivers/serial/serial_bcm283x_pl011.c
>
> So making OF_BOARD do the right thing is trivial and I can work with
> you on that. I don't think it's too much effort anyway
What is the right thing you refer to here?
>
> >
> > There is an HAS_PRIOR_STAGE option which sets OF_BOARD at present. But
> > that doesn't seem right in the long term.
> >
> > The goal here is to standardise the passing of DT from a prior
Re: [PATCH 02/15] cmd: host: Mandate the filename parameter in the 'bind' command
On Tue, 26 Sept 2023 at 02:46, Bin Meng wrote: > > At present the host bind command does not require filename to be > provided. When it is not given NULL is passed to the host device > driver, which ends up failure afterwards. > > Change to mandate the filename so that it is useful. > > Signed-off-by: Bin Meng > --- > > cmd/host.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v5 13/16] test: dm: add SCMI base protocol test
On Tue, 26 Sept 2023 at 00:58, AKASHI Takahiro wrote: > > Added is a new unit test for SCMI base protocol, which will exercise all > the commands provided by the protocol, except SCMI_BASE_NOTIFY_ERRORS. > $ ut dm scmi_base > It is assumed that test.dtb is used as sandbox's device tree. > > Signed-off-by: AKASHI Takahiro > Reviewed-by: Etienne Carriere > --- > v4 > * fix a typo fix in v3 > s/scmi_protocol_message_attrs/scmi_base_protocol_message_attrs/ > v3 > * typo: s/scmi_base_protocol_attrs/scmi_base_protocol_message_attrs/ > * modify the code for dynamically allocated vendor/agent names > v2 > * use helper functions, removing direct uses of ops > --- > test/dm/scmi.c | 111 + > 1 file changed, 111 insertions(+) > Reviewed-by: Simon Glass
Re: [PATCH v5 04/16] test: dm: add protocol-specific channel test
On Tue, 26 Sept 2023 at 00:58, AKASHI Takahiro wrote: > > Any SCMI protocol may have its own channel. > Test this feature on sandbox as the necessary framework was added > in a prior commit. > > Signed-off-by: AKASHI Takahiro > --- > v5 > * new commit > --- > arch/sandbox/dts/test.dts | 1 + > test/dm/scmi.c| 20 ++-- > 2 files changed, 19 insertions(+), 2 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 2/2] image-host: increase path length when setting up the cipher.
On Wed, 27 Sept 2023 at 06:24, Hugo Cornelis
wrote:
>
> ---
> tools/image-host.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass
but please add commit message (always)
>
> diff --git a/tools/image-host.c b/tools/image-host.c
> index 0c92a2ddeb..9afcc02192 100644
> --- a/tools/image-host.c
> +++ b/tools/image-host.c
> @@ -361,7 +361,7 @@ static int fit_image_setup_cipher(struct
> image_cipher_info *info,
> int noffset)
> {
> char *algo_name;
> - char filename[128];
> + char filename[256];
> int ret = -1;
> int snprintf_return;
>
> --
> 2.34.1
>
Re: [PATCH 4/4] test: dm: add SCMI power domain protocol test
On Tue, 26 Sept 2023 at 01:01, AKASHI Takahiro wrote: > > This ut has tests for the SCMI power domain protocol as well as DM > interfaces for power domain devices. > > Signed-off-by: AKASHI Takahiro > --- > test/dm/scmi.c | 107 - > 1 file changed, 106 insertions(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH 3/4] sandbox: add SCMI power domain protocol support for testing
Hi AKASHI,
On Tue, 26 Sept 2023 at 01:01, AKASHI Takahiro
wrote:
>
> SCMI power domain management protocol is supported on sandbox
> for test purpose. Add fake agent interfaces and associated
> power domain devices.
>
> Signed-off-by: AKASHI Takahiro
> ---
> arch/sandbox/dts/test.dts| 6 +
> arch/sandbox/include/asm/scmi_test.h | 20 ++
> configs/sandbox_defconfig| 1 +
> drivers/firmware/scmi/sandbox-scmi_agent.c | 265 ++-
> drivers/firmware/scmi/sandbox-scmi_devices.c | 10 +
> 5 files changed, 301 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass
nit below
>
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index 36de6a37cf6d..38fcf42cd23d 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -693,6 +693,11 @@
> #address-cells = <1>;
> #size-cells = <0>;
>
> + pwrdom_scmi: protocol@11 {
> + reg = <0x11>;
> + #power-domain-cells = <1>;
> + };
> +
> clk_scmi: protocol@14 {
> reg = <0x14>;
> #clock-cells = <1>;
> @@ -1589,6 +1594,7 @@
>
> sandbox_scmi {
> compatible = "sandbox,scmi-devices";
> + power-domains = <_scmi 2>;
> clocks = <_scmi 2>, <_scmi 0>;
> resets = <_scmi 3>;
> regul0-supply = <_scmi>;
> diff --git a/arch/sandbox/include/asm/scmi_test.h
> b/arch/sandbox/include/asm/scmi_test.h
> index ccb0df6c148f..1b0f4464b98f 100644
> --- a/arch/sandbox/include/asm/scmi_test.h
> +++ b/arch/sandbox/include/asm/scmi_test.h
> @@ -6,10 +6,21 @@
> #ifndef __SANDBOX_SCMI_TEST_H
> #define __SANDBOX_SCMI_TEST_H
>
> +#include
> +
> struct udevice;
> struct sandbox_scmi_agent;
> struct sandbox_scmi_service;
>
> +/**
> + * struct sandbox_scmi_pwd
> + * @id:Identifier of the power domain used in the SCMI
> protocol
@pstate ?
> + */
> +struct sandbox_scmi_pwd {
> + uint id;
> + u32 pstate;
> +};
> +
> /**
> * struct sandbox_scmi_clk - Simulated clock exposed by SCMI
> * @id:Identifier of the clock used in the SCMI protocol
> @@ -45,6 +56,8 @@ struct sandbox_scmi_voltd {
>
[..]
Regards,
Simon
Re: [PATCH 5/4] mkimage: update man page and -h output
On Thu, 28 Sept 2023 at 02:03, Rasmus Villemoes wrote: > > The man page correctly said that -B was ignored without -E, while the > `mkimage -h` output suggested otherwise. Now that -B can actually be > used by itself, update the man page. > > While at it, also amend the `mkimage -h` line to mention the > connection with -E. > > The FDT header is a fixed 40 bytes, so its size cannot (and is not) > modified, while its alignment is a property of the address in RAM one > loads the FIT to, so not something mkimage can affect in any way. (In > the file itself, the header is of course at offset 0, which has all > possible alignments already.) > > Reported-by: Sean Anderson > Signed-off-by: Rasmus Villemoes > --- > doc/mkimage.1 | 6 -- > tools/mkimage.c | 2 +- > 2 files changed, 5 insertions(+), 3 deletions(-) Reviewed-by: Simon Glass Strange that your patch says 5/4
Re: [PATCH v5 11/16] firmware: scmi: add a check against availability of protocols
On Tue, 26 Sept 2023 at 00:58, AKASHI Takahiro wrote: > > Now that we have Base protocol support, we will be able to check if a given > protocol is really supported by the SCMI server (firmware). > > Signed-off-by: AKASHI Takahiro > Reviewed-by: Etienne Carriere > --- > v3 > * new; import this patch from my followup patch set > --- > drivers/firmware/scmi/scmi_agent-uclass.c | 41 +-- > 1 file changed, 38 insertions(+), 3 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH v5 03/16] firmware: scmi: support dummy channels for sandbox agent
On Tue, 26 Sept 2023 at 00:58, AKASHI Takahiro wrote: > > In sandbox scmi agent, channels are not used at all. But in this patch, > dummy channels are supported in order to test protocol-specific channels. > > Signed-off-by: AKASHI Takahiro > --- > v5 > * new commit > --- > arch/sandbox/include/asm/scmi_test.h | 13 > drivers/firmware/scmi/sandbox-scmi_agent.c | 90 ++ > 2 files changed, 103 insertions(+) > Reviewed-by: Simon Glass
Re: [RFC PATCH v2 2/8] binman: ti-secure: Add support for firewalling entities
Hi Manorit, On Tue, 26 Sept 2023 at 01:58, Manorit Chawdhry wrote: > > We can now firewall entities while loading them through our secure > entity TIFS, the required information should be present in the > certificate that is being parsed by TIFS. > > The following commit adds the support to enable the certificates to be > generated if the firewall configurations are present in the binman dtsi > nodes. > > Signed-off-by: Manorit Chawdhry > --- > tools/binman/btool/openssl.py | 16 +++- > tools/binman/etype/ti_secure.py | 85 > + > tools/binman/etype/x509_cert.py | 3 +- > 3 files changed, 101 insertions(+), 3 deletions(-) > Please do check that you have 100% test coverage here (binman test -T) Regards, Simon
Re: [PATCH 12/15] cmd: blk_common: Stop using hard-coded block size for Sandbox operations
On Tue, 26 Sept 2023 at 04:09, Bin Meng wrote:
>
> commit 3d2fc7971454 ("cmd: blk: Allow generic read/write operations to work
> in sandbox")
> used the hard-coded block size (512) for accessing the sandbox host
> device. Now that we have added support for non-512 block size for both
> Sandbox host device and blkmap driver, let's stop using the hard-coded
> block size.
>
> Signed-off-by: Bin Meng
> ---
>
> cmd/blk_common.c | 20 ++--
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
Reviewed-by: Simon Glass
Re: [PATCH 06/15] cmd: host: Print out the block size of the host device
On Tue, 26 Sept 2023 at 02:53, Bin Meng wrote: > > It's useful if we can print out the block size of the host device > in the "host info" command. > > Signed-off-by: Bin Meng > --- > > cmd/host.c | 7 --- > test/dm/host.c | 20 ++-- > 2 files changed, 14 insertions(+), 13 deletions(-) > Reviewed-by: Simon Glass
Re: [RFC PATCH v2 1/8] dtoc: openssl: Add GetHexOctet method
Hi Manorit,
On Tue, 26 Sept 2023 at 01:58, Manorit Chawdhry wrote:
>
> HexOctet format is used by openssl for FORMAT:HEX,OCT property in x509
> certificates. Add a helper function to extract the integer numbers in
> HEX,OCT format to pass to openssl directly.
>
> Signed-off-by: Manorit Chawdhry
> ---
> tools/dtoc/fdt_util.py | 20
> 1 file changed, 20 insertions(+)
>
> diff --git a/tools/dtoc/fdt_util.py b/tools/dtoc/fdt_util.py
> index f1f70568cfef..d51dbf5633d0 100644
> --- a/tools/dtoc/fdt_util.py
> +++ b/tools/dtoc/fdt_util.py
> @@ -100,6 +100,26 @@ def EnsureCompiled(fname, tmpdir=None,
> capture_stderr=False):
> command.run(dtc, *args, capture_stderr=capture_stderr)
> return dtb_output
>
> +def GetHexOctet(node, propname, default=None):
What is a hex octet?
> +"""Get an integer from a property in hex octet form required by openssl
> +
You should mention what size property is permitted.
> +Args:
> +node: Node object to read from
> +propname: property name to read
> +default: Default value to use if the node/property do not exist
> +
> +Returns:
> +Integer value read as a String in Hex Octet Form
> +"""
> +prop = node.props.get(propname)
> +if not isinstance(prop.value, list) or len(prop.value) != 2:
> +value = GetInt(node, propname)
> +elif isinstance(prop.value, list) and len(prop.value) == 2:
> +value = GetInt64(node, propname)
What if it is neither of those?
> +
> +hex_value = '%x' % (value)
> +return ('0' * (len(hex_value) & 1)) + hex_value
Can you do:
return f'{value:02x}'
?
> +
> def GetInt(node, propname, default=None):
> """Get an integer from a property
>
>
> --
> 2.41.0
>
Re: [RFC PATCH v2 3/8] binman: ftest: Add test for ti-secure firewall node
On Tue, 26 Sept 2023 at 01:58, Manorit Chawdhry wrote: > > Add test for TI firewalling node in ti-secure. > > Signed-off-by: Manorit Chawdhry > --- > tools/binman/ftest.py| 12 > tools/binman/test/311_ti_secure_firewall.dts | 28 > > 2 files changed, 40 insertions(+) > Reviewed-by: Simon Glass
Re: [PATCH 05/15] blk: host_dev: Sanity check on the size of host backing file
On Tue, 26 Sept 2023 at 02:49, Bin Meng wrote: > > Since we are emulating a block device, its size should be multiple > of the configured block size. > > Signed-off-by: Bin Meng > --- > > drivers/block/host_dev.c | 5 + > 1 file changed, 5 insertions(+) > Reviewed-by: Simon Glass
Re: [PATCH 04/15] blk: host_dev: Make host_sb_detach_file() and host_sb_ops static
On Tue, 26 Sept 2023 at 02:48, Bin Meng wrote: > > They are only used in drivers/block/host_dev.c. > > Signed-off-by: Bin Meng > --- > > drivers/block/host_dev.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 10/15] cmd: blk_common: Use macros for the return values
Hi Bin,
On Tue, 26 Sept 2023 at 02:54, Bin Meng wrote:
>
> Avoid using magic number 0/1 for the command result.
>
> Signed-off-by: Bin Meng
> ---
>
> cmd/blk_common.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/cmd/blk_common.c b/cmd/blk_common.c
> index 9f9d4327a9..ad9b16dc09 100644
> --- a/cmd/blk_common.c
> +++ b/cmd/blk_common.c
> @@ -25,18 +25,18 @@ int blk_common_cmd(int argc, char *const argv[], enum
> uclass_id uclass_id,
> case 2:
> if (strncmp(argv[1], "inf", 3) == 0) {
> blk_list_devices(uclass_id);
> - return 0;
> + return CMD_RET_SUCCESS;
I really don't like this...0 is success.
> } else if (strncmp(argv[1], "dev", 3) == 0) {
> if (blk_print_device_num(uclass_id, *cur_devnump)) {
> printf("\nno %s devices available\n",
> if_name);
> return CMD_RET_FAILURE;
> }
> - return 0;
> + return CMD_RET_SUCCESS;
> } else if (strncmp(argv[1], "part", 4) == 0) {
> if (blk_list_part(uclass_id))
> printf("\nno %s partition table available\n",
>if_name);
> - return 0;
> + return CMD_RET_SUCCESS;
> }
> return CMD_RET_USAGE;
> case 3:
> @@ -49,7 +49,7 @@ int blk_common_cmd(int argc, char *const argv[], enum
> uclass_id uclass_id,
> } else {
> return CMD_RET_FAILURE;
> }
> - return 0;
> + return CMD_RET_SUCCESS;
> } else if (strncmp(argv[1], "part", 4) == 0) {
> int dev = (int)dectoul(argv[2], NULL);
>
> @@ -58,7 +58,7 @@ int blk_common_cmd(int argc, char *const argv[], enum
> uclass_id uclass_id,
>if_name, dev);
> return CMD_RET_FAILURE;
> }
> - return 0;
> + return CMD_RET_SUCCESS;
> }
> return CMD_RET_USAGE;
>
> @@ -80,7 +80,7 @@ int blk_common_cmd(int argc, char *const argv[], enum
> uclass_id uclass_id,
>
> printf("%ld blocks read: %s\n", n,
>n == cnt ? "OK" : "ERROR");
> - return n == cnt ? 0 : 1;
> + return n == cnt ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
CMD_RET_FAILURE is OK, but I would prefer not to use CMD_RET_SUCCESS.
It is 0 and always will be.
It encourages people to do things like:
if (ret == CMD_RET_SUCCESS)
instead of
if (!ret)
It would eventually creep into everything, including our clean error handling.
> } else if (strcmp(argv[1], "write") == 0) {
> phys_addr_t paddr = hextoul(argv[2], NULL);
> lbaint_t blk = hextoul(argv[3], NULL);
> @@ -98,7 +98,7 @@ int blk_common_cmd(int argc, char *const argv[], enum
> uclass_id uclass_id,
>
> printf("%ld blocks written: %s\n", n,
>n == cnt ? "OK" : "ERROR");
> - return n == cnt ? 0 : 1;
> + return n == cnt ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
> } else {
> return CMD_RET_USAGE;
> }
> --
> 2.25.1
>
Regards,
Simon
Re: [PATCH 14/15] disk: part: Print out the unknown device uclass id
On Tue, 26 Sept 2023 at 04:09, Bin Meng wrote: > > It's helpful to output the device uclass id for unknown devices > during the debugging process. > > Signed-off-by: Bin Meng > --- > > disk/part.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH 01/15] blk: Use a macro for the typical block size
On Tue, 26 Sept 2023 at 02:45, Bin Meng wrote: > > Avoid using the magic number 512 directly. > > Signed-off-by: Bin Meng > --- > > common/usb_storage.c | 4 ++-- > drivers/ata/dwc_ahsata.c | 3 ++- > drivers/ata/fsl_sata.c | 3 ++- > drivers/ata/sata_mv.c| 3 ++- > drivers/ata/sata_sil.c | 3 ++- > drivers/block/blkmap.c | 2 +- > drivers/block/host_dev.c | 2 +- > drivers/mmc/mmc-uclass.c | 2 +- > drivers/nvme/nvme.c | 2 +- > include/blk.h| 2 ++ > 10 files changed, 16 insertions(+), 10 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH v2 1/2] binman: openssl: x509: ti_secure_rom: Add support for bootcore_opts
Hi Neha,
On Tue, 26 Sept 2023 at 22:08, Neha Malcom Francis wrote:
>
> According to the TRMs of K3 platform of devices, the ROM boot image
> format specifies a "Core Options Field" that provides the capability to
> set the boot core in lockstep when set to 0 or to split mode when set
> to 2. Add support for providing the same from the binman DTS. Also
> modify existing test case for ensuring future coverage.
>
> Signed-off-by: Neha Malcom Francis
> ---
> Link to J721E TRM: https://www.ti.com/lit/zip/spruil1
> Section 4.5.4.1 Boot Info
>
> tools/binman/btool/openssl.py | 6 --
> tools/binman/etype/ti_secure_rom.py | 12 ++--
> tools/binman/etype/x509_cert.py | 3 ++-
> tools/binman/test/297_ti_secure_rom.dts | 1 +
> 4 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/tools/binman/btool/openssl.py b/tools/binman/btool/openssl.py
> index aad3b61ae2..86cc56fbd7 100644
> --- a/tools/binman/btool/openssl.py
> +++ b/tools/binman/btool/openssl.py
> @@ -155,6 +155,7 @@ authInPlace = INTEGER:2
> C, ST, L, O, OU, CN and emailAddress
> cert_type (int): Certification type
> bootcore (int): Booting core
> +bootcore_opts(int): Booting core option (split/lockstep mode)
> load_addr (int): Load address of image
> sha (int): Hash function
>
> @@ -225,7 +226,7 @@ emailAddress =
> {req_dist_name_dict['emailAddress']}
>imagesize_sbl, hashval_sbl, load_addr_sysfw,
> imagesize_sysfw,
>hashval_sysfw, load_addr_sysfw_data, imagesize_sysfw_data,
>hashval_sysfw_data, sysfw_inner_cert_ext_boot_block,
> - dm_data_ext_boot_block):
> + dm_data_ext_boot_block, bootcore_opts):
> """Create a certificate
>
> Args:
> @@ -241,6 +242,7 @@ emailAddress =
> {req_dist_name_dict['emailAddress']}
> bootcore (int): Booting core
> load_addr (int): Load address of image
> sha (int): Hash function
> +bootcore_opts (int): Boot core option (split/lockstep mode)
>
> Returns:
> str: Tool output
> @@ -285,7 +287,7 @@ sysfw_data=SEQUENCE:sysfw_data
> [sbl]
> compType = INTEGER:1
> bootCore = INTEGER:16
> -compOpts = INTEGER:0
> +compOpts = INTEGER:{bootcore_opts}
> destAddr = FORMAT:HEX,OCT:{load_addr:08x}
> compSize = INTEGER:{imagesize_sbl}
> shaType = OID:{sha_type}
> diff --git a/tools/binman/etype/ti_secure_rom.py
> b/tools/binman/etype/ti_secure_rom.py
> index 9a7ac9e9e0..780f132ea5 100644
> --- a/tools/binman/etype/ti_secure_rom.py
> +++ b/tools/binman/etype/ti_secure_rom.py
> @@ -32,6 +32,7 @@ class Entry_ti_secure_rom(Entry_x509_cert):
> - core: core on which bootloader runs, valid cores are 'secure' and
> 'public'
> - content: phandle of SPL in case of legacy bootflow or phandles of
> component binaries
>in case of combined bootflow
> +- bootcore_opts (optional): split-mode (0) or lockstep mode (1) set
> to 0 by default
core-opts is what it is called in your .dts so you should use the same
name here. Please also regen the entries.rst file
>
> The following properties are only for generating a combined bootflow
> binary:
> - sysfw-inner-cert: boolean if binary contains sysfw inner
> certificate
> @@ -69,6 +70,7 @@ class Entry_ti_secure_rom(Entry_x509_cert):
> self.sw_rev = fdt_util.GetInt(self._node, 'sw-rev', 1)
> self.sha = fdt_util.GetInt(self._node, 'sha', 512)
> self.core = fdt_util.GetString(self._node, 'core', 'secure')
> +self.bootcore_opts = fdt_util.GetInt(self._node, 'core-opts')
> self.key_fname = self.GetEntryArgsOrProps([
> EntryArg('keyfile', str)], required=True)[0]
> if self.combined:
> @@ -103,11 +105,14 @@ class Entry_ti_secure_rom(Entry_x509_cert):
> else:
> self.cert_type = 2
> self.bootcore = 0
> -self.bootcore_opts = 32
> +if self.bootcore_opts is None:
> +self.bootcore_opts = 32
How come it is 32? I thought it was 0 or 1 (as documented above)?
Please add docs to explain this.
> else:
> self.cert_type = 1
> self.bootcore = 16
> -self.bootcore_opts = 0
> +if self.bootcore_opts is None:
> +self.bootcore_opts = 0
> +
> return super().GetCertificate(required=required, type='rom')
>
> def CombinedGetCertificate(self, required):
> @@ -126,6 +131,9 @@ class Entry_ti_secure_rom(Entry_x509_cert):
> self.num_comps = 3
> self.sha_type = SHA_OIDS[self.sha]
>
> +if self.bootcore_opts is None:
> +self.bootcore_opts = 0
> +
> # sbl
> self.content = fdt_util.GetPhandleList(self._node, 'content-sbl')
> input_data_sbl =
Re: [PATCH v3 4/4] malloc: Enable SYS_MALLOC_RUNTIME_INIT by default in SPL
On Thu, 28 Sept 2023 at 08:45, Sean Anderson wrote: > > On boards with size restrictions, 1-2k can be a significant fraction of the > binary size. Add a new SPL version of SYS_MALLOC_RUNTIME_INIT. As this > trades text size for BSS size, enable it by default only for boards with at > least 16k of BSS. > > Signed-off-by: Sean Anderson > --- > > Changes in v3: > - Don't enable SPL_SYS_MALLOC_RUNTIME_INIT if we are short on BSS > > Kconfig | 11 +++ > 1 file changed, 11 insertions(+) > Reviewed-by: Simon Glass
Re: [PATCH 2/5] core: return FDT_ADDR_T_NONE from devfdt_get_addr_[size_]name() on errors
On Wed, 27 Sept 2023 at 07:34, Matthias Schiffer wrote: > > Checking for the error cast to fdt_addr_t is rather awkward - IS_ERR() > can be used, but it's not really made to be used on fdt_addr_t, which > may not even be the same size as a native pointer. > > Most places in U-Boot only check for FDT_ADDR_T_NONE; let's adjust the > error return to match the expectation. > > Signed-off-by: Matthias Schiffer > --- > drivers/core/fdtaddr.c | 4 ++-- > include/dm/fdtaddr.h | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 09/15] blk: blkmap: Support mapping to device of any block size
On Tue, 26 Sept 2023 at 02:53, Bin Meng wrote: > > At present if a device to map has a block size other than 512, > the blkmap map process just fails. There is no reason why we > can't just use the block size of the mapped device. > > Signed-off-by: Bin Meng > --- > > drivers/block/blkmap.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Simon Glass
Re: [PATCHv10 10/15] net/lwip: add lwIP configuration
On Tue, 26 Sept 2023 at 03:45, Maxim Uvarov wrote: > > lwip configuration can be tuned with header file. > > Signed-off-by: Maxim Uvarov > --- > net/lwip/lwipopts.h | 178 > 1 file changed, 178 insertions(+) > create mode 100644 net/lwip/lwipopts.h Reviewed-by: Simon Glass
Re: [PATCH 1/2] dm: serial: fix serial_post_probe()
On Thu, 28 Sept 2023 at 18:47, Heinrich Schuchardt
wrote:
>
> The size of the name of a udevice is not limited.
>
> When setting the fixed sized name field of a stdio device we must ensure
> that the target string is NUL terminated to avoid buffer overflows.
>
> Fixes: 57d92753d4ca ("dm: Add a uclass for serial devices")
> Signed-off-by: Heinrich Schuchardt
> ---
> drivers/serial/serial-uclass.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass
Re: [PATCH 2/2] stdio: fix stdio_deregister_dev()
On Thu, 28 Sept 2023 at 18:47, Heinrich Schuchardt
wrote:
>
> When copying the name of a stdio device we must ensure that it is NUL
> terminated before passing it to strcmp() to avoid a buffer overrun.
>
> Truncating the name field leads to failure to deregister a stdio device.
> When copying we must ensure that the name field sizes match.
>
> Addresses-Coverity-ID: 350462 String not null terminated
> Fixes: 5294e97832a6 ("stdio: extend "name" to 32 symbols")
> Signed-off-by: Heinrich Schuchardt
> ---
> common/stdio.c | 6 +++---
> include/stdio_dev.h | 3 ++-
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
Reviewed-by: Simon Glass
Re: [PATCH v2 2/3] dm: prepare rkmtd UCLASS
Hi Johan, On Thu, 28 Sept 2023 at 12:51, Johan Jonker wrote: > > Prepare a rkmtd UCLASS in use for writing Rockchip boot blocks > in combination with existing userspace tools and rockusb command. > > Signed-off-by: Johan Jonker > Reviewed-by: Kever Yang > --- > disk/part.c| 4 > drivers/block/blk-uclass.c | 1 + > include/dm/uclass-id.h | 1 + > 3 files changed, 6 insertions(+) > A new uclass should have a sandbox test and a header file or some other docs describing it. REgards, Simon
Re: [PATCH v2 1/3] common: add prototype & rename populate_serial_number()
On Tue, 26 Sept 2023 at 10:36, Artur Rojek wrote: > > Rename populate_serial_number() to a more descriptive > eeprom_read_serial() and provide the missing function prototype. > > This is useful for boards that wish to read their serial number from > EEPROM at init. > > Signed-off-by: Artur Rojek > --- > > v2: - rename the function > - move function documentation from .c file to the prototype location > > cmd/tlv_eeprom.c | 14 +- > include/init.h | 14 ++ > 2 files changed, 15 insertions(+), 13 deletions(-) Reviewed-by: Simon Glass
Re: [PATCHv10 15/15] net/lwip: split net.h to net.h, arp.h and eth.h
On Tue, 26 Sept 2023 at 03:46, Maxim Uvarov wrote:
>
> current net.h has ethernet and protocol definitions. Protocol definitions
> overlap with lwIP protocol definitions and net.h can not be included from
> lwIP code. Splitting on logical blocks makes that work.
>
> Signed-off-by: Maxim Uvarov
> ---
> include/net.h | 189 +
> include/net/arp.h | 7 ++
> include/net/eth.h | 190 ++
> 3 files changed, 201 insertions(+), 185 deletions(-)
> create mode 100644 include/net/arp.h
> create mode 100644 include/net/eth.h
Reviewed-by: Simon Glass
Please void typedef
[..]
> -/**
> - * struct eth_ops - functions of Ethernet MAC controllers
> - *
> - * start: Prepare the hardware to send and receive packets
> - * send: Send the bytes passed in "packet" as a packet on the wire
> - * recv: Check if the hardware received a packet. If so, set the pointer to
> the
> - * packet buffer in the packetp parameter. If not, return an error or 0
> to
> - * indicate that the hardware receive FIFO is empty. If 0 is returned,
> the
> - * network stack will not process the empty packet, but free_pkt() will
> be
> - * called if supplied
> - * free_pkt: Give the driver an opportunity to manage its packet buffer
> memory
> - * when the network stack is finished processing it. This will only
> be
> - * called when no error was returned from recv - optional
> - * stop: Stop the hardware from looking for packets - may be called even if
> - * state == PASSIVE
> - * mcast: Join or leave a multicast group (for TFTP) - optional
> - * write_hwaddr: Write a MAC address to the hardware (used to pass it to
> Linux
> - * on some platforms like ARM). This function expects the
> - * eth_pdata::enetaddr field to be populated. The method can
> - * return -ENOSYS to indicate that this is not implemented for
> -this hardware - optional.
> - * read_rom_hwaddr: Some devices have a backup of the MAC address stored in a
> - * ROM on the board. This is how the driver should expose it
> - * to the network stack. This function should fill in the
> - * eth_pdata::enetaddr field - optional
> - * set_promisc: Enable or Disable promiscuous mode
> - * get_sset_count: Number of statistics counters
> - * get_string: Names of the statistic counters
> - * get_stats: The values of the statistic counters
> - */
> -struct eth_ops {
> - int (*start)(struct udevice *dev);
> - int (*send)(struct udevice *dev, void *packet, int length);
> - int (*recv)(struct udevice *dev, int flags, uchar **packetp);
> - int (*free_pkt)(struct udevice *dev, uchar *packet, int length);
> - void (*stop)(struct udevice *dev);
> - int (*mcast)(struct udevice *dev, const u8 *enetaddr, int join);
> - int (*write_hwaddr)(struct udevice *dev);
> - int (*read_rom_hwaddr)(struct udevice *dev);
> - int (*set_promisc)(struct udevice *dev, bool enable);
> - int (*get_sset_count)(struct udevice *dev);
> - void (*get_strings)(struct udevice *dev, u8 *data);
> - void (*get_stats)(struct udevice *dev, u64 *data);
> -};
> -
> -#define eth_get_ops(dev) ((struct eth_ops *)(dev)->driver->ops)
> -
> -struct udevice *eth_get_dev(void); /* get the current device */
> -/*
> - * The devname can be either an exact name given by the driver or device tree
> - * or it can be an alias of the form "eth%d"
> - */
> -struct udevice *eth_get_dev_by_name(const char *devname);
> -unsigned char *eth_get_ethaddr(void); /* get the current device MAC */
> -
> -/* Used only when NetConsole is enabled */
> -int eth_is_active(struct udevice *dev); /* Test device for active state */
> -int eth_init_state_only(void); /* Set active state */
> -void eth_halt_state_only(void); /* Set passive state */
> -
> -int eth_initialize(void); /* Initialize network subsystem */
> -void eth_try_another(int first_restart); /* Change the device */
> -void eth_set_current(void);/* set nterface to ethcur var */
> -
> -int eth_get_dev_index(void); /* get the device index */
> -
> -/**
> - * eth_env_set_enetaddr_by_index() - set the MAC address environment variable
> - *
> - * This sets up an environment variable with the given MAC address
> (@enetaddr).
> - * The environment variable to be set is defined by <@base_name><@index>addr.
> - * If @index is 0 it is omitted. For common Ethernet this means ethaddr,
> - * eth1addr, etc.
> - *
> - * @base_name: Base name for variable, typically "eth"
> - * @index: Index of interface being updated (>=0)
> - * @enetaddr: Pointer to MAC address to put into the variable
> - * Return: 0 if OK, other value on error
> - */
> -int eth_env_set_enetaddr_by_index(const char *base_name, int index,
> -uchar *enetaddr);
> -
> -
>
Re: [PATCH] linker_list: Fix ll_entry_get alignment
Hi Sean,
On Sat, 30 Sept 2023 at 09:23, Sean Anderson wrote:
>
> On 9/30/23 10:36, Sean Anderson wrote:
> > When ll_entry_get is used on a list entry ll_entry_declare'd in the same
> > file, the lack of alignment on the access will override the
> > ll_entry_declare alignment. This causes GCC to use the default section
> > alignment of 32 bytes. As list entries are not necessarily 32-byte aligned,
> > this will cause a gap in the linker list, corrupting further entries.
> >
> > As a specific example, get_fs_loader uses DM_DRIVER_GET(fs_loader) in the
> > same file where U_BOOT_DRIVER(fs_loader) is present. This causes a crash
> > when walking the driver list.
> >
> > Fix this by adding appropriate alignment to all accesses.
> >
> > Fixes: 42ebaae3a33 ("common: Implement support for linker-generated arrays")
> > Signed-off-by: Sean Anderson
> > ---
> >
> > include/linker_lists.h | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linker_lists.h b/include/linker_lists.h
> > index f9a2ee0c762..e0c8a01b9ba 100644
> > --- a/include/linker_lists.h
> > +++ b/include/linker_lists.h
> > @@ -209,7 +209,8 @@
> >*/
> > #define ll_entry_get(_type, _name, _list) \
> > ({ \
> > - extern _type _u_boot_list_2_##_list##_2_##_name;\
> > + extern _type __aligned(4) \
> > + _u_boot_list_2_##_list##_2_##_name; \
> > _type *_ll_result = \
> > &_u_boot_list_2_##_list##_2_##_name;\
> > _ll_result; \
> > @@ -229,7 +230,7 @@
> >* @_list: name of the list
> >*/
> > #define ll_entry_ref(_type, _name, _list) \
> > - ((_type *)&_u_boot_list_2_##_list##_2_##_name)
> > + ((_type __aligned(4) *)&_u_boot_list_2_##_list##_2_##_name)
>
> OK, so this causes an error in clang. And it isn't really necessary
> because the entry is already declared at this point.
>
> So I guess the right fix is to replace DM_DRIVER_GET with DM_DRIVER_REF in
> get_fs_loader. But this seems like a really big footgun. You can use the
> wrong one and there are no errors except at runtime. I wonder if we can add
> a warning of some kind?
I can imagine having a runtime check, something like:
ll_check(sizeof(struct something))
which checks that the linker list (end - start) is a multiple of the
struct size. Do you think that would find the problem?
If so, then it could be perhaps be turned into a link-time check. This
produces a list of the linker lists along with their individual
members:
or ll in $(nm /tmp/b/coreboot/u-boot |grep u_boot_list_2 |sed
's/.*_u_boot_list_2_\(.*\)_2_.*/\1/' |uniq); do echo; echo "linker
list: %ll"; nm /tmp/b/coreboot/u-boot |grep $ll; done
...
linker list: ut_str_test
011a9a20 D _u_boot_list_2_ut_str_test_2_str_dectoul
011a9a30 D _u_boot_list_2_ut_str_test_2_str_hextoul
011a9a40 D _u_boot_list_2_ut_str_test_2_str_itoa
011a9a50 D _u_boot_list_2_ut_str_test_2_str_simple_strtoul
011a9a60 D _u_boot_list_2_ut_str_test_2_str_simple_strtoull
011a9a70 D _u_boot_list_2_ut_str_test_2_str_trailing
011a9a80 D _u_boot_list_2_ut_str_test_2_str_upper
011a9a90 D _u_boot_list_2_ut_str_test_2_str_xtoa
011a9aa0 D _u_boot_list_2_ut_str_test_2_test_str_to_list
...
Then you can check that the address of each one increments by the same amount.
Maybe.
Regards,
Simon
Re: bootstd: Scanning for USB bootflow will remove existing SCSI bootflow
Hi Tony, On Tue, 26 Sept 2023 at 13:40, Tony Dinh wrote: > > Hi Simon, > > On Tue, Sep 26, 2023 at 4:37 AM Simon Glass wrote: > > > > Hi Tony, > > > > On Mon, 25 Sept 2023 at 14:02, Tony Dinh wrote: > > > > > > Hi Simon, > > > > > > Here is an observation during testing the bootflow command. > > > > > > If there is a SCSI bootflow, scanning for USB bootflow will remove that > > > existing > > > SCSI bootflow. To bring it back, I scanned for SCSI bootflow again, and > > > it was > > > back to normal. Perhaps there is some kind of indexing problem? > > > > Yes that's right. The 'botflow scan' command is not additive. The > > first thing it does is removing existing bootflows. > > Thanks for clarifying that. I assumed it is additive, because the > existing USB bootflow was not removed when I did a "bootflow scan > scsi" immediately after (see the end of the log). Yes, but I'm not sure what is going on there. Perhaps it is a bug? When you scan SCSI it should not also scan USB. Regards, SImon
Re: [PATCHv10 12/15] net/lwip: update .gitignore with lwIP
On Tue, 26 Sept 2023 at 03:45, Maxim Uvarov wrote: > > ignore lwIP library code and reused files from the lwIP. > > Signed-off-by: Maxim Uvarov > --- > net/lwip/.gitignore | 8 > 1 file changed, 8 insertions(+) > create mode 100644 net/lwip/.gitignore Reviewed-by: Simon Glass
Re: [PATCHv10 14/15] net/lwip: replace original net commands with lwip
On Tue, 26 Sept 2023 at 03:46, Maxim Uvarov wrote: > > Replace original commands: ping, tftp, dhcp and wget. > > Signed-off-by: Maxim Uvarov > --- > boot/bootmeth_efi.c | 18 +++--- > boot/bootmeth_pxe.c | 21 ++- > cmd/net.c | 86 + > cmd/pxe.c | 19 +- > include/net.h | 8 +++-- > include/net/ulwip.h | 64 + > 6 files changed, 113 insertions(+), 103 deletions(-) > create mode 100644 include/net/ulwip.h Reviewed-by: Simon Glass I don't really understand the error handling. I'm not sure why we have void functions everywhere?
Re: [PATCH v2] dm: core: Report bootph-pre-ram/sram node as pre-reloc after relocation : regression
Hi Massimo,
On Sun, 1 Oct 2023 at 07:29, Massimo Pegorer
wrote:
>
> Il giorno ven 29 set 2023 alle ore 13:23 Roger Quadros
> ha scritto:
> >
> >
> >
> > On 28/09/2023 22:18, Jonas Karlman wrote:
> > > Hi Roger,
> > >
> > > On 2023-09-28 14:59, Roger Quadros wrote:
> > >> Hi,
> > >>
> > >> On 21/08/2023 01:03, Jonas Karlman wrote:
> > >>> Nodes with bootph-pre-sram/ram props are bound in multiple phases:
> > >>> 1. At TPL (bootph-pre-sram) or SPL (bootph-pre-ram) phase
> > >>> 2. At U-Boot proper pre-relocation phase
> > >>> 3. At U-Boot proper normal phase
> > >>>
> > >>> However the binding and U-Boot Driver Model documentation indicate that
> > >>> only nodes marked with bootph-all or bootph-some-ram should be bound in
> > >>> the U-Boot proper pre-relocation phase.
> > >>>
> > >>> Change ofnode_pre_reloc to report a node with bootph-pre-ram/sram prop
> > >>> with a pre-reloc status only after U-Boot proper pre-relocation phase.
> > >>> Also update the ofnode_pre_reloc documentation to closer reflect the
> > >>> binding and driver model documentation.
> > >>>
> > >>> This changes behavior of what nodes are bound in the U-Boot proper
> > >>> pre-relocation phase. Change to bootph-all or add bootph-some-ram prop
> > >>> to restore prior behavior.
> > >>>
> > >>> Signed-off-by: Jonas Karlman
> > >>> Reviewed-by: Simon Glass
> > >>> ---
> > >>> Changes in v2:
> > >>> - Drop use of !! to convert into bool
> > >>> - Update documentation for ofnode_pre_reloc
> > >>> - Rewrite commit message
> > >>> - Collect r-b tag
> > >>
> > >> This patch breaks boot on AM642-EVM. Boot log at the end.
> > >
> > > From what I can tell your board use a lot of bootph-pre-ram.
> > > https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/k3-am642-evm-u-boot.dtsi
> > >
> > > What happens if you change to bootph-all or add bootph-some-ram next to
> > > bootph-pre-ram on devices that is also needed in U-Boot proper
> > > pre-relocation phase in addition to SPL phase?
> >
> > Then it works.
> >
> > BTW, AM62-SK is broken as well and I suppose most K3 TI boards would be
> > broken.
> >
> > Nishanth / Tom,
> >
> > What approach to take here?
> > Replacing bootph-pre-ram to bootph-all in *-u-boot.dtsi would be a quick
> > fix.
>
> An exact quick fix is to add bootph-some-ram next to bootph-pre-ram:
> this will have the same effects of bootph-pre-ram before the patch.
> Instead, replacing bootph-pre-ram with bootph-all will affect also
> other boot phases (e.g. TPL) if any.
Yes.
Since this is in -next we have plenty of time to get things figured
out before it ends up in a release.
Regards,
Simon
>
> Regards,
> Massimo
>
> > Then we need to mark nodes required only for SPL as bootph-pre-ram.
> >
> > Meanwhile I will suggest to revert the $subject patch till this is sorted
> > out
> > and gets a Ack from board maintainers.
> >
> > cheers,
> > -roger
> >
> > >
> > > Regards,
> > > Jonas
> > >
> > >>
> > >>>
> > >>> drivers/core/ofnode.c | 2 +-
> > >>> include/dm/ofnode.h | 8
> > >>> 2 files changed, 5 insertions(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> > >>> index 8df16e56af5c..b1e94b2d60df 100644
> > >>> --- a/drivers/core/ofnode.c
> > >>> +++ b/drivers/core/ofnode.c
> > >>> @@ -1353,7 +1353,7 @@ bool ofnode_pre_reloc(ofnode node)
> > >>> */
> > >>> if (ofnode_read_bool(node, "bootph-pre-ram") ||
> > >>> ofnode_read_bool(node, "bootph-pre-sram"))
> > >>> - return true;
> > >>> + return gd->flags & GD_FLG_RELOC;
> > >>>
> > >>> if (IS_ENABLED(CONFIG_OF_TAG_MIGRATE)) {
> > >>> /* detect and handle old tags */
> > >>> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> > >>> index 0f38b3e736de..13700f8266d7 100644
> > >>> --- a/include/dm/ofnode.h
> > >>> +++ b/include/dm/ofnode.h
> > >>> @@ -1198,15 +1198,15 @@ int ofnode_read_simple_size_cells(ofnode node);
> > >>> * determine if a node was bound in one of SPL/TPL stages.
> > >>> *
> > >>> * There are 4 settings currently in use
> > >>> - * - bootph-some-ram: U-Boot proper pre-relocation only
> > >>> + * - bootph-some-ram: U-Boot proper pre-relocation phase
> > >>> * - bootph-all: all phases
> > >>> * Existing platforms only use it to indicate nodes needed in
> > >>> * SPL. Should probably be replaced by bootph-pre-ram for new
> > >>> platforms.
> > >>> - * - bootph-pre-ram: SPL and U-Boot pre-relocation
> > >>> - * - bootph-pre-sram: TPL and U-Boot pre-relocation
> > >>> + * - bootph-pre-ram: SPL phase
> > >>> + * - bootph-pre-sram: TPL phase
> > >>> *
> > >>> * @node: node to check
> > >>> - * Return: true if node is needed in SPL/TL, false otherwise
> > >>> + * Return: true if node should be or was bound, false otherwise
> > >>> */
> > >>> bool ofnode_pre_reloc(ofnode node);
> > >>>
> > >>
> > >> ---boot log---
> > >>
> > >> U-Boot SPL 2023.10-rc4-00480-g9e644284ab (Sep 28 2023 - 15:53:09 +0300)
> > >> Resetting on cold
Re: [PATCH 5/5] treewide: use dev_read_addr_*_ptr() where appropriate
On Wed, 27 Sept 2023 at 07:34, Matthias Schiffer
wrote:
>
> A follow-up to commit 842fb5de424e
> ("drivers: use devfdt_get_addr_size_index_ptr when cast to pointer")
> and commit 320a1938b6f7
> ("drivers: use devfdt_get_addr_index_ptr when cast to pointer").
>
> In addition to using the *_ptr variants of these functions where the
> address is cast to a pointer, this also changes devfdt_get_addr_*() to
> dev_read_addr_*() in a few places. Some variable and field types are
> changed from fdt_addr_t or phys_addr_t to void* where the cast was
> happening later.
>
> This patch fixes a number of compile warnings when building a 32bit
> U-Boot with CONFIG_PHYS_64BIT=y. In some places, it also fixes error
> handling where the return value of dev_read_addr() etc. was checked for
> NULL instead of FDT_ADDR_T_NONE.
>
> Signed-off-by: Matthias Schiffer
> ---
>
> This seems to work correctly (tested on x86 sandbox and TI AM62x; I have
> not tested the Tegra, Sun4i and BCM drivers), but I have two questions:
>
> It is not entirely clear to me what the difference between
> dev_read_addr_ptr*() and dev_remap_addr*() etc. is, but some drivers mix
> both. Should dev_remap_addr*() be used for __iomem? Is __iomem used
> consistently in U-Boot at all?
Perhaps there is no difference in U-Boot due to virtual and physical
addresses being the same mostly?
>
> Furthermore, can devfdt_get_*() be replaced with dev_read_*()
> unconditionally? Is there any reason why devfdt_get_*() hasn't been
> dropped entirely in a treewide search-and-replace?
Yes you can replace it...dev_read calls the devfdt interface if !OF_LIVE
>
> The k3-sec-proxy change goes on top of my other patch "mailbox:
> k3-sec-proxy: fix error handling for missing scfg in FDT" I submitted
> yesterday.
>
>
> arch/arm/mach-k3/sysfw-loader.c | 16
> drivers/dma/ti/k3-udma.c | 5 ++---
> drivers/gpio/tegra186_gpio.c | 4 ++--
> drivers/mailbox/k3-sec-proxy.c| 18 +-
> drivers/phy/allwinner/phy-sun4i-usb.c | 12 ++--
> drivers/phy/phy-bcm-sr-pcie.c | 4 ++--
> drivers/ram/k3-am654-ddrss.c | 20 ++--
> drivers/ram/k3-ddrss/k3-ddrss.c | 23 ++-
> drivers/soc/ti/k3-navss-ringacc.c | 12 ++--
> 9 files changed, 55 insertions(+), 59 deletions(-)
>
Reviewed-by: Simon Glass
Re: [PATCHv10 08/15] net/lwip: implement wget cmd
On Tue, 26 Sept 2023 at 03:45, Maxim Uvarov wrote: > > U-Boot recently got support for an alternative network stack using LWIP. > Replace wget command with the LWIP variant while keeping the output and > error messages identical. > > Signed-off-by: Maxim Uvarov > --- > include/net/lwip.h | 14 + > net/lwip/Makefile | 1 + > net/lwip/apps/http/Makefile| 6 ++ > net/lwip/apps/http/lwip-wget.c | 105 + > 4 files changed, 126 insertions(+) > create mode 100644 net/lwip/apps/http/Makefile > create mode 100644 net/lwip/apps/http/lwip-wget.c > Reviewed-by: Simon Glass
Re: [PATCHv10 06/15] net/lwip: implement dhcp cmd
Hi Maxim,
On Tue, 26 Sept 2023 at 03:44, Maxim Uvarov wrote:
>
> U-Boot recently got support for an alternative network stack using LWIP.
> Replace dhcp command with the LWIP variant while keeping the output and
> error messages identical.
>
> Signed-off-by: Maxim Uvarov
> ---
> include/net/lwip.h | 12 +
> net/lwip/Makefile | 1 +
> net/lwip/apps/dhcp/lwip-dhcp.c | 85 ++
> 3 files changed, 98 insertions(+)
> create mode 100644 net/lwip/apps/dhcp/lwip-dhcp.c
Reviewed-by: Simon Glass
with nits
>
> diff --git a/include/net/lwip.h b/include/net/lwip.h
> index ab3db1a214..6a8fcef392 100644
> --- a/include/net/lwip.h
> +++ b/include/net/lwip.h
> @@ -17,3 +17,15 @@ int do_lwip_dns(struct cmd_tbl *cmdtp, int flag, int argc,
> * Other value < 0, if error
> */
> int ulwip_dns(char *name, char *varname);
> +
> +/**
> + * ulwip_dhcp() - create the DHCP request to obtain IP address.
> + *
> + * This function creates the DHCP request to obtain IP address. If DHCP
> server
> + * returns file name, this file will be downloaded with tftp. After this
> + * function you need to invoke the polling loop to process network
> communication.
> + *
> + * Returns: 0 if success
> + * Other value < 0, if error
> +*/
> +int ulwip_dhcp(void);
> diff --git a/net/lwip/Makefile b/net/lwip/Makefile
> index 5d8d5527c6..a3a33b7f71 100644
> --- a/net/lwip/Makefile
> +++ b/net/lwip/Makefile
> @@ -63,4 +63,5 @@ obj-$(CONFIG_NET) += lwip-external/src/netif/ethernet.o
> obj-$(CONFIG_NET) += port/if.o
> obj-$(CONFIG_NET) += port/sys-arch.o
>
> +obj-y += apps/dhcp/lwip-dhcp.o
> obj-y += apps/dns/lwip-dns.o
> diff --git a/net/lwip/apps/dhcp/lwip-dhcp.c b/net/lwip/apps/dhcp/lwip-dhcp.c
> new file mode 100644
> index 00..f0b0e26f6e
> --- /dev/null
> +++ b/net/lwip/apps/dhcp/lwip-dhcp.c
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * (C) Copyright 2023 Linaro Ltd.
> + */
> +
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include "lwip/timeouts.h"
> +
> +#include
> +#include
> +
> +#define DHCP_TMO_TIME 500 /* poll for DHCP state change */
> +#define DHCP_TMO_NUM 10 /* number of tries */
> +
> +typedef struct dhcp_priv {
can you drop typedef?
> + int num_tries;
> + struct netif *netif;
> +} dhcp_priv;
> +
> +static void dhcp_tmo(void *arg)
> +{
> + struct dhcp_priv *dpriv = (struct dhcp_priv *)arg;
> + struct netif *netif = dpriv->netif;
> + struct dhcp *dhcp;
> +
> + dhcp = netif_get_client_data(netif,
> LWIP_NETIF_CLIENT_DATA_INDEX_DHCP);
> + if (!dhcp)
> + return;
> +
> + if (dhcp->state == DHCP_STATE_BOUND) {
> + int err = 0;
> +
> + err -= env_set("bootfile", dhcp->boot_file_name);
> + err -= env_set("ipaddr",
> ip4addr_ntoa(>offered_ip_addr));
> + err -= env_set("netmask",
> ip4addr_ntoa(>offered_sn_mask));
> + err -= env_set("serverip",
> ip4addr_ntoa(>server_ip_addr));
> + if (err)
> + log_err("error update envs\n");
but don't you need to return the effort? How does a script know it failed?
I think this function need an error return...if that is not possible
then you need to stash the error somehow so you can report it when
needed.
> + log_info("DHCP client bound to address %s\n",
> ip4addr_ntoa(>offered_ip_addr));
> + free(dpriv);
> + ulwip_exit(err);
> + return;
> + }
> +
> + dpriv->num_tries--;
> + if (dpriv->num_tries < 0) {
> + log_err("DHCP client timeout\n");
> + free(dpriv);
> + ulwip_exit(-1);
> + return;
> + }
> +
> + sys_timeout(DHCP_TMO_TIME, dhcp_tmo, dpriv);
> +}
> +
> +int ulwip_dhcp(void)
> +{
> + struct netif *netif;
> + int eth_idx;
> + struct dhcp_priv *dpriv;
> +
> + dpriv = malloc(sizeof(struct dhcp_priv));
> + if (!dpriv)
> + return -EPERM;
-ENOMEM
> +
> + eth_idx = eth_get_dev_index();
> + if (eth_idx < 0)
> + return -EPERM;
> +
> + netif = netif_get_by_index(eth_idx + 1);
> + if (!netif)
> + return -ENOENT;
> +
> + dpriv->num_tries = DHCP_TMO_NUM;
> + dpriv->netif = netif;
> + sys_timeout(DHCP_TMO_TIME, dhcp_tmo, dpriv);
> +
> + return dhcp_start(netif) ? 0 : -EPERM;
Why -EPERM? Is it a permission error? Can we not get a real error number?
> +}
> --
> 2.30.2
>
Regards,
SImon
Re: [PATCHv10 13/15] net/lwip: connection between cmd and lwip apps
On Tue, 26 Sept 2023 at 03:45, Maxim Uvarov wrote: > > Signed-off-by: Maxim Uvarov > --- > cmd/Makefile | 1 + > cmd/net-lwip.c | 286 + > 2 files changed, 287 insertions(+) > create mode 100644 cmd/net-lwip.c > Reviewed-by: Simon Glass
Re: [PATCH] Revert "fs: ext4: check the minimal partition size to mount"
On Sat, 30 Sept 2023 at 14:42, Sean Anderson wrote: > > This check breaks small partitions (under 1024 blocks) because part_length > is in units of part.blksz and not bytes. Given the purpose of this > function, we really want to make sure the partition is SUPERBLOCK_START + > SUPERBLOCK_SIZE (2048) bytes so we can call ext4_read_superblock without > error. > > The obvious solution is to convert callers from things like > > ext4fs_mount(part_info.size) > > to > > ext4fs_mount(part_info.size * part_info.blksz); > > However, I'm not really a fan of the bloat that would cause, especially > since the error is now suppressed. I think the best course of action here > is to just revert the patch. > > This reverts commit 9905cae65e03335aefcb1ebfab5b7ee62d89f64e. > > Signed-off-by: Sean Anderson > --- > > fs/ext4/ext4_common.c | 4 > 1 file changed, 4 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 13/15] dm: blk: Drop blk_{read,write}_devnum()
On Tue, 26 Sept 2023 at 04:09, Bin Meng wrote:
>
> blk_{read,write}_devnum() are no longer used by anywhere in the
> source tree. Drop them.
>
> Signed-off-by: Bin Meng
> ---
>
> drivers/block/blk-uclass.c | 29 -
> include/blk.h | 26 --
> 2 files changed, 55 deletions(-)
>
Reviewed-by: Simon Glass
Re: [PATCH 08/15] cmd: blkmap: Make map_handlers[] and its .fn static
On Tue, 26 Sept 2023 at 02:52, Bin Meng wrote: > > These are only used in cmd/blkmap.c. > > Signed-off-by: Bin Meng > --- > > cmd/blkmap.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 1/5] core: fix doc comments of dev_read_addr*() and related functions
On Wed, 27 Sept 2023 at 07:34, Matthias Schiffer wrote: > > - The dev_read_addr_name*() family of functions has no "index" argument, > doc comments should refer to "name" > - Specify the error return for several devfdt_get_addr*() functions > > Signed-off-by: Matthias Schiffer > --- > include/dm/fdtaddr.h | 12 ++-- > include/dm/read.h| 6 +++--- > 2 files changed, 9 insertions(+), 9 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH] mkimage: allow internalization of data-position
Hi Lars,
On Thu, 28 Sept 2023 at 03:45, Lars Feyaerts wrote:
>
> Make it possible for data that was externalized using a static external
> position (-p) to be internalized. Enables the ability to convert
> existing FIT images built with -p to be converted to a FIT image where the
> data is internal, to be converted to a FIT image where the data is
> external relative to the end of the FIT (-E) or change the initial
> static external position to a different static external position (-p).
>
> Removing the original external-data-related properties ensures that
> they're not present after conversion. Without this, they would still be
> present in the resulting FIT even if the FIT has been, for example,
> internalized.
>
> Have checkpatch.pl skip warnings for use of fdtdec_* functions in
> tooling; livetree isn't used there.
>
> Signed-off-by: Lars Feyaerts
> ---
>
> scripts/checkpatch.pl | 4 ++--
> tools/fit_image.c | 26 +-
> 2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 62b764f6c38..488d73a0ed7 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2606,8 +2606,8 @@ sub u_boot_line {
> "Possible new uclass - make sure to add a sandbox
> driver, plus a test in test/dm/.c\n" . $herecurr);
> }
>
> - # try to get people to use the livetree API
> - if ($line =~ /^\+.*fdtdec_/) {
> + # try to get people to use the livetree API, except when changing
> tooling
> + if ($line =~ /^\+.*fdtdec_/ && $realfile !~ /^tools\//) {
Please put this in its own patch
> WARN("LIVETREE",
> "Use the livetree API (dev_read_...)\n" . $herecurr);
> }
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> index 9fe69ea0d9f..10f36e93422 100644
> --- a/tools/fit_image.c
> +++ b/tools/fit_image.c
> @@ -616,6 +616,8 @@ err:
> static int fit_import_data(struct image_tool_params *params, const char
> *fname)
> {
> void *fdt, *old_fdt;
> + void *data = NULL;
> + const char *ext_data_prop = NULL;
> int fit_size, new_size, size, data_base;
> int fd;
> struct stat sbuf;
> @@ -659,14 +661,28 @@ static int fit_import_data(struct image_tool_params
> *params, const char *fname)
> int buf_ptr;
> int len;
>
> - buf_ptr = fdtdec_get_int(fdt, node, "data-offset", -1);
> - len = fdtdec_get_int(fdt, node, "data-size", -1);
> - if (buf_ptr == -1 || len == -1)
> + /*
> +* FIT_DATA_OFFSET_PROP and FIT_DATA_POSITION_PROP are never
> both present,
> +* but if they are, prefer FIT_DATA_OFFSET_PROP as it was
> there first
> +*/
> + buf_ptr = fdtdec_get_int(fdt, node, FIT_DATA_POSITION_PROP,
> -1);
> + if (buf_ptr != -1) {
> + ext_data_prop = FIT_DATA_POSITION_PROP;
> + data = old_fdt + buf_ptr;
> + }
> + buf_ptr = fdtdec_get_int(fdt, node, FIT_DATA_OFFSET_PROP, -1);
> + if (buf_ptr != -1) {
> + ext_data_prop = FIT_DATA_OFFSET_PROP;
> + data = old_fdt + data_base + buf_ptr;
> + }
> + len = fdtdec_get_int(fdt, node, FIT_DATA_SIZE_PROP, -1);
> + if (!data || len == -1)
> continue;
> debug("Importing data size %x\n", len);
>
> - ret = fdt_setprop(fdt, node, "data",
> - old_fdt + data_base + buf_ptr, len);
> + ret = fdt_setprop(fdt, node, FIT_DATA_PROP, data, len);
> + ret = fdt_delprop(fdt, node, ext_data_prop);
> +
> if (ret) {
> debug("%s: Failed to write property: %s\n", __func__,
> fdt_strerror(ret));
> --
> 2.34.1
>
This bit:
Reviewed-by: Simon Glass
Can you add some docs about this, e.g. to mkimage.1 ?
Regards,
Simon
Re: [PATCH v3 0/4] malloc: Reduce size by initializing data at runtime
Hi Sean,
On Thu, 28 Sept 2023 at 08:45, Sean Anderson wrote:
>
> In my efforts to get SPL to fit into flash after some changes I made, I
> noticed that av_ is one of the largest variables in SPL. As it turns
> out, we can generate it at runtime, and the code is already there. This
> has the potential to save 1-2k across the board, for some (very) minor
> boot time increase.
>
> This series is based on [1], since this makes checking for SYS_MALLOC_F
> easier. Passing CI at [2].
>
> To measure the boot time difference, I applied the following patch:
>
> ---
> common/board_r.c | 5 +
> common/spl/spl.c | 4
> 2 files changed, 9 insertions(+)
>
> diff --git a/common/board_r.c b/common/board_r.c
> index 58a5986aa54..ca624b20d46 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -194,6 +194,7 @@ static int initr_barrier(void)
> return 0;
> }
>
> +static ulong malloc_begin, malloc_end;
> static int initr_malloc(void)
> {
> ulong malloc_start;
> @@ -208,8 +209,10 @@ static int initr_malloc(void)
> * reserve_noncached().
> */
> malloc_start = gd->relocaddr - TOTAL_MALLOC_LEN;
> + malloc_begin = timer_get_boot_us();
Perhaps this would be better done with bootstage, since then the
timing can be enabled / disabled, and reported along with other
timings.
> mem_malloc_init((ulong)map_sysmem(malloc_start, TOTAL_MALLOC_LEN),
> TOTAL_MALLOC_LEN);
> + malloc_end = timer_get_boot_us();
> gd->flags |= GD_FLG_FULL_MALLOC_INIT;
> return 0;
> }
> @@ -570,6 +573,8 @@ static int dm_announce(void)
>
> static int run_main_loop(void)
> {
> + printf("malloc_init took %luus (%lu %lu)\n", malloc_end -
> malloc_begin,
> + malloc_begin, malloc_end);
> #ifdef CONFIG_SANDBOX
> sandbox_main_loop_init();
> #endif
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index d74acec10b5..b34d1f4b4e6 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -755,7 +755,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> spl_set_bd();
>
> #if defined(CONFIG_SYS_SPL_MALLOC)
> + ulong malloc_begin = timer_get_boot_us();
> mem_malloc_init(SYS_SPL_MALLOC_START, CONFIG_SYS_SPL_MALLOC_SIZE);
> + ulong malloc_end = timer_get_boot_us();
> gd->flags |= GD_FLG_FULL_MALLOC_INIT;
> #endif
> if (!(gd->flags & GD_FLG_SPL_INIT)) {
> @@ -817,6 +819,8 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> spl_image.boot_device = BOOT_DEVICE_NONE;
> board_boot_order(spl_boot_list);
>
> + printf("malloc_init took %luus (%lu %lu)\n", malloc_end -
> malloc_begin,
> + malloc_begin, malloc_end);
debug() ?
> ret = boot_from_devices(_image, spl_boot_list,
> ARRAY_SIZE(spl_boot_list));
> if (ret) {
> --
> 2.25.1
>
> I found that MALLOC_CLEAR_ON_INIT dominated the mem_malloc_init time
> (taking around 150 ms in SPL on my board). After disabling it, I found
> that MALLOC_RUNTIME_INIT took around 5 us on average.
>
> [1] https://lore.kernel.org/u-boot/20230926141514.2101787-1-...@chromium.org/
> [2] https://source.denx.de/u-boot/custodians/u-boot-clk/-/pipelines/17900
>
> Changes in v3:
> - Use CONFIG_IS_ENABLED in conditionals
> - Don't enable SPL_SYS_MALLOC_RUNTIME_INIT if we are short on BSS
>
> Changes in v2:
> - Only mark malloc initialized after mem_malloc_init
> - Fix cALLOc condition
>
> Sean Anderson (4):
> common: Only mark malloc initialized after mem_malloc_init
> malloc: Don't use ifdefs for SYS_MALLOC_DEFAULT_TO_INIT
> malloc: Don't statically initialize av_ if using malloc_init
> malloc: Enable SYS_MALLOC_RUNTIME_INIT by default in SPL
>
> Kconfig | 27 +--
> common/board_r.c | 3 ++-
> common/dlmalloc.c | 16
> 3 files changed, 27 insertions(+), 19 deletions(-)
>
> --
> 2.35.1.1320.gc452695387.dirty
>
REgards,
SImon
Re: [PATCH 15/15] disk: part: Handle blkmap device in print_part_header()
On Tue, 26 Sept 2023 at 04:10, Bin Meng wrote: > > Print out the blkmap device type when showing partition header for > a blkmap device. > > Signed-off-by: Bin Meng > > --- > > disk/part.c | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH 11/15] dm: blk: Rename get_desc() and make it externally visible
On Tue, 26 Sept 2023 at 02:56, Bin Meng wrote: > > get_desc() can be useful outside blk-uclass.c. Let's change it to > an API and make it externally visible. > > Signed-off-by: Bin Meng > --- > > drivers/block/blk-uclass.c | 26 -- > include/blk.h | 12 > 2 files changed, 20 insertions(+), 18 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 07/15] blk: blkmap: Make bind/unbind routines static
On Tue, 26 Sept 2023 at 02:51, Bin Meng wrote: > > These 2 are only used in drivers/block/blkmap.c. > > Signed-off-by: Bin Meng > --- > > drivers/block/blkmap.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Reviewed-by: Simon Glass
[PATCH 32/34] x86: coreboot: Add a command to check and update CMOS RAM
Coreboot tables provide information about the CMOS-RAM checksum. Add a
command which can check and update this.
With this it is possible to adjust CMOS-RAM settings and tidy up the
checksum afterwards.
Signed-off-by: Simon Glass
---
cmd/Kconfig | 11 +++
cmd/x86/Makefile | 1 +
cmd/x86/cbcmos.c | 141 +++
doc/usage/cmd/cbcmos.rst | 42
doc/usage/index.rst | 1 +
test/cmd/coreboot.c | 42
6 files changed, 238 insertions(+)
create mode 100644 cmd/x86/cbcmos.c
create mode 100644 doc/usage/cmd/cbcmos.rst
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 987f7d8cf623..e6977dc91bb2 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2742,6 +2742,17 @@ config CMD_CBSYSINFO
memory by coreboot before jumping to U-Boot. It can be useful for
debugging the beaaviour of coreboot or U-Boot.
+config CMD_CBCMOS
+ bool "cbcmos"
+ depends on X86
+ default y if SYS_COREBOOT
+ help
+ This provides information options to check the CMOS RAM checksum,
+ if present, as well as to update it.
+
+ It is useful when coreboot CMOS-RAM settings must be examined or
+ updated.
+
config CMD_CYCLIC
bool "cyclic - Show information about cyclic functions"
depends on CYCLIC
diff --git a/cmd/x86/Makefile b/cmd/x86/Makefile
index 5f82204c87e6..565e20776755 100644
--- a/cmd/x86/Makefile
+++ b/cmd/x86/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0+
obj-$(CONFIG_CMD_CBSYSINFO) += cbsysinfo.o
+obj-$(CONFIG_CMD_CBCMOS) += cbcmos.o
obj-y += mtrr.o
obj-$(CONFIG_CMD_EXCEPTION) += exception.o
obj-$(CONFIG_USE_HOB) += hob.o
diff --git a/cmd/x86/cbcmos.c b/cmd/x86/cbcmos.c
new file mode 100644
index ..8d2f85fd0ec6
--- /dev/null
+++ b/cmd/x86/cbcmos.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Support for booting from coreboot
+ *
+ * Copyright 2021 Google LLC
+ */
+
+#define LOG_CATEGORY UCLASS_RTC
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+DECLARE_GLOBAL_DATA_PTR;
+
+const struct sysinfo_t *get_table(void)
+{
+ if (!gd->arch.coreboot_table) {
+ printf("No coreboot sysinfo table found\n");
+ return NULL;
+ }
+
+ return _sysinfo;
+}
+
+static int calc_sum(struct udevice *dev, uint start_bit, uint bit_count)
+{
+ uint start_byte = start_bit / 8;
+ uint byte_count = bit_count / 8;
+ int ret, i;
+ uint sum;
+
+ log_debug("Calc sum from %x: %x bytes\n", start_byte, byte_count);
+ sum = 0;
+ for (i = 0; i < bit_count / 8; i++) {
+ ret = rtc_read8(dev, start_bit / 8 + i);
+ if (ret < 0)
+ return ret;
+ sum += ret;
+ }
+
+ return (sum & 0xff) << 8 | (sum & 0xff00) >> 8;
+}
+
+/**
+ * prep_cbcmos() - Prepare for a CMOS-RAM command
+ *
+ * @tab: coreboot table
+ * @devnum: RTC device name to use, or NULL for the first one
+ * @dep: Returns RTC device on success
+ * Return: calculated checksum for CMOS RAM or -ve on error
+ */
+static int prep_cbcmos(const struct sysinfo_t *tab, const char *devname,
+ struct udevice **devp)
+{
+ struct udevice *dev;
+ int ret;
+
+ if (!tab)
+ return CMD_RET_FAILURE;
+ if (devname)
+ ret = uclass_get_device_by_name(UCLASS_RTC, devname, );
+ else
+ ret = uclass_first_device_err(UCLASS_RTC, );
+ if (ret) {
+ printf("Failed to get RTC device: %dE\n", ret);
+ return ret;
+ }
+
+ ret = calc_sum(dev, tab->cmos_range_start,
+ tab->cmos_range_end + 1 - tab->cmos_range_start);
+ if (ret < 0) {
+ printf("Failed to read RTC device: %dE\n", ret);
+ return ret;
+ }
+ *devp = dev;
+
+ return ret;
+}
+
+static int do_cbcmos_check(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ const struct sysinfo_t *tab = get_table();
+ struct udevice *dev;
+ u16 cur, sum;
+ int ret;
+
+ ret = prep_cbcmos(tab, argv[1], );
+ if (ret < 0)
+ return CMD_RET_FAILURE;
+ sum = ret;
+
+ ret = rtc_read16(dev, tab->cmos_checksum_location / 8, );
+ if (ret < 0) {
+ printf("Failed to read RTC device: %dE\n", ret);
+ return CMD_RET_FAILURE;
+ }
+ if (sum != cur) {
+ printf("Checksum %04x error: calculated %04x\n", cur, sum);
+ return CMD_RET_FAILURE;
+ }
+
+ return 0;
+}
+
+static int do_cbcmos_update(struct cmd_tbl *cmdtp, int flag, int argc,
+ char *const argv[])
+{
+ const struct sysinfo_t *tab = get_table();
+ struct udevice *dev;
+ u16 sum;
+ int ret;
+
+ ret = prep_cbcmos(tab,
[PATCH 34/34] x86: Enable RTC command by default
The real-time clock is needed for most X86 systems and it is useful to be able to read from it. Enable the rtc command by default. Signed-off-by: Simon Glass --- cmd/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/Kconfig b/cmd/Kconfig index e6977dc91bb2..080e104f9d76 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -2164,6 +2164,7 @@ config CMD_DATE config CMD_RTC bool "rtc" depends on DM_RTC + default y if X86 help Enable the 'rtc' command for low-level access to RTC devices. -- 2.42.0.582.g8ccd20d70d-goog
[PATCH 29/34] x86: coreboot: Add a test for cbsysinfo command
Add a simple test for this command, checking that coreboot has the
required features.
Signed-off-by: Simon Glass
---
test/cmd/Makefile | 1 +
test/cmd/coreboot.c | 37 +
2 files changed, 38 insertions(+)
create mode 100644 test/cmd/coreboot.c
diff --git a/test/cmd/Makefile b/test/cmd/Makefile
index 0be2cec89018..d6460b2ae5b9 100644
--- a/test/cmd/Makefile
+++ b/test/cmd/Makefile
@@ -14,6 +14,7 @@ endif
obj-y += exit.o mem.o
obj-$(CONFIG_CMD_ADDRMAP) += addrmap.o
obj-$(CONFIG_CMD_BDI) += bdinfo.o
+obj-$(CONFIG_COREBOOT_SYSINFO) += coreboot.o
obj-$(CONFIG_CMD_FDT) += fdt.o
obj-$(CONFIG_CONSOLE_TRUETYPE) += font.o
obj-$(CONFIG_CMD_HISTORY) += history.o
diff --git a/test/cmd/coreboot.c b/test/cmd/coreboot.c
new file mode 100644
index ..277c670c15e9
--- /dev/null
+++ b/test/cmd/coreboot.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Test for coreboot commands
+ *
+ * Copyright 2023 Google LLC
+ * Written by Simon Glass
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * test_cmd_cbsysinfo() - test the cbsysinfo command produces expected output
+ *
+ * This includes ensuring that the coreboot build has the expected options
+ * enabled
+ */
+static int test_cmd_cbsysinfo(struct unit_test_state *uts)
+{
+ ut_assertok(run_command("cbsysinfo", 0));
+ ut_assert_nextlinen("Coreboot table at");
+
+ /* Make sure the linear frame buffer is enabled */
+ ut_assert_skip_to_linen("Framebuffer");
+ ut_assert_nextlinen(" Phys addr");
+
+ ut_assert_skip_to_line("Chrome OS VPD: ");
+ ut_assert_nextlinen("RSDP");
+ ut_assert_nextlinen("Unimpl.");
+ ut_assert_console_end();
+
+ return 0;
+}
+CMD_TEST(test_cmd_cbsysinfo, UT_TESTF_CONSOLE_REC);
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 31/34] x86: coreboot: Enable support for the configuration editor
Enable cedit support along with required options and a simple style.
Signed-off-by: Simon Glass
---
arch/x86/dts/coreboot.dts| 7 +++
configs/coreboot64_defconfig | 2 ++
configs/coreboot_defconfig | 2 ++
3 files changed, 11 insertions(+)
diff --git a/arch/x86/dts/coreboot.dts b/arch/x86/dts/coreboot.dts
index 83beb82f37c7..78ca00d64db4 100644
--- a/arch/x86/dts/coreboot.dts
+++ b/arch/x86/dts/coreboot.dts
@@ -54,5 +54,12 @@
menu-inset = <3>;
menuitem-gap-y = <1>;
};
+
+ cedit-theme {
+ font-size = <30>;
+ menu-inset = <3>;
+ menuitem-gap-y = <1>;
+ menu-title-margin-x = <30>;
+ };
};
};
diff --git a/configs/coreboot64_defconfig b/configs/coreboot64_defconfig
index 42b46ba0d9b9..23b88b55663f 100644
--- a/configs/coreboot64_defconfig
+++ b/configs/coreboot64_defconfig
@@ -18,6 +18,7 @@ CONFIG_SHOW_BOOT_PROGRESS=y
CONFIG_USE_BOOTARGS=y
CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro"
CONFIG_BOOTCOMMAND="bootflow scan -l; if bootflow menu; then cls; bootflow
boot; fi"
+CONFIG_CEDIT=y
CONFIG_PRE_CONSOLE_BUFFER=y
CONFIG_SYS_CONSOLE_INFO_QUIET=y
CONFIG_LOG=y
@@ -47,6 +48,7 @@ CONFIG_TFTP_TSIZE=y
CONFIG_USE_ROOTPATH=y
CONFIG_REGMAP=y
CONFIG_SYSCON=y
+CONFIG_OFNODE_MULTI_TREE=y
# CONFIG_ACPIGEN is not set
CONFIG_SYS_IDE_MAXDEVICE=4
CONFIG_SYS_ATA_DATA_OFFSET=0
diff --git a/configs/coreboot_defconfig b/configs/coreboot_defconfig
index 5433a5ae0c2d..d4e44e00dcab 100644
--- a/configs/coreboot_defconfig
+++ b/configs/coreboot_defconfig
@@ -17,6 +17,7 @@ CONFIG_SHOW_BOOT_PROGRESS=y
CONFIG_USE_BOOTARGS=y
CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro"
CONFIG_BOOTCOMMAND="bootflow scan -l; if bootflow menu; then cls; bootflow
boot; fi"
+CONFIG_CEDIT=y
CONFIG_CONSOLE_RECORD=y
CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
CONFIG_PRE_CONSOLE_BUFFER=y
@@ -44,6 +45,7 @@ CONFIG_TFTP_TSIZE=y
CONFIG_USE_ROOTPATH=y
CONFIG_REGMAP=y
CONFIG_SYSCON=y
+CONFIG_OFNODE_MULTI_TREE=y
# CONFIG_ACPIGEN is not set
CONFIG_SYS_IDE_MAXDEVICE=4
CONFIG_SYS_ATA_DATA_OFFSET=0
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 28/34] x86: CI: Update coreboot
Update to a newer version which supports settings in CMOS RAM.
Signed-off-by: Simon Glass
---
.azure-pipelines.yml | 2 +-
.gitlab-ci.yml | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 421c5018b40b..83491e38f70c 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -255,7 +255,7 @@ stages:
cp images/spi-nor.img \${UBOOT_TRAVIS_BUILD_DIR}/;
fi
if [[ "\${TEST_PY_BD}" == "coreboot" ]]; then
- wget -O -
"https://drive.google.com/uc?id=1uJ2VkUQ8czWFZmhJQ90Tp8V_zrJ6BrBH=download;
|xz -dc >\${UBOOT_TRAVIS_BUILD_DIR}/coreboot.rom;
+ wget -O -
"https://drive.google.com/uc?id=1RWzwnrha-wsj8WgO2XHByo7vttxb62pJ=download;
|xz -dc >\${UBOOT_TRAVIS_BUILD_DIR}/coreboot.rom;
wget -O -
"https://drive.google.com/uc?id=149Cz-5SZXHNKpi9xg6R_5XITWohu348y=download;
>cbfstool;
chmod a+x cbfstool;
./cbfstool \${UBOOT_TRAVIS_BUILD_DIR}/coreboot.rom
add-flat-binary -f \${UBOOT_TRAVIS_BUILD_DIR}/u-boot.bin -n fallback/payload -c
LZMA -l 0x111 -e 0x111;
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 6efbd8021c8c..a9c2e578ce16 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -69,7 +69,7 @@ stages:
fi
- if [[ "${TEST_PY_BD}" == "coreboot" ]]; then
wget -O -
-
"https://drive.google.com/uc?id=1uJ2VkUQ8czWFZmhJQ90Tp8V_zrJ6BrBH=download;
|
+
"https://drive.google.com/uc?id=1RWzwnrha-wsj8WgO2XHByo7vttxb62pJ=download;
|
xz -dc >${UBOOT_TRAVIS_BUILD_DIR}/coreboot.rom;
wget -O -
"https://drive.google.com/uc?id=149Cz-5SZXHNKpi9xg6R_5XITWohu348y=download;
>
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 33/34] x86: coreboot: Allow building an expo for editing CMOS config
Coreboot provides the CMOS layout in the tables it passes to U-Boot.
Use that to build an editor for the CMOS settings.
Signed-off-by: Simon Glass
---
boot/Makefile | 4 +
boot/expo_build_cb.c| 244
cmd/cedit.c | 28
doc/board/coreboot/coreboot.rst | 6 +
doc/develop/cedit.rst | 2 +-
doc/usage/cmd/cbcmos.rst| 3 +
doc/usage/cmd/cedit.rst | 76 ++
include/expo.h | 8 ++
test/cmd/coreboot.c | 34 +
9 files changed, 404 insertions(+), 1 deletion(-)
create mode 100644 boot/expo_build_cb.c
diff --git a/boot/Makefile b/boot/Makefile
index ad608598d298..7fd39833e5b6 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -59,6 +59,10 @@ endif
obj-$(CONFIG_$(SPL_TPL_)EXPO) += expo.o scene.o expo_build.o
obj-$(CONFIG_$(SPL_TPL_)EXPO) += scene_menu.o scene_textline.o
+ifdef CONFIG_COREBOOT_SYSINFO
+obj-$(CONFIG_$(SPL_TPL_)EXPO) += expo_build_cb.o
+endif
+
obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += vbe.o
obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_REQUEST) += vbe_request.o
obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o
diff --git a/boot/expo_build_cb.c b/boot/expo_build_cb.c
new file mode 100644
index ..88b74b090f00
--- /dev/null
+++ b/boot/expo_build_cb.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Building an expo from an FDT description
+ *
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass
+ */
+
+#define LOG_CATEGORY LOGC_EXPO
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * struct build_info - Information to use when building
+ */
+struct build_info {
+ const struct cb_cmos_option_table *tab;
+ struct cedit_priv *priv;
+};
+
+/**
+ * convert_to_title() - Convert text to 'title' format and allocate a string
+ *
+ * Converts "this_is_a_test" to "This is a test" so it looks better
+ *
+ * @text: Text to convert
+ * Return: Allocated string, or NULL if out of memory
+ */
+static char *convert_to_title(const char *text)
+{
+ int len = strlen(text);
+ char *buf, *s;
+
+ buf = malloc(len + 1);
+ if (!buf)
+ return NULL;
+
+ for (s = buf; *text; s++, text++) {
+ if (s == buf)
+ *s = toupper(*text);
+ else if (*text == '_')
+ *s = ' ';
+ else
+ *s = *text;
+ }
+ *s = '\0';
+
+ return buf;
+}
+
+/**
+ * menu_build() - Build a menu and add it to a scene
+ *
+ * See doc/developer/expo.rst for a description of the format
+ *
+ * @info: Build information
+ * @entry: CMOS entry to build a menu for
+ * @scn: Scene to add the menu to
+ * @objp: Returns the object pointer
+ * Returns: 0 if OK, -ENOMEM if out of memory, -EINVAL if there is a format
+ * error, -ENOENT if there is a references to a non-existent string
+ */
+static int menu_build(struct build_info *info,
+ const struct cb_cmos_entries *entry, struct scene *scn,
+ struct scene_obj **objp)
+{
+ struct scene_obj_menu *menu;
+ const void *ptr, *end;
+ uint menu_id;
+ char *title;
+ int ret, i;
+
+ ret = scene_menu(scn, entry->name, 0, );
+ if (ret < 0)
+ return log_msg_ret("men", ret);
+ menu_id = ret;
+
+ title = convert_to_title(entry->name);
+ if (!title)
+ return log_msg_ret("con", -ENOMEM);
+
+ /* Set the title */
+ ret = scene_txt_str(scn, "title", 0, 0, title, NULL);
+ if (ret < 0)
+ return log_msg_ret("tit", ret);
+ menu->title_id = ret;
+
+ end = (void *)info->tab + info->tab->size;
+ for (ptr = (void *)info->tab + info->tab->header_length, i = 0;
+ptr < end; i++) {
+ const struct cb_cmos_enums *enums = ptr;
+ struct scene_menitem *item;
+ uint label;
+
+ ptr += enums->size;
+ if (enums->tag != CB_TAG_OPTION_ENUM ||
+ enums->config_id != entry->config_id)
+ continue;
+
+ ret = scene_txt_str(scn, enums->text, 0, 0, enums->text, NULL);
+ if (ret < 0)
+ return log_msg_ret("tit", ret);
+ label = ret;
+
+ ret = scene_menuitem(scn, menu_id, simple_xtoa(i), 0, 0, label,
+0, 0, 0, );
+ if (ret < 0)
+ return log_msg_ret("mi", ret);
+ item->value = enums->value;
+ }
+ *objp = >obj;
+
+ return 0;
+}
+
+/**
+ * scene_build() - Build a scene and all its objects
+ *
+ * See doc/developer/expo.rst for a description of the format
+ *
+ * @info: Build information
+ * @scn: Scene to add the object to
+ * Returns: 0 if OK, -ENOMEM if out of memory, -EINVAL if there is a
[PATCH 30/34] x86: coreboot: Show the option table
Update the cbsysinfo command to show the contents of the CMOS option
table.
While we are here, add some example output for this command, along with
mention of what the unimplemented tags are.
Signed-off-by: Simon Glass
---
cmd/x86/cbsysinfo.c | 73 ++-
doc/usage/cmd/cbsysinfo.rst | 99 +
test/cmd/coreboot.c | 7 +++
3 files changed, 178 insertions(+), 1 deletion(-)
diff --git a/cmd/x86/cbsysinfo.c b/cmd/x86/cbsysinfo.c
index 84822a3e3211..8c46203e1afa 100644
--- a/cmd/x86/cbsysinfo.c
+++ b/cmd/x86/cbsysinfo.c
@@ -185,6 +185,77 @@ static const char *timestamp_name(uint32_t id)
return "";
}
+static void show_option_vals(const struct cb_cmos_option_table *tab,
+uint id)
+{
+ const void *ptr, *end;
+ bool found = false;
+
+ end = (void *)tab + tab->size;
+ for (ptr = (void *)tab + tab->header_length; ptr < end;) {
+ const struct cb_record *rec = ptr;
+
+ switch (rec->tag) {
+ case CB_TAG_OPTION_ENUM: {
+ const struct cb_cmos_enums *enums = ptr;
+
+ if (enums->config_id == id) {
+ if (!found)
+ printf(" ");
+ printf(" %d:%s", enums->value, enums->text);
+ found = true;
+ }
+ break;
+ }
+ break;
+ case CB_TAG_OPTION_DEFAULTS:
+ case CB_TAG_OPTION_CHECKSUM:
+ case CB_TAG_OPTION:
+ break;
+ default:
+ printf("tag %x\n", rec->tag);
+ break;
+ }
+ ptr += rec->size;
+ }
+}
+
+static void show_option_table(const struct cb_cmos_option_table *tab)
+{
+ const void *ptr, *end;
+
+ print_ptr("option_table", tab);
+ if (!tab->size)
+ return;
+
+ printf(" Bit Len Cfg ID Name\n");
+ end = (void *)tab + tab->size;
+ for (ptr = (void *)tab + tab->header_length; ptr < end;) {
+ const struct cb_record *rec = ptr;
+
+ switch (rec->tag) {
+ case CB_TAG_OPTION: {
+ const struct cb_cmos_entries *entry = ptr;
+
+ printf("%4x %4x %3c %3x %-20s", entry->bit,
+ entry->length, entry->config, entry->config_id,
+ entry->name);
+ show_option_vals(tab, entry->config_id);
+ printf("\n");
+ break;
+ }
+ case CB_TAG_OPTION_ENUM:
+ case CB_TAG_OPTION_DEFAULTS:
+ case CB_TAG_OPTION_CHECKSUM:
+ break;
+ default:
+ printf("tag %x\n", rec->tag);
+ break;
+ }
+ ptr += rec->size;
+ }
+}
+
static void show_table(struct sysinfo_t *info, bool verbose)
{
struct cb_serial *ser = info->serial;
@@ -219,7 +290,7 @@ static void show_table(struct sysinfo_t *info, bool verbose)
printf("%12d: %02x:%-8s %016llx %016llx\n", i, mr->type,
get_mem_name(mr->type), mr->base, mr->size);
}
- print_ptr("option_table", info->option_table);
+ show_option_table(info->option_table);
print_hex("CMOS start", info->cmos_range_start);
if (info->cmos_range_start) {
diff --git a/doc/usage/cmd/cbsysinfo.rst b/doc/usage/cmd/cbsysinfo.rst
index 8c03a85169dc..ea6878e54232 100644
--- a/doc/usage/cmd/cbsysinfo.rst
+++ b/doc/usage/cmd/cbsysinfo.rst
@@ -23,3 +23,102 @@ Example
::
=> cbsysinfo
+Coreboot table at 500, size 5c4, records 1d (dec 29), decoded to
7dce4520, forwarded to 7ff9a000
+
+CPU KHz : 0
+Serial I/O port:
+ base:
+ pointer : 7ff9a370
+ type: 1
+ base: 03f8
+ baud: 0d115200
+ regwidth: 1
+ input_hz: 0d1843200
+ PCI addr: 0010
+Mem ranges : 7
+ id: type || base|| size
+ 0: 10:table 1000
+ 1: 01:ram 1000 0009f000
+ 2: 02:reserved 000a 0006
+ 3: 01:ram 0010 7fe6d000
+ 4: 10:table7ff6d000 00093000
+ 5: 02:reserved fec0 1000
+ 6: 02:reserved ff80 0080
+option_table: 7ff9a018
+ Bit Len Cfg ID Name
+ 0 180r 0 reserved_memory
+ 1801e 4 boot_option
[PATCH 25/34] expo: Drop scene_title_set()
This function is really just an assignment, so serves no useful
purpose. Drop it.
Signed-off-by: Simon Glass
---
boot/expo_build.c | 4 ++--
boot/scene.c | 7 ---
include/expo.h| 9 -
test/boot/expo.c | 2 +-
4 files changed, 3 insertions(+), 19 deletions(-)
diff --git a/boot/expo_build.c b/boot/expo_build.c
index 88b6f2ea7eb4..b2a651a314b6 100644
--- a/boot/expo_build.c
+++ b/boot/expo_build.c
@@ -405,7 +405,7 @@ static int scene_build(struct build_info *info, ofnode
scn_node,
if (ret < 0)
return log_msg_ret("tit", ret);
title_id = ret;
- scene_title_set(scn, title_id);
+ scn->title_id = title_id;
ret = add_txt_str(info, scn_node, scn, "prompt", 0);
if (ret < 0)
@@ -421,7 +421,7 @@ static int scene_build(struct build_info *info, ofnode
scn_node,
return 0;
}
-int build_it(struct build_info *info, ofnode root, struct expo **expp)
+static int build_it(struct build_info *info, ofnode root, struct expo **expp)
{
ofnode scenes, node;
struct expo *exp;
diff --git a/boot/scene.c b/boot/scene.c
index 56569e76e9f2..1369bcda13b8 100644
--- a/boot/scene.c
+++ b/boot/scene.c
@@ -71,13 +71,6 @@ void scene_destroy(struct scene *scn)
free(scn);
}
-int scene_title_set(struct scene *scn, uint id)
-{
- scn->title_id = id;
-
- return 0;
-}
-
int scene_obj_count(struct scene *scn)
{
struct scene_obj *obj;
diff --git a/include/expo.h b/include/expo.h
index bb329b9776dd..74b45606efe6 100644
--- a/include/expo.h
+++ b/include/expo.h
@@ -540,15 +540,6 @@ void scene_set_highlight_id(struct scene *scn, uint id);
*/
int scene_set_open(struct scene *scn, uint id, bool open);
-/**
- * scene_title_set() - set the scene title
- *
- * @scn: Scene to update
- * @title_id: Title ID to set
- * Returns: 0 if OK
- */
-int scene_title_set(struct scene *scn, uint title_id);
-
/**
* scene_obj_count() - Count the number of objects in a scene
*
diff --git a/test/boot/expo.c b/test/boot/expo.c
index 75f31fe66907..c7b8e9ba6749 100644
--- a/test/boot/expo.c
+++ b/test/boot/expo.c
@@ -152,7 +152,7 @@ static int expo_scene(struct unit_test_state *uts)
scn = NULL;
id = scene_new(exp, SCENE_NAME2, 0, );
ut_assertnonnull(scn);
- ut_assertok(scene_title_set(scn, title_id));
+ scn->title_id = title_id;
ut_asserteq(STR_SCENE_TITLE + 1, id);
ut_asserteq(STR_SCENE_TITLE + 2, exp->next_id);
ut_asserteq_ptr(exp, scn->expo);
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 26/34] expo: Add forward declaration for udevice to cedit
Some files may include this header file without first including dm.h
so add a forward declaration.
Signed-off-by: Simon Glass
---
include/cedit.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/cedit.h b/include/cedit.h
index f43cafa5aa24..f9a4a6d9e8eb 100644
--- a/include/cedit.h
+++ b/include/cedit.h
@@ -12,6 +12,7 @@
struct abuf;
struct expo;
struct scene;
+struct udevice;
struct video_priv;
enum {
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 27/34] x86: coreboot: Enable unit tests
Enable unit tests so we can run command-line tests in coreboot. Enable console recording, with enough space for the 'cbsysinfo' command. Add to the pre-relocation malloc() space to make room for this. Signed-off-by: Simon Glass --- configs/coreboot_defconfig | 4 1 file changed, 4 insertions(+) diff --git a/configs/coreboot_defconfig b/configs/coreboot_defconfig index 7d744e9962ae..5433a5ae0c2d 100644 --- a/configs/coreboot_defconfig +++ b/configs/coreboot_defconfig @@ -1,6 +1,7 @@ CONFIG_X86=y CONFIG_TEXT_BASE=0x111 CONFIG_SYS_MALLOC_LEN=0x200 +CONFIG_SYS_MALLOC_F_LEN=0x1000 CONFIG_NR_DRAM_BANKS=8 CONFIG_ENV_SIZE=0x1000 CONFIG_DEFAULT_DEVICE_TREE="coreboot" @@ -16,6 +17,8 @@ CONFIG_SHOW_BOOT_PROGRESS=y CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro" CONFIG_BOOTCOMMAND="bootflow scan -l; if bootflow menu; then cls; bootflow boot; fi" +CONFIG_CONSOLE_RECORD=y +CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000 CONFIG_PRE_CONSOLE_BUFFER=y CONFIG_SYS_CONSOLE_INFO_QUIET=y CONFIG_LOG=y @@ -60,3 +63,4 @@ CONFIG_CONSOLE_SCROLL_LINES=5 CONFIG_CMD_DHRYSTONE=y # CONFIG_GZIP is not set CONFIG_SMBIOS_PARSER=y +CONFIG_UNIT_TEST=y -- 2.42.0.582.g8ccd20d70d-goog
[PATCH 23/34] expo: Support menu-item values in cedit
Update the cedit read/write functions to support menu items with
values.
Signed-off-by: Simon Glass
---
boot/cedit.c| 148 +---
boot/scene_menu.c | 2 +-
doc/usage/cmd/cedit.rst | 15 +++-
test/boot/cedit.c | 8 +-
test/boot/files/expo_layout.dts | 4 +-
5 files changed, 117 insertions(+), 60 deletions(-)
diff --git a/boot/cedit.c b/boot/cedit.c
index cc1d5b763577..7fc800940fbe 100644
--- a/boot/cedit.c
+++ b/boot/cedit.c
@@ -294,11 +294,49 @@ static int get_cur_menuitem_text(const struct
scene_obj_menu *menu,
return 0;
}
+/**
+ * get_cur_menuitem_val() - Get the value of a menu's current item
+ *
+ * Obtains the value of the current item in the menu. If no value, then
+ * enumerates the items of a menu (0, 1, 2) and returns the sequence number of
+ * the currently selected item. If the first item is selected, this returns 0;
+ * if the second, 1; etc.
+ *
+ * @menu: Menu to check
+ * @valp: Returns current-item value / sequence number
+ * Return: 0 on success, else -ve error value
+ */
+static int get_cur_menuitem_val(const struct scene_obj_menu *menu, int *valp)
+{
+ const struct scene_menitem *mi;
+ int seq;
+
+ seq = 0;
+ list_for_each_entry(mi, >item_head, sibling) {
+ if (mi->id == menu->cur_item_id) {
+ *valp = mi->value == INT_MAX ? seq : mi->value;
+ return 0;
+ }
+ seq++;
+ }
+
+ return log_msg_ret("nf", -ENOENT);
+}
+
+/**
+ * write_dt_string() - Write a string to the devicetree, expanding if needed
+ *
+ * If this fails, it tries again after expanding the devicetree a little
+ *
+ * @buf: Buffer containing the devicetree
+ * @name: Property name to use
+ * @str: String value
+ * Return: 0 if OK, -EFAULT if something went horribly wrong
+ */
static int write_dt_string(struct abuf *buf, const char *name, const char *str)
{
int ret, i;
- /* write the text of the current item */
ret = -EAGAIN;
for (i = 0; ret && i < 2; i++) {
ret = fdt_property_string(abuf_data(buf), name, str);
@@ -316,6 +354,38 @@ static int write_dt_string(struct abuf *buf, const char
*name, const char *str)
return 0;
}
+/**
+ * write_dt_u32() - Write an int to the devicetree, expanding if needed
+ *
+ * If this fails, it tries again after expanding the devicetree a little
+ *
+ * @buf: Buffer containing the devicetree
+ * @name: Property name to use
+ * @lva: Integer value
+ * Return: 0 if OK, -EFAULT if something went horribly wrong
+ */
+static int write_dt_u32(struct abuf *buf, const char *name, uint val)
+{
+ int ret, i;
+
+ /* write the text of the current item */
+ ret = -EAGAIN;
+ for (i = 0; ret && i < 2; i++) {
+ ret = fdt_property_u32(abuf_data(buf), name, val);
+ if (!i) {
+ ret = check_space(ret, buf);
+ if (ret)
+ return log_msg_ret("rs2", -ENOMEM);
+ }
+ }
+
+ /* this should not happen */
+ if (ret)
+ return log_msg_ret("str", -EFAULT);
+
+ return 0;
+}
+
static int h_write_settings(struct scene_obj *obj, void *vpriv)
{
struct cedit_iter_priv *priv = vpriv;
@@ -340,23 +410,21 @@ static int h_write_settings(struct scene_obj *obj, void
*vpriv)
const struct scene_obj_menu *menu;
const char *str;
char name[80];
- int i;
+ int val;
/* write the ID of the current item */
menu = (struct scene_obj_menu *)obj;
- ret = -EAGAIN;
- for (i = 0; ret && i < 2; i++) {
- ret = fdt_property_u32(abuf_data(buf), obj->name,
- menu->cur_item_id);
- if (!i) {
- ret = check_space(ret, buf);
- if (ret)
- return log_msg_ret("res", -ENOMEM);
- }
- }
- /* this should not happen */
+ ret = write_dt_u32(buf, obj->name, menu->cur_item_id);
if (ret)
- return log_msg_ret("wrt", -EFAULT);
+ return log_msg_ret("wrt", ret);
+
+ snprintf(name, sizeof(name), "%s-value", obj->name);
+ ret = get_cur_menuitem_val(menu, );
+ if (ret < 0)
+ return log_msg_ret("cur", ret);
+ ret = write_dt_u32(buf, name, val);
+ if (ret)
+ return log_msg_ret("wr2", ret);
ret = get_cur_menuitem_text(menu, );
if (ret)
@@ -522,6 +590,14 @@ static int h_write_settings_env(struct scene_obj *obj,
void *vpriv)
[PATCH 24/34] expo: Drop unneceesary calls to expo_str()
The scene_txt_str() function calls expo_str() so there is no need to
call it beforehand. Drop this unnecessary code.
Signed-off-by: Simon Glass
---
boot/expo_build.c | 16 ++--
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/boot/expo_build.c b/boot/expo_build.c
index 24ec4fc542a7..88b6f2ea7eb4 100644
--- a/boot/expo_build.c
+++ b/boot/expo_build.c
@@ -47,7 +47,6 @@ int add_txt_str(struct build_info *info, ofnode node, struct
scene *scn,
const char *find_name, uint obj_id)
{
const char *text;
- uint str_id;
int ret;
info->err_prop = find_name;
@@ -68,12 +67,7 @@ int add_txt_str(struct build_info *info, ofnode node, struct
scene *scn,
return log_msg_ret("id", -EINVAL);
}
- ret = expo_str(scn->expo, find_name, 0, text);
- if (ret < 0)
- return log_msg_ret("add", ret);
- str_id = ret;
-
- ret = scene_txt_str(scn, find_name, obj_id, str_id, text, NULL);
+ ret = scene_txt_str(scn, find_name, obj_id, 0, text, NULL);
if (ret < 0)
return log_msg_ret("add", ret);
@@ -95,7 +89,6 @@ int add_txt_str_list(struct build_info *info, ofnode node,
struct scene *scn,
const char *find_name, int index, uint obj_id)
{
const char *text;
- uint str_id;
int ret;
ret = ofnode_read_string_index(node, find_name, index, );
@@ -115,12 +108,7 @@ int add_txt_str_list(struct build_info *info, ofnode node,
struct scene *scn,
return log_msg_ret("id", -EINVAL);
}
- ret = expo_str(scn->expo, find_name, 0, text);
- if (ret < 0)
- return log_msg_ret("add", ret);
- str_id = ret;
-
- ret = scene_txt_str(scn, find_name, obj_id, str_id, text, NULL);
+ ret = scene_txt_str(scn, find_name, obj_id, 0, text, NULL);
if (ret < 0)
return log_msg_ret("add", ret);
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 21/34] expo: Allow menu items to have values
At present menu items are stored according to their sequence number in
the menu. In some cases we may want to have holes in that sequence, or
not use a sequence at all.
Add a new 'value' property for menu items. This will be used for
reading and writing, if present. If there is no 'value' property, then
the normal sequence number will be used instead.
Signed-off-by: Simon Glass
---
arch/sandbox/dts/cedit.dtsi | 3 +++
boot/expo_build.c | 16
boot/scene_internal.h | 10 ++
boot/scene_menu.c | 17 +
doc/develop/expo.rst| 10 ++
include/expo.h | 2 ++
test/boot/expo.c| 1 +
test/boot/files/expo_layout.dts | 3 +++
8 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/arch/sandbox/dts/cedit.dtsi b/arch/sandbox/dts/cedit.dtsi
index 9bd84e629367..facd7a49befc 100644
--- a/arch/sandbox/dts/cedit.dtsi
+++ b/arch/sandbox/dts/cedit.dtsi
@@ -39,6 +39,9 @@
/* IDs for the menu items */
item-id = ;
+
+ /* values for the menu items */
+ item-value = <0 3 6>;
};
power-loss {
diff --git a/boot/expo_build.c b/boot/expo_build.c
index d66d619911df..24ec4fc542a7 100644
--- a/boot/expo_build.c
+++ b/boot/expo_build.c
@@ -228,10 +228,10 @@ static void list_strings(struct build_info *info)
static int menu_build(struct build_info *info, ofnode node, struct scene *scn,
uint id, struct scene_obj **objp)
{
+ const u32 *item_ids, *item_values;
struct scene_obj_menu *menu;
+ int ret, size, i, num_items;
uint title_id, menu_id;
- const u32 *item_ids;
- int ret, size, i;
const char *name;
name = ofnode_get_name(node);
@@ -255,9 +255,15 @@ static int menu_build(struct build_info *info, ofnode
node, struct scene *scn,
return log_msg_ret("itm", -EINVAL);
if (!size || size % sizeof(u32))
return log_msg_ret("isz", -EINVAL);
- size /= sizeof(u32);
+ num_items = size / sizeof(u32);
- for (i = 0; i < size; i++) {
+ item_values = ofnode_read_prop(node, "item-value", );
+ if (item_values) {
+ if (size != num_items * sizeof(u32))
+ return log_msg_ret("vsz", -EINVAL);
+ }
+
+ for (i = 0; i < num_items; i++) {
struct scene_menitem *item;
uint label, key, desc;
@@ -281,6 +287,8 @@ static int menu_build(struct build_info *info, ofnode node,
struct scene *scn,
desc, 0, 0, );
if (ret < 0)
return log_msg_ret("mi", ret);
+ if (item_values)
+ item->value = fdt32_to_cpu(item_values[i]);
}
*objp = >obj;
diff --git a/boot/scene_internal.h b/boot/scene_internal.h
index be25f6a8b967..ec9008ea593b 100644
--- a/boot/scene_internal.h
+++ b/boot/scene_internal.h
@@ -281,6 +281,16 @@ struct scene_menitem *scene_menuitem_find(const struct
scene_obj_menu *menu,
struct scene_menitem *scene_menuitem_find_seq(const struct scene_obj_menu
*menu,
uint seq);
+/**
+ * scene_menuitem_find_val() - Find the menu item with a given value
+ *
+ * @menu: Menu to check
+ * @find_val: Value to look for
+ * Return: menu item if found, else NULL
+ */
+struct scene_menitem *scene_menuitem_find_val(const struct scene_obj_menu
*menu,
+ int val);
+
/**
* scene_bbox_union() - update bouding box with the demensions of an object
*
diff --git a/boot/scene_menu.c b/boot/scene_menu.c
index dbf6eacb2983..002821ec1f59 100644
--- a/boot/scene_menu.c
+++ b/boot/scene_menu.c
@@ -62,6 +62,22 @@ struct scene_menitem *scene_menuitem_find_seq(const struct
scene_obj_menu *menu,
return NULL;
}
+struct scene_menitem *scene_menuitem_find_val(const struct scene_obj_menu
*menu,
+ int val)
+{
+ struct scene_menitem *item;
+ uint i;
+
+ i = 0;
+ list_for_each_entry(item, >item_head, sibling) {
+ if (item->value == val)
+ return item;
+ i++;
+ }
+
+ return NULL;
+}
+
/**
* update_pointers() - Update the pointer object and handle highlights
*
@@ -417,6 +433,7 @@ int scene_menuitem(struct scene *scn, uint menu_id, const
char *name, uint id,
item->desc_id = desc_id;
item->preview_id = preview_id;
item->flags = flags;
+ item->value = INT_MAX;
list_add_tail(>sibling, >item_head);
if (itemp)
diff --git a/doc/develop/expo.rst b/doc/develop/expo.rst
index d8115c463c1b..cc7c36173dbe 100644
--- a/doc/develop/expo.rst
+++ b/doc/develop/expo.rst
@@
[PATCH 16/34] video: Add a dark-grey console colour
This is useful for highlighting something with a black background, as
is needed with cedit when using a white-on-black console. Add this as
a new colour.
Signed-off-by: Simon Glass
---
boot/scene.c | 2 +-
drivers/video/video-uclass.c | 3 +++
include/video.h | 1 +
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/boot/scene.c b/boot/scene.c
index d4dfb49ada15..0b37971b1ff4 100644
--- a/boot/scene.c
+++ b/boot/scene.c
@@ -346,7 +346,7 @@ static void scene_render_background(struct scene_obj *obj,
bool box_only)
/* draw a background for the object */
if (CONFIG_IS_ENABLED(SYS_WHITE_ON_BLACK)) {
- fore = VID_BLACK;
+ fore = VID_DARK_GREY;
back = VID_WHITE;
} else {
fore = VID_LIGHT_GRAY;
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index f743ed74c818..51fafbb21747 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -281,6 +281,9 @@ static const struct vid_rgb colours[VID_COLOUR_COUNT] = {
{ 0xff, 0x00, 0xff }, /* bright magenta */
{ 0x00, 0xff, 0xff }, /* bright cyan */
{ 0xff, 0xff, 0xff }, /* white */
+
+ /* an extra one for menus */
+ { 0x40, 0x40, 0x40 }, /* dark gray */
};
u32 video_index_to_colour(struct video_priv *priv, enum colour_idx idx)
diff --git a/include/video.h b/include/video.h
index 4d8df9baaada..ae9ce03f5bfa 100644
--- a/include/video.h
+++ b/include/video.h
@@ -179,6 +179,7 @@ enum colour_idx {
VID_LIGHT_MAGENTA,
VID_LIGHT_CYAN,
VID_WHITE,
+ VID_DARK_GREY,
VID_COLOUR_COUNT
};
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 22/34] expo: Add a little more cedit CMOS logging
Add some more logging in the CMOS read/write code. Tidy up a few
comments while we are here.
Signed-off-by: Simon Glass
---
boot/cedit.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/boot/cedit.c b/boot/cedit.c
index a53a4f289906..cc1d5b763577 100644
--- a/boot/cedit.c
+++ b/boot/cedit.c
@@ -585,7 +585,7 @@ static int h_read_settings_env(struct scene_obj *obj, void
*vpriv)
/*
* note that no validation is done here, to make sure the ID is
-* valid * and actually points to a menu item
+* valid and actually points to a menu item
*/
menu->cur_item_id = val;
break;
@@ -719,6 +719,7 @@ int cedit_write_settings_cmos(struct expo *exp, struct
udevice *dev,
}
/* write the data to the RTC */
+ log_debug("Writing CMOS\n");
first = CMOS_MAX_BYTES;
last = -1;
for (i = 0, count = 0; i < CMOS_MAX_BYTES; i++) {
@@ -786,6 +787,7 @@ static int h_read_settings_cmos(struct scene_obj *obj, void
*vpriv)
}
/* update the current item */
+ log_debug("look for menuitem value %d in menu %d\n", val, menu->obj.id);
mi = scene_menuitem_find_seq(menu, val);
if (!mi)
return log_msg_ret("seq", -ENOENT);
@@ -820,7 +822,7 @@ int cedit_read_settings_cmos(struct expo *exp, struct
udevice *dev,
goto done;
}
- /* read the data to the RTC */
+ /* indicate what bytes were read from the RTC */
first = CMOS_MAX_BYTES;
last = -1;
for (i = 0, count = 0; i < CMOS_MAX_BYTES; i++) {
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 20/34] expo: Use standard numbering for save and discard
Set aside some expo IDs for 'save' and 'discard' buttons. This avoids
needing to store the IDs for these. Adjust the documentation and expo
tool for the new EXPOID_BASE_ID value.
Ignore these objects when saving and loading the cedit, since they do
not contain real data.
Adjust 'cedit run' to return failure when the user exits the expo
without saving. Update the test for this change as well.
Signed-off-by: Simon Glass
---
boot/cedit.c | 24 +---
boot/expo.c| 2 +-
doc/develop/cedit.rst | 7 ++-
doc/develop/expo.rst | 12
include/expo.h | 20
include/test/cedit-test.h | 30 +++---
test/boot/cedit.c | 14 --
test/boot/expo.c | 8
test/boot/files/expo_ids.h | 3 +--
tools/expo.py | 33 -
10 files changed, 112 insertions(+), 41 deletions(-)
diff --git a/boot/cedit.c b/boot/cedit.c
index b5f89e945bce..a53a4f289906 100644
--- a/boot/cedit.c
+++ b/boot/cedit.c
@@ -155,7 +155,7 @@ int cedit_run(struct expo *exp)
struct video_priv *vid_priv;
uint scene_id;
struct scene *scn;
- bool done;
+ bool done, save;
int ret;
cli_ch_init(cch);
@@ -165,6 +165,7 @@ int cedit_run(struct expo *exp)
scene_id = ret;
done = false;
+ save = false;
do {
struct expo_action act;
int ichar, key;
@@ -209,6 +210,15 @@ int cedit_run(struct expo *exp)
case EXPOACT_OPEN:
scene_set_open(scn, act.select.id, true);
cedit_arange(exp, vid_priv, scene_id);
+ switch (scn->highlight_id) {
+ case EXPOID_SAVE:
+ done = true;
+ save = true;
+ break;
+ case EXPOID_DISCARD:
+ done = true;
+ break;
+ }
break;
case EXPOACT_CLOSE:
scene_set_open(scn, act.select.id, false);
@@ -230,6 +240,8 @@ int cedit_run(struct expo *exp)
if (ret)
return log_msg_ret("end", ret);
+ if (!save)
+ return -EACCES;
return 0;
}
@@ -478,6 +490,9 @@ static int h_write_settings_env(struct scene_obj *obj, void
*vpriv)
const char *str;
int val, ret;
+ if (obj->id < EXPOID_BASE_ID)
+ return 0;
+
snprintf(var, sizeof(var), "c.%s", obj->name);
switch (obj->type) {
@@ -550,6 +565,9 @@ static int h_read_settings_env(struct scene_obj *obj, void
*vpriv)
char var[60];
int val;
+ if (obj->id < EXPOID_BASE_ID)
+ return 0;
+
snprintf(var, sizeof(var), "c.%s", obj->name);
switch (obj->type) {
@@ -645,7 +663,7 @@ static int h_write_settings_cmos(struct scene_obj *obj,
void *vpriv)
int val, ret;
uint i, seq;
- if (obj->type != SCENEOBJT_MENU)
+ if (obj->type != SCENEOBJT_MENU || obj->id < EXPOID_BASE_ID)
return 0;
menu = (struct scene_obj_menu *)obj;
@@ -735,7 +753,7 @@ static int h_read_settings_cmos(struct scene_obj *obj, void
*vpriv)
int val, ret;
uint i;
- if (obj->type != SCENEOBJT_MENU)
+ if (obj->type != SCENEOBJT_MENU || obj->id < EXPOID_BASE_ID)
return 0;
menu = (struct scene_obj_menu *)obj;
diff --git a/boot/expo.c b/boot/expo.c
index 20ca0b9bfa0a..2a1591cce6a4 100644
--- a/boot/expo.c
+++ b/boot/expo.c
@@ -30,7 +30,7 @@ int expo_new(const char *name, void *priv, struct expo **expp)
exp->priv = priv;
INIT_LIST_HEAD(>scene_head);
INIT_LIST_HEAD(>str_head);
- exp->next_id = 1;
+ exp->next_id = EXPOID_BASE_ID;
*expp = exp;
diff --git a/doc/develop/cedit.rst b/doc/develop/cedit.rst
index 82305b921f0a..310be8892404 100644
--- a/doc/develop/cedit.rst
+++ b/doc/develop/cedit.rst
@@ -94,7 +94,7 @@ them. Expo supports doing this with an enum, where every ID
is listed in the
enum::
enum {
-ZERO,
+ID_PROMPT = EXPOID_BASE_ID,
ID_PROMPT,
@@ -130,6 +130,11 @@ that means that something is wrong with your syntax, or
perhaps you have an ID
in the `.dts` file that is not mentioned in your enum. Check both files and try
again.
+Note that the first ID in your file must be no less that `EXPOID_BASE_ID` since
+IDs before that are reserved. The `expo.py` tool automatically obtains this
+value from the `expo.h` header file, but you must set the first ID to this
+enum value.
+
Use the command interface
[PATCH 13/34] video: Add a function to clear the display
Move the code from the 'cls' command into the console file, so it can
be called from elsewhere.
Signed-off-by: Simon Glass
---
cmd/cls.c | 25 +++--
common/console.c | 31 +++
include/console.h | 10 ++
3 files changed, 44 insertions(+), 22 deletions(-)
diff --git a/cmd/cls.c b/cmd/cls.c
index 1125a3f81bbb..80d0558d4679 100644
--- a/cmd/cls.c
+++ b/cmd/cls.c
@@ -7,33 +7,14 @@
*/
#include
#include
+#include
#include
-#include
-
-#define CSI "\x1b["
static int do_video_clear(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
{
- __maybe_unused struct udevice *dev;
-
- /*
-* Send clear screen and home
-*
-* FIXME(Heinrich Schuchardt ): This should go
-* through an API and only be written to serial terminals, not video
-* displays
-*/
- printf(CSI "2J" CSI "1;1H");
- if (IS_ENABLED(CONFIG_VIDEO_ANSI))
- return 0;
-
- if (IS_ENABLED(CONFIG_VIDEO)) {
- if (uclass_first_device_err(UCLASS_VIDEO_CONSOLE, ))
- return CMD_RET_FAILURE;
- if (vidconsole_clear_and_reset(dev))
- return CMD_RET_FAILURE;
- }
+ if (console_clear())
+ return CMD_RET_FAILURE;
return CMD_RET_SUCCESS;
}
diff --git a/common/console.c b/common/console.c
index 98c3ee6ca6b8..1ffda49c87e0 100644
--- a/common/console.c
+++ b/common/console.c
@@ -19,12 +19,15 @@
#include
#include
#include
+#include
#include
#include
#include
DECLARE_GLOBAL_DATA_PTR;
+#define CSI "\x1b["
+
static int on_console(const char *name, const char *value, enum env_op op,
int flags)
{
@@ -1010,6 +1013,34 @@ int console_init_f(void)
return 0;
}
+int console_clear(void)
+{
+ /*
+* Send clear screen and home
+*
+* FIXME(Heinrich Schuchardt ): This should go
+* through an API and only be written to serial terminals, not video
+* displays
+*/
+ printf(CSI "2J" CSI "1;1H");
+ if (IS_ENABLED(CONFIG_VIDEO_ANSI))
+ return 0;
+
+ if (IS_ENABLED(CONFIG_VIDEO)) {
+ struct udevice *dev;
+ int ret;
+
+ ret = uclass_first_device_err(UCLASS_VIDEO_CONSOLE, );
+ if (ret)
+ return ret;
+ ret = vidconsole_clear_and_reset(dev);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
static void stdio_print_current_devices(void)
{
char *stdinname, *stdoutname, *stderrname;
diff --git a/include/console.h b/include/console.h
index ceb733b5cb69..e29817e57b00 100644
--- a/include/console.h
+++ b/include/console.h
@@ -156,6 +156,16 @@ int console_announce_r(void);
*/
void console_puts_select_stderr(bool serial_only, const char *s);
+/**
+ * console_clear() - Clear the console
+ *
+ * Uses an ANSI sequence to clear the display, failing back to clearing the
+ * video display directly if !CONFIG_VIDEO_ANSI
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int console_clear(void);
+
/*
* CONSOLE multiplexing.
*/
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 19/34] expo: Set the initial next_id to 1
If expo_set_dynamic_start() is never called, the first scene created
will have an ID of 0, which is invalid. Correct this by setting a
default value.
Add a test to check this.
Signed-off-by: Simon Glass
---
boot/expo.c | 1 +
test/boot/expo.c | 23 +--
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/boot/expo.c b/boot/expo.c
index d030820e77ca..20ca0b9bfa0a 100644
--- a/boot/expo.c
+++ b/boot/expo.c
@@ -30,6 +30,7 @@ int expo_new(const char *name, void *priv, struct expo **expp)
exp->priv = priv;
INIT_LIST_HEAD(>scene_head);
INIT_LIST_HEAD(>str_head);
+ exp->next_id = 1;
*expp = exp;
diff --git a/test/boot/expo.c b/test/boot/expo.c
index 714fdfa415d1..8a84cbc71032 100644
--- a/test/boot/expo.c
+++ b/test/boot/expo.c
@@ -92,7 +92,7 @@ static int expo_base(struct unit_test_state *uts)
*name = '\0';
ut_assertnonnull(exp);
ut_asserteq(0, exp->scene_id);
- ut_asserteq(0, exp->next_id);
+ ut_asserteq(1, exp->next_id);
/* Make sure the name was allocated */
ut_assertnonnull(exp->name);
@@ -131,7 +131,7 @@ static int expo_scene(struct unit_test_state *uts)
ut_assertok(expo_new(EXPO_NAME, NULL, ));
scn = NULL;
- ut_asserteq(0, exp->next_id);
+ ut_asserteq(1, exp->next_id);
strcpy(name, SCENE_NAME1);
id = scene_new(exp, name, SCENE1, );
*name = '\0';
@@ -168,6 +168,25 @@ static int expo_scene(struct unit_test_state *uts)
}
BOOTSTD_TEST(expo_scene, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+/* Check creating a scene with no ID */
+static int expo_scene_no_id(struct unit_test_state *uts)
+{
+ struct scene *scn;
+ struct expo *exp;
+ char name[100];
+ int id;
+
+ ut_assertok(expo_new(EXPO_NAME, NULL, ));
+ ut_asserteq(1, exp->next_id);
+
+ strcpy(name, SCENE_NAME1);
+ id = scene_new(exp, SCENE_NAME1, 0, );
+ ut_asserteq(1, scn->id);
+
+ return 0;
+}
+BOOTSTD_TEST(expo_scene_no_id, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+
/* Check creating a scene with objects */
static int expo_object(struct unit_test_state *uts)
{
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 11/34] test: lmb: Move tests into the lib suite
These tests are marked as driver model tests, but have nothing to do
with driver model. As a result, they are run as part of 'ut dm' which
only exists for sandbox.
Move them to the 'lib' suite and drop the requirement for initing
devices, since they don't use devices.
Also put the lib_test_lmb_max_regions() macro inside the same #ifdef
as its function, to avoid a build error if the condition is false.
Signed-off-by: Simon Glass
---
test/lib/lmb.c | 36
1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/test/lib/lmb.c b/test/lib/lmb.c
index 162887503588..0eccd77114e1 100644
--- a/test/lib/lmb.c
+++ b/test/lib/lmb.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -205,8 +206,7 @@ static int lib_test_lmb_simple(struct unit_test_state *uts)
/* simulate 512 MiB RAM beginning at 1.5GiB */
return test_multi_alloc_512mb(uts, 0xE000);
}
-
-DM_TEST(lib_test_lmb_simple, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+LIB_TEST(lib_test_lmb_simple, 0);
/* Create two memory regions with one reserved region and allocate */
static int lib_test_lmb_simple_x2(struct unit_test_state *uts)
@@ -221,8 +221,7 @@ static int lib_test_lmb_simple_x2(struct unit_test_state
*uts)
/* simulate 512 MiB RAM beginning at 3.5GiB and 1 GiB */
return test_multi_alloc_512mb_x2(uts, 0xE000, 0x4000);
}
-
-DM_TEST(lib_test_lmb_simple_x2, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+LIB_TEST(lib_test_lmb_simple_x2, 0);
/* Simulate 512 MiB RAM, allocate some blocks that fit/don't fit */
static int test_bigblock(struct unit_test_state *uts, const phys_addr_t ram)
@@ -288,8 +287,7 @@ static int lib_test_lmb_big(struct unit_test_state *uts)
/* simulate 512 MiB RAM beginning at 1.5GiB */
return test_bigblock(uts, 0xE000);
}
-
-DM_TEST(lib_test_lmb_big, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+LIB_TEST(lib_test_lmb_big, 0);
/* Simulate 512 MiB RAM, allocate a block without previous reservation */
static int test_noreserved(struct unit_test_state *uts, const phys_addr_t ram,
@@ -364,7 +362,7 @@ static int lib_test_lmb_noreserved(struct unit_test_state
*uts)
return test_noreserved(uts, 0xE000, 4, 1);
}
-DM_TEST(lib_test_lmb_noreserved, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+LIB_TEST(lib_test_lmb_noreserved, 0);
static int lib_test_lmb_unaligned_size(struct unit_test_state *uts)
{
@@ -378,8 +376,8 @@ static int lib_test_lmb_unaligned_size(struct
unit_test_state *uts)
/* simulate 512 MiB RAM beginning at 1.5GiB */
return test_noreserved(uts, 0xE000, 5, 8);
}
+LIB_TEST(lib_test_lmb_unaligned_size, 0);
-DM_TEST(lib_test_lmb_unaligned_size, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
/*
* Simulate a RAM that starts at 0 and allocate down to address 0, which must
* fail as '0' means failure for the lmb_alloc functions.
@@ -421,8 +419,7 @@ static int lib_test_lmb_at_0(struct unit_test_state *uts)
return 0;
}
-
-DM_TEST(lib_test_lmb_at_0, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+LIB_TEST(lib_test_lmb_at_0, 0);
/* Check that calling lmb_reserve with overlapping regions fails. */
static int lib_test_lmb_overlapping_reserve(struct unit_test_state *uts)
@@ -459,9 +456,7 @@ static int lib_test_lmb_overlapping_reserve(struct
unit_test_state *uts)
return 0;
}
-
-DM_TEST(lib_test_lmb_overlapping_reserve,
- UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+LIB_TEST(lib_test_lmb_overlapping_reserve, 0);
/*
* Simulate 512 MiB RAM, reserve 3 blocks, allocate addresses in between.
@@ -590,8 +585,7 @@ static int lib_test_lmb_alloc_addr(struct unit_test_state
*uts)
/* simulate 512 MiB RAM beginning at 1.5GiB */
return test_alloc_addr(uts, 0xE000);
}
-
-DM_TEST(lib_test_lmb_alloc_addr, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+LIB_TEST(lib_test_lmb_alloc_addr, 0);
/* Simulate 512 MiB RAM, reserve 3 blocks, check addresses in between */
static int test_get_unreserved_size(struct unit_test_state *uts,
@@ -661,9 +655,7 @@ static int lib_test_lmb_get_free_size(struct
unit_test_state *uts)
/* simulate 512 MiB RAM beginning at 1.5GiB */
return test_get_unreserved_size(uts, 0xE000);
}
-
-DM_TEST(lib_test_lmb_get_free_size,
- UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+LIB_TEST(lib_test_lmb_get_free_size, 0);
#ifdef CONFIG_LMB_USE_MAX_REGIONS
static int lib_test_lmb_max_regions(struct unit_test_state *uts)
@@ -732,11 +724,9 @@ static int lib_test_lmb_max_regions(struct unit_test_state
*uts)
return 0;
}
+LIB_TEST(lib_test_lmb_max_regions, 0);
#endif
-DM_TEST(lib_test_lmb_max_regions,
- UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-
static int lib_test_lmb_flags(struct unit_test_state *uts)
{
const phys_addr_t ram = 0x4000;
@@ -822,6 +812,4 @@ static int lib_test_lmb_flags(struct unit_test_state *uts)
return 0;
}
-
[PATCH 09/34] test: font: Add dependencies on fonts
The font test needs two fonts. If one is not available, skip out early,
to avoid an error.
Signed-off-by: Simon Glass
---
test/cmd/font.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/test/cmd/font.c b/test/cmd/font.c
index 40682e5ce495..1fe05c1ead51 100644
--- a/test/cmd/font.c
+++ b/test/cmd/font.c
@@ -30,13 +30,17 @@ static int font_test_base(struct unit_test_state *uts)
ut_assertok(console_record_reset_enable());
ut_assertok(run_command("font list", 0));
ut_assert_nextline("nimbus_sans_l_regular");
- ut_assert_nextline("cantoraone_regular");
+ if (IS_ENABLED(CONFIG_CONSOLE_TRUETYPE_CANTORAONE))
+ ut_assert_nextline("cantoraone_regular");
ut_assertok(ut_check_console_end(uts));
ut_assertok(vidconsole_get_font_size(dev, , ));
ut_asserteq_str("nimbus_sans_l_regular", name);
ut_asserteq(18, size);
+ if (!IS_ENABLED(CONFIG_CONSOLE_TRUETYPE_CANTORAONE))
+ return 0;
+
max_metrics = 1;
if (IS_ENABLED(CONFIG_CONSOLE_TRUETYPE))
max_metrics = IF_ENABLED_INT(CONFIG_CONSOLE_TRUETYPE,
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 18/34] expo: Place menu items to the right of all labels
At present a fixed position is used for menu items, 200 pixels to the
right of the left side of the labels. This means that a menu item with
a very long label may overlap the items.
It seems better to calculate the maximum label width and then place the
items to the right of all of them.
To implement this, add a new struct to containing arrangement
information. Calculate it before doing the actual arrangement. Add a
new style item which sets the amount of space from the right side of
the labels to left side of the items.
Signed-off-by: Simon Glass
---
boot/cedit.c | 13 ---
boot/expo.c | 2 ++
boot/scene.c | 52 +--
boot/scene_internal.h | 20 +++--
boot/scene_menu.c | 9 +---
boot/scene_textline.c | 3 ++-
doc/develop/expo.rst | 4
include/expo.h| 12 ++
8 files changed, 104 insertions(+), 11 deletions(-)
diff --git a/boot/cedit.c b/boot/cedit.c
index 8c654dba6dc3..b5f89e945bce 100644
--- a/boot/cedit.c
+++ b/boot/cedit.c
@@ -52,10 +52,11 @@ struct cedit_iter_priv {
int cedit_arange(struct expo *exp, struct video_priv *vpriv, uint scene_id)
{
+ struct expo_arrange_info arr;
struct scene_obj_txt *txt;
struct scene_obj *obj;
struct scene *scn;
- int y;
+ int y, ret;
scn = expo_lookup_scene_id(exp, scene_id);
if (!scn)
@@ -69,6 +70,11 @@ int cedit_arange(struct expo *exp, struct video_priv *vpriv,
uint scene_id)
if (txt)
scene_obj_set_pos(scn, txt->obj.id, 200, 10);
+ memset(, '\0', sizeof(arr));
+ ret = scene_calc_arrange(scn, );
+ if (ret < 0)
+ return log_msg_ret("arr", ret);
+
y = 100;
list_for_each_entry(obj, >obj_head, sibling) {
switch (obj->type) {
@@ -78,12 +84,13 @@ int cedit_arange(struct expo *exp, struct video_priv
*vpriv, uint scene_id)
break;
case SCENEOBJT_MENU:
scene_obj_set_pos(scn, obj->id, 50, y);
- scene_menu_arrange(scn, (struct scene_obj_menu *)obj);
+ scene_menu_arrange(scn, ,
+ (struct scene_obj_menu *)obj);
y += 50;
break;
case SCENEOBJT_TEXTLINE:
scene_obj_set_pos(scn, obj->id, 50, y);
- scene_textline_arrange(scn,
+ scene_textline_arrange(scn, ,
(struct scene_obj_textline *)obj);
y += 50;
break;
diff --git a/boot/expo.c b/boot/expo.c
index cadb6a0ad6e3..d030820e77ca 100644
--- a/boot/expo.c
+++ b/boot/expo.c
@@ -259,6 +259,8 @@ int expo_apply_theme(struct expo *exp, ofnode node)
ofnode_read_u32(node, "font-size", >font_size);
ofnode_read_u32(node, "menu-inset", >menu_inset);
ofnode_read_u32(node, "menuitem-gap-y", >menuitem_gap_y);
+ ofnode_read_u32(node, "menu-title-margin-x",
+ >menu_title_margin_x);
list_for_each_entry(scn, >scene_head, sibling) {
ret = scene_apply_theme(scn, theme);
diff --git a/boot/scene.c b/boot/scene.c
index 0b37971b1ff4..56569e76e9f2 100644
--- a/boot/scene.c
+++ b/boot/scene.c
@@ -478,11 +478,59 @@ static int scene_obj_render(struct scene_obj *obj, bool
text_mode)
return 0;
}
+int scene_calc_arrange(struct scene *scn, struct expo_arrange_info *arr)
+{
+ struct scene_obj *obj;
+
+ arr->label_width = 0;
+ list_for_each_entry(obj, >obj_head, sibling) {
+ uint label_id = 0;
+ int width;
+
+ switch (obj->type) {
+ case SCENEOBJT_NONE:
+ case SCENEOBJT_IMAGE:
+ case SCENEOBJT_TEXT:
+ break;
+ case SCENEOBJT_MENU: {
+ struct scene_obj_menu *menu;
+
+ menu = (struct scene_obj_menu *)obj,
+ label_id = menu->title_id;
+ break;
+ }
+ case SCENEOBJT_TEXTLINE: {
+ struct scene_obj_textline *tline;
+
+ tline = (struct scene_obj_textline *)obj,
+ label_id = tline->label_id;
+ break;
+ }
+ }
+
+ if (label_id) {
+ int ret;
+
+ ret = scene_obj_get_hw(scn, label_id, );
+ if (ret < 0)
+ return log_msg_ret("hei", ret);
+ arr->label_width = max(arr->label_width, width);
+ }
+ }
+
+ return 0;
+}
+
int scene_arrange(struct scene *scn)
{
+ struct expo_arrange_info arr;
struct scene_obj *obj;
int ret;
+
[PATCH 12/34] test: print: Skip test on x86
These tests cannot work on x86 machines as memory at address zero is not writable. Add a condition to skip these. Signed-off-by: Simon Glass --- test/print_ut.c | 8 1 file changed, 8 insertions(+) diff --git a/test/print_ut.c b/test/print_ut.c index b26f6281b013..bb844d2542b7 100644 --- a/test/print_ut.c +++ b/test/print_ut.c @@ -170,6 +170,10 @@ static int print_display_buffer(struct unit_test_state *uts) u8 *buf; int i; + /* This test requires writable memory at zero */ + if (IS_ENABLED(CONFIG_X86)) + return -EAGAIN; + buf = map_sysmem(0, BUF_SIZE); memset(buf, '\0', BUF_SIZE); for (i = 0; i < 0x11; i++) @@ -275,6 +279,10 @@ static int print_do_hex_dump(struct unit_test_state *uts) u8 *buf; int i; + /* This test requires writable memory at zero */ + if (IS_ENABLED(CONFIG_X86)) + return -EAGAIN; + buf = map_sysmem(0, BUF_SIZE); memset(buf, '\0', BUF_SIZE); for (i = 0; i < 0x11; i++) -- 2.42.0.582.g8ccd20d70d-goog
[PATCH 14/34] sandbox: Add a dummy booti command
Add basic sandbox support for 'booti' so we can start to boot the test
ARMbian image. This is helpful in checking that it is parsed correctly.
Signed-off-by: Simon Glass
---
arch/sandbox/lib/bootm.c | 7 +++
cmd/Kconfig | 2 +-
cmd/booti.c | 2 +-
configs/tools-only_defconfig | 3 ++-
4 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/arch/sandbox/lib/bootm.c b/arch/sandbox/lib/bootm.c
index dc8b8e46cb41..a748ba650b12 100644
--- a/arch/sandbox/lib/bootm.c
+++ b/arch/sandbox/lib/bootm.c
@@ -78,3 +78,10 @@ int do_bootm_linux(int flag, int argc, char *argv[], struct
bootm_headers *image
return 0;
}
+
+/* used for testing 'booti' command */
+int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
+ bool force_reloc)
+{
+ return 0;
+}
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 34e1c91a15a9..987f7d8cf623 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -305,7 +305,7 @@ config CMD_BOOTZ
config CMD_BOOTI
bool "booti"
- depends on ARM64 || RISCV
+ depends on ARM64 || RISCV || SANDBOX
default y
help
Boot an AArch64 Linux Kernel image from memory.
diff --git a/cmd/booti.c b/cmd/booti.c
index 6ac39193db80..a20e65196f72 100644
--- a/cmd/booti.c
+++ b/cmd/booti.c
@@ -75,7 +75,7 @@ static int booti_start(struct cmd_tbl *cmdtp, int flag, int
argc,
unmap_sysmem((void *)ld);
ret = booti_setup(ld, _addr, _size, false);
- if (ret != 0)
+ if (ret || IS_ENABLED(CONFIG_SANDBOX))
return 1;
/* Handle BOOTM_STATE_LOADOS */
diff --git a/configs/tools-only_defconfig b/configs/tools-only_defconfig
index 3f588ea69bee..b54d2cefa100 100644
--- a/configs/tools-only_defconfig
+++ b/configs/tools-only_defconfig
@@ -6,8 +6,8 @@ CONFIG_SYS_LOAD_ADDR=0x0
CONFIG_PCI=y
# CONFIG_SANDBOX_SDL is not set
CONFIG_ANDROID_BOOT_IMAGE=y
-CONFIG_FIT=y
CONFIG_TIMESTAMP=y
+CONFIG_FIT=y
CONFIG_FIT_SIGNATURE=y
# CONFIG_BOOTSTD_FULL is not set
# CONFIG_BOOTMETH_CROS is not set
@@ -16,6 +16,7 @@ CONFIG_USE_BOOTCOMMAND=y
CONFIG_BOOTCOMMAND="run distro_bootcmd"
# CONFIG_CMD_BOOTD is not set
# CONFIG_CMD_BOOTM is not set
+# CONFIG_CMD_BOOTI is not set
# CONFIG_CMD_ELF is not set
# CONFIG_CMD_EXTENSION is not set
# CONFIG_CMD_DATE is not set
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 10/34] test: event: Only run test_event_probe() on sandbox
This needs test devices which are only present on sandbox. Add a check
for this and skip just this test if running on a real board.
Signed-off-by: Simon Glass
---
test/common/event.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/test/common/event.c b/test/common/event.c
index c0912a3437bb..b462694fc3b6 100644
--- a/test/common/event.c
+++ b/test/common/event.c
@@ -92,6 +92,9 @@ static int test_event_probe(struct unit_test_state *uts)
struct test_state state;
struct udevice *dev;
+ if (!IS_ENABLED(SANDBOX))
+ return -EAGAIN;
+
state.val = 0;
ut_assertok(event_register("pre", EVT_DM_PRE_PROBE, h_probe, ));
ut_assertok(event_register("post", EVT_DM_POST_PROBE, h_probe, ));
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 17/34] video: Avoid starting a new line to close to the bottom
When starting a new text line, an assumption is made that the current
vertical position is a multiple of the character height. When this is
not true, characters can be written after the end of the framebuffer.
This can causes crashes and strange errors from QEMU.
Adjust the scrolling check when processing a newline character, to
avoid any problems.
Add some comments to make things a little clearer.
Signed-off-by: Simon Glass
---
drivers/video/vidconsole-uclass.c | 4 +++-
include/video.h | 3 ++-
include/video_console.h | 8
3 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/video/vidconsole-uclass.c
b/drivers/video/vidconsole-uclass.c
index a312a1985242..a8d2199147dd 100644
--- a/drivers/video/vidconsole-uclass.c
+++ b/drivers/video/vidconsole-uclass.c
@@ -94,7 +94,9 @@ static void vidconsole_newline(struct udevice *dev)
priv->ycur += priv->y_charsize;
/* Check if we need to scroll the terminal */
- if ((priv->ycur + priv->y_charsize) / priv->y_charsize > priv->rows) {
+ if (vid_priv->rot % 2 ?
+ priv->ycur + priv->x_charsize > vid_priv->xsize :
+ priv->ycur + priv->y_charsize > vid_priv->ysize) {
vidconsole_move_rows(dev, 0, rows, priv->rows - rows);
for (i = 0; i < rows; i++)
vidconsole_set_row(dev, priv->rows - i - 1,
diff --git a/include/video.h b/include/video.h
index ae9ce03f5bfa..e20f1173762e 100644
--- a/include/video.h
+++ b/include/video.h
@@ -78,7 +78,8 @@ enum video_format {
*
* @xsize: Number of pixel columns (e.g. 1366)
* @ysize: Number of pixels rows (e.g.. 768)
- * @rot: Display rotation (0=none, 1=90 degrees clockwise, etc.)
+ * @rot: Display rotation (0=none, 1=90 degrees clockwise, etc.). THis
+ * does not affect @xsize and @ysize
* @bpix: Encoded bits per pixel (enum video_log2_bpp)
* @format:Pixel format (enum video_format)
* @vidconsole_drv_name: Driver to use for the text console, NULL to
diff --git a/include/video_console.h b/include/video_console.h
index bde67fa9a5a9..ecbf183b3d03 100644
--- a/include/video_console.h
+++ b/include/video_console.h
@@ -27,6 +27,14 @@ enum {
* Drivers must set up @rows, @cols, @x_charsize, @y_charsize in their probe()
* method. Drivers may set up @xstart_frac if desired.
*
+ * Note that these values relate to the rotated console, so that an 80x25
+ * console which is rotated 90 degrees will have rows=80 and cols=25
+ *
+ * The xcur_frac and ycur values refer to the unrotated coordinates, that is
+ * xcur_frac always advances with each character, even if its limit might be
+ * vid_priv->ysize instead of vid_priv->xsize if the console is rotated 90 or
+ * 270 degrees.
+ *
* @sdev: stdio device, acting as an output sink
* @xcur_frac: Current X position, in fractional units (VID_TO_POS(x))
* @ycur: Current Y position in pixels (0=top)
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 15/34] bootstd: Add a menu option to bootflow scan
Allow showing a menu and automatically booting, with 'bootflow scan'.
This is more convenient than using a script.
Signed-off-by: Simon Glass
---
cmd/bootflow.c | 27 +++--
doc/usage/cmd/bootflow.rst | 5 +++
test/boot/bootflow.c | 82 ++
3 files changed, 110 insertions(+), 4 deletions(-)
diff --git a/cmd/bootflow.c b/cmd/bootflow.c
index 1516f8a4193b..e4ed4ab83d9d 100644
--- a/cmd/bootflow.c
+++ b/cmd/bootflow.c
@@ -135,7 +135,7 @@ static int do_bootflow_scan(struct cmd_tbl *cmdtp, int
flag, int argc,
struct udevice *dev = NULL;
struct bootflow bflow;
bool all = false, boot = false, errors = false, no_global = false;
- bool list = false, no_hunter = false;
+ bool list = false, no_hunter = false, menu = false, text_mode = false;
int num_valid = 0;
const char *label = NULL;
bool has_args;
@@ -155,6 +155,8 @@ static int do_bootflow_scan(struct cmd_tbl *cmdtp, int
flag, int argc,
no_global = strchr(argv[1], 'G');
list = strchr(argv[1], 'l');
no_hunter = strchr(argv[1], 'H');
+ menu = strchr(argv[1], 'm');
+ text_mode = strchr(argv[1], 't');
argc--;
argv++;
}
@@ -213,15 +215,32 @@ static int do_bootflow_scan(struct cmd_tbl *cmdtp, int
flag, int argc,
}
if (list)
show_bootflow(i, , errors);
- if (boot && !bflow.err)
+ if (!menu && boot && !bflow.err)
bootflow_run_boot(, );
}
bootflow_iter_uninit();
if (list)
show_footer(i, num_valid);
- if (IS_ENABLED(CONFIG_CMD_BOOTFLOW_FULL) && !num_valid && !list)
- printf("No bootflows found; try again with -l\n");
+ if (IS_ENABLED(CONFIG_CMD_BOOTFLOW_FULL) && IS_ENABLED(CONFIG_EXPO)) {
+ if (!num_valid && !list) {
+ printf("No bootflows found; try again with -l\n");
+ } else if (menu) {
+ struct bootflow *sel_bflow;
+
+ ret = bootflow_handle_menu(std, text_mode, _bflow);
+ if (!ret && boot) {
+ ret = console_clear();
+ if (ret) {
+ log_err("Failed to clear console:
%dE\n",
+ ret);
+ return ret;
+ }
+
+ bootflow_run_boot(NULL, sel_bflow);
+ }
+ }
+ }
return 0;
}
diff --git a/doc/usage/cmd/bootflow.rst b/doc/usage/cmd/bootflow.rst
index 6a0645c28d07..795d5990bdc3 100644
--- a/doc/usage/cmd/bootflow.rst
+++ b/doc/usage/cmd/bootflow.rst
@@ -52,6 +52,8 @@ Flags are:
matters, since by then the system boots in the OS and U-Boot is no-longer
running. `bootflow scan -b` is a quick way to boot the first available OS.
A valid bootflow is one that made it all the way to the `loaded` state.
+Note that if `-m` is provided as well, booting is delayed until the user
+selects a bootflow.
-e
Used with -l to also show errors for each bootflow. The shows detailed
error
@@ -71,6 +73,9 @@ Flags are:
priority or label is tried, to see if more bootdevs can be discovered, but
this flag disables that process.
+-m
+Show a menu of available bootflows for the user to select. When used with
+-b it then boots the one that was selected, if any.
The optional argument specifies a particular bootdev to scan. This can either
be
the name of a bootdev or its sequence number (both shown with `bootdev list`).
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 04a5bd5d7a80..a57f5017364a 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -637,6 +637,88 @@ static int bootflow_cmd_menu(struct unit_test_state *uts)
}
BOOTSTD_TEST(bootflow_cmd_menu, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
+/* Check 'bootflow scan -m' to select a bootflow using a menu */
+static int bootflow_scan_menu(struct unit_test_state *uts)
+{
+ struct bootstd_priv *std;
+ const char **old_order, **new_order;
+ char prev[3];
+
+ /* get access to the current bootflow */
+ ut_assertok(bootstd_get_priv());
+
+ ut_assertok(prep_mmc_bootdev(uts, "mmc4", false, _order));
+
+ /* Add keypresses to move to and select the second one in the list */
+ prev[0] = CTL_CH('n');
+ prev[1] = '\r';
+ prev[2] = '\0';
+ ut_asserteq(2, console_in_puts(prev));
+
+ ut_assertok(run_command("bootflow scan -lm", 0));
+ new_order = std->bootdev_order;
+ std->bootdev_order = old_order;
+
+ ut_assert_skip_to_line("No more
[PATCH 05/34] test: Run bootstd tests only on sandbox
These make use of disk images which are not available on reak boards.
Add a new Kconfig to ensure these tests only run where they are valid.
Signed-off-by: Simon Glass
---
test/Kconfig | 5 +
test/Makefile | 2 +-
test/cmd_ut.c | 2 +-
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/test/Kconfig b/test/Kconfig
index 69d63a5ff7dd..022395e489bb 100644
--- a/test/Kconfig
+++ b/test/Kconfig
@@ -64,6 +64,11 @@ config UT_LIB_RSA
endif
+config UT_BOOTSTD
+ bool "Unit tests for standard boot"
+ depends on UNIT_TEST && SANDBOX
+ default y
+
config UT_COMPRESSION
bool "Unit test for compression"
depends on UNIT_TEST
diff --git a/test/Makefile b/test/Makefile
index 178773647a82..4e179cb3930a 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -26,7 +26,7 @@ obj-$(CONFIG_UT_TIME) += time_ut.o
obj-y += ut.o
ifeq ($(CONFIG_SPL_BUILD),)
-obj-$(CONFIG_UNIT_TEST) += boot/
+obj-$(CONFIG_$(SPL_)UT_BOOTSTD) += boot/
obj-$(CONFIG_UNIT_TEST) += common/
obj-y += log/
obj-$(CONFIG_$(SPL_)UT_UNICODE) += unicode_ut.o
diff --git a/test/cmd_ut.c b/test/cmd_ut.c
index d3505fe8d97e..f74930a7ecbe 100644
--- a/test/cmd_ut.c
+++ b/test/cmd_ut.c
@@ -57,7 +57,7 @@ static struct cmd_tbl cmd_ut_sub[] = {
#ifdef CONFIG_CMD_BDI
U_BOOT_CMD_MKENT(bdinfo, CONFIG_SYS_MAXARGS, 1, do_ut_bdinfo, "", ""),
#endif
-#ifdef CONFIG_BOOTSTD
+#ifdef CONFIG_UT_BOOTSTD
U_BOOT_CMD_MKENT(bootstd, CONFIG_SYS_MAXARGS, 1, do_ut_bootstd,
"", ""),
#endif
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 06/34] test: Handle use of stack pointer in bdinfo
This test assumes that the stack pointer is the same across two calls
to lmb_init_and_reserve() but this is not the case on x86, for example.
Add a special case to handle this, along with a detailed comment.
Signed-off-by: Simon Glass
---
test/cmd/bdinfo.c | 12
1 file changed, 12 insertions(+)
diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c
index 8c09281cac0d..0f09a5a3e1db 100644
--- a/test/cmd/bdinfo.c
+++ b/test/cmd/bdinfo.c
@@ -114,6 +114,18 @@ static int lmb_test_dump_region(struct unit_test_state
*uts,
end = base + size - 1;
flags = rgn->region[i].flags;
+ /*
+* this entry includes the stack (get_sp()) on many platforms
+* so will different each time lmb_init_and_reserve() is called.
+* We could instead have the bdinfo command put its lmb region
+* in a known location, so we can check it directly, rather than
+* calling lmb_init_and_reserve() to create a new (and hopefully
+* identical one). But for now this seems good enough.
+*/
+ if (!IS_ENABLED(CONFIG_SANDBOX) && i == 3) {
+ ut_assert_nextlinen(" %s[%d]\t[", name, i);
+ continue;
+ }
ut_assert_nextline(" %s[%d]\t[0x%llx-0x%llx], 0x%08llx bytes
flags: %x",
name, i, base, end, size, flags);
}
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 08/34] test: fdt: Add a special case for real boards
The error that this test checks for is only shown on sandbox. For real
boards, there is normally no error. Add a special case to handle this.
Signed-off-by: Simon Glass
---
test/cmd/fdt.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
index 1f103a1d7eb4..547085521758 100644
--- a/test/cmd/fdt.c
+++ b/test/cmd/fdt.c
@@ -160,7 +160,13 @@ static int fdt_test_addr(struct unit_test_state *uts)
set_working_fdt_addr(0);
ut_assert_nextline("Working FDT set to 0");
ut_asserteq(CMD_RET_FAILURE, run_command("fdt addr", 0));
- ut_assert_nextline("libfdt fdt_check_header(): FDT_ERR_BADMAGIC");
+
+ /*
+* sandbox fails the check for !blob since the 0 pointer is mapped to
+* memory somewhere other than at 0x0
+*/
+ if (IS_ENABLED(CONFIG_SANDBOX))
+ ut_assert_nextline("libfdt fdt_check_header():
FDT_ERR_BADMAGIC");
ut_assertok(ut_check_console_end(uts));
/* Set up a working FDT and try again */
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 04/34] test: Make UT_LIB_ASN1 depend on sandbox
This doesn't seem to work on a real board, so use the test on sandbox only. Signed-off-by: Simon Glass --- test/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Kconfig b/test/Kconfig index 830245b6f9a9..69d63a5ff7dd 100644 --- a/test/Kconfig +++ b/test/Kconfig @@ -31,6 +31,7 @@ if UT_LIB config UT_LIB_ASN1 bool "Unit test for asn1 compiler and decoder function" + depends on SANDBOX default y imply ASYMMETRIC_KEY_TYPE imply ASYMMETRIC_PUBLIC_KEY_SUBTYPE -- 2.42.0.582.g8ccd20d70d-goog
[PATCH 07/34] test: bdinfo: Add missing asserts
Calling into sub-test functions should be done using ut_assertok() so
that the test exits immediately on failure. Add those which are
missing.
Signed-off-by: Simon Glass
---
test/cmd/bdinfo.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c
index 0f09a5a3e1db..9ada7523fc27 100644
--- a/test/cmd/bdinfo.c
+++ b/test/cmd/bdinfo.c
@@ -136,8 +136,8 @@ static int lmb_test_dump_region(struct unit_test_state *uts,
static int lmb_test_dump_all(struct unit_test_state *uts, struct lmb *lmb)
{
ut_assert_nextline("lmb_dump_all:");
- lmb_test_dump_region(uts, >memory, "memory");
- lmb_test_dump_region(uts, >reserved, "reserved");
+ ut_assertok(lmb_test_dump_region(uts, >memory, "memory"));
+ ut_assertok(lmb_test_dump_region(uts, >reserved, "reserved"));
return 0;
}
@@ -188,7 +188,7 @@ static int bdinfo_test_move(struct unit_test_state *uts)
ut_assertok(test_num_l(uts, "fdt_size", (ulong)gd->fdt_size));
if (IS_ENABLED(CONFIG_VIDEO))
- test_video_info(uts);
+ ut_assertok(test_video_info(uts));
/* The gd->multi_dtb_fit may not be available, hence, #if below. */
#if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
@@ -199,7 +199,7 @@ static int bdinfo_test_move(struct unit_test_state *uts)
struct lmb lmb;
lmb_init_and_reserve(, gd->bd, (void *)gd->fdt_blob);
- lmb_test_dump_all(uts, );
+ ut_assertok(lmb_test_dump_all(uts, ));
if (IS_ENABLED(CONFIG_OF_REAL))
ut_assert_nextline("devicetree = %s",
fdtdec_get_srcname());
}
@@ -224,6 +224,9 @@ static int bdinfo_test_move(struct unit_test_state *uts)
ut_assertok(test_num_l(uts, "malloc base", gd_malloc_start()));
}
+ if (IS_ENABLED(CONFIG_X86))
+ ut_check_skip_to_linen(uts, " high end =");
+
ut_assertok(ut_check_console_end(uts));
return 0;
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 01/34] test: Run tests that don't need devices
Tests without the UT_TESTF_SCAN_FDT flag are currently skipped unless OF_LIVE is enabled. Drop this condition, so that they can run on non-sandbox builds, which often don't have OF_LIVE enabled. Signed-off-by: Simon Glass --- test/test-main.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/test-main.c b/test/test-main.c index 778bf0a18a0f..2a3b2ba364a8 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -476,8 +476,7 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, * (for sandbox we handle this by copying the tree, but not for other *boards) */ - if ((test->flags & UT_TESTF_SCAN_FDT) && - !(test->flags & UT_TESTF_LIVE_TREE) && + if (!(test->flags & UT_TESTF_LIVE_TREE) && (CONFIG_IS_ENABLED(OFNODE_MULTI_TREE) || !(test->flags & UT_TESTF_OTHER_FDT)) && (!runs || ut_test_run_on_flattree(test)) && -- 2.42.0.582.g8ccd20d70d-goog
[PATCH 02/34] test: Add a new suite for commands
Add a new suite for 'cmd' tests, used for testing commands. These are
kept in the test/cmd directory.
For now it is empty, but it will be used for coreboot-command tests.
Signed-off-by: Simon Glass
---
include/test/cmd.h| 15 +++
include/test/suites.h | 1 +
test/cmd/Makefile | 2 ++
test/cmd/cmd_ut_cmd.c | 21 +
test/cmd_ut.c | 6 ++
5 files changed, 45 insertions(+)
create mode 100644 include/test/cmd.h
create mode 100644 test/cmd/cmd_ut_cmd.c
diff --git a/include/test/cmd.h b/include/test/cmd.h
new file mode 100644
index ..c200570e423d
--- /dev/null
+++ b/include/test/cmd.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright 2023 Google LLC
+ * Written by Simon Glass
+ */
+
+#ifndef __TEST_CMD_H__
+#define __TEST_CMD_H__
+
+#include
+
+/* Declare a new command test */
+#define CMD_TEST(_name, _flags) UNIT_TEST(_name, _flags, cmd_test)
+
+#endif /* __TEST_CMD_H__ */
diff --git a/include/test/suites.h b/include/test/suites.h
index 1c7dc65966a1..2d4897d46b8e 100644
--- a/include/test/suites.h
+++ b/include/test/suites.h
@@ -34,6 +34,7 @@ int do_ut_bootstd(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[]);
int do_ut_bloblist(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[]);
+int do_ut_cmd(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
int do_ut_common(struct cmd_tbl *cmdtp, int flag, int argc, char *const
argv[]);
int do_ut_compression(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[]);
diff --git a/test/cmd/Makefile b/test/cmd/Makefile
index 8d70ac510a53..0be2cec89018 100644
--- a/test/cmd/Makefile
+++ b/test/cmd/Makefile
@@ -3,6 +3,8 @@
# Copyright (c) 2013 Google, Inc
# Copyright 2022-2023 Arm Limited and/or its affiliates
+obj-y += cmd_ut_cmd.o
+
ifdef CONFIG_HUSH_PARSER
obj-$(CONFIG_CONSOLE_RECORD) += test_echo.o
endif
diff --git a/test/cmd/cmd_ut_cmd.c b/test/cmd/cmd_ut_cmd.c
new file mode 100644
index ..1a5bfdb4a183
--- /dev/null
+++ b/test/cmd/cmd_ut_cmd.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2023 Google LLC
+ * Written by Simon Glass
+ *
+ * Unit tests for command functions
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+int do_ut_cmd(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
+{
+ struct unit_test *tests = UNIT_TEST_SUITE_START(cmd_test);
+ const int n_ents = UNIT_TEST_SUITE_COUNT(cmd_test);
+
+ return cmd_ut_category("cmd", "cmd_test_", tests, n_ents, argc, argv);
+}
diff --git a/test/cmd_ut.c b/test/cmd_ut.c
index 0f56409e8031..d3505fe8d97e 100644
--- a/test/cmd_ut.c
+++ b/test/cmd_ut.c
@@ -60,6 +60,9 @@ static struct cmd_tbl cmd_ut_sub[] = {
#ifdef CONFIG_BOOTSTD
U_BOOT_CMD_MKENT(bootstd, CONFIG_SYS_MAXARGS, 1, do_ut_bootstd,
"", ""),
+#endif
+#ifdef CONFIG_CMDLINE
+ U_BOOT_CMD_MKENT(cmd, CONFIG_SYS_MAXARGS, 1, do_ut_cmd, "", ""),
#endif
U_BOOT_CMD_MKENT(common, CONFIG_SYS_MAXARGS, 1, do_ut_common, "", ""),
#if defined(CONFIG_UT_DM)
@@ -188,6 +191,9 @@ static char ut_help_text[] =
#ifdef CONFIG_BOOTSTD
"\nbootstd - standard boot implementation"
#endif
+#ifdef CONFIG_CMDLINE
+ "\ncmd - test various commands"
+#endif
#ifdef CONFIG_SANDBOX
"\ncompression - compressors and bootm decompression"
#endif
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 03/34] test: Add helper to skip to partial console line
Sometimes we need to skip to a line but it includes addresses or other
information which can vary depending on the runtime conditions.
Add a new ut_assert_skip_to_linen() which is similar to the existing
ut_assert_skip_to_line() function but only checks that the console line
matches up to the length of the provided string.
Signed-off-by: Simon Glass
---
include/test/ut.h | 30 ++
test/ut.c | 27 +++
2 files changed, 57 insertions(+)
diff --git a/include/test/ut.h b/include/test/ut.h
index ea6ee95d7344..d3172af8083f 100644
--- a/include/test/ut.h
+++ b/include/test/ut.h
@@ -97,6 +97,23 @@ int ut_check_skipline(struct unit_test_state *uts);
*/
int ut_check_skip_to_line(struct unit_test_state *uts, const char *fmt, ...);
+/**
+ * ut_check_skip_to_linen() - skip output until a partial line is found
+ *
+ * This creates a string and then checks it against the following lines of
+ * console output obtained with console_record_readline() until it is found.
+ * Only the characters up to the length of the string are checked, so the line
+ * may extend further
+ *
+ * After the function returns, uts->expect_str holds the expected string and
+ * uts->actual_str holds the actual string read from the console.
+ *
+ * @uts: Test state
+ * @fmt: printf() format string to look for, followed by args
+ * Return: 0 if OK, -ENOENT if not found, other value on error
+ */
+int ut_check_skip_to_linen(struct unit_test_state *uts, const char *fmt, ...);
+
/**
* ut_check_console_end() - Check there is no more console output
*
@@ -359,6 +376,19 @@ int ut_check_console_dump(struct unit_test_state *uts, int
total_bytes);
__ret; \
})
+/* Assert that a following console output line matches */
+#define ut_assert_skip_to_linen(fmt, args...) ({
\
+ int __ret = 0; \
+ \
+ if (ut_check_skip_to_linen(uts, fmt, ##args)) { \
+ ut_failf(uts, __FILE__, __LINE__, __func__, \
+"console", "\nExpected '%s',\n got to '%s'", \
+uts->expect_str, uts->actual_str); \
+ return CMD_RET_FAILURE; \
+ } \
+ __ret; \
+})
+
/* Assert that there is no more console output */
#define ut_assert_console_end() ({ \
int __ret = 0; \
diff --git a/test/ut.c b/test/ut.c
index 28da417686e4..628e9dc98050 100644
--- a/test/ut.c
+++ b/test/ut.c
@@ -121,6 +121,33 @@ int ut_check_skipline(struct unit_test_state *uts)
return 0;
}
+int ut_check_skip_to_linen(struct unit_test_state *uts, const char *fmt, ...)
+{
+ va_list args;
+ int len;
+ int ret;
+
+ va_start(args, fmt);
+ len = vsnprintf(uts->expect_str, sizeof(uts->expect_str), fmt, args);
+ va_end(args);
+ if (len >= sizeof(uts->expect_str)) {
+ ut_fail(uts, __FILE__, __LINE__, __func__,
+ "unit_test_state->expect_str too small");
+ return -EOVERFLOW;
+ }
+ while (1) {
+ if (!console_record_avail())
+ return -ENOENT;
+ ret = readline_check(uts);
+ if (ret < 0)
+ return ret;
+
+ if (!strncmp(uts->expect_str, uts->actual_str,
+strlen(uts->expect_str)))
+ return 0;
+ }
+}
+
int ut_check_skip_to_line(struct unit_test_state *uts, const char *fmt, ...)
{
va_list args;
--
2.42.0.582.g8ccd20d70d-goog
[PATCH 00/34] x86: expo: Add support for editing coreboot CMOS RAM settings
U-Boot provides support for editing settings with an 'expo', as well as reading and writing settings to CMOS RAM. This series integrates expo functionality with coreboot, using the sysinfo table to get a list of settings, creating an expo with them and allowing them to be edited. A new CI test provides coverage for some coreboot-related commands. For this to work, a number of minor tweaks are made to existing tests, so that they pass on coreboot (x86) or at least are skipped when they cannot pass. Likely most of these fixes will apply to other boards too. It includes several other fixes and improvements: - new -m flag for 'bootflow scan' so it can display a menu automatically - Fix for video-scrolling crash with Truetype - menu items can now have individual integer values - menu items are lined up according to the width of all menu labels Simon Glass (34): test: Run tests that don't need devices test: Add a new suite for commands test: Add helper to skip to partial console line test: Make UT_LIB_ASN1 depend on sandbox test: Run bootstd tests only on sandbox test: Handle use of stack pointer in bdinfo test: bdinfo: Add missing asserts test: fdt: Add a special case for real boards test: font: Add dependencies on fonts test: event: Only run test_event_probe() on sandbox test: lmb: Move tests into the lib suite test: print: Skip test on x86 video: Add a function to clear the display sandbox: Add a dummy booti command bootstd: Add a menu option to bootflow scan video: Add a dark-grey console colour video: Avoid starting a new line to close to the bottom expo: Place menu items to the right of all labels expo: Set the initial next_id to 1 expo: Use standard numbering for save and discard expo: Allow menu items to have values expo: Add a little more cedit CMOS logging expo: Support menu-item values in cedit expo: Drop unneceesary calls to expo_str() expo: Drop scene_title_set() expo: Add forward declaration for udevice to cedit x86: coreboot: Enable unit tests x86: CI: Update coreboot x86: coreboot: Add a test for cbsysinfo command x86: coreboot: Show the option table x86: coreboot: Enable support for the configuration editor x86: coreboot: Add a command to check and update CMOS RAM x86: coreboot: Allow building an expo for editing CMOS config x86: Enable RTC command by default .azure-pipelines.yml | 2 +- .gitlab-ci.yml| 2 +- arch/sandbox/dts/cedit.dtsi | 3 + arch/sandbox/lib/bootm.c | 7 + arch/x86/dts/coreboot.dts | 7 + boot/Makefile | 4 + boot/cedit.c | 191 +++ boot/expo.c | 3 + boot/expo_build.c | 36 ++--- boot/expo_build_cb.c | 244 ++ boot/scene.c | 61 ++-- boot/scene_internal.h | 30 +++- boot/scene_menu.c | 26 +++- boot/scene_textline.c | 3 +- cmd/Kconfig | 14 +- cmd/bootflow.c| 27 +++- cmd/booti.c | 2 +- cmd/cedit.c | 28 cmd/cls.c | 25 +-- cmd/x86/Makefile | 1 + cmd/x86/cbcmos.c | 141 + cmd/x86/cbsysinfo.c | 73 - common/console.c | 31 configs/coreboot64_defconfig | 2 + configs/coreboot_defconfig| 6 + configs/tools-only_defconfig | 3 +- doc/board/coreboot/coreboot.rst | 6 + doc/develop/cedit.rst | 9 +- doc/develop/expo.rst | 26 +++- doc/usage/cmd/bootflow.rst| 5 + doc/usage/cmd/cbcmos.rst | 45 ++ doc/usage/cmd/cbsysinfo.rst | 99 doc/usage/cmd/cedit.rst | 91 ++- doc/usage/index.rst | 1 + drivers/video/vidconsole-uclass.c | 4 +- drivers/video/video-uclass.c | 3 + include/cedit.h | 1 + include/console.h | 10 ++ include/expo.h| 51 +-- include/test/cedit-test.h | 30 ++-- include/test/cmd.h| 15 ++ include/test/suites.h | 1 + include/test/ut.h | 30 include/video.h | 4 +- include/video_console.h | 8 + test/Kconfig | 6 + test/Makefile | 2 +- test/boot/bootflow.c | 82 ++ test/boot/cedit.c | 22 ++- test/boot/expo.c | 26 +++- test/boot/files/expo_ids.h| 3 +- test/boot/files/expo_layout.dts | 5 +- test/cmd/Makefile | 3 + test/cmd/bdinfo.c | 23 ++- test/cmd/cmd_ut_cmd.c | 21 +++ test/cmd/coreboot.c | 120 +++ test/cmd/fdt.c
[PATCH v3 08/12] video: Drop unnecessary truetype operations from SPL
Saving and restoring entries is used for expo and for the command line, which we don't use in SPL. Drop these methods. Signed-off-by: Simon Glass --- Changes in v3: - Add new patch to drop unnecessary truetype operations from SPL drivers/video/console_truetype.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/video/console_truetype.c b/drivers/video/console_truetype.c index 14fb81e9563c..602133575f8c 100644 --- a/drivers/video/console_truetype.c +++ b/drivers/video/console_truetype.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -799,6 +800,9 @@ static int truetype_entry_save(struct udevice *dev, struct abuf *buf) struct console_tt_store store; const uint size = sizeof(store); + if (spl_phase() <= PHASE_SPL) + return -ENOSYS; + /* * store the whole priv structure as it is simpler that picking out * what we need @@ -820,6 +824,9 @@ static int truetype_entry_restore(struct udevice *dev, struct abuf *buf) struct console_tt_priv *priv = dev_get_priv(dev); struct console_tt_store store; + if (spl_phase() <= PHASE_SPL) + return -ENOSYS; + memcpy(, abuf_data(buf), sizeof(store)); vc_priv->xcur_frac = store.cur.xpos_frac; @@ -844,6 +851,9 @@ static int truetype_set_cursor_visible(struct udevice *dev, bool visible, uint out, val; int ret; + if (spl_phase() <= PHASE_SPL) + return -ENOSYS; + if (!visible) return 0; -- 2.42.0.582.g8ccd20d70d-goog
[PATCH v3 11/12] x86: qemu: Expand ROM size
Expand the ROM for x86_64 to 2MB to make space for the font, as it is already on the edge. Signed-off-by: Simon Glass --- (no changes since v1) board/emulation/qemu-x86/Kconfig | 3 ++- configs/qemu-x86_64_defconfig| 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/board/emulation/qemu-x86/Kconfig b/board/emulation/qemu-x86/Kconfig index 787751abba4f..04b7a8aff89e 100644 --- a/board/emulation/qemu-x86/Kconfig +++ b/board/emulation/qemu-x86/Kconfig @@ -21,7 +21,8 @@ config BOARD_SPECIFIC_OPTIONS # dummy select X86_RESET_VECTOR select QEMU select QFW_PIO - select BOARD_ROMSIZE_KB_1024 + select BOARD_ROMSIZE_KB_1024 if TARGET_QEMU_X86 + select BOARD_ROMSIZE_KB_2048 if TARGET_QEMU_X86_64 imply VIRTIO_PCI imply VIRTIO_NET imply VIRTIO_BLK diff --git a/configs/qemu-x86_64_defconfig b/configs/qemu-x86_64_defconfig index 165f0b512c8b..6de3da2d8269 100644 --- a/configs/qemu-x86_64_defconfig +++ b/configs/qemu-x86_64_defconfig @@ -6,7 +6,7 @@ CONFIG_ENV_SIZE=0x4 CONFIG_MAX_CPUS=2 CONFIG_SPL_DM_SPI=y CONFIG_DEFAULT_DEVICE_TREE="qemu-x86_i440fx" -CONFIG_SPL_TEXT_BASE=0xfffd8000 +CONFIG_SPL_TEXT_BASE=0xfffd4000 CONFIG_SPL_SYS_MALLOC_F_LEN=0x2000 CONFIG_DEBUG_UART_BASE=0x3f8 CONFIG_DEBUG_UART_CLOCK=1843200 @@ -17,12 +17,12 @@ CONFIG_DEBUG_UART=y CONFIG_SMP=y CONFIG_GENERATE_PIRQ_TABLE=y CONFIG_GENERATE_MP_TABLE=y -CONFIG_X86_OFFSET_U_BOOT=0xfff0 +CONFIG_X86_OFFSET_U_BOOT=0xffe0 +CONFIG_SYS_MONITOR_BASE=0x0111 CONFIG_FIT=y CONFIG_SPL_LOAD_FIT=y CONFIG_BOOTSTD_FULL=y CONFIG_BOOTSTD_DEFAULTS=y -CONFIG_SYS_MONITOR_BASE=0x0111 CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y CONFIG_SHOW_BOOT_PROGRESS=y -- 2.42.0.582.g8ccd20d70d-goog
[PATCH v3 12/12] x86: qemu: Enable truetype fonts
Enable this feature to provide a larger font choice and more attractive menus. Expand the ROM for x86_64 to 2MB to make space for the font. Signed-off-by: Simon Glass --- Changes in v3: - Add new patch to enable truetype fonts in qemu-x86 and qemu-x86_64 configs/qemu-x86_64_defconfig | 1 + configs/qemu-x86_defconfig| 1 + 2 files changed, 2 insertions(+) diff --git a/configs/qemu-x86_64_defconfig b/configs/qemu-x86_64_defconfig index 6de3da2d8269..c73d3f96e53d 100644 --- a/configs/qemu-x86_64_defconfig +++ b/configs/qemu-x86_64_defconfig @@ -83,6 +83,7 @@ CONFIG_SPL_DM_RTC=y CONFIG_SYS_NS16550_PORT_MAPPED=y CONFIG_SPI=y CONFIG_USB_KEYBOARD=y +CONFIG_CONSOLE_TRUETYPE=y CONFIG_FRAMEBUFFER_SET_VESA_MODE=y CONFIG_FRAMEBUFFER_VESA_MODE_USER=y CONFIG_FRAMEBUFFER_VESA_MODE=0x144 diff --git a/configs/qemu-x86_defconfig b/configs/qemu-x86_defconfig index 4b2787d4aaef..123d5ad63e53 100644 --- a/configs/qemu-x86_defconfig +++ b/configs/qemu-x86_defconfig @@ -60,6 +60,7 @@ CONFIG_NVME_PCI=y CONFIG_SYS_NS16550_PORT_MAPPED=y CONFIG_SPI=y CONFIG_USB_KEYBOARD=y +CONFIG_CONSOLE_TRUETYPE=y CONFIG_FRAMEBUFFER_SET_VESA_MODE=y CONFIG_FRAMEBUFFER_VESA_MODE_USER=y CONFIG_FRAMEBUFFER_VESA_MODE=0x144 -- 2.42.0.582.g8ccd20d70d-goog
[PATCH v3 06/12] expo: Correct background colour
Use the correct background colour when using white-on-black.
Signed-off-by: Simon Glass
---
(no changes since v1)
boot/expo.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/boot/expo.c b/boot/expo.c
index 139d684f8e6e..cadb6a0ad6e3 100644
--- a/boot/expo.c
+++ b/boot/expo.c
@@ -190,10 +190,12 @@ int expo_render(struct expo *exp)
struct udevice *dev = exp->display;
struct video_priv *vid_priv = dev_get_uclass_priv(dev);
struct scene *scn = NULL;
+ enum colour_idx back;
u32 colour;
int ret;
- colour = video_index_to_colour(vid_priv, VID_WHITE);
+ back = CONFIG_IS_ENABLED(SYS_WHITE_ON_BLACK) ? VID_BLACK : VID_WHITE;
+ colour = video_index_to_colour(vid_priv, back);
ret = video_fill(dev, colour);
if (ret)
return log_msg_ret("fill", ret);
--
2.42.0.582.g8ccd20d70d-goog
[PATCH v3 10/12] x86: coreboot: Enable truetype fonts
Truetype fonts look better in the menu, so enable them.
Signed-off-by: Simon Glass
---
Changes in v3:
- Add new patch to enable truetype fonts in coreboot
arch/x86/dts/coreboot.dts| 10 ++
configs/coreboot64_defconfig | 3 ++-
configs/coreboot_defconfig | 3 ++-
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/arch/x86/dts/coreboot.dts b/arch/x86/dts/coreboot.dts
index 0eb31cae42c1..83beb82f37c7 100644
--- a/arch/x86/dts/coreboot.dts
+++ b/arch/x86/dts/coreboot.dts
@@ -45,4 +45,14 @@
bootph-some-ram;
compatible = "coreboot-fb";
};
+
+ bootstd {
+ compatible = "u-boot,boot-std";
+
+ theme {
+ font-size = <30>;
+ menu-inset = <3>;
+ menuitem-gap-y = <1>;
+ };
+ };
};
diff --git a/configs/coreboot64_defconfig b/configs/coreboot64_defconfig
index 1865f623f6e6..42b46ba0d9b9 100644
--- a/configs/coreboot64_defconfig
+++ b/configs/coreboot64_defconfig
@@ -9,11 +9,11 @@ CONFIG_PRE_CON_BUF_ADDR=0x10
CONFIG_X86_RUN_64BIT=y
CONFIG_VENDOR_COREBOOT=y
CONFIG_TARGET_COREBOOT=y
+CONFIG_SYS_MONITOR_BASE=0x0112
CONFIG_FIT=y
CONFIG_FIT_SIGNATURE=y
CONFIG_BOOTSTD_FULL=y
CONFIG_BOOTSTD_DEFAULTS=y
-CONFIG_SYS_MONITOR_BASE=0x0112
CONFIG_SHOW_BOOT_PROGRESS=y
CONFIG_USE_BOOTARGS=y
CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro"
@@ -61,6 +61,7 @@ CONFIG_SYS_NS16550_MEM32=y
CONFIG_SOUND=y
CONFIG_SOUND_I8254=y
CONFIG_VIDEO_COPY=y
+CONFIG_CONSOLE_TRUETYPE=y
CONFIG_CONSOLE_SCROLL_LINES=5
CONFIG_SPL_ACPI=y
CONFIG_CMD_DHRYSTONE=y
diff --git a/configs/coreboot_defconfig b/configs/coreboot_defconfig
index f1728dbfb5c9..7d744e9962ae 100644
--- a/configs/coreboot_defconfig
+++ b/configs/coreboot_defconfig
@@ -7,11 +7,11 @@ CONFIG_DEFAULT_DEVICE_TREE="coreboot"
CONFIG_PRE_CON_BUF_ADDR=0x10
CONFIG_VENDOR_COREBOOT=y
CONFIG_TARGET_COREBOOT=y
+CONFIG_SYS_MONITOR_BASE=0x0111
CONFIG_FIT=y
CONFIG_FIT_SIGNATURE=y
CONFIG_BOOTSTD_FULL=y
CONFIG_BOOTSTD_DEFAULTS=y
-CONFIG_SYS_MONITOR_BASE=0x0111
CONFIG_SHOW_BOOT_PROGRESS=y
CONFIG_USE_BOOTARGS=y
CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro"
@@ -55,6 +55,7 @@ CONFIG_SYS_NS16550_MEM32=y
CONFIG_SOUND=y
CONFIG_SOUND_I8254=y
CONFIG_VIDEO_COPY=y
+CONFIG_CONSOLE_TRUETYPE=y
CONFIG_CONSOLE_SCROLL_LINES=5
CONFIG_CMD_DHRYSTONE=y
# CONFIG_GZIP is not set
--
2.42.0.582.g8ccd20d70d-goog
[PATCH v3 03/12] bootstd: Add a return code to bootflow menu
Return an error when the user does not select an OS, so we know whether
to boot or not.
Move calling of bootflow_menu_run() into a separate function so we can
call it from other places.
Expand the test to cover these cases.
Add some documentation also, while we are here.
Signed-off-by: Simon Glass
---
Changes in v3:
- Add missing word 'function' in the commit message
Changes in v2:
- Add new patch to add a return code to bootflow menu
cmd/bootflow.c | 53 +++---
doc/usage/cmd/bootflow.rst | 67 ++
test/boot/bootflow.c | 15 +
3 files changed, 123 insertions(+), 12 deletions(-)
diff --git a/cmd/bootflow.c b/cmd/bootflow.c
index 300ad3aaa760..1516f8a4193b 100644
--- a/cmd/bootflow.c
+++ b/cmd/bootflow.c
@@ -89,6 +89,44 @@ static void show_footer(int count, int num_valid)
num_valid);
}
+/**
+ * bootflow_handle_menu() - Handle running the menu and updating cur bootflow
+ *
+ * This shows the menu, allows the user to select something and then prints
+ * what happened
+ *
+ * @std: bootstd information
+ * @text_mode: true to run the menu in text mode
+ * @bflowp: Returns selected bootflow, on success
+ * Return: 0 on success (a bootflow was selected), -EAGAIN if nothing was
+ * chosen, other -ve value on other error
+ */
+__maybe_unused static int bootflow_handle_menu(struct bootstd_priv *std,
+ bool text_mode,
+ struct bootflow **bflowp)
+{
+ struct bootflow *bflow;
+ int ret;
+
+ ret = bootflow_menu_run(std, text_mode, );
+ if (ret) {
+ if (ret == -EAGAIN) {
+ printf("Nothing chosen\n");
+ std->cur_bootflow = NULL;
+ } else {
+ printf("Menu failed (err=%d)\n", ret);
+ }
+
+ return ret;
+ }
+
+ printf("Selected: %s\n", bflow->os_name ? bflow->os_name : bflow->name);
+ std->cur_bootflow = bflow;
+ *bflowp = bflow;
+
+ return 0;
+}
+
static int do_bootflow_scan(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
{
@@ -455,18 +493,9 @@ static int do_bootflow_menu(struct cmd_tbl *cmdtp, int
flag, int argc,
if (ret)
return CMD_RET_FAILURE;
- ret = bootflow_menu_run(std, text_mode, );
- if (ret) {
- if (ret == -EAGAIN)
- printf("Nothing chosen\n");
- else {
- printf("Menu failed (err=%d)\n", ret);
- return CMD_RET_FAILURE;
- }
- }
-
- printf("Selected: %s\n", bflow->os_name ? bflow->os_name : bflow->name);
- std->cur_bootflow = bflow;
+ ret = bootflow_handle_menu(std, text_mode, );
+ if (ret)
+ return CMD_RET_FAILURE;
return 0;
}
diff --git a/doc/usage/cmd/bootflow.rst b/doc/usage/cmd/bootflow.rst
index ead493d0aaf8..6a0645c28d07 100644
--- a/doc/usage/cmd/bootflow.rst
+++ b/doc/usage/cmd/bootflow.rst
@@ -15,6 +15,7 @@ Synopis
bootflow read
bootflow boot
bootflow cmdline [set|get|clear|delete|auto] []
+bootfloe menu [-t]
Description
---
@@ -24,6 +25,9 @@ locate bootflows, list them and boot them.
See :doc:`../../develop/bootstd` for more information.
+Note that `CONFIG_BOOTSTD_FULL` (which enables `CONFIG_CMD_BOOTFLOW_FULL) must
+be enabled to obtain full functionality with this command. Otherwise, it only
+supports `bootflow scan` which scans and boots the first available bootflow.
bootflow scan
~
@@ -247,6 +251,16 @@ can be used to set the early console (or console) to a
suitable value so that
output appears on the serial port. This is only supported by the 16550 serial
driver so far.
+bootflow menu
+~
+
+This shows a menu with available bootflows. The user can select a particular
+bootflow, which then becomes the current one.
+
+The `-t` flag requests a text menu. Otherwise, if a display is available, a
+graphical menu is shown.
+
+
Example
---
@@ -658,6 +672,56 @@ Now the buffer can be accessed::
77b7e4e0: 320fc000 08e8ba0f c031300f b8df ...2.01.
77b7e4f0: 0020 6ad8000f 00858d10 5002 ..j...P
+This shows using a text menu to boot an OS::
+
+=> bootflow scan
+=> bootfl list
+=> bootfl menu -t
+U-Boot:Boot Menu
+
+UP and DOWN to choose, ENTER to select
+
+ >0 mmc1mmc1.bootdev.whole
+ 1 mmc1Fedora-Workstation-armhfp-31-1.9
(5.3.7-301.fc31.armv7hl)
+ 2 mmc1mmc1.bootdev.part_1
+ 3 mmc4mmc4.bootdev.whole
+ 4 mmc4Armbian
+ 5 mmc4mmc4.bootdev.part_1
+ 6 mmc5mmc5.bootdev.whole
+ 7 mmc5ChromeOS
+ 8
[PATCH v3 09/12] x86: Enable SSE in 64-bit mode
This is needed to support Truetype fonts. In any case, the compiler
expects SSE to be available in 64-bit mode. Enable it.
Signed-off-by: Simon Glass
Suggested-by: Bin Meng
---
(no changes since v1)
arch/x86/config.mk| 1 -
arch/x86/cpu/x86_64/cpu.c | 11 +++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/x86/config.mk b/arch/x86/config.mk
index 26ec1af2f0b0..b038b132985a 100644
--- a/arch/x86/config.mk
+++ b/arch/x86/config.mk
@@ -27,7 +27,6 @@ ifeq ($(IS_32BIT),y)
PLATFORM_CPPFLAGS += -march=i386 -m32
else
PLATFORM_CPPFLAGS += $(if $(CONFIG_SPL_BUILD),,-fpic) -fno-common -march=core2
-m64
-PLATFORM_CPPFLAGS += -mno-mmx -mno-sse
endif
PLATFORM_RELFLAGS += -fdata-sections -ffunction-sections -fvisibility=hidden
diff --git a/arch/x86/cpu/x86_64/cpu.c b/arch/x86/cpu/x86_64/cpu.c
index 2647bff891f8..54dc31bf6b71 100644
--- a/arch/x86/cpu/x86_64/cpu.c
+++ b/arch/x86/cpu/x86_64/cpu.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
DECLARE_GLOBAL_DATA_PTR;
@@ -39,11 +40,21 @@ int x86_mp_init(void)
return 0;
}
+/* enable SSE features */
+static void setup_cpu_features(void)
+{
+ asm ("mov %%cr4, %%rax\n" \
+ "or %0, %%rax\n" \
+ "mov %%rax, %%cr4\n" \
+ : : "i" (X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT) : "eax");
+}
+
int x86_cpu_reinit_f(void)
{
/* set the vendor to Intel so that native_calibrate_tsc() works */
gd->arch.x86_vendor = X86_VENDOR_INTEL;
gd->arch.has_mtrr = true;
+ setup_cpu_features();
return 0;
}
--
2.42.0.582.g8ccd20d70d-goog
[PATCH v3 04/12] x86: coreboot: Add a boot script
Provide the user with a list of available boot options. Selecting one causes it to be booted. Pressing causes U-Boot to return to the command-line prompt. Signed-off-by: Simon Glass --- Changes in v3: - Clear the screen before booting Changes in v2: - Add new patch to add a coreboot boot script configs/coreboot64_defconfig | 1 + configs/coreboot_defconfig | 1 + 2 files changed, 2 insertions(+) diff --git a/configs/coreboot64_defconfig b/configs/coreboot64_defconfig index 555d281ef3cf..1865f623f6e6 100644 --- a/configs/coreboot64_defconfig +++ b/configs/coreboot64_defconfig @@ -17,6 +17,7 @@ CONFIG_SYS_MONITOR_BASE=0x0112 CONFIG_SHOW_BOOT_PROGRESS=y CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro" +CONFIG_BOOTCOMMAND="bootflow scan -l; if bootflow menu; then cls; bootflow boot; fi" CONFIG_PRE_CONSOLE_BUFFER=y CONFIG_SYS_CONSOLE_INFO_QUIET=y CONFIG_LOG=y diff --git a/configs/coreboot_defconfig b/configs/coreboot_defconfig index edc38f1f5923..f1728dbfb5c9 100644 --- a/configs/coreboot_defconfig +++ b/configs/coreboot_defconfig @@ -15,6 +15,7 @@ CONFIG_SYS_MONITOR_BASE=0x0111 CONFIG_SHOW_BOOT_PROGRESS=y CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro" +CONFIG_BOOTCOMMAND="bootflow scan -l; if bootflow menu; then cls; bootflow boot; fi" CONFIG_PRE_CONSOLE_BUFFER=y CONFIG_SYS_CONSOLE_INFO_QUIET=y CONFIG_LOG=y -- 2.42.0.582.g8ccd20d70d-goog
[PATCH v3 01/12] efi: Correct handling of frame buffer
The efi_gop driver uses private fields from the video uclass to obtain a
pointer to the frame buffer. Use the platform data instead.
Check the VIDEO_COPY setting to determine which frame buffer to use. Once
the next stage is running (and making use of U-Boot's EFI boot services)
U-Boot does not handle copying from priv->fb to the hardware framebuffer,
so we must allow EFI to write directly to the hardware framebuffer.
We could provide a function to read this, but it seems better to just
document how it works. The original change ignored an explicit comment
in the video.h file ("Things that are private to the uclass: don't use
these in the driver") which is why this was missed when the VIDEO_COPY
feature was added.
Signed-off-by: Simon Glass
Fixes: 8f661a5b662 ("efi_loader: gop: Expose fb when 32bpp")
---
(no changes since v2)
Changes in v2:
- Rebase to -next
- Add some more comments to the header file
- Add fixes tag
include/video.h | 9 ++---
lib/efi_loader/efi_gop.c | 12 +++-
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/include/video.h b/include/video.h
index 5048116a3d57..4d8df9baaada 100644
--- a/include/video.h
+++ b/include/video.h
@@ -21,9 +21,12 @@ struct udevice;
* @align: Frame-buffer alignment, indicating the memory boundary the frame
* buffer should start on. If 0, 1MB is assumed
* @size: Frame-buffer size, in bytes
- * @base: Base address of frame buffer, 0 if not yet known
- * @copy_base: Base address of a hardware copy of the frame buffer. See
- * CONFIG_VIDEO_COPY.
+ * @base: Base address of frame buffer, 0 if not yet known. If
CONFIG_VIDEO_COPY
+ * is enabled, this is the software copy, so writes to this will not be
+ * visible until vidconsole_sync_copy() is called. If CONFIG_VIDEO_COPY is
+ * disabled, this is the hardware framebuffer.
+ * @copy_base: Base address of a hardware copy of the frame buffer. If
+ * CONFIG_VIDEO_COPY is disabled, this is not used.
* @copy_size: Size of copy framebuffer, used if @size is 0
* @hide_logo: Hide the logo (used for testing)
*/
diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
index 778b693f983a..a09db31eb465 100644
--- a/lib/efi_loader/efi_gop.c
+++ b/lib/efi_loader/efi_gop.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
#include
@@ -467,10 +468,10 @@ efi_status_t efi_gop_register(void)
struct efi_gop_obj *gopobj;
u32 bpix, format, col, row;
u64 fb_base, fb_size;
- void *fb;
efi_status_t ret;
struct udevice *vdev;
struct video_priv *priv;
+ struct video_uc_plat *plat;
/* We only support a single video output device for now */
if (uclass_first_device_err(UCLASS_VIDEO, )) {
@@ -483,9 +484,10 @@ efi_status_t efi_gop_register(void)
format = priv->format;
col = video_get_xsize(vdev);
row = video_get_ysize(vdev);
- fb_base = (uintptr_t)priv->fb;
- fb_size = priv->fb_size;
- fb = priv->fb;
+
+ plat = dev_get_uclass_plat(vdev);
+ fb_base = IS_ENABLED(CONFIG_VIDEO_COPY) ? plat->copy_base : plat->base;
+ fb_size = plat->size;
switch (bpix) {
case VIDEO_BPP16:
@@ -547,7 +549,7 @@ efi_status_t efi_gop_register(void)
}
gopobj->info.pixels_per_scanline = col;
gopobj->bpix = bpix;
- gopobj->fb = fb;
+ gopobj->fb = map_sysmem(fb_base, fb_size);
return EFI_SUCCESS;
}
--
2.42.0.582.g8ccd20d70d-goog
[PATCH v3 05/12] usb: Avoid unbinding devices in use by bootflows
When a USB device is unbound, it causes any bootflows attached to it to
be removed, via a call to bootdev_clear_bootflows() from
bootdev_pre_unbind(). This obviously makes it impossible to boot the
bootflow.
However, when booting a bootflow that relies on USB, usb_stop() is
called, which unbinds the device. For EFI, this happens in
efi_exit_boot_services() which means that the bootflow disappears
before it is finished with.
There is no need to unbind all the USB devices just to quiesce them.
Add a new usb_pause() call which removes them but leaves them bound.
This resolves a hang on x86 when booting a distro from USB. This was
found using a device with 4 bootflows, the last of which was USB.
Signed-off-by: Simon Glass
---
(no changes since v2)
Changes in v2:
- Add new patch to avoid unbinding devices in use by bootflows
boot/bootm.c | 2 +-
common/usb.c | 7 ++-
drivers/usb/host/usb-uclass.c | 14 --
include/usb.h | 15 ++-
4 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/boot/bootm.c b/boot/bootm.c
index b1c3afe0a3ad..5c9ba083e64c 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -501,7 +501,7 @@ ulong bootm_disable_interrupts(void)
* updated every 1 ms within the HCCA structure in SDRAM! For more
* details see the OpenHCI specification.
*/
- usb_stop();
+ usb_pause();
#endif
return iflag;
}
diff --git a/common/usb.c b/common/usb.c
index 836506dcd9e9..4d6ac69111e7 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -126,7 +126,7 @@ int usb_init(void)
/**
* Stop USB this stops the LowLevel Part and deregisters USB devices.
*/
-int usb_stop(void)
+int usb_pause(void)
{
int i;
@@ -144,6 +144,11 @@ int usb_stop(void)
return 0;
}
+int usb_stop(void)
+{
+ return usb_pause();
+}
+
/**
* Detect if a USB device has been plugged or unplugged.
*/
diff --git a/drivers/usb/host/usb-uclass.c b/drivers/usb/host/usb-uclass.c
index a1cd0ad2d669..c26c65d7986b 100644
--- a/drivers/usb/host/usb-uclass.c
+++ b/drivers/usb/host/usb-uclass.c
@@ -173,7 +173,7 @@ int usb_get_max_xfer_size(struct usb_device *udev, size_t
*size)
return ops->get_max_xfer_size(bus, size);
}
-int usb_stop(void)
+static int usb_finish(bool unbind_all)
{
struct udevice *bus;
struct udevice *rh;
@@ -195,7 +195,7 @@ int usb_stop(void)
/* Locate root hub device */
device_find_first_child(bus, );
- if (rh) {
+ if (rh && unbind_all) {
/*
* All USB devices are children of root hub.
* Unbinding root hub will unbind all of its children.
@@ -222,6 +222,16 @@ int usb_stop(void)
return err;
}
+int usb_stop(void)
+{
+ return usb_finish(true);
+}
+
+int usb_pause(void)
+{
+ return usb_finish(false);
+}
+
static void usb_scan_bus(struct udevice *bus, bool recurse)
{
struct usb_bus_priv *priv;
diff --git a/include/usb.h b/include/usb.h
index 09e3f0cb309c..ad39b09a6e45 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -265,7 +265,20 @@ int usb_kbd_deregister(int force);
*/
int usb_init(void);
-int usb_stop(void); /* stop the USB Controller */
+/**
+ * usb_stop() - stop the USB Controller and unbind all USB controllers/devices
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int usb_stop(void);
+
+/**
+ * usb_pause() - stop the USB Controller DMA, etc.
+ *
+ * Return: 0 if OK, -ve on error
+ */
+int usb_pause(void);
+
int usb_detect_change(void); /* detect if a USB device has been (un)plugged */
--
2.42.0.582.g8ccd20d70d-goog
[PATCH v3 07/12] video: Correct setting of cursor position
The ANSI codes are not correctly handled at present, in that the
requested X position is added to the current one.
Correct this and also call vidconsole_entry_start() to start a new text
line.
Signed-off-by: Simon Glass
---
(no changes since v1)
drivers/video/vidconsole-uclass.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/video/vidconsole-uclass.c
b/drivers/video/vidconsole-uclass.c
index 22d55df71f63..a312a1985242 100644
--- a/drivers/video/vidconsole-uclass.c
+++ b/drivers/video/vidconsole-uclass.c
@@ -125,6 +125,7 @@ void vidconsole_set_cursor_pos(struct udevice *dev, int x,
int y)
priv->xcur_frac = VID_TO_POS(x);
priv->xstart_frac = priv->xcur_frac;
priv->ycur = y;
+ vidconsole_entry_start(dev);
}
/**
@@ -134,8 +135,10 @@ void vidconsole_set_cursor_pos(struct udevice *dev, int x,
int y)
* @row: new row
* @col: new column
*/
-static void set_cursor_position(struct vidconsole_priv *priv, int row, int col)
+static void set_cursor_position(struct udevice *dev, int row, int col)
{
+ struct vidconsole_priv *priv = dev_get_uclass_priv(dev);
+
/*
* Ensure we stay in the bounds of the screen.
*/
@@ -144,9 +147,7 @@ static void set_cursor_position(struct vidconsole_priv
*priv, int row, int col)
if (col >= priv->cols)
col = priv->cols - 1;
- priv->ycur = row * priv->y_charsize;
- priv->xcur_frac = priv->xstart_frac +
- VID_TO_POS(col * priv->x_charsize);
+ vidconsole_position_cursor(dev, col, row);
}
/**
@@ -193,7 +194,7 @@ static void vidconsole_escape_char(struct udevice *dev,
char ch)
int row = priv->row_saved;
int col = priv->col_saved;
- set_cursor_position(priv, row, col);
+ set_cursor_position(dev, row, col);
priv->escape = 0;
return;
}
@@ -255,7 +256,7 @@ static void vidconsole_escape_char(struct udevice *dev,
char ch)
if (row < 0)
row = 0;
/* Right and bottom overflows are handled in the callee. */
- set_cursor_position(priv, row, col);
+ set_cursor_position(dev, row, col);
break;
}
case 'H':
@@ -279,7 +280,7 @@ static void vidconsole_escape_char(struct udevice *dev,
char ch)
if (col)
--col;
- set_cursor_position(priv, row, col);
+ set_cursor_position(dev, row, col);
break;
}
--
2.42.0.582.g8ccd20d70d-goog
[PATCH v2 28/36] expo: Add basic support for textline objects
A textline is a line of text which can be edited by the user. It has a
maximum length (in chracters) but otherwise there are no restrictions.
Signed-off-by: Simon Glass
---
(no changes since v1)
boot/Makefile | 3 +-
boot/scene_textline.c | 234 ++
include/expo.h| 36 +++
3 files changed, 272 insertions(+), 1 deletion(-)
create mode 100644 boot/scene_textline.c
diff --git a/boot/Makefile b/boot/Makefile
index 6ce983b83fa4..ad608598d298 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -56,7 +56,8 @@ ifdef CONFIG_SPL_BUILD
obj-$(CONFIG_SPL_LOAD_FIT) += common_fit.o
endif
-obj-$(CONFIG_$(SPL_TPL_)EXPO) += expo.o scene.o scene_menu.o expo_build.o
+obj-$(CONFIG_$(SPL_TPL_)EXPO) += expo.o scene.o expo_build.o
+obj-$(CONFIG_$(SPL_TPL_)EXPO) += scene_menu.o scene_textline.o
obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += vbe.o
obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_REQUEST) += vbe_request.o
diff --git a/boot/scene_textline.c b/boot/scene_textline.c
new file mode 100644
index ..2caa81ee1582
--- /dev/null
+++ b/boot/scene_textline.c
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Implementation of a menu in a scene
+ *
+ * Copyright 2023 Google LLC
+ * Written by Simon Glass
+ */
+
+#define LOG_CATEGORY LOGC_EXPO
+
+#include
+#include
+#include
+#include
+#include "scene_internal.h"
+
+int scene_textline(struct scene *scn, const char *name, uint id, uint
max_chars,
+ struct scene_obj_textline **tlinep)
+{
+ struct scene_obj_textline *tline;
+ char *buf;
+ int ret;
+
+ if (max_chars >= EXPO_MAX_CHARS)
+ return log_msg_ret("chr", -E2BIG);
+
+ ret = scene_obj_add(scn, name, id, SCENEOBJT_TEXTLINE,
+ sizeof(struct scene_obj_textline),
+ (struct scene_obj **));
+ if (ret < 0)
+ return log_msg_ret("obj", -ENOMEM);
+ abuf_init(>buf);
+ if (!abuf_realloc(>buf, max_chars + 1))
+ return log_msg_ret("buf", -ENOMEM);
+ buf = abuf_data(>buf);
+ *buf = '\0';
+ tline->pos = max_chars;
+ tline->max_chars = max_chars;
+
+ if (tlinep)
+ *tlinep = tline;
+
+ return tline->obj.id;
+}
+
+void scene_textline_calc_bbox(struct scene_obj_textline *tline,
+ struct vidconsole_bbox *bbox,
+ struct vidconsole_bbox *edit_bbox)
+{
+ const struct expo_theme *theme = >obj.scene->expo->theme;
+
+ bbox->valid = false;
+ scene_bbox_union(tline->obj.scene, tline->label_id, 0, bbox);
+ scene_bbox_union(tline->obj.scene, tline->edit_id, 0, bbox);
+
+ edit_bbox->valid = false;
+ scene_bbox_union(tline->obj.scene, tline->edit_id, theme->menu_inset,
+edit_bbox);
+}
+
+int scene_textline_calc_dims(struct scene_obj_textline *tline)
+{
+ struct scene *scn = tline->obj.scene;
+ struct vidconsole_bbox bbox;
+ struct scene_obj_txt *txt;
+ int ret;
+
+ txt = scene_obj_find(scn, tline->edit_id, SCENEOBJT_NONE);
+ if (!txt)
+ return log_msg_ret("dim", -ENOENT);
+
+ ret = vidconsole_nominal(scn->expo->cons, txt->font_name,
+txt->font_size, tline->max_chars, );
+ if (ret)
+ return log_msg_ret("nom", ret);
+
+ if (bbox.valid) {
+ tline->obj.dim.w = bbox.x1 - bbox.x0;
+ tline->obj.dim.h = bbox.y1 - bbox.y0;
+
+ scene_obj_set_size(scn, tline->edit_id, tline->obj.dim.w,
+ tline->obj.dim.h);
+ }
+
+ return 0;
+}
+
+int scene_textline_arrange(struct scene *scn, struct scene_obj_textline *tline)
+{
+ const bool open = tline->obj.flags & SCENEOF_OPEN;
+ bool point;
+ int x, y;
+ int ret;
+
+ x = tline->obj.dim.x;
+ y = tline->obj.dim.y;
+ if (tline->label_id) {
+ ret = scene_obj_set_pos(scn, tline->label_id, tline->obj.dim.x,
+ y);
+ if (ret < 0)
+ return log_msg_ret("tit", ret);
+
+ ret = scene_obj_set_pos(scn, tline->edit_id,
+ tline->obj.dim.x + 200, y);
+ if (ret < 0)
+ return log_msg_ret("tit", ret);
+
+ ret = scene_obj_get_hw(scn, tline->label_id, NULL);
+ if (ret < 0)
+ return log_msg_ret("hei", ret);
+
+ y += ret * 2;
+ }
+
+ point = scn->highlight_id == tline->obj.id;
+ point &= !open;
+ scene_obj_flag_clrset(scn, tline->edit_id, SCENEOF_POINT,
+ point ? SCENEOF_POINT : 0);
+
+ return 0;
+}
+
+int scene_textline_send_key(struct scene *scn, struct scene_obj_textline
*tline,
+ int key,
