Re: [PATCH v3 19/19] doc: board: ti: Update documentation for binman flow

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 4/24/23 22:49, Tom Rini wrote:
> On Mon, Apr 24, 2023 at 01:42:17PM -0600, Simon Glass wrote:
>> Hi Neha,
>>
>> On Fri, 21 Apr 2023 at 06:33, Neha Malcom Francis <n-francis@ti.com> wrote:
>>>
>>> Earlier documentation specified builds for generating bootloader images
>>> using an external TI repository k3-image-gen and core-secdev-k3. Modify
>>> this to using the binman flow so that user understands how to build the
>>> final boot images.
>>>
>>> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
>>> ---
>>>   doc/board/ti/am62x_sk.rst  | 20 ++++--------
>>>   doc/board/ti/j721e_evm.rst | 37 ++++++++-------------
>>>   doc/board/ti/k3.rst        | 67 ++++++++++----------------------------
>>>   3 files changed, 36 insertions(+), 88 deletions(-)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>>>
>>> diff --git a/doc/board/ti/am62x_sk.rst b/doc/board/ti/am62x_sk.rst
>>> index b1b7d99bef..f5e0774899 100644
>>> --- a/doc/board/ti/am62x_sk.rst
>>> +++ b/doc/board/ti/am62x_sk.rst
>>> @@ -115,23 +115,19 @@ Below is the pictorial representation of boot flow:
>>>
>>>   Sources:
>>>   --------
>>> -1. SYSFW:
>>> -       Tree: git://git.ti.com/k3-image-gen/k3-image-gen.git
>>> -       Branch: master
>>> -
>>> -2. ATF:
>>> +1. ATF:
>>>          Tree: https://github.com/ARM-software/arm-trusted-firmware.git
>>>          Branch: master
>>>
>>> -3. OPTEE:
>>> +2. OPTEE:
>>>          Tree: https://github.com/OP-TEE/optee_os.git
>>>          Branch: master
>>>
>>> -4. U-Boot:
>>> +3. U-Boot:
>>>          Tree: https://source.denx.de/u-boot/u-boot
>>>          Branch: master
>>>
>>> -5. TI Linux Firmware:
>>> +4. TI Linux Firmware:
>>>          Tree: git://git.ti.com/processor-firmware/ti-linux-firmware.git
>>>          Branch: ti-linux-firmware
>>>
>>> @@ -156,18 +152,14 @@ Build procedure:
>>>   .. code-block:: text
>>>
>>>    $ make ARCH=arm CROSS_COMPILE=arm-none-linux-gnueabihf- am62x_evm_r5_defconfig O=/tmp/r5
>>> - $ make ARCH=arm CROSS_COMPILE=arm-none-linux-gnueabihf- O=/tmp/r5
>>> - $ cd <k3-image-gen>
>>> - $ make ARCH=arm CROSS_COMPILE=arm-none-linux-gnueabihf- SOC=am62x SBL=/tmp/r5/spl/u-boot-spl.bin SYSFW_PATH=<path to ti-linux-firmware>/ti-sysfw/ti-fs-firmware-am62x-gp.bin
>>> -
>>> -Use the tiboot3.bin generated from last command
>>> + $ make ARCH=arm CROSS_COMPILE=arm-none-linux-gnueabihf- O=/tmp/r5 BINMAN_INDIRS=<path/to/ti-linux-firmware>
>>>
>>>   * 3.2 A53:
>>>
>>>   .. code-block:: text
>>>
>>>    $ make ARCH=arm CROSS_COMPILE=aarch64-none-linux-gnu- am62x_evm_a53_defconfig O=/tmp/a53
>>> - $ make ARCH=arm CROSS_COMPILE=aarch64-none-linux-gnu- ATF=<path to ATF dir>/build/k3/lite/release/bl31.bin TEE=<path to OPTEE OS dir>/out/arm-plat-k3/core/tee-pager_v2.bin DM=<path to ti-linux-firmware>/ti-dm/am62xx/ipc_echo_testb_mcu1_0_release_strip.xer5f O=/tmp/a53
>>> + $ make ARCH=arm CROSS_COMPILE=aarch64-none-linux-gnu- BL31=<path to ATF dir>/build/k3/lite/release/bl31.bin TEE=<path to OPTEE OS dir>/out/arm-plat-k3/core/tee-pager_v2.bin BINMAN_INDIRS=<path/to/ti-linux-firmware>
>>>
>>>   Target Images
>>>   --------------
>>> diff --git a/doc/board/ti/j721e_evm.rst b/doc/board/ti/j721e_evm.rst
>>> index e898601c41..c90e2b8279 100644
>>> --- a/doc/board/ti/j721e_evm.rst
>>> +++ b/doc/board/ti/j721e_evm.rst
>>> @@ -130,67 +130,56 @@ support. Below is the pictorial representation of boot flow:
>>>
>>>   Sources:
>>>   --------
>>> -1. SYSFW:
>>> -       Tree: git://git.ti.com/k3-image-gen/k3-image-gen.git
>>> -       Branch: master
>>> -
>>> -2. ATF:
>>> +1. ATF:
>>>          Tree: https://github.com/ARM-software/arm-trusted-firmware.git
>>>          Branch: master
>>>
>>> -3. OPTEE:
>>> +2. OPTEE:
>>>          Tree: https://github.com/OP-TEE/optee_os.git
>>>          Branch: master
>>>
>>> -4. DM Firmware:
>>> -       Tree: git://git.ti.com/processor-firmware/ti-linux-firmware.git
>>> -       Branch: ti-linux-firmware
>>> -
>>> -5. U-Boot:
>>> +3. U-Boot:
>>>          Tree: https://source.denx.de/u-boot/u-boot
>>>          Branch: master
>>>
>>> +4. TI Linux Firmware:
>>> +       Tree: git://git.ti.com/processor-firmware/ti-linux-firmware.git
>>> +       Branch: ti-linux-firmware
>>> +
>>>   Build procedure:
>>>   ----------------
>>> -1. SYSFW:
>>> -
>>> -.. code-block:: bash
>>> -
>>> -    make CROSS_COMPILE=arm-linux-gnueabihf- SOC=j721e
>>> -
>>> -2. ATF:
>>> +1. ATF:
>>>
>>>   .. code-block:: bash
>>>
>>>       make CROSS_COMPILE=aarch64-linux-gnu- ARCH=aarch64 PLAT=k3 TARGET_BOARD=generic SPD=opteed
>>>
>>> -3. OPTEE:
>>> +2. OPTEE:
>>>
>>>   .. code-block:: bash
>>>
>>>       make PLATFORM=k3-j721e CFG_ARM64_core=y
>>>
>>> -4. U-Boot:
>>> +3. U-Boot:
>>>
>>>   * 4.1 R5:
>>>
>>>   .. code-block:: bash
>>>
>>>       make CROSS_COMPILE=arm-linux-gnueabihf- j721e_evm_r5_defconfig O=build/r5

