[U-Boot] [PATCH v3 4/6] binman: add optional support for U-Boot image signing

[Anatolij Gustschin]

Hi Simon,

On Mon, 20 Nov 2017 08:40:29 -0700
Simon Glass sjg at chromium.org wrote:
...
> >  tools/binman/binman.py         |  3 +++
> >  tools/binman/cmdline.py        |  2 ++
> >  tools/binman/control.py        |  1 +
> >  tools/binman/image.py          | 23 +++++++++++++++++++++++
> >  tools/binman/signing/signer.py | 22 ++++++++++++++++++++++
> >  5 files changed, 51 insertions(+)
> >  create mode 100644 tools/binman/signing/signer.py  
> 
> This looks reasonable to me, but can you please add a test and also
> docs to the binman README for this new feature?
> 
> You might want to rebase on top of my recent fixes because there are
> some test problems at present.
> 
> dm/binman-working
> 
> I'll hopefully pull these in this week.

OK, thanks! Yes, I'll add some docs to README and try to come up
with a test.

...
> > diff --git a/tools/binman/control.py b/tools/binman/control.py
> > index ffa2bbd80f..2ad1ebf3fb 100644
> > --- a/tools/binman/control.py
> > +++ b/tools/binman/control.py
> > @@ -113,6 +113,7 @@ def Binman(options, args):
> >                  image.ProcessEntryContents()
> >                  image.WriteSymbols()
> >                  image.BuildImage()
> > +                image.SignImage(options)  
> 
> Can you put the info somewhere, or pass the info you need out of
> options? I am not keen on the options object bleeding into the image
> code.

Yes, will do in v4 patch. I can just pass options.keydir here and use
indir/outdir from tools.

...
> > +class ImageSigner(object):
> > +    def __init__(self, sign_func):
> > +        self.sign = sign_func
> > +
> > +def GetImageSigner(soc):  
> 
> Function comment.

OK, added in v4.

Thanks,
Anatolij