[RFC 2/6] core: extend struct driver_info to point to device

[Simon Glass]

Hi Walter,

On Thu, 21 May 2020 at 07:15, Walter Lozano <walter.lozano at collabora.com> wrote:
>
> 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(-)
> >>
[..]

> >> @@ -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?

Hopefully not. The linker should drop code that is not called, so it
won't become an issue.

>
> >> +       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.

Yes I just mean that here you are taking away the 'const' and I think
that little bit should be in its own patch. So take away the const,
but do nothing else.

[..]

Regards.

Simon