[PATCH 05/10] core: extend struct driver_info to point to device

[Walter Lozano]

On 4/6/20 12:59, Simon Glass wrote:
> Hi Walter,
>
> On Fri, 29 May 2020 at 12:15, Walter Lozano <walter.lozano at collabora.com> wrote:
>> Currently when creating an U_BOOT_DEVICE entry a struct driver_info
>> is declared, which contains the data needed to instantiate the device.
>> However, the actual device is created at runtime and there is no proper
>> way to get the device based on its struct driver_info.
>>
>> This patch extends struct driver_info adding a pointer to udevice which
>> is populated during the bind process, allowing to generate a set of
>> functions to get the device based on its struct driver_info.
>>
>> Signed-off-by: Walter Lozano <walter.lozano at collabora.com>
>> ---
>>   drivers/core/device.c | 26 +++++++++++++++++++++++---
>>   drivers/core/root.c   |  4 ++++
>>   include/dm/device.h   | 14 ++++++++++++++
>>   include/dm/platdata.h | 14 ++++++++++++++
>>   4 files changed, 55 insertions(+), 3 deletions(-)
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> Nits below
Thanks for taking the time!
>> diff --git a/drivers/core/device.c b/drivers/core/device.c
>> index a0ad080aaf..5adbc30849 100644
>> --- a/drivers/core/device.c
>> +++ b/drivers/core/device.c
>> @@ -250,6 +250,7 @@ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only,
>>   {
>>          struct driver *drv;
>>          uint platdata_size = 0;
>> +       int ret = 0;
> Can you drop = 0 ?
Noted!
>
>>          drv = lists_driver_lookup_name(info->name);
>>          if (!drv)
>> @@ -260,9 +261,16 @@ int device_bind_by_name(struct udevice *parent, bool pre_reloc_only,
>>   #if CONFIG_IS_ENABLED(OF_PLATDATA)
>>          platdata_size = info->platdata_size;
>>   #endif
>> -       return device_bind_common(parent, drv, info->name,
>> -                       (void *)info->platdata, 0, ofnode_null(), platdata_size,
>> -                       devp);
>> +       ret = device_bind_common(parent, drv, info->name,
>> +                                (void *)info->platdata, 0, ofnode_null(),
>> +                                platdata_size, devp);
>> +       if (ret)
>> +               return ret;
>> +#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> +       info->dev = *devp;
>> +#endif
>> +
>> +       return ret;
>>   }
>>
>>   static void *alloc_priv(int size, uint flags)
>> @@ -727,6 +735,18 @@ int device_get_global_by_ofnode(ofnode ofnode, struct udevice **devp)
>>          return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp);
>>   }
>>
>> +#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> +int device_get_by_driver_info(const struct driver_info *info,
>> +                             struct udevice **devp)
>> +{
>> +       struct udevice *dev;
>> +
>> +       dev = info->dev;
>> +
>> +       return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp);
>> +}
>> +#endif
>> +
>>   int device_find_first_child(const struct udevice *parent, struct udevice **devp)
>>   {
>>          if (list_empty(&parent->child_head)) {
>> diff --git a/drivers/core/root.c b/drivers/core/root.c
>> index c9ee56478a..8f47a6b356 100644
>> --- a/drivers/core/root.c
>> +++ b/drivers/core/root.c
>> @@ -346,6 +346,10 @@ int dm_init_and_scan(bool pre_reloc_only)
>>   {
>>          int ret;
>>
>> +#if CONFIG_IS_ENABLED(OF_PLATDATA)
>> +       populate_phandle_data();
>> +#endif
>> +
>>          ret = dm_init(IS_ENABLED(CONFIG_OF_LIVE));
>>          if (ret) {
>>                  debug("dm_init() failed: %d\n", ret);
>> diff --git a/include/dm/device.h b/include/dm/device.h
>> index 2cfe10766f..a3b3e5bc46 100644
>> --- a/include/dm/device.h
>> +++ b/include/dm/device.h
>> @@ -538,6 +538,20 @@ int device_find_global_by_ofnode(ofnode node, struct udevice **devp);
>>    */
>>   int device_get_global_by_ofnode(ofnode node, struct udevice **devp);
>>
>> +/**
>> + * device_get_by_driver_info() - Get a device based on driver_info
>> + *
>> + * Locates a device by its struct driver_info.
>> + *
>> + * The device is probed to activate it ready for use.
>> + *
>> + * @info: Struct driver_info
> Here you should mention using DM_GET_DRIVER() to find it.
OK, I'll add a reference.
>> + * @devp: Returns pointer to device if found, otherwise this is set to NULL
>> + * @return 0 if OK, -ve on error
>> + */
>> +int device_get_by_driver_info(const struct driver_info *info,
>> +                             struct udevice **devp);
>> +
>>   /**
>>    * device_find_first_child() - Find the first child of a device
>>    *
>> diff --git a/include/dm/platdata.h b/include/dm/platdata.h
>> index c972fa6936..238379b0e4 100644
>> --- a/include/dm/platdata.h
>> +++ b/include/dm/platdata.h
>> @@ -22,12 +22,14 @@
>>    * @name:      Driver name
>>    * @platdata:  Driver-specific platform data
>>    * @platdata_size: Size of platform data structure
>> + * @dev:       Device created from this structure data
>>    */
>>   struct driver_info {
>>          const char *name;
>>          const void *platdata;
>>   #if CONFIG_IS_ENABLED(OF_PLATDATA)
>>          uint platdata_size;
>> +       struct udevice *dev;
>>   #endif
>>   };
>>
>> @@ -43,4 +45,16 @@ struct driver_info {
>>   #define U_BOOT_DEVICES(__name)                                         \
>>          ll_entry_declare_list(struct driver_info, __name, driver_info)
>>
>> +/* Get a pointer to a given driver */
>> +#define DM_GET_DEVICE(__name)                                          \
>> +       ll_entry_get(struct driver_info, __name, driver_info)
>> +
>> +/**
>> + * populate_phandle_data() - Populates phandle data in platda
>> + *
>> + * This populates phandle data with an U_BOOT_DEVICE entry get by
>> + * DM_GET_DEVICE. The implementation of this function will be done
>> + * by dtoc when parsing dtb.
>> + */
>> +void populate_phandle_data(void);
> dm_populate_phandle_data()
Sure!
>
>>   #endif
>> --
>> 2.20.1
>>
Regards,

Walter