On 01/18/2018 07:13 PM, Alexander Graf wrote:
>
>
> 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.g...@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? :)
The changes in include/dm/device.h are not needed. I will revert these.
Maybe in future we will have to back link devices to handles but not now.
Regards
Heinrich
>
>>>
>>>> #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
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
