* [PATCH 0/8] dtoc: Improvements to warnings
@ 2021-07-04 18:19 Simon Glass
2021-07-04 18:19 ` [PATCH 1/8] dtoc: Avoid using subscripts on match objects Simon Glass
` (9 more replies)
0 siblings, 10 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-04 18:19 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Johan Jonker, Simon Glass, Walter Lozano
The warning about a missing driver can be a bit confusing and is not
specifically mentioned in the documentation. Also some parse errors result
in the driver simply being ignored, which is hard to diagnose.
This series updates dtoc to try harder to find common errors and report a
useful warning. It also adds a section about possible problems to the
of-platdata documentation.
Unfortunately the use of match-object subscripts is not supported in
Python 3.5 and earlier, so a patch is included to avoid this.
Finally, a short usage string it added to the help output, with dtoc being
converted to use ArgumentParser in the process.
Simon Glass (8):
dtoc: Avoid using subscripts on match objects
dtoc: Convert to use ArgumentParser
dtoc: Allow multiple warnings for a driver
dtoc: Correct the re_compat regular expression
dtoc: Add a stdout check in test_normalized_name()
dtoc: Detect unexpected suffix on .of_match
dtoc: Detect drivers which do not parse correctly
dtoc: Update documentation to cover warnings in more detail
doc/develop/driver-model/of-plat.rst | 53 +++++++++++
tools/dtoc/main.py | 51 +++++-----
tools/dtoc/src_scan.py | 45 +++++++--
tools/dtoc/test_src_scan.py | 133 ++++++++++++++++++++++++++-
4 files changed, 247 insertions(+), 35 deletions(-)
--
2.32.0.93.g670b81a890-goog
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/8] dtoc: Avoid using subscripts on match objects
2021-07-04 18:19 [PATCH 0/8] dtoc: Improvements to warnings Simon Glass
@ 2021-07-04 18:19 ` Simon Glass
2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
2021-07-04 18:19 ` [PATCH 2/8] dtoc: Convert to use ArgumentParser Simon Glass
` (8 subsequent siblings)
9 siblings, 2 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-04 18:19 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Johan Jonker, Simon Glass
These are not supported before Python 3.6 so avoid them.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/dtoc/src_scan.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py
index 2db96884c85..1dbb56712a3 100644
--- a/tools/dtoc/src_scan.py
+++ b/tools/dtoc/src_scan.py
@@ -555,7 +555,7 @@ class Scanner:
if ids_m:
ids_name = ids_m.group(1)
elif m_alias:
- self._driver_aliases[m_alias[2]] = m_alias[1]
+ self._driver_aliases[m_alias.group(2)] = m_alias.group(1)
# Make the updates based on what we found
for driver in drivers.values():
--
2.32.0.93.g670b81a890-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/8] dtoc: Convert to use ArgumentParser
2021-07-04 18:19 [PATCH 0/8] dtoc: Improvements to warnings Simon Glass
2021-07-04 18:19 ` [PATCH 1/8] dtoc: Avoid using subscripts on match objects Simon Glass
@ 2021-07-04 18:19 ` Simon Glass
2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
2021-07-04 18:19 ` [PATCH 3/8] dtoc: Allow multiple warnings for a driver Simon Glass
` (7 subsequent siblings)
9 siblings, 2 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-04 18:19 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Johan Jonker, Simon Glass
Use this parser instead of OptionParser, which is deprecated.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/dtoc/main.py | 51 ++++++++++++++++++++++++----------------------
1 file changed, 27 insertions(+), 24 deletions(-)
diff --git a/tools/dtoc/main.py b/tools/dtoc/main.py
index 93706de89bf..6f9b526bd74 100755
--- a/tools/dtoc/main.py
+++ b/tools/dtoc/main.py
@@ -21,7 +21,7 @@ options. For more information about the use of this options and tool please
see doc/driver-model/of-plat.rst
"""
-from optparse import OptionParser
+from argparse import ArgumentParser
import os
import sys
import unittest
@@ -51,7 +51,7 @@ def run_tests(processes, args):
result = unittest.TestResult()
sys.argv = [sys.argv[0]]
- test_name = args and args[0] or None
+ test_name = args.files and args.files[0] or None
test_dtoc.setup()
@@ -66,47 +66,50 @@ def RunTestCoverage():
"""Run the tests and check that we get 100% coverage"""
sys.argv = [sys.argv[0]]
test_util.RunTestCoverage('tools/dtoc/dtoc', '/main.py',
- ['tools/patman/*.py', '*/fdt*', '*test*'], options.build_dir)
+ ['tools/patman/*.py', '*/fdt*', '*test*'], args.build_dir)
if __name__ != '__main__':
sys.exit(1)
-parser = OptionParser()
-parser.add_option('-B', '--build-dir', type='string', default='b',
+epilog = '''Generate C code from devicetree files. See of-plat.rst for details'''
+
+parser = ArgumentParser(epilog=epilog)
+parser.add_argument('-B', '--build-dir', type=str, default='b',
help='Directory containing the build output')
-parser.add_option('-c', '--c-output-dir', action='store',
+parser.add_argument('-c', '--c-output-dir', action='store',
help='Select output directory for C files')
-parser.add_option('-C', '--h-output-dir', action='store',
+parser.add_argument('-C', '--h-output-dir', action='store',
help='Select output directory for H files (defaults to --c-output-di)')
-parser.add_option('-d', '--dtb-file', action='store',
+parser.add_argument('-d', '--dtb-file', action='store',
help='Specify the .dtb input file')
-parser.add_option('-i', '--instantiate', action='store_true', default=False,
+parser.add_argument('-i', '--instantiate', action='store_true', default=False,
help='Instantiate devices to avoid needing device_bind()')
-parser.add_option('--include-disabled', action='store_true',
+parser.add_argument('--include-disabled', action='store_true',
help='Include disabled nodes')
-parser.add_option('-o', '--output', action='store',
+parser.add_argument('-o', '--output', action='store',
help='Select output filename')
-parser.add_option('-p', '--phase', type=str,
+parser.add_argument('-p', '--phase', type=str,
help='set phase of U-Boot this invocation is for (spl/tpl)')
-parser.add_option('-P', '--processes', type=int,
+parser.add_argument('-P', '--processes', type=int,
help='set number of processes to use for running tests')
-parser.add_option('-t', '--test', action='store_true', dest='test',
+parser.add_argument('-t', '--test', action='store_true', dest='test',
default=False, help='run tests')
-parser.add_option('-T', '--test-coverage', action='store_true',
- default=False, help='run tests and check for 100% coverage')
-(options, args) = parser.parse_args()
+parser.add_argument('-T', '--test-coverage', action='store_true',
+ default=False, help='run tests and check for 100%% coverage')
+parser.add_argument('files', nargs='*')
+args = parser.parse_args()
# Run our meagre tests
-if options.test:
- ret_code = run_tests(options.processes, args)
+if args.test:
+ ret_code = run_tests(args.processes, args)
sys.exit(ret_code)
-elif options.test_coverage:
+elif args.test_coverage:
RunTestCoverage()
else:
- dtb_platdata.run_steps(args, options.dtb_file, options.include_disabled,
- options.output,
- [options.c_output_dir, options.h_output_dir],
- options.phase, instantiate=options.instantiate)
+ dtb_platdata.run_steps(args.files, args.dtb_file, args.include_disabled,
+ args.output,
+ [args.c_output_dir, args.h_output_dir],
+ args.phase, instantiate=args.instantiate)
--
2.32.0.93.g670b81a890-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/8] dtoc: Allow multiple warnings for a driver
2021-07-04 18:19 [PATCH 0/8] dtoc: Improvements to warnings Simon Glass
2021-07-04 18:19 ` [PATCH 1/8] dtoc: Avoid using subscripts on match objects Simon Glass
2021-07-04 18:19 ` [PATCH 2/8] dtoc: Convert to use ArgumentParser Simon Glass
@ 2021-07-04 18:19 ` Simon Glass
2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
2021-07-04 18:19 ` [PATCH 4/8] dtoc: Correct the re_compat regular expression Simon Glass
` (6 subsequent siblings)
9 siblings, 2 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-04 18:19 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Johan Jonker, Simon Glass
At present we show when a driver is missing but this is not always that
useful. There are various reasons way a driver may appear to be missing,
such as a parse error in the source code or a missing field in the driver
declaration.
Update the implementation to record all warnings for each driver, showing
only those which relate to drivers that are actually used. This avoids
spamming the user with warnings related to a driver for a different board.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/dtoc/src_scan.py | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py
index 1dbb56712a3..6c37a71e978 100644
--- a/tools/dtoc/src_scan.py
+++ b/tools/dtoc/src_scan.py
@@ -13,6 +13,7 @@ U_BOOT_DRIVER(), UCLASS_DRIVER and all struct declarations in header files.
See doc/driver-model/of-plat.rst for more informaiton
"""
+import collections
import os
import re
import sys
@@ -190,6 +191,9 @@ class Scanner:
value: Driver name declared with U_BOOT_DRIVER(driver_name)
_drivers_additional (list or str): List of additional drivers to use
during scanning
+ _warnings: Dict of warnings found:
+ key: Driver name
+ value: Set of warnings
_of_match: Dict holding information about compatible strings
key: Name of struct udevice_id variable
value: Dict of compatible info in that variable:
@@ -217,6 +221,7 @@ class Scanner:
self._driver_aliases = {}
self._drivers_additional = drivers_additional or []
self._missing_drivers = set()
+ self._warnings = collections.defaultdict(set)
self._of_match = {}
self._compat_to_driver = {}
self._uclass = {}
@@ -267,7 +272,10 @@ class Scanner:
aliases_c.remove(compat_c)
return compat_c, aliases_c
- self._missing_drivers.add(compat_list_c[0])
+ name = compat_list_c[0]
+ self._missing_drivers.add(name)
+ self._warnings[name].add(
+ 'WARNING: the driver %s was not found in the driver list' % name)
return compat_list_c[0], compat_list_c[1:]
@@ -577,9 +585,17 @@ class Scanner:
def show_warnings(self):
"""Show any warnings that have been collected"""
- for name in sorted(list(self._missing_drivers)):
- print('WARNING: the driver %s was not found in the driver list'
- % name)
+ used_drivers = [drv.name for drv in self._drivers.values() if drv.used]
+ missing = self._missing_drivers
+ for name in sorted(self._warnings.keys()):
+ if name in missing or name in used_drivers:
+ warns = sorted(list(self._warnings[name]))
+ # For now there is only ever one warning
+ print('%s: %s' % (name, warns[0]))
+ indent = ' ' * len(name)
+ if name in missing:
+ missing.remove(name)
+ print()
def scan_driver(self, fname):
"""Scan a driver file to build a list of driver names and aliases
--
2.32.0.93.g670b81a890-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/8] dtoc: Correct the re_compat regular expression
2021-07-04 18:19 [PATCH 0/8] dtoc: Improvements to warnings Simon Glass
` (2 preceding siblings ...)
2021-07-04 18:19 ` [PATCH 3/8] dtoc: Allow multiple warnings for a driver Simon Glass
@ 2021-07-04 18:19 ` Simon Glass
2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
2021-07-04 18:19 ` [PATCH 5/8] dtoc: Add a stdout check in test_normalized_name() Simon Glass
` (5 subsequent siblings)
9 siblings, 2 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-04 18:19 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Johan Jonker, Simon Glass
This expects a . before the field name (.e.g '.compatible = ...) but
presently accepts anything at all. Fix it.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/dtoc/src_scan.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py
index 6c37a71e978..6e8e1ba51a0 100644
--- a/tools/dtoc/src_scan.py
+++ b/tools/dtoc/src_scan.py
@@ -452,8 +452,8 @@ class Scanner:
# Collect the compatible string, e.g. 'rockchip,rk3288-grf'
compat = None
- re_compat = re.compile(r'{\s*.compatible\s*=\s*"(.*)"\s*'
- r'(,\s*.data\s*=\s*(\S*))?\s*},')
+ re_compat = re.compile(r'{\s*\.compatible\s*=\s*"(.*)"\s*'
+ r'(,\s*\.data\s*=\s*(\S*))?\s*},')
# This is a dict of compatible strings that were found:
# key: Compatible string, e.g. 'rockchip,rk3288-grf'
--
2.32.0.93.g670b81a890-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 5/8] dtoc: Add a stdout check in test_normalized_name()
2021-07-04 18:19 [PATCH 0/8] dtoc: Improvements to warnings Simon Glass
` (3 preceding siblings ...)
2021-07-04 18:19 ` [PATCH 4/8] dtoc: Correct the re_compat regular expression Simon Glass
@ 2021-07-04 18:19 ` Simon Glass
2021-07-04 18:19 ` [PATCH 6/8] dtoc: Detect unexpected suffix on .of_match Simon Glass
` (4 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-04 18:19 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Johan Jonker, Simon Glass
This test captures output but does not always check it. Add the missing
code and drop the old comment.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/dtoc/test_src_scan.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tools/dtoc/test_src_scan.py b/tools/dtoc/test_src_scan.py
index d6da03849f9..4e38b25a2f8 100644
--- a/tools/dtoc/test_src_scan.py
+++ b/tools/dtoc/test_src_scan.py
@@ -171,8 +171,7 @@ class TestSrcScan(unittest.TestCase):
self.assertEqual([], aliases)
self.assertEqual(1, len(scan._missing_drivers))
self.assertEqual({'rockchip_rk3288_grf'}, scan._missing_drivers)
- #'WARNING: the driver rockchip_rk3288_grf was not found in the driver list',
- #stdout.getvalue().strip())
+ self.assertEqual('', stdout.getvalue().strip())
i2c = 'I2C_UCLASS'
compat = {'rockchip,rk3288-grf': 'ROCKCHIP_SYSCON_GRF',
--
2.32.0.93.g670b81a890-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 6/8] dtoc: Detect unexpected suffix on .of_match
2021-07-04 18:19 [PATCH 0/8] dtoc: Improvements to warnings Simon Glass
` (4 preceding siblings ...)
2021-07-04 18:19 ` [PATCH 5/8] dtoc: Add a stdout check in test_normalized_name() Simon Glass
@ 2021-07-04 18:19 ` Simon Glass
2021-07-04 18:19 ` [PATCH 7/8] dtoc: Detect drivers which do not parse correctly Simon Glass
` (3 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-04 18:19 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Johan Jonker, Simon Glass
Some rockchip drivers use a suffix on the of_match line which is not
strictly valid. At present this causes the parsing to fail. Fix this
and offer a warning.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/dtoc/src_scan.py | 12 +++--
tools/dtoc/test_src_scan.py | 92 +++++++++++++++++++++++++++++++++++++
2 files changed, 101 insertions(+), 3 deletions(-)
diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py
index 6e8e1ba51a0..847677757d9 100644
--- a/tools/dtoc/src_scan.py
+++ b/tools/dtoc/src_scan.py
@@ -468,7 +468,7 @@ class Scanner:
# Matches the references to the udevice_id list
re_of_match = re.compile(
- r'\.of_match\s*=\s*(of_match_ptr\()?([a-z0-9_]+)(\))?,')
+ r'\.of_match\s*=\s*(of_match_ptr\()?([a-z0-9_]+)([^,]*),')
re_phase = re.compile('^\s*DM_PHASE\((.*)\).*$')
re_hdr = re.compile('^\s*DM_HEADER\((.*)\).*$')
@@ -514,6 +514,11 @@ class Scanner:
driver.uclass_id = m_id.group(1)
elif m_of_match:
compat = m_of_match.group(2)
+ suffix = m_of_match.group(3)
+ if suffix and suffix != ')':
+ self._warnings[driver.name].add(
+ "%s: Warning: unexpected suffix '%s' on .of_match line for compat '%s'" %
+ (fname, suffix, compat))
elif m_phase:
driver.phase = m_phase.group(1)
elif m_hdr:
@@ -586,13 +591,14 @@ class Scanner:
def show_warnings(self):
"""Show any warnings that have been collected"""
used_drivers = [drv.name for drv in self._drivers.values() if drv.used]
- missing = self._missing_drivers
+ missing = self._missing_drivers.copy()
for name in sorted(self._warnings.keys()):
if name in missing or name in used_drivers:
warns = sorted(list(self._warnings[name]))
- # For now there is only ever one warning
print('%s: %s' % (name, warns[0]))
indent = ' ' * len(name)
+ for warn in warns[1:]:
+ print('%-s: %s' % (indent, warn))
if name in missing:
missing.remove(name)
print()
diff --git a/tools/dtoc/test_src_scan.py b/tools/dtoc/test_src_scan.py
index 4e38b25a2f8..62500e80fe7 100644
--- a/tools/dtoc/test_src_scan.py
+++ b/tools/dtoc/test_src_scan.py
@@ -20,6 +20,9 @@ from patman import tools
OUR_PATH = os.path.dirname(os.path.realpath(__file__))
+EXPECT_WARN = {'rockchip_rk3288_grf':
+ {'WARNING: the driver rockchip_rk3288_grf was not found in the driver list'}}
+
class FakeNode:
"""Fake Node object for testing"""
def __init__(self):
@@ -152,6 +155,7 @@ class TestSrcScan(unittest.TestCase):
'nvidia,tegra20-i2c-dvc': 'TYPE_DVC'}, drv.compat)
self.assertEqual('i2c_bus', drv.priv)
self.assertEqual(1, len(scan._drivers))
+ self.assertEqual({}, scan._warnings)
def test_normalized_name(self):
"""Test operation of get_normalized_compat_name()"""
@@ -172,6 +176,7 @@ class TestSrcScan(unittest.TestCase):
self.assertEqual(1, len(scan._missing_drivers))
self.assertEqual({'rockchip_rk3288_grf'}, scan._missing_drivers)
self.assertEqual('', stdout.getvalue().strip())
+ self.assertEqual(EXPECT_WARN, scan._warnings)
i2c = 'I2C_UCLASS'
compat = {'rockchip,rk3288-grf': 'ROCKCHIP_SYSCON_GRF',
@@ -188,6 +193,7 @@ class TestSrcScan(unittest.TestCase):
self.assertEqual('', stdout.getvalue().strip())
self.assertEqual('rockchip_rk3288_grf', name)
self.assertEqual([], aliases)
+ self.assertEqual(EXPECT_WARN, scan._warnings)
prop.value = 'rockchip,rk3288-srf'
with test_util.capture_sys_output() as (stdout, _):
@@ -195,6 +201,7 @@ class TestSrcScan(unittest.TestCase):
self.assertEqual('', stdout.getvalue().strip())
self.assertEqual('rockchip_rk3288_grf', name)
self.assertEqual(['rockchip_rk3288_srf'], aliases)
+ self.assertEqual(EXPECT_WARN, scan._warnings)
def test_scan_errors(self):
"""Test detection of scanning errors"""
@@ -489,3 +496,88 @@ U_BOOT_DRIVER(%s) = {
self.assertEqual(3, seq)
self.assertEqual({'mypath': 3}, uc.alias_path_to_num)
self.assertEqual({2: node, 3: node}, uc.alias_num_to_node)
+
+ def test_scan_warnings(self):
+ """Test detection of scanning warnings"""
+ buff = '''
+static const struct udevice_id tegra_i2c_ids[] = {
+ { .compatible = "nvidia,tegra114-i2c", .data = TYPE_114 },
+ { }
+};
+
+U_BOOT_DRIVER(i2c_tegra) = {
+ .name = "i2c_tegra",
+ .id = UCLASS_I2C,
+ .of_match = tegra_i2c_ids + 1,
+};
+'''
+ # The '+ 1' above should generate a warning
+
+ prop = FakeProp()
+ prop.name = 'compatible'
+ prop.value = 'rockchip,rk3288-grf'
+ node = FakeNode()
+ node.props = {'compatible': prop}
+
+ # get_normalized_compat_name() uses this to check for root node
+ node.parent = FakeNode()
+
+ scan = src_scan.Scanner(None, None)
+ scan._parse_driver('file.c', buff)
+ self.assertEqual(
+ {'i2c_tegra':
+ {"file.c: Warning: unexpected suffix ' + 1' on .of_match line for compat 'tegra_i2c_ids'"}},
+ scan._warnings)
+
+ tprop = FakeProp()
+ tprop.name = 'compatible'
+ tprop.value = 'nvidia,tegra114-i2c'
+ tnode = FakeNode()
+ tnode.props = {'compatible': tprop}
+
+ # get_normalized_compat_name() uses this to check for root node
+ tnode.parent = FakeNode()
+
+ with test_util.capture_sys_output() as (stdout, _):
+ scan.get_normalized_compat_name(node)
+ scan.get_normalized_compat_name(tnode)
+ self.assertEqual('', stdout.getvalue().strip())
+
+ self.assertEqual(2, len(scan._missing_drivers))
+ self.assertEqual({'rockchip_rk3288_grf', 'nvidia_tegra114_i2c'},
+ scan._missing_drivers)
+ with test_util.capture_sys_output() as (stdout, _):
+ scan.show_warnings()
+ self.assertIn('rockchip_rk3288_grf', stdout.getvalue())
+
+ # This should show just the rockchip warning, since the tegra driver
+ # is not in self._missing_drivers
+ scan._missing_drivers.remove('nvidia_tegra114_i2c')
+ with test_util.capture_sys_output() as (stdout, _):
+ scan.show_warnings()
+ self.assertIn('rockchip_rk3288_grf', stdout.getvalue())
+ self.assertNotIn('tegra_i2c_ids', stdout.getvalue())
+
+ # Do a similar thing with used drivers. By marking the tegra driver as
+ # used, the warning related to that driver will be shown
+ drv = scan._drivers['i2c_tegra']
+ drv.used = True
+ with test_util.capture_sys_output() as (stdout, _):
+ scan.show_warnings()
+ self.assertIn('rockchip_rk3288_grf', stdout.getvalue())
+ self.assertIn('tegra_i2c_ids', stdout.getvalue())
+
+ # Add a warning to make sure multiple warnings are shown
+ scan._warnings['i2c_tegra'].update(
+ scan._warnings['nvidia_tegra114_i2c'])
+ del scan._warnings['nvidia_tegra114_i2c']
+ with test_util.capture_sys_output() as (stdout, _):
+ scan.show_warnings()
+ self.assertEqual('''i2c_tegra: WARNING: the driver nvidia_tegra114_i2c was not found in the driver list
+ : file.c: Warning: unexpected suffix ' + 1' on .of_match line for compat 'tegra_i2c_ids'
+
+rockchip_rk3288_grf: WARNING: the driver rockchip_rk3288_grf was not found in the driver list
+
+''',
+ stdout.getvalue())
+ self.assertIn('tegra_i2c_ids', stdout.getvalue())
--
2.32.0.93.g670b81a890-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 7/8] dtoc: Detect drivers which do not parse correctly
2021-07-04 18:19 [PATCH 0/8] dtoc: Improvements to warnings Simon Glass
` (5 preceding siblings ...)
2021-07-04 18:19 ` [PATCH 6/8] dtoc: Detect unexpected suffix on .of_match Simon Glass
@ 2021-07-04 18:19 ` Simon Glass
2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
2021-07-04 18:19 ` [PATCH 8/8] dtoc: Update documentation to cover warnings in more detail Simon Glass
` (2 subsequent siblings)
9 siblings, 2 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-04 18:19 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Johan Jonker, Simon Glass
At present if a driver is missing a uclass or compatible stirng, this
is silently ignored. This makes sense in most cases, particularly for
the compatible string, since it is not required except when the driver
is used with of-platdata.
But it is also not very helpful. When there is some sort of problem
with a driver, the missing compatible string (for example) may be the
cause.
Add a warning in this case, showing it only for drivers which are used
by the build.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/dtoc/src_scan.py | 7 ++++++-
tools/dtoc/test_src_scan.py | 38 +++++++++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py
index 847677757d9..3bef59d616e 100644
--- a/tools/dtoc/src_scan.py
+++ b/tools/dtoc/src_scan.py
@@ -546,7 +546,12 @@ class Scanner:
# The driver does not have a uclass or compat string.
# The first is required but the second is not, so just
# ignore this.
- pass
+ if not driver.uclass_id:
+ warn = 'Missing .uclass'
+ else:
+ warn = 'Missing .compatible'
+ self._warnings[driver.name].add('%s in %s' %
+ (warn, fname))
driver = None
ids_name = None
compat = None
diff --git a/tools/dtoc/test_src_scan.py b/tools/dtoc/test_src_scan.py
index 62500e80fe7..f03cf8ed7c7 100644
--- a/tools/dtoc/test_src_scan.py
+++ b/tools/dtoc/test_src_scan.py
@@ -581,3 +581,41 @@ rockchip_rk3288_grf: WARNING: the driver rockchip_rk3288_grf was not found in th
''',
stdout.getvalue())
self.assertIn('tegra_i2c_ids', stdout.getvalue())
+
+ def scan_uclass_warning(self):
+ """Test a missing .uclass in the driver"""
+ buff = '''
+static const struct udevice_id tegra_i2c_ids[] = {
+ { .compatible = "nvidia,tegra114-i2c", .data = TYPE_114 },
+ { }
+};
+
+U_BOOT_DRIVER(i2c_tegra) = {
+ .name = "i2c_tegra",
+ .of_match = tegra_i2c_ids,
+};
+'''
+ scan = src_scan.Scanner(None, None)
+ scan._parse_driver('file.c', buff)
+ self.assertEqual(
+ {'i2c_tegra': {'Missing .uclass in file.c'}},
+ scan._warnings)
+
+ def scan_compat_warning(self):
+ """Test a missing .compatible in the driver"""
+ buff = '''
+static const struct udevice_id tegra_i2c_ids[] = {
+ { .compatible = "nvidia,tegra114-i2c", .data = TYPE_114 },
+ { }
+};
+
+U_BOOT_DRIVER(i2c_tegra) = {
+ .name = "i2c_tegra",
+ .id = UCLASS_I2C,
+};
+'''
+ scan = src_scan.Scanner(None, None)
+ scan._parse_driver('file.c', buff)
+ self.assertEqual(
+ {'i2c_tegra': {'Missing .compatible in file.c'}},
+ scan._warnings)
--
2.32.0.93.g670b81a890-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 8/8] dtoc: Update documentation to cover warnings in more detail
2021-07-04 18:19 [PATCH 0/8] dtoc: Improvements to warnings Simon Glass
` (6 preceding siblings ...)
2021-07-04 18:19 ` [PATCH 7/8] dtoc: Detect drivers which do not parse correctly Simon Glass
@ 2021-07-04 18:19 ` Simon Glass
2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
2021-07-11 23:00 ` [PATCH 6/8] dtoc: Detect unexpected suffix on .of_match Simon Glass
2021-07-11 23:00 ` [PATCH 5/8] dtoc: Add a stdout check in test_normalized_name() Simon Glass
9 siblings, 2 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-04 18:19 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Johan Jonker, Simon Glass, Walter Lozano
When things go wrong it can be confusing to figure out what to change.
Add a few more details to the documentation.
Fix a 'make htmldocs' warning while we are here.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
doc/develop/driver-model/of-plat.rst | 53 ++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/doc/develop/driver-model/of-plat.rst b/doc/develop/driver-model/of-plat.rst
index 74f1932473b..e2763965839 100644
--- a/doc/develop/driver-model/of-plat.rst
+++ b/doc/develop/driver-model/of-plat.rst
@@ -597,6 +597,59 @@ as a macro included in the driver definition::
+Problems
+--------
+
+In some cases you will you see something like this::
+
+ WARNING: the driver rockchip_rk3188_grf was not found in the driver list
+
+The driver list is a list of drivers, each with a name. The name is in the
+U_BOOT_DRIVER() declaration, repeated twice, one in brackets and once as the
+.name member. For example, in the following declaration the driver name is
+`rockchip_rk3188_grf`::
+
+ U_BOOT_DRIVER(rockchip_rk3188_grf) = {
+ .name = "rockchip_rk3188_grf",
+ .id = UCLASS_SYSCON,
+ .of_match = rk3188_syscon_ids + 1,
+ .bind = rk3188_syscon_bind_of_plat,
+ };
+
+The first name U_BOOT_DRIVER(xx) is used to create a linker symbol so that the
+driver can be accessed at build-time without any overhead. The second one
+(.name = "xx") is used at runtime when something wants to print out the driver
+name.
+
+The dtoc tool expects to be able to find a driver for each compatible string in
+the devicetree. For example, if the devicetree has::
+
+ grf: grf@20008000 {
+ compatible = "rockchip,rk3188-grf", "syscon";
+ reg = <0x20008000 0x200>;
+ u-boot,dm-spl;
+ };
+
+then dtoc looks at the first compatible string ("rockchip,rk3188-grf"),
+converts that to a C identifier (rockchip_rk3188_grf) and then looks for that.
+
+Various things can cause dtoc to fail to find the driver and it tries to
+warn about these. For example:
+
+ rockchip_rk3188_uart: Missing .compatible in drivers/serial/serial_rockchip.c
+ : WARNING: the driver rockchip_rk3188_uart was not found in the driver list
+
+Without a compatible string a driver cannot be used by dtoc, even if the
+compatible string is not actually needed at runtime.
+
+If the problem is simply that there are multiple compatible strings, the
+DM_DRIVER_ALIAS() macro can be used to tell dtoc about this and avoid a problem.
+
+Checks are also made the referenced driver has a .compatible member and a .id
+member. The first provides the array of compatible strings and the second
+provides the uclass ID.
+
+
Caveats
-------
--
2.32.0.93.g670b81a890-goog
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 1/8] dtoc: Avoid using subscripts on match objects
2021-07-04 18:19 ` [PATCH 1/8] dtoc: Avoid using subscripts on match objects Simon Glass
@ 2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
1 sibling, 0 replies; 25+ messages in thread
From: Walter Lozano @ 2021-07-05 18:04 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Johan Jonker
Hi Simon,
On 7/4/21 3:19 PM, Simon Glass wrote:
> These are not supported before Python 3.6 so avoid them.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> tools/dtoc/src_scan.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
Thanks!
Walter
> diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py
> index 2db96884c85..1dbb56712a3 100644
> --- a/tools/dtoc/src_scan.py
> +++ b/tools/dtoc/src_scan.py
> @@ -555,7 +555,7 @@ class Scanner:
> if ids_m:
> ids_name = ids_m.group(1)
> elif m_alias:
> - self._driver_aliases[m_alias[2]] = m_alias[1]
> + self._driver_aliases[m_alias.group(2)] = m_alias.group(1)
>
> # Make the updates based on what we found
> for driver in drivers.values():
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/8] dtoc: Convert to use ArgumentParser
2021-07-04 18:19 ` [PATCH 2/8] dtoc: Convert to use ArgumentParser Simon Glass
@ 2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
1 sibling, 0 replies; 25+ messages in thread
From: Walter Lozano @ 2021-07-05 18:04 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Johan Jonker
Hi Simon,
On 7/4/21 3:19 PM, Simon Glass wrote:
> Use this parser instead of OptionParser, which is deprecated.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> tools/dtoc/main.py | 51 ++++++++++++++++++++++++----------------------
> 1 file changed, 27 insertions(+), 24 deletions(-)
Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
Thanks!
Walter
>
> diff --git a/tools/dtoc/main.py b/tools/dtoc/main.py
> index 93706de89bf..6f9b526bd74 100755
> --- a/tools/dtoc/main.py
> +++ b/tools/dtoc/main.py
> @@ -21,7 +21,7 @@ options. For more information about the use of this options and tool please
> see doc/driver-model/of-plat.rst
> """
>
> -from optparse import OptionParser
> +from argparse import ArgumentParser
> import os
> import sys
> import unittest
> @@ -51,7 +51,7 @@ def run_tests(processes, args):
>
> result = unittest.TestResult()
> sys.argv = [sys.argv[0]]
> - test_name = args and args[0] or None
> + test_name = args.files and args.files[0] or None
>
> test_dtoc.setup()
>
> @@ -66,47 +66,50 @@ def RunTestCoverage():
> """Run the tests and check that we get 100% coverage"""
> sys.argv = [sys.argv[0]]
> test_util.RunTestCoverage('tools/dtoc/dtoc', '/main.py',
> - ['tools/patman/*.py', '*/fdt*', '*test*'], options.build_dir)
> + ['tools/patman/*.py', '*/fdt*', '*test*'], args.build_dir)
>
>
> if __name__ != '__main__':
> sys.exit(1)
>
> -parser = OptionParser()
> -parser.add_option('-B', '--build-dir', type='string', default='b',
> +epilog = '''Generate C code from devicetree files. See of-plat.rst for details'''
> +
> +parser = ArgumentParser(epilog=epilog)
> +parser.add_argument('-B', '--build-dir', type=str, default='b',
> help='Directory containing the build output')
> -parser.add_option('-c', '--c-output-dir', action='store',
> +parser.add_argument('-c', '--c-output-dir', action='store',
> help='Select output directory for C files')
> -parser.add_option('-C', '--h-output-dir', action='store',
> +parser.add_argument('-C', '--h-output-dir', action='store',
> help='Select output directory for H files (defaults to --c-output-di)')
> -parser.add_option('-d', '--dtb-file', action='store',
> +parser.add_argument('-d', '--dtb-file', action='store',
> help='Specify the .dtb input file')
> -parser.add_option('-i', '--instantiate', action='store_true', default=False,
> +parser.add_argument('-i', '--instantiate', action='store_true', default=False,
> help='Instantiate devices to avoid needing device_bind()')
> -parser.add_option('--include-disabled', action='store_true',
> +parser.add_argument('--include-disabled', action='store_true',
> help='Include disabled nodes')
> -parser.add_option('-o', '--output', action='store',
> +parser.add_argument('-o', '--output', action='store',
> help='Select output filename')
> -parser.add_option('-p', '--phase', type=str,
> +parser.add_argument('-p', '--phase', type=str,
> help='set phase of U-Boot this invocation is for (spl/tpl)')
> -parser.add_option('-P', '--processes', type=int,
> +parser.add_argument('-P', '--processes', type=int,
> help='set number of processes to use for running tests')
> -parser.add_option('-t', '--test', action='store_true', dest='test',
> +parser.add_argument('-t', '--test', action='store_true', dest='test',
> default=False, help='run tests')
> -parser.add_option('-T', '--test-coverage', action='store_true',
> - default=False, help='run tests and check for 100% coverage')
> -(options, args) = parser.parse_args()
> +parser.add_argument('-T', '--test-coverage', action='store_true',
> + default=False, help='run tests and check for 100%% coverage')
> +parser.add_argument('files', nargs='*')
> +args = parser.parse_args()
>
> # Run our meagre tests
> -if options.test:
> - ret_code = run_tests(options.processes, args)
> +if args.test:
> + ret_code = run_tests(args.processes, args)
> sys.exit(ret_code)
>
> -elif options.test_coverage:
> +elif args.test_coverage:
> RunTestCoverage()
>
> else:
> - dtb_platdata.run_steps(args, options.dtb_file, options.include_disabled,
> - options.output,
> - [options.c_output_dir, options.h_output_dir],
> - options.phase, instantiate=options.instantiate)
> + dtb_platdata.run_steps(args.files, args.dtb_file, args.include_disabled,
> + args.output,
> + [args.c_output_dir, args.h_output_dir],
> + args.phase, instantiate=args.instantiate)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/8] dtoc: Allow multiple warnings for a driver
2021-07-04 18:19 ` [PATCH 3/8] dtoc: Allow multiple warnings for a driver Simon Glass
@ 2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
1 sibling, 0 replies; 25+ messages in thread
From: Walter Lozano @ 2021-07-05 18:04 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Johan Jonker
Hi Simon,
On 7/4/21 3:19 PM, Simon Glass wrote:
> At present we show when a driver is missing but this is not always that
> useful. There are various reasons way a driver may appear to be missing,
Did you mean "why" instead of "way"?
> such as a parse error in the source code or a missing field in the driver
> declaration.
>
> Update the implementation to record all warnings for each driver, showing
> only those which relate to drivers that are actually used. This avoids
> spamming the user with warnings related to a driver for a different board.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> tools/dtoc/src_scan.py | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
Thank you, it is indeed something worth to be added!
Walter
>
> diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py
> index 1dbb56712a3..6c37a71e978 100644
> --- a/tools/dtoc/src_scan.py
> +++ b/tools/dtoc/src_scan.py
> @@ -13,6 +13,7 @@ U_BOOT_DRIVER(), UCLASS_DRIVER and all struct declarations in header files.
> See doc/driver-model/of-plat.rst for more informaiton
> """
>
> +import collections
> import os
> import re
> import sys
> @@ -190,6 +191,9 @@ class Scanner:
> value: Driver name declared with U_BOOT_DRIVER(driver_name)
> _drivers_additional (list or str): List of additional drivers to use
> during scanning
> + _warnings: Dict of warnings found:
> + key: Driver name
> + value: Set of warnings
> _of_match: Dict holding information about compatible strings
> key: Name of struct udevice_id variable
> value: Dict of compatible info in that variable:
> @@ -217,6 +221,7 @@ class Scanner:
> self._driver_aliases = {}
> self._drivers_additional = drivers_additional or []
> self._missing_drivers = set()
> + self._warnings = collections.defaultdict(set)
> self._of_match = {}
> self._compat_to_driver = {}
> self._uclass = {}
> @@ -267,7 +272,10 @@ class Scanner:
> aliases_c.remove(compat_c)
> return compat_c, aliases_c
>
> - self._missing_drivers.add(compat_list_c[0])
> + name = compat_list_c[0]
> + self._missing_drivers.add(name)
> + self._warnings[name].add(
> + 'WARNING: the driver %s was not found in the driver list' % name)
>
> return compat_list_c[0], compat_list_c[1:]
>
> @@ -577,9 +585,17 @@ class Scanner:
>
> def show_warnings(self):
> """Show any warnings that have been collected"""
> - for name in sorted(list(self._missing_drivers)):
> - print('WARNING: the driver %s was not found in the driver list'
> - % name)
> + used_drivers = [drv.name for drv in self._drivers.values() if drv.used]
> + missing = self._missing_drivers
> + for name in sorted(self._warnings.keys()):
> + if name in missing or name in used_drivers:
> + warns = sorted(list(self._warnings[name]))
> + # For now there is only ever one warning
> + print('%s: %s' % (name, warns[0]))
> + indent = ' ' * len(name)
> + if name in missing:
> + missing.remove(name)
> + print()
>
> def scan_driver(self, fname):
> """Scan a driver file to build a list of driver names and aliases
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/8] dtoc: Correct the re_compat regular expression
2021-07-04 18:19 ` [PATCH 4/8] dtoc: Correct the re_compat regular expression Simon Glass
@ 2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
1 sibling, 0 replies; 25+ messages in thread
From: Walter Lozano @ 2021-07-05 18:04 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Johan Jonker
Hi Simon,
On 7/4/21 3:19 PM, Simon Glass wrote:
> This expects a . before the field name (.e.g '.compatible = ...) but
> presently accepts anything at all. Fix it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> tools/dtoc/src_scan.py | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
Thanks!
Walter
>
> diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py
> index 6c37a71e978..6e8e1ba51a0 100644
> --- a/tools/dtoc/src_scan.py
> +++ b/tools/dtoc/src_scan.py
> @@ -452,8 +452,8 @@ class Scanner:
>
> # Collect the compatible string, e.g. 'rockchip,rk3288-grf'
> compat = None
> - re_compat = re.compile(r'{\s*.compatible\s*=\s*"(.*)"\s*'
> - r'(,\s*.data\s*=\s*(\S*))?\s*},')
> + re_compat = re.compile(r'{\s*\.compatible\s*=\s*"(.*)"\s*'
> + r'(,\s*\.data\s*=\s*(\S*))?\s*},')
>
> # This is a dict of compatible strings that were found:
> # key: Compatible string, e.g. 'rockchip,rk3288-grf'
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 7/8] dtoc: Detect drivers which do not parse correctly
2021-07-04 18:19 ` [PATCH 7/8] dtoc: Detect drivers which do not parse correctly Simon Glass
@ 2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
1 sibling, 0 replies; 25+ messages in thread
From: Walter Lozano @ 2021-07-05 18:04 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Johan Jonker
On 7/4/21 3:19 PM, Simon Glass wrote:
> At present if a driver is missing a uclass or compatible stirng, this
Most probably it should be "string"
> is silently ignored. This makes sense in most cases, particularly for
> the compatible string, since it is not required except when the driver
> is used with of-platdata.
>
> But it is also not very helpful. When there is some sort of problem
> with a driver, the missing compatible string (for example) may be the
> cause.
>
> Add a warning in this case, showing it only for drivers which are used
> by the build.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> tools/dtoc/src_scan.py | 7 ++++++-
> tools/dtoc/test_src_scan.py | 38 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 44 insertions(+), 1 deletion(-)
Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
Thanks!
Walter
>
> diff --git a/tools/dtoc/src_scan.py b/tools/dtoc/src_scan.py
> index 847677757d9..3bef59d616e 100644
> --- a/tools/dtoc/src_scan.py
> +++ b/tools/dtoc/src_scan.py
> @@ -546,7 +546,12 @@ class Scanner:
> # The driver does not have a uclass or compat string.
> # The first is required but the second is not, so just
> # ignore this.
> - pass
> + if not driver.uclass_id:
> + warn = 'Missing .uclass'
> + else:
> + warn = 'Missing .compatible'
> + self._warnings[driver.name].add('%s in %s' %
> + (warn, fname))
> driver = None
> ids_name = None
> compat = None
> diff --git a/tools/dtoc/test_src_scan.py b/tools/dtoc/test_src_scan.py
> index 62500e80fe7..f03cf8ed7c7 100644
> --- a/tools/dtoc/test_src_scan.py
> +++ b/tools/dtoc/test_src_scan.py
> @@ -581,3 +581,41 @@ rockchip_rk3288_grf: WARNING: the driver rockchip_rk3288_grf was not found in th
> ''',
> stdout.getvalue())
> self.assertIn('tegra_i2c_ids', stdout.getvalue())
> +
> + def scan_uclass_warning(self):
> + """Test a missing .uclass in the driver"""
> + buff = '''
> +static const struct udevice_id tegra_i2c_ids[] = {
> + { .compatible = "nvidia,tegra114-i2c", .data = TYPE_114 },
> + { }
> +};
> +
> +U_BOOT_DRIVER(i2c_tegra) = {
> + .name = "i2c_tegra",
> + .of_match = tegra_i2c_ids,
> +};
> +'''
> + scan = src_scan.Scanner(None, None)
> + scan._parse_driver('file.c', buff)
> + self.assertEqual(
> + {'i2c_tegra': {'Missing .uclass in file.c'}},
> + scan._warnings)
> +
> + def scan_compat_warning(self):
> + """Test a missing .compatible in the driver"""
> + buff = '''
> +static const struct udevice_id tegra_i2c_ids[] = {
> + { .compatible = "nvidia,tegra114-i2c", .data = TYPE_114 },
> + { }
> +};
> +
> +U_BOOT_DRIVER(i2c_tegra) = {
> + .name = "i2c_tegra",
> + .id = UCLASS_I2C,
> +};
> +'''
> + scan = src_scan.Scanner(None, None)
> + scan._parse_driver('file.c', buff)
> + self.assertEqual(
> + {'i2c_tegra': {'Missing .compatible in file.c'}},
> + scan._warnings)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 8/8] dtoc: Update documentation to cover warnings in more detail
2021-07-04 18:19 ` [PATCH 8/8] dtoc: Update documentation to cover warnings in more detail Simon Glass
@ 2021-07-05 18:04 ` Walter Lozano
2021-07-20 18:32 ` Simon Glass
2021-07-11 23:00 ` Simon Glass
1 sibling, 1 reply; 25+ messages in thread
From: Walter Lozano @ 2021-07-05 18:04 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List; +Cc: Johan Jonker, Walter Lozano
Hi Simon,
On 7/4/21 3:19 PM, Simon Glass wrote:
> When things go wrong it can be confusing to figure out what to change.
> Add a few more details to the documentation.
>
> Fix a 'make htmldocs' warning while we are here.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> doc/develop/driver-model/of-plat.rst | 53 ++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/doc/develop/driver-model/of-plat.rst b/doc/develop/driver-model/of-plat.rst
> index 74f1932473b..e2763965839 100644
> --- a/doc/develop/driver-model/of-plat.rst
> +++ b/doc/develop/driver-model/of-plat.rst
> @@ -597,6 +597,59 @@ as a macro included in the driver definition::
>
>
>
> +Problems
> +--------
> +
> +In some cases you will you see something like this::
> +
> + WARNING: the driver rockchip_rk3188_grf was not found in the driver list
> +
> +The driver list is a list of drivers, each with a name. The name is in the
> +U_BOOT_DRIVER() declaration, repeated twice, one in brackets and once as the
> +.name member. For example, in the following declaration the driver name is
> +`rockchip_rk3188_grf`::
> +
> + U_BOOT_DRIVER(rockchip_rk3188_grf) = {
> + .name = "rockchip_rk3188_grf",
> + .id = UCLASS_SYSCON,
> + .of_match = rk3188_syscon_ids + 1,
> + .bind = rk3188_syscon_bind_of_plat,
> + };
> +
> +The first name U_BOOT_DRIVER(xx) is used to create a linker symbol so that the
> +driver can be accessed at build-time without any overhead. The second one
> +(.name = "xx") is used at runtime when something wants to print out the driver
> +name.
> +
> +The dtoc tool expects to be able to find a driver for each compatible string in
> +the devicetree. For example, if the devicetree has::
> +
> + grf: grf@20008000 {
> + compatible = "rockchip,rk3188-grf", "syscon";
> + reg = <0x20008000 0x200>;
> + u-boot,dm-spl;
> + };
> +
> +then dtoc looks at the first compatible string ("rockchip,rk3188-grf"),
> +converts that to a C identifier (rockchip_rk3188_grf) and then looks for that.
> +
> +Various things can cause dtoc to fail to find the driver and it tries to
> +warn about these. For example:
> +
> + rockchip_rk3188_uart: Missing .compatible in drivers/serial/serial_rockchip.c
> + : WARNING: the driver rockchip_rk3188_uart was not found in the driver list
> +
> +Without a compatible string a driver cannot be used by dtoc, even if the
> +compatible string is not actually needed at runtime.
> +
> +If the problem is simply that there are multiple compatible strings, the
> +DM_DRIVER_ALIAS() macro can be used to tell dtoc about this and avoid a problem.
> +
> +Checks are also made the referenced driver has a .compatible member and a .id
> +member. The first provides the array of compatible strings and the second
> +provides the uclass ID.
> +
This paragraph sound strange, maybe
"Checks are are also made to confirm that the referenced driver..."
but your English is better than mine.
Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
Regards,
Walter
> +
> Caveats
> -------
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 8/8] dtoc: Update documentation to cover warnings in more detail
2021-07-04 18:19 ` [PATCH 8/8] dtoc: Update documentation to cover warnings in more detail Simon Glass
2021-07-05 18:04 ` Walter Lozano
@ 2021-07-11 23:00 ` Simon Glass
1 sibling, 0 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-11 23:00 UTC (permalink / raw)
To: Walter Lozano
Cc: Johan Jonker, Walter Lozano, Simon Glass, U-Boot Mailing List
Hi Simon,
On 7/4/21 3:19 PM, Simon Glass wrote:
> When things go wrong it can be confusing to figure out what to change.
> Add a few more details to the documentation.
>
> Fix a 'make htmldocs' warning while we are here.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> doc/develop/driver-model/of-plat.rst | 53 ++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 7/8] dtoc: Detect drivers which do not parse correctly
2021-07-04 18:19 ` [PATCH 7/8] dtoc: Detect drivers which do not parse correctly Simon Glass
2021-07-05 18:04 ` Walter Lozano
@ 2021-07-11 23:00 ` Simon Glass
1 sibling, 0 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-11 23:00 UTC (permalink / raw)
To: Walter Lozano; +Cc: Johan Jonker, Simon Glass, U-Boot Mailing List
On 7/4/21 3:19 PM, Simon Glass wrote:
> At present if a driver is missing a uclass or compatible stirng, this
Most probably it should be "string"
> is silently ignored. This makes sense in most cases, particularly for
> the compatible string, since it is not required except when the driver
> is used with of-platdata.
>
> But it is also not very helpful. When there is some sort of problem
> with a driver, the missing compatible string (for example) may be the
> cause.
>
> Add a warning in this case, showing it only for drivers which are used
> by the build.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> tools/dtoc/src_scan.py | 7 ++++++-
> tools/dtoc/test_src_scan.py | 38 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 44 insertions(+), 1 deletion(-)
Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
Thanks!
Walter
>
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/8] dtoc: Detect unexpected suffix on .of_match
2021-07-04 18:19 [PATCH 0/8] dtoc: Improvements to warnings Simon Glass
` (7 preceding siblings ...)
2021-07-04 18:19 ` [PATCH 8/8] dtoc: Update documentation to cover warnings in more detail Simon Glass
@ 2021-07-11 23:00 ` Simon Glass
2021-07-11 23:00 ` [PATCH 5/8] dtoc: Add a stdout check in test_normalized_name() Simon Glass
9 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-11 23:00 UTC (permalink / raw)
To: Simon Glass; +Cc: Johan Jonker, U-Boot Mailing List
Some rockchip drivers use a suffix on the of_match line which is not
strictly valid. At present this causes the parsing to fail. Fix this
and offer a warning.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/dtoc/src_scan.py | 12 +++--
tools/dtoc/test_src_scan.py | 92 +++++++++++++++++++++++++++++++++++++
2 files changed, 101 insertions(+), 3 deletions(-)
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/8] dtoc: Add a stdout check in test_normalized_name()
2021-07-04 18:19 [PATCH 0/8] dtoc: Improvements to warnings Simon Glass
` (8 preceding siblings ...)
2021-07-11 23:00 ` [PATCH 6/8] dtoc: Detect unexpected suffix on .of_match Simon Glass
@ 2021-07-11 23:00 ` Simon Glass
9 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-11 23:00 UTC (permalink / raw)
To: Simon Glass; +Cc: Johan Jonker, U-Boot Mailing List
This test captures output but does not always check it. Add the missing
code and drop the old comment.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
tools/dtoc/test_src_scan.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/8] dtoc: Correct the re_compat regular expression
2021-07-04 18:19 ` [PATCH 4/8] dtoc: Correct the re_compat regular expression Simon Glass
2021-07-05 18:04 ` Walter Lozano
@ 2021-07-11 23:00 ` Simon Glass
1 sibling, 0 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-11 23:00 UTC (permalink / raw)
To: Walter Lozano; +Cc: Johan Jonker, Simon Glass, U-Boot Mailing List
Hi Simon,
On 7/4/21 3:19 PM, Simon Glass wrote:
> This expects a . before the field name (.e.g '.compatible = ...) but
> presently accepts anything at all. Fix it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> tools/dtoc/src_scan.py | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
Thanks!
Walter
>
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/8] dtoc: Allow multiple warnings for a driver
2021-07-04 18:19 ` [PATCH 3/8] dtoc: Allow multiple warnings for a driver Simon Glass
2021-07-05 18:04 ` Walter Lozano
@ 2021-07-11 23:00 ` Simon Glass
2021-07-12 2:23 ` Simon Glass
1 sibling, 1 reply; 25+ messages in thread
From: Simon Glass @ 2021-07-11 23:00 UTC (permalink / raw)
To: Walter Lozano; +Cc: Johan Jonker, Simon Glass, U-Boot Mailing List
Hi Simon,
On 7/4/21 3:19 PM, Simon Glass wrote:
> At present we show when a driver is missing but this is not always that
> useful. There are various reasons way a driver may appear to be missing,
Did you mean "why" instead of "way"?
> such as a parse error in the source code or a missing field in the driver
> declaration.
>
> Update the implementation to record all warnings for each driver, showing
> only those which relate to drivers that are actually used. This avoids
> spamming the user with warnings related to a driver for a different board.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> tools/dtoc/src_scan.py | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
Thank you, it is indeed something worth to be added!
Walter
>
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/8] dtoc: Convert to use ArgumentParser
2021-07-04 18:19 ` [PATCH 2/8] dtoc: Convert to use ArgumentParser Simon Glass
2021-07-05 18:04 ` Walter Lozano
@ 2021-07-11 23:00 ` Simon Glass
1 sibling, 0 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-11 23:00 UTC (permalink / raw)
To: Walter Lozano; +Cc: Johan Jonker, Simon Glass, U-Boot Mailing List
Hi Simon,
On 7/4/21 3:19 PM, Simon Glass wrote:
> Use this parser instead of OptionParser, which is deprecated.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> tools/dtoc/main.py | 51 ++++++++++++++++++++++++----------------------
> 1 file changed, 27 insertions(+), 24 deletions(-)
Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
Thanks!
Walter
>
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/8] dtoc: Avoid using subscripts on match objects
2021-07-04 18:19 ` [PATCH 1/8] dtoc: Avoid using subscripts on match objects Simon Glass
2021-07-05 18:04 ` Walter Lozano
@ 2021-07-11 23:00 ` Simon Glass
1 sibling, 0 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-11 23:00 UTC (permalink / raw)
To: Walter Lozano; +Cc: Johan Jonker, Simon Glass, U-Boot Mailing List
Hi Simon,
On 7/4/21 3:19 PM, Simon Glass wrote:
> These are not supported before Python 3.6 so avoid them.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> tools/dtoc/src_scan.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
Thanks!
Walter
Applied to u-boot-dm, thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/8] dtoc: Allow multiple warnings for a driver
2021-07-11 23:00 ` Simon Glass
@ 2021-07-12 2:23 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-12 2:23 UTC (permalink / raw)
To: Walter Lozano; +Cc: Johan Jonker, U-Boot Mailing List
On Sun, 11 Jul 2021 at 17:00, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Simon,
>
> On 7/4/21 3:19 PM, Simon Glass wrote:
> > At present we show when a driver is missing but this is not always that
> > useful. There are various reasons way a driver may appear to be missing,
>
> Did you mean "why" instead of "way"?
>
> > such as a parse error in the source code or a missing field in the driver
> > declaration.
> >
> > Update the implementation to record all warnings for each driver, showing
> > only those which relate to drivers that are actually used. This avoids
> > spamming the user with warnings related to a driver for a different board.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > tools/dtoc/src_scan.py | 24 ++++++++++++++++++++----
> > 1 file changed, 20 insertions(+), 4 deletions(-)
>
> Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
>
> Thank you, it is indeed something worth to be added!
>
> Walter
>
> >
> Applied to u-boot-dm, thanks!
(Note: I fixed the nits in this patch and other other one when
applying...thanks for the review!)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 8/8] dtoc: Update documentation to cover warnings in more detail
2021-07-05 18:04 ` Walter Lozano
@ 2021-07-20 18:32 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2021-07-20 18:32 UTC (permalink / raw)
To: Walter Lozano; +Cc: U-Boot Mailing List, Johan Jonker, Walter Lozano
Hi Walter,
On Mon, 5 Jul 2021 at 12:04, Walter Lozano <wlozano@collabora.com> wrote:
>
> Hi Simon,
>
> On 7/4/21 3:19 PM, Simon Glass wrote:
> > When things go wrong it can be confusing to figure out what to change.
> > Add a few more details to the documentation.
> >
> > Fix a 'make htmldocs' warning while we are here.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > doc/develop/driver-model/of-plat.rst | 53 ++++++++++++++++++++++++++++
> > 1 file changed, 53 insertions(+)
> >
> > diff --git a/doc/develop/driver-model/of-plat.rst b/doc/develop/driver-model/of-plat.rst
> > index 74f1932473b..e2763965839 100644
> > --- a/doc/develop/driver-model/of-plat.rst
> > +++ b/doc/develop/driver-model/of-plat.rst
> > @@ -597,6 +597,59 @@ as a macro included in the driver definition::
> >
> >
> >
> > +Problems
> > +--------
> > +
> > +In some cases you will you see something like this::
> > +
> > + WARNING: the driver rockchip_rk3188_grf was not found in the driver list
> > +
> > +The driver list is a list of drivers, each with a name. The name is in the
> > +U_BOOT_DRIVER() declaration, repeated twice, one in brackets and once as the
> > +.name member. For example, in the following declaration the driver name is
> > +`rockchip_rk3188_grf`::
> > +
> > + U_BOOT_DRIVER(rockchip_rk3188_grf) = {
> > + .name = "rockchip_rk3188_grf",
> > + .id = UCLASS_SYSCON,
> > + .of_match = rk3188_syscon_ids + 1,
> > + .bind = rk3188_syscon_bind_of_plat,
> > + };
> > +
> > +The first name U_BOOT_DRIVER(xx) is used to create a linker symbol so that the
> > +driver can be accessed at build-time without any overhead. The second one
> > +(.name = "xx") is used at runtime when something wants to print out the driver
> > +name.
> > +
> > +The dtoc tool expects to be able to find a driver for each compatible string in
> > +the devicetree. For example, if the devicetree has::
> > +
> > + grf: grf@20008000 {
> > + compatible = "rockchip,rk3188-grf", "syscon";
> > + reg = <0x20008000 0x200>;
> > + u-boot,dm-spl;
> > + };
> > +
> > +then dtoc looks at the first compatible string ("rockchip,rk3188-grf"),
> > +converts that to a C identifier (rockchip_rk3188_grf) and then looks for that.
> > +
> > +Various things can cause dtoc to fail to find the driver and it tries to
> > +warn about these. For example:
> > +
> > + rockchip_rk3188_uart: Missing .compatible in drivers/serial/serial_rockchip.c
> > + : WARNING: the driver rockchip_rk3188_uart was not found in the driver list
> > +
> > +Without a compatible string a driver cannot be used by dtoc, even if the
> > +compatible string is not actually needed at runtime.
> > +
> > +If the problem is simply that there are multiple compatible strings, the
> > +DM_DRIVER_ALIAS() macro can be used to tell dtoc about this and avoid a problem.
> > +
> > +Checks are also made the referenced driver has a .compatible member and a .id
> > +member. The first provides the array of compatible strings and the second
> > +provides the uclass ID.
> > +
>
> This paragraph sound strange, maybe
>
> "Checks are are also made to confirm that the referenced driver..."
>
> but your English is better than mine.
>
> Reviewed-by: Walter Lozano <walter.lozano@collabora.com>
Thanks, I fixed this when applying.
- Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2021-07-20 18:33 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-04 18:19 [PATCH 0/8] dtoc: Improvements to warnings Simon Glass
2021-07-04 18:19 ` [PATCH 1/8] dtoc: Avoid using subscripts on match objects Simon Glass
2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
2021-07-04 18:19 ` [PATCH 2/8] dtoc: Convert to use ArgumentParser Simon Glass
2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
2021-07-04 18:19 ` [PATCH 3/8] dtoc: Allow multiple warnings for a driver Simon Glass
2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
2021-07-12 2:23 ` Simon Glass
2021-07-04 18:19 ` [PATCH 4/8] dtoc: Correct the re_compat regular expression Simon Glass
2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
2021-07-04 18:19 ` [PATCH 5/8] dtoc: Add a stdout check in test_normalized_name() Simon Glass
2021-07-04 18:19 ` [PATCH 6/8] dtoc: Detect unexpected suffix on .of_match Simon Glass
2021-07-04 18:19 ` [PATCH 7/8] dtoc: Detect drivers which do not parse correctly Simon Glass
2021-07-05 18:04 ` Walter Lozano
2021-07-11 23:00 ` Simon Glass
2021-07-04 18:19 ` [PATCH 8/8] dtoc: Update documentation to cover warnings in more detail Simon Glass
2021-07-05 18:04 ` Walter Lozano
2021-07-20 18:32 ` Simon Glass
2021-07-11 23:00 ` Simon Glass
2021-07-11 23:00 ` [PATCH 6/8] dtoc: Detect unexpected suffix on .of_match Simon Glass
2021-07-11 23:00 ` [PATCH 5/8] dtoc: Add a stdout check in test_normalized_name() Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.