Skip to content

Conversation

@EvilOlaf
Copy link
Member

@EvilOlaf EvilOlaf commented Dec 16, 2025

Description

copy patches
rewrite patches
???
profit!

How Has This Been Tested?

  • build

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

Summary by CodeRabbit

  • New Features

    • Apple BCE audio driver added (staging)
    • Apple SMC: MMIO interface, dynamic I/O base, ACPI support, per-device refactor, key query wrappers
    • T2 Mac: float-based fan control and per-fan manual control
    • Trackpad support for Apple T2 Macs
    • iMacPro added to Apple SMC whitelist
    • Battery charge limit exposed via sysfs
  • Bug Fixes

    • Fixed camera freeze on T2 Macs when turning off camera
    • Fixed DDI lane quirk for MacBookPro15,1
    • Improved GPU switching to integrated graphics on Apple systems
    • Restored Magic Trackpad 1 support
    • Discard invalid BIOS framebuffers in Intel i915 probe
  • Kernel Upgrade

    • Kernel version bumped to 6.19

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 16, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Kernel edge bumped from 6.18 to 6.19 and a set of patches add Apple-specific drivers/features (Apple BCE, applesmc refactor, ACPI/MMIO support, T2 fan/trackpad support), graphics quirks, HID updates, and an i915 fbdev safety check.

Changes

Cohort / File(s) Summary
Kernel Version Configuration
config/sources/families/include/uefi_common.inc
Updates edge branch KERNEL_MAJOR_MINOR from 6.18 to 6.19.
Apple BCE (staging) integration
patch/kernel/archive/uefi-x86-6.19/1002-Put-apple-bce-in-drivers-staging.patch, drivers/staging/...
Adds apple-bce Kconfig & Makefile; wires obj-$(CONFIG_APPLE_BCE) += apple-bce/ and sources new Kconfig from drivers/staging/Kconfig.
Apple BCE VHCI URB cancellation fix
patch/kernel/archive/uefi-x86-6.19/1003-Fix-freezing-on-turning-off-camera.patch
Adds BCE_VHCI_URB_CANCELLED enum and adjusts URB cancellation logic to avoid races.
i915 / gmux / graphics quirks
patch/kernel/archive/uefi-x86-6.19/2008-i915-4-lane-quirk-for-mbp15-1.patch, patch/kernel/archive/uefi-x86-6.19/2009-apple-gmux-allow-switching-to-igpu-at-probe.patch
Adds QUIRK_DDI_A_FORCE_4_LANES quirk for MacBookPro15,1; registers quirk handler; enables probe deferral and iGPU switch support; exports vga_set_default_device.
AppleSMC: static→per-device refactor
patch/kernel/archive/uefi-x86-6.19/3001-applesmc-convert-static-structures-to-drvdata.patch
Replaces global static state with struct applesmc_device; updates many helper signatures and call sites to use per-device context; restructures probe/remove and lifecycle.
AppleSMC: dynamic I/O port base
patch/kernel/archive/uefi-x86-6.19/3002-applesmc-make-io-port-base-addr-dynamic.patch
Adds u16 port_base to applesmc_device; converts I/O helpers to accept smc and use smc->port_base + offset.
AppleSMC: platform → ACPI migration
patch/kernel/archive/uefi-x86-6.19/3003-applesmc-switch-to-acpi_device-from-platform.patch
Migrates driver from platform_driver to acpi_driver; adds ACPI IDs, CRS resource walking, ACPI add/remove ops, and uses dev_set_drvdata with acpi_device.
AppleSMC key-wrapper API
patch/kernel/archive/uefi-x86-6.19/3004-applesmc-key-interface-wrappers.patch
Introduces port_-prefixed wrappers and new accessors get_smc_key_by_index and get_smc_key_info to abstract port/MMIO variants.
AppleSMC MMIO interface
patch/kernel/archive/uefi-x86-6.19/3005-applesmc-basic-mmio-interface-implementation.patch
Adds optional MMIO communication path (detection, mapping, enablement); read/write/get operations choose MMIO when available, fallback to port path otherwise.
AppleSMC T2 fan & features
patch/kernel/archive/uefi-x86-6.19/3006-applesmc-fan-support-on-T2-Macs.patch, patch/kernel/archive/uefi-x86-6.19/3007-applesmc-Add-iMacPro-to-applesmc_whitelist.patch, patch/kernel/archive/uefi-x86-6.19/3009-applesmc-battery-charge-limiter.patch
Adds float-based fan handling and per-fan manual controls, converts float↔u32 helpers, extends DMI whitelist for iMacPro, and adds BCLM sysfs battery charge limit group.
AppleSMC remove signature change
patch/kernel/archive/uefi-x86-6.19/3008-applesmc-make-applesmc_remove-void.patch
Changes applesmc_remove return type from int to void.
HID: ignore T2 trackpad in hid-apple
patch/kernel/archive/uefi-x86-6.19/4003-HID-apple-ignore-the-trackpad-on-T2-Macs.patch
Adds APPLE_IGNORE_MOUSE quirk and skips binding hid-apple for certain T2 USB devices.
HID magicmouse: T2 trackpad support & regression fix
patch/kernel/archive/uefi-x86-6.19/4004-HID-magicmouse-Add-support-for-trackpads-found-on-T2.patch, patch/kernel/archive/uefi-x86-6.19/4005-HID-magicmouse-fix-regression-breaking-support-for-M.patch
Implements T2-specific touchpad handling, per-model properties and raw-event/setup paths; fixes report registration for Magic Trackpad 1.
i915 fbdev BIOS framebuffer check
patch/kernel/archive/uefi-x86-6.19/7001-drm-i915-fbdev-Discard-BIOS-framebuffers-exceeding-h.patch
Replaces size-only check with strict dimension mismatch validation to discard invalid BIOS framebuffers.

