From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 52AECC433EF for ; Tue, 15 Feb 2022 11:54:14 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id A0E5583A64; Tue, 15 Feb 2022 12:53:49 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="eRr9t9Ow"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id AA281839E8; Tue, 15 Feb 2022 12:53:43 +0100 (CET) Received: from mail-ed1-x536.google.com (mail-ed1-x536.google.com [IPv6:2a00:1450:4864:20::536]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 0E3E4838C4 for ; Tue, 15 Feb 2022 12:53:38 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=alpernebiyasak@gmail.com Received: by mail-ed1-x536.google.com with SMTP id u18so31572788edt.6 for ; Tue, 15 Feb 2022 03:53:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:from:subject:to:cc :references:content-language:in-reply-to:content-transfer-encoding; bh=br0b/3DmBK02SeKjtRmzBYJxRTCwEc2kFBYnTmmzKP8=; b=eRr9t9OwvKJY7b4g9CbBSpZEs8lxqPnGhko8T/F+TVbru0QXIpWdxLIHFvOvSDtBAD ECZzwsilKdHW4WWaB437TxnLXtGmsdS3hxyimPVIK1IjpsHhngULnuYM2au4XWOgujUt BjeLPnQX3OaUl9F1jA5c97XXD9M4pXxLcpM5PaAc4O7LooCsrgWWBHYDBD7QDkv78Dqi ADIzAKNmPASVtQO9UX8DF0MqX71uZkQcwY8e2GJvVEXlRMU6i6MXg9DEO3tsbwQkESi4 /O6u7Ahu3mx7/aO0rPoiGd8k6c5q7lps1594lckI0Snc2cMICLAxCS5+5t7ASwu5woDv QbzA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:from :subject:to:cc:references:content-language:in-reply-to :content-transfer-encoding; bh=br0b/3DmBK02SeKjtRmzBYJxRTCwEc2kFBYnTmmzKP8=; b=4yRcmFIW6L9gs9Ea4I1XdV+Op3KsCo9h8IqmOWEj5azt6SFF7r23Wrsqby6+9YyInE xOVu9xdCLMSalVIoseglptN8FNUvQWCPFr2/UhiPn6lWZ3DdWLHyMMcxvC0kO4+loZOR uTNE1s7pt1MS24q4hu1V2bs5AcFjK/6ncQTYiX17AJ606HuOw4oRhMCWaOjKfeDatmLy 1Zr2uBD9pl3NCNxMD+67569o1fs8x5JkozbQt98i+mn2ytA3GLD4onVNLuPV+8ZWeVyo ezkI4g6VJK7I9aS1eHOik2YDPdz96U4TJbGkS4R9oeHr6bU1atc5lTRRgestPLWjrJh6 lhlQ== X-Gm-Message-State: AOAM531r4jSMEwZZ5XxCkezZJW03Vi9KkLo3390lkoCiAxvYAXy/zlgl 02CHCz+WUxuMjDFvOLD8Afk= X-Google-Smtp-Source: ABdhPJwj6bYV/TcZbR+gfTl23OIAGjFPj0FxXS+6MGsGqxGbnoRF1NQa9Y1b2TGpaIj+VBxTqy9nxA== X-Received: by 2002:a05:6402:5109:: with SMTP id m9mr1896208edd.325.1644926017702; Tue, 15 Feb 2022 03:53:37 -0800 (PST) Received: from [192.168.0.74] ([178.233.26.119]) by smtp.gmail.com with ESMTPSA id eo14sm13561378edb.46.2022.02.15.03.53.35 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 15 Feb 2022 03:53:37 -0800 (PST) Message-ID: <25cb670b-523d-0db9-cf26-a9f29ae664fc@gmail.com> Date: Tue, 15 Feb 2022 14:45:59 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:91.0) Gecko/20100101 Thunderbird/91.6.0 From: Alper Nebi Yasak Subject: Re: [PATCH 17/24] binman: fit: Refactor to reduce function size To: Simon Glass , U-Boot Mailing List Cc: huang lin , Jeffy Chen , Kever Yang , Tom Rini , Philippe Reynes , Ivan Mikhaylov , Jan Kiszka References: <20220208185008.35843-1-sjg@chromium.org> <20220208114935.17.If9396449efcf221fff4e68e48264faf22b0ffc66@changeid> Content-Language: en-US In-Reply-To: <20220208114935.17.If9396449efcf221fff4e68e48264faf22b0ffc66@changeid> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean On 08/02/2022 21:50, Simon Glass wrote: > Split subnode and property processing into separate functions to make > the _AddNode() function a little smaller. Tweak a few comments. > > This does not change any functionality. > > Signed-off-by: Simon Glass > --- I know this just moves code around a bit, but I think the code here could be cleaned up much further with a bit of redesign. I'm not sure of the details, but was thinking of at least: - self._add_fit_image() to handle image/* subnodes - self._add_fit_config() to handle configuration/* subnodes - self._gen_fdt_nodes() to handle template nodes by calling the above - Switching away from recursion to iterating subnodes of fixed nodes > > tools/binman/etype/fit.py | 116 ++++++++++++++++++++++++-------------- > 1 file changed, 73 insertions(+), 43 deletions(-) > > diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py > index 6ad4a686df..b159844960 100644 > --- a/tools/binman/etype/fit.py > +++ b/tools/binman/etype/fit.py > @@ -141,12 +141,82 @@ class Entry_fit(Entry): > super().ReadNode() > > def ReadEntries(self): I think the following functions could be moved out of this one into the class scope, even including _AddNode. > + def _process_prop(pname, prop): > + """Process special properties > + > + Handles properties with generated values. At present the only > + supported property is 'default', i.e. the default device tree in > + the configurations node. > + > + Args: > + pname (str): Name of property > + prop (Prop): Property to process > + """ > + if pname == 'default': > + val = prop.value > + # Handle the 'default' property > + if val.startswith('@'): > + if not self._fdts: > + return > + if not self._fit_default_dt: > + self.Raise("Generated 'default' node requires default-dt entry argument") > + if self._fit_default_dt not in self._fdts: > + self.Raise("default-dt entry argument '%s' not found in fdt list: %s" % > + (self._fit_default_dt, > + ', '.join(self._fdts))) > + seq = self._fdts.index(self._fit_default_dt) > + val = val[1:].replace('DEFAULT-SEQ', str(seq + 1)) > + fsw.property_string(pname, val) > + return > + fsw.property(pname, prop.bytes) > + > + def _generate_node(subnode, depth, in_images): > + """Generate nodes from a template > + > + This creates one node for each member of self._fdts using the > + provided template. If a property value contains 'NAME' it is > + replaced with the filename of the FDT. If a property value contains > + SEQ it is replaced with the node sequence number, where 1 is the > + first. > + > + Args: > + subnode (None): Generator node to process > + depth: Current node depth (0 is the base 'fit' node) > + in_images: True if this is inside the 'images' node, so that > + 'data' properties should be generated > + """ > + if self._fdts: > + # Generate nodes for each FDT > + for seq, fdt_fname in enumerate(self._fdts): > + node_name = subnode.name[1:].replace('SEQ', > + str(seq + 1)) > + fname = tools.GetInputFilename(fdt_fname + '.dtb') > + with fsw.add_node(node_name): > + for pname, prop in subnode.props.items(): > + val = prop.bytes.replace( > + b'NAME', tools.ToBytes(fdt_fname)) > + val = val.replace( > + b'SEQ', tools.ToBytes(str(seq + 1))) > + fsw.property(pname, val) > + > + # Add data for 'images' nodes (but not 'config') > + if depth == 1 and in_images: > + fsw.property('data', > + tools.ReadFile(fname)) > + else: > + if self._fdts is None: > + if self._fit_list_prop: > + self.Raise("Generator node requires '%s' entry argument" % > + self._fit_list_prop.value) > + else: > + self.Raise("Generator node requires 'fit,fdt-list' property") > + > def _AddNode(base_node, depth, node): > """Add a node to the FIT > > Args: > base_node: Base Node of the FIT (with 'description' property) > - depth: Current node depth (0 is the base node) > + depth: Current node depth (0 is the base 'fit' node) > node: Current node to process > > There are two cases to deal with: > @@ -156,23 +226,7 @@ class Entry_fit(Entry): > """ > for pname, prop in node.props.items(): > if not pname.startswith('fit,'): > - if pname == 'default': > - val = prop.value > - # Handle the 'default' property > - if val.startswith('@'): > - if not self._fdts: > - continue > - if not self._fit_default_dt: > - self.Raise("Generated 'default' node requires default-dt entry argument") > - if self._fit_default_dt not in self._fdts: > - self.Raise("default-dt entry argument '%s' not found in fdt list: %s" % > - (self._fit_default_dt, > - ', '.join(self._fdts))) > - seq = self._fdts.index(self._fit_default_dt) > - val = val[1:].replace('DEFAULT-SEQ', str(seq + 1)) > - fsw.property_string(pname, val) > - continue > - fsw.property(pname, prop.bytes) > + _process_prop(pname, prop) > > rel_path = node.path[len(base_node.path):] > in_images = rel_path.startswith('/images') > @@ -195,31 +249,7 @@ class Entry_fit(Entry): > # fsw.add_node() or _AddNode() for it. > pass > elif self.GetImage().generate and subnode.name.startswith('@'): > - if self._fdts: > - # Generate notes for each FDT > - for seq, fdt_fname in enumerate(self._fdts): > - node_name = subnode.name[1:].replace('SEQ', > - str(seq + 1)) > - fname = tools.GetInputFilename(fdt_fname + '.dtb') > - with fsw.add_node(node_name): > - for pname, prop in subnode.props.items(): > - val = prop.bytes.replace( > - b'NAME', tools.ToBytes(fdt_fname)) > - val = val.replace( > - b'SEQ', tools.ToBytes(str(seq + 1))) > - fsw.property(pname, val) > - > - # Add data for 'fdt' nodes (but not 'config') > - if depth == 1 and in_images: > - fsw.property('data', > - tools.ReadFile(fname)) > - else: > - if self._fdts is None: > - if self._fit_list_prop: > - self.Raise("Generator node requires '%s' entry argument" % > - self._fit_list_prop.value) > - else: > - self.Raise("Generator node requires 'fit,fdt-list' property") > + _generate_node(subnode, depth, in_images) > else: > with fsw.add_node(subnode.name): > _AddNode(base_node, depth + 1, subnode)