U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] cmd: fastboot: default the buffer address to $fastboot_addr_r
@ 2026-11-02 22:08 Igor Opaniuk
  2026-11-03  4:16 ` E Shattow
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Igor Opaniuk @ 2026-11-02 22:08 UTC (permalink / raw)
  To: Mattijs Korpershoek, u-boot
  Cc: Tom Rini, Casey Connolly, Sumit Garg, Neil Armstrong,
	Kaustabh Chakraborty, u-boot-qcom, E Shattow, Igor Opaniuk

The fastboot command downloads to CONFIG_FASTBOOT_BUF_ADDR unless -l is
given. Platforms that only know a usable address at run time leave that
at 0 and put the address in the fastboot_addr_r environment variable:
mach-snapdragon allocates the buffer from free memory, and so does the
Samsung exynos-mobile board. Their environments wrap the command in
"fastboot -l $fastboot_addr_r usb 0", but a plain "fastboot usb 0"
still downloads to address 0.

Use fastboot_addr_r when it is set. -l still overrides it, and boards
without the variable keep using CONFIG_FASTBOOT_BUF_ADDR.

Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
---
Tested on a Qualcomm QCM2290 board with qcom_defconfig: before, the
first "fastboot stage" after a plain "fastboot usb 0" crashed U-Boot.
With the patch a 100 MiB download lands at $fastboot_addr_r (CRC32 of
the buffer matches the file), and "fastboot -l ${ramdisk_addr_r} usb 0"
still places it at the given address.

Refusing to start with a zero address instead was not an option:
sandbox uses address 0 as its buffer.
---
Changes in v2:
- Leave the command help text as it was: it describes what the user
  can pass, not how the default is chosen (E Shattow)
- Reword the fastboot_addr_r paragraph in doc/android/fastboot.rst
  (E Shattow)
- Link to v1: https://lore.kernel.org/r/20261102-fastboot-addr-r-v1-1-7ef1a2ac8fd1@oss.qualcomm.com
---
 cmd/fastboot.c           | 4 +++-
 doc/android/fastboot.rst | 8 +++++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/cmd/fastboot.c b/cmd/fastboot.c
index ace877b6e28..795c3138cbb 100644
--- a/cmd/fastboot.c
+++ b/cmd/fastboot.c
@@ -7,6 +7,7 @@
  * Rob Herring <robh@kernel.org>
  */
 #include <command.h>
+#include <env.h>
 #include <fastboot.h>
 #include <net.h>
 #include <vsprintf.h>
@@ -88,7 +89,8 @@ static int do_fastboot_usb(int argc, char *const argv[],
 static int do_fastboot(struct cmd_tbl *cmdtp, int flag, int argc,
 		       char *const argv[])
 {
-	uintptr_t buf_addr = (uintptr_t)NULL;
+	/* Boards which place the buffer at run time put it in fastboot_addr_r */
+	uintptr_t buf_addr = env_get_hex("fastboot_addr_r", 0);
 	size_t buf_size = 0;
 
 	if (argc < 2)
diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst
index 8e15f287f9b..ba817e8c1eb 100644
--- a/doc/android/fastboot.rst
+++ b/doc/android/fastboot.rst
@@ -68,9 +68,11 @@ General configuration
 The fastboot protocol requires a large memory buffer for
 downloads. This buffer should be as large as possible for a
 platform. The location of the buffer and size are set with
-``CONFIG_FASTBOOT_BUF_ADDR`` and ``CONFIG_FASTBOOT_BUF_SIZE``. These
-may be overridden on the fastboot command line using ``-l`` and
-``-s``.
+``CONFIG_FASTBOOT_BUF_ADDR`` and ``CONFIG_FASTBOOT_BUF_SIZE``. If the
+``fastboot_addr_r`` environment variable is set, its value is used as
+the buffer address instead of ``CONFIG_FASTBOOT_BUF_ADDR``, which lets
+a platform choose the address at run time. The address and size may be
+overridden on the fastboot command line using ``-l`` and ``-s``.
 
 Fastboot in SPL
 ^^^^^^^^^^^^^^^

---
base-commit: 3973f3c4d24df75ca90d7c0a9f03e5e5c9b48f32
change-id: 20261102-fastboot-addr-r-28fe6ea5615f

Best regards,
-- 
Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] cmd: fastboot: default the buffer address to $fastboot_addr_r
  2026-11-02 22:08 [PATCH v2] cmd: fastboot: default the buffer address to $fastboot_addr_r Igor Opaniuk
@ 2026-11-03  4:16 ` E Shattow
  2026-11-03 22:03 ` Guillaume La Roque
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: E Shattow @ 2026-11-03  4:16 UTC (permalink / raw)
  To: Igor Opaniuk, Mattijs Korpershoek, u-boot
  Cc: Tom Rini, Casey Connolly, Sumit Garg, Neil Armstrong,
	Kaustabh Chakraborty, u-boot-qcom

Hi Igor,

On 11/2/26 14:08, Igor Opaniuk wrote:
> The fastboot command downloads to CONFIG_FASTBOOT_BUF_ADDR unless -l is
> given. Platforms that only know a usable address at run time leave that
> at 0 and put the address in the fastboot_addr_r environment variable:
> mach-snapdragon allocates the buffer from free memory, and so does the
> Samsung exynos-mobile board. Their environments wrap the command in
> "fastboot -l $fastboot_addr_r usb 0", but a plain "fastboot usb 0"
> still downloads to address 0.
> 
> Use fastboot_addr_r when it is set. -l still overrides it, and boards
> without the variable keep using CONFIG_FASTBOOT_BUF_ADDR.
> 
> Signed-off-by: Igor Opaniuk <igor.opaniuk@oss.qualcomm.com>
> ---
> Tested on a Qualcomm QCM2290 board with qcom_defconfig: before, the
> first "fastboot stage" after a plain "fastboot usb 0" crashed U-Boot.
> With the patch a 100 MiB download lands at $fastboot_addr_r (CRC32 of
> the buffer matches the file), and "fastboot -l ${ramdisk_addr_r} usb 0"
> still places it at the given address.
> 
> Refusing to start with a zero address instead was not an option:
> sandbox uses address 0 as its buffer.
> ---
> Changes in v2:
> - Leave the command help text as it was: it describes what the user
>   can pass, not how the default is chosen (E Shattow)
> - Reword the fastboot_addr_r paragraph in doc/android/fastboot.rst
>   (E Shattow)
> - Link to v1: https://lore.kernel.org/r/20261102-fastboot-addr-r-v1-1-7ef1a2ac8fd1@oss.qualcomm.com
> ---
>  cmd/fastboot.c           | 4 +++-
>  doc/android/fastboot.rst | 8 +++++---
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/cmd/fastboot.c b/cmd/fastboot.c
> index ace877b6e28..795c3138cbb 100644
> --- a/cmd/fastboot.c
> +++ b/cmd/fastboot.c
> @@ -7,6 +7,7 @@
>   * Rob Herring <robh@kernel.org>
>   */
>  #include <command.h>
> +#include <env.h>
>  #include <fastboot.h>
>  #include <net.h>
>  #include <vsprintf.h>
> @@ -88,7 +89,8 @@ static int do_fastboot_usb(int argc, char *const argv[],
>  static int do_fastboot(struct cmd_tbl *cmdtp, int flag, int argc,
>  		       char *const argv[])
>  {
> -	uintptr_t buf_addr = (uintptr_t)NULL;
> +	/* Boards which place the buffer at run time put it in fastboot_addr_r */
> +	uintptr_t buf_addr = env_get_hex("fastboot_addr_r", 0);
>  	size_t buf_size = 0;
>  
>  	if (argc < 2)
> diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst
> index 8e15f287f9b..ba817e8c1eb 100644
> --- a/doc/android/fastboot.rst
> +++ b/doc/android/fastboot.rst
> @@ -68,9 +68,11 @@ General configuration
>  The fastboot protocol requires a large memory buffer for
>  downloads. This buffer should be as large as possible for a
>  platform. The location of the buffer and size are set with
> -``CONFIG_FASTBOOT_BUF_ADDR`` and ``CONFIG_FASTBOOT_BUF_SIZE``. These
> -may be overridden on the fastboot command line using ``-l`` and
> -``-s``.
> +``CONFIG_FASTBOOT_BUF_ADDR`` and ``CONFIG_FASTBOOT_BUF_SIZE``. If the
> +``fastboot_addr_r`` environment variable is set, its value is used as
> +the buffer address instead of ``CONFIG_FASTBOOT_BUF_ADDR``, which lets
> +a platform choose the address at run time. The address and size may be
> +overridden on the fastboot command line using ``-l`` and ``-s``.

It looks better than it was, and now I do understand what you are
writing. There are three-or-four sources of information here into the logic:

1. Build time

2. Strictly U-Boot environment

3. Fastboot server/client requests  (and here, does the client request
override the server command line? so... potentially split to #3 and #4
sources of information here into the logic)

Does each source described after the previous source implicitly override
the previous source?

Is there a functional reason we did not mention both address and size
configurations for all of the above sources (notably no mention of
fastboot buffer size override as environment variable)?

The word ordering flows more naturally if it is "here is a stack of two
related things A1 and B1, now we override those with A2 and B2, and
again override with A3 and B3, and finally A4 and B4." If it's more
complex than this perhaps we should just dump this into a formatted
table and not bother describing it more than "Order of precedence" table
name. If it is not so complex then it should be said in one or two
sentences that simply list the order of precedence.

>  
>  Fastboot in SPL
>  ^^^^^^^^^^^^^^^
> 
> ---
> base-commit: 3973f3c4d24df75ca90d7c0a9f03e5e5c9b48f32
> change-id: 20261102-fastboot-addr-r-28fe6ea5615f
> 
> Best regards,

I do not want to go far beyond what you're trying to fix in this one
patch but it does open more questions for me now that I understand it.
Let me know what you think, thanks!

-E

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] cmd: fastboot: default the buffer address to $fastboot_addr_r
  2026-11-02 22:08 [PATCH v2] cmd: fastboot: default the buffer address to $fastboot_addr_r Igor Opaniuk
  2026-11-03  4:16 ` E Shattow
