Hi Peter,
On 6/21/24 10:56 AM, Peter Robinson wrote:
rk3568 is now upstream, there was a PR upstream for this for some
time, similar to rk3588, albeit not as long as rk56x. In some cases
the issue here is the speed of review of upstream ATF. I don't think
that means it should go into something like this.
If the BL31 blob doesn't go into boot-firmware, I don't see the benefit
of boot-firmware for Rockchip platforms to be honest. Instead of telling
people to get their firmware from github.com/rockchip-linux/rkbin we'll
tell them to get one from rkbin and the other from boot-firmware. And I
have a feeling that if that's how it'll go that the vendor will just not
care about boot-firmware as they would anyway need to keep updating
rkbin as well.
Well it could go there, but I currently build ATF for all Rockchip
platforms we support.
That's nice to hear :)
It doesn't matter whose fault it is for not being upstreamed earlier,
I agree. There's also no reason something can't be included and then
dropped at a later time when things land upstream in appropriate
places.
True BUT every now and then we get someone who wants to keep using the
blobs for some reason. It's up to us to allow this or not though but
wanted to let you know that this happens sometimes. (not our clients,
people from the community).
Also, it's nice to be able to debug stuff by comparing the blob and
open-source implementation behaviors. And sometimes even if there's an
OSS implementation it isn't necessarily featureful, c.f. crypto support
on the current patch series for RK3588 TF-A.
Also, I am not sure it brings any benefit to remove binaries once they
are in the git history to be honest, maybe in terms of checkout size if
you don't bring the history (shallow clone I think it's called?).
the fact is, we still don't have a **merged** open-source BL31 for
RK3588 2 and half years after the Rock5B from Radxa was announced. So in
that whole timeframe, we have to work with blobs (or maintain your own
forked TF-A whenever patches are posted and hope it works well enough).
Agreed, the rk356x has *finally* been merged[1], it was sent as a PR
around 2 years before. I have highlighted this to Linaro/arm in the
past that this delay really isn't good enough.
[1]
https://github.com/ARM-software/arm-trusted-firmware/tree/master/plat/rockchip/rk3568
Thanks, I agree this isn't really a nice situation. We cannot complain
the vendors aren't doing their job of mainlining stuff if we take too
long when they do (baring normal review process highlighting mistakes,
errors, etc...).
The point I was trying to make, and I clearly didn't was. In the case
where there's open source code do we want to include vendor binaries?
Then I guess we had the same problem in failing to expose our point
since I had the same one, would those open-source based blobs be allowed
in that repo?
I would suggest the following limitations:
- blobs from vanilla/unpatched upstreamed open-source projects aren't
allowed
- blobs based on open-source projects with permissive licensing that
allow those blobs to be shared as blobs without sources,
FYI, the DDR bin is printing stuff on the console, so we had to modify
it (with a tool from Rockchip) to remove the gibberish breaking the
terminal by setting the appropriate controller, mux and baudrate (for
our products, there's no one size fits all :) ). The question is how to
handle this since we cannot realistically store every possible
permutation of that binary for each UART controller, mux of UART
controller and baudrate (the only parameters **we** modify, but there
are tons of others).
I think those sort of things should be mostly quiet TBH.
Mostly != fully. The console doesn't break consistently so I assume that
if it even prints a tiny bit of info it would still be able to break
stuff. I assume we could have binman run some logic to replace the uart
controller, mux and baudrate. Not sure we want that but we could.
Adjusting binaries on the fly sounds problematic to me. Is the process
open or something that's NDA between rockchip and their customers?
All I can say is that I asked this tool to be open-sourced to Rockchip,
it's currently an x86_64-only blob here:
https://github.com/rockchip-linux/rkbin/blob/master/tools/ddrbin_tool
Mark Kettenis has written something to change the baudrate:
https://github.com/openbsd/ports/blob/master/sysutils/u-boot/rk3588/files/rkbinpatch.c
Also, the DDR bin is passing the console info to the BL31 binary through
ATAGS, so if you don't fix the DDR bin baudrate, controller and mux, the
BL31 will also not be fixed.
Ultimately we should work with the vendor to fix those so they're
consistent? Why do you're products have issues, are they using
Consistent in which way? I didn't get what you were suggesting.
baudrate that is different to all the other rockchip devices?
Rockchip binaries default to 1.5MBaud while Theobroma boards are all
115.2KBaud so we are consistent across all our product base.
As for the controller and mux, each controller has like 2 to 4 muxes and
there are ten controllers on RK3588 for example. Rockchip provides a
reference design for each SoC but nothing prevents you from using a
different controller/mux than the one in the reference design. Which is
what our HW department has done for the three devices I've brought up in
the last two years. I assume because we wanted to use the functions
exposed on the controller+mux used in the reference design and you only
have so many permutations possible.
I suspect that the other HW board vendors just want to limit the amount
of work to do in SW and just follow as close as possible the reference
design.
Cheers,
Quentin
