michallenc commented on code in PR #18373: URL: https://github.com/apache/nuttx/pull/18373#discussion_r2783300433
########## boards/xtensa/esp32/common/src/Make.defs: ########## @@ -178,6 +178,8 @@ ifeq ($(CONFIG_ETC_ROMFS),y) RCSRCS = etc/init.d/rc.sysinit etc/init.d/rcS endif +CSRCS += esp32_boot_image.c Review Comment: Should be compiled only if `CONFIG_BOARDCTL_BOOT_IMAGE` is enabled, otherwise it doesn't make sense. ########## boards/xtensa/esp32/common/src/esp32_boot_image.c: ########## @@ -0,0 +1,247 @@ +/**************************************************************************** + * boards/xtensa/esp32/common/src/esp32_boot_image.c + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <debug.h> +#include <fcntl.h> +#include <sys/ioctl.h> +#include <unistd.h> + +#include <arch/esp32/partition.h> +#include <nuttx/board.h> +#include <nuttx/cache.h> +#include <nuttx/irq.h> + +#include "esp_loader.h" +#include "esp_app_format.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define ESP32_APP_IMAGE_MAGIC 0xE9 + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct esp32_boot_loader_args_s +{ + uint32_t entry_addr; + uint32_t drom_addr; + uint32_t drom_vaddr; + uint32_t drom_size; + uint32_t irom_addr; + uint32_t irom_vaddr; + uint32_t irom_size; +}; + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static void IRAM_ATTR esp32_boot_loader_stub(void *arg); + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: esp32_boot_loader_stub + * + * Description: + * This function resides in IRAM and is responsible for switching MMU + * mappings and jumping to the new application. + * + ****************************************************************************/ + +static void IRAM_ATTR esp32_boot_loader_stub(void *arg) +{ + struct esp32_boot_loader_args_s *args = + (struct esp32_boot_loader_args_s *)arg; + void (*entry_point)(void) = (void (*)(void))args->entry_addr; + + /* Disable interrupts */ + + up_irq_disable(); + + /* Disable cache */ + +#ifdef CONFIG_ARCH_CHIP_ESP32 + cache_read_disable(0); + cache_flush(0); +#else + cache_hal_disable(CACHE_LL_LEVEL_EXT_MEM, CACHE_TYPE_ALL); +#endif + + /* Map new segments */ + + map_rom_segments(args->drom_addr, args->drom_vaddr, args->drom_size, + args->irom_addr, args->irom_vaddr, args->irom_size); + + /* Jump to entry point */ + + entry_point(); + + /* Should never reach here */ + + while (1) Review Comment: I think we should rather assert than end in an infinite loop. ########## boards/xtensa/esp32/common/src/esp32_boot_image.c: ########## @@ -0,0 +1,247 @@ +/**************************************************************************** + * boards/xtensa/esp32/common/src/esp32_boot_image.c + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <debug.h> +#include <fcntl.h> +#include <sys/ioctl.h> +#include <unistd.h> + +#include <arch/esp32/partition.h> +#include <nuttx/board.h> +#include <nuttx/cache.h> +#include <nuttx/irq.h> + +#include "esp_loader.h" +#include "esp_app_format.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define ESP32_APP_IMAGE_MAGIC 0xE9 + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct esp32_boot_loader_args_s +{ + uint32_t entry_addr; + uint32_t drom_addr; + uint32_t drom_vaddr; + uint32_t drom_size; + uint32_t irom_addr; + uint32_t irom_vaddr; + uint32_t irom_size; +}; + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static void IRAM_ATTR esp32_boot_loader_stub(void *arg); + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: esp32_boot_loader_stub + * + * Description: + * This function resides in IRAM and is responsible for switching MMU + * mappings and jumping to the new application. + * + ****************************************************************************/ + +static void IRAM_ATTR esp32_boot_loader_stub(void *arg) +{ + struct esp32_boot_loader_args_s *args = + (struct esp32_boot_loader_args_s *)arg; + void (*entry_point)(void) = (void (*)(void))args->entry_addr; + + /* Disable interrupts */ + + up_irq_disable(); + + /* Disable cache */ + +#ifdef CONFIG_ARCH_CHIP_ESP32 + cache_read_disable(0); + cache_flush(0); +#else + cache_hal_disable(CACHE_LL_LEVEL_EXT_MEM, CACHE_TYPE_ALL); +#endif + + /* Map new segments */ + + map_rom_segments(args->drom_addr, args->drom_vaddr, args->drom_size, + args->irom_addr, args->irom_vaddr, args->irom_size); + + /* Jump to entry point */ + + entry_point(); + + /* Should never reach here */ + + while (1) + { + } +} + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: board_boot_image + * + * Description: + * Boot a new application image. + * + * Input Parameters: + * path - Path to the image file/partition + * hdr_size - Size of the image header (unused for ESP32) + * + * Returned Value: + * Does not return on success; returns error code on failure. + * + ****************************************************************************/ + +int board_boot_image(FAR const char *path, uint32_t hdr_size) +{ + int fd; + int ret; + uint32_t offset; + esp_image_header_t image_header; + esp_image_segment_header_t segment_hdr; + struct esp32_boot_loader_args_s args = + { + 0 + }; + + uint32_t current_offset; + int i; + + /* Check for legacy format (not supported) */ + +#ifdef CONFIG_ESP32_APP_FORMAT_LEGACY + ferr("ERROR: Legacy format not supported for board_boot_image\n"); + return -ENOTSUP; +#endif + + /* Open the image file */ + + fd = open(path, O_RDONLY); + if (fd < 0) + { + ferr("ERROR: Failed to open %s: %d\n", path, errno); + return -errno; + } + + /* Get partition offset */ + + ret = ioctl(fd, OTA_IMG_GET_OFFSET, (unsigned long)&offset); + if (ret < 0) + { + ferr("ERROR: Failed to get partition offset: %d\n", errno); + close(fd); + return -errno; + } + + /* Read image header */ + + ret = read(fd, &image_header, sizeof(esp_image_header_t)); Review Comment: I am not much familiar with esp32 boot process, but from what I see this can't work with nxboot nor likely with mcuboot, because they don't have `esp_image_header_t` at the beginning of image. They have their own nxboot/mcuboot headers. I don't think you need to read the header at all. `BOARDIOC_BOOT_IMAGE` ioctl already gives you the path to the partition where the image is located and size of the header -> thus the offset to actual image. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
