Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: initial Rock 5B+ edge kernel support #7457

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

fridtjof
Copy link
Collaborator

Description

The Rock 5B+ does not have an upstream device tree, but is of course very similar to Rock 5B.

Starting from mainline's rock-5b.dts, and cross referencing vendor device trees for the 5B+ I arrived at a device tree that ~works with an edge kernel.

As with the Rock 5B, U-Boot mainline is used to boot said kernel. I also already bumped the tree/version to mainline 2024.10 according to #7135.

How Has This Been Tested?

Image built with:

./compile.sh build BOARD=rock-5b-plus BRANCH=edge BUILD_DESKTOP=no BUILD_MINIMAL=no KERNEL_CONFIGURE=no RELEASE=noble

  • Board boots up fine from an SD card, other boot methods not tested
  • USB ports ~work, however:
    • Type-C port only works in one orientation right now
    • Top USB3 Type-A port only seems to work at USB2 speeds. No idea here.
  • Ethernet works at 1GBit/s and 2.5Gbit/s
  • Wifi scans, connects, works
  • Bluetooth works (finds devices, no connection testing)
  • Video I/O (HDMI0/1, HDMI IN, DisplayPort) not tested
  • M.2/PCIe not tested

Checklist:

Please delete options that are not relevant.

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings (not sure, CI will tell me i guess?)

@github-actions github-actions bot added size/large PR with 250 lines or more Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... Patches Patches related to kernel, U-Boot, ... labels Nov 11, 2024
@fridtjof
Copy link
Collaborator Author

fyi, here's a diff of rock-5b (as patched by armbian-build) vs rock-5b+ DTS:

5b-5bp.txt

(Github wouldn't let me upload as .patch, right before listing that extension as supported. shrug)

@EvilOlaf
Copy link
Member

I suggest to test this against 6.12 as well since the rollover to that version might happen soon
#7455
#7372

@fridtjof
Copy link
Collaborator Author

I suggest to test this against 6.12 as well since the rollover to that version might happen soon #7455 #7372

Good point, putting that on the todo list.

Of note, because I forgot to mention this before; Currently, there seems to be a problem with mmc?

dwmmc_rockchip fe2c0000.mmc: swiotlb buffer is full (sz: 1048576 bytes), total 32768 (slots), used 3087 (slots)

Not sure what to do about this; the vendor kernel does not show these messages, but i'm not sure if it's just not logging this or if the problem is actually gone there.

@igorpecovnik igorpecovnik added the 11 Milestone: Fourth quarter release label Nov 11, 2024
@@ -3,7 +3,7 @@ BOARD_NAME="Rock 5B Plus"
BOARDFAMILY="rockchip-rk3588"
BOARD_MAINTAINER=""
BOOTCONFIG="rock-5b-plus-rk3588_defconfig"
KERNEL_TARGET="vendor"
KERNEL_TARGET="edge,vendor"
FULL_DESKTOP="yes"
BOOT_LOGO="desktop"
BOOT_FDT_FILE="rockchip/rk3588-radxa-rock-5b+.dtb"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upstream name will likely be rk3588-rock-5b-plus, same as in radxa tree. Suggest a rename to match the most likely upstream name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I vaguely remember things breaking with vendor u-boot for some reason when using a different name, but I might be remembering wrong.

@HeyMeco do you know anything more?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fridtjof
Changing the name from BOOT_FDT_FILE would be no issue (that's for the kernel). As long as BOOTCONFIG (that tells U-Boot what to use) stays as is and the name change is also being done in armbian/linux-rockchip

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would overriding it in the post_family_config_branch_edge hook for now work as well? Then this PR does not implicitly break the vendor kernel until it's changed there (could be done in a trivial PR to change it on both sides then)

@igorpecovnik igorpecovnik added the Work in progress Unfinished / work in progress label Nov 13, 2024
@fridtjof
Copy link
Collaborator Author

Tested with #7455.

Boots fine, and there's no regressions vs. what I tested on 6.11

I did briefly test HDMI0 by plugging in a 4K monitor and installing ubuntu-desktop (to avoid having to build yet another image). The output, unfortunately is pretty garbled. Best i can describe it is colors are broken, and the dimensions are off (too wide, not tall enough).
Is that considered a blocker for merging? (I personally don't need HDMI output urgently, so I'd defer that to some other time)

@igorpecovnik
Copy link
Member

Is that considered a blocker for merging?

No, that is probably common. Here, for desktop, its better to start with desktop image. As its assembled right. Upgrade from CLI is not working well, also our desktop. This part we need to work on.

@igorpecovnik
Copy link
Member

Tested with #7455.

Merged, so you can proceed on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
11 Milestone: Fourth quarter release Hardware Hardware related like kernel, U-Boot, ... Needs review Seeking for review Patches Patches related to kernel, U-Boot, ... size/large PR with 250 lines or more Work in progress Unfinished / work in progress
Development

Successfully merging this pull request may close these issues.

5 participants