[PATCH 1/2] config: Add 'update_bootimg' command to update flash.bin on Phytec's imx8mm

[Benjamin Hahn]

Hi Lukasz,

On 01.08.24 14:54, Lukasz Majewski wrote:
> This command allows easy update on SD card (hence the update_mmc_part=1)
> of the flash.bin generated during u-boot build.
>
> Signed-off-by: Lukasz Majewski <lukma at denx.de>
> ---
>   include/configs/phycore_imx8mm.h | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/include/configs/phycore_imx8mm.h b/include/configs/phycore_imx8mm.h
> index ce6dc87c69..fdeb11933f 100644
> --- a/include/configs/phycore_imx8mm.h
> +++ b/include/configs/phycore_imx8mm.h
> @@ -29,6 +29,17 @@
>   	"mmcdev=" __stringify(CONFIG_SYS_MMC_ENV_DEV) "\0" \
>   	"mmcpart=1\0" \
>   	"mmcroot=2\0" \
> +	"update_mmc_part=1\0" \

You define the update_mmc_part variable here, but do not use it 
anywhere. You use the mmcdev variable later which is good, because the 
function then works not only for SD-Card, but also for eMMC.

In the commit description you say it is for updating SD-Card, which 
would be mmc dev 1, but the name of the variable would indicate that is 
is for a partition.
As I understand it you want to update just the U-Boot, which is not 
inside any partition, so you would not need this variable.

> +	"update_offset=0x42\0" \
> +	"update_filename=flash.bin\0" \
> +	"hostname=/tftpboot/lukma/\0" \
We adivse to use /srv/tftp  as tftp dir in our BSP documentation. But I 
don't think you need that variable at all, because the dhcp command 
should find the tftp dir automatically.
> +	"update_bootimg="						\
> +		"mmc dev ${mmcdev} ; "		\
> +		"if dhcp ${hostname}/${update_filename} ; then "	\

Does this work? For me it does not. As far as I know the syntax of this command is "dhcp <loadaddr> <filename>".
So I would expect this to be:
dhcp ${loadaddr} ${update_filename}

> +		"setexpr fw_sz ${filesize} / 0x200 ; "	/* SD block size */ \
> +		"setexpr fw_sz ${fw_sz} + 1 ; "				\
Why do you add one here? Is this important for something? For me it 
works also without the one.
It is also described in our BSP documentation without adding one. See:
https://phytec.github.io/doc-bsp-yocto/bsp/imx8/imx8mp/mainline-head.html#flash-emmc-u-boot-image-via-network-from-running-u-boot 

(This is for our imx8mp mainline release but is the same for imx8mm with 
another offset).

Benjamin
> +		"mmc write ${loadaddr} ${update_offset} ${fw_sz} ; "	\
> +		"fi\0" \
>   	"mmcautodetect=yes\0" \
>   	"mmcargs=setenv bootargs console=${console} " \
>   		"root=/dev/mmcblk${mmcdev}p${mmcroot} rootwait rw\0" \