Hi Neil,
Thank you for the patch.
On mer., oct. 16, 2024 at 17:46, Neil Armstrong <neil.armstr...@linaro.org>
wrote:
> The two tools that create android boot images, mkbootimg and the fastboot
> client, set the kernel address by default to 0x11008000.
>
> U-boot always honors this field, and will try to copy the ramdisk to
> whatever value is set in the header, which won't be mapped to the actual RAM
> on
> most platforms, resulting in the kernel obviously not booting.
>
> All the targets in U-Boot right now will download the android boot image to
> CONFIG_SYS_LOAD_ADDR, which means that it will already have been downloaded to
> some location that is suitable to use the ramdisk in-place for header
> version 0 to 2. For header version 3 and later, the ramdisk can't be
> used in-place to use ramdisk_addr_r in this case.
Is said in [1] I found some boot issues on Beagle Play with boot image
v4.
If a new series need to be send, please also adapt the commit message a
bit as it fails checkpatch:
$ ~/work/amlogic/u-boot/ neil/android-fixes ./scripts/checkpatch.pl --strict
--u-boot --git HEAD^..HEAD
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per
line)
#10:
whatever value is set in the header, which won't be mapped to the actual RAM on
total: 0 errors, 1 warnings, 0 checks, 59 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Commit 19a75a41096f ("image: android: handle ramdisk default address") has
style problems, please review.
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO ENOSYS MINMAX
MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE
PREFER_ETHER_ADDR_COPY USLEEP_RANGE
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
[1] https://lore.kernel.org/r/all/87ed4f2ccc....@baylibre.com/
>
> Signed-off-by: Neil Armstrong <neil.armstr...@linaro.org>
> ---
> boot/image-android.c | 36 +++++++++++++++++++++++++++++-------
> 1 file changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/boot/image-android.c b/boot/image-android.c
> index 3adcc69a392..a261bb63999 100644
> --- a/boot/image-android.c
> +++ b/boot/image-android.c
> @@ -14,6 +14,7 @@
> #include <linux/libfdt.h>
>
> #define ANDROID_IMAGE_DEFAULT_KERNEL_ADDR 0x10008000
> +#define ANDROID_IMAGE_DEFAULT_RAMDISK_ADDR 0x11000000
>
> static char andr_tmp_str[ANDR_BOOT_ARGS_SIZE + 1];
>
> @@ -405,9 +406,24 @@ int android_image_get_ramdisk(const void *hdr, const
> void *vendor_boot_img,
>
> if (!img_data.ramdisk_size)
> return -ENOENT;
> -
> + /*
> + * Android tools can generate a boot.img with default load address
> + * or 0, even though it doesn't really make a lot of sense, and it
> + * might be valid on some platforms, we treat that address as
> + * the default value for this field, and try to pass ramdisk
> + * in place if possible.
> + */
> if (img_data.header_version > 2) {
> - ramdisk_ptr = img_data.ramdisk_addr;
> + /* Ramdisk can't be used in-place, copy it to ramdisk_addr_r */
> + if (img_data.ramdisk_addr ==
> ANDROID_IMAGE_DEFAULT_RAMDISK_ADDR) {
> + ramdisk_ptr = env_get_ulong("ramdisk_addr_r", 16, 0);
> + if (!ramdisk_ptr) {
> + printf("Invalid ramdisk_addr_r to copy ramdisk
> into\n");
> + return -EINVAL;
> + }
> + } else {
> + ramdisk_ptr = img_data.ramdisk_addr;
> + }
> memcpy((void *)(ramdisk_ptr), (void
> *)img_data.vendor_ramdisk_ptr,
> img_data.vendor_ramdisk_size);
> ramdisk_ptr += img_data.vendor_ramdisk_size;
> @@ -420,15 +436,21 @@ int android_image_get_ramdisk(const void *hdr, const
> void *vendor_boot_img,
> img_data.bootconfig_size);
> }
> } else {
> - ramdisk_ptr = img_data.ramdisk_addr;
> - memcpy((void *)(ramdisk_ptr), (void *)img_data.ramdisk_ptr,
> - img_data.ramdisk_size);
> + /* Ramdisk can be used in-place, use current ptr */
> + if (img_data.ramdisk_addr == 0 ||
> + img_data.ramdisk_addr ==
> ANDROID_IMAGE_DEFAULT_RAMDISK_ADDR) {
> + ramdisk_ptr = img_data.ramdisk_ptr;
> + } else {
> + ramdisk_ptr = img_data.ramdisk_addr;
> + memcpy((void *)(ramdisk_ptr), (void
> *)img_data.ramdisk_ptr,
> + img_data.ramdisk_size);
> + }
> }
>
> printf("RAM disk load addr 0x%08lx size %u KiB\n",
> - img_data.ramdisk_addr, DIV_ROUND_UP(img_data.ramdisk_size,
> 1024));
> + ramdisk_ptr, DIV_ROUND_UP(img_data.ramdisk_size, 1024));
>
> - *rd_data = img_data.ramdisk_addr;
> + *rd_data = ramdisk_ptr;
>
> *rd_len = img_data.ramdisk_size;
> return 0;
>
> --
> 2.34.1
