Hi Simon,
On 4/6/20 12:59, Simon Glass wrote:
Hi Walter,
On Fri, 29 May 2020 at 12:15, Walter Lozano <walter.loz...@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.loz...@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
