Ambarella upstream #73
Conversation
phodina
force-pushed
the
ambarella-upstream
branch
from
d9c7d51 to
0887d7d
Compare
docs/fw_binaries.md
Outdated
|
|
||
| [example](../src/snagrecover/templates/ambarella-cv22.yaml) | ||
|
|
||
| Ambarella SDK requires NDA in order to build the Amboot which is proprietary |
There was a problem hiding this comment.
which is a proprietary bootloader
There was a problem hiding this comment.
So there's these file that after the DRAM is initialized are loaded and facilitate the next stage in the bootloader process:
find ./usr/share/ambausb-4.3.2/platform -iname "*.elf"
./usr/share/ambausb-4.3.2/platform/s5l/bld/s5lm_bld_nand.elf
./usr/share/ambausb-4.3.2/platform/s5l/bld/s5l_bld_emmc.elf
./usr/share/ambausb-4.3.2/platform/s5l/bld/s5l_bld_nand.elf
./usr/share/ambausb-4.3.2/platform/s5l/bld/s5l_4Gb_bld_emmc.elf
./usr/share/ambausb-4.3.2/platform/s5l/bld/s5lm_bld_spinor.elf
./usr/share/ambausb-4.3.2/platform/s5l/bld/s5lm_bld_emmc.elf
./usr/share/ambausb-4.3.2/platform/s5l/bld/s5lm_bld_spinor_spansion.elf
./usr/share/ambausb-4.3.2/platform/s5l/bld/s5l_4Gb_bld_nand.elf
./usr/share/ambausb-4.3.2/platform/cv25/bld/cv25_bld_linux_emmc.elf
./usr/share/ambausb-4.3.2/platform/cv25/bld/cv25_bld_linux_stack.elf
./usr/share/ambausb-4.3.2/platform/cv25/bld/cv25_bld_rtos_emmc.elf
./usr/share/ambausb-4.3.2/platform/cv25/bld/cv25_bld_rtos_nand.elf
./usr/share/ambausb-4.3.2/platform/cv25/bld/cv25_bld_linux_nand.elf
./usr/share/ambausb-4.3.2/platform/cv2fs/bld/cv2fs_bld_rtos_emmc.elf
./usr/share/ambausb-4.3.2/platform/cv2fs/bld/cv2fs_bld_linux_nand.elf
./usr/share/ambausb-4.3.2/platform/cv28/bld/cv28_bld_linux_nand.elf
./usr/share/ambausb-4.3.2/platform/cv28/bld/cv28_bld_linux_emmc.elf
./usr/share/ambausb-4.3.2/platform/s5/bld/s5_bld_emmc.elf
./usr/share/ambausb-4.3.2/platform/s5/bld/s5_bld_nand.elf
./usr/share/ambausb-4.3.2/platform/cv22/bld/cv22_bld_linux_nand.elf
./usr/share/ambausb-4.3.2/platform/cv22/bld/cv22_bld_linux_stack.elf
./usr/share/ambausb-4.3.2/platform/cv22/bld/cv22_bld_linux_emmc.elf
./usr/share/ambausb-4.3.2/platform/cv2/bld/cv2_bld_linux_emmc.elf
./usr/share/ambausb-4.3.2/platform/cv2/bld/cv2_bld_linux_nand.elf
./usr/share/ambausb-4.3.2/platform/s3l/bld/s3lm_bld_nand.elf
./usr/share/ambausb-4.3.2/platform/s3l/bld/s3l_bld_spinor.elf
./usr/share/ambausb-4.3.2/platform/s3l/bld/s3l_bld_emmc.elf
./usr/share/ambausb-4.3.2/platform/s3l/bld/s3l_bld_nand.elf
./usr/share/ambausb-4.3.2/platform/s3l/bld/s3lm_bld_emmc.elf
./usr/share/ambausb-4.3.2/platform/s3l/bld/s3lm_bld_spinor.elf
./usr/share/ambausb-4.3.2/platform/s3l/bld/s3lm_bld_spinor_spansion.elf
./usr/share/ambausb-4.3.2/platform/s3l/bld/s3l_bld_spinor_spansion.elf
./usr/share/ambausb-4.3.2/platform/cv1/bld/cv1_bld_linux_emmc.elf
./usr/share/ambausb-4.3.2/platform/cv1/bld/cv1_bld_linux_nand.elf
./usr/share/ambausb-4.3.2/platform/s2l/bld/s2lm_bld_nand.elf
./usr/share/ambausb-4.3.2/platform/s2l/bld/s2l_bld_spinor_spansion.elf
./usr/share/ambausb-4.3.2/platform/s2l/bld/s2l_bld_nand.elf
./usr/share/ambausb-4.3.2/platform/s2l/bld/s2l_bld_spinor.elf
./usr/share/ambausb-4.3.2/platform/s2l/bld/s2l_bld_emmc.elf
./usr/share/ambausb-4.3.2/platform/s2l/bld/s2lm_bld_spinor.elf
./usr/share/ambausb-4.3.2/platform/s2l/bld/s2lm_bld_spinor_spansion.elf
./usr/share/ambausb-4.3.2/platform/s2l/bld/s2lm_bld_emmc.elf
./usr/share/ambausb-4.3.2/platform/s6lm/bld/s6lm_bld_nand.elf
./usr/share/ambausb-4.3.2/platform/s6lm/bld/s6lm_bld_stack.elf
./usr/share/ambausb-4.3.2/platform/s6lm/bld/s6lm_bld_emmc.elf
There was a problem hiding this comment.
The ELF is preconfigured for the platform and the nonvolatile storage - nand, emmc or spinor. It also sets up the USB device.
Sure user could provide their own implementation.
There was a problem hiding this comment.
They provide a patch for mainline kernel. This available publicly online and probably could be reimplemented.
https://github.com/adipierro/ambarella/blob/master/ambarella/kernel/linux-3.10-ambarella.patch
phodina
force-pushed
the
ambarella-upstream
branch
from
0887d7d to
36c9634
Compare
|
Also not sure about the ADS files as they are crucial to setup the DRAM: I mean if they could be shared or the user needs to install the tool from vendor and use them from the disc. |
|
Hi, thanks a lot for the big contrib :) I'm a bit short on time right now but I'll review this as soon as I can. |
|
@rgantois Sure, no problem. |
phodina
force-pushed
the
ambarella-upstream
branch
3 times, most recently
from
15a3961 to
70e1964
Compare
phodina
force-pushed
the
ambarella-upstream
branch
from
70e1964 to
59d68e0
Compare
Implements the Ambarella USB protocol handler with: - Command/response packet handling - File transfer support - Device info queries - Error handling
Implements firmware handling for Ambarella devices: - Bootloader and DRAM script loading - Firmware info extraction - Board/firmware info structure packing - Error handling
Implements recovery flow for Ambarella devices: - ADS script parser for DRAM configuration - DRAM initialization support - Bootloader loading and execution - Firmware flashing with proper addressing - Complete recovery process handling
Adds recovery configuration template for CV22: - Device identification (VID/PID) - Required firmware files - Recovery steps and flow - Documentation and notes
phodina
force-pushed
the
ambarella-upstream
branch
from
59d68e0 to
6a0fc53
Compare
|
@rgantois rebased to solve the conflicts and reformatted the code to pass all the checks |
| 2. The USB cable is connected to the correct port (usually the OTG port) | ||
| 3. The board is powered on | ||
|
|
||
| ### DRAM initialization fails |
There was a problem hiding this comment.
Same comment as above, this applies to every board which relies on closed-source firmware binaries.
| family: sunxi | ||
| v853: | ||
| family: sunxi | ||
| cv22: |
There was a problem hiding this comment.
SoC models should only appear once in supported_socs.yaml, either in the "tested" section or the "untested" one. This one appears in both.
There was a problem hiding this comment.
Okay will keep the cv22 under tested, rest untested as I don't have the HW.
|
|
||
| zynqmp_run(port, fw_name, fw_blob, subfw_name) | ||
| elif soc_family == "bcm": | ||
| from snagrecover.firmware.bcm import bcm_run |
There was a problem hiding this comment.
commit-wide comment: the commit history of Snagboot should be bisectable as much as possible, meaning each commit should allow Snagboot to run without errors.
In this commit, you're adding references to modules which don't exist yet.
I'd prefer if you reordered the commits so that each one is valid Python. So in this specific case, the commits adding the protocol and firmware code should come before this one.
There was a problem hiding this comment.
Okay, will rearrange the code so it's self contained and does not require any future code.
| pid: 0xc022 # CV22 product ID | ||
|
|
||
| # Recovery protocol | ||
| protocol: amba |
There was a problem hiding this comment.
this is information about some internal details of the snagrecover code, it shouldn't be included in the user interface.
| # DRAM initialization script | ||
| dram_script: | ||
| path: cv22_lpddr4_408MHz.ads | ||
| description: DRAM initialization script for CV22 LPDDR4 at 408MHz |
There was a problem hiding this comment.
It's nice to have a description here, but it's also repetitive, as this information is already given in the fw_binaries docs. I don't have a definitive opinion about this yet.
|
|
||
| # Recovery steps | ||
| steps: | ||
| - name: Initialize DRAM |
There was a problem hiding this comment.
The problem with this is that it's documentation for how snagrecover works internally. So if the recovery process or the nomenclature evolves at some point, we'll have to keep this template up to date as well.
Moreover, the user ideally shouldn't have to worry about these details.
thanks for review, will refactor the comments |
No description provided.