Hi Simon,
Thanks for your time.
On 6/7/20 16:21, Simon Glass wrote:
Hi Walter,
On Fri, 19 Jun 2020 at 15:11, Walter Lozano <walter.loz...@collabora.com> wrote:
Based on several reports and discussions [1], [2] it is clear that U-Boot's
footprint is always a concern, and any kind of reduction is an
improvement.
In particular dtb is one of the sources of footprint increment, as
U-Boot uses the same dtb as Linux. However is interesting to note that
U-Boot does not require all the nodes and properties declared in it.
Some improvements in this sense are already present, such as
removing properties based on configuration and using specific "u-boot"
properties to keep only specific node in SPL. However, this require
manual configuration.
Additionally reducing dtb, will allow ATF for better handing FDT buffer, which
is an issue in some contexts [3].
In order to improve this situation, this patch adds a proof of concept
for dtb shrink. The idea behind this is simple, remove all the nodes
from dtb which compatible string is not supported by any driver present.
This approach makes sense for those configuration where Linux is
expected to have its own dtb.
This patch is based on the work of Simon Glass present in [4] which adds
support to dtoc for parsing compatible strings.
Some early untested results shows that the reduction in size is 50 % in
case of mx6_cuboxi_defconfig, which is promising.
Some additional reduction could be possible by only keeping the nodes for
whose compatible string is supported by any enabled driver. However,
this requires to add extra logic to parse config files and map
configuration to compatible strings.
This proof of concept uses fdtgrep to implement the node removal, but
the idea is to implement this logic inside the dtoc for better handling.
[1]
http://patchwork.ozlabs.org/project/uboot/patch/20200525093539.1.Ibf2d19439cde35e39192a9d4a8dad23539fae2e6@changeid/
[2]
http://u-boot.10912.n7.nabble.com/PATCH-v1-00-15-add-basic-driver-support-for-broadcom-NS3-soc-tt412411.html#none
[3] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/4512
[4] https://gitlab.denx.de/u-boot/custodians/u-boot-dm/-/tree/dtoc-working
Signed-off-by: Walter Lozano <walter.loz...@collabora.com>
---
tools/dtoc/dtb_platdata.py | 42 +++++++++++++++++++++++++++++++++++---
1 file changed, 39 insertions(+), 3 deletions(-)
diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
index 21cce5afb5..1df13b2cd2 100644
--- a/tools/dtoc/dtb_platdata.py
+++ b/tools/dtoc/dtb_platdata.py
@@ -399,7 +399,10 @@ class DtbPlatdata(object):
"""Scan the driver folders to build a list of driver names and
possible
aliases
"""
- for (dirpath, dirnames, filenames) in os.walk('/home/sglass/u'):
+ basedir = sys.argv[0].replace('tools/dtoc/dtoc', '')
Instead of this logic, can we pass the source-tree location into dtoc
with a flag? It could default to the current dir perhaps.
Sure, no problem at all.
+ if basedir == '':
+ basedir = './'
+ for (dirpath, dirnames, filenames) in os.walk(basedir):
for fn in filenames:
if not fn.endswith('.c'):
continue
@@ -802,6 +805,32 @@ class DtbPlatdata(object):
self.out(''.join(self.get_buf()))
self.close_output()
+ def shrink(self):
+ """Generate a shrunk version of DTB bases on valid drivers
+
+ This function removes nodes from dtb which compatible string is not
+ found in drivers. The output is saved in a file with suffix name
-shrink.dtb
+ """
+ compat = []
+ cmd = './tools/fdtgrep '
+ #print(self._drivers)
+ for dr in self._drivers.values():
+ compat = compat + dr.compat
+
+ for cp in compat:
+ #print(cp)
+ cmd += ' -c ' + cp
+
+ cmd += ' -O dtb -o ' + self._dtb_fname.replace('.dtb', '') +
'-shrink.dtb ' + self._dtb_fname
+
+ if False:
+ with open('dt_shrink.sh', 'w+') as script:
+ script.write(cmd)
+
+ os.system(cmd)
+
+ return
+
def run_steps(args, dtb_file, config_file, include_disabled, output):
"""Run all the steps of the dtoc tool
@@ -816,6 +845,10 @@ def run_steps(args, dtb_file, config_file,
include_disabled, output):
if not args:
raise ValueError('Please specify a command: struct, platdata')
+ skip_scan = False
+ if args == ['shrink']:
+ skip_scan = True
I think that would be better as a positive variable, like 'scan'.
Yes, I agree. The idea was to test the general idea, and check if it
could be useful.
+
plat = DtbPlatdata(dtb_file, config_file, include_disabled)
plat.scan_drivers()
plat.scan_dtb()
@@ -823,14 +856,17 @@ def run_steps(args, dtb_file, config_file,
include_disabled, output):
plat.scan_config()
plat.scan_reg_sizes()
Are those two needed with this new command?
Probably not in this version, but I was planning to use scan_config to
check which drivers are enabled. Actually a there is a newer version in
the link I previously left. However, as I had some issues working on top
of the dtoc-working branch this version is back ported, in order to make
is user to run some early/quick tests.
https://gitlab.collabora.com/wlozano/u-boot/-/tree/wip
plat.setup_output(output)
- structs = plat.scan_structs()
- plat.scan_phandles()
+ if not skip_scan:
+ structs = plat.scan_structs()
+ plat.scan_phandles()
for cmd in args[0].split(','):
if cmd == 'struct':
plat.generate_structs(structs)
elif cmd == 'platdata':
plat.generate_tables()
+ elif cmd == 'shrink':
+ plat.shrink()
else:
raise ValueError("Unknown command '%s': (use: struct, platdata)" %
cmd)
--
2.20.1
Regards,
Walter