Sequence Diagram(s)

sequenceDiagram
    participant ACPI as ACPI bus
    participant Kernel as applesmc driver
    participant HW as Apple SMC (port/MMIO)
    participant Sysfs as userspace/sysfs

    ACPI->>Kernel: acpi_device add (walk CRS resources)
    Kernel->>Kernel: init_resources (detect iomem or ports)
    alt MMIO resource found & enabled
        Kernel->>HW: map MMIO, probe status
        HW-->>Kernel: status OK
        Kernel->>Kernel: enable MMIO path
    else fallback to ports
        Kernel->>HW: request I/O ports (port_base)
        HW-->>Kernel: ports reserved
    end
    Kernel->>Sysfs: create sysfs nodes (fan, BCLM, sensors)
    Sysfs->>Kernel: user read/write (e.g., set fan speed)
    Kernel->>Kernel: choose (MMIO ? mmio_read/write : port_read/write)
    Kernel->>HW: perform read/write
    HW-->>Kernel: response
    Kernel-->>Sysfs: return result
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Areas requiring extra attention:

  • applesmc refactor & signature changes (3001): many call-site updates, mutex/state migration.
  • ACPI migration & resource walking (3003): CRS handling, dev_set_drvdata usage, error paths.
  • MMIO path (3005): detection, mapping, fallback, and cleanup correctness.
  • T2-specific logic (3006, 4004): float handling, per-fan manual keys, and per-model touchpad configs.
  • HID/magicmouse report registration changes (4005): ensure no regressions for existing devices.

Possibly related PRs

Suggested reviewers

  • rpardini
  • igorpecovnik
  • amazingfate
  • clee
  • joekhoobyar
  • pyavitz

Poem

🐰 In tunnels of code I nimbly hop,

Port or MMIO, I never stop,
From globals to per-device care,
Fans and trackpads dance in air,
Six‑nineteen blooms — a kernel crop.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'uefi-x86: bump edge to 6.19-rc1' directly and clearly summarizes the main change: updating the kernel version from 6.18 to 6.19-rc1 for the edge branch, which is the primary modification across all patches.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

❤️ Share

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

@github-actions github-actions bot added 02 Milestone: First quarter release size/large PR with 250 lines or more Needs review Seeking for review Hardware Hardware related like kernel, U-Boot, ... Patches Patches related to kernel, U-Boot, ... labels Dec 16, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 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_mtp and magicmouse_setup_input_spi have redundant return logic. The pattern if (ret) return ret; return 0; can be simplified to return 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 void return type is correct for kernel 6.2+ ACPI driver compatibility. However, the bare return; 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 *key directly to u32 * and dereferencing may violate strict aliasing rules, leading to undefined behavior on some compilers with aggressive optimizations. Consider using memcpy or get_unaligned_le32/get_unaligned_be32 for 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) and iomem_write_smc (line 174).


133-136: Unconditional print_hex_dump_bytes may cause log spam.

print_hex_dump_bytes always 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 exp exceeds 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: Static data_length persists across devices.

The data_length variable at line 833 remains static, 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_length into struct applesmc_device if 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.

📥 Commits

Reviewing files that changed from the base of the PR and between f2dd38e and e711d63.

