Hi Ilias, On Fri, 13 Dec 2024 at 05:13, Ilias Apalodimas <[email protected]> wrote: > > Hi Simon, > > On Fri, 13 Dec 2024 at 05:53, Simon Glass <[email protected]> wrote: > > > > Hi Ilias, > > > > On Thu, 12 Dec 2024 at 08:45, Ilias Apalodimas > > <[email protected]> wrote: > > > > > > On Thu, 12 Dec 2024 at 15:45, Simon Glass <[email protected]> wrote: > > > > > > > > Hi Ilias, > > > > > > > > On Thu, 12 Dec 2024 at 00:49, Ilias Apalodimas > > > > <[email protected]> wrote: > > > > > > > > > > On Thu, 12 Dec 2024 at 00:38, Simon Glass <[email protected]> wrote: > > > > > > > > > > > > This uses a few global variables at present. With the bootflow we > > > > > > have > > > > > > the required parameters, so add a function which accepts these. > > > > > > Update > > > > > > the existing function to call the new one with the globals. > > > > > > > > > > > > Signed-off-by: Simon Glass <[email protected]> > > > > > > --- > > > > > > > > > > > > (no changes since v1) > > > > > > > > > > > > lib/efi_loader/efi_bootbin.c | 29 +++++++++++++++++++++++++---- > > > > > > 1 file changed, 25 insertions(+), 4 deletions(-) > > > > > > > > > > > > diff --git a/lib/efi_loader/efi_bootbin.c > > > > > > b/lib/efi_loader/efi_bootbin.c > > > > > > index 8febd325f34..8ebd48547cb 100644 > > > > > > --- a/lib/efi_loader/efi_bootbin.c > > > > > > +++ b/lib/efi_loader/efi_bootbin.c > > > > > > @@ -209,18 +209,22 @@ out: > > > > > > } > > > > > > > > > > > > /** > > > > > > - * efi_binary_run() - run loaded UEFI image > > > > > > + * efi_binary_run_() - run loaded UEFI image > > > > > > * > > > > > > * @image_ptr: memory address of the UEFI image > > > > > > * @size: size of the UEFI image > > > > > > * @fdt: device-tree > > > > > > + * @device: EFI device-path > > > > > > + * @image: EFI image-path > > > > > > * > > > > > > * Execute an EFI binary image loaded at @image. > > > > > > * @size may be zero if the binary is loaded with U-Boot load > > > > > > command. > > > > > > * > > > > > > * Return: status code > > > > > > */ > > > > > > -efi_status_t efi_binary_run(void *image_ptr, size_t size, void > > > > > > *fdt) > > > > > > +efi_status_t efi_binary_run_(void *image_ptr, size_t size, void > > > > > > *fdt, > > > > > > + struct efi_device_path *device, > > > > > > + struct efi_device_path *image) > > > > > > { > > > > > > > > > > This needs to be static here. Not the next patch. But I don't think we > > > > > need this at all (look below) > > > > > > > > > > > efi_status_t ret; > > > > > > > > > > > > @@ -236,6 +240,23 @@ efi_status_t efi_binary_run(void *image_ptr, > > > > > > size_t size, void *fdt) > > > > > > if (ret != EFI_SUCCESS) > > > > > > return ret; > > > > > > > > > > > > - return efi_run_image(image_ptr, size, bootefi_device_path, > > > > > > - bootefi_image_path); > > > > > > + return efi_run_image(image_ptr, size, device, image); > > > > > > +} > > > > > > + > > > > > > +/** > > > > > > + * efi_binary_run() - run loaded UEFI image > > > > > > + * > > > > > > + * @image: memory address of the UEFI image > > > > > > + * @size: size of the UEFI image > > > > > > + * @fdt: device-tree > > > > > > + * > > > > > > + * Execute an EFI binary image loaded at @image. > > > > > > + * @size may be zero if the binary is loaded with U-Boot load > > > > > > command. > > > > > > + * > > > > > > + * Return: status code > > > > > > + */ > > > > > > +efi_status_t efi_binary_run(void *image, size_t size, void *fdt) > > > > > > +{ > > > > > > + return efi_binary_run_(image, size, fdt, > > > > > > bootefi_device_path, > > > > > > + bootefi_image_path); > > > > > > } > > > > > > > > > > We don't need functions and wrappers for such a simple call. Just > > > > > update all the callisites with the extra parameters and keep one > > > > > function > > > > > > > > If you look at the two call sites, you'll see that they would need to > > > > pass bootefi_device_path and bootefi_image_path in. but these are > > > > static variables in this file. > > > > > > Ah fair enough, then please rename efi_binary_run_ -> _efi_binary_run > > > ands make it static on this patch > > > > I don't mind where the underscore goes. I changed my mind about that > > when the dtc maintainer pointed out that C-library functions start > > with underscore, so putting an underscore at the start of the function > > is confusing things. > > > > "The underscore prefix is reserved for functions and types used by the > > compiler and standard library. The standard library can use these > > names freely."[1] > > > > But I don't mind. We don't even use the C library except for sandbox > > and tools. So let me know what you think. > > I don't mind really. you can do __efi_binary_run() or keep whjat you > have. I jsut find the leading underscores a bit easier to read and > spot, but not a huge deal
Yes I find them easier to read also. I'll go with a single underscore, since __ is supposed to be for start-up code, or something. But Tom asked me not to send anything until Wednesday and I'm heading off for a few weeks at EOW. Let's see what you decide on the big series, and then perhaps I can send both before I go. Regards, Simon > > [1] > > https://stackoverflow.com/questions/39625352/why-do-some-functions-in-c-have-an-underscore-prefix
