diff mbox series

[v2,15/25] binman: Read the fit entries only once

Message ID 20220223230040.159317-16-sjg@chromium.org
State Superseded
Delegated to: Simon Glass
Headers show
Series binman: rockchip: Migrate from rockchip SPL_FIT_GENERATOR script | expand

Commit Message

Simon Glass Feb. 23, 2022, 11 p.m. UTC
At present the entries are read twice, once by the entry_Section class
and once by the FIT implementation. This is harmless but can be confusing
when debugging. Fix it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/binman/etype/fit.py | 1 -
 1 file changed, 1 deletion(-)

Comments

Alper Nebi Yasak March 3, 2022, 9:10 p.m. UTC | #1
On 24/02/2022 02:00, Simon Glass wrote:
> At present the entries are read twice, once by the entry_Section class
> and once by the FIT implementation. This is harmless but can be confusing
> when debugging. Fix it.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> (no changes since v1)
> 
>  tools/binman/etype/fit.py | 1 -
>  1 file changed, 1 deletion(-)

This is actually necessary so that we can use 'self' as the section in
Entry.Create(self, ...) when creating /image/* sections, which requires
some section-related attributes set by super().ReadNode(). It was one of
the things I was planning to fix as well, thanks!

Reviewed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>

> diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> index 2b82955226..7b0c94dfee 100644
> --- a/tools/binman/etype/fit.py
> +++ b/tools/binman/etype/fit.py
> @@ -185,7 +185,6 @@ class Entry_fit(Entry_section):
>          self.mkimage = None
>  
>      def ReadNode(self):
> -        self.ReadEntries()
>          super().ReadNode()

The entire function can be removed, but I think some stuff from
__init__() actually belongs here so I'm fine with keeping it.

>  
>      def _get_operation(self, subnode):
Simon Glass March 6, 2022, 3:08 a.m. UTC | #2
Hi Alper,

On Thu, 3 Mar 2022 at 14:16, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
>
> On 24/02/2022 02:00, Simon Glass wrote:
> > At present the entries are read twice, once by the entry_Section class
> > and once by the FIT implementation. This is harmless but can be confusing
> > when debugging. Fix it.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> >  tools/binman/etype/fit.py | 1 -
> >  1 file changed, 1 deletion(-)
>
> This is actually necessary so that we can use 'self' as the section in
> Entry.Create(self, ...) when creating /image/* sections, which requires
> some section-related attributes set by super().ReadNode(). It was one of
> the things I was planning to fix as well, thanks!

OK good.

>
> Reviewed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
>
> > diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> > index 2b82955226..7b0c94dfee 100644
> > --- a/tools/binman/etype/fit.py
> > +++ b/tools/binman/etype/fit.py
> > @@ -185,7 +185,6 @@ class Entry_fit(Entry_section):
> >          self.mkimage = None
> >
> >      def ReadNode(self):
> > -        self.ReadEntries()
> >          super().ReadNode()
>
> The entire function can be removed, but I think some stuff from
> __init__() actually belongs here so I'm fine with keeping it.

Ah yes. I added a patch to move things as you say.

>
> >
> >      def _get_operation(self, subnode):

Regards,
Simon
diff mbox series

Patch

diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index 2b82955226..7b0c94dfee 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -185,7 +185,6 @@  class Entry_fit(Entry_section):
         self.mkimage = None
 
     def ReadNode(self):
-        self.ReadEntries()
         super().ReadNode()
 
     def _get_operation(self, subnode):