[PATCH 01/10] dtoc: add support to scan drivers
Walter Lozano
walter.lozano at collabora.com
Mon Jun 8 17:49:33 CEST 2020
Hi Simon,
On 4/6/20 12:59, Simon Glass wrote:
> Hi Walter,
>
> On Fri, 29 May 2020 at 12:15, 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 | 7 ++++
>> tools/dtoc/dtb_platdata.py | 83 ++++++++++++++++++++++++++++++++++++--
>> 2 files changed, 86 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/dm/device.h b/include/dm/device.h
>> index 975eec5d0e..2cfe10766f 100644
>> --- a/include/dm/device.h
>> +++ b/include/dm/device.h
>> @@ -282,6 +282,13 @@ 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
>> + * 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..23cfda2f88 100644
>> --- a/tools/dtoc/dtb_platdata.py
>> +++ b/tools/dtoc/dtb_platdata.py
>> @@ -13,6 +13,8 @@ static data.
>>
>> import collections
>> import copy
>> +import os
>> +import re
>> import sys
>>
>> from dtoc import fdt
>> @@ -140,6 +142,9 @@ class DtbPlatdata(object):
>> _include_disabled: true to include nodes marked status = "disabled"
>> _outfile: The current output file (sys.stdout or a real file)
>> _lines: Stashed list of output lines for outputting in the future
>> + _aliases: Dict that hold aliases for compatible strings
> key: The driver name, i.e. the part between brackets in U_BOOT_DRIVER(xx) ??
> value: ...
Noted.
>> + _drivers: List of valid driver names found in drivers/
>> + _driver_aliases: Dict that holds aliases for driver names
> key:
> vaue:
OK.
>
>> """
>> def __init__(self, dtb_fname, include_disabled):
>> self._fdt = None
>> @@ -149,6 +154,35 @@ class DtbPlatdata(object):
>> self._outfile = None
>> self._lines = []
>> self._aliases = {}
>> + self._drivers = []
>> + self._driver_aliases = {}
>> +
>> + def get_normalized_compat_name(self, node):
>> + """Get a node's normalized compat name
>> +
>> + Returns a valid driver name by retrieving node's first compatible
>> + string as a C identifier and perfomrming a check against _drivers
> performing
Noted.
>
>> + and a lookup in driver_aliases rising a warning in case of failure.
> s/ rising/, printing/
OK.
>
>> +
>> + Args:
>> + node: Node object to check
>> + Return:
>> + Tuple:
>> + Driver name associated with the first compatible string
>> + List of C identifiers for all the other compatible strings
>> + (possibly empty)
> Can you update this comment to explain what is returned when it is not found?
Sure.
>> + """
>> + compat_c, aliases_c = get_compat_name(node)
>> + if compat_c not in self._drivers:
>> + compat_c_old = compat_c
>> + compat_c = self._driver_aliases.get(compat_c)
>> + if not compat_c:
>> + print('WARNING: the driver %s was not found in the driver list' % (compat_c_old))
> This creates lots of warnings at present. Either we need a patch to
> clean up the differences in the source code, or we need to disable the
> warning.
Regarding this, maybe we should have a list of driver names we don't
expect to support, like simple_bus. For this to work probably the best
approach is to have a config option similar to CONFIG_OF_REMOVE_PROPS,
so each config can add their owns.
> Also the warning is not really actionable. It needs to add mention of
> U_BOOT_DEVICE and ...ALIAS.
I agree with you. Thanks.
>
> In future we can scan the compatible strings and tell the user what
> changes to make, I suppose.
Yes, it will be a great improvement!
>
>> + compat_c = compat_c_old
>> + else: # pragma: no cover
> Need to fix the coverage here
Noted. I will a few more tests.
>
>> + aliases_c = [compat_c_old] + aliases_c
>> +
>> + return compat_c, aliases_c
>>
>> def setup_output(self, fname):
>> """Set up the output destination
>> @@ -243,6 +277,46 @@ class DtbPlatdata(object):
>> return PhandleInfo(max_args, args)
>> return None
>>
>> + def scan_driver(self, fn):
>> + """Scan a driver file to build a list of driver names and aliases
>> +
>> + This procedure will populate self._drivers and self._driver_aliases
>> +
>> + Args
>> + fn: Driver filename to scan
>> + """
>> + with open(fn) as fd:
>> +
> drop blank line
OK.
>> + buff = fd.read()
>> +
>> + # The following re will search for driver names declared as
>> + # U_BOOT_DRIVER(driver_name)
>> + drivers = re.findall('U_BOOT_DRIVER\((.*)\)', buff)
>> +
>> + for driver in drivers:
>> + self._drivers.append(driver)
>> +
>> + # The following re will search for driver aliases declared as
>> + # U_BOOT_DRIVER_ALIAS(alias, driver_name)
>> + driver_aliases = re.findall('U_BOOT_DRIVER_ALIAS\(\s*(\w+)\s*,\s*(\w+)\s*\)', buff)
>> +
>> + for alias in driver_aliases: # pragma: no cover
>> + if len(alias) != 2:
>> + continue
>> + self._driver_aliases[alias[1]] = alias[0]
>> +
>> + def scan_drivers(self):
>> + """Scan the driver folders to build a list of driver names and aliases
>> +
>> + This procedure will populate self._drivers and self._driver_aliases
>> +
>> + """
>> + for (dirpath, dirnames, filenames) in os.walk('./'):
> This doesn't work for out-of-tree cases (make O=xxx). You may need to
> pass $(srctree) to dtc as an argument.
Thanks for pointing at this.
>> + 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 +427,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 = {}
>>
>> # Get a list of all the valid properties in this node.
>> @@ -377,14 +451,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 +535,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 +636,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
>>
Thanks once again for your review
Regards,
Walter
More information about the U-Boot
mailing list