Hi Simon,
On 20/5/20 00:07, Simon Glass wrote:
Hi Walter,
On Wed, 13 May 2020 at 14:14, 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 | 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
Sure.
+ if (ret)
+ return ret;
+
+#if CONFIG_IS_ENABLED(OF_PLATDATA)
+ info->dev = *devp;
+#endif
+
+ return ret;
+
and here
Sure.
}
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:-)
Sure, thanks!
+ 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
I think that as populate_phandle_data() is only defined when OF_PLATDATA
is enabled (as it is created by dtoc), this will trigger and link error
on those board which don't use this config. Is my understanding correct?
+ 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.
If we move this change to a separate patch, this will fail to compile as
struct driver_info needs to be updated by device_bind_by_name to fill
info->dev. I can move both things to a different patch, in that case.
Please advice.
/**
* 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
Sure
#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?
It shouldn't. It basically does something like this
void populate_phandle_data(void) {
dtv_dmc_at_20020000.clocks[0].node =
DM_GET_DEVICE(clock_controller_at_20000000);
dtv_dmc_at_20020000.clocks[1].node =
DM_GET_DEVICE(clock_controller_at_20000000);
dtv_serial_at_20064000.clocks[0].node =
DM_GET_DEVICE(clock_controller_at_20000000);
dtv_serial_at_20064000.clocks[1].node =
DM_GET_DEVICE(clock_controller_at_20000000);
}
as we discuss previously, it is all about to be able to use DM_GET_DEVICE.
#endif
--
2.20.1
Regards,
Walter
