[PATCH v3 2/8] doc: Add gpt command documentation

[Joshua Watt]

On Fri, Aug 25, 2023 at 7:57 PM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 8/25/23 21:38, Joshua Watt wrote:
> > Adds initial documentation for the gpt command
>
> Thanks a lot for filling the gap.
>
> >
> > Signed-off-by: Joshua Watt <JPEWhacker at gmail.com>
> > ---
> >   doc/usage/cmd/gpt.rst | 141 ++++++++++++++++++++++++++++++++++++++++++
> >   doc/usage/index.rst   |   1 +
> >   2 files changed, 142 insertions(+)
> >   create mode 100644 doc/usage/cmd/gpt.rst
> >
> > diff --git a/doc/usage/cmd/gpt.rst b/doc/usage/cmd/gpt.rst
> > new file mode 100644
> > index 0000000000..f6e082fb94
> > --- /dev/null
> > +++ b/doc/usage/cmd/gpt.rst
> > @@ -0,0 +1,141 @@
> > +.. SPDX-License-Identifier: GPL-2.0+
> > +
> > +gpt command
> > +===========
> > +
> > +Synopsis
> > +--------
> > +
> > +::
> > +
> > +    gpt repair <interface> <device no>
> > +    gpt write <interface> <device no> <partition string>
> > +    gpt verify <interface> <device no> <partition string>
>
> <partition string> is not required. Please, add brackets [].

It's not optional; there is no standardized string for the list of GPT
partitions like there is for MBR

>
> > +    gpt setenv <interface> <device no> <partition name>
>
> gpt setenv can be called without partition name which leads to a crash.
>
> If that parameter is meant to be optional, this should be indicated
> here. Otherwise an argc check is missing.

I don't believe the argument is optional, but either way it can be
fixed in a subsequent patch series. I'd rather not hold up the
documentation to fix bugs

>
> > +    gpt enumerate <interface> <device no>
> > +    gpt guid <interface> <device no> [<varname>]
> > +    gpt read <interface> <device no> [<varname>]
> > +    gpt swap <interface> <dev> <name1> <name2>
> > +    gpt rename <interface> <dev> <part> <name>
>
> Thanks a lot for providing a man-page for this command.
>
> If <device no> and <dev> both relate to the same object type, we should
> use the same text. <dev> would be fine.
>
> The sequence looks random. Please, sort the sub-commands either
> logically or alphabetically.

They are sorted to match the gpt command help text. Reordering of both
can be done later if desired.

