[U-Boot] [RFC PATCH v2 3/3] tools: Add a tool to get an overview of the usage of CONFIG options

Jean-Jacques Hiblot jjhiblot at ti.com
Thu Oct 18 11:38:11 UTC 2018


Simon,


On 09/10/2018 18:20, Simon Glass wrote:
> Hi Jean-Jacques,
>
> On 3 October 2018 at 07:53, Jean-Jacques Hiblot <jjhiblot at ti.com> wrote:
>> configs2csv.py is tool that allow to check how some options are used for a
>> particular subset of platforms.
>> The purpose is to identify the targets that are actually using one or more
>> options of interest.
>> For example, it can tell what targets are still using CONFIG_DM_I2_COMPAT.
>> It relies on the config database produced by tools/moveconfig.py.
>> If the database doesn't exist, it will build it for the restricted set of
>> the selected platforms. Once the database is built, it is much faster than
>> greping the configs directory and more accurate as it relies on the
>> information found in u-boot.cfg instead of defconfigs.
>> It possible to look for options in the u-boot, the SPL or the TPL
>> configurations. It can also perform diffs between those configurations.
>>
>> usage: configs2csv.py [-h] [-X] [--u-boot] [--spl] [--tpl] [--diff]
>>                        [--rebuild-db] [-j JOBS] [-o OUTPUT] [--no-header]
>>                        [--discard-empty] [-i] [--soc SOC] [--vendor VENDOR]
>>                        [--arch ARCH] [--cpu CPU] [--board BOARD]
>>                        [--target TARGET]
>>                        OPTION [OPTION ...]
>>
>> all filtering parameters (OPTION, vendor, arch, ...) accept regexp.
>> ex: configs2csv.py .*DM_I2C.* --soc 'omap[2345]|k3' will match
>> CONFIG_DM_I2C and CONFIG_DM_I2C_COMPAT and look for it only for targets
>> using the omap2, omap3, omap4, omap5 or k3 SOCs.
>>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
>>
>> ---
>>
>> Changes in v2:
>> - basically rewrote the whole thing
>> - use tools/moveconfig.py to generate the database of configs
>> - use tools/find_defconfigs.py to get the list of defconfigs off interest
>> - removed diff with .config. tools/moveconfig.py does a better job
>>
>>   tools/configs2csv.py | 387 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 387 insertions(+)
>>   create mode 100755 tools/configs2csv.py
> This looks like a useful tool and a big improvement on the movconfig
> starting point. Style comments below.
Thanks for the review. I'll send a v3 taking care of the comments in a 
couple of days.
>
>> diff --git a/tools/configs2csv.py b/tools/configs2csv.py
>> new file mode 100755
>> index 0000000..70b6602
>> --- /dev/null
>> +++ b/tools/configs2csv.py
>> @@ -0,0 +1,387 @@
>> +#!/usr/bin/env python
>> +# SPDX-License-Identifier: GPL-2.0+
>> +#
>> +# Author: JJ Hiblot <jjhiblot at ti.com>
>> +#
>> +
>> +"""
>> +scan the configuration of specified targets (ie defconfigs) and outputs a
>> +summary in a csv file.
>> +Useful tool to check what platform is using a particular set of options.
>> +
>> +
>> +How does it work?
>> +-----------------
>> +
>> +This tools uses the config database produced by tools/moveconfig.py (called
>> +with option -B to get all the configs: SPl, TPL and u-boot). If the database
>> +is not present, it will build it. A rebuild can be forced with the option
>> +'--rebuild-db'.
>> +
>> +The list of the targets of interest can be specified by a set of filter (soc,
>> +vendor, defconfig name, ..). All those filters are actually regexp, allowing
>> +for complex selection. The selection process is done by
>> +tools/find_defconfigs.py
>> +ex: --soc omap[23] --vendor 'ti|compulab' will inspect the omap2 and omap3
>> +platforms from TI and compulab
>> +
>> +
>> +examples:
>> +---------
>> +
>> +
>> +1) Get an overview of the usage of CONFIG_DM, CONFIG_SPL_DM, and DM/I2C related
>> +   options for platforms with omap5 or k3 SOC in u-boot and in SPL
>> +
>> +$ tools/configs2csv.py CONFIG_SPL_DM CONFIG_DM CONFIG_DM_I2C.* --vendor ti \
>> +       --soc 'omap5|k3' -X --u-boot --spl  -o dummy.csv
>> +
> Is this output below showing the contents of the ,csv file in a non-CVS format?
Yes. This represents what we would see in a spreadsheet viewer.
The csv output is not easily readable.
>> +vendor   soc            defconfig            type     CONFIG_DM  CONFIG_DM_I2C  CONFIG_DM_I2C_COMPAT  CONFIG_SPL_DM
>> +ti      omap5     am57xx_evm_defconfig        SPL         X                                               X
>> +ti      omap5     am57xx_evm_defconfig        u-boot      X         X                    X                X
>> +ti      omap5     am57xx_hs_evm_defconfig     SPL         X                                               X
>> +ti      omap5     am57xx_hs_evm_defconfig     u-boot      X         X                    X                X
>> +ti      k3        am65x_evm_a53_defconfig     SPL         X                                               X
>> +ti      k3        am65x_evm_a53_defconfig     u-boot      X                                               X
>> +ti      omap5     dra7xx_evm_defconfig        SPL         X                                               X
>> +ti      omap5     dra7xx_evm_defconfig        u-boot      X         X                    X                X
>> +ti      omap5     dra7xx_hs_evm_defconfig     SPL         X                                               X
>> +ti      omap5     dra7xx_hs_evm_defconfig     u-boot      X         X                    X                X
>> +ti      omap5     omap5_uevm_defconfig        SPL
>> +ti      omap5     omap5_uevm_defconfig        u-boot
>> +
>> +
>> +This shows quickly that DM is not supported at all for omap5_uevm, that
>> +only am65x_evm_a53 in not using DM_I2C in u-boot, and finally that DM_I2C is
>> +not enabled in the SPL for any platform although SPL_DM is.
>> +Also all the other platforms that enabled DM_I2C, also enabled
>> +CONFIG_DM_I2C_COMPAT.
>> +
>> +
>> +2) Check differences in config between SPL, TPL and u-boot (--diff option)
>> +
>> +Some platforms may disable/enable stuff in the configuration header files if
>> +in SPl. This makes it hard to know the usage of a variable by just looking at
>> +the .config. This is specially true for DM stuff.
>> +
>> +$ tools/configs2csv.py CONFIG\(_SPL\)?_DM_.*  --vendor ti \
>> +  --soc 'omap5|k3' --diff --spl --u-boot > dummy.csv
>> +
>> +vendor    soc        defconfig             CONFIG_DM_I2C CONFIG_DM_I2C_COMPAT  CONFIG_DM_STDIO  CONFIG_DM_WARN
>> +ti        omap5    am57xx_evm_defconfig       u-boot         u-boot                 u-boot           u-boot
>> +ti        omap5    am57xx_hs_evm_defconfig    u-boot         u-boot                 u-boot           u-boot
>> +ti        k3       am65x_evm_a53_defconfig                                          u-boot           u-boot
>> +ti        omap5    dra7xx_evm_defconfig       u-boot         u-boot                 u-boot           u-boot
>> +ti        omap5    dra7xx_hs_evm_defconfig    u-boot         u-boot                 u-boot           u-boot
>> +
>> +This shows that k3 has no real config diff between SPl and u-boot. whereas am57
>> +and dra7 have different settings for DM_I2C and DM_I2C_COMPAT
>> +
>> +"""
>> +
>> +import argparse
>> +import csv
>> +import os
>> +import re
>> +import sys
>> +from collections import namedtuple
>> +from itertools import combinations
>> +
>> +import find_defconfigs
>> +
>> +CONFIG_DATABASE = 'moveconfig.db'
>> +target = namedtuple("target", ["defconfig", "binary_type"])
>> +
>> +
>> +class db:
> Please capitalise the class name. Also how about ConfigDb or something
> a little longer than db? Below you actually use db as a variable name.
>
>> +
>> +    """ db is an object that store a collection of targets
>> +    a target is identified buy its defconfig and its binary type. ex:
>> +    (omap3_evm_defconfig,SPL)
>> +    The main purpose of this object is to output a CSV file that describes all
>> +    the targets.
>> +    There is also the possibility to create a "diff" db from a db. This new db
>> +    contains a summary of the differences between target of same defconfig.
>> +    """
>> +
>> +    def __init__(self):
>> +        self.targets = dict()
>> +
>> +    def add_target(self, target):
>> +        self.targets[target] = dict()
>> +
>> +    def add_option(self, target, option, value):
>> +        self.targets[target][option] = value
>> +
>> +    def add_options(self, target, dic):
>> +        self.targets[target].update(dic)
>> +
>> +    def output_csv(
>> +            self, output, show_X=False, header=True, left_columns=None, discard_empty_rows=False):
> Please add function comments with Args and Returns (if you have
> argument and return value) along with what the function does.
>
>> +        all_options = set()
>> +        if len(self.targets) == 0:
>> +            return
>> +
>> +        if discard_empty_rows:
>> +            dic = {k: self.targets[k] for k in self.targets if self.targets[k]}
>> +        else:
>> +            dic = self.targets.copy()
>> +        for target in dic.keys():
>> +            for option in dic[target].keys():
>> +                all_options.add(option)
>> +                if show_X:
>> +                    dic[target][option] = "X"
>> +            if left_columns:
>> +                left_columns(target, dic, header=False)
>> +
>> +        columns = []
>> +        if left_columns:
>> +            columns.extend(left_columns(None, header=True))
>> +        columns.extend(sorted(all_options))
>> +
>> +        writer = csv.DictWriter(output, fieldnames=columns,
>> +                                lineterminator='\n')
>> +        if header:
>> +            writer.writeheader()
>> +        for target in sorted(dic.keys()):
>> +            writer.writerow(dic[target])
>> +
>> +    def diff_one_defconfig(self, defconfig):
>> +        """ This function creates a dictionary of the differences between the
>> +        binaries os a single target.
>> +        For example, for "dra7xx_evm_defconfig" it will compute the diffence
>> +        between the options used to compile u-boot and the SPL (not the TPL
>> +        because this platform doesn't have it).
>> +        The return value looks as follow: { 'CONFIG_DM_I2C: "u-boot",
>> +        CONFIG_SPL_BUILD:"SPL", CONFIG_DUMMY_SPI_FREQ: "diff" }.
>> +
>> +        The algorithm can probably be optimized, but I didn't care enough.
>> +        algo is:
>> +        - return immediately is there is only one binary type (u-boot)
>> +        - create a dic that is merge of the dic for all the binary types
>> +        - for each binary type, compare its dic to the merged_dic. If it is
>> +        different then break. It means that at least one option is different.
>> +        - if no difference has been found, then return
>> +        - at this point, we know that there is at least one diff. For each
>> +        binary types and for all options used for this binary type, check if it
>> +        is in the merged dic and, if so, if its value is the same. update our
>> +        return dic with the proper description.
>> +
> Drop blank line before """
>
> Returns:
>     dict:
>        key: ...
>        value: ...
>
> See buildman,, etc. for examples.
>
>> +        """
>> +
>> +        diffs = dict()
>> +        diff_found = False
>> +
>> +        # get all binary types (spl, TPL, u-boot) generated by this defconfig
>> +        all_binary_types = sorted(
>> +            set([t.binary_type for t in self.targets.keys() if t.defconfig == defconfig]))
>> +
>> +        # If there is only one type of binary, no need to do a diff
>> +        if len(all_binary_types) <= 1:
>> +            return None
>> +
>> +        # create a dict with all options:values used by all binaries
>> +        merged_dic = dict()
>> +        for bin_type in all_binary_types:
>> +            merged_dic.update(self.targets[target(defconfig, bin_type)])
>> +
>> +        # check if all binaries have the same options (should be the case for
>> +        # most of the defconfigs)
>> +        for bin_type in all_binary_types:
>> +            if self.targets[target(defconfig, bin_type)] != merged_dic:
>> +                diff_found = True
>> +                break
>> +        if not diff_found:
>> +            return None
>> +
>> +        # at this point, we know that there are some options that differ
>> +        # between binaries (either not present or different)
>> +
>> +        # Get a list (actually a set) of the options that are different
>> +        differing_keys = set()
>> +        for bin_type in all_binary_types:
>> +            dic = self.targets[target(defconfig, bin_type)]
>> +            for opt, value in merged_dic.items():
>> +                if dic.get(opt, None) != value:
>> +                    differing_keys.add(opt)
>> +
>> +        # create a dictionary that summarize the differences
>> +        for bin_type in all_binary_types:
>> +            dic = self.targets[target(defconfig, bin_type)]
>> +            for opt in differing_keys:
>> +                dic_value = dic.get(opt, None)
>> +                merged_value = merged_dic.get(opt, None)
>> +                previous = diffs.get(opt, None)
>> +                if dic_value:
>> +                    if dic_value != merged_value:
>> +                        diffs[opt] = "diff"
>> +                    elif previous != "diff":
>> +                        diffs[opt] = ' / '.join(
>> +                            [previous, bin_type]) if previous else bin_type
>> +
>> +        return diffs
>> +
>> +    def diff(self):
>> +        """ create a new db that contains the differences between the binaries
>> +        for all the targets in the db """
> """ goes on its own line. Looks like this needs Returns comment.
>
>> +        diff_db = db()
>> +        # get a list of all the targets
>> +        all_defconfigs = set([t.defconfig for t in self.targets.keys()])
>> +        # for every target of the list, get a dictionary of the difference.
>> +        # if the dictionary is not empty, add it the new db
>> +        for defconfig in all_defconfigs:
>> +            diff_dic = self.diff_one_defconfig(defconfig)
>> +            if diff_dic:
>> +                diff_db.add_target(target(defconfig, None))
>> +                diff_db.add_options(target(defconfig, None), diff_dic)
>> +        return diff_db
>> +
>> +
>> +def read_db(boards, option_filter, binary_types):
> Needs function comment again.
>
>> +    defconfig = ""
>> +    _db = db()
>> +
>> +    # Read in the database
>> +    with open(CONFIG_DATABASE) as fd:
>> +        for line in fd.readlines():
>> +            line = line.rstrip()
>> +            if not line:  # Separator between defconfigs.
>> +                # We do not really care. We detect a new config by the absence
>> +                # of ' 'at the beginning of the line
>> +                pass
>> +            elif line[0] == ' ':  # CONFIG_xxx line
>> +                if t and option_filter(line):
>> +                    config, value = line.strip().split('=', 1)
>> +                    _db.add_option(t, config, value)
>> +            else:  # New defconfig
>> +                infos = line.split()
>> +                defconfig = infos[0]
>> +                try:
>> +                    binary_type = infos[1]
>> +                except:
>> +                    binary_type = "u-boot"
>> +                if binary_type in binary_types and defconfig in boards:
>> +                    t = target(defconfig, binary_type)
>> +                    _db.add_target(t)
>> +                else:
>> +                    t = None
>> +    return _db
>> +
>> +
>> +def main():
>> +    parser = argparse.ArgumentParser(description="Show CONFIG options usage")
>> +    parser.add_argument("options", metavar='OPTION', type=str, nargs='+',
>> +                        help="regexp to filter on options.\
>> +        ex: CONFIG_DM_I2C_COMPAT or '.*DM_MMC.*'")
>> +    parser.add_argument(
>> +        "-X", help="show a X instead of the value of the option",
> Please capitalise the help, e.g. 'Show a X'
>
>> +                        action="store_true")
>> +    parser.add_argument("--u-boot", help="parse the u-boot configs",
>> +                        action="store_true")
>> +    parser.add_argument("--spl", help="parse the SPL configs",
>> +                        action="store_true")
>> +    parser.add_argument("--tpl", help="parse the TPL configs",
>> +                        action="store_true")
>> +    parser.add_argument("--diff",
>> +                        help="show only the options that differs between the selected configs (SPL, TPL, u-boot)",
>> +                        action="store_true")
>> +    parser.add_argument("--rebuild-db",
>> +                        help="Force a rebuild of the config database",
>> +                        action="store_true")
>> +    parser.add_argument('-j', '--jobs',
>> +                        help='the number of jobs to run simultaneously')
>> +    parser.add_argument('-o', '--output',
>> +                        help='The output CSV filename. uses stdout if not specified')
>> +    parser.add_argument('--no-header', help='Do not put the header at the top',
>> +                        action="store_true")
>> +    parser.add_argument('--discard-empty', action="store_true",
>> +                        help='Discard the empty rows (defconfigs that do not enable at least one option)')
>> +
>> +    find_defconfigs.update_parser_with_default_options(parser)
>> +    args = parser.parse_args()
>> +
>> +    # generate db file if needed or requested
>> +    # The job of generating the db is actually done by tools/moveconfig.py
>> +    # (called with -B)
>> +    if args.rebuild_db or not os.path.isfile(CONFIG_DATABASE):
>> +        find_defconfig_args = ["--{} '{}'".format(f, getattr(args, f))
>> +                               for f in find_defconfigs.get_default_options()
>> +                               if getattr(args, f)]
>> +        if args.jobs:
>> +            jobs_option = "-j {}".format(args.jobs)
>> +        else:
>> +            jobs_option = ""
>> +
>> +        rc = os.system(
>> +            "tools/find_defconfigs.py {} | tools/moveconfig.py -B {} -d - 1>&2 "
>> +            .format(" ".join(find_defconfig_args), jobs_option))
>> +        if rc:
>> +            sys.exit(1)
> This is fine, but I wonder why you don't import this module and call
> it directly? It could return a dict, perhaps?
I tried at first but ended up with a lot of changes in moveconfig.py.
moveconfig.py has evolved and grown in complexity overtime. It could be 
split into small independent programs using a generic core to build and 
parse the configs. I am not sure that it is worth it though.

