Hi Walter,
On Fri, 29 May 2020 at 12:15, Walter Lozano <walter.loz...@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.loz...@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 <s...@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
