Re: [RFC PATCH 15/31] efi_memory: add an event handler to update memory map
On Mon, 10 Jun 2024 at 18:54, Simon Glass wrote: > > Hi, > > On Mon, 10 Jun 2024 at 09:42, Sughosh Ganu wrote: > > > > On Mon, 10 Jun 2024 at 20:47, Heinrich Schuchardt > > wrote: > > > > > > On 07.06.24 20:52, Sughosh Ganu wrote: > > > > There are events that would be used to notify other interested modules > > > > of any changes in available and occupied memory. This would happen > > > > when a module allocates or reserves memory, or frees up memory. These > > > > > > I am worried about the "frees up memory" case. > > > > > > When LMB frees memory we cannot add it back to EFI conventional memory > > > as there might still be a file image lingering around that EFI should > > > not overwrite. It has to stay marked as EfiLoaderCode or EfiLoaderData. > > > > So here is my basic doubt. Why would LMB free up memory if it still > > has a valid image. If that is the case, the lmb_free API should not be > > called? > > > > -sughosh > > > > > > > > > > How do you ensure that if a region reserved by LMB notification as > > > EfiLoaderData is coalesced with some other allocation LMB is not > > > requested to mark the coalesced region as reserved? > > > > > > @Tom > > > > > > Clinging to the existing logic that you can do anything when loading > > > files is obviously leading us into coding hell. > > > > > > If somebody wants to load two images into the same location, he should > > > be forced to unload the first image. This will allow us to have a single > > > memory management system. > > It seems we really shouldn't use the words 'allocate' and 'free' when > talking about LMB. They are simply reservations. Correct and while at it can we please make the code less confusing to read. What we today mark as reserved isnt even trully reserved as it can be overwritten. struct lmb_region memory -> available memory we added on LMB. That's fine struct lmb_region reserved -> can we rename this to 'used' and rename LMB_NOOVERWRITE to LMB_RESERVED? Thanks /Ilias > I believe we have got > into this situation due to an assumption that these two things are the > same, but in U-Boot they certainly are not. LMB is a very lighweight > and temporary reservation system to be used for a single boot process. > > Regards, > Simon > > > > > > > > Best regards > > > > > > Heinrich > > > > > > > changes in memory map should be notified to other interested modules > > > > so that the allocated memory does not get overwritten. Add an event > > > > handler in the EFI memory module to update the EFI memory map > > > > accordingly when such changes happen. As a consequence, any subsequent > > > > memory request would honour the updated memory map and only available > > > > memory would be allocated from. > > > > > > > > Signed-off-by: Sughosh Ganu > > > > --- > > > > lib/efi_loader/efi_memory.c | 70 ++--- > > > > 1 file changed, 58 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > > > > index 435e580fb3..93244161b0 100644 > > > > --- a/lib/efi_loader/efi_memory.c > > > > +++ b/lib/efi_loader/efi_memory.c > > > > @@ -73,6 +73,10 @@ struct efi_pool_allocation { > > > > #if CONFIG_IS_ENABLED(MEM_MAP_UPDATE_NOTIFY) > > > > extern bool is_addr_in_ram(uintptr_t addr); > > > > > > > > +static efi_status_t __efi_add_memory_map_pg(u64 start, u64 pages, > > > > + int memory_type, > > > > + bool overlap_only_ram); > > > > + > > > > static void efi_map_update_notify(u64 addr, u64 size, u8 op) > > > > { > > > > struct event_efi_mem_map_update efi_map = {0}; > > > > @@ -84,6 +88,34 @@ static void efi_map_update_notify(u64 addr, u64 > > > > size, u8 op) > > > > if (is_addr_in_ram((uintptr_t)addr)) > > > > event_notify(EVT_EFI_MEM_MAP_UPDATE, &efi_map, > > > > sizeof(efi_map)); > > > > } > > > > + > > > > +static int lmb_mem_map_update_sync(void *ctx, struct event *event) > > > > +{ > > > > + u8 op; > > > > + u64 addr; > > > > + u64 pages; > > > > + efi_status_t status; > > > > + struct event_lmb_map_update *lmb_map = &event->data.lmb_map; > > > > + > > > > + addr = (uintptr_t)map_sysmem(lmb_map->base, 0); > > > > + pages = efi_size_in_pages(lmb_map->size + (addr & EFI_PAGE_MASK)); > > > > + op = lmb_map->op; > > > > + addr &= ~EFI_PAGE_MASK; > > > > + > > > > + if (op != MAP_OP_RESERVE && op != MAP_OP_FREE) { > > > > + log_debug("Invalid map update op received (%d)\n", op); > > > > + return -1; > > > > + } > > > > + > > > > + status = __efi_add_memory_map_pg(addr, pages, > > > > + op == MAP_OP_FREE ? > > > > + EFI_CONVENTIONAL_MEMORY : > > > > + EFI_BOOT_SERVICES_DATA, > > > > + true); > > > > + > > > > + return
[PATCH] arm: dts: k3-am625-beagleplay: Package TIFS Stub
Add support for packaging the TIFS Stub as it's required for basic Low Power Modes like Deep Sleep. Signed-off-by: Dhruva Gole --- arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi | 33 +++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi index fb2032068d1c..5e2248a4a668 100644 --- a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi +++ b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi @@ -69,6 +69,23 @@ }; }; + tifsstub-gp { + filename = "tifsstub.bin_gp"; + ti-secure-rom { + content = <&tifsstub_gp>; + core = "secure"; + load = <0x6>; + sw-rev = ; + keyfile = "ti-degenerate-key.pem"; + tifsstub; + }; + tifsstub_gp: tifsstub-gp.bin { + filename = "ti-sysfw/ti-fs-stub-firmware-am62x-gp.bin"; + type = "blob-ext"; + optional; + }; + }; + ti-spl_unsigned { filename = "tispl.bin_unsigned"; pad-byte = <0xff>; @@ -105,6 +122,19 @@ }; }; + tifsstub-gp { + description = "tifsstub"; + type = "firmware"; + arch = "arm32"; + compression = "none"; + os = "tifsstub-gp"; + load = <0x9dc0>; + entry = <0x9dc0>; + blob-ext { + filename = "tifsstub.bin_gp"; + }; + }; + dm { description = "DM binary"; type = "firmware"; @@ -148,7 +178,8 @@ conf-0 { description = "k3-am625-beagleplay"; firmware = "atf"; - loadables = "tee", "dm", "spl"; + loadables = "tee", "dm", "spl", + "tifsstub-gp"; fdt = "fdt-0"; }; }; -- 2.34.1
Re: [RFC PATCH 15/31] efi_memory: add an event handler to update memory map
hi Ilias, On Wed, 12 Jun 2024 at 11:19, Ilias Apalodimas wrote: > > [...] > > > > > > > > > > + struct event_lmb_map_update *lmb_map = > > > > > > > > > &event->data.lmb_map; > > > > > > > > > + > > > > > > > > > + addr = (uintptr_t)map_sysmem(lmb_map->base, 0); > > > > > > > > > + pages = efi_size_in_pages(lmb_map->size + (addr & > > > > > > > > > EFI_PAGE_MASK)); > > > > > > > > > + op = lmb_map->op; > > > > > > > > > + addr &= ~EFI_PAGE_MASK; > > > > > > > > > + > > > > > > > > > + if (op != MAP_OP_RESERVE && op != MAP_OP_FREE) { > > > > > > > > > + log_debug("Invalid map update op received > > > > > > > > > (%d)\n", op); > > > > > > > > > + return -1; > > > > > > > > > + } > > > > > > > > > + > > > > > > > > > + status = __efi_add_memory_map_pg(addr, pages, > > > > > > > > > +op == MAP_OP_FREE ? > > > > > > > > > +EFI_CONVENTIONAL_MEMORY : > > > > > > > > > > > > > > > > This is dangerous. LMB might turn memory that is marked as > > > > > > > > EfiReservedMemory which the OS must respect into > > > > > > > > EfiBootServicesData > > > > > > > > which the OS may discard. > > > > > > > > > > > > > > > > E.g. initr_lmb() is being called after efi_memory_init(). > > > > > > > > > > > > > > > > Getting all cases of synchronization properly tested seems very > > > > > > > > hard to > > > > > > > > me. Everything would be much easier if we had only a single > > > > > > > > memory > > > > > > > > management system. > > > > > > > > > > > > > > Yes, Sughosh is working on the single memory reservation system > > > > > > > for > > > > > > > everyone to use. This pairs with the single memory allocation > > > > > > > system > > > > > > > (malloc) that we have. Parts of the code base that aren't keeping > > > > > > > these > > > > > > > systems up to date / obeying their results need to be corrected. > > > > > > > > > > > > The EFI allocations don't happen until boot time...so why do we need > > > > > > to do this now? We can instead have an EFI function to scan LMB and > > > > > > add to its memory map. > > > > > > > > > > We're talking about reservations, not allocations. So yes, when > > > > > someone > > > > > is making their reservation, they need to make it. I don't understand > > > > > your question. > > > > > > > > As I understand it, this is used to tell EFI about a memory reservation. > > > > > > This patch, or this series? This series isn't about EFI. This patch is, > > > yes. > > > > > > > But the EFI code can scan the LMB reservations just before booting and > > > > update its tables. I don't see a need to keep them in sync before the > > > > boot actually happens. > > > > > > But that wouldn't work. If something needs to reserve a region it needs > > > to do it when it starts using it. It's not about the EFI map for the OS, > > > it's about making sure that U-Boot doesn't scribble over a now-reserved > > > area. > > > > I'm not convinced of that yet. EFI does not do allocations until it > > starts loading images, > > It does in some cases. E.g The efi variables allocate some pages when > the subsystem starts, the TCG protocol allocates the EventLog once > it's installed and I am pretty sure we have more. > > > and it uses LMB for those (or at least it does > > with bootstd). I'm just trying to keep this all as simple as possible. > > Heinrich already pointed out a potential danger in the current design. > If an EFI allocation happens *before* LMB comes up, we might end up > updating the efi memory map with the wrong attributes. That would lead > to the OS discarding memory areas that should be preserved and we > won't figure that out until the OS boots and blows up. Like I mentioned in one of the earlier replies [1], this is actually not an issue, as long as modules are freeing memory that was actually allocated/reserved by them. In case of the EFI subsystem marking some memory region as EfiReservedMemory, this will send a notification to the LMB module, and the LMB module will mark this region as reserved. So unless some LMB user decides on freeing up memory that was not reserved by it, this should be just fine. Making use of the LMB API's as the common backend for allocating memory is a separate thing, and I will look into that. But the above highlighted issue is not one, with correctly working code. -sughosh [1] - https://lists.denx.de/pipermail/u-boot/2024-June/555883.html
Re: [RFC PATCH 04/31] lmb: remove local instances of the lmb structure variable
Am 12. Juni 2024 04:41:39 MESZ schrieb Simon Glass : >Hi Tom, > >On Tue, 11 Jun 2024 at 16:55, Tom Rini wrote: >> >> On Tue, Jun 11, 2024 at 04:08:56PM -0600, Simon Glass wrote: >> > Hi Tom, >> > >> > On Tue, 11 Jun 2024 at 15:01, Tom Rini wrote: >> > > >> > > On Tue, Jun 11, 2024 at 12:52:22PM -0600, Simon Glass wrote: >> > > > Hi Sughosh, >> > > > >> > > > On Fri, 7 Jun 2024 at 12:53, Sughosh Ganu >> > > > wrote: >> > > > > >> > > > > With the move of the LMB structure to a persistent state, there is no >> > > > > need to declare the variable locally, and pass it as part of the LMB >> > > > > API's. Remove all local variable instances and change the API's >> > > > > correspondingly. >> > > > > >> > > > > Signed-off-by: Sughosh Ganu >> > > > > --- >> > > > > arch/arc/lib/cache.c | 4 +- >> > > > > arch/arm/lib/stack.c | 4 +- >> > > > > arch/arm/mach-apple/board.c | 17 ++- >> > > > > arch/arm/mach-snapdragon/board.c | 17 ++- >> > > > > arch/arm/mach-stm32mp/dram_init.c| 7 +- >> > > > > arch/arm/mach-stm32mp/stm32mp1/cpu.c | 6 +- >> > > > > arch/m68k/lib/bootm.c| 7 +- >> > > > > arch/microblaze/lib/bootm.c | 4 +- >> > > > > arch/mips/lib/bootm.c| 9 +- >> > > > > arch/nios2/lib/bootm.c | 4 +- >> > > > > arch/powerpc/cpu/mpc85xx/mp.c| 4 +- >> > > > > arch/powerpc/include/asm/mp.h| 4 +- >> > > > > arch/powerpc/lib/bootm.c | 14 +- >> > > > > arch/riscv/lib/bootm.c | 4 +- >> > > > > arch/sh/lib/bootm.c | 4 +- >> > > > > arch/x86/lib/bootm.c | 4 +- >> > > > > arch/xtensa/lib/bootm.c | 4 +- >> > > > > board/xilinx/common/board.c | 7 +- >> > > > > boot/bootm.c | 26 ++-- >> > > > > boot/bootm_os.c | 5 +- >> > > > > boot/image-board.c | 32 ++--- >> > > > > boot/image-fdt.c | 29 ++--- >> > > > > cmd/bdinfo.c | 6 +- >> > > > > cmd/booti.c | 2 +- >> > > > > cmd/bootz.c | 2 +- >> > > > > cmd/load.c | 7 +- >> > > > > drivers/iommu/apple_dart.c | 7 +- >> > > > > drivers/iommu/sandbox_iommu.c| 15 +-- >> > > > > fs/fs.c | 7 +- >> > > > > include/image.h | 22 +--- >> > > > > include/lmb.h| 39 +++--- >> > > > > lib/lmb.c| 81 ++-- >> > > > > net/tftp.c | 5 +- >> > > > > net/wget.c | 5 +- >> > > > > test/cmd/bdinfo.c| 2 +- >> > > > > test/lib/lmb.c | 187 >> > > > > +-- >> > > > > 36 files changed, 270 insertions(+), 333 deletions(-) >> > > > >> > > > This isn't necessary...and it will make things harder. You can have a >> > > > global 'lmb' while still allowing passing a different pointer when >> > > > needed. >> > > >> > > There's only one reservation checking system and list of known >> > > reservations, keep in mind. >> > >> > There is only one driver model, too, but we use a pointer. It makes >> > tests much easier. >> > >> > In fact I see elsewhere in this series that it causes problems with >> > tests. Best to use a pointer so it is easy to update. >> >> Maybe? I worry that will lead to thinking that we still, like today, >> have many LMB lists, rather than a single LMB list that everyone must >> use. > >Do people worry about that with driver model? > >Also IMO there is only really one LMB list today. We create it at the >start of bootm and then it is done when we boot. The file-loading >stuff is what makes all this confusing...and with bootstd that is >under control as well. As we have a command line interface bootstd is not the only case to consider. We should not start making assumptions about the sequence of commands entered manually. Best regards Heinrich > >At lot of this effort seems to be about dealing with random scripts >which load things. We want to make sure we complain if something >overlaps. But we should be making the bootstd case work nicely and >doing things within that framework. Also EFI sort-of has its own >thing, which it is very-much in control of. > >Overall I think this is a bit more subtle that just combining allocators. > >Regards, >Simon
Re: [RFC PATCH 15/31] efi_memory: add an event handler to update memory map
Am 12. Juni 2024 04:42:00 MESZ schrieb Simon Glass : >Hi Tom, > >On Tue, 11 Jun 2024 at 16:54, Tom Rini wrote: >> >> On Tue, Jun 11, 2024 at 04:22:25PM -0600, Simon Glass wrote: >> > Hi Tom, >> > >> > On Tue, 11 Jun 2024 at 15:01, Tom Rini wrote: >> > > >> > > On Tue, Jun 11, 2024 at 12:52:19PM -0600, Simon Glass wrote: >> > > > Hi, >> > > > >> > > > On Tue, 11 Jun 2024 at 08:36, Tom Rini wrote: >> > > > > >> > > > > On Tue, Jun 11, 2024 at 12:17:16PM +0200, Heinrich Schuchardt wrote: >> > > > > > On 07.06.24 20:52, Sughosh Ganu wrote: >> > > > > > > There are events that would be used to notify other interested >> > > > > > > modules >> > > > > > > of any changes in available and occupied memory. This would >> > > > > > > happen >> > > > > > > when a module allocates or reserves memory, or frees up memory. >> > > > > > > These >> > > > > > > changes in memory map should be notified to other interested >> > > > > > > modules >> > > > > > > so that the allocated memory does not get overwritten. Add an >> > > > > > > event >> > > > > > > handler in the EFI memory module to update the EFI memory map >> > > > > > > accordingly when such changes happen. As a consequence, any >> > > > > > > subsequent >> > > > > > > memory request would honour the updated memory map and only >> > > > > > > available >> > > > > > > memory would be allocated from. >> > > > > > > >> > > > > > > Signed-off-by: Sughosh Ganu >> > > > > > > --- >> > > > > > > lib/efi_loader/efi_memory.c | 70 >> > > > > > > ++--- >> > > > > > > 1 file changed, 58 insertions(+), 12 deletions(-) >> > > > > > > >> > > > > > > diff --git a/lib/efi_loader/efi_memory.c >> > > > > > > b/lib/efi_loader/efi_memory.c >> > > > > > > index 435e580fb3..93244161b0 100644 >> > > > > > > --- a/lib/efi_loader/efi_memory.c >> > > > > > > +++ b/lib/efi_loader/efi_memory.c >> > > > > > > @@ -73,6 +73,10 @@ struct efi_pool_allocation { >> > > > > > > #if CONFIG_IS_ENABLED(MEM_MAP_UPDATE_NOTIFY) >> > > > > > > extern bool is_addr_in_ram(uintptr_t addr); >> > > > > > > >> > > > > > > +static efi_status_t __efi_add_memory_map_pg(u64 start, u64 >> > > > > > > pages, >> > > > > > > + int memory_type, >> > > > > > > + bool overlap_only_ram); >> > > > > > > + >> > > > > > > static void efi_map_update_notify(u64 addr, u64 size, u8 op) >> > > > > > > { >> > > > > > > struct event_efi_mem_map_update efi_map = {0}; >> > > > > > > @@ -84,6 +88,34 @@ static void efi_map_update_notify(u64 addr, >> > > > > > > u64 size, u8 op) >> > > > > > > if (is_addr_in_ram((uintptr_t)addr)) >> > > > > > > event_notify(EVT_EFI_MEM_MAP_UPDATE, &efi_map, >> > > > > > > sizeof(efi_map)); >> > > > > > > } >> > > > > > > + >> > > > > > > +static int lmb_mem_map_update_sync(void *ctx, struct event >> > > > > > > *event) >> > > > > > > +{ >> > > > > > > + u8 op; >> > > > > > > + u64 addr; >> > > > > > > + u64 pages; >> > > > > > > + efi_status_t status; >> > > > > > > + struct event_lmb_map_update *lmb_map = &event->data.lmb_map; >> > > > > > > + >> > > > > > > + addr = (uintptr_t)map_sysmem(lmb_map->base, 0); >> > > > > > > + pages = efi_size_in_pages(lmb_map->size + (addr & >> > > > > > > EFI_PAGE_MASK)); >> > > > > > > + op = lmb_map->op; >> > > > > > > + addr &= ~EFI_PAGE_MASK; >> > > > > > > + >> > > > > > > + if (op != MAP_OP_RESERVE && op != MAP_OP_FREE) { >> > > > > > > + log_debug("Invalid map update op received (%d)\n", >> > > > > > > op); >> > > > > > > + return -1; >> > > > > > > + } >> > > > > > > + >> > > > > > > + status = __efi_add_memory_map_pg(addr, pages, >> > > > > > > +op == MAP_OP_FREE ? >> > > > > > > +EFI_CONVENTIONAL_MEMORY : >> > > > > > >> > > > > > This is dangerous. LMB might turn memory that is marked as >> > > > > > EfiReservedMemory which the OS must respect into >> > > > > > EfiBootServicesData >> > > > > > which the OS may discard. >> > > > > > >> > > > > > E.g. initr_lmb() is being called after efi_memory_init(). >> > > > > > >> > > > > > Getting all cases of synchronization properly tested seems very >> > > > > > hard to >> > > > > > me. Everything would be much easier if we had only a single memory >> > > > > > management system. >> > > > > >> > > > > Yes, Sughosh is working on the single memory reservation system for >> > > > > everyone to use. This pairs with the single memory allocation system >> > > > > (malloc) that we have. Parts of the code base that aren't keeping >> > > > > these >> > > > > systems up to date / obeying their results need to be corrected. >> > > > >> > > > The EFI allocations don't happen until boot time...so why do we need >> > > > to do this now? We can instead have an EFI function to scan LMB and >> > > > add to its memory map. >> > > >> > > We're talking about reserv
Re: [PATCH 5/9] fdt: Correct condition for bloblist existing
[...] > > > >> --- > > > >> > > > >> lib/fdtdec.c | 12 ++-- > > > >> 1 file changed, 10 insertions(+), 2 deletions(-) > > > >> > > > >> diff --git a/lib/fdtdec.c b/lib/fdtdec.c > > > >> index b2c59ab3818..b141244e3b9 100644 > > > >> --- a/lib/fdtdec.c > > > >> +++ b/lib/fdtdec.c > > > >> @@ -1669,8 +1669,16 @@ int fdtdec_setup(void) > > > >> { > > > >> int ret = -ENOENT; > > > >> > > > >> - /* If allowing a bloblist, check that first */ > > > >> - if (CONFIG_IS_ENABLED(BLOBLIST)) { > > > >> + /* > > > >> +* If allowing a bloblist, check that first. This would be > > > >> better > > > >> +* handled with an OF_BLOBLIST Kconfig, but that caused far > > > >> too much > > > >> +* argument, so add a hack here, used e.g. by chromebook_coral > > > > > > > > I am a bit confused by this comment - It means you will not use > > > > OF_BLOBLIST, > > > > but actually you are using it below. Is it a typo? > > > > > > Basically it would be cleaner to have a separate, phase-specific > > > Kconfig control as to whether the DT can come from the bloblist (I > > > can't remember the Kconfig name I suggested, nor the patch as it was > > > last year sometime). But for now I am adding this hack to get a few > > > boards working again. > > > > I am a bit confused. > > First of all the comment is innapropriate. We went through a lengthy > > discussion on BLOBLIST/OF_BLOSLIST etc and, even Tom chimed in and we > > made up our minds. Why are you adding this comment now? Why do code > > comments have to illustrate your personal opinion -- which was > > rejected? > > I'm sorry for the tone of the comment. I am not trying to offend > anyone here and I'm happy to change the language. Yes please a comment explaining why that piece of code is there is far more intuitive. > As I probably > mentioned at the time, my accepted patch breaks my workflow and > several boards. I hope you can understand how frustrating that sort of > thing can be. Yes, I do and I am fine with a short-term patch that fixes issues, as long as its not too intrusive. There is a very thin line between quick and dirty fixes to spaghetti unreadable code. But we should have comments and/or commit messages indicating that this needs a proper fix > Also, now that I have my lab back up and running and I > would like these boards to work again on mainline! > > > > > Grepping for OF_BLOBLIST, I can't find any matches, so is the above if a > > typo? > > Remember, it was a patch you rejected :-) I don't maintain any of that. I only gave some feedback along the lines of "bloblist was designed to be auto-discoverable, I don't see how adding an explicit Kconfig helps". IIRC we eventually followed what Tom suggested. In any case, the amount of bike-shedding in the topic is too much. Do you mind explaining the problem in your workflow again? Perhaps we can find a solution that is integrated in bloblist_maybe_init() instead of injecting ifs on when a bloblist should or should not be searched for all over the place. Regards /Ilias > > Regards, > Simon > > > > > > > > > Thanks > > /Ilias > > > > > > > > > > >> > > > >> +* The necessary test is whether the previous stage passed a > > > >> bloblist, > > > >> +* not whether this one creates one. > > > >> +*/ > > > >> + if (CONFIG_IS_ENABLED(OF_BLOBLIST) && > > > >> + (spl_prev_phase() != PHASE_TPL || > > > >> +!IS_ENABLED(CONFIG_TPL_BLOBLIST))) { > > > >> ret = bloblist_maybe_init(); > > > >> if (!ret) { > > > >> gd->fdt_blob = > > > >> bloblist_find(BLOBLISTT_CONTROL_FDT, 0); > > > >> -- > > > >> 2.34.1 > > > >> > > > > > > > > Regards, > > > > Raymond > > > > > > Regards, > > > Simon
Re: [PATCH 01/16] lib: fdtdec: Handle multiple memory nodes
Am 11. Juni 2024 20:51:55 MESZ schrieb Simon Glass : >Hi Jiaxun, > >On Tue, 11 Jun 2024 at 07:48, Jiaxun Yang wrote: >> >> >> >> 在2024年6月11日六月 下午1:26,Heinrich Schuchardt写道: >> Hi Heinrich, >> >> Thanks for your comments, will fix most of them in next reversion. >> >> [...] >> >> -gd->bd->bi_dram[bank].start = (phys_addr_t)res.start; >> > >> > The conversion is superfluous. >> >> This is necessary as phys_addr_t can be unsigned long or unsigned long long. In C++ you would get an error. In C an assignment to an integer type of different size does not lead to a warning. Best regards Heinrich >> >> > >> [...] >> >> +__maybe_unused const char *final_name; >> >> +__maybe_unused int final_reg; >> > >> > '__maybe_unused' is superfluous. >> >> This is necessary as when LOG_DEBUG is disabled these two variables won't >> be referenced. Compilers would complain about it. >> >> > >> >> >> >> gd->ram_base = (unsigned long)~0; >> > >> > gd->ram_base = ULONG_MAX; >> > >> >> >> [...] >> >> Thanks >> -- > >Can you please add a test for this function? Perhaps test/dm/fdtdec.c >might be a good place? > >Regards, >Simon > >> - Jiaxun
Re: [PATCH 17/42] google: Disable TPMv2 on most Chromebooks
On Tue, 11 Jun 2024 at 23:02, Simon Glass wrote: > > This feature is not present on older Chromebooks, so disable the > setting. > > Signed-off-by: Simon Glass > --- > > configs/chromebook_link64_defconfig| 1 + > configs/chromebook_link_defconfig | 1 + > configs/chromebook_samus_defconfig | 1 + > configs/chromebook_samus_tpl_defconfig | 1 + > configs/nyan-big_defconfig | 4 +--- > configs/snow_defconfig | 1 + > 6 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/configs/chromebook_link64_defconfig > b/configs/chromebook_link64_defconfig > index 7cf23b29e46..9583f87bf0f 100644 > --- a/configs/chromebook_link64_defconfig > +++ b/configs/chromebook_link64_defconfig > @@ -80,6 +80,7 @@ CONFIG_SYS_NS16550=y > CONFIG_SYS_NS16550_PORT_MAPPED=y > CONFIG_SPI=y > CONFIG_TPM_TIS_LPC=y > +# CONFIG_TPM_V2 is not set > CONFIG_USB_STORAGE=y > CONFIG_USB_KEYBOARD=y > CONFIG_FRAMEBUFFER_SET_VESA_MODE=y > diff --git a/configs/chromebook_link_defconfig > b/configs/chromebook_link_defconfig > index a9f91dd9b26..75a7a488a92 100644 > --- a/configs/chromebook_link_defconfig > +++ b/configs/chromebook_link_defconfig > @@ -70,6 +70,7 @@ CONFIG_SYS_NS16550_PORT_MAPPED=y > CONFIG_SOUND=y > CONFIG_SPI=y > CONFIG_TPM_TIS_LPC=y > +# CONFIG_TPM_V2 is not set > CONFIG_USB_STORAGE=y > CONFIG_USB_KEYBOARD=y > CONFIG_VIDEO_COPY=y > diff --git a/configs/chromebook_samus_defconfig > b/configs/chromebook_samus_defconfig > index 40cc449b9b3..8cdad8d2344 100644 > --- a/configs/chromebook_samus_defconfig > +++ b/configs/chromebook_samus_defconfig > @@ -74,6 +74,7 @@ CONFIG_SOUND_I8254=y > CONFIG_SOUND_RT5677=y > CONFIG_SPI=y > CONFIG_TPM_TIS_LPC=y > +# CONFIG_TPM_V2 is not set > CONFIG_USB_STORAGE=y > CONFIG_USB_KEYBOARD=y > CONFIG_VIDEO_COPY=y > diff --git a/configs/chromebook_samus_tpl_defconfig > b/configs/chromebook_samus_tpl_defconfig > index 3e7298f16af..1be57560f89 100644 > --- a/configs/chromebook_samus_tpl_defconfig > +++ b/configs/chromebook_samus_tpl_defconfig > @@ -96,6 +96,7 @@ CONFIG_SOUND_RT5677=y > CONFIG_SPI=y > CONFIG_TPL_SYSRESET=y > CONFIG_TPM_TIS_LPC=y > +# CONFIG_TPM_V2 is not set > CONFIG_USB_STORAGE=y > CONFIG_USB_KEYBOARD=y > CONFIG_FRAMEBUFFER_SET_VESA_MODE=y > diff --git a/configs/nyan-big_defconfig b/configs/nyan-big_defconfig > index 4dec710cf8d..78fb7580da7 100644 > --- a/configs/nyan-big_defconfig > +++ b/configs/nyan-big_defconfig > @@ -11,8 +11,6 @@ CONFIG_DEFAULT_DEVICE_TREE="tegra124-nyan-big" > CONFIG_SPL_TEXT_BASE=0x80108000 > CONFIG_SPL_STACK=0x800c > CONFIG_BOOTSTAGE_STASH_ADDR=0x8300 > -CONFIG_DEBUG_UART_BASE=0x70006000 > -CONFIG_DEBUG_UART_CLOCK=40800 > CONFIG_TEGRA124=y > CONFIG_TARGET_NYAN_BIG=y > CONFIG_TEGRA_GPU=y > @@ -75,7 +73,6 @@ CONFIG_DM_REGULATOR=y > CONFIG_REGULATOR_AS3722=y > CONFIG_DM_REGULATOR_FIXED=y > CONFIG_PWM_TEGRA=y > -CONFIG_DEBUG_UART_SHIFT=2 > CONFIG_SYS_NS16550=y > CONFIG_SOUND=y > CONFIG_I2S=y > @@ -83,6 +80,7 @@ CONFIG_I2S_TEGRA=y > CONFIG_SOUND_MAX98090=y > CONFIG_TEGRA114_SPI=y > CONFIG_TPM_TIS_INFINEON=y > +# CONFIG_TPM_V2 is not set > CONFIG_USB=y > CONFIG_USB_EHCI_HCD=y > CONFIG_USB_EHCI_TEGRA=y > diff --git a/configs/snow_defconfig b/configs/snow_defconfig > index 3a617c6cf40..2c0757194bd 100644 > --- a/configs/snow_defconfig > +++ b/configs/snow_defconfig > @@ -88,6 +88,7 @@ CONFIG_SOUND_MAX98095=y > CONFIG_SOUND_WM8994=y > CONFIG_EXYNOS_SPI=y > CONFIG_TPM_TIS_INFINEON=y > +# CONFIG_TPM_V2 is not set > CONFIG_USB=y > CONFIG_USB_XHCI_HCD=y > CONFIG_USB_XHCI_DWC3=y > -- > 2.34.1 > Reviewed-by: Ilias Apalodimas
Re: [RFC PATCH 15/31] efi_memory: add an event handler to update memory map
[...] > > > > > > > > + struct event_lmb_map_update *lmb_map = &event->data.lmb_map; > > > > > > > > + > > > > > > > > + addr = (uintptr_t)map_sysmem(lmb_map->base, 0); > > > > > > > > + pages = efi_size_in_pages(lmb_map->size + (addr & > > > > > > > > EFI_PAGE_MASK)); > > > > > > > > + op = lmb_map->op; > > > > > > > > + addr &= ~EFI_PAGE_MASK; > > > > > > > > + > > > > > > > > + if (op != MAP_OP_RESERVE && op != MAP_OP_FREE) { > > > > > > > > + log_debug("Invalid map update op received (%d)\n", > > > > > > > > op); > > > > > > > > + return -1; > > > > > > > > + } > > > > > > > > + > > > > > > > > + status = __efi_add_memory_map_pg(addr, pages, > > > > > > > > +op == MAP_OP_FREE ? > > > > > > > > +EFI_CONVENTIONAL_MEMORY : > > > > > > > > > > > > > > This is dangerous. LMB might turn memory that is marked as > > > > > > > EfiReservedMemory which the OS must respect into > > > > > > > EfiBootServicesData > > > > > > > which the OS may discard. > > > > > > > > > > > > > > E.g. initr_lmb() is being called after efi_memory_init(). > > > > > > > > > > > > > > Getting all cases of synchronization properly tested seems very > > > > > > > hard to > > > > > > > me. Everything would be much easier if we had only a single memory > > > > > > > management system. > > > > > > > > > > > > Yes, Sughosh is working on the single memory reservation system for > > > > > > everyone to use. This pairs with the single memory allocation system > > > > > > (malloc) that we have. Parts of the code base that aren't keeping > > > > > > these > > > > > > systems up to date / obeying their results need to be corrected. > > > > > > > > > > The EFI allocations don't happen until boot time...so why do we need > > > > > to do this now? We can instead have an EFI function to scan LMB and > > > > > add to its memory map. > > > > > > > > We're talking about reservations, not allocations. So yes, when someone > > > > is making their reservation, they need to make it. I don't understand > > > > your question. > > > > > > As I understand it, this is used to tell EFI about a memory reservation. > > > > This patch, or this series? This series isn't about EFI. This patch is, > > yes. > > > > > But the EFI code can scan the LMB reservations just before booting and > > > update its tables. I don't see a need to keep them in sync before the > > > boot actually happens. > > > > But that wouldn't work. If something needs to reserve a region it needs > > to do it when it starts using it. It's not about the EFI map for the OS, > > it's about making sure that U-Boot doesn't scribble over a now-reserved > > area. > > I'm not convinced of that yet. EFI does not do allocations until it > starts loading images, It does in some cases. E.g The efi variables allocate some pages when the subsystem starts, the TCG protocol allocates the EventLog once it's installed and I am pretty sure we have more. > and it uses LMB for those (or at least it does > with bootstd). I'm just trying to keep this all as simple as possible. Heinrich already pointed out a potential danger in the current design. If an EFI allocation happens *before* LMB comes up, we might end up updating the efi memory map with the wrong attributes. That would lead to the OS discarding memory areas that should be preserved and we won't figure that out until the OS boots and blows up. Cheers /Ilias > > Regards, > Simon
Re: [RFC PATCH 04/31] lmb: remove local instances of the lmb structure variable
On Wed, 12 Jun 2024 at 05:41, Simon Glass wrote: > > Hi Tom, > > On Tue, 11 Jun 2024 at 16:55, Tom Rini wrote: > > > > On Tue, Jun 11, 2024 at 04:08:56PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Tue, 11 Jun 2024 at 15:01, Tom Rini wrote: > > > > > > > > On Tue, Jun 11, 2024 at 12:52:22PM -0600, Simon Glass wrote: > > > > > Hi Sughosh, > > > > > > > > > > On Fri, 7 Jun 2024 at 12:53, Sughosh Ganu > > > > > wrote: > > > > > > > > > > > > With the move of the LMB structure to a persistent state, there is > > > > > > no > > > > > > need to declare the variable locally, and pass it as part of the LMB > > > > > > API's. Remove all local variable instances and change the API's > > > > > > correspondingly. > > > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > > --- > > > > > > arch/arc/lib/cache.c | 4 +- > > > > > > arch/arm/lib/stack.c | 4 +- > > > > > > arch/arm/mach-apple/board.c | 17 ++- > > > > > > arch/arm/mach-snapdragon/board.c | 17 ++- > > > > > > arch/arm/mach-stm32mp/dram_init.c| 7 +- > > > > > > arch/arm/mach-stm32mp/stm32mp1/cpu.c | 6 +- > > > > > > arch/m68k/lib/bootm.c| 7 +- > > > > > > arch/microblaze/lib/bootm.c | 4 +- > > > > > > arch/mips/lib/bootm.c| 9 +- > > > > > > arch/nios2/lib/bootm.c | 4 +- > > > > > > arch/powerpc/cpu/mpc85xx/mp.c| 4 +- > > > > > > arch/powerpc/include/asm/mp.h| 4 +- > > > > > > arch/powerpc/lib/bootm.c | 14 +- > > > > > > arch/riscv/lib/bootm.c | 4 +- > > > > > > arch/sh/lib/bootm.c | 4 +- > > > > > > arch/x86/lib/bootm.c | 4 +- > > > > > > arch/xtensa/lib/bootm.c | 4 +- > > > > > > board/xilinx/common/board.c | 7 +- > > > > > > boot/bootm.c | 26 ++-- > > > > > > boot/bootm_os.c | 5 +- > > > > > > boot/image-board.c | 32 ++--- > > > > > > boot/image-fdt.c | 29 ++--- > > > > > > cmd/bdinfo.c | 6 +- > > > > > > cmd/booti.c | 2 +- > > > > > > cmd/bootz.c | 2 +- > > > > > > cmd/load.c | 7 +- > > > > > > drivers/iommu/apple_dart.c | 7 +- > > > > > > drivers/iommu/sandbox_iommu.c| 15 +-- > > > > > > fs/fs.c | 7 +- > > > > > > include/image.h | 22 +--- > > > > > > include/lmb.h| 39 +++--- > > > > > > lib/lmb.c| 81 ++-- > > > > > > net/tftp.c | 5 +- > > > > > > net/wget.c | 5 +- > > > > > > test/cmd/bdinfo.c| 2 +- > > > > > > test/lib/lmb.c | 187 > > > > > > +-- > > > > > > 36 files changed, 270 insertions(+), 333 deletions(-) > > > > > > > > > > This isn't necessary...and it will make things harder. You can have a > > > > > global 'lmb' while still allowing passing a different pointer when > > > > > needed. > > > > > > > > There's only one reservation checking system and list of known > > > > reservations, keep in mind. > > > > > > There is only one driver model, too, but we use a pointer. It makes > > > tests much easier. > > > > > > In fact I see elsewhere in this series that it causes problems with > > > tests. Best to use a pointer so it is easy to update. I don't mind keeping it, but OTOH we are fundamentally changing how lmb works, do we need to expose a ptr to make tests easier? We should be aiming for code that is more readable and easier to maintain. Isn't it better to just rewrite the tests replicating the existing cases & add new potential ones? > > > > Maybe? I worry that will lead to thinking that we still, like today, > > have many LMB lists, rather than a single LMB list that everyone must > > use. > > Do people worry about that with driver model? > > Also IMO there is only really one LMB list today. We create it at the > start of bootm and then it is done when we boot. The file-loading > stuff is what makes all this confusing...and with bootstd that is > under control as well. > > At lot of this effort seems to be about dealing with random scripts > which load things. We want to make sure we complain if something > overlaps. But we should be making the bootstd case work nicely and > doing things within that framework. Also EFI sort-of has its own > thing, which it is very-much in control of. > I agree that things should be nicely integrated in bootstd, but does keeping a ptr as an argument help with that ? > Overall I think this is a bit more subtle that just combining allocators. Regards /Ilias > > Regards, > Simon
Re: [RFC PATCH 15/31] efi_memory: add an event handler to update memory map
Hi Tom, On Tue, 11 Jun 2024 at 16:54, Tom Rini wrote: > > On Tue, Jun 11, 2024 at 04:22:25PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Tue, 11 Jun 2024 at 15:01, Tom Rini wrote: > > > > > > On Tue, Jun 11, 2024 at 12:52:19PM -0600, Simon Glass wrote: > > > > Hi, > > > > > > > > On Tue, 11 Jun 2024 at 08:36, Tom Rini wrote: > > > > > > > > > > On Tue, Jun 11, 2024 at 12:17:16PM +0200, Heinrich Schuchardt wrote: > > > > > > On 07.06.24 20:52, Sughosh Ganu wrote: > > > > > > > There are events that would be used to notify other interested > > > > > > > modules > > > > > > > of any changes in available and occupied memory. This would happen > > > > > > > when a module allocates or reserves memory, or frees up memory. > > > > > > > These > > > > > > > changes in memory map should be notified to other interested > > > > > > > modules > > > > > > > so that the allocated memory does not get overwritten. Add an > > > > > > > event > > > > > > > handler in the EFI memory module to update the EFI memory map > > > > > > > accordingly when such changes happen. As a consequence, any > > > > > > > subsequent > > > > > > > memory request would honour the updated memory map and only > > > > > > > available > > > > > > > memory would be allocated from. > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > > > --- > > > > > > > lib/efi_loader/efi_memory.c | 70 > > > > > > > ++--- > > > > > > > 1 file changed, 58 insertions(+), 12 deletions(-) > > > > > > > > > > > > > > diff --git a/lib/efi_loader/efi_memory.c > > > > > > > b/lib/efi_loader/efi_memory.c > > > > > > > index 435e580fb3..93244161b0 100644 > > > > > > > --- a/lib/efi_loader/efi_memory.c > > > > > > > +++ b/lib/efi_loader/efi_memory.c > > > > > > > @@ -73,6 +73,10 @@ struct efi_pool_allocation { > > > > > > > #if CONFIG_IS_ENABLED(MEM_MAP_UPDATE_NOTIFY) > > > > > > > extern bool is_addr_in_ram(uintptr_t addr); > > > > > > > > > > > > > > +static efi_status_t __efi_add_memory_map_pg(u64 start, u64 pages, > > > > > > > + int memory_type, > > > > > > > + bool overlap_only_ram); > > > > > > > + > > > > > > > static void efi_map_update_notify(u64 addr, u64 size, u8 op) > > > > > > > { > > > > > > > struct event_efi_mem_map_update efi_map = {0}; > > > > > > > @@ -84,6 +88,34 @@ static void efi_map_update_notify(u64 addr, > > > > > > > u64 size, u8 op) > > > > > > > if (is_addr_in_ram((uintptr_t)addr)) > > > > > > > event_notify(EVT_EFI_MEM_MAP_UPDATE, &efi_map, > > > > > > > sizeof(efi_map)); > > > > > > > } > > > > > > > + > > > > > > > +static int lmb_mem_map_update_sync(void *ctx, struct event > > > > > > > *event) > > > > > > > +{ > > > > > > > + u8 op; > > > > > > > + u64 addr; > > > > > > > + u64 pages; > > > > > > > + efi_status_t status; > > > > > > > + struct event_lmb_map_update *lmb_map = &event->data.lmb_map; > > > > > > > + > > > > > > > + addr = (uintptr_t)map_sysmem(lmb_map->base, 0); > > > > > > > + pages = efi_size_in_pages(lmb_map->size + (addr & > > > > > > > EFI_PAGE_MASK)); > > > > > > > + op = lmb_map->op; > > > > > > > + addr &= ~EFI_PAGE_MASK; > > > > > > > + > > > > > > > + if (op != MAP_OP_RESERVE && op != MAP_OP_FREE) { > > > > > > > + log_debug("Invalid map update op received (%d)\n", > > > > > > > op); > > > > > > > + return -1; > > > > > > > + } > > > > > > > + > > > > > > > + status = __efi_add_memory_map_pg(addr, pages, > > > > > > > +op == MAP_OP_FREE ? > > > > > > > +EFI_CONVENTIONAL_MEMORY : > > > > > > > > > > > > This is dangerous. LMB might turn memory that is marked as > > > > > > EfiReservedMemory which the OS must respect into EfiBootServicesData > > > > > > which the OS may discard. > > > > > > > > > > > > E.g. initr_lmb() is being called after efi_memory_init(). > > > > > > > > > > > > Getting all cases of synchronization properly tested seems very > > > > > > hard to > > > > > > me. Everything would be much easier if we had only a single memory > > > > > > management system. > > > > > > > > > > Yes, Sughosh is working on the single memory reservation system for > > > > > everyone to use. This pairs with the single memory allocation system > > > > > (malloc) that we have. Parts of the code base that aren't keeping > > > > > these > > > > > systems up to date / obeying their results need to be corrected. > > > > > > > > The EFI allocations don't happen until boot time...so why do we need > > > > to do this now? We can instead have an EFI function to scan LMB and > > > > add to its memory map. > > > > > > We're talking about reservations, not allocations. So yes, when someone > > > is making their reservation, they need to make it. I don't understand > > > your question. > > > > As I understand it, this is used to tell EFI about a memory
Re: [RFC PATCH 04/31] lmb: remove local instances of the lmb structure variable
Hi Tom, On Tue, 11 Jun 2024 at 16:55, Tom Rini wrote: > > On Tue, Jun 11, 2024 at 04:08:56PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Tue, 11 Jun 2024 at 15:01, Tom Rini wrote: > > > > > > On Tue, Jun 11, 2024 at 12:52:22PM -0600, Simon Glass wrote: > > > > Hi Sughosh, > > > > > > > > On Fri, 7 Jun 2024 at 12:53, Sughosh Ganu > > > > wrote: > > > > > > > > > > With the move of the LMB structure to a persistent state, there is no > > > > > need to declare the variable locally, and pass it as part of the LMB > > > > > API's. Remove all local variable instances and change the API's > > > > > correspondingly. > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > --- > > > > > arch/arc/lib/cache.c | 4 +- > > > > > arch/arm/lib/stack.c | 4 +- > > > > > arch/arm/mach-apple/board.c | 17 ++- > > > > > arch/arm/mach-snapdragon/board.c | 17 ++- > > > > > arch/arm/mach-stm32mp/dram_init.c| 7 +- > > > > > arch/arm/mach-stm32mp/stm32mp1/cpu.c | 6 +- > > > > > arch/m68k/lib/bootm.c| 7 +- > > > > > arch/microblaze/lib/bootm.c | 4 +- > > > > > arch/mips/lib/bootm.c| 9 +- > > > > > arch/nios2/lib/bootm.c | 4 +- > > > > > arch/powerpc/cpu/mpc85xx/mp.c| 4 +- > > > > > arch/powerpc/include/asm/mp.h| 4 +- > > > > > arch/powerpc/lib/bootm.c | 14 +- > > > > > arch/riscv/lib/bootm.c | 4 +- > > > > > arch/sh/lib/bootm.c | 4 +- > > > > > arch/x86/lib/bootm.c | 4 +- > > > > > arch/xtensa/lib/bootm.c | 4 +- > > > > > board/xilinx/common/board.c | 7 +- > > > > > boot/bootm.c | 26 ++-- > > > > > boot/bootm_os.c | 5 +- > > > > > boot/image-board.c | 32 ++--- > > > > > boot/image-fdt.c | 29 ++--- > > > > > cmd/bdinfo.c | 6 +- > > > > > cmd/booti.c | 2 +- > > > > > cmd/bootz.c | 2 +- > > > > > cmd/load.c | 7 +- > > > > > drivers/iommu/apple_dart.c | 7 +- > > > > > drivers/iommu/sandbox_iommu.c| 15 +-- > > > > > fs/fs.c | 7 +- > > > > > include/image.h | 22 +--- > > > > > include/lmb.h| 39 +++--- > > > > > lib/lmb.c| 81 ++-- > > > > > net/tftp.c | 5 +- > > > > > net/wget.c | 5 +- > > > > > test/cmd/bdinfo.c| 2 +- > > > > > test/lib/lmb.c | 187 > > > > > +-- > > > > > 36 files changed, 270 insertions(+), 333 deletions(-) > > > > > > > > This isn't necessary...and it will make things harder. You can have a > > > > global 'lmb' while still allowing passing a different pointer when > > > > needed. > > > > > > There's only one reservation checking system and list of known > > > reservations, keep in mind. > > > > There is only one driver model, too, but we use a pointer. It makes > > tests much easier. > > > > In fact I see elsewhere in this series that it causes problems with > > tests. Best to use a pointer so it is easy to update. > > Maybe? I worry that will lead to thinking that we still, like today, > have many LMB lists, rather than a single LMB list that everyone must > use. Do people worry about that with driver model? Also IMO there is only really one LMB list today. We create it at the start of bootm and then it is done when we boot. The file-loading stuff is what makes all this confusing...and with bootstd that is under control as well. At lot of this effort seems to be about dealing with random scripts which load things. We want to make sure we complain if something overlaps. But we should be making the bootstd case work nicely and doing things within that framework. Also EFI sort-of has its own thing, which it is very-much in control of. Overall I think this is a bit more subtle that just combining allocators. Regards, Simon
[PATCH v3 9/9] doc: convert README.LED to .rst documentation
Convert README.LED to .rst documentation and include all the relevant documentation in the status_led.h. Signed-off-by: Christian Marangi --- doc/README.LED | 77 -- doc/api/index.rst | 1 + doc/api/status_led.rst | 35 +++ include/status_led.h | 224 - 4 files changed, 256 insertions(+), 81 deletions(-) delete mode 100644 doc/README.LED create mode 100644 doc/api/status_led.rst diff --git a/doc/README.LED b/doc/README.LED deleted file mode 100644 index c21c9d53ec3..000 --- a/doc/README.LED +++ /dev/null @@ -1,77 +0,0 @@ -Status LED - - -This README describes the status LED API. - -The API is defined by the include file include/status_led.h - -The first step is to enable CONFIG_LED_STATUS in menuconfig: -> Device Drivers > LED Support. - -If the LED support is only for specific board, enable -CONFIG_LED_STATUS_BOARD_SPECIFIC in the menuconfig. - -Status LEDS 0 to 5 are enabled by the following configurations at menuconfig: -CONFIG_STATUS_LED0, CONFIG_STATUS_LED1, ... CONFIG_STATUS_LED5 - -The following should be configured for each of the enabled LEDs: -CONFIG_STATUS_LED_BIT -CONFIG_STATUS_LED_STATE -CONFIG_STATUS_LED_FREQ -Where is an integer 1 through 5 (empty for 0). - -CONFIG_STATUS_LED_BIT is passed into the __led_* functions to identify which LED -is being acted on. As such, the value choose must be unique with with respect to -the other CONFIG_STATUS_LED_BIT's. Mapping the value to a physical LED is the -reponsiblity of the __led_* function. - -CONFIG_STATUS_LED_STATE is the initial state of the LED. It should be set to one -of these values: CONFIG_LED_STATUS_OFF or CONFIG_LED_STATUS_ON. - -CONFIG_STATUS_LED_FREQ determines the LED blink frequency. -Values range from 2 to 10. - -Some other LED macros -- - -CONFIG_STATUS_LED_BOOT is the LED to light when the board is booting. -This must be a valid LED number (0-5). - -CONFIG_STATUS_LED_RED is the red LED. It is used to signal errors. This must be -a valid LED number (0-5). Other similar color LED's macros are -CONFIG_STATUS_LED_GREEN, CONFIG_STATUS_LED_YELLOW and CONFIG_STATUS_LED_BLUE. - -General LED functions -- -The following functions should be defined: - -__led_init is called once to initialize the LED to CONFIG_STATUS_LED_STATE. -One time start up code should be placed here. - -__led_set is called to change the state of the LED. - -__led_toggle is called to toggle the current state of the LED. - -Colour LED - - -Colour LED's are at present only used by ARM. - -The functions names explain their purpose. - -coloured_LED_init -red_LED_on -red_LED_off -green_LED_on -green_LED_off -yellow_LED_on -yellow_LED_off -blue_LED_on -blue_LED_off - -These are weakly defined in arch/arm/lib/board.c to noops. Where applicable, define -these functions in the board specific source. - -TBD : Describe older board dependent macros similar to what is done for - -TBD : Describe general support via asm/status_led.h diff --git a/doc/api/index.rst b/doc/api/index.rst index 51b2013af36..d40e90801d1 100644 --- a/doc/api/index.rst +++ b/doc/api/index.rst @@ -22,6 +22,7 @@ U-Boot API documentation rng sandbox serial + status_led sysreset timer unicode diff --git a/doc/api/status_led.rst b/doc/api/status_led.rst new file mode 100644 index 000..44bbea47157 --- /dev/null +++ b/doc/api/status_led.rst @@ -0,0 +1,35 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Status LED +== + +.. kernel-doc:: include/status_led.h + :doc: Overview + +CONFIG_STATUS_LED Description +- + +.. kernel-doc:: include/status_led.h + :doc: CONFIG Description + +Special Status LED Configs +-- +.. kernel-doc:: include/status_led.h + :doc: LED Status Config + +Colour Status LED Configs +- +.. kernel-doc:: include/status_led.h + :doc: LED Colour Config + +Required API + + +.. kernel-doc:: include/status_led.h + :doc: Required API + +Status LED API +-- + +.. kernel-doc:: include/status_led.h + :internal: diff --git a/include/status_led.h b/include/status_led.h index 7de40551621..6893d1d68e0 100644 --- a/include/status_led.h +++ b/include/status_led.h @@ -4,18 +4,102 @@ * Wolfgang Denk, DENX Software Engineering, w...@denx.de. */ -/* - * The purpose of this code is to signal the operational status of a +/** + * DOC: Overview + * + * Status LED is a Low-Level way to handle LEDs to signal state of the + * bootloader, for example boot progress, file transfer fail, activity + * of some sort like tftp transfer, mtd write/erase. + * + * The original usage these API were to signal the operational status of a * target which usually boots over the network; while running in * PCBoot, a status LED is blinking. As soon as a valid BOOTP reply * message
[PATCH v3 8/9] ubi: implement support for LED status activity
Implement support for LED status activity. If the feature is enabled, make the defined ACTIVITY LED to signal ubi write operation. Signed-off-by: Christian Marangi --- cmd/ubi.c | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/cmd/ubi.c b/cmd/ubi.c index 0a6a80bdd10..37c16048b19 100644 --- a/cmd/ubi.c +++ b/cmd/ubi.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -417,7 +418,19 @@ int ubi_volume_begin_write(char *volume, void *buf, size_t size, int ubi_volume_write(char *volume, void *buf, size_t size) { - return ubi_volume_begin_write(volume, buf, size, size); + int ret; + +#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + status_led_activity_start(CONFIG_LED_STATUS_ACTIVITY); +#endif + + ret = ubi_volume_begin_write(volume, buf, size, size); + +#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + status_led_activity_stop(CONFIG_LED_STATUS_ACTIVITY); +#endif + + return ret; } int ubi_volume_read(char *volume, char *buf, size_t size) -- 2.43.0
[PATCH v3 7/9] mtd: implement support for LED status activity
Implement support for LED status activity. If the feature is enabled, make the defined ACTIVITY LED to signal mtd write or erase operations. Signed-off-by: Christian Marangi --- cmd/mtd.c | 19 +++ 1 file changed, 19 insertions(+) diff --git a/cmd/mtd.c b/cmd/mtd.c index 9189f45cabd..a10805d05a5 100644 --- a/cmd/mtd.c +++ b/cmd/mtd.c @@ -11,6 +11,7 @@ #include #include #include +#include #if CONFIG_IS_ENABLED(CMD_MTD_OTP) #include #endif @@ -559,6 +560,11 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc, while (mtd_block_isbad(mtd, off)) off += mtd->erasesize; +#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + if (!read) + status_led_activity_start(CONFIG_LED_STATUS_ACTIVITY); +#endif + /* Loop over the pages to do the actual read/write */ while (remaining) { /* Skip the block if it is bad */ @@ -586,6 +592,11 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc, io_op.oobbuf += io_op.oobretlen; } +#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + if (!read) + status_led_activity_stop(CONFIG_LED_STATUS_ACTIVITY); +#endif + if (!ret && dump) mtd_dump_device_buf(mtd, start_off, buf, len, woob); @@ -653,6 +664,10 @@ static int do_mtd_erase(struct cmd_tbl *cmdtp, int flag, int argc, erase_op.addr = off; erase_op.len = mtd->erasesize; +#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + status_led_activity_start(CONFIG_LED_STATUS_ACTIVITY); +#endif + while (len) { if (!scrub) { ret = mtd_block_isbad(mtd, erase_op.addr); @@ -681,6 +696,10 @@ static int do_mtd_erase(struct cmd_tbl *cmdtp, int flag, int argc, erase_op.addr += mtd->erasesize; } +#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + status_led_activity_stop(CONFIG_LED_STATUS_ACTIVITY); +#endif + if (ret && ret != -EIO) ret = CMD_RET_FAILURE; else -- 2.43.0
[PATCH v3 6/9] tftp: implement support for LED status activity
Implement support for LED status activity. If the feature is enabled, make the defined ACTIVITY LED to signal traffic. Signed-off-by: Christian Marangi --- net/tftp.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/net/tftp.c b/net/tftp.c index 2e335413492..0515dc34d4b 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -19,6 +19,7 @@ #include #include #include "bootp.h" +#include DECLARE_GLOBAL_DATA_PTR; @@ -193,6 +194,9 @@ static void new_transfer(void) #ifdef CONFIG_CMD_TFTPPUT tftp_put_final_block_sent = 0; #endif +#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + status_led_activity_start(CONFIG_LED_STATUS_ACTIVITY); +#endif } #ifdef CONFIG_CMD_TFTPPUT @@ -302,6 +306,9 @@ static void tftp_complete(void) time_start * 1000, "/s"); } puts("\ndone\n"); +#ifdef CONFIG_LED_STATUS_ACTIVITY_ENABLE + status_led_activity_stop(CONFIG_LED_STATUS_ACTIVITY); +#endif if (!tftp_put_active) efi_set_bootdev("Net", "", tftp_filename, map_sysmem(tftp_load_addr, 0), -- 2.43.0
[PATCH v3 5/9] led: status_led: add new activity LED config and functions
Add a new activity LED config and additional functions to implement a simple software blink feature to signal activity of any kind. Usual activity might be a file transfer with TFTP, a flash write... User of this API will call status_led_activity_start/stop() on each activity and LED will be toggled based on the defined FREQ config value. With this enabled, cyclic API are also enabled as this makes use of cyclic API to handle LED blink. Signed-off-by: Christian Marangi --- drivers/led/Kconfig | 16 +++ drivers/misc/status_led.c | 43 +++ include/status_led.h | 2 ++ 3 files changed, 61 insertions(+) diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig index 6c4f02d71f2..869ed78e87f 100644 --- a/drivers/led/Kconfig +++ b/drivers/led/Kconfig @@ -359,6 +359,22 @@ config LED_STATUS_BOOT endif # LED_STATUS_BOOT_ENABLE +config LED_STATUS_ACTIVITY_ENABLE + bool "Enable BOOT LED" + select CYCLIC + help + Enable to turn an LED on when the board is doing some + activity (flash write, file download). + +if LED_STATUS_ACTIVITY_ENABLE + +config LED_STATUS_ACTIVITY + int "LED to light when the board is doing some activity" + help + Valid enabled LED device number. + +endif # LED_STATUS_ACTIVITY_ENABLE + config LED_STATUS_RED_ENABLE bool "Enable red LED" help diff --git a/drivers/misc/status_led.c b/drivers/misc/status_led.c index 882273cc322..6d5207508d8 100644 --- a/drivers/misc/status_led.c +++ b/drivers/misc/status_led.c @@ -5,6 +5,7 @@ */ #include +#include #include /* @@ -23,6 +24,7 @@ typedef struct { int state; int period; int cnt; + struct cyclic_info *cyclic; } led_dev_t; led_dev_t led_dev[] = { @@ -142,3 +144,44 @@ void status_led_toggle(int led) __led_toggle(ld->mask); } + +static void status_led_activity_toggle(void *ctx) +{ + __led_toggle(*(led_id_t *)ctx); +} + +void status_led_activity_start(int led) +{ + led_dev_t *ld; + + ld = status_get_led_dev(led); + if (!ld) + return; + + if (ld->cyclic) { + printf("Cyclic for activity status LED %d already registered. THIS IS AN ERROR.\n", + led); + cyclic_unregister(ld->cyclic); + } + + status_led_set(led, CONFIG_LED_STATUS_BLINKING); + + ld->cyclic = cyclic_register(status_led_activity_toggle, +ld->period * 500, "activity", &ld->mask); + if (!ld->cyclic) + printf("Registering of cyclic function for activity status LED %d failed\n", + led); +} + +void status_led_activity_stop(int led) +{ + led_dev_t *ld; + + ld = status_get_led_dev(led); + if (!ld) + return; + + cyclic_unregister(ld->cyclic); + ld->cyclic = NULL + status_led_set(led, CONFIG_LED_STATUS_OFF); +} diff --git a/include/status_led.h b/include/status_led.h index fe0c84fb4b4..7de40551621 100644 --- a/include/status_led.h +++ b/include/status_led.h @@ -39,6 +39,8 @@ void status_led_init(void); void status_led_tick(unsigned long timestamp); void status_led_set(int led, int state); void status_led_toggle(int led); +void status_led_activity_start(int led); +void status_led_activity_stop(int led); /* MVS v1 **/ #if (defined(CONFIG_MVS) && CONFIG_MVS < 2) -- 2.43.0
[PATCH v3 4/9] led: status_led: add warning when an invalid Status LED ID is used
Add a warning when an invalid Status LED ID is used to make the user aware of bad configurations. Signed-off-by: Christian Marangi --- drivers/misc/status_led.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/misc/status_led.c b/drivers/misc/status_led.c index 93bfb410662..882273cc322 100644 --- a/drivers/misc/status_led.c +++ b/drivers/misc/status_led.c @@ -105,8 +105,10 @@ void status_led_tick(ulong timestamp) static led_dev_t *status_get_led_dev(int led) { - if (led < 0 || led >= MAX_LED_DEV) + if (led < 0 || led >= MAX_LED_DEV) { + printf("Invalid Status LED ID %d.", led) return NULL; + } if (!status_led_init_done) status_led_init(); -- 2.43.0
[PATCH v3 3/9] led: status_led: add function to toggle a status LED
Add function to toggle a status LED by using the status LED ID reference configs. Signed-off-by: Christian Marangi --- drivers/misc/status_led.c | 28 +++- include/status_led.h | 1 + 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/drivers/misc/status_led.c b/drivers/misc/status_led.c index a6e9c03a02e..93bfb410662 100644 --- a/drivers/misc/status_led.c +++ b/drivers/misc/status_led.c @@ -103,17 +103,24 @@ void status_led_tick(ulong timestamp) } } -void status_led_set(int led, int state) +static led_dev_t *status_get_led_dev(int led) { - led_dev_t *ld; - if (led < 0 || led >= MAX_LED_DEV) - return; + return NULL; if (!status_led_init_done) status_led_init(); - ld = &led_dev[led]; + return &led_dev[led]; +} + +void status_led_set(int led, int state) +{ + led_dev_t *ld; + + ld = status_get_led_dev(led); + if (!ld) + return; ld->state = state; if (state == CONFIG_LED_STATUS_BLINKING) { @@ -122,3 +129,14 @@ void status_led_set(int led, int state) } __led_set (ld->mask, state); } + +void status_led_toggle(int led) +{ + led_dev_t *ld; + + ld = status_get_led_dev(led); + if (!ld) + return; + + __led_toggle(ld->mask); +} diff --git a/include/status_led.h b/include/status_led.h index 5ce4522b029..fe0c84fb4b4 100644 --- a/include/status_led.h +++ b/include/status_led.h @@ -38,6 +38,7 @@ void status_led_init(void); void status_led_tick(unsigned long timestamp); void status_led_set(int led, int state); +void status_led_toggle(int led); /* MVS v1 **/ #if (defined(CONFIG_MVS) && CONFIG_MVS < 2) -- 2.43.0
[PATCH v3 2/9] led: status_led: add support for white LED colour
Add support for white LED colour present on many devices. Signed-off-by: Christian Marangi --- cmd/legacy_led.c| 6 ++ common/board_f.c| 2 ++ drivers/led/Kconfig | 14 ++ drivers/misc/gpio_led.c | 12 include/status_led.h| 4 5 files changed, 38 insertions(+) diff --git a/cmd/legacy_led.c b/cmd/legacy_led.c index 5256255f052..40dbc05a651 100644 --- a/cmd/legacy_led.c +++ b/cmd/legacy_led.c @@ -57,6 +57,9 @@ static const led_tbl_t led_commands[] = { #endif #ifdef CONFIG_LED_STATUS_BLUE { "blue", CONFIG_LED_STATUS_BLUE, blue_led_off, blue_led_on, NULL }, +#endif +#ifdef CONFIG_LED_STATUS_WHITE + { "white", CONFIG_LED_STATUS_WHITE, white_led_off, white_led_on, NULL }, #endif { NULL, 0, NULL, NULL, NULL } }; @@ -180,6 +183,9 @@ U_BOOT_CMD( #endif #ifdef CONFIG_LED_STATUS_BLUE "blue|" +#endif +#ifdef CONFIG_LED_STATUS_WHITE + "white|" #endif "all] [on|off|toggle|blink] [blink-freq in ms]", "[led_name] [on|off|toggle|blink] sets or clears led(s)" diff --git a/common/board_f.c b/common/board_f.c index 039d6d712d0..54e2009339e 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -72,6 +72,8 @@ __weak void yellow_led_on(void) {} __weak void yellow_led_off(void) {} __weak void blue_led_on(void) {} __weak void blue_led_off(void) {} +__weak void white_led_on(void) {} +__weak void white_led_off(void) {} /* * Why is gd allocated a register? Prior to reloc it might be better to diff --git a/drivers/led/Kconfig b/drivers/led/Kconfig index 9837960198d..6c4f02d71f2 100644 --- a/drivers/led/Kconfig +++ b/drivers/led/Kconfig @@ -415,6 +415,20 @@ config LED_STATUS_GREEN endif # LED_STATUS_GREEN_ENABLE +config LED_STATUS_WHITE_ENABLE + bool "Enable white LED" + help + Enable white status LED. + +if LED_STATUS_WHITE_ENABLE + +config LED_STATUS_WHITE + int "White LED identification" + help + Valid enabled LED device number (0-5). + +endif # LED_STATUS_WHITE_ENABLE + config LED_STATUS_CMD bool "Enable status LED commands" diff --git a/drivers/misc/gpio_led.c b/drivers/misc/gpio_led.c index 0f3682e1465..de84c206b6b 100644 --- a/drivers/misc/gpio_led.c +++ b/drivers/misc/gpio_led.c @@ -112,3 +112,15 @@ void blue_led_off(void) __led_set(GPIO_BIT(CONFIG_LED_STATUS_BLUE), CONFIG_LED_STATUS_OFF); } #endif + +#ifdef CONFIG_LED_STATUS_WHITE +void white_led_on(void) +{ + __led_set(GPIO_BIT(CONFIG_LED_STATUS_WHITE), CONFIG_LED_STATUS_ON); +} + +void white_led_off(void) +{ + __led_set(GPIO_BIT(CONFIG_LED_STATUS_WHITE), CONFIG_LED_STATUS_OFF); +} +#endif diff --git a/include/status_led.h b/include/status_led.h index 6707ab1d29d..5ce4522b029 100644 --- a/include/status_led.h +++ b/include/status_led.h @@ -87,6 +87,8 @@ void yellow_led_on(void); void yellow_led_off(void); void blue_led_on(void); void blue_led_off(void); +void white_led_on(void); +void white_led_off(void); #else .extern LED_init .extern red_led_on @@ -97,6 +99,8 @@ void blue_led_off(void); .extern green_led_off .extern blue_led_on .extern blue_led_off + .extern white_led_on + .extern white_led_off #endif #endif /* _STATUS_LED_H_ */ -- 2.43.0
[PATCH v3 1/9] misc: gpio_led: fix broken coloured LED status functions
The GPIO LED driver is a backend to provide LED status functions via the GPIO common functions. The coloured LED functions are currently broken and deviates from what is written in README.LED Quoting the README.LED: CONFIG_STATUS_LED_RED is the red LED. It is used to signal errors. This must be a valid LED number (0-5). Other similar color LED's macros are CONFIG_STATUS_LED_GREEN, CONFIG_STATUS_LED_YELLOW and CONFIG_STATUS_LED_BLUE. Hence the value of the config must refer to the ID. Currently this is not the case and the driver expect the GPIO ID to be put in these config. On top of this to actually have these functions, a never define in Kconfig config must be declared to actually compile them. (CONFIG_GPIO_LED_STUBS) To fix this and the GPIO problem, introduce some define that reference the LED_STATUS_BIT config to have the ID->BIT connection (as described in Docs) and drop the never defined config. The gpio_led already provide a wrapper to the functions and should not be enabled if the board provide their custom function hence it's ok to also provide the wrapper for the colour functions. Signed-off-by: Christian Marangi --- drivers/misc/gpio_led.c | 31 +++ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/drivers/misc/gpio_led.c b/drivers/misc/gpio_led.c index 30679f80cf1..0f3682e1465 100644 --- a/drivers/misc/gpio_led.c +++ b/drivers/misc/gpio_led.c @@ -52,56 +52,63 @@ void __led_toggle(led_id_t mask) gpio_set_value(mask, !gpio_get_value(mask)); } -#ifdef CONFIG_GPIO_LED_STUBS - /* 'generic' override of colored LED stubs, to use GPIO functions instead */ +/* We support up to 6 LEDs, LED 0 STATUS BIT doesn't have the number suffix */ +#define GPIO_BIT0 CONFIG_LED_STATUS_BIT +#define GPIO_BIT1 CONFIG_LED_STATUS_BIT1 +#define GPIO_BIT2 CONFIG_LED_STATUS_BIT2 +#define GPIO_BIT3 CONFIG_LED_STATUS_BIT3 +#define GPIO_BIT4 CONFIG_LED_STATUS_BIT4 +#define GPIO_BIT5 CONFIG_LED_STATUS_BIT5 +/* C preprocessor magic way to generate a GPIO_LED reference */ +#define GPIO_BIT(id) ___PASTE(GPIO_BIT, id) + #ifdef CONFIG_LED_STATUS_RED + void red_led_on(void) { - __led_set(CONFIG_LED_STATUS_RED, CONFIG_LED_STATUS_ON); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_RED), CONFIG_LED_STATUS_ON); } void red_led_off(void) { - __led_set(CONFIG_LED_STATUS_RED, CONFIG_LED_STATUS_OFF); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_RED), CONFIG_LED_STATUS_OFF); } #endif #ifdef CONFIG_LED_STATUS_GREEN void green_led_on(void) { - __led_set(CONFIG_LED_STATUS_GREEN, CONFIG_LED_STATUS_ON); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_GREEN), CONFIG_LED_STATUS_ON); } void green_led_off(void) { - __led_set(CONFIG_LED_STATUS_GREEN, CONFIG_LED_STATUS_OFF); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_GREEN), CONFIG_LED_STATUS_OFF); } #endif #ifdef CONFIG_LED_STATUS_YELLOW void yellow_led_on(void) { - __led_set(CONFIG_LED_STATUS_YELLOW, CONFIG_LED_STATUS_ON); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_YELLOW), CONFIG_LED_STATUS_ON); } void yellow_led_off(void) { - __led_set(CONFIG_LED_STATUS_YELLOW, CONFIG_LED_STATUS_OFF); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_YELLOW), CONFIG_LED_STATUS_OFF); } #endif #ifdef CONFIG_LED_STATUS_BLUE void blue_led_on(void) { - __led_set(CONFIG_LED_STATUS_BLUE, CONFIG_LED_STATUS_ON); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_BLUE), CONFIG_LED_STATUS_ON); } void blue_led_off(void) { - __led_set(CONFIG_LED_STATUS_BLUE, CONFIG_LED_STATUS_OFF); + __led_set(GPIO_BIT(CONFIG_LED_STATUS_BLUE), CONFIG_LED_STATUS_OFF); } #endif - -#endif /* CONFIG_GPIO_LED_STUBS */ -- 2.43.0
[PATCH v3 0/9] misc: introduce STATUS LED activity function
This series expand the STATUS LED framework with a new color and a big new feature. One thing that many device need is a way to communicate to the user that the device is actually doing something. This is especially useful for recovery steps where an user (for example) insert an USB drive, keep a button pressed and the device autorecover. There is currently no way to signal the user externally that the bootloader is processing/recoverying aside from setting a LED on. A solid LED on is not enough and won't actually signal any kind of progress. Solution is the good old blinking LED but uboot doesn't suggest (and support) interrupts and almost all the LED are usually GPIO LED that doesn't support HW blink. To fix this and handle the problem of device not supporting HW blink, expand the STATUS LED framework with new API. We introduce a new config LED_STATUS_ACTIVITY, that similar to the RED, GREEN and others, takes the LED ID set in the LED_STATUS config and is used as the global LED for activity operations. We add status_led_activity_start/stop() used to turn the activity LED on and register a cyclic to make it blink. LED will continue to blink until _stop() is called. Function that will start some kind of action will first call _start() and then at the end will call stop(). If (on the broken implementation) a _start() is called before calling a _stop(), the Cyclic is first unregister and is registered again. Support for this is initially added to the tftp, mtd and ubi command. This also contains a big fixup for the gpio_led driver that currently deviates from the Documentation and make the coloured status led feature unusable. Changes v3: - Add log on Status LED error conditions Changes v2: - Add Documentation conversion - Rework to Cyclic and simplify implementation Christian Marangi (9): misc: gpio_led: fix broken coloured LED status functions led: status_led: add support for white LED colour led: status_led: add function to toggle a status LED led: status_led: add warning when an invalid Status LED ID is used led: status_led: add new activity LED config and functions tftp: implement support for LED status activity mtd: implement support for LED status activity ubi: implement support for LED status activity doc: convert README.LED to .rst documentation cmd/legacy_led.c | 6 + cmd/mtd.c | 19 cmd/ubi.c | 15 ++- common/board_f.c | 2 + doc/README.LED| 77 - doc/api/index.rst | 1 + doc/api/status_led.rst| 35 ++ drivers/led/Kconfig | 30 + drivers/misc/gpio_led.c | 41 +-- drivers/misc/status_led.c | 75 - include/status_led.h | 231 +- net/tftp.c| 7 ++ 12 files changed, 440 insertions(+), 99 deletions(-) delete mode 100644 doc/README.LED create mode 100644 doc/api/status_led.rst -- 2.43.0
Re: [RFC PATCH 04/31] lmb: remove local instances of the lmb structure variable
On Tue, Jun 11, 2024 at 04:08:56PM -0600, Simon Glass wrote: > Hi Tom, > > On Tue, 11 Jun 2024 at 15:01, Tom Rini wrote: > > > > On Tue, Jun 11, 2024 at 12:52:22PM -0600, Simon Glass wrote: > > > Hi Sughosh, > > > > > > On Fri, 7 Jun 2024 at 12:53, Sughosh Ganu wrote: > > > > > > > > With the move of the LMB structure to a persistent state, there is no > > > > need to declare the variable locally, and pass it as part of the LMB > > > > API's. Remove all local variable instances and change the API's > > > > correspondingly. > > > > > > > > Signed-off-by: Sughosh Ganu > > > > --- > > > > arch/arc/lib/cache.c | 4 +- > > > > arch/arm/lib/stack.c | 4 +- > > > > arch/arm/mach-apple/board.c | 17 ++- > > > > arch/arm/mach-snapdragon/board.c | 17 ++- > > > > arch/arm/mach-stm32mp/dram_init.c| 7 +- > > > > arch/arm/mach-stm32mp/stm32mp1/cpu.c | 6 +- > > > > arch/m68k/lib/bootm.c| 7 +- > > > > arch/microblaze/lib/bootm.c | 4 +- > > > > arch/mips/lib/bootm.c| 9 +- > > > > arch/nios2/lib/bootm.c | 4 +- > > > > arch/powerpc/cpu/mpc85xx/mp.c| 4 +- > > > > arch/powerpc/include/asm/mp.h| 4 +- > > > > arch/powerpc/lib/bootm.c | 14 +- > > > > arch/riscv/lib/bootm.c | 4 +- > > > > arch/sh/lib/bootm.c | 4 +- > > > > arch/x86/lib/bootm.c | 4 +- > > > > arch/xtensa/lib/bootm.c | 4 +- > > > > board/xilinx/common/board.c | 7 +- > > > > boot/bootm.c | 26 ++-- > > > > boot/bootm_os.c | 5 +- > > > > boot/image-board.c | 32 ++--- > > > > boot/image-fdt.c | 29 ++--- > > > > cmd/bdinfo.c | 6 +- > > > > cmd/booti.c | 2 +- > > > > cmd/bootz.c | 2 +- > > > > cmd/load.c | 7 +- > > > > drivers/iommu/apple_dart.c | 7 +- > > > > drivers/iommu/sandbox_iommu.c| 15 +-- > > > > fs/fs.c | 7 +- > > > > include/image.h | 22 +--- > > > > include/lmb.h| 39 +++--- > > > > lib/lmb.c| 81 ++-- > > > > net/tftp.c | 5 +- > > > > net/wget.c | 5 +- > > > > test/cmd/bdinfo.c| 2 +- > > > > test/lib/lmb.c | 187 +-- > > > > 36 files changed, 270 insertions(+), 333 deletions(-) > > > > > > This isn't necessary...and it will make things harder. You can have a > > > global 'lmb' while still allowing passing a different pointer when > > > needed. > > > > There's only one reservation checking system and list of known > > reservations, keep in mind. > > There is only one driver model, too, but we use a pointer. It makes > tests much easier. > > In fact I see elsewhere in this series that it causes problems with > tests. Best to use a pointer so it is easy to update. Maybe? I worry that will lead to thinking that we still, like today, have many LMB lists, rather than a single LMB list that everyone must use. -- Tom signature.asc Description: PGP signature
Re: [RFC PATCH 15/31] efi_memory: add an event handler to update memory map
On Tue, Jun 11, 2024 at 04:22:25PM -0600, Simon Glass wrote: > Hi Tom, > > On Tue, 11 Jun 2024 at 15:01, Tom Rini wrote: > > > > On Tue, Jun 11, 2024 at 12:52:19PM -0600, Simon Glass wrote: > > > Hi, > > > > > > On Tue, 11 Jun 2024 at 08:36, Tom Rini wrote: > > > > > > > > On Tue, Jun 11, 2024 at 12:17:16PM +0200, Heinrich Schuchardt wrote: > > > > > On 07.06.24 20:52, Sughosh Ganu wrote: > > > > > > There are events that would be used to notify other interested > > > > > > modules > > > > > > of any changes in available and occupied memory. This would happen > > > > > > when a module allocates or reserves memory, or frees up memory. > > > > > > These > > > > > > changes in memory map should be notified to other interested modules > > > > > > so that the allocated memory does not get overwritten. Add an event > > > > > > handler in the EFI memory module to update the EFI memory map > > > > > > accordingly when such changes happen. As a consequence, any > > > > > > subsequent > > > > > > memory request would honour the updated memory map and only > > > > > > available > > > > > > memory would be allocated from. > > > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > > --- > > > > > > lib/efi_loader/efi_memory.c | 70 > > > > > > ++--- > > > > > > 1 file changed, 58 insertions(+), 12 deletions(-) > > > > > > > > > > > > diff --git a/lib/efi_loader/efi_memory.c > > > > > > b/lib/efi_loader/efi_memory.c > > > > > > index 435e580fb3..93244161b0 100644 > > > > > > --- a/lib/efi_loader/efi_memory.c > > > > > > +++ b/lib/efi_loader/efi_memory.c > > > > > > @@ -73,6 +73,10 @@ struct efi_pool_allocation { > > > > > > #if CONFIG_IS_ENABLED(MEM_MAP_UPDATE_NOTIFY) > > > > > > extern bool is_addr_in_ram(uintptr_t addr); > > > > > > > > > > > > +static efi_status_t __efi_add_memory_map_pg(u64 start, u64 pages, > > > > > > + int memory_type, > > > > > > + bool overlap_only_ram); > > > > > > + > > > > > > static void efi_map_update_notify(u64 addr, u64 size, u8 op) > > > > > > { > > > > > > struct event_efi_mem_map_update efi_map = {0}; > > > > > > @@ -84,6 +88,34 @@ static void efi_map_update_notify(u64 addr, u64 > > > > > > size, u8 op) > > > > > > if (is_addr_in_ram((uintptr_t)addr)) > > > > > > event_notify(EVT_EFI_MEM_MAP_UPDATE, &efi_map, > > > > > > sizeof(efi_map)); > > > > > > } > > > > > > + > > > > > > +static int lmb_mem_map_update_sync(void *ctx, struct event *event) > > > > > > +{ > > > > > > + u8 op; > > > > > > + u64 addr; > > > > > > + u64 pages; > > > > > > + efi_status_t status; > > > > > > + struct event_lmb_map_update *lmb_map = &event->data.lmb_map; > > > > > > + > > > > > > + addr = (uintptr_t)map_sysmem(lmb_map->base, 0); > > > > > > + pages = efi_size_in_pages(lmb_map->size + (addr & > > > > > > EFI_PAGE_MASK)); > > > > > > + op = lmb_map->op; > > > > > > + addr &= ~EFI_PAGE_MASK; > > > > > > + > > > > > > + if (op != MAP_OP_RESERVE && op != MAP_OP_FREE) { > > > > > > + log_debug("Invalid map update op received (%d)\n", op); > > > > > > + return -1; > > > > > > + } > > > > > > + > > > > > > + status = __efi_add_memory_map_pg(addr, pages, > > > > > > +op == MAP_OP_FREE ? > > > > > > +EFI_CONVENTIONAL_MEMORY : > > > > > > > > > > This is dangerous. LMB might turn memory that is marked as > > > > > EfiReservedMemory which the OS must respect into EfiBootServicesData > > > > > which the OS may discard. > > > > > > > > > > E.g. initr_lmb() is being called after efi_memory_init(). > > > > > > > > > > Getting all cases of synchronization properly tested seems very hard > > > > > to > > > > > me. Everything would be much easier if we had only a single memory > > > > > management system. > > > > > > > > Yes, Sughosh is working on the single memory reservation system for > > > > everyone to use. This pairs with the single memory allocation system > > > > (malloc) that we have. Parts of the code base that aren't keeping these > > > > systems up to date / obeying their results need to be corrected. > > > > > > The EFI allocations don't happen until boot time...so why do we need > > > to do this now? We can instead have an EFI function to scan LMB and > > > add to its memory map. > > > > We're talking about reservations, not allocations. So yes, when someone > > is making their reservation, they need to make it. I don't understand > > your question. > > As I understand it, this is used to tell EFI about a memory reservation. This patch, or this series? This series isn't about EFI. This patch is, yes. > But the EFI code can scan the LMB reservations just before booting and > update its tables. I don't see a need to keep them in sync before the > boot actually happens. But that wouldn't work. If something needs to reserve a region it nee
Re: [u-boot-test-hooks PATCH 4/4] qemu-loongarch64: New board
On Tue, 11 Jun 2024 at 15:01, Jiaxun Yang wrote: > > Signed-off-by: Jiaxun Yang > --- > bin/travis-ci/conf.qemu-loongarch64_na | 12 > py/travis-ci/u_boot_boardenv_qemu_loongarch64_na.py | 11 +++ > 2 files changed, 23 insertions(+) > create mode 100644 bin/travis-ci/conf.qemu-loongarch64_na > create mode 100644 py/travis-ci/u_boot_boardenv_qemu_loongarch64_na.py > Reviewed-by: Simon Glass > diff --git a/bin/travis-ci/conf.qemu-loongarch64_na > b/bin/travis-ci/conf.qemu-loongarch64_na > new file mode 100644 > index ..e8860bb40326 > --- /dev/null > +++ b/bin/travis-ci/conf.qemu-loongarch64_na > @@ -0,0 +1,12 @@ > +# SPDX-License-Identifier: MIT > +# > +# Copyright (c) 2024 Jiaxun Yang > +# > + > +console_impl=qemu > +qemu_machine="virt" > +qemu_binary="qemu-system-loongarch64" > +qemu_extra_args="-m 1G -nographic -netdev > user,id=net0,tftp=${UBOOT_TRAVIS_BUILD_DIR} -device > virtio-net-pci,netdev=net0 -device virtio-rng-pci" > +qemu_kernel_args="-bios ${U_BOOT_BUILD_DIR}/u-boot.bin" > +reset_impl=none > +flash_impl=none > diff --git a/py/travis-ci/u_boot_boardenv_qemu_loongarch64_na.py > b/py/travis-ci/u_boot_boardenv_qemu_loongarch64_na.py > new file mode 100644 > index ..8a9f747f0457 > --- /dev/null > +++ b/py/travis-ci/u_boot_boardenv_qemu_loongarch64_na.py > @@ -0,0 +1,11 @@ > +import os > +import travis_tftp > + > +env__net_uses_pci = True > +env__net_dhcp_server = True > +env__net_tftp_readable_file = travis_tftp.file2env('u-boot') > +env__efi_loader_helloworld_file = > travis_tftp.file2env('lib/efi_loader/helloworld.efi') > +env__efi_loader_grub_file = travis_tftp.file2env('grub_loongarch64.efi') > +env__efi_fit_tftp_file = { > +"dn" : os.environ['UBOOT_TRAVIS_BUILD_DIR'], > +} > -- > 2.43.0 >
Re: [u-boot-test-hooks PATCH 3/4] qemu-xtensa-dc233c: New board
On Tue, 11 Jun 2024 at 15:01, Jiaxun Yang wrote: > > Signed-off-by: Jiaxun Yang > --- > bin/travis-ci/conf.qemu-xtensa-dc233c_na | 12 > .../u_boot_boardenv_qemu_xtensa_dc233c_na.py | 6 ++ > 2 files changed, 18 insertions(+) > create mode 100644 bin/travis-ci/conf.qemu-xtensa-dc233c_na > create mode 100644 py/travis-ci/u_boot_boardenv_qemu_xtensa_dc233c_na.py > Reviewed-by: Simon Glass It is nice to always add a commit message. > diff --git a/bin/travis-ci/conf.qemu-xtensa-dc233c_na > b/bin/travis-ci/conf.qemu-xtensa-dc233c_na > new file mode 100644 > index ..fc3b5880e5c1 > --- /dev/null > +++ b/bin/travis-ci/conf.qemu-xtensa-dc233c_na > @@ -0,0 +1,12 @@ > +# SPDX-License-Identifier: MIT > +# > +# Copyright (c) 2024 Jiaxun Yang > +# > + > +console_impl=qemu > +qemu_machine="virt" > +qemu_binary="qemu-system-xtensa" > +qemu_extra_args="-cpu dc233c -nographic -netdev > user,id=net0,tftp=${UBOOT_TRAVIS_BUILD_DIR} -device > virtio-net-pci,netdev=net0 -device virtio-rng-pci -semihosting" > +qemu_kernel_args="-kernel ${U_BOOT_BUILD_DIR}/u-boot.elf" > +reset_impl=none > +flash_impl=none > diff --git a/py/travis-ci/u_boot_boardenv_qemu_xtensa_dc233c_na.py > b/py/travis-ci/u_boot_boardenv_qemu_xtensa_dc233c_na.py > new file mode 100644 > index ..8fdb24b284c7 > --- /dev/null > +++ b/py/travis-ci/u_boot_boardenv_qemu_xtensa_dc233c_na.py > @@ -0,0 +1,6 @@ > +import os > +import travis_tftp > + > +env__net_uses_pci = True > +env__net_dhcp_server = True > +env__net_tftp_readable_file = travis_tftp.file2env('u-boot') > -- > 2.43.0 >
Re: [u-boot-test-hooks PATCH 2/4] qemu-arm64be: New board
On Tue, 11 Jun 2024 at 15:01, Jiaxun Yang wrote: > > Signed-off-by: Jiaxun Yang > --- > bin/travis-ci/conf.qemu_arm64be_na | 13 + > py/travis-ci/u_boot_boardenv_qemu_arm64be_na.py | 10 ++ > 2 files changed, 23 insertions(+) > create mode 100644 bin/travis-ci/conf.qemu_arm64be_na > create mode 100644 py/travis-ci/u_boot_boardenv_qemu_arm64be_na.py > Reviewed-by: Simon Glass > diff --git a/bin/travis-ci/conf.qemu_arm64be_na > b/bin/travis-ci/conf.qemu_arm64be_na > new file mode 100644 > index ..3929636932e6 > --- /dev/null > +++ b/bin/travis-ci/conf.qemu_arm64be_na > @@ -0,0 +1,13 @@ > +# SPDX-License-Identifier: MIT > +# > +# Copyright (c) 2024 Jiaxun Yang > +# > + > +console_impl=qemu > +qemu_machine="virt" > +qemu_helper_script="swtpm" > +qemu_binary="qemu-system-aarch64" > +qemu_extra_args="-cpu cortex-a57 -nographic -netdev > user,id=net0,tftp=${UBOOT_TRAVIS_BUILD_DIR} -device e1000,netdev=net0 -device > virtio-rng-pci -semihosting -chardev > socket,id=chrtpm,path=/tmp/tpm/swtpm-sock -tpmdev > emulator,id=tpm0,chardev=chrtpm -device tpm-tis-device,tpmdev=tpm0" > +qemu_kernel_args="-bios ${U_BOOT_BUILD_DIR}/u-boot.bin" > +reset_impl=none > +flash_impl=none > diff --git a/py/travis-ci/u_boot_boardenv_qemu_arm64be_na.py > b/py/travis-ci/u_boot_boardenv_qemu_arm64be_na.py > new file mode 100644 > index ..5746b37dbcf5 > --- /dev/null > +++ b/py/travis-ci/u_boot_boardenv_qemu_arm64be_na.py > @@ -0,0 +1,10 @@ > +import os > +import travis_tftp > + > +env__net_uses_pci = True > +env__net_dhcp_server = True > +env__net_tftp_readable_file = travis_tftp.file2env('u-boot.bin', 0x4040) > +env__efi_fit_tftp_file = { > +'addr' : 0x4040, > +"dn" : os.environ['UBOOT_TRAVIS_BUILD_DIR'], > +} > -- > 2.43.0 >
Re: [u-boot-test-hooks PATCH 1/4] qemu-vexpress*: Pass -audio none
On Tue, 11 Jun 2024 at 15:01, Jiaxun Yang wrote: > > Those boards have build in sound card which cause > problems on CI runner. > > Pass -audio none to disable sound card backends. > > Signed-off-by: Jiaxun Yang > --- > bin/travis-ci/conf.vexpress_ca15_tc2_qemu | 2 +- > bin/travis-ci/conf.vexpress_ca9x4_qemu| 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > Reviewed-by: Simon Glass > diff --git a/bin/travis-ci/conf.vexpress_ca15_tc2_qemu > b/bin/travis-ci/conf.vexpress_ca15_tc2_qemu > index 83d703269377..55e40038b379 100644 > --- a/bin/travis-ci/conf.vexpress_ca15_tc2_qemu > +++ b/bin/travis-ci/conf.vexpress_ca15_tc2_qemu > @@ -21,7 +21,7 @@ > console_impl=qemu > qemu_machine="vexpress-a15" > qemu_binary="qemu-system-arm" > -qemu_extra_args="-nographic -m 1G -net user,tftp=${UBOOT_TRAVIS_BUILD_DIR} > -net nic" > +qemu_extra_args="-nographic -m 1G -audio none -net > user,tftp=${UBOOT_TRAVIS_BUILD_DIR} -net nic" > qemu_kernel_args="-kernel ${U_BOOT_BUILD_DIR}/u-boot" > reset_impl=none > flash_impl=none > diff --git a/bin/travis-ci/conf.vexpress_ca9x4_qemu > b/bin/travis-ci/conf.vexpress_ca9x4_qemu > index d07be2b12984..b5ed72934c3a 100644 > --- a/bin/travis-ci/conf.vexpress_ca9x4_qemu > +++ b/bin/travis-ci/conf.vexpress_ca9x4_qemu > @@ -21,7 +21,7 @@ > console_impl=qemu > qemu_machine="vexpress-a9" > qemu_binary="qemu-system-arm" > -qemu_extra_args="-nographic -m 1G -net user,tftp=${UBOOT_TRAVIS_BUILD_DIR} > -net nic" > +qemu_extra_args="-nographic -m 1G -audio none -net > user,tftp=${UBOOT_TRAVIS_BUILD_DIR} -net nic" > qemu_kernel_args="-kernel ${U_BOOT_BUILD_DIR}/u-boot" > reset_impl=none > flash_impl=none > -- > 2.43.0 >
Re: [RFC PATCH 15/31] efi_memory: add an event handler to update memory map
Hi Tom, On Tue, 11 Jun 2024 at 15:01, Tom Rini wrote: > > On Tue, Jun 11, 2024 at 12:52:19PM -0600, Simon Glass wrote: > > Hi, > > > > On Tue, 11 Jun 2024 at 08:36, Tom Rini wrote: > > > > > > On Tue, Jun 11, 2024 at 12:17:16PM +0200, Heinrich Schuchardt wrote: > > > > On 07.06.24 20:52, Sughosh Ganu wrote: > > > > > There are events that would be used to notify other interested modules > > > > > of any changes in available and occupied memory. This would happen > > > > > when a module allocates or reserves memory, or frees up memory. These > > > > > changes in memory map should be notified to other interested modules > > > > > so that the allocated memory does not get overwritten. Add an event > > > > > handler in the EFI memory module to update the EFI memory map > > > > > accordingly when such changes happen. As a consequence, any subsequent > > > > > memory request would honour the updated memory map and only available > > > > > memory would be allocated from. > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > --- > > > > > lib/efi_loader/efi_memory.c | 70 > > > > > ++--- > > > > > 1 file changed, 58 insertions(+), 12 deletions(-) > > > > > > > > > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > > > > > index 435e580fb3..93244161b0 100644 > > > > > --- a/lib/efi_loader/efi_memory.c > > > > > +++ b/lib/efi_loader/efi_memory.c > > > > > @@ -73,6 +73,10 @@ struct efi_pool_allocation { > > > > > #if CONFIG_IS_ENABLED(MEM_MAP_UPDATE_NOTIFY) > > > > > extern bool is_addr_in_ram(uintptr_t addr); > > > > > > > > > > +static efi_status_t __efi_add_memory_map_pg(u64 start, u64 pages, > > > > > + int memory_type, > > > > > + bool overlap_only_ram); > > > > > + > > > > > static void efi_map_update_notify(u64 addr, u64 size, u8 op) > > > > > { > > > > > struct event_efi_mem_map_update efi_map = {0}; > > > > > @@ -84,6 +88,34 @@ static void efi_map_update_notify(u64 addr, u64 > > > > > size, u8 op) > > > > > if (is_addr_in_ram((uintptr_t)addr)) > > > > > event_notify(EVT_EFI_MEM_MAP_UPDATE, &efi_map, > > > > > sizeof(efi_map)); > > > > > } > > > > > + > > > > > +static int lmb_mem_map_update_sync(void *ctx, struct event *event) > > > > > +{ > > > > > + u8 op; > > > > > + u64 addr; > > > > > + u64 pages; > > > > > + efi_status_t status; > > > > > + struct event_lmb_map_update *lmb_map = &event->data.lmb_map; > > > > > + > > > > > + addr = (uintptr_t)map_sysmem(lmb_map->base, 0); > > > > > + pages = efi_size_in_pages(lmb_map->size + (addr & EFI_PAGE_MASK)); > > > > > + op = lmb_map->op; > > > > > + addr &= ~EFI_PAGE_MASK; > > > > > + > > > > > + if (op != MAP_OP_RESERVE && op != MAP_OP_FREE) { > > > > > + log_debug("Invalid map update op received (%d)\n", op); > > > > > + return -1; > > > > > + } > > > > > + > > > > > + status = __efi_add_memory_map_pg(addr, pages, > > > > > +op == MAP_OP_FREE ? > > > > > +EFI_CONVENTIONAL_MEMORY : > > > > > > > > This is dangerous. LMB might turn memory that is marked as > > > > EfiReservedMemory which the OS must respect into EfiBootServicesData > > > > which the OS may discard. > > > > > > > > E.g. initr_lmb() is being called after efi_memory_init(). > > > > > > > > Getting all cases of synchronization properly tested seems very hard to > > > > me. Everything would be much easier if we had only a single memory > > > > management system. > > > > > > Yes, Sughosh is working on the single memory reservation system for > > > everyone to use. This pairs with the single memory allocation system > > > (malloc) that we have. Parts of the code base that aren't keeping these > > > systems up to date / obeying their results need to be corrected. > > > > The EFI allocations don't happen until boot time...so why do we need > > to do this now? We can instead have an EFI function to scan LMB and > > add to its memory map. > > We're talking about reservations, not allocations. So yes, when someone > is making their reservation, they need to make it. I don't understand > your question. As I understand it, this is used to tell EFI about a memory reservation. But the EFI code can scan the LMB reservations just before booting and update its tables. I don't see a need to keep them in sync before the boot actually happens. Regards, Simon
Re: [RFC PATCH 04/31] lmb: remove local instances of the lmb structure variable
Hi Tom, On Tue, 11 Jun 2024 at 15:01, Tom Rini wrote: > > On Tue, Jun 11, 2024 at 12:52:22PM -0600, Simon Glass wrote: > > Hi Sughosh, > > > > On Fri, 7 Jun 2024 at 12:53, Sughosh Ganu wrote: > > > > > > With the move of the LMB structure to a persistent state, there is no > > > need to declare the variable locally, and pass it as part of the LMB > > > API's. Remove all local variable instances and change the API's > > > correspondingly. > > > > > > Signed-off-by: Sughosh Ganu > > > --- > > > arch/arc/lib/cache.c | 4 +- > > > arch/arm/lib/stack.c | 4 +- > > > arch/arm/mach-apple/board.c | 17 ++- > > > arch/arm/mach-snapdragon/board.c | 17 ++- > > > arch/arm/mach-stm32mp/dram_init.c| 7 +- > > > arch/arm/mach-stm32mp/stm32mp1/cpu.c | 6 +- > > > arch/m68k/lib/bootm.c| 7 +- > > > arch/microblaze/lib/bootm.c | 4 +- > > > arch/mips/lib/bootm.c| 9 +- > > > arch/nios2/lib/bootm.c | 4 +- > > > arch/powerpc/cpu/mpc85xx/mp.c| 4 +- > > > arch/powerpc/include/asm/mp.h| 4 +- > > > arch/powerpc/lib/bootm.c | 14 +- > > > arch/riscv/lib/bootm.c | 4 +- > > > arch/sh/lib/bootm.c | 4 +- > > > arch/x86/lib/bootm.c | 4 +- > > > arch/xtensa/lib/bootm.c | 4 +- > > > board/xilinx/common/board.c | 7 +- > > > boot/bootm.c | 26 ++-- > > > boot/bootm_os.c | 5 +- > > > boot/image-board.c | 32 ++--- > > > boot/image-fdt.c | 29 ++--- > > > cmd/bdinfo.c | 6 +- > > > cmd/booti.c | 2 +- > > > cmd/bootz.c | 2 +- > > > cmd/load.c | 7 +- > > > drivers/iommu/apple_dart.c | 7 +- > > > drivers/iommu/sandbox_iommu.c| 15 +-- > > > fs/fs.c | 7 +- > > > include/image.h | 22 +--- > > > include/lmb.h| 39 +++--- > > > lib/lmb.c| 81 ++-- > > > net/tftp.c | 5 +- > > > net/wget.c | 5 +- > > > test/cmd/bdinfo.c| 2 +- > > > test/lib/lmb.c | 187 +-- > > > 36 files changed, 270 insertions(+), 333 deletions(-) > > > > This isn't necessary...and it will make things harder. You can have a > > global 'lmb' while still allowing passing a different pointer when > > needed. > > There's only one reservation checking system and list of known > reservations, keep in mind. There is only one driver model, too, but we use a pointer. It makes tests much easier. In fact I see elsewhere in this series that it causes problems with tests. Best to use a pointer so it is easy to update. Regards, Simon
Re: [PATCH] arm: dts: mvebu: Update DTS for Thecus N2350 board
Hi Dragan, On Sun, Jun 9, 2024 at 6:07 PM Dragan Simic wrote: > > Hello Tony, > > Please see a few comments below. > > On 2024-06-10 02:34, Tony Dinh wrote: > > - Change the spi-max-frequency to 5000 (50 Mhz). According to the > > data sheet[1], the MX25L3205D max frequency is 86 Mhz. Using 50 Mhz in > > the DTS to ensure u-boot is consistent with what Linux kernel expected. > > - Update GPIO fan to conform to the latest DT binding. > > There's no need for the bullet points, plain prose will do fine instead. I like bullet points :) All my patch descriptions have bullet points when there are 2 or more different items. > > > [1] > > MX25L3205D-MX25L1605D-MX25L6405D-Macronix-MX25L3205DM2I-12G-datasheet.pdf > > Unless you can provide a real, working URL for the datasheet, this > reference > should actually be deleted. It was my oversight. That should have been this URL: https://www.macronix.com/Lists/Datasheet/Attachments/8575/MX25L3205D,%203V,%2032Mb,%20v1.5.pdf I'll wait for Stefan to review the patch and let him decide whether I should send in a V2 patch or he will modify the commit description. Thanks for the comments! All the best, Tony > > > Signed-off-by: Tony Dinh > > --- > > > > arch/arm/dts/armada-385-thecus-n2350.dts | 10 +- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/arch/arm/dts/armada-385-thecus-n2350.dts > > b/arch/arm/dts/armada-385-thecus-n2350.dts > > index 253cf01130..fdaa444e51 100644 > > --- a/arch/arm/dts/armada-385-thecus-n2350.dts > > +++ b/arch/arm/dts/armada-385-thecus-n2350.dts > > @@ -2,7 +2,7 @@ > > /* > > * Device Tree file for Thecus N2350 board > > * > > - * Copyright (C) 2018-2023 Tony Dinh > > + * Copyright (C) 2018-2024 Tony Dinh > > * Copyright (C) 2018 Manuel Jung > > */ > > > > @@ -143,9 +143,9 @@ > > fan { > > compatible = "gpio-fan"; > > gpios = <&gpio1 16 GPIO_ACTIVE_HIGH>; > > - gpio-fan,speed-map = <0 0 > > - 600 1 > > - 3000 2 >; > > + gpio-fan,speed-map = <0 0>, > > + <600 1>, > > + <3000 2 >; > > pinctrl-0 = <&pmx_fan>; > > pinctrl-names = "default"; > > }; > > @@ -415,7 +415,7 @@ > > compatible = "jedec,spi-nor"; > > reg = <0>; > > > > - spi-max-frequency = <10800>; > > + spi-max-frequency = <5000>; > > spi-cpha; > > > > partition@0 {
Re: [PATCH v2 1/1] drivers: bootcount: Add support for FAT filesystem
On Tue, Jun 11, 2024 at 06:29:34PM +0200, Quentin Schulz wrote: > Hi Vasileios, > > On 6/11/24 6:20 PM, Vasileios Amoiridis wrote: > > On Tue, Jun 11, 2024 at 10:06:59AM -0600, Tom Rini wrote: > > > On Tue, Jun 11, 2024 at 05:41:19PM +0200, Quentin Schulz wrote: > > > > Hi Vasileios, > > > > > > > > On 6/11/24 5:27 PM, Vasileios Amoiridis wrote: > > > > > On Tue, Jun 11, 2024 at 11:33:12AM +0200, Quentin Schulz wrote: > > > > > > Hi Vasileios, > > > > > > > > > > > > On 6/10/24 8:51 PM, Vasileios Amoiridis wrote: > > > > > > > Add support to save boot count variable in a file in a FAT > > > > > > > filesystem. > > > > > > > > > > > > > > Signed-off-by: Vasileios Amoiridis > > > > > > > --- > > > > > > > doc/README.bootcount | 12 ++--- > > > > > > > drivers/bootcount/Kconfig | 53 > > > > > > > +-- > > > > > > > drivers/bootcount/Makefile| 2 +- > > > > > > > .../{bootcount_ext.c => bootcount_fs.c} | 12 ++--- > > > > > > > 4 files changed, 50 insertions(+), 29 deletions(-) > > > > > > > rename drivers/bootcount/{bootcount_ext.c => bootcount_fs.c} > > > > > > > (81%) > > > > > > > > > > > > > > diff --git a/doc/README.bootcount b/doc/README.bootcount > > > > > > > index f6c5f82f..0f4ffb68 100644 > > > > > > > --- a/doc/README.bootcount > > > > > > > +++ b/doc/README.bootcount > > > > > > > @@ -23,15 +23,15 @@ It is the responsibility of some application > > > > > > > code > > > > > > (typically a Linux > > > > > > > application) to reset the variable "bootcount" to 0 when the > > > > > > > system > > > > > > booted > > > > > > > successfully, thus allowing for more boot cycles. > > > > > > > > > > > > > > -CONFIG_BOOTCOUNT_EXT > > > > > > > +CONFIG_BOOTCOUNT_FS > > > > > > > > > > > > > > > > > > > > > -This adds support for maintaining boot count in a file on an EXT > > > > > > filesystem. > > > > > > > -The file to use is defined by: > > > > > > > +This adds support for maintaining boot count in a file on a > > > > > > > filesystem. > > > > > > > +Supported filesystems are FAT and EXT. The file to use is > > > > > > > defined by: > > > > > > > > > > > > > > -CONFIG_SYS_BOOTCOUNT_EXT_INTERFACE > > > > > > > -CONFIG_SYS_BOOTCOUNT_EXT_DEVPART > > > > > > > -CONFIG_SYS_BOOTCOUNT_EXT_NAME > > > > > > > +CONFIG_SYS_BOOTCOUNT_FS_INTERFACE > > > > > > > +CONFIG_SYS_BOOTCOUNT_FS_DEVPART > > > > > > > +CONFIG_SYS_BOOTCOUNT_FS_NAME > > > > > > > > > > > > > > The format of the file is: > > > > > > > > > > > > > > diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig > > > > > > > index 3c56253b..d3679eb5 100644 > > > > > > > --- a/drivers/bootcount/Kconfig > > > > > > > +++ b/drivers/bootcount/Kconfig > > > > > > > @@ -25,10 +25,9 @@ config BOOTCOUNT_GENERIC > > > > > > > Set to the address where the bootcount and > > > > > > > bootcount magic > > > > > > > will be stored. > > > > > > > > > > > > > > -config BOOTCOUNT_EXT > > > > > > > - bool "Boot counter on EXT filesystem" > > > > > > > - depends on FS_EXT4 > > > > > > > - select EXT4_WRITE > > > > > > > +config BOOTCOUNT_FS > > > > > > > + bool "Boot counter on a filesystem" > > > > > > > + depends on FS_EXT4 || FS_FAT > > > > > > Do we really need this 'depends on' here? Especially if we have a > > > > > > choice > > > > > > below... > > > > > > > > > > > Well, probably this is redundant indeed. > > > > > > > > > > > > help > > > > > > > Add support for maintaining boot count in a file on > > > > > > > an EXT > > > > > > The help text is still mentioning EXT here. > > > > > > > > > > > > > > > > Ahh, I missed that. > > > > > > > > > > > I would recommend removing it, or listing the supported filesystems > > > > > > at the > > > > > > moment. While I assume you tested with FAT, I assume that with > > > > > > FS_ANY, any > > > > > > FS should be supported? > > > > > > > > > > > > > > > > Well, I tested it with both FAT and EXT4 and it works. AFAIU, due to > > > > > the > > > > > implementation of the filesystem handling code in U-Boot, if the fs > > > > > supports > > > > > a write a function, then it should work. But I cannot test for other > > > > > filesystems apart from FAT and EXT4 so I think it's better to limit > > > > > the > > > > > option to these two. > > > > > > > > > > > > > I guess we can let people figure things out themselves and add new > > > > options > > > > for when they have tested them, no strong opinion here. > > > > > > > > > > > filesystem. > > > > > > > @@ -184,26 +183,48 @@ config SYS_BOOTCOUNT_SINGLEWORD > > > > > > > This option enables packing boot count magic value > > > > > > > and boot count > > > > > > > into single word (32 bits). > > > > > > > > > > > > > > -config SYS_BOOTCOUNT_EXT_INTERFACE > > > > > > > - string "Interface on which to find boot c
[PATCH NFC 20/20] CI: Dockerfile: Replace some URL with mirror sites
Somehow when I'm trying to build dockerimage on 11 June 2024 multiple upstream sites are down. Arm site is returning 500 error, nasm.us domain expired, ftpmirror.gnu.org is down worldwide. I belive those problems are not permanent so made this change NFC. NOT FOR COMMIT! Signed-off-by: Jiaxun Yang --- tools/docker/Dockerfile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index 3f1bbfce669e..67d10f11f43f 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -205,7 +205,7 @@ RUN git clone https://gitlab.com/qemu-project/qemu.git /tmp/qemu && \ rm -rf /tmp/qemu # Build fiptool -RUN git clone https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git /tmp/tf-a && \ +RUN git clone https://github.com/ARM-software/arm-trusted-firmware.git /tmp/tf-a && \ cd /tmp/tf-a/ && \ git checkout lts-v2.10.4 && \ cd tools/fiptool && \ @@ -260,6 +260,8 @@ RUN mkdir /tmp/trace && \ # Build coreboot RUN wget -O - https://coreboot.org/releases/coreboot-24.05.tar.xz | tar -C /tmp -xJ && \ cd /tmp/coreboot-24.05 && \ +sed -i 's,https://ftpmirror.gnu.org,https://ftp.gnu.org/gnu,g' ./util/crossgcc/buildgcc && \ +sed -i 's,NASM_BASE_URL=.*,NASM_BASE_URL="https://distfiles.macports.org/nasm",g' ./util/crossgcc/buildgcc && \ make crossgcc-i386 CPUS=$(nproc) && \ make -C payloads/coreinfo olddefconfig && \ make -C payloads/coreinfo && \ -- 2.43.0
[PATCH NFC 19/20] Use Jiaxun's CI Image
Use Jiaxun's CI Image for demonstration. NOT FOR COMMIT! Signed-off-by: Jiaxun Yang --- .azure-pipelines.yml| 4 ++-- .gitlab-ci.yml | 4 ++-- tools/docker/Dockerfile | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 2506814725e1..494c7edaf549 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -2,7 +2,7 @@ variables: windows_vm: windows-2019 ubuntu_vm: ubuntu-22.04 macos_vm: macOS-12 - ci_runner_image: trini/u-boot-gitlab-ci-runner:jammy-20240227-14Mar2024 + ci_runner_image: ghcr.io/flygoat/u-boot-ci-docker:main # Add '-u 0' options for Azure pipelines, otherwise we get "permission # denied" error when it tries to "useradd -m -u 1001 vsts_azpcontainer", # since our $(ci_runner_image) user is not root. @@ -203,7 +203,7 @@ stages: # the below corresponds to .gitlab-ci.yml "before_script" cd \${WORK_DIR} git config --global --add safe.directory \${WORK_DIR} - git clone --depth=1 https://source.denx.de/u-boot/u-boot-test-hooks /tmp/uboot-test-hooks + git clone --depth=1 https://github.com/FlyGoat/u-boot-test-hooks.git /tmp/uboot-test-hooks ln -s travis-ci /tmp/uboot-test-hooks/bin/\`hostname\` ln -s travis-ci /tmp/uboot-test-hooks/py/\`hostname\` grub-mkimage --prefix=\"\" -o ~/grub_x86.efi -O i386-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index efb84c3b119f..5b61b5780a32 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -10,7 +10,7 @@ default: # Grab our configured image. The source for this is found # in the u-boot tree at tools/docker/Dockerfile -image: ${MIRROR_DOCKER}/trini/u-boot-gitlab-ci-runner:jammy-20240227-14Mar2024 +image: ghcr.io/flygoat/u-boot-ci-docker:main # We run some tests in different order, to catch some failures quicker. stages: @@ -26,7 +26,7 @@ stages: before_script: # Clone uboot-test-hooks - git config --global --add safe.directory "${CI_PROJECT_DIR}" -- git clone --depth=1 https://source.denx.de/u-boot/u-boot-test-hooks /tmp/uboot-test-hooks +- git clone --depth=1 https://github.com/FlyGoat/u-boot-test-hooks.git /tmp/uboot-test-hooks - ln -s travis-ci /tmp/uboot-test-hooks/bin/`hostname` - ln -s travis-ci /tmp/uboot-test-hooks/py/`hostname` - grub-mkimage --prefix="" -o ~/grub_x86.efi -O i386-efi normal echo lsefimmap lsefi lsefisystab efinet tftp minicmd diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index 26b0e36fec05..3f1bbfce669e 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -276,9 +276,9 @@ USER uboot:uboot # Populate the cache for pip to use. Get these via wget as the # COPY / ADD directives don't work as we need them to. -RUN wget -O /tmp/pytest-requirements.txt https://source.denx.de/u-boot/u-boot/-/raw/master/test/py/requirements.txt -RUN wget -O /tmp/sphinx-requirements.txt https://source.denx.de/u-boot/u-boot/-/raw/master/doc/sphinx/requirements.txt -RUN wget -O /tmp/buildman-requirements.txt https://source.denx.de/u-boot/u-boot/-/raw/master/tools/buildman/requirements.txt +RUN wget -O /tmp/pytest-requirements.txt https://gitlab.com/FlyGoat/u-boot/-/raw/b4/docker-image/test/py/requirements.txt +RUN wget -O /tmp/sphinx-requirements.txt https://gitlab.com/FlyGoat/u-boot/-/raw/b4/docker-image/doc/sphinx/requirements.txt +RUN wget -O /tmp/buildman-requirements.txt https://gitlab.com/FlyGoat/u-boot/-/raw/b4/docker-image/tools/buildman/requirements.txt RUN virtualenv -p /usr/bin/python3 /tmp/venv && \ . /tmp/venv/bin/activate && \ pip install -r /tmp/pytest-requirements.txt \ -- 2.43.0
[PATCH 18/20] doc: ci: Document how to run pipeline on gitlab.com
It's now possible to run CI job on gitlab.com. Document the process. Signed-off-by: Jiaxun Yang --- doc/develop/ci_testing.rst | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/doc/develop/ci_testing.rst b/doc/develop/ci_testing.rst index ffaacedc3d88..b616d1c807d3 100644 --- a/doc/develop/ci_testing.rst +++ b/doc/develop/ci_testing.rst @@ -46,9 +46,10 @@ resources the project has available. For Custodians, it is a matter of enabling the pipeline feature in your project repository following the standard GitLab documentation. For non-custodians, the pipeline itself is part of the tree and should be able to be used on any GitLab instance, with whatever -runners you are able to provide. While it is intended to be able to run this -pipeline on the free public instances provided at https://gitlab.com/ a problem -with our squashfs tests currently prevents this. +runners you are able to provide. To run this pipeline on the free public +instances provided at https://gitlab.com/ you will need to fork the repository, +enable the CI/CD feature [1]_ for the repository, rise pipeline timeout [2]_ to +at least 2 hours and then push your changes to the repository. To push to Gitlab without triggering a pipeline use: @@ -74,3 +75,8 @@ developing features. In that case, it can be useful as part of your own testing cycle to edit these pipelines in separate local commits to pair them down to just the jobs you're interested in. These changes must be removed prior to submission. + +References +-- +.. [1] https://docs.gitlab.com/ee/ci/quick_start/ +.. [2] https://docs.gitlab.com/ee/ci/pipelines/settings.html#set-a-limit-for-how-long-jobs-can-run -- 2.43.0
[PATCH 17/20] CI: Dockerfile: Add LoongArch64 support
Install LoongArch64 toolchains, build LoongArch64 QEMU, build LoongArch64 GRUB. Signed-off-by: Jiaxun Yang --- tools/docker/Dockerfile | 21 +++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index e3b2c2d57555..26b0e36fec05 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -19,6 +19,7 @@ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-arc-linux.tar.xz | tar -C /opt -xJ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-arm-linux-gnueabi.tar.xz | tar -C /opt -xJ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-i386-linux.tar.xz | tar -C /opt -xJ +RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-loongarch64-linux.tar.xz | tar -C /opt -xJ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-m68k-linux.tar.xz | tar -C /opt -xJ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-mips-linux.tar.xz | tar -C /opt -xJ RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-microblaze-linux.tar.xz | tar -C /opt -xJ @@ -131,7 +132,7 @@ RUN git config --global user.name "U-Boot CI" && \ # Make kernels readable for libguestfs tools to work correctly RUN chmod +r /boot/vmlinu* -# Build GRUB UEFI targets for ARM & RISC-V, 32-bit and 64-bit +# Build GRUB UEFI targets for ARM & LoongArch64 & RISC-V, 32-bit and 64-bit RUN git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \ cd /tmp/grub && \ git checkout grub-2.12 && \ @@ -165,6 +166,20 @@ RUN git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \ search search_fs_file search_fs_uuid search_label serial sleep test \ true && \ make clean && \ + ./configure --target=loongarch64 --with-platform=efi \ + CC=gcc \ + TARGET_CC=/opt/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-gcc \ + TARGET_OBJCOPY=/opt/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-objcopy \ + TARGET_STRIP=/opt/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-strip \ + TARGET_NM=/opt/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-nm \ + TARGET_RANLIB=/opt/gcc-13.2.0-nolibc/loongarch64-linux/bin/loongarch64-linux-ranlib && \ + make && \ + ./grub-mkimage -O loongarch64-efi -o /opt/grub/grubloongarch64.efi --prefix= -d \ + grub-core cat chain configfile echo efinet ext2 fat halt help linux \ + lsefisystab loadenv lvm minicmd normal part_msdos part_gpt reboot \ + search search_fs_file search_fs_uuid search_label serial sleep test \ + true && \ + make clean && \ ./configure --target=riscv64 --with-platform=efi \ CC=gcc \ TARGET_CC=/opt/gcc-13.2.0-nolibc/riscv64-linux/bin/riscv64-linux-gcc \ @@ -183,7 +198,9 @@ RUN git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \ RUN git clone https://gitlab.com/qemu-project/qemu.git /tmp/qemu && \ cd /tmp/qemu && \ git checkout v9.0.0 && \ - ./configure --prefix=/opt/qemu --target-list="aarch64-softmmu,arm-softmmu,i386-softmmu,m68k-softmmu,mips-softmmu,mips64-softmmu,mips64el-softmmu,mipsel-softmmu,ppc-softmmu,riscv32-softmmu,riscv64-softmmu,sh4-softmmu,x86_64-softmmu,xtensa-softmmu" && \ + git cherry-pick 16b1ecee52effa3346fb34dcc351e4645e4ab53e && \ + git cherry-pick 085446905000d6b80978815594a7cd34d54ff46b && \ + ./configure --prefix=/opt/qemu --target-list="aarch64-softmmu,arm-softmmu,i386-softmmu,loongarch64-softmmu,m68k-softmmu,mips-softmmu,mips64-softmmu,mips64el-softmmu,mipsel-softmmu,ppc-softmmu,riscv32-softmmu,riscv64-softmmu,sh4-softmmu,x86_64-softmmu,xtensa-softmmu" && \ make -j$(nproc) all install && \ rm -rf /tmp/qemu -- 2.43.0
[PATCH 16/20] CI: Dockerfile: Bump various software version
Bump base OS to Ubuntu noble, grub to 2.12, QEMU to 9.0 (drop cherry-pick for m68k as it's already in 9.0) and other tools to latest version. Remove unsupported python2. Install some required python packages for buildman/binman. Signed-off-by: Jiaxun Yang --- tools/docker/Dockerfile | 35 +-- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index 5824d371f259..e3b2c2d57555 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -2,7 +2,7 @@ # This Dockerfile is used to build an image containing basic stuff to be used # to build U-Boot and run our test suites. -FROM ubuntu:jammy-20240227 +FROM ubuntu:noble MAINTAINER Tom Rini LABEL Description=" This image is for building U-Boot inside a container" @@ -12,7 +12,7 @@ ENV DEBIAN_FRONTEND=noninteractive # Add LLVM repository RUN apt-get update && apt-get install -y gnupg2 wget xz-utils && rm -rf /var/lib/apt/lists/* RUN wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add - -RUN echo deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-17 main | tee /etc/apt/sources.list.d/llvm.list +RUN echo deb http://apt.llvm.org/noble/ llvm-toolchain-noble-17 main | tee /etc/apt/sources.list.d/llvm.list # Manually install the kernel.org "Crosstool" based toolchains for gcc-13.2.0 RUN wget -O - https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/13.2.0/x86_64-gcc-13.2.0-nolibc-aarch64-linux.tar.xz | tar -C /opt -xJ @@ -55,6 +55,7 @@ RUN apt-get update && apt-get install -y \ gawk \ gdisk \ git \ + gnat \ gnu-efi \ gnutls-dev \ graphviz \ @@ -95,13 +96,14 @@ RUN apt-get update && apt-get install -y \ parted \ pkg-config \ python-is-python3 \ - python2.7 \ python3 \ python3-dev \ + python3-jsonschema \ python3-pip \ python3-pyelftools \ python3-sphinx \ python3-virtualenv \ + python3-yaml \ rpm2cpio \ sbsigntool \ socat \ @@ -118,6 +120,7 @@ RUN apt-get update && apt-get install -y \ vboot-utils \ xilinx-bootgen \ xxd \ + yamllint \ zip \ && rm -rf /var/lib/apt/lists/* @@ -131,9 +134,7 @@ RUN chmod +r /boot/vmlinu* # Build GRUB UEFI targets for ARM & RISC-V, 32-bit and 64-bit RUN git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \ cd /tmp/grub && \ - git checkout grub-2.06 && \ - git cherry-pick 049efdd72eb7baa7b2bf8884391ee7fe650da5a0 && \ - git cherry-pick 403d6540cd608b2706cfa0cb4713f7e4b490ff45 && \ + git checkout grub-2.12 && \ ./bootstrap && \ mkdir -p /opt/grub && \ ./configure --target=aarch64 --with-platform=efi \ @@ -181,10 +182,7 @@ RUN git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \ RUN git clone https://gitlab.com/qemu-project/qemu.git /tmp/qemu && \ cd /tmp/qemu && \ - git checkout v8.2.0 && \ - git format-patch 0c7ffc977195~..0c7ffc977195 && \ - git am 0001-hw-net-cadence_gem-Fix-MDIO_OP_xxx-values.patch && \ - git cherry-pick d3c79c3974 && \ + git checkout v9.0.0 && \ ./configure --prefix=/opt/qemu --target-list="aarch64-softmmu,arm-softmmu,i386-softmmu,m68k-softmmu,mips-softmmu,mips64-softmmu,mips64el-softmmu,mipsel-softmmu,ppc-softmmu,riscv32-softmmu,riscv64-softmmu,sh4-softmmu,x86_64-softmmu,xtensa-softmmu" && \ make -j$(nproc) all install && \ rm -rf /tmp/qemu @@ -192,7 +190,7 @@ RUN git clone https://gitlab.com/qemu-project/qemu.git /tmp/qemu && \ # Build fiptool RUN git clone https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git /tmp/tf-a && \ cd /tmp/tf-a/ && \ - git checkout v2.10.0 && \ + git checkout lts-v2.10.4 && \ cd tools/fiptool && \ make && \ mkdir -p /usr/local/bin && \ @@ -200,12 +198,12 @@ RUN git clone https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git /tmp/t rm -rf /tmp/tf-a # Build genimage (required by some targets to generate disk images) -RUN wget -O - https://github.com/pengutronix/genimage/releases/download/v14/genimage-14.tar.xz | tar -C /tmp -xJ && \ - cd /tmp/genimage-14 && \ +RUN wget -O - https://github.com/pengutronix/genimage/releases/download/v17/genimage-17.tar.xz | tar -C /tmp -xJ && \ + cd /tmp/genimage-17 && \ ./configure && \ make -j$(nproc) && \ make install && \ - rm -rf /tmp/genimage-14 + rm -rf /tmp/genimage-17 # Build libtpms RUN git clone https://github.com/stefanberger/libtpms /tmp/libtpms && \ @@ -236,22 +234,23 @@ RUN mkdir /tmp/trace && \ cd /tmp/trace/libtracefs && \ make -j$(nproc) && \ sudo make install && \ -git clone https://github.com/rostedt/trace-cmd.git /tmp/trace/trace-cmd && \ +git clone https://git.kernel.org/pub/scm/utils/tra
[PATCH 15/20] CI: Dockerfile: Set global git name & email config
Set global git name & email config so we don't have to setup it for every project. Signed-off-by: Jiaxun Yang --- tools/docker/Dockerfile | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile index cda87354566d..5824d371f259 100644 --- a/tools/docker/Dockerfile +++ b/tools/docker/Dockerfile @@ -121,6 +121,10 @@ RUN apt-get update && apt-get install -y \ zip \ && rm -rf /var/lib/apt/lists/* +# Setup Git +RUN git config --global user.name "U-Boot CI" && \ + git config --global user.email u-b...@denx.de + # Make kernels readable for libguestfs tools to work correctly RUN chmod +r /boot/vmlinu* @@ -128,8 +132,6 @@ RUN chmod +r /boot/vmlinu* RUN git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \ cd /tmp/grub && \ git checkout grub-2.06 && \ - git config --global user.name "GitLab CI Runner" && \ - git config --global user.email tr...@konsulko.com && \ git cherry-pick 049efdd72eb7baa7b2bf8884391ee7fe650da5a0 && \ git cherry-pick 403d6540cd608b2706cfa0cb4713f7e4b490ff45 && \ ./bootstrap && \ @@ -180,9 +182,6 @@ RUN git clone git://git.savannah.gnu.org/grub.git /tmp/grub && \ RUN git clone https://gitlab.com/qemu-project/qemu.git /tmp/qemu && \ cd /tmp/qemu && \ git checkout v8.2.0 && \ - # config user.name and user.email to make 'git am' happy - git config user.name u-boot && \ - git config user.email u-b...@denx.de && \ git format-patch 0c7ffc977195~..0c7ffc977195 && \ git am 0001-hw-net-cadence_gem-Fix-MDIO_OP_xxx-values.patch && \ git cherry-pick d3c79c3974 && \ -- 2.43.0
[PATCH 14/20] CI: GitLab: Split build_world tasks
Current build_world task runs for too long on public gitlab runner. Split the job as what we've done to azure pipeline. Signed-off-by: Jiaxun Yang --- .gitlab-ci.yml | 103 + 1 file changed, 59 insertions(+), 44 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4c17abea468a..efb84c3b119f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -91,56 +91,71 @@ stages: - "*.css" expire_in: 1 week -.world_build: +.world_build_template: &world_build_dfn stage: world build rules: - when: always - -build all 32bit ARM platforms: - extends: .world_build - script: -- ret=0; - git config --global --add safe.directory "${CI_PROJECT_DIR}"; - ./tools/buildman/buildman -o /tmp -PEWM arm -x aarch64 || ret=$?; - if [[ $ret -ne 0 ]]; then -./tools/buildman/buildman -o /tmp -seP; -exit $ret; - fi; - -build all 64bit ARM platforms: - extends: .world_build - script: -- virtualenv -p /usr/bin/python3 /tmp/venv -- . /tmp/venv/bin/activate -- ret=0; - git config --global --add safe.directory "${CI_PROJECT_DIR}"; - ./tools/buildman/buildman -o /tmp -PEWM aarch64 || ret=$?; - if [[ $ret -ne 0 ]]; then -./tools/buildman/buildman -o /tmp -seP; -exit $ret; - fi; - -build all PowerPC platforms: - extends: .world_build script: - ret=0; git config --global --add safe.directory "${CI_PROJECT_DIR}"; - ./tools/buildman/buildman -o /tmp -P -E -W powerpc || ret=$?; - if [[ $ret -ne 0 ]]; then -./tools/buildman/buildman -o /tmp -seP; -exit $ret; - fi; - -build all other platforms: - extends: .world_build - script: -- ret=0; - git config --global --add safe.directory "${CI_PROJECT_DIR}"; - ./tools/buildman/buildman -o /tmp -PEWM -x arm,powerpc || ret=$?; - if [[ $ret -ne 0 ]]; then -./tools/buildman/buildman -o /tmp -seP; -exit $ret; - fi; + if [[ "${BUILDMAN}" != "" ]]; then + ret=0; + tools/buildman/buildman -o /tmp -PEWM ${BUILDMAN} ${OVERRIDE} || ret=$?; + if [[ $ret -ne 0 ]]; then + tools/buildman/buildman -o /tmp -seP ${BUILDMAN}; + exit $ret; + fi; + fi + +am33xx_at91_kirkwood_mvebu_omap: + variables: +BUILDMAN: "am33xx at91_kirkwood mvebu omap -x siemens" + <<: *world_build_dfn + +amlogic_bcm_boundary_engicam_siemens_technexion_oradex: + variables: +BUILDMAN: "amlogic bcm boundary engicam siemens technexion toradex -x mips" + <<: *world_build_dfn + +arm_nxp_minus_imx: + variables: +BUILDMAN: "freescale -x powerpc,m68k,imx,mx" + <<: *world_build_dfn + +imx: + variables: +BUILDMAN: "mx imx -x boundary,engicam,technexion,toradex" + <<: *world_build_dfn + +rk: + variables: +BUILDMAN: "rk" + <<: *world_build_dfn + +sunxi: + variables: +BUILDMAN: "sunxi" + <<: *world_build_dfn + +powerpc: + variables: +BUILDMAN: "powerpc" + <<: *world_build_dfn + +arm_catch_all: + variables: +BUILDMAN: "arm -x aarch64,am33xx,at91,bcm,ls1,kirkwood,mvebu,omap,rk,siemens,mx,sunxi,technexion,toradex" + <<: *world_build_dfn + +aarch64_catch_all: + variables: +BUILDMAN: "aarch64 -x amlogic,bcm,engicam,imx,ls1,ls2,lx216,mvebu,rk,siemens,sunxi,toradex" + <<: *world_build_dfn + +everything_but_arm_and_powerpc: + variables: +BUILDMAN: "-x arm,powerpc" + <<: *world_build_dfn .testsuites: stage: testsuites -- 2.43.0
[PATCH 13/20] CI: Ensure pip install is always performed in venv
Since Ubuntu focal it's nolonger permitted to perform global pip install. Ensure that pip install is always performed in venv. For buildman alone, all dependencies are already in docker so there is no need to perform pip install. Signed-off-by: Jiaxun Yang --- .azure-pipelines.yml | 16 ++-- .gitlab-ci.yml | 13 - 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 37b569b13ab0..2506814725e1 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -150,6 +150,8 @@ stages: - script: | git config --global --add safe.directory $(work_dir) export USER=azure + virtualenv -p /usr/bin/python3 /tmp/venv + . /tmp/venv/bin/activate pip install -r test/py/requirements.txt pip install -r tools/buildman/requirements.txt pip install asteval pylint==3.2.3 pyopenssl @@ -183,7 +185,10 @@ stages: image: $(ci_runner_image) options: $(container_option) steps: - - script: make pip + - script: | + virtualenv -p /usr/bin/python3 /tmp/venv + . /tmp/venv/bin/activate + make pip - job: create_test_py_wrapper_script displayName: 'Create and stage a wrapper for test.py runs' @@ -217,7 +222,11 @@ stages: if [ -n "\${BUILD_ENV}" ]; then export \${BUILD_ENV}; fi + virtualenv -p /usr/bin/python3 /tmp/venv + . /tmp/venv/bin/activate pip install -r tools/buildman/requirements.txt + pip install -r test/py/requirements.txt + pip install pytest-azurepipelines tools/buildman/buildman -o \${UBOOT_TRAVIS_BUILD_DIR} -w -E -W -e --board \${TEST_PY_BD} \${OVERRIDE} cp ~/grub_x86.efi \${UBOOT_TRAVIS_BUILD_DIR}/ cp ~/grub_x64.efi \${UBOOT_TRAVIS_BUILD_DIR}/ @@ -241,10 +250,6 @@ stages: /opt/coreboot/cbfstool \${UBOOT_TRAVIS_BUILD_DIR}/coreboot.rom remove -n fallback/payload; /opt/coreboot/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; fi - virtualenv -p /usr/bin/python3 /tmp/venv - . /tmp/venv/bin/activate - pip install -r test/py/requirements.txt - pip install pytest-azurepipelines export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:\${PATH} export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci # "\${var:+"-k \$var"}" expands to "" if \$var is empty, "-k \$var" if not @@ -504,7 +509,6 @@ stages: # make environment variables available as tests are running inside a container export BUILDMAN="${BUILDMAN}" git config --global --add safe.directory ${WORK_DIR} - pip install -r tools/buildman/requirements.txt EOF cat << "EOF" >> build.sh if [[ "${BUILDMAN}" != "" ]]; then diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 18c4c430c63d..4c17abea468a 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -50,6 +50,10 @@ stages: - if [ -n "${BUILD_ENV}" ]; then export ${BUILD_ENV}; fi +- virtualenv -p /usr/bin/python3 /tmp/venv +- . /tmp/venv/bin/activate +- pip install -r tools/buildman/requirements.txt +- pip install -r test/py/requirements.txt - tools/buildman/buildman -o ${UBOOT_TRAVIS_BUILD_DIR} -w -E -W -e --board ${TEST_PY_BD} ${OVERRIDE} - cp ~/grub_x86.efi $UBOOT_TRAVIS_BUILD_DIR/ @@ -74,9 +78,6 @@ stages: /opt/coreboot/cbfstool ${UBOOT_TRAVIS_BUILD_DIR}/coreboot.rom remove -n fallback/payload; /opt/coreboot/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; fi -- virtualenv -p /usr/bin/python3 /tmp/venv -- . /tmp/venv/bin/activate -- pip install -r test/py/requirements.txt # "${var:+"-k $var"}" expands to "" if $var is empty, "-k $var" if not - export PATH=/opt/qemu/bin:/tmp/uboot-test-hooks/bin:${PATH}; export PYTHONPATH=/tmp/uboot-test-hooks/py/travis-ci; @@ -100,7 +101,6 @@ build all 32bit ARM platforms: script: - ret=0; git config --global --add safe.directory "${CI_PROJECT_DIR}"; - pip install -r tools/buildman/requirements.txt; ./tools/buildman/buildman -o /tmp -PEWM arm -x aarch64 || ret=$?; if [[ $ret -ne 0 ]]; then ./tools/buildman/buildman -o /tmp -seP; @@ -114,7 +114,6 @@ build all 64bit ARM platforms: - . /tmp/venv/bin/activate - ret=0; git config --global --add safe.directory "${CI_PROJECT_DIR}"; - pip install -r tools/buildman/requirements.txt; ./tools/buildman/buildman -o /tmp -PEWM aarch64 || ret=$?; if [[ $ret -ne 0 ]]; then ./tools/buildman/buildman -o /tmp -seP; @@ -212,6
[PATCH 12/20] cyclic: Rise default CYCLIC_MAX_CPU_TIME_US to 5000
The default value CYCLIC_MAX_CPU_TIME_US was 1000, which is a little bit too low for slower hardware and sandbox. On my MIPS Boston FPGA board with interaptiv CPU, wdt_cyclic can easily take 3200 us to run. On azure pipeline sandbox_clang, wdt_cyclic some times goes beyond 1300 us. Raise default value to 5000, which is the value already taken by octeon_nic32. This is still sufficent to maintain system responsiveness. Signed-off-by: Jiaxun Yang --- common/Kconfig | 2 +- configs/octeon_nic23_defconfig | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/common/Kconfig b/common/Kconfig index 5e3070e92539..4bb9f08977aa 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -628,7 +628,7 @@ if CYCLIC config CYCLIC_MAX_CPU_TIME_US int "Sets the max allowed time for a cyclic function in us" - default 1000 + default 5000 help The max allowed time for a cyclic function in us. If a functions takes longer than this duration this function will get unregistered diff --git a/configs/octeon_nic23_defconfig b/configs/octeon_nic23_defconfig index f7c35536a021..5a8db5a0876b 100644 --- a/configs/octeon_nic23_defconfig +++ b/configs/octeon_nic23_defconfig @@ -25,7 +25,6 @@ CONFIG_SYS_PBSIZE=276 CONFIG_SYS_CONSOLE_ENV_OVERWRITE=y # CONFIG_SYS_DEVICE_NULLDEV is not set CONFIG_CYCLIC=y -CONFIG_CYCLIC_MAX_CPU_TIME_US=5000 CONFIG_ARCH_MISC_INIT=y CONFIG_BOARD_EARLY_INIT_F=y CONFIG_BOARD_LATE_INIT=y -- 2.43.0
[PATCH 11/20] lib/charset & efi: Fix possible unaligned accesses
As per armv7 arch spec, for A-profile CPU if translation is disabled, then the default memory type is Device(-nGnRnE) instead of Normal, which requires that alignment be enforced. This means in some cases we can't perform unaligned access even after allow_unaligned is called. We do have many platforms that didn't have translation enabled in U-Boot, and QEMU started to enforce this since 9.0. Fix by using unaligned access helper for UTF-16 memory read/write to ensure we don't do any unaligned access in U-Boot. Signed-off-by: Jiaxun Yang --- lib/charset.c| 21 - lib/efi_loader/efi_device_path.c | 11 ++- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/lib/charset.c b/lib/charset.c index 182c92a50c48..af5f3ad16d9b 100644 --- a/lib/charset.c +++ b/lib/charset.c @@ -11,6 +11,7 @@ #include #include #include +#include /** * codepage_437 - Unicode to codepage 437 translation table @@ -215,7 +216,7 @@ s32 utf16_get(const u16 **src) return -1; if (!**src) return 0; - code = **src; + code = get_unaligned_le16(*src); ++*src; if (code >= 0xDC00 && code <= 0xDFFF) return -1; @@ -242,12 +243,12 @@ int utf16_put(s32 code, u16 **dst) if ((code >= 0xD800 && code <= 0xDFFF) || code >= 0x11) return -1; if (code < 0x1) { - **dst = code; + put_unaligned_le16(code, *dst); } else { code -= 0x1; - **dst = code >> 10 | 0xD800; + put_unaligned_le16(code >> 10 | 0xD800, *dst); ++*dst; - **dst = (code & 0x3ff) | 0xDC00; + put_unaligned_le16((code & 0x3ff) | 0xDC00, *dst); } ++*dst; return 0; @@ -392,7 +393,7 @@ int __efi_runtime u16_strncmp(const u16 *s1, const u16 *s2, size_t n) int ret = 0; for (; n; --n, ++s1, ++s2) { - ret = *s1 - *s2; + ret = get_unaligned_le16(s1) - get_unaligned_le16(s2); if (ret || !*s1) break; } @@ -403,7 +404,7 @@ int __efi_runtime u16_strncmp(const u16 *s1, const u16 *s2, size_t n) size_t __efi_runtime u16_strnlen(const u16 *in, size_t count) { size_t i; - for (i = 0; count-- && in[i]; i++); + for (i = 0; count-- && get_unaligned_le16(in + i); i++); return i; } @@ -417,8 +418,10 @@ u16 *u16_strcpy(u16 *dest, const u16 *src) u16 *tmp = dest; for (;; dest++, src++) { - *dest = *src; - if (!*src) + u16 code = get_unaligned_le16(src); + + put_unaligned_le16(code, dest); + if (!code) break; } @@ -463,7 +466,7 @@ uint8_t *utf16_to_utf8(uint8_t *dest, const uint16_t *src, size_t size) uint32_t code_high = 0; while (size--) { - uint32_t code = *src++; + uint32_t code = get_unaligned_le16(src++); if (code_high) { if (code >= 0xDC00 && code <= 0xDFFF) { diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index aec224d84662..481f9effdb6d 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -18,7 +18,7 @@ #include #include #include -#include +#include #include /* U16_MAX */ /* template END node: */ @@ -867,13 +867,6 @@ static void path_to_uefi(void *uefi, const char *src) { u16 *pos = uefi; - /* -* efi_set_bootdev() calls this routine indirectly before the UEFI -* subsystem is initialized. So we cannot assume unaligned access to be -* enabled. -*/ - allow_unaligned(); - while (*src) { s32 code = utf8_get(&src); @@ -883,7 +876,7 @@ static void path_to_uefi(void *uefi, const char *src) code = '\\'; utf16_put(code, &pos); } - *pos = 0; + put_unaligned_le16(0, pos); } /** -- 2.43.0
[PATCH 10/20] tests/test_event_dump: Relax match rule for output
event_dump.py relies on addr2line to obtain source location information, however newer addr2line is unable to determine line numbers for some functions. With addr2line from binutils 2.34 we got: Event typeId Source location -- -- EVT_FT_FIXUP bootmeth_vbe_ft_fixup :? EVT_FT_FIXUP bootmeth_vbe_simple_ft_fixup:? EVT_LAST_STAGE_INIT install_smbios_table:? EVT_MISC_INIT_F sandbox_early_getopt_check arch/sandbox/cpu/start.c:61 EVT_TEST h_adder_simple :? Which will fail the test. Relax the source location regex to .*:.*, this is sufficent to show that addr2line is being called and returned a possible line number. Signed-off-by: Jiaxun Yang --- test/py/tests/test_event_dump.py | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/py/tests/test_event_dump.py b/test/py/tests/test_event_dump.py index e282c67335cd..e87825abcd1a 100644 --- a/test/py/tests/test_event_dump.py +++ b/test/py/tests/test_event_dump.py @@ -16,9 +16,9 @@ def test_event_dump(u_boot_console): out = util.run_and_log(cons, ['scripts/event_dump.py', sandbox]) expect = '''.*Event typeId Source location -- -- -EVT_FT_FIXUP bootmeth_vbe_ft_fixup .*boot/vbe_request.c:.* -EVT_FT_FIXUP bootmeth_vbe_simple_ft_fixup.*boot/vbe_simple_os.c:.* -EVT_LAST_STAGE_INIT install_smbios_table .*lib/efi_loader/efi_smbios.c:.* -EVT_MISC_INIT_F sandbox_early_getopt_check .*arch/sandbox/cpu/start.c:.* -EVT_TEST h_adder_simple .*test/common/event.c:''' +EVT_FT_FIXUP bootmeth_vbe_ft_fixup .*:.* +EVT_FT_FIXUP bootmeth_vbe_simple_ft_fixup.*:.* +EVT_LAST_STAGE_INIT install_smbios_table.*:.* +EVT_MISC_INIT_F sandbox_early_getopt_check .*:.* +EVT_TEST h_adder_simple .*:''' assert re.match(expect, out, re.MULTILINE) is not None -- 2.43.0
[PATCH 09/20] binman: Workaround lz4 cli padding in test cases
Newer lz4 util is not happy with any padding at end of file, it would abort with error message like: Stream followed by undecodable data at position 43. Workaround by skipping testCompUtilPadding test case and manually strip padding in testCompressSectionSize test case. Signed-off-by: Jiaxun Yang --- tools/binman/ftest.py | 7 +-- tools/binman/test/184_compress_section_size.dts | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 99fc606bd855..1107084bc058 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -4518,6 +4518,8 @@ class TestFunctional(unittest.TestCase): dtb.Scan() props = self._GetPropTree(dtb, ['offset', 'image-pos', 'size', 'uncomp-size']) +data = data[:0x30] +data = data.rstrip(b'\xff') orig = self._decompress(data) self.assertEqual(COMPRESS_DATA + U_BOOT_DATA, orig) expected = { @@ -6118,8 +6120,9 @@ fdt fdtmapExtract the devicetree blob from the fdtmap def testCompUtilPadding(self): """Test padding of compression algorithms""" -# Skip zstd because it doesn't support padding -for bintool in [v for k,v in self.comp_bintools.items() if k != 'zstd']: +# Skip zstd and lz4 because they doesn't support padding +for bintool in [v for k,v in self.comp_bintools.items() +if not k in ['zstd', 'lz4']]: self._CheckBintool(bintool) data = bintool.compress(COMPRESS_DATA) self.assertNotEqual(COMPRESS_DATA, data) diff --git a/tools/binman/test/184_compress_section_size.dts b/tools/binman/test/184_compress_section_size.dts index 95ed30add1aa..1c1dbd5f580f 100644 --- a/tools/binman/test/184_compress_section_size.dts +++ b/tools/binman/test/184_compress_section_size.dts @@ -6,6 +6,7 @@ section { size = <0x30>; compress = "lz4"; + pad-byte = <0xff>; blob { filename = "compress"; }; -- 2.43.0
[PATCH 08/20] py: Bump pylint version and clear warnings
Bump pylint to 3.2.3 as old versions are not working with python 3.12. Clear warnings, mostly E0606: (possibly-used-before-assignment). Signed-off-by: Jiaxun Yang --- .azure-pipelines.yml | 2 +- .gitlab-ci.yml| 2 +- doc/develop/python_cq.rst | 4 ++-- test/py/tests/test_ums.py | 1 + test/py/tests/test_usb.py | 1 + tools/binman/etype/fdtmap.py | 1 + tools/binman/etype/fit.py | 1 + tools/binman/etype/image_header.py| 1 + tools/binman/etype/pre_load.py| 2 ++ tools/binman/etype/ti_board_config.py | 1 + tools/binman/etype/x509_cert.py | 1 + tools/binman/ftest.py | 1 + tools/binman/state.py | 1 + tools/buildman/builder.py | 2 ++ tools/microcode-tool.py | 1 + tools/patman/test_checkpatch.py | 2 ++ tools/qconfig.py | 1 + 17 files changed, 21 insertions(+), 4 deletions(-) diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 27f69583c655..37b569b13ab0 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -152,7 +152,7 @@ stages: export USER=azure pip install -r test/py/requirements.txt pip install -r tools/buildman/requirements.txt - pip install asteval pylint==2.12.2 pyopenssl + pip install asteval pylint==3.2.3 pyopenssl export PATH=${PATH}:~/.local/bin echo "[MASTER]" >> .pylintrc echo "load-plugins=pylint.extensions.docparams" >> .pylintrc diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 165f765a8332..18c4c430c63d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -214,7 +214,7 @@ Run pylint: - git config --global --add safe.directory "${CI_PROJECT_DIR}" - pip install -r test/py/requirements.txt - pip install -r tools/buildman/requirements.txt -- pip install asteval pylint==2.12.2 pyopenssl +- pip install asteval pylint==3.2.3 pyopenssl - export PATH=${PATH}:~/.local/bin - echo "[MASTER]" >> .pylintrc - echo "load-plugins=pylint.extensions.docparams" >> .pylintrc diff --git a/doc/develop/python_cq.rst b/doc/develop/python_cq.rst index 1e209ff197d6..c8a75a5b7a7b 100644 --- a/doc/develop/python_cq.rst +++ b/doc/develop/python_cq.rst @@ -23,7 +23,7 @@ regressions in any module. To run this locally you should use this version of pylint:: # pylint --version -pylint 2.11.1 +pylint 3.2.3 astroid 2.8.6 Python 3.8.10 (default, Sep 28 2021, 16:10:42) [GCC 9.3.0] @@ -31,7 +31,7 @@ To run this locally you should use this version of pylint:: You should be able to select and this install other required tools with:: -pip install pylint==2.11.1 +pip install pylint==3.2.3 pip install -r test/py/requirements.txt pip install asteval pyopenssl diff --git a/test/py/tests/test_ums.py b/test/py/tests/test_ums.py index 749b1606235c..55372e42a928 100644 --- a/test/py/tests/test_ums.py +++ b/test/py/tests/test_ums.py @@ -118,6 +118,7 @@ def test_ums(u_boot_console, env__usb_dev_port, env__block_devs): test_f = u_boot_utils.PersistentRandomFile(u_boot_console, 'ums.bin', 1024 * 1024); +mounted_test_fn = None if have_writable_fs_partition: mounted_test_fn = mount_point + '/' + mount_subdir + test_f.fn diff --git a/test/py/tests/test_usb.py b/test/py/tests/test_usb.py index fb3d20f0826b..27105cd1d5e1 100644 --- a/test/py/tests/test_usb.py +++ b/test/py/tests/test_usb.py @@ -564,6 +564,7 @@ def test_usb_load(u_boot_console): part_detect = 1 addr = u_boot_utils.find_ram_base(u_boot_console) +file, size = 0, 0 if fs == 'fat': file, size = test_usb_fatload_fatwrite(u_boot_console) elif fs == 'ext4': diff --git a/tools/binman/etype/fdtmap.py b/tools/binman/etype/fdtmap.py index f1f6217940f2..6b4ca497f871 100644 --- a/tools/binman/etype/fdtmap.py +++ b/tools/binman/etype/fdtmap.py @@ -106,6 +106,7 @@ class Entry_fdtmap(Entry): Returns: FDT map binary data """ +fsw = None def _AddNode(node): """Add a node to the FDT map""" for pname, prop in node.props.items(): diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index 2c14b15b03cd..dfbb6de7b63e 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -808,6 +808,7 @@ class Entry_fit(Entry_section): data_size = fdt_util.GetInt(node, "data-size") # Contents are inside the FIT +offset, size = 0, 0 if data_prop is not None: # GetOffset() returns offset of a fdt_property struct, # which has 3 fdt32_t members before the actual data. diff --git a/tools/binman/etype/image_header.py b/tools/binman/etype/image_header.py index 24011884958
[PATCH 07/20] py: Bump requirements versions
Bump python packages to reasonable versions to fix build error with Python 3.12. Link: https://github.com/pytest-dev/pytest/pull/11094 Link: https://github.com/yaml/pyyaml/issues/736 Link: https://github.com/pypa/setuptools/issues/4002 Signed-off-by: Jiaxun Yang --- test/py/requirements.txt| 6 +++--- tools/buildman/requirements.txt | 8 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/py/requirements.txt b/test/py/requirements.txt index f24a842bfe6f..b7658d5d 100644 --- a/test/py/requirements.txt +++ b/test/py/requirements.txt @@ -6,7 +6,7 @@ pycryptodomex==3.19.1 pyelftools==0.27 pygit2==1.13.3 pyparsing==3.0.7 -pytest==6.2.5 -pytest-xdist==2.5.0 +pytest==7.4.4 +pytest-xdist==3.6.1 requests==2.31.0 -setuptools==65.5.1 +setuptools==70 diff --git a/tools/buildman/requirements.txt b/tools/buildman/requirements.txt index 350da42c0ebf..2b40d8e2499a 100644 --- a/tools/buildman/requirements.txt +++ b/tools/buildman/requirements.txt @@ -1,4 +1,4 @@ -jsonschema==4.17.3 -pyyaml==6.0 -yamllint==1.26.3 -setuptools==65.5.1 +jsonschema==4.22.0 +pyyaml==6.0.1 +yamllint==1.35.1 +setuptools==70 -- 2.43.0
[PATCH 06/20] py: Remove unused entries in requirements.txt
Many packages in requirements.txt are not actually imported/used by any test case, some of them was for python2 compatibility. Remove them to keep our environment clean. Signed-off-by: Jiaxun Yang --- doc/sphinx/requirements.txt | 1 - test/py/requirements.txt| 18 -- 2 files changed, 19 deletions(-) diff --git a/doc/sphinx/requirements.txt b/doc/sphinx/requirements.txt index 426f41e1a028..12447386d29b 100644 --- a/doc/sphinx/requirements.txt +++ b/doc/sphinx/requirements.txt @@ -10,7 +10,6 @@ MarkupSafe==2.1.3 packaging==23.2 Pygments==2.17.2 requests==2.31.0 -six==1.16.0 snowballstemmer==2.2.0 Sphinx==7.2.6 sphinx-prompt==1.8.0 diff --git a/test/py/requirements.txt b/test/py/requirements.txt index 0f67c3c61949..f24a842bfe6f 100644 --- a/test/py/requirements.txt +++ b/test/py/requirements.txt @@ -1,30 +1,12 @@ -atomicwrites==1.4.1 -attrs==19.3.0 concurrencytest==0.1.2 coverage==4.5.4 -extras==1.0.0 filelock==3.0.12 -fixtures==3.0.0 -importlib-metadata==0.23 -linecache2==1.0.0 -more-itertools==7.2.0 packaging==23.2 -pbr==5.4.3 -pluggy==0.13.0 -py==1.11.0 pycryptodomex==3.19.1 pyelftools==0.27 pygit2==1.13.3 pyparsing==3.0.7 pytest==6.2.5 pytest-xdist==2.5.0 -python-mimeparse==1.6.0 -python-subunit==1.3.0 requests==2.31.0 setuptools==65.5.1 -six==1.16.0 -testtools==2.3.0 -traceback2==1.4.0 -unittest2==1.1.0 -wcwidth==0.1.7 -zipp==0.6.0 -- 2.43.0
[PATCH 05/20] doc/sphinx: Remove usage of six
We don't support python2 any more so there is no point to use six here. Signed-off-by: Jiaxun Yang --- doc/sphinx/kfigure.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/sphinx/kfigure.py b/doc/sphinx/kfigure.py index dea7f91ef5ab..9467e8d52ac0 100644 --- a/doc/sphinx/kfigure.py +++ b/doc/sphinx/kfigure.py @@ -58,7 +58,6 @@ from docutils.parsers.rst.directives import images import sphinx from sphinx.util.nodes import clean_astext -from six import iteritems import kernellog @@ -540,7 +539,7 @@ def add_kernel_figure_to_std_domain(app, doctree): docname = app.env.docname labels = std.data["labels"] -for name, explicit in iteritems(doctree.nametypes): +for name, explicit in doctree.nametypes.items(): if not explicit: continue labelid = doctree.nameids[name] -- 2.43.0
[PATCH 04/20] py: Replace usage of configparser.read_fp
configparser.read_fp is deprecated long ago. Replace with relevant API. Signed-off-by: Jiaxun Yang --- tools/buildman/bsettings.py | 2 +- tools/patman/settings.py| 9 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/tools/buildman/bsettings.py b/tools/buildman/bsettings.py index e225ac2ca0f4..7dbc638d5841 100644 --- a/tools/buildman/bsettings.py +++ b/tools/buildman/bsettings.py @@ -29,7 +29,7 @@ def setup(fname=''): settings.read(config_fname) def add_file(data): -settings.readfp(io.StringIO(data)) +settings.read_file(io.StringIO(data), data) def get_items(section): """Get the items from a section of the config. diff --git a/tools/patman/settings.py b/tools/patman/settings.py index 636983e32da8..308626107037 100644 --- a/tools/patman/settings.py +++ b/tools/patman/settings.py @@ -41,7 +41,6 @@ class _ProjectConfigParser(ConfigParser.ConfigParser): - Merge general default settings/aliases with project-specific ones. # Sample config used for tests below... ->>> from io import StringIO >>> sample_config = ''' ... [alias] ... me: Peter P. @@ -59,25 +58,25 @@ class _ProjectConfigParser(ConfigParser.ConfigParser): # Check to make sure that bogus project gets general alias. >>> config = _ProjectConfigParser("zzz") ->>> config.readfp(StringIO(sample_config)) +>>> config.read_string(sample_config) >>> str(config.get("alias", "enemies")) 'Evil ' # Check to make sure that alias gets overridden by project. >>> config = _ProjectConfigParser("sm") ->>> config.readfp(StringIO(sample_config)) +>>> config.read_string(sample_config) >>> str(config.get("alias", "enemies")) 'Green G. ' # Check to make sure that settings get merged with project. >>> config = _ProjectConfigParser("linux") ->>> config.readfp(StringIO(sample_config)) +>>> config.read_string(sample_config) >>> sorted((str(a), str(b)) for (a, b) in config.items("settings")) [('am_hero', 'True'), ('check_patch_use_tree', 'True'), ('process_tags', 'False')] # Check to make sure that settings works with unknown project. >>> config = _ProjectConfigParser("unknown") ->>> config.readfp(StringIO(sample_config)) +>>> config.read_string(sample_config) >>> sorted((str(a), str(b)) for (a, b) in config.items("settings")) [('am_hero', 'True')] """ -- 2.43.0
[PATCH 03/20] py: Replace distutils.core with setuptools
distutils is deprecated long ago and being removed in python 3.12. Signed-off-by: Jiaxun Yang --- tools/binman/setup.py | 2 +- tools/buildman/requirements.txt | 1 + tools/dtoc/setup.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/binman/setup.py b/tools/binman/setup.py index 9a9206eb044a..bec078a3d9b1 100644 --- a/tools/binman/setup.py +++ b/tools/binman/setup.py @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0+ -from distutils.core import setup +from setuptools import setup setup(name='binman', version='1.0', license='GPL-2.0+', diff --git a/tools/buildman/requirements.txt b/tools/buildman/requirements.txt index 4a31e69e4cb5..350da42c0ebf 100644 --- a/tools/buildman/requirements.txt +++ b/tools/buildman/requirements.txt @@ -1,3 +1,4 @@ jsonschema==4.17.3 pyyaml==6.0 yamllint==1.26.3 +setuptools==65.5.1 diff --git a/tools/dtoc/setup.py b/tools/dtoc/setup.py index 5e092fe0872a..ae9ad043b013 100644 --- a/tools/dtoc/setup.py +++ b/tools/dtoc/setup.py @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0+ -from distutils.core import setup +from setuptools import setup setup(name='dtoc', version='1.0', license='GPL-2.0+', -- 2.43.0
[PATCH 02/20] binman: Replace pkg_resources with importlib.resources
pkg_resources is deprecated long ago and being removed in python 3.12. Reimplement functions with importlib.resources. Link: https://docs.python.org/3/library/importlib.resources.html Signed-off-by: Jiaxun Yang --- tools/binman/control.py | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/binman/control.py b/tools/binman/control.py index 2f00279232b8..5549b0ad2185 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -8,12 +8,11 @@ from collections import OrderedDict import glob try: -import importlib.resources -except ImportError: # pragma: no cover +from importlib import resources +except ImportError: # for Python 3.6 -import importlib_resources +import importlib_resources as resources import os -import pkg_resources import re import sys @@ -96,12 +95,12 @@ def _ReadMissingBlobHelp(): msg = '' return tag, msg -my_data = pkg_resources.resource_string(__name__, 'missing-blob-help') +my_data = resources.files(__package__).joinpath('missing-blob-help').read_text() re_tag = re.compile('^([-a-z0-9]+):$') result = {} tag = None msg = '' -for line in my_data.decode('utf-8').splitlines(): +for line in my_data.splitlines(): if not line.startswith('#'): m_tag = re_tag.match(line) if m_tag: @@ -151,8 +150,9 @@ def GetEntryModules(include_testing=True): Returns: Set of paths to entry class filenames """ -glob_list = pkg_resources.resource_listdir(__name__, 'etype') -glob_list = [fname for fname in glob_list if fname.endswith('.py')] +directory = resources.files("binman.etype") +glob_list = [entry.name for entry in directory.iterdir() + if entry.name.endswith('.py')] return set([os.path.splitext(os.path.basename(item))[0] for item in glob_list if include_testing or '_testing' not in item]) @@ -735,7 +735,7 @@ def Binman(args): global state if args.full_help: -with importlib.resources.path('binman', 'README.rst') as readme: +with resources.path('binman', 'README.rst') as readme: tools.print_full_help(str(readme)) return 0 -- 2.43.0
[PATCH 01/20] py: Replace deprecated unittest APIs
assertEquals -> assertEqual assertRegexpMatches -> assertRegex Those APIs were deprecated long ago and being removed in latest python. Signed-off-by: Jiaxun Yang --- tools/binman/entry_test.py | 6 ++-- tools/binman/fdt_test.py| 48 ++--- tools/binman/ftest.py | 42 - tools/buildman/func_test.py | 74 ++--- tools/buildman/test.py | 2 +- 5 files changed, 86 insertions(+), 86 deletions(-) diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py index ac6582cf86a8..40d74d401a20 100644 --- a/tools/binman/entry_test.py +++ b/tools/binman/entry_test.py @@ -103,7 +103,7 @@ class TestEntry(unittest.TestCase): ent = entry.Entry.Create(None, self.GetNode(), 'missing', missing_etype=True) self.assertTrue(isinstance(ent, Entry_blob)) -self.assertEquals('missing', ent.etype) +self.assertEqual('missing', ent.etype) def testDecompressData(self): """Test the DecompressData() method of the base class""" @@ -111,8 +111,8 @@ class TestEntry(unittest.TestCase): base.compress = 'lz4' bintools = {} base.comp_bintool = base.AddBintool(bintools, '_testing') -self.assertEquals(tools.get_bytes(0, 1024), base.CompressData(b'abc')) -self.assertEquals(tools.get_bytes(0, 1024), base.DecompressData(b'abc')) +self.assertEqual(tools.get_bytes(0, 1024), base.CompressData(b'abc')) +self.assertEqual(tools.get_bytes(0, 1024), base.DecompressData(b'abc')) def testLookupOffset(self): """Test the lookup_offset() method of the base class""" diff --git a/tools/binman/fdt_test.py b/tools/binman/fdt_test.py index 7ef872954630..564c17708207 100644 --- a/tools/binman/fdt_test.py +++ b/tools/binman/fdt_test.py @@ -44,43 +44,43 @@ class TestFdt(unittest.TestCase): fname = self.GetCompiled('045_prop_test.dts') dt = FdtScan(fname) node = dt.GetNode('/binman/intel-me') -self.assertEquals('intel-me', node.name) +self.assertEqual('intel-me', node.name) val = fdt_util.GetString(node, 'filename') -self.assertEquals(str, type(val)) -self.assertEquals('me.bin', val) +self.assertEqual(str, type(val)) +self.assertEqual('me.bin', val) prop = node.props['intval'] -self.assertEquals(fdt.Type.INT, prop.type) -self.assertEquals(3, fdt_util.GetInt(node, 'intval')) +self.assertEqual(fdt.Type.INT, prop.type) +self.assertEqual(3, fdt_util.GetInt(node, 'intval')) prop = node.props['intarray'] -self.assertEquals(fdt.Type.INT, prop.type) -self.assertEquals(list, type(prop.value)) -self.assertEquals(2, len(prop.value)) -self.assertEquals([5, 6], +self.assertEqual(fdt.Type.INT, prop.type) +self.assertEqual(list, type(prop.value)) +self.assertEqual(2, len(prop.value)) +self.assertEqual([5, 6], [fdt_util.fdt32_to_cpu(val) for val in prop.value]) prop = node.props['byteval'] -self.assertEquals(fdt.Type.BYTE, prop.type) -self.assertEquals(chr(8), prop.value) +self.assertEqual(fdt.Type.BYTE, prop.type) +self.assertEqual(chr(8), prop.value) prop = node.props['bytearray'] -self.assertEquals(fdt.Type.BYTE, prop.type) -self.assertEquals(list, type(prop.value)) -self.assertEquals(str, type(prop.value[0])) -self.assertEquals(3, len(prop.value)) -self.assertEquals([chr(1), '#', '4'], prop.value) +self.assertEqual(fdt.Type.BYTE, prop.type) +self.assertEqual(list, type(prop.value)) +self.assertEqual(str, type(prop.value[0])) +self.assertEqual(3, len(prop.value)) +self.assertEqual([chr(1), '#', '4'], prop.value) prop = node.props['longbytearray'] -self.assertEquals(fdt.Type.INT, prop.type) -self.assertEquals(0x090a0b0c, fdt_util.GetInt(node, 'longbytearray')) +self.assertEqual(fdt.Type.INT, prop.type) +self.assertEqual(0x090a0b0c, fdt_util.GetInt(node, 'longbytearray')) prop = node.props['stringval'] -self.assertEquals(fdt.Type.STRING, prop.type) -self.assertEquals('message2', fdt_util.GetString(node, 'stringval')) +self.assertEqual(fdt.Type.STRING, prop.type) +self.assertEqual('message2', fdt_util.GetString(node, 'stringval')) prop = node.props['stringarray'] -self.assertEquals(fdt.Type.STRING, prop.type) -self.assertEquals(list, type(prop.value)) -self.assertEquals(3, len(prop.value)) -self.assertEquals(['another', 'multi-word', 'message'], prop.value) +self.assertEqual(fdt.Type.STRING, prop.type) +self.assertEqual(list, type(prop.value)) +self.assertEqual(3, len(prop.value)) +self.assertEqual(['an
[PATCH 00/20] New CI image and fixes
Hi all, This series build a new CI image based on Ubuntu focal with LoongArch64 support, fixed various python scripts for python 3.12, fixed various problems popped up when testing againt latest software. This change must be combined with test hook changes at [1]. Last two commits are for demonstration purpose and not for commit into repo. CI runs passed at azure [2] and public gitlab.com runner [3]. Thanks [1]: https://lore.kernel.org/u-boot/20240611210025.798978-1-jiaxun.y...@flygoat.com/ [2]: https://gitlab.com/FlyGoat/u-boot/-/pipelines/1327832544 [3]: https://flygoat.visualstudio.com/u-boot/_build/results?buildId=63&view=results Signed-off-by: Jiaxun Yang --- Jiaxun Yang (20): py: Replace deprecated unittest APIs binman: Replace pkg_resources with importlib.resources py: Replace distutils.core with setuptools py: Replace usage of configparser.read_fp doc/sphinx: Remove usage of six py: Remove unused entries in requirements.txt py: Bump requirements versions py: Bump pylint version and clear warnings binman: Workaround lz4 cli padding in test cases tests/test_event_dump: Relax match rule for output lib/charset & efi: Fix possible unaligned accesses cyclic: Rise default CYCLIC_MAX_CPU_TIME_US to 5000 CI: Ensure pip install is always performed in venv CI: GitLab: Split build_world tasks CI: Dockerfile: Set global git name & email config CI: Dockerfile: Bump various software version CI: Dockerfile: Add LoongArch64 support doc: ci: Document how to run pipeline on gitlab.com [NFC] Use Jiaxun's CI Image [NFC] CI: Dockerfile: Replace some URL with mirror sites .azure-pipelines.yml| 22 +++-- .gitlab-ci.yml | 122 ++-- common/Kconfig | 2 +- configs/octeon_nic23_defconfig | 1 - doc/develop/ci_testing.rst | 12 ++- doc/develop/python_cq.rst | 4 +- doc/sphinx/kfigure.py | 3 +- doc/sphinx/requirements.txt | 1 - lib/charset.c | 21 ++-- lib/efi_loader/efi_device_path.c| 11 +-- test/py/requirements.txt| 24 + test/py/tests/test_event_dump.py| 10 +- test/py/tests/test_ums.py | 1 + test/py/tests/test_usb.py | 1 + tools/binman/control.py | 18 ++-- tools/binman/entry_test.py | 6 +- tools/binman/etype/fdtmap.py| 1 + tools/binman/etype/fit.py | 1 + tools/binman/etype/image_header.py | 1 + tools/binman/etype/pre_load.py | 2 + tools/binman/etype/ti_board_config.py | 1 + tools/binman/etype/x509_cert.py | 1 + tools/binman/fdt_test.py| 48 +- tools/binman/ftest.py | 50 +- tools/binman/setup.py | 2 +- tools/binman/state.py | 1 + tools/binman/test/184_compress_section_size.dts | 1 + tools/buildman/bsettings.py | 2 +- tools/buildman/builder.py | 2 + tools/buildman/func_test.py | 74 +++--- tools/buildman/requirements.txt | 7 +- tools/buildman/test.py | 2 +- tools/docker/Dockerfile | 75 +-- tools/dtoc/setup.py | 2 +- tools/microcode-tool.py | 1 + tools/patman/settings.py| 9 +- tools/patman/test_checkpatch.py | 2 + tools/qconfig.py| 1 + 38 files changed, 293 insertions(+), 252 deletions(-) --- base-commit: 1ebd659cf020843fd8e8ef90d85a66941cbab6ec change-id: 20240610-docker-image-868126a1a929 Best regards, -- Jiaxun Yang
Re: [RFC PATCH 04/31] lmb: remove local instances of the lmb structure variable
On Tue, Jun 11, 2024 at 12:52:22PM -0600, Simon Glass wrote: > Hi Sughosh, > > On Fri, 7 Jun 2024 at 12:53, Sughosh Ganu wrote: > > > > With the move of the LMB structure to a persistent state, there is no > > need to declare the variable locally, and pass it as part of the LMB > > API's. Remove all local variable instances and change the API's > > correspondingly. > > > > Signed-off-by: Sughosh Ganu > > --- > > arch/arc/lib/cache.c | 4 +- > > arch/arm/lib/stack.c | 4 +- > > arch/arm/mach-apple/board.c | 17 ++- > > arch/arm/mach-snapdragon/board.c | 17 ++- > > arch/arm/mach-stm32mp/dram_init.c| 7 +- > > arch/arm/mach-stm32mp/stm32mp1/cpu.c | 6 +- > > arch/m68k/lib/bootm.c| 7 +- > > arch/microblaze/lib/bootm.c | 4 +- > > arch/mips/lib/bootm.c| 9 +- > > arch/nios2/lib/bootm.c | 4 +- > > arch/powerpc/cpu/mpc85xx/mp.c| 4 +- > > arch/powerpc/include/asm/mp.h| 4 +- > > arch/powerpc/lib/bootm.c | 14 +- > > arch/riscv/lib/bootm.c | 4 +- > > arch/sh/lib/bootm.c | 4 +- > > arch/x86/lib/bootm.c | 4 +- > > arch/xtensa/lib/bootm.c | 4 +- > > board/xilinx/common/board.c | 7 +- > > boot/bootm.c | 26 ++-- > > boot/bootm_os.c | 5 +- > > boot/image-board.c | 32 ++--- > > boot/image-fdt.c | 29 ++--- > > cmd/bdinfo.c | 6 +- > > cmd/booti.c | 2 +- > > cmd/bootz.c | 2 +- > > cmd/load.c | 7 +- > > drivers/iommu/apple_dart.c | 7 +- > > drivers/iommu/sandbox_iommu.c| 15 +-- > > fs/fs.c | 7 +- > > include/image.h | 22 +--- > > include/lmb.h| 39 +++--- > > lib/lmb.c| 81 ++-- > > net/tftp.c | 5 +- > > net/wget.c | 5 +- > > test/cmd/bdinfo.c| 2 +- > > test/lib/lmb.c | 187 +-- > > 36 files changed, 270 insertions(+), 333 deletions(-) > > This isn't necessary...and it will make things harder. You can have a > global 'lmb' while still allowing passing a different pointer when > needed. There's only one reservation checking system and list of known reservations, keep in mind. -- Tom signature.asc Description: PGP signature
Re: [RFC PATCH 15/31] efi_memory: add an event handler to update memory map
On Tue, Jun 11, 2024 at 12:52:19PM -0600, Simon Glass wrote: > Hi, > > On Tue, 11 Jun 2024 at 08:36, Tom Rini wrote: > > > > On Tue, Jun 11, 2024 at 12:17:16PM +0200, Heinrich Schuchardt wrote: > > > On 07.06.24 20:52, Sughosh Ganu wrote: > > > > There are events that would be used to notify other interested modules > > > > of any changes in available and occupied memory. This would happen > > > > when a module allocates or reserves memory, or frees up memory. These > > > > changes in memory map should be notified to other interested modules > > > > so that the allocated memory does not get overwritten. Add an event > > > > handler in the EFI memory module to update the EFI memory map > > > > accordingly when such changes happen. As a consequence, any subsequent > > > > memory request would honour the updated memory map and only available > > > > memory would be allocated from. > > > > > > > > Signed-off-by: Sughosh Ganu > > > > --- > > > > lib/efi_loader/efi_memory.c | 70 ++--- > > > > 1 file changed, 58 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > > > > index 435e580fb3..93244161b0 100644 > > > > --- a/lib/efi_loader/efi_memory.c > > > > +++ b/lib/efi_loader/efi_memory.c > > > > @@ -73,6 +73,10 @@ struct efi_pool_allocation { > > > > #if CONFIG_IS_ENABLED(MEM_MAP_UPDATE_NOTIFY) > > > > extern bool is_addr_in_ram(uintptr_t addr); > > > > > > > > +static efi_status_t __efi_add_memory_map_pg(u64 start, u64 pages, > > > > + int memory_type, > > > > + bool overlap_only_ram); > > > > + > > > > static void efi_map_update_notify(u64 addr, u64 size, u8 op) > > > > { > > > > struct event_efi_mem_map_update efi_map = {0}; > > > > @@ -84,6 +88,34 @@ static void efi_map_update_notify(u64 addr, u64 > > > > size, u8 op) > > > > if (is_addr_in_ram((uintptr_t)addr)) > > > > event_notify(EVT_EFI_MEM_MAP_UPDATE, &efi_map, > > > > sizeof(efi_map)); > > > > } > > > > + > > > > +static int lmb_mem_map_update_sync(void *ctx, struct event *event) > > > > +{ > > > > + u8 op; > > > > + u64 addr; > > > > + u64 pages; > > > > + efi_status_t status; > > > > + struct event_lmb_map_update *lmb_map = &event->data.lmb_map; > > > > + > > > > + addr = (uintptr_t)map_sysmem(lmb_map->base, 0); > > > > + pages = efi_size_in_pages(lmb_map->size + (addr & EFI_PAGE_MASK)); > > > > + op = lmb_map->op; > > > > + addr &= ~EFI_PAGE_MASK; > > > > + > > > > + if (op != MAP_OP_RESERVE && op != MAP_OP_FREE) { > > > > + log_debug("Invalid map update op received (%d)\n", op); > > > > + return -1; > > > > + } > > > > + > > > > + status = __efi_add_memory_map_pg(addr, pages, > > > > +op == MAP_OP_FREE ? > > > > +EFI_CONVENTIONAL_MEMORY : > > > > > > This is dangerous. LMB might turn memory that is marked as > > > EfiReservedMemory which the OS must respect into EfiBootServicesData > > > which the OS may discard. > > > > > > E.g. initr_lmb() is being called after efi_memory_init(). > > > > > > Getting all cases of synchronization properly tested seems very hard to > > > me. Everything would be much easier if we had only a single memory > > > management system. > > > > Yes, Sughosh is working on the single memory reservation system for > > everyone to use. This pairs with the single memory allocation system > > (malloc) that we have. Parts of the code base that aren't keeping these > > systems up to date / obeying their results need to be corrected. > > The EFI allocations don't happen until boot time...so why do we need > to do this now? We can instead have an EFI function to scan LMB and > add to its memory map. We're talking about reservations, not allocations. So yes, when someone is making their reservation, they need to make it. I don't understand your question. -- Tom signature.asc Description: PGP signature
[u-boot-test-hooks PATCH 4/4] qemu-loongarch64: New board
Signed-off-by: Jiaxun Yang --- bin/travis-ci/conf.qemu-loongarch64_na | 12 py/travis-ci/u_boot_boardenv_qemu_loongarch64_na.py | 11 +++ 2 files changed, 23 insertions(+) create mode 100644 bin/travis-ci/conf.qemu-loongarch64_na create mode 100644 py/travis-ci/u_boot_boardenv_qemu_loongarch64_na.py diff --git a/bin/travis-ci/conf.qemu-loongarch64_na b/bin/travis-ci/conf.qemu-loongarch64_na new file mode 100644 index ..e8860bb40326 --- /dev/null +++ b/bin/travis-ci/conf.qemu-loongarch64_na @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: MIT +# +# Copyright (c) 2024 Jiaxun Yang +# + +console_impl=qemu +qemu_machine="virt" +qemu_binary="qemu-system-loongarch64" +qemu_extra_args="-m 1G -nographic -netdev user,id=net0,tftp=${UBOOT_TRAVIS_BUILD_DIR} -device virtio-net-pci,netdev=net0 -device virtio-rng-pci" +qemu_kernel_args="-bios ${U_BOOT_BUILD_DIR}/u-boot.bin" +reset_impl=none +flash_impl=none diff --git a/py/travis-ci/u_boot_boardenv_qemu_loongarch64_na.py b/py/travis-ci/u_boot_boardenv_qemu_loongarch64_na.py new file mode 100644 index ..8a9f747f0457 --- /dev/null +++ b/py/travis-ci/u_boot_boardenv_qemu_loongarch64_na.py @@ -0,0 +1,11 @@ +import os +import travis_tftp + +env__net_uses_pci = True +env__net_dhcp_server = True +env__net_tftp_readable_file = travis_tftp.file2env('u-boot') +env__efi_loader_helloworld_file = travis_tftp.file2env('lib/efi_loader/helloworld.efi') +env__efi_loader_grub_file = travis_tftp.file2env('grub_loongarch64.efi') +env__efi_fit_tftp_file = { +"dn" : os.environ['UBOOT_TRAVIS_BUILD_DIR'], +} -- 2.43.0
[u-boot-test-hooks PATCH 3/4] qemu-xtensa-dc233c: New board
Signed-off-by: Jiaxun Yang --- bin/travis-ci/conf.qemu-xtensa-dc233c_na | 12 .../u_boot_boardenv_qemu_xtensa_dc233c_na.py | 6 ++ 2 files changed, 18 insertions(+) create mode 100644 bin/travis-ci/conf.qemu-xtensa-dc233c_na create mode 100644 py/travis-ci/u_boot_boardenv_qemu_xtensa_dc233c_na.py diff --git a/bin/travis-ci/conf.qemu-xtensa-dc233c_na b/bin/travis-ci/conf.qemu-xtensa-dc233c_na new file mode 100644 index ..fc3b5880e5c1 --- /dev/null +++ b/bin/travis-ci/conf.qemu-xtensa-dc233c_na @@ -0,0 +1,12 @@ +# SPDX-License-Identifier: MIT +# +# Copyright (c) 2024 Jiaxun Yang +# + +console_impl=qemu +qemu_machine="virt" +qemu_binary="qemu-system-xtensa" +qemu_extra_args="-cpu dc233c -nographic -netdev user,id=net0,tftp=${UBOOT_TRAVIS_BUILD_DIR} -device virtio-net-pci,netdev=net0 -device virtio-rng-pci -semihosting" +qemu_kernel_args="-kernel ${U_BOOT_BUILD_DIR}/u-boot.elf" +reset_impl=none +flash_impl=none diff --git a/py/travis-ci/u_boot_boardenv_qemu_xtensa_dc233c_na.py b/py/travis-ci/u_boot_boardenv_qemu_xtensa_dc233c_na.py new file mode 100644 index ..8fdb24b284c7 --- /dev/null +++ b/py/travis-ci/u_boot_boardenv_qemu_xtensa_dc233c_na.py @@ -0,0 +1,6 @@ +import os +import travis_tftp + +env__net_uses_pci = True +env__net_dhcp_server = True +env__net_tftp_readable_file = travis_tftp.file2env('u-boot') -- 2.43.0
[u-boot-test-hooks PATCH 2/4] qemu-arm64be: New board
Signed-off-by: Jiaxun Yang --- bin/travis-ci/conf.qemu_arm64be_na | 13 + py/travis-ci/u_boot_boardenv_qemu_arm64be_na.py | 10 ++ 2 files changed, 23 insertions(+) create mode 100644 bin/travis-ci/conf.qemu_arm64be_na create mode 100644 py/travis-ci/u_boot_boardenv_qemu_arm64be_na.py diff --git a/bin/travis-ci/conf.qemu_arm64be_na b/bin/travis-ci/conf.qemu_arm64be_na new file mode 100644 index ..3929636932e6 --- /dev/null +++ b/bin/travis-ci/conf.qemu_arm64be_na @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: MIT +# +# Copyright (c) 2024 Jiaxun Yang +# + +console_impl=qemu +qemu_machine="virt" +qemu_helper_script="swtpm" +qemu_binary="qemu-system-aarch64" +qemu_extra_args="-cpu cortex-a57 -nographic -netdev user,id=net0,tftp=${UBOOT_TRAVIS_BUILD_DIR} -device e1000,netdev=net0 -device virtio-rng-pci -semihosting -chardev socket,id=chrtpm,path=/tmp/tpm/swtpm-sock -tpmdev emulator,id=tpm0,chardev=chrtpm -device tpm-tis-device,tpmdev=tpm0" +qemu_kernel_args="-bios ${U_BOOT_BUILD_DIR}/u-boot.bin" +reset_impl=none +flash_impl=none diff --git a/py/travis-ci/u_boot_boardenv_qemu_arm64be_na.py b/py/travis-ci/u_boot_boardenv_qemu_arm64be_na.py new file mode 100644 index ..5746b37dbcf5 --- /dev/null +++ b/py/travis-ci/u_boot_boardenv_qemu_arm64be_na.py @@ -0,0 +1,10 @@ +import os +import travis_tftp + +env__net_uses_pci = True +env__net_dhcp_server = True +env__net_tftp_readable_file = travis_tftp.file2env('u-boot.bin', 0x4040) +env__efi_fit_tftp_file = { +'addr' : 0x4040, +"dn" : os.environ['UBOOT_TRAVIS_BUILD_DIR'], +} -- 2.43.0
[u-boot-test-hooks PATCH 0/4] QEMU Updates
Hi all, This series updated u-boot-test-hooks to adopt QEMU 9.0. It also introduced 3 new boards: qemu-arm64be, qemu-xtensa-dc233c and qemu-loongarch64. This is required for CI changes. I'm going to respin other series later to utilise those new hooks. Thanks Jiaxun Yang (4): qemu-vexpress*: Pass -audio none qemu-arm64be: New board qemu-xtensa-dc233c: New board qemu-loongarch64: New board bin/travis-ci/conf.qemu-loongarch64_na | 12 bin/travis-ci/conf.qemu-xtensa-dc233c_na| 12 bin/travis-ci/conf.qemu_arm64be_na | 13 + bin/travis-ci/conf.vexpress_ca15_tc2_qemu | 2 +- bin/travis-ci/conf.vexpress_ca9x4_qemu | 2 +- py/travis-ci/u_boot_boardenv_qemu_arm64be_na.py | 10 ++ py/travis-ci/u_boot_boardenv_qemu_loongarch64_na.py | 11 +++ .../u_boot_boardenv_qemu_xtensa_dc233c_na.py| 6 ++ 8 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 bin/travis-ci/conf.qemu-loongarch64_na create mode 100644 bin/travis-ci/conf.qemu-xtensa-dc233c_na create mode 100644 bin/travis-ci/conf.qemu_arm64be_na create mode 100644 py/travis-ci/u_boot_boardenv_qemu_arm64be_na.py create mode 100644 py/travis-ci/u_boot_boardenv_qemu_loongarch64_na.py create mode 100644 py/travis-ci/u_boot_boardenv_qemu_xtensa_dc233c_na.py -- 2.43.0
[u-boot-test-hooks PATCH 1/4] qemu-vexpress*: Pass -audio none
Those boards have build in sound card which cause problems on CI runner. Pass -audio none to disable sound card backends. Signed-off-by: Jiaxun Yang --- bin/travis-ci/conf.vexpress_ca15_tc2_qemu | 2 +- bin/travis-ci/conf.vexpress_ca9x4_qemu| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/travis-ci/conf.vexpress_ca15_tc2_qemu b/bin/travis-ci/conf.vexpress_ca15_tc2_qemu index 83d703269377..55e40038b379 100644 --- a/bin/travis-ci/conf.vexpress_ca15_tc2_qemu +++ b/bin/travis-ci/conf.vexpress_ca15_tc2_qemu @@ -21,7 +21,7 @@ console_impl=qemu qemu_machine="vexpress-a15" qemu_binary="qemu-system-arm" -qemu_extra_args="-nographic -m 1G -net user,tftp=${UBOOT_TRAVIS_BUILD_DIR} -net nic" +qemu_extra_args="-nographic -m 1G -audio none -net user,tftp=${UBOOT_TRAVIS_BUILD_DIR} -net nic" qemu_kernel_args="-kernel ${U_BOOT_BUILD_DIR}/u-boot" reset_impl=none flash_impl=none diff --git a/bin/travis-ci/conf.vexpress_ca9x4_qemu b/bin/travis-ci/conf.vexpress_ca9x4_qemu index d07be2b12984..b5ed72934c3a 100644 --- a/bin/travis-ci/conf.vexpress_ca9x4_qemu +++ b/bin/travis-ci/conf.vexpress_ca9x4_qemu @@ -21,7 +21,7 @@ console_impl=qemu qemu_machine="vexpress-a9" qemu_binary="qemu-system-arm" -qemu_extra_args="-nographic -m 1G -net user,tftp=${UBOOT_TRAVIS_BUILD_DIR} -net nic" +qemu_extra_args="-nographic -m 1G -audio none -net user,tftp=${UBOOT_TRAVIS_BUILD_DIR} -net nic" qemu_kernel_args="-kernel ${U_BOOT_BUILD_DIR}/u-boot" reset_impl=none flash_impl=none -- 2.43.0
Re: [PATCH v5] cmd: move ELF load and boot to lib/elf.c
On 06.06.2024 17:21, Tom Rini wrote: On Thu, Jun 06, 2024 at 09:02:43AM +0200, Heinrich Schuchardt wrote: On 6/5/24 20:43, Maxim Moskalets wrote: From: Maxim Moskalets Loading and running the ELF image is the responsibility of the library and should not be associated with the command line interface. It is also required to run ELF images from FIT with the bootm command so as not to depend on the command line interface. Signed-off-by: Maxim Moskalets In future versions please, include a change log separated from the commit message by ---, e.g. --- v6: foo changed to bar v5: xxx removed --- Best regards Heinrich Do you have any other feedback, Heinrich? I will take these comments into account in future patches. Are there any other comments? Best regards Maxim
Re: [PATCH 00/42] labgrid: Provide an integration with Labgrid
Hi, On Tue, 11 Jun 2024 at 14:02, Simon Glass wrote: > > Labgrid provides access to a hardware lab in an automated way. It is > possible to boot U-Boot on boards in the lab without physically touching > them. It relies on relays, USB UARTs and SD muxes, among other things. > > By way of background, about 4 years ago I wrong a thing called Labman[1] > which allowed my lab of about 30 devices to be operated remotely, using > tbot for the console and build integration. While it worked OK and I > used it for many bisects, I didn't take it any further. > > It turns out that there was already an existing program, called Labgrid, > which I did not know about at time (thank you Tom for telling me). It is > more rounded than Labman and has a number of advantages: > > - does not need udev rules, mostly > - has several existing users who rely on it > - supports multiple machines exporting their devices > > It lacks a 'lab check' feature and a few other things, but these can be > remedied. > > On and off over the past several weeks I have been experimenting with > Labgrid. I have managed to create an initial U-Boot integration (this > series) by adding various features to Labgrid[2] and the U-Boot test > hooks. > > I hope that this might inspire others to set up boards and run tests > automatically, rather than relying on infrequent, manual test. Perhaps > it may even be possible to have a number of labs available. > > Included in the integration are a number of simple scripts which make it > easy to connect to boards and run tests: > > ub-int > Build and boot on a target, starting an interactive session > > ub-cli > Build and boot on a target, ensure U-Boot starts and provide an > interactive > session from there > > ub-smoke > Smoke test U-Boot to check that it boots to a prompt on a target > > ub-bisect > Bisect a git tree to locate a failure on a particular target > > ub-pyt > Run U-Boot pytests on a target > > Some of these help to provide the same tbot[4] workflow which I have > relied on for several years, albeit much simpler versions. > > The goal here is to create some sort of script which can collect > patches from the mailing list, apply them and test them on a selection > of boards. I suspect that script already exists, so please let me know > what you suggest. > > I hope you find this interesting and take a look! > > [1] https://github.com/sjg20/u-boot/tree/lab6a > [2] https://github.com/labgrid-project/labgrid/pull/1411 > [3] https://github.com/sjg20/uboot-test-hooks/tree/labgrid > [4] https://tbot.tools/index.html > > > Simon Glass (42): > trace: Update test to tolerate different trace-cmd version > binman: efi: Correct entry docs > binman: Regenerate nxp docs > binman: ti: Regenerate entry docs > binman: Update the entrydocs header > buildman: Make mrproper an argument to _reconfigure() > buildman: Make mrproper an argument to _config_and_build() > buildman: Make mrproper an argument to run_commit() > buildman: Avoid rebuilding when --mrproper is used > buildman: Add a flag to force mrproper on failure > buildman: Retry the build for current source > buildman: Add a way to limit the number of buildmans > dm: core: Enhance comments on bind_drivers_pass() > initcall: Correct use of relocation offset > am33xx: Provide a function to set up the debug UART > sunxi: Mark scp as optional > google: Disable TPMv2 on most Chromebooks > meson: Correct driver declaration for meson_axg_gpio > test: Allow signaling that U-Boot is ready > test: Make bootstd init run only on sandbox > test: Use a constant for the test timeout > test: Pass stderr to stdout > test: Release board after tests complete > log: Allow tests to pass with CONFIG_LOGF_FUNC_PAD set > test: Allow connecting to a running board > test: Decode exceptions only with sandbox > test: Avoid failing skipped tests > test: dm: Show failing driver name > test: Check help output > test: Create a common function to get the config > test: Introduce the concept of a role > test: Move the receive code into a function > test: Separate out the exception handling > test: Detect dead connections > test: Tidy up remaining exceptions > test: Introduce lab mode > test: Improve handling of sending commands > test: Fix mulptiplex_log typo > test: Avoid double echo when starting up > test: Try to shut down the lab console gracefully > test: Add a section for closing the connection > CI: Allow running tests on sjg lab > > .gitlab-ci.yml| 151 + > arch/arm/dts/sunxi-u-boot.dtsi| 1 + > arch/arm/mach-omap2/am33xx/board.c| 18 +- > configs/chromebook_link64_defconfig | 1 + > configs/chromebook_link_defconfig | 1 + > configs/chromebook_samus_defconfig| 1 + > configs/chromebook_samus_tpl_defconfig| 1 + > configs/nyan-big_defconfig
[PATCH 00/42] labgrid: Provide an integration with Labgrid
Labgrid provides access to a hardware lab in an automated way. It is possible to boot U-Boot on boards in the lab without physically touching them. It relies on relays, USB UARTs and SD muxes, among other things. By way of background, about 4 years ago I wrong a thing called Labman[1] which allowed my lab of about 30 devices to be operated remotely, using tbot for the console and build integration. While it worked OK and I used it for many bisects, I didn't take it any further. It turns out that there was already an existing program, called Labgrid, which I did not know about at time (thank you Tom for telling me). It is more rounded than Labman and has a number of advantages: - does not need udev rules, mostly - has several existing users who rely on it - supports multiple machines exporting their devices It lacks a 'lab check' feature and a few other things, but these can be remedied. On and off over the past several weeks I have been experimenting with Labgrid. I have managed to create an initial U-Boot integration (this series) by adding various features to Labgrid[2] and the U-Boot test hooks. I hope that this might inspire others to set up boards and run tests automatically, rather than relying on infrequent, manual test. Perhaps it may even be possible to have a number of labs available. Included in the integration are a number of simple scripts which make it easy to connect to boards and run tests: ub-int Build and boot on a target, starting an interactive session ub-cli Build and boot on a target, ensure U-Boot starts and provide an interactive session from there ub-smoke Smoke test U-Boot to check that it boots to a prompt on a target ub-bisect Bisect a git tree to locate a failure on a particular target ub-pyt Run U-Boot pytests on a target Some of these help to provide the same tbot[4] workflow which I have relied on for several years, albeit much simpler versions. The goal here is to create some sort of script which can collect patches from the mailing list, apply them and test them on a selection of boards. I suspect that script already exists, so please let me know what you suggest. I hope you find this interesting and take a look! [1] https://github.com/sjg20/u-boot/tree/lab6a [2] https://github.com/labgrid-project/labgrid/pull/1411 [3] https://github.com/sjg20/uboot-test-hooks/tree/labgrid [4] https://tbot.tools/index.html Simon Glass (42): trace: Update test to tolerate different trace-cmd version binman: efi: Correct entry docs binman: Regenerate nxp docs binman: ti: Regenerate entry docs binman: Update the entrydocs header buildman: Make mrproper an argument to _reconfigure() buildman: Make mrproper an argument to _config_and_build() buildman: Make mrproper an argument to run_commit() buildman: Avoid rebuilding when --mrproper is used buildman: Add a flag to force mrproper on failure buildman: Retry the build for current source buildman: Add a way to limit the number of buildmans dm: core: Enhance comments on bind_drivers_pass() initcall: Correct use of relocation offset am33xx: Provide a function to set up the debug UART sunxi: Mark scp as optional google: Disable TPMv2 on most Chromebooks meson: Correct driver declaration for meson_axg_gpio test: Allow signaling that U-Boot is ready test: Make bootstd init run only on sandbox test: Use a constant for the test timeout test: Pass stderr to stdout test: Release board after tests complete log: Allow tests to pass with CONFIG_LOGF_FUNC_PAD set test: Allow connecting to a running board test: Decode exceptions only with sandbox test: Avoid failing skipped tests test: dm: Show failing driver name test: Check help output test: Create a common function to get the config test: Introduce the concept of a role test: Move the receive code into a function test: Separate out the exception handling test: Detect dead connections test: Tidy up remaining exceptions test: Introduce lab mode test: Improve handling of sending commands test: Fix mulptiplex_log typo test: Avoid double echo when starting up test: Try to shut down the lab console gracefully test: Add a section for closing the connection CI: Allow running tests on sjg lab .gitlab-ci.yml| 151 + arch/arm/dts/sunxi-u-boot.dtsi| 1 + arch/arm/mach-omap2/am33xx/board.c| 18 +- configs/chromebook_link64_defconfig | 1 + configs/chromebook_link_defconfig | 1 + configs/chromebook_samus_defconfig| 1 + configs/chromebook_samus_tpl_defconfig| 1 + configs/nyan-big_defconfig| 4 +- configs/snow_defconfig| 1 + drivers/core/lists.c | 16 ++ drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c | 2 +- drivers/pinctrl/meson/pinctrl-meson-axg.c | 4 +- drivers/pinctrl/meson/pin
Re: [u-boot-test-hooks PATCH 1/2] Create a common file for test scripts
Hi Tom, On Tue, 11 Jun 2024 at 10:07, Tom Rini wrote: > > On Mon, Jun 10, 2024 at 04:27:42PM -0600, Simon Glass wrote: > > > The top part of each of the u-boot-test-* files is common. Put it in > > a common script file to avoid duplication and to allow it to be > > replaced for the Labgrid integration. > > > > Signed-off-by: Simon Glass > [snip] > > +++ b/bin/u-boot-test-common > [snip] > > +bin_dir="`dirname $0`" > > So we always set bin_dir here... > > > +board_type="$1" > > +board_ident="$2" > > +hostname="`hostname`" > > + > > +. "${bin_dir}/${hostname}/conf.${board_type}_${board_ident}" > > diff --git a/bin/u-boot-test-console b/bin/u-boot-test-console > > index 0b6b4ac..ad90040 100755 > > --- a/bin/u-boot-test-console > > +++ b/bin/u-boot-test-console > > @@ -20,12 +20,7 @@ > > # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > > # DEALINGS IN THE SOFTWARE. > > > > -set -e > > - > > bin_dir="`dirname $0`" > > But never remove it from the other files. Yes that was my intent, but then I realised that it is used in u-boot-test-console, then wasn't sure it was a win... Perhaps I could do this in each script? . "$(dirname $0)/u-boot-test-common" and that would allow removing bin_dir. The above is pretty clear. Regards, Simon
[PATCH 42/42] CI: Allow running tests on sjg lab
Add a way to run tests on a real hardware lab. This is in the very early experimental stages. There are only 23 boards and 3 of those are broken! (bob, ff3399, samus). A fourth fails due to problems with the TPM tests. To try this, assuming you have gitlab access, set SJG_LAB=1, e.g.: git push -o ci.variable="SJG_LAB=1" dm HEAD:try Signed-off-by: Simon Glass --- .gitlab-ci.yml | 151 + 1 file changed, 151 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 165f765a833..6c362340341 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -17,6 +17,7 @@ stages: - testsuites - test.py - world build + - sjg-lab .buildman_and_testpy_template: &buildman_and_testpy_dfn stage: test.py @@ -482,3 +483,153 @@ coreboot test.py: TEST_PY_TEST_SPEC: "not sleep" TEST_PY_ID: "--id qemu" <<: *buildman_and_testpy_dfn + +.lab_template: &lab_dfn + stage: sjg-lab + rules: +- when: always + tags: [ 'lab' ] + script: +- if [[ -z "${SJG_LAB}" ]]; then +exit 0; + fi +# Environment: +# SRC - source tree +# OUT - output directory for builds +- export SRC="$(pwd)" +- export OUT="${SRC}/build/${BOARD}" +- export PATH=$PATH:~/bin +- export PATH=$PATH:/vid/software/devel/ubtest/u-boot-test-hooks/bin + +# Load it on the device +- ret=0 +- echo "role ${ROLE}" +- export strategy="-s uboot -e off" +# export verbose="-v" +- ${SRC}/test/py/test.py --role ${ROLE} --build-dir "${OUT}" +--capture=tee-sys -k "not bootstd"|| ret=$? +- U_BOOT_BOARD_IDENTITY="${ROLE}" u-boot-test-release || true +- if [[ $ret -ne 0 ]]; then +exit $ret; + fi + artifacts: +when: always +paths: + - "build/${BOARD}/test-log.html" + - "build/${BOARD}/multiplexed_log.css" +expire_in: 1 week + +rpi3: + variables: +ROLE: rpi3 + <<: *lab_dfn + +opi_pc: + variables: +ROLE: opi_pc + <<: *lab_dfn + +pcduino3_nano: + variables: +ROLE: pcduino3_nano + <<: *lab_dfn + +samus: + variables: +ROLE: samus + <<: *lab_dfn + +link: + variables: +ROLE: link + <<: *lab_dfn + +jerry: + variables: +ROLE: jerry + <<: *lab_dfn + +minnowmax: + variables: +ROLE: minnowmax + <<: *lab_dfn + +opi_pc2: + variables: +ROLE: opi_pc2 + <<: *lab_dfn + +bpi: + variables: +ROLE: bpi + <<: *lab_dfn + +rpi2: + variables: +ROLE: rpi2 + <<: *lab_dfn + +bob: + variables: +ROLE: bob + <<: *lab_dfn + +ff3399: + variables: +ROLE: ff3399 + <<: *lab_dfn + +coral: + variables: +ROLE: coral + <<: *lab_dfn + +rpi3z: + variables: +ROLE: rpi3z + <<: *lab_dfn + +bbb: + variables: +ROLE: bbb + <<: *lab_dfn + +kevin: + variables: +ROLE: kevin + <<: *lab_dfn + +pine64: + variables: +ROLE: pine64 + <<: *lab_dfn + +c4: + variables: +ROLE: c4 + <<: *lab_dfn + +rpi4: + variables: +ROLE: rpi4 + <<: *lab_dfn + +rpi0: + variables: +ROLE: rpi0 + <<: *lab_dfn + +snow: + variables: +ROLE: snow + <<: *lab_dfn + +pcduino3: + variables: +ROLE: pcduino3 + <<: *lab_dfn + +nyan-big: + variables: +ROLE: nyan-big + <<: *lab_dfn -- 2.34.1
[PATCH 41/42] test: Add a section for closing the connection
This can take a while and involve multiple steps (e.g. turning the board back off). Add a section for it and show the output. Signed-off-by: Simon Glass --- test/py/u_boot_console_base.py | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index 1b00821c5e9..95096fc123c 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -157,7 +157,10 @@ class ConsoleBase(object): """ if self.p: -self.p.close() +self.log.start_section('Stopping U-Boot') +close_type = self.p.close() +self.log.info(f'Close type: {close_type}') +self.log.end_section('Stopping U-Boot') self.logstream.close() def set_lab_mode(self): -- 2.34.1
[PATCH 40/42] test: Try to shut down the lab console gracefully
Send the Labgrid quit characters to ask it to exit gracefully. This typically allows it to power off the board being used. If that doesn't work, try the less graceful approach. Signed-off-by: Simon Glass --- test/py/u_boot_spawn.py | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index 261bdf62e15..000e2a364e4 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -16,6 +16,9 @@ import termios import time import traceback +# Character to send (twice) to exit the terminal +EXIT_CHAR = 0x1d# FS (Ctrl + ]) + class Timeout(Exception): """An exception sub-class that indicates that a timeout occurred.""" @@ -301,15 +304,25 @@ class Spawn: None. Returns: -Nothing. +str: Type of closure completed """ +self.send(chr(EXIT_CHAR) * 2) +# Wait about 10 seconds for Labgrid to close and power off the board +for _ in range(100): +if not self.isalive(): +return 'normal' +time.sleep(0.1) + +# That didn't work, so try closing the PTY os.close(self.fd) for _ in range(100): if not self.isalive(): -break +return 'break' time.sleep(0.1) +return 'timeout' + def get_expect_output(self): """Return the output read by expect() -- 2.34.1
[PATCH 39/42] test: Avoid double echo when starting up
There is a very annoying bug at present where the terminal echos part of the first command sent to the board. This happens because the terminal is still set to echo for a period until Labgrid starts up and can change this. Fix this by disabling echo (and other terminal features) as soon as the spawn happens. Signed-off-by: Simon Glass --- test/py/u_boot_spawn.py | 10 ++ 1 file changed, 10 insertions(+) diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index 62eb4118731..261bdf62e15 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -12,6 +12,7 @@ import pytest import signal import select import sys +import termios import time import traceback @@ -118,10 +119,19 @@ class Spawn: os._exit(255) try: +new = termios.tcgetattr(self.fd) +old = new +new[3] = new[3] & ~(termios.ICANON | termios.ISIG) +new[3] = new[3] & ~termios.ECHO +new[6][termios.VMIN] = 0 +new[6][termios.VTIME] = 0 +termios.tcsetattr(self.fd, termios.TCSANOW, new) + self.poll = select.poll() self.poll.register(self.fd, select.POLLIN | select.POLLPRI | select.POLLERR | select.POLLHUP | select.POLLNVAL) except: +termios.tcsetattr(self.fd, termios.TCSANOW, old) self.close() raise -- 2.34.1
[PATCH 38/42] test: Fix mulptiplex_log typo
Fix a typo in a comment. Signed-off-by: Simon Glass --- test/py/u_boot_console_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index eda48cd35f7..1b00821c5e9 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -97,7 +97,7 @@ class ConsoleBase(object): Can only usefully be called by sub-classes. Args: -log: A mulptiplex_log.Logfile object, to which the U-Boot output +log: A multiplexed_log.Logfile object, to which the U-Boot output will be logged. config: A configuration data structure, as built by conftest.py. max_fifo_fill: The maximum number of characters to send to U-Boot -- 2.34.1
[PATCH 37/42] test: Improve handling of sending commands
We expect commands to be echoed and this should happen quite quickly, since U-Boot is sitting at the prompt waiting for a command. Reduce the timeout for this situation. Try to produce a more useful error message when something goes wrong. Also handle the case where the connection has gone away since the last command was issued. Signed-off-by: Simon Glass --- test/py/u_boot_console_base.py | 35 -- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index cfde57649f1..eda48cd35f7 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -31,6 +31,7 @@ PAT_RE = 1 # Timeout before expecting the console to be ready (in milliseconds) TIMEOUT_MS = 3 # Standard timeout +TIMEOUT_CMD_MS = 1 # Command-echo timeout # Timeout for board preparation in lab mode. This needs to be enough to build # U-Boot, write it to the board and then boot the board. Since this process is @@ -274,22 +275,28 @@ class ConsoleBase(object): try: self.at_prompt = False +if not self.p: +raise BootFail( +f"Lab failure: Connection lost when sending command '{cmd}'") + if send_nl: cmd += '\n' -while cmd: -# Limit max outstanding data, so UART FIFOs don't overflow -chunk = cmd[:self.max_fifo_fill] -cmd = cmd[self.max_fifo_fill:] -self.p.send(chunk) -if not wait_for_echo: -continue -chunk = re.escape(chunk) -chunk = chunk.replace('\\\n', '[\r\n]') -m = self.p.expect([chunk] + self.bad_patterns) -if m != 0: -self.at_prompt = False -raise BootFail('Bad pattern found on console: ' + -self.bad_pattern_ids[m - 1]) +rem = cmd # Remaining to be sent +with self.temporary_timeout(TIMEOUT_CMD_MS): +while rem: +# Limit max outstanding data, so UART FIFOs don't overflow +chunk = rem[:self.max_fifo_fill] +rem = rem[self.max_fifo_fill:] +self.p.send(chunk) +if not wait_for_echo: +continue +chunk = re.escape(chunk) +chunk = chunk.replace('\\\n', '[\r\n]') +m = self.p.expect([chunk] + self.bad_patterns) +if m != 0: +self.at_prompt = False +raise BootFail(f"Failed to get echo on console (cmd '{cmd}':rem '{rem}'): " + +self.bad_pattern_ids[m - 1]) if not wait_for_prompt: return if wait_for_reboot: -- 2.34.1
[PATCH 36/42] test: Introduce lab mode
There is quite a bit of code in pytest to try to start up U-Boot on a board, with timeouts, expects, etc. This is tedious to maintain and is peripheral to the test system's purpose. It seems better to put this logic in the lab itself, where is can provide such support. With Labgrid we can use the UbootStrategy class to get the board into a useful state, however it needs to do it. Then it can report to pytest by writing a suitable string along with the U-Boot version it detected. Add support for detecting 'lab mode' and simply assume that all is well in that case. Collect the version string when Labgrid says it is ready. Signed-off-by: Simon Glass --- test/py/u_boot_console_base.py | 68 ++ 1 file changed, 53 insertions(+), 15 deletions(-) diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index dbf54eadd0f..cfde57649f1 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -23,13 +23,21 @@ pattern_stop_autoboot_prompt = re.compile('Hit any key to stop autoboot: ') pattern_unknown_command = re.compile('Unknown command \'.*\' - try \'help\'') pattern_error_notification = re.compile('## Error: ') pattern_error_please_reset = re.compile('### ERROR ### Please RESET the board ###') -pattern_ready_prompt = re.compile('U-Boot is ready') +pattern_ready_prompt = re.compile('{lab ready in (.*)s: (.*)}') +pattern_lab_mode = re.compile('{lab mode.*}') PAT_ID = 0 PAT_RE = 1 # Timeout before expecting the console to be ready (in milliseconds) -TIMEOUT_MS = 3 +TIMEOUT_MS = 3 # Standard timeout + +# Timeout for board preparation in lab mode. This needs to be enough to build +# U-Boot, write it to the board and then boot the board. Since this process is +# under the control of another program (e.g. Labgrid), it will failure sooner +# if something goes way. So use a very long timeout here to cover all possible +# situations. +TIMEOUT_PREPARE_MS = 3 * 60 * 1000 bad_pattern_defs = ( ('spl_signon', pattern_u_boot_spl_signon), @@ -117,6 +125,7 @@ class ConsoleBase(object): self.at_prompt = False self.at_prompt_logevt = None +self.lab_mode = False def get_spawn(self): # This is not called, ssubclass must define this. @@ -150,40 +159,69 @@ class ConsoleBase(object): self.p.close() self.logstream.close() +def set_lab_mode(self): +"""Select lab mode + +This tells us that we will get a 'lab ready' message when the board is +ready for use. We don't need to look for signon messages. +""" +self.log.info(f'test.py: Lab mode is active') +self.p.timeout = TIMEOUT_PREPARE_MS +self.lab_mode = True + def wait_for_boot_prompt(self, loop_num = 1): """Wait for the boot up until command prompt. This is for internal use only. """ try: +self.log.info('Waiting for U-Boot to be ready') bcfg = self.config.buildconfig config_spl_serial = bcfg.get('config_spl_serial', 'n') == 'y' env_spl_skipped = self.config.env.get('env__spl_skipped', False) env_spl_banner_times = self.config.env.get('env__spl_banner_times', 1) -while loop_num > 0: +while not self.lab_mode and loop_num > 0: loop_num -= 1 while config_spl_serial and not env_spl_skipped and env_spl_banner_times > 0: -m = self.p.expect([pattern_u_boot_spl_signon] + - self.bad_patterns) -if m != 0: +m = self.p.expect([pattern_u_boot_spl_signon, + pattern_lab_mode] + self.bad_patterns) +if m == 1: +self.set_lab_mode() +break +elif m != 0: raise BootFail('Bad pattern found on SPL console: ' + -self.bad_pattern_ids[m - 1]) + self.bad_pattern_ids[m - 1]) env_spl_banner_times -= 1 -m = self.p.expect([pattern_u_boot_main_signon] + self.bad_patterns) -if m != 0: -raise BootFail('Bad pattern found on console: ' + -self.bad_pattern_ids[m - 1]) -self.u_boot_version_string = self.p.after +if not self.lab_mode: +m = self.p.expect([pattern_u_boot_main_signon, + pattern_lab_mode] + self.bad_patterns) +if m == 1: +self.set_lab_mode() +elif m != 0: +raise BootFail('Bad pattern found on console: ' + + self.bad_pattern_ids[m - 1]) +if not self.lab_mode: +
[PATCH 35/42] test: Tidy up remaining exceptions
Use the new handle_exception() function from ConsoleBase also. Signed-off-by: Simon Glass --- test/py/u_boot_console_base.py | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index b656a1f38cd..dbf54eadd0f 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -14,7 +14,7 @@ import pytest import re import sys import u_boot_spawn -from u_boot_spawn import BootFail, Timeout, Unexpected +from u_boot_spawn import BootFail, Timeout, Unexpected, handle_exception # Regexes for text we expect U-Boot to send to the console. pattern_u_boot_spl_signon = re.compile('(U-Boot SPL \\d{4}\\.\\d{2}[^\r\n]*\\))') @@ -268,12 +268,12 @@ class ConsoleBase(object): # indentation. return self.p.before.strip('\r\n') except Timeout as exc: -self.log.error(str(exc)) -self.cleanup_spawn() +handle_exception(self.config, self, self.log, exc, 'Lab failure', + True) raise -except BootFail as ex: -self.log.error(str(ex)) -self.cleanup_spawn() +except BootFail as exc: +handle_exception(self.config, self, self.log, exc, 'Boot fail', + True, self.get_spawn_output()) raise finally: self.log.timestamp() -- 2.34.1
[PATCH 34/42] test: Detect dead connections
When the connection to a board dies, assume it is dead forever until some user action is taken. Skip all remaining tests. This avoids CI runs taking an hour, with hundreds of 30-second timeouts all to no avail. Signed-off-by: Simon Glass --- test/py/conftest.py | 19 +-- test/py/u_boot_spawn.py | 38 ++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/test/py/conftest.py b/test/py/conftest.py index 5de8d7b0e23..42af1abd72e 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -25,6 +25,7 @@ import re from _pytest.runner import runtestprotocol import subprocess import sys +from u_boot_spawn import BootFail, Timeout, Unexpected, handle_exception # Globals: The HTML log file, and the connection to the U-Boot console. log = None @@ -280,6 +281,7 @@ def pytest_configure(config): ubconfig.gdbserver = gdbserver ubconfig.no_prompt_wait = config.getoption('no_prompt_wait') ubconfig.dtb = build_dir + '/arch/sandbox/dts/test.dtb' +ubconfig.connection_ok = True env_vars = ( 'board_type', @@ -446,8 +448,21 @@ def u_boot_console(request): Returns: The fixture value. """ - -console.ensure_spawned() +if not ubconfig.connection_ok: +pytest.skip('Cannot get target connection') +return None +try: +console.ensure_spawned() +except OSError as err: +handle_exception(ubconfig, console, log, err, 'Lab failure', True) +except Timeout as err: +handle_exception(ubconfig, console, log, err, 'Lab timeout', True) +except BootFail as err: +handle_exception(ubconfig, console, log, err, 'Boot fail', True, + console.get_spawn_output()) +except Unexpected: +handle_exception(ubconfig, console, log, err, 'Unexpected test output', + False) return console anchors = {} diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index 57ea845ad4c..62eb4118731 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -8,6 +8,7 @@ Logic to spawn a sub-process and interact with its stdio. import os import re import pty +import pytest import signal import select import sys @@ -28,6 +29,43 @@ class BootFail(Exception): class Unexpected(Exception): """An exception sub-class that indicates that unexpected test was seen.""" + +def handle_exception(ubconfig, console, log, err, name, fatal, output=''): +"""Handle an exception from the console + +Exceptions can occur when there is unexpected output or due to the board +crashing or hanging. Some exceptions are likely fatal, where retrying will +just chew up time to no available. In those cases it is best to cause +further tests be skipped. + +Args: +ubconfig (ArbitraryAttributeContainer): ubconfig object +log (Logfile): Place to log errors +console (ConsoleBase): Console to clean up, if fatal +err (Exception): Exception which was thrown +name (str): Name of problem, to log +fatal (bool): True to abort all tests +output (str): Extra output to report on boot failure. This can show the + target's console output as it tried to boot +""" +msg = f'{name}: ' +if fatal: +msg += 'Marking connection bad - no other tests will run' +else: +msg += 'Assuming that lab is healthy' +print(msg) +log.error(msg) +log.error(f'Error: {err}') + +if output: +msg += f'; output {output}' + +if fatal: +ubconfig.connection_ok = False +console.cleanup_spawn() +pytest.exit(msg) + + class Spawn: """Represents the stdio of a freshly created sub-process. Commands may be sent to the process, and responses waited for. -- 2.34.1
[PATCH 33/42] test: Separate out the exception handling
The tests currently catch a very board Exception in each case. This is thrown even in the event of a coding error. We want to handle exceptions differently depending on their severity, so that we can avoid hour-long delays waiting for a board that is clearly broken. As a first step, create some new exception types, separating out those which are simply an unexpected result from executed a command, from those which indicate some kind of hardware failure. Signed-off-by: Simon Glass --- test/py/u_boot_console_base.py | 26 ++ test/py/u_boot_spawn.py| 11 +++ 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index a61eec31148..b656a1f38cd 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -14,6 +14,7 @@ import pytest import re import sys import u_boot_spawn +from u_boot_spawn import BootFail, Timeout, Unexpected # Regexes for text we expect U-Boot to send to the console. pattern_u_boot_spl_signon = re.compile('(U-Boot SPL \\d{4}\\.\\d{2}[^\r\n]*\\))') @@ -164,13 +165,13 @@ class ConsoleBase(object): m = self.p.expect([pattern_u_boot_spl_signon] + self.bad_patterns) if m != 0: -raise Exception('Bad pattern found on SPL console: ' + +raise BootFail('Bad pattern found on SPL console: ' + self.bad_pattern_ids[m - 1]) env_spl_banner_times -= 1 m = self.p.expect([pattern_u_boot_main_signon] + self.bad_patterns) if m != 0: -raise Exception('Bad pattern found on console: ' + +raise BootFail('Bad pattern found on console: ' + self.bad_pattern_ids[m - 1]) self.u_boot_version_string = self.p.after while True: @@ -181,13 +182,9 @@ class ConsoleBase(object): if m == 2: self.p.send(' ') continue -raise Exception('Bad pattern found on console: ' + +raise BootFail('Bad pattern found on console: ' + self.bad_pattern_ids[m - 3]) -except Exception as ex: -self.log.error(str(ex)) -self.cleanup_spawn() -raise finally: self.log.timestamp() @@ -253,7 +250,7 @@ class ConsoleBase(object): m = self.p.expect([chunk] + self.bad_patterns) if m != 0: self.at_prompt = False -raise Exception('Bad pattern found on console: ' + +raise BootFail('Bad pattern found on console: ' + self.bad_pattern_ids[m - 1]) if not wait_for_prompt: return @@ -263,14 +260,18 @@ class ConsoleBase(object): m = self.p.expect([self.prompt_compiled] + self.bad_patterns) if m != 0: self.at_prompt = False -raise Exception('Bad pattern found on console: ' + +raise BootFail('Missing prompt on console: ' + self.bad_pattern_ids[m - 1]) self.at_prompt = True self.at_prompt_logevt = self.logstream.logfile.cur_evt # Only strip \r\n; space/TAB might be significant if testing # indentation. return self.p.before.strip('\r\n') -except Exception as ex: +except Timeout as exc: +self.log.error(str(exc)) +self.cleanup_spawn() +raise +except BootFail as ex: self.log.error(str(ex)) self.cleanup_spawn() raise @@ -329,8 +330,9 @@ class ConsoleBase(object): text = re.escape(text) m = self.p.expect([text] + self.bad_patterns) if m != 0: -raise Exception('Bad pattern found on console: ' + -self.bad_pattern_ids[m - 1]) +raise Unexpected( +"Unexpected pattern found on console (exp '{text}': " + +self.bad_pattern_ids[m - 1]) def drain_console(self): """Read from and log the U-Boot console for a short time. diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index cb0d8f702ba..57ea845ad4c 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -17,6 +17,17 @@ import traceback class Timeout(Exception): """An exception sub-class that indicates that a timeout occurred.""" +class BootFail(Exception): +"""An exception sub-class that indicates that a boot failure occurred. + +This is used when a bad pattern is seen when waiting for the boot prompt. +It is regarded as fatal, to avoid trying to boot the
[PATCH 32/42] test: Move the receive code into a function
There is quite a bit of code to deal with receiving data from the target so move it into its own receive() function. Signed-off-by: Simon Glass --- test/py/u_boot_spawn.py | 39 +++ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index 81a09a9d639..cb0d8f702ba 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -139,6 +139,32 @@ class Spawn: os.write(self.fd, data.encode(errors='replace')) +def receive(self, num_bytes): +"""Receive data from the sub-process's stdin. + +Args: +num_bytes (int): Maximum number of bytes to read + +Returns: +str: The data received + +Raises: +ValueError if U-Boot died +""" +try: +c = os.read(self.fd, num_bytes).decode(errors='replace') +except OSError as err: +# With sandbox, try to detect when U-Boot exits when it +# shouldn't and explain why. This is much more friendly than +# just dying with an I/O error +if self.decode_signal and err.errno == 5: # I/O error +alive, _, info = self.checkalive() +if alive: +raise err +raise ValueError('U-Boot exited with %s' % info) +raise +return c + def expect(self, patterns): """Wait for the sub-process to emit specific data. @@ -195,18 +221,7 @@ class Spawn: events = self.poll.poll(poll_maxwait) if not events: raise Timeout() -try: -c = os.read(self.fd, 1024).decode(errors='replace') -except OSError as err: -# With sandbox, try to detect when U-Boot exits when it -# shouldn't and explain why. This is much more friendly than -# just dying with an I/O error -if self.decode_signal and err.errno == 5: # I/O error -alive, _, info = self.checkalive() -if alive: -raise err -raise ValueError('U-Boot exited with %s' % info) -raise +c = self.receive(1024) if self.logfile_read: self.logfile_read.write(c) self.buf += c -- 2.34.1
[PATCH 31/42] test: Introduce the concept of a role
In Labgrid there is the concept of a 'role', which is similar to the U-Boot board ID in U-Boot's pytest subsystem. The role indicates both the target and information about the U-Boot build to use. It can also provide any amount of other configuration. The information is obtained using the 'labgrid-client query' operation. Make use of this in tests, so that only the role is required in gitlab and other situations. The board type and other things can be queried as needed. Use a new 'u-boot-test-getrole' script to obtain the requested information. With this it is possible to run lab tests in gitlab with just a single 'ROLE' variable for each board. Signed-off-by: Simon Glass --- test/py/conftest.py | 31 +++ 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/test/py/conftest.py b/test/py/conftest.py index 6547c6922c6..5de8d7b0e23 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -23,6 +23,7 @@ from pathlib import Path import pytest import re from _pytest.runner import runtestprotocol +import subprocess import sys # Globals: The HTML log file, and the connection to the U-Boot console. @@ -79,6 +80,7 @@ def pytest_addoption(parser): parser.addoption('--gdbserver', default=None, help='Run sandbox under gdbserver. The argument is the channel '+ 'over which gdbserver should communicate, e.g. localhost:1234') +parser.addoption('--role', help='U-Boot board role (for Labgrid)') parser.addoption('--no-prompt-wait', default=False, action='store_true', help="Assume that U-Boot is ready and don't wait for a prompt") @@ -130,12 +132,33 @@ def get_details(config): str: Build directory str: Source directory """ -board_type = config.getoption('board_type') -board_identity = config.getoption('board_identity') +role = config.getoption('role') build_dir = config.getoption('build_dir') +if role: +board_identity = role +cmd = ['u-boot-test-getrole', role, '--configure'] +env = os.environ.copy() +if build_dir: +env['U_BOOT_BUILD_DIR'] = build_dir +proc = subprocess.run(cmd, capture_output=True, encoding='utf-8', + env=env) +if proc.returncode: +raise ValueError(proc.stderr) +print('conftest: lab:', proc.stdout) +vals = {} +for line in proc.stdout.splitlines(): +item, value = line.split(' ', maxsplit=1) +k = item.split(':')[-1] +vals[k] = value +print('conftest: lab info:', vals) +board_type, default_build_dir, source_dir = (vals['board'], +vals['build_dir'], vals['source_dir']) +else: +board_type = config.getoption('board_type') +board_identity = config.getoption('board_identity') -source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR)) -default_build_dir = source_dir + '/build-' + board_type +source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR)) +default_build_dir = source_dir + '/build-' + board_type if not build_dir: build_dir = default_build_dir -- 2.34.1
[PATCH 30/42] test: Create a common function to get the config
The settings are decoded in two places. Combine them into a new function, before (in a future patch) expanding the number of items. Signed-off-by: Simon Glass --- test/py/conftest.py | 41 - 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/test/py/conftest.py b/test/py/conftest.py index ca66b9d9e61..6547c6922c6 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -117,14 +117,36 @@ def run_build(config, source_dir, build_dir, board_type, log): runner.close() log.status_pass('OK') -def pytest_xdist_setupnodes(config, specs): -"""Clear out any 'done' file from a previous build""" -global build_done_file -build_dir = config.getoption('build_dir') +def get_details(config): +"""Obtain salient details about the board and directories to use + +Args: +config (pytest.Config): pytest configuration + +Returns: +tuple: +str: Board type (U-Boot build name) +str: Identity for the lab board +str: Build directory +str: Source directory +""" board_type = config.getoption('board_type') +board_identity = config.getoption('board_identity') +build_dir = config.getoption('build_dir') + source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR)) +default_build_dir = source_dir + '/build-' + board_type if not build_dir: -build_dir = source_dir + '/build-' + board_type +build_dir = default_build_dir + +return board_type, board_identity, build_dir, source_dir + +def pytest_xdist_setupnodes(config, specs): +"""Clear out any 'done' file from a previous build""" +global build_done_file + +build_dir = get_details(config)[2] + build_done_file = Path(build_dir) / 'build.done' if build_done_file.exists(): os.remove(build_done_file) @@ -163,17 +185,10 @@ def pytest_configure(config): global console global ubconfig -source_dir = os.path.dirname(os.path.dirname(TEST_PY_DIR)) +board_type, board_identity, build_dir, source_dir = get_details(config) -board_type = config.getoption('board_type') board_type_filename = board_type.replace('-', '_') - -board_identity = config.getoption('board_identity') board_identity_filename = board_identity.replace('-', '_') - -build_dir = config.getoption('build_dir') -if not build_dir: -build_dir = source_dir + '/build-' + board_type mkdir_p(build_dir) result_dir = config.getoption('result_dir') -- 2.34.1
[PATCH 29/42] test: Check help output
The current test doesn't check anything about the output. If a bug results in junk before the output, this is not currently detected. Add a check for the first line being the one expected. Signed-off-by: Simon Glass --- test/py/tests/test_help.py | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/py/tests/test_help.py b/test/py/tests/test_help.py index 153133cf28f..2325ff69229 100644 --- a/test/py/tests/test_help.py +++ b/test/py/tests/test_help.py @@ -7,7 +7,11 @@ import pytest def test_help(u_boot_console): """Test that the "help" command can be executed.""" -u_boot_console.run_command('help') +lines = u_boot_console.run_command('help') +if u_boot_console.config.buildconfig.get('config_cmd_2048', 'n') == 'y': +assert lines.splitlines()[0] == "2048 - The 2048 game" +else: +assert lines.splitlines()[0] == "? - alias for 'help'" @pytest.mark.boardspec('sandbox') def test_help_no_devicetree(u_boot_console): -- 2.34.1
[PATCH 28/42] test: dm: Show failing driver name
When a driver is not registered properly it is not clear which one it is. Adjust test_dm_compat() to show this. Signed-off-by: Simon Glass --- test/py/tests/test_dm.py | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/py/tests/test_dm.py b/test/py/tests/test_dm.py index 68d4ea12235..be94971e455 100644 --- a/test/py/tests/test_dm.py +++ b/test/py/tests/test_dm.py @@ -13,8 +13,11 @@ def test_dm_compat(u_boot_console): for line in response[:-1].split('\n')[2:]) response = u_boot_console.run_command('dm compat') +bad_drivers = set() for driver in drivers: -assert driver in response +if not driver in response: +bad_drivers.add(driver) +assert not bad_drivers # check sorting - output looks something like this: # testacpi 0 [ ] testacpi_drv |-- acpi-test -- 2.34.1
[PATCH 27/42] test: Avoid failing skipped tests
When a test returns -EAGAIN this should not be considered a failure. Fix what seems to be a problem case, where the pytests see a failure when a test has merely been skipped. Signed-off-by: Simon Glass --- test/test-main.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/test/test-main.c b/test/test-main.c index 3fa6f6e32ec..cda1a186390 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -448,7 +448,7 @@ static int ut_run_test(struct unit_test_state *uts, struct unit_test *test, static int ut_run_test_live_flat(struct unit_test_state *uts, struct unit_test *test) { - int runs; + int runs, ret; if ((test->flags & UT_TESTF_OTHER_FDT) && !IS_ENABLED(CONFIG_SANDBOX)) return skip_test(uts); @@ -458,8 +458,11 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, if (CONFIG_IS_ENABLED(OF_LIVE)) { if (!(test->flags & UT_TESTF_FLAT_TREE)) { uts->of_live = true; - ut_assertok(ut_run_test(uts, test, test->name)); - runs++; + ret = ut_run_test(uts, test, test->name); + if (ret != -EAGAIN) { + ut_assertok(ret); + runs++; + } } } @@ -483,8 +486,11 @@ static int ut_run_test_live_flat(struct unit_test_state *uts, (!runs || ut_test_run_on_flattree(test)) && !(gd->flags & GD_FLG_FDT_CHANGED)) { uts->of_live = false; - ut_assertok(ut_run_test(uts, test, test->name)); - runs++; + ret = ut_run_test(uts, test, test->name); + if (ret != -EAGAIN) { + ut_assertok(ret); + runs++; + } } return 0; -- 2.34.1
[PATCH 26/42] test: Decode exceptions only with sandbox
When a real board fails we don't want to decode the exception. Reserve that behaviour for sandbox. Also avoid raising a new exception on failure - just re-raise the existing one. Signed-off-by: Simon Glass --- test/py/u_boot_console_sandbox.py | 2 +- test/py/u_boot_spawn.py | 10 ++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/test/py/u_boot_console_sandbox.py b/test/py/u_boot_console_sandbox.py index 27c6db8d719..7bc44c78b8b 100644 --- a/test/py/u_boot_console_sandbox.py +++ b/test/py/u_boot_console_sandbox.py @@ -58,7 +58,7 @@ class ConsoleSandbox(ConsoleBase): if self.use_dtb: cmd += ['-d', self.config.dtb] cmd += self.sandbox_flags -return Spawn(cmd, cwd=self.config.source_dir) +return Spawn(cmd, cwd=self.config.source_dir, decode_signal=True) def restart_uboot_with_flags(self, flags, expect_reset=False, use_dtb=True): """Run U-Boot with the given command-line flags diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index 7421da49aa9..81a09a9d639 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -25,18 +25,20 @@ class Spawn: output: accumulated output from expect() """ -def __init__(self, args, cwd=None): +def __init__(self, args, cwd=None, decode_signal=False): """Spawn (fork/exec) the sub-process. Args: args: array of processs arguments. argv[0] is the command to execute. cwd: the directory to run the process in, or None for no change. +decode_signal (bool): True to indicate the exception number when +something goes wrong Returns: Nothing. """ - +self.decode_signal = decode_signal self.waited = False self.exit_code = 0 self.exit_info = '' @@ -199,12 +201,12 @@ class Spawn: # With sandbox, try to detect when U-Boot exits when it # shouldn't and explain why. This is much more friendly than # just dying with an I/O error -if err.errno == 5: # Input/output error +if self.decode_signal and err.errno == 5: # I/O error alive, _, info = self.checkalive() if alive: raise err raise ValueError('U-Boot exited with %s' % info) -raise err +raise if self.logfile_read: self.logfile_read.write(c) self.buf += c -- 2.34.1
[PATCH 23/42] test: Release board after tests complete
When a board is finished with, the lab may want to power it off, or perform some other function. Add a new script which is called when tests are complete. Signed-off-by: Simon Glass --- test/py/u_boot_console_exec_attach.py | 10 ++ 1 file changed, 10 insertions(+) diff --git a/test/py/u_boot_console_exec_attach.py b/test/py/u_boot_console_exec_attach.py index 8dd8cc1230c..5f4916b7da2 100644 --- a/test/py/u_boot_console_exec_attach.py +++ b/test/py/u_boot_console_exec_attach.py @@ -70,3 +70,13 @@ class ConsoleExecAttach(ConsoleBase): raise return s + +def close(self): +super().close() + +self.log.action('Releasing board') +args = [self.config.board_type, self.config.board_identity] +cmd = ['u-boot-test-release'] + args +runner = self.log.get_runner(cmd[0], sys.stdout) +runner.run(cmd) +runner.close() -- 2.34.1
[PATCH 25/42] test: Allow connecting to a running board
Sometimes we know that the board is already running the right software, so provide an option to allow running of tests directly, without first resetting the board. This saves time when re-running a test where only the Python code is changing. Signed-off-by: Simon Glass --- test/py/conftest.py | 3 +++ test/py/u_boot_console_base.py| 14 ++ test/py/u_boot_console_exec_attach.py | 21 - 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/test/py/conftest.py b/test/py/conftest.py index fc9dd3a83f8..ca66b9d9e61 100644 --- a/test/py/conftest.py +++ b/test/py/conftest.py @@ -79,6 +79,8 @@ def pytest_addoption(parser): parser.addoption('--gdbserver', default=None, help='Run sandbox under gdbserver. The argument is the channel '+ 'over which gdbserver should communicate, e.g. localhost:1234') +parser.addoption('--no-prompt-wait', default=False, action='store_true', +help="Assume that U-Boot is ready and don't wait for a prompt") def run_build(config, source_dir, build_dir, board_type, log): """run_build: Build U-Boot @@ -238,6 +240,7 @@ def pytest_configure(config): ubconfig.board_type = board_type ubconfig.board_identity = board_identity ubconfig.gdbserver = gdbserver +ubconfig.no_prompt_wait = config.getoption('no_prompt_wait') ubconfig.dtb = build_dir + '/arch/sandbox/dts/test.dtb' env_vars = ( diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index e4f86f6af5b..a61eec31148 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -413,11 +413,17 @@ class ConsoleBase(object): if not self.config.gdbserver: self.p.timeout = TIMEOUT_MS self.p.logfile_read = self.logstream -if expect_reset: -loop_num = 2 +if self.config.no_prompt_wait: +# Send an empty command to set up the 'expect' logic. This has +# the side effect of ensuring that there was no partial command +# line entered +self.run_command(' ') else: -loop_num = 1 -self.wait_for_boot_prompt(loop_num = loop_num) +if expect_reset: +loop_num = 2 +else: +loop_num = 1 +self.wait_for_boot_prompt(loop_num = loop_num) self.at_prompt = True self.at_prompt_logevt = self.logstream.logfile.cur_evt except Exception as ex: diff --git a/test/py/u_boot_console_exec_attach.py b/test/py/u_boot_console_exec_attach.py index 5f4916b7da2..42fc15197b9 100644 --- a/test/py/u_boot_console_exec_attach.py +++ b/test/py/u_boot_console_exec_attach.py @@ -59,15 +59,18 @@ class ConsoleExecAttach(ConsoleBase): args = [self.config.board_type, self.config.board_identity] s = Spawn(['u-boot-test-console'] + args) -try: -self.log.action('Resetting board') -cmd = ['u-boot-test-reset'] + args -runner = self.log.get_runner(cmd[0], sys.stdout) -runner.run(cmd) -runner.close() -except: -s.close() -raise +if self.config.no_prompt_wait: +self.log.action('Connecting to board without reset') +else: +try: +self.log.action('Resetting board') +cmd = ['u-boot-test-reset'] + args +runner = self.log.get_runner(cmd[0], sys.stdout) +runner.run(cmd) +runner.close() +except: +s.close() +raise return s -- 2.34.1
[PATCH 24/42] log: Allow tests to pass with CONFIG_LOGF_FUNC_PAD set
This setting pads out the function names. Adjust the test to handle this, since some boards use it. Signed-off-by: Simon Glass --- test/py/tests/test_log.py | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/py/tests/test_log.py b/test/py/tests/test_log.py index 140dcb9aa2b..79808674bbe 100644 --- a/test/py/tests/test_log.py +++ b/test/py/tests/test_log.py @@ -27,13 +27,16 @@ def test_log_format(u_boot_console): cons = u_boot_console with cons.log.section('format'): -run_with_format('all', 'NOTICE.arch,file.c:123-func() msg') +pad = int(u_boot_console.config.buildconfig.get('config_logf_func_pad')) +padding = ' ' * (pad - len('func')) + +run_with_format('all', f'NOTICE.arch,file.c:123-{padding}func() msg') output = cons.run_command('log format') assert output == 'Log format: clFLfm' -run_with_format('fm', 'func() msg') -run_with_format('clfm', 'NOTICE.arch,func() msg') -run_with_format('FLfm', 'file.c:123-func() msg') +run_with_format('fm', f'{padding}func() msg') +run_with_format('clfm', f'NOTICE.arch,{padding}func() msg') +run_with_format('FLfm', f'file.c:123-{padding}func() msg') run_with_format('lm', 'NOTICE. msg') run_with_format('m', 'msg') -- 2.34.1
[PATCH 22/42] test: Pass stderr to stdout
Some tests may output things to stderr. Ensure that this output is not dropped, by redirecting it to stdout Signed-off-by: Simon Glass --- test/py/u_boot_spawn.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py index 7c48d96210e..7421da49aa9 100644 --- a/test/py/u_boot_spawn.py +++ b/test/py/u_boot_spawn.py @@ -10,6 +10,7 @@ import re import pty import signal import select +import sys import time import traceback @@ -57,6 +58,7 @@ class Spawn: signal.signal(signal.SIGHUP, signal.SIG_DFL) if cwd: os.chdir(cwd) +sys.stderr = sys.stdout os.execvp(args[0], args) except: print('CHILD EXECEPTION:') -- 2.34.1
[PATCH 21/42] test: Use a constant for the test timeout
Declare a constant rather than open-coding the same value twice. Signed-off-by: Simon Glass --- test/py/u_boot_console_base.py | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index e230caf37e1..e4f86f6af5b 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -27,6 +27,9 @@ pattern_ready_prompt = re.compile('U-Boot is ready') PAT_ID = 0 PAT_RE = 1 +# Timeout before expecting the console to be ready (in milliseconds) +TIMEOUT_MS = 3 + bad_pattern_defs = ( ('spl_signon', pattern_u_boot_spl_signon), ('main_signon', pattern_u_boot_main_signon), @@ -397,7 +400,7 @@ class ConsoleBase(object): # Reset the console timeout value as some tests may change # its default value during the execution if not self.config.gdbserver: -self.p.timeout = 3 +self.p.timeout = TIMEOUT_MS return try: self.log.start_section('Starting U-Boot') @@ -408,7 +411,7 @@ class ConsoleBase(object): # future, possibly per-test to be optimal. This works for 'help' # on board 'seaboard'. if not self.config.gdbserver: -self.p.timeout = 3 +self.p.timeout = TIMEOUT_MS self.p.logfile_read = self.logstream if expect_reset: loop_num = 2 -- 2.34.1
[PATCH 20/42] test: Make bootstd init run only on sandbox
Tests for standard boot need disks to be set up, which can only be done on sandbox, since adjusting disks on real hardware is not currently supported. Mark the init function as sandbox-only. Signed-off-by: Simon Glass --- test/py/tests/test_ut.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py index c169c835e38..58205066ec8 100644 --- a/test/py/tests/test_ut.py +++ b/test/py/tests/test_ut.py @@ -470,6 +470,7 @@ def test_ut_dm_init(u_boot_console): fh.write(data) @pytest.mark.buildconfigspec('cmd_bootflow') +@pytest.mark.buildconfigspec('sandbox') def test_ut_dm_init_bootstd(u_boot_console): """Initialise data for bootflow tests""" -- 2.34.1
[PATCH 19/42] test: Allow signaling that U-Boot is ready
When Labgrid is used, it can get U-Boot ready for running tests. It prints a message when it has done so. Add logic to detect this message and accept it. Signed-off-by: Simon Glass --- test/py/u_boot_console_base.py | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index 3e01be11029..e230caf37e1 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -22,6 +22,7 @@ pattern_stop_autoboot_prompt = re.compile('Hit any key to stop autoboot: ') pattern_unknown_command = re.compile('Unknown command \'.*\' - try \'help\'') pattern_error_notification = re.compile('## Error: ') pattern_error_please_reset = re.compile('### ERROR ### Please RESET the board ###') +pattern_ready_prompt = re.compile('U-Boot is ready') PAT_ID = 0 PAT_RE = 1 @@ -170,15 +171,15 @@ class ConsoleBase(object): self.bad_pattern_ids[m - 1]) self.u_boot_version_string = self.p.after while True: -m = self.p.expect([self.prompt_compiled, +m = self.p.expect([self.prompt_compiled, pattern_ready_prompt, pattern_stop_autoboot_prompt] + self.bad_patterns) -if m == 0: +if m == 0 or m == 1: break -if m == 1: +if m == 2: self.p.send(' ') continue raise Exception('Bad pattern found on console: ' + -self.bad_pattern_ids[m - 2]) +self.bad_pattern_ids[m - 3]) except Exception as ex: self.log.error(str(ex)) -- 2.34.1
[PATCH 18/42] meson: Correct driver declaration for meson_axg_gpio
This should use the driver macros so that the driver appears in the linker list. Fix this. Fixes: 8587839f19d ("pinctrl: meson: add axg support") Signed-off-by: Simon Glass --- drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c | 2 +- drivers/pinctrl/meson/pinctrl-meson-axg.c | 4 ++-- drivers/pinctrl/meson/pinctrl-meson-axg.h | 2 +- drivers/pinctrl/meson/pinctrl-meson-g12a.c| 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c b/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c index 52c726cf038..15ebd574ef1 100644 --- a/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c +++ b/drivers/pinctrl/meson/pinctrl-meson-axg-pmx.c @@ -179,7 +179,7 @@ static const struct dm_gpio_ops meson_axg_gpio_ops = { .direction_output = meson_gpio_direction_output, }; -const struct driver meson_axg_gpio_driver = { +U_BOOT_DRIVER(meson_axg_gpio) = { .name = "meson-axg-gpio", .id = UCLASS_GPIO, .probe = meson_gpio_probe, diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg.c b/drivers/pinctrl/meson/pinctrl-meson-axg.c index 94e09cd3f8a..ed3f92b2d75 100644 --- a/drivers/pinctrl/meson/pinctrl-meson-axg.c +++ b/drivers/pinctrl/meson/pinctrl-meson-axg.c @@ -939,7 +939,7 @@ struct meson_pinctrl_data meson_axg_periphs_pinctrl_data = { .num_groups = ARRAY_SIZE(meson_axg_periphs_groups), .num_funcs = ARRAY_SIZE(meson_axg_periphs_functions), .num_banks = ARRAY_SIZE(meson_axg_periphs_banks), - .gpio_driver= &meson_axg_gpio_driver, + .gpio_driver= DM_DRIVER_REF(meson_axg_gpio), .pmx_data = &meson_axg_periphs_pmx_banks_data, }; @@ -953,7 +953,7 @@ struct meson_pinctrl_data meson_axg_aobus_pinctrl_data = { .num_groups = ARRAY_SIZE(meson_axg_aobus_groups), .num_funcs = ARRAY_SIZE(meson_axg_aobus_functions), .num_banks = ARRAY_SIZE(meson_axg_aobus_banks), - .gpio_driver= &meson_axg_gpio_driver, + .gpio_driver= DM_DRIVER_REF(meson_axg_gpio), .pmx_data = &meson_axg_aobus_pmx_banks_data, }; diff --git a/drivers/pinctrl/meson/pinctrl-meson-axg.h b/drivers/pinctrl/meson/pinctrl-meson-axg.h index c8d2b3af036..a6581bab500 100644 --- a/drivers/pinctrl/meson/pinctrl-meson-axg.h +++ b/drivers/pinctrl/meson/pinctrl-meson-axg.h @@ -61,6 +61,6 @@ struct meson_pmx_axg_data { } extern const struct pinctrl_ops meson_axg_pinctrl_ops; -extern const struct driver meson_axg_gpio_driver; +extern U_BOOT_DRIVER(meson_axg_gpio); #endif /* __PINCTRL_MESON_AXG_H__ */ diff --git a/drivers/pinctrl/meson/pinctrl-meson-g12a.c b/drivers/pinctrl/meson/pinctrl-meson-g12a.c index 24f47f82558..67114df6824 100644 --- a/drivers/pinctrl/meson/pinctrl-meson-g12a.c +++ b/drivers/pinctrl/meson/pinctrl-meson-g12a.c @@ -1253,7 +1253,7 @@ static struct meson_pinctrl_data meson_g12a_periphs_pinctrl_data = { .num_groups = ARRAY_SIZE(meson_g12a_periphs_groups), .num_funcs = ARRAY_SIZE(meson_g12a_periphs_functions), .num_banks = ARRAY_SIZE(meson_g12a_periphs_banks), - .gpio_driver= &meson_axg_gpio_driver, + .gpio_driver= DM_DRIVER_REF(meson_axg_gpio), .pmx_data = &meson_g12a_periphs_pmx_banks_data, }; @@ -1267,7 +1267,7 @@ static struct meson_pinctrl_data meson_g12a_aobus_pinctrl_data = { .num_groups = ARRAY_SIZE(meson_g12a_aobus_groups), .num_funcs = ARRAY_SIZE(meson_g12a_aobus_functions), .num_banks = ARRAY_SIZE(meson_g12a_aobus_banks), - .gpio_driver= &meson_axg_gpio_driver, + .gpio_driver= DM_DRIVER_REF(meson_axg_gpio), .pmx_data = &meson_g12a_aobus_pmx_banks_data, }; -- 2.34.1
[PATCH 17/42] google: Disable TPMv2 on most Chromebooks
This feature is not present on older Chromebooks, so disable the setting. Signed-off-by: Simon Glass --- configs/chromebook_link64_defconfig| 1 + configs/chromebook_link_defconfig | 1 + configs/chromebook_samus_defconfig | 1 + configs/chromebook_samus_tpl_defconfig | 1 + configs/nyan-big_defconfig | 4 +--- configs/snow_defconfig | 1 + 6 files changed, 6 insertions(+), 3 deletions(-) diff --git a/configs/chromebook_link64_defconfig b/configs/chromebook_link64_defconfig index 7cf23b29e46..9583f87bf0f 100644 --- a/configs/chromebook_link64_defconfig +++ b/configs/chromebook_link64_defconfig @@ -80,6 +80,7 @@ CONFIG_SYS_NS16550=y CONFIG_SYS_NS16550_PORT_MAPPED=y CONFIG_SPI=y CONFIG_TPM_TIS_LPC=y +# CONFIG_TPM_V2 is not set CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y CONFIG_FRAMEBUFFER_SET_VESA_MODE=y diff --git a/configs/chromebook_link_defconfig b/configs/chromebook_link_defconfig index a9f91dd9b26..75a7a488a92 100644 --- a/configs/chromebook_link_defconfig +++ b/configs/chromebook_link_defconfig @@ -70,6 +70,7 @@ CONFIG_SYS_NS16550_PORT_MAPPED=y CONFIG_SOUND=y CONFIG_SPI=y CONFIG_TPM_TIS_LPC=y +# CONFIG_TPM_V2 is not set CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y CONFIG_VIDEO_COPY=y diff --git a/configs/chromebook_samus_defconfig b/configs/chromebook_samus_defconfig index 40cc449b9b3..8cdad8d2344 100644 --- a/configs/chromebook_samus_defconfig +++ b/configs/chromebook_samus_defconfig @@ -74,6 +74,7 @@ CONFIG_SOUND_I8254=y CONFIG_SOUND_RT5677=y CONFIG_SPI=y CONFIG_TPM_TIS_LPC=y +# CONFIG_TPM_V2 is not set CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y CONFIG_VIDEO_COPY=y diff --git a/configs/chromebook_samus_tpl_defconfig b/configs/chromebook_samus_tpl_defconfig index 3e7298f16af..1be57560f89 100644 --- a/configs/chromebook_samus_tpl_defconfig +++ b/configs/chromebook_samus_tpl_defconfig @@ -96,6 +96,7 @@ CONFIG_SOUND_RT5677=y CONFIG_SPI=y CONFIG_TPL_SYSRESET=y CONFIG_TPM_TIS_LPC=y +# CONFIG_TPM_V2 is not set CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y CONFIG_FRAMEBUFFER_SET_VESA_MODE=y diff --git a/configs/nyan-big_defconfig b/configs/nyan-big_defconfig index 4dec710cf8d..78fb7580da7 100644 --- a/configs/nyan-big_defconfig +++ b/configs/nyan-big_defconfig @@ -11,8 +11,6 @@ CONFIG_DEFAULT_DEVICE_TREE="tegra124-nyan-big" CONFIG_SPL_TEXT_BASE=0x80108000 CONFIG_SPL_STACK=0x800c CONFIG_BOOTSTAGE_STASH_ADDR=0x8300 -CONFIG_DEBUG_UART_BASE=0x70006000 -CONFIG_DEBUG_UART_CLOCK=40800 CONFIG_TEGRA124=y CONFIG_TARGET_NYAN_BIG=y CONFIG_TEGRA_GPU=y @@ -75,7 +73,6 @@ CONFIG_DM_REGULATOR=y CONFIG_REGULATOR_AS3722=y CONFIG_DM_REGULATOR_FIXED=y CONFIG_PWM_TEGRA=y -CONFIG_DEBUG_UART_SHIFT=2 CONFIG_SYS_NS16550=y CONFIG_SOUND=y CONFIG_I2S=y @@ -83,6 +80,7 @@ CONFIG_I2S_TEGRA=y CONFIG_SOUND_MAX98090=y CONFIG_TEGRA114_SPI=y CONFIG_TPM_TIS_INFINEON=y +# CONFIG_TPM_V2 is not set CONFIG_USB=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_TEGRA=y diff --git a/configs/snow_defconfig b/configs/snow_defconfig index 3a617c6cf40..2c0757194bd 100644 --- a/configs/snow_defconfig +++ b/configs/snow_defconfig @@ -88,6 +88,7 @@ CONFIG_SOUND_MAX98095=y CONFIG_SOUND_WM8994=y CONFIG_EXYNOS_SPI=y CONFIG_TPM_TIS_INFINEON=y +# CONFIG_TPM_V2 is not set CONFIG_USB=y CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_DWC3=y -- 2.34.1
[PATCH 16/42] sunxi: Mark scp as optional
This binary does not prevent the system from booting. Mark it optional so that U-Boot can be built without it. Signed-off-by: Simon Glass --- arch/arm/dts/sunxi-u-boot.dtsi | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi index 0909a67883e..e1a9a7f5d4c 100644 --- a/arch/arm/dts/sunxi-u-boot.dtsi +++ b/arch/arm/dts/sunxi-u-boot.dtsi @@ -90,6 +90,7 @@ scp { filename = "scp.bin"; missing-msg = "scp-sunxi"; + optional; }; }; #endif -- 2.34.1
[PATCH 15/42] am33xx: Provide a function to set up the debug UART
Since commit 0dba45864b2a ("arm: Init the debug UART") the debug UART is set up in _main() before early_system_init() is called. Add a suitable board_debug_uart_init() function to set up the UART in SPL. Signed-off-by: Simon Glass --- arch/arm/mach-omap2/am33xx/board.c | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/arch/arm/mach-omap2/am33xx/board.c b/arch/arm/mach-omap2/am33xx/board.c index 78c1e965c9f..84a60dedd72 100644 --- a/arch/arm/mach-omap2/am33xx/board.c +++ b/arch/arm/mach-omap2/am33xx/board.c @@ -490,9 +490,6 @@ void early_system_init(void) */ save_omap_boot_params(); #endif -#ifdef CONFIG_DEBUG_UART_OMAP - debug_uart_init(); -#endif #ifdef CONFIG_SPL_BUILD spl_early_init(); @@ -533,3 +530,18 @@ static int am33xx_dm_post_init(void) return 0; } EVENT_SPY_SIMPLE(EVT_DM_POST_INIT_F, am33xx_dm_post_init); + +#ifdef CONFIG_DEBUG_UART_BOARD_INIT +void board_debug_uart_init(void) +{ + if (u_boot_first_phase()) { + hw_data_init(); + set_uart_mux_conf(); + setup_early_clocks(); + uart_soft_reset(); + + /* avoid uart gibberish by allowing the clocks to settle */ + mdelay(50); + } +} +#endif -- 2.34.1
[PATCH 14/42] initcall: Correct use of relocation offset
The relocation offset can change in some initcall sequences. Handle this and make sure it is used for all debugging statements in init_run_list() Update the trace test to match. Signed-off-by: Simon Glass --- lib/initcall.c | 6 -- test/py/tests/test_trace.py | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/initcall.c b/lib/initcall.c index c8e2b0f6a38..2686b9aed5c 100644 --- a/lib/initcall.c +++ b/lib/initcall.c @@ -49,13 +49,14 @@ static int initcall_is_event(init_fnc_t func) */ int initcall_run_list(const init_fnc_t init_sequence[]) { - ulong reloc_ofs = calc_reloc_ofs(); + ulong reloc_ofs; const init_fnc_t *ptr; enum event_t type; init_fnc_t func; int ret = 0; for (ptr = init_sequence; func = *ptr, func; ptr++) { + reloc_ofs = calc_reloc_ofs(); type = initcall_is_event(func); if (type) { @@ -84,7 +85,8 @@ int initcall_run_list(const init_fnc_t init_sequence[]) sprintf(buf, "event %d/%s", type, event_type_name(type)); } else { - sprintf(buf, "call %p", func); + sprintf(buf, "call %p", + (char *)func - reloc_ofs); } printf("initcall failed at %s (err=%dE)\n", buf, ret); diff --git a/test/py/tests/test_trace.py b/test/py/tests/test_trace.py index f41d4cf71f0..ec1e624722c 100644 --- a/test/py/tests/test_trace.py +++ b/test/py/tests/test_trace.py @@ -175,7 +175,7 @@ def check_funcgraph(cons, fname, proftool, map_fname, trace_dat): # Then look for this: # u-boot-1 0. 282.101375: funcgraph_exit: 0.006 us | } # Then check for this: -# u-boot-1 0. 282.101375: funcgraph_entry:0.000 us | initcall_is_event(); +# u-boot-1 0. 282.101375: funcgraph_entry:0.000 us | calc_reloc_ofs(); expected_indent = None found_start = False @@ -199,7 +199,7 @@ def check_funcgraph(cons, fname, proftool, map_fname, trace_dat): # The next function after initf_bootstage() exits should be # initcall_is_event() -assert upto == 'initcall_is_event()' +assert upto == 'calc_reloc_ofs()' # Now look for initf_dm() and dm_timer_init() so we can check the bootstage # time -- 2.34.1
[PATCH 13/42] dm: core: Enhance comments on bind_drivers_pass()
This part of driver model is a little subtle, so add some more comments to promote better understanding. Signed-off-by: Simon Glass --- drivers/core/lists.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/core/lists.c b/drivers/core/lists.c index 2839a9b7371..a84e6a98cb1 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -8,6 +8,7 @@ #define LOG_CATEGORY LOGC_DM +#include #include #include #include @@ -50,6 +51,21 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id) return NULL; } +/** + * bind_drivers_pass() - Perform a pass of driver binding + * + * Work through the driver_info records binding a driver for each one. If the + * binding fails, continue binding others, but return the error. + * + * For OF_PLATDATA we must bind parent devices before their children. So only + * children of bound parents are bound on each call to this function. When a + * child is left unbound, -EAGAIN is returned, indicating that this function + * should be called again + * + * @parent: Parent device to use when binding each child device + * Return: 0 if OK, -EAGAIN if unbound children exist, -ENOENT if there is no + * driver for one of the devices, other -ve on other error + */ static int bind_drivers_pass(struct udevice *parent, bool pre_reloc_only) { struct driver_info *info = -- 2.34.1
[PATCH 12/42] buildman: Add a way to limit the number of buildmans
Buildman uses all available CPUs by default, so running more than one or two concurrent processes is not normally useful. However in some CI cases we want to be able to run several jobs at once to save time. For example, in a lab situation we may want to run a test on 20 boards at a time, since only the build step actually takes much CPU. Add an option which allows such a limit. When buildman starts up, it waits until the number of running processes goes below the limit, then claims a spot in the list. The list is maintained with a temporary file. Note that the temp file is user-specific, since it is hard to create a locked temporary file which can be accessed by any user. In most cases, only one user is running jobs on a machine, so this should not matter. Signed-off-by: Simon Glass --- tools/buildman/buildman.rst| 5 ++ tools/buildman/cmdline.py | 2 + tools/buildman/control.py | 140 - tools/buildman/pyproject.toml | 6 +- tools/buildman/test.py | 121 tools/u_boot_pylib/terminal.py | 7 +- 6 files changed, 277 insertions(+), 4 deletions(-) diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst index bd0482af5f7..b8ff3bf1ab2 100644 --- a/tools/buildman/buildman.rst +++ b/tools/buildman/buildman.rst @@ -1286,6 +1286,11 @@ then buildman hangs. Failing to handle any eventuality is a bug in buildman and should be reported. But you can use -T0 to disable threading and hopefully figure out the root cause of the build failure. +For situations where buildman is invoked from multiple running processes, it is +sometimes useful to have buildman wait until the others have finished. Use the +--process-limit option for this: --process-limit 1 will allow only one buildman +to process jobs at a time. + Build summary - diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 8dc5a8787b5..544a391a464 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -129,6 +129,8 @@ def add_after_m(parser): default=False, help="Use an O= (output) directory per board rather than per thread") parser.add_argument('--print-arch', action='store_true', default=False, help="Print the architecture for a board (ARCH=)") +parser.add_argument('--process-limit', type=int, + default=0, help='Limit to number of buildmans running at once') parser.add_argument('-r', '--reproducible-builds', action='store_true', help='Set SOURCE_DATE_EPOCH=0 to suuport a reproducible build') parser.add_argument('-R', '--regen-board-list', type=str, diff --git a/tools/buildman/control.py b/tools/buildman/control.py index f2dd87814c3..464835c5be5 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -7,10 +7,13 @@ This holds the main control logic for buildman, when not running tests. """ +import getpass import multiprocessing import os import shutil import sys +import tempfile +import time from buildman import boards from buildman import bsettings @@ -21,10 +24,23 @@ from patman import gitutil from patman import patchstream from u_boot_pylib import command from u_boot_pylib import terminal -from u_boot_pylib.terminal import tprint +from u_boot_pylib import tools +from u_boot_pylib.terminal import print_clear, tprint TEST_BUILDER = None +# Space-separated list of buildman process IDs currently running jobs +RUNNING_FNAME = f'buildmanq.{getpass.getuser()}' + +# Lock file for access to RUNNING_FILE +LOCK_FNAME = f'{RUNNING_FNAME}.lock' + +# Wait time for access to lock (seconds) +LOCK_WAIT_S = 10 + +# Wait time to start running +RUN_WAIT_S = 300 + def get_plural(count): """Returns a plural 's' if count is not 1""" return 's' if count != 1 else '' @@ -578,6 +594,125 @@ def calc_adjust_cfg(adjust_cfg, reproducible_builds): return adjust_cfg +def read_procs(tmpdir=tempfile.gettempdir()): +"""Read the list of running buildman processes + +If the list is corrupted, returns an empty list + +Args: +tmpdir (str): Temporary directory to use (for testing only) +""" +running_fname = os.path.join(tmpdir, RUNNING_FNAME) +procs = [] +if os.path.exists(running_fname): +items = tools.read_file(running_fname, binary=False).split() +try: +procs = [int(x) for x in items] +except ValueError: # Handle invalid format +pass +return procs + + +def check_pid(pid): +"""Check for existence of a unix PID + + https://stackoverflow.com/questions/568271/how-to-check-if-there-exists-a-process-with-a-given-pid-in-python + +Args: +pid (int): PID to check + +Returns: +True if it exists, else False +""" +try: +os.kill(pid, 0) +except OSError: +return False +else: +return True + + +def write_procs(procs, tmpdir=tempfile.gettempdir()): +"""Write the list of
[PATCH 11/42] buildman: Retry the build for current source
Buildman retries a failed build when processing a branch, but does not do this when building current source. It is useful to do this retry in both cases, so add the logic for it. Signed-off-by: Simon Glass --- tools/buildman/builderthread.py | 8 1 file changed, 8 insertions(+) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index bbe2f6f0d24..55658487abf 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -755,6 +755,14 @@ class BuilderThread(threading.Thread): self.mrproper, self.builder.config_only, True, self.builder.force_build_failures, job.work_in_output, job.adjust_cfg) +failed = result.return_code or result.stderr +if failed and not self.mrproper: +result, request_config = self.run_commit(None, brd, work_dir, +True, self.builder.fallback_mrproper, +self.builder.config_only, True, +self.builder.force_build_failures, +job.work_in_output, job.adjust_cfg) + result.commit_upto = 0 self._write_result(result, job.keep_outputs, job.work_in_output) self._send_result(result) -- 2.34.1
[PATCH 10/42] buildman: Add a flag to force mrproper on failure
When a file is removed by a commit (e.g. include/common.h yay!) it can cause incremental build failures since one of the dependency files from a previous build may mention the file. Add an option to run 'make mrproper' automatically when a build fails. This can be used to automatically resolve the problem, without always adding the large overhead of 'make mrproper' to every build. Signed-off-by: Simon Glass --- tools/buildman/builder.py | 18 ++ tools/buildman/builderthread.py | 6 -- tools/buildman/buildman.rst | 3 ++- tools/buildman/cmdline.py | 4 +++- tools/buildman/control.py | 1 + 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index f35175b4598..c4384f53e8d 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -256,14 +256,14 @@ class Builder: def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1, no_subdirs=False, full_path=False, verbose_build=False, - mrproper=False, per_board_out_dir=False, - config_only=False, squash_config_y=False, - warnings_as_errors=False, work_in_output=False, - test_thread_exceptions=False, adjust_cfg=None, - allow_missing=False, no_lto=False, reproducible_builds=False, - force_build=False, force_build_failures=False, - force_reconfig=False, in_tree=False, - force_config_on_failure=False, make_func=None): + mrproper=False, fallback_mrproper=False, + per_board_out_dir=False, config_only=False, + squash_config_y=False, warnings_as_errors=False, + work_in_output=False, test_thread_exceptions=False, + adjust_cfg=None, allow_missing=False, no_lto=False, + reproducible_builds=False, force_build=False, + force_build_failures=False, force_reconfig=False, + in_tree=False, force_config_on_failure=False, make_func=None): """Create a new Builder object Args: @@ -283,6 +283,7 @@ class Builder: PATH verbose_build: Run build with V=1 and don't use 'make -s' mrproper: Always run 'make mrproper' when configuring +fallback_mrproper: Run 'make mrproper' and retry on build failure per_board_out_dir: Build in a separate persistent directory per board rather than a thread-specific directory config_only: Only configure each build, don't build it @@ -352,6 +353,7 @@ class Builder: self.force_reconfig = force_reconfig self.in_tree = in_tree self.force_config_on_failure = force_config_on_failure +self.fallback_mrproper = fallback_mrproper if not self.squash_config_y: self.config_filenames += EXTRA_CONFIG_FILENAMES diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index c0b1067e3f7..bbe2f6f0d24 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -705,8 +705,10 @@ class BuilderThread(threading.Thread): # with a reconfig. if self.builder.force_config_on_failure: result, request_config = self.run_commit(commit_upto, -brd, work_dir, True, self.mrproper, False, True, -False, job.work_in_output, job.adjust_cfg) +brd, work_dir, True, +self.mrproper or self.builder.fallback_mrproper, +False, True, False, job.work_in_output, +job.adjust_cfg) did_config = True if not self.builder.force_reconfig: do_config = request_config diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst index aae2477b5c3..bd0482af5f7 100644 --- a/tools/buildman/buildman.rst +++ b/tools/buildman/buildman.rst @@ -995,7 +995,8 @@ By default, buildman doesn't execute 'make mrproper' prior to building the first commit for each board. This reduces the amount of work 'make' does, and hence speeds up the build. To force use of 'make mrproper', use -the -m flag. This flag will slow down any buildman invocation, since it increases the amount -of work done on any build. +of work done on any build. An alternative is to use the --fallback-mrproper +flag, which retries the build with 'make mrproper' only after a build failure. One possible application of buildman is as part of a continual edit, build, edit, build, ... cycle; repeatedly applying buildman to the same change or diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 03211bd5aa5..8dc5a8787b5 100644 --- a/tools/
[PATCH 09/42] buildman: Avoid rebuilding when --mrproper is used
When this flag is enabled, 'make mrproper' is always used when reconfiguring, so there is no point in doing it again. Update this. Signed-off-by: Simon Glass --- tools/buildman/builderthread.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 0a7ff2e083e..c0b1067e3f7 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -700,7 +700,7 @@ class BuilderThread(threading.Thread): job.work_in_output, job.adjust_cfg) failed = result.return_code or result.stderr did_config = do_config -if failed and not do_config: +if failed and not do_config and not self.mrproper: # If our incremental build failed, try building again # with a reconfig. if self.builder.force_config_on_failure: -- 2.34.1
[PATCH 08/42] buildman: Make mrproper an argument to run_commit()
Pass this in so the caller can change it independently of the member variable. Signed-off-by: Simon Glass --- tools/buildman/builderthread.py | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index ff63f9397e6..0a7ff2e083e 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -448,9 +448,9 @@ class BuilderThread(threading.Thread): result.cmd_list = cmd_list return result, do_config -def run_commit(self, commit_upto, brd, work_dir, do_config, config_only, - force_build, force_build_failures, work_in_output, - adjust_cfg): +def run_commit(self, commit_upto, brd, work_dir, do_config, mrproper, + config_only, force_build, force_build_failures, + work_in_output, adjust_cfg): """Build a particular commit. If the build is already done, and we are not forcing a build, we skip @@ -461,6 +461,7 @@ class BuilderThread(threading.Thread): brd (Board): Board to build work_dir (str): Directory to which the source will be checked out do_config (bool): True to run a make _defconfig on the source +mrproper (bool): True to run mrproper first config_only (bool): Only configure the source, do not build it force_build (bool): Force a build even if one was previously done force_build_failures (bool): Force a bulid if the previous result @@ -501,7 +502,7 @@ class BuilderThread(threading.Thread): if self.toolchain: commit = self._checkout(commit_upto, work_dir) result, do_config = self._config_and_build( -commit_upto, brd, work_dir, do_config, self.mrproper, +commit_upto, brd, work_dir, do_config, mrproper, config_only, adjust_cfg, commit, out_dir, out_rel_dir, result) result.already_done = False @@ -692,7 +693,8 @@ class BuilderThread(threading.Thread): force_build = False for commit_upto in range(0, len(job.commits), job.step): result, request_config = self.run_commit(commit_upto, brd, -work_dir, do_config, self.builder.config_only, +work_dir, do_config, self.mrproper, +self.builder.config_only, force_build or self.builder.force_build, self.builder.force_build_failures, job.work_in_output, job.adjust_cfg) @@ -703,8 +705,8 @@ class BuilderThread(threading.Thread): # with a reconfig. if self.builder.force_config_on_failure: result, request_config = self.run_commit(commit_upto, -brd, work_dir, True, False, True, False, -job.work_in_output, job.adjust_cfg) +brd, work_dir, True, self.mrproper, False, True, +False, job.work_in_output, job.adjust_cfg) did_config = True if not self.builder.force_reconfig: do_config = request_config @@ -748,7 +750,7 @@ class BuilderThread(threading.Thread): else: # Just build the currently checked-out build result, request_config = self.run_commit(None, brd, work_dir, True, -self.builder.config_only, True, +self.mrproper, self.builder.config_only, True, self.builder.force_build_failures, job.work_in_output, job.adjust_cfg) result.commit_upto = 0 -- 2.34.1
[PATCH 07/42] buildman: Make mrproper an argument to _config_and_build()
Pass this in so the caller can change it independently of the member variable. Signed-off-by: Simon Glass --- tools/buildman/builderthread.py | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 5d4426bf0d1..ff63f9397e6 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -381,7 +381,7 @@ class BuilderThread(threading.Thread): commit = 'current' return commit -def _config_and_build(self, commit_upto, brd, work_dir, do_config, +def _config_and_build(self, commit_upto, brd, work_dir, do_config, mrproper, config_only, adjust_cfg, commit, out_dir, out_rel_dir, result): """Do the build, configuring first if necessary @@ -391,6 +391,7 @@ class BuilderThread(threading.Thread): brd (Board): Board to create arguments for work_dir (str): Directory to which the source will be checked out do_config (bool): True to run a make _defconfig on the source +mrproper (bool): True to run mrproper first config_only (bool): Only configure the source, do not build it adjust_cfg (list of str): See the cfgutil module and run_commit() commit (Commit): Commit only being built @@ -421,7 +422,7 @@ class BuilderThread(threading.Thread): if do_config or adjust_cfg: result = self._reconfigure( commit, brd, cwd, args, env, config_args, config_out, cmd_list, -self.mrproper) +mrproper) do_config = False # No need to configure next time if adjust_cfg: cfgutil.adjust_cfg_file(cfg_file, adjust_cfg) @@ -500,8 +501,9 @@ class BuilderThread(threading.Thread): if self.toolchain: commit = self._checkout(commit_upto, work_dir) result, do_config = self._config_and_build( -commit_upto, brd, work_dir, do_config, config_only, -adjust_cfg, commit, out_dir, out_rel_dir, result) +commit_upto, brd, work_dir, do_config, self.mrproper, +config_only, adjust_cfg, commit, out_dir, out_rel_dir, +result) result.already_done = False result.toolchain = self.toolchain -- 2.34.1