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

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

[Lukasz Majewski]

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 "	\
+		"setexpr fw_sz ${filesize} / 0x200 ; "	/* SD block size */ \
+		"setexpr fw_sz ${fw_sz} ; "				\
+		"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" \
-- 
2.39.2

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

[Fabio Estevam]

Hi Lukasz,

On Tue, Aug 6, 2024 at 5:11 AM Lukasz Majewski <lukma@denx.de> 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

It looks good to me now.

Teresa/Benjamin/Cem,

Are you happy with this version?

Thanks

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

[Benjamin Hahn]

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 because it can't 
find it. So you can drop the ${hostname}.
Also drop the initialization of ${hostname} above since it is no longer 
needed.

Should be like this:

dhcp ${loadaddr} ${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.

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

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

[Lukasz Majewski]

[-- 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 --]

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

[Teresa Remmet]

Hello Lukasz,

Am Mittwoch, dem 07.08.2024 um 13:04 +0200 schrieb Lukasz Majewski:
> 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.

do I understand it correct that you just want to be able to define sub
dirs with a variable? This does make sense but you should not pre-
define them here.

And I think the variable name $hostname is kind of miss leading. Maybe
you can find some different name for it?

Regards,
Teresa

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

-- 
PHYTEC Messtechnik GmbH | Barcelona-Allee 1 | 55129 Mainz, Germany

Geschäftsführer: Dipl.-Ing. Michael Mitezki, Dipl.-Ing. Bodo Huber,
Dipl.-Ing. (FH) Markus Lickes | Handelsregister Mainz HRB 4656 |
Finanzamt Mainz | St.Nr. 266500608, DE 149059855

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

[Lukasz Majewski]

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

Hi Teresa,

> Hello Lukasz,
> 
> Am Mittwoch, dem 07.08.2024 um 13:04 +0200 schrieb Lukasz Majewski:
> > 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.  
> 
> do I understand it correct that you just want to be able to define sub
> dirs with a variable? 

Yes, correct.

> This does make sense but you should not pre-
> define them here.

Ok. I will remove the "hostname=/srv/tftp" environment variable.

> 
> And I think the variable name $hostname is kind of miss leading. Maybe
> you can find some different name for it?

The ${hostname} is used in a few places already in that way. Maybe you
have better idea for this variable's name?

> 
> Regards,
> Teresa
> 
> >   
> > > 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  
> 




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 --]

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

[Teresa Remmet]

Hello Lukasz

Am Donnerstag, dem 08.08.2024 um 12:25 +0200 schrieb Lukasz Majewski:
> Hi Teresa,
> 
> > Hello Lukasz,
> > 
> > Am Mittwoch, dem 07.08.2024 um 13:04 +0200 schrieb Lukasz Majewski:
> > > 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.  
> > 
> > do I understand it correct that you just want to be able to define
> > sub
> > dirs with a variable? 
> 
> Yes, correct.
> 
> > This does make sense but you should not pre-
> > define them here.
> 
> Ok. I will remove the "hostname=/srv/tftp" environment variable.
> 
> > 
> > And I think the variable name $hostname is kind of miss
> > leading. Maybe
> > you can find some different name for it?
> 
> The ${hostname} is used in a few places already in that way. Maybe
> you
> have better idea for this variable's name?

As it is ${update_filename} maybe ${update_filepath}?

Teresa

> 
> > 
> > Regards,
> > Teresa
> > 
> > >   
> > > > 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  
> > 
> 
> 
> 
> 
> 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

-- 
PHYTEC Messtechnik GmbH | Barcelona-Allee 1 | 55129 Mainz, Germany

Geschäftsführer: Dipl.-Ing. Michael Mitezki, Dipl.-Ing. Bodo Huber,
Dipl.-Ing. (FH) Markus Lickes | Handelsregister Mainz HRB 4656 |
Finanzamt Mainz | St.Nr. 266500608, DE 149059855