Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update U-Boot to 2025.01 #3878

Merged
merged 12 commits into from
Feb 18, 2025
Merged

Update U-Boot to 2025.01 #3878

merged 12 commits into from
Feb 18, 2025

Conversation

sairon
Copy link
Member

@sairon sairon commented Feb 14, 2025

Update U-Boot to 2025.01 and refresh all patches. Details in individual commits.

Summary by CodeRabbit

  • New Features

    • Added support for new boards (ODROID-M1S, NabuCasa Green) and expanded boot options on Raspberry Pi, including NVMe boot and a new command to load file content into environment variables.
  • Improvements

    • Enhanced power management and eMMC performance with optimized boot sequences and improved regulator controls.
    • Updated LED indicators and reset controller functions for clearer status display.
    • Upgraded U-Boot to version 2025.01 across platforms for better compatibility and performance.
  • Bug Fixes

    • Resolved issues with Ethernet configuration, PCIe/NVMe address translation, and boot sequence handling for more reliable operation.

Needs minor patch adjustment, also fixed patch numbering.
Move the patch for eMMC so it's applied only for N2 specifically and update it
for 2025.01.
No changes in patches necessary after moving the N2 patch.
Changes needed in bcmstb PCIe driver due to upstream refactoring, rest only
refreshed. All patches now target the same version, so we can drop one of the
series.
No patches here, just version bump.
Updated and refreshed patches, added a patch to disable OF_UPSTREAM which is
now needed.
Drop patch that has been mostly merged upstream. The change is that HS400 would
stay enabled but let's get back to what upstream does.
Drop all patches as M1S support should be now merged to U-Boot and DTS taken
from upstream.
@sairon sairon added os u-boot U-Boot related issue labels Feb 14, 2025
mkeficapsule requires gnutls to be built first but it's not among dependencies.
Since we don't need the tool, we can disable it.

DFU is also not used on HAOS and it implies EFI_LOADER that we already disable.
Moreover, that also sets SET_DFU_ALT_INFO and leads to linker failure on some
platforms where it's not implemented.
There were more changes needed in the Green config to use correct memory layout
due to upstream changes, otherwise we'll have malloc failures in U-Boot proper.
@sairon sairon marked this pull request as ready for review February 17, 2025 16:12
Copy link

coderabbitai bot commented Feb 17, 2025

📝 Walkthrough

Walkthrough

This set of patches makes widespread modifications to U-Boot and its configuration across several platforms. Changes include new power management functions (such as enabling a regulator on the Tinker board), updates to device trees and defconfigs to disable eMMC DDR52 mode in favor of HS200 support on Odroid boards, and extensive additions for the NabuCasa Green board. In addition, the patches improve NVMe driver logic (including address translation and PRP handling), update Raspberry Pi boot order, add reset controller drivers, introduce a new fileenv command, and update numerous buildroot configuration files, including bumping U-Boot versions from 2024.x to 2025.01.

Changes

File(s) / Group Change Summary
board/asus/tinker/patches/uboot/0001-*
.../tinker-rk3288/tinker-rk3288.c
Added new function rockchip_set_regulator_on and modified power_init_board to enable the LDO2 regulator (“vcc33_mipi”) with updated header includes.
board/hardkernel/odroid-m1 and odroid-m1s/patches/uboot/0001-0004-**
.../odroid-m1/, .../odroid-m1s/
Disabled eMMC DDR52 mode and enabled HS200 mode; added board support files for ODROID-M1S; adjusted device tree aliases, SD/MMC interfaces, and Ethernet properties for improved reliability.
board/nabucasa/green/patches/uboot/0001-0011-*
.../nabucasa/green/
Introduced new DTS/U-Boot DTS files, defconfig, Kconfig, source and header files for the NabuCasa Green board. Changes include boot order modifications, SPI flash settings, LED renaming, and memory address corrections.
board/raspberrypi/patches/uboot/0001-0012-* and 2024.01/
.../raspberrypi/
Updated boot order (adding NVMe in rpi.env), revised NVMe driver calculations and address handling (including use of void pointers and virtual-to-bus translation), modified USB xHCI compatibility, added Broadcom reset controllers, improved PCIe support (for BCM2712), and adjusted PL011 UART initialization.
buildroot-external/configs/ (green, khadas_vim3, odroid_c2, odroid_c4, odroid_m1, odroid_m1s, odroid_n2, odroid_xu4, rpi2, rpi3(_64), rpi3, rpi4, tinker, yellow) Updated U-Boot version identifiers (from 2024.01/2024.10 to 2025.01) and modified BR2_GLOBAL_PATCH_DIR paths where applicable.
bootloader/uboot.config, cmd/fileenv.c, board/hardkernel/patches-meson/0001-* Added DFU-related configuration options and a new fileenv command to load string content from files; introduced a hack to limit eMMC clock frequency for certain boards.

Sequence Diagram(s)

sequenceDiagram
    participant BL as Bootloader
    participant PI as power_init_board
    participant RS as rockchip_set_regulator_on
    participant REG as Regulator API

    BL->>PI: Boot process initiation
    PI->>RS: Call rockchip_set_regulator_on("vcc33_mipi", 3300000)
    RS->>REG: Retrieve regulator by name
    REG-->>RS: Regulator handle (or error)
    RS->>REG: Set regulator voltage
    REG-->>RS: Voltage set status
    RS->>REG: Enable regulator
    REG-->>RS: Enable status
    RS-->>PI: Return success/error code
    PI->>BL: Complete board power initialization (return 0 on success)
Loading
sequenceDiagram
    participant CMD as NVMe Command
    participant NV as NVMe Driver
    participant TX as nvme_virt_to_bus

    CMD->>NV: Execute NVMe operation (e.g., nvme_identify)
    NV->>TX: Request bus address conversion for buffer
    TX-->>NV: Return bus address corresponding to virtual address
    NV->>NV: Compute PRP list and prepare command
    NV->>CMD: Issue NVMe command with translated address
Loading

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🔭 Outside diff range comments (4)
buildroot-external/board/raspberrypi/patches/uboot/0012-pci-pcie-brcmstb-HACK-add-DT-property-for-skipping-d.patch (1)

47-54: 🛠️ Refactor suggestion

Document the new device tree property.

The implementation is correct, but please add documentation for the new u-boot,no-probe property:

  1. Add a comment block above this code explaining the property's purpose
  2. Update the relevant device tree binding documentation
  3. Add an example device tree snippet showing usage
buildroot-external/board/nabucasa/green/patches/uboot/0005-board-green-add-Green-board-to-read-board-specific-E.patch (2)

118-166: 🛠️ Refactor suggestion

Improve robustness of EEPROM data handling.

Several improvements could make the code more robust:

  1. Use symbolic constants for header validation
  2. Add bounds checking for array copies
  3. Add error handling for invalid version/serial number
  4. Consider using a struct for the EEPROM layout
