[PATCH 05/10] core: extend struct driver_info to point to device

[Simon Glass]

Hi Walter,

On Fri, 29 May 2020 at 13:21, Walter Lozano <walter.lozano at 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 at 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 at 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