meson64-current: fix broken patch and overall rewrite patches #9020
Conversation
EvilOlaf
requested review from
NicoD-SBC,
SteeManMI,
Tonymac32,
adeepn,
igorpecovnik,
pyavitz,
rpardini and
teknoid
as code owners
WalkthroughCollection of kernel patches for meson64-6.12 adding board device-tree nodes (NPU, reboot), driver and keymap additions (multitouch, RC map), USB/hub quirk tweak, GPU/DRM timing updates, memory/nomap behavior change, hwmon OF integration, and pinctrl PWM pinmux additions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings🔇 Additional comments (2)
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. Comment |
github-actions
bot
added
02
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
patch/kernel/archive/meson64-6.12/board-t95z-add-rc-remote-keymap.patch (1)
66-66: Fix malformed email address in copyright header.The email address is missing the closing angle bracket '>', which creates a malformed copyright notice.
Apply this diff:
- * Copyright (c) 2023 Christian Hewitt <[email protected] + * Copyright (c) 2023 Christian Hewitt <[email protected]>patch/kernel/archive/meson64-6.12/general-memory-marked-nomap.patch (2)
20-36: Removal of memory overlap safety check – verify why secmon region fails overlap validation instead of removing the check.The patch removes validation that prevents marking already-reserved memory as
nomap. This conflicts with upstream kernel maintainer philosophy: recent discussions (2024–2025) on MEMBLOCK_NOMAP show that core maintainers prefer conservative, targeted fixes and expect firmware/device tree to properly declare unusable regions upfront rather than having the kernel remove safety checks.The correct approach for meson64 secure monitor regions is to declare them in the device tree's reserved-memory section with the
no-mapflag (as documented in upstream meson64 patches). If the secmon region is failing thememblock_overlaps_regionandmemblock_is_region_reservedchecks, this indicates:
- The bootloader may not be reserving secmon correctly, or
- The device tree reserved-memory declaration is missing or incomplete
Removing the check works around the problem rather than fixing it. Potential issues include:
- Memory conflicts if multiple subsystems attempt to use the same region
- Undefined behavior when reserved memory is marked unmapped without proper DTS declaration
- Risk of the workaround masking real firmware/bootloader issues
Verify why secmon reservation fails the existing overlap check and whether the meson64 DTS properly declares secmon in reserved-memory before bypassing these guards.
1-15: This "HACK" patch should be documented as temporary and tracked for replacement with proper upstream DTS fixes.The kernel issue with secmon@5000000 reservation failures is a known, recurring problem on Amlogic Meson boards. Web research confirms the proper upstream solution is not to bypass the overlap check, but to correct the device-tree reserved-memory nodes to match the actual bootloader/ATF layout. This patch masks a device-tree configuration problem rather than addressing it.
Before keeping this patch, you should:
- Document in the patch commit message or a separate file that this is temporary and the long-term plan is to align the meson64 DTS with upstream Amlogic fixes
- Reference upstream DTS patches that address secmon reserved regions properly (multiple patches exist for meson-gx/g12/t7 families)
- Track when this workaround can be removed once DTS configuration is corrected
The approach of removing validation is fragile and will diverge further from mainline if not addressed.
patch/kernel/archive/meson64-6.12/general-input-touchscreen-Add-D-WAV-Multitouch.patch (3)
151-169: Missing__packedattribute ondwav_rawstruct.The struct comment claims "Total 25 bytes" but the struct lacks
__packedattribute. On architectures with alignment requirements, the compiler may insert padding, causing incorrect parsing of USB packet data.Apply this fix:
-struct dwav_raw { /* Total 25 bytes */ +struct dwav_raw { /* Total 25 bytes */ unsigned char header; /* frame header 0xAA*/ unsigned char press; /* Touch flag (1:valid touch data, 0:touch finished) */ @@ -167,7 +167,7 @@ struct dwav_raw { /* Total 25 bytes */ unsigned short y5; unsigned short x5; unsigned char tail; /* frame end 0xCC */ -}; +} __packed;
580-586: Memory leak:dwav_usb_mt->fingernot freed in error path.When the probe function fails after allocating
dwav_usb_mt->finger(lines 495-497), theerr_free_memcleanup label freesinput_devanddwav_usb_mtbut never freesdwav_usb_mt->finger, causing a memory leak.Apply this fix:
err_free_mem: if (input_dev) input_free_device(input_dev); + kfree(dwav_usb_mt->finger); kfree(dwav_usb_mt); return err;
589-610: Memory leak:dwav_usb_mt->fingernot freed in disconnect.The disconnect function frees
dwav_usb_mtbut notdwav_usb_mt->finger, causing a memory leak on device removal.Apply this fix:
dwav_usb_mt_free_buffers(interface_to_usbdev(intf), dwav_usb_mt); + kfree(dwav_usb_mt->finger); kfree(dwav_usb_mt); }
🧹 Nitpick comments (9)
patch/kernel/archive/meson64-6.12/board-t95z-add-rc-remote-keymap.patch (2)
6-10: Remove WIP prefixes from commit message.The commit message contains multiple "WIP:" (Work In Progress) prefixes, which are unusual for production patches. These should be removed before merging to maintain a clean git history.
Apply this diff to clean up the commit message:
-WIP: media: rc: add keymap for Sunvell T95Z Plus -WIP: dt-bindings: media: rc: add rc-sunvell-t95z-plus +media: rc: add keymap for Sunvell T95Z Plus +dt-bindings: media: rc: add rc-sunvell-t95z-plus Add a binding for the rc-sunvell-t95z-plus remote keymap -WIP: dt-bindings: add sunvell vendor prefix +dt-bindings: add sunvell vendor prefix Add vendor prefix for Shenzhen Sunvell Electronics Co., Ltd
41-42: Add period to vendor description for consistency.The vendor description on line 42 is missing a trailing period, which is inconsistent with the typical format in vendor-prefixes.yaml files.
Apply this diff:
"^sunvell,.*": - description: Shenzhen Sunvell Electronics Co., Ltd + description: Shenzhen Sunvell Electronics Co., Ltd.patch/kernel/archive/meson64-6.12/general-input-touchscreen-Add-D-WAV-Multitouch.patch (4)
51-60: Kconfig help text is incomplete.The help text at line 59 says
module will be called dwav-usb-mt.but is missing the standard preamble. Compare with line 48-49 which shows the proper format:To compile this driver as a module, choose M here: the module will be called...Also, using
select USB(line 54) rather thandepends on USBis unconventional and may force-enable USB even when the user doesn't want it.Consider this fix:
config TOUCHSCREEN_DWAV_USB_MT tristate "D-WAV Scientific USB MultiTouch" depends on USB_ARCH_HAS_HCD - select USB + depends on USB help Say Y here if you have a D-WAV Scientific USB(HID) based MultiTouch controller. - module will be called dwav-usb-mt. + To compile this driver as a module, choose M here: the + module will be called dwav-usb-mt.
96-101: Duplicate vendor/device ID definitions.The vendor and device IDs are defined both in
hid-ids.h(via patch) and locally in the driver. This duplication is common for out-of-tree patches to maintain self-containment, but if this patch is intended for upstream, the driver should include<linux/hid.h>and use the centralized definitions fromhid-ids.h.
206-236:input_sync()andinput_mt_report_pointer_emulation()called inside loop.Both
input_mt_report_pointer_emulation()(line 234) andinput_sync()(line 235) are called inside the per-finger loop. The idiomatic pattern is to report all slots first, then call these functions once after the loop completes.for (id = 0; id < max_finger; id++) { if (dwav_usb_mt->finger[id].status == TS_EVENT_UNKNOWN) continue; if (dwav_usb_mt->finger[id].x >= max_x || dwav_usb_mt->finger[id].y >= max_y) continue; input_mt_slot(dwav_usb_mt->input, id); if (dwav_usb_mt->finger[id].status != TS_EVENT_RELEASE) { input_mt_report_slot_state(dwav_usb_mt->input, MT_TOOL_FINGER, true); input_report_abs(dwav_usb_mt->input, ABS_MT_POSITION_X, dwav_usb_mt->finger[id].x); input_report_abs(dwav_usb_mt->input, ABS_MT_POSITION_Y, dwav_usb_mt->finger[id].y); input_report_abs(dwav_usb_mt->input, ABS_MT_PRESSURE, max_press); } else { input_mt_report_slot_state(dwav_usb_mt->input, MT_TOOL_FINGER, false); dwav_usb_mt->finger[id].status = TS_EVENT_UNKNOWN; } - input_mt_report_pointer_emulation(dwav_usb_mt->input, true); - input_sync(dwav_usb_mt->input); } + input_mt_report_pointer_emulation(dwav_usb_mt->input, true); + input_sync(dwav_usb_mt->input); }
631-632: Missing newline at end of file.Line 632 indicates
\ No newline at end of file. While this doesn't affect functionality, it's a common convention to end files with a newline, and some tools may warn about this.Add a newline after
MODULE_ALIAS("dwav_usb_mt");patch/kernel/archive/meson64-6.12/hwmon-emc2305-fixups-for-driver.patch (3)
68-73: pwm_max integration is sound; consider optional validation of DT overridesExtending
struct emc2305_datawithpwm_max, initializing it in both pdata and non-pdata probe paths, and switchingemc2305_set_pwm()to cap againstdata->pwm_maxgives DT a clean way to narrow the usable PWM range without affecting legacy users.If you are comfortable diverging slightly from the RPi-origin patch, an optional robustness improvement would be to sanity-check DT-provided
pwm-min/pwm-maxagainst[EMC2305_FAN_MIN..EMC2305_FAN_MAX]and ensurepwm_min <= pwm_max, so obviously bad DT doesn’t only surface later as unexpected-EINVALfrom set_pwm.Also applies to: 79-81, 171-186
131-152: devm_thermal_of_cooling_device_register usage is balanced; avoid storing ERR_PTR in cdev_data (optional)Switching to
devm_thermal_of_cooling_device_register()whendev->of_nodeis present and guardingthermal_cooling_device_unregister()withif (!dev->of_node)makes the OF and non-OF teardown paths consistent and avoids double-unregistering devm-managed cooling devices.One optional robustness improvement is to avoid persisting
ERR_PTR()values intocdev_data[cdev_idx].cdevon registration failure (i.e., only assign into the array after theIS_ERRcheck). That reduces the chance of future cleanup or debug code accidentally acting on an error pointer.Also applies to: 155-167
1-21: Verify rewritten patch against upstream RPi commit to avoid REWRITE_PATCHES driftSince this patch is derived from the Raspberry Pi tree (commit
233096b8a9023f7e02960543c85447d46af81e81in the header) and you mention an overall meson64-current rewrite, it would be good to diffdrivers/hwmon/emc2305.cafter applying this patch against that upstream commit to confirm there’s no semantic drift introduced by the rewrite tooling (in particular around DT properties and cooling-device wiring), then reuse the same patch for other kernel branches as needed. Based on learnings, REWRITE_PATCHES previously caused subtle DT/interrupt changes that only surfaced later.
📜 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.
📒 Files selected for processing (10)
patch/kernel/archive/meson64-6.12/board-khadas-vim3-add-npu-node.patch(2 hunks)patch/kernel/archive/meson64-6.12/board-odroid-sm1-reset.patch(3 hunks)patch/kernel/archive/meson64-6.12/board-odroidc2-usb-hub-disable-autosuspend-for-Genesys-Logic-.patch(1 hunks)patch/kernel/archive/meson64-6.12/board-t95z-add-rc-remote-keymap.patch(1 hunks)patch/kernel/archive/meson64-6.12/general-gpu-drm-add-new-display-resolution-2560x1440.patch(1 hunks)patch/kernel/archive/meson64-6.12/general-input-touchscreen-Add-D-WAV-Multitouch.patch(2 hunks)patch/kernel/archive/meson64-6.12/general-memory-marked-nomap.patch(1 hunks)patch/kernel/archive/meson64-6.12/general-usb-core-improve-handling-of-hubs-with-no-ports.patch(1 hunks)patch/kernel/archive/meson64-6.12/hwmon-emc2305-fixups-for-driver.patch(4 hunks)patch/kernel/archive/meson64-6.12/meson-g12b-pinctrl-Add-missing-pinmux-for-pwm.patch(6 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: EvilOlaf
Repo: armbian/build PR: 8968
File: patch/u-boot/u-boot-sunxi/arm64-dts-sun50i-h6-orangepi.dtsi-Rollback-r_rsb-to-r_i2c.patch:36-36
Timestamp: 2025-11-20T18:20:11.985Z
Learning: The rewrite-patches tool (REWRITE_PATCHES=yes) in the Armbian build system can inadvertently introduce semantic changes when the u-boot/kernel git base revision differs from expected state. The tool applies patches, commits them, and re-exports them using git format-patch, which can cause the re-exported patch to reflect the base revision's state rather than preserving the original patch intent. This is particularly problematic for device tree changes like interrupt specifications. The tool currently lacks validation mechanisms to detect such semantic drift, and affected patches must be manually corrected after rewriting.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.
Learnt from: leggewie
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-08-29T18:44:47.732Z
Learning: When creating GitHub issues for code improvements in the Armbian build project, include the relevant patches/diffs in the issue description or open a PR directly instead of just describing the changes. This makes it more efficient for the assignee to handle the improvements.
Learnt from: glneo
Repo: armbian/build PR: 8913
File: config/sources/families/include/k3_common.inc:57-60
Timestamp: 2025-11-11T20:56:20.303Z
Learning: In config/sources/families/include/k3_common.inc, the OP-TEE build command at line 59 should be updated in a future PR to explicitly set CROSS_COMPILE64=aarch64-linux-gnu- and CROSS_COMPILE32=arm-linux-gnueabihf- instead of relying on OP-TEE's internal defaults, for better clarity and maintainability. User glneo agreed to address this in a separate PR.
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-199
Timestamp: 2025-09-25T18:37:00.330Z
Learning: In PR armbian/build#8661, line 235 of lib/functions/compilation/armbian-kernel.sh already contains the corrected comment "BPF link support for netfilter hooks" for NETFILTER_BPF_LINK, not the misleading "BPF_SYSCALL" comment that was flagged during review.
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:65-72
Timestamp: 2025-09-14T11:37:35.089Z
Learning: In the Armbian build system, patch directories referenced in BOOTPATCHDIR and KERNELPATCHDIR configurations can be non-existent without causing build failures. Empty patch directories are also ignored by git, so missing patch directories should not be flagged as errors during code review.
📚 Learning: 2025-11-20T18:20:11.985Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8968
File: patch/u-boot/u-boot-sunxi/arm64-dts-sun50i-h6-orangepi.dtsi-Rollback-r_rsb-to-r_i2c.patch:36-36
Timestamp: 2025-11-20T18:20:11.985Z
Learning: The rewrite-patches tool (REWRITE_PATCHES=yes) in the Armbian build system can inadvertently introduce semantic changes when the u-boot/kernel git base revision differs from expected state. The tool applies patches, commits them, and re-exports them using git format-patch, which can cause the re-exported patch to reflect the base revision's state rather than preserving the original patch intent. This is particularly problematic for device tree changes like interrupt specifications. The tool currently lacks validation mechanisms to detect such semantic drift, and affected patches must be manually corrected after rewriting.
Applied to files:
patch/kernel/archive/meson64-6.12/board-odroidc2-usb-hub-disable-autosuspend-for-Genesys-Logic-.patchpatch/kernel/archive/meson64-6.12/board-khadas-vim3-add-npu-node.patchpatch/kernel/archive/meson64-6.12/board-odroid-sm1-reset.patch
📚 Learning: 2025-07-23T07:30:52.265Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8417
File: config/boards/orangepi5pro.csc:57-58
Timestamp: 2025-07-23T07:30:52.265Z
Learning: In the Armbian build system, BOOTPATCHDIR can contain board-specific subdirectories (e.g., board_orangepi5pro) for applying patches to specific boards only. The framework automatically checks if such board-specific subdirectories exist for the board being built and applies those patches accordingly.
Applied to files:
patch/kernel/archive/meson64-6.12/board-odroid-sm1-reset.patch
📚 Learning: 2025-09-14T11:37:35.089Z
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:65-72
Timestamp: 2025-09-14T11:37:35.089Z
Learning: In the Armbian build system, patch directories referenced in BOOTPATCHDIR and KERNELPATCHDIR configurations can be non-existent without causing build failures. Empty patch directories are also ignored by git, so missing patch directories should not be flagged as errors during code review.
Applied to files:
patch/kernel/archive/meson64-6.12/board-odroid-sm1-reset.patch
📚 Learning: 2025-03-31T22:20:48.475Z
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
Applied to files:
patch/kernel/archive/meson64-6.12/board-odroid-sm1-reset.patch
📚 Learning: 2025-03-31T22:20:41.849Z
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
Applied to files:
patch/kernel/archive/meson64-6.12/board-odroid-sm1-reset.patch
🔇 Additional comments (25)
patch/kernel/archive/meson64-6.12/board-khadas-vim3-add-npu-node.patch (2)
1-4: Patch metadata and formatting look consistentHeader (From/Subject/index) and Armbian trailer match the usual style used by the Armbian rewrite tooling and kernel patch format. I don’t see any metadata issues that would affect application or traceability.
Also applies to: 11-11, 26-27
18-21: Enabling&npunode is minimal and low‑riskThis hunk only flips the existing
&npunode tostatus = "okay";, without touching clocks, interrupts, or other DT properties. That keeps the rewrite safe from the semantic‑drift problems we’ve seen with more complex DT patches, while achieving the intended enablement for Khadas VIM3 NPU. Based on learnings, this limited scope is good for avoiding REWRITE_PATCHES‑induced surprises.patch/kernel/archive/meson64-6.12/board-odroidc2-usb-hub-disable-autosuspend-for-Genesys-Logic-.patch (1)
22-28: Quirk constant now matches the stated “disable autosuspend” intentSwitching
.driver_infofromHUB_QUIRK_CHECK_PORT_AUTOSUSPENDtoHUB_QUIRK_DISABLE_AUTOSUSPENDaligns the code with the commit subject and message (fully disabling autosuspend on Genesys Logic hubs, rather than merely checking per-port autosuspend). The hunk is minimal and self‑contained, so it looks safe with respect to the rewrite‑patches tooling; semantics are exactly the stronger quirk you describe for GXBB/Odroid‑C2 and won’t affect dwc3‑based SoCs mentioned in the header. Based on learnings, this looks like a faithful rewrite.If you haven’t already, please re‑check on a C2 that USB devices enumerating behind the Genesys hub still behave as expected (no regressions in suspend/resume) and that the broader 6.12.y kernel build completes without additional USB warnings.
patch/kernel/archive/meson64-6.12/general-usb-core-improve-handling-of-hubs-with-no-ports.patch (1)
39-48: Zero‑port hub handling matches upstream behavior and intentThis change mirrors the upstream
hub_configure()patch from Heiner Kallweit: zero‑port hubs now emit andev_info()and immediately return-ENODEVinstead of going through the genericfailpath with an error message. That keeps the functional outcome (hub rejected with-ENODEV) while downgrading log severity for the “hub has no ports” case seen on S905W‑class SoCs, and it matches the accepted upstream implementation.(lists.infradead.org)Please confirm that this patch was rebased verbatim from the same upstream commit against your 6.12 base (no extra hunks introduced by REWRITE_PATCHES), and that dmesg on affected meson64 boards now shows the info‑level line instead of the old error.
patch/kernel/archive/meson64-6.12/board-t95z-add-rc-remote-keymap.patch (4)
54-54: LGTM!The Makefile entry is correctly added in alphabetical order with proper formatting.
77-113: LGTM!The keymap table is well-structured with valid NEC protocol scancodes and appropriate Linux input key codes. The inline comments helpfully document alternative key functions.
115-138: LGTM!The module initialization, registration, and metadata follow the standard kernel RC subsystem patterns correctly.
147-147: LGTM!The RC map macro is correctly defined with proper naming convention and alphabetical ordering, matching the name used in the keymap implementation.
patch/kernel/archive/meson64-6.12/meson-g12b-pinctrl-Add-missing-pinmux-for-pwm.patch (3)
1-17: Patch metadata looks good.The patch header follows standard git format-patch conventions with proper attribution, clear subject line, and commit message that references hardware documentation. The file change summary accurately reflects the modifications.
27-67: Device tree pinctrl nodes are well-structured and consistent.The five new PWM pinctrl node definitions follow a consistent pattern with proper nesting and appropriate
bias-disablesettings for PWM outputs. The group-to-function mappings are logical and match the PWM channel naming (pwm_b_h → pwm_b, pwm_c_z → pwm_c, etc.).
72-130: Pinctrl driver changes are consistent with device tree definitions.All five new PWM pins are properly defined with:
- Pin array definitions mapping to specific GPIO pins (GPIOH_7, GPIOZ_0/1/2, GPIOA_4)
- Group registrations in
meson_g12a_periphs_groups[]with appropriate mux function numbers- Function group array updates that correctly associate each pin with its PWM channel
The mappings between device tree nodes and driver definitions are complete and consistent across all five new pins. However, given the rewrite-patches tool can introduce semantic changes when git base revisions differ, verification that this patch applies cleanly to the 6.12 kernel tree is recommended to ensure no semantic drift occurred during re-export.
patch/kernel/archive/meson64-6.12/board-odroid-sm1-reset.patch (2)
1-12: Patch header/trailer rewrite looks consistent and preserves authorshipHeader now uses a zero
FromSHA, simplified subject, and anArmbiantrailer, in line with how REWRITE_PATCHES exports kernel patches in this tree. Both originalSigned-off-bylines are intact and the file/summary lines match the actual diff. No changes requested here.Based on learnings, this looks like a purely mechanical rewrite without the kind of DT semantic drift we previously saw from REWRITE_PATCHES.
Also applies to: 15-15, 36-37
22-30: meson64-reboot DT node matches known-good fix; just confirm old C4-only patch is goneThe new
reboot: meson64-rebootnode looks correct:
compatible = "meson64,reboot";matches the meson64 reboot driver expectations seen in Armbian forum examples. (forum.armbian.com)sys_reset = <0x84000009>;andsys_poweroff = <0x84000008>;are the standard PSCI SYSTEM_RESET/System_OFF function IDs. (lists.linaro.org)- The three GPIOs use
&gpio_aowith pinsGPIOE_2,GPIOAO_6, andGPIOAO_3, which aligns with prior fixes that moved the TF_IO control to the AO bank on these boards. (lists.infradead.org)One thing to double-check at the PR level: ensure the old
board-odroidc4-reset.patch(which patched only the C4 DTS) is removed or adjusted so you don’t end up with twomeson64-rebootnodes for C4/HC4 once this common dtsi change lands. Duplicated root-level nodes with the same name would cause DT compilation issues.If you want, I can sketch a small grep/script to scan the patch set for any remaining meson64-reboot additions in board-specific DTS files.
patch/kernel/archive/meson64-6.12/general-gpu-drm-add-new-display-resolution-2560x1440.patch (2)
18-24: HDMI_241500 VCLK definition and PLL case look self‑consistentThe new
MESON_VCLK_HDMI_241500enum, itsmeson_vclk_paramsentry, and thecase 4830000:block inmeson_vclk_set()line up correctly:
.pll_freq = 4830000matches the new switchcase 4830000:.- The divider chain in the comment (
4830 /2 /1 /2 /5 /1 => /1 /1) matches the struct fields (pll_od1 = 2,pll_od2 = 1,pll_od3 = 2,vid_pll_div = VID_PLL_DIV_5,vclk_div = 1), yielding a 241.5 MHz pixel clock as intended.- All derived frequencies (
phy_freq,venc_freq,vclk_freq,pixel_freq) are internally consistent at 2.415 GHz / 241.5 MHz.From the patch alone there’s no sign of rewrite drift in these values; they appear to be a faithful lift of the vendor settings. Please just make sure, during testing, that a 2560x1440@60 mode actually selects this VCLK entry and comes up stable on real hardware. Based on learnings, this guards against subtle REWRITE_PATCHES regressions in clock setup.
Also applies to: 27-43, 46-53
61-67: Horizontal mode limit bump correctly enables 2560‑wide HDMI modesRaising the
hdisplayupper bound inmeson_venc_hdmi_supported_mode()from 1920 to 2560 while keeping the lower bound at 400 is exactly what’s needed to admit 2560x1440; the existingvdisplayupper bound of 1920 already covers 1440, so no further change is required here. This is a minimal, targeted change that shouldn’t affect other modes.patch/kernel/archive/meson64-6.12/general-input-touchscreen-Add-D-WAV-Multitouch.patch (6)
19-28: HID IDs addition looks correct.The vendor and device IDs for ODROID VU5/VU7PLUS are properly defined. Note that
USB_DEVICE_ID_DWAV_MULTITOUCHis defined here but the driver usesUSB_DEVICE_ID_VU7(0x0005) defined locally in the driver source. This is consistent since both values are 0x0005, but the naming differs.
34-41: HID quirks fix looks correct.Adding ODROID VU5 and VU7PLUS to
hid_ignore_listensures these devices bypass the generic HID driver, allowing the custom D-WAV touchscreen driver to handle them. This appears to be the fix mentioned in the PR description for the broken patch due to hid-quirks.c changes.
70-71: Makefile entry is correct.
296-334: URB completion handler is correctly implemented.Proper handling of URB status codes, appropriate use of
GFP_ATOMICin interrupt context, andusb_mark_last_busy()for autosuspend.
336-409: Power management callbacks are properly implemented.The open/close and suspend/resume handlers follow standard USB input driver patterns with proper URB lifecycle management and autopm reference counting.
250-280: Verify byte order conversion against D-WAV device protocol documentation.The concern about
cpu_to_be16()is technically sound—device coordinate data should be converted FROM device format TO CPU byte order usingbe16_to_cpu()orle16_to_cpu(), not the reverse. However, verification requires confirming the D-WAV device's native byte order format. Standard USB HID devices typically use little-endian, which would suggestle16_to_cpu()is correct. Confirm the device protocol and apply the appropriate fix accordingly.patch/kernel/archive/meson64-6.12/hwmon-emc2305-fixups-for-driver.patch (4)
31-37: New FAN_ register macros and removal of EMC2305_DEVICE look consistent*The added FAN_STATUS, FAN_STALL_STATUS, and FAN_CFG(n) offsets follow the same 0x10 stride pattern as the existing FAN_DRIVE/MIN_DRIVE/TACH macros, and dropping
EMC2305_DEVICEis fine as long as there are no remaining references (the build will surface any).Also applies to: 45-47
54-63: OF match table correctly wires EMC230x compatibles into the driverThe
emc2305_dt_idstable covers bothsmsc,*andmicrochip,emc230[1-5]compatibles, and hooking it into.of_match_tableensures proper DT-based binding without altering non-DT users.Also applies to: 203-207
88-129: emc2305_get_tz_of cleanly handles optional DT propertiesThe helper cleanly treats
emc2305,cooling-levels,emc2305,pwm-max,emc2305,pwm-min, andemc2305,pwm-channelas optional overrides on top of sane defaults, while propagating real parse errors. This keeps non-DT and DT users aligned and lets board DTS tune behavior without needing platform data.
193-197: Fault acknowledgement reads at the end of probe are reasonableReading
EMC2305_REG_FAN_STALL_STATUSandEMC2305_REG_FAN_STATUSunconditionally at the end of probe to clear any pre-existing faults and silence SMBus alerts is a sensible initialization step, and ignoring failures here keeps probe behavior simple while still attempting to de-latch faults.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
patch/kernel/archive/meson64-6.12/general-input-touchscreen-Add-D-WAV-Multitouch.patch (2)
630-632: Missing newline at end of file.The file is missing a trailing newline character, which may cause warnings from some compilers and tools.
MODULE_ALIAS("dwav_usb_mt"); +
250-294: Usele16_to_cpu()for coordinate conversion, notcpu_to_be16().The USB HID specification (HID 1.11) mandates little-endian byte order for multibyte numeric fields in HID reports. D-WAV USB touchscreen devices follow this standard and send coordinates as little-endian 16-bit values. The current code uses
cpu_to_be16()which converts to big-endian—the opposite of what is needed. Replace withle16_to_cpu()to correctly convert the device's little-endian coordinates to the CPU's native format. On big-endian systems, this bug would cause coordinate values to be completely incorrect; on little-endian systems, accidental correctness masks the semantic error.
📜 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.
📒 Files selected for processing (10)
patch/kernel/archive/meson64-6.12/board-khadas-vim3-add-npu-node.patch(2 hunks)patch/kernel/archive/meson64-6.12/board-odroid-sm1-reset.patch(3 hunks)patch/kernel/archive/meson64-6.12/board-odroidc2-usb-hub-disable-autosuspend-for-Genesys-Logic-.patch(1 hunks)patch/kernel/archive/meson64-6.12/board-t95z-add-rc-remote-keymap.patch(1 hunks)patch/kernel/archive/meson64-6.12/general-gpu-drm-add-new-display-resolution-2560x1440.patch(1 hunks)patch/kernel/archive/meson64-6.12/general-input-touchscreen-Add-D-WAV-Multitouch.patch(2 hunks)patch/kernel/archive/meson64-6.12/general-memory-marked-nomap.patch(1 hunks)patch/kernel/archive/meson64-6.12/general-usb-core-improve-handling-of-hubs-with-no-ports.patch(1 hunks)patch/kernel/archive/meson64-6.12/hwmon-emc2305-fixups-for-driver.patch(4 hunks)patch/kernel/archive/meson64-6.12/meson-g12b-pinctrl-Add-missing-pinmux-for-pwm.patch(6 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: EvilOlaf
Repo: armbian/build PR: 8968
File: patch/u-boot/u-boot-sunxi/arm64-dts-sun50i-h6-orangepi.dtsi-Rollback-r_rsb-to-r_i2c.patch:36-36
Timestamp: 2025-11-20T18:20:11.985Z
Learning: The rewrite-patches tool (REWRITE_PATCHES=yes) in the Armbian build system can inadvertently introduce semantic changes when the u-boot/kernel git base revision differs from expected state. The tool applies patches, commits them, and re-exports them using git format-patch, which can cause the re-exported patch to reflect the base revision's state rather than preserving the original patch intent. This is particularly problematic for device tree changes like interrupt specifications. The tool currently lacks validation mechanisms to detect such semantic drift, and affected patches must be manually corrected after rewriting.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:76-86
Timestamp: 2025-03-31T22:20:48.475Z
Learning: For the Armbian build project, maintaining consistency with existing patches across U-Boot versions (such as between 2025.01 and 2025.04) is prioritized over refactoring individual patches for code improvements.
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
Learnt from: leggewie
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-08-29T18:44:47.732Z
Learning: When creating GitHub issues for code improvements in the Armbian build project, include the relevant patches/diffs in the issue description or open a PR directly instead of just describing the changes. This makes it more efficient for the assignee to handle the improvements.
Learnt from: glneo
Repo: armbian/build PR: 8913
File: config/sources/families/include/k3_common.inc:57-60
Timestamp: 2025-11-11T20:56:20.303Z
Learning: In config/sources/families/include/k3_common.inc, the OP-TEE build command at line 59 should be updated in a future PR to explicitly set CROSS_COMPILE64=aarch64-linux-gnu- and CROSS_COMPILE32=arm-linux-gnueabihf- instead of relying on OP-TEE's internal defaults, for better clarity and maintainability. User glneo agreed to address this in a separate PR.
Learnt from: amazingfate
Repo: armbian/build PR: 8619
File: config/sources/families/rockchip.conf:65-72
Timestamp: 2025-09-14T11:37:35.089Z
Learning: In the Armbian build system, patch directories referenced in BOOTPATCHDIR and KERNELPATCHDIR configurations can be non-existent without causing build failures. Empty patch directories are also ignored by git, so missing patch directories should not be flagged as errors during code review.
📚 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:
patch/kernel/archive/meson64-6.12/board-t95z-add-rc-remote-keymap.patch
📚 Learning: 2025-11-20T18:20:11.985Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8968
File: patch/u-boot/u-boot-sunxi/arm64-dts-sun50i-h6-orangepi.dtsi-Rollback-r_rsb-to-r_i2c.patch:36-36
Timestamp: 2025-11-20T18:20:11.985Z
Learning: The rewrite-patches tool (REWRITE_PATCHES=yes) in the Armbian build system can inadvertently introduce semantic changes when the u-boot/kernel git base revision differs from expected state. The tool applies patches, commits them, and re-exports them using git format-patch, which can cause the re-exported patch to reflect the base revision's state rather than preserving the original patch intent. This is particularly problematic for device tree changes like interrupt specifications. The tool currently lacks validation mechanisms to detect such semantic drift, and affected patches must be manually corrected after rewriting.
Applied to files:
patch/kernel/archive/meson64-6.12/board-khadas-vim3-add-npu-node.patchpatch/kernel/archive/meson64-6.12/meson-g12b-pinctrl-Add-missing-pinmux-for-pwm.patchpatch/kernel/archive/meson64-6.12/board-odroid-sm1-reset.patch
🔇 Additional comments (18)
patch/kernel/archive/meson64-6.12/board-odroidc2-usb-hub-disable-autosuspend-for-Genesys-Logic-.patch (1)
22-27: LGTM!The quirk change from
HUB_QUIRK_CHECK_PORT_AUTOSUSPENDtoHUB_QUIRK_DISABLE_AUTOSUSPENDis a reasonable fix for the Odroid C2 USB issues. The patch header clearly documents the rationale and scope (GXBB devices only).patch/kernel/archive/meson64-6.12/general-memory-marked-nomap.patch (1)
20-35: Acceptable workaround with documented rationale.The patch is appropriately labeled as a "HACK" and addresses a known issue with reserved memory reporting on Amlogic platforms since Linux 5.10. The removed guard was causing false positives for the
secmon@5000000node, and while it removes a safety check, this is a well-established workaround in the Amlogic ecosystem.patch/kernel/archive/meson64-6.12/general-usb-core-improve-handling-of-hubs-with-no-ports.patch (1)
39-48: LGTM!The early return with
-ENODEVis appropriate here since at this point inhub_configure, no cleanup-requiring resources have been allocated for the zero-port hub case. Changing the message from error to info level correctly reflects that this is an expected hardware limitation on some Amlogic SoCs (S905W) rather than an actual error condition.patch/kernel/archive/meson64-6.12/board-t95z-add-rc-remote-keymap.patch (2)
37-45: LGTM!The vendor prefix for Sunvell is correctly added in alphabetical order and follows the standard pattern for vendor-prefixes.yaml entries.
58-138: LGTM!The RC keymap driver follows the standard kernel pattern with proper module initialization, SPDX license identifier, and NEC protocol usage. The keymap table is well-structured with clear comments indicating button functions.
patch/kernel/archive/meson64-6.12/board-khadas-vim3-add-npu-node.patch (1)
18-21: LGTM!Simple and correct DT node enablement for the A311D NPU. The
&npureference points to the NPU node defined in the parent SoC dtsi.patch/kernel/archive/meson64-6.12/general-gpu-drm-add-new-display-resolution-2560x1440.patch (3)
18-42: LGTM!The VCLK parameters for 241.5 MHz pixel clock (2560x1440@60Hz) are correctly calculated. The PLL chain dividers (4830000 / 2 / 1 / 2 / 5 / 1 = 241500) properly derive the target pixel frequency.
46-56: LGTM!The PLL m and frac values for the 4830 MHz case complete the configuration for the new resolution.
61-69: LGTM!Extending the horizontal display limit from 1920 to 2560 correctly enables 2560-wide modes. The vertical limit at 1920 still accommodates 1440 vertical.
patch/kernel/archive/meson64-6.12/board-odroid-sm1-reset.patch (1)
22-30: LGTM!The reboot node correctly defines the ARM Trusted Firmware SMC call IDs for system reset (0x84000009) and poweroff (0x84000008), along with the SD card power GPIO references needed for proper power sequencing on Odroid SM1 devices.
patch/kernel/archive/meson64-6.12/general-input-touchscreen-Add-D-WAV-Multitouch.patch (2)
19-29: LGTM!The HID device IDs are correctly defined and properly placed at the end of the hid-ids.h file.
34-41: LGTM!Adding the ODROID VU5 and VU7PLUS to the HID ignore list ensures these devices are handled by the dedicated touchscreen driver rather than the generic HID subsystem.
patch/kernel/archive/meson64-6.12/meson-g12b-pinctrl-Add-missing-pinmux-for-pwm.patch (1)
1-134: Patch structure and pinctrl additions look correct.The patch properly adds missing PWM pinmux definitions for the G12B family (A311D/S922X):
- Pin arrays are correctly defined following the existing naming convention
- GROUP() macros use appropriate function mux values (2, 3, 5)
- Device tree bindings follow the standard pinmux node format with
muxsub-nodes- Function group arrays are properly extended
The changes are consistent with the A311D Quick Reference Manual and S922X Public Datasheet as stated in the commit message.
patch/kernel/archive/meson64-6.12/hwmon-emc2305-fixups-for-driver.patch (5)
54-62: OF device ID table looks reasonable.The compatible strings cover the main variants. Note that only
emc2305has thesmsccompatible string while the other variants (emc2301/2302/2303) only havemicrochipcompatibles. This appears intentional based on the device history (SMSC was acquired by Microchip).
88-129: OF property parsing logic is correct.The function properly handles optional device tree properties by treating
-EINVAL(property not found) as acceptable while propagating other errors. Thereturn 0at line 128 correctly indicates success after all optional properties have been processed.
131-166: Cooling device registration properly uses managed API for OF path.The conditional registration approach is correct:
devm_thermal_of_cooling_device_register()for OF nodes — cleanup is automaticthermal_cooling_device_register()fallback for non-OF — requires manual unregisterThe corresponding change in
emc2305_unset_tz(lines 162-166) correctly skips manual unregistration when OF is used.
189-198: Fault acknowledgment at probe end is appropriate.Reading the status registers to clear existing faults is a sensible approach to prevent spurious SMBus alerts. Not checking return values is acceptable here since this is best-effort fault clearing at the end of probe — failure would only mean the device might continue alerting, which is non-critical.
170-186: Probe correctly initializespwm_maxbefore OF property override.The initialization properly sets
pwm_max = EMC2305_FAN_MAXin both platform data and default paths (lines 174 and 180) before the optional OF property parsing can override it. This ensures a valid default is always set.
|
✅ This PR has been reviewed and approved — all set for merge! |
2 participants
Description
hid-quircks.c.This change now hit
currentand broke latest 6.12.y compile.How Has This Been Tested?
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.