From mboxrd@z Thu Jan 1 00:00:00 1970 From: Walter Lozano Date: Mon, 8 Jun 2020 12:49:33 -0300 Subject: [PATCH 01/10] dtoc: add support to scan drivers In-Reply-To: References: <20200529181521.22073-1-walter.lozano@collabora.com> <20200529181521.22073-2-walter.lozano@collabora.com> Message-ID: <023aedfc-cc83-ee7a-41eb-23238cb19e14@collabora.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, On 4/6/20 12:59, Simon Glass wrote: > Hi Walter, > > On Fri, 29 May 2020 at 12:15, Walter Lozano 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 >> --- >> 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