Am 16. Juli 2020 03:36:13 MESZ schrieb AKASHI Takahiro
<takahiro.aka...@linaro.org>:
>Heinrich,
>
>On Wed, Jul 15, 2020 at 12:45:18PM +0200, Heinrich Schuchardt wrote:
>> Using UPDATE_TFTP the firmware can be updated from tFTP by writing to
>NOR
>> flash. The same is possible by defining a dfu command in
>CONFIG_PREBOOT.
>>
>> The dfu command cannot only write to NOR but also to other devices.
>In
>> README.dfutfp UPDATE_TFTP has been marked as deprecated. It is not
>used
>> by any board.
>>
>> Remove tFTP update via CONFIG_UPDATE_TFTP.
>
>This description is not accurate.
What are you missing?
>
>> Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de>
>> ---
>> Currently Gitlab CI lacks a test for dfu tftp. I will try to set one
>up.
>> When it is properly tested I will send a final patch series.
>> But beforehand I would like to know if eliminating UPDATE_TFTP is ok.
>>
>> Best regards
>>
>> Heinrich
>> ---
>> README | 8 ---
>> common/Kconfig | 17 ------
>> common/main.c | 3 -
>> common/update.c | 147
>+--------------------------------------------
>> doc/README.dfutftp | 3 -
>> doc/README.update | 97 ------------------------------
>> 6 files changed, 2 insertions(+), 273 deletions(-)
>> delete mode 100644 doc/README.update
>>
>> diff --git a/README b/README
>> index 2384966a39..c53f72cfa6 100644
>> --- a/README
>> +++ b/README
>> @@ -2104,14 +2104,6 @@ The following options need to be configured:
>>
>> Please see board_init_f function.
>>
>> -- Automatic software updates via TFTP server
>> - CONFIG_UPDATE_TFTP
>> - CONFIG_UPDATE_TFTP_CNT_MAX
>> - CONFIG_UPDATE_TFTP_MSEC_MAX
>> -
>> - These options enable and control the auto-update feature;
>> - for a more detailed description refer to doc/README.update.
>> -
>> - MTD Support (mtdparts command, UBI support)
>> CONFIG_MTD_UBI_WL_THRESHOLD
>> This parameter defines the maximum difference between the
>> highest
>> diff --git a/common/Kconfig b/common/Kconfig
>> index 67b3818fde..ca42ba37b7 100644
>> --- a/common/Kconfig
>> +++ b/common/Kconfig
>> @@ -1014,23 +1014,6 @@ endmenu
>>
>> menu "Update support"
>>
>> -config UPDATE_TFTP
>> - bool "Auto-update using fitImage via TFTP"
>> - depends on FIT
>> - help
>> - This option allows performing update of NOR with data in fitImage
>> - sent via TFTP boot.
>
>NAK.
>If this configuration is removed, common/update.c won't be compiled in.
>There is a chain of dependency:
>DFU -> DFU_TFTP -> DFU_OVER_TFTP ->(implicitly) UPDATE_TFTP
No. In common/Makefile there are orginally two lines for update.o
obj-$(CONFIG_UPDATE_TFTP) += update.o
obj-$(CONFIG_DFU_TFTP) += update.o
>
>> -config UPDATE_TFTP_CNT_MAX
>> - int "The number of connection retries during auto-update"
>> - default 0
>> - depends on UPDATE_TFTP
>> -
>> -config UPDATE_TFTP_MSEC_MAX
>> - int "Delay in mSec to wait for the TFTP server during auto-update"
>> - default 100
>> - depends on UPDATE_TFTP
>> -
>> config ANDROID_AB
>> bool "Android A/B updates"
>> default n
>> diff --git a/common/main.c b/common/main.c
>> index 4b3cd302c3..62ab3344e5 100644
>> --- a/common/main.c
>> +++ b/common/main.c
>> @@ -50,9 +50,6 @@ void main_loop(void)
>> if (IS_ENABLED(CONFIG_USE_PREBOOT))
>> run_preboot_environment_command();
>>
>> - if (IS_ENABLED(CONFIG_UPDATE_TFTP))
>> - update_tftp(0UL, NULL, NULL);
>> -
>> s = bootdelay_process();
>> if (cli_process_fdt(&s))
>> cli_secure_boot_cmd(s);
>> diff --git a/common/update.c b/common/update.c
>> index c8dd346a09..caf74e63db 100644
>> --- a/common/update.c
>> +++ b/common/update.c
>> @@ -14,10 +14,6 @@
>> #error "CONFIG_FIT and CONFIG_OF_LIBFDT are required for auto-update
>feature"
>> #endif
>>
>> -#if defined(CONFIG_UPDATE_TFTP) && !defined(CONFIG_MTD_NOR_FLASH)
>> -#error "CONFIG_UPDATE_TFTP and !CONFIG_MTD_NOR_FLASH needed for
>legacy behaviour"
>> -#endif
>> -
>> #include <command.h>
>> #include <env.h>
>> #include <flash.h>
>> @@ -26,7 +22,6 @@
>> #include <malloc.h>
>> #include <dfu.h>
>> #include <errno.h>
>> -#include <mtd/cfi_flash.h>
>>
>> /* env variable holding the location of the update file */
>> #define UPDATE_FILE_ENV "updatefile"
>> @@ -46,10 +41,7 @@
>>
>> extern ulong tftp_timeout_ms;
>> extern int tftp_timeout_count_max;
>> -#ifdef CONFIG_MTD_NOR_FLASH
>> -extern flash_info_t flash_info[];
>> -static uchar *saved_prot_info;
>> -#endif
>> +
>> static int update_load(char *filename, ulong msec_max, int cnt_max,
>ulong addr)
>> {
>> int size, rv;
>> @@ -98,122 +90,6 @@ static int update_load(char *filename, ulong
>msec_max, int cnt_max, ulong addr)
>> return rv;
>> }
>>
>> -#ifdef CONFIG_MTD_NOR_FLASH
>> -static int update_flash_protect(int prot, ulong addr_first, ulong
>addr_last)
>> -{
>> - uchar *sp_info_ptr;
>> - ulong s;
>> - int i, bank, cnt;
>> - flash_info_t *info;
>> -
>> - sp_info_ptr = NULL;
>> -
>> - if (prot == 0) {
>> - saved_prot_info =
>> - calloc(CONFIG_SYS_MAX_FLASH_BANKS *
>> CONFIG_SYS_MAX_FLASH_SECT,
>1);
>> - if (!saved_prot_info)
>> - return 1;
>> - }
>> -
>> - for (bank = 0; bank < CONFIG_SYS_MAX_FLASH_BANKS; ++bank) {
>> - cnt = 0;
>> - info = &flash_info[bank];
>> -
>> - /* Nothing to do if the bank doesn't exist */
>> - if (info->sector_count == 0)
>> - return 0;
>> -
>> - /* Point to current bank protection information */
>> - sp_info_ptr = saved_prot_info + (bank *
>CONFIG_SYS_MAX_FLASH_SECT);
>> -
>> - /*
>> - * Adjust addr_first or addr_last if we are on bank boundary.
>> - * Address space between banks must be continuous for other
>> - * flash functions (like flash_sect_erase or flash_write) to
>> - * succeed. Banks must also be numbered in correct order,
>> - * according to increasing addresses.
>> - */
>> - if (addr_last > info->start[0] + info->size - 1)
>> - addr_last = info->start[0] + info->size - 1;
>> - if (addr_first < info->start[0])
>> - addr_first = info->start[0];
>> -
>> - for (i = 0; i < info->sector_count; i++) {
>> - /* Save current information about protected sectors */
>> - if (prot == 0) {
>> - s = info->start[i];
>> - if ((s >= addr_first) && (s <= addr_last))
>> - sp_info_ptr[i] = info->protect[i];
>> -
>> - }
>> -
>> - /* Protect/unprotect sectors */
>> - if (sp_info_ptr[i] == 1) {
>> -#if defined(CONFIG_SYS_FLASH_PROTECTION)
>> - if (flash_real_protect(info, i, prot))
>> - return 1;
>> -#else
>> - info->protect[i] = prot;
>> -#endif
>> - cnt++;
>> - }
>> - }
>> -
>> - if (cnt) {
>> - printf("%sProtected %d sectors\n",
>> - prot ? "": "Un-", cnt);
>> - }
>> - }
>> -
>> - if((prot == 1) && saved_prot_info)
>> - free(saved_prot_info);
>> -
>> - return 0;
>> -}
>> -#endif
>> -
>> -static int update_flash(ulong addr_source, ulong addr_first, ulong
>size)
>> -{
>> -#ifdef CONFIG_MTD_NOR_FLASH
>> - ulong addr_last = addr_first + size - 1;
>> -
>> - /* round last address to the sector boundary */
>> - if (flash_sect_roundb(&addr_last) > 0)
>> - return 1;
>> -
>> - if (addr_first >= addr_last) {
>> - printf("Error: end address exceeds addressing space\n");
>> - return 1;
>> - }
>> -
>> - /* remove protection on processed sectors */
>> - if (update_flash_protect(0, addr_first, addr_last) > 0) {
>> - printf("Error: could not unprotect flash sectors\n");
>> - return 1;
>> - }
>> -
>> - printf("Erasing 0x%08lx - 0x%08lx", addr_first, addr_last);
>> - if (flash_sect_erase(addr_first, addr_last) > 0) {
>> - printf("Error: could not erase flash\n");
>> - return 1;
>> - }
>> -
>> - printf("Copying to flash...");
>> - if (flash_write((char *)addr_source, addr_first, size) > 0) {
>> - printf("Error: could not copy to flash\n");
>> - return 1;
>> - }
>> - printf("done\n");
>> -
>> - /* enable protection on processed sectors */
>> - if (update_flash_protect(1, addr_first, addr_last) > 0) {
>> - printf("Error: could not protect flash sectors\n");
>> - return 1;
>> - }
>> -#endif
>> - return 0;
>> -}
>> -
>> static int update_fit_getparams(const void *fit, int noffset, ulong
>*addr,
>> ulong *fladdr, ulong *size)
>> {
>> @@ -235,20 +111,9 @@ int update_tftp(ulong addr, char *interface,
>char *devstring)
>> char *filename, *env_addr, *fit_image_name;
>> ulong update_addr, update_fladdr, update_size;
>> int images_noffset, ndepth, noffset;
>> - bool update_tftp_dfu;
>> int ret = 0;
>> void *fit;
>>
>> - if (interface == NULL && devstring == NULL) {
>> - update_tftp_dfu = false;
>> - } else if (interface && devstring) {
>> - update_tftp_dfu = true;
>> - } else {
>> - pr_err("Interface: %s and devstring: %s not supported!\n",
>> - interface, devstring);
>> - return -EINVAL;
>> - }
>> -
>> /* use already present image */
>> if (addr)
>> goto got_update_file;
>> @@ -315,15 +180,7 @@ got_update_file:
>> goto next_node;
>> }
>>
>> - if (!update_tftp_dfu) {
>> - if (update_flash(update_addr, update_fladdr,
>> - update_size)) {
>> - printf("Error: can't flash update, aborting\n");
>> - ret = 1;
>> - goto next_node;
>> - }
>> - } else if (fit_image_check_type(fit, noffset,
>> - IH_TYPE_FIRMWARE)) {
>> + if (fit_image_check_type(fit, noffset, IH_TYPE_FIRMWARE)) {
>> ret = dfu_tftp_write(fit_image_name, update_addr,
>> update_size, interface, devstring);
>> if (ret)
>> diff --git a/doc/README.dfutftp b/doc/README.dfutftp
>> index a3341bbb61..b279f542a1 100644
>> --- a/doc/README.dfutftp
>> +++ b/doc/README.dfutftp
>> @@ -52,9 +52,6 @@ Environment variables
>>
>> The "dfu tftp" command can be used in the "preboot" environment
>variable
>> (when it is enabled by defining CONFIG_PREBOOT).
>> -This is the preferable way of using this command in the early boot
>stage
>> -as opposed to legacy update_tftp() function invocation.
>> -
>
>There are still two places in which README.update file is referred to
>while you try to delete it.
Thanks for catching this.
>
>
>> Beagle Bone Black (BBB) setup
>> -----------------------------
>> diff --git a/doc/README.update b/doc/README.update
>> deleted file mode 100644
>> index bf4379279e..0000000000
>> --- a/doc/README.update
>> +++ /dev/null
>> @@ -1,97 +0,0 @@
>> -Automatic software update from a TFTP server
>> -============================================
>> -
>> -Overview
>> ---------
>> -
>> -This feature allows to automatically store software updates present
>on a TFTP
>> -server in NOR Flash. In more detail: a TFTP transfer of a file given
>in
>> -environment variable 'updatefile' from server 'serverip' is
>attempted during
>> -boot. The update file should be a FIT file, and can contain one or
>more
>> -updates. Each update in the update file has an address in NOR Flash
>where it
>> -should be placed, updates are also protected with a SHA-1 checksum.
>If the
>> -TFTP transfer is successful, the hash of each update is verified,
>and if the
>> -verification is positive, the update is stored in Flash.
>> -
>> -The auto-update feature is enabled by the CONFIG_UPDATE_TFTP macro:
>> -
>> -#define CONFIG_UPDATE_TFTP 1
>> -
>> -
>
>If you really want to delete this file, you must move the following
>text to somewhere else.
>Otherwise, FIT related information will be lost.
The text below is incorrect. You should not use any definition in a board
include.
Dependencies should be in the Kconfig.
Best regards
Heinrich
>
>-Takahiro Akashi
>
>> -Note that when enabling auto-update, Flash support must be turned
>on. Also,
>> -one must enable FIT and LIBFDT support:
>> -
>> -#define CONFIG_FIT 1
>> -#define CONFIG_OF_LIBFDT 1
>> -
>> -The auto-update feature uses the following configuration knobs:
>> -
>> -- CONFIG_UPDATE_LOAD_ADDR
>> -
>> - Normally, TFTP transfer of the update file is done to the address
>specified
>> - in environment variable 'loadaddr'. If this variable is not
>present, the
>> - transfer is made to the address given in CONFIG_UPDATE_LOAD_ADDR
>(0x100000
>> - by default).
>> -
>> -- CONFIG_UPDATE_TFTP_CNT_MAX
>> - CONFIG_UPDATE_TFTP_MSEC_MAX
>> -
>> - These knobs control the timeouts during initial connection to the
>TFTP
>> - server. Since a transfer is attempted during each boot, it is
>undesirable to
>> - have a long delay when a TFTP server is not present.
>> - CONFIG_UPDATE_TFTP_MSEC_MAX specifies the number of milliseconds
>to wait for
>> - the server to respond to initial connection, and
>CONFIG_UPDATE_TFTP_CNT_MAX
>> - gives the number of such connection retries.
>CONFIG_UPDATE_TFTP_CNT_MAX must
>> - be non-negative and is 0 by default, CONFIG_UPDATE_TFTP_MSEC_MAX
>must be
>> - positive and is 100 by default.
>> -
>> -Since the update file is in FIT format, it is created from an *.its
>file using
>> -the mkimage tool. dtc tool with support for binary includes, e.g. in
>version
>> -1.2.0 or later, must also be available on the system where the
>update file is
>> -to be prepared. Refer to the doc/uImage.FIT/ directory for more
>details on FIT
>> -images.
>> -
>> -
>> -Example .its files
>> -------------------
>> -
>> -- doc/uImage.FIT/update_uboot.its
>> -
>> - A simple example that can be used to create an update file for
>automatically
>> - replacing U-Boot image on a system.
>> -
>> - Assuming that an U-Boot image u-boot.bin is present in the current
>working
>> - directory, and that the address given in the 'load' property in
>the
>> - 'update_uboot.its' file is where the U-Boot is stored in Flash,
>the
>> - following command will create the actual update file
>'update_uboot.itb':
>> -
>> - mkimage -f update_uboot.its update_uboot.itb
>> -
>> - Place 'update_uboot.itb' on a TFTP server, for example as
>> - '/tftpboot/update_uboot.itb', and set the 'updatefile' variable
>> - appropriately, for example in the U-Boot prompt:
>> -
>> - setenv updatefile /tftpboot/update_uboot.itb
>> - saveenv
>> -
>> - Now, when the system boots up and the update TFTP server specified
>in the
>> - 'serverip' environment variable is accessible, the new U-Boot
>image will be
>> - automatically stored in Flash.
>> -
>> - NOTE: do make sure that the 'u-boot.bin' image used to create the
>update
>> - file is a good, working image. Also make sure that the address in
>Flash
>> - where the update will be placed is correct. Making mistake here
>and
>> - attempting the auto-update can render the system unusable.
>> -
>> -- doc/uImage.FIT/update3.its
>> -
>> - An example containing three updates. It can be used to update
>Linux kernel,
>> - ramdisk and FDT blob stored in Flash. The procedure for preparing
>the update
>> - file is similar to the example above.
>> -
>> -TFTP update via DFU
>> --------------------
>> -
>> -- It is now possible to update firmware (bootloader, kernel, rootfs,
>etc.) via
>> - TFTP by using DFU (Device Firmware Upgrade). More information can
>be found in
>> - ./doc/README.dfutftp documentation entry.
>> --
>> 2.27.0
>>