>> +
>> +    # get a list of defconfigs matching the rules
>> +    targets = [t for t in find_defconfigs.get_matching_boards(args)]
>> +    defconfigs = [t.defconfig for t in targets]
>> +
>> +    # create a list of binary types we are interested in
>> +    binary_types = []
>> +    if args.spl:
>> +        binary_types.append("SPL")
>> +    if args.tpl:
>> +        binary_types.append("TPL")
>> +    if args.u_boot or not binary_types:
>> +        binary_types.append("u-boot")
>> +
>> +    # define a function used to filter on the options
>> +    rules = [re.compile("   {}=".format(cfg_opt))
>> +             for cfg_opt in args.options]
>> +
>> +    def match_any_rule(line):
>> +        for r in rules:
>> +            if r.match(line):
>> +                return True
>> +        return False
>> +
>> +    # read the database
>> +    db = read_db(defconfigs, match_any_rule, binary_types)
>> +
>> +    target_dict = {}
>> +    for t in targets:
>> +        target_dict[t.defconfig] = t
>> +
>> +    def populate_left_columns(target=None, dic=None, header=True):
>> +        if header:
>> +            return ["vendor", "soc", "defconfig", "type"]
>> +        else:
>> +            dic[target]["vendor"] = target_dict[target.defconfig].vendor
>> +            dic[target]["soc"] = target_dict[target.defconfig].soc
>> +            dic[target]["defconfig"] = target.defconfig
>> +            dic[target]["type"] = target.binary_type
>> +
>> +    def populate_left_columns_diff(target=None, dic=None, header=True):
>> +        if header:
>> +            return ["vendor", "soc", "defconfig"]
>> +        else:
>> +            dic[target]["vendor"] = target_dict[target.defconfig].vendor
>> +            dic[target]["soc"] = target_dict[target.defconfig].soc
>> +            dic[target]["defconfig"] = target.defconfig
>> +
>> +    if args.output:
>> +        out = open(args.output, "w")
>> +    else:
>> +        out = sys.stdout
>> +
>> +    if args.diff:
>> +        db.diff().output_csv(output=out, show_X=False,
>> +                             header=not args.no_header,
>> +                             discard_empty_rows=args.discard_empty,
>> +                             left_columns=populate_left_columns_diff)
>> +    else:
>> +        db.output_csv(output=out, show_X=args.X, header=not args.no_header,
>> +                      discard_empty_rows=args.discard_empty,
>> +                      left_columns=populate_left_columns)
>> +
>> +    if out != sys.stdout:
>> +        out.close()
>> +
>> +if __name__ == '__main__':
>> +    main()
>> --
>> 2.7.4
>>
> Regards,
> Simon
>



More information about the U-Boot mailing list