CROSS_COMPILE is superfluous here.

>>> -    make CROSS_COMPILE=arm-linux-gnueabihf- O=build/r5
>>> +    make CROSS_COMPILE=arm-linux-gnueabihf- O=build/r5 BINMAN_INDIRS=<path/to/ti-linux-firmware>

Don't we have to indicate the locations of the TF-A binary and SYSFW_PATH?

>>>
>>>   * 4.2 A72:
>>>
>>>   .. code-block:: bash
>>>
>>>       make CROSS_COMPILE=aarch64-linux-gnu- j721e_evm_a72_defconfig O=build/a72

CROSS_COMPILE is superfluous here.

>>> -    make CROSS_COMPILE=aarch64-linux-gnu- ATF=<ATF dir>/build/k3/generic/release/bl31.bin TEE=<OPTEE OS dir>/out/arm-plat-k3/core/tee-pager_v2.bin DM=<DM firmware>/ti-dm/j721e/ipc_echo_testb_mcu1_0_release_strip.xer5f O=build/a72
>>> +    make CROSS_COMPILE=aarch64-linux-gnu- BL31=<ATF dir>/build/k3/generic/release/bl31.bin TEE=<OPTEE OS dir>/out/arm-plat-k3/core/tee-pager_v2.bin BINMAN_INDIRS=<path/to/ti-linux-firmware>

Where do we indicate SYSFW_PATH?

>>
>> Is there an 'O=build/a72' missing there?
>
> Not strictly missing, no. One of those style things for Heinrich is, we
> should be consistent in our examples both in terms of O= (when not
> strictly required, it is for the _r5 variant since you must keep some of
> that build for the Cortex-A part, so that you can get something that
> boots the physical machine, in the end).
>
>> You can combine these as well, by using 'j721e_evm_a72_defconfig all'
>> to both configure and build.
>>
>> Also it might be nice to keep the line length shorter if you can.
>
> Other parts for Heinrich to chime in, to have consistent on all the
> board examples.
>

O= is not really needed outside of 'make tests'.
So I think dropping it from examples is fine if the document in itself
stays consistent.

We should try to stick to 80 characters per line in documentation for
the sake of readers who read the source (as described in
https://u-boot.readthedocs.io/en/latest/develop/docstyle.html). Even in
HTML pages it is not nice if you have to scroll horizontally to read the
complete command.

Please, use \ to split the bash command over multiple lines.

Best regards

Heinrich