[PATCH] Revert "cmd: pxe_utils: Check fdtcontroladdr in label_boot"

With the change here, all extlinux.conf files with only "KERNEL /fitImage" don't work anymore. One common example of this would be those files generated by thee Poky/OE WIC bootimg-partition bootloader partition generator.
This reverts commit d5ba6188dfbf6bb68354bec86e483623f1f6dae2.
Reported-by: Neil Armstrong neil.armstrong@linaro.org Reported-by: Quentin Schulz quentin.schulz@theobroma-systems.com Signed-off-by: Tom Rini trini@konsulko.com --- boot/pxe_utils.c | 8 +------- drivers/net/tsec.c | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 8133006875f9..96528aa14c03 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -616,10 +616,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) * Scenario 2: If there is an fdt_addr specified, pass it along to * bootm, and adjust argc appropriately. * - * Scenario 3: If there is an fdtcontroladdr specified, pass it along to - * bootm, and adjust argc appropriately. - * - * Scenario 4: fdt blob is not available. + * Scenario 3: fdt blob is not available. */ bootm_argv[3] = env_get("fdt_addr_r");
@@ -724,9 +721,6 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (!bootm_argv[3]) bootm_argv[3] = env_get("fdt_addr");
- if (!bootm_argv[3]) - bootm_argv[3] = env_get("fdtcontroladdr"); - if (bootm_argv[3]) { if (!bootm_argv[2]) bootm_argv[2] = "-"; diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c index d69a9ff47736..519ea14b070e 100644 --- a/drivers/net/tsec.c +++ b/drivers/net/tsec.c @@ -156,7 +156,7 @@ static int tsec_mcast_addr(struct udevice *dev, const u8 *mcast_mac, int join) return 0; }
-static int __maybe_unused tsec_set_promisc(struct udevice *dev, bool enable) +static int tsec_set_promisc(struct udevice *dev, bool enable) { struct tsec_private *priv = dev_get_priv(dev); struct tsec __iomem *regs = priv->regs;

Hi Tom,
On 12/13/22 15:29, Tom Rini wrote:
With the change here, all extlinux.conf files with only "KERNEL /fitImage" don't work anymore. One common example of this would be those files generated by thee Poky/OE WIC bootimg-partition bootloader partition generator.
This reverts commit d5ba6188dfbf6bb68354bec86e483623f1f6dae2.
FYI, this patch has been in U-Boot for three releases already, hence why I was reluctant to ask for a revert when I discovered the issue (that Neil already had reported in August, well before me).
Adding people who worked on the reverted patch in Cc so they can give their 2¢ on it.
Cheers, Quentin
Reported-by: Neil Armstrong neil.armstrong@linaro.org Reported-by: Quentin Schulz quentin.schulz@theobroma-systems.com Signed-off-by: Tom Rini trini@konsulko.com
boot/pxe_utils.c | 8 +------- drivers/net/tsec.c | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 8133006875f9..96528aa14c03 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -616,10 +616,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) * Scenario 2: If there is an fdt_addr specified, pass it along to * bootm, and adjust argc appropriately. *
* Scenario 3: If there is an fdtcontroladdr specified, pass it along to
* bootm, and adjust argc appropriately.
*
* Scenario 4: fdt blob is not available.
*/ bootm_argv[3] = env_get("fdt_addr_r");* Scenario 3: fdt blob is not available.
@@ -724,9 +721,6 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (!bootm_argv[3]) bootm_argv[3] = env_get("fdt_addr");
- if (!bootm_argv[3])
bootm_argv[3] = env_get("fdtcontroladdr");
- if (bootm_argv[3]) { if (!bootm_argv[2]) bootm_argv[2] = "-";
diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c index d69a9ff47736..519ea14b070e 100644 --- a/drivers/net/tsec.c +++ b/drivers/net/tsec.c @@ -156,7 +156,7 @@ static int tsec_mcast_addr(struct udevice *dev, const u8 *mcast_mac, int join) return 0; }
-static int __maybe_unused tsec_set_promisc(struct udevice *dev, bool enable) +static int tsec_set_promisc(struct udevice *dev, bool enable) { struct tsec_private *priv = dev_get_priv(dev); struct tsec __iomem *regs = priv->regs;

On Tue, Dec 13, 2022 at 03:34:45PM +0100, Quentin Schulz wrote:
Hi Tom,
On 12/13/22 15:29, Tom Rini wrote:
With the change here, all extlinux.conf files with only "KERNEL /fitImage" don't work anymore. One common example of this would be those files generated by thee Poky/OE WIC bootimg-partition bootloader partition generator.
This reverts commit d5ba6188dfbf6bb68354bec86e483623f1f6dae2.
FYI, this patch has been in U-Boot for three releases already, hence why I was reluctant to ask for a revert when I discovered the issue (that Neil already had reported in August, well before me).
Adding people who worked on the reverted patch in Cc so they can give their 2¢ on it.
Sigh, I misreaad what git describe showed me as 202_3_.01 not 202_2_.01.

On 13/12/2022 14:34, Quentin Schulz wrote:
Hi Tom,
On 12/13/22 15:29, Tom Rini wrote:
With the change here, all extlinux.conf files with only "KERNEL /fitImage" don't work anymore. One common example of this would be those files generated by thee Poky/OE WIC bootimg-partition bootloader partition generator.
This reverts commit d5ba6188dfbf6bb68354bec86e483623f1f6dae2.
FYI, this patch has been in U-Boot for three releases already, hence why I was reluctant to ask for a revert when I discovered the issue (that Neil already had reported in August, well before me).
Adding people who worked on the reverted patch in Cc so they can give their 2¢ on it.
Sending a quick reply (as the original author of this patch) as I likely won't have bandwidth to investigate this properly until 2023.
The use case here (PXE with fdtcontroladdr) is still occasionally useful for us as we don't use FIT images but instead bundle U-Boot and the device tree in a prior boot stage, typically Trusted Firmware-A. As PXE is not regularly used in our team though, I think we'd be open to this revert in the short term so long as we can agree on a better design to be implemented in the medium term.
We are also active Poky/OE users here, but I think this more about supporting different options for device tree storage.
Peter
Cheers, Quentin
Reported-by: Neil Armstrong neil.armstrong@linaro.org Reported-by: Quentin Schulz quentin.schulz@theobroma-systems.com Signed-off-by: Tom Rini trini@konsulko.com
boot/pxe_utils.c | 8 +------- drivers/net/tsec.c | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 8133006875f9..96528aa14c03 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -616,10 +616,7 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) * Scenario 2: If there is an fdt_addr specified, pass it along to * bootm, and adjust argc appropriately. * - * Scenario 3: If there is an fdtcontroladdr specified, pass it along to - * bootm, and adjust argc appropriately. - * - * Scenario 4: fdt blob is not available. + * Scenario 3: fdt blob is not available. */ bootm_argv[3] = env_get("fdt_addr_r"); @@ -724,9 +721,6 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label) if (!bootm_argv[3]) bootm_argv[3] = env_get("fdt_addr"); - if (!bootm_argv[3]) - bootm_argv[3] = env_get("fdtcontroladdr");
if (bootm_argv[3]) { if (!bootm_argv[2]) bootm_argv[2] = "-"; diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c index d69a9ff47736..519ea14b070e 100644 --- a/drivers/net/tsec.c +++ b/drivers/net/tsec.c @@ -156,7 +156,7 @@ static int tsec_mcast_addr(struct udevice *dev, const u8 *mcast_mac, int join) return 0; } -static int __maybe_unused tsec_set_promisc(struct udevice *dev, bool enable) +static int tsec_set_promisc(struct udevice *dev, bool enable) { struct tsec_private *priv = dev_get_priv(dev); struct tsec __iomem *regs = priv->regs;
participants (3)
-
Peter Hoyes
-
Quentin Schulz
-
Tom Rini