| Message ID | 20240801125416.1582652-1-lukma@denx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Fabio Estevam |
| Headers | show |
| Series | [1/2] config: Add 'update_bootimg' command to update flash.bin on Phytec's imx8mm | expand |
Hi Lukasz, On Thu, Aug 1, 2024 at 9:54 AM Lukasz Majewski <lukma@denx.de> wrote: > "mmcdev=" __stringify(CONFIG_SYS_MMC_ENV_DEV) "\0" \ > "mmcpart=1\0" \ > "mmcroot=2\0" \ > + "update_mmc_part=1\0" \ > + "update_offset=0x42\0" \ > + "update_filename=flash.bin\0" \ > + "hostname=/tftpboot/lukma/\0" \ I don't think all the users will like to use this tftpboot path :-)
On Thu, Aug 01, 2024 at 10:13:13AM -0300, Fabio Estevam wrote: > Hi Lukasz, > > On Thu, Aug 1, 2024 at 9:54 AM Lukasz Majewski <lukma@denx.de> wrote: > > > "mmcdev=" __stringify(CONFIG_SYS_MMC_ENV_DEV) "\0" \ > > "mmcpart=1\0" \ > > "mmcroot=2\0" \ > > + "update_mmc_part=1\0" \ > > + "update_offset=0x42\0" \ > > + "update_filename=flash.bin\0" \ > > + "hostname=/tftpboot/lukma/\0" \ > > I don't think all the users will like to use this tftpboot path :-) And while we're touching these boards, can we get them migrated to plain text environment please?
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@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" \
Hi Benjamin, > 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@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. Yes, for updating u-boot it is not required. I will remove it. > > > + "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. I can follow your documentation adn add /srv/tftp > > + "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} > When you don't provide the <loadaddr> the one from ${loadaddr} is used by default. However, yes - the latter - i.e. one with ${loadaddr} is more readable and shall be used. > > + "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? Yes, it is. The / division is only giving you the value without reminder. As the filesize can be not aligned to 0x200 you wouldn't flash the whole binary. > For me it > works also without the one. Sometimes it works, sometimes not - it depends if you crop the relevant part. > 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 > If you are 100% sure that all the time the size of the binary is aligned to 0x200, then you can omit the 1 as well. > (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" \ > > 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
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" \ + "update_offset=0x42\0" \ + "update_filename=flash.bin\0" \ + "hostname=/tftpboot/lukma/\0" \ + "update_bootimg=" \ + "mmc dev ${mmcdev} ; " \ + "if dhcp ${hostname}/${update_filename} ; then " \ + "setexpr fw_sz ${filesize} / 0x200 ; " /* SD block size */ \ + "setexpr fw_sz ${fw_sz} + 1 ; " \ + "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" \
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@denx.de> --- include/configs/phycore_imx8mm.h | 11 +++++++++++ 1 file changed, 11 insertions(+)