[RFC 2/6] core: extend struct driver_info to point to device

[Simon Glass]

Hi Walter,

On Wed, 13 May 2020 at 14:14, 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        | 25 +++++++++++++++++++++++--
>  drivers/core/root.c          |  6 +++++-
>  include/dm/device-internal.h |  2 +-
>  include/dm/device.h          | 13 +++++++++++++
>  include/dm/platdata.h        |  6 ++++++
>  5 files changed, 48 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/core/device.c b/drivers/core/device.c
> index 0157bb1fe0..2476f170a5 100644
> --- a/drivers/core/device.c
> +++ b/drivers/core/device.c
> @@ -246,10 +246,11 @@ int device_bind_ofnode(struct udevice *parent, const struct driver *drv,
>  }
>
>  int device_bind_by_name(struct udevice *parent, bool pre_reloc_only,
> -                       const struct driver_info *info, struct udevice **devp)
> +                       struct driver_info *info, struct udevice **devp)
>  {
>         struct driver *drv;
>         uint platdata_size = 0;
> +       int ret = 0;
>
>         drv = lists_driver_lookup_name(info->name);
>         if (!drv)
> @@ -260,9 +261,19 @@ 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,
> +       ret = device_bind_common(parent, drv, info->name,
>                         (void *)info->platdata, 0, ofnode_null(), platdata_size,
>                         devp);
> +

drop blank line

> +       if (ret)
> +               return ret;
> +
> +#if CONFIG_IS_ENABLED(OF_PLATDATA)
> +       info->dev = *devp;
> +#endif
> +
> +       return ret;
> +

and here

>  }
>
>  static void *alloc_priv(int size, uint flags)
> @@ -727,6 +738,16 @@ 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(struct driver_info * info, struct udevice **devp)
> +{
> +       struct udevice *dev;
> +
> +       dev = info->dev;

blank line before return:-)

> +       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 14df16c280..8f47a6b356 100644
> --- a/drivers/core/root.c
> +++ b/drivers/core/root.c
> @@ -25,7 +25,7 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> -static const struct driver_info root_info = {
> +static struct driver_info root_info = {
>         .name           = "root_driver",
>  };
>
> @@ -346,6 +346,10 @@ int dm_init_and_scan(bool pre_reloc_only)
>  {
>         int ret;
>
> +#if CONFIG_IS_ENABLED(OF_PLATDATA)

This one can use if() I think

> +       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-internal.h b/include/dm/device-internal.h
> index 294d6c1810..5145fb4e14 100644
> --- a/include/dm/device-internal.h
> +++ b/include/dm/device-internal.h
> @@ -81,7 +81,7 @@ int device_bind_with_driver_data(struct udevice *parent,
>   * @return 0 if OK, -ve on error
>   */
>  int device_bind_by_name(struct udevice *parent, bool pre_reloc_only,
> -                       const struct driver_info *info, struct udevice **devp);
> +                       struct driver_info *info, struct udevice **devp);

That change should really be a separate patch I think.

>
>  /**
>   * device_ofdata_to_platdata() - Read platform data for a device
> diff --git a/include/dm/device.h b/include/dm/device.h
> index d5b3e732c3..590c1f99ce 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -537,6 +537,19 @@ 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
> + * @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(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..17eef7c7a3 100644
> --- a/include/dm/platdata.h
> +++ b/include/dm/platdata.h
> @@ -28,6 +28,7 @@ struct driver_info {
>         const void *platdata;
>  #if CONFIG_IS_ENABLED(OF_PLATDATA)
>         uint platdata_size;
> +       struct udevice *dev;

Please also update comment for this struct

>  #endif
>  };
>
> @@ -43,4 +44,9 @@ 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)
> +
> +void populate_phandle_data(void);

Function comment. Also can this ever fail?

>  #endif
> --
> 2.20.1
>

Regards,
SImon