Hi Hrushikesh, Thank you for the patch.
v4 is in good shape already. I have a couple of small remarks below. Please have a look. On Thu, Oct 23, 2025 at 13:39, Hrushikesh Salunke <[email protected]> wrote: > Introduces support for Device Firmware Upgrade (DFU) over PCIe in > U-Boot. Traditionally, the DFU protocol is used over USB, where a > device enters DFU mode and allows a host to upload firmware or binary > images directly via the USB interface. This is a widely adopted and > convenient method for updating firmware. > > In the context of Texas Instruments (TI) SoCs, PCIe can be used as a > boot interface in a manner that differs from the conventional > "PCIe Boot" process, which typically refers to booting an OS or > firmware image from an NVMe SSD or other PCIe-attached storage devices. > Instead, TI SoCs can be configured as a PCIe Endpoint, allowing a > connected PCIe Root Complex (host) to transfer images directly into the > device’s memory over the PCIe bus for boot purposes. This mechanism is > analogous to DFU over USB, but leverages the high-speed PCIe link and > does not depend on traditional storage devices. > > By extending the DFU framework in U-Boot to support PCIe, it will be > possible to flash images over PCIe. While this implementation is > motivated by TI SoC use cases, the framework is generic and can be > adopted by everyone for platforms that support PCIe Endpoint mode. > Platforms with hardware support for PCIe-based memory loading can use > this to implement PCIe as a boot mode, as well as to enable flashing > and recovery scenarios similar to DFU over USB. > > In summary, enable support for: > - DFU-style flashing of firmware/images over PCIe, analogous to existing > USB DFU workflows > - PCIe as a boot mode where a host can load images directly into device > memory using DFU over PCIe > > Signed-off-by: Hrushikesh Salunke <[email protected]> > --- > This patch is based on commit > 4e4a9de31de Merge branch 'next' of git://source.denx.de/u-boot-usb into next > > Changes since v3 > - Addressed feedback from Mattijs Korpershoek > - Renamed all Kconfig symbols to include SPL_ prefix to avoid potential > naming clashes with future U-Boot proper DFU over PCIe implementation: > * PCI_DFU_SPL_LOAD_FIT_ADDRESS -> SPL_PCI_DFU_LOAD_FIT_ADDRESS > * PCI_DFU_BAR_SIZE -> SPL_PCI_DFU_BAR_SIZE > * PCI_DFU_MAGIC_WORD -> SPL_PCI_DFU_MAGIC_WORD > * PCI_DFU_VENDOR_ID -> SPL_PCI_DFU_VENDOR_ID > * PCI_DFU_DEVICE_ID -> SPL_PCI_DFU_DEVICE_ID > * PCI_DFU_BOOT_PHASE -> SPL_PCI_DFU_BOOT_PHASE > - Changed strncpy to strlcpy for safer string copying > - Fixed boot phase string length check to enforce 63 character max as > documented in Kconfig > - Improved error message clarity for boot phase string length validation > - Removed verbose debug print statements from normal operation > > v3 : > https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/ > > common/spl/Kconfig | 62 ++++++++++++++++++++++++++++++ > common/spl/spl_dfu.c | 91 ++++++++++++++++++++++++++++++++++++++++++++ > common/spl/spl_ram.c | 18 +++++++++ > drivers/Makefile | 1 + > 4 files changed, 172 insertions(+) > > diff --git a/common/spl/Kconfig b/common/spl/Kconfig > index f34b96efc02..86464c17799 100644 > --- a/common/spl/Kconfig > +++ b/common/spl/Kconfig > @@ -1278,6 +1278,14 @@ config SPL_PCI > necessary driver support. This enables the drivers in drivers/pci > as part of an SPL build. > > +config SPL_PCI_ENDPOINT > + bool "Support for PCI endpoint drivers" > + help > + Enable this configuration option to support configurable PCI > + endpoints at SPL. This should be enabled if the platform has > + a PCI controllers that can operate in endpoint mode (as a device > + connected to PCI host or bridge). > + > config SPL_PCH > bool "Support PCH drivers" > help > @@ -1338,6 +1346,60 @@ config SPL_RAM_DEVICE > be already in memory when SPL takes over, e.g. loaded by the boot > ROM. > > +config SPL_PCI_DFU > + bool "PCIe boot support" > + depends on SPL_PCI_ENDPOINT > + help > + This config enables support to download bootloaders over PCIe > + when device is acting as an PCI endpoint. > + > +if SPL_PCI_DFU > + > +config SPL_PCI_DFU_SPL_LOAD_FIT_ADDRESS > + hex "Address to load FIT image when booting via DFU over PCIe" > + help > + Specify the load address of the fit image that will be loaded > + by SPL via DFU over PCIe. > + > +config SPL_PCI_DFU_BAR_SIZE > + hex "BAR size to advertise for PCIe DFU" > + default 0x800000 > + help > + This config sets the size of BAR to be advertised to the Root > + Complex. The size should be large enough to fit the FIT image > + being downloaded via DFU over PCIe. > + > +config SPL_PCI_DFU_MAGIC_WORD > + hex "Completion magic word for PCIe DFU boot" > + default 0xdeadbeef > + help > + Specify the magic word which will be written to a specific > + address to signal the completion of transfer of FIT image > + when using DFU over PCIe to download the image. Size of magic > + word should be 32-bit. > + > +config SPL_PCI_DFU_VENDOR_ID > + hex "PCI Vendor ID for PCI endpoint" > + help > + PCI Vendor ID for endpoint device for DFU over PCIe. This should > + be set to your assigned 16-bit PCI Vendor ID. > + > +config SPL_PCI_DFU_DEVICE_ID > + hex "PCI Vendor ID for PCI endpoint" > + help > + A 16-bit PCI Vendor ID for endpoint device for DFU over PCIe. > + > +config SPL_PCI_DFU_BOOT_PHASE > + string "Current boot phase for PCI DFU boot" > + help > + Specify the current boot phase when booting via DFU over PCIe. > + This value can be read by the root complex to determine the > + current boot phase. Value of this config is written to memory > + location (BAR_start + PCI_DFU_BAR_SIZE - 70). Max size of this > + config is 63 bytes. > + > +endif > + > config SPL_REMOTEPROC > bool "Support REMOTEPROCS" > default y if (CPU_V7R && ARCH_K3) > diff --git a/common/spl/spl_dfu.c b/common/spl/spl_dfu.c > index e9f381c392c..b09f82790c9 100644 > --- a/common/spl/spl_dfu.c > +++ b/common/spl/spl_dfu.c > @@ -15,6 +15,17 @@ > #include <usb.h> > #include <dfu.h> > #include <linux/printk.h> > +#include <pci_ep.h> > +#include <dm/uclass.h> > +#include <cpu_func.h> > +#include <linux/io.h> > + > +/* > + * Macros define size of magic word and boot phase string > + * in bytes. > + */ > +#define MAGIC_WORD_SIZE 4 > +#define BOOT_PHASE_STRING_SIZE 63 > > static int run_dfu(int usb_index, char *interface, char *devstring) > { > @@ -32,11 +43,91 @@ exit: > return ret; > } > > +#ifdef CONFIG_SPL_PCI_DFU > +static int dfu_over_pcie(void) > +{ > + u32 offset, magic_word; > + volatile void *addr; > + struct udevice *dev; > + struct pci_bar bar; > + struct pci_ep_header hdr; > + uint fn = 0; > + int ret; > + char *bootphase; > + > + uclass_get_device_by_seq(UCLASS_PCI_EP, 0, &dev); > + if (!dev) { > + pr_err("Failed to get pci ep device\n"); > + return -ENODEV; > + } > + > + hdr.deviceid = CONFIG_SPL_PCI_DFU_DEVICE_ID; > + hdr.vendorid = CONFIG_SPL_PCI_DFU_VENDOR_ID; > + hdr.baseclass_code = PCI_BASE_CLASS_MEMORY; > + hdr.subclass_code = PCI_CLASS_MEMORY_RAM; > + > + ret = pci_ep_write_header(dev, fn, &hdr); > + if (ret) { > + pr_err("Failed to write header: %d\n", ret); > + return ret; > + } > + > + bar.barno = BAR_0; > + bar.phys_addr = (dma_addr_t)CONFIG_SPL_PCI_DFU_SPL_LOAD_FIT_ADDRESS; > + bar.flags = PCI_BASE_ADDRESS_SPACE_MEMORY | > + PCI_BASE_ADDRESS_MEM_TYPE_32 | > + PCI_BASE_ADDRESS_MEM_PREFETCH; > + > + bar.size = CONFIG_SPL_PCI_DFU_BAR_SIZE; > + > + ret = pci_ep_set_bar(dev, fn, &bar); > + if (ret) { > + pr_err("Failed to set bar: %d\n", ret); > + return ret; > + } > + > + ret = pci_ep_start(dev); > + if (ret) { > + pr_err("Failed to start ep: %d\n", ret); > + return ret; > + } > + > + addr = (void *)CONFIG_SPL_PCI_DFU_SPL_LOAD_FIT_ADDRESS; > + offset = CONFIG_SPL_PCI_DFU_BAR_SIZE - MAGIC_WORD_SIZE; > + > + if (sizeof(CONFIG_SPL_PCI_DFU_BOOT_PHASE) > BOOT_PHASE_STRING_SIZE) { > + pr_err("Not copying boot phase. String too long\n"); > + } else { > + bootphase = (char *)(addr + CONFIG_SPL_PCI_DFU_BAR_SIZE - > + (BOOT_PHASE_STRING_SIZE + MAGIC_WORD_SIZE + > 1)); > + strlcpy(bootphase, CONFIG_SPL_PCI_DFU_BOOT_PHASE, > + sizeof(CONFIG_SPL_PCI_DFU_BOOT_PHASE) + 1); > + } > + > + addr = addr + offset; > + magic_word = CONFIG_SPL_PCI_DFU_MAGIC_WORD; > + (*(int *)addr) = 0; > + flush_dcache_all(); > + for (;;) { > + if (*(int *)addr == magic_word) > + break; > + invalidate_dcache_all(); > + } > + > + return 0; > +} > +#endif > + > int spl_dfu_cmd(int usbctrl, char *dfu_alt_info, char *interface, char > *devstr) > { > char *str_env; > int ret; > > +#ifdef CONFIG_SPL_PCI_DFU > + if (spl_boot_device() == BOOT_DEVICE_PCIE) > + return dfu_over_pcie(); > +#endif > + > /* set default environment */ > env_set_default(NULL, 0); > str_env = env_get(dfu_alt_info); > diff --git a/common/spl/spl_ram.c b/common/spl/spl_ram.c > index 71b7a8374bb..0c501cf02f2 100644 > --- a/common/spl/spl_ram.c > +++ b/common/spl/spl_ram.c > @@ -27,6 +27,11 @@ static ulong spl_ram_load_read(struct spl_load_info *load, > ulong sector, > if (IS_ENABLED(CONFIG_SPL_LOAD_FIT)) { > addr = IF_ENABLED_INT(CONFIG_SPL_LOAD_FIT, > CONFIG_SPL_LOAD_FIT_ADDRESS); > + > +#ifdef CONFIG_SPL_PCI_DFU > + if (spl_boot_device() == BOOT_DEVICE_PCIE) > + addr = CONFIG_SPL_PCI_DFU_SPL_LOAD_FIT_ADDRESS; > +#endif Can't we rewrite this using IF_ENABLED_INT() similarly to what has been done for CONFIG_SPL_LOAD_FIT_ADDRESS ? This should allow us to get rid of the #ifdef CONFIG_SPL_PCI_DFU part. > } > addr += sector; > if (CONFIG_IS_ENABLED(IMAGE_PRE_LOAD)) > @@ -47,6 +52,11 @@ static int spl_ram_load_image(struct spl_image_info > *spl_image, > if (IS_ENABLED(CONFIG_SPL_LOAD_FIT)) { > addr = IF_ENABLED_INT(CONFIG_SPL_LOAD_FIT, > CONFIG_SPL_LOAD_FIT_ADDRESS); > + > +#ifdef CONFIG_SPL_PCI_DFU > + if (spl_boot_device() == BOOT_DEVICE_PCIE) > + addr = CONFIG_SPL_PCI_DFU_SPL_LOAD_FIT_ADDRESS; > +#endif Same here > } > > if (CONFIG_IS_ENABLED(IMAGE_PRE_LOAD)) { > @@ -64,6 +74,11 @@ static int spl_ram_load_image(struct spl_image_info > *spl_image, > spl_dfu_cmd(0, "dfu_alt_info_ram", "ram", "0"); > #endif > > +#if CONFIG_IS_ENABLED(PCI_DFU) > + if (bootdev->boot_device == BOOT_DEVICE_PCIE) > + spl_dfu_cmd(0, "dfu_alt_info_ram", "ram", "0"); > +#endif > + Same here. Please rewrite using if (IS_ENABLED(CONFIG_SPL_PCI_DFU)). This way we are consistent with the rest of the file. > if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) && > image_get_magic(header) == FDT_MAGIC) { > struct spl_load_info load; > @@ -102,3 +117,6 @@ SPL_LOAD_IMAGE_METHOD("RAM", 0, BOOT_DEVICE_RAM, > spl_ram_load_image); > #if CONFIG_IS_ENABLED(DFU) > SPL_LOAD_IMAGE_METHOD("DFU", 0, BOOT_DEVICE_DFU, spl_ram_load_image); > #endif > +#if CONFIG_IS_ENABLED(PCI_DFU) > +SPL_LOAD_IMAGE_METHOD("PCIE", 0, BOOT_DEVICE_PCIE, spl_ram_load_image); > +#endif > diff --git a/drivers/Makefile b/drivers/Makefile > index 7560008a842..77fc66eb8ba 100644 > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -26,6 +26,7 @@ obj-$(CONFIG_MULTIPLEXER) += mux/ > obj-$(CONFIG_$(PHASE_)ETH) += net/ > obj-$(CONFIG_$(PHASE_)PCH) += pch/ > obj-$(CONFIG_$(PHASE_)PCI) += pci/ > +obj-$(CONFIG_$(PHASE_)PCI_ENDPOINT) += pci_endpoint/ > obj-$(CONFIG_$(PHASE_)PHY) += phy/ > obj-$(CONFIG_$(PHASE_)PINCTRL) += pinctrl/ > obj-$(CONFIG_$(PHASE_)POWER) += power/ > -- > 2.34.1
