uefi-x86: bump edge to 6.19-rc1
#9086
Conversation
WalkthroughBumps edge kernel from 6.18→6.19 and applies a set of Apple-related kernel patches: adds Apple BCE, VHCI fixes, i915/gmux quirks, extensive applesmc refactors (per-device state, dynamic port base, ACPI/MMIO support, T2 fan/trackpad features), HID/magicmouse updates, and an i915 fbdev framebuffer validation change. Changes
Sequence Diagram(s)sequenceDiagram
participant ACPI as ACPI bus
participant Driver as applesmc driver
participant HW as Apple SMC (Port/MMIO)
participant Sysfs as userspace/sysfs
ACPI->>Driver: acpi_device add (applesmc_add)
Driver->>Driver: walk CRS resources (ports / iomem)
alt MMIO resource present
Driver->>HW: map MMIO region
HW-->>Driver: MMIO accessible
Driver->>Driver: enable MMIO path (port_read/write -> mmio_read/write)
else no MMIO (or enable failed)
Driver->>HW: request I/O ports (port_base)
HW-->>Driver: ports reserved
Driver->>Driver: enable port I/O path
end
Driver->>Sysfs: create nodes (fans, BCLM, sensors)
Sysfs->>Driver: user read/write (e.g., set fan/BCLM)
Driver->>Driver: select path (MMIO ? mmio_* : port_*)
Driver->>HW: perform read/write
HW-->>Driver: return data/status
Driver-->>Sysfs: return result to userspace
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas to focus during review:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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: 9
🧹 Nitpick comments (6)
patch/kernel/archive/uefi-x86-6.19/4004-HID-magicmouse-Add-support-for-trackpads-found-on-T2.patch (1)
239-264: Consider simplifying return pattern.Both
magicmouse_setup_input_mtpandmagicmouse_setup_input_spihave redundant return logic. The patternif (ret) return ret; return 0;can be simplified toreturn ret;.static int magicmouse_setup_input_mtp(struct input_dev *input, struct hid_device *hdev) { int ret = magicmouse_setup_input_int_tpd(input, hdev, J314_TP_MIN_X, J314_TP_MIN_Y, J314_TP_MAX_X, J314_TP_MAX_Y, J314_TP_RES_X, J314_TP_RES_Y, true); - if (ret) - return ret; - - return 0; + return ret; } static int magicmouse_setup_input_spi(struct input_dev *input, struct hid_device *hdev) { int ret = magicmouse_setup_input_int_tpd(input, hdev, J314_TP_MIN_X, J314_TP_MIN_Y, J314_TP_MAX_X, J314_TP_MAX_Y, J314_TP_RES_X, J314_TP_RES_Y, true); - if (ret) - return ret; - - return 0; + return ret; }patch/kernel/archive/uefi-x86-6.19/3008-applesmc-make-applesmc_remove-void.patch (1)
19-30: Optional: Remove redundant bare return statement.The change to
voidreturn type is correct for kernel 6.2+ ACPI driver compatibility. However, the barereturn;at the end of a void function is redundant and can be removed entirely.- return; }patch/kernel/archive/uefi-x86-6.19/3005-applesmc-basic-mmio-interface-implementation.patch (2)
97-106: Potential strict-aliasing violation with pointer cast.Casting
const char *keydirectly tou32 *and dereferencing may violate strict aliasing rules, leading to undefined behavior on some compilers with aggressive optimizations. Consider usingmemcpyorget_unaligned_le32/get_unaligned_be32for safe type punning.- u32 key_int = *((u32 *) key); + u32 key_int; + memcpy(&key_int, key, sizeof(key_int));The same pattern appears in
iomem_get_smc_key_type(line 144) andiomem_write_smc(line 174).
133-136: Unconditionalprint_hex_dump_bytesmay cause log spam.
print_hex_dump_bytesalways prints to the kernel log regardless of debug level. This could cause significant log noise on every SMC read/write. Consider making these conditional on the dynamic debug flag or removing them for production.This also applies to lines 189 and 194 in
iomem_write_smc.patch/kernel/archive/uefi-x86-6.19/3006-applesmc-fan-support-on-T2-Macs.patch (1)
51-61: Potential undefined behavior with negative shift amount.If
expexceeds 23 (which represents values ≥ 2^24 ≈ 16.7M), the expression(23 - exp)becomes negative, causing undefined behavior in the right shift. While fan speeds are unlikely to reach such values, adding a bounds check would be defensive.static inline u32 applesmc_float_to_u32(u32 d) { u8 sign = (u8) ((d >> 31) & 1); s32 exp = (s32) ((d >> 23) & 0xff) - 0x7f; u32 fr = d & ((1u << 23) - 1); - if (sign || exp < 0) + if (sign || exp < 0 || exp > 23) return 0; return (u32) ((1u << exp) + (fr >> (23 - exp))); }patch/kernel/archive/uefi-x86-6.19/3001-applesmc-convert-static-structures-to-drvdata.patch (1)
830-849: Staticdata_lengthpersists across devices.The
data_lengthvariable at line 833 remainsstatic, meaning it's shared across all instances. If multiple AppleSMC devices with different light sensor data lengths exist, the first device's length would be used for all. This is pre-existing behavior but worth noting in a multi-device refactor.Consider moving
data_lengthintostruct applesmc_deviceif multi-device support is intended:struct applesmc_device { ... + int light_sensor_data_length; };
📜 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 (18)
config/sources/families/include/uefi_common.inc(1 hunks)patch/kernel/archive/uefi-x86-6.19/1002-Put-apple-bce-in-drivers-staging.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/1003-Fix-freezing-on-turning-off-camera.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/2008-i915-4-lane-quirk-for-mbp15-1.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/2009-apple-gmux-allow-switching-to-igpu-at-probe.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/3001-applesmc-convert-static-structures-to-drvdata.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/3002-applesmc-make-io-port-base-addr-dynamic.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/3003-applesmc-switch-to-acpi_device-from-platform.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/3004-applesmc-key-interface-wrappers.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/3005-applesmc-basic-mmio-interface-implementation.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/3006-applesmc-fan-support-on-T2-Macs.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/3007-applesmc-Add-iMacPro-to-applesmc_whitelist.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/3008-applesmc-make-applesmc_remove-void.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/3009-applesmc-battery-charge-limiter.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/4003-HID-apple-ignore-the-trackpad-on-T2-Macs.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/4004-HID-magicmouse-Add-support-for-trackpads-found-on-T2.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/4005-HID-magicmouse-fix-regression-breaking-support-for-M.patch(1 hunks)patch/kernel/archive/uefi-x86-6.19/7001-drm-i915-fbdev-Discard-BIOS-framebuffers-exceeding-h.patch(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.
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 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: 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: 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: 8746
File: config/sources/families/filogic.conf:61-68
Timestamp: 2025-10-13T02:26:18.249Z
Learning: In the Armbian build system, the "edge" kernel branch is intended to be bleeding-edge and experimental. It does not require hardware testing or stability verification before merging, as it's expected to contain potentially unstable or untested code for early adopters and developers.
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-07-21T04:12:02.439Z
Learning: In the Armbian build system, for recurring maintenance tasks like kernel version bumping, TODO comments should use generic version formats (e.g., "MAJOR.MINOR-rc1") rather than specific version numbers (e.g., "6.17-rc1") to avoid the need for frequent comment updates that would create unnecessary maintenance overhead.
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: SuperKali
Repo: armbian/build PR: 8608
File: lib/functions/compilation/patch/drivers_network.sh:361-363
Timestamp: 2025-09-11T06:06:52.328Z
Learning: For the RTW88 SDIO RF path detection fix patch (004-rtw88-sdio-rf-path-detection-fix.patch), SuperKali prefers using exact version matching (eq 6.1 || eq 6.16) rather than range checks, as the patch has only been tested on those specific kernel versions and may be mainstreamed soon.
📚 Learning: 2025-11-06T15:36:04.682Z
Learnt from: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.
Applied to files:
config/sources/families/include/uefi_common.incpatch/kernel/archive/uefi-x86-6.19/4003-HID-apple-ignore-the-trackpad-on-T2-Macs.patchpatch/kernel/archive/uefi-x86-6.19/3006-applesmc-fan-support-on-T2-Macs.patchpatch/kernel/archive/uefi-x86-6.19/2008-i915-4-lane-quirk-for-mbp15-1.patchpatch/kernel/archive/uefi-x86-6.19/3005-applesmc-basic-mmio-interface-implementation.patchpatch/kernel/archive/uefi-x86-6.19/1002-Put-apple-bce-in-drivers-staging.patchpatch/kernel/archive/uefi-x86-6.19/3007-applesmc-Add-iMacPro-to-applesmc_whitelist.patchpatch/kernel/archive/uefi-x86-6.19/3003-applesmc-switch-to-acpi_device-from-platform.patchpatch/kernel/archive/uefi-x86-6.19/4004-HID-magicmouse-Add-support-for-trackpads-found-on-T2.patchpatch/kernel/archive/uefi-x86-6.19/3002-applesmc-make-io-port-base-addr-dynamic.patch
📚 Learning: 2025-11-10T22:05:40.490Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8913
File: config/sources/families/k3-beagle.conf:16-16
Timestamp: 2025-11-10T22:05:40.490Z
Learning: In the Armbian build system, kernel branches using non-mainline/vendor forks (like BeagleBoard's linux repository) should be named "vendor" or "vendor-rt" rather than "current" or "edge". The "current" and "edge" naming is reserved for mainline kernel branches. This affects both the case statement in family config files (e.g., `vendor | vendor-rt)` instead of `current | current-rt)`) and the corresponding KERNEL_TARGET declarations in board config files.
Applied to files:
config/sources/families/include/uefi_common.inc
📚 Learning: 2025-12-12T23:10:00.819Z
Learnt from: tabrisnet
Repo: armbian/build PR: 9058
File: config/sources/families/spacemit.conf:39-45
Timestamp: 2025-12-12T23:10:00.819Z
Learning: In the Armbian build system, vendor kernel forks can use "vendor-edge" branch naming for bleeding-edge or pre-release vendor kernels, as demonstrated in config/sources/families/k3.conf. The typical vendor branch naming pattern is: "vendor" or "vendor-rt" for stable vendor releases, and "vendor-edge" for bleeding-edge/pre-release vendor versions. The "edge" naming without "vendor-" prefix is reserved for mainline kernel branches.
Applied to files:
config/sources/families/include/uefi_common.inc
📚 Learning: 2025-04-30T16:16:47.150Z
Learnt from: The-going
Repo: armbian/build PR: 8147
File: config/sources/families/include/sunxi64_common.inc:38-39
Timestamp: 2025-04-30T16:16:47.150Z
Learning: The Armbian build system references Linux kernel versions in the form "tag:v6.14.4" in the KERNELBRANCH variable, even when point release tags might not be directly visible in the upstream repository in the same form.
Applied to files:
config/sources/families/include/uefi_common.inc
📚 Learning: 2025-07-21T04:12:02.439Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-07-21T04:12:02.439Z
Learning: In the Armbian build system, for recurring maintenance tasks like kernel version bumping, TODO comments should use generic version formats (e.g., "MAJOR.MINOR-rc1") rather than specific version numbers (e.g., "6.17-rc1") to avoid the need for frequent comment updates that would create unnecessary maintenance overhead.
Applied to files:
config/sources/families/include/uefi_common.inc
📚 Learning: 2025-10-13T02:26:18.249Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8746
File: config/sources/families/filogic.conf:61-68
Timestamp: 2025-10-13T02:26:18.249Z
Learning: In the Armbian build system, the "edge" kernel branch is intended to be bleeding-edge and experimental. It does not require hardware testing or stability verification before merging, as it's expected to contain potentially unstable or untested code for early adopters and developers.
Applied to files:
config/sources/families/include/uefi_common.inc
📚 Learning: 2025-06-25T03:40:52.109Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:40:52.109Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, when KERNELSOURCE is not explicitly declared in a case branch, the system falls back to a default KERNELSOURCE value, so missing KERNELSOURCE declarations are not critical issues.
Applied to files:
config/sources/families/include/uefi_common.inc
📚 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/sources/families/include/uefi_common.inc
📚 Learning: 2025-06-25T03:42:09.086Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8330
File: config/sources/families/sun55iw3.conf:32-36
Timestamp: 2025-06-25T03:42:09.086Z
Learning: In Armbian build system configuration files like config/sources/families/*.conf, KERNELSOURCE is explicitly declared when using unofficial or 3rd party kernel repositories (like the "dev" branch using https://github.com/apritzel/linux), but can be omitted when using the standard mainline kernel (like the "edge" branch) since it will fall back to the default mainline source.
Applied to files:
config/sources/families/include/uefi_common.inc
📚 Learning: 2025-07-27T15:54:35.119Z
Learnt from: pyavitz
Repo: armbian/build PR: 8421
File: config/kernel/linux-sunxi64-edge.config:2168-2173
Timestamp: 2025-07-27T15:54:35.119Z
Learning: In the Armbian build system, staging 802.11 drivers like CONFIG_RTLLIB, CONFIG_RTL8192E, CONFIG_R8712U are kept enabled alongside upstream rtw88 drivers because rtw88 is still under development and suffers from reliability issues. The staging drivers serve as necessary fallbacks when upstream drivers are unstable, prioritizing working hardware over avoiding technical conflicts.
Applied to files:
patch/kernel/archive/uefi-x86-6.19/1002-Put-apple-bce-in-drivers-staging.patch
🔇 Additional comments (34)
patch/kernel/archive/uefi-x86-6.19/2008-i915-4-lane-quirk-for-mbp15-1.patch (1)
1-76: Well-structured i915 quirk patch following kernel conventions.This patch correctly implements the DDI A 4-lane forcing quirk for MacBookPro15,1:
- The quirk handler function (
quirk_ddi_a_force_4_lanes) follows the established pattern of other quirk handlers in the same file- The quirk check in
intel_ddi_a_force_4_lanes()is correctly placed after the existingddi_a_4_lanescheck- The documentation comment clearly explains the dual-GPU Mac boot scenario
- The PCI IDs (0x3e9b for Coffee Lake GT2, 0x106b for Apple, 0x0176 for MBP15,1 subsystem) are correctly formatted
This is consistent with the T2-specific patches from the linux-t2 project for the uefi-x86 edge kernel. Based on learnings, uefi-x86 edge branches are specifically configured for Apple T2-based x86 machines.
patch/kernel/archive/uefi-x86-6.19/1002-Put-apple-bce-in-drivers-staging.patch (2)
68-73: LGTM!The Makefile conversion from
obj-m(out-of-tree) toobj-$(CONFIG_APPLE_BCE)(in-tree conditional) is correct for staging driver integration. The module object list is preserved properly.
32-39: Good placement strategy for Armbian compatibility.Placing the
obj-$(CONFIG_APPLE_BCE)entry near the top of the staging Makefile as documented in the patch header helps avoid merge conflicts with Armbian's WiFi driver patches. Based on learnings, this is a T2-specific driver for Apple hardware support.config/sources/families/include/uefi_common.inc (1)
45-45: LGTM. Kernel version bump to 6.19 for edge branch is correct and follows the existing pattern. Linux 6.19-rc1 is available, and the patches directory atpatch/kernel/archive/uefi-x86-6.19exists with the required patches for Apple T2 hardware support.patch/kernel/archive/uefi-x86-6.19/2009-apple-gmux-allow-switching-to-igpu-at-probe.patch (3)
20-34: LGTM: Probe deferral logic is correct.The addition of the probe deferral check follows standard kernel patterns and properly coordinates with vga_switcheroo for GMUX-based GPU switching.
52-61: LGTM: Symbol export is appropriate.Exporting
vga_set_default_devicewith GPL licensing allows the apple-gmux module to programmatically set the default VGA device, which is necessary for the IGD switching functionality.
34-51: The behavioral change is intentional and documented. The commit message explicitly states this is a distribution-specific patch not meant for upstream, designed to allow both GPUs to probe at startup. The removedpdev != vga_default_device()check is by design—the patch includes complementary logic in apple-gmux.c with aforce_igdmodule parameter to control GPU switching at probe time. The gmux driver then manages which GPU becomes the default device. This approach allows users to use the iGPU without requiring EFI variable configuration.patch/kernel/archive/uefi-x86-6.19/1003-Fix-freezing-on-turning-off-camera.patch (5)
21-21: Good approach to preserve the original state.Introducing the
old_statevariable is the correct solution for maintaining consistent logic flow. The state is captured before any modifications, which prevents the logic checks from being affected by the early state change to CANCELLED.Also applies to: 30-30
32-36: Effective guard against double-cancellation.The early return when the URB is already CANCELLED prevents redundant operations and potential race conditions. The spinlock protection ensures thread-safety. Returning 0 is appropriate since the cancellation is already in progress.
42-42: Correct usage of old_state for logic flow decisions.Using
old_stateinstead ofvurb->statein these conditionals is essential because the state was set to CANCELLED earlier in the function. This preserves the original logic:
- Line 42: Determines whether queue pause is needed based on whether the URB was already posted
- Line 51: Decides whether to resume the queue based on the original state
Also applies to: 51-51
59-67: Appropriate addition of CANCELLED state to the enum.The new
BCE_VHCI_URB_CANCELLEDstate is correctly added to the enum. Placement at the end maintains ABI compatibility, and the formatting follows kernel coding conventions.
38-38: The early CANCELLED state transition is properly protected and intentionally designed.The patch correctly prevents race conditions by:
- Setting state early while holding the spinlock
- Checking for double-cancellation using the saved old_state
- Using old_state for all subsequent logic decisions (lines 42, 51)
This design ensures that concurrent code cannot observe a partially-cancelled URB state, and no code path depends on observing intermediate states during cancellation.
patch/kernel/archive/uefi-x86-6.19/4003-HID-apple-ignore-the-trackpad-on-T2-Macs.patch (2)
41-75: LGTM!The
APPLE_IGNORE_MOUSEflag is consistently added to all 9 WELLSPRINGT2 device entries, ensuring T2 trackpads are properly routed to the hid-magicmouse driver.
32-33: Operator precedence bug: add parentheses around bitwise AND.The
&operator has lower precedence than&&, soquirks & APPLE_IGNORE_MOUSE && hdev->type == HID_TYPE_USBMOUSEis parsed asquirks & (APPLE_IGNORE_MOUSE && hdev->type == HID_TYPE_USBMOUSE). This will produce incorrect results.Apply this diff:
- if (quirks & APPLE_IGNORE_MOUSE && hdev->type == HID_TYPE_USBMOUSE) + if ((quirks & APPLE_IGNORE_MOUSE) && hdev->type == HID_TYPE_USBMOUSE)Likely an incorrect or invalid review comment.
patch/kernel/archive/uefi-x86-6.19/4005-HID-magicmouse-fix-regression-breaking-support-for-M.patch (1)
25-48: LGTM!The fix correctly consolidates the
HID_ANY_IDcase into thedefaultpath, ensuring Magic Trackpad 1 devices receive proper report registration via the inner switch's default case.patch/kernel/archive/uefi-x86-6.19/4004-HID-magicmouse-Add-support-for-trackpads-found-on-T2.patch (2)
23-121: LGTM!The per-model touchpad dimension macros follow the existing pattern and correctly use float casts for resolution calculations.
314-417: LGTM!The probe logic correctly handles T2 devices with appropriate bus/type checks, and the device ID table is complete for all 9 T2 Mac models. The structure aligns well with the related patch 4003 for hid-apple coordination. Based on learnings, this follows the established T2-specific patch pattern for the uefi-x86 family.
patch/kernel/archive/uefi-x86-6.19/7001-drm-i915-fbdev-Discard-BIOS-framebuffers-exceeding-h.patch (2)
26-28: LGTM!The message update from "too small" to "not valid" correctly reflects the broader condition check that now catches both undersized and oversized BIOS framebuffers.
21-25: Verify version targeting for this BIOS framebuffer fix patch.The logic change from
>to!=correctly addresses display corruption on 4K panels by discarding any mismatched BIOS framebuffer, not just undersized ones. However, this patch is dated January 2022 and is being carried forward across kernel versions 6.12, 6.18, and 6.19 in armbian, suggesting it remains non-mainlined. Consider applying exact version matching (e.g.,eq 6.19) rather than carrying it across multiple kernel releases, in case the fix is mainlined in a future release.patch/kernel/archive/uefi-x86-6.19/3007-applesmc-Add-iMacPro-to-applesmc_whitelist.patch (1)
18-29: LGTM!The DMI whitelist entry for iMacPro follows the established pattern used for other Apple hardware (Macmini, MacPro). The patch is well-formed with proper attribution.
patch/kernel/archive/uefi-x86-6.19/3005-applesmc-basic-mmio-interface-implementation.patch (1)
295-332: LGTM on MMIO initialization and cleanup logic.The
applesmc_try_enable_iomemfunction has proper error handling with clean goto-based cleanup, and theiomem_base_setflag correctly controls MMIO usage and cleanup. The LDKN version check (≥2) ensures MMIO is only enabled on compatible hardware.patch/kernel/archive/uefi-x86-6.19/3006-applesmc-fan-support-on-T2-Macs.patch (2)
196-224: Control flow in legacy path is correct.The legacy
FANS_MANUALpath properly checks the read result before attempting the bit manipulation and write. Both the new per-fan key path and the legacy bitmask path correctly propagate errors to the commonout:label.
84-114: LGTM on float type detection and conversion.The implementation correctly detects FLOAT_TYPE entries and applies appropriate conversion. The soft-float implementation handles typical fan speed ranges appropriately.
Verify the float value endianness matches SMC hardware expectations on T2 Macs.
patch/kernel/archive/uefi-x86-6.19/3002-applesmc-make-io-port-base-addr-dynamic.patch (3)
36-44: Well-structured preparation for dynamic port base.The refactor cleanly converts hardcoded port addresses to per-device offsets. The
port_basefield initialization is handled in subsequent patches (3003) where ACPI resources are parsed.
107-229: LGTM on I/O function refactoring.All port I/O functions (
send_command,send_byte,read_smc,write_smc, etc.) are consistently updated to use the per-devicesmc->port_baseoffset. The parameter threading is correct throughout the call chains.
283-307: Module init/exit correctly use the port base constant.The
request_regionandrelease_regioncalls in module init/exit are updated to useAPPLESMC_PORT_BASEconsistently, maintaining the same I/O region management as before while allowing the per-deviceport_baseto vary in ACPI scenarios.patch/kernel/archive/uefi-x86-6.19/3009-applesmc-battery-charge-limiter.patch (2)
35-45: LGTM on the show function.The
applesmc_BCLM_showfunction correctly reads the BCLM key and formats it for sysfs output usingsysfs_emit.
62-92: LGTM on module lifecycle integration.The BCLM_group is correctly integrated into
applesmc_create_moduleswith proper error handling and reverse-order cleanup in both the error path andapplesmc_destroy_modules.patch/kernel/archive/uefi-x86-6.19/3004-applesmc-key-interface-wrappers.patch (2)
263-295: LGTM on the entry-by-index refactoring.The refactored
applesmc_get_entry_by_indexproperly uses the new wrapper functions and maintains the same functionality with cleaner separation of concerns.
233-244: The return value check change fromif (ret)toif (ret < 0)is correct and intentional. Theread_smcwrapper only returns 0 on success or negative error codes—no positive values are possible. All underlying port-level functions (port_wait_status,port_send_command, etc.) follow the kernel convention of returning 0 for success and negative error codes (-EIO) for failures. This change makes the error checking semantically explicit and aligns with standard Linux kernel conventions; it does not mask errors.patch/kernel/archive/uefi-x86-6.19/3003-applesmc-switch-to-acpi_device-from-platform.patch (2)
121-145: LGTM on ACPI resource walker.The
applesmc_walk_resourcescallback correctly handles ACPI resource enumeration: it validates IO resource length, sets port_base only once, and properly signals completion or failure at END_TAG.
174-201: LGTM on ACPI driver definition.The ACPI driver structure correctly defines the device ID table (APP0001), ops callbacks, and PM operations. This follows standard kernel ACPI driver patterns.
patch/kernel/archive/uefi-x86-6.19/3001-applesmc-convert-static-structures-to-drvdata.patch (2)
421-467: LGTM on probe/remove lifecycle management.The
applesmc_probefunction correctly allocates, initializes, and registers per-device state with proper error handling and cleanup paths. Theapplesmc_removefunction properly tears down in reverse order with mutex destruction.
1006-1040: LGTM on per-device backlight handling.The workqueue and LED classdev are properly initialized per-device. Using
container_ofinapplesmc_backlight_setcorrectly retrieves the device context from the work struct.
patch/kernel/archive/uefi-x86-6.19/1002-Put-apple-bce-in-drivers-staging.patch
Outdated
Show resolved
Hide resolved
| +static bool force_igd; | ||
| +module_param(force_igd, bool, 0); | ||
| +MODULE_PARM_DESC(force_idg, "Switch gpu to igd on module load. Make sure that you have apple-set-os set up and the iGPU is in `lspci -s 00:02.0`. (default: false) (bool)"); |
There was a problem hiding this comment.
Fix typos in module parameter name.
Lines 82 and 100 use force_idg instead of force_igd (transposed 'd' and 'g').
Apply this diff to fix the typos:
-MODULE_PARM_DESC(force_idg, "Switch gpu to igd on module load. Make sure that you have apple-set-os set up and the iGPU is in `lspci -s 00:02.0`. (default: false) (bool)");
+MODULE_PARM_DESC(force_igd, "Switch gpu to igd on module load. Make sure that you have apple-set-os set up and the iGPU is in `lspci -s 00:02.0`. (default: false) (bool)");- pr_err("force_idg is true, but couldn't find iGPU at 00:02.0! Is apple-set-os working?");
+ pr_err("force_igd is true, but couldn't find iGPU at 00:02.0! Is apple-set-os working?");Also applies to: 100-100
🤖 Prompt for AI Agents
In
patch/kernel/archive/uefi-x86-6.19/2009-apple-gmux-allow-switching-to-igpu-at-probe.patch
around lines 80-82 (and also at line 100), the MODULE_PARM_DESC uses the
misspelled parameter name "force_idg"; update those occurrences to "force_igd"
to match the declared static bool force_igd and the module_param call, keeping
the rest of the description text unchanged.
| + if (force_igd) { | ||
| + struct pci_dev *pdev; | ||
| + | ||
| + pdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(2, 0)); | ||
| + if (pdev) { | ||
| + pr_info("Switching to IGD"); | ||
| + gmux_switchto(VGA_SWITCHEROO_IGD); | ||
| + vga_set_default_device(pdev); | ||
| + } else { | ||
| + pr_err("force_idg is true, but couldn't find iGPU at 00:02.0! Is apple-set-os working?"); | ||
| + } | ||
| + } |
There was a problem hiding this comment.
Resource leak: Missing pci_dev_put() after pci_get_domain_bus_and_slot().
Line 94 calls pci_get_domain_bus_and_slot() which increments the PCI device reference count, but there's no corresponding pci_dev_put() to release it. This creates a reference leak.
Apply this diff to fix the leak:
if (force_igd) {
struct pci_dev *pdev;
pdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(2, 0));
if (pdev) {
pr_info("Switching to IGD");
gmux_switchto(VGA_SWITCHEROO_IGD);
vga_set_default_device(pdev);
+ pci_dev_put(pdev);
} else {
pr_err("force_igd is true, but couldn't find iGPU at 00:02.0! Is apple-set-os working?");
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| + if (force_igd) { | |
| + struct pci_dev *pdev; | |
| + | |
| + pdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(2, 0)); | |
| + if (pdev) { | |
| + pr_info("Switching to IGD"); | |
| + gmux_switchto(VGA_SWITCHEROO_IGD); | |
| + vga_set_default_device(pdev); | |
| + } else { | |
| + pr_err("force_idg is true, but couldn't find iGPU at 00:02.0! Is apple-set-os working?"); | |
| + } | |
| + } | |
| if (force_igd) { | |
| struct pci_dev *pdev; | |
| pdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(2, 0)); | |
| if (pdev) { | |
| pr_info("Switching to IGD"); | |
| gmux_switchto(VGA_SWITCHEROO_IGD); | |
| vga_set_default_device(pdev); | |
| pci_dev_put(pdev); | |
| } else { | |
| pr_err("force_igd is true, but couldn't find iGPU at 00:02.0! Is apple-set-os working?"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
patch/kernel/archive/uefi-x86-6.19/2009-apple-gmux-allow-switching-to-igpu-at-probe.patch
around lines 91 to 102, pci_get_domain_bus_and_slot() is used to obtain pdev but
pci_dev_put() is never called; add a corresponding pci_dev_put(pdev) after
you're done with the device: call pci_dev_put(pdev) after
vga_set_default_device() in the success path and also call pci_dev_put(pdev) in
the else/error path (or restructure to a single cleanup label) so the reference
is always released.
|
|
||
| static void applesmc_idev_poll(struct input_dev *idev) | ||
| { | ||
| + struct applesmc_device *smc = dev_get_drvdata(&idev->dev); | ||
| s16 x, y; | ||
|
|
||
| - if (applesmc_read_s16(MOTION_SENSOR_X_KEY, &x)) | ||
| + if (applesmc_read_s16(smc, MOTION_SENSOR_X_KEY, &x)) | ||
| return; | ||
| - if (applesmc_read_s16(MOTION_SENSOR_Y_KEY, &y)) | ||
| + if (applesmc_read_s16(smc, MOTION_SENSOR_Y_KEY, &y)) | ||
| return; | ||
|
|
||
| x = -x; | ||
| - input_report_abs(idev, ABS_X, x - rest_x); | ||
| - input_report_abs(idev, ABS_Y, y - rest_y); | ||
| + input_report_abs(idev, ABS_X, x - smc->rest_x); | ||
| + input_report_abs(idev, ABS_Y, y - smc->rest_y); | ||
| input_sync(idev); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for the patch file and examine it
find . -name "*3001-applesmc-convert-static-structures-to-drvdata.patch" -type f | head -1Repository: armbian/build
Length of output: 150
🏁 Script executed:
cat ./patch/kernel/archive/uefi-x86-6.12/3001-applesmc-convert-static-structures-to-drvdata.patchRepository: armbian/build
Length of output: 37354
Add missing drvdata initialization for input device.
applesmc_idev_poll retrieves smc via dev_get_drvdata(&idev->dev), but the patch never calls dev_set_drvdata(&smc->idev->dev, smc) to initialize this. Setting the parent device does not propagate drvdata. Add dev_set_drvdata(&smc->idev->dev, smc); in applesmc_create_accelerometer() after allocating the input device, or this will cause a NULL pointer dereference when the polling function runs.
| +static int applesmc_init_resources(struct applesmc_device *smc) | ||
| +{ | ||
| + int ret; | ||
| + | ||
| + ret = acpi_walk_resources(smc->dev->handle, METHOD_NAME__CRS, | ||
| + applesmc_walk_resources, smc); | ||
| + if (ACPI_FAILURE(ret)) | ||
| + return -ENXIO; | ||
| + | ||
| + if (!request_region(smc->port_base, APPLESMC_NR_PORTS, "applesmc")) | ||
| + return -ENXIO; | ||
| + | ||
| + return 0; | ||
| +} | ||
| + | ||
| +static void applesmc_free_resources(struct applesmc_device *smc) | ||
| +{ | ||
| + release_region(smc->port_base, APPLESMC_NR_PORTS); | ||
| +} |
There was a problem hiding this comment.
Potential issue: applesmc_free_resources may release unrequested region.
If acpi_walk_resources succeeds but request_region fails at line 156, the function returns -ENXIO but port_base_set remains true. Later cleanup via applesmc_free_resources would call release_region on a region that was never successfully requested.
Consider adding a guard or resetting state on failure:
static int applesmc_init_resources(struct applesmc_device *smc)
{
int ret;
ret = acpi_walk_resources(smc->dev->handle, METHOD_NAME__CRS,
applesmc_walk_resources, smc);
if (ACPI_FAILURE(ret))
return -ENXIO;
- if (!request_region(smc->port_base, APPLESMC_NR_PORTS, "applesmc"))
+ if (!request_region(smc->port_base, APPLESMC_NR_PORTS, "applesmc")) {
+ smc->port_base_set = false;
return -ENXIO;
+ }
return 0;
}
static void applesmc_free_resources(struct applesmc_device *smc)
{
+ if (!smc->port_base_set)
+ return;
release_region(smc->port_base, APPLESMC_NR_PORTS);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| +static int applesmc_init_resources(struct applesmc_device *smc) | |
| +{ | |
| + int ret; | |
| + | |
| + ret = acpi_walk_resources(smc->dev->handle, METHOD_NAME__CRS, | |
| + applesmc_walk_resources, smc); | |
| + if (ACPI_FAILURE(ret)) | |
| + return -ENXIO; | |
| + | |
| + if (!request_region(smc->port_base, APPLESMC_NR_PORTS, "applesmc")) | |
| + return -ENXIO; | |
| + | |
| + return 0; | |
| +} | |
| + | |
| +static void applesmc_free_resources(struct applesmc_device *smc) | |
| +{ | |
| + release_region(smc->port_base, APPLESMC_NR_PORTS); | |
| +} | |
| static int applesmc_init_resources(struct applesmc_device *smc) | |
| { | |
| int ret; | |
| ret = acpi_walk_resources(smc->dev->handle, METHOD_NAME__CRS, | |
| applesmc_walk_resources, smc); | |
| if (ACPI_FAILURE(ret)) | |
| return -ENXIO; | |
| if (!request_region(smc->port_base, APPLESMC_NR_PORTS, "applesmc")) { | |
| smc->port_base_set = false; | |
| return -ENXIO; | |
| } | |
| return 0; | |
| } | |
| static void applesmc_free_resources(struct applesmc_device *smc) | |
| { | |
| if (!smc->port_base_set) | |
| return; | |
| release_region(smc->port_base, APPLESMC_NR_PORTS); | |
| } |
🤖 Prompt for AI Agents
In
patch/kernel/archive/uefi-x86-6.19/3003-applesmc-switch-to-acpi_device-from-platform.patch
around lines 147 to 165, applesmc_free_resources currently always calls
release_region which can release a region that was never successfully requested
if request_region failed; fix by tracking whether the region was successfully
requested (e.g. set a smc->port_base_set flag immediately after a successful
request_region) and only call release_region when that flag is true, and ensure
applesmc_init_resources clears/does not set the flag on failure so cleanup is
safe.
| + bool port_base_set, iomem_base_set; | ||
| u16 port_base; | ||
| + u8 *__iomem iomem_base; | ||
| + u32 iomem_base_addr, iomem_base_size; |
There was a problem hiding this comment.
Incorrect __iomem annotation placement.
The __iomem attribute should be placed between the * and the variable name for proper sparse checking.
- u8 *__iomem iomem_base;
+ u8 __iomem *iomem_base;🤖 Prompt for AI Agents
In
patch/kernel/archive/uefi-x86-6.19/3005-applesmc-basic-mmio-interface-implementation.patch
around lines 54-57, the __iomem attribute placement is wrong; change the
declaration u8 *__iomem iomem_base; to place the annotation between the '*' and
the variable name (i.e., u8 * __iomem iomem_base) so sparse recognizes the MMIO
pointer, leaving the other variables unchanged.
| +static ssize_t applesmc_BCLM_store(struct device *dev, | ||
| + struct device_attribute *attr, char *sysfsbuf, size_t count) | ||
| +{ | ||
| + struct applesmc_device *smc = dev_get_drvdata(dev); | ||
| + u8 val; | ||
| + | ||
| + if (kstrtou8(sysfsbuf, 10, &val) < 0) | ||
| + return -EINVAL; | ||
| + | ||
| + if (val < 0 || val > 100) | ||
| + return -EINVAL; | ||
| + | ||
| + if (applesmc_write_key(smc, "BCLM", &val, 1)) | ||
| + return -ENODEV; | ||
| + return count; | ||
| +} |
There was a problem hiding this comment.
Fix function signature and remove dead code in applesmc_BCLM_store.
Two issues in this function:
-
Line 19: The
sysfsbufparameter should beconst char *to match the kernel'ssysfs_ops.storesignature. Usingchar *may cause compiler warnings or type mismatches. -
Line 27: The condition
val < 0is always false sincevalis declared asu8(unsigned). Onlyval > 100is meaningful.
static ssize_t applesmc_BCLM_store(struct device *dev,
- struct device_attribute *attr, char *sysfsbuf, size_t count)
+ struct device_attribute *attr, const char *sysfsbuf, size_t count)
{
struct applesmc_device *smc = dev_get_drvdata(dev);
u8 val;
if (kstrtou8(sysfsbuf, 10, &val) < 0)
return -EINVAL;
- if (val < 0 || val > 100)
+ if (val > 100)
return -EINVAL;
if (applesmc_write_key(smc, "BCLM", &val, 1))
return -ENODEV;
return count;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| +static ssize_t applesmc_BCLM_store(struct device *dev, | |
| + struct device_attribute *attr, char *sysfsbuf, size_t count) | |
| +{ | |
| + struct applesmc_device *smc = dev_get_drvdata(dev); | |
| + u8 val; | |
| + | |
| + if (kstrtou8(sysfsbuf, 10, &val) < 0) | |
| + return -EINVAL; | |
| + | |
| + if (val < 0 || val > 100) | |
| + return -EINVAL; | |
| + | |
| + if (applesmc_write_key(smc, "BCLM", &val, 1)) | |
| + return -ENODEV; | |
| + return count; | |
| +} | |
| static ssize_t applesmc_BCLM_store(struct device *dev, | |
| struct device_attribute *attr, const char *sysfsbuf, size_t count) | |
| { | |
| struct applesmc_device *smc = dev_get_drvdata(dev); | |
| u8 val; | |
| if (kstrtou8(sysfsbuf, 10, &val) < 0) | |
| return -EINVAL; | |
| if (val > 100) | |
| return -EINVAL; | |
| if (applesmc_write_key(smc, "BCLM", &val, 1)) | |
| return -ENODEV; | |
| return count; | |
| } |
🤖 Prompt for AI Agents
In patch/kernel/archive/uefi-x86-6.19/3009-applesmc-battery-charge-limiter.patch
around lines 18 to 33, update the applesmc_BCLM_store function signature so the
sysfsbuf parameter is declared const char *sysfsbuf to match the kernel
sysfs_ops.store prototype, and remove the dead check "if (val < 0)" since val is
a u8; keep only the validation for val > 100 and the existing kstrtou8,
applesmc_write_key call, and return count behavior.
| +static int magicmouse_setup_input_t2(struct input_dev *input, | ||
| + struct hid_device *hdev) | ||
| +{ | ||
| + int min_x, min_y, max_x, max_y, res_x, res_y; | ||
| + | ||
| + for (size_t i = 0; i < ARRAY_SIZE(magicmouse_t2_configs); i++) { | ||
| + if (magicmouse_t2_configs[i].id == hdev->product) { | ||
| + min_x = magicmouse_t2_configs[i].min_x; | ||
| + min_y = magicmouse_t2_configs[i].min_y; | ||
| + max_x = magicmouse_t2_configs[i].max_x; | ||
| + max_y = magicmouse_t2_configs[i].max_y; | ||
| + res_x = magicmouse_t2_configs[i].res_x; | ||
| + res_y = magicmouse_t2_configs[i].res_y; | ||
| + } | ||
| + } | ||
| + | ||
| + int ret = magicmouse_setup_input_int_tpd(input, hdev, min_x, min_y, | ||
| + max_x, max_y, res_x, res_y, false); | ||
| if (ret) | ||
| return ret; | ||
|
|
There was a problem hiding this comment.
Uninitialized variables used if product ID not found in config array.
If hdev->product doesn't match any entry in magicmouse_t2_configs, the local variables min_x, min_y, max_x, max_y, res_x, res_y remain uninitialized and are passed to magicmouse_setup_input_int_tpd(), causing undefined behavior. Additionally, the loop should break when a match is found.
Apply this diff:
static int magicmouse_setup_input_t2(struct input_dev *input,
struct hid_device *hdev)
{
- int min_x, min_y, max_x, max_y, res_x, res_y;
+ const struct magicmouse_t2_properties *props = NULL;
for (size_t i = 0; i < ARRAY_SIZE(magicmouse_t2_configs); i++) {
if (magicmouse_t2_configs[i].id == hdev->product) {
- min_x = magicmouse_t2_configs[i].min_x;
- min_y = magicmouse_t2_configs[i].min_y;
- max_x = magicmouse_t2_configs[i].max_x;
- max_y = magicmouse_t2_configs[i].max_y;
- res_x = magicmouse_t2_configs[i].res_x;
- res_y = magicmouse_t2_configs[i].res_y;
+ props = &magicmouse_t2_configs[i];
+ break;
}
}
- int ret = magicmouse_setup_input_int_tpd(input, hdev, min_x, min_y,
- max_x, max_y, res_x, res_y, false);
+ if (!props) {
+ hid_err(hdev, "Unknown T2 trackpad product ID: 0x%x\n", hdev->product);
+ return -ENODEV;
+ }
+
+ int ret = magicmouse_setup_input_int_tpd(input, hdev, props->min_x,
+ props->min_y, props->max_x,
+ props->max_y, props->res_x,
+ props->res_y, false);
if (ret)
return ret;🤖 Prompt for AI Agents
In
patch/kernel/archive/uefi-x86-6.19/4004-HID-magicmouse-Add-support-for-trackpads-found-on-T2.patch
around lines 266–286, the local vars min_x/min_y/max_x/max_y/res_x/res_y may
remain uninitialized if hdev->product is not found in magicmouse_t2_configs and
the loop doesn't break on match; initialize these variables to safe defaults (or
declare a bool/found flag), break out of the loop immediately after a match, and
after the loop check the flag (or validate values) and return an appropriate
error (e.g., -ENODEV or -EINVAL) if no matching config was found before calling
magicmouse_setup_input_int_tpd().
| if ((id->bus == BUS_SPI || id->bus == BUS_HOST) && id->vendor == SPI_VENDOR_ID_APPLE && | ||
| - hdev->type != HID_TYPE_SPI_MOUSE) | ||
| - return -ENODEV; | ||
| + hdev->type != HID_TYPE_SPI_MOUSE) | ||
| + return -ENODEV; |
There was a problem hiding this comment.
Whitespace inconsistency in indentation.
Lines 311-313 appear to have mixed space+tab indentation that differs from the surrounding context. This may have been introduced during patch rewriting.
🤖 Prompt for AI Agents
In
patch/kernel/archive/uefi-x86-6.19/4004-HID-magicmouse-Add-support-for-trackpads-found-on-T2.patch
around lines 309-313 there is mixed space+tab indentation on the wrapped
condition and the return line; normalize the indentation to match the
surrounding block (use the same tab/space style as the rest of the file, e.g.,
replace the leading spaces before "return -ENODEV;" with a single tab so the
line aligns with the other indented statements) and ensure no trailing mixed
whitespace remains.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
patch/kernel/archive/uefi-x86-6.19/1002-Put-apple-bce-in-drivers-staging.patch (1)
62-63: Trailing whitespace on line 62.Line 62 appears to contain only whitespace characters before the final help text line. While this may not cause functional issues, it's typically considered poor style in Kconfig files and some kernel checkpatch scripts may flag it.
- - If "M" is selected, the module will be called apple-bce. + If "M" is selected, the module will be called apple-bce.
📜 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 (1)
patch/kernel/archive/uefi-x86-6.19/1002-Put-apple-bce-in-drivers-staging.patch(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.
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 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: 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: 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: 8746
File: config/sources/families/filogic.conf:61-68
Timestamp: 2025-10-13T02:26:18.249Z
Learning: In the Armbian build system, the "edge" kernel branch is intended to be bleeding-edge and experimental. It does not require hardware testing or stability verification before merging, as it's expected to contain potentially unstable or untested code for early adopters and developers.
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-07-21T04:12:02.439Z
Learning: In the Armbian build system, for recurring maintenance tasks like kernel version bumping, TODO comments should use generic version formats (e.g., "MAJOR.MINOR-rc1") rather than specific version numbers (e.g., "6.17-rc1") to avoid the need for frequent comment updates that would create unnecessary maintenance overhead.
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: SuperKali
Repo: armbian/build PR: 8608
File: lib/functions/compilation/patch/drivers_network.sh:361-363
Timestamp: 2025-09-11T06:06:52.328Z
Learning: For the RTW88 SDIO RF path detection fix patch (004-rtw88-sdio-rf-path-detection-fix.patch), SuperKali prefers using exact version matching (eq 6.1 || eq 6.16) rather than range checks, as the patch has only been tested on those specific kernel versions and may be mainstreamed soon.
📚 Learning: 2025-11-06T15:36:04.682Z
Learnt from: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.
Applied to files:
patch/kernel/archive/uefi-x86-6.19/1002-Put-apple-bce-in-drivers-staging.patch
📚 Learning: 2025-09-25T18:37:00.330Z
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.
Applied to files:
patch/kernel/archive/uefi-x86-6.19/1002-Put-apple-bce-in-drivers-staging.patch
📚 Learning: 2025-07-27T15:54:35.119Z
Learnt from: pyavitz
Repo: armbian/build PR: 8421
File: config/kernel/linux-sunxi64-edge.config:2168-2173
Timestamp: 2025-07-27T15:54:35.119Z
Learning: In the Armbian build system, staging 802.11 drivers like CONFIG_RTLLIB, CONFIG_RTL8192E, CONFIG_R8712U are kept enabled alongside upstream rtw88 drivers because rtw88 is still under development and suffers from reliability issues. The staging drivers serve as necessary fallbacks when upstream drivers are unstable, prioritizing working hardware over avoiding technical conflicts.
Applied to files:
patch/kernel/archive/uefi-x86-6.19/1002-Put-apple-bce-in-drivers-staging.patch
🔇 Additional comments (1)
patch/kernel/archive/uefi-x86-6.19/1002-Put-apple-bce-in-drivers-staging.patch (1)
68-73: LGTM!The Makefile change from
obj-mtoobj-$(CONFIG_APPLE_BCE)correctly integrates the module with the kernel's Kconfig system, allowing proper build control based on the configuration option.
|
i just realized uefi-arm64 is affected by this too. converted to draft |
|
@rpardini the patch |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
patch/kernel/archive/uefi-arm64-6.19/net-stmicro-stmmac-Phytium-onboard-ethernet-drivers-and-ACPI-glue-for-6.x.patch.disabled (2)
316-322: Dead code in error path.The error cleanup checks
plat_dat->exitandplat_dat->bsp_priv, but neither is ever set indwmac_phytium_parse_config_acpi(). This entire block is effectively dead code. Either remove it or properly initialize the exit callback if cleanup is needed.err_exit: - if (plat_dat->exit) - plat_dat->exit(pdev, plat_dat->bsp_priv); - return ret; }
441-458: Duplicate of kernel'sfwnode_get_phy_mode().This
fw_get_phy_mode()function duplicates the kernel's existingfwnode_get_phy_mode()which is already used indwmac-phytium.cat line 178. This inconsistency could lead to behavioral differences and maintenance burden.Consider using
fwnode_get_phy_mode()from<linux/of_net.h>or<linux/property.h>consistently throughout.
📜 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 (2)
patch/kernel/archive/uefi-arm64-6.19/board-hikey960-usb.patch(1 hunks)patch/kernel/archive/uefi-arm64-6.19/net-stmicro-stmmac-Phytium-onboard-ethernet-drivers-and-ACPI-glue-for-6.x.patch.disabled(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-12-19T13:56:45.124Z
Learning: When reviewing kernel or u-boot version bump PRs in the Armbian build system, check if patches existed in previous kernel version directories (e.g., sunxi-6.12, sunxi-6.13) before describing them as new features. If a patch and the majority of its contents existed previously with no major functionality changes, focus the review on the actual changes: the version bump itself and patch compatibility adjustments. Don't describe existing patches being ported/maintained across versions as new features or drivers—this is misleading. The patches are existing code being re-aligned to work with the new upstream version.
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-12-17T05:09:02.306Z
Learning: In the Armbian build system, kernel patches (e.g., in patch/kernel/archive/sunxi-6.18/patches.armbian/) contain device tree overlays and other code that have existed for years and are maintained/ported across kernel versions. When reviewing PRs that modify these patch files, focus on the actual changes being made (e.g., Makefile fixes, new additions) rather than reviewing the entire existing content within the patch as if it were new code. The patch file contents are existing, stable code unless explicitly modified in the PR diff.
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: 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: 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: 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: 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: 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.
📚 Learning: 2025-12-17T15:46:22.709Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 9096
File: patch/kernel/archive/sunxi-6.18/patches.armbian/arm64-dts-sun50i-h616-add-overlays.patch:4-5
Timestamp: 2025-12-17T15:46:22.709Z
Learning: In Armbian kernel patches, when patches are renamed or reorganized, the old Subject line may be intentionally preserved in the patch header (as a second Subject line) for archival/historical purposes to document the previous patch name. This is an Armbian-specific practice for tracking patch evolution and should not be flagged as an error when the maintainer has intentionally included it for documentation.
Applied to files:
patch/kernel/archive/uefi-arm64-6.19/board-hikey960-usb.patchpatch/kernel/archive/uefi-arm64-6.19/net-stmicro-stmmac-Phytium-onboard-ethernet-drivers-and-ACPI-glue-for-6.x.patch.disabled
📚 Learning: 2025-12-17T05:09:02.306Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-12-17T05:09:02.306Z
Learning: In the Armbian build system, kernel patches (e.g., in patch/kernel/archive/sunxi-6.18/patches.armbian/) contain device tree overlays and other code that have existed for years and are maintained/ported across kernel versions. When reviewing PRs that modify these patch files, focus on the actual changes being made (e.g., Makefile fixes, new additions) rather than reviewing the entire existing content within the patch as if it were new code. The patch file contents are existing, stable code unless explicitly modified in the PR diff.
Applied to files:
patch/kernel/archive/uefi-arm64-6.19/net-stmicro-stmmac-Phytium-onboard-ethernet-drivers-and-ACPI-glue-for-6.x.patch.disabled
📚 Learning: 2025-12-19T13:56:45.124Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-12-19T13:56:45.124Z
Learning: When reviewing kernel or u-boot version bump PRs in the Armbian build system, check if patches existed in previous kernel version directories (e.g., sunxi-6.12, sunxi-6.13) before describing them as new features. If a patch and the majority of its contents existed previously with no major functionality changes, focus the review on the actual changes: the version bump itself and patch compatibility adjustments. Don't describe existing patches being ported/maintained across versions as new features or drivers—this is misleading. The patches are existing code being re-aligned to work with the new upstream version.
Applied to files:
patch/kernel/archive/uefi-arm64-6.19/net-stmicro-stmmac-Phytium-onboard-ethernet-drivers-and-ACPI-glue-for-6.x.patch.disabled
📚 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/uefi-arm64-6.19/net-stmicro-stmmac-Phytium-onboard-ethernet-drivers-and-ACPI-glue-for-6.x.patch.disabled
📚 Learning: 2025-11-06T15:36:04.682Z
Learnt from: rpardini
Repo: armbian/build PR: 8879
File: config/sources/families/uefi-x86.conf:0-0
Timestamp: 2025-11-06T15:36:04.682Z
Learning: As of PR #8879, the uefi-x86 family in the Armbian build system now includes kernel patches for the first time. The current and edge branches for uefi-x86 are specifically configured for Apple T2-based x86 machines, including T2-specific patches from the linux-t2 project and custom kernel configuration options for Apple hardware drivers.
Applied to files:
patch/kernel/archive/uefi-arm64-6.19/net-stmicro-stmmac-Phytium-onboard-ethernet-drivers-and-ACPI-glue-for-6.x.patch.disabled
📚 Learning: 2025-09-09T07:30:22.419Z
Learnt from: amazingfate
Repo: armbian/build PR: 8595
File: lib/functions/compilation/patch/drivers_network.sh:0-0
Timestamp: 2025-09-09T07:30:22.419Z
Learning: RTW_WARN_LMT sed workaround in driver_rtl8852bs() function in lib/functions/compilation/patch/drivers_network.sh is necessary for kernel 6.1 compatibility, even when the upstream wifi-rtl8852bs driver has the macro defined for newer kernels. The workaround ensures cross-kernel compatibility from 6.1 to 6.16+.
Applied to files:
patch/kernel/archive/uefi-arm64-6.19/net-stmicro-stmmac-Phytium-onboard-ethernet-drivers-and-ACPI-glue-for-6.x.patch.disabled
🔇 Additional comments (4)
patch/kernel/archive/uefi-arm64-6.19/board-hikey960-usb.patch (3)
1-11: Archeology header correctly documents patch origin.The X-Git-Archeology header properly tracks the patch history from the original August 2023 commit, following Armbian conventions.
57-72: Endpoint remapping logic is correct.The modifications properly establish the USB hub as an intermediary between the DWC3 USB controller and the Type-C port manager. The bidirectional endpoint references are correctly maintained, creating the topology:
dwc3_role_switch ↔ hikey_usb_ep0 ↔ [usb-hub] ↔ hikey_usb_ep1 ↔ rt1711h_ep.
33-38: Compatible string "hisilicon,usbhub" is supported in kernel 6.19.The hisi_hikey_usb driver in drivers/misc/hisi_hikey_usb.c provides support for the onboard USB GPIO hub on HiKey960, and the device tree syntax is correct. No breaking changes are present in kernel 6.19 relative to 6.18 for this compatible string.
patch/kernel/archive/uefi-arm64-6.19/net-stmicro-stmmac-Phytium-onboard-ethernet-drivers-and-ACPI-glue-for-6.x.patch.disabled (1)
1-6: Patch correctly disabled pending fixes.The
.disabledextension is appropriate given the reported compilation failure. Based on the review, the following should be addressed before re-enabling:
- Fix indentation (spaces → tabs) in
stmmac_get_platform_resourceschanges- Add error handling for
clk_prepare_enable()andstmmac_acpi_clock_setup()calls- Remove or update the duplicate
fw_get_phy_mode()function- Use proper error codes instead of
-1instmmac_acpi_clock_setup()The PR author already identified this patch as problematic - these notes may help when updating it for 6.19 compatibility.
....19/net-stmicro-stmmac-Phytium-onboard-ethernet-drivers-and-ACPI-glue-for-6.x.patch.disabled
Show resolved
Hide resolved
....19/net-stmicro-stmmac-Phytium-onboard-ethernet-drivers-and-ACPI-glue-for-6.x.patch.disabled
Show resolved
Hide resolved
....19/net-stmicro-stmmac-Phytium-onboard-ethernet-drivers-and-ACPI-glue-for-6.x.patch.disabled
Show resolved
Hide resolved
....19/net-stmicro-stmmac-Phytium-onboard-ethernet-drivers-and-ACPI-glue-for-6.x.patch.disabled
Show resolved
Hide resolved
1 participant
Description
copy patches
rewrite patches
???
profit!
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Kernel Upgrade
✏️ Tip: You can customize this high-level summary in your review settings.