macsmc: Leave SMC policy to firmware and expose J616s/Neo thermal pressure

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}$":

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 <key>``.

arch/arm64/boot/dts/apple/t6040-j616s.dts

@@ -3851,23 +3851,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
@@ -4523,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

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

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,8 +106,10 @@ struct macsmc_fan_test {
 	u8 mode[2];
 	u32 target[2];
 	u8 ftst;
+	int writes;
 	int ftst_writes;
 	int refused;
+	int mode_reads;
 	smc_key fail_key;
 };
 
@@ -124,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'):
@@ -149,6 +152,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 +222,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 +262,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 +305,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 +332,67 @@ 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)
+/* 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;
 
-	fan_control = false;
-	t->fans[0].cool_state = 1;
-	macsmc_hwmon_fans_apply(&t->hwmon);
-	KUNIT_EXPECT_EQ(test, t->ftst, 1);
+	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_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);
-	fan_test_released(test, t);
+	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);
 }
 
-/* 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, 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 = {};
 
-	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);
+	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);
 }
 
-/* Once stopped (suspend, shutdown, removal), no state takes a fan back. */
-static void macsmc_fan_test_cooling_stopped(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;
 
-	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);
+	dev_set_drvdata(&dev, &t->hwmon);
+	KUNIT_ASSERT_EQ(test, macsmc_hwmon_set_fan(&t->hwmon, 0, 2500), 0);
+	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 +402,10 @@ 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_legacy_write_only),
+	KUNIT_CASE(macsmc_fan_test_untouched),
+	KUNIT_CASE(macsmc_fan_test_suspend_manual),
+	KUNIT_CASE(macsmc_fan_test_unlock_fails),
 	{}
 };