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. > + 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'. > + > 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? > 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, Simon