Skip to content

Add board Luckfox Nova W #8306

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

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

Add board Luckfox Nova W #8306

wants to merge 5 commits into from

Conversation

nikvoid
Copy link

@nikvoid nikvoid commented Jun 16, 2025

Description

Add initial support for board Luckfox Nova W.
Luckfox Nova W is recently produced board on SoC Rockchip RK3308B: 4 x Cortex-A35 CPU, 512 RAM, 8 GB eMMC.
It also has Ethernet, WIFI & BT, USB-A and USB-C ports, soldered microphone.
Product page: https://www.luckfox.com/Luckfox-Nova?ci=646
image

But beware: it is very fresh board, has almost no documentation and only buildroot SDK with kernel 5.10 is available from vendor.

Documentation

I decided to create separate directory for vendor kernel patches. This kernel must be the same as rk35xx-vendor-6.1, but this SoC is not RK35XX.
Used U-Boot is mainline.
Instead of DKMS AIC8800 driver I enabled the one from kernel sources, because I needed to apply patches to it. And image builds faster than with DKMS.

UART4 is used for serial console and by proprietary firmware blobs.

How Has This Been Tested?

  • eMMC does not work in mainline kernel, current and edge
  • Boot Noble 24.04 with vendor kernel rk6.1
  • USB host and peripheral is working
  • WIFI is working
  • SPI May have quirks, though spidev-test appears to be working
  • SPI1-M1 MISO is not receiving data
  • SPI2 is working

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
  • Any dependent changes have been merged and published in downstream modules

Copy link
Contributor

coderabbitai bot commented Jun 16, 2025

Walkthrough

Adds support for the Luckfox Nova W (Rockchip RK3308B) by introducing a board configuration (config/boards/luckfox-nova-w.csc) that specifies boot/kernel/runtime settings, kernel source/versions, firmware blobs, and board-specific tweaks. Adds a Rockchip64 U-Boot bootscript for ttyS4 (config/bootscripts/boot-rockchip64-ttyS4.cmd) implementing armbianEnv merging, DT overlay/fixup handling, and kernel boot logic. Adds a vendor Linux kernel config (config/kernel/linux-luckfox-nova-w-rk3308-vendor.config) tailored for RK3308 with extensive drivers and options.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

Reason: multiple new files with moderate logic density (board config functions and a nontrivial U-Boot bootscript) and a large kernel config file. Changes are heterogeneous (init scripts, shell functions, config content) requiring separate checks per file but limited in file count.

Possibly related PRs

Suggested labels

BSP

Suggested reviewers

  • clee
  • joekhoobyar
  • brentr
  • rpardini
  • pyavitz
  • leggewie
  • igorpecovnik

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between bb9471e and 2c80e8e.

