Hi Rob,
On 20 September 2017 at 08:23, Rob Clark robdclark@gmail.com wrote:
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..
- 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)
The problem is that there are 3 void pointers and I have no idea what they really are and what to pass. We have to have some coding standards in U-Boot. I am not looking for a detailed description of the purpose of the function, but one line and a description of the arguments is the minimum we should have for exported functions.
I think it is a great idea to link to the spec as well, particularly if it can be a URL.
- 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.
Sounds good to me. No hurry, but it's nice to know that this is heading in the right direction. The EFI API is really awful IMO. The obfuscation is so painful.
Regards, Simon