On Tue, Jan 07, 2025 at 06:57:50AM -0700, Simon Glass wrote: > Hi Heinrich, > > On Tue, 7 Jan 2025 at 06:11, Heinrich Schuchardt <[email protected]> wrote: > > > > On 07.01.25 13:15, Simon Glass wrote: > > > Hi Heinrich, > > > > > > On Mon, 6 Jan 2025 at 10:00, Heinrich Schuchardt <[email protected]> > > > wrote: > > >> > > >> On 06.01.25 15:47, Simon Glass wrote: > > >>> This test was hamstrung in code review so this series is an attempt to > > >>> complete the intended functionality: > > >>> > > >>> - Check memory allocations look correct > > >>> - Check that exit-boot-services removes active-DMA devices > > >>> - Check that the bootflow is still present after testapp finishes > > >>> > > >>> The EFI functionality duplicates bootm_announce_and_cleanup() and still > > >>> uses the defunct board_quiesce_devices() so a nice cleanup would be to > > >>> call the bootm function instead, with suitable modifications. That would > > >>> allow bootstage to work too. > > >>> > > >>> This series is based on sjg/master since the EFI logging was rejected so > > >>> far. > > >> > > >> Yes, it was rejected because a solution at the lib/log.c level would be > > >> more generic. > > > > > > As I mentioned, that idea isn't suitable for programmatic use. > > > > What can be done with show_addr("mem", rec->memory); that log_debug() > > does not offer or which you could not do with a new log function in > > lib/log.c that takes variadic arguments? > > There are asserts in [1], for example. How do you propose to handle > that? See [2] for my previous explanation, quoted here: > > > CONFIG_LOG with a bloblist option would be a great idea, but it's hard > > to programmatically scan text...plus only the external call sites are > > actually logged. > > Also see the discussion on the original patch [3]. There was also your > reply at [4], but I think you missed that this is intended for use in > unit tests (i.e. with ut_assert()). > > You also requested that this be generalised, rather than being > EFI-loader-specific. I have no objection to that, but don't have a use > case for it yet, so have deferred that to later. It's a fairly simple > change, if/when needed. If the series was not NAKed, I'd be happy to > do it now. > > > > > > >> > > >> Tom suggested not to send patches that are for private enjoyment to the > > >> mailing list. > > > > > > My contributions to U-Boot are only ever about private enjoyment :-) > > > > > > Do you have any comments on the patches? > > Regards, > Simon > > [1] > https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/ > [2] > https://lore.kernel.org/u-boot/CAFLszTjxOE_037+kR0jgdax80sBombYo_k0YgiuVnP=kzco...@mail.gmail.com/ > [3] > https://lore.kernel.org/u-boot/CAC_iWjKtaN54B98OKbkoXkC_GmKJ=x+M4=uy_o6rosopzad...@mail.gmail.com/ > [4] > https://lore.kernel.org/u-boot/[email protected]/
Looking at the logging portions of the original series again, especially if this was made generic, we probably don't want to print to actual console every time we're making a note of some memory allocation for example, that would be unreadable outside of a debug context. The point of this really seems to be "log things for verifying in tests later". Does that end up being useful? I don't know. Heinrich or Ilias, do the tests in [1] look generally useful? -- Tom
signature.asc
Description: PGP signature