📒 Files selected for processing (2)
  • config/boards/luckfox-nova-w.csc (1 hunks)
  • config/kernel/linux-luckfox-nova-w-rk3308-vendor.config (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • config/boards/luckfox-nova-w.csc
  • config/kernel/linux-luckfox-nova-w-rk3308-vendor.config
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added size/large PR with 250 lines or more 08 Milestone: Third quarter release Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... Patches Patches related to kernel, U-Boot, ... labels Jun 16, 2025
@coderabbitai coderabbitai bot added the Ready to merge Reviewed, tested and ready for merge label Jun 16, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (3)
config/bootscripts/boot-rockchip64-ttyS4.cmd (1)

62-66: Overlay restore path leaves overlay_error set to true

After restoring the original DT, overlay_error is left unchanged. Subsequent logic or scripts that inspect this variable will still think an overlay error is active.

-load ${devtype} ${devnum}:${distro_bootpart} ${fdt_addr_r} ${prefix}dtb/${fdtfile}
+load ${devtype} ${devnum}:${distro_bootpart} ${fdt_addr_r} ${prefix}dtb/${fdtfile}
+setenv overlay_error "false"
config/boards/luckfox-nova-w.csc (1)

53-55: mv may abort if the firmware path does not exist

On early images the upper-case directory might already be in lower-case form,
causing mv to fail and abort the whole build script.

-mv "${SDCARD}/lib/firmware/aic8800/SDIO/aic8800DC" "${SDCARD}/lib/firmware/aic8800/SDIO/aic8800dc"
+mv -f "${SDCARD}/lib/firmware/aic8800/SDIO/aic8800DC" \
+      "${SDCARD}/lib/firmware/aic8800/SDIO/aic8800dc" 2>/dev/null || true
config/kernel/linux-luckfox-nova-w-rk3308-vendor.config (1)

465-488: Heavy debug & tracing options will waste scarce 512 MB DRAM

CONFIG_DYNAMIC_DEBUG, FTRACE, DEBUG_INFO_*, PROVE_LOCKING, full
PREEMPT, HZ_1000, etc. are all enabled. Great for bring-up, but they cost
both memory (≈ 40-60 MB extra) and performance on a tiny Cortex-A35.

Consider shipping two defconfigs:
debug (current), and
production with

  • CONFIG_PREEMPT_VOLUNTARY
  • CONFIG_HZ_250
  • # CONFIG_DYNAMIC_DEBUG is not set
  • # CONFIG_DEBUG_INFO_* is not set
  • # CONFIG_FUNCTION_TRACER is not set

This drops idle RAM usage by ~10 % on RK3308.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 989f255 and ebce12f.

⛔ Files ignored due to path filters (7)
  • patch/kernel/rockchip64-vendor-6.1/0000.patching_config.yaml is excluded by !patch/**
  • patch/kernel/rockchip64-vendor-6.1/aic8800-select-rk-platform.patch is excluded by !patch/**
  • patch/kernel/rockchip64-vendor-6.1/dt/rk3308-luckfox-nova-w.dts is excluded by !patch/**
  • patch/kernel/rockchip64-vendor-6.1/qcaspi-rkspi-fix.patch is excluded by !patch/**
  • patch/kernel/rockchip64-vendor-6.1/rfkill-rk-sdio-rescan.patch is excluded by !patch/**
  • patch/kernel/rockchip64-vendor-6.1/usbotg-fix.patch is excluded by !patch/**
  • patch/u-boot/v2025.04/board_luckfox-nova-w/add-board-luckfox-nova-w-rk3308b.patch is excluded by !patch/**
📒 Files selected for processing (3)
  • config/boards/luckfox-nova-w.csc (1 hunks)
  • config/bootscripts/boot-rockchip64-ttyS4.cmd (1 hunks)
  • config/kernel/linux-luckfox-nova-w-rk3308-vendor.config (1 hunks)
🔇 Additional comments (1)
config/bootscripts/boot-rockchip64-ttyS4.cmd (1)

37-41: rootdev remains a hard-coded /dev/mmcblk0p1 while partuuid is resolved

You correctly detect the partition UUID (line 38) but do not use it.
Using the device node in one place and PARTUUID in another breaks when
/dev/mmcblk0 numbering changes (USB-boot, NVMe, different eMMC order, etc.).

-setenv rootdev "/dev/mmcblk0p1"
+setenv rootdev "PARTUUID=${partuuid}"

Alternatively keep rootdev default and replace ${rootdev} with
PARTUUID=${partuuid} directly in bootargs.

@github-actions github-actions bot removed the Ready to merge Reviewed, tested and ready for merge label Jun 16, 2025
@coderabbitai coderabbitai bot requested a review from clee June 16, 2025 11:31
@coderabbitai coderabbitai bot added the Ready to merge Reviewed, tested and ready for merge label Jun 16, 2025
Copy link
Member

Choose a reason for hiding this comment

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

Please note that these patches must not break existing hardware support, and it's best to submit them to this repository: https://github.com/armbian/linux-rockchip.

Copy link
Author

Choose a reason for hiding this comment

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

I know, but they may break, and now this is a single board with these patches

# eMMC works properly only in vendor branch
KERNEL_TARGET="vendor"

KERNELSOURCE='https://github.com/armbian/linux-rockchip.git'
Copy link
Member

Choose a reason for hiding this comment

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

I suggest referring to other RK3308-based devices, and ideally using the mainline kernel.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. AFAIR rk bsp is mostly designed to support rk35xx family, not sure how much work has been done for rk33xx. Though most of if is mainlined and this shall be preferred.

Copy link
Author

Choose a reason for hiding this comment

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

I could not get emmc working in mainline kernels. May be I'll revisit this later to derive patches

Copy link
Member

Choose a reason for hiding this comment

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

Please ensure this patch does not break aic8800 support on other Rockchip devices.

Copy link
Author

Choose a reason for hiding this comment

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

Maybe just make patches board-specific to be sure? Could you say how to do that?

# eMMC works properly only in vendor branch
KERNEL_TARGET="vendor"

KERNELSOURCE='https://github.com/armbian/linux-rockchip.git'
Copy link
Member

Choose a reason for hiding this comment

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

Agreed. AFAIR rk bsp is mostly designed to support rk35xx family, not sure how much work has been done for rk33xx. Though most of if is mainlined and this shall be preferred.

@TheSnowfield TheSnowfield removed the Ready to merge Reviewed, tested and ready for merge label Jun 17, 2025
@igorpecovnik igorpecovnik added the Work in progress Unfinished / work in progress label Jun 19, 2025
@EvilOlaf EvilOlaf added the Can be closed? Ping developers on stalled issue / PR label Aug 3, 2025
@leggewie
Copy link
Collaborator

@nikvoid are you still interested to get your work merged?

@nikvoid
Copy link
Author

nikvoid commented Aug 17, 2025

Yes. I made some of requested changes, but not sure what to do about kernel patch directory. I cannot test patches on other boards, so do I need to rename directory to rk3308-rockchip-something?

@coderabbitai coderabbitai bot added the BSP Board Support Packages label Aug 17, 2025
@igorpecovnik
Copy link
Member

igorpecovnik commented Aug 17, 2025

config/bootscripts/boot-rockchip64-ttyS4.cmd

Also dedicated bootloader script just for this board is not acceptable. We already have a maintaining nightmare and @djurny is trying to come out with some generic solution. Perhaps he can provide some tips how to address this part. Adding new board should have as minimal impact on common things as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
08 Milestone: Third quarter release BSP Board Support Packages Can be closed? Ping developers on stalled issue / PR 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.

6 participants