📒 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.inc
  • patch/kernel/archive/uefi-x86-6.19/4003-HID-apple-ignore-the-trackpad-on-T2-Macs.patch
  • patch/kernel/archive/uefi-x86-6.19/3006-applesmc-fan-support-on-T2-Macs.patch
  • patch/kernel/archive/uefi-x86-6.19/2008-i915-4-lane-quirk-for-mbp15-1.patch
  • patch/kernel/archive/uefi-x86-6.19/3005-applesmc-basic-mmio-interface-implementation.patch
  • patch/kernel/archive/uefi-x86-6.19/1002-Put-apple-bce-in-drivers-staging.patch
  • patch/kernel/archive/uefi-x86-6.19/3007-applesmc-Add-iMacPro-to-applesmc_whitelist.patch
  • patch/kernel/archive/uefi-x86-6.19/3003-applesmc-switch-to-acpi_device-from-platform.patch
  • patch/kernel/archive/uefi-x86-6.19/4004-HID-magicmouse-Add-support-for-trackpads-found-on-T2.patch
  • patch/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 existing ddi_a_4_lanes check
  • 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) to obj-$(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 at patch/kernel/archive/uefi-x86-6.19 exists 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_device with 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 removed pdev != vga_default_device() check is by design—the patch includes complementary logic in apple-gmux.c with a force_igd module 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_state variable 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_state instead of vurb->state in 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_CANCELLED state 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_MOUSE flag 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 &&, so quirks & APPLE_IGNORE_MOUSE && hdev->type == HID_TYPE_USBMOUSE is parsed as quirks & (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_ID case into the default path, 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_iomem function has proper error handling with clean goto-based cleanup, and the iomem_base_set flag 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_MANUAL path 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 common out: 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_base field 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-device smc->port_base offset. The parameter threading is correct throughout the call chains.


283-307: Module init/exit correctly use the port base constant.

The request_region and release_region calls in module init/exit are updated to use APPLESMC_PORT_BASE consistently, maintaining the same I/O region management as before while allowing the per-device port_base to 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_show function correctly reads the BCLM key and formats it for sysfs output using sysfs_emit.


62-92: LGTM on module lifecycle integration.

The BCLM_group is correctly integrated into applesmc_create_modules with proper error handling and reverse-order cleanup in both the error path and applesmc_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_index properly uses the new wrapper functions and maintains the same functionality with cleaner separation of concerns.


233-244: The return value check change from if (ret) to if (ret < 0) is correct and intentional. The read_smc wrapper 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_resources callback 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_probe function correctly allocates, initializes, and registers per-device state with proper error handling and cleanup paths. The applesmc_remove function 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_of in applesmc_backlight_set correctly retrieves the device context from the work struct.

Comment on lines +80 to +82
+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)");
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Comment on lines +91 to +102
+ 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?");
+ }
+ }
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

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.

Suggested change
+ 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.

Comment on lines +518 to +537

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);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 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 -1

Repository: armbian/build

Length of output: 150


🏁 Script executed:

cat ./patch/kernel/archive/uefi-x86-6.12/3001-applesmc-convert-static-structures-to-drvdata.patch

Repository: 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.

Comment on lines +147 to +165
+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);
+}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
+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.

Comment on lines +54 to +57
+ bool port_base_set, iomem_base_set;
u16 port_base;
+ u8 *__iomem iomem_base;
+ u32 iomem_base_addr, iomem_base_size;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Comment on lines +18 to +33
+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;
+}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix function signature and remove dead code in applesmc_BCLM_store.

Two issues in this function:

  1. Line 19: The sysfsbuf parameter should be const char * to match the kernel's sysfs_ops.store signature. Using char * may cause compiler warnings or type mismatches.

  2. Line 27: The condition val < 0 is always false since val is declared as u8 (unsigned). Only val > 100 is 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.

Suggested change
+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.

Comment on lines +266 to +286
+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;

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

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().

Comment on lines +309 to +313
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;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 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.

📥 Commits

Reviewing files that changed from the base of the PR and between e711d63 and d774942.

📒 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-m to obj-$(CONFIG_APPLE_BCE) correctly integrates the module with the kernel's Kconfig system, allowing proper build control based on the configuration option.

@EvilOlaf EvilOlaf marked this pull request as draft December 17, 2025 16:04
@EvilOlaf
Copy link
Member Author

i just realized uefi-arm64 is affected by this too. converted to draft

@EvilOlaf
Copy link
Member Author

@rpardini the patch net: stmicro: stmmac: Phytium onboard ethernet drivers and ACPI glue breaks compilation.
https://paste.armbian.com/umepeviqaq line 22350

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

02 Milestone: First quarter release Hardware Hardware related like kernel, U-Boot, ... Needs review Seeking for review Patches Patches related to kernel, U-Boot, ... size/large PR with 250 lines or more

Development

Successfully merging this pull request may close these issues.

1 participant