hi Kever,
On Wed, 29 Jun 2022 at 09:11, Kever Yang <kever.y...@rock-chips.com> wrote:
>
> Hi Sughosh,
>
> Thanks for your patch.
>
> On 2022/5/16 14:12, Sughosh Ganu wrote:
> > hi Peter,
> >
> > On Sat, 14 May 2022 at 13:44, Peter Robinson <pbrobin...@gmail.com> wrote:
> >> On Fri, May 13, 2022 at 7:50 AM Sughosh Ganu <sughosh.g...@linaro.org>
> >> wrote:
> >>> Add support for updating the idbloader and u-boot images through the
> >>> UEFI capsule update functionality. Enable the modules required for
> >>> supporting the functionality.
> >>>
> >>> The implementation is for the updatable images placed on a GPT
> >>> partitioned storage device. With the GPT partition devices, the
> >>> partition information can be obtained at runtime, and hence the
> >>> dfu_alt_info variable needed for the updates is being populated at
> >>> runtime.
> >>>
> >>> This requires the partitions containing the idbloader and u-boot
> >>> images to be created with the PartitionTypeGUID values set to the
> >>> corresponding image type GUID values defined in the platform's config
> >>> header(ROCKPI_4{B,C}_IDBLOADER_IMAGE_GUID and
> >>> ROCKPI_4{B,C}_UBOOT_IMAGE_GUID).
> >> I think it would be useful to break this patch down into more than one
> >> patch. At least the core changes to evb_rk3399/evb-rk3399.c and the
> >> board specific changes. Also given these changes arre likely of
> >> interest to other boards should they go to somewhere more central
> >> than evb_rk3399/evb-rk3399.c to enable less code duplication across
> >> rk3399 or even rockchip devices as a whole?
> > I believe I can move the logic to set dfu_alt_info to some place like
> > arch/arm/mach-rockchip/board.c. The definition of rk_board_late_init()
> > can stay in evb-rk3399.c since it is populating the capsule update
> > structures for the rk3399 family of platforms. Does that sound okay to
> > you? If so, I will make the changes and send a V2, or if you have
> > some other solution in mind, please let me know. Thanks.
>
> Other than Peter's command, please make sure:
>
> All the source code relate to rockpi should go to board/radxa instead of
> board/rockchip/evb-rk3399.c
Apologies for the late reply. I was working on some other stuff and
hence this got put on the backburner. With regards to your review
comment of putting the board code under board/radxa/, how do I get the
code under board/radxa to compile. The RockPi4 boards have
CONFIG_SYS_VENDOR="rockchip" and CONFIG_SYS_BOARD="evb_rk3399". With
these configs, code under board/rockchip/evb_rk3399/ gets built. Is
there some way I can get code under some other board directory to
build?
I was thinking of putting the common code under
arch/arm/mach-rockchip/board.c, and the board specific declarations
under board/rockchip/evb_rk3399/evb_rk3399.c, along with the splitting
of the changes in two patches. Is that fine with you?
-sughosh
>
>
> Thanks,
>
> - Kever
>
> >
> > -sughosh
> >
> >> Peter
> >>
> >>> Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org>
> >>> ---
> >>> arch/arm/mach-rockchip/Kconfig | 1 +
> >>> board/rockchip/evb_rk3399/evb-rk3399.c | 190 ++++++++++++++++++++++++-
> >>> configs/rock-pi-4-rk3399_defconfig | 4 +
> >>> configs/rock-pi-4c-rk3399_defconfig | 4 +
> >>> include/configs/rk3399_common.h | 16 +++
> >>> 5 files changed, 214 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/arm/mach-rockchip/Kconfig
> >>> b/arch/arm/mach-rockchip/Kconfig
> >>> index 18aff5480b..6d13e7b92e 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/board/rockchip/evb_rk3399/evb-rk3399.c
> >>> b/board/rockchip/evb_rk3399/evb-rk3399.c
> >>> index abb76585cf..cc976d9471 100644
> >>> --- a/board/rockchip/evb_rk3399/evb-rk3399.c
> >>> +++ b/board/rockchip/evb_rk3399/evb-rk3399.c
> >>> @@ -5,11 +5,27 @@
> >>>
> >>> #include <common.h>
> >>> #include <dm.h>
> >>> +#include <efi_loader.h>
> >>> #include <init.h>
> >>> #include <log.h>
> >>> +#include <mmc.h>
> >>> +#include <part.h>
> >>> +#include <uuid.h>
> >>> #include <asm/arch-rockchip/periph.h>
> >>> #include <power/regulator.h>
> >>>
> >>> +#define ROCKPI4_UPDATABLE_IMAGES 2
> >>> +
> >>> +#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
> >>> +struct efi_fw_image fw_images[ROCKPI4_UPDATABLE_IMAGES] = {0};
> >>> +
> >>> +struct efi_capsule_update_info update_info = {
> >>> + .images = fw_images,
> >>> +};
> >>> +
> >>> +u8 num_image_type_guids = ARRAY_SIZE(fw_images);
> >>> +#endif
> >>> +
> >>> #ifndef CONFIG_SPL_BUILD
> >>> int board_early_init_f(void)
> >>> {
> >>> @@ -29,4 +45,176 @@ int board_early_init_f(void)
> >>> out:
> >>> return 0;
> >>> }
> >>> -#endif
> >>> +
> >>> +#if defined(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) &&
> >>> defined(CONFIG_EFI_PARTITION)
> >>> +
> >>> +#define DFU_ALT_BUF_LEN SZ_1K
> >>> +
> >>> +static bool board_is_rockpi_4b(void)
> >>> +{
> >>> + 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 -1;
> >>> +
> >>> + if (mmc_init(mmc))
> >>> + return -1;
> >>> +
> >>> + *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)
> >>> + 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);
> >>> +}
> >>> +
> >>> +int rk_board_late_init(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)
> >>> + return ret;
> >>> +
> >>> + 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++;
> >>> + }
> >>> + }
> >>> +
> >>> + return 0;
> >>> +}
> >>> +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT && CONFIG_EFI_PARTITION */
> >>> +#endif /* !CONFIG_SPL_BUILD */
> >>> diff --git a/configs/rock-pi-4-rk3399_defconfig
> >>> b/configs/rock-pi-4-rk3399_defconfig
> >>> index 80d1e63b59..ca3e217603 100644
> >>> --- a/configs/rock-pi-4-rk3399_defconfig
> >>> +++ b/configs/rock-pi-4-rk3399_defconfig
> >>> @@ -21,6 +21,7 @@ CONFIG_SPL_STACK_R=y
> >>> CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000
> >>> CONFIG_TPL=y
> >>> CONFIG_CMD_BOOTZ=y
> >>> +CONFIG_CMD_DFU=y
> >>> CONFIG_CMD_GPT=y
> >>> CONFIG_CMD_MMC=y
> >>> CONFIG_CMD_PCI=y
> >>> @@ -31,6 +32,7 @@ CONFIG_SPL_OF_CONTROL=y
> >>> CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names
> >>> interrupt-parent assigned-clocks assigned-clock-rates
> >>> assigned-clock-parents"
> >>> CONFIG_ENV_IS_IN_MMC=y
> >>> CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> >>> +CONFIG_DFU_MMC=y
> >>> CONFIG_ROCKCHIP_GPIO=y
> >>> CONFIG_SYS_I2C_ROCKCHIP=y
> >>> CONFIG_MISC=y
> >>> @@ -76,3 +78,5 @@ CONFIG_VIDEO_ROCKCHIP=y
> >>> CONFIG_DISPLAY_ROCKCHIP_HDMI=y
> >>> CONFIG_SPL_TINY_MEMSET=y
> >>> CONFIG_ERRNO_STR=y
> >>> +CONFIG_EFI_CAPSULE_ON_DISK=y
> >>> +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> >>> diff --git a/configs/rock-pi-4c-rk3399_defconfig
> >>> b/configs/rock-pi-4c-rk3399_defconfig
> >>> index bda4b70dbf..4c89e7e918 100644
> >>> --- a/configs/rock-pi-4c-rk3399_defconfig
> >>> +++ b/configs/rock-pi-4c-rk3399_defconfig
> >>> @@ -21,6 +21,7 @@ CONFIG_SPL_STACK_R=y
> >>> CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000
> >>> CONFIG_TPL=y
> >>> CONFIG_CMD_BOOTZ=y
> >>> +CONFIG_CMD_DFU=y
> >>> CONFIG_CMD_GPT=y
> >>> CONFIG_CMD_MMC=y
> >>> CONFIG_CMD_PCI=y
> >>> @@ -31,6 +32,7 @@ CONFIG_SPL_OF_CONTROL=y
> >>> CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names
> >>> interrupt-parent assigned-clocks assigned-clock-rates
> >>> assigned-clock-parents"
> >>> CONFIG_ENV_IS_IN_MMC=y
> >>> CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> >>> +CONFIG_DFU_MMC=y
> >>> CONFIG_ROCKCHIP_GPIO=y
> >>> CONFIG_SYS_I2C_ROCKCHIP=y
> >>> CONFIG_MISC=y
> >>> @@ -76,3 +78,5 @@ CONFIG_VIDEO_ROCKCHIP=y
> >>> CONFIG_DISPLAY_ROCKCHIP_HDMI=y
> >>> CONFIG_SPL_TINY_MEMSET=y
> >>> CONFIG_ERRNO_STR=y
> >>> +CONFIG_EFI_CAPSULE_ON_DISK=y
> >>> +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> >>> diff --git a/include/configs/rk3399_common.h
> >>> b/include/configs/rk3399_common.h
> >>> index 8e13737666..28b514a4ea 100644
> >>> --- a/include/configs/rk3399_common.h
> >>> +++ b/include/configs/rk3399_common.h
> >>> @@ -38,6 +38,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.25.1
> >>>
