mbox series

[meta-rockchip,0/6] rkbin: factoring ddrbin do_deploy, customize ddrbin and bump rkbin

Message ID 20250311-ddrbin-custom-v1-0-e5c994ac25e1@cherry.de
Headers show
Series rkbin: factoring ddrbin do_deploy, customize ddrbin and bump rkbin | expand

Message

Quentin Schulz March 11, 2025, 11:26 a.m. UTC
This simplifies rockchip-rkbin-ddr do_deploy to only have one do_deploy
task common to all supported SoC instead of having to add a new task
override for each new SoC. It does require to set a few more variables
though.

Additionally, this allows to finally customize the DDR binary blob using
Rockchip's ddrbin_tool.py tool. This is typically useful if your board
isn't following Rockchip's HW reference design, e.g. using a different
UART controller or pins for serial output, or using a baudrate different
from the default of 1500000.

Finally it bumps OP-TEE OS, TF-A and DDR bin recipes to use the latest
commit in rkbin git repo.

I am not too fond of using a file as input for the customization, but
this is what ddrbin_tool.py uses and there are possibly too many
corner-cases to handle by using fine-grained variables. So for now, I
decided to go with that but that is for sure not the only possible
implementation!

Note that the bump commit wasn't tested.

Note that I plan on backporting all but the last two patches to
scarthgap (and I assume styhead too). The patch before the last would
break backward compatibility if someone appends to the
do_deploy:<override> task. The last patch could be merged but there's no
absolute need for it?

Lastly, the patch about customizing DDR bin needs to be modified before
being backported as it uses UNPACKDIR which doesn't exist in scarthgap.
The logic should be safe to use with ${S} (for scarthgap) though!

@Trevor, maybe you can check if that would fix your issue with the
RK3308 DDR blob you wanted to keep on that outdated version which is one
of the last versions where the UART you want to use exists? It'd be nice
to get rid of the rk3308-specific recipes I believe :)

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
Quentin Schulz (6):
      bsp: rkbin: ddr: store directory path for the DDR bin blob in a variable
      bsp: rkbin: ddr: make deployed name configurable
      bsp: rkbin: add native recipe for tools (ddrbin_tool.py)
      bsp: rkbin: ddr: allow to customize DDR bin blob
      bsp: rkbin: ddr: factor out do_deploy to be SoC-agnostic
      bsp: rkbin: bump to latest commit in master branch

 README                                         | 20 +++++-
 recipes-bsp/rkbin/rockchip-rkbin-ddr_git.bb    | 87 +++++++++++++++++++++-----
 recipes-bsp/rkbin/rockchip-rkbin-native_git.bb | 21 +++++++
 recipes-bsp/rkbin/rockchip-rkbin.inc           |  2 +-
 4 files changed, 111 insertions(+), 19 deletions(-)
---
base-commit: 0aba79e47951b049bcdd39e07f660c665e257393
change-id: 20250311-ddrbin-custom-f83918406e3a

Best regards,

Comments

Quentin Schulz April 10, 2025, 11:01 a.m. UTC | #1
Hi all,

On 3/11/25 12:26 PM, Quentin Schulz wrote:
> This simplifies rockchip-rkbin-ddr do_deploy to only have one do_deploy
> task common to all supported SoC instead of having to add a new task
> override for each new SoC. It does require to set a few more variables
> though.
> 
> Additionally, this allows to finally customize the DDR binary blob using
> Rockchip's ddrbin_tool.py tool. This is typically useful if your board
> isn't following Rockchip's HW reference design, e.g. using a different
> UART controller or pins for serial output, or using a baudrate different
> from the default of 1500000.
> 
> Finally it bumps OP-TEE OS, TF-A and DDR bin recipes to use the latest
> commit in rkbin git repo.
> 
> I am not too fond of using a file as input for the customization, but
> this is what ddrbin_tool.py uses and there are possibly too many
> corner-cases to handle by using fine-grained variables. So for now, I
> decided to go with that but that is for sure not the only possible
> implementation!
> 
> Note that the bump commit wasn't tested.
> 
> Note that I plan on backporting all but the last two patches to
> scarthgap (and I assume styhead too). The patch before the last would
> break backward compatibility if someone appends to the
> do_deploy:<override> task. The last patch could be merged but there's no
> absolute need for it?
> 
> Lastly, the patch about customizing DDR bin needs to be modified before
> being backported as it uses UNPACKDIR which doesn't exist in scarthgap.
> The logic should be safe to use with ${S} (for scarthgap) though!
> 
> @Trevor, maybe you can check if that would fix your issue with the
> RK3308 DDR blob you wanted to keep on that outdated version which is one
> of the last versions where the UART you want to use exists? It'd be nice
> to get rid of the rk3308-specific recipes I believe :)
> 

