[U-Boot] [PATCH v2 14/18] efi_loader: provide links between devices EFI handles

[Alexander Graf]

On 18.01.18 17:18, Heinrich Schuchardt wrote:
> On 01/18/2018 05:09 PM, Alexander Graf wrote:
>>
>>
>> On 17.01.18 20:16, Heinrich Schuchardt wrote:
>>> U-Boot devices and EFI handles can be related, e.g. an
>>> IDE disk relates to a handle with the EFI_BLOCK_IO_PROTOCOL.
>>> Provide pointers to store these links.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>> ---
>>> v2
>>> 	no change
>>> ---
>>>  include/dm/device.h           | 4 ++++
>>>  include/efi_loader.h          | 2 ++
>>>  lib/efi_loader/efi_boottime.c | 1 +
>>>  3 files changed, 7 insertions(+)
>>>
>>> diff --git a/include/dm/device.h b/include/dm/device.h
>>> index 813e49f330..e5c54fe7b6 100644
>>> --- a/include/dm/device.h
>>> +++ b/include/dm/device.h
>>> @@ -11,6 +11,7 @@
>>>  #ifndef _DM_DEVICE_H
>>>  #define _DM_DEVICE_H
>>>  
>>> +#include <efi_loader.h>
>>>  #include <dm/ofnode.h>
>>>  #include <dm/uclass-id.h>
>>>  #include <fdtdec.h>
>>> @@ -144,6 +145,9 @@ struct udevice {
>>>  	uint32_t flags;
>>>  	int req_seq;
>>>  	int seq;
>>> +#ifdef EFI_LOADER
>>> +	efi_handle_t handle;
>>> +#endif
>>
>> I fail to find where you actually make use of the handle inside

Care to answer here too? :)

>>
>>>  #ifdef CONFIG_DEVRES
>>>  	struct list_head devres_head;
>>>  #endif
>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>> index 4060348695..711c901eda 100644
>>> --- a/include/efi_loader.h
>>> +++ b/include/efi_loader.h
>>> @@ -139,6 +139,8 @@ struct efi_object {
>>>  	struct list_head protocols;
>>>  	/* The object spawner can either use this for data or as identifier */
>>>  	void *handle;
>>> +	/* Device */
>>> +	struct udevice *dev;
>>>  };
>>>  
>>>  /**
>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>>> index 5a3349ecb2..4b3b63e39a 100644
>>> --- a/lib/efi_loader/efi_boottime.c
>>> +++ b/lib/efi_loader/efi_boottime.c
>>> @@ -362,6 +362,7 @@ efi_status_t efi_create_handle(efi_handle_t *handle)
>>>  			      (void **)&obj);
>>>  	if (r != EFI_SUCCESS)
>>>  		return r;
>>> +	obj->dev = NULL;
>>
>> How about we just zero initialize the whole struct?
> 
> All other fields are initialized in efi_add_handle().
> So why invest more CPU cycles?

I'm mostly concerned that we get into a situation where people don't
fully grasp the flow of what gets initialized where and nasty bugs happen.

So I guess we should either initialize obj->dev in efi_add_handle() or
fully zero initialize obj, like we do for most other callers of
efi_add_handle().


Alex