Re: [PATCH v3] config: Add 'update_bootimg' command to update flash.bin on Phytec's imx8mm

[Lukasz Majewski <lukma@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 2777 bytes --]

Hi Benjamin,

> Hi Lukasz,
> 
> On 06.08.24 10:11, Lukasz Majewski wrote:
> > This command allows easy update on SD card or eMMC of the flash.bin
> > generated (with binman) during u-boot build.
> >
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
> > ---
> > Changes for v2:
> > - Remove 'update_mmc_part' variable
> > - Change path for hostname
> > - Use full version of dhcp command (${loadaddr} added)
> >
> > Changes for v3:
> > - Remove +1 when calculating the size of binary to be written
> > ---
> >   include/configs/phycore_imx8mm.h | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> >
> > diff --git a/include/configs/phycore_imx8mm.h
> > b/include/configs/phycore_imx8mm.h index ce6dc87c69..361e665676
> > 100644 --- a/include/configs/phycore_imx8mm.h
> > +++ b/include/configs/phycore_imx8mm.h
> > @@ -29,6 +29,16 @@
> >   	"mmcdev=" __stringify(CONFIG_SYS_MMC_ENV_DEV) "\0" \
> >   	"mmcpart=1\0" \
> >   	"mmcroot=2\0" \
> > +	"update_offset=0x42\0" \
> > +	"update_filename=flash.bin\0" \
> > +	"hostname=/srv/tftp/\0" \
> > +	"update_bootimg="
> > 	\
> > +		"mmc dev ${mmcdev} ; "		\
> > +		"if dhcp ${loadaddr}
> > ${hostname}/${update_filename} ; then "	\  
> 
> The dhcp and tftp command should take the filepath from the tftp 
> directory and get the tftp directory automatically not the absolute
> path. So this will try to get the file from 
> /path_to_tftp_folder/srv/tftp/flash.bin and will fail

And that is why you do have the ${hostname}/${update_filename}, so when
you have several exported directories - you can specify which one
without the need to modify the 'update_bootimg' command.

> because it
> can't find it. So you can drop the ${hostname}.

I would prefer to keep the ${hostname}/${update_filename} part.

> Also drop the initialization of ${hostname} above since it is no
> longer needed.
> 
> Should be like this:
> 
> dhcp ${loadaddr} ${update_filename}
> 

IMHO the line shall be:

dhcp ${loadaddr} ${hostname}/${update_filename}

> > +		"setexpr fw_sz ${filesize} / 0x200 ; "	/*
> > SD block size */ \
> > +		"setexpr fw_sz ${fw_sz} ; "  
> I think you can drop this line since it does not do anything.
> 

Ok.

> 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" \  
> 
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]