[RFC 2/6] core: extend struct driver_info to point to device
Walter Lozano
walter.lozano at collabora.com
Thu May 21 15:15:49 CEST 2020
Hi Simon,
On 20/5/20 00:07, Simon Glass wrote:
> 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
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
More information about the U-Boot
mailing list