>
> > +
> > +Description
> > +-----------
> > +
> > +The gpt command lets users read, create, modify, or verify the GPT (GUID
> > +Partition Table) partition layout.
> > +
> > +The syntax of the text description of the partition list is similar to
> > +the one used by the 'mbr' command. The string contains one or more partition
>
> Please, link the mbr page:
>
> by the :doc:`mbr command <mbr>`.
>
> > +descriptors, each separated by a ";". Each descriptor contains one or more
> > +fields, with each field separated by a ",". Fields are either of the form
> > +"key=value" to set a specific value, or simple "flag" to set a boolean flag
>
> At this point it remains unclear to the reader what this ;-separated
> string format relates to.
> Is it an output format?
> Is it a format used in variables?
> Is it used for the parameter 'partition string'?
>
> Maybe describe it after the parameters and relate it to the 'partition
> string' parameter.
>
> Please, describe all parameters (in this indented format):
>
> interface
>      interface for accessing the block device (mmc, sata, scsi, usb, ....)
>
> device no
>      device number
>
> ...
>
> > +
> > +The first descriptor can optionally be used to describe parameters for the
> > +whole disk with the following fields:
> > +
> > +* uuid_disk=UUID - Set the UUID for the disk
> > +
> > +Partition descriptors can have the following fields:
> > +* name=NAME - The partition name, required
>
> Maybe better
>
> name=<NAME>
>
> This is not rendered as an unordered list but everything is in one line.
> Please, add the missing blank lines.
>
> Please, generate the HTML documentation as described in
> https://u-boot.readthedocs.io/en/latest/build/documentation.html
> and check the output before resubmitting.
>
> > +* start=BYTES - The partition start offset in bytes, required
> > +* size=BYTES - The partition size, in bytes or "-" to expand it to the whole free area
> > +* bootable - Set the legacy bootable flag
> > +* uuid=UUID - Set the partition UUID, optional if CONFIG_RANDOM_UUID=y is enabled
> > +* type=UUID - Set the partition type GUID, requires CONFIG_PARTITION_TYPE_GUID=y
> > +
>
> The following should be in a separate 'Examples' section to match the
> other man-pages.
>
> > +Here is an example how to create a 6 partitions, some of the predefined sizes:
> > +
> > +::
> > +
> > +    => setenv gpt_parts 'uuid_disk=bec9fc2a-86c1-483d-8a0e-0109732277d7;
> > +        name=boot,start=4M,size=128M,bootable,type=ebd0a0a2-b9e5-4433-87c0-68b6b72699c7,
> > +        name=rootfs,size=3072M,type=0fc63daf-8483-4772-8e79-3d69d8477de4;
> > +        name=system-data,size=512M,type=0fc63daf-8483-4772-8e79-3d69d8477de4;
> > +        name=[ext],size=-,type=0fc63daf-8483-4772-8e79-3d69d8477de4;
> > +        name=user,size=-,type=0fc63daf-8483-4772-8e79-3d69d8477de4;
> > +        name=modules,size=100M,type=0fc63daf-8483-4772-8e79-3d69d8477de4;
> > +        name=ramdisk,size=8M,type=0fc63daf-8483-4772-8e79-3d69d8477de4
> > +    => gpt write mmc 0 $gpt_parts
> > +
> > +
> > +If 'uuid' is not specified, but CONFIG_RANDOM_UUID is enabled, a random UUID
> > +will be generated for the partition
> > +
> > +The 'gpt verify' command returns 0 if the layout matches the one on the storage
> > +device or 1 if not. To check if the layout on the MMC #0 storage device
> > +matches the provided text description one has to issue following command:
>
> The command can be used with and without partition parameter. This
> should be described here.
>
> I would prefer:
>
> sets the return value $? to 0 (true) if the layout
> or 1 (false) if not
>
> > +
> > +::
> > +
> > +    => gpt verify mmc 0 $gpt_parts
> > +
> > +The verify sub-command is especially useful in the system update scripts:
> > +
> > +::
> > +
> > +    => if gpt verify mmc 0 $gpt_parts; then
> > +         echo GPT layout needs to be updated
> > +         ...
> > +       fi
> > +
> > +The 'gpt write' command returns 0 on success write or 1 on failure.
> > +
> > +The 'gpt setenv' command will set a series of environment variables with
> > +information about a particular partition. The variables are:
> > +
> > +* gpt_partition_addr (the starting offset of the partition, in hexadecimal blocks)
> > +* gpt_partition_size (the size of the partition, in hexadecimal blocks)
>
> The blocks that are not hexadecimal, the numbers are.
>
> gpt_partition_addr (first block of the partition as hexadecimal number)
> gpt_partition_size (number of blocks as hexadecimal number)
>
> > +* gpt_partition_name (the name of the partition)
> > +* gpt_partition_entry (the partition number in the table, e.g. 1, 2, 3, etc.)
>
> This value is currently decimal but this seems to be a bug. We need a
> hexadecimal number to use this variable in a command like
>
>      load host 0:$gpt_partition_entry $loadaddr filename
>
> Another bug is that this command assumes that partitions are
> continuously numbered.
>
> > +
> > +To get the information about the partition named 'rootfs', issue the following
> > +command:
> > +
> > +::
> > +    => gpt setenv mmc 0 rootfs
> > +    => echo ${gpt_partition_addr}
> > +    2000
> > +    => echo ${gpt_partition_size}
> > +    14a000
> > +    => echo ${gpt_partition_name}
> > +    rootfs
> > +    => echo ${gpt_partition_entry}
> > +    2
> > +
> > +The 'gpt enumerate' command will set the variable 'gpt_partition_list' with the
> > +list of partition names on the device. For example:
> > +
> > +::
> > +    => gpt enumerate
> > +    => echo gpt_partition_list
> > +    boot rootfs system-data [ext] user modules ramdisk
>
> As we cannot see from the output if there is a partition 'user' and a
> partition 'modules' or only a single partition 'user modules' this
> sub-command seems to need some rework.

Can be fixed later.

>
> Best regards
>
> Heinrich
>
> > +
> > +The 'gpt guid' command will report the GUID of a disk. If 'varname' is
> > +specified, the command will set the variable to the GUID, otherwise it will be
> > +printed out. For example:
> > +
> > +::
> > +    => gpt guid mmc 0
> > +    bec9fc2a-86c1-483d-8a0e-0109732277d7
> > +    => gpt guid mmc gpt_disk_uuid
> > +    => echo ${gpt_disk_uuid}
> > +    bec9fc2a-86c1-483d-8a0e-0109732277d7
> > +
> > +The 'gpt read' command will print out the current state of the GPT partition
> > +table. If 'varname' is specified, the variable will be filled with a partition
> > +string as described above that is suitable for passing to other 'gpt' commands.
> > +If omitted, a human readable description is printed out.
> > +CONFIG_CMD_GPT_RENAME=y is required.
> > +
> > +The 'gpt swap' command changes the names of all partitions that are named
> > +'name1' to be 'name2', and all partitions named 'name2' to be 'name1'.
> > +CONFIG_CMD_GPT_RENAME=y is required.
> > +
> > +The 'gpt rename' command renames all partitions named 'part' to be 'name1'.
> > +CONFIG_CMD_GPT_RENAME=y is required.
> > +
> > +Configuration
> > +-------------
> > +
> > +To use the 'gpt' command you must specify CONFIG_CMD_GPT=y. To enable 'gpt
> > +read', 'gpt swap' and 'gpt rename', you must specify CONFIG_CMD_GPT_RENAME=y.
> > diff --git a/doc/usage/index.rst b/doc/usage/index.rst
> > index 3326ec82ff..4bfaabbd16 100644
> > --- a/doc/usage/index.rst
> > +++ b/doc/usage/index.rst
> > @@ -65,6 +65,7 @@ Shell commands
> >      cmd/for
> >      cmd/fwu_mdata
> >      cmd/gpio
> > +   cmd/gpt
> >      cmd/host
> >      cmd/imxtract
> >      cmd/load
>