Any feedback?

Cheers,
Quentin
Trevor Woerner April 15, 2025, 6:14 p.m. UTC | #2
On Tue 2025-03-11 @ 12:26:29 PM, Quentin Schulz via lists.yoctoproject.org wrote:
> @Trevor, maybe you can check if that would fix your issue with the
> RK3308 DDR blob you wanted to keep on that outdated version which is one
> of the last versions where the UART you want to use exists? It'd be nice
> to get rid of the rk3308-specific recipes I believe :)

Yes, that would be nice!

If I understand all of this correctly, I tried using the ddrbin_tool.py to
unpack the parameters of the latest version of the ddr.bin for the rk3308.
In other words, I applied this patchset, set RKBIN_RK3308_LATEST = "1", and
did a build. I then unpacked the parameters of the ddrbin that was in the
deploy directory. According to the values that were placed in the parameter
file by this tool, the baud rate of the uart is already set to 1,500,000. Yet
I'm still seeing gibberish with the latest version. So I don't think using
this tool to re-set the baud rate to 1,500,000 is going to work. For the
time-being I'll just leave things as they are.

Sorry for taking so long. I had wanted to play around with it and see if I
could get this working. I'm going to try anyway, if nothing else it would
prove that this mechanism is working correctly. But it does not look like it
will fix anything with respect to the rk3308.
Quentin Schulz April 16, 2025, 10:17 a.m. UTC | #3
Hi Trevor,

On 4/15/25 8:14 PM, Trevor Woerner via lists.yoctoproject.org wrote:
> On Tue 2025-03-11 @ 12:26:29 PM, Quentin Schulz via lists.yoctoproject.org wrote:
>> @Trevor, maybe you can check if that would fix your issue with the
>> RK3308 DDR blob you wanted to keep on that outdated version which is one
>> of the last versions where the UART you want to use exists? It'd be nice
>> to get rid of the rk3308-specific recipes I believe :)
> 
> Yes, that would be nice!
> 
> If I understand all of this correctly, I tried using the ddrbin_tool.py to
> unpack the parameters of the latest version of the ddr.bin for the rk3308.
> In other words, I applied this patchset, set RKBIN_RK3308_LATEST = "1", and
> did a build. I then unpacked the parameters of the ddrbin that was in the
> deploy directory. According to the values that were placed in the parameter
> file by this tool, the baud rate of the uart is already set to 1,500,000. Yet
> I'm still seeing gibberish with the latest version. So I don't think using
> this tool to re-set the baud rate to 1,500,000 is going to work. For the
> time-being I'll just leave things as they are.
> 

If I'm not mistaken, we kept the old rkbin for the RK3308 because it is 
one of the latest ones that uses UART0 (M0) for its serial output and 
the new ones only have either UART2 or UART4.

I would recommend to set

uart id=0
uart iomux=0

in a ddrbin_param.txt for your Rock Pi S provided you want to expose the 
UART on pins listed here https://wiki.radxa.com/RockpiS/dev/serial-console

For the ROC-3308-CC you have options as UART0, UART1 and UART2_M0 are 
exposed on the headers. Considering we use the same rkbin, I would 
assume you want it to be on UART0 so the same can be done in a 
ddrbin_param.txt.

Considering there are no iomux on UART0, you possibly could omit `uart 
iomux=0` but eh, doesn't hurt probably?

> Sorry for taking so long. I had wanted to play around with it and see if I
> could get this working. I'm going to try anyway, if nothing else it would
> prove that this mechanism is working correctly. But it does not look like it
> will fix anything with respect to the rk3308.
> 

Did/Can you try with the uart id and uart iomux changed in ddrbin_param.txt?

Cheers,
Quentin