Re: [PATCH 01/17] sunxi: Add support for AXP305 PMIC
On 06/01/2021 23:33, Jaehoon Chung wrote: > On 1/6/21 7:11 PM, André Przywara wrote: >> On 05/01/2021 22:36, Jaehoon Chung wrote: >> >> Hi, >> >> thanks for having a look! >> >>> Hi Jernej >>> >>> On 1/3/21 6:26 PM, Jernej Skrabec wrote: This PMIC can be found on H616 boards and it's very similar to AXP805 and AXP806. >>> >>> Is there any plan to cleanup codes? >> >> There is no support for either of these, we use the PMICs only in >> Trusted Firmware or Linux. So nothing to consolidate, yet. This might >> change in the future, and then of course we will use a common code base. >> >>> Signed-off-by: Jernej Skrabec --- arch/arm/mach-sunxi/pmic_bus.c | 6 +++ board/sunxi/board.c| 10 +++-- drivers/power/Kconfig | 13 +- drivers/power/Makefile | 1 + drivers/power/axp305.c | 80 ++ include/axp305.h | 17 include/axp_pmic.h | 3 ++ 7 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 drivers/power/axp305.c create mode 100644 include/axp305.h diff --git a/arch/arm/mach-sunxi/pmic_bus.c b/arch/arm/mach-sunxi/pmic_bus.c index dea42de833f1..0394ce856448 100644 --- a/arch/arm/mach-sunxi/pmic_bus.c +++ b/arch/arm/mach-sunxi/pmic_bus.c @@ -18,6 +18,8 @@ #define AXP209_I2C_ADDR 0x34 +#define AXP305_I2C_ADDR 0x36 + #define AXP221_CHIP_ADDR 0x68 #define AXP221_CTRL_ADDR 0x3e #define AXP221_INIT_DATA 0x3e @@ -64,6 +66,8 @@ int pmic_bus_read(u8 reg, u8 *data) return i2c_read(AXP152_I2C_ADDR, reg, 1, data, 1); #elif defined CONFIG_AXP209_POWER return i2c_read(AXP209_I2C_ADDR, reg, 1, data, 1); +#elif defined CONFIG_AXP305_POWER + return i2c_read(AXP305_I2C_ADDR, reg, 1, data, 1); #elif defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER # ifdef CONFIG_MACH_SUN6I return p2wi_read(reg, data); @@ -81,6 +85,8 @@ int pmic_bus_write(u8 reg, u8 data) return i2c_write(AXP152_I2C_ADDR, reg, 1, &data, 1); #elif defined CONFIG_AXP209_POWER return i2c_write(AXP209_I2C_ADDR, reg, 1, &data, 1); +#elif defined CONFIG_AXP305_POWER + return i2c_write(AXP305_I2C_ADDR, reg, 1, &data, 1); #elif defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER # ifdef CONFIG_MACH_SUN6I return p2wi_write(reg, data); diff --git a/board/sunxi/board.c b/board/sunxi/board.c index 708a27ed78e9..54ff9bc92396 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -634,16 +634,18 @@ void sunxi_board_init(void) #endif #if defined CONFIG_AXP152_POWER || defined CONFIG_AXP209_POWER || \ - defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \ - defined CONFIG_AXP818_POWER + defined CONFIG_AXP221_POWER || defined CONFIG_AXP305_POWER || \ + defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER power_failed = axp_init(); #if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || \ defined CONFIG_AXP818_POWER power_failed |= axp_set_dcdc1(CONFIG_AXP_DCDC1_VOLT); #endif +#if !defined(CONFIG_AXP305_POWER) power_failed |= axp_set_dcdc2(CONFIG_AXP_DCDC2_VOLT); power_failed |= axp_set_dcdc3(CONFIG_AXP_DCDC3_VOLT); +#endif #if !defined(CONFIG_AXP209_POWER) && !defined(CONFIG_AXP818_POWER) power_failed |= axp_set_dcdc4(CONFIG_AXP_DCDC4_VOLT); #endif @@ -656,8 +658,10 @@ void sunxi_board_init(void) defined CONFIG_AXP818_POWER power_failed |= axp_set_aldo1(CONFIG_AXP_ALDO1_VOLT); #endif +#if !defined(CONFIG_AXP305_POWER) power_failed |= axp_set_aldo2(CONFIG_AXP_ALDO2_VOLT); -#if !defined(CONFIG_AXP152_POWER) +#endif +#if !defined(CONFIG_AXP152_POWER) && !defined(CONFIG_AXP305_POWER) power_failed |= axp_set_aldo3(CONFIG_AXP_ALDO3_VOLT); #endif #ifdef CONFIG_AXP209_POWER diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index 02050f6f3569..d17cf2d9112a 100644 --- a/drivers/power/Kconfig +++ b/drivers/power/Kconfig @@ -48,6 +48,15 @@ config AXP221_POWER Select this to enable support for the axp221/axp223 pmic found on most A23 and A31 boards. +config AXP305_POWER + bool "axp305 pmic support" + depends on MACH_SUN50I_H616 + select AXP_PMIC_BUS + select CMD_POWEROFF + ---help--- + Select this to enable support for the axp305 pmic found on most + H616 boards. + config AXP809_POWER bool "axp809 pmic support" depends on MACH_SUN9I @@ -127,11 +136,12 @@ c
[v2 0/6] Add Vendor Authorized Boot (VAB) support
This is the 2nd version of patchset to add Vendor Authorized Boot (VAB) support for Intel Agilex SoC device. Vendor Authorized Boot is a security feature for authenticating the images such as U-Boot, ARM trusted Firmware, Linux kernel, device tree blob and etc loaded from FIT. After those images are loaded from FIT, the VAB certificate and signature block appended at the end of each image are sent to Secure Device Manager (SDM) for authentication. U-Boot will validate the SHA384 of the image against the SHA384 hash stored in the VAB certificate before sending the image to SDM for authentication. Patch status: Have changes: Patch 1, 2, 5, 6 Other patches unchanged. Detail changelog can find in commit message. v1->v2: Patch 1: - Create common macro TARGET_SOCFPGA_SOC64 for Stratix10 and Agilex. This is new patch in 2nd version of VAB series. This is code clean up without functionality change. Patch 2: - Renamed CONFIG_SECURE_VAB_AUTH* to CONFIG_SOCFPGA_SECURE_VAB_AUTH* - Changes in secure_vab.c - Changed to use SZ_1K for 1024 - Updated comment in secure_vab.c of "... the certificate for T" - The code will report error before end of the function if reach maximum retry. - In board_prep_linux function, only execute linux_qspi_enable command if it exists in enviroment variable. It is optional. Patch 5: - Move CONFIG_BOOTCOMMAND to defconfig. This is new patch in 2nd version of VAB series. This is code clean up without functionality change. Patch 6: - Renamed CONFIG_SECURE_VAB_AUTH to CONFIG_SOCFPGA_SECURE_VAB_AUTH - Add CONFIG_BOOTCOMMAND in defconfig History: [v1]: https://patchwork.ozlabs.org/project/uboot/cover/20201110070505.26935-1-elly.siew.chin@intel.com/ These patchsets have dependency on: Enable ARM Trusted Firmware for U-Boot https://patchwork.ozlabs.org/project/uboot/cover/20201224102113.32972-1-elly.siew.chin@intel.com/ Siew Chin Lim (6): arm: socfpga: Move Stratix10 and Agilex to use TARGET_SOCFPGA_SOC64 arm: socfpga: soc64: Support Vendor Authorized Boot (VAB) arm: socfpga: cmd: Support 'vab' command arm: socfpga: dts: soc64: Update filename in binman node of FIT image with VAB support configs: socfpga: soc64: Move CONFIG_BOOTCOMMAND to defconfig configs: socfpga: Add defconfig for Agilex with VAB support arch/arm/Kconfig | 6 +- arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi | 22 +++ arch/arm/mach-socfpga/Kconfig | 20 +++ arch/arm/mach-socfpga/Makefile | 3 + arch/arm/mach-socfpga/include/mach/mailbox_s10.h | 1 + arch/arm/mach-socfpga/include/mach/reset_manager.h | 3 +- arch/arm/mach-socfpga/include/mach/secure_vab.h| 63 +++ .../arm/mach-socfpga/include/mach/system_manager.h | 3 +- arch/arm/mach-socfpga/secure_vab.c | 193 + arch/arm/mach-socfpga/vab.c| 37 common/Kconfig.boot| 2 +- configs/socfpga_agilex_atf_defconfig | 2 + configs/socfpga_agilex_defconfig | 2 + ..._atf_defconfig => socfpga_agilex_vab_defconfig} | 5 +- configs/socfpga_stratix10_atf_defconfig| 2 + configs/socfpga_stratix10_defconfig| 2 + drivers/ddr/altera/Kconfig | 6 +- drivers/fpga/Kconfig | 2 +- drivers/sysreset/Kconfig | 2 +- include/configs/socfpga_soc64_common.h | 9 +- 20 files changed, 363 insertions(+), 22 deletions(-) create mode 100644 arch/arm/mach-socfpga/include/mach/secure_vab.h create mode 100644 arch/arm/mach-socfpga/secure_vab.c create mode 100644 arch/arm/mach-socfpga/vab.c copy configs/{socfpga_agilex_atf_defconfig => socfpga_agilex_vab_defconfig} (91%) -- 2.13.0
[v2 1/6] arm: socfpga: Move Stratix10 and Agilex to use TARGET_SOCFPGA_SOC64
Create common macro TARGET_SOCFPGA_SOC64 for Stratix10 and Agilex. Signed-off-by: Siew Chin Lim --- v2 --- - This is new patch in 2nd version of VAB series. This is code clean up without functional change. --- arch/arm/Kconfig| 6 +++--- arch/arm/mach-socfpga/Kconfig | 5 + arch/arm/mach-socfpga/include/mach/reset_manager.h | 3 +-- arch/arm/mach-socfpga/include/mach/system_manager.h | 3 +-- drivers/ddr/altera/Kconfig | 6 +++--- drivers/fpga/Kconfig| 2 +- drivers/sysreset/Kconfig| 2 +- 7 files changed, 15 insertions(+), 12 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index fbe90875ae..7eea261fe9 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -974,7 +974,7 @@ config ARCH_SOCFPGA bool "Altera SOCFPGA family" select ARCH_EARLY_INIT_R select ARCH_MISC_INIT if !TARGET_SOCFPGA_ARRIA10 - select ARM64 if TARGET_SOCFPGA_STRATIX10 || TARGET_SOCFPGA_AGILEX + select ARM64 if TARGET_SOCFPGA_SOC64 select CPU_V7A if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10 select DM select DM_SERIAL @@ -986,7 +986,7 @@ config ARCH_SOCFPGA select SPL_LIBGENERIC_SUPPORT select SPL_NAND_SUPPORT if SPL_NAND_DENALI select SPL_OF_CONTROL - select SPL_SEPARATE_BSS if TARGET_SOCFPGA_STRATIX10 || TARGET_SOCFPGA_AGILEX + select SPL_SEPARATE_BSS if TARGET_SOCFPGA_SOC64 select SPL_SERIAL_SUPPORT select SPL_SYSRESET select SPL_WATCHDOG_SUPPORT @@ -995,7 +995,7 @@ config ARCH_SOCFPGA select SYS_THUMB_BUILD if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10 select SYSRESET select SYSRESET_SOCFPGA if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10 - select SYSRESET_SOCFPGA_SOC64 if TARGET_SOCFPGA_STRATIX10 || TARGET_SOCFPGA_AGILEX + select SYSRESET_SOCFPGA_SOC64 if TARGET_SOCFPGA_SOC64 imply CMD_DM imply CMD_MTDPARTS imply CRC32_VERIFY diff --git a/arch/arm/mach-socfpga/Kconfig b/arch/arm/mach-socfpga/Kconfig index 4d4ff16337..9b1abdaabd 100644 --- a/arch/arm/mach-socfpga/Kconfig +++ b/arch/arm/mach-socfpga/Kconfig @@ -38,6 +38,7 @@ config TARGET_SOCFPGA_AGILEX select FPGA_INTEL_SDM_MAILBOX select NCORE_CACHE select SPL_CLK if SPL + select TARGET_SOCFPGA_SOC64 config TARGET_SOCFPGA_ARRIA5 bool @@ -75,12 +76,16 @@ config TARGET_SOCFPGA_GEN5 imply SPL_SYS_MALLOC_SIMPLE imply SPL_USE_TINY_PRINTF +config TARGET_SOCFPGA_SOC64 + bool + config TARGET_SOCFPGA_STRATIX10 bool select ARMV8_MULTIENTRY select ARMV8_SET_SMPEN select BINMAN if SPL_ATF select FPGA_INTEL_SDM_MAILBOX + select TARGET_SOCFPGA_SOC64 choice prompt "Altera SOCFPGA board select" diff --git a/arch/arm/mach-socfpga/include/mach/reset_manager.h b/arch/arm/mach-socfpga/include/mach/reset_manager.h index 7844ad14cb..8c25325e45 100644 --- a/arch/arm/mach-socfpga/include/mach/reset_manager.h +++ b/arch/arm/mach-socfpga/include/mach/reset_manager.h @@ -43,8 +43,7 @@ void socfpga_per_reset_all(void); #include #elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10) #include -#elif defined(CONFIG_TARGET_SOCFPGA_STRATIX10) || \ - defined(CONFIG_TARGET_SOCFPGA_AGILEX) +#elif defined(CONFIG_TARGET_SOCFPGA_SOC64) #include #endif diff --git a/arch/arm/mach-socfpga/include/mach/system_manager.h b/arch/arm/mach-socfpga/include/mach/system_manager.h index f816954717..5603eaa3d0 100644 --- a/arch/arm/mach-socfpga/include/mach/system_manager.h +++ b/arch/arm/mach-socfpga/include/mach/system_manager.h @@ -8,8 +8,7 @@ phys_addr_t socfpga_get_sysmgr_addr(void); -#if defined(CONFIG_TARGET_SOCFPGA_STRATIX10) || \ - defined(CONFIG_TARGET_SOCFPGA_AGILEX) +#if defined(CONFIG_TARGET_SOCFPGA_SOC64) #include #else #define SYSMGR_ROMCODEGRP_CTRL_WARMRSTCFGPINMUXBIT(0) diff --git a/drivers/ddr/altera/Kconfig b/drivers/ddr/altera/Kconfig index 8f590dc5f6..4660d20def 100644 --- a/drivers/ddr/altera/Kconfig +++ b/drivers/ddr/altera/Kconfig @@ -1,8 +1,8 @@ config SPL_ALTERA_SDRAM bool "SoCFPGA DDR SDRAM driver in SPL" depends on SPL - depends on TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10 || TARGET_SOCFPGA_STRATIX10 || TARGET_SOCFPGA_AGILEX - select RAM if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_STRATIX10 || TARGET_SOCFPGA_AGILEX - select SPL_RAM if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_STRATIX10 || TARGET_SOCFPGA_AGILEX + depends on TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10 || TARGET_SOCFPGA_SOC64 + select RAM if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_SOC64 + select SPL_RAM if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_SOC64 help Enable DDR SDRAM controller for the SoCFPGA devices. diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index 425
[v2 2/6] arm: socfpga: soc64: Support Vendor Authorized Boot (VAB)
Vendor Authorized Boot is a security feature for authenticating the images such as U-Boot, ARM trusted Firmware, Linux kernel, device tree blob and etc loaded from FIT. After those images are loaded from FIT, the VAB certificate and signature block appended at the end of each image are sent to Secure Device Manager (SDM) for authentication. U-Boot will validate the SHA384 of the image against the SHA384 hash stored in the VAB certificate before sending the image to SDM for authentication. Signed-off-by: Siew Chin Lim --- v2 --- - Renamed SECURE_VAB_AUTH* to SOCFPGA_SECURE_VAB_AUTH* - Changes in secure_vab.c - Changed to use SZ_1K for 1024 - Updated comment in secure_vab.c of "... the certificate for T" - The code will report error before end of the function if reach maximum retry. - In board_prep_linux function, only execute linux_qspi_enable command if it exists in enviroment variable. It is optional. --- arch/arm/mach-socfpga/Kconfig| 15 ++ arch/arm/mach-socfpga/Makefile | 2 + arch/arm/mach-socfpga/include/mach/mailbox_s10.h | 1 + arch/arm/mach-socfpga/include/mach/secure_vab.h | 63 arch/arm/mach-socfpga/secure_vab.c | 193 +++ common/Kconfig.boot | 2 +- 6 files changed, 275 insertions(+), 1 deletion(-) create mode 100644 arch/arm/mach-socfpga/include/mach/secure_vab.h create mode 100644 arch/arm/mach-socfpga/secure_vab.c diff --git a/arch/arm/mach-socfpga/Kconfig b/arch/arm/mach-socfpga/Kconfig index 9b1abdaabd..0c35406232 100644 --- a/arch/arm/mach-socfpga/Kconfig +++ b/arch/arm/mach-socfpga/Kconfig @@ -6,6 +6,21 @@ config ERR_PTR_OFFSET config NR_DRAM_BANKS default 1 +config SOCFPGA_SECURE_VAB_AUTH + bool "Enable boot image authentication with Secure Device Manager" + depends on TARGET_SOCFPGA_AGILEX + select FIT_IMAGE_POST_PROCESS + select SHA384 + select SHA512_ALGO + select SPL_FIT_IMAGE_POST_PROCESS + help +All images loaded from FIT will be authenticated by Secure Device +Manager. + +config SOCFPGA_SECURE_VAB_AUTH_ALLOW_NON_FIT_IMAGE + bool "Allow non-FIT VAB signed images" + depends on SOCFPGA_SECURE_VAB_AUTH + config SPL_SIZE_LIMIT default 0x1 if TARGET_SOCFPGA_GEN5 diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile index 82b681d870..1f1e21766d 100644 --- a/arch/arm/mach-socfpga/Makefile +++ b/arch/arm/mach-socfpga/Makefile @@ -4,6 +4,7 @@ # Wolfgang Denk, DENX Software Engineering, w...@denx.de. # # Copyright (C) 2012-2017 Altera Corporation +# Copyright (C) 2017-2020 Intel Corporation obj-y += board.o obj-y += clock_manager.o @@ -47,6 +48,7 @@ obj-y += mailbox_s10.o obj-y += misc_s10.o obj-y += mmu-arm64_s10.o obj-y += reset_manager_s10.o +obj-$(CONFIG_SOCFPGA_SECURE_VAB_AUTH) += secure_vab.o obj-y += system_manager_s10.o obj-y += timer_s10.o obj-y += wrap_pinmux_config_s10.o diff --git a/arch/arm/mach-socfpga/include/mach/mailbox_s10.h b/arch/arm/mach-socfpga/include/mach/mailbox_s10.h index 4d783119ea..fbaf11597e 100644 --- a/arch/arm/mach-socfpga/include/mach/mailbox_s10.h +++ b/arch/arm/mach-socfpga/include/mach/mailbox_s10.h @@ -118,6 +118,7 @@ enum ALT_SDM_MBOX_RESP_CODE { #define MBOX_RECONFIG_MSEL 7 #define MBOX_RECONFIG_DATA 8 #define MBOX_RECONFIG_STATUS 9 +#define MBOX_VAB_SRC_CERT 11 #define MBOX_QSPI_OPEN 50 #define MBOX_QSPI_CLOSE51 #define MBOX_QSPI_DIRECT 59 diff --git a/arch/arm/mach-socfpga/include/mach/secure_vab.h b/arch/arm/mach-socfpga/include/mach/secure_vab.h new file mode 100644 index 00..42588588e8 --- /dev/null +++ b/arch/arm/mach-socfpga/include/mach/secure_vab.h @@ -0,0 +1,63 @@ +/* SPDX-License-Identifier: GPL-2.0 + * + * Copyright (C) 2020 Intel Corporation + * + */ + +#ifndef_SECURE_VAB_H_ +#define_SECURE_VAB_H_ + +#include +#include +#include + +#define VAB_DATA_SZ64 + +#define SDM_CERT_MAGIC_NUM 0x25D04E7F +#define FCS_HPS_VAB_MAGIC_NUM 0xD0564142 + +#define MAX_CERT_SIZE (SZ_4K) + +/* + * struct fcs_hps_vab_certificate_data + * @vab_cert_magic_num: VAB Certificate Magic Word (0xD0564142) + * @flags: TBD + * @fcs_data: Data words being certificate signed. + * @cert_sign_keychain: Certificate Signing Keychain + */ +struct fcs_hps_vab_certificate_data { + u32 vab_cert_magic_num; /* offset 0x10 */ + u32 flags; + u8 rsvd0_1[8]; + u8 fcs_sha384[SHA384_SUM_LEN]; /* offset 0x20 */ +}; + +/* + * struct fcs_hps_vab_certificate_header + * @cert_magic_num: Certificate Magic Word (0x25D04E7F) + * @cert_data_sz: size of this certificate header (0x80) + * Includes magic number all the way to the certificate + * signing keychain (excludes cert. signing keychain) + * @cert_ver: Certificate Version + *
[v2 3/6] arm: socfpga: cmd: Support 'vab' command
Support 'vab' command to perform vendor authentication. Command format: vab addr len Authorize 'len' bytes starting at 'addr' via vendor public key Signed-off-by: Siew Chin Lim --- arch/arm/mach-socfpga/Makefile | 1 + arch/arm/mach-socfpga/vab.c| 37 + 2 files changed, 38 insertions(+) create mode 100644 arch/arm/mach-socfpga/vab.c diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile index 1f1e21766d..9e63296b38 100644 --- a/arch/arm/mach-socfpga/Makefile +++ b/arch/arm/mach-socfpga/Makefile @@ -51,6 +51,7 @@ obj-y += reset_manager_s10.o obj-$(CONFIG_SOCFPGA_SECURE_VAB_AUTH) += secure_vab.o obj-y += system_manager_s10.o obj-y += timer_s10.o +obj-$(CONFIG_SOCFPGA_SECURE_VAB_AUTH) += vab.o obj-y += wrap_pinmux_config_s10.o obj-y += wrap_pll_config_s10.o endif diff --git a/arch/arm/mach-socfpga/vab.c b/arch/arm/mach-socfpga/vab.c new file mode 100644 index 00..4c592a87cf --- /dev/null +++ b/arch/arm/mach-socfpga/vab.c @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2020 Intel Corporation + * + */ + +#include +#include +#include +#include + +static int do_vab(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + unsigned long addr, len; + + if (argc < 3) + return CMD_RET_USAGE; + + addr = simple_strtoul(argv[1], NULL, 16); + len = simple_strtoul(argv[2], NULL, 16); + + if (socfpga_vendor_authentication((void *)&addr, (size_t *)&len) == 0) + printf("%s 0x%016lx (%ld bytes)\n", + "Image Authentication passed at address", addr, len); + else + return CMD_RET_FAILURE; + + return 0; +} + +U_BOOT_CMD( + vab,3, 2, do_vab, + "perform vendor authorization", + "addr len - authorize 'len' bytes starting at\n" + " 'addr' via vendor public key" +); -- 2.13.0
[v2 4/6] arm: socfpga: dts: soc64: Update filename in binman node of FIT image with VAB support
FIT image of Vendor Authentication Coot (VAB) contains signed images. Signed-off-by: Siew Chin Lim --- arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi | 22 ++ 1 file changed, 22 insertions(+) diff --git a/arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi b/arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi index cf365590a8..4b30473743 100644 --- a/arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi +++ b/arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi @@ -117,4 +117,26 @@ }; }; +#if defined(CONFIG_SOCFPGA_SECURE_VAB_AUTH) +&uboot_blob { + filename = "signed-u-boot-nodtb.bin"; +}; + +&atf_blob { + filename = "signed-bl31.bin"; +}; + +&uboot_fdt_blob { + filename = "signed-u-boot.dtb"; +}; + +&kernel_blob { + filename = "signed-Image"; +}; + +&kernel_fdt_blob { + filename = "signed-linux.dtb"; +}; +#endif + #endif -- 2.13.0
[v2 5/6] configs: socfpga: soc64: Move CONFIG_BOOTCOMMAND to defconfig
CONFIG_BOOTCOMMAND have been moved to Kconfig.boot. This patch move the CONFIG_BOOTCOMMAND macro from socfpga_soc64_common.h to *_defconfig file for both Stratix 10 and Agilex. Signed-off-by: Siew Chin Lim --- v2 --- - This is new patch in 2nd version of VAB series. This is code clean up without functionality change. --- configs/socfpga_agilex_atf_defconfig| 2 ++ configs/socfpga_agilex_defconfig| 2 ++ configs/socfpga_stratix10_atf_defconfig | 2 ++ configs/socfpga_stratix10_defconfig | 2 ++ include/configs/socfpga_soc64_common.h | 9 + 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/configs/socfpga_agilex_atf_defconfig b/configs/socfpga_agilex_atf_defconfig index ad87a8098f..704a9d56b9 100644 --- a/configs/socfpga_agilex_atf_defconfig +++ b/configs/socfpga_agilex_atf_defconfig @@ -19,6 +19,8 @@ CONFIG_SPL_LOAD_FIT_ADDRESS=0x0200 CONFIG_BOOTDELAY=5 CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="earlycon" +CONFIG_USE_BOOTCOMMAND=y +CONFIG_BOOTCOMMAND="run fatscript; run mmcfitload; run linux_qspi_enable; run mmcfitboot" CONFIG_SPL_CACHE=y CONFIG_SPL_SPI_LOAD=y CONFIG_SYS_SPI_U_BOOT_OFFS=0x0200 diff --git a/configs/socfpga_agilex_defconfig b/configs/socfpga_agilex_defconfig index 230d3c2ce5..3d5d39fe0f 100644 --- a/configs/socfpga_agilex_defconfig +++ b/configs/socfpga_agilex_defconfig @@ -18,6 +18,8 @@ CONFIG_DEFAULT_DEVICE_TREE="socfpga_agilex_socdk" CONFIG_BOOTDELAY=5 CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="earlycon" +CONFIG_USE_BOOTCOMMAND=y +CONFIG_BOOTCOMMAND="run fatscript; run mmcload; run linux_qspi_enable; run mmcboot" CONFIG_SPL_CACHE=y CONFIG_SPL_SPI_LOAD=y CONFIG_HUSH_PARSER=y diff --git a/configs/socfpga_stratix10_atf_defconfig b/configs/socfpga_stratix10_atf_defconfig index 1005ba979e..344277a6cc 100644 --- a/configs/socfpga_stratix10_atf_defconfig +++ b/configs/socfpga_stratix10_atf_defconfig @@ -19,6 +19,8 @@ CONFIG_SPL_LOAD_FIT_ADDRESS=0x0200 CONFIG_BOOTDELAY=5 CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="earlycon" +CONFIG_USE_BOOTCOMMAND=y +CONFIG_BOOTCOMMAND="run fatscript; run mmcfitload; run linux_qspi_enable; run mmcfitboot" CONFIG_SPL_SPI_LOAD=y CONFIG_SYS_SPI_U_BOOT_OFFS=0x0200 CONFIG_SPL_ATF=y diff --git a/configs/socfpga_stratix10_defconfig b/configs/socfpga_stratix10_defconfig index 3df44bb88d..2d145e1a5f 100644 --- a/configs/socfpga_stratix10_defconfig +++ b/configs/socfpga_stratix10_defconfig @@ -20,6 +20,8 @@ CONFIG_SPL_OPTIMIZE_INLINING=y CONFIG_BOOTDELAY=5 CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="earlycon" +CONFIG_USE_BOOTCOMMAND=y +CONFIG_BOOTCOMMAND="run fatscript; run mmcload; run linux_qspi_enable; run mmcboot" CONFIG_SPL_SPI_LOAD=y CONFIG_HUSH_PARSER=y CONFIG_SYS_PROMPT="SOCFPGA_STRATIX10 # " diff --git a/include/configs/socfpga_soc64_common.h b/include/configs/socfpga_soc64_common.h index fdcd7d3e9a..35200833f1 100644 --- a/include/configs/socfpga_soc64_common.h +++ b/include/configs/socfpga_soc64_common.h @@ -79,19 +79,12 @@ unsigned int cm_get_qspi_controller_clk_hz(void); #endif /* CONFIG_CADENCE_QSPI */ /* - * Boot arguments passed to the boot command. The value of - * CONFIG_BOOTARGS goes into the environment value "bootargs". - * Do note the value will override also the chosen node in FDT blob. + * Enviroment variable */ - #ifdef CONFIG_FIT #define CONFIG_BOOTFILE "kernel.itb" -#define CONFIG_BOOTCOMMAND "run fatscript; run mmcfitload;run linux_qspi_enable;" \ - "run mmcfitboot" #else #define CONFIG_BOOTFILE "Image" -#define CONFIG_BOOTCOMMAND "run fatscript; run mmcload;run linux_qspi_enable;" \ - "run mmcboot" #endif #define CONFIG_EXTRA_ENV_SETTINGS \ -- 2.13.0
[v2 6/6] configs: socfpga: Add defconfig for Agilex with VAB support
Booting Agilex with Vendor Authorized Boot. Signed-off-by: Siew Chin Lim --- v2 --- - Renamed CONFIG_SECURE_VAB_AUTH to CONFIG_SOCFPGA_SECURE_VAB_AUTH - Add BOOTCOMMAND macro in defconfig --- .../{socfpga_agilex_atf_defconfig => socfpga_agilex_vab_defconfig} | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) copy configs/{socfpga_agilex_atf_defconfig => socfpga_agilex_vab_defconfig} (92%) diff --git a/configs/socfpga_agilex_atf_defconfig b/configs/socfpga_agilex_vab_defconfig similarity index 92% copy from configs/socfpga_agilex_atf_defconfig copy to configs/socfpga_agilex_vab_defconfig index 704a9d56b9..1e28efea5e 100644 --- a/configs/socfpga_agilex_atf_defconfig +++ b/configs/socfpga_agilex_vab_defconfig @@ -1,5 +1,4 @@ CONFIG_ARM=y -CONFIG_ARM_SMCCC=y CONFIG_SPL_LDSCRIPT="arch/arm/mach-socfpga/u-boot-spl-soc64.lds" CONFIG_ARCH_SOCFPGA=y CONFIG_SYS_TEXT_BASE=0x20 @@ -8,6 +7,7 @@ CONFIG_ENV_SIZE=0x1000 CONFIG_ENV_OFFSET=0x200 CONFIG_DM_GPIO=y CONFIG_NR_DRAM_BANKS=2 +CONFIG_SOCFPGA_SECURE_VAB_AUTH=y CONFIG_TARGET_SOCFPGA_AGILEX_SOCDK=y CONFIG_IDENT_STRING="socfpga_agilex" CONFIG_SPL_FS_FAT=y @@ -15,12 +15,13 @@ CONFIG_SPL_TEXT_BASE=0xFFE0 CONFIG_FIT=y CONFIG_SPL_LOAD_FIT=y CONFIG_SPL_LOAD_FIT_ADDRESS=0x0200 +# CONFIG_LEGACY_IMAGE_FORMAT is not set # CONFIG_USE_SPL_FIT_GENERATOR is not set CONFIG_BOOTDELAY=5 CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="earlycon" CONFIG_USE_BOOTCOMMAND=y -CONFIG_BOOTCOMMAND="run fatscript; run mmcfitload; run linux_qspi_enable; run mmcfitboot" +CONFIG_BOOTCOMMAND="run fatscript; run mmcfitload; run mmcfitboot" CONFIG_SPL_CACHE=y CONFIG_SPL_SPI_LOAD=y CONFIG_SYS_SPI_U_BOOT_OFFS=0x0200 -- 2.13.0
[PATCH V3] net: dwc_eth_qos: Pad descriptors to cacheline size
The DWMAC4 IP has the possibility to skip up to 7 AXI bus width size words after the descriptor. Use this to pad the descriptors to cacheline size and remove the need for noncached memory altogether. Moreover, this lets Tegra use the generic cache flush / invalidate operations. Signed-off-by: Marek Vasut Cc: Joe Hershberger Cc: Patrice Chotard Cc: Patrick Delaunay Cc: Ramon Fried Cc: Stephen Warren --- V2: Consider AXI bus width, disable noncached memory on STM32MP1 and Tegra V3: - Replace max() with ALIGN() - Keep noncached memory enabled on tegra for r8169 --- drivers/net/dwc_eth_qos.c | 129 +++-- include/configs/stm32mp1.h | 1 - 2 files changed, 51 insertions(+), 79 deletions(-) diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index db1102562f6..dd0d64f7772 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -209,6 +209,7 @@ struct eqos_dma_regs { #define EQOS_DMA_SYSBUS_MODE_BLEN8 BIT(2) #define EQOS_DMA_SYSBUS_MODE_BLEN4 BIT(1) +#define EQOS_DMA_CH0_CONTROL_DSL_SHIFT 18 #define EQOS_DMA_CH0_CONTROL_PBLX8 BIT(16) #define EQOS_DMA_CH0_TX_CONTROL_TXPBL_SHIFT16 @@ -239,37 +240,15 @@ struct eqos_tegra186_regs { #define EQOS_AUTO_CAL_STATUS_ACTIVEBIT(31) /* Descriptors */ - -#define EQOS_DESCRIPTOR_WORDS 4 -#define EQOS_DESCRIPTOR_SIZE (EQOS_DESCRIPTOR_WORDS * 4) /* We assume ARCH_DMA_MINALIGN >= 16; 16 is the EQOS HW minimum */ #define EQOS_DESCRIPTOR_ALIGN ARCH_DMA_MINALIGN #define EQOS_DESCRIPTORS_TX4 #define EQOS_DESCRIPTORS_RX4 #define EQOS_DESCRIPTORS_NUM (EQOS_DESCRIPTORS_TX + EQOS_DESCRIPTORS_RX) -#define EQOS_DESCRIPTORS_SIZE ALIGN(EQOS_DESCRIPTORS_NUM * \ - EQOS_DESCRIPTOR_SIZE, ARCH_DMA_MINALIGN) #define EQOS_BUFFER_ALIGN ARCH_DMA_MINALIGN #define EQOS_MAX_PACKET_SIZE ALIGN(1568, ARCH_DMA_MINALIGN) #define EQOS_RX_BUFFER_SIZE(EQOS_DESCRIPTORS_RX * EQOS_MAX_PACKET_SIZE) -/* - * Warn if the cache-line size is larger than the descriptor size. In such - * cases the driver will likely fail because the CPU needs to flush the cache - * when requeuing RX buffers, therefore descriptors written by the hardware - * may be discarded. Architectures with full IO coherence, such as x86, do not - * experience this issue, and hence are excluded from this condition. - * - * This can be fixed by defining CONFIG_SYS_NONCACHED_MEMORY which will cause - * the driver to allocate descriptors from a pool of non-cached memory. - */ -#if EQOS_DESCRIPTOR_SIZE < ARCH_DMA_MINALIGN -#if !defined(CONFIG_SYS_NONCACHED_MEMORY) && \ - !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) && !defined(CONFIG_X86) -#warning Cache line size is larger than descriptor size -#endif -#endif - struct eqos_desc { u32 des0; u32 des1; @@ -282,12 +261,17 @@ struct eqos_desc { #define EQOS_DESC3_LD BIT(28) #define EQOS_DESC3_BUF1V BIT(24) +#define EQOS_AXI_WIDTH_32 4 +#define EQOS_AXI_WIDTH_64 8 +#define EQOS_AXI_WIDTH_128 16 + struct eqos_config { bool reg_access_always_ok; int mdio_wait; int swr_wait; int config_mac; int config_mac_mdio; + unsigned int axi_bus_width; phy_interface_t (*interface)(struct udevice *dev); struct eqos_ops *ops; }; @@ -330,9 +314,8 @@ struct eqos_priv { int phyaddr; u32 max_speed; void *descs; - struct eqos_desc *tx_descs; - struct eqos_desc *rx_descs; int tx_desc_idx, rx_desc_idx; + unsigned int desc_size; void *tx_dma_buf; void *rx_dma_buf; void *rx_pkt; @@ -358,63 +341,42 @@ struct eqos_priv { * not have the same constraints since they are 1536 bytes large, so they * are unlikely to share cache-lines. */ -static void *eqos_alloc_descs(unsigned int num) +static void *eqos_alloc_descs(struct eqos_priv *eqos, unsigned int num) { -#ifdef CONFIG_SYS_NONCACHED_MEMORY - return (void *)noncached_alloc(EQOS_DESCRIPTORS_SIZE, - EQOS_DESCRIPTOR_ALIGN); -#else - return memalign(EQOS_DESCRIPTOR_ALIGN, EQOS_DESCRIPTORS_SIZE); -#endif + eqos->desc_size = ALIGN(sizeof(struct eqos_desc), + (unsigned int)ARCH_DMA_MINALIGN); + + return memalign(eqos->desc_size, num * eqos->desc_size); } static void eqos_free_descs(void *descs) { -#ifdef CONFIG_SYS_NONCACHED_MEMORY - /* FIXME: noncached_alloc() has no opposite */ -#else free(descs); -#endif } -static void eqos_inval_desc_tegra186(void *desc) +static struct eqos_desc *eqos_get_desc(struct eqos_priv *eqos, + unsigned int num, bool rx) { -#ifndef CONFIG_SYS_NONCACHED_MEMORY - unsigned long start = (unsigned long)desc & ~(ARCH_DMA_MINALIGN - 1); - unsigned long end = ALIGN
Re: [PATCH v4 0/3] tools/sunxi: Use mkimage for SPL generation
On 1/6/21 3:24 AM, Andre Przywara wrote: > Hi, > > a small update fixing the strncpy bug and addressing too long DT names > properly. Thanks to Samuel for pointing this out. > == > > So far creating a bootable SPL image for Allwinner based boards uses > the mksunxiboot tool. Most other platforms seemed to have integrated this > kind of functionality into the common mkimage tool. > > Since there is nothing special about the Allwinner image in this respect, > just add support for the so-called "eGON" image type into mkimage. If there > was a particular reason this hasn't been done before, please let me know. > > This will eventually allow us to remove mksunxiboot, but I leave it around > for now in case of regressions and since some people depend on it from > external projects. > > Patch 1/3 splits some existing sunxi specific header file, so we can > share the eGON header definition between the tools and the SPL. > Patch 2/3 adds the actual support to mkimage, patch 3/3 then switches > the Makefile to use mkimage instead of mksunxiboot. > > I tested all 152 Allwinner boards by building each > u-boot-sunxi-with-spl.bin and comparing them against the version created > using mksunxiboot (using SOURCE_DATE_EPOCH and .scmversion to create > reproducible builds, and by reverting just patch 3/3). > All files before and after were identical. > > Cheers, > Andre > > Changelog v3 .. v4: > - properly check DT name length before inserting into header > > Changelog v2 .. v3: > - factor out eGON struct into separate header file > - only print extended header information when applicable > - remove redundant alignment > - minor cosmetic fixes > > Changelog v1 .. v2: > - Drop already merged cleanup patch (v1 1/3) > - replace relative include path > - remove already defined ALIGN macro > - rebase against current master > > Andre Przywara (3): > sunxi: Factor out eGON BROM header description > tools: mkimage: Add Allwinner eGON support > sunxi: Use mkimage for SPL boot image generation > > arch/arm/include/asm/arch-sunxi/spl.h | 65 +--- > common/image.c| 1 + > include/image.h | 1 + > include/sunxi_image.h | 82 > scripts/Makefile.spl | 8 +- > tools/Makefile| 1 + > tools/sunxi_egon.c| 136 ++ > 7 files changed, 226 insertions(+), 68 deletions(-) > create mode 100644 include/sunxi_image.h > create mode 100644 tools/sunxi_egon.c For the series: Reviewed-by: Samuel Holland Tested-by: Samuel Holland
[PATCH 3/4] pinctrl: at91-pio4: convert to dev_read_prop
Use dev_read_prop instead of using the fdt_read_property which reads from the GD struct's fdt. This way the node is accessed via the device config instead of the global struct, which makes code more portable and GD independent. Signed-off-by: Eugen Hristev --- drivers/pinctrl/pinctrl-at91-pio4.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c index bf85cc916a..58f4d8cb39 100644 --- a/drivers/pinctrl/pinctrl-at91-pio4.c +++ b/drivers/pinctrl/pinctrl-at91-pio4.c @@ -36,7 +36,7 @@ static const struct pinconf_param conf_params[] = { { "input-debounce", PIN_CONFIG_INPUT_DEBOUNCE, 0 }, }; -static u32 atmel_pinctrl_get_pinconf(const void *blob, int node) +static u32 atmel_pinctrl_get_pinconf(struct udevice *config) { const struct pinconf_param *params; u32 param, arg, conf = 0; @@ -44,7 +44,7 @@ static u32 atmel_pinctrl_get_pinconf(const void *blob, int node) for (i = 0; i < ARRAY_SIZE(conf_params); i++) { params = &conf_params[i]; - if (!fdt_get_property(blob, node, params->property, NULL)) + if (!dev_read_prop(config, params->property, NULL)) continue; param = params->param; @@ -115,7 +115,7 @@ static int atmel_pinctrl_set_state(struct udevice *dev, struct udevice *config) u32 i, conf; int count; - conf = atmel_pinctrl_get_pinconf(blob, node); + conf = atmel_pinctrl_get_pinconf(config); count = fdtdec_get_int_array_count(blob, node, "pinmux", cells, ARRAY_SIZE(cells)); -- 2.25.1
[PATCH 2/4] dt-bindings: pinctrl: at91-pio4: add property for drive strength
Add drive strength property which is equivalent with the one in Linux Signed-off-by: Eugen Hristev --- doc/device-tree-bindings/pinctrl/atmel,at91-pio4-pinctrl.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/device-tree-bindings/pinctrl/atmel,at91-pio4-pinctrl.txt b/doc/device-tree-bindings/pinctrl/atmel,at91-pio4-pinctrl.txt index a376c6fba5..9252dc154e 100644 --- a/doc/device-tree-bindings/pinctrl/atmel,at91-pio4-pinctrl.txt +++ b/doc/device-tree-bindings/pinctrl/atmel,at91-pio4-pinctrl.txt @@ -28,6 +28,8 @@ Optional properties: - GENERIC_PINCONFIG: generic pinconfig options to use, bias-disable, bias-pull-down, bias-pull-up, drive-open-drain, input-schmitt-enable, input-debounce. +- atmel,drive-strength: 0 or 1 for low drive, 2 for medium drive and 3 for +high drive. The default value is low drive. Example: -- 2.25.1
[PATCH 1/4] dt-bindings: pinctrl: at91-pio4: update license to SPDX style
Update license header to SPDX style Signed-off-by: Eugen Hristev --- include/dt-bindings/pinctrl/at91.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/dt-bindings/pinctrl/at91.h b/include/dt-bindings/pinctrl/at91.h index 616f5ce400..5afb1863c3 100644 --- a/include/dt-bindings/pinctrl/at91.h +++ b/include/dt-bindings/pinctrl/at91.h @@ -1,9 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ /* * This header provides constants for most at91 pinctrl bindings. * * Copyright (C) 2013 Jean-Christophe PLAGNIOL-VILLARD - * - * GPLv2 only */ #ifndef __DT_BINDINGS_AT91_PINCTRL_H__ -- 2.25.1
[PATCH 4/4] pinctrl: at91-pio4: implement drive strength support
Implement drive strength support, by preserving the same bindings as in Linux. Signed-off-by: Eugen Hristev --- arch/arm/mach-at91/include/mach/atmel_pio4.h | 1 + drivers/pinctrl/pinctrl-at91-pio4.c | 8 include/dt-bindings/pinctrl/at91.h | 4 3 files changed, 13 insertions(+) diff --git a/arch/arm/mach-at91/include/mach/atmel_pio4.h b/arch/arm/mach-at91/include/mach/atmel_pio4.h index f348b05bc8..35ac7b2d40 100644 --- a/arch/arm/mach-at91/include/mach/atmel_pio4.h +++ b/arch/arm/mach-at91/include/mach/atmel_pio4.h @@ -52,6 +52,7 @@ struct atmel_pio4_port { #define ATMEL_PIO_DRVSTR_LO(1 << 16) #define ATMEL_PIO_DRVSTR_ME(2 << 16) #define ATMEL_PIO_DRVSTR_HI(3 << 16) +#define ATMEL_PIO_DRVSTR_OFFSET16 #define ATMEL_PIO_CFGR_EVTSEL_MASK GENMASK(26, 24) #define ATMEL_PIO_CFGR_EVTSEL_FALLING (0 << 24) #define ATMEL_PIO_CFGR_EVTSEL_RISING (1 << 24) diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c index 58f4d8cb39..350ea0a1f3 100644 --- a/drivers/pinctrl/pinctrl-at91-pio4.c +++ b/drivers/pinctrl/pinctrl-at91-pio4.c @@ -34,6 +34,7 @@ static const struct pinconf_param conf_params[] = { { "input-schmitt-disable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 0 }, { "input-schmitt-enable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1 }, { "input-debounce", PIN_CONFIG_INPUT_DEBOUNCE, 0 }, + { "atmel,drive-strength", PIN_CONFIG_DRIVE_STRENGTH, 0 }, }; static u32 atmel_pinctrl_get_pinconf(struct udevice *config) @@ -41,6 +42,7 @@ static u32 atmel_pinctrl_get_pinconf(struct udevice *config) const struct pinconf_param *params; u32 param, arg, conf = 0; u32 i; + u32 val; for (i = 0; i < ARRAY_SIZE(conf_params); i++) { params = &conf_params[i]; @@ -82,6 +84,12 @@ static u32 atmel_pinctrl_get_pinconf(struct udevice *config) conf |= ATMEL_PIO_IFSCEN_MASK; } break; + case PIN_CONFIG_DRIVE_STRENGTH: + dev_read_u32(config, params->property, &val); + conf &= (~ATMEL_PIO_DRVSTR_MASK); + conf |= (val << ATMEL_PIO_DRVSTR_OFFSET) + & ATMEL_PIO_DRVSTR_MASK; + break; default: printf("%s: Unsupported configuration parameter: %u\n", __func__, param); diff --git a/include/dt-bindings/pinctrl/at91.h b/include/dt-bindings/pinctrl/at91.h index 5afb1863c3..3831f91fb3 100644 --- a/include/dt-bindings/pinctrl/at91.h +++ b/include/dt-bindings/pinctrl/at91.h @@ -42,4 +42,8 @@ #define AT91_PERIPH_C 3 #define AT91_PERIPH_D 4 +#define ATMEL_PIO_DRVSTR_LO1 +#define ATMEL_PIO_DRVSTR_ME2 +#define ATMEL_PIO_DRVSTR_HI3 + #endif /* __DT_BINDINGS_AT91_PINCTRL_H__ */ -- 2.25.1
Re: [PATCH] arm: at91: configs: Enable CONFIG_SYS_NAND_USE_FLASH_BBT on all boards
On 26.11.2020 14:16, Alexander Dahl wrote: > Hello, > > Am Donnerstag, 26. November 2020, 09:21:35 CET schrieb Eugen Hristev: >> From: Nicolas Ferre >> >> As highlighted by Stefan in the commit e074d0f79b2e ("arm: at91: >> gardena-smart-gateway-at91sam: Enable CONFIG_SYS_NAND_USE_FLASH_BBT") >> it's important to use BBT when Linux enables it. We use it for a long >> time on all our boards. >> >> Signed-off-by: Nicolas Ferre >> Cc: Stefan Roese >> Cc: Eugen Hristev >> Cc: Tom Rini > > Acked-by: Alexander Dahl > > Greets > Alex > Applied to u-boot-atmel/master
Re: [PATCH v2 00/34] Sama7g5 Evaluation Kit support
On 05.12.2020 12:02, Eugen Hristev wrote: > Hello, > > This series adds support for sama7g5 SoC DT and the sama7g5ek board. > I kept the original incremental development for this board, with each > commit's author and designated change, for traceability and for easier > reviewing. > > The series starts from a basic devicetree and ends with a fully functional > board including SD-Card/MMC, i2c eeproms, ethernet. > > Thanks, > Eugen > > Changes in v2: > - fixed compatible for i2c eeprom, mac was not read correctly in latest > u-boot version > - fixed some checkpatch warnings and some spacing > > Claudiu Beznea (20): >board: atmel: sama7g5ek: add SYS_MALLOC_F_LEN to SYS_INIT_SP_ADDR >configs: sama7g5ek: set malloc pool to 68K >configs: sama7g5ek: enable pll driver >ARM: dts: sama7g5: move clock frequencies for xtals in board file >ARM: dts: sama7g5: add slow rc and main rc oscillators >ARM: dts: sama7g5: add u-boot,dm-pre-reloc bindings for xtals >ARM: dts: sama7g5: add slow clock bindings >ARM: dts: sama7g5: add PMC bindings >ARM: dts: sama7g5: switch to PMC bindings >configs: sama7g5: enable CONFIG_CPU >ARM: dts: sama7g5: add CPU bindings >configs: sama7g5: use PIT64B >ARM: dts: sama7g5: enable autoboot >ARM: dts: sama7g5: add pit64b support >configs: sama7g5ek: enable mii command >ARM: dts: sama7g5: add GMAC0 >ARM: dts: sama7g5: add GMAC1 >board: atmel: sama7g5ek: increase arp timeout and retry count >configs: sama7g5ek: enable support for KSZ9131 >configs: sama7g5ek: enable CCF > > Eugen Hristev (13): >ARM: dts: sama7g5: add initial DT for sama7g5 SoC >board: atmel: sama7g5ek: add initial support for sama7g5ek >ARM: dts: at91: sama7g5: add pinctrl node >ARM: dts: at91: sama7g5ek: add pinctrl for sdmmc1 and flx3 >ARM: dts: at91: sama7g5: add assigned clocks for sdmmc1 >ARM: dts: at91: sama7g5: add node for sdmmc0 >ARM: dts: at91: sama7g5ek: enable sdmmc0 with pinctrl >board: atmel: sama7g5ek: clean-up header bootcommand >configs: sama7g5: add mmc config for sdmmc0 >ARM: dts: at91: sama7g5: add flexcom1 and i2c subnode >ARM: dts: sama7g5ek: add i2c1 bus and eeproms >board: atmel: sama7g5ek: add support for MAC address retreival >configs: sama7g5ek: add i2c and eeprom > > Nicolas Ferre (1): >ARM: dts: sama7g5ek: fix TXC pin configuration > > arch/arm/dts/Makefile | 3 + > arch/arm/dts/sama7g5.dtsi | 170 > arch/arm/dts/sama7g5ek-u-boot.dtsi | 65 ++ > arch/arm/dts/sama7g5ek.dts | 202 + > arch/arm/mach-at91/Kconfig | 8 ++ > board/atmel/sama7g5ek/Kconfig | 15 +++ > board/atmel/sama7g5ek/MAINTAINERS | 8 ++ > board/atmel/sama7g5ek/Makefile | 7 + > board/atmel/sama7g5ek/sama7g5ek.c | 76 +++ > configs/sama7g5ek_mmc1_defconfig | 70 ++ > configs/sama7g5ek_mmc_defconfig| 70 ++ > include/configs/sama7g5ek.h| 45 +++ > 12 files changed, 739 insertions(+) > create mode 100644 arch/arm/dts/sama7g5.dtsi > create mode 100644 arch/arm/dts/sama7g5ek-u-boot.dtsi > create mode 100644 arch/arm/dts/sama7g5ek.dts > create mode 100644 board/atmel/sama7g5ek/Kconfig > create mode 100644 board/atmel/sama7g5ek/MAINTAINERS > create mode 100644 board/atmel/sama7g5ek/Makefile > create mode 100644 board/atmel/sama7g5ek/sama7g5ek.c > create mode 100644 configs/sama7g5ek_mmc1_defconfig > create mode 100644 configs/sama7g5ek_mmc_defconfig > create mode 100644 include/configs/sama7g5ek.h > Applied to u-boot-atmel/master
Re: [PATCH] doc/sandbox: improve formatting of command line options
On Wed, 30 Dec 2020 at 10:10, Heinrich Schuchardt wrote: > > Show the command line options in bold. > > Signed-off-by: Heinrich Schuchardt > --- > doc/arch/sandbox.rst | 28 ++-- > 1 file changed, 14 insertions(+), 14 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH RFC v2 3/5] lib: Add support for ECDSA image signing
Hi Alexandru, On Wed, 30 Dec 2020 at 14:00, Alexandru Gagniuc wrote: > > mkimage supports rsa2048, and rsa4096 signatures. With newer silicon > now supporting hardware-accelerated ECDSA, it makes sense to expand > signing support to elliptic curves. > > Implement host-side ECDSA signing and verification with libcrypto. > Device-side implementation of signature verification is beyond the > scope of this patch. > > Signed-off-by: Alexandru Gagniuc > --- > common/image-sig.c | 14 +- > include/u-boot/ecdsa.h | 27 > lib/ecdsa/ecdsa-libcrypto.c | 300 > tools/Makefile | 3 + > 4 files changed, 342 insertions(+), 2 deletions(-) > create mode 100644 include/u-boot/ecdsa.h > create mode 100644 lib/ecdsa/ecdsa-libcrypto.c > > diff --git a/common/image-sig.c b/common/image-sig.c > index 21dafe6b91..2548b3eb0f 100644 > --- a/common/image-sig.c > +++ b/common/image-sig.c > @@ -15,6 +15,7 @@ > DECLARE_GLOBAL_DATA_PTR; > #endif /* !USE_HOSTCC*/ > #include > +#include > #include > #include > > @@ -82,8 +83,17 @@ struct crypto_algo crypto_algos[] = { > .sign = rsa_sign, > .add_verify_data = rsa_add_verify_data, > .verify = rsa_verify, > - } > - > + }, > +#if defined(USE_HOSTCC) > + /* Currently, only host support exists for ECDSA */ > + { > + .name = "ecdsa256", > + .key_len = ECDSA256_BYTES, > + .sign = ecdsa_sign, > + .add_verify_data = ecdsa_add_verify_data, > + .verify = ecdsa_verify, > + }, > +#endif > }; > > struct padding_algo padding_algos[] = { > diff --git a/include/u-boot/ecdsa.h b/include/u-boot/ecdsa.h > new file mode 100644 > index 00..a13a7267e1 > --- /dev/null > +++ b/include/u-boot/ecdsa.h > @@ -0,0 +1,27 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Copyright (c) 2020, Alexandru Gagniuc . > + */ > + > +#ifndef _ECDSA_H > +#define _ECDSA_H > + > +#include > +#include > + > +/** > + * crypto_algo API impementation for ECDSA; > + * @see "struct crypto_algo" > + * @{ > + */ > +int ecdsa_sign(struct image_sign_info *info, const struct image_region > region[], > + int region_count, uint8_t **sigp, uint *sig_len); > +int ecdsa_verify(struct image_sign_info *info, > +const struct image_region region[], int region_count, > +uint8_t *sig, uint sig_len); > +int ecdsa_add_verify_data(struct image_sign_info *info, void *keydest); Please always add full comments when you export functions, or have a non-trivial static function. > +/** @} */ > + > +#define ECDSA256_BYTES (256 / 8) > + > +#endif > diff --git a/lib/ecdsa/ecdsa-libcrypto.c b/lib/ecdsa/ecdsa-libcrypto.c > new file mode 100644 > index 00..ff491411d0 > --- /dev/null > +++ b/lib/ecdsa/ecdsa-libcrypto.c > @@ -0,0 +1,300 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * ECDSA image signing implementation using libcrypto backend > + * > + * The signature is a binary representation of the (R, S) points, padded to > the > + * key size. The signature will be (2 * key_size_bits) / 8 bytes. > + * > + * Deviations from behavior of RSA equivalent: > + * - Verification uses private key. This is not technically required, but a > + *limitation on how clumsy the openssl API is to use. I'm not quite sure what the implications are on this. If this is public key crypto, the private key is not available, so how can you verify with it? > + * - Handling of keys and key paths: > + *- The '-K' key directory option must contain path to the key file, > + * instead of the key directory. If we make this change we should update RSA to do the same. > + *- No assumptions are made about the file extension of the key > + *- The 'key-name-hint' property is only used for naming devicetree > nodes, > + * but is not used for looking up keys on the filesystem. > + * > + * Copyright (c) 2020, Alexandru Gagniuc > + */ > + > +#include > +#include > +#include > +#include > +#include > + > +struct signer { > + EVP_PKEY *evp_key; > + EC_KEY *ecdsa_key; > + void *hash; > + void *signature;/* Do not free() !*/ need comments > +}; > + > +static int alloc_ctx(struct signer *ctx, const struct image_sign_info *info) > +{ > + memset(ctx, 0, sizeof(*ctx)); > + > + if (!OPENSSL_init_ssl(0, NULL)) { > + fprintf(stderr, "Failure to init SSL library\n"); > + return -1; > + } > + > + ctx->hash = malloc(info->checksum->checksum_len); > + ctx->signature = malloc(info->crypto->key_len * 2); > + > + if (!ctx->hash || !ctx->signature) > + return -1; > + > + return 0; > +} > + > +static void free_ctx(struct signer *ctx) > +{ > + if (ctx->ecdsa_key) > + EC_KEY_free(ctx->ecdsa_key); > + > + if (ctx->evp_key) > +
Re: [PATCH RFC v2 2/5] lib/rsa: Make fdt_add_bignum() available outside of RSA code
Hi Alexandru, On Wed, 30 Dec 2020 at 14:00, Alexandru Gagniuc wrote: > > fdt_add_bignum() is useful for algorithms other than just RSA. To > allow its use for ECDSA, move it to a common file under lib/. > > The new file is suffixed with '-libcrypto' because it has a direct > dependency on openssl. This is due to the use of the "BIGNUM *" type. > > Signed-off-by: Alexandru Gagniuc > --- > include/u-boot/fdt-libcrypto.h | 15 +++ > lib/fdt-libcrypto.c| 72 ++ > lib/rsa/rsa-sign.c | 65 +- > tools/Makefile | 1 + > 4 files changed, 89 insertions(+), 64 deletions(-) > create mode 100644 include/u-boot/fdt-libcrypto.h > create mode 100644 lib/fdt-libcrypto.c Reviewed-by: Simon Glass > > diff --git a/include/u-boot/fdt-libcrypto.h b/include/u-boot/fdt-libcrypto.h > new file mode 100644 > index 00..a6873487ed > --- /dev/null > +++ b/include/u-boot/fdt-libcrypto.h > @@ -0,0 +1,15 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > +/* > + * Copyright (c) 2020, Alexandru Gagniuc > + * Copyright (c) 2013, Google Inc. > + */ > + > +#ifndef _FDT_LIBCRYPTO_H > +#define _FDT_LIBCRYPTO_H > + > +#include > + > +int fdt_add_bignum(void *blob, int noffset, const char *prop_name, > + BIGNUM *num, int num_bits); Please add a full function comment. Regards, Simon
Re: [PATCH 1/1] fs/fat: implement fsuuid command
On Wed, 30 Dec 2020 at 16:53, Heinrich Schuchardt wrote: > > The FAT file system does not have a UUID but a 4 byte volume ID. > Let the fsuuid command show it in - format. > > Signed-off-by: Heinrich Schuchardt > --- > fs/fat/fat.c | 18 ++ > fs/fs.c | 2 +- > include/fat.h | 12 > 3 files changed, 31 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass Is this something the FS tests can test?
Re: [PATCH RFC v2 5/5] test/py: ecdsa: Add test for mkimage ECDSA signing
Hi Alexandru, On Wed, 30 Dec 2020 at 14:00, Alexandru Gagniuc wrote: > > Add a test to make sure that the ECDSA signatures generated by > mkimage can be verified successfully. pyCryptodomex was chosen as the > crypto library because it integrates much better with python code. > Using openssl would have been unnecessarily painful. > > Signed-off-by: Alexandru Gagniuc > --- > test/py/tests/test_fit_ecdsa.py | 111 > 1 file changed, 111 insertions(+) > create mode 100644 test/py/tests/test_fit_ecdsa.py > This test looks fine but the functions need full comments. I do think it might be worth putting the code in test_vboot, particularly when you get to the sandbox implementation. For installing the Python library, you might need to update the docs in test/ and perhaps install things in .gitlab-ci.yml and .azure... > diff --git a/test/py/tests/test_fit_ecdsa.py b/test/py/tests/test_fit_ecdsa.py > new file mode 100644 > index 00..957964d329 > --- /dev/null > +++ b/test/py/tests/test_fit_ecdsa.py > @@ -0,0 +1,111 @@ > +# SPDX-License-Identifier: GPL-2.0+ > +# > +# Copyright (c) 2020, Alexandru Gagniuc > + > +""" > +Test ECDSA signing of FIT images > + > +This test uses mkimage to sign an existing FIT image with an ECDSA key. The > +signature is then extracted, and verified against pyCryptodome. > +This test doesn't run the sandbox. It only checks the host tool 'mkimage' > +""" > + > +import pytest > +import u_boot_utils as util > +from Cryptodome.Hash import SHA256 > +from Cryptodome.PublicKey import ECC > +from Cryptodome.Signature import DSS > + > +class SignableFitImage(object): > +""" Helper to manipulate a FIT image on disk """ > +def __init__(self, cons, file_name): > +self.fit = file_name > +self.cons = cons > +self.signable_nodes = set() > + > +def __fdt_list(self, path): > +return util.run_and_log(self.cons, f'fdtget -l {self.fit} {path}') > + > +def __fdt_set(self, node, **prop_value): > +for prop, value in prop_value.items(): > +util.run_and_log(self.cons, f'fdtput -ts {self.fit} {node} > {prop} {value}') > + > +def __fdt_get_binary(self, node, prop): > +numbers = util.run_and_log(self.cons, f'fdtget -tbi {self.fit} > {node} {prop}') > + > +bignum = bytearray() > +for little_num in numbers.split(): > +bignum.append(int(little_num)) > + > +return bignum > + > +def find_signable_image_nodes(self): > +for node in self.__fdt_list('/images').split(): > +image = f'/images/{node}' > +if 'signature' in self.__fdt_list(image): > +self.signable_nodes.add(image) > + > +return self.signable_nodes > + > +def change_signature_algo_to_ecdsa(self): > +for image in self.signable_nodes: > +self.__fdt_set(f'{image}/signature', algo='sha256,ecdsa256') > + > +def sign(self, mkimage, key_file): > +util.run_and_log(self.cons, [mkimage, '-F', self.fit, > f'-k{key_file}']) > + > +def check_signatures(self, key): > +for image in self.signable_nodes: > +raw_sig = self.__fdt_get_binary(f'{image}/signature', 'value') > +raw_bin = self.__fdt_get_binary(image, 'data') > + > +sha = SHA256.new(raw_bin) > +verifier = DSS.new(key, 'fips-186-3') > +verifier.verify(sha, bytes(raw_sig)) > + > + > +@pytest.mark.buildconfigspec('fit_signature') > +@pytest.mark.requiredtool('dtc') > +@pytest.mark.requiredtool('fdtget') > +@pytest.mark.requiredtool('fdtput') > +def test_fit_ecdsa(u_boot_console): > +"""TODO: Document me > +""" > +def generate_ecdsa_key(): > +return ECC.generate(curve='prime256v1') > + > +def assemble_fit_image(dest_fit, its, destdir): > +dtc_args = f'-I dts -O dtb -i {destdir}' > +util.run_and_log(cons, [mkimage, '-D', dtc_args, '-f', its, > dest_fit]) > + > +def dtc(dts): > +dtb = dts.replace('.dts', '.dtb') > +util.run_and_log(cons, f'dtc {datadir}/{dts} -O dtb -o > {tempdir}/{dtb}') > + > +cons = u_boot_console > +mkimage = cons.config.build_dir + '/tools/mkimage' > +datadir = cons.config.source_dir + '/test/py/tests/vboot/' > +tempdir = cons.config.result_dir > +key_file = f'{tempdir}/ecdsa-test-key.pem' > +fit_file = f'{tempdir}/test.fit' > +dtc('sandbox-kernel.dts') > + > +key = generate_ecdsa_key() > + > +# Create a number kernel image with zeroes > +with open(f'{tempdir}/test-kernel.bin', 'w') as fd: > +fd.write(500 * chr(0)) > + > +with open(key_file, 'w') as f: > +f.write(key.export_key(format='PEM')) > + > +assemble_fit_image(fit_file, f'{datadir}/sign-images-sha256.its', > tempdir) > + > +fit = SignableFitImage(cons, fit_file) > +nodes = fit.find_signable_image_nodes() > +if len(nodes) == 0: > +raise ValueError('FIT image has no "/image
Re: [PATCH 3/9] part: Give several functions more useful return values
On Thu, 31 Dec 2020 at 15:49, Sean Anderson wrote: > > Several functions in disk/part.c just return -1 on error. This makes them > return different errnos for different failures. This helps callers > differentiate between failures, even if they cannot read stdout. > > Signed-off-by: Sean Anderson > --- > > disk/part.c | 50 -- > 1 file changed, 28 insertions(+), 22 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 2/9] test: dm: Add test for fastboot mmc partition naming
On Thu, 31 Dec 2020 at 15:49, Sean Anderson wrote: > > This test verifies the mapping between fastboot partitions and partitions > as understood by U-Boot. It also tests the creation of GPT partitions, > though that is not the primary goal. > > Signed-off-by: Sean Anderson > --- > > configs/sandbox64_defconfig | 2 ++ > configs/sandbox_defconfig | 2 ++ > test/dm/Makefile| 3 ++ > test/dm/fastboot.c | 64 + > 4 files changed, 71 insertions(+) > create mode 100644 test/dm/fastboot.c Reviewed-by: Simon Glass
Re: [PATCH 5/9] part: Support string block devices in part_get_info_by_dev_and_name
On Thu, 31 Dec 2020 at 15:49, Sean Anderson wrote: > > This adds support for things like "#partname" and "0.1#partname". The block > device parsing is done like in blk_get_device_part_str. > > Signed-off-by: Sean Anderson > --- > > disk/part.c | 32 +--- > 1 file changed, 17 insertions(+), 15 deletions(-) Reviewed-by: Simon Glass Does the function comment need updating? > > diff --git a/disk/part.c b/disk/part.c > index 39c6b00a59..e07dd67fd9 100644 > --- a/disk/part.c > +++ b/disk/part.c > @@ -707,29 +707,31 @@ static int part_get_info_by_dev_and_name(const char > *dev_iface, > struct blk_desc **dev_desc, > struct disk_partition *part_info) > { > - char *ep; > - const char *part_str; > - int dev_num, ret; > + char *dup_str = NULL; > + const char *dev_str, *part_str; > + int ret; > > + /* Separate device and partition name specification */ > part_str = strchr(dev_part_str, '#'); > - if (!part_str || part_str == dev_part_str) > - return -EINVAL; > - > - dev_num = simple_strtoul(dev_part_str, &ep, 16); > - if (ep != part_str) { > - /* Not all the first part before the # was parsed. */ > + if (part_str) { > + dup_str = strdup(dev_part_str); > + dup_str[part_str - dev_part_str] = 0; > + dev_str = dup_str; > + part_str++; > + } else { > return -EINVAL; > } > - part_str++; > > - *dev_desc = blk_get_dev(dev_iface, dev_num); > - if (!*dev_desc) { > - printf("Could not find %s %d\n", dev_iface, dev_num); > - return -ENODEV; > - } > + ret = blk_get_device_by_str(dev_iface, dev_str, dev_desc); > + if (ret) > + goto cleanup; > + > ret = part_get_info_by_name(*dev_desc, part_str, part_info); > if (ret < 0) > printf("Could not find \"%s\" partition\n", part_str); > + > +cleanup: > + free(dup_str); > return ret; > } > > -- > 2.25.1 >
Re: [BUG] doc/develop/logging.rst
Hi Heinrich, On Thu, 31 Dec 2020 at 17:07, Heinrich Schuchardt wrote: > > Hello Simon, > > I am trying to port the updated Sphinx build system from the Linux > kernel to U-Boot so that we can use Sphinx 3. > > With Sphinx 3 I get an error because the same C elements are described > twice: > > Warning, treated as error: > doc/develop/logging:39: ./include/log.h:3: > Duplicate C declaration, also defined at develop/logging:79. > > I think the whole chapter "Logging API" can be moved to doc/api/ and all > other kernel-doc:: should be replaced by cross-references. That seems OK to me. Regards, Simon
Re: [PATCH 7/9] fastboot: Move part_get_info_by_name_or_alias after raw_part_get_info_by_name
On Thu, 31 Dec 2020 at 15:49, Sean Anderson wrote: > > This makes the next commit more readable by doing the move now. > > Signed-off-by: Sean Anderson > --- > > drivers/fastboot/fb_mmc.c | 44 +++ > 1 file changed, 22 insertions(+), 22 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 9/9] fastboot: Document alternate partition names
On Thu, 31 Dec 2020 at 15:49, Sean Anderson wrote: > > This documents the new partition names added in the previous commit. > > Signed-off-by: Sean Anderson > --- > > doc/android/fastboot.rst | 28 > 1 file changed, 28 insertions(+) > Reviewed-by: Simon Glass (for content, not location)
Re: [PATCH 1/1] doc: update Kernel documentation build system
On Thu, 31 Dec 2020 at 17:21, Heinrich Schuchardt wrote: > > Update the docomentation build system according to Linux v5.11-rc1. > > With this patch we can build the HTML documentation using either of > Sphinx 2 and Sphinx 3. > > Signed-off-by: Heinrich Schuchardt > --- > Except for doc/develop/logging.rst I have already sent patches for > erroneous pages that did not build with Sphinx 3. > --- > doc/conf.py | 141 -- > doc/sphinx/automarkup.py | 290 +++ > doc/sphinx/cdomain.py | 93 +- > doc/sphinx/kernel_abi.py | 194 + > doc/sphinx/kernel_feat.py | 169 +++ > doc/sphinx/kerneldoc.py | 15 +- > doc/sphinx/kernellog.py | 6 +- > doc/sphinx/kfigure.py | 6 +- > doc/sphinx/load_config.py | 27 +- > doc/sphinx/maintainers_include.py | 197 + > doc/sphinx/parallel-wrapper.sh| 33 +++ > doc/sphinx/parse-headers.pl | 6 +- > doc/sphinx/requirements.txt | 5 +- > scripts/kernel-doc| 450 ++ > 14 files changed, 1476 insertions(+), 156 deletions(-) > create mode 100644 doc/sphinx/automarkup.py > create mode 100644 doc/sphinx/kernel_abi.py > create mode 100644 doc/sphinx/kernel_feat.py > create mode 100755 doc/sphinx/maintainers_include.py > create mode 100644 doc/sphinx/parallel-wrapper.sh Reviewed-by: Simon Glass
Re: [PATCH 1/1] mtd: misplaced log.h and dm/devres.h
On Sun, 3 Jan 2021 at 08:10, Heinrich Schuchardt wrote: > > log.h and dm/devres.h are U-Boot includes. So placing them > behind #ifndef __UBOOT__ does not make any sense. > > Fixes: f7ae49fc4f36 ("common: Drop log.h from common header") > Fixes: 61b29b826838 ("dm: core: Require users of devres to include the > header") > Signed-off-by: Heinrich Schuchardt > --- > drivers/mtd/mtdcore.c | 2 -- > 1 file changed, 2 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 2/5] log: make debug_cond() function like
On Mon, 4 Jan 2021 at 00:03, Heinrich Schuchardt wrote: > > Change debug_cond() such that it can be used instead of a function like > debug(). > > Signed-off-by: Heinrich Schuchardt > --- > include/log.h | 17 - > 1 file changed, 8 insertions(+), 9 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 4/5] log: convert pr_*() to logging
Hi Heinrich, On Mon, 4 Jan 2021 at 00:03, Heinrich Schuchardt wrote: > > In drivers we use a family of printing functions including pr_err() and > pr_cont(). CONFIG_LOGLEVEL is used to control which of these lead to output > via printf(). > > Our logging functions allow finer grained control of output. So replace > printf() by the matching logging functions. The usage of CONFIG_LOGLEVEL > remains unchanged. > > Signed-off-by: Heinrich Schuchardt > --- > include/linux/bitops.h | 4 ++- > include/linux/printk.h | 82 +++--- > 2 files changed, 48 insertions(+), 38 deletions(-) > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h > index 16f28993f5..d2e5ca026e 100644 > --- a/include/linux/bitops.h > +++ b/include/linux/bitops.h > @@ -6,7 +6,6 @@ > #include > #include > #include > -#include > > #ifdef __KERNEL__ > #define BIT(nr)(1UL << (nr)) > @@ -19,6 +18,9 @@ > #define BITS_TO_LONGS(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long)) > #endif > > +/* kernel.h includes log.h which include bitops.h */ > +#include > + > /* > * Create a contiguous bitmask starting at bit position @l and ending at > * position @h. For example > diff --git a/include/linux/printk.h b/include/linux/printk.h > index 088513ad29..5e85513853 100644 > --- a/include/linux/printk.h > +++ b/include/linux/printk.h > @@ -1,6 +1,7 @@ > #ifndef __KERNEL_PRINTK__ > #define __KERNEL_PRINTK__ > > +#include > #include > #include > > @@ -28,49 +29,56 @@ > 0; \ > }) > > -#define __printk(level, fmt, ...) \ > -({ \ > - level < CONFIG_LOGLEVEL ? printk(fmt, ##__VA_ARGS__) : 0; \ > -}) > - > #ifndef pr_fmt > #define pr_fmt(fmt) fmt > #endif > > -#define pr_emerg(fmt, ...) \ > - __printk(0, pr_fmt(fmt), ##__VA_ARGS__) > -#define pr_alert(fmt, ...) \ > - __printk(1, pr_fmt(fmt), ##__VA_ARGS__) > -#define pr_crit(fmt, ...) \ > - __printk(2, pr_fmt(fmt), ##__VA_ARGS__) > -#define pr_err(fmt, ...) \ > - __printk(3, pr_fmt(fmt), ##__VA_ARGS__) > -#define pr_warning(fmt, ...) \ > - __printk(4, pr_fmt(fmt), ##__VA_ARGS__) > -#define pr_warn pr_warning > -#define pr_notice(fmt, ...) \ > - __printk(5, pr_fmt(fmt), ##__VA_ARGS__) > -#define pr_info(fmt, ...) \ > - __printk(6, pr_fmt(fmt), ##__VA_ARGS__) > - > -#define pr_cont(fmt, ...) \ > - printk(fmt, ##__VA_ARGS__) > - > -/* pr_devel() should produce zero code unless DEBUG is defined */ > -#ifdef DEBUG > -#define pr_devel(fmt, ...) \ > - __printk(7, pr_fmt(fmt), ##__VA_ARGS__) > -#else > -#define pr_devel(fmt, ...) \ > - no_printk(pr_fmt(fmt), ##__VA_ARGS__) > -#endif > +#define pr_emerg(fmt, ...) \ > +({ \ > + CONFIG_LOGLEVEL > 0 ? log_emerg(fmt, ##__VA_ARGS__) : 0;\ Should this use CONFIG_VAL(LOGLEVEL) ? Also I don't suppose there is a way to implement this in the __printk() macro? > +}) > +#define pr_alert(fmt, ...) \ > +({ \ > + CONFIG_LOGLEVEL > 1 ? log_alert(fmt, ##__VA_ARGS__) : 0;\ > +}) > +#define pr_crit(fmt, ...) \ > +({ \ > + CONFIG_LOGLEVEL > 2 ? log_crit(fmt, ##__VA_ARGS__) : 0; \ > +}) > +#define pr_err(fmt, ...) \ > +({ \ > + CONFIG_LOGLEVEL > 3 ? log_err(fmt, ##__VA_ARGS__) : 0; \ > +}) > +#define pr_warn(fmt, ...) \ > +({ \ > + CONFIG_LOGLEVEL > 4 ? log_warning(fmt, ##__VA_ARGS__) : 0; \ > +}) > +#define pr_notice(fmt, ...)\ > +({ \ > + CONFIG_LOGLEVEL > 5 ? log_notice(fmt, ##__VA_ARGS__) : 0; \ > +}) > +#define pr_info(fmt, ...) \ > +({ \ > + CONFIG_LOGLEVEL > 6 ? log_info(fmt, ##__VA_ARGS__) : 0; \ > +}) > +#define pr_debug(fmt, ...) \ > +({ \ > + CONFIG_LOGLEVEL > 7 ? log_debug(fmt, ##__VA_ARGS__) : 0;\ > +}) > +#define pr_devel(fmt, ...) \ > +({ \ > +
Re: [PATCH 1/5] ram: k3-j721e: rename BIT_MASK()
On Mon, 4 Jan 2021 at 00:03, Heinrich Schuchardt wrote: > > The macro BIT_MASK is already defined in include/linux/bitops.h. To avoid > name collisions rename BIT_MASK() in drivers/ram/k3-j721e/lpddr4_private.h > to LPDDR4_BIT_MASK(). > > Remove superfluous parantheses. > Remove superfluous comparison to 0. > > Signed-off-by: Heinrich Schuchardt > --- > drivers/ram/k3-j721e/lpddr4.c | 14 +++--- > drivers/ram/k3-j721e/lpddr4_private.h | 20 > 2 files changed, 19 insertions(+), 15 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 3/5] log: provide missing macros
On Mon, 4 Jan 2021 at 00:03, Heinrich Schuchardt wrote: > > With commit d094a0734cee ("log: allow for message continuation") we have > defined a special log level and category for message continuation. Let's > have a macro for using these. > > If logging is enabled log_cont() will create a continuation log output with > the same logging level and category as the previous message. > > If logging is not enabled, log_cont() will print like printf(). > > Provide macros for logging levels LOG_EMERG, LOG_ALERT, LOG_CRIT. > > Signed-off-by: Heinrich Schuchardt > --- > include/log.h | 8 > 1 file changed, 8 insertions(+) Reviewed-by: Simon Glass
Re: [PATCH 5/5] test: unit test for pr_err(), pr_cont()
On Mon, 4 Jan 2021 at 00:03, Heinrich Schuchardt wrote: > > Provide a unit test for printing via pr_err() and pr_cont(). > > Signed-off-by: Heinrich Schuchardt > --- > test/log/Makefile | 1 + > test/log/pr_cont_test.c | 45 + > 2 files changed, 46 insertions(+) > create mode 100644 test/log/pr_cont_test.c > Reviewed-by: Simon Glass
Re: [PATCH 1/1] sandbox: keep time offset when resetting
Hi Heinrich, On Wed, 30 Dec 2020 at 10:07, Heinrich Schuchardt wrote: > > The UEFI Self Certification Test (SCT) checks the SetTime() service with > the following steps: > > * set date > * reset > * check date matches > > To be compliant the sandbox should keep the offset to the host RTC during > resets. The implementation uses the environment variable > UBOOT_SB_TIME_OFFSET to persist the offset. > > Signed-off-by: Heinrich Schuchardt > --- > arch/sandbox/cpu/os.c | 25 + > doc/arch/sandbox.rst | 7 +++ > drivers/rtc/i2c_rtc_emul.c | 4 +++- > include/os.h | 18 ++ > 4 files changed, 53 insertions(+), 1 deletion(-) Sandbox writes driver settings to a state file that can be read on start-up. Is that suitable here? Regards, Simon
Re: [PATCH] net: eth-uclass: Change uclass driver name to ethernet
Hi David, On Wed, 6 Jan 2021 at 20:56, David Wu wrote: > > dev_read_alias_seq() used uc_drv->name compared to alias > stem string, Ethernet's alias stem uses "ethernet", which > does not match the eth-uclass driver name "eth", can not > get the correct index of ethernet alias namer. So it seems > change uclass driver name to match the alias stem is a more > reasonable way. > > Signed-off-by: David Wu > --- > net/eth-uclass.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/eth-uclass.c b/net/eth-uclass.c > index e14695c0f1..7dd962db64 100644 > --- a/net/eth-uclass.c > +++ b/net/eth-uclass.c > @@ -594,7 +594,7 @@ static int eth_pre_remove(struct udevice *dev) > } > > UCLASS_DRIVER(eth) = { Should change this one too ^ > - .name = "eth", > + .name = "ethernet", > .id = UCLASS_ETH, > .post_bind = eth_post_bind, > .pre_unbind = eth_pre_unbind, > -- > 2.19.1 > > Regards, Simon >
Re: [v2 2/6] arm: socfpga: soc64: Support Vendor Authorized Boot (VAB)
On Thu, 7 Jan 2021 at 03:03, Siew Chin Lim wrote: > > Vendor Authorized Boot is a security feature for authenticating > the images such as U-Boot, ARM trusted Firmware, Linux kernel, > device tree blob and etc loaded from FIT. After those images are > loaded from FIT, the VAB certificate and signature block appended > at the end of each image are sent to Secure Device Manager (SDM) > for authentication. U-Boot will validate the SHA384 of the image > against the SHA384 hash stored in the VAB certificate before > sending the image to SDM for authentication. > > Signed-off-by: Siew Chin Lim > > --- > v2 > --- > - Renamed SECURE_VAB_AUTH* to SOCFPGA_SECURE_VAB_AUTH* > - Changes in secure_vab.c > - Changed to use SZ_1K for 1024 > - Updated comment in secure_vab.c of "... the certificate for T" > - The code will report error before end of the function if reach > maximum retry. > - In board_prep_linux function, only execute linux_qspi_enable > command if it exists in enviroment variable. It is optional. > --- > arch/arm/mach-socfpga/Kconfig| 15 ++ > arch/arm/mach-socfpga/Makefile | 2 + > arch/arm/mach-socfpga/include/mach/mailbox_s10.h | 1 + > arch/arm/mach-socfpga/include/mach/secure_vab.h | 63 > arch/arm/mach-socfpga/secure_vab.c | 193 > +++ > common/Kconfig.boot | 2 +- > 6 files changed, 275 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/mach-socfpga/include/mach/secure_vab.h > create mode 100644 arch/arm/mach-socfpga/secure_vab.c I think this could use a few more function comments. Also try to use if() instead of #if Regards, Simon
Re: [PATCH 2/3] pci: Make auto-config code a little more robust
On Sun, 3 Jan 2021 at 15:05, Phil Sutter wrote: > > On my DS414, some PCI devices return odd values when probing BAR sizes. > An obvious case is all-ones response, the Linux driver > (drivers/pci/probe.c) catches those explicitly and a comment explains > that either bit 0 or bit 1 must be clear (depending on MEM or IO type). > Other BARs return e.g. 0xffff or 0xfff4 and thus manage to break > size calculation due to the "middle" zeroes. Mitigate that copying more > or less what Linux does and do a "find least bit set". > > Signed-off-by: Phil Sutter > --- > drivers/pci/pci_auto.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > Reviewed-by: Simon Glass
Re: [v2 4/6] arm: socfpga: dts: soc64: Update filename in binman node of FIT image with VAB support
On Thu, 7 Jan 2021 at 03:03, Siew Chin Lim wrote: > > FIT image of Vendor Authentication Coot (VAB) contains signed images. > > Signed-off-by: Siew Chin Lim > --- > arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi | 22 ++ > 1 file changed, 22 insertions(+) > I'm not quite sure what is happening here, but consider using two separate files rather than what looks like a patch over an existing one. > diff --git a/arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi > b/arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi > index cf365590a8..4b30473743 100644 > --- a/arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi > +++ b/arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi > @@ -117,4 +117,26 @@ > }; > }; > > +#if defined(CONFIG_SOCFPGA_SECURE_VAB_AUTH) > +&uboot_blob { > + filename = "signed-u-boot-nodtb.bin"; > +}; > + > +&atf_blob { > + filename = "signed-bl31.bin"; > +}; > + > +&uboot_fdt_blob { > + filename = "signed-u-boot.dtb"; > +}; > + > +&kernel_blob { > + filename = "signed-Image"; > +}; > + > +&kernel_fdt_blob { > + filename = "signed-linux.dtb"; > +}; > +#endif > + > #endif > -- > 2.13.0 >
Re: [PATCH v4 3/3] sunxi: Use mkimage for SPL boot image generation
On Wed, 6 Jan 2021 at 02:25, Andre Przywara wrote: > > Switch the SPL boot image generation from using mksunxiboot to the new > sunxi_egon format of mkimage. > > Verified to create identical results for all 152 Allwinner boards. > > Signed-off-by: Andre Przywara > --- > scripts/Makefile.spl | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Simon Glass Do we need to delete the old tool?
Re: [v2 3/6] arm: socfpga: cmd: Support 'vab' command
On Thu, 7 Jan 2021 at 03:03, Siew Chin Lim wrote: > > Support 'vab' command to perform vendor authentication. > > Command format: vab addr len > Authorize 'len' bytes starting at 'addr' via vendor public key > > Signed-off-by: Siew Chin Lim > --- > arch/arm/mach-socfpga/Makefile | 1 + > arch/arm/mach-socfpga/vab.c| 37 + > 2 files changed, 38 insertions(+) > create mode 100644 arch/arm/mach-socfpga/vab.c Reviewed-by: Simon Glass This feature needs some documentation somewhere, or at least a pointer to it.
Re: [PATCH 1/9] mmc: sandbox: Add support for writing
On Thu, 31 Dec 2020 at 15:49, Sean Anderson wrote: > > This adds support writing to the sandbox mmc backed by an in-memory buffer. > The unit test has been updated to test reading, writing, and erasing. I'm > not sure what MMC erase to; I picked 0, but if it's 0xFF then that can be > easily changed. > > Signed-off-by: Sean Anderson > --- > > drivers/mmc/sandbox_mmc.c | 41 ++- > test/dm/mmc.c | 19 +- > 2 files changed, 50 insertions(+), 10 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH RFC v2 4/5] doc: signature.txt: Document devicetree format for ECDSA keys
On Wed, 30 Dec 2020 at 14:00, Alexandru Gagniuc wrote: > > Signed-off-by: Alexandru Gagniuc > --- > doc/uImage.FIT/signature.txt | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass
Re: [PATCH 6/9] fastboot: Remove mmcpart argument from raw_part_get_info_by_name
On Thu, 31 Dec 2020 at 15:49, Sean Anderson wrote: > > The only thing mmcpart was used for was to pass to blk_dselect_hwpart. > This calls blk_dselect_hwpart directly from raw_part_get_info_by_name. The > error handling is dropped, but it is reintroduced in the next commit > (albeit less specificly). > > Signed-off-by: Sean Anderson > --- > > drivers/fastboot/fb_mmc.c | 41 +-- > 1 file changed, 18 insertions(+), 23 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 1/1] doc: man-page base command
On Thu, 31 Dec 2020 at 07:46, Heinrich Schuchardt wrote: > > Provide a description of the base command. > > Signed-off-by: Heinrich Schuchardt > --- > doc/usage/base.rst | 23 +++ > doc/usage/index.rst | 1 + > 2 files changed, 24 insertions(+) > create mode 100644 doc/usage/base.rst > Reviewed-by: Simon Glass
Re: [PATCH 4/9] part: Support getting whole disk from part_get_info_by_dev_and_name_or_num
On Thu, 31 Dec 2020 at 15:49, Sean Anderson wrote: > > This adds an option to part_get_info_by_dev_and_name_or_num to allow > callers to specify whether whole-disk partitions are fine. > > Signed-off-by: Sean Anderson > --- > > cmd/ab_select.c | 3 ++- > disk/part.c | 5 +++-- > include/part.h | 6 +- > 3 files changed, 10 insertions(+), 4 deletions(-) > Reviewed-by: Simon Glass > diff --git a/cmd/ab_select.c b/cmd/ab_select.c > index 6298fcfb60..3e46663d6e 100644 > --- a/cmd/ab_select.c > +++ b/cmd/ab_select.c > @@ -22,7 +22,8 @@ static int do_ab_select(struct cmd_tbl *cmdtp, int flag, > int argc, > > /* Lookup the "misc" partition from argv[2] and argv[3] */ > if (part_get_info_by_dev_and_name_or_num(argv[2], argv[3], > -&dev_desc, &part_info) < 0) { > +&dev_desc, &part_info, > +false) < 0) { > return CMD_RET_FAILURE; > } > > diff --git a/disk/part.c b/disk/part.c > index 5e354e256f..39c6b00a59 100644 > --- a/disk/part.c > +++ b/disk/part.c > @@ -736,7 +736,8 @@ static int part_get_info_by_dev_and_name(const char > *dev_iface, > int part_get_info_by_dev_and_name_or_num(const char *dev_iface, > const char *dev_part_str, > struct blk_desc **dev_desc, > -struct disk_partition *part_info) > +struct disk_partition *part_info, > +int allow_whole_dev) bool? > { > int ret; > > @@ -750,7 +751,7 @@ int part_get_info_by_dev_and_name_or_num(const char > *dev_iface, > * directly. > */ > ret = blk_get_device_part_str(dev_iface, dev_part_str, > - dev_desc, part_info, 1); > + dev_desc, part_info, allow_whole_dev); > if (ret < 0) > printf("Couldn't find partition %s %s\n", >dev_iface, dev_part_str); > diff --git a/include/part.h b/include/part.h > index 55be724d20..778cb36199 100644 > --- a/include/part.h > +++ b/include/part.h > @@ -226,12 +226,16 @@ int part_get_info_by_name(struct blk_desc *dev_desc, > * @param[in] dev_part_str Input partition description, like "0#misc" or > "0:1" > * @param[out] dev_descPlace to store the device description pointer > * @param[out] part_info Place to store the partition information > + * @param[in] allow_whole_dev true to allow the user to select partition 0 > + * (which means the whole device), false to require a valid > + * partition number >= 1 > * @return 0 on success, or a negative on error > */ > int part_get_info_by_dev_and_name_or_num(const char *dev_iface, > const char *dev_part_str, > struct blk_desc **dev_desc, > -struct disk_partition *part_info); > +struct disk_partition *part_info, > +int allow_whole_dev); > > /** > * part_set_generic_name() - create generic partition like hda1 or sdb2 > -- > 2.25.1 > Regards, Simon
Re: [PATCH] drivers: tee: i2c trampoline driver
Hi Jorge, On Wed, 6 Jan 2021 at 10:23, Jorge Ramirez-Ortiz, Foundries wrote: > > On 29/12/20, Simon Glass wrote: > > Hi Jorge, > > > > On Tue, 29 Dec 2020 at 01:30, Jorge Ramirez-Ortiz, Foundries > > wrote: > > > > > > On 28/12/20, Simon Glass wrote: > > > > Hi Jorge, > > > > > > > > On Mon, 21 Dec 2020 at 11:15, Jorge Ramirez-Ortiz > > > > wrote: > > > > > > > > > > This commit gives the secure world access to the I2C bus so it can > > > > > communicate with I2C slaves (tipically those would be secure elements > > > > > > > > typically > > > > > > ok > > > > > > > > > > > > like the NXP SE050). > > > > > > > > > > Tested on imx8mmevk. > > > > > > > > We don't seem to have any optee tests in U-Boot at present. I vaguely > > > > recall they were coming at some point. I think we need: > > > > > > > > - a sandbox fake drive for optee, that understands and responds to the > > > > 6 uclass calls at a basic level > > > > - an update to get_invoke_func() that provides a sandbox function too > > > > > > > > Then we should be able to run optee tests in CI. > > > > > > > > It is not a lot of work, but I don't think we should add to optee > > > > until this is resolved. > > > > > > um, ok but shouldnt this infrastructure better rest on a maintainer's > > > roadmap rather than on an off-the-blue request? I mean, had I known I > > > could have done it in parallel but now I'll need to find the time to > > > do this. > > > > We always need tests in U-Boot, so if you are not writing a test it > > would be a good question to ask as to how you can do that. Actually > > patman sometimes warns about that, but perhaps only in certain > > situations. > > > > Actually I see that there is a test - it is hidden under the generic > > unit tests so I didn't see it. See dm/test/tee.c > > > > I'll make some comments on the patch. > > > > > > > > also notice that Linux's equivalent patchset was merged back in the > > > summer (ie, this is not untested code). > > > > > > https://lkml.org/lkml/2020/8/12/276 > > > > I don't see any tests in that patch though...are they somewhere > else? > > hey > > no you are right, I didnt post any tests to linux. And the more I > think about it the less convinced I am that we needed one. > > This commit is nothing more than a RPC TEE service that shares a > buffer with U-boot and then sends requests to an I2C chip. > > > Or do you justmean people have been running similar code? > > I am not sure many people is using it yet other than a number of our > customers at Foundries.io across different projects. But the number of > users is at a steady growth (everybody seems to need this > functionality). > > In our use case this trampoline code is used to access the NXP SE050 > from OP-TEE over I2C via: > https://github.com/foundriesio/plug-and-trust > > (from that link there is access to a lot of information if you are > interested) > > But of course, it can be used by OP-TEE to access (or even just probe) > any chip. > > > If so, > > that's fair enough but it doesn't really help us much. Lots of people > > test code manually before submitting patches, at least for their use > > case, but this is an open-source project. Over time people want to > > change and expand the code, and it is very hard for them to do that if > > there are no automated tests. > > Right. And I dont disagree (everything should be testable) > jfyi I maintain a couple of platforms here so I am familiar with this > project (been using it on/off for a couple of decades already...umm > time flies, seems like yesterday). > > But I dont think we need a test that verifies this service since > this is just an amalgamation of two other functions that should be > tested somewhere else (ie TEE RPC and I2C transfers). > > IMO, if they dont exist already, u-boot would benefit from: > - a test that verifies TEE RPC Do we have that? If not, we need it. > - a test that verifies TEE buffer sharing with U-BOOT I'm not sure what that is, but if there is code in U-Boot it should be tested. > - a test that verifies I2C reading/writing This is test/dm/i2c.c > > But not so much from > - a test that verifies TEE I2C trampoline service I suppose with the above this is trivial to write? Often tests are just a few lines of code but they provide coverage for the feature. > > I am going to repost the patch addressing some of your concerns (I > found some other issues) and if you still think that having a test > will be convenient sure we can go ahead and work on it. OK ta. Regards, Simon
Re: [PATCH] lib: cosmetic update of CONFIG_LIB_ELF description
On Mon, 4 Jan 2021 at 07:33, Patrick Delaunay wrote: > > Change 2 typo error in CONFIG_LIB_ELF description: > - Supoort => Support > - fir => for > > Signed-off-by: Patrick Delaunay > --- > > lib/Kconfig | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Reviewed-by: Simon Glass
Re: [PATCH 8/9] fastboot: Allow u-boot-style partitions
On Mon, 4 Jan 2021 at 09:53, Sean Anderson wrote: > > > > On 12/31/20 5:48 PM, Sean Anderson wrote: > > This adds support for partitions of the form "dev.hwpart:part" and > > "dev#partname". This allows one to flash to eMMC boot partitions without > > having to use CONFIG_FASTBOOT_MMC_BOOT1_SUPPORT. It also allows one to > > flash to an entire device without needing CONFIG_FASTBOOT_MMC_USER_NAME. > > Lastly, one can also flash MMC devices other than > > CONFIG_FASTBOOT_FLASH_MMC_DEV. > > > > Because devices can be specified explicitly, > CONFIG_FASTBOOT_FLASH_MMC_DEV > > is used only when necessary for existing functionality. For those cases, > > fastboot_mmc_get_dev has been added as a helper function. This allows > > > > There should be no conflicts with the existing system, but just in > case, I > > have ordered detection of these names after all existing names. > > > > The fastboot_mmc_part test has been updated for these new names. > > > > Signed-off-by: Sean Anderson > > --- > > > > drivers/fastboot/fb_mmc.c | 150 +++--- > > test/dm/fastboot.c| 37 +- > > 2 files changed, 127 insertions(+), 60 deletions(-) > > > > diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c > > index b0610d3151..a52b1e3ed6 100644 > > --- a/drivers/fastboot/fb_mmc.c > > +++ b/drivers/fastboot/fb_mmc.c > > @@ -37,6 +37,7 @@ static int raw_part_get_info_by_name(struct > blk_desc *dev_desc, > > char *raw_part_desc; > > const char *argv[2]; > > const char **parg = argv; > > +int ret; > > > > /* check for raw partition descriptor */ > > strcpy(env_desc_name, "fastboot_raw_partition_"); > > @@ -60,7 +61,7 @@ static int raw_part_get_info_by_name(struct > blk_desc *dev_desc, > > > > info->start = simple_strtoul(argv[0], NULL, 0); > > info->size = simple_strtoul(argv[1], NULL, 0); > > -info->blksz = dev_desc->blksz; > > +info->blksz = *dev_desc->blksz; > > strncpy((char *)info->name, name, PART_NAME_LEN); > > Looks like this slipped through while rebasing. The above two hunks > shouldn't have been included; will be fixed in v2. Reviewed-by: Simon Glass
Re: [PATCH v4 2/3] tools: mkimage: Add Allwinner eGON support
On Wed, 6 Jan 2021 at 02:25, Andre Przywara wrote: > > So far we used the separate mksunxiboot tool for generating a bootable > image for Allwinner SPLs, probably just for historical reasons. > > Use the mkimage framework to generate a so called eGON image the > Allwinner BROM expects. > The new image type is called "sunxi_egon", to differentiate it > from the (still to be implemented) secure boot TOC0 image. > > Signed-off-by: Andre Przywara > --- > common/image.c| 1 + > include/image.h | 1 + > include/sunxi_image.h | 1 + > tools/Makefile| 1 + > tools/sunxi_egon.c| 136 ++ > 5 files changed, 140 insertions(+) > create mode 100644 tools/sunxi_egon.c > Reviewed-by: Simon Glass
Re: [PATCH RFC v2 1/5] lib: Rename rsa-checksum.c to hash-checksum.c
Hi Alexandru, On Wed, 30 Dec 2020 at 14:00, Alexandru Gagniuc wrote: > > rsa-checksum.c sontains the hash_calculate() implementations. Despite > the "rsa-" file prefix, this function is useful for other algorithms. > > To prevent confusion, move this file to lib/crypto, and rename it to > hash-checksum.c, to give it a more "generic" feel. It looks like it is moving to lib/ ? > > Signed-off-by: Alexandru Gagniuc > --- > common/image-fit-sig.c | 2 +- > common/image-sig.c | 2 +- > include/image.h| 2 +- > include/u-boot/{rsa-checksum.h => hash-checksum.h} | 0 > lib/Makefile | 1 + > lib/crypto/pkcs7_verify.c | 2 +- > lib/crypto/x509_public_key.c | 2 +- > lib/{rsa/rsa-checksum.c => hash-checksum.c}| 3 ++- > lib/rsa/Makefile | 2 +- > tools/Makefile | 3 ++- > 10 files changed, 11 insertions(+), 8 deletions(-) > rename include/u-boot/{rsa-checksum.h => hash-checksum.h} (100%) > rename lib/{rsa/rsa-checksum.c => hash-checksum.c} (96%) Reviewed-by: Simon Glass
Re: [PATCH V3] dm: core: Add late driver remove option
On 12/10/20 6:44 PM, Simon Glass wrote: [...] Note also the semantics of what is going on here. The idea of the existing OS_PREPARE and ACTIVE_DMA flags is that the default for device_remove() is to remove everything, but if you provide a flag then it just removes those things. Your flag is the opposite to that, which is why you are changing so much code. I obviously cannot remove everything, see the example above. Do you understand what I am saying about inverting the flag? No, please elaborate. The normal situation should be to remove everything. Removing only non-late things (which I want to call 'basic', or something like that) should be an option, like we have DM_REMOVE_OS_PREPARE. We cannot remove everything at once, because then various real hardware cannot work properly. So there needs to be some way to remove e.g. the clock drivers later. Marking a couple of drivers as "remove-late" is much less intrusive than marking most drivers as "do-not-remove-early". How do you propose this "inverted" remove flag would look like ? [...]
Re: [PATCH V3] dm: core: Add late driver remove option
Hi Marek, On Thu, 7 Jan 2021 at 05:42, Marek Vasut wrote: > > On 12/10/20 6:44 PM, Simon Glass wrote: > > [...] > > > Note also the semantics of what is going on here. The idea of the > > existing OS_PREPARE and ACTIVE_DMA flags is that the default for > > device_remove() is to remove everything, but if you provide a flag > > then it just removes those things. Your flag is the opposite to that, > > which is why you are changing so much code. > > I obviously cannot remove everything, see the example above. > >>> > >>> Do you understand what I am saying about inverting the flag? > >> > >> No, please elaborate. > > > > The normal situation should be to remove everything. Removing only > > non-late things (which I want to call 'basic', or something like that) > > should be an option, like we have DM_REMOVE_OS_PREPARE. > > We cannot remove everything at once, because then various real hardware > cannot work properly. So there needs to be some way to remove e.g. the > clock drivers later. Marking a couple of drivers as "remove-late" is > much less intrusive than marking most drivers as "do-not-remove-early". I'm not suggesting marking drivers as 'do not remove early'. I am referring to the parameter of the device_remove() function. Mostly what I am asking for is better naming, and (I hope) a simpler implementation. > > How do you propose this "inverted" remove flag would look like ? Similar to the existing flags. See for example DM_FLAG_ACTIVE_DMA. I have this on my list to look at more closely and should do so in the next week. Regards, Simon
[PATCH 1/2] video: seps525: Align driver with latest treewide changes
Some structures/functions have been renamed which are not reflected in this driver. Commit 8a8d24bdf174 ("dm: treewide: Rename ..._platdata variables to just ..._plat"), commit c69cda25c9b5 ("dm: treewide: Rename dev_get_platdata() to dev_get_plat()"), commit caa4daa2ae3d ("dm: treewide: Rename 'platdata' variables to just 'plat'") and commit 41575d8e4c33 ("dm: treewide: Rename auto_alloc_size members to be shorter") are describing these changes. Signed-off-by: Michal Simek --- Driver was developed based on master branch and next got more changes. --- drivers/video/seps525.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/video/seps525.c b/drivers/video/seps525.c index 369e5e6afc87..74c8721e1e11 100644 --- a/drivers/video/seps525.c +++ b/drivers/video/seps525.c @@ -299,7 +299,7 @@ static int seps525_probe(struct udevice *dev) static int seps525_bind(struct udevice *dev) { - struct video_uc_platdata *plat = dev_get_uclass_platdata(dev); + struct video_uc_plat *plat = dev_get_uclass_plat(dev); plat->size = WIDTH * HEIGHT * 16; @@ -320,8 +320,8 @@ U_BOOT_DRIVER(seps525_video) = { .id = UCLASS_VIDEO, .of_match = seps525_ids, .ops = &seps525_ops, - .platdata_auto_alloc_size = sizeof(struct video_uc_platdata), + .plat_auto = sizeof(struct video_uc_plat), .bind = seps525_bind, .probe = seps525_probe, - .priv_auto_alloc_size = sizeof(struct seps525_priv), + .priv_auto = sizeof(struct seps525_priv), }; -- 2.30.0
[PATCH 2/2] arm64: zynqmp: Enable seps and related video configs
Enable this driver to get build and probe for our platform. Signed-off-by: Michal Simek --- configs/xilinx_zynqmp_virt_defconfig | 10 ++ 1 file changed, 10 insertions(+) diff --git a/configs/xilinx_zynqmp_virt_defconfig b/configs/xilinx_zynqmp_virt_defconfig index e400cd4702ee..0bf4b7d692e9 100644 --- a/configs/xilinx_zynqmp_virt_defconfig +++ b/configs/xilinx_zynqmp_virt_defconfig @@ -54,6 +54,7 @@ CONFIG_CMD_SPI=y CONFIG_CMD_USB=y CONFIG_CMD_USB_MASS_STORAGE=y CONFIG_CMD_TFTPPUT=y +CONFIG_CMD_BMP=y CONFIG_CMD_CACHE=y CONFIG_CMD_TIME=y CONFIG_CMD_TIMER=y @@ -155,6 +156,15 @@ CONFIG_USB_ETHER=y CONFIG_USB_ETH_CDC=y CONFIG_USB_HOST_ETHER=y CONFIG_USB_ETHER_ASIX=y +CONFIG_DM_VIDEO=y +CONFIG_VIDEO_COPY=y +CONFIG_DISPLAY=y +CONFIG_VIDEO_SEPS525=y +CONFIG_LCD=y +CONFIG_SPLASH_SCREEN=y +CONFIG_BMP_16BPP=y +CONFIG_BMP_24BPP=y +CONFIG_BMP_32BPP=y CONFIG_WDT=y CONFIG_WDT_CDNS=y CONFIG_PANIC_HANG=y -- 2.30.0
Re: [PATCH v4 3/3] sunxi: Use mkimage for SPL boot image generation
On 07/01/2021 12:36, Simon Glass wrote: Hi Simon, thanks for the review! > On Wed, 6 Jan 2021 at 02:25, Andre Przywara wrote: >> >> Switch the SPL boot image generation from using mksunxiboot to the new >> sunxi_egon format of mkimage. >> >> Verified to create identical results for all 152 Allwinner boards. >> >> Signed-off-by: Andre Przywara >> --- >> scripts/Makefile.spl | 8 >> 1 file changed, 4 insertions(+), 4 deletions(-) > > Reviewed-by: Simon Glass > > Do we need to delete the old tool? I would rather not quite do this yet, instead leave it around for a bit. There is for instance sunxi-tools, that relies on this, and it's also packaged as part of Debian's u-boot-tools package. We surely will transition them over, but this might take a while. Cheers, Andre
Re: [PATCH v3 1/2] ARM: dts: sunxi: h6: Update DT files
On 06/01/2021 17:02, Jernej Skrabec wrote: > Updated H6 DT files are based on Linux 5.11-rc1 release. > > Signed-off-by: Jernej Skrabec Reviewed-by: Andre Przywara Two things to note: - This relies on the U-Boot patch to sun8i-emac to recognise rgmii-id and friends, otherwise Ethernet won't work. The patch should ideally be merged before those ones. - This decreases DT compatibilty with older kernels, which don't know about the H6 RTC (introduced in Linux v5.4). The IOSC fixed clock got replaced by <&rtc 2>, so this won't be available everywhere. Cheers, Andre > --- > arch/arm/dts/sun50i-h6-beelink-gs1.dts | 70 +++- > arch/arm/dts/sun50i-h6-cpu-opp.dtsi | 117 ++ > arch/arm/dts/sun50i-h6-orangepi-lite2.dts| 71 +++- > arch/arm/dts/sun50i-h6-orangepi-one-plus.dts | 41 +- > arch/arm/dts/sun50i-h6-orangepi.dtsi | 72 +++- > arch/arm/dts/sun50i-h6-pine-h64.dts | 102 +++-- > arch/arm/dts/sun50i-h6.dtsi | 394 +-- > 7 files changed, 794 insertions(+), 73 deletions(-) > create mode 100644 arch/arm/dts/sun50i-h6-cpu-opp.dtsi > > diff --git a/arch/arm/dts/sun50i-h6-beelink-gs1.dts > b/arch/arm/dts/sun50i-h6-beelink-gs1.dts > index 0dc33c90dd60..7c9dbde645b5 100644 > --- a/arch/arm/dts/sun50i-h6-beelink-gs1.dts > +++ b/arch/arm/dts/sun50i-h6-beelink-gs1.dts > @@ -1,11 +1,10 @@ > -// SPDX-License-Identifier: (GPL-2.0+ or MIT) > -/* > - * Copyright (C) 2019 Clément Péron > - */ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +// Copyright (C) 2019 Clément Péron > > /dts-v1/; > > #include "sun50i-h6.dtsi" > +#include "sun50i-h6-cpu-opp.dtsi" > > #include > > @@ -25,6 +24,7 @@ > connector { > compatible = "hdmi-connector"; > type = "a"; > + ddc-en-gpios = <&pio 7 2 GPIO_ACTIVE_HIGH>; /* PH2 */ > > port { > hdmi_con_in: endpoint { > @@ -33,6 +33,13 @@ > }; > }; > > + ext_osc32k: ext_osc32k_clk { > + #clock-cells = <0>; > + compatible = "fixed-clock"; > + clock-frequency = <32768>; > + clock-output-names = "ext_osc32k"; > + }; > + > leds { > compatible = "gpio-leds"; > > @@ -51,12 +58,38 @@ > regulator-max-microvolt = <500>; > regulator-always-on; > }; > + > + sound-spdif { > + compatible = "simple-audio-card"; > + simple-audio-card,name = "sun50i-h6-spdif"; > + > + simple-audio-card,cpu { > + sound-dai = <&spdif>; > + }; > + > + simple-audio-card,codec { > + sound-dai = <&spdif_out>; > + }; > + }; > + > + spdif_out: spdif-out { > + #sound-dai-cells = <0>; > + compatible = "linux,spdif-dit"; > + }; > +}; > + > +&cpu0 { > + cpu-supply = <®_dcdca>; > }; > > &de { > status = "okay"; > }; > > +&dwc3 { > + status = "okay"; > +}; > + > &ehci0 { > status = "okay"; > }; > @@ -64,12 +97,17 @@ > &emac { > pinctrl-names = "default"; > pinctrl-0 = <&ext_rgmii_pins>; > - phy-mode = "rgmii"; > + phy-mode = "rgmii-id"; > phy-handle = <&ext_rgmii_phy>; > phy-supply = <®_aldo2>; > status = "okay"; > }; > > +&gpu { > + mali-supply = <®_dcdcc>; > + status = "okay"; > +}; > + > &hdmi { > status = "okay"; > }; > @@ -201,13 +239,16 @@ > reg_dcdca: dcdca { > regulator-always-on; > regulator-min-microvolt = <81>; > - regulator-max-microvolt = <108>; > + regulator-max-microvolt = <116>; > + regulator-ramp-delay = <2500>; > regulator-name = "vdd-cpu"; > }; > > reg_dcdcc: dcdcc { > + regulator-enable-ramp-delay = <32000>; > regulator-min-microvolt = <81>; > regulator-max-microvolt = <108>; > + regulator-ramp-delay = <2500>; > regulator-name = "vdd-gpu"; > }; > > @@ -232,6 +273,11 @@ > }; > }; > > +&r_ir { > + linux,rc-map-name = "rc-beelink-gs1"; > + status = "okay"; > +}; > + > &r_pio { > /* >* PL0 and PL1 are used for PMIC I2C > @@ -243,6 +289,14 @@ > vcc-pm-supply = <®_aldo1>; > }; > > +&rtc { > + clocks = <&ext_osc32k>; > +}; > + > +&spdif { > + status = "okay"; > +}; > + > &uart0 { > pinctrl-names = "default"; > pinctrl-0 = <&uart0_ph_pins>; > @@ -258,3 +312,7 @@ > usb0_vbus-supply = <®_vcc5v>; > status = "okay"; > }; > + > +&usb3phy { > + status = "okay"; > +}; > diff --git a/arch/a
Re: [PATCH v3 2/2] sunxi: Add support for Tanix TX6
On 06/01/2021 17:02, Jernej Skrabec wrote: > This commit adds support for Tanix TX6 TV box, based on H6. It's low end > H6 board, with 3 GiB of RAM, eMMC, fast ethernet, USB, IR and other > peripherals. > > DT file is taken from Linux 5.11-rc1 release. > > Signed-off-by: Jernej Skrabec Reviewed-by: Andre Przywara Thanks! Andre > --- > arch/arm/dts/Makefile| 3 +- > arch/arm/dts/sun50i-h6-tanix-tx6.dts | 124 +++ > board/sunxi/MAINTAINERS | 6 ++ > configs/tanix_tx6_defconfig | 10 +++ > 4 files changed, 142 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/dts/sun50i-h6-tanix-tx6.dts > create mode 100644 configs/tanix_tx6_defconfig > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > index fd47e408f826..e00aed1ec207 100644 > --- a/arch/arm/dts/Makefile > +++ b/arch/arm/dts/Makefile > @@ -607,7 +607,8 @@ dtb-$(CONFIG_MACH_SUN50I_H6) += \ > sun50i-h6-beelink-gs1.dtb \ > sun50i-h6-orangepi-lite2.dtb \ > sun50i-h6-orangepi-one-plus.dtb \ > - sun50i-h6-pine-h64.dtb > + sun50i-h6-pine-h64.dtb \ > + sun50i-h6-tanix-tx6.dtb > dtb-$(CONFIG_MACH_SUN50I) += \ > sun50i-a64-amarula-relic.dtb \ > sun50i-a64-bananapi-m64.dtb \ > diff --git a/arch/arm/dts/sun50i-h6-tanix-tx6.dts > b/arch/arm/dts/sun50i-h6-tanix-tx6.dts > new file mode 100644 > index ..be81330db14f > --- /dev/null > +++ b/arch/arm/dts/sun50i-h6-tanix-tx6.dts > @@ -0,0 +1,124 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +// Copyright (c) 2019 Jernej Skrabec > + > +/dts-v1/; > + > +#include "sun50i-h6.dtsi" > +#include "sun50i-h6-cpu-opp.dtsi" > + > +#include > + > +/ { > + model = "Tanix TX6"; > + compatible = "oranth,tanix-tx6", "allwinner,sun50i-h6"; > + > + aliases { > + serial0 = &uart0; > + }; > + > + chosen { > + stdout-path = "serial0:115200n8"; > + }; > + > + connector { > + compatible = "hdmi-connector"; > + ddc-en-gpios = <&pio 7 2 GPIO_ACTIVE_HIGH>; /* PH2 */ > + type = "a"; > + > + port { > + hdmi_con_in: endpoint { > + remote-endpoint = <&hdmi_out_con>; > + }; > + }; > + }; > + > + reg_vcc3v3: vcc3v3 { > + compatible = "regulator-fixed"; > + regulator-name = "vcc3v3"; > + regulator-min-microvolt = <330>; > + regulator-max-microvolt = <330>; > + }; > + > + reg_vdd_cpu_gpu: vdd-cpu-gpu { > + compatible = "regulator-fixed"; > + regulator-name = "vdd-cpu-gpu"; > + regulator-min-microvolt = <1135000>; > + regulator-max-microvolt = <1135000>; > + }; > +}; > + > +&cpu0 { > + cpu-supply = <®_vdd_cpu_gpu>; > +}; > + > +&de { > + status = "okay"; > +}; > + > +&dwc3 { > + status = "okay"; > +}; > + > +&ehci0 { > + status = "okay"; > +}; > + > +&ehci3 { > + status = "okay"; > +}; > + > +&gpu { > + mali-supply = <®_vdd_cpu_gpu>; > + status = "okay"; > +}; > + > +&hdmi { > + status = "okay"; > +}; > + > +&hdmi_out { > + hdmi_out_con: endpoint { > + remote-endpoint = <&hdmi_con_in>; > + }; > +}; > + > +&mmc0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&mmc0_pins>; > + vmmc-supply = <®_vcc3v3>; > + cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; > + bus-width = <4>; > + status = "okay"; > +}; > + > +&ohci0 { > + status = "okay"; > +}; > + > +&ohci3 { > + status = "okay"; > +}; > + > +&r_ir { > + linux,rc-map-name = "rc-tanix-tx5max"; > + status = "okay"; > +}; > + > +&uart0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&uart0_ph_pins>; > + status = "okay"; > +}; > + > +&usb2otg { > + dr_mode = "host"; > + status = "okay"; > +}; > + > +&usb2phy { > + status = "okay"; > +}; > + > +&usb3phy { > + status = "okay"; > +}; > diff --git a/board/sunxi/MAINTAINERS b/board/sunxi/MAINTAINERS > index d3755ae41a9d..1b37a9899edd 100644 > --- a/board/sunxi/MAINTAINERS > +++ b/board/sunxi/MAINTAINERS > @@ -489,6 +489,12 @@ S: Maintained > F: configs/Sunchip_CX-A99_defconfig > W: https://linux-sunxi.org/Sunchip_CX-A99 > > +TANIX TX6 BOARD > +M: Jernej Skrabec > +S: Maintained > +F: configs/tanix_tx6_defconfig > +W: https://linux-sunxi.org/Tanix_TX6 > + > TBS A711 BOARD > M: Maxime Ripard > S: Maintained > diff --git a/configs/tanix_tx6_defconfig b/configs/tanix_tx6_defconfig > new file mode 100644 > index ..9ce812ecc35d > --- /dev/null > +++ b/configs/tanix_tx6_defconfig > @@ -0,0 +1,10 @@ > +CONFIG_ARM=y > +CONFIG_ARCH_SUNXI=y > +CONFIG_SPL=y > +CONFIG_MACH_SUN50I_H6=y > +CONFIG_SUNXI_DRAM_H6_DDR3_1333=y > +CONFIG_DRAM_CLK=648 > +CONFIG_MMC0_CD_PIN="PF6" > +CONFIG_MMC_SUNXI_SLOT_EXTRA=2 > +CONFIG_DEFAULT_DEVICE_TREE="sun50i-h6-tanix-tx6" > +# CONFIG_SYS_M
Re: [GIT PULL] TI changes for v2021.04 next
On Thu, Jan 07, 2021 at 11:20:41AM +0530, Lokesh Vutla wrote: > Hi Tom, > Please find the PR for next branch targeted for v2021.04 release. > Details about the PR are updated in the tag message. > > Gitlab build report: > https://gitlab.denx.de/u-boot/custodians/u-boot-ti/-/pipelines/5778 > > The following changes since commit c15f44acf9d473f4682bfdc63b8aebd313492b15: > > Prepare v2021.01-rc4 (2020-12-21 15:03:24 -0500) > > are available in the Git repository at: > > https://gitlab.denx.de/u-boot/custodians/u-boot-ti.git tags/ti-v2021.04-next The PR doesn't apply cleanly now in a few cases, can you take a look at drivers/video/ti/am335x-fb.c and test/dm/test-fdt.c? > for you to fetch changes up to feff9fd0d893067bdc2e08969b5c916d10e9fc12: > > remoteproc: ti_k3_arm64: Program CNTFID0 register in GTC (2021-01-07 > 09:55:34 +0530) [snip] > Pali Rohár (3): > Nokia RX-51: Convert to CONFIG_DM_MMC This is also bringing up a conflict, and I think part of the real problem is that we shouldn't be having a single U_BOOT_DRVINFOS (formerly U_BOOT_DEVICES) but one per peripheral. It may work but it's not intentional (adding Simon in case he wants to correct me here). Thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 4/9] part: Support getting whole disk from part_get_info_by_dev_and_name_or_num
On 1/7/21 7:35 AM, Simon Glass wrote: > On Thu, 31 Dec 2020 at 15:49, Sean Anderson wrote: >> >> This adds an option to part_get_info_by_dev_and_name_or_num to allow >> callers to specify whether whole-disk partitions are fine. >> >> Signed-off-by: Sean Anderson >> --- >> >> cmd/ab_select.c | 3 ++- >> disk/part.c | 5 +++-- >> include/part.h | 6 +- >> 3 files changed, 10 insertions(+), 4 deletions(-) >> > > Reviewed-by: Simon Glass > >> diff --git a/cmd/ab_select.c b/cmd/ab_select.c >> index 6298fcfb60..3e46663d6e 100644 >> --- a/cmd/ab_select.c >> +++ b/cmd/ab_select.c >> @@ -22,7 +22,8 @@ static int do_ab_select(struct cmd_tbl *cmdtp, int flag, int argc, >> >> /* Lookup the "misc" partition from argv[2] and argv[3] */ >> if (part_get_info_by_dev_and_name_or_num(argv[2], argv[3], >> -&dev_desc, &part_info) < 0) { >> +&dev_desc, &part_info, >> +false) < 0) { >> return CMD_RET_FAILURE; >> } >> >> diff --git a/disk/part.c b/disk/part.c >> index 5e354e256f..39c6b00a59 100644 >> --- a/disk/part.c >> +++ b/disk/part.c >> @@ -736,7 +736,8 @@ static int part_get_info_by_dev_and_name(const char *dev_iface, >> int part_get_info_by_dev_and_name_or_num(const char *dev_iface, >> const char *dev_part_str, >> struct blk_desc **dev_desc, >> -struct disk_partition *part_info) >> +struct disk_partition *part_info, >> +int allow_whole_dev) > > bool? It's int to match the allow_whole_dev argument of blk_get_device_part_str. > >> { >> int ret; >> >> @@ -750,7 +751,7 @@ int part_get_info_by_dev_and_name_or_num(const char *dev_iface, >> * directly. >> */ >> ret = blk_get_device_part_str(dev_iface, dev_part_str, >> - dev_desc, part_info, 1); >> + dev_desc, part_info, allow_whole_dev); >> if (ret < 0) >> printf("Couldn't find partition %s %s\n", >> dev_iface, dev_part_str); >> diff --git a/include/part.h b/include/part.h >> index 55be724d20..778cb36199 100644 >> --- a/include/part.h >> +++ b/include/part.h >> @@ -226,12 +226,16 @@ int part_get_info_by_name(struct blk_desc *dev_desc, >>* @param[in] dev_part_str Input partition description, like "0#misc" or "0:1" >>* @param[out] dev_descPlace to store the device description pointer >>* @param[out] part_info Place to store the partition information >> + * @param[in] allow_whole_dev true to allow the user to select partition 0 >> + * (which means the whole device), false to require a valid >> + * partition number >= 1 >>* @return 0 on success, or a negative on error >>*/ >> int part_get_info_by_dev_and_name_or_num(const char *dev_iface, >> const char *dev_part_str, >> struct blk_desc **dev_desc, >> -struct disk_partition *part_info); >> +struct disk_partition *part_info, >> +int allow_whole_dev); >> >> /** >>* part_set_generic_name() - create generic partition like hda1 or sdb2 >> -- >> 2.25.1 >> > > Regards, > Simon >
RE: [v2 4/6] arm: socfpga: dts: soc64: Update filename in binman node of FIT image with VAB support
Hi Simon, > -Original Message- > From: Simon Glass > Sent: Thursday, January 7, 2021 8:37 PM > To: Lim, Elly Siew Chin > Cc: U-Boot Mailing List ; Marek Vasut > ; Tan, Ley Foon ; See, Chin Liang > ; Simon Goldschmidt > ; Chee, Tien Fong > ; Westergreen, Dalon > ; Gan, Yau Wai > Subject: Re: [v2 4/6] arm: socfpga: dts: soc64: Update filename in binman node > of FIT image with VAB support > > On Thu, 7 Jan 2021 at 03:03, Siew Chin Lim > wrote: > > > > FIT image of Vendor Authentication Coot (VAB) contains signed images. > > > > Signed-off-by: Siew Chin Lim > > --- > > arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi | 22 > > ++ > > 1 file changed, 22 insertions(+) > > > > I'm not quite sure what is happening here, but consider using two separate > files > rather than what looks like a patch over an existing one. > There are two boot flow will use binman (socfpga_soc64_fit-u-boot.dtsi) to generate u-boot.fit and kernel.fit: 1. socfpga_agilex_atf_defconfig (boot via ATF) 2. socfpga_agilex_vab_defconfig (boot via ATF with VAB enabled, support authentication on bl31, u-boot, Linux images) The binman node settings are the same for both flows. With VAB enabled, all inputs file need to be signed before generate FIT image. We would like to use different input file name to remind user that they need to sign all bl31, u-boot, Linux images when using binman to generate FIT image. Due to the binman node settings are identical and only the file name need to be different, so we prefer to share the same socfpga_soc64_fit-u-boot.dtsi for both flows. Thanks, Siew Chin > > > > diff --git a/arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi > > b/arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi > > index cf365590a8..4b30473743 100644 > > --- a/arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi > > +++ b/arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi > > @@ -117,4 +117,26 @@ > > }; > > }; > > > > +#if defined(CONFIG_SOCFPGA_SECURE_VAB_AUTH) > > +&uboot_blob { > > + filename = "signed-u-boot-nodtb.bin"; }; > > + > > +&atf_blob { > > + filename = "signed-bl31.bin"; > > +}; > > + > > +&uboot_fdt_blob { > > + filename = "signed-u-boot.dtb"; }; > > + > > +&kernel_blob { > > + filename = "signed-Image"; > > +}; > > + > > +&kernel_fdt_blob { > > + filename = "signed-linux.dtb"; }; #endif > > + > > #endif > > -- > > 2.13.0 > >
Re: [PATCH 5/9] part: Support string block devices in part_get_info_by_dev_and_name
On 1/7/21 7:35 AM, Simon Glass wrote: On Thu, 31 Dec 2020 at 15:49, Sean Anderson wrote: This adds support for things like "#partname" and "0.1#partname". The block device parsing is done like in blk_get_device_part_str. Signed-off-by: Sean Anderson --- disk/part.c | 32 +--- 1 file changed, 17 insertions(+), 15 deletions(-) Reviewed-by: Simon Glass Does the function comment need updating? Yes. Will be updated. --Sean diff --git a/disk/part.c b/disk/part.c index 39c6b00a59..e07dd67fd9 100644 --- a/disk/part.c +++ b/disk/part.c @@ -707,29 +707,31 @@ static int part_get_info_by_dev_and_name(const char *dev_iface, struct blk_desc **dev_desc, struct disk_partition *part_info) { - char *ep; - const char *part_str; - int dev_num, ret; + char *dup_str = NULL; + const char *dev_str, *part_str; + int ret; + /* Separate device and partition name specification */ part_str = strchr(dev_part_str, '#'); - if (!part_str || part_str == dev_part_str) - return -EINVAL; - - dev_num = simple_strtoul(dev_part_str, &ep, 16); - if (ep != part_str) { - /* Not all the first part before the # was parsed. */ + if (part_str) { + dup_str = strdup(dev_part_str); + dup_str[part_str - dev_part_str] = 0; + dev_str = dup_str; + part_str++; + } else { return -EINVAL; } - part_str++; - *dev_desc = blk_get_dev(dev_iface, dev_num); - if (!*dev_desc) { - printf("Could not find %s %d\n", dev_iface, dev_num); - return -ENODEV; - } + ret = blk_get_device_by_str(dev_iface, dev_str, dev_desc); + if (ret) + goto cleanup; + ret = part_get_info_by_name(*dev_desc, part_str, part_info); if (ret < 0) printf("Could not find \"%s\" partition\n", part_str); + +cleanup: + free(dup_str); return ret; } -- 2.25.1
Re: [PATCH 22/26] dm: core: Access device ofnode through functions
Hi Patrick, On Mon, 4 Jan 2021 at 06:02, Patrick DELAUNAY wrote: > > Hi Simon, > > > On 12/19/20 6:40 PM, Simon Glass wrote: > > At present ofnode is present in the device even if it is never used. With > > of-platdata this field is not used, so can be removed. In preparation for > > this, change the access to go through inline functions. > > > > Signed-off-by: Simon Glass > > --- > > > > arch/arm/mach-stm32mp/pwr_regulator.c | 2 +- > > board/synopsys/hsdk/clk-lib.c | 2 +- > > drivers/ata/mtk_ahci.c| 3 ++- > > drivers/clk/meson/axg.c | 2 +- > > drivers/clk/meson/g12a.c | 2 +- > > drivers/clk/meson/gxbb.c | 2 +- > > drivers/core/device.c | 2 +- > > drivers/core/root.c | 2 +- > > drivers/gpio/mpc8xxx_gpio.c | 4 ++-- > > drivers/gpio/octeon_gpio.c| 2 +- > > drivers/misc/swap_case.c | 2 +- > > drivers/mmc/octeontx_hsmmc.c | 23 +-- > > drivers/mtd/nand/raw/octeontx_nand.c | 2 +- > > drivers/mtd/nand/spi/core.c | 2 +- > > drivers/net/fm/eth.c | 4 ++-- > > drivers/net/fsl_enetc.c | 8 > > drivers/net/fsl_enetc_mdio.c | 2 +- > > drivers/net/mdio-ipq4019.c| 4 ++-- > > drivers/net/mdio_mux_i2creg.c | 2 +- > > drivers/net/mvmdio.c | 4 ++-- > > drivers/net/octeontx/smi.c| 2 +- > > drivers/net/tsec.c| 3 ++- > > drivers/phy/phy-ti-am654.c| 2 +- > > drivers/power/domain/meson-ee-pwrc.c | 4 ++-- > > drivers/power/domain/meson-gx-pwrc-vpu.c | 4 ++-- > > drivers/power/regulator/pbias_regulator.c | 3 ++- > > drivers/pwm/pwm-meson.c | 9 ++--- > > drivers/reset/reset-socfpga.c | 2 +- > > drivers/spi/fsl_dspi.c| 6 -- > > drivers/tee/optee/core.c | 2 +- > > drivers/usb/cdns3/core.c | 4 ++-- > > drivers/usb/dwc3/core.c | 2 +- > > drivers/usb/dwc3/dwc3-generic.c | 6 +++--- > > drivers/usb/dwc3/dwc3-meson-g12a.c| 2 +- > > drivers/usb/dwc3/dwc3-meson-gxl.c | 2 +- > > drivers/usb/gadget/dwc2_udc_otg.c | 4 ++-- > > drivers/usb/host/dwc3-octeon-glue.c | 2 +- > > drivers/usb/host/dwc3-sti-glue.c | 5 +++-- > > drivers/usb/host/ehci-mx6.c | 2 +- > > drivers/usb/host/xhci-dwc3.c | 2 +- > > drivers/usb/mtu3/mtu3_core.c | 2 +- > > drivers/usb/mtu3/mtu3_plat.c | 4 ++-- > > drivers/usb/musb-new/ti-musb.c| 2 +- > > drivers/video/nexell_display.c| 2 +- > > drivers/video/rockchip/rk_mipi.c | 2 +- > > include/dm/device.h | 23 +-- > > include/dm/read.h | 2 +- > > include/linux/mtd/mtd.h | 4 ++-- > > net/mdio-mux-uclass.c | 2 +- > > net/mdio-uclass.c | 8 > > 50 files changed, 113 insertions(+), 82 deletions(-) > > in all the modified drivers, > for the functions ofnode_(dev->node,...) modified to > ofnode_(dev_ofnode(dev), > they can also modified to dev_XXX function : > > ofnode_read_u32(dev_ofnode(dev), ...) => dev_read_u32(dev,) > ofnode_read_string(dev_ofnode(dev), ...) => dev_read_string(dev,) > ofnode_valid(dev_ofnode(dev)) => dev_has_ofnode(dev) > > but you prefer perhaps minizes the modifications in this patchset and if > it is the case : > > Reviewed-by: Patrick Delaunay > > > You can found some other examples below. Thanks for that. I wonder if we can press coccinelle into service here. I think the dev_read() API came later so people didn't know about it? Regards, Simon
Re: [RFC] mmc: fsl_esdhc_imx: Use esdhc_soc_data flags to set host caps
On Wed, Jan 6, 2021 at 7:06 PM Tim Harvey wrote: > > On Tue, Jan 5, 2021 at 2:20 PM Adam Ford wrote: > > > > On Tue, Jan 5, 2021 at 4:07 PM Jaehoon Chung wrote: > > > > > > Hi Adam, > > > > > +CC: Tim Harvey, > > > > > On 12/31/20 2:39 AM, Adam Ford wrote: > > > > The Linux driver automatically can detect and enable UHS, HS200, HS400 > > > > and HS400_ES automatically without extra flags being placed into the > > > > device tree. > > > > > > > > Right now, for U-Boot to use UHS, HS200 or HS400, the extra flags are > > > > needed. Let's go through the esdhc_soc_data flags and enable the > > > > host caps where applicable. > > > > > > > > Suggested-by: Fabio Estevam > > > > Signed-off-by: Adam Ford > > > > --- > > > > I am not an expert on the SD/MMC standards, but I used the Linux > > > > driver as a model, and made the assumption that the USDHC flag needs > > > > to be set in order to use the extra speeds. > > > > > > Looks good to me. If it can't parse property from device-tree, it needs > > > to set host_caps as proper mode. > > > Does it work fine about UHS mode? AFAIK, there was an issue about not > > > detecting host capabilities. > > > > From the troubleshooting I've done, without this patch the host didn't > > show it was capable of UHS, but with this patch, the card doesn't show > > UHS, so I think there is something wrong still, but this at least gets > > us past it being a host caps issue. > > I was able to get it working with HS400ES. > > > > If you want me to hold off until we get UHS working, I can hold off. > > I know Tim was having issues as well. > > > > I was able to confirm the UHS SDR104 works on the Renesas RZ/G2H board > > that I have using the same care, so I think the issue may be unique to > > the esdhc driver. > > > > I'm all for this patch and getting rid of the custom u-boot props that > were added to enable UHS. > > I don't think there is any need to wait as this patch does not cause > any issue with microSD, just exposes something and if you hit the > issue we are hitting with the microSD failing to acknowledge the > voltage switch at least it falls back to legacy mode so it doesn't > hurt to have this in place, esp given the benefit of enabling > HS200/HS400/HS400ES. I could use some help understanding the microSD > failing to acknowledge the voltage switch as I'm completely out of > ideas. > > The IMX6 SDHC supports UHS as well but it isn't enabled by this patch > - have you tried enabling UHS for IMX6? I have an i.MX6Q laying around somewhere, but I haven't tested it. I would expect the patch to enable the settings based on the host caps. If they're not set in the host flag, I think a separate patch would be appropriate to update the i.MX6 flags regardless of whether or not this patch gets accepted. There are so many variants of i.MX6, I am reluctant to do it for fear of breaking something. adam > > Best regards, > > Tim
Re: [PATCH] board: phytec: imx8mp: Add PHYTEC phyCORE-i.MX8MP support
Hi Theresa, On 16.12.20 10:51, Teresa Remmet wrote: > Add initial support PHYTEC phyCORE-i.MX8MP SOM. > > Supported features: > - 2GB LPDDR4 RAM > - eMMC > - external SD > - debug UART2 > - watchdog > > Signed-off-by: Teresa Remmet > --- > arch/arm/dts/Makefile|1 + > arch/arm/dts/phycore-imx8mp-u-boot.dtsi | 110 ++ > arch/arm/dts/phycore-imx8mp.dts | 224 > arch/arm/mach-imx/imx8m/Kconfig |7 + > board/phytec/phycore_imx8mp/Kconfig | 12 + > board/phytec/phycore_imx8mp/MAINTAINERS |9 + > board/phytec/phycore_imx8mp/Makefile | 11 + > board/phytec/phycore_imx8mp/lpddr4_timing.c | 1849 > ++ > board/phytec/phycore_imx8mp/phycore-imx8mp.c | 39 + > board/phytec/phycore_imx8mp/spl.c| 139 ++ > configs/phycore-imx8mp_defconfig | 98 ++ > include/configs/phycore_imx8mp.h | 110 ++ > 12 files changed, 2609 insertions(+) > create mode 100644 arch/arm/dts/phycore-imx8mp-u-boot.dtsi > create mode 100644 arch/arm/dts/phycore-imx8mp.dts > create mode 100644 board/phytec/phycore_imx8mp/Kconfig > create mode 100644 board/phytec/phycore_imx8mp/MAINTAINERS > create mode 100644 board/phytec/phycore_imx8mp/Makefile > create mode 100644 board/phytec/phycore_imx8mp/lpddr4_timing.c > create mode 100644 board/phytec/phycore_imx8mp/phycore-imx8mp.c > create mode 100644 board/phytec/phycore_imx8mp/spl.c > create mode 100644 configs/phycore-imx8mp_defconfig > create mode 100644 include/configs/phycore_imx8mp.h > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > index fd47e408f826..5c4130272023 100644 > --- a/arch/arm/dts/Makefile > +++ b/arch/arm/dts/Makefile > @@ -779,6 +779,7 @@ dtb-$(CONFIG_ARCH_IMX8M) += \ > imx8mm-beacon-kit.dtb \ > imx8mq-phanbell.dtb \ > imx8mp-evk.dtb \ > + phycore-imx8mp.dtb \ > imx8mq-pico-pi.dtb > > dtb-$(CONFIG_ARCH_IMXRT) += imxrt1050-evk.dtb \ > diff --git a/arch/arm/dts/phycore-imx8mp-u-boot.dtsi > b/arch/arm/dts/phycore-imx8mp-u-boot.dtsi > new file mode 100644 > index ..1b504618031c > --- /dev/null > +++ b/arch/arm/dts/phycore-imx8mp-u-boot.dtsi > @@ -0,0 +1,110 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH > + * Author: Teresa Remmet > + */ > + > +/ { > + wdt-reboot { > + compatible = "wdt-reboot"; > + wdt = <&wdog1>; > + u-boot,dm-spl; > + }; > +}; > + > +&{/soc@0} { > + u-boot,dm-pre-reloc; > + u-boot,dm-spl; > +}; > + > +&clk { > + u-boot,dm-spl; > + u-boot,dm-pre-reloc; > +}; > + > +&osc_32k { > + u-boot,dm-spl; > + u-boot,dm-pre-reloc; > +}; > + > +&osc_24m { > + u-boot,dm-spl; > + u-boot,dm-pre-reloc; > +}; > + > +&aips1 { > + u-boot,dm-spl; > + u-boot,dm-pre-reloc; > +}; > + > +&aips2 { > + u-boot,dm-spl; > +}; > + > +&aips3 { > + u-boot,dm-spl; > +}; > + > +&iomuxc { > + u-boot,dm-spl; > +}; > + > +®_usdhc2_vmmc { > + u-boot,dm-spl; > +}; > + > +&pinctrl_uart2 { > + u-boot,dm-spl; > +}; > + > +&pinctrl_usdhc2_gpio { > + u-boot,dm-spl; > +}; > + > +&pinctrl_usdhc2 { > + u-boot,dm-spl; > +}; > + > +&pinctrl_usdhc3 { > + u-boot,dm-spl; > +}; > + > +&gpio1 { > + u-boot,dm-spl; > +}; > + > +&gpio2 { > + u-boot,dm-spl; > +}; > + > +&gpio3 { > + u-boot,dm-spl; > +}; > + > +&gpio4 { > + u-boot,dm-spl; > +}; > + > +&gpio5 { > + u-boot,dm-spl; > +}; > + > +&uart2 { > + u-boot,dm-spl; > +}; > + > +&i2c1 { > + u-boot,dm-spl; > +}; > + > +&usdhc2 { > + u-boot,dm-spl; > +}; > + > +&usdhc3 { > + u-boot,dm-spl; > +}; > + > +&wdog1 { > + u-boot,dm-spl; > +}; > diff --git a/arch/arm/dts/phycore-imx8mp.dts b/arch/arm/dts/phycore-imx8mp.dts > new file mode 100644 > index ..7f97d193950a > --- /dev/null > +++ b/arch/arm/dts/phycore-imx8mp.dts > @@ -0,0 +1,224 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH > + * Author: Teresa Remmet > + */ > + > +/dts-v1/; > + > +#include "imx8mp.dtsi" > + > +/ { > + model = "PHYTEC phyCORE-i.MX8MP"; > + compatible = "phytec,imx8mp-phycore-som", "fsl,imx8mp"; > + > + chosen { > + stdout-path = &uart2; > + }; > + > + memory@4000 { > + device_type = "memory"; > + reg = <0x0 0x4000 0 0x8000>; > + }; > + > + reg_usdhc2_vmmc: regulator-usdhc2 { > + compatible = "regulator-fixed"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_reg_usdhc2_vmmc>; > + regulator-name = "VSD_3V3"; > + regulator-min-microvolt = <330>; > + regulator-max-microvolt = <330>; > + gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + startup-delay-us
[PATCH v2 0/9] fastboot: Add better support for specifying partitions
This series adds support for flashing eMMC boot partitions, and for flashing whole partitions. Specifically, it does this by using the existing U-Boot naming scheme to specify partitions, and not by adding new KConfig options. I have added tests for partition naming, but not for the whole flash process (though that could be a future project). I have tested this on one board, but I have *not* tested the mt85-specific KConfigs. Hopefully this series can be a way to phase out those options. Changes in v2: - Update documentation for part_get_info_by_dev_and_name - Move partition documentation under doc/usage Sean Anderson (9): mmc: sandbox: Add support for writing test: dm: Add test for fastboot mmc partition naming part: Give several functions more useful return values part: Support getting whole disk from part_get_info_by_dev_and_name_or_num part: Support string block devices in part_get_info_by_dev_and_name fastboot: Remove mmcpart argument from raw_part_get_info_by_name fastboot: Move part_get_info_by_name_or_alias after raw_part_get_info_by_name fastboot: Allow u-boot-style partitions fastboot: Partition specification cmd/ab_select.c | 3 +- configs/sandbox64_defconfig | 2 + configs/sandbox_defconfig | 2 + disk/part.c | 90 doc/android/fastboot.rst| 4 + doc/usage/index.rst | 1 + doc/usage/part.rst | 33 ++ drivers/fastboot/fb_mmc.c | 200 +--- drivers/mmc/sandbox_mmc.c | 41 +++- include/part.h | 6 +- test/dm/Makefile| 3 + test/dm/fastboot.c | 95 + test/dm/mmc.c | 19 +++- 13 files changed, 362 insertions(+), 137 deletions(-) create mode 100644 doc/usage/part.rst create mode 100644 test/dm/fastboot.c -- 2.25.1
[PATCH v2 1/9] mmc: sandbox: Add support for writing
This adds support writing to the sandbox mmc backed by an in-memory buffer. The unit test has been updated to test reading, writing, and erasing. I'm not sure what MMCs erase to; I picked 0, but if it's 0xFF then that can be easily changed. Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- (no changes since v1) drivers/mmc/sandbox_mmc.c | 41 ++- test/dm/mmc.c | 19 +- 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/drivers/mmc/sandbox_mmc.c b/drivers/mmc/sandbox_mmc.c index e86ea8fe09..3daf708451 100644 --- a/drivers/mmc/sandbox_mmc.c +++ b/drivers/mmc/sandbox_mmc.c @@ -17,6 +17,17 @@ struct sandbox_mmc_plat { struct mmc mmc; }; +#define MMC_CSIZE 0 +#define MMC_CMULT 8 /* 8 because the card is high-capacity */ +#define MMC_BL_LEN_SHIFT 10 +#define MMC_BL_LEN BIT(MMC_BL_LEN_SHIFT) +#define MMC_CAPACITY (((MMC_CSIZE + 1) << (MMC_CMULT + 2)) \ + * MMC_BL_LEN) /* 1 MiB */ + +struct sandbox_mmc_priv { + u8 buf[MMC_CAPACITY]; +}; + /** * sandbox_mmc_send_cmd() - Emulate SD commands * @@ -26,6 +37,10 @@ struct sandbox_mmc_plat { static int sandbox_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd, struct mmc_data *data) { + struct sandbox_mmc_priv *priv = dev_get_priv(dev); + struct mmc *mmc = mmc_get_mmc_dev(dev); + static ulong erase_start, erase_end; + switch (cmd->cmdidx) { case MMC_CMD_ALL_SEND_CID: memset(cmd->response, '\0', sizeof(cmd->response)); @@ -44,8 +59,9 @@ static int sandbox_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd, break; case MMC_CMD_SEND_CSD: cmd->response[0] = 0; - cmd->response[1] = 10 << 16;/* 1 << block_len */ - cmd->response[2] = 0; + cmd->response[1] = (MMC_BL_LEN_SHIFT << 16) | + ((MMC_CSIZE >> 16) & 0x3f); + cmd->response[2] = (MMC_CSIZE & 0x) << 16; cmd->response[3] = 0; break; case SD_CMD_SWITCH_FUNC: { @@ -59,13 +75,27 @@ static int sandbox_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd, break; } case MMC_CMD_READ_SINGLE_BLOCK: - memset(data->dest, '\0', data->blocksize); - break; case MMC_CMD_READ_MULTIPLE_BLOCK: - strcpy(data->dest, "this is a test"); + memcpy(data->dest, &priv->buf[cmd->cmdarg * data->blocksize], + data->blocks * data->blocksize); + break; + case MMC_CMD_WRITE_SINGLE_BLOCK: + case MMC_CMD_WRITE_MULTIPLE_BLOCK: + memcpy(&priv->buf[cmd->cmdarg * data->blocksize], data->src, + data->blocks * data->blocksize); break; case MMC_CMD_STOP_TRANSMISSION: break; + case SD_CMD_ERASE_WR_BLK_START: + erase_start = cmd->cmdarg; + break; + case SD_CMD_ERASE_WR_BLK_END: + erase_end = cmd->cmdarg; + break; + case MMC_CMD_ERASE: + memset(&priv->buf[erase_start * mmc->write_bl_len], '\0', + (erase_end - erase_start + 1) * mmc->write_bl_len); + break; case SD_CMD_APP_SEND_OP_COND: cmd->response[0] = OCR_BUSY | OCR_HCS; cmd->response[1] = 0; @@ -148,5 +178,6 @@ U_BOOT_DRIVER(mmc_sandbox) = { .bind = sandbox_mmc_bind, .unbind = sandbox_mmc_unbind, .probe = sandbox_mmc_probe, + .priv_auto_alloc_size = sizeof(struct sandbox_mmc_priv), .platdata_auto_alloc_size = sizeof(struct sandbox_mmc_plat), }; diff --git a/test/dm/mmc.c b/test/dm/mmc.c index 4e5136c850..f744452ff2 100644 --- a/test/dm/mmc.c +++ b/test/dm/mmc.c @@ -29,16 +29,25 @@ static int dm_test_mmc_blk(struct unit_test_state *uts) { struct udevice *dev; struct blk_desc *dev_desc; - char cmp[1024]; + int i; + char write[1024], read[1024]; ut_assertok(uclass_get_device(UCLASS_MMC, 0, &dev)); ut_assertok(blk_get_device_by_str("mmc", "0", &dev_desc)); - /* Read a few blocks and look for the string we expect */ + /* Write a few blocks and verify that we get the same data back */ ut_asserteq(512, dev_desc->blksz); - memset(cmp, '\0', sizeof(cmp)); - ut_asserteq(2, blk_dread(dev_desc, 0, 2, cmp)); - ut_assertok(strcmp(cmp, "this is a test")); + for (i = 0; i < sizeof(write); i++) + write[i] = i; + ut_asserteq(2, blk_dwrite(dev_desc, 0, 2, write)); + ut_asserteq(2, blk_dread(dev_desc, 0, 2, read)); + ut_asserteq_mem(write, read, sizeof(write)); + + /* Now erase them */ + memset(write, '\0', sizeof(write)); + ut_asserteq(2, blk_derase(d
[PATCH v2 2/9] test: dm: Add test for fastboot mmc partition naming
This test verifies the mapping between fastboot partitions and partitions as understood by U-Boot. It also tests the creation of GPT partitions, though that is not the primary goal. Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- (no changes since v1) configs/sandbox64_defconfig | 2 ++ configs/sandbox_defconfig | 2 ++ test/dm/Makefile| 3 ++ test/dm/fastboot.c | 64 + 4 files changed, 71 insertions(+) create mode 100644 test/dm/fastboot.c diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index 5fbbfd7236..e61aa71fbc 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -109,6 +109,8 @@ CONFIG_CPU=y CONFIG_DM_DEMO=y CONFIG_DM_DEMO_SIMPLE=y CONFIG_DM_DEMO_SHAPE=y +CONFIG_FASTBOOT_FLASH=y +CONFIG_FASTBOOT_FLASH_MMC_DEV=0 CONFIG_GPIO_HOG=y CONFIG_DM_GPIO_LOOKUP_LABEL=y CONFIG_PM8916_GPIO=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index f1ec701a9f..ba53426df4 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -134,6 +134,8 @@ CONFIG_DM_DEMO_SHAPE=y CONFIG_DMA=y CONFIG_DMA_CHANNELS=y CONFIG_SANDBOX_DMA=y +CONFIG_FASTBOOT_FLASH=y +CONFIG_FASTBOOT_FLASH_MMC_DEV=0 CONFIG_GPIO_HOG=y CONFIG_DM_GPIO_LOOKUP_LABEL=y CONFIG_PM8916_GPIO=y diff --git a/test/dm/Makefile b/test/dm/Makefile index 46e076ed09..be4385c709 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -91,5 +91,8 @@ obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o ifneq ($(CONFIG_PINMUX),) obj-$(CONFIG_PINCONF) += pinmux.o endif +ifneq ($(CONFIG_EFI_PARTITION)$(CONFIG_FASTBOOT_FLASH_MMC),) +obj-y += fastboot.o +endif endif endif # !SPL diff --git a/test/dm/fastboot.c b/test/dm/fastboot.c new file mode 100644 index 00..8f905d8fa8 --- /dev/null +++ b/test/dm/fastboot.c @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2015 Google, Inc + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define FB_ALIAS_PREFIX "fastboot_partition_alias_" + +static int dm_test_fastboot_mmc_part(struct unit_test_state *uts) +{ + char response[FASTBOOT_RESPONSE_LEN] = {0}; + char str_disk_guid[UUID_STR_LEN + 1]; + struct blk_desc *mmc_dev_desc, *fb_dev_desc; + struct disk_partition part_info; + struct disk_partition parts[2] = { + { + .start = 48, /* GPT data takes up the first 34 blocks or so */ + .size = 1, + .name = "test1", + }, + { + .start = 49, + .size = 1, + .name = "test2", + }, + }; + + ut_assertok(blk_get_device_by_str("mmc", + __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV), + &mmc_dev_desc)); + if (CONFIG_IS_ENABLED(RANDOM_UUID)) { + gen_rand_uuid_str(parts[0].uuid, UUID_STR_FORMAT_STD); + gen_rand_uuid_str(parts[1].uuid, UUID_STR_FORMAT_STD); + gen_rand_uuid_str(str_disk_guid, UUID_STR_FORMAT_STD); + } + ut_assertok(gpt_restore(mmc_dev_desc, str_disk_guid, parts, + ARRAY_SIZE(parts))); + + /* "Classic" partition labels */ + ut_asserteq(1, fastboot_mmc_get_part_info("test1", &fb_dev_desc, + &part_info, response)); + ut_asserteq(2, fastboot_mmc_get_part_info("test2", &fb_dev_desc, + &part_info, response)); + + /* Test aliases */ + ut_assertnull(env_get(FB_ALIAS_PREFIX "test3")); + ut_assertok(env_set(FB_ALIAS_PREFIX "test3", "test1")); + ut_asserteq(1, fastboot_mmc_get_part_info("test3", &fb_dev_desc, + &part_info, response)); + ut_assertok(env_set(FB_ALIAS_PREFIX "test3", NULL)); + + return 0; +} +DM_TEST(dm_test_fastboot_mmc_part, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); -- 2.25.1
[PATCH v2 3/9] part: Give several functions more useful return values
Several functions in disk/part.c just return -1 on error. This makes them return different errnos for different failures. This helps callers differentiate between failures, even if they cannot read stdout. Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- (no changes since v1) disk/part.c | 50 -- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/disk/part.c b/disk/part.c index b69fd345f3..5e354e256f 100644 --- a/disk/part.c +++ b/disk/part.c @@ -354,7 +354,7 @@ int part_get_info(struct blk_desc *dev_desc, int part, } #endif /* CONFIG_HAVE_BLOCK_DEVICE */ - return -1; + return -ENOENT; } int part_get_info_whole_disk(struct blk_desc *dev_desc, @@ -416,7 +416,7 @@ int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str, *dev_desc = get_dev_hwpart(ifname, dev, hwpart); if (!(*dev_desc) || ((*dev_desc)->type == DEV_TYPE_UNKNOWN)) { debug("** Bad device %s %s **\n", ifname, dev_hwpart_str); - dev = -ENOENT; + dev = -ENODEV; goto cleanup; } @@ -440,7 +440,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, struct blk_desc **dev_desc, struct disk_partition *info, int allow_whole_dev) { - int ret = -1; + int ret; const char *part_str; char *dup_str = NULL; const char *dev_str; @@ -482,7 +482,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, if (0 == strcmp(ifname, "ubi")) { if (!ubifs_is_mounted()) { printf("UBIFS not mounted, use ubifsmount to mount volume first!\n"); - return -1; + return -EINVAL; } *dev_desc = NULL; @@ -504,6 +504,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, /* If still no dev_part_str, it's an error */ if (!dev_part_str) { printf("** No device specified **\n"); + ret = -ENODEV; goto cleanup; } @@ -520,8 +521,10 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, /* Look up the device */ dev = blk_get_device_by_str(ifname, dev_str, dev_desc); - if (dev < 0) + if (dev < 0) { + ret = dev; goto cleanup; + } /* Convert partition ID string to number */ if (!part_str || !*part_str) { @@ -538,6 +541,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, if (*ep || (part == 0 && !allow_whole_dev)) { printf("** Bad partition specification %s %s **\n", ifname, dev_part_str); + ret = -ENOENT; goto cleanup; } } @@ -551,6 +555,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, if (!(*dev_desc)->lba) { printf("** Bad device size - %s %s **\n", ifname, dev_str); + ret = -EINVAL; goto cleanup; } @@ -562,6 +567,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, if ((part > 0) || (!allow_whole_dev)) { printf("** No partition table - %s %s **\n", ifname, dev_str); + ret = -EPROTONOSUPPORT; goto cleanup; } @@ -630,7 +636,6 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, *info = tmpinfo; } else { printf("** No valid partitions found **\n"); - ret = -1; goto cleanup; } } @@ -638,7 +643,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, printf("** Invalid partition type \"%.32s\"" " (expect \"" BOOT_PART_TYPE "\")\n", info->type); - ret = -1; + ret = -EINVAL; goto cleanup; } @@ -674,7 +679,7 @@ int part_get_info_by_name_type(struct blk_desc *dev_desc, const char *name, } } - return -1; + return -ENOENT; } int part_get_info_by_name(struct blk_desc *dev_desc, const char *name, @@ -704,7 +709,7 @@ static int part_get_info_by_dev_and_name(const char *dev_iface, { char *ep; const char *part_str; - int dev_num; + int dev_num, ret; part_str = strchr(dev_part_str, '#'); if (!part_str || part_str == dev_part_str) @@ -720,13 +725,12 @@ static int part
[PATCH v2 4/9] part: Support getting whole disk from part_get_info_by_dev_and_name_or_num
This adds an option to part_get_info_by_dev_and_name_or_num to allow callers to specify whether whole-disk partitions are fine. Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- (no changes since v1) cmd/ab_select.c | 3 ++- disk/part.c | 5 +++-- include/part.h | 6 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/cmd/ab_select.c b/cmd/ab_select.c index 6298fcfb60..3e46663d6e 100644 --- a/cmd/ab_select.c +++ b/cmd/ab_select.c @@ -22,7 +22,8 @@ static int do_ab_select(struct cmd_tbl *cmdtp, int flag, int argc, /* Lookup the "misc" partition from argv[2] and argv[3] */ if (part_get_info_by_dev_and_name_or_num(argv[2], argv[3], -&dev_desc, &part_info) < 0) { +&dev_desc, &part_info, +false) < 0) { return CMD_RET_FAILURE; } diff --git a/disk/part.c b/disk/part.c index 5e354e256f..39c6b00a59 100644 --- a/disk/part.c +++ b/disk/part.c @@ -736,7 +736,8 @@ static int part_get_info_by_dev_and_name(const char *dev_iface, int part_get_info_by_dev_and_name_or_num(const char *dev_iface, const char *dev_part_str, struct blk_desc **dev_desc, -struct disk_partition *part_info) +struct disk_partition *part_info, +int allow_whole_dev) { int ret; @@ -750,7 +751,7 @@ int part_get_info_by_dev_and_name_or_num(const char *dev_iface, * directly. */ ret = blk_get_device_part_str(dev_iface, dev_part_str, - dev_desc, part_info, 1); + dev_desc, part_info, allow_whole_dev); if (ret < 0) printf("Couldn't find partition %s %s\n", dev_iface, dev_part_str); diff --git a/include/part.h b/include/part.h index 55be724d20..778cb36199 100644 --- a/include/part.h +++ b/include/part.h @@ -226,12 +226,16 @@ int part_get_info_by_name(struct blk_desc *dev_desc, * @param[in] dev_part_str Input partition description, like "0#misc" or "0:1" * @param[out] dev_descPlace to store the device description pointer * @param[out] part_info Place to store the partition information + * @param[in] allow_whole_dev true to allow the user to select partition 0 + * (which means the whole device), false to require a valid + * partition number >= 1 * @return 0 on success, or a negative on error */ int part_get_info_by_dev_and_name_or_num(const char *dev_iface, const char *dev_part_str, struct blk_desc **dev_desc, -struct disk_partition *part_info); +struct disk_partition *part_info, +int allow_whole_dev); /** * part_set_generic_name() - create generic partition like hda1 or sdb2 -- 2.25.1
[PATCH v2 5/9] part: Support string block devices in part_get_info_by_dev_and_name
This adds support for things like "#partname" and "0.1#partname". The block device parsing is done like in blk_get_device_part_str. Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- Changes in v2: - Update documentation disk/part.c | 41 ++--- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/disk/part.c b/disk/part.c index 39c6b00a59..aaeafbb0b2 100644 --- a/disk/part.c +++ b/disk/part.c @@ -692,12 +692,13 @@ int part_get_info_by_name(struct blk_desc *dev_desc, const char *name, * Get partition info from device number and partition name. * * Parse a device number and partition name string in the form of - * "device_num#partition_name", for example "0#misc". If the partition - * is found, sets dev_desc and part_info accordingly with the information - * of the partition with the given partition_name. + * "devicenum.hwpartnum#partition_name", for example "0.1#misc". devicenum and + * hwpartnum are both optional, defaulting to 0. If the partition is found, + * sets dev_desc and part_info accordingly with the information of the + * partition with the given partition_name. * * @param[in] dev_iface Device interface - * @param[in] dev_part_str Input string argument, like "0#misc" + * @param[in] dev_part_str Input string argument, like "0.1#misc" * @param[out] dev_desc Place to store the device description pointer * @param[out] part_info Place to store the partition information * @return 0 on success, or a negative on error @@ -707,29 +708,31 @@ static int part_get_info_by_dev_and_name(const char *dev_iface, struct blk_desc **dev_desc, struct disk_partition *part_info) { - char *ep; - const char *part_str; - int dev_num, ret; + char *dup_str = NULL; + const char *dev_str, *part_str; + int ret; + /* Separate device and partition name specification */ part_str = strchr(dev_part_str, '#'); - if (!part_str || part_str == dev_part_str) - return -EINVAL; - - dev_num = simple_strtoul(dev_part_str, &ep, 16); - if (ep != part_str) { - /* Not all the first part before the # was parsed. */ + if (part_str) { + dup_str = strdup(dev_part_str); + dup_str[part_str - dev_part_str] = 0; + dev_str = dup_str; + part_str++; + } else { return -EINVAL; } - part_str++; - *dev_desc = blk_get_dev(dev_iface, dev_num); - if (!*dev_desc) { - printf("Could not find %s %d\n", dev_iface, dev_num); - return -ENODEV; - } + ret = blk_get_device_by_str(dev_iface, dev_str, dev_desc); + if (ret) + goto cleanup; + ret = part_get_info_by_name(*dev_desc, part_str, part_info); if (ret < 0) printf("Could not find \"%s\" partition\n", part_str); + +cleanup: + free(dup_str); return ret; } -- 2.25.1
[PATCH v2 7/9] fastboot: Move part_get_info_by_name_or_alias after raw_part_get_info_by_name
This makes the next commit more readable by doing the move now. Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- (no changes since v1) drivers/fastboot/fb_mmc.c | 44 +++ 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c index f6cacd711b..b0610d3151 100644 --- a/drivers/fastboot/fb_mmc.c +++ b/drivers/fastboot/fb_mmc.c @@ -28,28 +28,6 @@ struct fb_mmc_sparse { struct blk_desc *dev_desc; }; -static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc, - const char *name, struct disk_partition *info) -{ - int ret; - - ret = part_get_info_by_name(dev_desc, name, info); - if (ret < 0) { - /* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */ - char env_alias_name[25 + PART_NAME_LEN + 1]; - char *aliased_part_name; - - /* check for alias */ - strcpy(env_alias_name, "fastboot_partition_alias_"); - strncat(env_alias_name, name, PART_NAME_LEN); - aliased_part_name = env_get(env_alias_name); - if (aliased_part_name != NULL) - ret = part_get_info_by_name(dev_desc, - aliased_part_name, info); - } - return ret; -} - static int raw_part_get_info_by_name(struct blk_desc *dev_desc, const char *name, struct disk_partition *info) @@ -98,6 +76,28 @@ static int raw_part_get_info_by_name(struct blk_desc *dev_desc, return 0; } +static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc, + const char *name, struct disk_partition *info) +{ + int ret; + + ret = part_get_info_by_name(dev_desc, name, info); + if (ret < 0) { + /* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */ + char env_alias_name[25 + PART_NAME_LEN + 1]; + char *aliased_part_name; + + /* check for alias */ + strcpy(env_alias_name, "fastboot_partition_alias_"); + strncat(env_alias_name, name, PART_NAME_LEN); + aliased_part_name = env_get(env_alias_name); + if (aliased_part_name != NULL) + ret = part_get_info_by_name(dev_desc, + aliased_part_name, info); + } + return ret; +} + /** * fb_mmc_blk_write() - Write/erase MMC in chunks of FASTBOOT_MAX_BLK_WRITE * -- 2.25.1
[PATCH v2 9/9] fastboot: Partition specification
This documents the way U-Boot understands partitions specifications. This also updates the fastboot documentation for the changes in the previous commit. Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- Changes in v2: - Move partition documentation under doc/usage doc/android/fastboot.rst | 4 doc/usage/index.rst | 1 + doc/usage/part.rst | 33 + 3 files changed, 38 insertions(+) create mode 100644 doc/usage/part.rst diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst index 2877c3cbaa..7b6eff82b4 100644 --- a/doc/android/fastboot.rst +++ b/doc/android/fastboot.rst @@ -151,6 +151,10 @@ The device index starts from ``a`` and refers to the interface (e.g. USB controller, SD/MMC controller) or disk index. The partition index starts from ``1`` and describes the partition number on the particular device. +Alternatively, partition types may be specified using :ref:`U-Boot's partition +syntax `. This allows specifying partitions like ``0.1``, +``0#boot``, or ``:3``. The interface is always ``mmc``. + Writing Partition Table --- diff --git a/doc/usage/index.rst b/doc/usage/index.rst index fbb2c0481c..a22ace03e8 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -4,6 +4,7 @@ Use U-Boot .. toctree:: netconsole + part Shell commands -- diff --git a/doc/usage/part.rst b/doc/usage/part.rst new file mode 100644 index 00..e58b529147 --- /dev/null +++ b/doc/usage/part.rst @@ -0,0 +1,33 @@ +.. SPDX-License-Identifier: GPL-2.0+ +.. _partitions: + +Partitions +== + +Many U-Boot commands allow specifying partitions like:: + +some_command + +or like:: + +some_command
[PATCH v2 8/9] fastboot: Allow u-boot-style partitions
This adds support for partitions of the form "dev.hwpart:part" and "dev#partname". This allows one to flash to eMMC boot partitions without having to use CONFIG_FASTBOOT_MMC_BOOT1_SUPPORT. It also allows one to flash to an entire device without needing CONFIG_FASTBOOT_MMC_USER_NAME. Lastly, one can also flash MMC devices other than CONFIG_FASTBOOT_FLASH_MMC_DEV. Because devices can be specified explicitly, CONFIG_FASTBOOT_FLASH_MMC_DEV is used only when necessary for existing functionality. For those cases, fastboot_mmc_get_dev has been added as a helper function. This allows There should be no conflicts with the existing system, but just in case, I have ordered detection of these names after all existing names. The fastboot_mmc_part test has been updated for these new names. Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- Changes in v2: - Fix accidental reversion of earlier changes drivers/fastboot/fb_mmc.c | 147 +++--- test/dm/fastboot.c| 37 +- 2 files changed, 125 insertions(+), 59 deletions(-) diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c index b0610d3151..1b8d35877b 100644 --- a/drivers/fastboot/fb_mmc.c +++ b/drivers/fastboot/fb_mmc.c @@ -76,12 +76,37 @@ static int raw_part_get_info_by_name(struct blk_desc *dev_desc, return 0; } -static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc, - const char *name, struct disk_partition *info) +static int do_get_part_info(struct blk_desc **dev_desc, const char *name, + struct disk_partition *info) { int ret; - ret = part_get_info_by_name(dev_desc, name, info); + /* First try partition names on the default device */ + *dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV); + if (*dev_desc) { + ret = part_get_info_by_name(*dev_desc, name, info); + if (ret >= 0) + return ret; + + /* Then try raw partitions */ + ret = raw_part_get_info_by_name(*dev_desc, name, info); + if (ret >= 0) + return ret; + } + + /* Then try dev.hwpart:part */ + ret = part_get_info_by_dev_and_name_or_num("mmc", name, dev_desc, + info, true); + return ret; +} + +static int part_get_info_by_name_or_alias(struct blk_desc **dev_desc, + const char *name, + struct disk_partition *info) +{ + int ret; + + ret = do_get_part_info(dev_desc, name, info); if (ret < 0) { /* strlen("fastboot_partition_alias_") + PART_NAME_LEN + 1 */ char env_alias_name[25 + PART_NAME_LEN + 1]; @@ -92,8 +117,8 @@ static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc, strncat(env_alias_name, name, PART_NAME_LEN); aliased_part_name = env_get(env_alias_name); if (aliased_part_name != NULL) - ret = part_get_info_by_name(dev_desc, - aliased_part_name, info); + ret = do_get_part_info(dev_desc, aliased_part_name, + info); } return ret; } @@ -424,27 +449,49 @@ int fastboot_mmc_get_part_info(const char *part_name, struct blk_desc **dev_desc, struct disk_partition *part_info, char *response) { - int r = 0; + int ret; - *dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV); - if (!*dev_desc) { - fastboot_fail("block device not found", response); - return -ENOENT; - } if (!part_name || !strcmp(part_name, "")) { fastboot_fail("partition not given", response); return -ENOENT; } - if (raw_part_get_info_by_name(*dev_desc, part_name, part_info) < 0) { - r = part_get_info_by_name_or_alias(*dev_desc, part_name, part_info); - if (r < 0) { - fastboot_fail("partition not found", response); - return r; + ret = part_get_info_by_name_or_alias(dev_desc, part_name, part_info); + if (ret < 0) { + switch (ret) { + case -ENOSYS: + case -EINVAL: + fastboot_fail("invalid partition or device", response); + break; + case -ENODEV: + fastboot_fail("no such device", response); + break; + case -ENOENT: + fastboot_fail("no such partition", response); + break; + case -EPROTONOSUPPORT: + fastboot_fail("unknown partition table type", response); +
[PATCH v2 6/9] fastboot: Remove mmcpart argument from raw_part_get_info_by_name
The only thing mmcpart was used for was to pass to blk_dselect_hwpart. This calls blk_dselect_hwpart directly from raw_part_get_info_by_name. The error handling is dropped, but it is reintroduced in the next commit (albeit less specificly). Signed-off-by: Sean Anderson Reviewed-by: Simon Glass --- (no changes since v1) drivers/fastboot/fb_mmc.c | 41 +-- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c index ae8e8e512f..f6cacd711b 100644 --- a/drivers/fastboot/fb_mmc.c +++ b/drivers/fastboot/fb_mmc.c @@ -51,7 +51,8 @@ static int part_get_info_by_name_or_alias(struct blk_desc *dev_desc, } static int raw_part_get_info_by_name(struct blk_desc *dev_desc, - const char *name, struct disk_partition *info, int *mmcpart) +const char *name, +struct disk_partition *info) { /* strlen("fastboot_raw_partition_") + PART_NAME_LEN + 1 */ char env_desc_name[23 + PART_NAME_LEN + 1]; @@ -85,8 +86,13 @@ static int raw_part_get_info_by_name(struct blk_desc *dev_desc, strncpy((char *)info->name, name, PART_NAME_LEN); if (raw_part_desc) { - if (strcmp(strsep(&raw_part_desc, " "), "mmcpart") == 0) - *mmcpart = simple_strtoul(raw_part_desc, NULL, 0); + if (strcmp(strsep(&raw_part_desc, " "), "mmcpart") == 0) { + ulong mmcpart = simple_strtoul(raw_part_desc, NULL, 0); + int ret = blk_dselect_hwpart(dev_desc, mmcpart); + + if (ret) + return ret; + } } return 0; @@ -419,7 +425,6 @@ int fastboot_mmc_get_part_info(const char *part_name, struct disk_partition *part_info, char *response) { int r = 0; - int mmcpart; *dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV); if (!*dev_desc) { @@ -431,7 +436,7 @@ int fastboot_mmc_get_part_info(const char *part_name, return -ENOENT; } - if (raw_part_get_info_by_name(*dev_desc, part_name, part_info, &mmcpart) < 0) { + if (raw_part_get_info_by_name(*dev_desc, part_name, part_info) < 0) { r = part_get_info_by_name_or_alias(*dev_desc, part_name, part_info); if (r < 0) { fastboot_fail("partition not found", response); @@ -455,7 +460,6 @@ void fastboot_mmc_flash_write(const char *cmd, void *download_buffer, { struct blk_desc *dev_desc; struct disk_partition info; - int mmcpart = 0; dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV); if (!dev_desc || dev_desc->type == DEV_TYPE_UNKNOWN) { @@ -528,16 +532,12 @@ void fastboot_mmc_flash_write(const char *cmd, void *download_buffer, } #endif - if (raw_part_get_info_by_name(dev_desc, cmd, &info, &mmcpart) == 0) { - if (blk_dselect_hwpart(dev_desc, mmcpart)) { - pr_err("Failed to select hwpart\n"); - fastboot_fail("Failed to select hwpart", response); + if (raw_part_get_info_by_name(dev_desc, cmd, &info) != 0) { + if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) { + pr_err("cannot find partition: '%s'\n", cmd); + fastboot_fail("cannot find partition", response); return; } - } else if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) { - pr_err("cannot find partition: '%s'\n", cmd); - fastboot_fail("cannot find partition", response); - return; } if (is_sparse_image(download_buffer)) { @@ -580,7 +580,6 @@ void fastboot_mmc_erase(const char *cmd, char *response) struct disk_partition info; lbaint_t blks, blks_start, blks_size, grp_size; struct mmc *mmc = find_mmc_device(CONFIG_FASTBOOT_FLASH_MMC_DEV); - int mmcpart = 0; if (mmc == NULL) { pr_err("invalid mmc device\n"); @@ -614,16 +613,12 @@ void fastboot_mmc_erase(const char *cmd, char *response) } #endif - if (raw_part_get_info_by_name(dev_desc, cmd, &info, &mmcpart) == 0) { - if (blk_dselect_hwpart(dev_desc, mmcpart)) { - pr_err("Failed to select hwpart\n"); - fastboot_fail("Failed to select hwpart", response); + if (raw_part_get_info_by_name(dev_desc, cmd, &info) != 0) { + if (part_get_info_by_name_or_alias(dev_desc, cmd, &info) < 0) { + pr_err("cannot find partition: '%s'\n", cmd); + fastboot_fail("cannot find partition", response); return; } - } else if (part_get_info_by_name_or_alias
[PATCH] board: mediatek: disable watchdog on BananaPi R2
From: Matthias Brugger Watchdog timeout comes in before we are able to load the kernel and reset the watchdog. Disable the watchdog late in the boot process to be able to boot the board. Signed-off-by: Matthias Brugger --- board/mediatek/mt7623/mt7623_rfb.c | 8 configs/mt7623n_bpir2_defconfig| 1 + 2 files changed, 9 insertions(+) diff --git a/board/mediatek/mt7623/mt7623_rfb.c b/board/mediatek/mt7623/mt7623_rfb.c index 984e75ccaf..22120be412 100644 --- a/board/mediatek/mt7623/mt7623_rfb.c +++ b/board/mediatek/mt7623/mt7623_rfb.c @@ -5,6 +5,7 @@ #include #include +#include DECLARE_GLOBAL_DATA_PTR; @@ -40,3 +41,10 @@ int mmc_get_env_dev(void) return mmc_get_boot_dev(); } #endif + +#ifdef CONFIG_BOARD_LATE_INIT +int board_late_init(void) +{ + wdt_stop(gd->watchdog_dev); +} +#endif diff --git a/configs/mt7623n_bpir2_defconfig b/configs/mt7623n_bpir2_defconfig index 9177c17dff..fe0b6259dd 100644 --- a/configs/mt7623n_bpir2_defconfig +++ b/configs/mt7623n_bpir2_defconfig @@ -14,6 +14,7 @@ CONFIG_FIT_VERBOSE=y CONFIG_BOOTDELAY=3 CONFIG_SYS_CONSOLE_IS_IN_ENV=y CONFIG_DEFAULT_FDT_FILE="mt7623n-bananapi-bpi-r2.dtb" +CONFIG_BOARD_LATE_INIT=y # CONFIG_DISPLAY_BOARDINFO is not set CONFIG_SYS_PROMPT="U-Boot> " CONFIG_CMD_BOOTMENU=y -- 2.29.2
Re: [PATCH 1/2] video: seps525: Align driver with latest treewide changes
On Thu, 7 Jan 2021 at 06:26, Michal Simek wrote: > > Some structures/functions have been renamed which are not reflected in this > driver. > Commit 8a8d24bdf174 ("dm: treewide: Rename ..._platdata variables to just > ..._plat"), commit c69cda25c9b5 ("dm: treewide: Rename dev_get_platdata() > to dev_get_plat()"), commit caa4daa2ae3d ("dm: treewide: Rename 'platdata' > variables to just 'plat'") and commit 41575d8e4c33 ("dm: treewide: Rename > auto_alloc_size members to be shorter") are describing these changes. > > Signed-off-by: Michal Simek > --- > > Driver was developed based on master branch and next got more changes. > --- > drivers/video/seps525.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > Reviewed-by: Simon Glass
Re: [v2 4/6] arm: socfpga: dts: soc64: Update filename in binman node of FIT image with VAB support
Hi Siew Chin, On Thu, 7 Jan 2021 at 07:13, Lim, Elly Siew Chin wrote: > > Hi Simon, > > > -Original Message- > > From: Simon Glass > > Sent: Thursday, January 7, 2021 8:37 PM > > To: Lim, Elly Siew Chin > > Cc: U-Boot Mailing List ; Marek Vasut > > ; Tan, Ley Foon ; See, Chin Liang > > ; Simon Goldschmidt > > ; Chee, Tien Fong > > ; Westergreen, Dalon > > ; Gan, Yau Wai > > Subject: Re: [v2 4/6] arm: socfpga: dts: soc64: Update filename in binman > > node > > of FIT image with VAB support > > > > On Thu, 7 Jan 2021 at 03:03, Siew Chin Lim > > wrote: > > > > > > FIT image of Vendor Authentication Coot (VAB) contains signed images. > > > > > > Signed-off-by: Siew Chin Lim > > > --- > > > arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi | 22 > > > ++ > > > 1 file changed, 22 insertions(+) > > > > > > > I'm not quite sure what is happening here, but consider using two separate > > files > > rather than what looks like a patch over an existing one. > > > > There are two boot flow will use binman (socfpga_soc64_fit-u-boot.dtsi) to > generate u-boot.fit and kernel.fit: > 1. socfpga_agilex_atf_defconfig (boot via ATF) > 2. socfpga_agilex_vab_defconfig (boot via ATF with VAB enabled, support > authentication on bl31, u-boot, Linux images) > > The binman node settings are the same for both flows. With VAB enabled, all > inputs file need to be signed before generate FIT image. We would like to use > different input file name to remind user that they need to sign all bl31, > u-boot, Linux images when using binman to generate FIT image. > > Due to the binman node settings are identical and only the file name need to > be different, so we prefer to share the same socfpga_soc64_fit-u-boot.dtsi > for both flows. Reviewed-by: Simon Glass OK I see. Who does the signing of the inputs? Is that something binman could/should do? Regards, Simon
Re: [PATCH RFC v2 3/5] lib: Add support for ECDSA image signing
On 1/7/21 6:35 AM, Simon Glass wrote: Hi Alexandru, Hi Simon, (pun alert!) A lot of your comments have to do with comments. I use comments as a tool to add something of value to code. When the code is self-documenting, comments don't help much. See kernel coding style chapter 8. What comments can do, is increase code size and break code flow -- there is a cost to having them. I'm certain you've seen functions that need to be scrolled up and down because of a huge comment smack in the middle. I'll address individual comment comments below. On Wed, 30 Dec 2020 at 14:00, Alexandru Gagniuc wrote: mkimage supports rsa2048, and rsa4096 signatures. With newer silicon now supporting hardware-accelerated ECDSA, it makes sense to expand signing support to elliptic curves. Implement host-side ECDSA signing and verification with libcrypto. Device-side implementation of signature verification is beyond the scope of this patch. Signed-off-by: Alexandru Gagniuc --- common/image-sig.c | 14 +- include/u-boot/ecdsa.h | 27 lib/ecdsa/ecdsa-libcrypto.c | 300 tools/Makefile | 3 + 4 files changed, 342 insertions(+), 2 deletions(-) create mode 100644 include/u-boot/ecdsa.h create mode 100644 lib/ecdsa/ecdsa-libcrypto.c diff --git a/common/image-sig.c b/common/image-sig.c index 21dafe6b91..2548b3eb0f 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -15,6 +15,7 @@ DECLARE_GLOBAL_DATA_PTR; #endif /* !USE_HOSTCC*/ #include +#include #include #include @@ -82,8 +83,17 @@ struct crypto_algo crypto_algos[] = { .sign = rsa_sign, .add_verify_data = rsa_add_verify_data, .verify = rsa_verify, - } - + }, +#if defined(USE_HOSTCC) + /* Currently, only host support exists for ECDSA */ + { + .name = "ecdsa256", + .key_len = ECDSA256_BYTES, + .sign = ecdsa_sign, + .add_verify_data = ecdsa_add_verify_data, + .verify = ecdsa_verify, + }, +#endif }; struct padding_algo padding_algos[] = { diff --git a/include/u-boot/ecdsa.h b/include/u-boot/ecdsa.h new file mode 100644 index 00..a13a7267e1 --- /dev/null +++ b/include/u-boot/ecdsa.h @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2020, Alexandru Gagniuc . + */ + +#ifndef _ECDSA_H +#define _ECDSA_H + +#include +#include + +/** + * crypto_algo API impementation for ECDSA; + * @see "struct crypt_algo" + * @{ + */ +int ecdsa_sign(struct image_sign_info *info, const struct image_region region[], + int region_count, uint8_t **sigp, uint *sig_len); +int ecdsa_verify(struct image_sign_info *info, +const struct image_region region[], int region_count, +uint8_t *sig, uint sig_len); +int ecdsa_add_verify_data(struct image_sign_info *info, void *keydest); Please always add full comments when you export functions, or have a non-trivial static function. I disagree. These functions implement and are designed to be used via the crypt_algo API. One should understand the crypt_algo API, and any deviations in behavior would be a bug. So there is no scenario in which comments here would be useful. +/** @} */ + +#define ECDSA256_BYTES (256 / 8) + +#endif diff --git a/lib/ecdsa/ecdsa-libcrypto.c b/lib/ecdsa/ecdsa-libcrypto.c new file mode 100644 index 00..ff491411d0 --- /dev/null +++ b/lib/ecdsa/ecdsa-libcrypto.c @@ -0,0 +1,300 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * ECDSA image signing implementation using libcrypto backend + * + * The signature is a binary representation of the (R, S) points, padded to the + * key size. The signature will be (2 * key_size_bits) / 8 bytes. + * + * Deviations from behavior of RSA equivalent: + * - Verification uses private key. This is not technically required, but a + *limitation on how clumsy the openssl API is to use. I'm not quite sure what the implications are on this. If this is public key crypto, the private key is not available, so how can you verify with it? I presume this is fixable, but only as an academic exercise. This file is for mkimage, which doesn't do standalone verification. The way you verify is in u-boot. That is the subject of another series. + * - Handling of keys and key paths: + *- The '-K' key directory option must contain path to the key file, + * instead of the key directory. If we make this change we should update RSA to do the same. Of course, but is there agreement as to this direction? There seem to be some hidden subtleties to key-name-hint that I don't fully understand yet. + *- No assumptions are made about the file extension of the key + *- The 'key-name-hint' property is only used for naming devicetree nodes, + * but is not used for looking up keys on the filesystem. + * + * Copyright (c) 2020, Alexandru Gagni
Re: [PATCH V3] net: dwc_eth_qos: Pad descriptors to cacheline size
On 1/7/21 3:12 AM, Marek Vasut wrote: > The DWMAC4 IP has the possibility to skip up to 7 AXI bus width size words > after the descriptor. Use this to pad the descriptors to cacheline size and > remove the need for noncached memory altogether. Moreover, this lets Tegra > use the generic cache flush / invalidate operations. Tested-by: Stephen Warren Reviewed-by: Stephen Warren
Re: [PATCH RFC v2 5/5] test/py: ecdsa: Add test for mkimage ECDSA signing
On 1/7/21 6:35 AM, Simon Glass wrote: Hi Alexandru, On Wed, 30 Dec 2020 at 14:00, Alexandru Gagniuc wrote: Add a test to make sure that the ECDSA signatures generated by mkimage can be verified successfully. pyCryptodomex was chosen as the crypto library because it integrates much better with python code. Using openssl would have been unnecessarily painful. Signed-off-by: Alexandru Gagniuc --- test/py/tests/test_fit_ecdsa.py | 111 1 file changed, 111 insertions(+) create mode 100644 test/py/tests/test_fit_ecdsa.py This test looks fine but the functions need full comments. I do think it might be worth putting the code in test_vboot, particularly when you get to the sandbox implementation. test_vboot seems to be testing the bootm command, while with this test I'm only looking to test the host-side (mkimage). In the next series, I won't have a software implementation of ECDSA, like RSA_MOD_EXP. I will use the ROM on the stm32mp. So there won't be somthing testable in the sandbox. For installing the Python library, you might need to update the docs in test/ and perhaps install things in .gitlab-ci.yml and .azure... Sure, let me look into this. diff --git a/test/py/tests/test_fit_ecdsa.py b/test/py/tests/test_fit_ecdsa.py new file mode 100644 index 00..957964d329 --- /dev/null +++ b/test/py/tests/test_fit_ecdsa.py @@ -0,0 +1,111 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (c) 2020, Alexandru Gagniuc + +""" +Test ECDSA signing of FIT images + +This test uses mkimage to sign an existing FIT image with an ECDSA key. The +signature is then extracted, and verified against pyCryptodome. +This test doesn't run the sandbox. It only checks the host tool 'mkimage' +""" + +import pytest +import u_boot_utils as util +from Cryptodome.Hash import SHA256 +from Cryptodome.PublicKey import ECC +from Cryptodome.Signature import DSS + +class SignableFitImage(object): +""" Helper to manipulate a FIT image on disk """ +def __init__(self, cons, file_name): +self.fit = file_name +self.cons = cons +self.signable_nodes = set() + +def __fdt_list(self, path): +return util.run_and_log(self.cons, f'fdtget -l {self.fit} {path}') + +def __fdt_set(self, node, **prop_value): +for prop, value in prop_value.items(): +util.run_and_log(self.cons, f'fdtput -ts {self.fit} {node} {prop} {value}') + +def __fdt_get_binary(self, node, prop): +numbers = util.run_and_log(self.cons, f'fdtget -tbi {self.fit} {node} {prop}') + +bignum = bytearray() +for little_num in numbers.split(): +bignum.append(int(little_num)) + +return bignum + +def find_signable_image_nodes(self): +for node in self.__fdt_list('/images').split(): +image = f'/images/{node}' +if 'signature' in self.__fdt_list(image): +self.signable_nodes.add(image) + +return self.signable_nodes + +def change_signature_algo_to_ecdsa(self): +for image in self.signable_nodes: +self.__fdt_set(f'{image}/signature', algo='sha256,ecdsa256') + +def sign(self, mkimage, key_file): +util.run_and_log(self.cons, [mkimage, '-F', self.fit, f'-k{key_file}']) + +def check_signatures(self, key): +for image in self.signable_nodes: +raw_sig = self.__fdt_get_binary(f'{image}/signature', 'value') +raw_bin = self.__fdt_get_binary(image, 'data') + +sha = SHA256.new(raw_bin) +verifier = DSS.new(key, 'fips-186-3') +verifier.verify(sha, bytes(raw_sig)) + + +@pytest.mark.buildconfigspec('fit_signature') +@pytest.mark.requiredtool('dtc') +@pytest.mark.requiredtool('fdtget') +@pytest.mark.requiredtool('fdtput') +def test_fit_ecdsa(u_boot_console): +"""TODO: Document me +""" +def generate_ecdsa_key(): +return ECC.generate(curve='prime256v1') + +def assemble_fit_image(dest_fit, its, destdir): +dtc_args = f'-I dts -O dtb -i {destdir}' +util.run_and_log(cons, [mkimage, '-D', dtc_args, '-f', its, dest_fit]) + +def dtc(dts): +dtb = dts.replace('.dts', '.dtb') +util.run_and_log(cons, f'dtc {datadir}/{dts} -O dtb -o {tempdir}/{dtb}') + +cons = u_boot_console +mkimage = cons.config.build_dir + '/tools/mkimage' +datadir = cons.config.source_dir + '/test/py/tests/vboot/' +tempdir = cons.config.result_dir +key_file = f'{tempdir}/ecdsa-test-key.pem' +fit_file = f'{tempdir}/test.fit' +dtc('sandbox-kernel.dts') + +key = generate_ecdsa_key() + +# Create a number kernel image with zeroes +with open(f'{tempdir}/test-kernel.bin', 'w') as fd: +fd.write(500 * chr(0)) + +with open(key_file, 'w') as f: +f.write(key.export_key(format='PEM')) + +assemble_fit_image(fit_file, f'{datadir}/sign-images-sha256.its', tempdir) + +fit = SignableFitImage(cons, fit_file
Re: [PATCH RFC v2 3/5] lib: Add support for ECDSA image signing
On Thu, Jan 07, 2021 at 10:27:50AM -0600, Alex G. wrote: > On 1/7/21 6:35 AM, Simon Glass wrote: > > Hi Alexandru, > > Hi Simon, > > (pun alert!) A lot of your comments have to do with comments. I use comments > as a tool to add something of value to code. When the code is > self-documenting, comments don't help much. > See kernel coding style chapter 8. Comments for comments sake are bad. Comments so that we can also have reasonable generated documentation are good. Function prototypes fall in to that second category to me, with few exceptions (and that we have lots of bad examples isn't a good reason). The function names may well make it obvious what's going on but the comments allow for generated documentation to include that when explaining the not so obvious parts. Thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH RFC v2 3/5] lib: Add support for ECDSA image signing
Hi Alex, On Thu, 7 Jan 2021 at 09:27, Alex G. wrote: > > On 1/7/21 6:35 AM, Simon Glass wrote: > > Hi Alexandru, > > Hi Simon, > > (pun alert!) A lot of your comments have to do with comments. I use > comments as a tool to add something of value to code. When the code is > self-documenting, comments don't help much. > See kernel coding style chapter 8. > > What comments can do, is increase code size and break code flow -- there > is a cost to having them. I'm certain you've seen functions that need to > be scrolled up and down because of a huge comment smack in the middle. > I'll address individual comment comments below. > Don't get me started on comments in the kernel...there seems to almost be a ban on them :-) We used to follow the same approach but now we have comments for non-trivial code. Comments and tests are closely related. - if there is no comment, we don't know what the function is supposed to do so we can't change it (there is no contract), we are not sure if line 5 is a bug or a real quirk, casual readers can't understand what is going on, the automated docs don't produce anything, no one wants to refactor it, etc. - if there is no test, presumably the code doesn't work now, if it ever did > > > > On Wed, 30 Dec 2020 at 14:00, Alexandru Gagniuc > > wrote: > >> > >> mkimage supports rsa2048, and rsa4096 signatures. With newer silicon > >> now supporting hardware-accelerated ECDSA, it makes sense to expand > >> signing support to elliptic curves. > >> > >> Implement host-side ECDSA signing and verification with libcrypto. > >> Device-side implementation of signature verification is beyond the > >> scope of this patch. > >> > >> Signed-off-by: Alexandru Gagniuc > >> --- > >> common/image-sig.c | 14 +- > >> include/u-boot/ecdsa.h | 27 > >> lib/ecdsa/ecdsa-libcrypto.c | 300 > >> tools/Makefile | 3 + > >> 4 files changed, 342 insertions(+), 2 deletions(-) > >> create mode 100644 include/u-boot/ecdsa.h > >> create mode 100644 lib/ecdsa/ecdsa-libcrypto.c > >> > >> diff --git a/common/image-sig.c b/common/image-sig.c > >> index 21dafe6b91..2548b3eb0f 100644 > >> --- a/common/image-sig.c > >> +++ b/common/image-sig.c > >> @@ -15,6 +15,7 @@ > >> DECLARE_GLOBAL_DATA_PTR; > >> #endif /* !USE_HOSTCC*/ > >> #include > >> +#include > >> #include > >> #include > >> > >> @@ -82,8 +83,17 @@ struct crypto_algo crypto_algos[] = { > >> .sign = rsa_sign, > >> .add_verify_data = rsa_add_verify_data, > >> .verify = rsa_verify, > >> - } > >> - > >> + }, > >> +#if defined(USE_HOSTCC) > >> + /* Currently, only host support exists for ECDSA */ > >> + { > >> + .name = "ecdsa256", > >> + .key_len = ECDSA256_BYTES, > >> + .sign = ecdsa_sign, > >> + .add_verify_data = ecdsa_add_verify_data, > >> + .verify = ecdsa_verify, > >> + }, > >> +#endif > >> }; > >> > >> struct padding_algo padding_algos[] = { > >> diff --git a/include/u-boot/ecdsa.h b/include/u-boot/ecdsa.h > >> new file mode 100644 > >> index 00..a13a7267e1 > >> --- /dev/null > >> +++ b/include/u-boot/ecdsa.h > >> @@ -0,0 +1,27 @@ > >> +/* SPDX-License-Identifier: GPL-2.0+ */ > >> +/* > >> + * Copyright (c) 2020, Alexandru Gagniuc . > >> + */ > >> + > >> +#ifndef _ECDSA_H > >> +#define _ECDSA_H > >> + > >> +#include > >> +#include > >> + > >> +/** > >> + * crypto_algo API impementation for ECDSA; > >> + * @see "struct crypt_algo" > >> + * @{ > >> + */ > >> +int ecdsa_sign(struct image_sign_info *info, const struct image_region > >> region[], > >> + int region_count, uint8_t **sigp, uint *sig_len); > >> +int ecdsa_verify(struct image_sign_info *info, > >> +const struct image_region region[], int region_count, > >> +uint8_t *sig, uint sig_len); > >> +int ecdsa_add_verify_data(struct image_sign_info *info, void *keydest); > > > > Please always add full comments when you export functions, or have a > > non-trivial static function. > > I disagree. These functions implement and are designed to be used via > the crypt_algo API. One should understand the crypt_algo API, and any > deviations in behavior would be a bug. So there is no scenario in which > comments here would be useful. Please add full comments on exported function, no exceptions. Again, this is not Linux and people don't have as much time to cogitate on code. They are just trying to get their device working. > > >> +/** @} */ > >> + > >> +#define ECDSA256_BYTES (256 / 8) > >> + > >> +#endif > >> diff --git a/lib/ecdsa/ecdsa-libcrypto.c b/lib/ecdsa/ecdsa-libcrypto.c > >> new file mode 100644 > >> index 00..ff491411d0 > >> --- /dev/null > >> +++ b/lib/ecdsa/ecdsa-libcrypto.c > >> @@ -0,0 +1,300 @@ > >> +// SPDX-License-Identifier: GPL-2.0+ > >> +/* > >> + * ECDSA image signing implement
Re: [PATCH RFC v2 5/5] test/py: ecdsa: Add test for mkimage ECDSA signing
Hi Alex, On Thu, 7 Jan 2021 at 09:44, Alex G. wrote: > > > > On 1/7/21 6:35 AM, Simon Glass wrote: > > Hi Alexandru, > > > > On Wed, 30 Dec 2020 at 14:00, Alexandru Gagniuc > > wrote: > >> > >> Add a test to make sure that the ECDSA signatures generated by > >> mkimage can be verified successfully. pyCryptodomex was chosen as the > >> crypto library because it integrates much better with python code. > >> Using openssl would have been unnecessarily painful. > >> > >> Signed-off-by: Alexandru Gagniuc > >> --- > >> test/py/tests/test_fit_ecdsa.py | 111 > >> 1 file changed, 111 insertions(+) > >> create mode 100644 test/py/tests/test_fit_ecdsa.py > >> > > > > This test looks fine but the functions need full comments. I do think > > it might be worth putting the code in test_vboot, particularly when > > you get to the sandbox implementation. > > test_vboot seems to be testing the bootm command, while with this test It also runs fit_check_sign to check the signature. > I'm only looking to test the host-side (mkimage). In the next series, I > won't have a software implementation of ECDSA, like RSA_MOD_EXP. I will > use the ROM on the stm32mp. So there won't be somthing testable in the > sandbox. I'm not sure that is a good idea. With driver model you'll end up creating a ECDSA driver I suppose, so implementing it for sandbox should be possible. Is it a complicated algorithm? Without that, I'm not even sure how fit_check_sign could work? > [..] Regards, Simon
Re: [PATCH V3] net: dwc_eth_qos: Pad descriptors to cacheline size
On 1/7/21 5:33 PM, Stephen Warren wrote: On 1/7/21 3:12 AM, Marek Vasut wrote: The DWMAC4 IP has the possibility to skip up to 7 AXI bus width size words after the descriptor. Use this to pad the descriptors to cacheline size and remove the need for noncached memory altogether. Moreover, this lets Tegra use the generic cache flush / invalidate operations. Tested-by: Stephen Warren Reviewed-by: Stephen Warren Thanks. This also really needs a TB/RB from ST before this is applied.
Re: [PATCH RFC v2 5/5] test/py: ecdsa: Add test for mkimage ECDSA signing
On 1/7/21 11:31 AM, Simon Glass wrote: Hi Alex, On Thu, 7 Jan 2021 at 09:44, Alex G. wrote: On 1/7/21 6:35 AM, Simon Glass wrote: Hi Alexandru, On Wed, 30 Dec 2020 at 14:00, Alexandru Gagniuc wrote: Add a test to make sure that the ECDSA signatures generated by mkimage can be verified successfully. pyCryptodomex was chosen as the crypto library because it integrates much better with python code. Using openssl would have been unnecessarily painful. Signed-off-by: Alexandru Gagniuc --- test/py/tests/test_fit_ecdsa.py | 111 1 file changed, 111 insertions(+) create mode 100644 test/py/tests/test_fit_ecdsa.py This test looks fine but the functions need full comments. I do think it might be worth putting the code in test_vboot, particularly when you get to the sandbox implementation. test_vboot seems to be testing the bootm command, while with this test It also runs fit_check_sign to check the signature. Hmm, it backends on tools/check_fit_sign. Would be an interesting execrise to extend it ecdsa signatures, but that would take significantly more effort than the simple test I am proposing here. I'm only looking to test the host-side (mkimage). In the next series, I won't have a software implementation of ECDSA, like RSA_MOD_EXP. I will use the ROM on the stm32mp. So there won't be somthing testable in the sandbox. I'm not sure that is a good idea. With driver model you'll end up creating a ECDSA driver I suppose, so implementing it for sandbox should be possible. Is it a complicated algorithm? A software implementation of ECDSA is outside the scope of my project. Without that, I'm not even sure how fit_check_sign could work? It uses the ops->verify in ecdsa-libcrypto, does it not? [..] Regards, Simon
Re: [PATCH RFC v2 3/5] lib: Add support for ECDSA image signing
On 1/7/21 11:29 AM, Simon Glass wrote: Hi Alex, On Thu, 7 Jan 2021 at 09:27, Alex G. wrote: On 1/7/21 6:35 AM, Simon Glass wrote: Hi Alexandru, Hi Simon, (pun alert!) A lot of your comments have to do with comments. I use comments as a tool to add something of value to code. When the code is self-documenting, comments don't help much. See kernel coding style chapter 8. What comments can do, is increase code size and break code flow -- there is a cost to having them. I'm certain you've seen functions that need to be scrolled up and down because of a huge comment smack in the middle. I'll address individual comment comments below. Don't get me started on comments in the kernel...there seems to almost be a ban on them :-) We used to follow the same approach but now we have comments for non-trivial code. Comments and tests are closely related. - if there is no comment, we don't know what the function is supposed to do so we can't change it (there is no contract), we are not sure if line 5 is a bug or a real quirk, casual readers can't understand what is going on, the automated docs don't produce anything, no one wants to refactor it, etc. - if there is no test, presumably the code doesn't work now, if it ever did I will add each of the comments you are requesting, but not for the reasons quoted. The comments that I could write won't add anything of value -- they would just make the code larger, and increase the cognitive load. You are the maintainer, and starting an argument would be pointless. In the end, comments don't get compiled, and the code works just the same :) [snip] Again, this is not Linux and people don't have as much time to cogitate on code. I resent that statement. It takes me longer to do a task in u-boot than it would take me to do a similar task in linux. In the context of comments (note I intentionally did not say 'documentation'), does plastering the same information over and over in a way that hides the essence really help? I think the current path is misguided, and will only aggravate the problem. (I'll add the comments, though) They are just trying to get their device working. That's true for linux too. [snip] All other comments will be addressed in v3
Re: [PATCH v2] cmd: mmc: add mmcboot command
> I think that it's not good about building disk.c by default when CONFIG_CMD_MMC is enabled. Agree, its not an optimal solution. However, we chose to do it this way because: - It causes minimal change to the size of binary (~200 bytes). - To be consistent with upstream code where it is built similarly for usb/scsi etc. -- Sent from: http://u-boot.10912.n7.nabble.com/
Re: [PATCH RFC v2 3/5] lib: Add support for ECDSA image signing
On 1/7/21 11:25 AM, Tom Rini wrote: On Thu, Jan 07, 2021 at 10:27:50AM -0600, Alex G. wrote: On 1/7/21 6:35 AM, Simon Glass wrote: Hi Alexandru, Hi Simon, (pun alert!) A lot of your comments have to do with comments. I use comments as a tool to add something of value to code. When the code is self-documenting, comments don't help much. See kernel coding style chapter 8. Comments for comments sake are bad. Comments so that we can also have reasonable generated documentation are good. Function prototypes fall in to that second category to me, with few exceptions (and that we have lots of bad examples isn't a good reason). The function names may well make it obvious what's going on but the comments allow for generated documentation to include that when explaining the not so obvious parts. Thanks! The problem with generated documentation is that it's not very useful. People add comments for the sake of comments to have something generated. Most often you end up with a detailed description of all the details, but almost never the big picture. Nowadays, I don't even waste my time reading generated documentation. I am getting ready to send the new series with the goodies requested by Simon. I don't find the new comments to be useful, and I find some to spread out the code such that it hurts readability. They will be there because you and Simon asked nicely, although I think you're wrong. Alex
[PATCH v3 0/6] Add support for ECDSA image signing (with test)
## Purpose and intent The ROM code on the STM32MP requires an ECDSA-signed FSBL. Maintaining verified boot through FIT images would require switching to an RSA key after SPL. This would be stupid, so this series is focused on enabling ECDSA signing. The use case that I am focused on is signing an existing FIT image: mkimage -F some-existing.fit --signing-key some/key.pem I don't care about signing while assembling the FIT. The reason is that I want the machine that builds things to be separate from the machine that has access to the super-secret-key.pem. The purpose of this series is to lay the foundations for ECDSA FIT signature verification on STM32MP. # Implementation ## Signing I initially tried to model this after the RSA implementation (rsa-sign.c), but that didn't go well for a few reasons: (a) The openssl/libcrypto API is a pain in the ass (b) The RSA path doesn't have a way to pass a specific key file. On point (a), I don't want to spend too much time battling a C API for crypto. I find pyCryptodomex to be vastly superior, but that is not available for mkimage. I am thus focusing on the simple case of key in, signature out. On point (b), the RSA path takes the FDT property 'key-name-hint' to decide which key file to read from disk. In the context of "which fdt node describes my signing key", this makes sense. On the other hand, 'key-name-hint' is also used as the basename of where the key is on the filesystem. This leads to some funny search paths, such as "some/dir/(null).key" So I am using the -K option to mkimage as the _full_ path to the key file. It doesn't have to be named .key, it doesn't have to be named .crt, and it doesn't have to exist in a particular directory (as is the case for the RSA path). Take that as is for here -- we can discuss the merits of this in a separate thread. ## Verification This will be implemented in a future patch series. The proof of concept is on github: https://github.com/mrnuke/u-boot/commits/patch-stm32-ecdsa # Testing test/py/tests/test_fit_ecdsa.py is implementing a test for mkimage. It lets mkimage run wild, tehn verifies the signature against pyCryptodomex -- see earlier point on for I didn't use openssl. # Things not yet resolved: - is mkimage '-k' supposed to be a directory or file path I'm hoping I can postpone answering this question pending further discussion. # Changes since v1 and v2: - Added lots of function comments - Replaced hardcoded error numbers with more meaningful errno numbers - CHanged some error paths to use 'return log_msg_ret' Alexandru Gagniuc (6): lib: Rename rsa-checksum.c to hash-checksum.c lib/rsa: Make fdt_add_bignum() available outside of RSA code lib: Add support for ECDSA image signing doc: signature.txt: Document devicetree format for ECDSA keys test/py: Add pycryptodomex to list of required pakages test/py: ecdsa: Add test for mkimage ECDSA signing common/image-fit-sig.c| 2 +- common/image-sig.c| 13 +- doc/uImage.FIT/signature.txt | 7 +- include/image.h | 5 +- include/u-boot/ecdsa.h| 94 ++ include/u-boot/fdt-libcrypto.h| 27 ++ .../{rsa-checksum.h => hash-checksum.h} | 0 lib/Makefile | 1 + lib/crypto/pkcs7_verify.c | 2 +- lib/crypto/x509_public_key.c | 2 +- lib/ecdsa/ecdsa-libcrypto.c | 306 ++ lib/fdt-libcrypto.c | 72 + lib/{rsa/rsa-checksum.c => hash-checksum.c} | 3 +- lib/rsa/Makefile | 2 +- lib/rsa/rsa-sign.c| 65 +--- test/py/requirements.txt | 1 + test/py/tests/test_fit_ecdsa.py | 111 +++ tools/Makefile| 7 +- 18 files changed, 645 insertions(+), 75 deletions(-) create mode 100644 include/u-boot/ecdsa.h create mode 100644 include/u-boot/fdt-libcrypto.h rename include/u-boot/{rsa-checksum.h => hash-checksum.h} (100%) create mode 100644 lib/ecdsa/ecdsa-libcrypto.c create mode 100644 lib/fdt-libcrypto.c rename lib/{rsa/rsa-checksum.c => hash-checksum.c} (96%) create mode 100644 test/py/tests/test_fit_ecdsa.py -- 2.26.2
[PATCH v3 1/6] lib: Rename rsa-checksum.c to hash-checksum.c
rsa-checksum.c sontains the hash_calculate() implementations. Despite the "rsa-" file prefix, this function is useful for other algorithms. To prevent confusion, move this file to lib/crypto, and rename it to hash-checksum.c, to give it a more "generic" feel. Signed-off-by: Alexandru Gagniuc --- common/image-fit-sig.c | 2 +- common/image-sig.c | 2 +- include/image.h| 2 +- include/u-boot/{rsa-checksum.h => hash-checksum.h} | 0 lib/Makefile | 1 + lib/crypto/pkcs7_verify.c | 2 +- lib/crypto/x509_public_key.c | 2 +- lib/{rsa/rsa-checksum.c => hash-checksum.c}| 3 ++- lib/rsa/Makefile | 2 +- tools/Makefile | 3 ++- 10 files changed, 11 insertions(+), 8 deletions(-) rename include/u-boot/{rsa-checksum.h => hash-checksum.h} (100%) rename lib/{rsa/rsa-checksum.c => hash-checksum.c} (96%) diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c index 5401d9411b..7fcbb47235 100644 --- a/common/image-fit-sig.c +++ b/common/image-fit-sig.c @@ -15,7 +15,7 @@ DECLARE_GLOBAL_DATA_PTR; #include #include #include -#include +#include #define IMAGE_MAX_HASHED_NODES 100 diff --git a/common/image-sig.c b/common/image-sig.c index f3c209ae8b..21dafe6b91 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -16,7 +16,7 @@ DECLARE_GLOBAL_DATA_PTR; #endif /* !USE_HOSTCC*/ #include #include -#include +#include #define IMAGE_MAX_HASHED_NODES 100 diff --git a/include/image.h b/include/image.h index 41473dbb9c..a55b11b3ae 100644 --- a/include/image.h +++ b/include/image.h @@ -1258,7 +1258,7 @@ struct image_region { }; #if IMAGE_ENABLE_VERIFY -# include +# include #endif struct checksum_algo { const char *name; diff --git a/include/u-boot/rsa-checksum.h b/include/u-boot/hash-checksum.h similarity index 100% rename from include/u-boot/rsa-checksum.h rename to include/u-boot/hash-checksum.h diff --git a/lib/Makefile b/lib/Makefile index 851a80ef3b..cf64188ba5 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -60,6 +60,7 @@ endif obj-$(CONFIG_$(SPL_)ACPIGEN) += acpi/ obj-$(CONFIG_$(SPL_)MD5) += md5.o obj-$(CONFIG_$(SPL_)RSA) += rsa/ +obj-$(CONFIG_FIT_SIGNATURE) += hash-checksum.o obj-$(CONFIG_SHA1) += sha1.o obj-$(CONFIG_SHA256) += sha256.o obj-$(CONFIG_SHA512_ALGO) += sha512.o diff --git a/lib/crypto/pkcs7_verify.c b/lib/crypto/pkcs7_verify.c index 320ba49f79..3c411f651f 100644 --- a/lib/crypto/pkcs7_verify.c +++ b/lib/crypto/pkcs7_verify.c @@ -15,7 +15,7 @@ #include #include #include -#include +#include #include #include #else diff --git a/lib/crypto/x509_public_key.c b/lib/crypto/x509_public_key.c index 91810a8640..d557ab27ae 100644 --- a/lib/crypto/x509_public_key.c +++ b/lib/crypto/x509_public_key.c @@ -19,7 +19,7 @@ #include #ifdef __UBOOT__ #include -#include +#include #else #include #include diff --git a/lib/rsa/rsa-checksum.c b/lib/hash-checksum.c similarity index 96% rename from lib/rsa/rsa-checksum.c rename to lib/hash-checksum.c index e60debb7df..d732ecc38f 100644 --- a/lib/rsa/rsa-checksum.c +++ b/lib/hash-checksum.c @@ -13,7 +13,8 @@ #else #include "fdt_host.h" #endif -#include +#include +#include int hash_calculate(const char *name, const struct image_region region[], diff --git a/lib/rsa/Makefile b/lib/rsa/Makefile index 8b75d41f04..c9ac72c1e2 100644 --- a/lib/rsa/Makefile +++ b/lib/rsa/Makefile @@ -5,6 +5,6 @@ # (C) Copyright 2000-2007 # Wolfgang Denk, DENX Software Engineering, w...@denx.de. -obj-$(CONFIG_$(SPL_TPL_)RSA_VERIFY) += rsa-verify.o rsa-checksum.o +obj-$(CONFIG_$(SPL_TPL_)RSA_VERIFY) += rsa-verify.o obj-$(CONFIG_$(SPL_TPL_)RSA_VERIFY_WITH_PKEY) += rsa-keyprop.o obj-$(CONFIG_RSA_SOFTWARE_EXP) += rsa-mod-exp.o diff --git a/tools/Makefile b/tools/Makefile index 253a6b9706..b1595ad814 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -67,7 +67,7 @@ LIBFDT_OBJS := $(addprefix libfdt/, fdt.o fdt_ro.o fdt_wip.o fdt_sw.o fdt_rw.o \ fdt_strerror.o fdt_empty_tree.o fdt_addresses.o fdt_overlay.o) RSA_OBJS-$(CONFIG_FIT_SIGNATURE) := $(addprefix lib/rsa/, \ - rsa-sign.o rsa-verify.o rsa-checksum.o \ + rsa-sign.o rsa-verify.o \ rsa-mod-exp.o) AES_OBJS-$(CONFIG_FIT_CIPHER) := $(addprefix lib/aes/, \ @@ -105,6 +105,7 @@ dumpimage-mkimage-objs := aisimage.o \ $(ROCKCHIP_OBS) \ socfpgaimage.o \ lib/crc16.o \ + lib/hash-checksum.o \ lib/sha1.o \ lib/sha256.o \ lib/sha512.o \ -- 2.26.2
[PATCH v3 2/6] lib/rsa: Make fdt_add_bignum() available outside of RSA code
fdt_add_bignum() is useful for algorithms other than just RSA. To allow its use for ECDSA, move it to a common file under lib/. The new file is suffixed with '-libcrypto' because it has a direct dependency on openssl. This is due to the use of the "BIGNUM *" type. Signed-off-by: Alexandru Gagniuc --- include/u-boot/fdt-libcrypto.h | 27 + lib/fdt-libcrypto.c| 72 ++ lib/rsa/rsa-sign.c | 65 +- tools/Makefile | 1 + 4 files changed, 101 insertions(+), 64 deletions(-) create mode 100644 include/u-boot/fdt-libcrypto.h create mode 100644 lib/fdt-libcrypto.c diff --git a/include/u-boot/fdt-libcrypto.h b/include/u-boot/fdt-libcrypto.h new file mode 100644 index 00..5142f37039 --- /dev/null +++ b/include/u-boot/fdt-libcrypto.h @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2020, Alexandru Gagniuc + * Copyright (c) 2013, Google Inc. + */ + +#ifndef _FDT_LIBCRYPTO_H +#define _FDT_LIBCRYPTO_H + +#include + +/** + * fdt_add_bignum() - Write a libcrypto BIGNUM as an FDT property + * + * Convert a libcrypto BIGNUM * into a big endian array of integers. + * + * @blob: FDT blob to modify + * @noffset: Offset of the FDT node + * @prop_name: What to call the property in the FDT + * @num: pointer to a libcrypto big number + * @num_bits: How big is 'num' in bits? + * @return 0 if all good all working, -ve on horror + */ +int fdt_add_bignum(void *blob, int noffset, const char *prop_name, + BIGNUM *num, int num_bits); + +#endif /* _FDT_LIBCRYPTO_H */ diff --git a/lib/fdt-libcrypto.c b/lib/fdt-libcrypto.c new file mode 100644 index 00..ecb0344c8f --- /dev/null +++ b/lib/fdt-libcrypto.c @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2020, Alexandru Gagniuc + * Copyright (c) 2013, Google Inc. + */ + +#include +#include + +int fdt_add_bignum(void *blob, int noffset, const char *prop_name, + BIGNUM *num, int num_bits) +{ + int nwords = num_bits / 32; + int size; + uint32_t *buf, *ptr; + BIGNUM *tmp, *big2, *big32, *big2_32; + BN_CTX *ctx; + int ret; + + tmp = BN_new(); + big2 = BN_new(); + big32 = BN_new(); + big2_32 = BN_new(); + + /* +* Note: This code assumes that all of the above succeed, or all fail. +* In practice memory allocations generally do not fail (unless the +* process is killed), so it does not seem worth handling each of these +* as a separate case. Technicaly this could leak memory on failure, +* but a) it won't happen in practice, and b) it doesn't matter as we +* will immediately exit with a failure code. +*/ + if (!tmp || !big2 || !big32 || !big2_32) { + fprintf(stderr, "Out of memory (bignum)\n"); + return -ENOMEM; + } + ctx = BN_CTX_new(); + if (!ctx) { + fprintf(stderr, "Out of memory (bignum context)\n"); + return -ENOMEM; + } + BN_set_word(big2, 2L); + BN_set_word(big32, 32L); + BN_exp(big2_32, big2, big32, ctx); /* B = 2^32 */ + + size = nwords * sizeof(uint32_t); + buf = malloc(size); + if (!buf) { + fprintf(stderr, "Out of memory (%d bytes)\n", size); + return -ENOMEM; + } + + /* Write out modulus as big endian array of integers */ + for (ptr = buf + nwords - 1; ptr >= buf; ptr--) { + BN_mod(tmp, num, big2_32, ctx); /* n = N mod B */ + *ptr = cpu_to_fdt32(BN_get_word(tmp)); + BN_rshift(num, num, 32); /* N = N/B */ + } + + /* +* We try signing with successively increasing size values, so this +* might fail several times +*/ + ret = fdt_setprop(blob, noffset, prop_name, buf, size); + free(buf); + BN_free(tmp); + BN_free(big2); + BN_free(big32); + BN_free(big2_32); + + return ret ? -FDT_ERR_NOSPACE : 0; +} diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c index 1f0d81bd7a..557c690a6d 100644 --- a/lib/rsa/rsa-sign.c +++ b/lib/rsa/rsa-sign.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -680,70 +681,6 @@ int rsa_get_params(RSA *key, uint64_t *exponent, uint32_t *n0_invp, return ret; } -static int fdt_add_bignum(void *blob, int noffset, const char *prop_name, - BIGNUM *num, int num_bits) -{ - int nwords = num_bits / 32; - int size; - uint32_t *buf, *ptr; - BIGNUM *tmp, *big2, *big32, *big2_32; - BN_CTX *ctx; - int ret; - - tmp = BN_new(); - big2 = BN_new(); - big32 = BN_new(); - big2_32 = BN_new(); - - /* -* Note: This code assumes that all of the above succeed, or all fail. -* In practice me
[PATCH v3 4/6] doc: signature.txt: Document devicetree format for ECDSA keys
Signed-off-by: Alexandru Gagniuc --- doc/uImage.FIT/signature.txt | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/doc/uImage.FIT/signature.txt b/doc/uImage.FIT/signature.txt index a3455889ed..0139295d33 100644 --- a/doc/uImage.FIT/signature.txt +++ b/doc/uImage.FIT/signature.txt @@ -142,7 +142,7 @@ public key in U-Boot's control FDT (using CONFIG_OF_CONTROL). Public keys should be stored as sub-nodes in a /signature node. Required properties are: -- algo: Algorithm name (e.g. "sha1,rsa2048") +- algo: Algorithm name (e.g. "sha1,rsa2048" or "sha256,ecdsa256") Optional properties are: @@ -167,6 +167,11 @@ For RSA the following are mandatory: - rsa,r-squared: (2^num-bits)^2 as a big-endian multi-word integer - rsa,n0-inverse: -1 / modulus[0] mod 2^32 +For ECDSA the following are mandatory: +- ecdsa,curve: Name of ECDSA curve (e.g. "prime256v1") +- ecdsa,x-point: Public key X coordinate as a big-endian multi-word integer +- ecdsa,y-point: Public key Y coordinate as a big-endian multi-word integer + These parameters can be added to a binary device tree using parameter -K of the mkimage command:: -- 2.26.2
[PATCH v3 5/6] test/py: Add pycryptodomex to list of required pakages
We wish to use pycryptodomex to verify code paths involving ECDSA signatures. Add it to requirements.txt so that they get picked up automatically .gitlab and .azure tasks Signed-off-by: Alexandru Gagniuc --- test/py/requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/test/py/requirements.txt b/test/py/requirements.txt index cf251186f4..5a25a9d5d5 100644 --- a/test/py/requirements.txt +++ b/test/py/requirements.txt @@ -10,6 +10,7 @@ packaging==19.2 pbr==5.4.3 pluggy==0.13.0 py==1.8.0 +pycryptodomex==3.9.8 pyparsing==2.4.2 pytest==5.2.1 python-mimeparse==1.6.0 -- 2.26.2
[PATCH v3 3/6] lib: Add support for ECDSA image signing
mkimage supports rsa2048, and rsa4096 signatures. With newer silicon now supporting hardware-accelerated ECDSA, it makes sense to expand signing support to elliptic curves. Implement host-side ECDSA signing and verification with libcrypto. Device-side implementation of signature verification is beyond the scope of this patch. Signed-off-by: Alexandru Gagniuc --- common/image-sig.c | 11 +- include/image.h | 3 + include/u-boot/ecdsa.h | 94 +++ lib/ecdsa/ecdsa-libcrypto.c | 306 tools/Makefile | 3 + 5 files changed, 415 insertions(+), 2 deletions(-) create mode 100644 include/u-boot/ecdsa.h create mode 100644 lib/ecdsa/ecdsa-libcrypto.c diff --git a/common/image-sig.c b/common/image-sig.c index 21dafe6b91..4b7d01f6c2 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -15,6 +15,7 @@ DECLARE_GLOBAL_DATA_PTR; #endif /* !USE_HOSTCC*/ #include +#include #include #include @@ -82,8 +83,14 @@ struct crypto_algo crypto_algos[] = { .sign = rsa_sign, .add_verify_data = rsa_add_verify_data, .verify = rsa_verify, - } - + }, + { + .name = "ecdsa256", + .key_len = ECDSA256_BYTES, + .sign = ecdsa_sign, + .add_verify_data = ecdsa_add_verify_data, + .verify = ecdsa_verify, + }, }; struct padding_algo padding_algos[] = { diff --git a/include/image.h b/include/image.h index a55b11b3ae..6628173dca 100644 --- a/include/image.h +++ b/include/image.h @@ -1199,16 +1199,19 @@ int calculate_hash(const void *data, int data_len, const char *algo, # if defined(CONFIG_FIT_SIGNATURE) # define IMAGE_ENABLE_SIGN1 # define IMAGE_ENABLE_VERIFY 1 +# define IMAGE_ENABLE_VERIFY_ECDSA1 # define FIT_IMAGE_ENABLE_VERIFY 1 # include # else # define IMAGE_ENABLE_SIGN0 # define IMAGE_ENABLE_VERIFY 0 +# define IMAGE_ENABLE_VERIFY_ECDSA 0 # define FIT_IMAGE_ENABLE_VERIFY 0 # endif #else # define IMAGE_ENABLE_SIGN 0 # define IMAGE_ENABLE_VERIFY CONFIG_IS_ENABLED(RSA_VERIFY) +# define IMAGE_ENABLE_VERIFY_ECDSA 0 # define FIT_IMAGE_ENABLE_VERIFY CONFIG_IS_ENABLED(FIT_SIGNATURE) #endif diff --git a/include/u-boot/ecdsa.h b/include/u-boot/ecdsa.h new file mode 100644 index 00..979690d966 --- /dev/null +++ b/include/u-boot/ecdsa.h @@ -0,0 +1,94 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2020, Alexandru Gagniuc . + */ + +#ifndef _ECDSA_H +#define _ECDSA_H + +#include +#include +#include + +/** + * crypto_algo API impementation for ECDSA; + * @see "struct crypto_algo" + * @{ + */ +#if IMAGE_ENABLE_SIGN +/** + * sign() - calculate and return signature for given input data + * + * @info: Specifies key and FIT information + * @data: Pointer to the input data + * @data_len: Data length + * @sigp: Set to an allocated buffer holding the signature + * @sig_len: Set to length of the calculated hash + * + * This computes input data signature according to selected algorithm. + * Resulting signature value is placed in an allocated buffer, the + * pointer is returned as *sigp. The length of the calculated + * signature is returned via the sig_len pointer argument. The caller + * should free *sigp. + * + * @return: 0, on success, -ve on error + */ +int ecdsa_sign(struct image_sign_info *info, const struct image_region region[], + int region_count, uint8_t **sigp, uint *sig_len); + +/** + * add_verify_data() - Add verification information to FDT + * + * Add public key information to the FDT node, suitable for + * verification at run-time. The information added depends on the + * algorithm being used. I just copypasted this from rsa.h. + * + * @info: Specifies key and FIT information + * @keydest: Destination FDT blob for public key data + * @return: 0, on success, -ENOSPC if the keydest FDT blob ran out of space, + * other -ve value on error + */ +int ecdsa_add_verify_data(struct image_sign_info *info, void *keydest); +#else +static inline +int ecdsa_sign(struct image_sign_info *info, const struct image_region region[], + int region_count, uint8_t **sigp, uint *sig_len) +{ + return -ENXIO; +} + +static inline +int ecdsa_add_verify_data(struct image_sign_info *info, void *keydest) +{ + return -ENXIO; +} +#endif + +#if IMAGE_ENABLE_VERIFY_ECDSA +/** + * verify() - Verify a signature against some data + * + * @info: Specifies key and FIT information + * @data: Pointer to the input data + * @data_len: Data length + * @sig: Signature + * @sig_len: Number of bytes in signature + * @return 0 if verified, -ve on error + */ +int ecdsa_verify(struct image_sign_info *info, +const struct image_region region[], int region_count, +uint8_t *sig, uint sig_len); +#else +static inline +int ecdsa_verify(struc
[PATCH v3 6/6] test/py: ecdsa: Add test for mkimage ECDSA signing
Add a test to make sure that the ECDSA signatures generated by mkimage can be verified successfully. pyCryptodomex was chosen as the crypto library because it integrates much better with python code. Using openssl would have been unnecessarily painful. Signed-off-by: Alexandru Gagniuc --- test/py/tests/test_fit_ecdsa.py | 111 1 file changed, 111 insertions(+) create mode 100644 test/py/tests/test_fit_ecdsa.py diff --git a/test/py/tests/test_fit_ecdsa.py b/test/py/tests/test_fit_ecdsa.py new file mode 100644 index 00..6cadb0cbb5 --- /dev/null +++ b/test/py/tests/test_fit_ecdsa.py @@ -0,0 +1,111 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (c) 2020,2021 Alexandru Gagniuc + +""" +Test ECDSA signing of FIT images + +This test uses mkimage to sign an existing FIT image with an ECDSA key. The +signature is then extracted, and verified against pyCryptodome. +This test doesn't run the sandbox. It only checks the host tool 'mkimage' +""" + +import pytest +import u_boot_utils as util +from Cryptodome.Hash import SHA256 +from Cryptodome.PublicKey import ECC +from Cryptodome.Signature import DSS + +class SignableFitImage(object): +""" Helper to manipulate a FIT image on disk """ +def __init__(self, cons, file_name): +self.fit = file_name +self.cons = cons +self.signable_nodes = set() + +def __fdt_list(self, path): +return util.run_and_log(self.cons, f'fdtget -l {self.fit} {path}') + +def __fdt_set(self, node, **prop_value): +for prop, value in prop_value.items(): +util.run_and_log(self.cons, f'fdtput -ts {self.fit} {node} {prop} {value}') + +def __fdt_get_binary(self, node, prop): +numbers = util.run_and_log(self.cons, f'fdtget -tbi {self.fit} {node} {prop}') + +bignum = bytearray() +for little_num in numbers.split(): +bignum.append(int(little_num)) + +return bignum + +def find_signable_image_nodes(self): +for node in self.__fdt_list('/images').split(): +image = f'/images/{node}' +if 'signature' in self.__fdt_list(image): +self.signable_nodes.add(image) + +return self.signable_nodes + +def change_signature_algo_to_ecdsa(self): +for image in self.signable_nodes: +self.__fdt_set(f'{image}/signature', algo='sha256,ecdsa256') + +def sign(self, mkimage, key_file): +util.run_and_log(self.cons, [mkimage, '-F', self.fit, f'-k{key_file}']) + +def check_signatures(self, key): +for image in self.signable_nodes: +raw_sig = self.__fdt_get_binary(f'{image}/signature', 'value') +raw_bin = self.__fdt_get_binary(image, 'data') + +sha = SHA256.new(raw_bin) +verifier = DSS.new(key, 'fips-186-3') +verifier.verify(sha, bytes(raw_sig)) + + +@pytest.mark.buildconfigspec('fit_signature') +@pytest.mark.requiredtool('dtc') +@pytest.mark.requiredtool('fdtget') +@pytest.mark.requiredtool('fdtput') +def test_fit_ecdsa(u_boot_console): +""" Test that signatures generated by mkimage are legible. """ +def generate_ecdsa_key(): +return ECC.generate(curve='prime256v1') + +def assemble_fit_image(dest_fit, its, destdir): +dtc_args = f'-I dts -O dtb -i {destdir}' +util.run_and_log(cons, [mkimage, '-D', dtc_args, '-f', its, dest_fit]) + +def dtc(dts): +dtb = dts.replace('.dts', '.dtb') +util.run_and_log(cons, f'dtc {datadir}/{dts} -O dtb -o {tempdir}/{dtb}') + +cons = u_boot_console +mkimage = cons.config.build_dir + '/tools/mkimage' +datadir = cons.config.source_dir + '/test/py/tests/vboot/' +tempdir = cons.config.result_dir +key_file = f'{tempdir}/ecdsa-test-key.pem' +fit_file = f'{tempdir}/test.fit' +dtc('sandbox-kernel.dts') + +key = generate_ecdsa_key() + +# Create a fake kernel image -- zeroes will do just fine +with open(f'{tempdir}/test-kernel.bin', 'w') as fd: +fd.write(500 * chr(0)) + +# invokations of mkimage expect to read the key from disk +with open(key_file, 'w') as f: +f.write(key.export_key(format='PEM')) + +assemble_fit_image(fit_file, f'{datadir}/sign-images-sha256.its', tempdir) + +fit = SignableFitImage(cons, fit_file) +nodes = fit.find_signable_image_nodes() +if len(nodes) == 0: +raise ValueError('FIT image has no "/image" nodes with "signature"') + +fit.change_signature_algo_to_ecdsa() +fit.sign(mkimage, key_file) +fit.check_signatures(key) -- 2.26.2
Re: [PATCH v5 1/3] sunxi: board: extract creating a unique sid into a helper function
On 03/01/2021 18:36, Jernej Skrabec wrote: > From: Andre Heider > > Refactor setup_environment() so we can use the created sid for a > Bluetooth address too. > > Signed-off-by: Andre Heider > Acked-by: Maxime Ripard > [rebased] > Signed-off-by: Jernej Skrabec Confirmed to be indeed just moving this code out and reversing the check. Reviewed-by: Andre Przywara Cheers, Andre > --- > board/sunxi/board.c | 121 > 1 file changed, 66 insertions(+), 55 deletions(-) > > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > index 708a27ed78e9..4a29e351141b 100644 > --- a/board/sunxi/board.c > +++ b/board/sunxi/board.c > @@ -789,6 +789,38 @@ static void parse_spl_header(const uint32_t spl_addr) > env_set_hex("fel_scriptaddr", spl->fel_script_address); > } > > +static bool get_unique_sid(unsigned int *sid) > +{ > + if (sunxi_get_sid(sid) != 0) > + return false; > + > + if (!sid[0]) > + return false; > + > + /* > + * The single words 1 - 3 of the SID have quite a few bits > + * which are the same on many models, so we take a crc32 > + * of all 3 words, to get a more unique value. > + * > + * Note we only do this on newer SoCs as we cannot change > + * the algorithm on older SoCs since those have been using > + * fixed mac-addresses based on only using word 3 for a > + * long time and changing a fixed mac-address with an > + * u-boot update is not good. > + */ > +#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \ > +!defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \ > +!defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33) > + sid[3] = crc32(0, (unsigned char *)&sid[1], 12); > +#endif > + > + /* Ensure the NIC specific bytes of the mac are not all 0 */ > + if ((sid[3] & 0xff) == 0) > + sid[3] |= 0x80; > + > + return true; > +} > + > /* > * Note this function gets called multiple times. > * It must not make any changes to env variables which already exist. > @@ -799,61 +831,40 @@ static void setup_environment(const void *fdt) > unsigned int sid[4]; > uint8_t mac_addr[6]; > char ethaddr[16]; > - int i, ret; > - > - ret = sunxi_get_sid(sid); > - if (ret == 0 && sid[0] != 0) { > - /* > - * The single words 1 - 3 of the SID have quite a few bits > - * which are the same on many models, so we take a crc32 > - * of all 3 words, to get a more unique value. > - * > - * Note we only do this on newer SoCs as we cannot change > - * the algorithm on older SoCs since those have been using > - * fixed mac-addresses based on only using word 3 for a > - * long time and changing a fixed mac-address with an > - * u-boot update is not good. > - */ > -#if !defined(CONFIG_MACH_SUN4I) && !defined(CONFIG_MACH_SUN5I) && \ > -!defined(CONFIG_MACH_SUN6I) && !defined(CONFIG_MACH_SUN7I) && \ > -!defined(CONFIG_MACH_SUN8I_A23) && !defined(CONFIG_MACH_SUN8I_A33) > - sid[3] = crc32(0, (unsigned char *)&sid[1], 12); > -#endif > - > - /* Ensure the NIC specific bytes of the mac are not all 0 */ > - if ((sid[3] & 0xff) == 0) > - sid[3] |= 0x80; > - > - for (i = 0; i < 4; i++) { > - sprintf(ethaddr, "ethernet%d", i); > - if (!fdt_get_alias(fdt, ethaddr)) > - continue; > - > - if (i == 0) > - strcpy(ethaddr, "ethaddr"); > - else > - sprintf(ethaddr, "eth%daddr", i); > - > - if (env_get(ethaddr)) > - continue; > - > - /* Non OUI / registered MAC address */ > - mac_addr[0] = (i << 4) | 0x02; > - mac_addr[1] = (sid[0] >> 0) & 0xff; > - mac_addr[2] = (sid[3] >> 24) & 0xff; > - mac_addr[3] = (sid[3] >> 16) & 0xff; > - mac_addr[4] = (sid[3] >> 8) & 0xff; > - mac_addr[5] = (sid[3] >> 0) & 0xff; > - > - eth_env_set_enetaddr(ethaddr, mac_addr); > - } > - > - if (!env_get("serial#")) { > - snprintf(serial_string, sizeof(serial_string), > - "%08x%08x", sid[0], sid[3]); > - > - env_set("serial#", serial_string); > - } > + int i; > + > + if (!get_unique_sid(sid)) > + return; > + > + for (i = 0; i < 4; i++) { > + sprintf(ethaddr, "ethernet%d", i); > + if (!fdt_get_alias(fdt, ethaddr)) > + continue; > + > + if (i == 0) > +
Re: [PATCH v5 2/3] arm: sunxi: add a config option to fixup a Bluetooth address
On 03/01/2021 18:36, Jernej Skrabec wrote: Hi, > From: Andre Heider > > Some Bluetooth controllers, like the BCM4345C5 of the Orange Pi 3, > ship with the controller default address. > > Add a config option to fix it up so it can function properly. > > Signed-off-by: Andre Heider > Tested-by: Ondrej Jirman > Acked-by: Maxime Ripard > [rebased] > Signed-off-by: Jernej Skrabec > --- > arch/arm/mach-sunxi/Kconfig | 11 +++ > board/sunxi/board.c | 34 ++ > 2 files changed, 45 insertions(+) > > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig > index 49ef217f08c0..269aef5f01a1 100644 > --- a/arch/arm/mach-sunxi/Kconfig > +++ b/arch/arm/mach-sunxi/Kconfig > @@ -1016,4 +1016,15 @@ config PINEPHONE_DT_SELECTION > Enable this option to automatically select the device tree for the > correct PinePhone hardware revision during boot. > > +config FIXUP_BDADDR This naming is a bit generic, and the BD in there is slightly confusing. I understand that this is the official term used in Bluetooth land, but it would be helpful to put this piece of context into the symbol name as well. Especially since "bd" is used for "board" all over the place in U-Boot. Maybe stay closer to the actual content of the symbol, which is the DT compatible *name*, and *not* an address? BLUETOOTH_DT_DEVICE_FIXUP or the like? > + string "Fixup the Bluetooth controller address" > + default "" > + help > + This option specifies the DT compatible name of the Bluetooth > + controller for which to set the "local-bd-address" property. > + Set this option if your device ships with the Bluetooth controller > + default address. > + The used address is "bdaddr" if set, and "ethaddr" with the LSB > + flipped elsewise. > + > endif > diff --git a/board/sunxi/board.c b/board/sunxi/board.c > index 4a29e351141b..d19119b7eb36 100644 > --- a/board/sunxi/board.c > +++ b/board/sunxi/board.c > @@ -908,6 +908,38 @@ int misc_init_r(void) > return 0; > } > > +static void fixup_bd_address(void *blob) similar comment to the naming in this function, to avoid the "bd" in here. The code itself looks fine, it's really just about the naming. The variable names inside the function are fine, since the context is clear. Thanks! Andre > +{ > + /* Some devices ship with a Bluetooth controller default address. > + * Set a valid address through the device tree. > + */ > + uchar tmp[ETH_ALEN], bdaddr[ETH_ALEN]; > + unsigned int sid[4]; > + int i; > + > + if (!CONFIG_FIXUP_BDADDR[0]) > + return; > + > + if (eth_env_get_enetaddr("bdaddr", tmp)) { > + /* Convert between the binary formats of the corresponding > stacks */ > + for (i = 0; i < ETH_ALEN; ++i) > + bdaddr[i] = tmp[ETH_ALEN - i - 1]; > + } else { > + if (!get_unique_sid(sid)) > + return; > + > + bdaddr[0] = ((sid[3] >> 0) & 0xff) ^ 1; > + bdaddr[1] = (sid[3] >> 8) & 0xff; > + bdaddr[2] = (sid[3] >> 16) & 0xff; > + bdaddr[3] = (sid[3] >> 24) & 0xff; > + bdaddr[4] = (sid[0] >> 0) & 0xff; > + bdaddr[5] = 0x02; > + } > + > + do_fixup_by_compat(blob, CONFIG_FIXUP_BDADDR, > +"local-bd-address", bdaddr, ETH_ALEN, 1); > +} > + > int ft_board_setup(void *blob, struct bd_info *bd) > { > int __maybe_unused r; > @@ -918,6 +950,8 @@ int ft_board_setup(void *blob, struct bd_info *bd) >*/ > setup_environment(blob); > > + fixup_bd_address(blob); > + > #ifdef CONFIG_VIDEO_DT_SIMPLEFB > r = sunxi_simplefb_setup(blob); > if (r) >
Re: [PATCH v5 3/3] arm64: dts: sun50i: Add support for Orange Pi 3
On 03/01/2021 18:36, Jernej Skrabec wrote: > From: Andre Heider Hi, > dts file is taken from Linux 5.11-rc1 tag. > > The Bluetooth controller of this device ships with a default address, > use the new CONFIG_FIXUP_BDADDR option to fix it up. > > Signed-off-by: Andre Heider > Acked-by: Maxime Ripard > [Updated OrangePi 3 DT, rebase and config update] > Signed-off-by: Jernej Skrabec As already mentioned, this CONFIG_PSCI_RESET removal should go. The rest looks alright, the .dts is identical to the Linux version. With that removed: Reviewed-by: Andre Przywara Thanks, Andre > --- > arch/arm/dts/Makefile | 1 + > arch/arm/dts/sun50i-h6-orangepi-3.dts | 345 ++ > board/sunxi/MAINTAINERS | 5 + > configs/orangepi_3_defconfig | 13 + > 4 files changed, 364 insertions(+) > create mode 100644 arch/arm/dts/sun50i-h6-orangepi-3.dts > create mode 100644 configs/orangepi_3_defconfig > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile > index e00aed1ec207..607571d04b25 100644 > --- a/arch/arm/dts/Makefile > +++ b/arch/arm/dts/Makefile > @@ -605,6 +605,7 @@ dtb-$(CONFIG_MACH_SUN50I_H5) += \ > sun50i-h5-orangepi-zero-plus2.dtb > dtb-$(CONFIG_MACH_SUN50I_H6) += \ > sun50i-h6-beelink-gs1.dtb \ > + sun50i-h6-orangepi-3.dtb \ > sun50i-h6-orangepi-lite2.dtb \ > sun50i-h6-orangepi-one-plus.dtb \ > sun50i-h6-pine-h64.dtb \ > diff --git a/arch/arm/dts/sun50i-h6-orangepi-3.dts > b/arch/arm/dts/sun50i-h6-orangepi-3.dts > new file mode 100644 > index ..15c9dd8c4479 > --- /dev/null > +++ b/arch/arm/dts/sun50i-h6-orangepi-3.dts > @@ -0,0 +1,345 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +// Copyright (C) 2019 Ondřej Jirman > + > +/dts-v1/; > + > +#include "sun50i-h6.dtsi" > +#include "sun50i-h6-cpu-opp.dtsi" > + > +#include > + > +/ { > + model = "OrangePi 3"; > + compatible = "xunlong,orangepi-3", "allwinner,sun50i-h6"; > + > + aliases { > + serial0 = &uart0; > + serial1 = &uart1; > + }; > + > + chosen { > + stdout-path = "serial0:115200n8"; > + }; > + > + connector { > + compatible = "hdmi-connector"; > + ddc-en-gpios = <&pio 7 2 GPIO_ACTIVE_HIGH>; /* PH2 */ > + type = "a"; > + > + port { > + hdmi_con_in: endpoint { > + remote-endpoint = <&hdmi_out_con>; > + }; > + }; > + }; > + > + ext_osc32k: ext_osc32k_clk { > + #clock-cells = <0>; > + compatible = "fixed-clock"; > + clock-frequency = <32768>; > + clock-output-names = "ext_osc32k"; > + }; > + > + leds { > + compatible = "gpio-leds"; > + > + power { > + label = "orangepi:red:power"; > + gpios = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */ > + default-state = "on"; > + }; > + > + status { > + label = "orangepi:green:status"; > + gpios = <&r_pio 0 7 GPIO_ACTIVE_HIGH>; /* PL7 */ > + }; > + }; > + > + reg_vcc5v: vcc5v { > + /* board wide 5V supply directly from the DC jack */ > + compatible = "regulator-fixed"; > + regulator-name = "vcc-5v"; > + regulator-min-microvolt = <500>; > + regulator-max-microvolt = <500>; > + regulator-always-on; > + }; > + > + reg_vcc33_wifi: vcc33-wifi { > + /* Always on 3.3V regulator for WiFi and BT */ > + compatible = "regulator-fixed"; > + regulator-name = "vcc33-wifi"; > + regulator-min-microvolt = <330>; > + regulator-max-microvolt = <330>; > + regulator-always-on; > + vin-supply = <®_vcc5v>; > + }; > + > + reg_vcc_wifi_io: vcc-wifi-io { > + /* Always on 1.8V/300mA regulator for WiFi and BT IO */ > + compatible = "regulator-fixed"; > + regulator-name = "vcc-wifi-io"; > + regulator-min-microvolt = <180>; > + regulator-max-microvolt = <180>; > + regulator-always-on; > + vin-supply = <®_vcc33_wifi>; > + }; > + > + wifi_pwrseq: wifi-pwrseq { > + compatible = "mmc-pwrseq-simple"; > + clocks = <&rtc 1>; > + clock-names = "ext_clock"; > + reset-gpios = <&r_pio 1 3 GPIO_ACTIVE_LOW>; /* PM3 */ > + post-power-on-delay-ms = <200>; > + }; > +}; > + > +&cpu0 { > + cpu-supply = <®_dcdca>; > +}; > + > +&de { > + status = "okay"; > +}; > + > +&dwc3 { > + status = "okay"; > +}; > + > +&ehci0 { > + status = "okay"; > +}; > + > +&ehci3 { > + status = "okay"; > +}; > + > +&gpu { > + mali-supply = <®_dcdcc>; > + status = "okay"; > +
[PATCH 1/4] board: sl28: move ethernet aliases to variant specific dtsi
The variants differ in their network configuration. Move the first two network aliases to the proper variant device tree includes. This is in prepartion for variant 1 and 2 support which has a different network port mapping. The network aliases for the two internal ports will stay in the common dtsi because they are present on all board variants. This might leave a hole if there is no ethernet1 alias. This is intended. Signed-off-by: Michael Walle --- arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi | 2 -- arch/arm/dts/fsl-ls1028a-kontron-sl28-var3-u-boot.dtsi | 6 ++ arch/arm/dts/fsl-ls1028a-kontron-sl28-var4-u-boot.dtsi | 7 +++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi b/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi index 9b4e0298a7..84e974e84d 100644 --- a/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi +++ b/arch/arm/dts/fsl-ls1028a-kontron-sl28-u-boot.dtsi @@ -10,8 +10,6 @@ i2c1 = &i2c3; i2c2 = &i2c4; rtc0 = &rtc; - ethernet0 = &enetc0; - ethernet1 = &enetc1; ethernet2 = &enetc2; ethernet3 = &enetc6; }; diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var3-u-boot.dtsi b/arch/arm/dts/fsl-ls1028a-kontron-sl28-var3-u-boot.dtsi index 79b771e074..879a76415b 100644 --- a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var3-u-boot.dtsi +++ b/arch/arm/dts/fsl-ls1028a-kontron-sl28-var3-u-boot.dtsi @@ -1,2 +1,8 @@ // SPDX-License-Identifier: GPL-2.0+ #include "fsl-ls1028a-kontron-sl28-u-boot.dtsi" + +/ { + aliases { + ethernet0 = &enetc0; + }; +}; diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var4-u-boot.dtsi b/arch/arm/dts/fsl-ls1028a-kontron-sl28-var4-u-boot.dtsi index 79b771e074..fce4694682 100644 --- a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var4-u-boot.dtsi +++ b/arch/arm/dts/fsl-ls1028a-kontron-sl28-var4-u-boot.dtsi @@ -1,2 +1,9 @@ // SPDX-License-Identifier: GPL-2.0+ #include "fsl-ls1028a-kontron-sl28-u-boot.dtsi" + +/ { + aliases { + ethernet0 = &enetc0; + ethernet1 = &enetc1; + }; +}; -- 2.20.1