Re: [RFC PATCH 10/10] arm: migrate away from sections.c
On Sat, May 20, 2023 at 02:55:47PM -0600, Sam Edwards wrote: > This patch effectively reverts 3ebd1cbc49f0005092d69cf0d9a6e64d7a1c300b. Also 47bd65ef057fb71b02b32741d5cfcaf03e2f0918 ? > > The approach taken in that commit was to have the section-marking > symbols generated into empty sections by the compiler, for the linker > script to include at the correct location. The rationale was that at > the time, the linker considered linker-assigned symbols to be dynamic > when they were in PIC (PIEs or shared libraries), which meant they were > represented at runtime by a R_ARM_ABS32 relocation (by symbol name) > rather than by M_ARM_RELATIVE. > > That commit landed in March 2013, but GNU ld later changed its behavior > on 2016-02-23 to default linker-assigned symbols to dynamic only in > shared libraries (not PIE), so this approach is unnecessary. > > I am removing it, because: > 1) It required keeping sections.c in sync with multiple linker scripts. > 2) It added complexity to the linker scripts, making them less readable. > 3) It added unnecessary sections to the output, which can't be merged >because the sections are sometimes of different types. > 4) The linker may insert sections not explicitly named in the script >somewhere between explicit sections; having the marker symbols >outside of the sections they were marking meant the markers could >end up with an unintended section inserted within that region. > > Signed-off-by: Sam Edwards > Cc: Albert ARIBAUD > Thanks /Ilias
Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain
On Mon, May 22, 2023 at 01:37:26PM -0600, Sam Edwards wrote: > Hi Ilias, > > On 5/22/23 00:52, Ilias Apalodimas wrote: > > I can help clean up the arm architecture even further. I was toying > > with the idea of having page-aligned sections and eventually map > > u-boot with proper permissions per section. Right now (at least for > > the majority of arm platforms) we are doing RWX for all the memory, > > apart from devices that are mapped as RW. I do have an awfully hacky > > PoC around, but the linker script cleanup is more than welcome. > > Glad to hear it (and excited by the idea of proper W^X)! Yes that's my end goal here. Looking around the code we have in U-Boot regarding the mmu configuration, it's a lot easier (and cleaner) to fix the linker script, add symbols for ro_start, rw_start etc and layout the binary in a way we can easily map it, instead of leaving it as is and try to fix the mapping in c code. > The linker script > cleanup (i.e. deleting those pesky `sections.c` files and going back to > linker-assigned symbols) can really happen whenever; it won't cause a > problem on any version of GNU ld from <7 years ago. Perhaps a series of > patches (one per arch) doing that should be landed first? Yes probably, because I specifically remember digging through the history of why the sections were defined like that in the first place. > > > It's probably not a mailing list issue. I only got the efi related > > patches on my mailbox. The recipients were generated with > > get_maintainers.pl? Heinirch and I only received the efi* portions as > > we maintain that subsystem > > Well, it's true that you and Heinrich weren't Cc: on every email in the > series. I just went with patman's default behavior. > > But every patch was sent To: the u-boot list, and I do see the whole series > showing up on the archive. Did you not even receive the other patches in the > series via the list? > > Cheers, > Sam Cheers /Ilias
Re: [PATCH v3 2/3] Boot var automatic management for removable medias
On Tue, May 16, 2023 at 10:17:14AM -0400, Raymond Mao wrote: > Hi Ilias, > > On Tue, 16 May 2023 at 01:59, Ilias Apalodimas > wrote: > > > On Tue, May 02, 2023 at 12:12:19PM -0700, Raymond Mao wrote: > > > Changes for complying to EFI spec §3.5.1.1 > > > 'Removable Media Boot Behavior'. > > > Boot variables can be automatically generated during a removable > > > media is probed. At the same time, unused boot variables will be > > > detected and removed. > > > > > > Signed-off-by: Raymond Mao > > > --- > > > Changes in v2 > > > - Ignore EFI_NOT_FOUND returned from > > > efi_bootmgr_update_media_device_boot_option which means no boot > > > options scanned. > > > Changes in v3 > > > - Split the patch into moving and renaming functions and > > > individual patches for each changed functionality > > > > > > lib/efi_loader/efi_disk.c | 7 +++ > > > lib/efi_loader/efi_variable.c | 10 +- > > > lib/efi_loader/efi_variable_tee.c | 5 + > > > 3 files changed, 21 insertions(+), 1 deletion(-) > > > > > > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c > > > index d2256713a8..ca5f07f2ec 100644 > > > --- a/lib/efi_loader/efi_disk.c > > > +++ b/lib/efi_loader/efi_disk.c > > > @@ -687,6 +687,13 @@ int efi_disk_probe(void *ctx, struct event *event) > > > return -1; > > > } > > > > > > + /* only do the boot option management when UEFI sub-system is > > initialized */ > > > + if (efi_obj_list_initialized == EFI_SUCCESS) { > > > + ret = efi_bootmgr_update_media_device_boot_option(); > > > + if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND) > > > + return -1; > > > + } > > > + > > > return 0; > > > } > > > > > > diff --git a/lib/efi_loader/efi_variable.c > > b/lib/efi_loader/efi_variable.c > > > index be95ed44e6..fe71144358 100644 > > > --- a/lib/efi_loader/efi_variable.c > > > +++ b/lib/efi_loader/efi_variable.c > > > @@ -476,6 +476,14 @@ efi_status_t efi_init_variables(void) > > > log_err("Invalid EFI variable seed\n"); > > > } > > > > > > + ret = efi_init_secure_state(); > > > + if (ret != EFI_SUCCESS) > > > + return ret; > > > > > > - return efi_init_secure_state(); > > > + /* update boot option management after variable service > > initialized */ > > > + ret = efi_bootmgr_update_media_device_boot_option(); > > > + if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND) > > > + return ret; > > > + > > > + return EFI_SUCCESS; > > > } > > > diff --git a/lib/efi_loader/efi_variable_tee.c > > b/lib/efi_loader/efi_variable_tee.c > > > index dfef18435d..2995d4a583 100644 > > > --- a/lib/efi_loader/efi_variable_tee.c > > > +++ b/lib/efi_loader/efi_variable_tee.c > > > @@ -748,5 +748,10 @@ efi_status_t efi_init_variables(void) > > > if (ret != EFI_SUCCESS) > > > return ret; > > > > > > + /* update boot option management after variable service > > initialized */ > > > + ret = efi_bootmgr_update_media_device_boot_option(); > > > + if (ret != EFI_SUCCESS && ret != EFI_NOT_FOUND) > > > + return ret; > > > > You don't need this if, just return ret > > > We have to differentiate EFI_NOT_FOUND here for no removable medias are > detected, otherwise efi_init_obj_list() will return failure if the return > code of efi_init_variables() is not EFI_SUCCESS. > Ah fair enough. This seems fine then, please resend the series with the changes in patch #3 Thanks /Ilias > > > > > + > > > return EFI_SUCCESS; > > > } > > > -- > > > 2.25.1 > > > > > > > Regards > > /Ilias > >
Re: [PATCH] imx8m: soc.c: demote some printfs to debug
On 5/22/2023 5:27 PM, Rasmus Villemoes wrote: Getting Found /vpu_g1@3830 node Modify /vpu_g1@3830:status disabled Found /vpu_g2@3831 node Modify /vpu_g2@3831:status disabled etc. on the console on every boot is needlessly verbose. Demote the "Found ..." lines to debug(), which is consistent with other instances in soc.c. Signed-off-by: Rasmus Villemoes Reviewed-by: Peng Fan --- arch/arm/mach-imx/imx8m/soc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 5a4f8358c9..f5c82dff35 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -736,7 +736,7 @@ static int disable_fdt_nodes(void *blob, const char *const nodes_path[], int siz if (nodeoff < 0) continue; /* Not found, skip it */ - printf("Found %s node\n", nodes_path[i]); + debug("Found %s node\n", nodes_path[i]); add_status: rc = fdt_setprop(blob, nodeoff, "status", status, strlen(status) + 1); @@ -1265,7 +1265,7 @@ int ft_system_setup(void *blob, struct bd_info *bd) if (nodeoff >= 0) { const char *speed = "high-speed"; - printf("Found %s node\n", usb_dwc3_path[v]); + debug("Found %s node\n", usb_dwc3_path[v]); usb_modify_speed:
[PATCH] board: xilinx: Add missing prototypes
From: Algapally Santosh Sagar Add missing prototypes to fix the below sparse warnings 1. warning: no previous prototype for 'soc_name_decode' [-Wmissing-prototypes] 2. warning: no previous prototype for 'soc_detection' [-Wmissing-prototypes] 3. warning: no previous prototype for 'board_name_decode' [-Wmissing-prototypes] 4. warning: no previous prototype for 'board_detection' [-Wmissing-prototypes] Signed-off-by: Algapally Santosh Sagar Signed-off-by: Ashok Reddy Soma --- board/xilinx/common/board.h | 7 +++ 1 file changed, 7 insertions(+) diff --git a/board/xilinx/common/board.h b/board/xilinx/common/board.h index 69e642429b..922c9d557a 100644 --- a/board/xilinx/common/board.h +++ b/board/xilinx/common/board.h @@ -11,4 +11,11 @@ int board_late_init_xilinx(void); int xilinx_read_eeprom(void); +char *board_name_decode(void); + +bool board_detection(void); + +char *soc_name_decode(void); + +bool soc_detection(void); #endif /* BOARD_XILINX_COMMON_BOARD_H */ -- 2.17.1
Re: [RFC PATCH 07/10] arm: efi_loader: discard hash, unwind information
Am 22. Mai 2023 21:25:50 MESZ schrieb Sam Edwards : >Hi Ilias, > >On 5/22/23 01:00, Ilias Apalodimas wrote: >> The reason we end up with both hash and gnu.hash is because the hash >> style is set to 'both'. Should we perhaps use (and strip) only one of >> them? > >If we do keep one, it should probably be .hash -- see commit b02bfc4dfc. > >I admit I'm completely mystified as to why we need the hash tables at all. The >ELF spec says those are just for the dynamic linker, but neither the EFI code >nor the self-relocating thunk require it, and I don't know of any target where >the u-boot ELF itself is the shipped binary. For all I know, there never was a >need to include .hash and Albert's commit fixed whatever problem he was facing >only accidentally. Do you have any insights? > Ubuntu's and Debian's u-boot-qemu package ships uboot.elf for multiple architectures. Cf. https://packages.debian.org/bullseye/all/u-boot-qemu/filelist You can pass uboot.elf as -kernel parameter to QEMU. Best regards Heinrich >LLD's --hash-style option doesn't appear to have a "none" option or I'd just >be making use of that here. > >Cheers, >Sam
Re: [PATCH] mmc: fix improper use of memset
Hi, Peng Fan! Thank you for your review. :) On 5/22/23 19:44, Peng Fan wrote: This looks correct to me. BTW: do you met any issues during test? I do not think I understand the question. Are you asking, "Did you send this patch because the current MMC driver was having problems on a real device?" If so, the answer is no. I only sent this to resolve a compiler warning. Or are you asking, "Did you have any problems while testing after making this change?" If so, the answer is still no. But I also do not know whether or not my use of this driver was hitting this specific memset() at all. Kind regards, Sam
Re: [PATCH v6 5/8] efi_loader: check lowest supported version
On Tue, 23 May 2023 at 06:36, Ilias Apalodimas wrote: > > On Fri, May 19, 2023 at 07:32:11PM +0900, Masahisa Kojima wrote: > > The FMP Payload Header which EDK II capsule generation scripts > > insert has a firmware version. > > This commit reads the lowest supported version stored in the > > device tree, then check if the firmware version in FMP payload header > > of the ongoing capsule is equal or greater than the > > lowest supported version. If the firmware version is lower than > > lowest supported version, capsule update will not be performed. > > > > Signed-off-by: Masahisa Kojima > > --- > > Changes in v6: > > - get aligned to the latest implementation > > > > Changes in v5: > > - newly implement the device tree based versioning > > > > Changes in v4: > > - use log_err() instead of printf() > > > > Changes in v2: > > - add error message when the firmware version is lower than > > lowest supported version > > > > lib/efi_loader/efi_firmware.c | 19 ++- > > 1 file changed, 18 insertions(+), 1 deletion(-) > > > > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c > > index 00cf9a088a..7cd0016765 100644 > > --- a/lib/efi_loader/efi_firmware.c > > +++ b/lib/efi_loader/efi_firmware.c > > @@ -424,7 +424,8 @@ static void efi_firmware_get_fw_version(const void > > **p_image, > > * @image_index Image index > > * @statePointer to fmp state > > * > > - * Verify the capsule file > > + * Verify the capsule authentication and check if the fw_version > > + * is equal or greater than the lowest supported version. > > * > > * Return: status code > > */ > > @@ -434,11 +435,27 @@ efi_status_t efi_firmware_verify_image(const void > > **p_image, > > u8 image_index, > > struct fmp_state *state) > > { > > + u32 lsv; > > efi_status_t ret; > > + efi_guid_t *image_type_id; > > > > ret = efi_firmware_capsule_authenticate(p_image, p_image_size); > > efi_firmware_get_fw_version(p_image, p_image_size, state); > > > > + /* check lowest_supported_version if capsule authentication passes */ > > + if (ret == EFI_SUCCESS) { > > What's the point of this here? Can;'t we move this check right after > efi_firmware_capsule_authenticate() and return a security violation if that > failed? Yes, I agree. Thanks, Masahisa Kojima > > > + image_type_id = efi_firmware_get_image_type_id(image_index); > > + if (!image_type_id) > > + return EFI_INVALID_PARAMETER; > > + > > + efi_firmware_get_lsv_from_dtb(image_index, image_type_id, > > &lsv); > > + if (state->fw_version < lsv) { > > + log_err("Firmware version %u too low. Expecting >= > > %u. Aborting update\n", > > + state->fw_version, lsv); > > + return EFI_INVALID_PARAMETER; > > + } > > + } > > + > > return ret; > > } > > > > -- > > 2.17.1 > > > > Thanks > /Ilias
Re: [PATCH v6 2/8] efi_loader: store firmware version into FmpState variable
On Tue, 23 May 2023 at 06:24, Ilias Apalodimas wrote: > > Hi Kojima-san, > > On Fri, May 19, 2023 at 07:32:08PM +0900, Masahisa Kojima wrote: > > Firmware version management is not implemented in the current > > FMP protocol. > > EDK II reference implementation capsule generation script inserts > > the FMP Payload Header right before the payload, FMP Payload Header > > contains the firmware version and lowest supported version. > > > > This commit utilizes the FMP Payload Header, reads the header and > > stores the firmware version into "FmpState" EFI non-volatile variable. > > indicates the image index, since FMP protocol handles multiple > > image indexes. > > Note that lowest supported version included in the FMP Payload Header > > is not used. If the platform uses file-based EFI variable storage, > > it can be tampered. The file-based EFI variable storage is not the > > right place to store the lowest supported version for anti-rollback > > protection. > > > > This change is compatible with the existing FMP implementation. > > This change does not mandate the FMP Payload Header. > > If no FMP Payload Header is found in the capsule file, fw_version, > > lowest supported version, last attempt version and last attempt > > status is 0 and this is the same behavior as existing FMP > > implementation. > > > > Signed-off-by: Masahisa Kojima > > --- > > Changed in v6: > > - only store the fw_version in the FmpState EFI variable > > > > Changes in v4: > > - move lines that are the same in both branches out of the if statement > > - s/EDK2/EDK II/ > > - create print result function > > - set last_attempt_version when capsule authentication failed > > - use log_err() instead of printf() > > > > Changes in v3: > > - exclude CONFIG_FWU_MULTI_BANK_UPDATE case > > - set image_type_id as a vendor field of FmpState variable > > - set READ_ONLY flag for FmpState variable > > - add error code for FIT image case > > > > Changes in v2: > > - modify indent > > > > [...] > > > +/** > > + * efi_firmware_get_fw_version - get fw_version from FMP payload header > > + * @p_image: Pointer to new image > > + * @p_image_size:Pointer to size of new image > > + * @statePointer to fmp state > > + * > > + * Parse the FMP payload header and fill the fmp_state structure. > > + * If no FMP payload header is found, fmp_state structure is not updated. > > + * > > + */ > > +static void efi_firmware_get_fw_version(const void **p_image, > > + efi_uintn_t *p_image_size, > > + struct fmp_state *state) > > +{ > > + const void *image = *p_image; > > + efi_uintn_t image_size = *p_image_size; > > + const struct fmp_payload_header *header; > > + u32 fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE; > > + > > + header = image; > > + if (header->signature == fmp_hdr_signature) { > > + /* FMP header is inserted above the capsule payload */ > > + state->fw_version = header->fw_version; > > > > - if (!memcmp(&header->signature, &fmp_hdr_signature, > > - sizeof(fmp_hdr_signature))) { > > - /* > > - * When building the capsule with the scripts in > > - * edk2, a FMP header is inserted above the capsule > > - * payload. Compensate for this header to get the > > - * actual payload that is to be updated. > > - */ > > image += header->header_size; > > image_size -= header->header_size; > > } > > > > > *p_image = image; > > *p_image_size = image_size; > > Can we get rid of the extra image/image_size here and move this inside the > if()? Yes, thanks. > > > - return EFI_SUCCESS; > > +} > > + > > +/** > > + * efi_firmware_verify_image - verify image > > The name is a bit generic here, we need something which describes what > happens better. The verification already happens in > efi_firmware_capsule_authenticate(). In the "[PATCH v6 5/8] efi_loader: check lowest supported version" in this series, checking lowest_supported_version is added in efi_firmware_verify_image(). So, can we keep this function name? Regards, Masahisa Kojima > Maybe efi_prepare_capsule() or something like that ? > > > + * @p_image: Pointer to new image > > + * @p_image_size:Pointer to size of new image > > + * @image_index Image index > > + * @statePointer to fmp state > > + * > > + * Verify the capsule file > > + * > > + * Return: status code > > + */ > > +static > > +efi_status_t efi_firmware_verify_image(const void **p_image, > > +efi_uintn_t *p_image_size, > > +u8 image_index, > > +struct fmp_state *state) > > +{ > > + efi_status_t ret; > > + > > + ret = efi_firmware_capsule_authenticate(p_image, p_image_size); > > +
Re: [PATCH] mmc: fix improper use of memset
On 5/19/2023 3:47 AM, Sam Edwards wrote: Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button Buffers created through DEFINE_(CACHE_)ALIGN_BUFFER are actually pointers to the real underlying buffer. Using sizeof(...) is not appropriate in this case. Signed-off-by: Sam Edwards --- drivers/mmc/mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index 1af6af82e6..72c1076c56 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -2262,7 +2262,7 @@ static int mmc_startup_v4(struct mmc *mmc) return 0; if (!mmc->ext_csd) - memset(ext_csd_bkup, 0, sizeof(ext_csd_bkup)); + memset(ext_csd_bkup, 0, MMC_MAX_BLOCK_LEN); This looks correct to me. BTW: do you met any issues during test? Reviewed-by: Peng Fan err = mmc_send_ext_csd(mmc, ext_csd); if (err) -- 2.39.2
Re: [PATCH] imx: imx8mp: Add support for Polyhex Debix Model A SBC
On 5/22/2023 6:58 AM, Gilles Talis wrote: + * Copyright 2022 Ideas on Board Oy Time? This was taken as is from the kernel. Hence, the reason why the date is unchanged. Then it is fine to keep as it is. Just note this in commit log. Regards, Peng.
[PATCH 8/8] video: rockchip: dw_mipi_dsi: Fix GRF access
From: Ondrej Jirman Use proper register base and access method to access GRF registers. GRF registers start at a completely different base, and need special access method, that sets the change mask in the 16 MSBs. Signed-off-by: Ondrej Jirman --- drivers/video/rockchip/dw_mipi_dsi_rockchip.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c index fd885ac8ccb6..117c3db21ac1 100644 --- a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c +++ b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -30,6 +31,9 @@ #include #include +#include +#include + #define USEC_PER_SEC 100L /* @@ -197,6 +201,7 @@ struct dw_rockchip_dsi_priv { struct mipi_dsi_device device; void __iomem *base; struct udevice *panel; + void __iomem *grf; /* Optional external dphy */ struct phy phy; @@ -752,16 +757,13 @@ static int dw_mipi_dsi_rockchip_set_bl(struct udevice *dev, int percent) static void dw_mipi_dsi_rockchip_config(struct dw_rockchip_dsi_priv *dsi) { if (dsi->cdata->lanecfg1_grf_reg) - dsi_write(dsi, dsi->cdata->lanecfg1_grf_reg, - dsi->cdata->lanecfg1); + rk_setreg(dsi->grf + dsi->cdata->lanecfg1_grf_reg, dsi->cdata->lanecfg1); if (dsi->cdata->lanecfg2_grf_reg) - dsi_write(dsi, dsi->cdata->lanecfg2_grf_reg, - dsi->cdata->lanecfg2); + rk_setreg(dsi->grf + dsi->cdata->lanecfg2_grf_reg, dsi->cdata->lanecfg2); if (dsi->cdata->enable_grf_reg) - dsi_write(dsi, dsi->cdata->enable_grf_reg, - dsi->cdata->enable); + rk_setreg(dsi->grf + dsi->cdata->enable_grf_reg, dsi->cdata->enable); } static int dw_mipi_dsi_rockchip_bind(struct udevice *dev) @@ -794,6 +796,8 @@ static int dw_mipi_dsi_rockchip_probe(struct udevice *dev) return -EINVAL; } + priv->grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF); + i = 0; while (cdata[i].reg) { if (cdata[i].reg == (fdt_addr_t)priv->base) { -- 2.40.1
[PATCH 7/8] video: rockchip: dw_mipi_dsi: Correct check for lacking phy phandle
From: Ondrej Jirman If phy is not defined in DT (eg. on rk3399), generic_phy_get_by_name will return -ENODATA. Handle that case correctly. Signed-off-by: Ondrej Jirman --- drivers/video/rockchip/dw_mipi_dsi_rockchip.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c index 6d8b1e6f541a..fd885ac8ccb6 100644 --- a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c +++ b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c @@ -814,9 +814,9 @@ static int dw_mipi_dsi_rockchip_probe(struct udevice *dev) * NULL if it's not initialized. */ ret = generic_phy_get_by_name(dev, "dphy", &priv->phy); - if ((ret) && (ret != -ENODEV)) { + if (ret && ret != -ENODATA) { dev_err(dev, "failed to get mipi dphy: %d\n", ret); - return -EINVAL; + return ret; } priv->pclk = devm_clk_get(dev, "pclk"); -- 2.40.1
[PATCH 6/8] video: rockchip: dw_mipi_dsi: Fix best_rate calculation
From: Ondrej Jirman pllref_clk is unused after being retrieved. fin needs to be set to dsi->ref clock's rate for the following calculation to work. Otherwise fin is undefined, and calculation return bogus number based on undefined variable. Signed-off-by: Ondrej Jirman --- drivers/video/rockchip/dw_mipi_dsi_rockchip.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c index 5e8db6bd2e63..6d8b1e6f541a 100644 --- a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c +++ b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c @@ -505,7 +505,6 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, struct display_timing *timings, unsigned int _prediv, best_prediv; unsigned long _fbdiv, best_fbdiv; unsigned long min_delta = ULONG_MAX; - unsigned int pllref_clk; bpp = mipi_dsi_pixel_format_to_bpp(format); if (bpp < 0) { @@ -537,7 +536,7 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, struct display_timing *timings, return 0; } - pllref_clk = clk_get_rate(dsi->ref); + fin = clk_get_rate(dsi->ref); fout = target_mbps * USEC_PER_SEC; /* constraint: 5Mhz <= Fref / N <= 40MHz */ -- 2.40.1
[PATCH 4/8] video: rockchip: dw_mipi_dsi: Fix error path checks in probe function
From: Ondrej Jirman Wrong return codes were checked in several places. Check the proper ones. Signed-off-by: Ondrej Jirman --- drivers/video/rockchip/dw_mipi_dsi_rockchip.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c index b1b5328595e0..b7d6b51703c0 100644 --- a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c +++ b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c @@ -822,6 +822,7 @@ static int dw_mipi_dsi_rockchip_probe(struct udevice *dev) priv->pclk = devm_clk_get(dev, "pclk"); if (IS_ERR(priv->pclk)) { + ret = PTR_ERR(priv->pclk); dev_err(dev, "peripheral clock get error %d\n", ret); return ret; } @@ -833,7 +834,8 @@ static int dw_mipi_dsi_rockchip_probe(struct udevice *dev) } else { priv->ref = devm_clk_get(dev, "ref"); - if (ret) { + if (IS_ERR(priv->ref)) { + ret = PTR_ERR(priv->ref); dev_err(dev, "pll reference clock get error %d\n", ret); return ret; } @@ -841,7 +843,8 @@ static int dw_mipi_dsi_rockchip_probe(struct udevice *dev) priv->rst = devm_reset_control_get_by_index(device->dev, 0); if (IS_ERR(priv->rst)) { - dev_err(dev, "missing dsi hardware reset\n"); + ret = PTR_ERR(priv->rst); + dev_err(dev, "missing dsi hardware reset %d\n", ret); return ret; } -- 2.40.1
[PATCH 5/8] video: rockchip: dw_mipi_dsi: Return 0 from dsi_phy_init on success
From: Ondrej Jirman ret is undefined if external phy is not used resulting in bogus error being returned in that scenario. Signed-off-by: Ondrej Jirman --- drivers/video/rockchip/dw_mipi_dsi_rockchip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c index b7d6b51703c0..5e8db6bd2e63 100644 --- a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c +++ b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c @@ -460,7 +460,7 @@ static int dsi_phy_init(void *priv_data) dw_mipi_dsi_phy_write(dsi, HS_TX_DATA_LANE_EXIT_STATE_TIME_CONTROL, BIT(5) | ns2bc(dsi, 100)); - return ret; + return 0; } static void dsi_phy_post_set_mode(void *priv_data, unsigned long mode_flags) -- 2.40.1
[PATCH 3/8] video: rockchip: dw_mipi_dsi: Fix external phy existnece check
From: Ondrej Jirman &priv->phy is always true. Compiler warns about this loudly. Use a propper check for phy device allocation. Without this fix using this driver with SoC that doesn't use external phy (eg. RK3399) doesn't work. Signed-off-by: Ondrej Jirman --- drivers/video/rockchip/dw_mipi_dsi_rockchip.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c index ca548a60b750..b1b5328595e0 100644 --- a/drivers/video/rockchip/dw_mipi_dsi_rockchip.c +++ b/drivers/video/rockchip/dw_mipi_dsi_rockchip.c @@ -344,7 +344,7 @@ static int dsi_phy_init(void *priv_data) struct dw_rockchip_dsi_priv *dsi = dev_get_priv(dev); int ret, i, vco; - if (&dsi->phy) { + if (dsi->phy.dev) { ret = generic_phy_configure(&dsi->phy, &dsi->phy_opts); if (ret) { dev_err(dsi->dsi_host, @@ -527,7 +527,7 @@ dw_mipi_dsi_get_lane_mbps(void *priv_data, struct display_timing *timings, } /* for external phy only the mipi_dphy_config is necessary */ - if (&dsi->phy) { + if (dsi->phy.dev) { phy_mipi_dphy_get_default_config(timings->pixelclock.typ * 10 / 8, bpp, lanes, &dsi->phy_opts); @@ -827,7 +827,7 @@ static int dw_mipi_dsi_rockchip_probe(struct udevice *dev) } /* Get a ref clock only if not using an external phy. */ - if (&priv->phy) { + if (priv->phy.dev) { dev_dbg(dev, "setting priv->ref to NULL\n"); priv->ref = NULL; -- 2.40.1
[PATCH 1/8] video: rockchip: vop: Fix whitespace
From: Ondrej Jirman Fix confusing use of indentation. Signed-off-by: Ondrej Jirman --- drivers/video/rockchip/rk_vop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/rockchip/rk_vop.c b/drivers/video/rockchip/rk_vop.c index dab9902fda73..c514e2a0e449 100644 --- a/drivers/video/rockchip/rk_vop.c +++ b/drivers/video/rockchip/rk_vop.c @@ -432,7 +432,7 @@ int rk_vop_probe(struct udevice *dev) ret = reset_assert(&ahb_rst); if (ret) { dev_err(dev, "failed to assert ahb reset (ret=%d)\n", ret); - return ret; + return ret; } udelay(20); -- 2.40.1
[PATCH 2/8] video: dw_mipi_dsi: Fix hsync/vsync settings
From: Ondrej Jirman These must be read from timings->flags, like other DSI HOST drivers do. And they must not be inverted either. Low means low. Without this fix, panel drivers that set *SYNC_LOW produce corrupted output on screen (shifted horizobnntally and vertivally by back porch distance). Signed-off-by: Ondrej Jirman --- drivers/video/dw_mipi_dsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/video/dw_mipi_dsi.c b/drivers/video/dw_mipi_dsi.c index 92e388ac1e42..22fef7e8825f 100644 --- a/drivers/video/dw_mipi_dsi.c +++ b/drivers/video/dw_mipi_dsi.c @@ -538,9 +538,9 @@ static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi, break; } - if (device->mode_flags & DISPLAY_FLAGS_VSYNC_HIGH) + if (timings->flags & DISPLAY_FLAGS_VSYNC_LOW) val |= VSYNC_ACTIVE_LOW; - if (device->mode_flags & DISPLAY_FLAGS_HSYNC_HIGH) + if (timings->flags & DISPLAY_FLAGS_HSYNC_LOW) val |= HSYNC_ACTIVE_LOW; dsi_write(dsi, DSI_DPI_VCID, DPI_VCID(dsi->channel)); -- 2.40.1
[PATCH 0/8] Some fixes for the rockchip dw_mipi_dsi driver
From: Ondrej Jirman These are just random fixes for bugs I found while eyeballing the code and porting it to work with RK3399. Please take a look. Thank you and regards, Ondrej Jirman Ondrej Jirman (8): video: rockchip: vop: Fix whitespace video: dw_mipi_dsi: Fix hsync/vsync settings video: rockchip: dw_mipi_dsi: Fix external phy existnece check video: rockchip: dw_mipi_dsi: Fix error path checks in probe function video: rockchip: dw_mipi_dsi: Return 0 from dsi_phy_init on success video: rockchip: dw_mipi_dsi: Fix best_rate calculation video: rockchip: dw_mipi_dsi: Correct check for lacking phy phandle video: rockchip: dw_mipi_dsi: Fix GRF access drivers/video/dw_mipi_dsi.c | 4 +- drivers/video/rockchip/dw_mipi_dsi_rockchip.c | 38 +++ drivers/video/rockchip/rk_vop.c | 2 +- 3 files changed, 25 insertions(+), 19 deletions(-) -- 2.40.1
Re: [PATCH v6 5/8] efi_loader: check lowest supported version
On Fri, May 19, 2023 at 07:32:11PM +0900, Masahisa Kojima wrote: > The FMP Payload Header which EDK II capsule generation scripts > insert has a firmware version. > This commit reads the lowest supported version stored in the > device tree, then check if the firmware version in FMP payload header > of the ongoing capsule is equal or greater than the > lowest supported version. If the firmware version is lower than > lowest supported version, capsule update will not be performed. > > Signed-off-by: Masahisa Kojima > --- > Changes in v6: > - get aligned to the latest implementation > > Changes in v5: > - newly implement the device tree based versioning > > Changes in v4: > - use log_err() instead of printf() > > Changes in v2: > - add error message when the firmware version is lower than > lowest supported version > > lib/efi_loader/efi_firmware.c | 19 ++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c > index 00cf9a088a..7cd0016765 100644 > --- a/lib/efi_loader/efi_firmware.c > +++ b/lib/efi_loader/efi_firmware.c > @@ -424,7 +424,8 @@ static void efi_firmware_get_fw_version(const void > **p_image, > * @image_index Image index > * @statePointer to fmp state > * > - * Verify the capsule file > + * Verify the capsule authentication and check if the fw_version > + * is equal or greater than the lowest supported version. > * > * Return: status code > */ > @@ -434,11 +435,27 @@ efi_status_t efi_firmware_verify_image(const void > **p_image, > u8 image_index, > struct fmp_state *state) > { > + u32 lsv; > efi_status_t ret; > + efi_guid_t *image_type_id; > > ret = efi_firmware_capsule_authenticate(p_image, p_image_size); > efi_firmware_get_fw_version(p_image, p_image_size, state); > > + /* check lowest_supported_version if capsule authentication passes */ > + if (ret == EFI_SUCCESS) { What's the point of this here? Can;'t we move this check right after efi_firmware_capsule_authenticate() and return a security violation if that failed? > + image_type_id = efi_firmware_get_image_type_id(image_index); > + if (!image_type_id) > + return EFI_INVALID_PARAMETER; > + > + efi_firmware_get_lsv_from_dtb(image_index, image_type_id, &lsv); > + if (state->fw_version < lsv) { > + log_err("Firmware version %u too low. Expecting >= %u. > Aborting update\n", > + state->fw_version, lsv); > + return EFI_INVALID_PARAMETER; > + } > + } > + > return ret; > } > > -- > 2.17.1 > Thanks /Ilias
Re: [PATCH v6 4/8] efi_loader: get lowest supported version from device tree
On Fri, May 19, 2023 at 07:32:10PM +0900, Masahisa Kojima wrote: > This commit gets the lowest supported version from device tree, > then fills the lowest supported version in FMP->GetImageInfo(). > > Signed-off-by: Masahisa Kojima > --- > Changed in v6: > - fw_version is removed from device tree > > .../firmware/firmware-version.txt | 22 > lib/efi_loader/efi_firmware.c | 50 ++- > 2 files changed, 71 insertions(+), 1 deletion(-) > create mode 100644 doc/device-tree-bindings/firmware/firmware-version.txt > > diff --git a/doc/device-tree-bindings/firmware/firmware-version.txt > b/doc/device-tree-bindings/firmware/firmware-version.txt > new file mode 100644 > index 00..ee90ce3117 > --- /dev/null > +++ b/doc/device-tree-bindings/firmware/firmware-version.txt > @@ -0,0 +1,22 @@ > +firmware-version bindings > +--- > + > +Required properties: > +- image-type-id : guid for image blob type > +- image-index: image index > +- lowest-supported-version : lowest supported version > + > +Example: > + > + firmware-version { > + image1 { > + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8"; > + image-index = <1>; > + lowest-supported-version = <3>; > + }; > + image2 { > + image-type-id = "5A7021F5-FEF2-48B4-AABA-832E777418C0"; > + image-index = <2>; > + lowest-supported-version = <7>; > + }; > + }; > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c > index 64ceefa212..00cf9a088a 100644 > --- a/lib/efi_loader/efi_firmware.c > +++ b/lib/efi_loader/efi_firmware.c > @@ -144,6 +144,51 @@ efi_status_t EFIAPI > efi_firmware_set_package_info_unsupported( > return EFI_EXIT(EFI_UNSUPPORTED); > } > > +/** > + * efi_firmware_get_lsv_from_dtb - get lowest supported version from dtb > + * @image_index: Image index > + * @image_type_id: Image type id > + * @lsv: Pointer to store the lowest supported version > + * > + * Read the firmware version information from dtb. > + */ > +static void efi_firmware_get_lsv_from_dtb(u8 image_index, > + efi_guid_t *image_type_id, u32 *lsv) > +{ > + const void *fdt = gd->fdt_blob; > + const fdt32_t *val; > + const char *guid_str; > + int len, offset, index; > + int parent; > + > + *lsv = 0; > + > + parent = fdt_subnode_offset(fdt, 0, "firmware-version"); > + if (parent < 0) > + return; > + > + fdt_for_each_subnode(offset, fdt, parent) { > + efi_guid_t guid; > + > + guid_str = fdt_getprop(fdt, offset, "image-type-id", &len); > + if (!guid_str) > + continue; > + uuid_str_to_bin(guid_str, guid.b, UUID_STR_FORMAT_GUID); > + > + val = fdt_getprop(fdt, offset, "image-index", &len); > + if (!val) > + continue; > + index = fdt32_to_cpu(*val); > + > + if (!guidcmp(&guid, image_type_id) && index == image_index) { > + val = fdt_getprop(fdt, offset, > + "lowest-supported-version", &len); > + if (val) > + *lsv = fdt32_to_cpu(*val); > + } > + } > +} > + > /** > * efi_firmware_fill_version_info - fill the version information > * @image_info: Image information > @@ -171,8 +216,11 @@ void efi_firmware_fill_version_info(struct > efi_firmware_image_descriptor *image_ > else > image_info->version = 0; > > + efi_firmware_get_lsv_from_dtb(fw_array->image_index, > + &fw_array->image_type_id, > + > &image_info->lowest_supported_image_version); > + > image_info->version_name = NULL; /* not supported */ > - image_info->lowest_supported_image_version = 0; > image_info->last_attempt_version = 0; > image_info->last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS; > } > -- > 2.17.1 > Reviewed-by: Ilias Apalodimas
Re: [PATCH v6 6/8] mkeficapsule: add FMP Payload Header
On Fri, May 19, 2023 at 07:32:12PM +0900, Masahisa Kojima wrote: > Current mkeficapsule tool does not provide firmware > version management. EDK II reference implementation inserts > the FMP Payload Header right before the payload. > It coutains the fw_version and lowest supported version. > > This commit adds a new parameters required to generate > the FMP Payload Header for mkeficapsule tool. > '-v' indicates the firmware version. > > When mkeficapsule tool is invoked without '-v' option, > FMP Payload Header is not inserted, the behavior is same as > current implementation. > > The lowest supported version included in the FMP Payload Header > is not used, the value stored in the device tree is used instead. > > Signed-off-by: Masahisa Kojima > --- > No update since v5 > > Changes in v5: > - remove --lsv since we use the lowest_supported_version in the dtb > > Changes in v3: > - remove '-f' option > - move some definitions into tools/eficapsule.h > - add dependency check of fw_version and lowest_supported_version > - remove unexpected modification of existing fprintf() call > - add documentation > > Newly created in v2 > > doc/mkeficapsule.1 | 10 ++ > tools/eficapsule.h | 30 ++ > tools/mkeficapsule.c | 37 + > 3 files changed, 73 insertions(+), 4 deletions(-) > > diff --git a/doc/mkeficapsule.1 b/doc/mkeficapsule.1 > index 1ca245a10f..c4c2057d5c 100644 > --- a/doc/mkeficapsule.1 > +++ b/doc/mkeficapsule.1 > @@ -61,6 +61,16 @@ Specify an image index > .BI "-I\fR,\fB --instance " instance > Specify a hardware instance > > +.PP > +FMP Payload Header is inserted right before the payload if > +.BR --fw-version > +is specified > + > + > +.TP > +.BI "-v\fR,\fB --fw-version " firmware-version > +Specify a firmware version, 0 if omitted > + > .PP > For generation of firmware accept empty capsule > .BR --guid > diff --git a/tools/eficapsule.h b/tools/eficapsule.h > index 072a4b5598..753fb73313 100644 > --- a/tools/eficapsule.h > +++ b/tools/eficapsule.h > @@ -113,4 +113,34 @@ struct efi_firmware_image_authentication { > struct win_certificate_uefi_guid auth_info; > } __packed; > > +/* fmp payload header */ > +#define SIGNATURE_16(A, B) ((A) | ((B) << 8)) > +#define SIGNATURE_32(A, B, C, D) \ > + (SIGNATURE_16(A, B) | (SIGNATURE_16(C, D) << 16)) > + > +#define FMP_PAYLOAD_HDR_SIGNATURESIGNATURE_32('M', 'S', 'S', '1') > + > +/** > + * struct fmp_payload_header - EDK2 header for the FMP payload > + * > + * This structure describes the header which is preprended to the > + * FMP payload by the edk2 capsule generation scripts. > + * > + * @signature: Header signature used to identify the > header > + * @header_size: Size of the structure > + * @fw_version: Firmware versions used > + * @lowest_supported_version:Lowest supported version (not used) > + */ > +struct fmp_payload_header { > + uint32_t signature; > + uint32_t header_size; > + uint32_t fw_version; > + uint32_t lowest_supported_version; > +}; > + > +struct fmp_payload_header_params { > + bool have_header; > + uint32_t fw_version; > +}; > + > #endif /* _EFI_CAPSULE_H */ > diff --git a/tools/mkeficapsule.c b/tools/mkeficapsule.c > index b71537beee..52be1f122e 100644 > --- a/tools/mkeficapsule.c > +++ b/tools/mkeficapsule.c > @@ -41,6 +41,7 @@ static struct option options[] = { > {"guid", required_argument, NULL, 'g'}, > {"index", required_argument, NULL, 'i'}, > {"instance", required_argument, NULL, 'I'}, > + {"fw-version", required_argument, NULL, 'v'}, > {"private-key", required_argument, NULL, 'p'}, > {"certificate", required_argument, NULL, 'c'}, > {"monotonic-count", required_argument, NULL, 'm'}, > @@ -60,6 +61,7 @@ static void print_usage(void) > "\t-g, --guid guid for image blob type\n" > "\t-i, --index update image index\n" > "\t-I, --instanceupdate hardware instance\n" > + "\t-v, --fw-version firmware version\n" > "\t-p, --private-key private key file\n" > "\t-c, --certificate signer's certificate > file\n" > "\t-m, --monotonic-count monotonic count\n" > @@ -402,6 +404,7 @@ static void free_sig_data(struct auth_context *ctx) > */ > static int create_fwbin(char *path, char *bin, efi_guid_t *guid, > unsigned long index, unsigned long instance, > + struct fmp_payload_header_params *fmp_ph_params, > uint64_t mcount, char *privkey_file, char *cert_file, > uint16_t oemflags) > { > @@ -410,10 +413,11 @@ static int create_fwbin(char *path, char *bin, > efi_guid_t *guid, > struct efi_firmware_management_capsule_image_header image; > struct auth_context auth_context; > FILE *f; > - uint8_t *data
Re: [PATCH v6 3/8] efi_loader: versioning support in GetImageInfo
On Fri, May 19, 2023 at 07:32:09PM +0900, Masahisa Kojima wrote: > Current FMP->GetImageInfo() always return 0 for the firmware > version, user can not identify which firmware version is currently > running through the EFI interface. > > This commit reads the "FmpState" EFI variable, then fills the > firmware version in FMP->GetImageInfo(). > > Now FMP->GetImageInfo() and ESRT have the meaningful version number. > > Signed-off-by: Masahisa Kojima > --- > Changes in v6: > - create function to fill the version information > > lib/efi_loader/efi_firmware.c | 41 ++- > 1 file changed, 35 insertions(+), 6 deletions(-) > > diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c > index fc085e3c08..64ceefa212 100644 > --- a/lib/efi_loader/efi_firmware.c > +++ b/lib/efi_loader/efi_firmware.c > @@ -144,6 +144,39 @@ efi_status_t EFIAPI > efi_firmware_set_package_info_unsupported( > return EFI_EXIT(EFI_UNSUPPORTED); > } > > +/** > + * efi_firmware_fill_version_info - fill the version information > + * @image_info: Image information > + * @fw_array:Pointer to size of new image > + * > + * Fill the version information into image_info strucrure. > + * > + */ > +static > +void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor > *image_info, > + struct efi_fw_image *fw_array) > +{ > + u16 varname[13]; /* u"FmpState" */ > + efi_status_t ret; > + efi_uintn_t size; > + struct fmp_state var_state = { 0 }; > + > + efi_create_indexed_name(varname, sizeof(varname), "FmpState", > + fw_array->image_index); > + size = sizeof(var_state); > + ret = efi_get_variable_int(varname, &fw_array->image_type_id, > +NULL, &size, &var_state, NULL); > + if (ret == EFI_SUCCESS) > + image_info->version = var_state.fw_version; > + else > + image_info->version = 0; > + > + image_info->version_name = NULL; /* not supported */ > + image_info->lowest_supported_image_version = 0; > + image_info->last_attempt_version = 0; > + image_info->last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS; > +} > + > /** > * efi_fill_image_desc_array - populate image descriptor array > * @image_info_size: Size of @image_info > @@ -193,11 +226,10 @@ static efi_status_t efi_fill_image_desc_array( > image_info[i].image_index = fw_array[i].image_index; > image_info[i].image_type_id = fw_array[i].image_type_id; > image_info[i].image_id = fw_array[i].image_index; > - > image_info[i].image_id_name = fw_array[i].fw_name; > > - image_info[i].version = 0; /* not supported */ > - image_info[i].version_name = NULL; /* not supported */ > + efi_firmware_fill_version_info(&image_info[i], &fw_array[i]); > + > image_info[i].size = 0; > image_info[i].attributes_supported = > IMAGE_ATTRIBUTE_IMAGE_UPDATABLE | > @@ -210,9 +242,6 @@ static efi_status_t efi_fill_image_desc_array( > image_info[0].attributes_setting |= > IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED; > > - image_info[i].lowest_supported_image_version = 0; > - image_info[i].last_attempt_version = 0; > - image_info[i].last_attempt_status = LAST_ATTEMPT_STATUS_SUCCESS; > image_info[i].hardware_instance = 1; > image_info[i].dependencies = NULL; > } > -- > 2.17.1 > Reviewed-by: Ilias Apalodimas
Re: [PATCH v6 2/8] efi_loader: store firmware version into FmpState variable
Hi Kojima-san, On Fri, May 19, 2023 at 07:32:08PM +0900, Masahisa Kojima wrote: > Firmware version management is not implemented in the current > FMP protocol. > EDK II reference implementation capsule generation script inserts > the FMP Payload Header right before the payload, FMP Payload Header > contains the firmware version and lowest supported version. > > This commit utilizes the FMP Payload Header, reads the header and > stores the firmware version into "FmpState" EFI non-volatile variable. > indicates the image index, since FMP protocol handles multiple > image indexes. > Note that lowest supported version included in the FMP Payload Header > is not used. If the platform uses file-based EFI variable storage, > it can be tampered. The file-based EFI variable storage is not the > right place to store the lowest supported version for anti-rollback > protection. > > This change is compatible with the existing FMP implementation. > This change does not mandate the FMP Payload Header. > If no FMP Payload Header is found in the capsule file, fw_version, > lowest supported version, last attempt version and last attempt > status is 0 and this is the same behavior as existing FMP > implementation. > > Signed-off-by: Masahisa Kojima > --- > Changed in v6: > - only store the fw_version in the FmpState EFI variable > > Changes in v4: > - move lines that are the same in both branches out of the if statement > - s/EDK2/EDK II/ > - create print result function > - set last_attempt_version when capsule authentication failed > - use log_err() instead of printf() > > Changes in v3: > - exclude CONFIG_FWU_MULTI_BANK_UPDATE case > - set image_type_id as a vendor field of FmpState variable > - set READ_ONLY flag for FmpState variable > - add error code for FIT image case > > Changes in v2: > - modify indent > [...] > +/** > + * efi_firmware_get_fw_version - get fw_version from FMP payload header > + * @p_image: Pointer to new image > + * @p_image_size:Pointer to size of new image > + * @statePointer to fmp state > + * > + * Parse the FMP payload header and fill the fmp_state structure. > + * If no FMP payload header is found, fmp_state structure is not updated. > + * > + */ > +static void efi_firmware_get_fw_version(const void **p_image, > + efi_uintn_t *p_image_size, > + struct fmp_state *state) > +{ > + const void *image = *p_image; > + efi_uintn_t image_size = *p_image_size; > + const struct fmp_payload_header *header; > + u32 fmp_hdr_signature = FMP_PAYLOAD_HDR_SIGNATURE; > + > + header = image; > + if (header->signature == fmp_hdr_signature) { > + /* FMP header is inserted above the capsule payload */ > + state->fw_version = header->fw_version; > > - if (!memcmp(&header->signature, &fmp_hdr_signature, > - sizeof(fmp_hdr_signature))) { > - /* > - * When building the capsule with the scripts in > - * edk2, a FMP header is inserted above the capsule > - * payload. Compensate for this header to get the > - * actual payload that is to be updated. > - */ > image += header->header_size; > image_size -= header->header_size; > } > > *p_image = image; > *p_image_size = image_size; Can we get rid of the extra image/image_size here and move this inside the if()? > - return EFI_SUCCESS; > +} > + > +/** > + * efi_firmware_verify_image - verify image The name is a bit generic here, we need something which describes what happens better. The verification already happens in efi_firmware_capsule_authenticate(). Maybe efi_prepare_capsule() or something like that ? > + * @p_image: Pointer to new image > + * @p_image_size:Pointer to size of new image > + * @image_index Image index > + * @statePointer to fmp state > + * > + * Verify the capsule file > + * > + * Return: status code > + */ > +static > +efi_status_t efi_firmware_verify_image(const void **p_image, > +efi_uintn_t *p_image_size, > +u8 image_index, > +struct fmp_state *state) > +{ > + efi_status_t ret; > + > + ret = efi_firmware_capsule_authenticate(p_image, p_image_size); > + efi_firmware_get_fw_version(p_image, p_image_size, state); > + > + return ret; > } > > /** > @@ -331,6 +454,7 @@ efi_status_t EFIAPI efi_firmware_fit_set_image( > u16 **abort_reason) > { > efi_status_t status; > + struct fmp_state state = { 0 }; > > EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image, > image_size, vendor_code, progress, abort_reason); > @@ -338,13 +462,16 @@ efi_status_t EFIAPI efi_firmware_fit_set_image( > if (!image || image_index != 1) >
Re: [RFC PATCH 0/5] LWIP stack integration
On Mon, May 22, 2023 at 12:40:49PM -0400, Maxim Uvarov wrote: > On Mon, 22 May 2023 at 10:20, Tom Rini wrote: > > > On Mon, May 22, 2023 at 04:33:57PM +0300, Ilias Apalodimas wrote: > > > Hi Maxim > > > > > > On Mon, 22 May 2023 at 12:01, Maxim Uvarov > > wrote: > > > > > > > > My measurements for binary after LTO looks like: > > > > > > > > U-boot WGET | LWIP WGET + ping | LWIP WGET| diff bytes| diff % > > > > 870728| 915000| 912560 | > > 41832| 4.8 > > > > > > > > > I think you'll need to analyze that a bit more. First of all I don't > > > think the '+ping' tab is useful. What is is trying to achieve? > > > > To show the difference of extra bytes if we add a ping app on top. > > > > > - How was LWIP compiled? > > > > It has a really huge configuration. I tried to turn off everything off > everything what can impact on size but still make http app work: > #define LWIP_HAVE_LOOPIF0 > #define LWIP_NETCONN0 > #define LWIP_SOCKET 0 > #define SO_REUSE0 > #define LWIP_STATS 0 > #define PPP_SUPPORT 0 > > Disabling loopback: > #define LWIP_NETIF_LOOPBACK 0 > can lower to 912288 bytes. > > And it's the same compilation option (optimization for size) as the main > u-boot. I will do more experiments, but I think the goal is not to turn off > everything. > > > > > - Was ipv6 supported? > > > > No. I.e. when I sent results it was enabled on the compilation stage but > not used. I just checked that size remains the same if IPv6 is not even > compiled. > > > > > - Can we strip it down even further? > > > > > > > There is always room for optimization. I think I tried to turn off > everything that is configurable with defines. I can play with disable IP > reassembly and things like that or figure out which functions have more > size and if it's possible to exclude them. > > > > > In general please give as much information as you can with what we > > > gain in functionality from LWIP with those extra bytes of code. > > > > > The main idea is to reuse a maintainable IP stack outside of U-boot. LWIP > can give a nice separation between IP stack code and network application > code. I.e. application should not take care about any TCP details (SYN, > ACK, retransmission, reassembly etc) and should open connection and use > functions similar to recv() and send() to transfer data. Data means > application data, no network packets. And LWIP allows > us to do that. > Because LWIP has an API similar to sockets, it has to be very easy to port > a linux application to LWIP. Then you can test it with a tap device. Then > copy sources to U-boot, add a small integration layer (cmd command to > call), compile and use. > > So my suggestion was: > - do not maintain new network stack code in the current U-boot. Use lwip > sources as an external project. All bugs related to network stack go to > lwip project first, then sync with U-boot. > - maintain network apps code* or > -- inside U-boot. Write our own code for application and maintain it > inside U-boot. > -- inside LWIP. Add examples to LWIP which are suitable for both U-boot > and LWIP. > > * Let's define a U-boot network application as a cmd command. It might be > ping, wget (http or https download), telnet, arp dns etc.. > > Let's consider the real use case, like HTTPS download client. We need to > enable TLS connection, validate certificates, then do http download. > Looking at the current code of wget command it's quite difficult to > implement this due to the application having some protol level things. On > the other side we can find embedTLS examples to do https download on > sockets. If LWIP socket API is ported then the only thing you need to do is > change socket() -> lwip_socket(), recv()->lwip_recv(), send()->lwip_send() > and etc, even function names are similar. If LWIP socket API is not > supported, then use callback API for recv() and send(), which are also > easy. > > So yes we add extra bytes, but that will allow us to write more complex > apps, use standard debug tools, use applications with very minimal > integration changes, use help from the LWIP community to fix protocol bugs, > etc.. > Bunch of things already implemented there: > - ipv6 > - dhcp > - snmp > - igmp > - dns > - tcp and udp and raw. > - loopback > - netconn > - socket > - stats > - ppp > (I just followed configurable defines). > > > And please make sure to disable the previous support, my guess fro that > > much growth is that you didn't. > > > > # CONFIG_PROT_TCP is not set > # CONFIG_PROT_UDP is not set > # CONFIG_UDP_CHECKSUM is not set > # CONFIG_UDP_FUNCTION_FASTBOOT is not set > # CONFIG_CMD_WGET is not set I think you need to step back and figure out a better way to measure the size change and growth. I am not interested in a path that long term means two networking stacks in U-Boot. I a
Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain
On Mon, May 22, 2023 at 01:59:33PM -0600, Sam Edwards wrote: > Hi Tom, > > On 5/22/23 09:30, Tom Rini wrote: > > I think objcopy is a bit of a stretch at this > > point and it's not clear from the above if you're also making use of the > > assembler. > > I agree, since getting llvm-objcopy to play nice with this currently > requires that I make a handful of small hack edits to the makefiles. It does > offer a great advantage over GNU's objcopy in that it doesn't balk at ELFs > from a foreign arch, but I'm only supporting it "opportunistically" right > now. > > I do make use of LLVM's assembler, but LLVM bundles its assembler inside the > Clang binary (`clang -cc1as` and/or just pass .S files to Clang) rather than > installing a separate program. This is not to be confused with `llvm-as` > which is for bitcode/IR manipulation only. But in general, if you're using > Clang, you're also using the LLVM assembler. Ah, ok. > > We might also want to look at backporting > > scripts/Makefile.clang from the current kernel build system and then > > adapting the "guess the --target argument" logic based on CONFIG_$ARCH > > rather than ARCH= (which we don't use). That would also solve the LTO > > problem as that's a result of us missing some flags that the kernel has > > as LLVM+LTO (logically) requires LLVM LD not GNU LD. > > Having something like Linux's `LLVM=1` to enable LLVM would be ideal, I > think. I probably won't be doing that backporting in this patch series since > my goal for now is just to fill in some of the pitfalls for people like me > who are too stubborn to install a GNU toolchain. It honestly shouldn't be too much work to just backport that file (and move/remove some of the logic we have scattered elsewhere instead). > > At that point, and once the EFI guid_t warning is resolved to everyones > > satisfaction we can put qemu_arm* + clang in the CI loop, to catch new > > warnings there. I've already got clang + Pi in my CI loop, but that > > doesn't fail on warnings. > > Do you mean, on the current master branch, and only Clang (no LLD)? I'm in > favor, but since a few of the patches in this series (#3-5) are to support > some of the libcalls that LLVM's codegen likes to emit, I'd be surprised if > that worked on all targets. Feel free to pull whatever necessary patches of > mine here into your own series though. :) The reason it's not in CI right now is that there's compiler warnings, due to the EFI guid_t alignment thing that's being discussed in another thread. I am running (and so booting and running pytest) in my CI loop clang-built U-Boot for Pi 3 (32bit and 64bit) and a few other targets too. But there's also certainly room to improve and I think some of the issues you've addressed here are why other targets for example are too large to boot. -- Tom signature.asc Description: PGP signature
Re: [PATCH 3/3] arm: dts: imx8mp: Sync with Linux 6.3
On Mon, May 22, 2023 at 3:49 PM Fabio Estevam wrote: > > Hi Tim, > > On Fri, May 19, 2023 at 8:00 PM Tim Harvey wrote: > > > Fabio, > > > > There's more to be done here also. With this patch, and with the > > spba-bus added to u-boot.dtsi, if you try to enable usb (usb start) > > you get: > > starting USB... > > Bus usb@3820: > > Enable clock-controller@3038 failed > > probe failed, error -2 > > No working controllers found > > > > So until we get this figured out please don't apply this. > > I don't have any imx8mp-based board here to debug this problem, so it > would be nice > if someone else could investigate this. I can do some testing on the imx8mp-beacon board, but it will likely be a few days before I can get to it. adam > > Thanks
Re: [PATCH 3/3] arm: dts: imx8mp: Sync with Linux 6.3
Hi Tim, On Fri, May 19, 2023 at 8:00 PM Tim Harvey wrote: > Fabio, > > There's more to be done here also. With this patch, and with the > spba-bus added to u-boot.dtsi, if you try to enable usb (usb start) > you get: > starting USB... > Bus usb@3820: > Enable clock-controller@3038 failed > probe failed, error -2 > No working controllers found > > So until we get this figured out please don't apply this. I don't have any imx8mp-based board here to debug this problem, so it would be nice if someone else could investigate this. Thanks
Re: [PATCH v6 1/8] efi_loader: add the number of image entries in efi_capsule_update_info
On Fri, May 19, 2023 at 07:32:07PM +0900, Masahisa Kojima wrote: > The number of image array entries global variable is required > to support EFI capsule update. This information is exposed as a > num_image_type_guids variable, but this information > should be included in the efi_capsule_update_info structure. > > This commit adds the num_images member in the > efi_capsule_update_info structure. All board files supporting > EFI capsule update are updated. > > Signed-off-by: Masahisa Kojima > --- > Newly created in v6 > > arch/arm/mach-rockchip/board.c | 4 ++-- > board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c| 2 +- > board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 2 +- > board/emulation/qemu-arm/qemu-arm.c| 2 +- > board/kontron/pitx_imx8m/pitx_imx8m.c | 2 +- > board/kontron/sl-mx8mm/sl-mx8mm.c | 2 +- > board/kontron/sl28/sl28.c | 2 +- > board/rockchip/evb_rk3399/evb-rk3399.c | 2 +- > board/sandbox/sandbox.c| 2 +- > board/socionext/developerbox/developerbox.c| 2 +- > board/st/stm32mp1/stm32mp1.c | 2 +- > board/xilinx/common/board.c| 2 +- > include/efi_loader.h | 3 ++- > lib/efi_loader/efi_firmware.c | 6 +++--- > lib/fwu_updates/fwu.c | 2 +- > 15 files changed, 19 insertions(+), 18 deletions(-) > > diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c > index f1f70c81d0..8daa74b3eb 100644 > --- a/arch/arm/mach-rockchip/board.c > +++ b/arch/arm/mach-rockchip/board.c > @@ -41,7 +41,7 @@ static bool updatable_image(struct disk_partition *info) > uuid_str_to_bin(info->type_guid, image_type_guid.b, > UUID_STR_FORMAT_GUID); > > - for (i = 0; i < num_image_type_guids; i++) { > + for (i = 0; i < update_info.num_images; i++) { > if (!guidcmp(&fw_images[i].image_type_id, &image_type_guid)) { > ret = true; > break; > @@ -59,7 +59,7 @@ static void set_image_index(struct disk_partition *info, > int index) > uuid_str_to_bin(info->type_guid, image_type_guid.b, > UUID_STR_FORMAT_GUID); > > - for (i = 0; i < num_image_type_guids; i++) { > + for (i = 0; i < update_info.num_images; i++) { > if (!guidcmp(&fw_images[i].image_type_id, &image_type_guid)) { > fw_images[i].image_index = index; > break; > diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > index 466174679e..b79a2380aa 100644 > --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > @@ -54,10 +54,10 @@ struct efi_fw_image fw_images[] = { > > struct efi_capsule_update_info update_info = { > .dfu_string = "mmc 2=flash-bin raw 0 0x1B00 mmcpart 1", > + .num_images = ARRAY_SIZE(fw_images), > .images = fw_images, > }; > > -u8 num_image_type_guids = ARRAY_SIZE(fw_images); > #endif /* EFI_HAVE_CAPSULE_SUPPORT */ > > > diff --git a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > index b373e45df9..af070ec315 100644 > --- a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > +++ b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > @@ -50,10 +50,10 @@ struct efi_fw_image fw_images[] = { > > struct efi_capsule_update_info update_info = { > .dfu_string = "mmc 2=flash-bin raw 0x42 0x1D00 mmcpart 1", > + .num_images = ARRAY_SIZE(fw_images), > .images = fw_images, > }; > > -u8 num_image_type_guids = ARRAY_SIZE(fw_images); > #endif /* EFI_HAVE_CAPSULE_SUPPORT */ > > int board_phys_sdram_size(phys_size_t *size) > diff --git a/board/emulation/qemu-arm/qemu-arm.c > b/board/emulation/qemu-arm/qemu-arm.c > index 34ed3e8ae6..dfea0d92a3 100644 > --- a/board/emulation/qemu-arm/qemu-arm.c > +++ b/board/emulation/qemu-arm/qemu-arm.c > @@ -47,10 +47,10 @@ struct efi_fw_image fw_images[] = { > }; > > struct efi_capsule_update_info update_info = { > + .num_images = ARRAY_SIZE(fw_images) > .images = fw_images, > }; > > -u8 num_image_type_guids = ARRAY_SIZE(fw_images); > #endif /* EFI_HAVE_CAPSULE_SUPPORT */ > > static struct mm_region qemu_arm64_mem_map[] = { > diff --git a/board/kontron/pitx_imx8m/pitx_imx8m.c > b/board/kontron/pitx_imx8m/pitx_imx8m.c > index fcda86bc1b..4548e7c1df 100644 > --- a/board/kontron/pitx_imx8m/pitx_imx8m.c > +++ b/board/kontron/pitx_imx8m/pitx_imx8m.c > @@ -43,10 +43,10 @@ struct efi_fw_image fw_images[] = { > > struct efi_capsule_update_info update_info = { > .dfu_string = "mmc 0=flash-bin raw 0x42 0x1000 mmcpart 1", > + .num_images = ARRAY
Re: [PATCH 0/8] Cleanup unaligned access macros
Hi Jens On Mon, May 22, 2023 at 02:22:30PM +0200, Jens Wiklander wrote: > Hi, > > There are two versions of get/set_unaligned, get_unaligned_be64, > put_unaligned_le64 etc in U-Boot causing confusion (and bugs). > > In this patch-set, I'm trying to fix that with a single unified version of > the access macros to be used across all archs. This work is inspired by > similar changes in this Linux kernel by Arnd Bergman, > https://lore.kernel.org/lkml/20210514100106.3404011-1-a...@kernel.org/ > > Thanks, > Jens Thanks for the cleanup. For the series Reviewed-by: Ilias Apalodimas Although I'd like to hear from arch maintainers as well. Tom, This did pass all the CI successfully, but regardless I think it should be pulled into -next. If you want me to pick it up via the TPM tree please let me know. Thanks /Ilias > > Jens Wiklander (8): > arm: use asm-generic/unaligned.h > sh: use asm-generic/unaligned.h > mips: use asm-generic/unaligned.h > m68k: use asm-generic/unaligned.h > powerpc: use asm-generic/unaligned.h > fs/btrfs: use asm/unaligned.h > linux/unaligned: remove unused access_ok.h > asm-generic: simplify unaligned.h > > arch/arm/include/asm/unaligned.h | 21 +-- > arch/m68k/include/asm/unaligned.h| 17 +- > arch/mips/include/asm/unaligned.h| 23 +-- > arch/powerpc/include/asm/unaligned.h | 18 +- > arch/sh/include/asm/unaligned.h | 22 +-- > fs/btrfs/crypto/hash.c | 2 +- > include/asm-generic/unaligned.h | 89 +++- > include/linux/unaligned/access_ok.h | 66 - > 8 files changed, 83 insertions(+), 175 deletions(-) > delete mode 100644 include/linux/unaligned/access_ok.h > > -- > 2.34.1 >
Re: [RFC PATCH 07/10] arm: efi_loader: discard hash, unwind information
Hi Sam, On Mon, 22 May 2023 at 22:25, Sam Edwards wrote: > > Hi Ilias, > > On 5/22/23 01:00, Ilias Apalodimas wrote: > > The reason we end up with both hash and gnu.hash is because the hash > > style is set to 'both'. Should we perhaps use (and strip) only one of > > them? > > If we do keep one, it should probably be .hash -- see commit b02bfc4dfc. > > I admit I'm completely mystified as to why we need the hash tables at > all. The ELF spec says those are just for the dynamic linker, but > neither the EFI code nor the self-relocating thunk require it, and I > don't know of any target where the u-boot ELF itself is the shipped > binary. Me neither > For all I know, there never was a need to include .hash and > Albert's commit fixed whatever problem he was facing only accidentally. > Do you have any insights? Unfortunately not. I just started looking up the linker scripts myself. > > LLD's --hash-style option doesn't appear to have a "none" option or I'd > just be making use of that here. Indeed. I am fine with the patch regardless, switching the makefile to only produce one of them is a nit anyway, since we'll eventually get rid of them Thanks /Ilias > > Cheers, > Sam
Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain
Am 22. Mai 2023 21:37:26 MESZ schrieb Sam Edwards : >Hi Ilias, > >On 5/22/23 00:52, Ilias Apalodimas wrote: >> I can help clean up the arm architecture even further. I was toying >> with the idea of having page-aligned sections and eventually map >> u-boot with proper permissions per section. Right now (at least for >> the majority of arm platforms) we are doing RWX for all the memory, >> apart from devices that are mapped as RW. I do have an awfully hacky >> PoC around, but the linker script cleanup is more than welcome. > >Glad to hear it (and excited by the idea of proper W^X)! The linker script >cleanup (i.e. deleting those pesky `sections.c` files and going back to >linker-assigned symbols) can really happen whenever; it won't cause a problem >on any version of GNU ld from <7 years ago. Perhaps a series of patches (one >per arch) doing that should be landed first? > >> It's probably not a mailing list issue. I only got the efi related >> patches on my mailbox. The recipients were generated with >> get_maintainers.pl? Heinirch and I only received the efi* portions as >> we maintain that subsystem > >Well, it's true that you and Heinrich weren't Cc: on every email in the >series. I just went with patman's default behavior. > >But every patch was sent To: the u-boot list, and I do see the whole series >showing up on the archive. Did you not even receive the other patches in the >series via the list? The series can be retrieved from patchwork. I personally dislike patman for this eclectic behavior. Git send-email is doing the expected. Best regards Heinrich > >Cheers, >Sam
Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain
Hi Tom, On 5/22/23 09:30, Tom Rini wrote: I think objcopy is a bit of a stretch at this point and it's not clear from the above if you're also making use of the assembler. I agree, since getting llvm-objcopy to play nice with this currently requires that I make a handful of small hack edits to the makefiles. It does offer a great advantage over GNU's objcopy in that it doesn't balk at ELFs from a foreign arch, but I'm only supporting it "opportunistically" right now. I do make use of LLVM's assembler, but LLVM bundles its assembler inside the Clang binary (`clang -cc1as` and/or just pass .S files to Clang) rather than installing a separate program. This is not to be confused with `llvm-as` which is for bitcode/IR manipulation only. But in general, if you're using Clang, you're also using the LLVM assembler. We might also want to look at backporting scripts/Makefile.clang from the current kernel build system and then adapting the "guess the --target argument" logic based on CONFIG_$ARCH rather than ARCH= (which we don't use). That would also solve the LTO problem as that's a result of us missing some flags that the kernel has as LLVM+LTO (logically) requires LLVM LD not GNU LD. Having something like Linux's `LLVM=1` to enable LLVM would be ideal, I think. I probably won't be doing that backporting in this patch series since my goal for now is just to fill in some of the pitfalls for people like me who are too stubborn to install a GNU toolchain. At that point, and once the EFI guid_t warning is resolved to everyones satisfaction we can put qemu_arm* + clang in the CI loop, to catch new warnings there. I've already got clang + Pi in my CI loop, but that doesn't fail on warnings. Do you mean, on the current master branch, and only Clang (no LLD)? I'm in favor, but since a few of the patches in this series (#3-5) are to support some of the libcalls that LLVM's codegen likes to emit, I'd be surprised if that worked on all targets. Feel free to pull whatever necessary patches of mine here into your own series though. :) Cheers, Sam
Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain
Hi Ilias, On 5/22/23 00:52, Ilias Apalodimas wrote: I can help clean up the arm architecture even further. I was toying with the idea of having page-aligned sections and eventually map u-boot with proper permissions per section. Right now (at least for the majority of arm platforms) we are doing RWX for all the memory, apart from devices that are mapped as RW. I do have an awfully hacky PoC around, but the linker script cleanup is more than welcome. Glad to hear it (and excited by the idea of proper W^X)! The linker script cleanup (i.e. deleting those pesky `sections.c` files and going back to linker-assigned symbols) can really happen whenever; it won't cause a problem on any version of GNU ld from <7 years ago. Perhaps a series of patches (one per arch) doing that should be landed first? It's probably not a mailing list issue. I only got the efi related patches on my mailbox. The recipients were generated with get_maintainers.pl? Heinirch and I only received the efi* portions as we maintain that subsystem Well, it's true that you and Heinrich weren't Cc: on every email in the series. I just went with patman's default behavior. But every patch was sent To: the u-boot list, and I do see the whole series showing up on the archive. Did you not even receive the other patches in the series via the list? Cheers, Sam
Re: [RFC PATCH 07/10] arm: efi_loader: discard hash, unwind information
Hi Ilias, On 5/22/23 01:00, Ilias Apalodimas wrote: The reason we end up with both hash and gnu.hash is because the hash style is set to 'both'. Should we perhaps use (and strip) only one of them? If we do keep one, it should probably be .hash -- see commit b02bfc4dfc. I admit I'm completely mystified as to why we need the hash tables at all. The ELF spec says those are just for the dynamic linker, but neither the EFI code nor the self-relocating thunk require it, and I don't know of any target where the u-boot ELF itself is the shipped binary. For all I know, there never was a need to include .hash and Albert's commit fixed whatever problem he was facing only accidentally. Do you have any insights? LLD's --hash-style option doesn't appear to have a "none" option or I'd just be making use of that here. Cheers, Sam
Re: [RFC PATCH 08/10] arm: efi_loader: move .dynamic out of .text in EFI
On 5/22/23 02:10, Heinrich Schuchardt wrote: Hi Heinrich, .dynamic should be aligned. Structure Elf64_Dyn requires at least 8 byte alignment. As best as I can tell, linkers (certainly lld[1], apparently also GNU ld judging by its default linker scripts) themselves set the proper word alignment on the .dynamic section that they synthesize for their internal input object. That alignment requirement bubbles up to the output section, so explicit alignment here should not be necessary. The symbol _etext below should be 512 aligned as in arch/arm/lib/crt0_aarch64_efi.S we have set FileAlignment = 0x200. Ah, that definitely needs to be fixed since the .rela.* sections might not have the 512 alignment. I've updated my local branch, though this also needs to be addressed in the current master branch. Cheers, Sam [1]: https://github.com/llvm/llvm-project/blob/acce2a315945e386a7be6f014ebe90c2a28f38d9/lld/ELF/SyntheticSections.cpp#L1246
Re: [RFC PATCH 0/5] LWIP stack integration
On Mon, 22 May 2023 at 10:20, Tom Rini wrote: > On Mon, May 22, 2023 at 04:33:57PM +0300, Ilias Apalodimas wrote: > > Hi Maxim > > > > On Mon, 22 May 2023 at 12:01, Maxim Uvarov > wrote: > > > > > > My measurements for binary after LTO looks like: > > > > > > U-boot WGET | LWIP WGET + ping | LWIP WGET| diff bytes| diff % > > > 870728| 915000| 912560 | > 41832| 4.8 > > > > > > I think you'll need to analyze that a bit more. First of all I don't > > think the '+ping' tab is useful. What is is trying to achieve? > To show the difference of extra bytes if we add a ping app on top. > > - How was LWIP compiled? > It has a really huge configuration. I tried to turn off everything off everything what can impact on size but still make http app work: #define LWIP_HAVE_LOOPIF0 #define LWIP_NETCONN0 #define LWIP_SOCKET 0 #define SO_REUSE0 #define LWIP_STATS 0 #define PPP_SUPPORT 0 Disabling loopback: #define LWIP_NETIF_LOOPBACK 0 can lower to 912288 bytes. And it's the same compilation option (optimization for size) as the main u-boot. I will do more experiments, but I think the goal is not to turn off everything. > > - Was ipv6 supported? > No. I.e. when I sent results it was enabled on the compilation stage but not used. I just checked that size remains the same if IPv6 is not even compiled. > > - Can we strip it down even further? > > > There is always room for optimization. I think I tried to turn off everything that is configurable with defines. I can play with disable IP reassembly and things like that or figure out which functions have more size and if it's possible to exclude them. > > In general please give as much information as you can with what we > > gain in functionality from LWIP with those extra bytes of code. > > The main idea is to reuse a maintainable IP stack outside of U-boot. LWIP can give a nice separation between IP stack code and network application code. I.e. application should not take care about any TCP details (SYN, ACK, retransmission, reassembly etc) and should open connection and use functions similar to recv() and send() to transfer data. Data means application data, no network packets. And LWIP allows us to do that. Because LWIP has an API similar to sockets, it has to be very easy to port a linux application to LWIP. Then you can test it with a tap device. Then copy sources to U-boot, add a small integration layer (cmd command to call), compile and use. So my suggestion was: - do not maintain new network stack code in the current U-boot. Use lwip sources as an external project. All bugs related to network stack go to lwip project first, then sync with U-boot. - maintain network apps code* or -- inside U-boot. Write our own code for application and maintain it inside U-boot. -- inside LWIP. Add examples to LWIP which are suitable for both U-boot and LWIP. * Let's define a U-boot network application as a cmd command. It might be ping, wget (http or https download), telnet, arp dns etc.. Let's consider the real use case, like HTTPS download client. We need to enable TLS connection, validate certificates, then do http download. Looking at the current code of wget command it's quite difficult to implement this due to the application having some protol level things. On the other side we can find embedTLS examples to do https download on sockets. If LWIP socket API is ported then the only thing you need to do is change socket() -> lwip_socket(), recv()->lwip_recv(), send()->lwip_send() and etc, even function names are similar. If LWIP socket API is not supported, then use callback API for recv() and send(), which are also easy. So yes we add extra bytes, but that will allow us to write more complex apps, use standard debug tools, use applications with very minimal integration changes, use help from the LWIP community to fix protocol bugs, etc.. Bunch of things already implemented there: - ipv6 - dhcp - snmp - igmp - dns - tcp and udp and raw. - loopback - netconn - socket - stats - ppp (I just followed configurable defines). And please make sure to disable the previous support, my guess fro that > much growth is that you didn't. > # CONFIG_PROT_TCP is not set # CONFIG_PROT_UDP is not set # CONFIG_UDP_CHECKSUM is not set # CONFIG_UDP_FUNCTION_FASTBOOT is not set # CONFIG_CMD_WGET is not set BR, Maxim. > > -- > Tom >
Re: Pull request: u-boot-rockchip-20230519
On Fri, May 19, 2023 at 06:09:51PM +0800, Kever Yang wrote: > Hi Tom, > > Please pull the updates for rockchip platform: > rk3588 driver: > - Sync the reset driver with kernel code; > - Enable pcie controller and phy support; > - Enable USB controller and phy support; > Board level dts and config update: > - boost eMMC performance for some of rk3399 boards; > - boot from SPI NOR flash for rk356x boards; > - Other board level updates; > > CI: > https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/pipelines/16404 > > Thanks, > - Kever > > The following changes since commit 6e1852ca2c418e2536ead4b51c4d84a59926b3f1: > > Merge tag 'efi-2023-07-rc3' of > https://source.denx.de/u-boot/custodians/u-boot-efi (2023-05-16 11:23:30 > -0400) > > are available in the Git repository at: > > https://source.denx.de/u-boot/custodians/u-boot-rockchip.git > tags/u-boot-rockchip-20230519 > > for you to fetch changes up to fd6e425be243dce518a02710482514faccf3c211: > > rockchip: rk3588-rock-5b: Enable boot from SPI NOR flash (2023-05-19 > 08:50:44 +0800) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain
On Mon, May 22, 2023 at 12:39:04PM +0200, Michal Simek wrote: > Hi > > On 5/21/23 06:59, Sam Edwards wrote: > > On 5/20/23 22:26, Heinrich Schuchardt wrote: > > > Hello Sam, > > > > Hi Heinrich! Good to hear from you. > > > > > I guess the documentation and the CI testing would also have to be > > > adjusted. > > > > Ah, yeah, those are going to be big things for me to look at when this > > series starts to mature out of the RFC phase. CI is definitely important > > so that the hard-won compatibility doesn't just decay away. :) > > > > > What about non-ARM architectures? > > > > If there's a groundswell of demand for building U-Boot on LLVM, I'd be > > willing to collaborate with others on getting the other architectures up > > to parity with GNU. But since the linker scripts, relocation thunks, > > sections, and whatnot are all arch-specific, I'm only focusing on ARM > > for now (which is both the arch I need and one of the more common ones). > > > > Is there a particular arch you'd like to see next? It seems everything > > U-Boot supports is supported by LLVM, except for Microblaze, NIOS2, and > > SH. > > > > > Could you, please, describe how built with lld so that reviewers can test > > > it. > > > > I've been building with: > > > > nice make CC='clang --target=armv7l-none-eabi' \ > > LTO_FINAL_LDFLAGS=-fuse-ld=lld LD=ld.lld OBJCOPY=llvm-objcopy > > > > ...though mostly at this stage I'm just hoping for folks to confirm that > > this patchset causes no regressions in their existing GNU environments. > > (Feedback from LLVM-land would be appreciated nonetheless, though!!!) > > Dockerfile in repo as I see is using 3 toolchain categories. > 1. llvm deb repo > 2. kernel.org > 3. others - xtensa/arc > > For CI loop you should pretty much provide a way how to get toolchain. > That's why would be good to figure it out and then I am happy to take a look > at changed you have done for Zynq. > Definitely nice to see this happening and I expect more warnings will be > visible and they should be fixed. So, we can trivially add lld to the Dockerfile, it's just listing lld-16 in the install list. I think objcopy is a bit of a stretch at this point and it's not clear from the above if you're also making use of the assembler. We might also want to look at backporting scripts/Makefile.clang from the current kernel build system and then adapting the "guess the --target argument" logic based on CONFIG_$ARCH rather than ARCH= (which we don't use). That would also solve the LTO problem as that's a result of us missing some flags that the kernel has as LLVM+LTO (logically) requires LLVM LD not GNU LD. At that point, and once the EFI guid_t warning is resolved to everyones satisfaction we can put qemu_arm* + clang in the CI loop, to catch new warnings there. I've already got clang + Pi in my CI loop, but that doesn't fail on warnings. -- Tom signature.asc Description: PGP signature
Re: [PATCH v4 23/23] configs: am64x: Enable TI_SECURE_DEV options
Andrew Davis writes: >>> > > Depending on when we expect this binman series to go in should guide > how we handle this. My hope is that this can go into -next very > soon, but that would still mean it won't hit master branch until > v2023.10. > > Fixing the issue Kamlesh describes below in time for v2023.07 > would be my preference then (if Tom is willing to take it as a fix > for v2023.07 that is). I know this fix will be unneeded once > this binman series goes in so it feels like throw away work, > but I don't want AM64x HS-FS broken until v2023.10 :( > >> If we do not have TI_SECURE_DEV option enabled, generated >> tispl.bin_fs will not have capability too parse signed u-boot.img_fs. >> >> tispl.bin_fs will be able to parse u-boot.img_unsigned. >> > > Are you sure about these two above statements? SPL should be able to > parse signed FIT images on GP with or without TI_SECURE_DEV. The way I understand, POST_PROCESS flag is enabled by default if we enable TI_SECURE_DEV option, spl can parse signed images if POST_PROCESS flag is enabled. If we enable POST_PROCESS flag seperately without enabling TI_SECURE flag, GP spl should be able to parse signed images, but that is not defined in current am64 defconfig. That flag is enabled in am62x defconfig, that is able to parse signed u-boot without TI_SECURE_DEV enabled. That was actually one of my concern, if there is no TI_SECURE_DEV enabled, don't generate signed images at all in binman flow, because they won't work. But going forward as we are planning to not have new devices with GP, we can ignore this thing. I got your point though, I'll just go ahead and send seperate patch for enabling TI_SECURE_DEV and the fix for generating tispl.bin for GP > >> If we enable TI_SECURE_DEV in KIG flow, only tispl.bin_HS will be >> generated, which breaks the GP flow.
Re: [RFC PATCH 0/5] LWIP stack integration
On Mon, May 22, 2023 at 04:33:57PM +0300, Ilias Apalodimas wrote: > Hi Maxim > > On Mon, 22 May 2023 at 12:01, Maxim Uvarov wrote: > > > > My measurements for binary after LTO looks like: > > > > U-boot WGET | LWIP WGET + ping | LWIP WGET| diff bytes| diff % > > 870728| 915000| 912560 | 41832 > > | 4.8 > > > I think you'll need to analyze that a bit more. First of all I don't > think the '+ping' tab is useful. What is is trying to achieve? > - How was LWIP compiled? > - Was ipv6 supported? > - Can we strip it down even further? > > In general please give as much information as you can with what we > gain in functionality from LWIP with those extra bytes of code. And please make sure to disable the previous support, my guess fro that much growth is that you didn't. -- Tom signature.asc Description: PGP signature
Re: [PATCH v4 23/23] configs: am64x: Enable TI_SECURE_DEV options
On 5/22/23 7:35 AM, Kamlesh Gurudasani wrote: Neha Malcom Francis writes: Hi Andrew On 18/05/23 22:09, Andrew Davis wrote: On 5/18/23 9:27 AM, Neha Malcom Francis wrote: From: Kamlesh Gurudasani AM64x family of SoCs by default will have some level of security enforcement checking. Enable CONFIG_TI_SECURE_DEVICE by default so all levels of secure SoCs will boot with binman. Signed-off-by: Kamlesh Gurudasani Signed-off-by: Neha Francis Signed-off-by: Neha Malcom Francis (apologies for the incorrect tags) --- This fix is independent of the binman changes and should go in first anyway to keep bisectability. Andrew This fix breaks KIG flow though, which is why it was decided to be put in along with the binman series. Depending on when we expect this binman series to go in should guide how we handle this. My hope is that this can go into -next very soon, but that would still mean it won't hit master branch until v2023.10. Fixing the issue Kamlesh describes below in time for v2023.07 would be my preference then (if Tom is willing to take it as a fix for v2023.07 that is). I know this fix will be unneeded once this binman series goes in so it feels like throw away work, but I don't want AM64x HS-FS broken until v2023.10 :( If we do not have TI_SECURE_DEV option enabled, generated tispl.bin_fs will not have capability too parse signed u-boot.img_fs. tispl.bin_fs will be able to parse u-boot.img_unsigned. Are you sure about these two above statements? SPL should be able to parse signed FIT images on GP with or without TI_SECURE_DEV. If we enable TI_SECURE_DEV in KIG flow, only tispl.bin_HS will be generated, which breaks the GP flow. Unless, the patch to fix the issue of generating tispl.bin is merged. That would be the better solution, if GP cannot use tispl.bin_HS currently then the tispl.bin generation fix should go first, then this patch, then the rest of binman changes can go in after (next cycle). Andrew
Re: [PATCH] global: Use proper project name U-Boot
On 2023/5/17 15:17, Michal Simek wrote: Use proper project name in comments, Kconfig, readmes. Signed-off-by: Michal Simek [...] diff --git a/fs/btrfs/compat.h b/fs/btrfs/compat.h index 9cf8a10c76c5..02173dea5f48 100644 --- a/fs/btrfs/compat.h +++ b/fs/btrfs/compat.h @@ -46,7 +46,7 @@ /* * Read data from device specified by @desc and @part * - * U-boot equivalent of pread(). + * U-Boot equivalent of pread(). * * Return the bytes of data read. * Return <0 for error. diff --git a/fs/btrfs/extent-io.h b/fs/btrfs/extent-io.h index 6b0c87da969d..5c5c579d1eaf 100644 --- a/fs/btrfs/extent-io.h +++ b/fs/btrfs/extent-io.h @@ -8,7 +8,7 @@ * Use pointer to provide better alignment. * - Remove max_cache_size related interfaces * Includes free_extent_buffer_nocache() - * As we don't cache eb in U-boot. + * As we don't cache eb in U-Boot. * - Include headers * * Write related functions are kept as we still need to modify dummy extent For the btrfs part: Reviewed-by: Qu Wenruo Thanks, Qu
Re: [PATCH] global: Use proper project name U-Boot
On 17/05/2023 09:17, Michal Simek wrote: Use proper project name in comments, Kconfig, readmes. Signed-off-by: Michal Simek For amlogic changes: Reviewed-by: Neil Armstrong --- I am ignoring these for now because they can break automated scripts or user setting that's why they should be fixed separately. doc/board/amlogic/pre-generated-fip.rst:77:- bl33.bin: U-boot binary image You can add this change safely, Neil
Re: [PATCH 2/2] arm: mach-k3: j721s2: clk-data.c: Add main_uart5 clock data
On 12/05/23 23:12, Bryan Brattlof wrote: > Hi Bhavya! > > On May 11, 2023 thus sayeth Bhavya Kapoor: >> Add main_uart5 clocks in clk-data.c for J721S2. Now, >> main_uart5 clocks will be set up while booting the J721S2 SoC. >> >> Signed-off-by: Bhavya Kapoor >> --- >> arch/arm/mach-k3/j721s2/clk-data.c | 7 +-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/mach-k3/j721s2/clk-data.c >> b/arch/arm/mach-k3/j721s2/clk-data.c >> index ad6bd991b7..0c5c321c1e 100644 >> --- a/arch/arm/mach-k3/j721s2/clk-data.c >> +++ b/arch/arm/mach-k3/j721s2/clk-data.c >> @@ -247,6 +247,7 @@ static const struct clk_data clk_list[] = { >> CLK_MUX("emmcsd1_lb_clksel_out0", emmcsd1_lb_clksel_out0_parents, 2, >> 0x1080b4, 16, 1, 0), >> CLK_MUX("mcu_clkout_mux_out0", mcu_clkout_mux_out0_parents, 2, >> 0x40f08010, 0, 1, 0), >> CLK_DIV_DEFFREQ("usart_programmable_clock_divider_out0", >> "hsdiv4_16fft_main_1_hsdivout0_clk", 0x1081c0, 0, 2, 0, 0, 4800), >> +CLK_DIV("usart_programmable_clock_divider_out5", >> "hsdiv4_16fft_main_1_hsdivout0_clk", 0x1081d4, 0, 2, 0, 0), > Is this being used as an alternate console? idk if it would be > appropriate to use CLK_DIV_DEFFREQ macro here to setup the uart's > divider here. > > ~Bryan Hi Bryan, yes we will be using this as alternative console. And CLK-DIV macro is appropriate here and tested and working fine as well ~B-Kapoor
[PATCH v1 3/3] arch/arm64: meson-a1: dts: move pwrc node to bus
This is necessary so that pwrc can be used together with peripherals when described in a bus node. For example, in the future, this will be USB. Signed-off-by: Alexey Romanov --- arch/arm/dts/meson-a1.dtsi | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/arm/dts/meson-a1.dtsi b/arch/arm/dts/meson-a1.dtsi index 9776b2d8798..f3560cbc3a4 100644 --- a/arch/arm/dts/meson-a1.dtsi +++ b/arch/arm/dts/meson-a1.dtsi @@ -65,12 +65,6 @@ sm: secure-monitor { compatible = "amlogic,meson-gxbb-sm"; - - pwrc: power-controller { - compatible = "amlogic,meson-a1-pwrc"; - #power-domain-cells = <1>; - status = "okay"; - }; }; soc { @@ -161,6 +155,12 @@ #address-cells = <0>; }; + pwrc: power-controller { + compatible = "amlogic,meson-a1-pwrc"; + #power-domain-cells = <1>; + status = "okay"; + }; + usb: usb@fe004400 { compatible = "amlogic,meson-a1-usb-ctrl"; reg = <0x0 0xfe004400 0x0 0xa0>; -- 2.38.1
[PATCH v1 2/3] drivers: meson: introduce secure power controller driver
This patch adds Power controller driver support for Amlogic A1 family using secure monitor calls. The power domains register only can access in secure world. Signed-off-by: Alexey Romanov --- drivers/power/domain/Kconfig | 7 + drivers/power/domain/Makefile | 1 + drivers/power/domain/meson-secure-pwrc.c | 160 + include/dt-bindings/power/meson-a1-power.h | 32 + 4 files changed, 200 insertions(+) create mode 100644 drivers/power/domain/meson-secure-pwrc.c create mode 100644 include/dt-bindings/power/meson-a1-power.h diff --git a/drivers/power/domain/Kconfig b/drivers/power/domain/Kconfig index 7e1b8c072fa..411c210756a 100644 --- a/drivers/power/domain/Kconfig +++ b/drivers/power/domain/Kconfig @@ -68,6 +68,13 @@ config MESON_EE_POWER_DOMAIN Enable support for manipulating Amlogic Meson Everything-Else power domains. +config MESON_SECURE_POWER_DOMAIN + bool "Enable Amlogic Secure power domain driver" + depends on POWER_DOMAIN && ARCH_MESON && MESON_A1 + help + Enable support for manipulating Amlogic Meson Secure power domains. + Support for Amlogic A1 series. + config SANDBOX_POWER_DOMAIN bool "Enable the sandbox power domain test driver" depends on POWER_DOMAIN && SANDBOX diff --git a/drivers/power/domain/Makefile b/drivers/power/domain/Makefile index e6244776216..aa5a4ba57cd 100644 --- a/drivers/power/domain/Makefile +++ b/drivers/power/domain/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_IMX8MP_HSIOMIX_BLKCTRL) += imx8mp-hsiomix.o obj-$(CONFIG_MTK_POWER_DOMAIN) += mtk-power-domain.o obj-$(CONFIG_MESON_GX_VPU_POWER_DOMAIN) += meson-gx-pwrc-vpu.o obj-$(CONFIG_MESON_EE_POWER_DOMAIN) += meson-ee-pwrc.o +obj-$(CONFIG_MESON_SECURE_POWER_DOMAIN) += meson-secure-pwrc.o obj-$(CONFIG_SANDBOX_POWER_DOMAIN) += sandbox-power-domain.o obj-$(CONFIG_SANDBOX_POWER_DOMAIN) += sandbox-power-domain-test.o obj-$(CONFIG_TEGRA186_POWER_DOMAIN) += tegra186-power-domain.o diff --git a/drivers/power/domain/meson-secure-pwrc.c b/drivers/power/domain/meson-secure-pwrc.c new file mode 100644 index 000..f70f8e02423 --- /dev/null +++ b/drivers/power/domain/meson-secure-pwrc.c @@ -0,0 +1,160 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2023 SberDevices, Inc. + * Author: Alexey Romanov + */ + +#include +#include +#include +#include +#include + +struct meson_secure_pwrc_domain_desc { + char *name; + size_t index; +}; + +struct meson_secure_pwrc_domain_data { + unsigned int count; + struct meson_secure_pwrc_domain_desc *domains; +}; + +struct meson_secure_pwrc_priv { + const struct meson_secure_pwrc_domain_data *data; +}; + +static int meson_secure_pwrc_on(struct power_domain *power_domain) +{ + struct meson_secure_pwrc_priv *priv = dev_get_priv(power_domain->dev); + struct meson_secure_pwrc_domain_desc *pwrc_domain; + int err; + + pwrc_domain = &priv->data->domains[power_domain->id]; + + err = meson_sm_pwrdm_on(pwrc_domain->index); + if (err) { + pr_err("meson_sm_pwrdm_on() failed (%d)\n", err); + return err; + } + + pr_debug("enable %s power domain\n", pwrc_domain->name); + + return 0; +} + +static int meson_secure_pwrc_off(struct power_domain *power_domain) +{ + struct meson_secure_pwrc_priv *priv = dev_get_priv(power_domain->dev); + struct meson_secure_pwrc_domain_desc *pwrc_domain; + int err; + + pwrc_domain = &priv->data->domains[power_domain->id]; + + err = meson_sm_pwrdm_off(pwrc_domain->index); + if (err) { + pr_err("meson_sm_pwrdm_off() failed (%d)\n", err); + return err; + } + + pr_debug("disable %s power domain\n", pwrc_domain->name); + + return 0; +} + +static int meson_secure_pwrc_of_xlate(struct power_domain *power_domain, + struct ofnode_phandle_args *args) +{ + struct meson_secure_pwrc_priv *priv = dev_get_priv(power_domain->dev); + struct meson_secure_pwrc_domain_desc *pwrc_domain; + + if (args->args_count < 1) { + pr_err("invalid args count: %d\n", args->args_count); + return -EINVAL; + } + + power_domain->id = args->args[0]; + + if (power_domain->id >= priv->data->count) { + pr_err("domain with ID=%lu is invalid\n", power_domain->id); + return -EINVAL; + } + + pwrc_domain = &priv->data->domains[power_domain->id]; + + if (!pwrc_domain->name) { + pr_err("domain with ID=%lu is invalid\n", power_domain->id); + return -EINVAL; + } + + return 0; +} + +#define SEC_PD(__name) \ +[PWRC_##__name##_ID] = \ +{ \ + .name = #__name,\ + .index = PWRC_##__name##_ID,\ +} + +static struct meson_secure
[PATCH v1 1/3] arch/arm: meson: sm: introduce power domain functions
This commit adds functions to manage secure power domain for Amlogic SoC's using smc functionality. Signed-off-by: Alexey Romanov --- arch/arm/include/asm/arch-meson/sm.h | 30 arch/arm/mach-meson/sm.c | 14 + 2 files changed, 44 insertions(+) diff --git a/arch/arm/include/asm/arch-meson/sm.h b/arch/arm/include/asm/arch-meson/sm.h index 53b75176493..4b1d564bc48 100644 --- a/arch/arm/include/asm/arch-meson/sm.h +++ b/arch/arm/include/asm/arch-meson/sm.h @@ -58,4 +58,34 @@ enum { */ int meson_sm_get_reboot_reason(void); +#define PWRDM_OFF 0 +#define PWRDM_ON 1 + +/** + * meson_sm_pwrdm_set - do command at specified power domain. + * + * @index: power domain index. + * @cmd: command index. + * @return: zero on success or error code on failure. + */ +int meson_sm_pwrdm_set(size_t index, int cmd); + +/** + * meson_sm_pwrdm_off - disable specified power domain. + * + * @index: power domain index. + * @return: zero on success or error code on failure. + */ +#define meson_sm_pwrdm_off(index) \ + meson_sm_pwrdm_set(index, PWRDM_OFF) + +/** + * meson_sm_pwrdm_on - enable specified power domain. + * + * @index: power domain index. + * @return: zero on success or error code on failure. + */ +#define meson_sm_pwrdm_on(index) \ + meson_sm_pwrdm_set(index, PWRDM_ON) + #endif /* __MESON_SM_H__ */ diff --git a/arch/arm/mach-meson/sm.c b/arch/arm/mach-meson/sm.c index f2ca7e76932..d600c64d0be 100644 --- a/arch/arm/mach-meson/sm.c +++ b/arch/arm/mach-meson/sm.c @@ -24,6 +24,7 @@ #define FN_EFUSE_READ 0x8230 #define FN_EFUSE_WRITE 0x8231 #define FN_CHIP_ID 0x8244 +#define FN_PWRDM_SET 0x8293 static void *shmem_input; static void *shmem_output; @@ -137,3 +138,16 @@ int meson_sm_get_reboot_reason(void) /* The SMC call is not used, we directly use AO_SEC_SD_CFG15 */ return FIELD_GET(REBOOT_REASON_MASK, reason); } + +int meson_sm_pwrdm_set(size_t index, int cmd) +{ + struct pt_regs regs; + + regs.regs[0] = FN_PWRDM_SET; + regs.regs[1] = index; + regs.regs[2] = cmd; + + smc_call(®s); + + return regs.regs[0]; +} -- 2.38.1
[PATCH v1 0/3] Meson Secure PWRC Driver
Hello! This patch set adds support (driver and sm functions) for working with power domain controller using secure monitor for Amlogic A1-series. Additionally, in the future, the driver can be used for other Amlgoic SoC series, such as S4. Alexey Romanov (3): arch/arm: meson: sm: introduce power domain functions drivers: meson: introduce secure power controller driver arch/arm64: meson-a1: dts: move pwrc node to bus arch/arm/dts/meson-a1.dtsi | 12 +- arch/arm/include/asm/arch-meson/sm.h | 30 arch/arm/mach-meson/sm.c | 14 ++ drivers/power/domain/Kconfig | 7 + drivers/power/domain/Makefile | 1 + drivers/power/domain/meson-secure-pwrc.c | 160 + include/dt-bindings/power/meson-a1-power.h | 32 + 7 files changed, 250 insertions(+), 6 deletions(-) create mode 100644 drivers/power/domain/meson-secure-pwrc.c create mode 100644 include/dt-bindings/power/meson-a1-power.h -- 2.38.1
Re: USB mass storage gadget on SAMA5D2
Hi, I think there may be some racing in the driver. (Purely assumption as a tinyusb maintainer) If I enable DBG_ALL in atmel_usba_udc.h, the block device is enermurated although with I/O error. [1337613.189788] usb 1-1: new high-speed USB device number 7 using xhci_hcd [1337613.674551] usb 1-1: New USB device found, idVendor=dead, idProduct=beef, bcdDevice= 2.17 [1337613.674565] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0 [1337613.674568] usb 1-1: Product: USB download gadget [1337613.674572] usb 1-1: Manufacturer: U-Boot [1337613.866033] usb-storage 1-1:1.0: USB Mass Storage device detected [1337613.866645] scsi host0: usb-storage 1-1:1.0 [1337614.997803] scsi 0:0:0:0: Direct-Access LinuxUMS disk 0 PQ: 0 ANSI: 2 [1337615.230004] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337615.706637] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337616.183308] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337616.659937] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337617.140086] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337617.616632] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337618.073323] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337618.549927] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337619.026540] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337619.499944] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337619.976679] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337620.453285] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337620.916597] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337621.393267] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337621.869676] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337622.346597] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337622.823361] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337623.293287] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337623.635357] sd 0:0:0:0: [sda] Read Capacity(10) failed: Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK [1337623.635369] sd 0:0:0:0: [sda] Sense not available. [1337623.635376] sd 0:0:0:0: [sda] 0 512-byte logical blocks: (0 B/0 B) [1337623.635379] sd 0:0:0:0: [sda] 0-byte physical blocks [1337623.756597] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337624.233274] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337624.709945] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337625.186639] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337625.663266] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337626.136617] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337626.478078] sd 0:0:0:0: [sda] Write Protect is off [1337626.478088] sd 0:0:0:0: [sda] Mode Sense: 00 00 00 00 [1337626.599928] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337627.076606] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337627.553276] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337628.029936] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337628.499858] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337628.973267] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337629.313846] sd 0:0:0:0: [sda] Asking for cache data failed [1337629.313861] sd 0:0:0:0: [sda] Assuming drive cache: write through [1337629.314517] sd 0:0:0:0: [sda] Attached SCSI removable disk [1337629.436603] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337629.906353] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337630.373268] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337630.843284] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337631.306595] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337631.783253] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337632.259935] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337632.736612] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337633.213283] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337633.689933] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337634.146549] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337634.626586] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337635.096677] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337635.569893] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337636.043195] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337636.519877] usb 1-1: reset high-speed USB device number 7 using xhci_hcd [1337636.993235] usb 1-1: reset high-speed
Re: [RFC PATCH 0/5] LWIP stack integration
Hi Maxim On Mon, 22 May 2023 at 12:01, Maxim Uvarov wrote: > > My measurements for binary after LTO looks like: > > U-boot WGET | LWIP WGET + ping | LWIP WGET| diff bytes| diff % > 870728| 915000| 912560 | 41832| > 4.8 I think you'll need to analyze that a bit more. First of all I don't think the '+ping' tab is useful. What is is trying to achieve? - How was LWIP compiled? - Was ipv6 supported? - Can we strip it down even further? In general please give as much information as you can with what we gain in functionality from LWIP with those extra bytes of code. Thanks /Ilias > BR, > Maxim. > > On Fri, 19 May 2023 at 09:52, Tom Rini wrote: >> >> On Fri, May 19, 2023 at 04:17:06PM +0300, Ilias Apalodimas wrote: >> > Hi Tom, >> > >> > Apologies for being late to the party >> > >> > > On Thu, May 11, 2023 at 09:52:04AM -0400, Tom Rini wrote: >> > > On Fri, May 05, 2023 at 10:25:24AM +, Maxim Uvarov wrote: >> > > >> > > > Greetings, >> > > > >> > > > This RFC patchset is an attempt to try to use an already existing IP >> > > > network stack inside U-boot. >> > > > U-Boot recently got basic TCP/IP support, implementing wget, but in >> > > > order to get a full IP stack >> > > > with new features (e.g ipv6), it would be preferable to use an >> > > > established embedded ip library, >> > > > instead of rewriting the code from scratch. >> > > > >> > > > For this experiment LWIP network stack was selected: >> > > > https://savannah.nongnu.org/git/?group=lwip >> > > > >> > > > LWIP main features include: >> > > > - Protocols: IP, IPv6, ICMP, ND, MLD, UDP, TCP, IGMP, ARP, PPPoS, PPPoE >> > > > - DHCP client, DNS client (incl. mDNS hostname resolver), AutoIP/APIPA >> > > > (Zeroconf), >> > > > SNMP agent (v1, v2c, v3, private MIB support & MIB compiler) >> > > > - APIs: specialized APIs for enhanced performance, optional >> > > > Berkeley-alike socket API >> > > > - Extended features: IP forwarding over multiple network interfaces, >> > > > TCP congestion control, >> > > > RTT estimation and fast recovery/fast retransmit >> > > > - Addon applications: HTTP(S) server, SNTP client, SMTP(S) client, >> > > > ping, NetBIOS nameserver, >> > > > mDNS responder, MQTT client, TFTP server. >> > > > >> > > > This RFC work is a demo to enable lwIP (lightweight IP) which is a >> > > > widely used open-source >> > > > TCP/IP stack designed for embedded systems for U-boot. That will allow >> > > > using already >> > > > written network applications for microcontrollers. >> > > > >> > > > lwIP is licensed under a BSD-style license: >> > > > http://lwip.wikia.com/wiki/License. >> > > > Which should be compatible with u-boot. >> > > > >> > > > In the current RFC I tried to use minimal changes to better see how >> > > > LWIP code can be embedded into >> > > > U-boot. Patches implement ping and wget commands work. Both commands >> > > > are currently copy pasting and >> > > > reusing lwIP examples. Whether we want to add the final application >> > > > in U-Boot or lwIP is up to >> > > > discussion, but the current approach was the easiest one for an RFC. >> > > >> > > I'm honestly not sure this is the most useful way of doing an RFC. The >> > > long term goal would be that we replace our existing net/ with lwIP, >> > > yes? So what I'd see as more valuable is what it looks like to limit >> > > yourself to either sandbox or some QEMU target, disable the current >> > > network stack, and instead use lwIP to support just cmd/net.c so that >> > > the scope of the conversion is visible. Then the size comparison you do >> > > should be between platform + net + cmd/net.c (and the rest of networking >> > > turned off) and platform + lwip + cmd/net.c converted. >> > >> > This might be a bit too much imho. How about replacing the TCP stack which >> > is new an under heavy devel as well. If we do that we could replace lwip >> > with the version Maxim proposes and check the difference between >> > U-boot + homegrown TCP + wget >> > U-Boot + LWIP (for tcp only) + new wget >> > >> > That would give us an idea before trying to replace the UDP portion which >> > is way bigger >> >> I guess we can try that as a starting point and see what things look >> like. My gut feeling however is that's not going to look like a win. >> >> -- >> Tom
Re: [EXTERNAL] Re: [PATCH v4 23/23] configs: am64x: Enable TI_SECURE_DEV options
Kamlesh Gurudasani writes: > Neha Malcom Francis writes: > >> Hi Andrew >> >> On 18/05/23 22:09, Andrew Davis wrote: >>> On 5/18/23 9:27 AM, Neha Malcom Francis wrote: From: Kamlesh Gurudasani AM64x family of SoCs by default will have some level of security enforcement checking. Enable CONFIG_TI_SECURE_DEVICE by default so all levels of secure SoCs will boot with binman. Signed-off-by: Kamlesh Gurudasani Signed-off-by: Neha Francis Signed-off-by: Neha Malcom Francis >> >> (apologies for the incorrect tags) >> --- >>> >>> This fix is independent of the binman changes and should go >>> in first anyway to keep bisectability. >>> >>> Andrew >>> >> >> This fix breaks KIG flow though, which is why it was decided to be put >> in along with the binman series. >> > If we do not have TI_SECURE_DEV option enabled, generated > tispl.bin_fs will not have capability too parse signed u-boot.img_fs. > > tispl.bin_fs will be able to parse u-boot.img_unsigned. > > If we enable TI_SECURE_DEV in KIG flow, only tispl.bin_HS will be > generated, which breaks the GP flow. By GP flow, I mean the scripts to support the GP
[PATCH] xilinx: versal-net: Add new versalnet loadpdi command
From: Algapally Santosh Sagar Versal NET loadpdi command is used for loading secure & non-secure pdi images. Signed-off-by: Algapally Santosh Sagar Signed-off-by: Michal Simek --- board/xilinx/versal-net/Kconfig | 8 board/xilinx/versal-net/Makefile | 1 + board/xilinx/versal-net/cmds.c | 81 3 files changed, 90 insertions(+) create mode 100644 board/xilinx/versal-net/cmds.c diff --git a/board/xilinx/versal-net/Kconfig b/board/xilinx/versal-net/Kconfig index 8f94d2bb399a..2484429d3cb3 100644 --- a/board/xilinx/versal-net/Kconfig +++ b/board/xilinx/versal-net/Kconfig @@ -6,4 +6,12 @@ if ARCH_VERSAL_NET +config CMD_VERSAL_NET + bool "Enable Versal NET specific commands" + default y + depends on ZYNQMP_FIRMWARE + help + Select this to enable Versal NET specific commands. + Commands like versalnet loadpdi are enabled by this. + endif diff --git a/board/xilinx/versal-net/Makefile b/board/xilinx/versal-net/Makefile index 2008d4e231c6..f9ff07c11c63 100644 --- a/board/xilinx/versal-net/Makefile +++ b/board/xilinx/versal-net/Makefile @@ -7,3 +7,4 @@ # obj-y := board.o +obj-$(CONFIG_CMD_VERSAL_NET) += cmds.o diff --git a/board/xilinx/versal-net/cmds.c b/board/xilinx/versal-net/cmds.c new file mode 100644 index ..b18a71fe52c0 --- /dev/null +++ b/board/xilinx/versal-net/cmds.c @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2023, Advanced Micro Devices, Inc. + * + * Michal Simek + */ + +#include +#include +#include +#include +#include +#include +#include + +/** + * do_versalnet_load_pdi - Handle the "versalnet load pdi" command-line command + * @cmdtp: Command data struct pointer + * @flag: Command flag + * @argc: Command-line argument count + * @argv: Array of command-line arguments + * + * Processes the Versal NET load pdi command + * + * Return: return 0 on success, Error value if command fails. + * CMD_RET_USAGE incase of incorrect/missing parameters. + */ +static int do_versalnet_load_pdi(struct cmd_tbl *cmdtp, int flag, int argc, +char * const argv[]) +{ + u32 buf_lo, buf_hi; + u32 ret_payload[PAYLOAD_ARG_CNT]; + ulong addr, *pdi_buf; + size_t len; + int ret; + + if (argc != cmdtp->maxargs) { + debug("pdi_load: incorrect parameters passed\n"); + return CMD_RET_USAGE; + } + + addr = simple_strtol(argv[1], NULL, 16); + if (!addr) { + debug("pdi_load: zero pdi_data address\n"); + return CMD_RET_USAGE; + } + + len = hextoul(argv[2], NULL); + if (!len) { + debug("pdi_load: zero size\n"); + return CMD_RET_USAGE; + } + + pdi_buf = (ulong *)ALIGN((ulong)addr, ARCH_DMA_MINALIGN); + if ((ulong)addr != (ulong)pdi_buf) { + memcpy((void *)pdi_buf, (void *)addr, len); + debug("Pdi addr:0x%lx aligned to 0x%lx\n", + addr, (ulong)pdi_buf); + } + + flush_dcache_range((ulong)pdi_buf, (ulong)pdi_buf + len); + + buf_lo = lower_32_bits((ulong)pdi_buf); + buf_hi = upper_32_bits((ulong)pdi_buf); + + ret = xilinx_pm_request(VERSAL_PM_LOAD_PDI, VERSAL_PM_PDI_TYPE, buf_lo, + buf_hi, 0, ret_payload); + if (ret) + printf("PDI load failed with err: 0x%08x\n", ret); + + return cmd_process_error(cmdtp, ret); +} + +static char versalnet_help_text[] = + "loadpdi addr len - Load pdi image\n" + "load pdi image at ddr address 'addr' with pdi image size 'len'\n" +; + +U_BOOT_CMD_WITH_SUBCMDS(versalnet, "Versal NET sub-system", versalnet_help_text, + U_BOOT_SUBCMD_MKENT(loadpdi, 3, 1, + do_versalnet_load_pdi)); -- 2.36.1
Re: [PATCH v4 23/23] configs: am64x: Enable TI_SECURE_DEV options
Neha Malcom Francis writes: > Hi Andrew > > On 18/05/23 22:09, Andrew Davis wrote: >> On 5/18/23 9:27 AM, Neha Malcom Francis wrote: >>> From: Kamlesh Gurudasani >>> >>> AM64x family of SoCs by default will have some level of security >>> enforcement checking. Enable CONFIG_TI_SECURE_DEVICE by default so all >>> levels of secure SoCs will boot with binman. >>> >>> Signed-off-by: Kamlesh Gurudasani >>> Signed-off-by: Neha Francis >>> Signed-off-by: Neha Malcom Francis > > (apologies for the incorrect tags) > >>> --- >> >> This fix is independent of the binman changes and should go >> in first anyway to keep bisectability. >> >> Andrew >> > > This fix breaks KIG flow though, which is why it was decided to be put > in along with the binman series. > If we do not have TI_SECURE_DEV option enabled, generated tispl.bin_fs will not have capability too parse signed u-boot.img_fs. tispl.bin_fs will be able to parse u-boot.img_unsigned. If we enable TI_SECURE_DEV in KIG flow, only tispl.bin_HS will be generated, which breaks the GP flow. Unless, the patch to fix the issue of generating tispl.bin is merged.
[PATCH 8/8] asm-generic: simplify unaligned.h
The get_unaligned()/put_unaligned() implementations are more complex than necessary. Move everything into one file and use a more compact implementation based on packed struct access and byte swapping macros. This patch is based on the Linux kernel commit 803f4e1eab7a ("asm-generic: simplify asm/unaligned.h") by Arnd Bergmann. Signed-off-by: Jens Wiklander --- include/asm-generic/unaligned.h | 89 +++-- 1 file changed, 73 insertions(+), 16 deletions(-) diff --git a/include/asm-generic/unaligned.h b/include/asm-generic/unaligned.h index 3d33a5a063e8..9e5d93ec3041 100644 --- a/include/asm-generic/unaligned.h +++ b/include/asm-generic/unaligned.h @@ -1,24 +1,81 @@ +/* SPDX-License-Identifier: GPL-2.0 */ #ifndef _GENERIC_UNALIGNED_H #define _GENERIC_UNALIGNED_H #include -#include -#include -#include - -/* - * Select endianness - */ -#if defined(__LITTLE_ENDIAN) -#define get_unaligned __get_unaligned_le -#define put_unaligned __put_unaligned_le -#elif defined(__BIG_ENDIAN) -#define get_unaligned __get_unaligned_be -#define put_unaligned __put_unaligned_be -#else -#error invalid endian -#endif +#define __get_unaligned_t(type, ptr) ({ \ + const struct { type x; } __packed * __pptr = (typeof(__pptr))(ptr); \ + __pptr->x; \ +}) + +#define __put_unaligned_t(type, val, ptr) do { \ + struct { type x; } __packed * __pptr = (typeof(__pptr))(ptr); \ + __pptr->x = (val); \ +} while (0) + +#define get_unaligned(ptr) __get_unaligned_t(typeof(*(ptr)), (ptr)) +#define put_unaligned(val, ptr) __put_unaligned_t(typeof(*(ptr)), (val), (ptr)) + +static inline u16 get_unaligned_le16(const void *p) +{ + return le16_to_cpu(__get_unaligned_t(__le16, p)); +} + +static inline u32 get_unaligned_le32(const void *p) +{ + return le32_to_cpu(__get_unaligned_t(__le32, p)); +} + +static inline u64 get_unaligned_le64(const void *p) +{ + return le64_to_cpu(__get_unaligned_t(__le64, p)); +} + +static inline void put_unaligned_le16(u16 val, void *p) +{ + __put_unaligned_t(__le16, cpu_to_le16(val), p); +} + +static inline void put_unaligned_le32(u32 val, void *p) +{ + __put_unaligned_t(__le32, cpu_to_le32(val), p); +} + +static inline void put_unaligned_le64(u64 val, void *p) +{ + __put_unaligned_t(__le64, cpu_to_le64(val), p); +} + +static inline u16 get_unaligned_be16(const void *p) +{ + return be16_to_cpu(__get_unaligned_t(__be16, p)); +} + +static inline u32 get_unaligned_be32(const void *p) +{ + return be32_to_cpu(__get_unaligned_t(__be32, p)); +} + +static inline u64 get_unaligned_be64(const void *p) +{ + return be64_to_cpu(__get_unaligned_t(__be64, p)); +} + +static inline void put_unaligned_be16(u16 val, void *p) +{ + __put_unaligned_t(__be16, cpu_to_be16(val), p); +} + +static inline void put_unaligned_be32(u32 val, void *p) +{ + __put_unaligned_t(__be32, cpu_to_be32(val), p); +} + +static inline void put_unaligned_be64(u64 val, void *p) +{ + __put_unaligned_t(__be64, cpu_to_be64(val), p); +} /* Allow unaligned memory access */ void allow_unaligned(void); -- 2.34.1
[PATCH 7/8] linux/unaligned: remove unused access_ok.h
linux/unaligned/access_ok.h is unused, so remove it. Signed-off-by: Jens Wiklander --- include/linux/unaligned/access_ok.h | 66 - 1 file changed, 66 deletions(-) delete mode 100644 include/linux/unaligned/access_ok.h diff --git a/include/linux/unaligned/access_ok.h b/include/linux/unaligned/access_ok.h deleted file mode 100644 index 5f46eee23c38.. --- a/include/linux/unaligned/access_ok.h +++ /dev/null @@ -1,66 +0,0 @@ -#ifndef _LINUX_UNALIGNED_ACCESS_OK_H -#define _LINUX_UNALIGNED_ACCESS_OK_H - -#include - -static inline u16 get_unaligned_le16(const void *p) -{ - return le16_to_cpup((__le16 *)p); -} - -static inline u32 get_unaligned_le32(const void *p) -{ - return le32_to_cpup((__le32 *)p); -} - -static inline u64 get_unaligned_le64(const void *p) -{ - return le64_to_cpup((__le64 *)p); -} - -static inline u16 get_unaligned_be16(const void *p) -{ - return be16_to_cpup((__be16 *)p); -} - -static inline u32 get_unaligned_be32(const void *p) -{ - return be32_to_cpup((__be32 *)p); -} - -static inline u64 get_unaligned_be64(const void *p) -{ - return be64_to_cpup((__be64 *)p); -} - -static inline void put_unaligned_le16(u16 val, void *p) -{ - *((__le16 *)p) = cpu_to_le16(val); -} - -static inline void put_unaligned_le32(u32 val, void *p) -{ - *((__le32 *)p) = cpu_to_le32(val); -} - -static inline void put_unaligned_le64(u64 val, void *p) -{ - *((__le64 *)p) = cpu_to_le64(val); -} - -static inline void put_unaligned_be16(u16 val, void *p) -{ - *((__be16 *)p) = cpu_to_be16(val); -} - -static inline void put_unaligned_be32(u32 val, void *p) -{ - *((__be32 *)p) = cpu_to_be32(val); -} - -static inline void put_unaligned_be64(u64 val, void *p) -{ - *((__be64 *)p) = cpu_to_be64(val); -} - -#endif /* _LINUX_UNALIGNED_ACCESS_OK_H */ -- 2.34.1
[PATCH 6/8] fs/btrfs: use asm/unaligned.h
Use asm/unaligned.h instead of linux/unaligned/access_ok.h for unaligned access. This is needed on architectures that doesn't handle unaligned accesses directly. Signed-off-by: Jens Wiklander --- fs/btrfs/crypto/hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/btrfs/crypto/hash.c b/fs/btrfs/crypto/hash.c index 891a2974be05..0a0b35fe9b96 100644 --- a/fs/btrfs/crypto/hash.c +++ b/fs/btrfs/crypto/hash.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0+ +#include #include -#include #include #include #include -- 2.34.1
[PATCH 5/8] powerpc: use asm-generic/unaligned.h
Powerpc configurations are apparently able to do unaligned accesses. But in an attempt to clean up and handle unaligned accesses in the same way we ignore that and use the common asm-generic/unaligned.h directly instead. Signed-off-by: Jens Wiklander --- arch/powerpc/include/asm/unaligned.h | 18 ++ 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/arch/powerpc/include/asm/unaligned.h b/arch/powerpc/include/asm/unaligned.h index 5f1b1e3c2137..7fb482abc383 100644 --- a/arch/powerpc/include/asm/unaligned.h +++ b/arch/powerpc/include/asm/unaligned.h @@ -1,16 +1,2 @@ -#ifndef _ASM_POWERPC_UNALIGNED_H -#define _ASM_POWERPC_UNALIGNED_H - -#ifdef __KERNEL__ - -/* - * The PowerPC can do unaligned accesses itself in big endian mode. - */ -#include -#include - -#define get_unaligned __get_unaligned_be -#define put_unaligned __put_unaligned_be - -#endif /* __KERNEL__ */ -#endif /* _ASM_POWERPC_UNALIGNED_H */ +/* SPDX-License-Identifier: GPL-2.0 */ +#include -- 2.34.1
[PATCH 4/8] m68k: use asm-generic/unaligned.h
M68k essentially duplicates the content of asm-generic/unaligned.h, with an exception for non-Coldfire configurations. Coldfire configurations are apparently able to do unaligned accesses. But in an attempt to clean up and handle unaligned accesses in the same way we ignore that and use the common asm-generic/unaligned.h directly instead. Signed-off-by: Jens Wiklander --- arch/m68k/include/asm/unaligned.h | 17 ++--- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/arch/m68k/include/asm/unaligned.h b/arch/m68k/include/asm/unaligned.h index 328aa0c316c9..7fb482abc383 100644 --- a/arch/m68k/include/asm/unaligned.h +++ b/arch/m68k/include/asm/unaligned.h @@ -1,15 +1,2 @@ -#ifndef _ASM_M68K_UNALIGNED_H -#define _ASM_M68K_UNALIGNED_H - -#ifdef CONFIG_COLDFIRE -#include -#else -#include -#endif - -#include - -#define get_unaligned __get_unaligned_be -#define put_unaligned __put_unaligned_be - -#endif /* _ASM_M68K_UNALIGNED_H */ +/* SPDX-License-Identifier: GPL-2.0 */ +#include -- 2.34.1
[PATCH 3/8] mips: use asm-generic/unaligned.h
Mips essentially duplicates the content of asm-generic/unaligned.h, so use that file directly instead. Signed-off-by: Jens Wiklander --- arch/mips/include/asm/unaligned.h | 23 +-- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/arch/mips/include/asm/unaligned.h b/arch/mips/include/asm/unaligned.h index debb9cf7aba6..7fb482abc383 100644 --- a/arch/mips/include/asm/unaligned.h +++ b/arch/mips/include/asm/unaligned.h @@ -1,23 +1,2 @@ /* SPDX-License-Identifier: GPL-2.0 */ -/* - * Copyright (C) 2007 Ralf Baechle (r...@linux-mips.org) - */ -#ifndef _ASM_MIPS_UNALIGNED_H -#define _ASM_MIPS_UNALIGNED_H - -#include -#if defined(__MIPSEB__) -#define get_unaligned __get_unaligned_be -#define put_unaligned __put_unaligned_be -#elif defined(__MIPSEL__) -#define get_unaligned __get_unaligned_le -#define put_unaligned __put_unaligned_le -#else -#error "MIPS, but neither __MIPSEB__, nor __MIPSEL__???" -#endif - -#include -#include -#include - -#endif /* _ASM_MIPS_UNALIGNED_H */ +#include -- 2.34.1
[PATCH 2/8] sh: use asm-generic/unaligned.h
Sh essentially duplicates the content of asm-generic/unaligned.h, so use that file directly instead. Signed-off-by: Jens Wiklander --- arch/sh/include/asm/unaligned.h | 22 ++ 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/arch/sh/include/asm/unaligned.h b/arch/sh/include/asm/unaligned.h index 5acf0819620e..7fb482abc383 100644 --- a/arch/sh/include/asm/unaligned.h +++ b/arch/sh/include/asm/unaligned.h @@ -1,20 +1,2 @@ -#ifndef _ASM_SH_UNALIGNED_H -#define _ASM_SH_UNALIGNED_H - -/* Copy from linux-kernel. */ - -/* Other than SH4A, SH can't handle unaligned accesses. */ -#include -#if defined(__BIG_ENDIAN__) -#define get_unaligned __get_unaligned_be -#define put_unaligned __put_unaligned_be -#elif defined(__LITTLE_ENDIAN__) -#define get_unaligned __get_unaligned_le -#define put_unaligned __put_unaligned_le -#endif - -#include -#include -#include - -#endif /* _ASM_SH_UNALIGNED_H */ +/* SPDX-License-Identifier: GPL-2.0 */ +#include -- 2.34.1
[PATCH 0/8] Cleanup unaligned access macros
Hi, There are two versions of get/set_unaligned, get_unaligned_be64, put_unaligned_le64 etc in U-Boot causing confusion (and bugs). In this patch-set, I'm trying to fix that with a single unified version of the access macros to be used across all archs. This work is inspired by similar changes in this Linux kernel by Arnd Bergman, https://lore.kernel.org/lkml/20210514100106.3404011-1-a...@kernel.org/ Thanks, Jens Jens Wiklander (8): arm: use asm-generic/unaligned.h sh: use asm-generic/unaligned.h mips: use asm-generic/unaligned.h m68k: use asm-generic/unaligned.h powerpc: use asm-generic/unaligned.h fs/btrfs: use asm/unaligned.h linux/unaligned: remove unused access_ok.h asm-generic: simplify unaligned.h arch/arm/include/asm/unaligned.h | 21 +-- arch/m68k/include/asm/unaligned.h| 17 +- arch/mips/include/asm/unaligned.h| 23 +-- arch/powerpc/include/asm/unaligned.h | 18 +- arch/sh/include/asm/unaligned.h | 22 +-- fs/btrfs/crypto/hash.c | 2 +- include/asm-generic/unaligned.h | 89 +++- include/linux/unaligned/access_ok.h | 66 - 8 files changed, 83 insertions(+), 175 deletions(-) delete mode 100644 include/linux/unaligned/access_ok.h -- 2.34.1
[PATCH 1/8] arm: use asm-generic/unaligned.h
Arm duplicates the content of asm-generic/unaligned.h, so use that file directly instead. Signed-off-by: Jens Wiklander --- arch/arm/include/asm/unaligned.h | 21 ++--- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/arch/arm/include/asm/unaligned.h b/arch/arm/include/asm/unaligned.h index 0a228fb8eea8..7fb482abc383 100644 --- a/arch/arm/include/asm/unaligned.h +++ b/arch/arm/include/asm/unaligned.h @@ -1,19 +1,2 @@ -#ifndef _ASM_ARM_UNALIGNED_H -#define _ASM_ARM_UNALIGNED_H - -#include -#include -#include - -/* - * Select endianness - */ -#if __BYTE_ORDER == __LITTLE_ENDIAN -#define get_unaligned __get_unaligned_le -#define put_unaligned __put_unaligned_le -#else -#define get_unaligned __get_unaligned_be -#define put_unaligned __put_unaligned_be -#endif - -#endif /* _ASM_ARM_UNALIGNED_H */ +/* SPDX-License-Identifier: GPL-2.0 */ +#include -- 2.34.1
Re: [PATCH 0/2] Fix the sparse warnings
On 5/19/23 13:38, Venkatesh Yadav Abbarapu wrote: Fix sparse warnings in below files - Add fallthrough statement in the switch case - Add missing header Algapally Santosh Sagar (2): clk: zynqmp: Add fallthrough statement in the switch case mach-zynqmp: handoff: Add missing header arch/arm/mach-zynqmp/handoff.c | 1 + drivers/clk/clk_zynqmp.c | 1 + 2 files changed, 2 insertions(+) Applied. M
Re: [PATCH v2] arm64: versal-net: Detect and display bootmode
On 5/16/23 16:47, Ashok Reddy Soma wrote: Read boodmode register using versal_net_get_bootmode() in board_late_init and prepare corresponding distro boot command sequence based on it. versal_net_get_bootmode() will be changed to use smc calls later, but for now directly reads the register. Signed-off-by: Ashok Reddy Soma --- Changes in v2: - Remove check for mmc/sdhci node enabled or not in EMMC bootmode - Remove check for sdhci node enabled or not in SD0 and SD1 bootmode .../mach-versal-net/include/mach/hardware.h | 21 board/xilinx/versal-net/board.c | 114 ++ 2 files changed, 135 insertions(+) diff --git a/arch/arm/mach-versal-net/include/mach/hardware.h b/arch/arm/mach-versal-net/include/mach/hardware.h index c5e4e22040..89b84a2efc 100644 --- a/arch/arm/mach-versal-net/include/mach/hardware.h +++ b/arch/arm/mach-versal-net/include/mach/hardware.h @@ -27,7 +27,13 @@ struct iou_scntrs_regs { u32 base_frequency_id_register; /* 0x20 */ }; +struct crp_regs { + u32 reserved0[128]; + u32 boot_mode_usr; /* 0x200 */ +}; + #define VERSAL_NET_CRL_APB_BASEADDR 0xEB5E +#define VERSAL_NET_CRP_BASEADDR0xF126 #define VERSAL_NET_IOU_SCNTR_SECURE 0xEC92 #define CRL_APB_TIMESTAMP_REF_CTRL_CLKACT_BIT BIT(25) @@ -36,6 +42,7 @@ struct iou_scntrs_regs { #define IOU_SCNTRS_CONTROL_EN 1 #define crlapb_base ((struct crlapb_regs *)VERSAL_NET_CRL_APB_BASEADDR) +#define crp_base ((struct crp_regs *)VERSAL_NET_CRP_BASEADDR) #define iou_scntr_secure ((struct iou_scntrs_regs *)VERSAL_NET_IOU_SCNTR_SECURE) #define PMC_TAP 0xF11A @@ -49,6 +56,20 @@ struct iou_scntrs_regs { # define PLATFORM_VERSION_MASKGENMASK(31, 28) #define PMC_TAP_USERCODE (PMC_TAP + 0x8) +/* Bootmode setting values */ +#define BOOT_MODES_MASK0x000F +#define QSPI_MODE_24BIT0x0001 +#define QSPI_MODE_32BIT0x0002 +#define SD_MODE0x0003 /* sd 0 */ +#define SD_MODE1 0x0005 /* sd 1 */ +#define EMMC_MODE 0x0006 +#define USB_MODE 0x0007 +#define OSPI_MODE 0x0008 +#define SD1_LSHFT_MODE 0x000E /* SD1 Level shifter */ +#define JTAG_MODE 0x +#define BOOT_MODE_USE_ALT 0x100 +#define BOOT_MODE_ALT_SHIFT12 + enum versal_net_platform { VERSAL_NET_SILICON = 0, VERSAL_NET_SPP = 1, diff --git a/board/xilinx/versal-net/board.c b/board/xilinx/versal-net/board.c index 6724c7290f..6595d6f3e8 100644 --- a/board/xilinx/versal-net/board.c +++ b/board/xilinx/versal-net/board.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -165,8 +166,32 @@ int board_early_init_r(void) return 0; } +static u8 versal_net_get_bootmode(void) +{ + u8 bootmode; + u32 reg = 0; + + reg = readl(&crp_base->boot_mode_usr); + + if (reg >> BOOT_MODE_ALT_SHIFT) + reg >>= BOOT_MODE_ALT_SHIFT; + + bootmode = reg & BOOT_MODES_MASK; + + return bootmode; +} + int board_late_init(void) { + u8 bootmode; + struct udevice *dev; + int bootseq = -1; + int bootseq_len = 0; + int env_targets_len = 0; + const char *mode; + char *new_targets; + char *env_targets; + if (!(gd->flags & GD_FLG_ENV_DEFAULT)) { debug("Saved variables - Skipping\n"); return 0; @@ -175,6 +200,95 @@ int board_late_init(void) if (!IS_ENABLED(CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG)) return 0; + bootmode = versal_net_get_bootmode(); + + puts("Bootmode: "); + switch (bootmode) { + case USB_MODE: + puts("USB_MODE\n"); + mode = "usb_dfu0 usb_dfu1"; + break; + case JTAG_MODE: + puts("JTAG_MODE\n"); + mode = "jtag pxe dhcp"; + break; + case QSPI_MODE_24BIT: + puts("QSPI_MODE_24\n"); + mode = "xspi0"; + break; + case QSPI_MODE_32BIT: + puts("QSPI_MODE_32\n"); + mode = "xspi0"; + break; + case OSPI_MODE: + puts("OSPI_MODE\n"); + mode = "xspi0"; + break; + case EMMC_MODE: + puts("EMMC_MODE\n"); + mode = "mmc"; + bootseq = dev_seq(dev); + break; + case SD_MODE: + puts("SD_MODE\n"); + if (uclass_get_device_by_name(UCLASS_MMC, + "mmc@f104", &dev)) { + puts("Boot from SD0 but without SD0 enabled!\n"); + return -1; + } + debug("mmc0 device found at %p, seq %d\n", dev, dev_seq(dev)); + + mode = "mmc"; + bootseq = dev_
Re: [PATCH] dt-bindings: u-boot: Add variables for bootscript location
On 5/11/23 16:33, Michal Simek wrote: From: Algapally Santosh Sagar Add bootscr-address and offset-from-ram-start properties to help in easier picking of boot script file when automated flows are used. The bootscr-address holds the full 64 bit address of the bootscript file. The bootscr-offset-from-ram-start holds the offset address of the bootscript file from the start of the ram base in systems with RAM detection. Signed-off-by: Algapally Santosh Sagar Signed-off-by: Michal Simek --- The patch targets dtschema repository. We would like to get opinion about option names before sending PR via github. I sent pull request to dt-schema. https://github.com/devicetree-org/dt-schema/pull/105 Thanks, Michal
Re: [PATCH v3 2/2] firmware: zynqmp: Move permission to change config object message
Hi, On 5/22/23 12:55, Stefan Herbrechtsmeier wrote: Hi Michal, Am 18.05.2023 um 13:29 schrieb Michal Simek: On 5/17/23 16:11, Stefan Herbrechtsmeier wrote: Am 17.05.2023 um 14:12 schrieb Michal Simek: On 5/16/23 16:05, Stefan Herbrechtsmeier wrote: From: Stefan Herbrechtsmeier Move the permission to change a config object message from zynqmp_pmufw_load_config_object function to zynqmp_pmufw_node function to simplify the code and check the permission only if required. Signed-off-by: Stefan Herbrechtsmeier --- Changes in v4: - Reword - Move the check back to zynqmp_pmufw_node because the check need to be run after the config object load. - Return error in zynqmp_pmufw_config_close and zynqmp_pmufw_node Changes in v3: - Added drivers/firmware/firmware-zynqmp.c | 36 ++ 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index 2b1ad5d2c3..6dc745bd14 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -63,29 +63,32 @@ static unsigned int xpm_configobject_close[] = { int zynqmp_pmufw_config_close(void) { - zynqmp_pmufw_load_config_object(xpm_configobject_close, - sizeof(xpm_configobject_close)); - return 0; + return zynqmp_pmufw_load_config_object(xpm_configobject_close, + sizeof(xpm_configobject_close)); } int zynqmp_pmufw_node(u32 id) { - static bool skip_config; - int ret; + static bool checked; + static bool skip; I see interesting behavior in connection to these variables. I did this change and keep test variable to see behavior. diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index 6dc745bd1424..becbea7b64ea 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -67,10 +67,14 @@ int zynqmp_pmufw_config_close(void) sizeof(xpm_configobject_close)); } +static bool checked; +static bool skip; + int zynqmp_pmufw_node(u32 id) { - static bool checked; - static bool skip; + static bool test; + + printf("%s, id %d, ch %d, skp %d - test %d\n", __func__, id, checked, skip, test); if (!checked) { checked = true; @@ -379,6 +391,9 @@ static int zynqmp_firmware_bind(struct udevice *dev) int ret; struct udevice *child; + checked = 0; + skip = 0; + if ((IS_ENABLED(CONFIG_SPL_BUILD) && IS_ENABLED(CONFIG_SPL_POWER_DOMAIN) && IS_ENABLED(CONFIG_ZYNQMP_POWER_DOMAIN)) || zynqmp_power_domain zynqmp_power_domain: Request for id: 34 zynqmp_pmufw_node, id 34, ch 0, skp 0 - test 255/815a2fa zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 255/815a2fa ---zynqmp_pmufw_node ACCESS OK zynqmp_pmufw_load_config_object zynqmp_pmufw_load_config_object44 ---zynqmp_pmufw_node ACCESS OK zynqmp_pmufw_load_config_object zynqmp_pmufw_load_config_object44 zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 34 zynq_serial_setbrg: CLK U-Boot 2023.07-rc2-00053-gaf7817988644-dirty (May 17 2023 - 14:03:37 +0200) CPU: ZynqMP Silicon: v3 Chip: xck26 zynqmp_power_domain zynqmp_power_domain: Request for id: 38 zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/815a2fa ---zynqmp_pmufw_node ACCESS OK zynqmp_pmufw_load_config_object zynqmp_pmufw_load_config_object44 zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 38 Detected name: zynqmp-smk-k26-xcl2g-revA-sck-kv-g-revB Model: ZynqMP KV260 revB Board: Xilinx ZynqMP DRAM: 2 GiB (effective 4 GiB) zynqmp_power_domain zynqmp_power_domain: Request for id: 46 zynqmp_pmufw_node, id 46, ch 0, skp 0 - test 0/7ffd42fa zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 0/7ffd42fa ---zynqmp_pmufw_node ACCESS OK zynqmp_pmufw_load_config_object zynqmp_pmufw_load_config_object44 ---zynqmp_pmufw_node ACCESS OK zynqmp_pmufw_load_config_object zynqmp_pmufw_load_config_object44 zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 46 PMUFW: v1.1 zynqmp_power_domain zynqmp_power_domain: Request for id: 38 zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/7ffd42fa ---zynqmp_pmufw_node ACCESS OK As you see test variable is in BSS section but it is not initialized at this stage. If you look at arch/arm/lib/crt0_64.S debug uart is called before calling board_init_f and bss is cleared before board_init_r is called. What does "but BSS and initialized non-const data are still not available" mean? Could we use variables from the data section like "static bool check = true"? Yes - when you move that variable to data section then it should be fine. Or just move it like this struct fru_table fru_d
[PATCH v2 1/1] arm: dts: icnova-a20-adb4006: Add board support
Add board support for ICnova A20 SomPi compute module on ICnova ADB4006 development board. Specification: SoM - Processor: Allwinner A20 Cortex-A7 Dual Core at 1GHz - 512MB DDR3 RAM - Fast Ethernet (Phy: Realtek RTL8201CP) ADB4006 - I2C - 2x USB 2.0 - 1x Fast Ethernet port - 1x SATA - 2x buttons (PWRON, Boot) - 2x LEDS - serial console - HDMI - µSD-Card slot - Audio Line-In / Line-Out - GPIO pinheaders https://wiki.in-circuit.de/index.php5?title=ICnova_ADB4006 https://wiki.in-circuit.de/index.php5?title=ICnova_A20_SODIMM changes in v2: - rebase on v2023.07-rc2 - remove pin defines from defconfig - get dts reviewed on the linux mailing list and scheduled for kernel 6.5 [1] [1] https://git.kernel.org/pub/scm/linux/kernel/git/sunxi/linux.git commit de2bdfb7f79d5c655eb056d459e02be2c7f13c8b Signed-off-by: Ludwig Kormann --- arch/arm/dts/Makefile | 1 + arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts | 137 ++ arch/arm/dts/sun7i-a20-icnova-a20.dtsi| 62 board/sunxi/MAINTAINERS | 5 + configs/icnova-a20-adb4006_defconfig | 20 +++ 5 files changed, 225 insertions(+) create mode 100644 arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts create mode 100644 arch/arm/dts/sun7i-a20-icnova-a20.dtsi create mode 100644 configs/icnova-a20-adb4006_defconfig diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 935b2f1517..0b0636a532 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -631,6 +631,7 @@ dtb-$(CONFIG_MACH_SUN7I) += \ sun7i-a20-haoyu-marsboard.dtb \ sun7i-a20-hummingbird.dtb \ sun7i-a20-i12-tvbox.dtb \ + sun7i-a20-icnova-a20-adb4006.dtb \ sun7i-a20-icnova-swac.dtb \ sun7i-a20-itead-ibox.dtb \ sun7i-a20-lamobo-r1.dtb \ diff --git a/arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts b/arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts new file mode 100644 index 00..577ead1d02 --- /dev/null +++ b/arch/arm/dts/sun7i-a20-icnova-a20-adb4006.dts @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: (GPL-2.0 OR MIT) +// Copyright (C) 2023 In-Circuit GmbH + +/dts-v1/; + +#include "sun7i-a20-icnova-a20.dtsi" + +#include +#include + +/ { + model = "In-Circuit ICnova A20 ADB4006"; + compatible = "incircuit,icnova-a20-adb4006", "incircuit,icnova-a20", +"allwinner,sun7i-a20"; + + aliases { + serial0 = &uart0; + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; + + hdmi-connector { + compatible = "hdmi-connector"; + type = "a"; + + port { + hdmi_con_in: endpoint { + remote-endpoint = <&hdmi_out_con>; + }; + }; + }; + + leds { + compatible = "gpio-leds"; + + led-0 { + function = LED_FUNCTION_POWER; + color = ; + gpios = <&pio 7 21 GPIO_ACTIVE_HIGH>; /* PH21 */ + default-state = "on"; + }; + + led-1 { + function = LED_FUNCTION_HEARTBEAT; + color = ; + gpios = <&pio 7 20 GPIO_ACTIVE_HIGH>; /* PH20 */ + linux,default-trigger = "heartbeat"; + }; + }; +}; + +&ahci { + target-supply = <®_ahci_5v>; + status = "okay"; +}; + +&codec { + status = "okay"; +}; + +&de { + status = "okay"; +}; + +&ehci0 { + status = "okay"; +}; + +&ehci1 { + status = "okay"; +}; + +&hdmi { + status = "okay"; +}; + +&hdmi_out { + hdmi_out_con: endpoint { + remote-endpoint = <&hdmi_con_in>; + }; +}; + +&mmc0 { + vmmc-supply = <®_vcc3v3>; + bus-width = <4>; + cd-gpios = <&pio 7 1 GPIO_ACTIVE_LOW>; /* PH1 */ + status = "okay"; +}; + +&ohci0 { + status = "okay"; +}; + +&ohci1 { + status = "okay"; +}; + +&otg_sram { + status = "okay"; +}; + +®_ahci_5v { + status = "okay"; +}; + +&ac_power_supply { + status = "okay"; +}; + +®_usb1_vbus { + status = "okay"; +}; + +®_usb2_vbus { + status = "okay"; +}; + +&uart0 { + pinctrl-names = "default"; + pinctrl-0 = <&uart0_pb_pins>; + status = "okay"; +}; + +&usb_otg { + dr_mode = "otg"; + status = "okay"; +}; + +&usbphy { + usb0_id_det-gpios = <&pio 7 4 GPIO_ACTIVE_HIGH>; /* PH4 */ + usb0_vbus_det-gpios = <&pio 7 5 GPIO_ACTIVE_HIGH>; /* PH5 */ + usb1_vbus-supply = <®_usb1_vbus>; + usb2_vbus-supply = <®_usb2_vbus>; + status = "okay"; +}; diff --git a/arch/arm/dts/sun7i-a20-icnova-a20.dtsi b/arch/arm/dts/sun7i-a20-icnova-a20.dtsi new file mode 100644 index 00..46616c6bc8 --- /dev/null +++ b/arch/arm/dts/sun7i-a20-icnova-a20.dtsi @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: (GPL
Re: [PATCH v3 2/2] firmware: zynqmp: Move permission to change config object message
Hi Michal, Am 18.05.2023 um 13:29 schrieb Michal Simek: On 5/17/23 16:11, Stefan Herbrechtsmeier wrote: Am 17.05.2023 um 14:12 schrieb Michal Simek: On 5/16/23 16:05, Stefan Herbrechtsmeier wrote: From: Stefan Herbrechtsmeier Move the permission to change a config object message from zynqmp_pmufw_load_config_object function to zynqmp_pmufw_node function to simplify the code and check the permission only if required. Signed-off-by: Stefan Herbrechtsmeier --- Changes in v4: - Reword - Move the check back to zynqmp_pmufw_node because the check need to be run after the config object load. - Return error in zynqmp_pmufw_config_close and zynqmp_pmufw_node Changes in v3: - Added drivers/firmware/firmware-zynqmp.c | 36 ++ 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index 2b1ad5d2c3..6dc745bd14 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -63,29 +63,32 @@ static unsigned int xpm_configobject_close[] = { int zynqmp_pmufw_config_close(void) { - zynqmp_pmufw_load_config_object(xpm_configobject_close, - sizeof(xpm_configobject_close)); - return 0; + return zynqmp_pmufw_load_config_object(xpm_configobject_close, + sizeof(xpm_configobject_close)); } int zynqmp_pmufw_node(u32 id) { - static bool skip_config; - int ret; + static bool checked; + static bool skip; I see interesting behavior in connection to these variables. I did this change and keep test variable to see behavior. diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index 6dc745bd1424..becbea7b64ea 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -67,10 +67,14 @@ int zynqmp_pmufw_config_close(void) sizeof(xpm_configobject_close)); } +static bool checked; +static bool skip; + int zynqmp_pmufw_node(u32 id) { - static bool checked; - static bool skip; + static bool test; + + printf("%s, id %d, ch %d, skp %d - test %d\n", __func__, id, checked, skip, test); if (!checked) { checked = true; @@ -379,6 +391,9 @@ static int zynqmp_firmware_bind(struct udevice *dev) int ret; struct udevice *child; + checked = 0; + skip = 0; + if ((IS_ENABLED(CONFIG_SPL_BUILD) && IS_ENABLED(CONFIG_SPL_POWER_DOMAIN) && IS_ENABLED(CONFIG_ZYNQMP_POWER_DOMAIN)) || zynqmp_power_domain zynqmp_power_domain: Request for id: 34 zynqmp_pmufw_node, id 34, ch 0, skp 0 - test 255/815a2fa zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 255/815a2fa ---zynqmp_pmufw_node ACCESS OK zynqmp_pmufw_load_config_object zynqmp_pmufw_load_config_object44 ---zynqmp_pmufw_node ACCESS OK zynqmp_pmufw_load_config_object zynqmp_pmufw_load_config_object44 zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 34 zynq_serial_setbrg: CLK U-Boot 2023.07-rc2-00053-gaf7817988644-dirty (May 17 2023 - 14:03:37 +0200) CPU: ZynqMP Silicon: v3 Chip: xck26 zynqmp_power_domain zynqmp_power_domain: Request for id: 38 zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/815a2fa ---zynqmp_pmufw_node ACCESS OK zynqmp_pmufw_load_config_object zynqmp_pmufw_load_config_object44 zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 38 Detected name: zynqmp-smk-k26-xcl2g-revA-sck-kv-g-revB Model: ZynqMP KV260 revB Board: Xilinx ZynqMP DRAM: 2 GiB (effective 4 GiB) zynqmp_power_domain zynqmp_power_domain: Request for id: 46 zynqmp_pmufw_node, id 46, ch 0, skp 0 - test 0/7ffd42fa zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 0/7ffd42fa ---zynqmp_pmufw_node ACCESS OK zynqmp_pmufw_load_config_object zynqmp_pmufw_load_config_object44 ---zynqmp_pmufw_node ACCESS OK zynqmp_pmufw_load_config_object zynqmp_pmufw_load_config_object44 zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 46 PMUFW: v1.1 zynqmp_power_domain zynqmp_power_domain: Request for id: 38 zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/7ffd42fa ---zynqmp_pmufw_node ACCESS OK As you see test variable is in BSS section but it is not initialized at this stage. If you look at arch/arm/lib/crt0_64.S debug uart is called before calling board_init_f and bss is cleared before board_init_r is called. What does "but BSS and initialized non-const data are still not available" mean? Could we use variables from the data section like "static bool check = true"? Yes - when you move that variable to data section then it should be fine. Or just move it like this struct fru_table fru_data __section(".data"); Okay, I will use
Re: [RFC PATCH 00/10] Improve ARM target's support for LLVM toolchain
Hi On 5/21/23 06:59, Sam Edwards wrote: On 5/20/23 22:26, Heinrich Schuchardt wrote: Hello Sam, Hi Heinrich! Good to hear from you. I guess the documentation and the CI testing would also have to be adjusted. Ah, yeah, those are going to be big things for me to look at when this series starts to mature out of the RFC phase. CI is definitely important so that the hard-won compatibility doesn't just decay away. :) What about non-ARM architectures? If there's a groundswell of demand for building U-Boot on LLVM, I'd be willing to collaborate with others on getting the other architectures up to parity with GNU. But since the linker scripts, relocation thunks, sections, and whatnot are all arch-specific, I'm only focusing on ARM for now (which is both the arch I need and one of the more common ones). Is there a particular arch you'd like to see next? It seems everything U-Boot supports is supported by LLVM, except for Microblaze, NIOS2, and SH. Could you, please, describe how built with lld so that reviewers can test it. I've been building with: nice make CC='clang --target=armv7l-none-eabi' \ LTO_FINAL_LDFLAGS=-fuse-ld=lld LD=ld.lld OBJCOPY=llvm-objcopy ...though mostly at this stage I'm just hoping for folks to confirm that this patchset causes no regressions in their existing GNU environments. (Feedback from LLVM-land would be appreciated nonetheless, though!!!) Dockerfile in repo as I see is using 3 toolchain categories. 1. llvm deb repo 2. kernel.org 3. others - xtensa/arc For CI loop you should pretty much provide a way how to get toolchain. That's why would be good to figure it out and then I am happy to take a look at changed you have done for Zynq. Definitely nice to see this happening and I expect more warnings will be visible and they should be fixed. Thanks, Michal
Re: [PATCH] imx8m: soc.c: demote some printfs to debug
On 22.05.23 11:27, Rasmus Villemoes wrote: > Getting > > Found /vpu_g1@3830 node > Modify /vpu_g1@3830:status disabled > Found /vpu_g2@3831 node > Modify /vpu_g2@3831:status disabled > > etc. on the console on every boot is needlessly verbose. Demote the > "Found ..." lines to debug(), which is consistent with other instances > in soc.c. > > Signed-off-by: Rasmus Villemoes Reviewed-by: Frieder Schrempf
[PATCH] imx8m: soc.c: demote some printfs to debug
Getting Found /vpu_g1@3830 node Modify /vpu_g1@3830:status disabled Found /vpu_g2@3831 node Modify /vpu_g2@3831:status disabled etc. on the console on every boot is needlessly verbose. Demote the "Found ..." lines to debug(), which is consistent with other instances in soc.c. Signed-off-by: Rasmus Villemoes --- arch/arm/mach-imx/imx8m/soc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 5a4f8358c9..f5c82dff35 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -736,7 +736,7 @@ static int disable_fdt_nodes(void *blob, const char *const nodes_path[], int siz if (nodeoff < 0) continue; /* Not found, skip it */ - printf("Found %s node\n", nodes_path[i]); + debug("Found %s node\n", nodes_path[i]); add_status: rc = fdt_setprop(blob, nodeoff, "status", status, strlen(status) + 1); @@ -1265,7 +1265,7 @@ int ft_system_setup(void *blob, struct bd_info *bd) if (nodeoff >= 0) { const char *speed = "high-speed"; - printf("Found %s node\n", usb_dwc3_path[v]); + debug("Found %s node\n", usb_dwc3_path[v]); usb_modify_speed: -- 2.37.2
Re: [PATCH v12 00/10] introduce Arm FF-A support
Hi Simon, > Adding support for Arm FF-A v1.0 (Arm Firmware Framework for Armv8-A) [A]. > > FF-A specifies interfaces that enable a pair of software execution > environments aka partitions to > communicate with each other. A partition could be a VM in the Normal or > Secure world, an > application in S-EL0, or a Trusted OS in S-EL1. > > FF-A is a discoverable bus and similar to architecture features. > FF-A bus is discovered using ARM_SMCCC_FEATURES mechanism performed > by the PSCI driver. > >=> dm tree > > Class Index Probed DriverName >--- >... > firmware 0 [ + ] psci |-- psci > ffa 0 [ ] arm_ffa | `-- arm_ffa >... > > Clients are able to probe then use the FF-A bus by calling the DM class > searching APIs (e.g: uclass_first_device). > > This implementation of the specification provides support for Aarch64. > > The FF-A driver uses the SMC ABIs defined by the FF-A specification to: > > - Discover the presence of secure partitions (SPs) of interest > - Access an SP's service through communication protocols > (e.g: EFI MM communication protocol) > > The FF-A support provides the following features: > > - Being generic by design and can be used by any Arm 64-bit platform > - FF-A support can be compiled and used without EFI > - Support for SMCCCv1.2 x0-x17 registers > - Support for SMC32 calling convention > - Support for 32-bit and 64-bit FF-A direct messaging > - Support for FF-A MM communication (compatible with EFI boot time) > - Enabling FF-A and MM communication in Corstone1000 platform as a use > case > - A Uclass driver providing generic FF-A methods. > - An Arm FF-A device driver providing Arm-specific methods and reusing > the Uclass methods. > - A sandbox emulator for Arm FF-A, emulates the FF-A side of the Secure > World and provides > FF-A ABIs inspection methods. > - An FF-A sandbox device driver for FF-A communication with the emulated > Secure World. > The driver leverages the FF-A Uclass to establish FF-A communication. > - Sandbox FF-A test cases. > - A new command called armffa is provided as an example of how to access > the > FF-A bus > > For more details about the FF-A support please refer to [B] and refer to [C] > for > how to use the armffa command. > > Please find at [D] an example of the expected boot logs when enabling > FF-A support for a platform. In this example the platform is > Corstone1000. But it can be any Arm 64-bit platform. > > Changelog of the major changes: > === > > v12: > > * remove the global variable (dscvry_info), use uc_priv instead > * replace dscvry_info.invoke_ffa_fn() with a weak invoke_ffa_fn >(user drivers can override it) > * improve FFA_PARTITION_INFO_GET implementation >(clients no longer need to calloc a buffer) > * remove reparenting by making the sandbox emulator parent of the FF-A device > in the DT > * improve argument checks for the armffa command > * address nits > > v11: [11] > > * move ffa_try_discovery() from the uclass to the Arm FF-A driver > * rename ffa_try_discovery() to arm_ffa_discover() > * add arm_ prefix to the Arm FF-A driver functions > * use U_BOOT_CMD_WITH_SUBCMDS for armffa command > * store the sandbox emulator pointer in the FF-A device uc_priv (struct > ffa_priv) > * set the emulator as parent of the sandbox FF-A device > * rename select_ffa_mm_comms() to select_mm_comms() > * improve the logic of MM transport selection in mm_communicate() > * use ut_asserteq_mem() in uuid_str_to_le_bin test case > * address nits > > v10: [10] > > * provide the FF-A driver operations through the Uclass (arm-ffa-uclass.c) > * move the generic FF-A methods to the Uclass > * keep Arm specific methods in the Arm driver (arm-ffa.c renamed from core.c) > * split the FF-A sandbox support into an emulator (ffa-emul-uclass.c) and a > driver (sandbox_ffa.c) > * use the FF-A driver Uclass operations by clients (armffa command, tests, MM > comms) > * use uclass_first_device to search and probe the FF-A device (whether it is > on Arm or on sandbox) > * address nits > > v9: [9] > > * integrate the FF-A bus discovery in the DM and use ARM_SMCCC_FEATURES for > binding > * align FF-A sandbox driver with FF-A discovery through DM > * use DM class APIs to probe and interact with the FF-A bus (in FF-A MM > comms, armffa command, sandbox tests) > * add documentation for the armffa command: doc/usage/cmd/armffa.rst > * introduce testcase for uuid_str_to_le_bin > > v8: [8] > > * pass the FF-A bus device to the bus operations > * isolate the compilation choices between FF-A and OP-TEE > * drop OP-TEE configs from Corstone-1000 defconfig > * make ffa_get_partitions_info() second argument to be an SP count in both > modes > > v7: [7] > > *
Re: [RFC PATCH 0/5] LWIP stack integration
My measurements for binary after LTO looks like: U-boot WGET | LWIP WGET + ping | LWIP WGET| diff bytes| diff % 870728| 915000| 912560 | 41832 | 4.8 BR, Maxim. On Fri, 19 May 2023 at 09:52, Tom Rini wrote: > On Fri, May 19, 2023 at 04:17:06PM +0300, Ilias Apalodimas wrote: > > Hi Tom, > > > > Apologies for being late to the party > > > > > On Thu, May 11, 2023 at 09:52:04AM -0400, Tom Rini wrote: > > > On Fri, May 05, 2023 at 10:25:24AM +, Maxim Uvarov wrote: > > > > > > > Greetings, > > > > > > > > This RFC patchset is an attempt to try to use an already existing IP > network stack inside U-boot. > > > > U-Boot recently got basic TCP/IP support, implementing wget, but in > order to get a full IP stack > > > > with new features (e.g ipv6), it would be preferable to use an > established embedded ip library, > > > > instead of rewriting the code from scratch. > > > > > > > > For this experiment LWIP network stack was selected: > > > > https://savannah.nongnu.org/git/?group=lwip > > > > > > > > LWIP main features include: > > > > - Protocols: IP, IPv6, ICMP, ND, MLD, UDP, TCP, IGMP, ARP, PPPoS, > PPPoE > > > > - DHCP client, DNS client (incl. mDNS hostname resolver), > AutoIP/APIPA (Zeroconf), > > > > SNMP agent (v1, v2c, v3, private MIB support & MIB compiler) > > > > - APIs: specialized APIs for enhanced performance, optional > Berkeley-alike socket API > > > > - Extended features: IP forwarding over multiple network interfaces, > TCP congestion control, > > > > RTT estimation and fast recovery/fast retransmit > > > > - Addon applications: HTTP(S) server, SNTP client, SMTP(S) client, > ping, NetBIOS nameserver, > > > > mDNS responder, MQTT client, TFTP server. > > > > > > > > This RFC work is a demo to enable lwIP (lightweight IP) which is a > widely used open-source > > > > TCP/IP stack designed for embedded systems for U-boot. That will > allow using already > > > > written network applications for microcontrollers. > > > > > > > > lwIP is licensed under a BSD-style license: > http://lwip.wikia.com/wiki/License. > > > > Which should be compatible with u-boot. > > > > > > > > In the current RFC I tried to use minimal changes to better see how > LWIP code can be embedded into > > > > U-boot. Patches implement ping and wget commands work. Both commands > are currently copy pasting and > > > > reusing lwIP examples. Whether we want to add the final application > in U-Boot or lwIP is up to > > > > discussion, but the current approach was the easiest one for an RFC. > > > > > > I'm honestly not sure this is the most useful way of doing an RFC. The > > > long term goal would be that we replace our existing net/ with lwIP, > > > yes? So what I'd see as more valuable is what it looks like to limit > > > yourself to either sandbox or some QEMU target, disable the current > > > network stack, and instead use lwIP to support just cmd/net.c so that > > > the scope of the conversion is visible. Then the size comparison you > do > > > should be between platform + net + cmd/net.c (and the rest of > networking > > > turned off) and platform + lwip + cmd/net.c converted. > > > > This might be a bit too much imho. How about replacing the TCP stack > which > > is new an under heavy devel as well. If we do that we could replace lwip > > with the version Maxim proposes and check the difference between > > U-boot + homegrown TCP + wget > > U-Boot + LWIP (for tcp only) + new wget > > > > That would give us an idea before trying to replace the UDP portion which > > is way bigger > > I guess we can try that as a starting point and see what things look > like. My gut feeling however is that's not going to look like a win. > > -- > Tom >
[PATCH] phy: rockchip: inno-usb2: fix phy reg=0 case
The support for #address-cells=2 has a loophole: if the reg is actually 0, but the #address-cells is actually 1, like in such case below: syscon { #address-cells = <1>; phy { reg = <0 0x10>; }; }; then the second u32 of the 'reg' is the size, not the address. The code should check for the parent's #address-cells value, and not assume that if the first u32 is 0, then the #address-cells is 2, and the reg property is something like reg = <0 0xff00 0x10>; Fixed this by looking for the #address-cells value and retrieving the reg address only if this is ==2. To avoid breaking anything I also kept the check `if reg==0` as some DT's may have a wrong #address-cells as parent and even if this commit is correct, it might break the existing wrong device-trees. Fixes: d538efb9adcf ("phy: rockchip: inno-usb2: Add support #address_cells = 2") Signed-off-by: Eugen Hristev --- drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c index 22e2797eea28..e5e02b6765b1 100644 --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c @@ -186,7 +186,7 @@ static int rockchip_usb2phy_probe(struct udevice *dev) } /* support address_cells=2 */ - if (reg == 0) { + if (dev_read_addr_cells(dev) == 2 && reg == 0) { if (ofnode_read_u32_index(dev_ofnode(dev), "reg", 1, ®)) { dev_err(dev, "%s must have reg[1]\n", ofnode_get_name(dev_ofnode(dev))); -- 2.34.1
Re: [RFC PATCH 08/10] arm: efi_loader: move .dynamic out of .text in EFI
On 5/20/23 22:55, Sam Edwards wrote: This is not proper: A .text section is SHT_PROGBITS, while the .dynamic section is SHT_DYNAMIC. Attempting to combine them like this creates a section type mismatch. It does seem that GNU ld does not complain, but LLVM's lld considers this an error. Signed-off-by: Sam Edwards Cc: Heinrich Schuchardt --- arch/arm/lib/elf_aarch64_efi.lds | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arch/arm/lib/elf_aarch64_efi.lds b/arch/arm/lib/elf_aarch64_efi.lds index 5dd9809169..986f13936d 100644 --- a/arch/arm/lib/elf_aarch64_efi.lds +++ b/arch/arm/lib/elf_aarch64_efi.lds @@ -24,10 +24,9 @@ SECTIONS *(.gnu.linkonce.t.*) *(.srodata) *(.rodata*) - . = ALIGN(16); .dynamic should be aligned. Structure Elf64_Dyn requires at least 8 byte alignment. - *(.dynamic); . = ALIGN(512); The symbol _etext below should be 512 aligned as in arch/arm/lib/crt0_aarch64_efi.S we have set FileAlignment = 0x200. Best regards Heinrich } + .dynamic : { *(.dynamic) } .rela.dyn : { *(.rela.dyn) } .rela.plt : { *(.rela.plt) } .rela.got : { *(.rela.got) }
Re: [RFC PATCH] mmc: zynq_sdhci: Dependable card detect
Hi Ashok, Am 18.05.2023 um 08:20 schrieb Soma, Ashok Reddy: We haven't seen any instability issue so far on ZynqMP boards and no one reported any such issue till now. It isn't an instability issue. The high level at the card detect input is not marked as stable until a short low level is detected. We are already taking care of card detection stability issue with below waiting logic. https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/mmc/zynq_sdhci.c#L1161 This code makes problems for us. The SDHCI_CARD_STATE_STABLE signal isn't set without a card and we see a "Sdhci card detect state not stable" message. Could you please answer below questions. 1. is this observed in SPL only or full u-boot ? Both. We have enable environment support in SPL and see the "Sdhci card detect state not stable" message after "Loading Environment from MMC..." in SPL and "MMC: " in full U-Boot. 2. is this observed in dynamic config case only ? because your patch is in dynamic path. No. I have override the return value of the zynqmp_pm_is_function_supported function and see the same error. Without dynamic case I don't see a possibility to set the SD_CONFIG_EMMC_SEL value. 3. "On our hardware we get a "Sdhci card detect state not stable" error in the SPL if no mmc card is present" Are you seeing stability issue when card is not present ? No, the SDHCI_CARD_STATE_STABLE signal is never set. can you please elaborate more about the issue. I have change the code to monitor the SDHCI_PRESENT_STATE signal. Without a card at boot the SDHCI_CARD_STATE_STABLE signal is unset. After card insert the SDHCI_CARD_PRESENT signal and the SDHCI_CARD_STATE_STABLE signal is set after some time. After card remove the SDHCI_CARD_STATE_STABLE signal is set and the SDHCI_CARD_PRESENT signal is unset after some time. It doesn't matter if a physical card is insert or the SD_CONFIG_EMMC_SEL config is used to emulate a present card. Without a short card present signal on the card detect logic the SDHCI_CARD_STATE_STABLE signal isn't set. 4. It is not convincing to me that you are trying to select eMMC for some time and then switching back to SD. I don't find a documentation what the consequence of this config is but this config is set based on the value of "non-removable" and setting this config with enabled clock leads to a functional SDHCI_CARD_STATE_STABLE signal even after unset of the config. Maybe the SD_CDn_CTRL register have the same result but this isn't available via PMU. Regards Stefan -Original Message- From: U-Boot On Behalf Of Stefan Herbrechtsmeier Sent: Tuesday, May 16, 2023 7:51 PM To: u-boot@lists.denx.de Cc: Stefan Herbrechtsmeier ; Jaehoon Chung ; Simek, Michal ; Peng Fan Subject: [RFC PATCH] mmc: zynq_sdhci: Dependable card detect CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email. From: Stefan Herbrechtsmeier The card detect logic needs a short card present signal to work dependable. Without a present card the SDHCI_CARD_STATE_STABLE signal is not set dependable after a reset. Use the internal fixed card present signal to initiate the card detect logic. Signed-off-by: Stefan Herbrechtsmeier --- On our hardware we get a "Sdhci card detect state not stable" error in the SPL if no mmc card is present. It is unclear if this patch is the correct solution, but a short card inserts or a fixed card present signal leads to a SDHCI_CARD_STATE_STABLE signal with and without card. drivers/mmc/zynq_sdhci.c | 20 1 file changed, 20 insertions(+) diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index e44868aaec..a88feeb367 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -1075,6 +1075,26 @@ static int sdhci_zynqmp_set_dynamic_config(struct arasan_sdhci_priv *priv, return ret; } + /* The card detect logic needs a short card present signal to work +* dependable. Without a present card the SDHCI_CARD_STATE_STABLE +* signal is not set dependable after a reset. Use the internal +* fixed card present signal to initiate the card detect logic. +*/ + if (!dev_read_bool(dev, "non-removable")) { + ret = zynqmp_pm_set_sd_config(priv->node_id, SD_CONFIG_EMMC_SEL, + 1); + if (ret) { + dev_err(dev, "SD_CONFIG_EMMC_SEL failed\n"); + return ret; + } + ret = zynqmp_pm_set_sd_config(priv->node_id, SD_CONFIG_EMMC_SEL, + 0); + if (ret) { + dev_err(dev, "SD_CONFIG_EMMC_SEL failed\n"); + return ret; + } + } + return 0; } #endif -- 2.3
Re: [PATCH v6 1/8] efi_loader: add the number of image entries in efi_capsule_update_info
On Fri, May 19, 2023 at 07:32:07PM +0900, Masahisa Kojima wrote: > The number of image array entries global variable is required > to support EFI capsule update. This information is exposed as a > num_image_type_guids variable, but this information > should be included in the efi_capsule_update_info structure. > > This commit adds the num_images member in the > efi_capsule_update_info structure. All board files supporting > EFI capsule update are updated. > > Signed-off-by: Masahisa Kojima > --- > Newly created in v6 > > arch/arm/mach-rockchip/board.c | 4 ++-- > board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c| 2 +- > board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c | 2 +- > board/emulation/qemu-arm/qemu-arm.c| 2 +- > board/kontron/pitx_imx8m/pitx_imx8m.c | 2 +- > board/kontron/sl-mx8mm/sl-mx8mm.c | 2 +- > board/kontron/sl28/sl28.c | 2 +- > board/rockchip/evb_rk3399/evb-rk3399.c | 2 +- > board/sandbox/sandbox.c| 2 +- > board/socionext/developerbox/developerbox.c| 2 +- > board/st/stm32mp1/stm32mp1.c | 2 +- > board/xilinx/common/board.c| 2 +- > include/efi_loader.h | 3 ++- > lib/efi_loader/efi_firmware.c | 6 +++--- > lib/fwu_updates/fwu.c | 2 +- > 15 files changed, 19 insertions(+), 18 deletions(-) > > diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c > index f1f70c81d0..8daa74b3eb 100644 > --- a/arch/arm/mach-rockchip/board.c > +++ b/arch/arm/mach-rockchip/board.c > @@ -41,7 +41,7 @@ static bool updatable_image(struct disk_partition *info) > uuid_str_to_bin(info->type_guid, image_type_guid.b, > UUID_STR_FORMAT_GUID); > > - for (i = 0; i < num_image_type_guids; i++) { > + for (i = 0; i < update_info.num_images; i++) { > if (!guidcmp(&fw_images[i].image_type_id, &image_type_guid)) { > ret = true; > break; > @@ -59,7 +59,7 @@ static void set_image_index(struct disk_partition *info, > int index) > uuid_str_to_bin(info->type_guid, image_type_guid.b, > UUID_STR_FORMAT_GUID); > > - for (i = 0; i < num_image_type_guids; i++) { > + for (i = 0; i < update_info.num_images; i++) { > if (!guidcmp(&fw_images[i].image_type_id, &image_type_guid)) { > fw_images[i].image_index = index; > break; > diff --git a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > index 466174679e..b79a2380aa 100644 > --- a/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > +++ b/board/advantech/imx8mp_rsb3720a1/imx8mp_rsb3720a1.c > @@ -54,10 +54,10 @@ struct efi_fw_image fw_images[] = { > > struct efi_capsule_update_info update_info = { > .dfu_string = "mmc 2=flash-bin raw 0 0x1B00 mmcpart 1", > + .num_images = ARRAY_SIZE(fw_images), > .images = fw_images, > }; > > -u8 num_image_type_guids = ARRAY_SIZE(fw_images); > #endif /* EFI_HAVE_CAPSULE_SUPPORT */ > > > diff --git a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > index b373e45df9..af070ec315 100644 > --- a/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > +++ b/board/compulab/imx8mm-cl-iot-gate/imx8mm-cl-iot-gate.c > @@ -50,10 +50,10 @@ struct efi_fw_image fw_images[] = { > > struct efi_capsule_update_info update_info = { > .dfu_string = "mmc 2=flash-bin raw 0x42 0x1D00 mmcpart 1", > + .num_images = ARRAY_SIZE(fw_images), > .images = fw_images, > }; > > -u8 num_image_type_guids = ARRAY_SIZE(fw_images); > #endif /* EFI_HAVE_CAPSULE_SUPPORT */ > > int board_phys_sdram_size(phys_size_t *size) > diff --git a/board/emulation/qemu-arm/qemu-arm.c > b/board/emulation/qemu-arm/qemu-arm.c > index 34ed3e8ae6..dfea0d92a3 100644 > --- a/board/emulation/qemu-arm/qemu-arm.c > +++ b/board/emulation/qemu-arm/qemu-arm.c > @@ -47,10 +47,10 @@ struct efi_fw_image fw_images[] = { > }; > > struct efi_capsule_update_info update_info = { > + .num_images = ARRAY_SIZE(fw_images) > .images = fw_images, > }; > > -u8 num_image_type_guids = ARRAY_SIZE(fw_images); > #endif /* EFI_HAVE_CAPSULE_SUPPORT */ > > static struct mm_region qemu_arm64_mem_map[] = { > diff --git a/board/kontron/pitx_imx8m/pitx_imx8m.c > b/board/kontron/pitx_imx8m/pitx_imx8m.c > index fcda86bc1b..4548e7c1df 100644 > --- a/board/kontron/pitx_imx8m/pitx_imx8m.c > +++ b/board/kontron/pitx_imx8m/pitx_imx8m.c > @@ -43,10 +43,10 @@ struct efi_fw_image fw_images[] = { > > struct efi_capsule_update_info update_info = { > .dfu_string = "mmc 0=flash-bin raw 0x42 0x1000 mmcpart 1", > + .num_images = ARRAY
Re: [RFC PATCH 08/10] arm: efi_loader: move .dynamic out of .text in EFI
Hi Sam, On Sat, 20 May 2023 at 23:56, Sam Edwards wrote: > > This is not proper: A .text section is SHT_PROGBITS, > while the .dynamic section is SHT_DYNAMIC. Attempting to > combine them like this creates a section type mismatch. > > It does seem that GNU ld does not complain, but LLVM's lld > considers this an error. > > Signed-off-by: Sam Edwards > Cc: Heinrich Schuchardt > --- > > arch/arm/lib/elf_aarch64_efi.lds | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/arch/arm/lib/elf_aarch64_efi.lds > b/arch/arm/lib/elf_aarch64_efi.lds > index 5dd9809169..986f13936d 100644 > --- a/arch/arm/lib/elf_aarch64_efi.lds > +++ b/arch/arm/lib/elf_aarch64_efi.lds > @@ -24,10 +24,9 @@ SECTIONS > *(.gnu.linkonce.t.*) > *(.srodata) > *(.rodata*) > - . = ALIGN(16); > - *(.dynamic); > . = ALIGN(512); > } > + .dynamic : { *(.dynamic) } This looks correct. However, this changed recently on commit d7ddeb66a6ce ("efi_loader: fix building aarch64 EFI binaries"). Heinrich any chance you can repeat your tests? Thanks /Ilias > .rela.dyn : { *(.rela.dyn) } > .rela.plt : { *(.rela.plt) } > .rela.got : { *(.rela.got) } > -- > 2.39.2 >
Re: [RFC PATCH 07/10] arm: efi_loader: discard hash, unwind information
Hi Sam On Sat, 20 May 2023 at 23:56, Sam Edwards wrote: > > LLD tends to put these at the very beginning of the file, only > for the .text 0x0 directive to end up going backward and > overlapping them, creating an error. > > Since they don't appear to be used at runtime, just discard them. > > Signed-off-by: Sam Edwards > --- > > arch/arm/lib/elf_arm_efi.lds | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm/lib/elf_arm_efi.lds b/arch/arm/lib/elf_arm_efi.lds > index 767ebda635..0b6cc5bcb6 100644 > --- a/arch/arm/lib/elf_arm_efi.lds > +++ b/arch/arm/lib/elf_arm_efi.lds > @@ -62,5 +62,8 @@ SECTIONS > *(.dynstr) > *(.note.gnu.build-id) > *(.comment) > + *(.hash) > + *(.gnu.hash) The reason we end up with both hash and gnu.hash is because the hash style is set to 'both'. Should we perhaps use (and strip) only one of them? Thanks /Ilias > + *(.ARM.exidx) > } > } > -- > 2.39.2 >