Hi Walter,
On Thu, 21 May 2020 at 07:15, Walter Lozano <walter.loz...@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.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(-)
> >>
[..]
> >> @@ -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
