On Wed, Sep 20, 2017 at 9:49 AM, Simon Glass sjg@chromium.org wrote:
Hi Heinrich,
On 15 September 2017 at 00:35, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 08/31/2017 02:52 PM, Simon Glass wrote:
On 27 August 2017 at 06:53, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_loader/efi_boottime.c | 77 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 1069da7d79..c5a17b6252 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1052,9 +1052,84 @@ static efi_status_t EFIAPI efi_disconnect_controller(void *controller_handle, void *driver_image_handle, void *child_handle)
Can you add a function comment?
Hello Simon,
the API functions (here DisconnectController) are documented in the UEFI spec. Is your idea that we should duplicate part of this information for all API functions? Or do you miss a comment on implementation details?
I think the code in U-Boot should stand alone, so arguments should be described here along with a one-line function description. The args are all void which is not good, but makes it even more important to document.
couple notes, fwiw..
1) As someone else implementing parts of UEFI interface, I'd find link to section in spec (or perhaps to http://wiki.phoenix.com/) to be more useful than re-writing the spec in our own words (and possibly getting it wrong)
2) Leif introduced (iirc, in the stub HII or unicode patch) efi_handle_t, and efi_string_t (and maybe we should add efi_char_t).. which we should probably use more extensively. Although I haven't wanted to go on a major housecleaning while we still have so many patches pending on list.. but would be a nice cleanup to make at some point.
BR, -R
{
struct efi_driver_binding_protocol *binding_protocol;efi_handle_t child_handle_buffer;unsigned long driver_count;efi_handle_t *driver_handle_buffer;size_t i;UINTN number_of_children;Can we somehow use a lower-case type for this? Otherwise U-Boot will start to look like EFI and I will have to start taking antidepressants.
In different places the EFI API requires a bitness dependent unsigned integer.
Shall we globally rename UINTN to uintn? This is my patch to blame: 503f2695548 (efi_loader: correct size for tpl level)
What does bitness-dependent mean? Do you mean 32-bit? It looks like this is just passed to a function, so could be uint or instead?
If it is 32-bit then uint32_t should do.
Regards, Simon