-#define CONTENT_SIZE 44
-#define ETHADDR_OFFSET 0X22
-#define ETHADDR_LEN 6
-#define SERIALNUM_OFFSET 0X10
-#define SERIALNUM_LEN 18
-#define VERSION_OFFSET 0X0C
-#define VERSION_LEN 4
+/* EEPROM layout */
+#define EEPROM_MAGIC 0x35363636  /* "3566" */
+struct board_config {
+    u32 magic;           /* offset: 0x00 */
+    u8  reserved1[8];    /* offset: 0x04 */
+    u8  version[4];      /* offset: 0x0C */
+    u8  serial[18];      /* offset: 0x10 */
+    u8  mac[6];          /* offset: 0x22 */
+    u8  reserved2[14];   /* offset: 0x28 */
+} __packed;
+
+static int validate_board_config(const struct board_config *cfg)
+{
+    if (cfg->magic != EEPROM_MAGIC) {
+        printf("Invalid EEPROM magic: 0x%08x\n", cfg->magic);
+        return -EINVAL;
+    }
+    /* Add more validation as needed */
+    return 0;
+}

 int misc_init_r(void)
 {
-    unsigned char data[CONTENT_SIZE];
-    unsigned char ethaddr[ETHADDR_LEN];
-    unsigned char serialnum[SERIALNUM_LEN + 1];
-    unsigned char version[VERSION_LEN];
+    struct board_config config;
+    char serial_str[sizeof(config.serial) + 1];
     struct udevice *dev;
     int ret;

     ret = uclass_first_device_err(UCLASS_I2C_EEPROM, &dev);
     if (ret) {
         printf("UCLASS_I2C_EEPROM get error\n");
         return 0;
     }

-    ret = i2c_eeprom_read(dev, 0, data, sizeof(data));
+    ret = i2c_eeprom_read(dev, 0, &config, sizeof(config));
     if (ret) {
         printf("UCLASS_I2C_EEPROM read error %d\n", ret);
         return 0;
     }

-    if (!(data[0] == 3 && data[1] == 5 && data[2] == 6 && data[3] == 6))
-    {
-        printf("EEPROM header not ok\n");
+    ret = validate_board_config(&config);
+    if (ret) {
         return 0;
     }

-    memcpy(version , data + VERSION_OFFSET, VERSION_LEN);
-    printf("Board revision: %c%c\n", version[2], version[3]);
+    printf("Board revision: %c%c\n", config.version[2], config.version[3]);

-    memcpy(serialnum, data + SERIALNUM_OFFSET, SERIALNUM_LEN);
-    serialnum[SERIALNUM_LEN] = 0;
-    env_set("serial#", serialnum);
+    memcpy(serial_str, config.serial, sizeof(config.serial));
+    serial_str[sizeof(config.serial)] = '\0';
+    env_set("serial#", serial_str);

-    memcpy(ethaddr, data + ETHADDR_OFFSET, ETHADDR_LEN);
-    if (is_valid_ethaddr(ethaddr))
-        eth_env_set_enetaddr("ethaddr", ethaddr);
+    if (is_valid_ethaddr(config.mac))
+        eth_env_set_enetaddr("ethaddr", config.mac);

     return 0;
 }

183-184: ⚠️ Potential issue

Fix I2C_EEPROM_ADDR configuration.

The I2C_EEPROM_ADDR is set to 0x0 but the device tree specifies 0x50.

 CONFIG_I2C_EEPROM=y
-CONFIG_SYS_I2C_EEPROM_ADDR=0x0
+CONFIG_SYS_I2C_EEPROM_ADDR=0x50
buildroot-external/patches/uboot/0001-CMD-read-string-from-fileinto-env.patch (1)

55-85: ⚠️ Potential issue

Add safety checks to prevent buffer overflows and improve input validation.

The current implementation has several security concerns:

  1. No validation of the memory address range
  2. No size limits on file reading
  3. Unsafe string processing without bounds checking

Consider these improvements:

 int do_fileenv(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
 {
+	const size_t MAX_FILE_SIZE = 4096;  // Reasonable limit for env strings
+	ulong addr;
+	char *buf;
+	size_t size;
+
 	if (argc < 6)
 		return CMD_RET_USAGE;
 
+	addr = simple_strtoul(argv[3], NULL, 16);
+	if (!addr) {
+		printf("Error: Invalid address\n");
+		return CMD_RET_FAILURE;
+	}
+
 	fs_argv[0] = "fatload";
 	fs_argv[1] = argv[1];
 	fs_argv[2] = argv[2];
 	fs_argv[3] = argv[3];
 	fs_argv[4] = argv[4];
 
 	if (do_fat_fsload(cmdtp, 0, 5, fs_argv) != 0)
 		return 1;
 
-	char *addr = (char *)simple_strtoul(argv[3], NULL, 16);
-	size_t size = env_get_hex("filesize", 0);
+	size = env_get_hex("filesize", 0);
+	if (size == 0 || size > MAX_FILE_SIZE) {
+		printf("Error: File size %zu exceeds maximum allowed size %zu\n",
+		       size, MAX_FILE_SIZE);
+		return CMD_RET_FAILURE;
+	}
+
+	buf = (char *)addr;
+	if (!buf) {
+		printf("Error: Invalid buffer\n");
+		return CMD_RET_FAILURE;
+	}
 
 	// Prepare string
-	addr[size] = 0x00;
-	char *s = addr;
+	buf[size] = 0x00;
+	char *s = buf;
+	size_t processed = 0;
 	while(*s != 0x00) {
+		if (processed++ >= size) {
+			printf("Error: String processing exceeded file size\n");
+			return CMD_RET_FAILURE;
+		}
 		if (isprint(*s)) {
 			s++;
 		}
@@ -81,7 +81,7 @@ int do_fileenv(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
 		}
 	}
 
-	return env_set(argv[5], addr);
+	return env_set(argv[5], buf);
 }
🧹 Nitpick comments (21)
buildroot-external/board/raspberrypi/patches/uboot/0004-nvme-improve-readability-of-nvme_setup_prps.patch (1)

6-7: Fix typo in commit message.

There's a typo in the commit message: "discriptive" should be "descriptive".

-appropriate and adding variables with discriptive name.
+appropriate and adding variables with descriptive name.
buildroot-external/board/raspberrypi/patches/uboot/0012-pci-pcie-brcmstb-HACK-add-DT-property-for-skipping-d.patch (2)

1-18: Verify the timeline for replacing this temporary hack.

While the commit message clearly explains the rationale for this workaround, it's marked as a HACK and needs proper resolution when RP1 control from U-Boot becomes necessary. Consider:

  1. Adding a TODO comment in the code to track this technical debt
  2. Creating a follow-up issue to implement a proper fix
  3. Documenting potential side effects in case someone needs to control RP1 from U-Boot

35-46: Enhance debug information for probe skipping.

While the basic functionality is correct, consider enhancing the debug message to include:

  1. Device information for easier troubleshooting
  2. Reason for skipping (device tree property value)
 if (pcie->no_probe) {
-    printf("PCIe BRCM: skipping probe\n");
+    printf("PCIe BRCM: skipping probe for %s (u-boot,no-probe=true)\n",
+           dev->name);
     return 0;
 }
buildroot-external/board/hardkernel/odroid-n2/patches/uboot/0001-HACK-mmc-meson-gx-limit-f_max-to-24-MHz-on-the-first.patch (2)

31-39: Consider making the frequency hack configurable.

Setting meson_gx_f_max_hack = true by default means all cards will first try with limited frequency, which could impact performance for cards known to work at higher speeds. Consider:

  1. Making this configurable through device tree or Kconfig
  2. Adding a board-specific whitelist/blacklist of known problematic eMMC vendors

55-68: Enhance error reporting and handling.

The error handling logic is good, but could be improved:

-			printf("Card failed to initialize at %d Hz, disabling meson_gx hack.\n",
-			       mmc->clock);
+			printf("eMMC initialization failed at %d Hz (meson_gx hack enabled). Retrying at full speed...\n",
+			       mmc->clock);
+			if (IS_ENABLED(CONFIG_MMC_DEBUG))
+				printf("Card caps: 0x%x\n", mmc->card_caps);
buildroot-external/board/raspberrypi/patches/uboot/0008-reset-reset-brcmstb-Add-Broadcom-STB-reset-controlle.patch (1)

127-136: Consider adding error logging in probe function.

The probe function could benefit from more descriptive error logging to aid in debugging.

 static int brcmstb_reset_probe(struct udevice *dev)
 {
 	struct brcmstb_reset_priv *priv = dev_get_priv(dev);
 
 	priv->base = dev_remap_addr(dev);
-	if (!priv->base)
+	if (!priv->base) {
+		dev_err(dev, "failed to map device memory\n");
 		return -EINVAL;
+	}
 
 	return 0;
 }
buildroot-external/board/raspberrypi/patches/uboot/0009-reset-reset-brcmstb-rescal-Add-Broadcom-RESCAL-reset.patch (2)

90-115: Consider using dev_err for error messages.

The error messages should use dev_err instead of printf for consistency with U-Boot's error logging system.

 	reg = readl(priv->base + BRCM_RESCAL_START);
 	writel(reg | BRCM_RESCAL_START_BIT, priv->base + BRCM_RESCAL_START);
 	reg = readl(priv->base + BRCM_RESCAL_START);
 	if (!(reg & BRCM_RESCAL_START_BIT)) {
-		printf("failed to start SATA/PCIe rescal\n");
+		dev_err(rst->dev, "failed to start SATA/PCIe rescal\n");
 		return -EIO;
 	}
 
 	ret = read_poll_timeout(readl, reg, (reg & BRCM_RESCAL_STATUS_BIT),
 				100, 1000, priv->base + BRCM_RESCAL_STATUS);
 	if (ret) {
-		printf("time out on SATA/PCIe rescal\n");
+		dev_err(rst->dev, "time out on SATA/PCIe rescal\n");
 		return ret;
 	}

140-149: Consider adding error logging in probe function.

The probe function could benefit from more descriptive error logging to aid in debugging.

 static int brcm_rescal_reset_probe(struct udevice *dev)
 {
 	struct brcm_rescal_reset_priv *priv = dev_get_priv(dev);
 
 	priv->base = dev_remap_addr(dev);
-	if (!priv->base)
+	if (!priv->base) {
+		dev_err(dev, "failed to map device memory\n");
 		return -EINVAL;
+	}
 
 	return 0;
 }
buildroot-external/board/nabucasa/green/patches/uboot/0005-board-green-add-Green-board-to-read-board-specific-E.patch (2)

1-9: Consider enhancing the commit message with implementation details.

While the commit message explains the purpose, it would be helpful to include:

  • The EEPROM model (24c32)
  • The data structure being read (version, serial number, MAC address)
  • The I2C address (0x50)

33-37: Consider adding more descriptive properties to the EEPROM node.

While the basic properties are present, consider adding:

  • label property for better identification
  • size property to document the total EEPROM size
  • Documentation about the data layout in a comment
 eeprom: eeprom@50 {
 	compatible = "atmel,24c32";
 	reg = <0x50>;
 	pagesize = <32>;
+	label = "board-config";
+	size = <4096>;
+	/* Data layout:
+	 * 0x00-0x03: Header (3566)
+	 * 0x0C-0x0F: Version
+	 * 0x10-0x21: Serial Number
+	 * 0x22-0x27: MAC Address
+	 */
 };
buildroot-external/board/asus/tinker/patches/uboot/0001-pmic-enable-LDO2-vcc33_mipi-at-bootup.patch (1)

29-51: Enhance error messages in the regulator helper function.

The error handling is good, but the debug messages could be more descriptive to aid in troubleshooting.

Consider this improvement:

-		debug("%s: Cannot find regulator %s\n", __func__, name);
+		debug("%s: Failed to get regulator '%s' (ret=%d)\n", __func__, name, ret);
-		debug("%s: Cannot set regulator %s\n", __func__, name);
+		debug("%s: Failed to set regulator '%s' to %uuV (ret=%d)\n", __func__, name, uv, ret);
-		debug("%s: Cannot enable regulator %s\n", __func__, name);
+		debug("%s: Failed to enable regulator '%s' (ret=%d)\n", __func__, name, ret);
buildroot-external/patches/uboot/0001-CMD-read-string-from-fileinto-env.patch (1)

87-92: Improve command help text with size limits and security notes.

The help text should include information about size limits and usage constraints.

 U_BOOT_CMD(
 	fileenv, 6, 0, do_fileenv,
 	"Read file and store it into env.",
-	"<interface> <dev:part> <addr> <filename> <envname>\n"
-	"    - Read file from fat32 and store it as env."
+	"<interface> <dev:part> <addr> <filename> <envname>\n"
+	"    - Read file (max 4KB) from fat32 and store it as env.\n"
+	"    - Only printable characters are preserved, others are replaced with null."
 );
buildroot-external/board/raspberrypi/patches/uboot/0006-nvme-translate-virtual-addresses-into-the-bus-s-addr.patch (1)

45-59: Convert pointer arithmetic to bus-address translation.

These updates (e.g., using nvme_virt_to_bus(dev, prp_pool + page_size) and nvme_virt_to_bus(dev, buffer)) similarly fix potential address offset issues by leveraging the newly introduced translation function. Make sure to test thoroughly on platforms that have different bus-memory mapping offsets.

buildroot-external/board/raspberrypi/patches/uboot/0010-pci-pcie-brcmstb-Add-basic-support-for-BCM2712-PCIe.patch (4)

33-64: Add documentation references for newly introduced macros.
The newly defined register addresses and bitmasks look appropriate for BCM2712 PCIe support. However, it might be helpful to include a reference to the official hardware documentation or an inline comment for these magic constants to clarify their purpose.


155-174: Provide context on “magic” MDIO registers.
The PLL initialization routine includes looping over arrays of magic register addresses and values. Consider adding descriptive comments or references to official documentation for future maintainers.


183-227: Reset handling logic looks good but consider unified approach.
Introducing separate functions for generic and BCM2712-specific reset logic is clear. However, you could consider a more unified structure if more SoC variants appear later. This avoids scattered conditionals in the probe paths.


229-326: Check usage of magic constant 0x12 for L1SS errata.
While applying the L1SS fix, you set tmp |= 0x12. For better clarity, consider defining a named macro (e.g. PCIE_L1SS_PM_CLK_PERIOD_18NS) to indicate what the value represents.

-       tmp |= 0x12;
+       #define PCIE_L1SS_PM_CLK_PERIOD_18NS 0x12
+       tmp |= PCIE_L1SS_PM_CLK_PERIOD_18NS;
buildroot-external/board/nabucasa/green/patches/uboot/0004-arch-arm-dts-green-spi-nor-uboot.patch (1)

141-150: Select only necessary SPI Flash drivers.
Enabling CONFIG_SPI_FLASH_GIGADEVICE and CONFIG_SPI_FLASH_WINBOND is sensible if both hardware types are in use. If not, disabling unused drivers can streamline the build.

buildroot-external/board/nabucasa/green/patches/uboot/0001-arch-arm-dts-Add-NabuCasa-Green-board.patch (3)

628-631: Document the SPI flash mode limitation.

The comment indicates that Dual I/O mode is used due to pin conflicts with eMMC, but this limitation should be documented more prominently.

Add a detailed comment explaining the limitation:

-	/* Dual I/O mode as the D2 pin conflicts with the eMMC */
+	/*
+	 * Using Dual I/O mode instead of Quad I/O mode because the D2 pin
+	 * conflicts with the eMMC. This reduces the maximum theoretical
+	 * throughput but ensures compatibility with both peripherals.
+	 */

240-241: Consider making ethernet delay values configurable.

The TX and RX delay values are hardcoded. Consider making these configurable through environment variables for better flexibility during board bring-up and testing.

+	/* Configurable delay values for fine-tuning */
+	tx_delay = <CONFIG_GMAC1_TX_DELAY>;
+	rx_delay = <CONFIG_GMAC1_RX_DELAY>;
-	tx_delay = <0x47>;
-	rx_delay = <0x27>;

760-834: Consider enabling additional security features.

The current configuration could benefit from enabling security features like verified boot and secure boot if supported by the hardware.

Consider adding these configurations:

+CONFIG_CHAIN_OF_TRUST=y
+CONFIG_FIT_SIGNATURE=y
+CONFIG_RSA=y
+CONFIG_RSA_VERIFY=y
+CONFIG_FIT_ROLLBACK_PROTECT=y
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a666bb and 6044f92.

📒 Files selected for processing (57)
  • buildroot-external/board/asus/tinker/patches/uboot/0001-pmic-enable-LDO2-vcc33_mipi-at-bootup.patch (1 hunks)
  • buildroot-external/board/hardkernel/odroid-m1/patches/uboot/0001-odroid-m1-Disable-eMMC-DDR52-mode-enable-HS200.patch (0 hunks)
  • buildroot-external/board/hardkernel/odroid-m1s/patches/uboot/0001-board-rockchip-Add-Hardkernel-ODROID-M1S.patch (0 hunks)
  • buildroot-external/board/hardkernel/odroid-m1s/patches/uboot/0002-Apply-fixes-for-mainline-u-boot.patch (0 hunks)
  • buildroot-external/board/hardkernel/odroid-m1s/patches/uboot/0003-Improve-reliability-of-eMMC.patch (0 hunks)
  • buildroot-external/board/hardkernel/odroid-m1s/patches/uboot/0004-Fix-ethernet-properties.patch (0 hunks)
  • buildroot-external/board/hardkernel/odroid-n2/patches/uboot/0001-HACK-mmc-meson-gx-limit-f_max-to-24-MHz-on-the-first.patch (5 hunks)
  • buildroot-external/board/nabucasa/green/patches/uboot/0001-arch-arm-dts-Add-NabuCasa-Green-board.patch (3 hunks)
  • buildroot-external/board/nabucasa/green/patches/uboot/0002-configs-green-Support-USB-boot.patch (2 hunks)
  • buildroot-external/board/nabucasa/green/patches/uboot/0003-arch-arm-dts-rk3566-green-boot-order.patch (3 hunks)
  • buildroot-external/board/nabucasa/green/patches/uboot/0004-arch-arm-dts-green-spi-nor-uboot.patch (4 hunks)
  • buildroot-external/board/nabucasa/green/patches/uboot/0005-board-green-add-Green-board-to-read-board-specific-E.patch (8 hunks)
  • buildroot-external/board/nabucasa/green/patches/uboot/0006-green-spl-loader-uboot-order-sd-emmc-spi_nor.patch (2 hunks)
  • buildroot-external/board/nabucasa/green/patches/uboot/0007-green-Fix-SPI-Flash-alias.patch (2 hunks)
  • buildroot-external/board/nabucasa/green/patches/uboot/0008-arch-arm-dts-green-drop-SPI-NOR-flash-partitions.patch (2 hunks)
  • buildroot-external/board/nabucasa/green/patches/uboot/0009-green-Do-not-use-eMMC-DDR52-mode-enable-HS200.patch (3 hunks)
  • buildroot-external/board/nabucasa/green/patches/uboot/0010-arch-arm64-dts-green-Improve-LED-representation.patch (2 hunks)
  • buildroot-external/board/nabucasa/green/patches/uboot/0011-configs-green-fix-bss-and-stack-address-and-disable-.patch (1 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/0001-rpi-add-NVMe-to-boot-order.patch (2 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/0002-Revert-nvme-Correct-the-prps-per-page-calculation-me.patch (2 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/0003-usb-xhci-brcm-Make-driver-compatible-with-downstream.patch (2 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/0004-nvme-improve-readability-of-nvme_setup_prps.patch (2 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/0005-nvme-Use-pointer-for-CPU-addressed-buffers.patch (4 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/0006-nvme-translate-virtual-addresses-into-the-bus-s-addr.patch (3 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/0007-Revert-pci-Check-region-ranges-are-addressable.patch (2 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/0008-reset-reset-brcmstb-Add-Broadcom-STB-reset-controlle.patch (4 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/0009-reset-reset-brcmstb-rescal-Add-Broadcom-RESCAL-reset.patch (4 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/0010-pci-pcie-brcmstb-Add-basic-support-for-BCM2712-PCIe.patch (14 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/0011-ARM-bcm2835-add-BCM2712-config-option.patch (3 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/0012-pci-pcie-brcmstb-HACK-add-DT-property-for-skipping-d.patch (3 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0001-rpi-Use-CONFIG_OF_BOARD-instead-of-CONFIG_EMBED.patch (0 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0002-rpi-add-NVMe-to-boot-order.patch (0 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0003-Revert-nvme-Correct-the-prps-per-page-calculation-me.patch (0 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0004-usb-xhci-brcm-Make-driver-compatible-with-downstream.patch (0 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0005-nvme-improve-readability-of-nvme_setup_prps.patch (0 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0006-nvme-Use-pointer-for-CPU-addressed-buffers.patch (0 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0007-nvme-translate-virtual-addresses-into-the-bus-s-addr.patch (0 hunks)
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0008-Revert-pci-Check-region-ranges-are-addressable.patch (0 hunks)
  • buildroot-external/board/raspberrypi/yellow/patches/uboot/0001-drivers-bcm283x-don-t-explicitly-disable-init.patch (3 hunks)
  • buildroot-external/board/raspberrypi/yellow/patches/uboot/0002-drivers-bcm283x-allow-to-spawn-any-PL011-UART.patch (2 hunks)
  • buildroot-external/bootloader/uboot.config (1 hunks)
  • buildroot-external/configs/green_defconfig (1 hunks)
  • buildroot-external/configs/khadas_vim3_defconfig (1 hunks)
  • buildroot-external/configs/odroid_c2_defconfig (2 hunks)
  • buildroot-external/configs/odroid_c4_defconfig (2 hunks)
  • buildroot-external/configs/odroid_m1_defconfig (2 hunks)
  • buildroot-external/configs/odroid_m1s_defconfig (1 hunks)
  • buildroot-external/configs/odroid_n2_defconfig (2 hunks)
  • buildroot-external/configs/odroid_xu4_defconfig (2 hunks)
  • buildroot-external/configs/rpi2_defconfig (1 hunks)
  • buildroot-external/configs/rpi3_64_defconfig (1 hunks)
  • buildroot-external/configs/rpi3_defconfig (1 hunks)
  • buildroot-external/configs/rpi4_64_defconfig (1 hunks)
  • buildroot-external/configs/rpi4_defconfig (1 hunks)
  • buildroot-external/configs/tinker_defconfig (1 hunks)
  • buildroot-external/configs/yellow_defconfig (1 hunks)
  • buildroot-external/patches/uboot/0001-CMD-read-string-from-fileinto-env.patch (3 hunks)
💤 Files with no reviewable changes (13)
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0008-Revert-pci-Check-region-ranges-are-addressable.patch
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0004-usb-xhci-brcm-Make-driver-compatible-with-downstream.patch
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0002-rpi-add-NVMe-to-boot-order.patch
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0005-nvme-improve-readability-of-nvme_setup_prps.patch
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0007-nvme-translate-virtual-addresses-into-the-bus-s-addr.patch
  • buildroot-external/board/hardkernel/odroid-m1/patches/uboot/0001-odroid-m1-Disable-eMMC-DDR52-mode-enable-HS200.patch
  • buildroot-external/board/hardkernel/odroid-m1s/patches/uboot/0002-Apply-fixes-for-mainline-u-boot.patch
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0001-rpi-Use-CONFIG_OF_BOARD-instead-of-CONFIG_EMBED.patch
  • buildroot-external/board/hardkernel/odroid-m1s/patches/uboot/0001-board-rockchip-Add-Hardkernel-ODROID-M1S.patch
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0003-Revert-nvme-Correct-the-prps-per-page-calculation-me.patch
  • buildroot-external/board/raspberrypi/patches/uboot/2024.01/0006-nvme-Use-pointer-for-CPU-addressed-buffers.patch
  • buildroot-external/board/hardkernel/odroid-m1s/patches/uboot/0004-Fix-ethernet-properties.patch
  • buildroot-external/board/hardkernel/odroid-m1s/patches/uboot/0003-Improve-reliability-of-eMMC.patch
✅ Files skipped from review due to trivial changes (2)
  • buildroot-external/configs/rpi4_defconfig
  • buildroot-external/configs/rpi3_64_defconfig
🔇 Additional comments (92)
buildroot-external/board/raspberrypi/patches/uboot/0004-nvme-improve-readability-of-nvme_setup_prps.patch (2)

22-24: LGTM! Good use of const qualifiers.

The changes improve code clarity by:

  1. Making immutable values explicit through const declarations
  2. Moving the prps_per_page calculation closer to related declarations
  3. Preventing accidental modifications to these values

37-40: LGTM! Improved readability of pointer manipulation.

The introduction of the next_prp_list variable with a descriptive name makes the code more maintainable by breaking down the complex expression into clearer steps.

buildroot-external/board/raspberrypi/patches/uboot/0012-pci-pcie-brcmstb-HACK-add-DT-property-for-skipping-d.patch (1)

27-34: LGTM! Clean struct modification.

The new no_probe boolean field is well-placed alongside other boolean configuration flags in the structure.

buildroot-external/board/hardkernel/odroid-n2/patches/uboot/0001-HACK-mmc-meson-gx-limit-f_max-to-24-MHz-on-the-first.patch (3)

1-20: Consider documenting timeline for a proper fix.

While the commit message clearly explains the rationale, the "HACK" prefix suggests this is a temporary workaround. Consider documenting:

  1. Whether this is intended as a temporary or permanent solution
  2. If temporary, what would be the proper long-term fix
  3. Any plans or timeline for implementing a more robust solution

44-54: LGTM! Clear implementation of the frequency cap.

The implementation is straightforward and well-commented.


73-82: LGTM! Clean addition of the control flag.

The new flag is well-placed and uses appropriate type.

buildroot-external/board/raspberrypi/patches/uboot/0008-reset-reset-brcmstb-Add-Broadcom-STB-reset-controlle.patch (3)

30-34: LGTM! Clear and informative Kconfig entry.

The configuration option is well-documented with a descriptive help text that clearly explains its purpose.


47-47: LGTM! Clean Makefile addition.

The build configuration follows the established pattern in the Makefile.


92-115: LGTM! Clean implementation of assert/deassert functions.

The functions correctly handle bank offsets and include appropriate delays for stability after deassert operations.

buildroot-external/board/raspberrypi/patches/uboot/0009-reset-reset-brcmstb-rescal-Add-Broadcom-RESCAL-reset.patch (2)

31-35: LGTM! Clear and informative Kconfig entry.

The configuration option is well-documented with a descriptive help text that clearly identifies the target SoCs.


48-48: LGTM! Clean Makefile addition.

The build configuration follows the established pattern in the Makefile.

buildroot-external/board/nabucasa/green/patches/uboot/0005-board-green-add-Green-board-to-read-board-specific-E.patch (1)

50-54: LGTM! Kconfig changes follow U-Boot conventions.

The configuration options are well-structured and properly documented.

Also applies to: 73-87

buildroot-external/board/raspberrypi/yellow/patches/uboot/0001-drivers-bcm283x-don-t-explicitly-disable-init.patch (2)

24-35: LGTM! Removal of forced initialization skip.

The removal of the forced initialization skip is a good change, as it allows proper initialization of PL011 UARTs when needed. The comment indicates this was a temporary workaround, and with the clock configuration in place, reinitialization should now work consistently.


41-47: LGTM! Addition of PL011 clock configuration.

Adding the explicit clock configuration (48MHz) for PL011 UARTs is a good improvement that addresses the root cause of initialization issues.

buildroot-external/board/asus/tinker/patches/uboot/0001-pmic-enable-LDO2-vcc33_mipi-at-bootup.patch (1)

53-60: LGTM! Clear power initialization sequence.

The power initialization is straightforward and properly handles errors from the regulator helper function.

buildroot-external/board/raspberrypi/yellow/patches/uboot/0002-drivers-bcm283x-allow-to-spawn-any-PL011-UART.patch (1)

25-63: LGTM! Removal of unnecessary pinmux checking.

The removal of the pinmux checking is a good improvement because:

  1. U-Boot already handles pinmuxing
  2. It enables the use of additional PL011 UARTs on Raspberry Pi 4
  3. It simplifies the probing logic
buildroot-external/bootloader/uboot.config (1)

21-23: DFU Configuration Options Added

The new lines introduce DFU-related options that are explicitly disabled by default. This inclusion makes the DFU features available for future use (especially in light of recent changes to power management and boot configurations across various boards) without altering the current runtime behavior. Ensure that these settings align with the intended usage across all supported platforms and that no conflicts arise with other board-specific patches.

buildroot-external/board/raspberrypi/patches/uboot/0001-rpi-add-NVMe-to-boot-order.patch (1)

22-23: LGTM! The boot order is well-structured.

The addition of NVMe to the boot targets is logical, maintaining MMC as the primary boot device while adding NVMe before USB and network boot options. This enhancement properly supports the NVMe capabilities of the Compute Module 4 I/O Board.

buildroot-external/board/raspberrypi/patches/uboot/0003-usb-xhci-brcm-Make-driver-compatible-with-downstream.patch (1)

23-24: LGTM! The compatibility string update aligns with downstream kernel.

The change from "brcm,generic-xhci" to "generic-xhci" ensures compatibility with the downstream device tree, which is essential for proper USB driver functionality.

buildroot-external/board/raspberrypi/patches/uboot/0002-Revert-nvme-Correct-the-prps-per-page-calculation-me.patch (1)

33-34: LGTM! The PRP calculation aligns with NVMe specification.

The revert correctly adjusts the PRPs per page calculation to reserve the last entry for the PRP List pointer when handling multi-page transfers, as required by the NVMe specification.

buildroot-external/board/raspberrypi/patches/uboot/0007-Revert-pci-Check-region-ranges-are-addressable.patch (1)

25-41:

❓ Verification inconclusive

LGTM! The revert fixes USB device detection.

The consolidation of debug messages and removal of range checks resolves boot hangs with USB via XHCI on Raspberry Pi 4 32-bit. However, please verify that this doesn't impact other PCI devices.

Please verify PCI device functionality on other boards after this revert:


🏁 Script executed:

#!/bin/bash
# Search for PCI device tests and configurations
rg -A 5 "pci|PCI" configs/ buildroot-external/board/

Length of output: 85041


Action Required: Verify PCI Functionality on Other Boards

LGTM! The revert resolves the observed USB device detection issue on Raspberry Pi 4 32-bit by consolidating debug messages and removing the redundant range checks in the PCI region handling (see changes in buildroot-external/board/raspberrypi/patches/uboot/0007-Revert-pci-Check-region-ranges-are-addressable.patch). However, because our initial search for PCI-related tests and configuration files produced mixed output without a clear picture of how other boards utilize PCI, please perform manual verification on non-Raspberry Pi boards (such as those under arm-uefi, khadas, or hardkernel) to ensure that the revert does not negatively impact other PCI devices.

  • File to check: buildroot-external/board/raspberrypi/patches/uboot/0007-Revert-pci-Check-region-ranges-are-addressable.patch (affecting drivers/pci/pci-uclass.c)
  • Action: Manually verify or run relevant PCI tests on other boards to confirm proper functionality.
buildroot-external/board/nabucasa/green/patches/uboot/0007-green-Fix-SPI-Flash-alias.patch (3)

1-13: LGTM! Well-structured commit message.

The commit message clearly explains the purpose of the changes and their impact.


22-22: LGTM! Essential SPL configuration added.

Adding CONFIG_SPL_DM_SEQ_ALIAS=y is necessary for proper device enumeration in SPL, ensuring consistent device aliases between SPL and regular U-Boot.


30-37:

❓ Verification inconclusive

LGTM! Comprehensive SPI flash support improvements.

The changes enhance SPI flash support through:

  1. Setting bus 4 as default, enabling regular U-Boot access
  2. Adding SFDP support for better flash chip compatibility
  3. Maintaining organized vendor support

Let's verify if bus 4 is the correct SPI bus for this board:


🏁 Script executed:

#!/bin/bash
# Search for SPI bus definitions in device tree files
rg -A 5 "spi[0-9]" buildroot-external/board/nabucasa/green/

Length of output: 61


Approval Pending Manual Verification on SPI Bus Assignment

The patch clearly enhances SPI flash support by:

  • Setting the default SPI bus to 4 for U‑Boot access,
  • Adding SFDP support for improved flash chip compatibility, and
  • Reordering vendor support configurations for clarity.

However, the automated search for SPI bus definitions via the provided command returned no output. Please manually verify that bus 4 is the correct default (e.g., by checking the board’s device tree or configuration files) to ensure this change aligns with the board’s hardware specification.

buildroot-external/board/raspberrypi/patches/uboot/0011-ARM-bcm2835-add-BCM2712-config-option.patch (4)

1-13: Well-structured commit message!

The commit message clearly explains the purpose of adding BCM2712 support and its dependencies. Good job following the patch submission guidelines.


27-31: Clean implementation of BCM2712 config!

The configuration option is well-implemented with appropriate dependencies and constraints, correctly enforcing 64-bit mode for BCM2712.


43-44: Good addition of BCM2712-specific reset controller dependencies!

The conditional selection of reset controllers is well-implemented, ensuring they're only required when BCM2712 is enabled.


1-48:

❓ Verification inconclusive

Verify the existence of implementation code for BCM2712.

While the Kconfig changes are well-structured, we should verify that the corresponding implementation code exists for BCM2712 support.


🏁 Script executed:

#!/bin/bash
# Search for BCM2712-related implementation code
echo "Searching for BCM2712 implementation files..."
rg -l "BCM2712|bcm2712" --type c --type cpp --type h

echo -e "\nSearching for BRCMSTB reset controller implementation..."
rg -l "RESET_BRCMSTB" --type c --type cpp --type h

Length of output: 633


I've produced a new verification script that broadens the search across all files (i.e. without restricting file types). This should help us determine whether any BCM2712 or RESET_BRCMSTB implementation references appear in the codebase.

Please run the following script:

Once you have the output from this script, we can decide if there is a need for further action or if manual verification is required.

If the new search finds the expected implementation references, we can update our review comment accordingly. If not, manual verification might be necessary to confirm that the patch does not leave behind any missing implementation details.

Please provide the output of this script so we can proceed further.


🏁 Script executed:

[run_scripts]
#!/bin/bash
echo "Searching for any reference to 'BCM2712' in the repository..."
rg -i -l "BCM2712" .

echo -e "\nSearching for any reference to 'RESET_BRCMSTB' in the repository..."
rg -i -l "RESET_BRCMSTB" .

Length of output: 2227


Double-check BCM2712 Implementation Code

While the Kconfig changes are well structured, our repository search only revealed references to BCM2712 and RESET_BRCMSTB in configuration and patch files (e.g., defconfigs and patch files) and did not uncover any dedicated C-source implementation. Please manually verify if the actual implementation code for BCM2712 support (and the associated reset controllers) exists elsewhere in the repository or is provided in separate patches. If the implementation is missing, additional code may be required to fully support this feature.

buildroot-external/board/nabucasa/green/patches/uboot/0011-configs-green-fix-bss-and-stack-address-and-disable-.patch (4)

1-17: Well-structured commit message with clear documentation!

The commit message effectively communicates the purpose of the changes and includes a reference to the upstream changes that motivated this patch.


69-76: LGTM! Appropriate handling of device tree configuration.

Disabling CONFIG_OF_UPSTREAM is the correct approach since there's no upstream device tree for the Green board. The remaining DT configurations are properly maintained.


26-46:

❓ Verification inconclusive

Verify memory layout compatibility with the Green board.

The removal of custom memory addresses (TEXT_BASE, stack, BSS) in favor of upstream defaults is a good practice. However, please ensure these changes:

  1. Don't affect the board's boot process
  2. Maintain compatibility with the existing ATF (ARM Trusted Firmware) configuration
  3. Work with the current memory map of the Green board

🏁 Script executed:

#!/bin/bash
# Description: Check for any memory-related configurations in other board files
# that might depend on the removed addresses

# Search for any references to the removed memory addresses
echo "Searching for references to removed memory addresses..."
rg -A 3 "0x00a00000|0xc00000|0x600000|0x400000|0x4000000" \
  --type-add 'dts:*.dts' \
  --type-add 'dtsi:*.dtsi' \
  --type dts \
  --type dtsi

# Check for memory layout in device tree
echo "Checking memory layout in device tree..."
rg -A 10 "memory@|reserved-memory@" \
  --type-add 'dts:*.dts' \
  --type-add 'dtsi:*.dtsi' \
  --type dts \
  --type dtsi

Length of output: 749


Verify memory layout compatibility for the Green board

The removal of custom memory addresses (e.g., TEXT_BASE, custom stack and BSS settings) to adopt upstream defaults looks correct, but please manually ensure the following:

  • The board’s boot process remains unaffected and functions correctly with the new defaults.
  • The ARM Trusted Firmware (ATF) and related initialization components are fully compatible.
  • There are no leftover references in device tree files (or elsewhere) expecting the old memory addresses (e.g., 0x00a00000, 0xc00000, 0x600000, 0x400000).

To aid in this verification, please run the following script to list and search through any device tree files for these addresses:

#!/bin/bash
echo "Listing device tree files (.dts and .dtsi):"
fd -e dts -e dtsi .
echo "Searching for removed memory address references within these files..."
fd -e dts -e dtsi . -x grep -H -n "0x00a00000\|0xc00000\|0x600000\|0x400000\|0x4000000"

If the script produces no output, double-check manually (or run with increased verbosity) to confirm that no configurations expect the removed addresses.


77-93:

❓ Verification inconclusive

Verify the impact of removing MMC HS200 support.

The removal of CONFIG_MMC_HS200_SUPPORT and CONFIG_SPL_MMC_HS200_SUPPORT might affect eMMC performance. Please confirm:

  1. The reason for removing HS200 support
  2. If this aligns with the hardware capabilities of the Green board
  3. The impact on eMMC performance and stability

🌐 Web query:

What are the performance implications of disabling HS200 mode in eMMC?

💡 Result:

Disabling HS200 mode in eMMC impacts performance by reducing maximum transfer speeds and altering system design trade-offs. Here's a breakdown:

Speed Reduction

  • HS200 provides up to 200 MB/s (theoretical) using 8-bit SDR at 1.8V signaling with a 200 MHz clock[4][7][8].
  • Without HS200, systems typically fall back to:
    • High-Speed DDR (DDR52): Up to 104 MB/s with 3.3V signaling and 52 MHz clock[1][4].
    • High-Speed SDR: Limited to 52 MB/s[4].

Key Implications

  1. Bandwidth Loss:

    • Sequential read/write operations lose ~48% throughput when downgraded from HS200 to DDR52[1][7].
    • Example: File transfers that take 10 seconds in HS200 would require ~19 seconds in DDR52.
  2. Power/Noise Trade-offs:

    • DDR52 operates at 3.3V instead of 1.8V, increasing power consumption but simplifying PCB design[1][3].
    • STM32MP15 platforms require hardware modifications (STPMIC1B regulator) for stable 1.8V operation in HS200[1][3].
  3. Mode Compatibility:

    • Disabling HS200 blocks access to HS400 (400 MB/s DDR mode), as HS400 builds on HS200 infrastructure[4][9].
    • Boot partition access may be restricted to legacy speeds (HS52 or lower)[6].
Metric HS200 Mode Fallback Mode (DDR52)
Transfer Rate 200 MB/s 104 MB/s
Voltage 1.8V 3.3V
Clock Frequency 200 MHz 52 MHz
Power Efficiency Higher Lower

Real-World Considerations

  • Actual throughput often falls below theoretical limits due to:
    • eMMC controller latency[7]
    • Bus contention with other peripherals[7]
    • NAND flash internal write speeds[4]
  • Some SoCs like i.MX8MQ require device tree modifications (pinctrl_usdhc1_200mhz) to maintain high-speed modes[2].

Disabling HS200 simplifies hardware design and reduces EMI but imposes significant performance penalties for data-intensive applications like video recording or OS booting[1][3][5]. The choice depends on whether power savings/design simplicity outweigh speed requirements.

Citations:


Verify removal of MMC HS200 support for performance and hardware compatibility.
The patch removes both CONFIG_MMC_HS200_SUPPORT and CONFIG_SPL_MMC_HS200_SUPPORT, which effectively disables HS200 mode. As confirmed, HS200 mode enables data transfer rates of up to 200 MB/s, and its removal forces the system to fall back to slower modes (e.g., DDR52 at around 104 MB/s or even lower), potentially reducing throughput by nearly 50% in sequential operations. In addition, disabling HS200 also precludes the possibility of using HS400 mode in the future.

Please confirm the following:

  • The rationale behind removing HS200 support—whether it’s due to limitations in the Green board hardware or a deliberate design choice for enhanced reliability/simplicity.
  • That the board’s eMMC interface is fully compatible with the fallback speed modes, including considerations for voltage and clock frequency differences.
  • The overall impact on system performance (e.g., boot times and data throughput) is acceptable for the intended use cases.
buildroot-external/board/raspberrypi/patches/uboot/0006-nvme-translate-virtual-addresses-into-the-bus-s-addr.patch (10)

36-37: Use nvme_virt_to_bus() instead of direct casting to buffer address.

Replacing the direct cast with nvme_virt_to_bus() properly accounts for non-transparent or offset PCIe mappings on certain boards (e.g., RPi 4). This helps ensure correctness across diverse architectures.


66-66: Local variable dma_addr is introduced for clarity.

Declaring a separate local variable dma_addr for bus addresses (instead of relying on casts) improves readability and reduces confusion between CPU vs. bus addresses. Good practice.


73-80: Write to device registers using nvme_virt_to_bus().

This change properly sets up the Admin Submission and Completion Queue addresses as bus addresses rather than CPU addresses. This is critical for correct MMIO usage on platforms with offset mappings.


84-89: Reference command PRP buffer through bus addresses.

Updating c.create_cq.prp1 via nvme_virt_to_bus ensures consistent usage of bus addresses, preventing subtle hardware mapping errors.


94-100: Maintain consistency in queue creation routines.

Analogous revision for the submission queue creation confirms that the entire NVMe queue infrastructure is using translated addresses.


102-108: Adopt bus translation for Identify command buffers.

Using bus addresses for prp1 and prp2 helps ensure that the Identify command can access buffers reliably in scenarios where the PCIe bus imposes non-zero offsets.


118-124: Use translated addresses for Get Features command.

Consistently applying nvme_virt_to_bus() reduces the risk of hardware-level address mismatch.


128-133: Apply address translation in Set Features as well.

The approach parallels the Get Features method, guaranteeing bus offsets are honored for all admin commands.


137-142: Translate PRP pointer in data read/write path.

Similar to the admin commands, the read/write path (block I/O) now fully observes bus address rules, further preventing potential data corruption from misaligned addresses.


166-169: New inline helper nvme_virt_to_bus().

Introducing this helper formalizes address translation, preventing repetitive casting and providing a single reference point if bus offsets ever need to be changed or extended.

buildroot-external/board/raspberrypi/patches/uboot/0005-nvme-Use-pointer-for-CPU-addressed-buffers.patch (10)

24-32: Switch to pointers for CPU-addressed buffers in nvme_setup_prps().

Changing the function signature to accept void *buffer instead of a DMA address cuts down on type conversions and clarifies the difference between CPU-viewed pointers and bus addresses. The usage of (uintptr_t)buffer for offset calculation is valid and keeps pointer arithmetic consistent.


41-45: Use pointer arithmetic instead of adjusting dma_addr.

Refactoring from dma_addr += (page_size - offset) to buffer += (page_size - offset) aligns the code with typical pointer handling, streamlining the prp setup.


66-99: Adopt void *buffer in nvme_identify.

This consolidates the approach to CPU addressable buffers. The usage of invalidate_dcache_range((uintptr_t)buffer, (uintptr_t)buffer + size) remains consistent with standard pointer-based memory ranges.


110-126: Eliminate redundant DMA type usage in nvme_get_features.

Replacing dma_addr_t dma_addr with void *buffer clarifies the buffer’s usage in CPU context. This matches the earlier changes and ensures code uniformity.


128-141: Use pointer-based approach for nvme_set_features.

Again, removing explicit DMA address usage helps maintain consistency with the rest of the code. This fosters better maintainability and avoids type casting pitfalls in future expansions.


145-150: Avoid passing null buffer for feature setting.

Here nvme_set_features(dev, NVME_FEAT_NUM_QUEUES, q_count, NULL, &result) effectively means no associated buffer is required. Confirm that no feature data needs to be exchanged in this path.


154-160: Safely replace dma_addr_t with pointer in nvme_get_info_from_identify.

Switching to an actual pointer for ctrl aligns well with the rest of the changes, improving clarity that this buffer is allocated in CPU memory space.


172-174: Use a void *temp_buffer in nvme_blk_rw.

Refactoring to a pointer for local read/write operations streamlines the logic and is consistent with the other pointer-based transitions.


194-205: Consistent pointer usage in nvme_print_info.

Removing explicit cast to dma_addr_t clarifies the function’s reliance on CPU-accessible memory. This finalizes the transition to pointer-based calls throughout the NVMe driver.


217-255: Header changes ensure uniform function signatures.

Shifting from dma_addr_t to void *buffer in the function prototypes is consistent with the underlying code changes, preventing confusion about buffer ownership and address spaces.

buildroot-external/board/raspberrypi/patches/uboot/0010-pci-pcie-brcmstb-Add-basic-support-for-BCM2712-PCIe.patch (4)

28-32: No functional code here to review.
These lines only reflect diff statistics and file counts without any functional changes.


69-96: Approval for new memory mapping entry.
Increasing the array size from 4 to 5 and specifying the additional PCIe memory region for BCM2712 is a straightforward and valid approach. This ensures that the new hardware address space is properly mapped.


100-132: Structs and enum expansions are consistent.
Defining new enumerations, data structures, and offsets to handle Soc-specific differences is a maintainable approach. This design nicely segregates logic for BCM2711 vs BCM2712.


327-388: Overall removal and of-to-plat transitions are sound.
The logic to avoid shutting down the bridge for BCM2712 aligns with the known hardware constraint. Error handling for optional resets is also appropriately guarded. Great job!

buildroot-external/configs/khadas_vim3_defconfig (1)

139-139:

✅ Verification successful

Verify U-Boot version number.

The specified version "2025.01" appears to be a future version, as it's beyond the current date. Please verify if this is the correct version number.


🌐 Web query:

What is the latest version of U-Boot?

💡 Result:

The latest stable version of U-Boot as of February 17, 2025 is v2025.01, released on January 6, 2025[1]. This follows U-Boot's quarterly release cycle where versions are named using year and month identifiers (YYYY.MM format)[1][2].

Key details about upcoming releases:

  • The next scheduled version v2025.04 is planned for April 7, 2025[1]
  • Current development status shows:
    • Merge window closed for v2025.04[1]
    • Release candidate phase active (rc2 released February 10, 2025)[1]

The project maintains a predictable release schedule with:

  • New versions every ~3 months
  • Merge windows opening for 21 days after each release
  • Release candidate phases following merge window closure[1]

For developers working with specific hardware platforms like Pinebook Pro or VisionFive boards, note that device-specific U-Boot implementations might lag behind mainline releases - some reports mention systems using versions as old as 2017.09[3][4]. Always verify compatibility with your target hardware before upgrading bootloaders[6][7].

Citations:


U-Boot version confirmed as current.

The version "2025.01" in the khadas_vim3_defconfig file is correct—it is the latest stable U-Boot version released on January 6, 2025. No changes are needed.

buildroot-external/configs/odroid_c4_defconfig (2)

137-137: Verify U-Boot version number.

The specified version "2025.01" appears to be a future version, as it's beyond the current date. Please verify if this is the correct version number.


7-7:

❓ Verification inconclusive

Verify removal of board-specific patches.

The board-specific patches directory has been removed from BR2_GLOBAL_PATCH_DIR. Please ensure that no critical patches were present in the removed directory.


🏁 Script executed:

#!/bin/bash
# Check for any patches in the removed directory
fd -t f . "$(BR2_EXTERNAL_HASSOS_PATH)/board/hardkernel/patches" -x echo "Found patch: {}"

Length of output: 319


Manual Verification Needed: Confirm Absence of Critical Board-Specific Patches

The update simplifies BR2_GLOBAL_PATCH_DIR to include only general patches, effectively removing the board-specific patches directory. However, our automated check did not yield conclusive information due to an unset BR2_EXTERNAL_HASSOS_PATH (resulting in directory lookup errors). Please manually verify that no essential board-specific patches (previously expected under the intended hardkernel/patches path) have been inadvertently removed.

buildroot-external/configs/odroid_c2_defconfig (2)

7-7: Verify removal of board-specific patches.

The board-specific patches directory has been removed from BR2_GLOBAL_PATCH_DIR. Please ensure that no critical patches were present in the removed directory.


139-139: Verify U-Boot version number.

The specified version "2025.01" appears to be a future version, as it's beyond the current date. Please verify if this is the correct version number.

buildroot-external/configs/odroid_xu4_defconfig (2)

139-139: Verify U-Boot version number.

The specified version "2025.01" appears to be a future version, as it's beyond the current date. Please verify if this is the correct version number.


8-8:

❓ Verification inconclusive

Review patch directory configuration inconsistency.

The patch directory configuration differs from other Odroid configs:

  • Other configs only keep general patches
  • This config retains both general and XU4-specific patches
  • Common Hardkernel patches are removed

Please verify if this inconsistency is intentional.


🏁 Script executed:

#!/bin/bash
# Compare patch directories across configs
echo "Checking patch directories in other Odroid configs..."
rg "BR2_GLOBAL_PATCH_DIR" "buildroot-external/configs/odroid_*_defconfig"

Length of output: 276


Action Required: Verify Patch Directory Consistency Across Odroid Configurations

The previous automated search did not yield any odroid config files to compare against. It appears that either the expected odroid config files aren’t present at the specified location or the naming pattern differs. Please perform a manual verification to ensure that keeping both the general and XU4-specific patches in this configuration is intentional and consistent with your design across similar boards.

  • Confirm whether other Odroid configuration files exist and, if so, how their BR2_GLOBAL_PATCH_DIR entries are set.
  • If other configurations manage patches differently, decide if an update is needed for consistency.

To aid further investigation, you might run the following script to search for odroid-related defconfig files and their patch directory settings:


🏁 Script executed:

[run_scripts]
#!/bin/bash
echo "Searching for all odroid defconfig files in buildroot-external/configs..."
fd -e defconfig "odroid" buildroot-external/configs

echo "Checking patch directory definitions in found odroid defconfig files..."
rg "BR2_GLOBAL_PATCH_DIR" buildroot-external/configs | grep -i "odroid"

Length of output: 1472


Review patch directory configuration differences in Odroid defconfigs.

In this configuration (odroid_xu4_defconfig), the BR2_GLOBAL_PATCH_DIR is set to include:

  • The general patches directory: $(BR2_EXTERNAL_HASSOS_PATH)/patches
  • The board-specific patches directory: $(BR2_EXTERNAL_HASSOS_PATH)/board/hardkernel/odroid-xu4/patches

Verification across related files revealed:

  • odroid_c2_defconfig & odroid_c4_defconfig: Only include the general patches directory.
  • odroid_m1s_defconfig & odroid_n2_defconfig: Likewise include a board-specific patches directory.
  • odroid_m1_defconfig: Uses the general patches directory along with an extra patches-rockchip directory.

Please verify whether the inclusion of board-specific patches for the XU4 (and similar boards) is intentional based on board-specific requirements, or if the configurations should be standardized across all Odroid boards. Additionally, confirm that the U-Boot update (to version 2025.01 on line 139) aligns with your overall board support strategy.

buildroot-external/configs/tinker_defconfig (1)

139-139:

✅ Verification successful

Verify U-Boot version validity.

The specified U-Boot version "2025.01" appears to be a future version. Please verify if this is the correct version number.


🌐 Web query:

What is the latest version of U-Boot?

💡 Result:

The latest stable release of U-Boot as of February 17, 2025 is v2025.01, released on January 6, 2025[1][2]. The project follows a quarterly release cycle with versions named using year and month identifiers (e.g., 2025.04 for April 2025)[1][2].

Key details about current development:

  • The next planned release is v2025.04 scheduled for April 7, 2025[1]
  • Release candidates are currently being tested:
    • v2025.04-rc1 (January 27, 2025)
    • v2025.04-rc2 (February 10, 2025)[1]
  • The merge window for new features closed on January 27, 2025[1]

U-Boot's versioning system uses:

  • Year in first position (2025)
  • Month in second position (01 = January)
  • Optional patch level (e.g., .1 for bugfix releases)[2]
  • Release candidates marked with "-rcX" suffix[2]

Developers should note that while releases are planned quarterly on first Mondays[1], delays can occur for technical reasons[1]. The project maintains multiple active branches during development cycles, with the "next" branch opening after rc2 releases[1].

Citations:


U-Boot Version Verified

The version "2025.01" is confirmed to be the latest stable release as of early 2025. No changes are needed in the defconfig.

buildroot-external/configs/rpi2_defconfig (1)

142-142: Same version update as in tinker_defconfig.

buildroot-external/configs/odroid_n2_defconfig (2)

7-7: LGTM! Improved patch directory specificity.

The patch directory path has been updated to be more specific to the Odroid N2 board, which improves organization and clarity.


139-139: Same version update as in tinker_defconfig.

buildroot-external/configs/rpi3_defconfig (1)

144-144: Same version update as in tinker_defconfig.

buildroot-external/configs/rpi4_64_defconfig (1)

144-144:

✅ Verification successful

Verify U-Boot version 2025.01.

Please ensure that U-Boot version 2025.01 exists and is stable.


🌐 Web query:

What is the latest stable version of U-Boot? Is version 2025.01 released and stable?

💡 Result:

The latest stable version of U-Boot is v2025.01, which was officially released on Mon 06 January 2025[1][4]. This version follows U-Boot's quarterly release cycle and adheres to its structured development phases:

  • Stability confirmation: After a merge window closure on Jan 27, 2025[1][4], the project entered a bug-fix phase for v2025.04 (the next release), indicating v2025.01 has completed all stabilization processes.
  • Versioning scheme: Uses a year-month format (e.g., "2025.01" = January 2025 release)[1][3].
  • Maintenance status: Listed as up-to-date in OpenEmbedded's package tracking[2], with Fabio Estevam as maintainer.

Upcoming releases include v2025.04 (scheduled for Apr 7, 2025)[1][4], currently in its release candidate phase with -rc2 published on Feb 10, 2025[1][7].

Citations:


U-Boot Version Update Verified

The update to BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2025.01" is correct. The web query confirms that version 2025.01 is indeed the latest stable release (released on January 06, 2025) and follows the quarterly release cycle. No further changes are needed.

buildroot-external/configs/odroid_m1_defconfig (2)

139-139: Verify U-Boot version 2025.01.

Please ensure that U-Boot version 2025.01 exists and is stable.


7-7:

✅ Verification successful

Verify removal of board-specific patches.

The board-specific patches directory $(BR2_EXTERNAL_HASSOS_PATH)/board/hardkernel/odroid-m1/patches has been removed from BR2_GLOBAL_PATCH_DIR. Please confirm if this is intentional and that no critical board-specific patches are being omitted.


🏁 Script executed:

#!/bin/bash
# Description: Check if there are any patches in the removed directory that might be needed.

# Test: Check if the removed patches directory exists and contains any patches
fd -t f -e patch . "board/hardkernel/odroid-m1/patches" || echo "Directory not found or empty"

Length of output: 244


Confirm Board-Specific Patch Removal

The script output indicates that the board-specific patch directory (board/hardkernel/odroid-m1/patches) does not exist or is empty—there are no board-specific patch files present. This suggests that its removal from BR2_GLOBAL_PATCH_DIR is unlikely to omit any critical patches. Please confirm that this change was intentional and that board-specific functionality isn’t inadvertently affected.

buildroot-external/configs/odroid_m1s_defconfig (1)

139-139: Verify U-Boot version 2025.01.

Please ensure that U-Boot version 2025.01 exists and is stable.

buildroot-external/configs/yellow_defconfig (1)

146-146: Verify U-Boot version 2025.01.

Please ensure that U-Boot version 2025.01 exists and is stable.

buildroot-external/configs/green_defconfig (1)

144-144:

❓ Verification inconclusive

Verify U-Boot version 2025.01 exists and is stable.

The specified U-Boot version appears to be from the future. Let's verify its existence and stability.

Additionally, please verify compatibility with the current kernel version (6.12.13) and test the boot process thoroughly.


🌐 Web query:

What is the latest stable version of U-Boot? Has version 2025.01 been released?

💡 Result:

The latest stable version of U-Boot is v2025.01, which was officially released on Mon 06 January 2025[1][3][4]. This follows U-Boot's quarterly release cycle, with versions identified by year and month (e.g., "2025.01" denotes January 2025).

Key details about the current status:

  • The merge window for the next planned release (v2025.04) closed on Mon 27 January 2025[1][4]
  • Release candidates for v2025.04 include:
    • v2025.04-rc1 (27 Jan 2025)
    • v2025.04-rc2 (10 Feb 2025)[1][4]
  • Future releases are scheduled for:
    • v2025.07: Mon 07 Jul 2025
    • v2025.10: Mon 06 Oct 2025
    • v2026.01: Mon 05 Jan 2026[1][4]

U-Boot maintains a structured release cycle with:

  • 3-month intervals between major versions
  • 21-day merge windows for new features after each release
  • A focus on bug fixes during the release candidate phase[1][3][4]

The version numbering system uses calendar-based identifiers since October 2008, where "YYYY.MM" indicates the release year and month[1][3].

Citations:


U-Boot v2025.01 Verified – Please Confirm Kernel Compatibility

We've confirmed via the latest release information that U-Boot v2025.01 is indeed the current stable version (released on January 6, 2025). The updated configuration in buildroot-external/configs/green_defconfig correctly reflects this version:

BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2025.01"

Next Steps:

  • Verify that U-Boot v2025.01 is fully compatible with the Linux kernel version 6.12.13.
  • Conduct thorough boot process tests to ensure system stability and functionality.
buildroot-external/board/nabucasa/green/patches/uboot/0004-arch-arm-dts-green-spi-nor-uboot.patch (4)

20-70: Check potential conflicts with multiple bootph-all properties.
Multiple pin groups and nodes (including eMMC, fsPI, etc.) now have bootph-all. While this ensures early availability, ensure there are no resource conflicts among these pin groups or other pinctrl consumers that also need initialization at boot.


76-82: Evaluate HS400 Enhanced Strobe support.
Adding mmc-hs400-enhanced-strobe can boost performance on supported eMMC modules. However, some boards experience instability with HS400 strobing. Recommend stress testing to confirm reliable operation or consider limiting to HS200 if issues arise.


84-99: Validate SPI maximum frequency configuration.
The spi-max-frequency = <24000000> might differ from the main DTS settings. Confirm that 24 MHz is the intended limit and that other references to 25 MHz (or higher) in the codebase will not introduce inconsistency or confusion.


125-137: Confirm reduced SPI frequency and bus width impact.
Dropping from 100 MHz/4-bit to 25 MHz/1-bit can significantly reduce throughput. Ensure the performance trade-off is acceptable or revert to a higher speed if your hardware validations allow.

buildroot-external/board/nabucasa/green/patches/uboot/0006-green-spl-loader-uboot-order-sd-emmc-spi_nor.patch (1)

19-20: Confirm new SPL boot device order.
You’ve reordered the boot sequence to prioritize SD/eMMC over SPI. Verify that this matches your preferred fallback strategy to ensure reliable booting in all scenarios.

buildroot-external/board/nabucasa/green/patches/uboot/0008-arch-arm-dts-green-drop-SPI-NOR-flash-partitions.patch (1)

19-44: Verify impact of removing SPI NOR partitions.
Removing these partition definitions simplifies the DTS but may break any existing flow or scripts that expect these partitions to store environment variables, SPL, or other data. Double-check that no critical functionality depends on them.

buildroot-external/board/nabucasa/green/patches/uboot/0003-arch-arm-dts-rk3566-green-boot-order.patch (3)

28-30: LGTM: Added bootph-all property.

The addition of bootph-all property to spiflash ensures the node is available in all boot phases.


43-45: LGTM: Improved node naming.

Renaming flash@0 to spiflash: flash@0 enhances code clarity by making the flash type explicit.


19-20:

❓ Verification inconclusive

Verify boot order change impact.

The boot order has been changed to prioritize SPI flash over SDHCI and SDMMC. This change could affect the boot process reliability.


🏁 Script executed:

#!/bin/bash
# Check for any other boot order configurations that might conflict
rg -A 2 "u-boot,spl-boot-order" 

Length of output: 1504


Action required: Confirm consistency of boot order configuration across patches

The DTS patch in buildroot-external/board/nabucasa/green/patches/uboot/0003-arch-arm-dts-rk3566-green-boot-order.patch now sets the boot order to prioritize SPI flash (&spiflash, &sdmmc0, &sdhci) instead of using the "same-as-spl" reference. Note the following observations from the repository search:

  • Patch 0003:

    • Old: "same-as-spl", &sdhci, &sdmmc0
    • New: &spiflash, &sdmmc0, &sdhci
      This change places SPI flash as the primary boot device.
  • Patch 0001:

    • Retains "same-as-spl", &sdhci, &sdmmc0 and does not reflect the new priority order.
  • Patch 0006:

    • Alters the boot order from &spiflash, &sdmmc0, &sdhci to &sdmmc0, &sdhci, &spiflash, effectively making SPI flash the last option in that context.

Please verify that these differing configurations are intentional for their respective contexts (DTS file versus SPL loader) and that they do not inadvertently conflict with one another, which could impact boot process reliability. It is especially important that any change in boot device priority is well-coordinated across all related patches.

buildroot-external/board/nabucasa/green/patches/uboot/0010-arch-arm64-dts-green-Improve-LED-representation.patch (1)

25-52: LGTM: Enhanced LED representation.

The changes improve clarity by:

  • Using descriptive node names (power, activity, user)
  • Adding explicit labels
  • Correcting LED colors to match their functions
buildroot-external/board/nabucasa/green/patches/uboot/0009-green-Do-not-use-eMMC-DDR52-mode-enable-HS200.patch (2)

31-36: LGTM: Addressed eMMC writing issues.

Removing DDR52 and HS400 modes while keeping HS200 mode addresses known eMMC writing failures on RK356x boards. This is well-documented in the upstream discussion.


54-55: LGTM: Added HS200 support configuration.

Adding CONFIG_MMC_HS200_SUPPORT and CONFIG_SPL_MMC_HS200_SUPPORT ensures proper support for HS200 mode.

buildroot-external/board/nabucasa/green/patches/uboot/0002-configs-green-Support-USB-boot.patch (3)

24-24: Verify boot recovery mechanism.

Removing CONFIG_SPL_ROCKCHIP_BACK_TO_BROM while adding CONFIG_ROCKCHIP_SPI_IMAGE changes the recovery path. Ensure there's still a way to recover from boot failures.

Also applies to: 27-27


106-113: LGTM: Comprehensive USB support added.

Added support for:

  • XHCI, EHCI, and OHCI HCD
  • DWC3 USB controller
  • Generic USB drivers

48-49:

❓ Verification inconclusive

Verify impact of increased SPL size.

The SPL maximum size has been doubled from 0x20000 to 0x40000. Ensure this doesn't exceed platform limitations.


🏁 Script executed:

#!/bin/bash
# Check for platform-specific SPL size limitations
rg -A 2 "CONFIG_SPL_MAX_SIZE" 

Length of output: 1393


Attention: Confirm Platform SPL Size Limits

The SPL maximum size has been doubled from 0x20000 to 0x40000 in the “0002-configs-green-Support-USB-boot.patch” (and reaffirmed in “0011-configs-green-fix-bss-and-stack-address-and-disable-.patch”). Please verify that this increased value fits within your platform’s hardware limitations. Also, note that the removal of BROM fallback might impact recovery options; ensure that an adequate alternative is in place if needed.

  • Confirm that the hardware supports a SPL max size of 0x40000.
  • Revisit any dependencies on the original 0x20000 setting elsewhere in the configuration.
  • Check that the removal of the BROM fallback is intentional and safe for recovery.
buildroot-external/board/nabucasa/green/patches/uboot/0001-arch-arm-dts-Add-NabuCasa-Green-board.patch (3)

20-26: LGTM!

The Makefile change correctly adds the device tree target for the NabuCasa Green board.


702-709:

❓ Verification inconclusive

Review USB host speed limitation.

The USB host1 XHCI controller is limited to high-speed mode while host0 supports full speed. This asymmetry should be documented.


🏁 Script executed:

#!/bin/bash
# Description: Check for similar USB speed limitations in other Rockchip boards

rg -B 2 -A 5 "maximum-speed.*high-speed" --type dts

Length of output: 81


Attention: USB Host Speed Configuration Verification Needed

  • The patch shows that the USB host1 XHCI controller is force-set to "high-speed" (lines 702–709). However, it's noted that host0 supports full speed, creating an asymmetry.
  • Since our initial search for similar configurations in other Rockchip DTS files resulted in an error (unrecognized file type: dts), please manually verify:
    • Whether other Rockchip board DTS files document similar USB speed settings.
    • If the high-speed limitation on host1 is intentional due to hardware design, and if so, ensure that this design decision is clearly documented.

46-48:

❓ Verification inconclusive

Verify boot order configuration.

The boot order is set to try booting from the same device as SPL, followed by eMMC (sdhci), and then SD card (sdmmc0). Please verify this matches the intended boot sequence for the board.


🏁 Script executed:

#!/bin/bash
# Description: Check for other boot order configurations in the codebase
# to ensure consistency with similar Rockchip boards.

rg -A 5 "u-boot,spl-boot-order" --type dtsi

Length of output: 72


Action Required: Verify Boot Order Configuration

The boot order defined in this patch—using "same-as-spl", &sdhci, &sdmmc0—needs to be confirmed against the board’s intended boot sequence. Note that the initial verification command using rg --type dtsi resulted in an error (unrecognized file type: dtsi), so its output couldn’t be used to cross-check consistency with other Rockchip board configurations.

Next Steps:

  • Manually verify that the boot order (SPL device, then the device represented by &sdhci, followed by &sdmmc0) is correct for this board.
  • Consider using a revised search query such as:
    rg -A 5 "u-boot,spl-boot-order" -g "*.dts" -g "*.dtsi"
    to locate similar configurations in device tree source files for consistency.

File:

  • buildroot-external/board/nabucasa/green/patches/uboot/0001-arch-arm-dts-Add-NabuCasa-Green-board.patch (lines 46-48)

To stay on the safe side, move the eMMC hack to more generic folder that's used
for all targets using the meson_gx eMMC driver (i.e. C2, C4 and N2). This is
still better than keeping it in hardkernel/patches which is applied only to
some hardkernel boards (like it was before bump to U-Boot 20205.01).
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🔭 Outside diff range comments (1)
buildroot-external/board/hardkernel/patches-meson/0001-HACK-mmc-meson-gx-limit-f_max-to-24-MHz-on-the-first.patch (1)

1-20: 💡 Verification agent

❓ Verification inconclusive

Consider documenting plans to remove this hack.

While the commit message clearly explains the problem and solution, since this is marked as a "HACK", it would be helpful to document:

  1. Whether this is a temporary solution
  2. Any plans or conditions for removing this hack in the future
  3. Any upstream discussions or alternative solutions being considered

Let's check if there are any related discussions or patches upstream:


🏁 Script executed:

#!/bin/bash
# Search for related discussions or patches in U-Boot mailing list archives
curl -s "https://lists.denx.de/pipermail/u-boot/2024-February/thread.html" | grep -i "meson.*mmc.*clock"

Length of output: 104


Update the commit message to clarify that this hack is temporary and outline conditions for its future removal.

The commit message explains the rationale behind limiting the maximum clock rate to 24 MHz and introduces a flag to remove this cap on failure. However, since the hack is clearly marked as such—and no upstream discussion was found in our mailing list check—it would be helpful to document whether this workaround is temporary and to specify the conditions or plans for its removal in the future.

🧹 Nitpick comments (1)
buildroot-external/board/hardkernel/patches-meson/0001-HACK-mmc-meson-gx-limit-f_max-to-24-MHz-on-the-first.patch (1)

55-68: Improve the error message clarity.

The error message could be more specific about what's happening next.

-			printf("Card failed to initialize at %d Hz, disabling meson_gx hack.\n",
+			printf("Card failed to initialize at %d Hz, retrying without frequency limitation.\n",
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6044f92 and f2d4371.

📒 Files selected for processing (4)
  • buildroot-external/board/hardkernel/patches-meson/0001-HACK-mmc-meson-gx-limit-f_max-to-24-MHz-on-the-first.patch (5 hunks)
  • buildroot-external/configs/odroid_c2_defconfig (2 hunks)
  • buildroot-external/configs/odroid_c4_defconfig (2 hunks)
  • buildroot-external/configs/odroid_n2_defconfig (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • buildroot-external/configs/odroid_c2_defconfig
  • buildroot-external/configs/odroid_n2_defconfig
  • buildroot-external/configs/odroid_c4_defconfig
🔇 Additional comments (3)
buildroot-external/board/hardkernel/patches-meson/0001-HACK-mmc-meson-gx-limit-f_max-to-24-MHz-on-the-first.patch (3)

31-38: LGTM! Flag initialization is well-placed.

The initialization of meson_gx_f_max_hack is appropriately placed after the config initialization but before the private data setup.


44-54: LGTM! Clock limitation is properly implemented.

The conditional clock limitation is well-implemented with clear documentation.


73-82: LGTM! Flag declaration is well-placed.

The meson_gx_f_max_hack flag is appropriately declared as a boolean and well-placed within the struct.

Copy link
Member

@agners agners left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@sairon sairon merged commit dc7b693 into dev Feb 18, 2025
3 checks passed
@sairon sairon deleted the uboot-2025.01 branch February 18, 2025 12:48
@sairon sairon mentioned this pull request Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed os u-boot U-Boot related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants