Rockchip SoC support #56
Conversation
|
That is a very interesting contribution :) thanks a lot! I'll have to take some time to review it thoroughly, but at first glance the overall code organization looks good to me. I'll see if I can get it to work on RK3308 since I have a rockpis lying around. |
|
fyi, this u-boot series may be interesting: https://patchwork.ozlabs.org/project/uboot/list/?series=445397 as, maybe, we can avoid having to download u-boot proper with DFU. I've yet to test it and check it's working as I understand. |
|
fwiw, tested this series, and managed to boot u-boot proper without DFU on rk3399 and rk3588. On rk3588, I've had to disable rc4 encryption so I've updated the PR accordingly |
|
Hi, I'm starting to review this and there's one thing I've noticed so far: your series is missing documentation on the images which need to be passed to snagrecover. Every SoC family needs a section in docs/fw_binaries.md which describes which firmware binaries are expected by snagrecover and what these binaries contain. The README will also have to be updated to specify that Rockchip SoCs are supported. I'll come back with more details once I've had time to test and review this in depth. |
There was a problem hiding this comment.
This looks pretty good! Apart for the comments in this review, there's also the point about docs/fw_binaries.md that I mentionned before.
I've confirmed that the first stage part works with a RockPiS board but unfortunately, there isn't SPL DFU support for the RK3308 SoC in U-Boot, so I couldn't test the second stage.
Thanks!
For SPL DFU, I've tested it on rk3399 with patches I've sent on the u-boot mailing list. The same day someone from Radxa sent patches for a bunch of rockchip SoCs to enable it but I've not tested the serie. You may want to test that. Nevertheless, these days, I've been using the ramboot-v1 patches mentioned here and they're working at least on rk3399 and rk3588*. It should work on rk3308 too. |
There was a problem hiding this comment.
Hello Arnaud, thanks very much for your work!
I have tested this on an RK3399 using DFU and successfully booted.
I have not tried the UBoot changes that allow using it without DFU.
I do think more documentation would be helpful for users to be able to use this feature.
| logger.info(f"Sleeping {delay}ms") | ||
| time.sleep(delay / 1000) | ||
| else: | ||
| cli_error("Missing xpl or code471/code472 binary configuration") |
There was a problem hiding this comment.
I think "Missing xPL or code471 ('*_ddr_*.bin')/code472 (*_usbplug_*.bin) binary configuration" would be even more helpful, as this would indicate what binaries "code471" and "code472" are supposed to be, which is not necessarily obvious if you're trying to use the tools without being super familiar with Rockchip internals.
| path: rk3399_uboot.bin | ||
| u-boot-fit: | ||
| # proper u-boot | ||
| path: u-boot.itb |
There was a problem hiding this comment.
When I tested this on RK3399, trying to load u-boot.itb directly failed for me.
Loading simple-bin.fit.itb, however, worked perfectly.
This required quite a few patches to U-Boot (including some specific to my board, which has rather poor support upstream).
There was a problem hiding this comment.
weird. I'll test later if the simple-bin.fit.itb is working for me too. In this case, I'll update things to use it.
There was a problem hiding this comment.
I've tested on my rock5c. Don't know why but simple-bin.fit.itb didn't work. I'll have to do more testing later :(
There was a problem hiding this comment.
Current implementation of support for the Rockchip SoCs, RAMboot and
SPL DFU are supported.
I'm having trouble understanding what you meant here. Was it something like:
Snagboot currently supports two methods for Rockchip SoC recovery: RAM boot, and SPL DFU
Otherwise, regarding this part:
As there's no implementation for Rockchip USB protocol support, it is
possible to push the DDR init files and SPL files from Rockchip but the next
step will fail.
IIUC, prebuilt files don't work because SPL won't expose a USB gadget. If that's correct, then I'd reword this:
Using a precompiled SPL from Rockchip will not work, as it is usually not configured to communicate over USB.
Finally, in this part:
Mainline u-boot does not support boot from RAM and boot from SPL DFU,
it has to be patched.
s/boot/booting
docs/fw_binaries.md
Outdated
|
|
||
| ## For Rockchip devices | ||
|
|
||
| While it is possible to use the DDR init files and SPL files from Rockchip, |
There was a problem hiding this comment.
is it really possible to use snagrecover with the prebuilt SPL images from Rockchip? I don't think that's true in the general case, since they don't have DFU support.
There was a problem hiding this comment.
It's possible to use rockchip files but as it was noted, it won't be helpful, as there's no rockusb support in snagflash. I've rewritten this part. Is it better now ?
There was a problem hiding this comment.
I suppose you meant "snagrecover" and not "snagflash". Nevertheless, this is a bit confusing to me. Could we use the prebuilt binaries from Rockchip if Snagrecover had a RockUSB implementation? IIRC the issue was that the prebuilt SPL didn't expose a USB gadget at all, but maybe I'm misremembering this.
docs/fw_binaries.md
Outdated
| it is easier to use the files generated by u-boot as support for the | ||
| Rokchip USB protocol is not implemented. | ||
| Nevertheless, this means that to boot u-boot proper, u-boot has to be | ||
| patched to get support for: |
There was a problem hiding this comment.
I think we should be clear here: using the prebuilt Rockchip SPL images doesn't work with Snagrecover. People have to patch U-Boot and build the images themselves.
There was a problem hiding this comment.
Same here. I've rewritten this part. Please, check if it's clearer/better now.
| - ``mkimage-in-simple-bin.mkimage-u-boot-tpl`` or ``mkimage-in-simple-bin.mkimage-rockchip-tpl`` | ||
| - ``mkimage-in-simple-bin.mkimage-u-boot-spl`` | ||
| - ``u-boot.itb`` | ||
|
|
There was a problem hiding this comment.
Nit: you're missing a "configuration:" after the "u-boot-fit:" entry.
There was a problem hiding this comment.
Hello Arnaud,
I tested your PR with a rk3308-based board and with u-boot 2025.01 with the patch from https://gitlab.collabora.com/hardware-enablement/rockchip-3588/u-boot/-/merge_requests/5/commits.
Here are a few minor suggestions.
Regards,
François
| crcbytes = crc.final() | ||
| chunk.append(crcbytes >> 8) | ||
| chunk.append(crcbytes & 0xff) | ||
| written = self.__write_chunk(addr, chunk) |
There was a problem hiding this comment.
I got an issue when using the 'Boot from RAM' method:
snagrecover -s rk3308 -f rockchip-ramboot.yaml failed with the following error :
[INFO] Starting recovery of rk3308 board
[INFO] Installing firmware code471
[INFO] Downloading code471...
[INFO] Done installing firmware code471
[INFO] Sleeping 1ms
[INFO] Installing firmware code472
[INFO] Downloading code472...
[INFO] CLI error: Failed to run code472 firmware: [Errno 110] Operation timed out
even though the board correctly booted u-boot proper.
It seems like, when sending the CRC in write_blob, the USB request times out.
A possible solution would be to set the timeout to a bigger value for this USB request (5 seconds were enough in my case).
Wireshark reports the following answers (target to host) for the CRC answer:
| timeout 1000ms | timeout 5000ms |
|---|---|
| URB status: No such file or directory (-ENOENT) (-2) | URB status: Success (0) |
| [Time from request: 1.000119000 seconds] | [Time from request: 4.623689000 seconds] |
There was a problem hiding this comment.
oh, that's sad. I've added a timeout of 5 seconds. Can you please test to see if it's fine now ?
There was a problem hiding this comment.
Hello Arnaud, thanks for your changes.
I have tested your fix and it works fine for me.
There was a problem hiding this comment.
I got an issue when using the 'Boot from RAM' method:
snagrecover -s rk3308 -f rockchip-ramboot.yamlfailed with the following error :[INFO] Starting recovery of rk3308 board [INFO] Installing firmware code471 [INFO] Downloading code471... [INFO] Done installing firmware code471 [INFO] Sleeping 1ms [INFO] Installing firmware code472 [INFO] Downloading code472... [INFO] CLI error: Failed to run code472 firmware: [Errno 110] Operation timed outeven though the board correctly booted u-boot proper.
It seems like, when sending the CRC in
write_blob, the USB request times out.A possible solution would be to set the timeout to a bigger value for this USB request (5 seconds were enough in my case). Wireshark reports the following answers (target to host) for the CRC answer:
timeout 1000ms timeout 5000ms
URB status: No such file or directory (-ENOENT) (-2) URB status: Success (0)
[Time from request: 1.000119000 seconds] [Time from request: 4.623689000 seconds]
Some SOCs (sorry I can't remember) require a longer timeout... I'm using 30s for it ;)
https://github.com/RadxaNaoki/rkusbboot/blob/main/rkusbboot.c#L75
| ``` | ||
| CONFIG_SPL_DM_USB_GADGET=y | ||
| CONFIG_SPL_USB_GADGET=y | ||
| CONFIG_SPL_DFU=y |
There was a problem hiding this comment.
I encountered an issue while using this configuration. After
recompilation of U-Boot, only my SPL was being updated, i needed to power cycle the board to
be able to update U-Boot Proper.
I found out that the board's DDR was not properly erased after a reset,
thus when using snagrecover with DFU to update both u-boot, SPL was correctly updated but would boot an old u-boot proper instead of retrieving it through DFU, since DFU comes after RAM in SPL boot order.
A solution to this would be to disable U-Boot's configuration option CONFIG_SPL_RAM_DEVICE=n Support booting from preloaded image in RAM.
Could you please add this option to the list along with the explanation that I gave?
There was a problem hiding this comment.
Oh, right. On reset/reboot, the RAM is not reset so it'll find the ramboot binary and use it instead of DFU. I've added some notes in the doc about that.
Add support for uploading blods to the maskrom memory using Rockchip maskrom protocol. Signed-off-by: Arnaud Patard <[email protected]>
``offset`` is too generic, so rename it to ``class_offset``. Signed-off-by: Arnaud Patard <[email protected]>
…in files Add support for reading and sending rockchip binary files, made with boot_merger. Signed-off-by: Arnaud Patard <[email protected]>
Add support for booting a rockchip bin file, made with boot_merger. It can be: - a file containing the rockchip miniloader, but in this case, it's not useful, as rockusb is not implemented - a file containing uboot tpl and spl. In this case, if u-boot is configured for SPL DFU, the snagrecover configuration file may specify an u-boot FIT image to load into the first DFU slot. This will allow booting u-boot proper and then to fastboot/ums/dfu/... to flash. Signed-off-by: Arnaud Patard <[email protected]>
| ``` | ||
|
|
||
| On some systems, rebooting the system won't reset the memory content. This means that | ||
| booting over DFU after having done a boot from RAM will result in u-boot loading the |
There was a problem hiding this comment.
Hello Arnaud,
thanks for adding a explanation and the solution to my problem.
I think the current explanation might mislead some users. U-Boot SPL will boot a U-Boot proper if it is present in RAM, whether it was put there by "boot from DFU" or by "boot from RAM".
This could lead some users that only use "boot from DFU" to not disable CONFIG_SPL_RAM_DEVICE.
We could maybe write something along the lines of: "This means that booting over DFU when there is already u-boot proper in RAM will boot the later and not download the new one from DFU, e.g. if you run snagrecover two times in a row, u-boot proper might not be updated in these cases"
|
|
||
| The patches have been sent to the u-boot mailing list by their authors | ||
| and not yet merged. The patches can all be found inside this | ||
| [merge request](https://gitlab.collabora.com/hardware-enablement/rockchip-3588/u-boot/-/merge_requests/7). |
There was a problem hiding this comment.
This branch is using outdated U-Boot RAM-boot patches, please update the MR to use latest version of the series, at the moment v2. Also plan on sending a v3 with a small update next few days/week.
|
|
||
| Snagboot can upload the ``CODE471_OPTION`` and ``CODE472_OPTION`` of a binary | ||
| generated with the ``boot_merger`` tool and configuration files from | ||
| [Rockchip rkbin](https://github.com/radxa/rkbin/tree/develop-v2024.03/). |
There was a problem hiding this comment.
Should probably link to "upstream" https://github.com/rockchip-linux/rkbin tree.
| [Rockchip rkbin](https://github.com/radxa/rkbin/tree/develop-v2024.03/). | |
| [Rockchip rkbin](https://github.com/rockchip-linux/rkbin). |
|
|
||
| [example](../src/snagrecover/templates/rockchip-ramboot.yaml) | ||
|
|
||
| When building u-boot with the previously mentioned patches, u-boot will generate two files: |
There was a problem hiding this comment.
With the latest U-Boot RK RAM-boot series you should build U-Boot with something like following to produce u-boot-rockchip-usb47X.bin files.
make <board>_defconfig rockchip-ramboot.config
| At the end of the build, the following files will be needed: | ||
|
|
||
| - ``mkimage-in-simple-bin.mkimage-u-boot-tpl`` or ``mkimage-in-simple-bin.mkimage-rockchip-tpl`` | ||
| - ``mkimage-in-simple-bin.mkimage-u-boot-spl`` |
There was a problem hiding this comment.
These mkimage-in-simple-bin.*-files are temporary files generated by binman during building of the final output u-boot-rockchip.bin-file.
This should probably not encourage users to rely on these files as they could be renamed and/or removed as part of a future update of U-Boot.
| **code471:** File to use for maskrom code 0x471. | ||
|
|
||
| configuration: | ||
| * path: Path to the TPL file. For instance, ``mkimage-in-simple-bin.mkimage-rockchip-tpl``. |
There was a problem hiding this comment.
mkimage-in-simple-bin.mkimage-rockchip-tpl is a temporary file used by binman and may not be a good example.
| if soc_model in NEWIDB_LIST: | ||
| return fw_blob | ||
|
|
||
| # Round to 4096 block size |
There was a problem hiding this comment.
What is the reason for this padding to 4 KiB? Typically the boot image is padded to 512 or 2048 bytes.
| cfg = dev.get_active_configuration() | ||
|
|
||
| eps_found = False | ||
| for intf in cfg.interfaces(): | ||
| ep_in, ep_out = None, None | ||
| for ep in intf.endpoints(): | ||
| is_bulk = (ep.bmAttributes & usb.ENDPOINT_TYPE_MASK) == usb.ENDPOINT_TYPE_BULK | ||
| is_in = (ep.bmAttributes & usb.ENDPOINT_TYPE_MASK) == usb.ENDPOINT_TYPE_BULK | ||
| if not is_bulk: | ||
| continue | ||
| is_in = (ep.bEndpointAddress & usb.ENDPOINT_DIR_MASK) == usb.ENDPOINT_IN | ||
| if is_in: | ||
| ep_in = ep.bEndpointAddress | ||
| else: | ||
| ep_out = ep.bEndpointAddress | ||
| if not ((ep_in is None) or (ep_out is None)): | ||
| eps_found = True | ||
| break | ||
| if not eps_found: | ||
| raise RochipBootRomError("No BULK IN/OUT endpoint pair found in device") | ||
| self.ep_in = ep_in | ||
| self.ep_out = ep_out |
There was a problem hiding this comment.
You do not really need the ep_in/ep_out for the ctrl_transfer done here, these endpoints are mostly needed for the RockUSB protocol, something that is not supported or used here.
| crc_sent = False | ||
| for chunk in utils.dnload_iter(blob, 4096): | ||
| chunk_len = len(chunk) | ||
| if chunk_len == 4096: | ||
| crc.process(chunk) | ||
| elif chunk_len == 4095: | ||
| chunk.append(0x00) | ||
| crc.process(chunk) | ||
| else: | ||
| crc.process(chunk) | ||
| crcbytes = crc.final() | ||
| chunk = bytearray(chunk) | ||
| chunk.append(crcbytes >> 8) | ||
| chunk.append(crcbytes & 0xff) | ||
| crc_sent = True | ||
| written = self.__write_chunk(code, chunk) | ||
| ret = written == len(chunk) | ||
| if ret is False: | ||
| return ret | ||
| total_written += written | ||
| if chunk_len+2 == 4096: | ||
| chunk = [0x00] | ||
| written = self.__write_chunk(code, chunk) | ||
| ret = written == len(chunk) | ||
| if ret is False: | ||
| return ret | ||
| if crc_sent is False: | ||
| chunk = bytearray() | ||
| crcbytes = crc.final() | ||
| chunk.append(crcbytes >> 8) | ||
| chunk.append(crcbytes & 0xff) | ||
| written = self.__write_chunk(code, chunk) | ||
| ret = written == len(chunk) | ||
| if ret is False: | ||
| return ret |
There was a problem hiding this comment.
This transfer loop can be simplified, please see my implementation at labgrid-project/labgrid#1721 for what I think is a cleaner and more simplified transfer loop.
| dev = get_usb(usb_addr) | ||
|
|
||
| # Blob made with boot_merger | ||
| if "xpl" in recovery_config["firmware"]: |
There was a problem hiding this comment.
The output from boot_merger is typically called loader-images, and could be a better name for this option.
xpl can be confused with U-Boot's use of xPL, i.e. all TPL/VPL/SPL phases prior to U-Boot proper.
| path: mkimage-in-simple-bin.mkimage-u-boot-tpl | ||
| delay: 1 | ||
| code472: | ||
| path: mkimage-in-simple-bin.mkimage-u-boot-spl |
There was a problem hiding this comment.
As noted earlier, these example paths is for temporary files that should probably not be considered output artifacts from a U-Boot build.
|
FYI, v3 patch set: https://lists.denx.de/pipermail/u-boot/2025-August/595680.html, @Kwiboo thanks for this and feel free to correct me if there's a newer series I tested this U-Boot series on RV1106 (my branch) and it's really neat to boot from RAM with FIT attached (no need in SPL DFU) and all the bundling is done by U-Boot in-tree FOSS tools. I managed to use mass storage over DWC3 then, works great. Also, might be interesting: https://github.com/collabora/rockchiprs/tree/main Seems to be one of the most straightforward implementations, also somehow avoids those SoC support lists and perhaps autodetects RC4 mode. Might be worth taking a look, I was going to base on this design before I saw this PR. I could possibly try to help on this if you would like, feel free to ask. |
Thanks and this should be the latest posted version. I will most likely send a v4 with a small update to reduce size and use relocation as mentioned in labgrid-project/labgrid#1721 The the crc16 validation done by BootROM can be very slow, to optimize U-Boot RAM boot times using |
Ah, that's the thing I faced. I wondered why was it taking like 30 seconds for me, despite all transfers being complete. Maybe then SPL DFU is worth it, as that could be faster. Will try to make this work on my rv1106 when I'm back at it (I tried, but IIRC there was some effort needed like enabling the PHY in SPL or some clocks). Thanks, I'll perhaps take a look if I can also enable FIT compression to make myself iterate more quickly in my U-Boot porting work p.s. I've emailed you about my RV1106 progress, could you please share your thoughts or link me to communities where I can get more involved as a beginner U-Boot/Linux upstreaming developer? Sorry if you have already read the email and just haven't replied yet, I thought the message might have got to spam folder. |
|
Hello @apatard , it's been a few months since this PR saw updates, and it seems like quite a few review comments have been accumulating. Do you have any news on this? It would be quite nice if we could get this integrated before the upcoming release :) |
7 participants
Add support for rockchip maskrom mode. It can send files generated with boot_merger and for SoC with older
IDB format, it can generated the blob at run time. For SoCs with IDB v2, some extra work is needed atm.
note: rockusb protocol is not supported, as booting with u-boot will allow using things like dfu / ums / fastboot