@ 2026-11-03 22:03 ` Guillaume La Roque
  2026-11-06 13:48 ` Mattijs Korpershoek
  2026-11-13 16:01 ` Mattijs Korpershoek
  3 siblings, 0 replies; 5+ messages in thread
From: Guillaume La Roque @ 2026-11-03 22:03 UTC (permalink / raw)
  To: Igor Opaniuk
  Cc: Mattijs Korpershoek, u-boot, Tom Rini, Casey Connolly, Sumit Garg,
	Neil Armstrong, Kaustabh Chakraborty, u-boot-qcom, E Shattow

On Mon, 02 Nov 2026 23:08:00 +0100, Igor Opaniuk <igor.opaniuk@oss.qualcomm.com> wrote:
> cmd: fastboot: default the buffer address to $fastboot_addr_r

Reviewed-by: Guillaume La Roque <glaroque@baylibre.com>

-- 
Guillaume La Roque <glaroque@baylibre.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] cmd: fastboot: default the buffer address to $fastboot_addr_r
  2026-11-02 22:08 [PATCH v2] cmd: fastboot: default the buffer address to $fastboot_addr_r Igor Opaniuk
  2026-11-03  4:16 ` E Shattow
  2026-11-03 22:03 ` Guillaume La Roque
@ 2026-11-06 13:48 ` Mattijs Korpershoek
  2026-11-13 16:01 ` Mattijs Korpershoek
  3 siblings, 0 replies; 5+ messages in thread
From: Mattijs Korpershoek @ 2026-11-06 13:48 UTC (permalink / raw)
  To: Igor Opaniuk
  Cc: Mattijs Korpershoek, u-boot, Tom Rini, Casey Connolly, Sumit Garg,
	Neil Armstrong, Kaustabh Chakraborty, u-boot-qcom, E Shattow

On Mon, 02 Nov 2026 23:08:00 +0100, Igor Opaniuk <igor.opaniuk@oss.qualcomm.com> wrote:
> The fastboot command downloads to CONFIG_FASTBOOT_BUF_ADDR unless -l is
> given. Platforms that only know a usable address at run time leave that
> at 0 and put the address in the fastboot_addr_r environment variable:
> mach-snapdragon allocates the buffer from free memory, and so does the
> Samsung exynos-mobile board. Their environments wrap the command in
> "fastboot -l $fastboot_addr_r usb 0", but a plain "fastboot usb 0"
> still downloads to address 0.
> 
> [...]

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

-- 
Mattijs Korpershoek <mkorpershoek@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] cmd: fastboot: default the buffer address to $fastboot_addr_r
  2026-11-02 22:08 [PATCH v2] cmd: fastboot: default the buffer address to $fastboot_addr_r Igor Opaniuk
                   ` (2 preceding siblings ...)
  2026-11-06 13:48 ` Mattijs Korpershoek
@ 2026-11-13 16:01 ` Mattijs Korpershoek
  3 siblings, 0 replies; 5+ messages in thread
From: Mattijs Korpershoek @ 2026-11-13 16:01 UTC (permalink / raw)
  To: u-boot, Igor Opaniuk
  Cc: Tom Rini, Casey Connolly, Sumit Garg, Neil Armstrong,
	Kaustabh Chakraborty, u-boot-qcom, E Shattow

Hi,

On Mon, 02 Nov 2026 23:08:00 +0100, Igor Opaniuk wrote:
> cmd: fastboot: default the buffer address to $fastboot_addr_r

Thanks, Applied to https://git.u-boot-project.org/u-boot/custodians/u-boot-dfu (u-boot-dfu-next)

[1/1] cmd: fastboot: default the buffer address to $fastboot_addr_r
      https://git.u-boot-project.org/u-boot/custodians/u-boot-dfu/-/commit/ea06563e7b647ef124a9b89450855950c9b569e9

--
Mattijs

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-13 16:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-02 22:08 [PATCH v2] cmd: fastboot: default the buffer address to $fastboot_addr_r Igor Opaniuk
2026-11-03  4:16 ` E Shattow
2026-11-03 22:03 ` Guillaume La Roque
2026-11-06 13:48 ` Mattijs Korpershoek
2026-11-13 16:01 ` Mattijs Korpershoek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox