Skip to content

Support for wolfBoot ZynqMP (UltraScale+ MPSoC) SD Card#699

Draft
dgarske wants to merge 6 commits intowolfSSL:masterfrom
dgarske:zynqmp_sdcard
Draft

Support for wolfBoot ZynqMP (UltraScale+ MPSoC) SD Card#699
dgarske wants to merge 6 commits intowolfSSL:masterfrom
dgarske:zynqmp_sdcard

Conversation

@dgarske
Copy link
Contributor

@dgarske dgarske commented Feb 26, 2026

No description provided.

@dgarske dgarske self-assigned this Feb 26, 2026
Copilot AI review requested due to automatic review settings February 26, 2026 21:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds SD card boot support for the AMD Zynq UltraScale+ MPSoC (ZynqMP) platform, complementing the existing QSPI boot capability. wolfBoot can now load firmware images from MBR-partitioned SD cards using the Arasan SDHCI controller.

Changes:

  • Implements SDHCI platform support for ZynqMP with register translation between Cadence SD4HC and Arasan layouts
  • Adds conditional compilation for QSPI initialization to support SD-only boot configurations
  • Documents SD card boot setup with MBR partitioning and dual A/B firmware images

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
hal/zynq.h Adds SD/SDHCI controller base addresses and clock/reset control register definitions
hal/zynq.c Implements SDHCI platform hooks with register translation and conditional QSPI initialization
docs/Targets.md Documents SD card boot configuration, partition layout, and setup procedures
config/examples/zynqmp_sdcard.config Provides complete SD card boot configuration with MBR partition support
Comments suppressed due to low confidence (2)

config/examples/zynqmp_sdcard.config:1

  • The size specification '128M' is ambiguous in sfdisk context. sfdisk expects sector counts or explicit +size notation. Use size=+128M or calculate the sector count (262144 sectors for 128MB with 512-byte sectors) to ensure consistent partition creation across different sfdisk versions.
# wolfBoot configuration for AMD ZynqMP ZCU102 - SD Card Boot

config/examples/zynqmp_sdcard.config:1

  • The size specifications '200M' should use explicit +size notation for clarity. Use size=+200M to ensure sfdisk interprets these as size specifications rather than potential sector counts.
# wolfBoot configuration for AMD ZynqMP ZCU102 - SD Card Boot

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings February 26, 2026 23:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings March 3, 2026 01:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +2130 to +2138
sfdisk sdcard.img <<EOF
label: dos
unit: sectors

1 : start=2048, size=128M, type=c, bootable
2 : size=200M, type=83
3 : size=200M, type=83
4 : type=83
EOF
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The SD card provisioning steps are in the wrong order and will corrupt the partition table. The sequence first writes the whole image (including MBR) to /dev/sdX, then runs mkfs.vfat -F 32 -n BOOT /dev/sdX1 to format partition 1. This is correct, as mkfs.vfat only writes to the partition area, not the MBR. However, since partition 2 and 3 already have signed images from the sdcard.img that was dd'd to the device, the formatting of partition 1 is valid. The actual issue is that BOOT.BIN is written to partition 1 but the sdcard.img already has an empty FAT32 partition 1 (from just sfdisk without a mkfs), so the workflow is: write whole image → format partition 1 → copy BOOT.BIN to partition 1. This is logically correct but the sdcard.img has an unformatted partition 1 (just partition table), so the dd if=sdcard.img followed by mkfs.vfat on sdX1 works. This seems fine actually.

More importantly: the sfdisk command in the SD image creation specifies unit: sectors but then uses size=128M and size=200M with MiB suffix notation. While modern sfdisk (util-linux >= 2.26) handles this correctly (the M suffix overrides unit: sectors), the unit: sectors line is misleading and inconsistent since all sizes use human-readable suffixes rather than sector counts. Consider removing the unit: sectors line or converting sizes to sector counts for clarity.

Copilot uses AI. Check for mistakes.
# Zynq UltraScale+ MPSoC ZU9EG - Quad-core ARM Cortex-A53
#
# This configuration enables SD card boot for the ZynqMP:
# FSBL -> BL31 (EL3) -> wolfBoot (EL2) -> Linux (EL1)
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The config file comment at line 5 describes the boot chain as FSBL -> BL31 (EL3) -> wolfBoot (EL2) -> Linux (EL1), omitting PMUFW. The BIF file and documentation both include PMUFW in the boot chain: FSBL -> PMUFW -> BL31 (EL3) -> wolfBoot (EL2). The comment should include PMUFW to be accurate.

Suggested change
# FSBL -> BL31 (EL3) -> wolfBoot (EL2) -> Linux (EL1)
# FSBL -> PMUFW -> BL31 (EL3) -> wolfBoot (EL2) -> Linux (EL1)

Copilot uses AI. Check for mistakes.
```

### Building Zynq with Xilinx tools (Vitis IDE)
wolfBoot runs from DDR at `0x7FF00000` (EL2, non-secure). All clock, MIO, and DDR initialization is handled by the FSBL/PMUFW before wolfBoot starts.
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The general description at line 1994 states "wolfBoot runs from DDR at 0x7FF00000" but this is only correct for QSPI boot. For SD card boot, wolfBoot runs at 0x8000000 (as specified in config/examples/zynqmp_sdcard.config and hal/zynq_sd.ld). Since the paragraph now introduces two boot paths, the general description should either be removed or clarified to indicate it applies to QSPI boot only. The SD card boot address is described later in the SD card section.

Suggested change
wolfBoot runs from DDR at `0x7FF00000` (EL2, non-secure). All clock, MIO, and DDR initialization is handled by the FSBL/PMUFW before wolfBoot starts.
For QSPI boot, wolfBoot runs from DDR at `0x7FF00000` (EL2, non-secure). All clock, MIO, and DDR initialization is handled by the FSBL/PMUFW before wolfBoot starts.

Copilot uses AI. Check for mistakes.
* Loaded by FSBL to end of DDR0
* For SD card boot see zynq_sd.ld
*/
psu_ddr_0_MEM_0 : ORIGIN = 0x7FF00000, LENGTH = 0x200000
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The psu_ddr_0_MEM_0 region for QSPI boot was extended from LENGTH = 0x100000 (1MB) to LENGTH = 0x200000 (2MB). With ORIGIN = 0x7FF00000, the region now extends to 0x80100000, which goes past the end of DDR0 at 0x80000000. On ZynqMP, addresses from 0x80000000 to 0x7FFFFFFFF map to FPGA/PL address space (not DRAM), so any code or data placed beyond 0x80000000 would be in device register space rather than RAM. The original 1MB limit was correct to stay within DDR0 bounds. While the SD card linker script zynq_sd.ld correctly places wolfBoot at 0x8000000 with 2MB, the zynq.ld (QSPI boot) increase to 2MB at 0x7FF00000 may allow the linker to place code in non-DDR memory if wolfBoot exceeds 1MB.

Suggested change
psu_ddr_0_MEM_0 : ORIGIN = 0x7FF00000, LENGTH = 0x200000
psu_ddr_0_MEM_0 : ORIGIN = 0x7FE00000, LENGTH = 0x200000

Copilot uses AI. Check for mistakes.
reg &= ~(SDHCI_SRS11_SDCFSL_MASK | SDHCI_SRS11_SDCFSH_MASK);
reg |= (((mclk & 0x0FF) << SDHCI_SRS11_SDCFSL_SHIFT) & SDHCI_SRS11_SDCFSL_MASK); /* lower 8 bits */
reg |= (((mclk & 0x300) << SDHCI_SRS11_SDCFSH_SHIFT) & SDHCI_SRS11_SDCFSH_SHIFT); /* upper 2 bits */
reg |= (((mclk & 0x300) << SDHCI_SRS11_SDCFSH_SHIFT) & SDHCI_SRS11_SDCFSH_MASK); /* upper 2 bits */
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The fix to the upper 2 bits of the SD clock divisor is still incorrect. The expression ((mclk & 0x300) << SDHCI_SRS11_SDCFSH_SHIFT) & SDHCI_SRS11_SDCFSH_MASK where SDHCI_SRS11_SDCFSH_SHIFT = 6 and SDHCI_SRS11_SDCFSH_MASK = 0xC0 will always produce 0.

The upper 2 bits of the divider are in mclk at bit positions 9:8 (value mclk & 0x300). These need to go into register bits 7:6. That requires a right-shift of 2, not a left-shift of 6. The corrected expression should be ((mclk & 0x300) >> 2) & SDHCI_SRS11_SDCFSH_MASK, or equivalently ((mclk >> 8) << SDHCI_SRS11_SDCFSH_SHIFT) & SDHCI_SRS11_SDCFSH_MASK.

Suggested change
reg |= (((mclk & 0x300) << SDHCI_SRS11_SDCFSH_SHIFT) & SDHCI_SRS11_SDCFSH_MASK); /* upper 2 bits */
reg |= (((mclk & 0x300) >> 2) & SDHCI_SRS11_SDCFSH_MASK); /* upper 2 bits */

Copilot uses AI. Check for mistakes.
Comment on lines +2013 to +2029

/* Set SD1 slot type to "Embedded Slot for One Device" (01).
* This feeds into the SDHCI Capabilities register bits 31:30 and makes
* the controller report card as always present, bypassing the physical
* CD pin that is not connected to the SDHCI controller on ZCU102. */
reg = IOU_SLCR_SD_CONFIG_REG2;
reg &= ~SD_CONFIG_REG2_SD1_SLOTTYPE_MASK;
reg |= (1UL << SD_CONFIG_REG2_SD1_SLOTTYPE_SHIFT); /* 01 = Embedded */
IOU_SLCR_SD_CONFIG_REG2 = reg;

/* The SDHCI Capabilities register latches IOU_SLCR values on controller
* reset. Issue SDIO1 reset via CRL_APB so the controller picks up the
* new slot type configuration. */
RST_LPD_IOU2 |= RST_LPD_IOU2_SDIO1; /* Assert SDIO1 reset */
for (i = 0; i < 100; i++) {} /* Brief delay */
RST_LPD_IOU2 &= ~RST_LPD_IOU2_SDIO1; /* De-assert SDIO1 reset */
for (i = 0; i < 1000; i++) {} /* Wait for controller ready */
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The sdhci_platform_init function unconditionally uses SD1-specific registers (SD_CONFIG_REG2_SD1_SLOTTYPE_MASK, SD_CONFIG_REG2_SD1_SLOTTYPE_SHIFT, RST_LPD_IOU2_SDIO1) even when ZYNQMP_SDHCI_BASE is configured to ZYNQMP_SD0_BASE. If a user overrides ZYNQMP_SDHCI_BASE to use SD0 (eMMC), the code will still configure the SD1 slot type and reset SD1, leaving SD0 unconfigured. The initialization logic should be conditional on the selected controller, using the SD0 registers (SD_CONFIG_REG2_SD0_SLOTTYPE_MASK, SD_CONFIG_REG2_SD0_SLOTTYPE_SHIFT, RST_LPD_IOU2_SDIO0) when ZYNQMP_SDHCI_BASE == ZYNQMP_SD0_BASE.

Suggested change
/* Set SD1 slot type to "Embedded Slot for One Device" (01).
* This feeds into the SDHCI Capabilities register bits 31:30 and makes
* the controller report card as always present, bypassing the physical
* CD pin that is not connected to the SDHCI controller on ZCU102. */
reg = IOU_SLCR_SD_CONFIG_REG2;
reg &= ~SD_CONFIG_REG2_SD1_SLOTTYPE_MASK;
reg |= (1UL << SD_CONFIG_REG2_SD1_SLOTTYPE_SHIFT); /* 01 = Embedded */
IOU_SLCR_SD_CONFIG_REG2 = reg;
/* The SDHCI Capabilities register latches IOU_SLCR values on controller
* reset. Issue SDIO1 reset via CRL_APB so the controller picks up the
* new slot type configuration. */
RST_LPD_IOU2 |= RST_LPD_IOU2_SDIO1; /* Assert SDIO1 reset */
for (i = 0; i < 100; i++) {} /* Brief delay */
RST_LPD_IOU2 &= ~RST_LPD_IOU2_SDIO1; /* De-assert SDIO1 reset */
for (i = 0; i < 1000; i++) {} /* Wait for controller ready */
uint32_t slot_mask;
uint32_t slot_shift;
uint32_t reset_bit;
/* Set the selected SDx slot type to "Embedded Slot for One Device" (01).
* This feeds into the SDHCI Capabilities register bits 31:30 and makes
* the controller report card as always present, bypassing the physical
* CD pin that is not connected to the SDHCI controller on ZCU102. */
if (ZYNQMP_SDHCI_BASE == ZYNQMP_SD0_BASE) {
slot_mask = SD_CONFIG_REG2_SD0_SLOTTYPE_MASK;
slot_shift = SD_CONFIG_REG2_SD0_SLOTTYPE_SHIFT;
reset_bit = RST_LPD_IOU2_SDIO0;
} else {
slot_mask = SD_CONFIG_REG2_SD1_SLOTTYPE_MASK;
slot_shift = SD_CONFIG_REG2_SD1_SLOTTYPE_SHIFT;
reset_bit = RST_LPD_IOU2_SDIO1;
}
reg = IOU_SLCR_SD_CONFIG_REG2;
reg &= ~slot_mask;
reg |= (1UL << slot_shift); /* 01 = Embedded */
IOU_SLCR_SD_CONFIG_REG2 = reg;
/* The SDHCI Capabilities register latches IOU_SLCR values on controller
* reset. Issue SDIOx reset via CRL_APB so the controller picks up the
* new slot type configuration. */
RST_LPD_IOU2 |= reset_bit; /* Assert SDIOx reset */
for (i = 0; i < 100; i++) {} /* Brief delay */
RST_LPD_IOU2 &= ~reset_bit; /* De-assert SDIOx reset */
for (i = 0; i < 1000; i++) {} /* Wait for controller ready */

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants