Skip to content

Conversation

@HeyMeco
Copy link
Collaborator

@HeyMeco HeyMeco commented Nov 21, 2025

Description

Fix Touch Screen orientation for Radxa Display 8HD with Rock5B+ (armbian/linux-rockchip#429)

How Has This Been Tested?

  • Apply udev rule manually and reboot -> Touchscreen works as expected

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced touchscreen support for Rock 5B+ board with optimized kernel driver configuration
    • Improved device orientation handling for Goodix touchscreen with updated system rules

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 21, 2025

Walkthrough

Configuration updates for Rock 5B Plus board to handle touchscreen udev rules and kernel driver settings. Renames an existing board configuration function to better reflect its udev rules handling purpose, adds Goodix touchscreen orientation rules, and modifies kernel touchscreen driver configuration from module to either built-in or disabled.

Changes

Cohort / File(s) Summary
Rock 5B Plus board configuration
config/boards/rock-5b-plus.conf
Renamed function post_family_tweaks__rock5b_naming_audios to post_family_tweaks__rock5bplus_udev_rules with updated log message. Added display alert for Goodix touch orientation and new udev rule for touchscreen orientation in 99-radxa-display8hd-touch.rules file.
Kernel RK35xx touchscreen drivers
config/kernel/linux-rk35xx-vendor.config
Changed CONFIG_TOUCHSCREEN_GOODIX from module (m) to disabled (not set). Changed CONFIG_TOUCHSCREEN_GT9XX from module (m) to built-in (y).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Function rename is straightforward with clear intent
  • Udev rule additions follow existing pattern
  • Kernel config changes are standard symbol toggling with no conditional logic

Suggested labels

BSP

Suggested reviewers

  • rpardini
  • krachlatte
  • igorpecovnik
  • catalinii
  • prahal
  • SuperKali
  • lanefu
  • efectn

Poem

🐰 A touch of the screen needs proper rules aligned,
Rock 5B Plus now has drivers refined,
Goodix finds its place, both kernel and rules,
Configuration tweaks—the rabbit's favorite tools! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically describes the main change: adding a udev rule for Radxa Display 8HD on Rock5B+, which aligns with the primary modifications across both files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • 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

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added 11 Milestone: Fourth quarter release size/small PR with less then 50 lines Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... labels Nov 21, 2025
@HeyMeco HeyMeco added this to the 25.11 milestone Nov 21, 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: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira 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 d5bceb9 and 454a37c.

📒 Files selected for processing (2)
  • config/boards/rock-5b-plus.conf (1 hunks)
  • config/kernel/linux-rk35xx-vendor.config (1 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.
Learnt from: HackingGate
Repo: armbian/build PR: 8665
File: config/boards/photonicat2.csc:4-4
Timestamp: 2025-10-26T10:41:35.118Z
Learning: In the Armbian build system, rk3576 boards consistently use BOARDFAMILY="rk35xx" for both vendor and edge kernel targets. The rk35xx family configuration sources rockchip64_common.inc, which provides edge and current kernel branch definitions, making these branches available even though they're not defined directly in rk35xx.conf.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8886
File: patch/kernel/rk35xx-vendor-6.1/dt/rk3528-nanopi-rev01.dts:9-9
Timestamp: 2025-11-08T06:39:24.527Z
Learning: In Armbian's Rockchip vendor kernel branches (rk35xx-vendor), device tree files may not require the same headers as mainline Linux. For example, input key definitions like KEY_BACK and KEY_VOLUMEUP may be handled differently in BSP kernels and don't necessarily need explicit includes like <dt-bindings/input/input.h> that would be required in mainline kernel device trees.
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.
Learnt from: libiunc
Repo: armbian/build PR: 8033
File: config/kernel/linux-starfive2-vendor.config:43-43
Timestamp: 2025-03-31T12:57:13.880Z
Learning: For StarFive2 platform kernel configurations, maintain alignment with vendor-provided configurations rather than modifying security settings like SECCOMP. This ensures hardware compatibility as intended by the manufacturer.
📚 Learning: 2025-11-02T20:49:56.719Z
Learnt from: igorpecovnik
Repo: armbian/build PR: 8849
File: config/boards/radxa-e54c.csc:14-28
Timestamp: 2025-11-02T20:49:56.719Z
Learning: In Armbian board configuration files (config/boards/*.conf, *.csc, etc.), do not use kernel_config_set, kernel_config_set_m, kernel_config_set_y, or custom_kernel_config__* functions to modify kernel configuration. Kernel configuration is associated with LINUXFAMILY/BOARDFAMILY, not individual BOARD. Board-specific kernel modifications cause inconsistency in kernel packages published to the apt repository because boards within a family share the same kernel packages. Kernel configuration changes must be made in the appropriate kernel config file (e.g., config/kernel/linux-*-*.config) or in family configuration files (config/sources/families/*.conf, *.inc) instead.

Applied to files:

  • config/boards/rock-5b-plus.conf
  • config/kernel/linux-rk35xx-vendor.config
📚 Learning: 2025-10-26T10:41:35.118Z
Learnt from: HackingGate
Repo: armbian/build PR: 8665
File: config/boards/photonicat2.csc:4-4
Timestamp: 2025-10-26T10:41:35.118Z
Learning: In the Armbian build system, rk3576 boards consistently use BOARDFAMILY="rk35xx" for both vendor and edge kernel targets. The rk35xx family configuration sources rockchip64_common.inc, which provides edge and current kernel branch definitions, making these branches available even though they're not defined directly in rk35xx.conf.

Applied to files:

  • config/boards/rock-5b-plus.conf
📚 Learning: 2025-09-14T06:32:29.806Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:222-230
Timestamp: 2025-09-14T06:32:29.806Z
Learning: In the Armbian build system, the write_uboot_platform() function implementations follow different patterns across Rockchip family files. The newer standard (used in rockchip64_common.inc and rk3506) includes 'status=none' parameter in dd commands, while older implementations (rk3288, rk322x) use an older pattern without this parameter. The rk3506 implementation correctly follows the current Rockchip family standard.

Applied to files:

  • config/boards/rock-5b-plus.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files, .wip and .conf extensions require BOARD_MAINTAINER to be present and contain at least one maintainer. If no maintainer is present, the board support rules state it does not qualify for standard support and must be moved to community support (.csc extension). This is documented at https://docs.armbian.com/User-Guide_Board-Support-Rules/

Applied to files:

  • config/boards/rock-5b-plus.conf
📚 Learning: 2025-09-18T03:36:17.862Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8537
File: config/boards/qcom-robotics-rb5.conf:1-1
Timestamp: 2025-09-18T03:36:17.862Z
Learning: In Armbian board configuration files (regardless of file extension), the standard pattern is to have only one line as a comment describing the board hardware specifications. This single-line description typically includes the SoC model, core count, RAM options, and key features like connectivity options, storage interfaces, and special features. The file extensions indicate board support status: .conf (standard support), .csc (community supported), .eos (end of support), .tvb (tvbox), .wip (work in progress).

Applied to files:

  • config/boards/rock-5b-plus.conf
📚 Learning: 2025-07-17T04:12:33.125Z
Learnt from: pyavitz
Repo: armbian/build PR: 8388
File: config/boards/kickpik2b.conf:1-14
Timestamp: 2025-07-17T04:12:33.125Z
Learning: In the Armbian build system, board configuration files (*.conf) follow a project standard of not including shebang lines, even though they contain bash code and are sourced by the build system. This is an established Armbian convention that individual contributors are expected to follow, and changes to this standard would require broader project maintainer approval.

Applied to files:

  • config/boards/rock-5b-plus.conf
📚 Learning: 2025-10-05T19:31:16.930Z
Learnt from: rpardini
Repo: armbian/build PR: 8718
File: config/boards/thinkpad-x13s.conf:185-185
Timestamp: 2025-10-05T19:31:16.930Z
Learning: In config/boards/thinkpad-x13s.conf, the x13s_obtain_firmware() function is reference-only utility code that never runs. It is kept for documentation purposes. Updates to firmware must be done at the armbian/firmware repository. TODO comments in this function are intentional reminders and should not be flagged.

Applied to files:

  • config/boards/rock-5b-plus.conf
📚 Learning: 2025-11-10T23:44:53.363Z
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-11-10T23:44:53.363Z
Learning: For the RV1106 family in Armbian (config/sources/families/rockchip-rv1106.conf), udev.children-max=1 is set in the kernel boot parameters to prevent early-boot OOM errors. The RV1106 has a single-core CPU, so parallelism in udev child processes provides no practical benefit. Testing higher values is impractical due to the difficulty of monitoring at this early boot stage.

Applied to files:

  • config/boards/rock-5b-plus.conf
📚 Learning: 2025-11-08T06:39:24.527Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8886
File: patch/kernel/rk35xx-vendor-6.1/dt/rk3528-nanopi-rev01.dts:9-9
Timestamp: 2025-11-08T06:39:24.527Z
Learning: In Armbian's Rockchip vendor kernel branches (rk35xx-vendor), device tree files may not require the same headers as mainline Linux. For example, input key definitions like KEY_BACK and KEY_VOLUMEUP may be handled differently in BSP kernels and don't necessarily need explicit includes like <dt-bindings/input/input.h> that would be required in mainline kernel device trees.

Applied to files:

  • config/boards/rock-5b-plus.conf
  • config/kernel/linux-rk35xx-vendor.config
📚 Learning: 2025-08-30T04:13:16.457Z
Learnt from: tabrisnet
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-08-30T04:13:16.457Z
Learning: Armbian kernel configuration files like linux-filogic-current.config are autogenerated overlays on top of arch defconfig. Comments added manually will be lost during future updates by maintainers, and explicit "CONFIG_OPTION is not set" statements aren't needed for mutually exclusive options since these are overlay configs that only specify changes from the base configuration.

Applied to files:

  • config/kernel/linux-rk35xx-vendor.config
📚 Learning: 2025-08-11T22:00:13.411Z
Learnt from: rafayahmed317
Repo: armbian/build PR: 8484
File: config/kernel/linux-rk35xx-vendor.config:904-906
Timestamp: 2025-08-11T22:00:13.411Z
Learning: In the linux-rk35xx-vendor kernel, the LTE driver options (CONFIG_LTE, CONFIG_LTE_RM310, CONFIG_LTE_EM05) are defined as boolean in drivers/net/lte/Kconfig and cannot be built as modules - they must be either built-in (=y) or disabled.

Applied to files:

  • config/kernel/linux-rk35xx-vendor.config
📚 Learning: 2025-09-27T21:50:15.915Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-sunxi64-current.config:94-94
Timestamp: 2025-09-27T21:50:15.915Z
Learning: When kernel config files are generated through Armbian's automated process (previous config → armbian-kernel.sh changes → make oldconfig → Armbian machinery processing), manual config file edits are not appropriate since they would be overwritten. Deprecated option handling should be implemented in the automated tooling instead.

Applied to files:

  • config/kernel/linux-rk35xx-vendor.config
📚 Learning: 2025-09-27T21:47:58.020Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-bcm2711-edge.config:859-861
Timestamp: 2025-09-27T21:47:58.020Z
Learning: In the Armbian build system, kernel configuration files in config/kernel/ are generated through an automated process: taking previous config → applying scripted changes from armbian-kernel.sh → running kernel's `make oldconfig` → processing by Armbian machinery back into config files. This automated process properly handles kernel configuration dependencies and reduces the likelihood of manual configuration errors.

Applied to files:

  • config/kernel/linux-rk35xx-vendor.config
📚 Learning: 2025-09-22T21:52:01.225Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: The kernel_config_set_m function in lib/functions/compilation/armbian-kernel.sh preserves existing 'y' (built-in) settings and only sets options to 'm' (module) when they are not already built-in, achieving "prefer modules but allow built-ins" behavior.

Applied to files:

  • config/kernel/linux-rk35xx-vendor.config
📚 Learning: 2025-09-22T21:52:01.225Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the kernel_config_modifying_hashes array is not universally required for all kernel configuration functions - some functions like armbian_kernel_config__netkit() operate without it, and adding entries with '=m' would be incorrect when the actual result might be '=y' for options already built-in.

Applied to files:

  • config/kernel/linux-rk35xx-vendor.config
🔇 Additional comments (2)
config/boards/rock-5b-plus.conf (2)

17-18: LGTM - Function rename reflects expanded scope.

The rename from post_family_tweaks__rock5b_naming_audios to post_family_tweaks__rock5bplus_udev_rules appropriately reflects that this function now handles both audio device naming and touchscreen orientation rules. The updated display alert message correctly identifies the board as "rock5bplus".


26-27: Touchscreen orientation rule implementation looks correct.

The udev rule correctly applies a libinput calibration matrix to fix touchscreen orientation for the Radxa Display 8HD. The matrix "0 -1 1 1 0 0" represents a 90-degree rotation transformation. The rule file name 99-radxa-display8hd-touch.rules is descriptive and follows naming conventions.

Note that this rule depends on the device being named "goodix-ts", which should be verified against the GT9XX driver implementation (see comment on the kernel config file).

The PR description mentions the touchscreen behavior was tested and works as expected after manual application and reboot, which provides good confidence in the calibration matrix values.

@github-actions
Copy link
Contributor

✅ This PR has been reviewed and approved — all set for merge!

@github-actions github-actions bot added Ready to merge Reviewed, tested and ready for merge and removed Needs review Seeking for review labels Nov 22, 2025
@igorpecovnik igorpecovnik merged commit 5540a53 into armbian:main Nov 22, 2025
1 check passed
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, ... Ready to merge Reviewed, tested and ready for merge size/small PR with less then 50 lines

Development

Successfully merging this pull request may close these issues.

2 participants