[RFC 1/6] dtoc: add support to scan drivers

[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:13, Walter Lozano<walter.lozano at collabora.com>  wrote:
> >> Currently dtoc scans dtbs to convert them to struct platdata and
> >> to generate U_BOOT_DEVICE entries. These entries need to be filled
> >> with the driver name, but at this moment the information used is the
> >> compatible name present in the dtb. This causes that only nodes with
> >> a compatible name that matches a driver name generate a working
> >> entry.
> >>
> >> In order to improve this behaviour, this patch adds to dtoc the
> >> capability of scan drivers source code to generate a list of valid driver
> >> names. This allows to rise a warning in the case that an U_BOOT_DEVICE
> >> entry will try to use a name not valid.
> >>
> >> Additionally, in order to add more flexibility to the solution, adds the
> >> U_BOOT_DRIVER_ALIAS macro, which generates no code at all, but allows an
> >> easy way to declare driver name aliases. Thanks to this, dtoc can look
> >> for the driver name based on its alias when it populates the U_BOOT_DEVICE entry.
> >>
> >> Signed-off-by: Walter Lozano<walter.lozano at collabora.com>
> >> ---
> >>   include/dm/device.h        |  6 +++++
> >>   tools/dtoc/dtb_platdata.py | 53 +++++++++++++++++++++++++++++++++++---
> >>   2 files changed, 55 insertions(+), 4 deletions(-)
> >>

[..]

> >> +
> >> +    def get_normalized_compat_name(self, node):
> > Needs function comment (also for those below).
> Sure, no problem.
> >> +        compat_c, aliases_c = get_compat_name(node)
> >> +        if compat_c not in self._drivers:
> >> +            try: # pragma: no cover
> > Instead of an exception can you use .get() and check for None?
> Sure.
> >> +                compat_c_old = compat_c
> >> +                compat_c = self._driver_aliases[compat_c]
> >> +                aliases_c = [compat_c_old] + aliases
> >> +            except:
> >> +                print('WARNING: the driver %s was not found in the driver list' % (compat_c))
>
> This is something I would like to check with you. As you can see, a
> warning is raised when a driver name is not found, which I think is the
> right thing to do at this point. However, this warning will be trigger
> but almost all the tests as the drivers names are no valid (there is no
> U_BOOT_DRIVER). How do you think is best to fix this?
>
> I see different possibilities here
>
> - To add specific driver names to self._drivers when running a test
>
> - To change driver names on tests to match a valid one
>
> Maybe you have a more clean way to deal with this.

You could add a new method 'set_test_mode()' dtb_platdata.py
You could add a flag to dtoc to disable the warning.
You could capture the output with test_util.capture_sys_output()

[..]

> >> @@ -353,7 +397,7 @@ class DtbPlatdata(object):
> >>           """
> >>           structs = {}
> >>           for node in self._valid_nodes:
> >> -            node_name, _ = get_compat_name(node)
> >> +            node_name, _ = self.get_normalized_compat_name(node)
> >>               fields = {}
> > I wonder how long this scan takes? Should we cache the results somewhere?
>
> Probably you are worried because this task will be repeated every time
> we compile U-boot, and it could be a potential issue when testing
> several different configs. However, at least in my setup the scan takes
> less than 1 sec, which I think is promising. Also I think that this scan
> is only perform for those configs which enable OF_PLATDATA, which are
> 15, should not cause much overhead.
>
> What do you think?

I think that is OK for now.

[..]

Regards,
Simon