On 31/10/25 20:22, Mattijs Korpershoek wrote:
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
[...]
+#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.
Thank you for the feedback. However, using IS_ENABLED() instead of
#ifdef in these locations causes compilation errors on platforms where
BOOT_DEVICE_PCIE is not defined. The BOOT_DEVICE_PCIE macro is only
defined in platform-specific headers, and is not available universally
across all architectures. With IS_ENABLED(CONFIG_SPL_PCI_DFU), the code
inside the conditional is still visible to the compiler even when the
config is disabled. This means the compiler will try to evaluate
BOOT_DEVICE_PCIE on platforms where it doesn't exist, resulting in a
compilation error like:
error: 'BOOT_DEVICE_PCIE' undeclared
The #ifdef CONFIG_SPL_PCI_DFU approach ensures that the code referencing
BOOT_DEVICE_PCIE is completely removed during preprocessing on platforms
that don't support PCIe DFU, preventing the compilation error. This is
similar to how other boot device checks are handled in U-Boot when the
boot device macro may not be universally defined across all platforms.
Regards,
Hrushikesh.
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
