Re: [PATCH v8 04/12] tools: mkeficapsule: add man page

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

Heinrich,

On Sat, Jan 01, 2022 at 11:02:52PM +0100, Heinrich Schuchardt wrote:
> On 12/20/21 06:02, AKASHI Takahiro wrote:
> > Add a man page for mkeficapsule command.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >   MAINTAINERS        |  1 +
> >   doc/mkeficapsule.1 | 95 ++++++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 96 insertions(+)
> >   create mode 100644 doc/mkeficapsule.1
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index e718ad213553..93ef5e297acc 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -723,6 +723,7 @@ S:	Maintained
> >   T:	git https://source.denx.de/u-boot/custodians/u-boot-efi.git
> >   F:	doc/api/efi.rst
> >   F:	doc/develop/uefi/*
> > +F:	doc/mkeficapsule.1
> >   F:	doc/usage/bootefi.rst
> >   F:	drivers/rtc/emul_rtc.c
> >   F:	include/capitalization.h
> > diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1
> > new file mode 100644
> > index 000000000000..837e09ab451e
> > --- /dev/null
> > +++ b/doc/mkeficapsule.1
> > @@ -0,0 +1,95 @@
> 
> Please, provide copyright information. Cf.
> https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man7/fanotify.7.

OK

> > +.TH MAEFICAPSULE 1 "May 2021"
> > +
> > +.SH NAME
> > +mkeficapsule \- Generate EFI capsule file for U-Boot
> > +
> > +.SH SYNOPSIS
> > +.B mkeficapsule
> > +.RB [\fIoptions\fP] " \fIcapsule-file\fP"
> 
> .RI [ options ] " capsule-file

I don't have any strong preference for those notations,
but I simply followed the existing format used in doc/kwboot.1
and doc/mkimage.1 which are the only two instances of
man pages in the U-Boot source.

So I'd like to see guidelines/rules for U-Boot first.

> > +
> > +.SH "DESCRIPTION"
> > +The
> > +\fBmkeficapsule\fP
> 
> .B mkeficapsule

ditto
I can find a use of "\fB" (and others) also in your reference:
> https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/man7/fanotify.7.

> > +command is used to create an EFI capsule file for use with the U-Boot
> > +EFI capsule update.
> > +A capsule file may contain various type of firmware blobs which
> > +are to be applied to the system and must be placed in the specific
> > +directory on the UEFI system partition. An update will be automatically
> > +executed at next reboot.
> > +
> > +Optionally, a capsule file can be signed with a given private key.
> > +In this case, the update will be authenticated by verifying the signature
> > +before applying.
> > +
> > +\fBmkeficapsule\fP supports two different format of image files:
> 
> .B mkeficapsule
> supports two different format of image files:
> 
> > +.TP
> > +.I raw image
> > +format is a single binary blob of any type of firmware.
> > +
> > +.TP
> > +.I FIT (Flattened Image Tree) image
> > +format
> > +is the same as used in the new \fIuImage\fP format and allows for
> 
> is the same as used in the new
> .I uImage
> format and allows for
> 
> But why would you use italics for uImage? It is neither a command nor a
> parameter.

I don't know, but will drop the decoration here.

> Please, rework the man page to avoid escape sequences.
>
> Best regards
> 
> Heinrich
> 
> > +multiple binary blobs in a single capsule file.
> > +This type of image file can be generated by \fBmkimage\fP.
> > +
> > +.SH "OPTIONS"
> > +One of \fB--fit\fP or \fB--raw\fP option must be specified.
> > +
> > +.TP
> > +.BI "-f, --fit \fIfit-image-file\fP"
> > +Specify a FIT image file
> > +
> > +.TP
> > +.BI "-r, --raw \fIraw-image-file\fP"
> > +Specify a raw image file
> > +
> > +.TP
> > +.BI "-i, --index \fIindex\fP"
> > +Specify an image index
> > +
> > +.TP
> > +.BI "-I, --instance \fIinstance\fP"
> > +Specify a hardware instance
> > +
> > +.TP
> > +.BI "-h, --help"
> > +Print a help message
> > +
> > +.TP 0
> > +.B With signing:
> > +
> > +\fB--private-key\fP, \fB--certificate\fP and \fB--monotonic-count\fP are
> > +all mandatory.
> > +
> > +.TP
> > +.BI "-p, --private-key \fIprivate-key-file\fP"
> > +Specify signer's private key file in PEM
> > +
> > +.TP
> > +.BI "-c, --certificate \fIcertificate-file\fP"
> > +Specify signer's certificate file in EFI certificate list format
> > +
> > +.TP
> > +.BI "-m, --monotonic-count \fIcount\fP"
> > +Specify a monotonic count which is set to be monotonically incremented
> > +at every firmware update.
> > +
> > +.TP
> > +.BI "-d, --dump_sig"
> > +Dump signature data into *.p7 file
> > +
> > +.PP
> > +.SH FILES
> > +.TP
> > +.BI "\fI/EFI/UpdateCapsule\fP"
> > +The directory in which all capsule files be placed
> > +
> > +.SH SEE ALSO
> > +.B mkimage
> > +
> > +.SH AUTHORS
> > +Written by AKASHI Takahiro <takahiro.akashi@linaro.org>
> 
> man man-pages discourages using an AUTHORS paragraph.

ditto
I see AUTHORS sections in doc/kwboot.1 and doc/mkimage.1.


> Please, put the information into copyright header.
> 
> Best regards
> 
> Heinrich
> 
> > +
> > +.SH HOMEPAGE
> > +http://www.denx.de/wiki/U-Boot/WebHome

For instance, no reference to "HOMEPAGE" in "man man-pages"
but doc/mkimage.1 has one.

-Takahiro Akashi

> 
> 
> 
>