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

[Simon Glass]

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(-)
>
> diff --git a/include/dm/device.h b/include/dm/device.h
> index 975eec5d0e..d5b3e732c3 100644
> --- a/include/dm/device.h
> +++ b/include/dm/device.h
> @@ -282,6 +282,12 @@ struct driver {
>  #define DM_GET_DRIVER(__name)                                          \
>         ll_entry_get(struct driver, __name, driver)
>
> +/* Declare a macro to state a alias for a driver name. This macro will

/*
 * Declare ...

> + * produce no code but its information will be parsed by tools like
> + * dtoc
> + */
> +#define U_BOOT_DRIVER_ALIAS(__name, __alias)
> +
>  /**
>   * dev_get_platdata() - Get the platform data for a device
>   *
> diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
> index ecfe0624d1..3f899a8bac 100644
> --- a/tools/dtoc/dtb_platdata.py
> +++ b/tools/dtoc/dtb_platdata.py
> @@ -14,6 +14,8 @@ static data.
>  import collections
>  import copy
>  import sys
> +import os
> +import re

please keep sorted

>
>  from dtoc import fdt
>  from dtoc import fdt_util
> @@ -149,6 +151,20 @@ class DtbPlatdata(object):
>          self._outfile = None
>          self._lines = []
>          self._aliases = {}
> +        self._drivers = []
> +        self._driver_aliases = {}

Please update comment above for what these new things are for.

> +
> +    def get_normalized_compat_name(self, node):

Needs function comment (also for those below).

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

> +                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))
> +
> +        return compat_c, aliases_c
>
>      def setup_output(self, fname):
>          """Set up the output destination
> @@ -243,6 +259,34 @@ class DtbPlatdata(object):
>              return PhandleInfo(max_args, args)
>          return None
>
> +    def scan_driver(self, fn):
> +        f = open(fn)

Can you avoid single-char var names? Also how about either using
tools.ReadFile() or:

with open(fname) as fd:
    ...rest of function

> +
> +        b = f.read()
> +
> +        drivers = re.findall('U_BOOT_DRIVER\((.*)\)', b)
> +
> +        for d in drivers:
> +            self._drivers.append(d)
> +
> +        driver_aliases = re.findall('U_BOOT_DRIVER_ALIAS\(\s*(\w+)\s*,\s*(\w+)\s*\)', b)

Can you add a comment before this with an example of the line you are
parsing? It helps make sense of the regex.

> +
> +        for a in driver_aliases: # pragma: no cover
> +            try:
> +                self._driver_aliases[a[1]] = a[0]
> +            except:
> +                pass

Again please avoid exceptions.

> +
> +    def scan_drivers(self):
> +        """Scan the driver folders to build a list of driver names and possible
> +        aliases
> +        """
> +        for (dirpath, dirnames, filenames) in os.walk('./'):
> +            for fn in filenames:
> +                if not fn.endswith('.c'):
> +                    continue
> +                self.scan_driver(dirpath + '/' + fn)
> +
>      def scan_dtb(self):
>          """Scan the device tree to obtain a tree of nodes and properties
>
> @@ -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?

>
>              # Get a list of all the valid properties in this node.
> @@ -377,14 +421,14 @@ class DtbPlatdata(object):
>
>          upto = 0
>          for node in self._valid_nodes:
> -            node_name, _ = get_compat_name(node)
> +            node_name, _ = self.get_normalized_compat_name(node)
>              struct = structs[node_name]
>              for name, prop in node.props.items():
>                  if name not in PROP_IGNORE_LIST and name[0] != '#':
>                      prop.Widen(struct[name])
>              upto += 1
>
> -            struct_name, aliases = get_compat_name(node)
> +            struct_name, aliases = self.get_normalized_compat_name(node)
>              for alias in aliases:
>                  self._aliases[alias] = struct_name
>
> @@ -461,7 +505,7 @@ class DtbPlatdata(object):
>          Args:
>              node: node to output
>          """
> -        struct_name, _ = get_compat_name(node)
> +        struct_name, _ = self.get_normalized_compat_name(node)
>          var_name = conv_name_to_c(node.name)
>          self.buf('static const struct %s%s %s%s = {\n' %
>                   (STRUCT_PREFIX, struct_name, VAL_PREFIX, var_name))
> @@ -562,6 +606,7 @@ def run_steps(args, dtb_file, include_disabled, output):
>          raise ValueError('Please specify a command: struct, platdata')
>
>      plat = DtbPlatdata(dtb_file, include_disabled)
> +    plat.scan_drivers()
>      plat.scan_dtb()
>      plat.scan_tree()
>      plat.scan_reg_sizes()
> --
> 2.20.1
>

Regards,
SImon