hi Peter, On Fri, 9 Sept 2022 at 18:07, Peter Robinson <pbrobin...@gmail.com> wrote: > > Hi Sughosh, > > Small nit of the subject, it should substitute rockpi for rockchip: if > the functions are generic.
I will change it to rockchip. > > > Add functions needed to support the UEFI capsule update feature on > > rockchip boards. Currently, the feature is being enabled on the > > RockPi4 boards with firmware images residing on GPT partitioned > > storage media. > > > > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org> > > --- > > Changes since V1: > > * Move the capsule update related functions to a more common place > > under mach-rockchip to facilitate further reuse. > > > > arch/arm/mach-rockchip/Kconfig | 1 + > > arch/arm/mach-rockchip/board.c | 185 ++++++++++++++++++++++++++++++++ > > include/configs/rk3399_common.h | 16 +++ > > 3 files changed, 202 insertions(+) > > > > diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig > > index c561a77e6a..7fac13c8ee 100644 > > --- a/arch/arm/mach-rockchip/Kconfig > > +++ b/arch/arm/mach-rockchip/Kconfig > > @@ -246,6 +246,7 @@ config ROCKCHIP_RK3399 > > select DM_PMIC > > select DM_REGULATOR_FIXED > > select BOARD_LATE_INIT > > + imply PARTITION_TYPE_GUID > > imply PRE_CONSOLE_BUFFER > > imply ROCKCHIP_COMMON_BOARD > > imply ROCKCHIP_SDRAM_COMMON > > diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c > > index cbe00d646c..d35d6a5965 100644 > > --- a/arch/arm/mach-rockchip/board.c > > +++ b/arch/arm/mach-rockchip/board.c > > @@ -6,11 +6,15 @@ > > #include <clk.h> > > #include <cpu_func.h> > > #include <dm.h> > > +#include <efi_loader.h> > > #include <fastboot.h> > > #include <init.h> > > #include <log.h> > > +#include <mmc.h> > > +#include <part.h> > > #include <ram.h> > > #include <syscon.h> > > +#include <uuid.h> > > #include <asm/cache.h> > > #include <asm/global_data.h> > > #include <asm/io.h> > > @@ -22,8 +26,189 @@ > > > > DECLARE_GLOBAL_DATA_PTR; > > > > +#if defined(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) && > > defined(CONFIG_EFI_PARTITION) > > + > > +#define DFU_ALT_BUF_LEN SZ_1K > > +#define ROCKPI4_UPDATABLE_IMAGES 2 > > The naming should be either made generic so it's suitable for all > rockchip devices, or be in the device specific fills. The above macro is not needed in this file. We can use num_image_type_guids instead of the macro. I will change this in the next version. > > > +extern struct efi_fw_image fw_images[]; > > + > > +static bool board_is_rockpi_4b(void) > > Same. Ultimately it looks like you've just moved the code from one > location to another. The way I see it: > * Generic functions with generic rockchip naming that are reusable > across any rockchip device that wishes to use capsule updates > * Board specific functions/variables etc in the board specific files. Okay. Based on your comments on V1, I was under the impression that you wanted me to move the code to a more generic location where it can be used by other rockchip boards. I have now moved the board specific bits to evb_rk3399.c keeping the computation of the image index and population of the dfu_alt_info in the board.c file. I will send the V3 shortly. -sughosh > > > +{ > > + return CONFIG_IS_ENABLED(TARGET_EVB_RK3399) && > > + of_machine_is_compatible("radxa,rockpi4b"); > > +} > > + > > +static bool board_is_rockpi_4c(void) > > +{ > > + return CONFIG_IS_ENABLED(TARGET_EVB_RK3399) && > > + of_machine_is_compatible("radxa,rockpi4c"); > > +} > > + > > +static bool updatable_image(struct disk_partition *info) > > +{ > > + int i; > > + bool ret = false; > > + efi_guid_t image_type_guid; > > + > > + uuid_str_to_bin(info->type_guid, image_type_guid.b, > > + UUID_STR_FORMAT_GUID); > > + > > + for (i = 0; i < ROCKPI4_UPDATABLE_IMAGES; i++) { > > + if (!guidcmp(&fw_images[i].image_type_id, > > &image_type_guid)) { > > + ret = true; > > + break; > > + } > > + } > > + > > + return ret; > > +} > > + > > +static void set_image_index(struct disk_partition *info, int index) > > +{ > > + int i; > > + efi_guid_t image_type_guid; > > + > > + uuid_str_to_bin(info->type_guid, image_type_guid.b, > > + UUID_STR_FORMAT_GUID); > > + > > + for (i = 0; i < ROCKPI4_UPDATABLE_IMAGES; i++) { > > + if (!guidcmp(&fw_images[i].image_type_id, > > &image_type_guid)) { > > + fw_images[i].image_index = index; > > + break; > > + } > > + } > > +} > > + > > +static int get_mmc_desc(struct blk_desc **desc) > > +{ > > + int ret; > > + struct mmc *mmc; > > + struct udevice *dev; > > + > > + /* > > + * For now the firmware images are assumed to > > + * be on the SD card > > + */ > > + ret = uclass_get_device(UCLASS_MMC, 1, &dev); > > + if (ret) > > + return -1; > > + > > + mmc = mmc_get_mmc_dev(dev); > > + if (!mmc) > > + return -ENODEV; > > + > > + if ((ret = mmc_init(mmc))) > > + return ret; > > + > > + *desc = mmc_get_blk_desc(mmc); > > + if (!*desc) > > + return -1; > > + > > + return 0; > > +} > > + > > +void set_dfu_alt_info(char *interface, char *devstr) > > +{ > > + const char *name; > > + bool first = true; > > + int p, len, devnum, ret; > > + char buf[DFU_ALT_BUF_LEN]; > > + struct disk_partition info; > > + struct blk_desc *desc = NULL; > > + > > + ret = get_mmc_desc(&desc); > > + if (ret) { > > + log_err("Unable to get mmc desc\n"); > > + return; > > + } > > + > > + memset(buf, 0, sizeof(buf)); > > + name = blk_get_if_type_name(desc->if_type); > > + devnum = desc->devnum; > > + len = strlen(buf); > > + > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, > > + "%s %d=", name, devnum); > > + > > + for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) { > > + if (part_get_info(desc, p, &info)) > > + continue; > > + > > + /* Add entry to dfu_alt_info only for updatable images */ > > + if (updatable_image(&info)) { > > + if (!first) > > + len += snprintf(buf + len, > > + DFU_ALT_BUF_LEN - len, ";"); > > + > > + len += snprintf(buf + len, DFU_ALT_BUF_LEN - len, > > + "%s%d_%s part %d %d", > > + name, devnum, info.name, devnum, p); > > + first = false; > > + } > > + } > > + > > + log_debug("dfu_alt_info => %s\n", buf); > > + env_set("dfu_alt_info", buf); > > +} > > + > > +static void gpt_capsule_update_setup(void) > > +{ > > + int p, i, ret; > > + struct disk_partition info; > > + struct blk_desc *desc = NULL; > > + > > + if (board_is_rockpi_4b()) { > > + efi_guid_t idbldr_image_type_guid = > > + ROCKPI_4B_IDBLOADER_IMAGE_GUID; > > + efi_guid_t uboot_image_type_guid = > > ROCKPI_4B_UBOOT_IMAGE_GUID; > > + > > + guidcpy(&fw_images[0].image_type_id, > > &idbldr_image_type_guid); > > + guidcpy(&fw_images[1].image_type_id, > > &uboot_image_type_guid); > > + > > + fw_images[0].fw_name = u"ROCKPI4B-IDBLOADER"; > > + fw_images[1].fw_name = u"ROCKPI4B-UBOOT"; > > + } else if (board_is_rockpi_4c()) { > > + efi_guid_t idbldr_image_type_guid = > > + ROCKPI_4C_IDBLOADER_IMAGE_GUID; > > + efi_guid_t uboot_image_type_guid = > > ROCKPI_4C_UBOOT_IMAGE_GUID; > > + > > + guidcpy(&fw_images[0].image_type_id, > > &idbldr_image_type_guid); > > + guidcpy(&fw_images[1].image_type_id, > > &uboot_image_type_guid); > > + > > + fw_images[0].fw_name = u"ROCKPI4C-IDBLOADER"; > > + fw_images[1].fw_name = u"ROCKPI4C-UBOOT"; > > + } > > + > > + ret = get_mmc_desc(&desc); > > + if (ret) { > > + log_err("Unable to get mmc desc\n"); > > + return; > > + } > > + > > + for (p = 1, i = 1; p <= MAX_SEARCH_PARTITIONS; p++) { > > + if (part_get_info(desc, p, &info)) > > + continue; > > + > > + /* > > + * Since we have a GPT partitioned device, the updatable > > + * images could be stored in any order. Populate the > > + * image_index at runtime. > > + */ > > + if (updatable_image(&info)) { > > + set_image_index(&info, i); > > + i++; > > + } > > + } > > +} > > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT && CONFIG_EFI_PARTITION */ > > + > > __weak int rk_board_late_init(void) > > { > > +#if defined(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) && > > defined(CONFIG_EFI_PARTITION) > > + gpt_capsule_update_setup(); > > +#endif > > + > > return 0; > > } > > > > diff --git a/include/configs/rk3399_common.h > > b/include/configs/rk3399_common.h > > index 2f9aee5819..f0a9ab8f83 100644 > > --- a/include/configs/rk3399_common.h > > +++ b/include/configs/rk3399_common.h > > @@ -24,6 +24,22 @@ > > #define CONFIG_SYS_SDRAM_BASE 0 > > #define SDRAM_MAX_SIZE 0xf8000000 > > > > +#define ROCKPI_4B_IDBLOADER_IMAGE_GUID \ > > + EFI_GUID(0x02f4d760, 0xcfd5, 0x43bd, 0x8e, 0x2d, \ > > + 0xa4, 0x2a, 0xcb, 0x33, 0xc6, 0x60) > > + > > +#define ROCKPI_4B_UBOOT_IMAGE_GUID \ > > + EFI_GUID(0x4ce292da, 0x1dd8, 0x428d, 0xa1, 0xc2, \ > > + 0x77, 0x74, 0x3e, 0xf8, 0xb9, 0x6e) > > + > > +#define ROCKPI_4C_IDBLOADER_IMAGE_GUID \ > > + EFI_GUID(0xfd68510c, 0x12d3, 0x4f0a, 0xb8, 0xd3, \ > > + 0xd8, 0x79, 0xe1, 0xd3, 0xa5, 0x40) > > + > > +#define ROCKPI_4C_UBOOT_IMAGE_GUID \ > > + EFI_GUID(0xb81fb4ae, 0xe4f3, 0x471b, 0x99, 0xb4, \ > > + 0x0b, 0x3d, 0xa5, 0x49, 0xce, 0x13) > > + > > #ifndef CONFIG_SPL_BUILD > > > > #define ENV_MEM_LAYOUT_SETTINGS \ > > -- > > 2.34.1 > >