From f272eb10be76910f6645aab0fa051421b59d2d20 Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 31 Oct 2026 15:49:49 +1100 Subject: [PATCH 1/6] dt-bindings: hwmon: apple,smc-hwmon: Deprecate host fan cooling Keep the former cooling-device specifier valid for older device trees, but describe that fan policy stays with the SMC firmware. Manual target writes remain available through hwmon. Assisted-by: LLM Signed-off-by: Ryan Murray --- .../devicetree/bindings/hwmon/apple,smc-hwmon.yaml | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml index b8e962334935f..fcfe475d4dcf2 100644 --- a/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml +++ b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml @@ -80,12 +80,10 @@ properties: "#cooling-cells": const: 3 + deprecated: true description: - Makes the fans cooling devices for thermal zones. The first cell is the - zero-based index of a fan, counting the fan-* child nodes in order; the - other two are the lower and upper cooling state. State 0 leaves the fan - to the SMC; states 1 to 8 run it at speeds spread evenly from its - minimum to its maximum. A fan needs all four of its speed and mode keys. + Former fan cooling-device specifier. Fan policy is handled by the SMC + firmware; the driver no longer registers fan cooling devices. patternProperties: "^current-[A-Za-z0-9]{4}$": From cc3766d3c0173d83bac1556dd67d5ad3c6c19197 Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 31 Oct 2026 15:49:49 +1100 Subject: [PATCH 2/6] hwmon: macsmc: Leave fan policy to firmware until a user requests control The host thermal ladder assumed the SMC needed a manual request to run the fans. Sustained macOS measurements instead show firmware ramping the fans while the host makes no SMC key writes. Remove the autonomous fan cooling devices and resume work; retain explicit fanX_target control and release on suspend, shutdown and removal. Read the mode before requesting manual control. In system mode, unlock first and wait using reads instead of provoking a refused mode write. Firmware without an unlock key fails without issuing a known unsupported write. Exercise manual targets, zero refused writes, untouched lifecycle paths, suspend release, resume without reapplication, and unlock failures with the existing KUnit SMC model. Assisted-by: LLM Signed-off-by: Ryan Murray --- Documentation/hwmon/macsmc-hwmon.rst | 5 + drivers/hwmon/macsmc-hwmon-test.c | 120 ++++------- drivers/hwmon/macsmc-hwmon.c | 293 ++++++--------------------- 3 files changed, 99 insertions(+), 319 deletions(-) diff --git a/Documentation/hwmon/macsmc-hwmon.rst b/Documentation/hwmon/macsmc-hwmon.rst index 2204745326cba..b0c78c3412088 100644 --- a/Documentation/hwmon/macsmc-hwmon.rst +++ b/Documentation/hwmon/macsmc-hwmon.rst @@ -31,6 +31,11 @@ as unsafe, allows manual speeds on every fan with a mode key whatever the Devicetree says. It cannot be proven that the system will fail safe if it overheats under a manual fan speed. +The driver leaves fan policy to the SMC until userspace writes a nonzero +``fanX_target``. Thermal zones do not take fans into manual mode. Suspend, +shutdown and removal release only fans taken over by such a request; resume +does not reapply it. A new target write is required after resume. + SoC die temperatures are described by the ``apple,soc-die-keys`` string list on the hwmon device node. The driver appends these temperature channels after the ``temperature-*`` children, in list order, with labels ``SoC die ``. diff --git a/drivers/hwmon/macsmc-hwmon-test.c b/drivers/hwmon/macsmc-hwmon-test.c index 312e752565e04..610a019ee59b8 100644 --- a/drivers/hwmon/macsmc-hwmon-test.c +++ b/drivers/hwmon/macsmc-hwmon-test.c @@ -2,7 +2,7 @@ /* * KUnit tests for the Apple SMC hwmon driver: channel numbering of the * temperature sensors, which thermal zones depend on, and manual fan control - * by userspace and by the thermal framework. + * by userspace. * * Included by macsmc-hwmon.c. */ @@ -106,6 +106,7 @@ struct macsmc_fan_test { u8 mode[2]; u32 target[2]; u8 ftst; + int writes; int ftst_writes; int refused; smc_key fail_key; @@ -149,6 +150,7 @@ static int fan_test_write(struct apple_smc *smc, smc_key key, const void *buf, s u32 value = 0; int i = fan_test_index(key); + t->writes++; memcpy(&value, buf, min(size, sizeof(value))); if (key == t->fail_key) return -EIO; @@ -218,9 +220,6 @@ static int macsmc_fan_test_init(struct kunit *test) t->hwmon.fan.count = 2; t->hwmon.fan.unlock_key = SMC_KEY(Ftst); mutex_init(&t->hwmon.fan.lock); - INIT_WORK(&t->hwmon.fan.cool_work, macsmc_hwmon_fans_cool_work); - for (i = 0; i < 2; i++) - t->fans[i].hwmon = &t->hwmon; test->priv = t; fan_control = true; kunit_activate_static_stub(test, apple_smc_read, fan_test_read); @@ -261,6 +260,7 @@ static void macsmc_fan_test_unlock_and_release(struct kunit *test) KUNIT_ASSERT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 0), 0); fan_test_released(test, t); KUNIT_EXPECT_EQ(test, t->ftst_writes, 2); + KUNIT_EXPECT_EQ(test, t->refused, 0); } /* Targets are clamped to [min, max]; negative ones are refused. */ @@ -303,14 +303,14 @@ static void macsmc_fan_test_two_fans(struct kunit *test) fan_test_released(test, t); } -/* Locked firmware without the unlock key: refused, nothing else written. */ +/* System mode without an unlock key: fail without attempting a mode write. */ static void macsmc_fan_test_no_unlock_key(struct kunit *test) { struct macsmc_fan_test *t = test->priv; t->hwmon.fan.unlock_key = 0; - KUNIT_EXPECT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 2500), -EIO); - KUNIT_EXPECT_EQ(test, t->ftst_writes, 0); + KUNIT_EXPECT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 2500), -EOPNOTSUPP); + KUNIT_EXPECT_EQ(test, t->writes, 0); KUNIT_EXPECT_EQ(test, t->mode[0], MACSMC_FAN_MODE_SYSTEM); KUNIT_EXPECT_EQ(test, t->hwmon.fan.manual, 0); } @@ -330,97 +330,51 @@ static void macsmc_fan_test_legacy(struct kunit *test) KUNIT_EXPECT_EQ(test, t->refused, 0); } -/* Cooling states spread over [min, max]; 0 asks for nothing. */ -static void macsmc_fan_test_state_rpm(struct kunit *test) -{ - KUNIT_EXPECT_EQ(test, macsmc_hwmon_fan_state_rpm(1350, 5777, 0), 0); - KUNIT_EXPECT_EQ(test, macsmc_hwmon_fan_state_rpm(1350, 5777, 1), 1350); - KUNIT_EXPECT_EQ(test, macsmc_hwmon_fan_state_rpm(1350, 5777, 4), 3247); - KUNIT_EXPECT_EQ(test, macsmc_hwmon_fan_state_rpm(1350, 5777, - MACSMC_FAN_COOLING_STATES), 5777); - /* Never past the maximum, whatever the state. */ - KUNIT_EXPECT_EQ(test, macsmc_hwmon_fan_state_rpm(1350, 5777, - MACSMC_FAN_COOLING_STATES + 5), 5777); -} - -/* Cooling needs no fan_control: unlock for a state, release at state 0. */ -static void macsmc_fan_test_cooling(struct kunit *test) +/* Suspend, resume and shutdown do not write untouched fan keys. */ +static void macsmc_fan_test_untouched(struct kunit *test) { struct macsmc_fan_test *t = test->priv; + struct platform_device pdev = {}; - fan_control = false; - t->fans[0].cool_state = 1; - macsmc_hwmon_fans_apply(&t->hwmon); - KUNIT_EXPECT_EQ(test, t->ftst, 1); - KUNIT_EXPECT_EQ(test, t->mode[0], MACSMC_FAN_MODE_MANUAL); - KUNIT_EXPECT_EQ(test, t->target[0], FAN_TEST_F1350); - /* The other fan asks for nothing and stays with the firmware. */ - KUNIT_EXPECT_EQ(test, t->mode[1], MACSMC_FAN_MODE_AUTO); - KUNIT_EXPECT_FALSE(test, t->fans[1].manual); - - t->fans[0].cool_state = MACSMC_FAN_COOLING_STATES; - macsmc_hwmon_fans_apply(&t->hwmon); - KUNIT_EXPECT_EQ(test, t->target[0], FAN_TEST_F5777); - KUNIT_EXPECT_EQ(test, t->ftst_writes, 1); - - t->fans[0].cool_state = 0; - macsmc_hwmon_fans_apply(&t->hwmon); + platform_set_drvdata(&pdev, &t->hwmon); + KUNIT_ASSERT_EQ(test, macsmc_hwmon_suspend(&pdev.dev), 0); + KUNIT_ASSERT_EQ(test, macsmc_hwmon_resume(&pdev.dev), 0); + macsmc_hwmon_shutdown(&pdev); + KUNIT_EXPECT_EQ(test, t->writes, 0); fan_test_released(test, t); } -/* A fan runs at the higher of the userspace target and the cooling state. */ -static void macsmc_fan_test_cooling_and_user(struct kunit *test) +/* Suspend releases a manual target; resume never reapplies it. */ +static void macsmc_fan_test_suspend_manual(struct kunit *test) { struct macsmc_fan_test *t = test->priv; + struct device dev = {}; + int writes; + dev_set_drvdata(&dev, &t->hwmon); KUNIT_ASSERT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 2500), 0); - t->fans[0].cool_state = 1; - macsmc_hwmon_fans_apply(&t->hwmon); - KUNIT_EXPECT_EQ(test, t->target[0], FAN_TEST_F2500); - - t->fans[0].cool_state = MACSMC_FAN_COOLING_STATES; - macsmc_hwmon_fans_apply(&t->hwmon); - KUNIT_EXPECT_EQ(test, t->target[0], FAN_TEST_F5777); - /* Userspace cannot take the fan below what cooling asks for. */ - KUNIT_ASSERT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 1350), 0); - KUNIT_EXPECT_EQ(test, t->target[0], FAN_TEST_F5777); - KUNIT_ASSERT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 0), 0); - KUNIT_EXPECT_EQ(test, t->mode[0], MACSMC_FAN_MODE_MANUAL); - - t->fans[0].cool_state = 0; - macsmc_hwmon_fans_apply(&t->hwmon); - fan_test_released(test, t); -} - -/* Once stopped (suspend, shutdown, removal), no state takes a fan back. */ -static void macsmc_fan_test_cooling_stopped(struct kunit *test) -{ - struct macsmc_fan_test *t = test->priv; - - t->fans[0].cool_state = 4; - t->fans[1].cool_state = 4; - macsmc_hwmon_fans_apply(&t->hwmon); - KUNIT_EXPECT_EQ(test, t->hwmon.fan.manual, 2); - - macsmc_hwmon_fans_stop(&t->hwmon); + KUNIT_ASSERT_EQ(test, macsmc_hwmon_suspend(&dev), 0); fan_test_released(test, t); - - macsmc_hwmon_fans_apply(&t->hwmon); + writes = t->writes; + KUNIT_EXPECT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 2500), -EBUSY); + KUNIT_ASSERT_EQ(test, macsmc_hwmon_resume(&dev), 0); + KUNIT_EXPECT_EQ(test, t->writes, writes); fan_test_released(test, t); - KUNIT_EXPECT_EQ(test, t->ftst_writes, 2); + /* A fresh userspace request is still accepted after resume. */ + KUNIT_ASSERT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 2500), 0); + KUNIT_ASSERT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 0), 0); + KUNIT_EXPECT_EQ(test, t->refused, 0); } /* A failed unlock leaves the fan with the firmware. */ -static void macsmc_fan_test_cooling_unlock_fails(struct kunit *test) +static void macsmc_fan_test_unlock_fails(struct kunit *test) { struct macsmc_fan_test *t = test->priv; t->fail_key = SMC_KEY(Ftst); - t->fans[0].cool_state = MACSMC_FAN_COOLING_STATES; - macsmc_hwmon_fans_apply(&t->hwmon); - KUNIT_EXPECT_EQ(test, t->mode[0], MACSMC_FAN_MODE_SYSTEM); - KUNIT_EXPECT_EQ(test, t->hwmon.fan.manual, 0); - KUNIT_EXPECT_FALSE(test, t->hwmon.fan.unlocked); + KUNIT_EXPECT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 2500), -EIO); + fan_test_released(test, t); + KUNIT_EXPECT_EQ(test, t->refused, 0); } static struct kunit_case macsmc_fan_test_cases[] = { @@ -430,11 +384,9 @@ static struct kunit_case macsmc_fan_test_cases[] = { KUNIT_CASE(macsmc_fan_test_two_fans), KUNIT_CASE(macsmc_fan_test_no_unlock_key), KUNIT_CASE(macsmc_fan_test_legacy), - KUNIT_CASE(macsmc_fan_test_state_rpm), - KUNIT_CASE(macsmc_fan_test_cooling), - KUNIT_CASE(macsmc_fan_test_cooling_and_user), - KUNIT_CASE(macsmc_fan_test_cooling_stopped), - KUNIT_CASE(macsmc_fan_test_cooling_unlock_fails), + KUNIT_CASE(macsmc_fan_test_untouched), + KUNIT_CASE(macsmc_fan_test_suspend_manual), + KUNIT_CASE(macsmc_fan_test_unlock_fails), {} }; diff --git a/drivers/hwmon/macsmc-hwmon.c b/drivers/hwmon/macsmc-hwmon.c index af67979f51213..a8e47ee4b98ff 100644 --- a/drivers/hwmon/macsmc-hwmon.c +++ b/drivers/hwmon/macsmc-hwmon.c @@ -36,7 +36,6 @@ #include #include #include -#include #define MAX_LABEL_LENGTH 32 @@ -88,13 +87,6 @@ struct macsmc_hwmon_fan { bool manual; /* The device tree allows a manual speed from userspace. */ bool manual_control; - /* Index of the fan node among the fan-* nodes: its cooling device id. */ - u32 index; - struct macsmc_hwmon *hwmon; - /* Speed asked for through the fan_control target, 0 for none. */ - long user_rpm; - /* Cooling state asked for by the thermal framework, 0 for none. */ - unsigned long cool_state; }; struct macsmc_hwmon_sensors { @@ -109,13 +101,6 @@ struct macsmc_hwmon_sensors { /* The firmware's own fan policy; it refuses a manual mode request. */ #define MACSMC_FAN_MODE_SYSTEM 3 -/* - * Cooling states of a fan with #cooling-cells: 0 leaves the fan to the - * firmware, 1 to MACSMC_FAN_COOLING_STATES run it at speeds spread evenly - * from its minimum to its maximum. - */ -#define MACSMC_FAN_COOLING_STATES 8 - /* How long the firmware gets to leave or re-enter its system mode */ #define MACSMC_FAN_MODE_TIMEOUT_MS 10000 #define MACSMC_FAN_MODE_POLL_MS 100 @@ -134,8 +119,6 @@ struct macsmc_hwmon_fans { bool unlocked; /* Fans in manual mode */ u32 manual; - /* Applies the cooling states, which can take seconds to unlock. */ - struct work_struct cool_work; /* Suspended, shut down or going away: every fan stays with the SMC. */ bool stopped; }; @@ -497,99 +480,52 @@ static int macsmc_hwmon_fan_manual(struct macsmc_hwmon *hwmon, if (fan->manual) return 0; - ret = macsmc_hwmon_write_key(hwmon->smc, &fan->mode, MACSMC_FAN_MODE_MANUAL); - if (ret < 0 && hwmon->fan.unlock_key && !hwmon->fan.unlocked && - !macsmc_hwmon_fan_read_mode(hwmon, fan, &mode) && - mode == MACSMC_FAN_MODE_SYSTEM) { - ret = apple_smc_write_u8(hwmon->smc, hwmon->fan.unlock_key, 1); - if (ret < 0) { - /* The key may have been set anyway; clear it. */ - apple_smc_write_u8(hwmon->smc, hwmon->fan.unlock_key, 0); - return ret; + /* Do not send a mode write the firmware is known to refuse. */ + ret = macsmc_hwmon_fan_read_mode(hwmon, fan, &mode); + if (ret) + return ret; + + if (mode == MACSMC_FAN_MODE_SYSTEM) { + if (!hwmon->fan.unlock_key) + return -EOPNOTSUPP; + + if (!hwmon->fan.unlocked) { + ret = apple_smc_write_u8(hwmon->smc, hwmon->fan.unlock_key, 1); + if (ret < 0) { + /* The key may have been set anyway; clear it. */ + apple_smc_write_u8(hwmon->smc, hwmon->fan.unlock_key, 0); + return ret; + } + hwmon->fan.unlocked = true; } - hwmon->fan.unlocked = true; - /* Ask again once the fan has left system mode. */ timeout = jiffies + msecs_to_jiffies(MACSMC_FAN_MODE_TIMEOUT_MS); for (;;) { ret = macsmc_hwmon_fan_read_mode(hwmon, fan, &mode); - if (!ret && mode == MACSMC_FAN_MODE_SYSTEM) - ret = -EBUSY; - else if (!ret) - ret = macsmc_hwmon_write_key(hwmon->smc, &fan->mode, - MACSMC_FAN_MODE_MANUAL); - if (ret >= 0 || time_after(jiffies, timeout)) + if (ret || mode != MACSMC_FAN_MODE_SYSTEM) + break; + if (time_after(jiffies, timeout)) { + ret = -ETIMEDOUT; break; + } msleep(MACSMC_FAN_MODE_POLL_MS); } - if (ret < 0) { - dev_err(hwmon->dev, "%s refused manual mode after unlock (%d)\n", - fan->label, ret); - if (!hwmon->fan.manual) - macsmc_hwmon_fans_relock(hwmon); - return ret; - } + if (ret) + goto relock; } + + ret = macsmc_hwmon_write_key(hwmon->smc, &fan->mode, MACSMC_FAN_MODE_MANUAL); if (ret < 0) - return ret; + goto relock; fan->manual = true; hwmon->fan.manual++; return 0; -} - -/* The speed of a cooling state of a fan running from @min to @max. */ -static long macsmc_hwmon_fan_state_rpm(long min, long max, unsigned long state) -{ - if (!state) - return 0; - state = min(state, MACSMC_FAN_COOLING_STATES); - return min + DIV_ROUND_CLOSEST((max - min) * (long)(state - 1), - MACSMC_FAN_COOLING_STATES - 1); -} - -/* - * Run a fan at the higher of the userspace target and the speed of its - * cooling state, or hand it to the firmware when neither asks for anything. - * Any failure leaves the fan with the firmware. Called with the fan lock held. - */ -static int macsmc_hwmon_fan_apply(struct macsmc_hwmon *hwmon, - struct macsmc_hwmon_fan *fan) -{ - unsigned long state = READ_ONCE(fan->cool_state); - long min, max, rpm; - int ret; - if (hwmon->fan.stopped || (!fan->user_rpm && !state)) - return macsmc_hwmon_fan_auto(hwmon, fan); - - /* - * The SMC does no sanity checks on requested fan speeds, so we need to. - */ - ret = macsmc_hwmon_read_key(hwmon->smc, &fan->min, 1, &min); - if (!ret) - ret = macsmc_hwmon_read_key(hwmon->smc, &fan->max, 1, &max); - if (!ret && (min <= 0 || max < min)) - ret = -EIO; - if (ret) { - macsmc_hwmon_fan_auto(hwmon, fan); - return ret; - } - - rpm = max(fan->user_rpm, macsmc_hwmon_fan_state_rpm(min, max, state)); - rpm = clamp(rpm, min, max); - - ret = macsmc_hwmon_fan_manual(hwmon, fan); - if (ret) - return ret; - - ret = macsmc_hwmon_write_key(hwmon->smc, &fan->set, rpm); - if (ret < 0) { - macsmc_hwmon_fan_auto(hwmon, fan); - return ret; - } - - return 0; +relock: + if (hwmon->fan.unlocked && !hwmon->fan.manual) + macsmc_hwmon_fans_relock(hwmon); + return ret; } /* Whether userspace may set this fan's speed. */ @@ -601,6 +537,7 @@ static bool macsmc_hwmon_fan_user_control(const struct macsmc_hwmon_fan *fan) static int macsmc_hwmon_set_fan(struct macsmc_hwmon *hwmon, int channel, long val) { struct macsmc_hwmon_fan *fan = &hwmon->fan.fans[channel]; + long min, max; int ret; if (!macsmc_hwmon_fan_user_control(fan)) @@ -610,11 +547,25 @@ static int macsmc_hwmon_set_fan(struct macsmc_hwmon *hwmon, int channel, long va guard(mutex)(&hwmon->fan.lock); - /* 0 drops the request; the fan goes back to the firmware unless cooling. */ - fan->user_rpm = val; - ret = macsmc_hwmon_fan_apply(hwmon, fan); - if (ret) - fan->user_rpm = 0; + if (!val) + return macsmc_hwmon_fan_auto(hwmon, fan); + if (hwmon->fan.stopped) + return -EBUSY; + + /* The SMC does no sanity checks on requested fan speeds. */ + ret = macsmc_hwmon_read_key(hwmon->smc, &fan->min, 1, &min); + if (!ret) + ret = macsmc_hwmon_read_key(hwmon->smc, &fan->max, 1, &max); + if (!ret && (min <= 0 || max < min)) + ret = -EIO; + if (!ret) + ret = macsmc_hwmon_fan_manual(hwmon, fan); + if (!ret) { + ret = macsmc_hwmon_write_key(hwmon->smc, &fan->set, clamp(val, min, max)); + if (ret >= 0) + return 0; + } + macsmc_hwmon_fan_auto(hwmon, fan); return ret; } @@ -635,129 +586,15 @@ static void macsmc_hwmon_fans_restore(struct macsmc_hwmon *hwmon) macsmc_hwmon_fan_auto(hwmon, &hwmon->fan.fans[i]); } -/* Bring every fan in line with its requests, unless stopped. */ -static void macsmc_hwmon_fans_apply(struct macsmc_hwmon *hwmon) -{ - u32 i; - int ret; - - guard(mutex)(&hwmon->fan.lock); - for (i = 0; i < hwmon->fan.count; i++) { - struct macsmc_hwmon_fan *fan = &hwmon->fan.fans[i]; - - ret = macsmc_hwmon_fan_apply(hwmon, fan); - if (ret) - dev_warn_ratelimited(hwmon->dev, - "%s left to the SMC: cannot run cooling state %lu (%d)\n", - fan->label, READ_ONCE(fan->cool_state), ret); - } -} - -static void macsmc_hwmon_fans_cool_work(struct work_struct *work) +static void macsmc_hwmon_fans_restore_action(void *data) { - macsmc_hwmon_fans_apply(container_of(work, struct macsmc_hwmon, fan.cool_work)); -} + struct macsmc_hwmon *hwmon = data; -/* - * Stop applying cooling states and hand every fan back to the firmware, for - * suspend, shutdown and removal. - */ -static void macsmc_hwmon_fans_stop(struct macsmc_hwmon *hwmon) -{ scoped_guard(mutex, &hwmon->fan.lock) hwmon->fan.stopped = true; - cancel_work_sync(&hwmon->fan.cool_work); macsmc_hwmon_fans_restore(hwmon); } -static void macsmc_hwmon_fans_restore_action(void *data) -{ - macsmc_hwmon_fans_stop(data); -} - -#if IS_ENABLED(CONFIG_THERMAL_OF) -static int macsmc_hwmon_fan_get_max_state(struct thermal_cooling_device *cdev, - unsigned long *state) -{ - *state = MACSMC_FAN_COOLING_STATES; - return 0; -} - -static int macsmc_hwmon_fan_get_cur_state(struct thermal_cooling_device *cdev, - unsigned long *state) -{ - struct macsmc_hwmon_fan *fan = cdev->devdata; - - *state = READ_ONCE(fan->cool_state); - return 0; -} - -/* - * Unlocking the fans takes the firmware seconds, so the state is applied - * from a work item rather than from the thermal zone update. - */ -static int macsmc_hwmon_fan_set_cur_state(struct thermal_cooling_device *cdev, - unsigned long state) -{ - struct macsmc_hwmon_fan *fan = cdev->devdata; - - if (state > MACSMC_FAN_COOLING_STATES) - return -EINVAL; - - WRITE_ONCE(fan->cool_state, state); - queue_work(system_freezable_wq, &fan->hwmon->fan.cool_work); - return 0; -} - -static const struct thermal_cooling_device_ops macsmc_hwmon_fan_cooling_ops = { - .get_max_state = macsmc_hwmon_fan_get_max_state, - .get_cur_state = macsmc_hwmon_fan_get_cur_state, - .set_cur_state = macsmc_hwmon_fan_set_cur_state, -}; - -/* - * With #cooling-cells = <3> on the hwmon node, every fan is a cooling device - * for the thermal zones that map it as <&hwmon fan-index lower upper>, the - * index counting the fan-* nodes in order. Without it the fans stay with the - * firmware as before. - */ -static void macsmc_hwmon_fans_register_cooling(struct macsmc_hwmon *hwmon) -{ - const u32 needed = HWMON_F_MIN | HWMON_F_MAX | HWMON_F_TARGET; - u32 cells, i; - - if (!hwmon->fan.count || - of_property_read_u32(hwmon->dev->of_node, "#cooling-cells", &cells)) - return; - if (cells != 3) { - dev_warn(hwmon->dev, "#cooling-cells must be 3; fans left to the SMC\n"); - return; - } - - for (i = 0; i < hwmon->fan.count; i++) { - struct macsmc_hwmon_fan *fan = &hwmon->fan.fans[i]; - struct thermal_cooling_device *cdev; - - if ((fan->attrs & needed) != needed || !fan->mode.macsmc_key) { - dev_warn(hwmon->dev, "%s has no speed or mode keys; not a cooling device\n", - fan->label); - continue; - } - - cdev = devm_thermal_of_cooling_device_register(hwmon->dev, fan->index, - "macsmc-fan", fan, - &macsmc_hwmon_fan_cooling_ops); - if (IS_ERR(cdev)) - dev_warn(hwmon->dev, "%s: cannot register a cooling device (%pe)\n", - fan->label, cdev); - } -} -#else -static void macsmc_hwmon_fans_register_cooling(struct macsmc_hwmon *hwmon) -{ -} -#endif - static int macsmc_hwmon_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, int channel, long *val) { @@ -1208,8 +1045,6 @@ static int macsmc_hwmon_populate_sensors(struct macsmc_hwmon *hwmon, for_each_child_of_node_with_prefix(hwmon_node, key_node, "fan-") { struct macsmc_hwmon_fan *fan = &hwmon->fan.fans[hwmon->fan.count]; - fan->hwmon = hwmon; - fan->index = n_fan++; if (!macsmc_hwmon_create_fan(hwmon->dev, hwmon->smc, key_node, fan)) hwmon->fan.count++; } @@ -1603,7 +1438,6 @@ static int macsmc_hwmon_probe(struct platform_device *pdev) ret = devm_mutex_init(&pdev->dev, &hwmon->fan.lock); if (ret) return ret; - INIT_WORK(&hwmon->fan.cool_work, macsmc_hwmon_fans_cool_work); ret = macsmc_hwmon_populate_sensors(hwmon, hwmon->dev->of_node); if (ret) { @@ -1665,12 +1499,6 @@ static int macsmc_hwmon_probe(struct platform_device *pdev) if (ret) return ret; - /* - * Released before the restore action above, which stops the cooling - * work and hands the fans back. - */ - macsmc_hwmon_fans_register_cooling(hwmon); - dev_dbg(hwmon->dev, "Registered SMC hwmon device. Sensors:\n"); dev_dbg(hwmon->dev, "Current: %d, Fans: %d, Power: %d, Temperature: %d, Voltage: %d", @@ -1692,27 +1520,22 @@ static void macsmc_hwmon_shutdown(struct platform_device *pdev) struct macsmc_hwmon *hwmon = platform_get_drvdata(pdev); if (hwmon) - macsmc_hwmon_fans_stop(hwmon); + macsmc_hwmon_fans_restore_action(hwmon); } static int macsmc_hwmon_suspend(struct device *dev) { - macsmc_hwmon_fans_stop(dev_get_drvdata(dev)); + macsmc_hwmon_fans_restore_action(dev_get_drvdata(dev)); return 0; } -/* Cooling states asked for before suspend apply again; the zones update them. */ +/* Manual requests end at suspend; resume never writes fan keys. */ static int macsmc_hwmon_resume(struct device *dev) { struct macsmc_hwmon *hwmon = dev_get_drvdata(dev); - scoped_guard(mutex, &hwmon->fan.lock) { - hwmon->fan.stopped = false; - /* The userspace targets were dropped when the fans went back. */ - for (u32 i = 0; i < hwmon->fan.count; i++) - hwmon->fan.fans[i].user_rpm = 0; - } - queue_work(system_freezable_wq, &hwmon->fan.cool_work); + guard(mutex)(&hwmon->fan.lock); + hwmon->fan.stopped = false; return 0; } From b8e84d08eef58535786a4ae9ca695d577d1439b4 Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 31 Oct 2026 15:49:50 +1100 Subject: [PATCH 3/6] power: supply: macsmc: Preserve firmware charge settings at probe Probing a battery should not reset its persistent charge policy. Stop resetting the charging inhibitors and undocumented CH0K/CH0B keys merely because the driver bound. The earlier CHLS repair cleared a force-discharge bit left without a limit by old kernels. Warn about that state without changing it; an explicit charge_behaviour=auto request still clears it while preserving the limit byte. Keep the existing serialization and explicit charge controls. Update the KUnit coverage to verify read-only observation and explicit recovery. Assisted-by: LLM Signed-off-by: Ryan Murray --- drivers/power/supply/macsmc-power-test.c | 39 ++++++++---------------- drivers/power/supply/macsmc-power.c | 37 +++------------------- 2 files changed, 17 insertions(+), 59 deletions(-) diff --git a/drivers/power/supply/macsmc-power-test.c b/drivers/power/supply/macsmc-power-test.c index 4a86c9bd9f155..b6f9b50173d01 100644 --- a/drivers/power/supply/macsmc-power-test.c +++ b/drivers/power/supply/macsmc-power-test.c @@ -637,41 +637,28 @@ static void macsmc_charge_serialized_test(struct kunit *test) KUNIT_EXPECT_EQ(test, t->chls, 0x013c); } -/* Probe clears force-discharge left without a limit, and only that. */ +/* A stale force-discharge bit is cleared only by an explicit auto request. */ static void macsmc_charge_clear_stray_discharge_test(struct kunit *test) { - static const struct { - u16 before, after; - unsigned int writes; - } cases[] = { - { 0x0100, 0x0000, 1 }, - { 0x0164, 0x0064, 1 }, - { 0x0105, 0x0005, 1 }, - { 0x0150, 0x0150, 0 }, /* auto-discharge with a limit stays */ - { 0x0050, 0x0050, 0 }, - { 0x0000, 0x0000, 0 }, - }; + static const u16 values[] = { 0x0100, 0x0164, 0x0105, 0x0150, 0x0050, 0x0000 }; struct macsmc_charge_test *t = test->priv; unsigned int i; t->power.has_chls = true; - for (i = 0; i < ARRAY_SIZE(cases); i++) { - t->chls = cases[i].before; + for (i = 0; i < ARRAY_SIZE(values); i++) { + t->chls = values[i]; t->nops = 0; - KUNIT_EXPECT_EQ_MSG(test, macsmc_battery_clear_stray_discharge(&t->power), 0, + macsmc_battery_get_charge_behaviour(&t->power); + KUNIT_EXPECT_EQ_MSG(test, t->chls, values[i], "read case %u", i); + KUNIT_EXPECT_EQ(test, charge_test_writes(t), 0U); + KUNIT_EXPECT_EQ_MSG(test, + charge_test_set(t, POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR, + POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO), + 0, "case %u", i); + KUNIT_EXPECT_EQ_MSG(test, t->chls, values[i] & ~CHLS_FORCE_DISCHARGE, "case %u", i); - KUNIT_EXPECT_EQ_MSG(test, t->chls, cases[i].after, "case %u", i); - KUNIT_EXPECT_EQ_MSG(test, charge_test_writes(t), cases[i].writes, "case %u", i); + KUNIT_EXPECT_EQ_MSG(test, charge_test_writes(t), 1U, "case %u", i); } - - t->chls = 0x0100; - t->fail_key = SMC_KEY(CHLS); - t->fail_err = -ETIMEDOUT; - KUNIT_EXPECT_EQ(test, macsmc_battery_clear_stray_discharge(&t->power), -ETIMEDOUT); - t->fail_write = true; - t->fail_err = -EIO; - KUNIT_EXPECT_EQ(test, macsmc_battery_clear_stray_discharge(&t->power), -EIO); - KUNIT_EXPECT_EQ(test, t->chls, 0x0100); } static struct kunit_case macsmc_charge_test_cases[] = { diff --git a/drivers/power/supply/macsmc-power.c b/drivers/power/supply/macsmc-power.c index 36d59e5ea315a..1bc23703654a4 100644 --- a/drivers/power/supply/macsmc-power.c +++ b/drivers/power/supply/macsmc-power.c @@ -786,25 +786,6 @@ static int macsmc_battery_set_property(struct power_supply *psy, return ret < 0 ? ret : 0; } -/* - * An earlier kernel could leave force-discharge set in CHLS without a charge limit, which this - * driver never asks of the SMC. Clear the bit once at probe; the limit byte is kept. - */ -static int macsmc_battery_clear_stray_discharge(struct macsmc_power *power) -{ - u16 chls; - int ret; - - guard(mutex)(&power->charge_lock); - ret = apple_smc_read_u16(power->smc, SMC_KEY(CHLS), &chls); - if (ret) - return ret; - if (!(chls & CHLS_FORCE_DISCHARGE) || macsmc_chls_has_limit(chls)) - return 0; - ret = apple_smc_write_u16(power->smc, SMC_KEY(CHLS), chls & ~CHLS_FORCE_DISCHARGE); - return ret < 0 ? ret : 0; -} - static int macsmc_battery_property_is_writeable(struct power_supply *psy, enum power_supply_property psp) { @@ -1108,26 +1089,16 @@ static int macsmc_power_probe(struct platform_device *pdev) of_machine_is_compatible("apple,j700"); power->status_from_current = of_machine_is_compatible("apple,j700"); - /* Reset "Optimised Battery Charging" flags to default state */ - if (power->has_chte) - apple_smc_write_u32(smc, SMC_KEY(CHTE), 0); - else if (power->has_ch0c) - apple_smc_write_u8(smc, SMC_KEY(CH0C), 0); - - if (power->has_ch0i) - apple_smc_write_u8(smc, SMC_KEY(CH0I), 0); - - apple_smc_write_u8(smc, SMC_KEY(CH0K), 0); - apple_smc_write_u8(smc, SMC_KEY(CH0B), 0); - + /* Preserve firmware charge policy until userspace changes it. */ /* Detect charge limit method (CHWA vs CHLS) */ if (apple_smc_read_flag(power->smc, SMC_KEY(CHWA), &flag) == 0) power->has_chwa = true; else if (apple_smc_read_u16(power->smc, SMC_KEY(CHLS), &vu16) >= 0) power->has_chls = true; - if (power->has_chls && macsmc_battery_clear_stray_discharge(power)) - dev_warn(dev, "Failed to clear force-discharge without a charge limit\n"); + if (power->has_chls && (vu16 & CHLS_FORCE_DISCHARGE) && + !macsmc_chls_has_limit(vu16)) + dev_warn(dev, "CHLS force-discharge without a limit; select charge_behaviour=auto to clear\n"); /* * CHLS carries auto-discharge, so a CHLS-only Mac can select auto From 03ce684ab51ace54241c96463bc0674a6339c5e1 Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 31 Oct 2026 15:49:50 +1100 Subject: [PATCH 4/6] arm64: dts: apple: Use firmware fan policy and expose thermal pressure Both firmware key inventories include the readable signed 32-bit mTPL key. Select it through the existing thermal-pressure property so the shared driver exposes its read-only level. On J616s macOS polls this key once per second; it is distinct from the temperature keys. Remove the J616s host active fan trips and maps. The SMC ramps the fans under sustained load without host key writes, so the earlier assumption behind the Linux fan ladder does not hold. Preserve every temperature channel, passive CPU/GPU cooling map, critical trip, and explicit manual fan control property. Assisted-by: LLM Signed-off-by: Ryan Murray --- arch/arm64/boot/dts/apple/t6040-j616s.dts | 66 ++--------------------- arch/arm64/boot/dts/apple/t8140-j700.dts | 2 + 2 files changed, 6 insertions(+), 62 deletions(-) diff --git a/arch/arm64/boot/dts/apple/t6040-j616s.dts b/arch/arm64/boot/dts/apple/t6040-j616s.dts index 69f0070af7659..79ed4de3c98be 100644 --- a/arch/arm64/boot/dts/apple/t6040-j616s.dts +++ b/arch/arm64/boot/dts/apple/t6040-j616s.dts @@ -322,41 +322,9 @@ * GPU is powered off its sensors report placeholder values from -4 to * 4 C. * - * The fans: left to themselves (SMC mode 3, with no OS asking for - * cooling) they stay off, or switch on late and ramp slowly, while the - * dies heat up under load. So the CPU die zone and the GPU zones also - * run both fans through a ladder of active trips; a fan takes the - * highest state any zone asks for and goes back to the SMC once no - * zone asks for anything. States 1-8 of a fan run it from its minimum - * (1350 rpm) to its maximum (5777 rpm) in even steps. + * Fan speeds are governed by the SMC firmware. Manual speeds are + * available only through an explicit userspace fan target request. */ -#define J616S_FAN_TRIP(label, node, temp, hyst) \ - label: node { \ - temperature = ; \ - hysteresis = ; \ - type = "active"; \ - }; - -#define J616S_FAN_TRIPS(zone) \ - J616S_FAN_TRIP(zone##_fan1, trip-fan1, 75000, 10000) \ - J616S_FAN_TRIP(zone##_fan2, trip-fan2, 80000, 5000) \ - J616S_FAN_TRIP(zone##_fan3, trip-fan3, 85000, 5000) \ - J616S_FAN_TRIP(zone##_fan4, trip-fan4, 90000, 5000) \ - J616S_FAN_TRIP(zone##_fan5, trip-fan5, 95000, 5000) - -#define J616S_FAN_MAP(trip_label, node, state) \ - node { \ - trip = <&trip_label>; \ - cooling-device = <&smc_hwmon 0 state state>, \ - <&smc_hwmon 1 state state>; \ - }; - -#define J616S_FAN_MAPS(zone) \ - J616S_FAN_MAP(zone##_fan1, map-fan1, 1) \ - J616S_FAN_MAP(zone##_fan2, map-fan2, 2) \ - J616S_FAN_MAP(zone##_fan3, map-fan3, 3) \ - J616S_FAN_MAP(zone##_fan4, map-fan4, 5) \ - J616S_FAN_MAP(zone##_fan5, map-fan5, 8) thermal-zones { cpu-die-thermal { @@ -365,7 +333,6 @@ thermal-sensors = <&smc_hwmon 0>; /* TCMz */ trips { - J616S_FAN_TRIPS(cpu_die) cpu_die_passive: cpu-die-passive { temperature = <113000>; @@ -381,7 +348,6 @@ }; cooling-maps { - J616S_FAN_MAPS(cpu_die) map-e { trip = <&cpu_die_passive>; @@ -928,7 +894,6 @@ thermal-sensors = <&smc_hwmon 17>; /* Tg05 */ trips { - J616S_FAN_TRIPS(gpu0) gpu0_passive: trip-passive { temperature = <101000>; @@ -944,7 +909,6 @@ }; cooling-maps { - J616S_FAN_MAPS(gpu0) map-e { trip = <&gpu0_passive>; @@ -983,7 +947,6 @@ thermal-sensors = <&smc_hwmon 18>; /* Tg0S */ trips { - J616S_FAN_TRIPS(gpu1) gpu1_passive: trip-passive { temperature = <101000>; @@ -999,7 +962,6 @@ }; cooling-maps { - J616S_FAN_MAPS(gpu1) map-e { trip = <&gpu1_passive>; @@ -1038,7 +1000,6 @@ thermal-sensors = <&smc_hwmon 19>; /* Tg0Y */ trips { - J616S_FAN_TRIPS(gpu2) gpu2_passive: trip-passive { temperature = <101000>; @@ -1054,7 +1015,6 @@ }; cooling-maps { - J616S_FAN_MAPS(gpu2) map-e { trip = <&gpu2_passive>; @@ -1093,7 +1053,6 @@ thermal-sensors = <&smc_hwmon 20>; /* Tg0k */ trips { - J616S_FAN_TRIPS(gpu3) gpu3_passive: trip-passive { temperature = <101000>; @@ -1109,7 +1068,6 @@ }; cooling-maps { - J616S_FAN_MAPS(gpu3) map-e { trip = <&gpu3_passive>; @@ -1148,7 +1106,6 @@ thermal-sensors = <&smc_hwmon 21>; /* Tg0z */ trips { - J616S_FAN_TRIPS(gpu4) gpu4_passive: trip-passive { temperature = <101000>; @@ -1164,7 +1121,6 @@ }; cooling-maps { - J616S_FAN_MAPS(gpu4) map-e { trip = <&gpu4_passive>; @@ -1203,7 +1159,6 @@ thermal-sensors = <&smc_hwmon 22>; /* Tg1V */ trips { - J616S_FAN_TRIPS(gpu5) gpu5_passive: trip-passive { temperature = <101000>; @@ -1219,7 +1174,6 @@ }; cooling-maps { - J616S_FAN_MAPS(gpu5) map-e { trip = <&gpu5_passive>; @@ -1258,7 +1212,6 @@ thermal-sensors = <&smc_hwmon 23>; /* Tg1l */ trips { - J616S_FAN_TRIPS(gpu6) gpu6_passive: trip-passive { temperature = <101000>; @@ -1274,7 +1227,6 @@ }; cooling-maps { - J616S_FAN_MAPS(gpu6) map-e { trip = <&gpu6_passive>; @@ -3851,23 +3803,13 @@ }; }; -/* - * Fan speeds, limits, targets and modes, and the common SMC sensors; every - * key exists on this machine's SMC. Both fans list their target (F0Tg, - * F1Tg) and mode (F0Md, F1Md) keys, so with the driver's fan_control - * parameter fanN_target becomes writable; by default the attributes are - * read-only, and the thermal zones above drive the fans. - */ +/* Fan monitoring and explicit userspace manual control. */ #include "hwmon-common.dtsi" #include "hwmon-fan-dual.dtsi" #include "hwmon-laptop.dtsi" -/* - * Both fans are cooling devices of the CPU die and GPU zones, as - * <&smc_hwmon fan-index lower upper>. - */ &smc_hwmon { - #cooling-cells = <3>; + apple,thermal-pressure-key = "mTPL"; /* * The SMC accepts a manual speed for both fans once its fan test diff --git a/arch/arm64/boot/dts/apple/t8140-j700.dts b/arch/arm64/boot/dts/apple/t8140-j700.dts index 043c6b5d085fb..a3044890100e2 100644 --- a/arch/arm64/boot/dts/apple/t8140-j700.dts +++ b/arch/arm64/boot/dts/apple/t8140-j700.dts @@ -625,6 +625,8 @@ }; &smc_hwmon { + apple,thermal-pressure-key = "mTPL"; + /* * Further channels, reported to userspace. They follow the six CPU * die temperatures in t8140.dtsi, which the thermal zones above From 3db457a10d5d2a5911732591570d707e06e264c9 Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 31 Oct 2026 15:58:22 +1100 Subject: [PATCH 5/6] hwmon: macsmc: Keep explicit control of write-only legacy mode keys The mode preflight should not introduce a read requirement for a legacy write-only mode key. Use the metadata to keep its existing direct write path, still solely under an explicit user target. Readable system-mode keys keep the unlock-before-write sequence. Cover this metadata case with the SMC model, asserting no mode read and no unlock operation for the legacy write-only path. Assisted-by: LLM Signed-off-by: Ryan Murray --- drivers/hwmon/macsmc-hwmon-test.c | 19 +++++++++++++++++++ drivers/hwmon/macsmc-hwmon.c | 5 +++++ 2 files changed, 24 insertions(+) diff --git a/drivers/hwmon/macsmc-hwmon-test.c b/drivers/hwmon/macsmc-hwmon-test.c index 610a019ee59b8..1f8fa33dc2e0e 100644 --- a/drivers/hwmon/macsmc-hwmon-test.c +++ b/drivers/hwmon/macsmc-hwmon-test.c @@ -109,6 +109,7 @@ struct macsmc_fan_test { int writes; int ftst_writes; int refused; + int mode_reads; smc_key fail_key; }; @@ -125,6 +126,7 @@ static int fan_test_read(struct apple_smc *smc, smc_key key, void *buf, size_t s switch (key & 0xff00ffff) { case __SMC_KEY('F', 0, 'M', 'd'): + t->mode_reads++; value = t->mode[i]; break; case __SMC_KEY('F', 0, 'M', 'n'): @@ -330,6 +332,22 @@ static void macsmc_fan_test_legacy(struct kunit *test) KUNIT_EXPECT_EQ(test, t->refused, 0); } +/* A legacy write-only mode key keeps working without a new read requirement. */ +static void macsmc_fan_test_legacy_write_only(struct kunit *test) +{ + struct macsmc_fan_test *t = test->priv; + + t->legacy = true; + t->mode[0] = MACSMC_FAN_MODE_AUTO; + t->fans[0].mode.info.flags &= ~APPLE_SMC_READABLE; + KUNIT_ASSERT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 2500), 0); + KUNIT_EXPECT_EQ(test, t->mode[0], MACSMC_FAN_MODE_MANUAL); + KUNIT_ASSERT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 0), 0); + KUNIT_EXPECT_EQ(test, t->mode[0], MACSMC_FAN_MODE_AUTO); + KUNIT_EXPECT_EQ(test, t->mode_reads, 0); + KUNIT_EXPECT_EQ(test, t->ftst_writes, 0); +} + /* Suspend, resume and shutdown do not write untouched fan keys. */ static void macsmc_fan_test_untouched(struct kunit *test) { @@ -384,6 +402,7 @@ static struct kunit_case macsmc_fan_test_cases[] = { KUNIT_CASE(macsmc_fan_test_two_fans), KUNIT_CASE(macsmc_fan_test_no_unlock_key), KUNIT_CASE(macsmc_fan_test_legacy), + KUNIT_CASE(macsmc_fan_test_legacy_write_only), KUNIT_CASE(macsmc_fan_test_untouched), KUNIT_CASE(macsmc_fan_test_suspend_manual), KUNIT_CASE(macsmc_fan_test_unlock_fails), diff --git a/drivers/hwmon/macsmc-hwmon.c b/drivers/hwmon/macsmc-hwmon.c index a8e47ee4b98ff..097fa6c40c610 100644 --- a/drivers/hwmon/macsmc-hwmon.c +++ b/drivers/hwmon/macsmc-hwmon.c @@ -480,6 +480,10 @@ static int macsmc_hwmon_fan_manual(struct macsmc_hwmon *hwmon, if (fan->manual) return 0; + /* Older firmware may expose a write-only mode key. */ + if (!(fan->mode.info.flags & APPLE_SMC_READABLE)) + goto set_manual; + /* Do not send a mode write the firmware is known to refuse. */ ret = macsmc_hwmon_fan_read_mode(hwmon, fan, &mode); if (ret) @@ -514,6 +518,7 @@ static int macsmc_hwmon_fan_manual(struct macsmc_hwmon *hwmon, goto relock; } +set_manual: ret = macsmc_hwmon_write_key(hwmon->smc, &fan->mode, MACSMC_FAN_MODE_MANUAL); if (ret < 0) goto relock; From 52cbd7f4041605e6afe8b969410baaf767f626a8 Mon Sep 17 00:00:00 2001 From: Ryan Murray Date: Sat, 31 Oct 2026 16:35:57 +1100 Subject: [PATCH 6/6] arm64: dts: apple: Isolate the final J616s firmware fan policy Keep ladder definitions and call sites stable while removing their fan trip/map instances in a final policy section. This makes independent CPU ladder updates apply cleanly in either aggregate PR landing order without restoring automatic fan writes. Drop the optional fan map on the passive trip as well, preserving the trip and its CPU cooling maps. The effective device tree retains firmware fan ownership, all existing sensor channels, and passive/critical protection. No GPU temperature producer, conversion or validity policy changes are included. Assisted-by: LLM Signed-off-by: Ryan Murray --- arch/arm64/boot/dts/apple/t6040-j616s.dts | 162 +++++++++++++++++++++- 1 file changed, 160 insertions(+), 2 deletions(-) diff --git a/arch/arm64/boot/dts/apple/t6040-j616s.dts b/arch/arm64/boot/dts/apple/t6040-j616s.dts index 79ed4de3c98be..df0ee7681ff47 100644 --- a/arch/arm64/boot/dts/apple/t6040-j616s.dts +++ b/arch/arm64/boot/dts/apple/t6040-j616s.dts @@ -322,9 +322,41 @@ * GPU is powered off its sensors report placeholder values from -4 to * 4 C. * - * Fan speeds are governed by the SMC firmware. Manual speeds are - * available only through an explicit userspace fan target request. + * The fans: left to themselves (SMC mode 3, with no OS asking for + * cooling) they stay off, or switch on late and ramp slowly, while the + * dies heat up under load. So the CPU die zone and the GPU zones also + * run both fans through a ladder of active trips; a fan takes the + * highest state any zone asks for and goes back to the SMC once no + * zone asks for anything. States 1-8 of a fan run it from its minimum + * (1350 rpm) to its maximum (5777 rpm) in even steps. */ +#define J616S_FAN_TRIP(label, node, temp, hyst) \ + label: node { \ + temperature = ; \ + hysteresis = ; \ + type = "active"; \ + }; + +#define J616S_FAN_TRIPS(zone) \ + J616S_FAN_TRIP(zone##_fan1, trip-fan1, 75000, 10000) \ + J616S_FAN_TRIP(zone##_fan2, trip-fan2, 80000, 5000) \ + J616S_FAN_TRIP(zone##_fan3, trip-fan3, 85000, 5000) \ + J616S_FAN_TRIP(zone##_fan4, trip-fan4, 90000, 5000) \ + J616S_FAN_TRIP(zone##_fan5, trip-fan5, 95000, 5000) + +#define J616S_FAN_MAP(trip_label, node, state) \ + node { \ + trip = <&trip_label>; \ + cooling-device = <&smc_hwmon 0 state state>, \ + <&smc_hwmon 1 state state>; \ + }; + +#define J616S_FAN_MAPS(zone) \ + J616S_FAN_MAP(zone##_fan1, map-fan1, 1) \ + J616S_FAN_MAP(zone##_fan2, map-fan2, 2) \ + J616S_FAN_MAP(zone##_fan3, map-fan3, 3) \ + J616S_FAN_MAP(zone##_fan4, map-fan4, 5) \ + J616S_FAN_MAP(zone##_fan5, map-fan5, 8) thermal-zones { cpu-die-thermal { @@ -333,6 +365,7 @@ thermal-sensors = <&smc_hwmon 0>; /* TCMz */ trips { + J616S_FAN_TRIPS(cpu_die) cpu_die_passive: cpu-die-passive { temperature = <113000>; @@ -348,6 +381,7 @@ }; cooling-maps { + J616S_FAN_MAPS(cpu_die) map-e { trip = <&cpu_die_passive>; @@ -894,6 +928,7 @@ thermal-sensors = <&smc_hwmon 17>; /* Tg05 */ trips { + J616S_FAN_TRIPS(gpu0) gpu0_passive: trip-passive { temperature = <101000>; @@ -909,6 +944,7 @@ }; cooling-maps { + J616S_FAN_MAPS(gpu0) map-e { trip = <&gpu0_passive>; @@ -947,6 +983,7 @@ thermal-sensors = <&smc_hwmon 18>; /* Tg0S */ trips { + J616S_FAN_TRIPS(gpu1) gpu1_passive: trip-passive { temperature = <101000>; @@ -962,6 +999,7 @@ }; cooling-maps { + J616S_FAN_MAPS(gpu1) map-e { trip = <&gpu1_passive>; @@ -1000,6 +1038,7 @@ thermal-sensors = <&smc_hwmon 19>; /* Tg0Y */ trips { + J616S_FAN_TRIPS(gpu2) gpu2_passive: trip-passive { temperature = <101000>; @@ -1015,6 +1054,7 @@ }; cooling-maps { + J616S_FAN_MAPS(gpu2) map-e { trip = <&gpu2_passive>; @@ -1053,6 +1093,7 @@ thermal-sensors = <&smc_hwmon 20>; /* Tg0k */ trips { + J616S_FAN_TRIPS(gpu3) gpu3_passive: trip-passive { temperature = <101000>; @@ -1068,6 +1109,7 @@ }; cooling-maps { + J616S_FAN_MAPS(gpu3) map-e { trip = <&gpu3_passive>; @@ -1106,6 +1148,7 @@ thermal-sensors = <&smc_hwmon 21>; /* Tg0z */ trips { + J616S_FAN_TRIPS(gpu4) gpu4_passive: trip-passive { temperature = <101000>; @@ -1121,6 +1164,7 @@ }; cooling-maps { + J616S_FAN_MAPS(gpu4) map-e { trip = <&gpu4_passive>; @@ -1159,6 +1203,7 @@ thermal-sensors = <&smc_hwmon 22>; /* Tg1V */ trips { + J616S_FAN_TRIPS(gpu5) gpu5_passive: trip-passive { temperature = <101000>; @@ -1174,6 +1219,7 @@ }; cooling-maps { + J616S_FAN_MAPS(gpu5) map-e { trip = <&gpu5_passive>; @@ -1212,6 +1258,7 @@ thermal-sensors = <&smc_hwmon 23>; /* Tg1l */ trips { + J616S_FAN_TRIPS(gpu6) gpu6_passive: trip-passive { temperature = <101000>; @@ -1227,6 +1274,7 @@ }; cooling-maps { + J616S_FAN_MAPS(gpu6) map-e { trip = <&gpu6_passive>; @@ -4465,3 +4513,113 @@ status = "disabled"; }; }; + +/* + * The SMC owns automatic fan policy. Drop the host fan trip/map instances + * after the board's thermal descriptions, retaining all passive and critical + * protection. Keep this final policy separate from ladder descriptions so + * independent ladder updates cannot restore autonomous SMC writes. + */ + +#define J616S_DELETE_FAN_TRIPS \ + /delete-node/ trip-fan1; \ + /delete-node/ trip-fan2; \ + /delete-node/ trip-fan3; \ + /delete-node/ trip-fan4; \ + /delete-node/ trip-fan5; + +/* Some CPU ladders also map full fan speed to the passive trip. */ +#define J616S_DELETE_FAN_MAPS \ + /delete-node/ map-fan1; \ + /delete-node/ map-fan2; \ + /delete-node/ map-fan3; \ + /delete-node/ map-fan4; \ + /delete-node/ map-fan5; \ + /delete-node/ map-fan6; + +/ { + thermal-zones { + cpu-die-thermal { + trips { + J616S_DELETE_FAN_TRIPS + }; + + cooling-maps { + J616S_DELETE_FAN_MAPS + }; + }; + + gpu0-thermal { + trips { + J616S_DELETE_FAN_TRIPS + }; + + cooling-maps { + J616S_DELETE_FAN_MAPS + }; + }; + + gpu1-thermal { + trips { + J616S_DELETE_FAN_TRIPS + }; + + cooling-maps { + J616S_DELETE_FAN_MAPS + }; + }; + + gpu2-thermal { + trips { + J616S_DELETE_FAN_TRIPS + }; + + cooling-maps { + J616S_DELETE_FAN_MAPS + }; + }; + + gpu3-thermal { + trips { + J616S_DELETE_FAN_TRIPS + }; + + cooling-maps { + J616S_DELETE_FAN_MAPS + }; + }; + + gpu4-thermal { + trips { + J616S_DELETE_FAN_TRIPS + }; + + cooling-maps { + J616S_DELETE_FAN_MAPS + }; + }; + + gpu5-thermal { + trips { + J616S_DELETE_FAN_TRIPS + }; + + cooling-maps { + J616S_DELETE_FAN_MAPS + }; + }; + + gpu6-thermal { + trips { + J616S_DELETE_FAN_TRIPS + }; + + cooling-maps { + J616S_DELETE_FAN_MAPS + }; + }; + }; +}; + +#undef J616S_DELETE_FAN_TRIPS +#undef J616S_DELETE_FAN_MAPS