| Message ID | 20200529181521.22073-6-walter.lozano@collabora.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | improve OF_PLATDATA support | expand |
On 29/5/20 15:15, Walter Lozano 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@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(-) > > 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; > > 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 I have tried to test this using sandbox_spl_defconfig but I've received a segmentation fault when trying to update info->dev, however this code works on iMX6. Could it be some kind of protection? Any thoughts? > + > + 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 > + * @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); > #endif
Hi Walter, On Fri, 29 May 2020 at 12:56, Walter Lozano <walter.lozano@collabora.com> wrote: > > > On 29/5/20 15:15, Walter Lozano 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@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(-) > > > > 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; > > > > 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 > > I have tried to test this using sandbox_spl_defconfig but I've received > a segmentation fault when trying to update info->dev, however this code > works on iMX6. > > Could it be some kind of protection? Any thoughts? Yes, see u-boot-dm/dtoc-working - arch/sandbox/cpu/u-boot-spl.lds has an attempt to move some of the list stuff into the data region. [..] Regards, Simon
Hi Simon, On 29/5/20 16:00, Simon Glass wrote: > Hi Walter, > > On Fri, 29 May 2020 at 12:56, Walter Lozano <walter.lozano@collabora.com> wrote: >> >> On 29/5/20 15:15, Walter Lozano 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@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(-) >>> >>> 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; >>> >>> 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 >> I have tried to test this using sandbox_spl_defconfig but I've received >> a segmentation fault when trying to update info->dev, however this code >> works on iMX6. >> >> Could it be some kind of protection? Any thoughts? > Yes, see u-boot-dm/dtoc-working - arch/sandbox/cpu/u-boot-spl.lds has > an attempt to move some of the list stuff into the data region. Thanks for confirmed it and also for the quick response. I'm about to start a deeper review to your work about tiny-dm now. Regards, Walter
Hi Walter, On Fri, 29 May 2020 at 13:21, Walter Lozano <walter.lozano@collabora.com> wrote: > > Hi Simon, > > On 29/5/20 16:00, Simon Glass wrote: > > Hi Walter, > > > > On Fri, 29 May 2020 at 12:56, Walter Lozano <walter.lozano@collabora.com> wrote: > >> > >> On 29/5/20 15:15, Walter Lozano 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@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(-) > >>> > >>> 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; > >>> > >>> 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 > >> I have tried to test this using sandbox_spl_defconfig but I've received > >> a segmentation fault when trying to update info->dev, however this code > >> works on iMX6. > >> > >> Could it be some kind of protection? Any thoughts? > > Yes, see u-boot-dm/dtoc-working - arch/sandbox/cpu/u-boot-spl.lds has > > an attempt to move some of the list stuff into the data region. > > Thanks for confirmed it and also for the quick response. I'm about to > start a deeper review to your work about tiny-dm now. OK, but don't take too much notice of it as it is very rough. One other thing that might be interesting is that I found a way to put links in data structures: #define DM_DECL_TINY_DRIVER(__name) \ ll_entry_decl(struct tiny_drv, __name, tiny_drv) /* * Get a pointer to a given tiny driver, for use in data structures. This * requires that the symbol be declared with DM_DECL_TINY_DRIVER() first */ #define DM_REF_TINY_DRIVER(__name) \ ll_entry_ref(struct tiny_drv, __name, tiny_drv) Regards, Simon
Hi Walter, On Fri, 29 May 2020 at 12:15, Walter Lozano <walter.lozano@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@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@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
On 4/6/20 12:59, Simon Glass wrote: > Hi Walter, > > On Fri, 29 May 2020 at 12:15, Walter Lozano <walter.lozano@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@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@chromium.org> > > Nits below Thanks for taking the time! >> 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 ? Noted! > >> 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. OK, I'll add a reference. >> + * @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() Sure! > >> #endif >> -- >> 2.20.1 >> Regards, Walter
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; 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 + * @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); #endif
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@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(-)