Hi Hrushikesh, On Mon, Nov 03, 2025 at 11:10, Hrushikesh Salunke <[email protected]> wrote:
> 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 I see. Sorry I missed that. Reviewed-by: Mattijs Korpershoek <[email protected]> I'll pick this up shortly to try to make it for v2026.01 > > 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
