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

[Simon Glass]

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

>
> 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 ?

>
>         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.

> + * @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()

>  #endif
> --
> 2.20.1
>

Regards,
Simon