[PATCH 0/8] mfd: macsmc: add rtc, hwmon and hid subdevices

[PATCH 0/8] mfd: macsmc: add rtc, hwmon and hid subdevices

[James Calligeros]

Hi all,

This series adds support for the remaining SMC subdevices. These are the
RTC, hwmon, and HID devices. They are being submitted together as the RTC
and hwmon drivers both require changes to the SMC DT schema.

The RTC driver is responsible for getting and setting the system clock,
and requires an NVMEM cell. This series replaces Sven's original RTC driver
submission [1].

The hwmon function is an interesting one. While each Apple Silicon device
exposes pretty similar sets of sensors, these all seem to be paired to
different SMC keys in the firmware interface. This is true even when the
sensors are on the SoC. For example, an M1 MacBook Pro will use different
keys to access the LITTLE core temperature sensors to an M1 Mac mini. This
necessitates describing which keys correspond to which sensors for each
device individually, and populating the hwmon structs at runtime. We do
this with a node in the device tree. This series includes only the keys
for sensors which we know to be common to all devices. The SMC is also
responsible for monitoring and controlling fan speeds on systems with fans,
which we expose via the hwmon driver.

The SMC also handles the hardware power button and lid switch. Power
button presses and lid opening/closing are emitted as HID events, so we
add a HID subdevice to handle them.

Note that this series is based on a branch with three additional commits
applied to add the parent SMC nodes to the relevant Devicetrees. This
was done to silence build errors. The series applies cleanly to 6.17-rc1.

Regards,

James

[1] https://lore.kernel.org/asahi/CAEg-Je84XxLWH7vznQmPRfjf6GxWOu75ZetwN7AdseAwfMLLrQ@mail.gmail.com/T/#t

Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
Hector Martin (2):
      rtc: Add new rtc-macsmc driver for Apple Silicon Macs
      input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid

James Calligeros (4):
      dt-bindings: hwmon: add Apple System Management Controller hwmon schema
      hwmon: Add Apple Silicon SMC hwmon driver
      arm64: dts: apple: add common hwmon sensors and fans
      arm64: dts: apple: t8103, t600x, t8112: add common hwmon nodes to devices

Sven Peter (2):
      dt-bindings: rtc: Add Apple SMC RTC
      arm64: dts: apple: t8103,t600x,t8112: Add SMC RTC node

 .../bindings/hwmon/apple,smc-hwmon.yaml  | 148 +++++
 .../bindings/mfd/apple,smc.yaml          |  54 ++
 .../bindings/rtc/apple,smc-rtc.yaml      |  35 +
 MAINTAINERS                              |   5 +
 .../boot/dts/apple/hwmon-common.dtsi     |  46 ++
 .../boot/dts/apple/hwmon-fan-dual.dtsi   |  27 +
 arch/arm64/boot/dts/apple/hwmon-fan.dtsi |  21 +
 .../boot/dts/apple/hwmon-laptop.dtsi     |  43 ++
 .../boot/dts/apple/hwmon-mac-mini.dtsi   |  19 +
 .../arm64/boot/dts/apple/t6001-j375c.dts |   2 +
 .../arm64/boot/dts/apple/t6002-j375d.dts |   2 +
 .../arm64/boot/dts/apple/t600x-die0.dtsi |   6 +
 .../boot/dts/apple/t600x-j314-j316.dtsi  |   4 +
 .../arm64/boot/dts/apple/t600x-j375.dtsi |   2 +
 arch/arm64/boot/dts/apple/t8103-j274.dts |   2 +
 arch/arm64/boot/dts/apple/t8103-j293.dts |   3 +
 arch/arm64/boot/dts/apple/t8103-j313.dts |   2 +
 arch/arm64/boot/dts/apple/t8103-j456.dts |   2 +
 arch/arm64/boot/dts/apple/t8103-j457.dts |   2 +
 .../arm64/boot/dts/apple/t8103-jxxx.dtsi |   2 +
 arch/arm64/boot/dts/apple/t8103.dtsi     |   6 +
 arch/arm64/boot/dts/apple/t8112-j413.dts |   2 +
 arch/arm64/boot/dts/apple/t8112-j473.dts |   2 +
 arch/arm64/boot/dts/apple/t8112-j493.dts |   3 +
 .../arm64/boot/dts/apple/t8112-jxxx.dtsi |   2 +
 arch/arm64/boot/dts/apple/t8112.dtsi     |   6 +
 drivers/hwmon/Kconfig                    |  12 +
 drivers/hwmon/Makefile                   |   1 +
 drivers/hwmon/macsmc_hwmon.c             | 858 +++++++++++++++++++++++++
 drivers/input/misc/Kconfig               |  11 +
 drivers/input/misc/Makefile              |   1 +
 drivers/input/misc/macsmc-hid.c          | 210 ++++++
 drivers/mfd/macsmc.c                     |   3 +
 drivers/rtc/Kconfig                      |  11 +
 drivers/rtc/Makefile                     |   1 +
 drivers/rtc/rtc-macsmc.c                 | 141 ++++
 36 files changed, 1697 insertions(+)
---
base-commit: 876d6a70b24869f96ebc8672caf86cb4bae72927
change-id: 20250816-macsmc-subdevs-87032c017d0c

Best regards,
-- 
James Calligeros <jcalligeros99@gmail.com>

[PATCH 1/8] dt-bindings: rtc: Add Apple SMC RTC

[James Calligeros]

From: Sven Peter <sven@kernel.org>

Apple Silicon Macs (M1, etc.) have an RTC that is part of the PMU IC,
but most of the PMU functionality is abstracted out by the SMC.
An additional RTC offset stored inside NVMEM is required to compute
the current date/time.

Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
Reviewed-by: Neal Gompa <neal@gompa.dev>
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 .../bindings/mfd/apple,smc.yaml          |  9 +++++++
 .../bindings/rtc/apple,smc-rtc.yaml      | 35 +++++++++++++++++++++++++
 MAINTAINERS                              |  1 +
 3 files changed, 45 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
index 8a10e270d421ecd703848f64af597de351fcfd74..38f077867bdeedba8a486a63e366e9c943a75681 100644
--- a/Documentation/devicetree/bindings/mfd/apple,smc.yaml
+++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
@@ -41,6 +41,9 @@ properties:
   reboot:
     $ref: /schemas/power/reset/apple,smc-reboot.yaml
 
+  rtc:
+    $ref: /schemas/rtc/apple,smc-rtc.yaml
+
 additionalProperties: false
 
 required:
@@ -75,5 +78,11 @@ examples:
           nvmem-cell-names = "shutdown_flag", "boot_stage",
                              "boot_error_count", "panic_count";
         };
+
+        rtc {
+          compatible = "apple,smc-rtc";
+          nvmem-cells = <&rtc_offset>;
+          nvmem-cell-names = "rtc_offset";
+       };
       };
     };
diff --git a/Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml b/Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..607b610665a28b3ea2e86bd90cb5f3f28ebac726
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/apple,smc-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple SMC RTC
+
+description:
+  Apple Silicon Macs (M1, etc.) have an RTC that is part of the PMU IC,
+  but most of the PMU functionality is abstracted out by the SMC.
+  An additional RTC offset stored inside NVMEM is required to compute
+  the current date/time.
+
+maintainers:
+  - Sven Peter <sven@kernel.org>
+
+properties:
+  compatible:
+    const: apple,smc-rtc
+
+  nvmem-cells:
+    items:
+      - description: 48bit RTC offset, specified in 32768 (2^15) Hz clock ticks
+
+  nvmem-cell-names:
+    items:
+      - const: rtc_offset
+
+required:
+  - compatible
+  - nvmem-cells
+  - nvmem-cell-names
+
+additionalProperties: false
diff --git a/MAINTAINERS b/MAINTAINERS
index fe168477caa45799dfe07de2f54de6d6a1ce0615..aaef8634985b35f54de1123ebb4176602066d177 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2397,6 +2397,7 @@ F:	Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
 F:	Documentation/devicetree/bindings/power/apple*
 F:	Documentation/devicetree/bindings/power/reset/apple,smc-reboot.yaml
 F:	Documentation/devicetree/bindings/pwm/apple,s5l-fpwm.yaml
+F:	Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml
 F:	Documentation/devicetree/bindings/spi/apple,spi.yaml
 F:	Documentation/devicetree/bindings/spmi/apple,spmi.yaml
 F:	Documentation/devicetree/bindings/watchdog/apple,wdt.yaml

-- 
2.50.1

[PATCH 2/8] dt-bindings: hwmon: add Apple System Management Controller hwmon schema

[James Calligeros]

Apple Silicon devices integrate a vast array of sensors, monitoring
current, power, temperature, and voltage across almost every part of
the system. The sensors themselves are all connected to the System
Management Controller (SMC). The SMC firmware exposes the data
reported by these sensors via its standard FourCC-based key-value
API. The SMC is also responsible for monitoring and controlling any
fans connected to the system, exposing them in the same way.

For reasons known only to Apple, each device exposes its sensors with
an almost totally unique set of keys. This is true even for devices
which share an SoC. An M1 Mac mini, for example, will report its core
temperatures on different keys to an M1 MacBook Pro. Worse still, the
SMC does not provide a way to enumerate the available keys at runtime,
nor do the keys follow any sort of reasonable or consistent naming
rules that could be used to deduce their purpose. We must therefore
know which keys are present on any given device, and which function
they serve, ahead of time.

Add a schema so that we can describe the available sensors for a given
Apple Silicon device in the Devicetree.

Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 .../bindings/hwmon/apple,smc-hwmon.yaml  | 148 +++++++++++++++++++++++++
 .../bindings/mfd/apple,smc.yaml          |  45 ++++++++
 2 files changed, 193 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..3ebc0463be4e1ce54005418feaa87ec7254dab6e
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
@@ -0,0 +1,148 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/apple,smc-hwmon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple SMC Hardware Monitoring
+
+description:
+  Apple's System Management Controller (SMC) exposes a vast array of
+  hardware monitoring sensors, including temperature probes, current and
+  voltage sense, power meters, and fan speeds. It also provides endpoints
+  to manually control the speed of each fan individually. Each Apple
+  Silicon device exposes a different set of endpoints via SMC keys. This
+  is true even when two machines share an SoC. The CPU core temperature
+  sensor keys on an M1 Mac mini are different to those on an M1 MacBook
+  Pro, for example.
+
+maintainers:
+  - James Calligeros <jcalligeros99@gmail.com>
+
+properties:
+  compatible:
+    const: apple,smc-hwmon
+
+  current:
+    description: SMC current sense endpoints
+    type: object
+    additionalProperties: false
+    patternProperties:
+      "^current-[A-Za-z0-9]{4}":
+        type: object
+        additionalProperties: false
+        required:
+          - apple,key-id
+        properties:
+          apple,key-id:
+            $ref: /schemas/types.yaml#/definitions/string
+            pattern: "^[A-Za-z0-9]{4}"
+            description: The SMC FourCC key of the desired current sensor.
+              Should match the node's suffix, but doesn't have to.
+          label:
+            $ref: /schemas/types.yaml#/definitions/string
+            description: Human-readable name for the sensor
+
+  fan:
+    description: SMC fan control endpoints. A fan is made up of five
+      SMC keys - the fan's current speed, its minimum speed, its maximum
+      speed, a writeable target speed, and a writeable mode. The SMC will
+      automatically manage system fans unless a 1 is written to the fan's
+      mode key.
+    type: object
+    additionalProperties: false
+    patternProperties:
+      "^fan-[A-Za-z0-9]{4}":
+        type: object
+        additionalProperties: false
+        required:
+          - apple,key-id
+        properties:
+          apple,key-id:
+            $ref: /schemas/types.yaml#/definitions/string
+            pattern: "^[A-Za-z0-9]{4}"
+            description: The SMC FourCC key of the desired fan. This is the
+              main key, which reports the fan's current speed. Sould match
+              the node's suffix, but doesn't have to.
+          apple,fan-minimum:
+            $ref: /schemas/types.yaml#/definitions/string
+            pattern: "^[A-Za-z0-9]{4}"
+            description: The minimum speed the current fan can run at
+          apple,fan-maximum:
+            $ref: /schemas/types.yaml#/definitions/string
+            pattern: "^[A-Za-z0-9]{4}"
+            description: The maximum speed the current fan can run at
+          apple,fan-target:
+            $ref: /schemas/types.yaml#/definitions/string
+            pattern: "^[A-Za-z0-9]{4}"
+            description: Writeable endpoint for setting desired fan speed
+          apple,fan-mode:
+            $ref: /schemas/types.yaml#/definitions/string
+            pattern: "^[A-Za-z0-9]{4}"
+            description: Writeable endpoint to enable/disable manual fan
+              control
+          label:
+            $ref: /schemas/types.yaml#/definitions/string
+            description: Human-readable name for the sensor
+
+  power:
+    description: SMC power meter endpoints
+    type: object
+    additionalProperties: false
+    patternProperties:
+      "^power-[A-Za-z0-9]{4}":
+        type: object
+        additionalProperties: false
+        required:
+          - apple,key-id
+        properties:
+          apple,key-id:
+            $ref: /schemas/types.yaml#/definitions/string
+            pattern: "^[A-Za-z0-9]{4}"
+            description: The SMC FourCC key of the desired power meter.
+              Should match the node's suffix, but doesn't have to.
+          label:
+            $ref: /schemas/types.yaml#/definitions/string
+            description: Human-readable name for the sensor
+
+  temperature:
+    description: SMC temperature sensor endpoints
+    type: object
+    additionalProperties: false
+    patternProperties:
+      "^temperature-[A-Za-z0-9]{4}":
+        type: object
+        additionalProperties: false
+        required:
+          - apple,key-id
+        properties:
+          apple,key-id:
+            $ref: /schemas/types.yaml#/definitions/string
+            pattern: "^[A-Za-z0-9]{4}"
+            description: The SMC FourCC key of the desired temperature
+              sensor. Should match the node's suffix, but doesn't have to.
+          label:
+            $ref: /schemas/types.yaml#/definitions/string
+            description: Human-readable name for the sensor
+
+  voltage:
+    description: SMC voltage sensor endpoints
+    type: object
+    additionalProperties: false
+    patternProperties:
+      "^voltage-[A-Za-z0-9]{4}":
+        type: object
+        additionalProperties: false
+        required:
+          - apple,key-id
+        properties:
+          apple,key-id:
+            $ref: /schemas/types.yaml#/definitions/string
+            pattern: "^[A-Za-z0-9]{4}"
+            description: The SMC FourCC key of the desired voltage
+              sensor. Should match the node's suffix, but doesn't have to.
+          label:
+            $ref: /schemas/types.yaml#/definitions/string
+            description: Human-readable name for the sensor
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
index 38f077867bdeedba8a486a63e366e9c943a75681..370928bb42010edca17839faf0bda4630611a7a2 100644
--- a/Documentation/devicetree/bindings/mfd/apple,smc.yaml
+++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
@@ -44,6 +44,9 @@ properties:
   rtc:
     $ref: /schemas/rtc/apple,smc-rtc.yaml
 
+  hwmon:
+    $ref: /schemas/hwmon/apple,smc-hwmon.yaml
+
 additionalProperties: false
 
 required:
@@ -84,5 +87,47 @@ examples:
           nvmem-cells = <&rtc_offset>;
           nvmem-cell-names = "rtc_offset";
        };
+
+        hwmon {
+          compatible = "apple,smc-hwmon";
+          current {
+            current-ID0R {
+              apple,key-id = "ID0R";
+              label = "AC Input Current";
+            };
+          };
+
+          fan {
+            fan-F0Ac {
+              apple,key-id = "F0Ac";
+              apple,fan-minimum = "F0Mn";
+              apple,fan-maximum = "F0Mx";
+              apple,fan-target = "F0Tg";
+              apple,fan-mode = "F0Md";
+              label = "Fan 1";
+            };
+          };
+
+          power {
+            power-PSTR {
+              apple,key-id = "PSTR";
+              label = "Total System Power";
+            };
+          };
+
+          temperature {
+            temperature-TW0P {
+              apple,key-id = "TW0P";
+              label = "WiFi/BT Module Temperature";
+            };
+          };
+
+          voltage {
+            voltage-VD0R {
+              apple,key-id = "VD0R";
+              label = "AC Input Voltage";
+            };
+          };
+        };
       };
     };

-- 
2.50.1

[PATCH 3/8] rtc: Add new rtc-macsmc driver for Apple Silicon Macs

[James Calligeros]

From: Hector Martin <marcan@marcan.st>

Apple Silicon Macs (M1, etc.) have an RTC that is part of the PMU IC,
but most of the PMU functionality is abstracted out by the SMC.
On T600x machines, the RTC counter must be accessed via the SMC to
get full functionality, and it seems likely that future machines
will move towards making SMC handle all RTC functionality.

The SMC RTC counter access is implemented on all current machines
as of the time of this writing, on firmware 12.x. However, the RTC
offset (needed to set the time) is still only accessible via direct
PMU access. To handle this, we expose the RTC offset as an NVMEM
cell from the SPMI PMU device node, and this driver consumes that
cell and uses it to compute/set the current time.

Reviewed-by: Neal Gompa <neal@gompa.dev>
Signed-off-by: Hector Martin <marcan@marcan.st>
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 MAINTAINERS              |   1 +
 drivers/mfd/macsmc.c     |   1 +
 drivers/rtc/Kconfig      |  11 ++
 drivers/rtc/Makefile     |   1 +
 drivers/rtc/rtc-macsmc.c | 141 +++++++++++++++++++++++++
 5 files changed, 155 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index aaef8634985b35f54de1123ebb4176602066d177..029117b921ea97d1276f5496ea06a3f918929b20 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2421,6 +2421,7 @@ F:	drivers/nvmem/apple-spmi-nvmem.c
 F:	drivers/pinctrl/pinctrl-apple-gpio.c
 F:	drivers/power/reset/macsmc-reboot.c
 F:	drivers/pwm/pwm-apple.c
+F:	drivers/rtc/rtc-macsmc.c
 F:	drivers/soc/apple/*
 F:	drivers/spi/spi-apple.c
 F:	drivers/spmi/spmi-apple-controller.c
diff --git a/drivers/mfd/macsmc.c b/drivers/mfd/macsmc.c
index 870c8b2028a8fc0e905c8934c2636824cbe5d527..59be894460d33afa754927630881532b548b7ad8 100644
--- a/drivers/mfd/macsmc.c
+++ b/drivers/mfd/macsmc.c
@@ -47,6 +47,7 @@
 static const struct mfd_cell apple_smc_devs[] = {
 	MFD_CELL_OF("macsmc-gpio", NULL, NULL, 0, 0, "apple,smc-gpio"),
 	MFD_CELL_OF("macsmc-reboot", NULL, NULL, 0, 0, "apple,smc-reboot"),
+	MFD_CELL_OF("macsmc-rtc", NULL, NULL, 0, 0, "apple,smc-rtc"),
 };
 
 static int apple_smc_cmd_locked(struct apple_smc *smc, u64 cmd, u64 arg,
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 64f6e9756aff4a1f6f6c50f9b4fc2140f66a8578..d28a46a89c85e6b30b402aec155e8972ed2aaa8e 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -2068,6 +2068,17 @@ config RTC_DRV_WILCO_EC
 	  This can also be built as a module. If so, the module will
 	  be named "rtc_wilco_ec".
 
+config RTC_DRV_MACSMC
+	tristate "Apple Mac System Management Controller RTC"
+	depends on MFD_MACSMC
+	help
+	  If you say yes here you get support for RTC functions
+	  inside Apple SPMI PMUs accessed through the SoC's
+	  System Management Controller
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rtc-macsmc.
+
 config RTC_DRV_MSC313
 	tristate "MStar MSC313 RTC"
         depends on ARCH_MSTARV7 || COMPILE_TEST
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 789bddfea99d8fcd024566891c37ee73e527cf93..bcb43b5878a562454986cbb9ab8cc45cec248dda 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_RTC_DRV_M48T35)	+= rtc-m48t35.o
 obj-$(CONFIG_RTC_DRV_M48T59)	+= rtc-m48t59.o
 obj-$(CONFIG_RTC_DRV_M48T86)	+= rtc-m48t86.o
 obj-$(CONFIG_RTC_DRV_MA35D1)	+= rtc-ma35d1.o
+obj-$(CONFIG_RTC_DRV_MACSMC)	+= rtc-macsmc.o
 obj-$(CONFIG_RTC_DRV_MAX31335)	+= rtc-max31335.o
 obj-$(CONFIG_RTC_DRV_MAX6900)	+= rtc-max6900.o
 obj-$(CONFIG_RTC_DRV_MAX6902)	+= rtc-max6902.o
diff --git a/drivers/rtc/rtc-macsmc.c b/drivers/rtc/rtc-macsmc.c
new file mode 100644
index 0000000000000000000000000000000000000000..05e360277f630f3368b2856aadef1f2b96426c37
--- /dev/null
+++ b/drivers/rtc/rtc-macsmc.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * Apple SMC RTC driver
+ * Copyright The Asahi Linux Contributors
+ */
+
+#include <linux/bitops.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/macsmc.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+
+/* 48-bit RTC */
+#define RTC_BYTES 6
+#define RTC_BITS (8 * RTC_BYTES)
+
+/* 32768 Hz clock */
+#define RTC_SEC_SHIFT 15
+
+struct macsmc_rtc {
+	struct device *dev;
+	struct apple_smc *smc;
+	struct rtc_device *rtc_dev;
+	struct nvmem_cell *rtc_offset;
+};
+
+static int macsmc_rtc_get_time(struct device *dev, struct rtc_time *tm)
+{
+	struct macsmc_rtc *rtc = dev_get_drvdata(dev);
+	u64 ctr = 0, off = 0;
+	time64_t now;
+	void *p_off;
+	size_t len;
+	int ret;
+
+	ret = apple_smc_read(rtc->smc, SMC_KEY(CLKM), &ctr, RTC_BYTES);
+	if (ret < 0)
+		return ret;
+	if (ret != RTC_BYTES)
+		return -EIO;
+
+	p_off = nvmem_cell_read(rtc->rtc_offset, &len);
+	if (IS_ERR(p_off))
+		return PTR_ERR(p_off);
+	if (len < RTC_BYTES) {
+		kfree(p_off);
+		return -EIO;
+	}
+
+	memcpy(&off, p_off, RTC_BYTES);
+	kfree(p_off);
+
+	/* Sign extend from 48 to 64 bits, then arithmetic shift right 15 bits to get seconds */
+	now = sign_extend64(ctr + off, RTC_BITS - 1) >> RTC_SEC_SHIFT;
+	rtc_time64_to_tm(now, tm);
+
+	return ret;
+}
+
+static int macsmc_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct macsmc_rtc *rtc = dev_get_drvdata(dev);
+	u64 ctr = 0, off = 0;
+	int ret;
+
+	ret = apple_smc_read(rtc->smc, SMC_KEY(CLKM), &ctr, RTC_BYTES);
+	if (ret < 0)
+		return ret;
+	if (ret != RTC_BYTES)
+		return -EIO;
+
+	/* This sets the offset such that the set second begins now */
+	off = (rtc_tm_to_time64(tm) << RTC_SEC_SHIFT) - ctr;
+	return nvmem_cell_write(rtc->rtc_offset, &off, RTC_BYTES);
+}
+
+static const struct rtc_class_ops macsmc_rtc_ops = {
+	.read_time = macsmc_rtc_get_time,
+	.set_time = macsmc_rtc_set_time,
+};
+
+static int macsmc_rtc_probe(struct platform_device *pdev)
+{
+	struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
+	struct macsmc_rtc *rtc;
+
+	/*
+	 * MFD will probe this device even without a node in the device tree,
+	 * thus bail out early if the SMC on the current machines does not
+	 * support RTC and has no node in the device tree.
+	 */
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
+	if (!rtc)
+		return -ENOMEM;
+
+	rtc->dev = &pdev->dev;
+	rtc->smc = smc;
+
+	rtc->rtc_offset = devm_nvmem_cell_get(&pdev->dev, "rtc_offset");
+	if (IS_ERR(rtc->rtc_offset))
+		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->rtc_offset),
+				     "Failed to get rtc_offset NVMEM cell\n");
+
+	rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
+	if (IS_ERR(rtc->rtc_dev))
+		return PTR_ERR(rtc->rtc_dev);
+
+	rtc->rtc_dev->ops = &macsmc_rtc_ops;
+	rtc->rtc_dev->range_min = S64_MIN >> (RTC_SEC_SHIFT + (64 - RTC_BITS));
+	rtc->rtc_dev->range_max = S64_MAX >> (RTC_SEC_SHIFT + (64 - RTC_BITS));
+
+	platform_set_drvdata(pdev, rtc);
+
+	return devm_rtc_register_device(rtc->rtc_dev);
+}
+
+static const struct of_device_id macsmc_rtc_of_table[] = {
+	{ .compatible = "apple,smc-rtc", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, macsmc_rtc_of_table);
+
+static struct platform_driver macsmc_rtc_driver = {
+	.driver = {
+		.name = "macsmc-rtc",
+		.of_match_table = macsmc_rtc_of_table,
+	},
+	.probe = macsmc_rtc_probe,
+};
+module_platform_driver(macsmc_rtc_driver);
+
+MODULE_LICENSE("Dual MIT/GPL");
+MODULE_DESCRIPTION("Apple SMC RTC driver");
+MODULE_AUTHOR("Hector Martin <marcan@marcan.st>");

-- 
2.50.1

[PATCH 4/8] hwmon: Add Apple Silicon SMC hwmon driver

[James Calligeros]

The System Management Controller on Apple Silicon devices is responsible
for integrating and exposing the data reported by the vast array of
hardware monitoring sensors present on these devices. It is also
responsible for fan control, and allows users to manually set fan
speeds if they so desire. Add a hwmon driver to expose current,
power, temperature, and voltage monitoring sensors, as well as
fan speed monitoring and control via the SMC on Apple Silicon devices.

The SMC firmware has no consistency between devices, even when they
share an SoC. The FourCC keys used to access sensors are almost
random. An M1 Mac mini will have different FourCCs for its CPU core
temperature sensors to an M1 MacBook Pro, for example. For this
reason, the valid sensors for a given device are specified in a
child of the SMC Devicetree node. The driver uses this information
to determine which sensors to make available at runtime.

Co-developed-by: Janne Grunau <j@jannau.net>
Signed-off-by: Janne Grunau <j@jannau.net>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 MAINTAINERS                  |   2 +
 drivers/hwmon/Kconfig        |  12 +
 drivers/hwmon/Makefile       |   1 +
 drivers/hwmon/macsmc_hwmon.c | 858 +++++++++++++++++++++++++
 drivers/mfd/macsmc.c         |   1 +
 5 files changed, 874 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 029117b921ea97d1276f5496ea06a3f918929b20..2eb23522654dd050262eb06e077587030cc335aa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2380,6 +2380,7 @@ F:	Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
 F:	Documentation/devicetree/bindings/dma/apple,admac.yaml
 F:	Documentation/devicetree/bindings/gpio/apple,smc-gpio.yaml
 F:	Documentation/devicetree/bindings/gpu/apple,agx.yaml
+F:	Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
 F:	Documentation/devicetree/bindings/i2c/apple,i2c.yaml
 F:	Documentation/devicetree/bindings/input/touchscreen/apple,z2-multitouch.yaml
 F:	Documentation/devicetree/bindings/interrupt-controller/apple,*
@@ -2407,6 +2408,7 @@ F:	drivers/clk/clk-apple-nco.c
 F:	drivers/cpufreq/apple-soc-cpufreq.c
 F:	drivers/dma/apple-admac.c
 F:	drivers/gpio/gpio-macsmc.c
+F:	drivers/hwmon/macsmc_hwmon.c
 F:	drivers/pmdomain/apple/
 F:	drivers/i2c/busses/i2c-pasemi-core.c
 F:	drivers/i2c/busses/i2c-pasemi-platform.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 9d28fcf7cd2a6f9e2f54694a717bd85ff4047b46..1ca6db71e4d9da32717fd14487c10944433ada41 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1164,6 +1164,18 @@ config SENSORS_LTQ_CPUTEMP
 	  If you say yes here you get support for the temperature
 	  sensor inside your CPU.
 
+config SENSORS_MACSMC_HWMON
+	tristate "Apple SMC (Apple Silicon)"
+	depends on MFD_MACSMC && OF
+	help
+	  This driver enables hwmon support for current, power, temperature,
+	  and voltage sensors, as well as fan speed reporting and control
+	  on Apple Silicon devices. Say Y here if you have an Apple Silicon
+	  device.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called macsmc_hwmon.
+
 config SENSORS_MAX1111
 	tristate "Maxim MAX1111 Serial 8-bit ADC chip and compatibles"
 	depends on SPI_MASTER
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index cd8bc4752b4dbf015c6eb46157626f4e8f87dfae..80fc8447aff15b3b8e8583dc755c8accb3b6a93e 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -147,6 +147,7 @@ obj-$(CONFIG_SENSORS_LTC4260)	+= ltc4260.o
 obj-$(CONFIG_SENSORS_LTC4261)	+= ltc4261.o
 obj-$(CONFIG_SENSORS_LTC4282)	+= ltc4282.o
 obj-$(CONFIG_SENSORS_LTQ_CPUTEMP) += ltq-cputemp.o
+obj-$(CONFIG_SENSORS_MACSMC_HWMON)	+= macsmc_hwmon.o
 obj-$(CONFIG_SENSORS_MAX1111)	+= max1111.o
 obj-$(CONFIG_SENSORS_MAX127)	+= max127.o
 obj-$(CONFIG_SENSORS_MAX16065)	+= max16065.o
diff --git a/drivers/hwmon/macsmc_hwmon.c b/drivers/hwmon/macsmc_hwmon.c
new file mode 100644
index 0000000000000000000000000000000000000000..543a1ab50fc3587cc88625ec703d92a7e7db0825
--- /dev/null
+++ b/drivers/hwmon/macsmc_hwmon.c
@@ -0,0 +1,858 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * Apple SMC hwmon driver for Apple Silicon platforms
+ *
+ * The System Management Controller on Apple Silicon devices is responsible for
+ * measuring data from sensors across the SoC and machine. These include power,
+ * temperature, voltage and current sensors. Some "sensors" actually expose
+ * derived values. An example of this is the key PHPC, which is an estimate
+ * of the heat energy being dissipated by the SoC.
+ *
+ * While each SoC only has one SMC variant, each platform exposes a different
+ * set of sensors. For example, M1 MacBooks expose battery telemetry sensors
+ * which are not present on the M1 Mac mini. For this reason, the available
+ * sensors for a given platform are described in the device tree in a child
+ * node of the SMC device. We must walk this list of available sensors and
+ * populate the required hwmon data structures at runtime.
+ *
+ * Originally based on a concept by Jean-Francois Bortolotti <jeff@borto.fr>
+ *
+ * Copyright The Asahi Linux Contributors
+ */
+
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/mfd/macsmc.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#define MAX_LABEL_LENGTH 32
+#define NUM_SENSOR_TYPES 5 /* temp, volt, current, power, fan */
+
+#define FLT_EXP_BIAS 127
+#define FLT_EXP_MASK GENMASK(30, 23)
+#define FLT_MANT_BIAS 23
+#define FLT_MANT_MASK GENMASK(22, 0)
+#define FLT_SIGN_MASK BIT(31)
+
+static bool melt_my_mac;
+module_param_unsafe(melt_my_mac, bool, 0644);
+MODULE_PARM_DESC(
+	melt_my_mac,
+	"Override the SMC to set your own fan speeds on supported machines");
+
+struct macsmc_hwmon_sensor {
+	struct apple_smc_key_info info;
+	smc_key macsmc_key;
+	char label[MAX_LABEL_LENGTH];
+};
+
+struct macsmc_hwmon_fan {
+	struct macsmc_hwmon_sensor now;
+	struct macsmc_hwmon_sensor min;
+	struct macsmc_hwmon_sensor max;
+	struct macsmc_hwmon_sensor set;
+	struct macsmc_hwmon_sensor mode;
+	char label[MAX_LABEL_LENGTH];
+	u32 attrs;
+	bool manual;
+};
+
+struct macsmc_hwmon_sensors {
+	struct hwmon_channel_info channel_info;
+	struct macsmc_hwmon_sensor *sensors;
+	u32 n_sensors;
+};
+
+struct macsmc_hwmon_fans {
+	struct hwmon_channel_info channel_info;
+	struct macsmc_hwmon_fan *fans;
+	u32 n_fans;
+};
+
+struct macsmc_hwmon {
+	struct device *dev;
+	struct apple_smc *smc;
+	struct device *hwmon_dev;
+	struct hwmon_chip_info chip_info;
+	/* Chip + sensor types + NULL */
+	const struct hwmon_channel_info *channel_infos[1 + NUM_SENSOR_TYPES + 1];
+	struct macsmc_hwmon_sensors temp;
+	struct macsmc_hwmon_sensors volt;
+	struct macsmc_hwmon_sensors curr;
+	struct macsmc_hwmon_sensors power;
+	struct macsmc_hwmon_fans fan;
+};
+
+static int macsmc_hwmon_read_label(struct device *dev,
+				   enum hwmon_sensor_types type, u32 attr,
+				   int channel, const char **str)
+{
+	struct macsmc_hwmon *hwmon = dev_get_drvdata(dev);
+
+	switch (type) {
+	case hwmon_temp:
+		if (channel >= hwmon->temp.n_sensors)
+			return -EINVAL;
+		*str = hwmon->temp.sensors[channel].label;
+		break;
+	case hwmon_in:
+		if (channel >= hwmon->volt.n_sensors)
+			return -EINVAL;
+		*str = hwmon->volt.sensors[channel].label;
+		break;
+	case hwmon_curr:
+		if (channel >= hwmon->curr.n_sensors)
+			return -EINVAL;
+		*str = hwmon->curr.sensors[channel].label;
+		break;
+	case hwmon_power:
+		if (channel >= hwmon->power.n_sensors)
+			return -EINVAL;
+		*str = hwmon->power.sensors[channel].label;
+		break;
+	case hwmon_fan:
+		if (channel >= hwmon->fan.n_fans)
+			return -EINVAL;
+		*str = hwmon->fan.fans[channel].label;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+/*
+ * A number of sensors report data in a 48.16 fixed-point decimal format that is
+ * not used by any other function of the SMC.
+ */
+static int macsmc_hwmon_read_ioft_scaled(struct apple_smc *smc, smc_key key,
+					 u64 *p, int scale)
+{
+	u64 val;
+	int ret;
+
+	ret = apple_smc_read_u64(smc, key, &val);
+	if (ret < 0)
+		return ret;
+
+	*p = mult_frac(val, scale, 65536);
+
+	return 0;
+}
+
+/*
+ * Many sensors report their data as IEEE-754 floats. No other SMC function uses
+ * them.
+ */
+static int macsmc_hwmon_read_f32_scaled(struct apple_smc *smc, smc_key key,
+					int *p, int scale)
+{
+	u32 fval;
+	u64 val;
+	int ret, exp;
+
+	ret = apple_smc_read_u32(smc, key, &fval);
+	if (ret < 0)
+		return ret;
+
+	val = ((u64)((fval & FLT_MANT_MASK) | BIT(23)));
+	exp = ((fval >> 23) & 0xff) - FLT_EXP_BIAS - FLT_MANT_BIAS;
+	if (scale < 0) {
+		val <<= 32;
+		exp -= 32;
+		val /= -scale;
+	} else {
+		val *= scale;
+	}
+
+	if (exp > 63)
+		val = U64_MAX;
+	else if (exp < -63)
+		val = 0;
+	else if (exp < 0)
+		val >>= -exp;
+	else if (exp != 0 && (val & ~((1UL << (64 - exp)) - 1))) /* overflow */
+		val = U64_MAX;
+	else
+		val <<= exp;
+
+	if (fval & FLT_SIGN_MASK) {
+		if (val > (-(s64)INT_MIN))
+			*p = INT_MIN;
+		else
+			*p = -val;
+	} else {
+		if (val > INT_MAX)
+			*p = INT_MAX;
+		else
+			*p = val;
+	}
+
+	return 0;
+}
+
+/*
+ * The SMC has keys of multiple types, denoted by a FourCC of the same format
+ * as the key ID. We don't know what data type a key encodes until we poke at it.
+ */
+static int macsmc_hwmon_read_key(struct apple_smc *smc,
+				 struct macsmc_hwmon_sensor *sensor, int scale,
+				 long *val)
+{
+	int ret = 0;
+
+	switch (sensor->info.type_code) {
+	/* 32-bit IEEE 754 float */
+	case _SMC_KEY("flt "): {
+		u32 flt_ = 0;
+
+		ret = macsmc_hwmon_read_f32_scaled(smc, sensor->macsmc_key,
+						   &flt_, scale);
+		*val = flt_;
+		break;
+	}
+	/* 48.16 fixed point decimal */
+	case _SMC_KEY("ioft"): {
+		u64 ioft = 0;
+
+		ret = macsmc_hwmon_read_ioft_scaled(smc, sensor->macsmc_key,
+						    &ioft, scale);
+		*val = (long)ioft;
+		break;
+	}
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	if (ret)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int macsmc_hwmon_write_f32_scaled(struct apple_smc *smc, smc_key key,
+					 int value, int scale)
+{
+	u64 val;
+	u32 fval = 0;
+	int exp = 0, neg;
+
+	val = abs(value);
+	neg = val != value;
+
+	if (scale > 1) {
+		val <<= 32;
+		exp = 32;
+		val /= scale;
+	} else if (scale < 1) {
+		val *= -scale;
+	}
+
+	if (val) {
+		int msb = __fls(val) - exp;
+
+		if (msb > 23) {
+			val >>= msb - 23;
+			exp -= msb - 23;
+		} else if (msb < 23) {
+			val <<= 23 - msb;
+			exp += msb;
+		}
+
+		fval = FIELD_PREP(FLT_SIGN_MASK, neg) |
+		       FIELD_PREP(FLT_EXP_MASK, exp + FLT_EXP_BIAS) |
+		       FIELD_PREP(FLT_MANT_MASK, val);
+	}
+
+	return apple_smc_write_u32(smc, key, fval);
+}
+
+static int macsmc_hwmon_write_key(struct apple_smc *smc,
+				  struct macsmc_hwmon_sensor *sensor, long val,
+				  int scale)
+{
+	switch (sensor->info.type_code) {
+	/* 32-bit IEEE 754 float */
+	case _SMC_KEY("flt "):
+		return macsmc_hwmon_write_f32_scaled(smc, sensor->macsmc_key,
+						     val, scale);
+	/* unsigned 8-bit integer */
+	case _SMC_KEY("ui8 "):
+		return apple_smc_write_u8(smc, sensor->macsmc_key, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int macsmc_hwmon_read_fan(struct macsmc_hwmon *hwmon, u32 attr, int chan,
+				 long *val)
+{
+	if (!(hwmon->fan.fans[chan].attrs & BIT(attr)))
+		return -EINVAL;
+
+	switch (attr) {
+	case hwmon_fan_input:
+		return macsmc_hwmon_read_key(
+			hwmon->smc, &hwmon->fan.fans[chan].now, 1, val);
+	case hwmon_fan_min:
+		return macsmc_hwmon_read_key(
+			hwmon->smc, &hwmon->fan.fans[chan].min, 1, val);
+	case hwmon_fan_max:
+		return macsmc_hwmon_read_key(
+			hwmon->smc, &hwmon->fan.fans[chan].max, 1, val);
+	case hwmon_fan_target:
+		return macsmc_hwmon_read_key(
+			hwmon->smc, &hwmon->fan.fans[chan].set, 1, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int macsmc_hwmon_write_fan(struct device *dev, u32 attr, int channel,
+				  long val)
+{
+	struct macsmc_hwmon *hwmon = dev_get_drvdata(dev);
+	int ret = 0;
+	long min = 0;
+	long max = 0;
+
+	if (!melt_my_mac || hwmon->fan.fans[channel].mode.macsmc_key == 0)
+		return -EOPNOTSUPP;
+
+	if ((channel >= hwmon->fan.n_fans) ||
+	    !(hwmon->fan.fans[channel].attrs & BIT(attr)) ||
+	    (attr != hwmon_fan_target))
+		return -EINVAL;
+
+	/*
+	 * The SMC does no sanity checks on requested fan speeds, so we need to.
+	 */
+	ret = macsmc_hwmon_read_key(hwmon->smc, &hwmon->fan.fans[channel].min,
+				    1, &min);
+	if (ret)
+		return ret;
+	ret = macsmc_hwmon_read_key(hwmon->smc, &hwmon->fan.fans[channel].max,
+				    1, &max);
+	if (ret)
+		return ret;
+
+	if (val >= min && val <= max) {
+		if (!hwmon->fan.fans[channel].manual) {
+			/* Write 1 to mode key for manual control */
+			ret = macsmc_hwmon_write_key(
+				hwmon->smc, &hwmon->fan.fans[channel].mode, 1,
+				1);
+			if (ret < 0)
+				return ret;
+
+			hwmon->fan.fans[channel].manual = true;
+			dev_info(
+				dev,
+				"Fan %d now under manual control! Set target speed to 0 for automatic control.\n",
+				channel + 1);
+		}
+		return macsmc_hwmon_write_key(
+			hwmon->smc, &hwmon->fan.fans[channel].set, val, 1);
+	} else if (!val) {
+		if (hwmon->fan.fans[channel].manual) {
+			dev_info(dev, "Returning control of fan %d to SMC.\n",
+				 channel + 1);
+			ret = macsmc_hwmon_write_key(
+				hwmon->smc, &hwmon->fan.fans[channel].mode, 0,
+				1);
+			if (ret < 0)
+				return ret;
+
+			hwmon->fan.fans[channel].manual = false;
+		}
+	} else {
+		dev_err(dev, "Requested fan speed %ld out of range [%ld, %ld]",
+			val, min, max);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int macsmc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+			     u32 attr, int channel, long *val)
+{
+	struct macsmc_hwmon *hwmon = dev_get_drvdata(dev);
+	int ret = 0;
+
+	switch (type) {
+	case hwmon_temp:
+		ret = macsmc_hwmon_read_key(
+			hwmon->smc, &hwmon->temp.sensors[channel], 1000, val);
+		break;
+	case hwmon_in:
+		ret = macsmc_hwmon_read_key(
+			hwmon->smc, &hwmon->volt.sensors[channel], 1000, val);
+		break;
+	case hwmon_curr:
+		ret = macsmc_hwmon_read_key(
+			hwmon->smc, &hwmon->curr.sensors[channel], 1000, val);
+		break;
+	case hwmon_power:
+		/* SMC returns power in Watts with acceptable precision to scale to uW */
+		ret = macsmc_hwmon_read_key(hwmon->smc,
+					    &hwmon->power.sensors[channel],
+					    1000000, val);
+		break;
+	case hwmon_fan:
+		ret = macsmc_hwmon_read_fan(hwmon, attr, channel, val);
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return ret;
+}
+
+static int macsmc_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
+			      u32 attr, int channel, long val)
+{
+	switch (type) {
+	case hwmon_fan:
+		return macsmc_hwmon_write_fan(dev, attr, channel, val);
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static umode_t macsmc_hwmon_fan_is_visible(const void *data, u32 attr,
+					   int channel)
+{
+	const struct macsmc_hwmon *hwmon = data;
+
+	if (channel >= hwmon->fan.n_fans)
+		return -EINVAL;
+
+	if (melt_my_mac && attr == hwmon_fan_target &&
+	    hwmon->fan.fans[channel].mode.macsmc_key != 0)
+		return 0644;
+
+	return 0444;
+}
+
+static umode_t macsmc_hwmon_is_visible(const void *data,
+				       enum hwmon_sensor_types type, u32 attr,
+				       int channel)
+{
+	switch (type) {
+	case hwmon_fan:
+		return macsmc_hwmon_fan_is_visible(data, attr, channel);
+	default:
+		break;
+	}
+
+	return 0444;
+}
+
+static const struct hwmon_ops macsmc_hwmon_ops = {
+	.is_visible = macsmc_hwmon_is_visible,
+	.read = macsmc_hwmon_read,
+	.read_string = macsmc_hwmon_read_label,
+	.write = macsmc_hwmon_write,
+};
+
+/*
+ * Get the key metadata, including key data type, from the SMC.
+ */
+static int macsmc_hwmon_parse_key(struct device *dev, struct apple_smc *smc,
+				  struct macsmc_hwmon_sensor *sensor,
+				  const char *key)
+{
+	int ret = 0;
+
+	ret = apple_smc_get_key_info(smc, _SMC_KEY(key), &sensor->info);
+	if (ret) {
+		dev_err(dev, "Failed to retrieve key info for %s\n", key);
+		return ret;
+	}
+	sensor->macsmc_key = _SMC_KEY(key);
+
+	return 0;
+}
+
+/*
+ * A sensor is a single key-value pair as made available by the SMC.
+ * The devicetree gives us the SMC key ID and a friendly name where the
+ * purpose of the sensor is known.
+ */
+static int macsmc_hwmon_create_sensor(struct device *dev, struct apple_smc *smc,
+				      struct device_node *sensor_node,
+				      struct macsmc_hwmon_sensor *sensor)
+{
+	const char *key, *label;
+	int ret = 0;
+
+	ret = of_property_read_string(sensor_node, "apple,key-id", &key);
+	if (ret) {
+		dev_err(dev, "Could not find apple,key-id in sensor node");
+		return ret;
+	}
+
+	ret = macsmc_hwmon_parse_key(dev, smc, sensor, key);
+	if (ret)
+		return ret;
+
+	if (!of_property_read_string(sensor_node, "label", &label))
+		strscpy_pad(sensor->label, label, sizeof(sensor->label));
+	else
+		strscpy_pad(sensor->label, key, sizeof(sensor->label));
+
+	return 0;
+}
+
+/*
+ * Fan data is exposed by the SMC as multiple sensors.
+ *
+ * The devicetree schema reuses apple,key-id for the actual fan speed sensor.
+ * Mix, max and target keys do not need labels, so we can reuse label
+ * for naming the entire fan.
+ */
+static int macsmc_hwmon_create_fan(struct device *dev, struct apple_smc *smc,
+				   struct device_node *fan_node,
+				   struct macsmc_hwmon_fan *fan)
+{
+	const char *label;
+	const char *now;
+	const char *min;
+	const char *max;
+	const char *set;
+	const char *mode;
+	int ret = 0;
+
+	ret = of_property_read_string(fan_node, "apple,key-id", &now);
+	if (ret) {
+		dev_err(dev, "apple,key-id not found in fan node!");
+		return -EINVAL;
+	}
+
+	ret = macsmc_hwmon_parse_key(dev, smc, &fan->now, now);
+	if (ret)
+		return ret;
+
+	if (!of_property_read_string(fan_node, "label", &label))
+		strscpy_pad(fan->label, label, sizeof(fan->label));
+	else
+		strscpy_pad(fan->label, now, sizeof(fan->label));
+
+	fan->attrs = HWMON_F_LABEL | HWMON_F_INPUT;
+
+	ret = of_property_read_string(fan_node, "apple,fan-minimum", &min);
+	if (ret)
+		dev_warn(dev, "No minimum fan speed key for %s", fan->label);
+	else {
+		if (!macsmc_hwmon_parse_key(dev, smc, &fan->min, min))
+			fan->attrs |= HWMON_F_MIN;
+	}
+
+	ret = of_property_read_string(fan_node, "apple,fan-maximum", &max);
+	if (ret)
+		dev_warn(dev, "No maximum fan speed key for %s", fan->label);
+	else {
+		if (!macsmc_hwmon_parse_key(dev, smc, &fan->max, max))
+			fan->attrs |= HWMON_F_MAX;
+	}
+
+	ret = of_property_read_string(fan_node, "apple,fan-target", &set);
+	if (ret)
+		dev_warn(dev, "No target fan speed key for %s", fan->label);
+	else {
+		if (!macsmc_hwmon_parse_key(dev, smc, &fan->set, set))
+			fan->attrs |= HWMON_F_TARGET;
+	}
+
+	ret = of_property_read_string(fan_node, "apple,fan-mode", &mode);
+	if (ret)
+		dev_warn(dev, "No fan mode key for %s", fan->label);
+	else {
+		ret = macsmc_hwmon_parse_key(dev, smc, &fan->mode, mode);
+		if (ret)
+			return ret;
+	}
+
+	/* Initialise fan control mode to automatic */
+	fan->manual = false;
+
+	return 0;
+}
+
+static int macsmc_hwmon_populate_sensors(struct macsmc_hwmon *hwmon,
+					 struct device_node *hwmon_node)
+{
+	struct device_node *group_node = NULL;
+
+	for_each_child_of_node(hwmon_node, group_node) {
+		struct device_node *key_node = NULL;
+		struct macsmc_hwmon_sensors *sensor_group = NULL;
+		struct macsmc_hwmon_fans *fan_group = NULL;
+		u32 n_keys = 0;
+		int i = 0;
+
+		n_keys = of_get_child_count(group_node);
+		if (!n_keys) {
+			dev_err(hwmon->dev, "No keys found in %s!\n",
+				group_node->name);
+			continue;
+		}
+
+		if (strcmp(group_node->name, "temperature") == 0)
+			sensor_group = &hwmon->temp;
+		else if (strcmp(group_node->name, "voltage") == 0)
+			sensor_group = &hwmon->volt;
+		else if (strcmp(group_node->name, "current") == 0)
+			sensor_group = &hwmon->curr;
+		else if (strcmp(group_node->name, "power") == 0)
+			sensor_group = &hwmon->power;
+		else if (strcmp(group_node->name, "fan") == 0)
+			fan_group = &hwmon->fan;
+		else {
+			dev_err(hwmon->dev, "Invalid group node: %s",
+				group_node->name);
+			continue;
+		}
+
+		if (sensor_group) {
+			sensor_group->sensors = devm_kzalloc(
+				hwmon->dev,
+				sizeof(struct macsmc_hwmon_sensor) * n_keys,
+				GFP_KERNEL);
+			if (!sensor_group->sensors) {
+				of_node_put(group_node);
+				return -ENOMEM;
+			}
+
+			for_each_child_of_node(group_node, key_node) {
+				if (!macsmc_hwmon_create_sensor(
+					    hwmon->dev, hwmon->smc, key_node,
+					    &sensor_group->sensors[i]))
+					i++;
+			}
+
+			sensor_group->n_sensors = i;
+			if (!sensor_group->n_sensors) {
+				dev_err(hwmon->dev,
+					"No valid sensor keys found in %s\n",
+					group_node->name);
+				continue;
+			}
+		} else if (fan_group) {
+			fan_group->fans = devm_kzalloc(
+				hwmon->dev,
+				sizeof(struct macsmc_hwmon_fan) * n_keys,
+				GFP_KERNEL);
+
+			if (!fan_group->fans) {
+				of_node_put(group_node);
+				return -ENOMEM;
+			}
+
+			for_each_child_of_node(group_node, key_node) {
+				if (!macsmc_hwmon_create_fan(
+					    hwmon->dev, hwmon->smc, key_node,
+					    &fan_group->fans[i]))
+					i++;
+			}
+
+			fan_group->n_fans = i;
+			if (!fan_group->n_fans) {
+				dev_err(hwmon->dev,
+					"No valid sensor fans found in %s\n",
+					group_node->name);
+				continue;
+			}
+		}
+	}
+
+	return 0;
+}
+
+/* Create NULL-terminated config arrays */
+static void macsmc_hwmon_populate_configs(u32 *configs, u32 num_keys, u32 flags)
+{
+	int idx = 0;
+
+	for (idx = 0; idx < num_keys; idx++)
+		configs[idx] = flags;
+
+	configs[idx + 1] = 0;
+}
+
+static void macsmc_hwmon_populate_fan_configs(u32 *configs, u32 num_keys,
+					      struct macsmc_hwmon_fans *fans)
+{
+	int idx = 0;
+
+	for (idx = 0; idx < num_keys; idx++)
+		configs[idx] = fans->fans[idx].attrs;
+
+	configs[idx + 1] = 0;
+}
+
+static const struct hwmon_channel_info *const macsmc_chip_channel_info =
+	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ);
+
+static int macsmc_hwmon_create_infos(struct macsmc_hwmon *hwmon)
+{
+	int i = 0;
+	struct hwmon_channel_info *channel_info;
+
+	/* chip */
+	hwmon->channel_infos[i++] = macsmc_chip_channel_info;
+
+	if (hwmon->temp.n_sensors) {
+		channel_info = &hwmon->temp.channel_info;
+		channel_info->type = hwmon_temp;
+		channel_info->config = devm_kzalloc(
+			hwmon->dev, sizeof(u32) * hwmon->temp.n_sensors + 1,
+			GFP_KERNEL);
+		if (!channel_info->config)
+			return -ENOMEM;
+
+		macsmc_hwmon_populate_configs((u32 *)channel_info->config,
+					      hwmon->temp.n_sensors,
+					      (HWMON_T_INPUT | HWMON_T_LABEL));
+		hwmon->channel_infos[i++] = channel_info;
+	}
+
+	if (hwmon->volt.n_sensors) {
+		channel_info = &hwmon->volt.channel_info;
+		channel_info->type = hwmon_in;
+		channel_info->config = devm_kzalloc(
+			hwmon->dev, sizeof(u32) * hwmon->volt.n_sensors + 1,
+			GFP_KERNEL);
+		if (!channel_info->config)
+			return -ENOMEM;
+
+		macsmc_hwmon_populate_configs((u32 *)channel_info->config,
+					      hwmon->volt.n_sensors,
+					      (HWMON_I_INPUT | HWMON_I_LABEL));
+		hwmon->channel_infos[i++] = channel_info;
+	}
+
+	if (hwmon->curr.n_sensors) {
+		channel_info = &hwmon->curr.channel_info;
+		channel_info->type = hwmon_curr;
+		channel_info->config = devm_kzalloc(
+			hwmon->dev, sizeof(u32) * hwmon->curr.n_sensors + 1,
+			GFP_KERNEL);
+		if (!channel_info->config)
+			return -ENOMEM;
+
+		macsmc_hwmon_populate_configs((u32 *)channel_info->config,
+					      hwmon->curr.n_sensors,
+					      (HWMON_C_INPUT | HWMON_C_LABEL));
+		hwmon->channel_infos[i++] = channel_info;
+	}
+
+	if (hwmon->power.n_sensors) {
+		channel_info = &hwmon->power.channel_info;
+		channel_info->type = hwmon_power;
+		channel_info->config = devm_kzalloc(
+			hwmon->dev, sizeof(u32) * hwmon->power.n_sensors + 1,
+			GFP_KERNEL);
+		if (!channel_info->config)
+			return -ENOMEM;
+
+		macsmc_hwmon_populate_configs((u32 *)channel_info->config,
+					      hwmon->power.n_sensors,
+					      (HWMON_P_INPUT | HWMON_P_LABEL));
+		hwmon->channel_infos[i++] = channel_info;
+	}
+
+	if (hwmon->fan.n_fans) {
+		channel_info = &hwmon->fan.channel_info;
+		channel_info->type = hwmon_fan;
+		channel_info->config = devm_kzalloc(
+			hwmon->dev, sizeof(u32) * hwmon->fan.n_fans + 1,
+			GFP_KERNEL);
+		if (!channel_info->config)
+			return -ENOMEM;
+
+		macsmc_hwmon_populate_fan_configs((u32 *)channel_info->config,
+						  hwmon->fan.n_fans,
+						  &hwmon->fan);
+		hwmon->channel_infos[i++] = channel_info;
+	}
+
+	return 0;
+}
+
+static int macsmc_hwmon_probe(struct platform_device *pdev)
+{
+	struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
+	struct macsmc_hwmon *hwmon;
+	struct device_node *hwmon_node;
+	int ret = 0;
+
+	hwmon = devm_kzalloc(&pdev->dev, sizeof(struct macsmc_hwmon),
+			     GFP_KERNEL);
+	if (!hwmon)
+		return -ENOMEM;
+
+	hwmon->dev = &pdev->dev;
+	hwmon->smc = smc;
+
+	hwmon_node = of_get_child_by_name(pdev->dev.parent->of_node, "hwmon");
+	if (!hwmon_node) {
+		dev_err(hwmon->dev, "SMC hwmon node not found in Devicetree\n");
+		return -ENODEV;
+	}
+
+	ret = macsmc_hwmon_populate_sensors(hwmon, hwmon_node);
+	if (ret)
+		dev_info(hwmon->dev, "Could not populate keys!\n");
+
+	of_node_put(hwmon_node);
+
+	if (!hwmon->temp.n_sensors && !hwmon->volt.n_sensors &&
+	    !hwmon->curr.n_sensors && !hwmon->power.n_sensors &&
+	    !hwmon->fan.n_fans) {
+		dev_err(hwmon->dev,
+			"No valid keys found of any supported type");
+		return -ENODEV;
+	}
+
+	ret = macsmc_hwmon_create_infos(hwmon);
+	if (ret)
+		return ret;
+
+	hwmon->chip_info.ops = &macsmc_hwmon_ops;
+	hwmon->chip_info.info =
+		(const struct hwmon_channel_info *const *)&hwmon->channel_infos;
+
+	hwmon->hwmon_dev = devm_hwmon_device_register_with_info(
+		&pdev->dev, "macsmc_hwmon", hwmon, &hwmon->chip_info, NULL);
+	if (IS_ERR(hwmon->hwmon_dev))
+		return dev_err_probe(hwmon->dev, PTR_ERR(hwmon->hwmon_dev),
+				     "Probing SMC hwmon device failed\n");
+
+	dev_info(hwmon->dev, "Registered SMC hwmon device. Sensors:");
+	dev_info(
+		hwmon->dev,
+		"Temperature: %d, Voltage: %d, Current: %d, Power: %d, Fans: %d",
+		hwmon->temp.n_sensors, hwmon->volt.n_sensors,
+		hwmon->curr.n_sensors, hwmon->power.n_sensors,
+		hwmon->fan.n_fans);
+
+	return 0;
+}
+
+static struct platform_driver macsmc_hwmon_driver = {
+	.probe = macsmc_hwmon_probe,
+	.driver = {
+		.name = "macsmc_hwmon",
+	},
+};
+module_platform_driver(macsmc_hwmon_driver);
+
+MODULE_DESCRIPTION("Apple Silicon SMC hwmon driver");
+MODULE_AUTHOR("James Calligeros <jcalligeros99@gmail.com>");
+MODULE_LICENSE("Dual MIT/GPL");
+MODULE_ALIAS("platform:macsmc_hwmon");
diff --git a/drivers/mfd/macsmc.c b/drivers/mfd/macsmc.c
index 59be894460d33afa754927630881532b548b7ad8..bc4adf2fcfdce6c5ecbc51ced0e5985cbd36f54d 100644
--- a/drivers/mfd/macsmc.c
+++ b/drivers/mfd/macsmc.c
@@ -48,6 +48,7 @@ static const struct mfd_cell apple_smc_devs[] = {
 	MFD_CELL_OF("macsmc-gpio", NULL, NULL, 0, 0, "apple,smc-gpio"),
 	MFD_CELL_OF("macsmc-reboot", NULL, NULL, 0, 0, "apple,smc-reboot"),
 	MFD_CELL_OF("macsmc-rtc", NULL, NULL, 0, 0, "apple,smc-rtc"),
+	MFD_CELL_OF("macsmc_hwmon", NULL, NULL, 0, 0, "apple,smc-hwmon"),
 };
 
 static int apple_smc_cmd_locked(struct apple_smc *smc, u64 cmd, u64 arg,

-- 
2.50.1

[PATCH 5/8] input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid

[James Calligeros]

From: Hector Martin <marcan@marcan.st>

This driver implements power button and lid switch support for Apple Mac
devices using SMC controllers driven by the macsmc driver.

In addition to basic input support, this also responds to the final
shutdown warning (when the power button is held down long enough) by
doing an emergency kernel poweroff. This allows the NVMe controller to
be cleanly shut down, which prevents data loss for in-cache data.

Signed-off-by: Hector Martin <marcan@marcan.st>
Co-developed-by: Sven Peter <sven@kernel.org>
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 MAINTAINERS                     |   1 +
 drivers/input/misc/Kconfig      |  11 ++
 drivers/input/misc/Makefile     |   1 +
 drivers/input/misc/macsmc-hid.c | 210 +++++++++++++++++++++++++
 drivers/mfd/macsmc.c            |   1 +
 5 files changed, 224 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2eb23522654dd050262eb06e077587030cc335aa..b3b5220fcd0d4bbef67613c8ee9afa880c0aa45d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2412,6 +2412,7 @@ F:	drivers/hwmon/macsmc_hwmon.c
 F:	drivers/pmdomain/apple/
 F:	drivers/i2c/busses/i2c-pasemi-core.c
 F:	drivers/i2c/busses/i2c-pasemi-platform.c
+F:	drivers/input/misc/macsmc-hid.c
 F:	drivers/input/touchscreen/apple_z2.c
 F:	drivers/iommu/apple-dart.c
 F:	drivers/iommu/io-pgtable-dart.c
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 0fb21c99a5e3d1230d7f40f99e0c2d360bbe80e8..a430c50e7f63f245bba56bd526026ec7901cf821 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -961,4 +961,15 @@ config INPUT_STPMIC1_ONKEY
 	  To compile this driver as a module, choose M here: the
 	  module will be called stpmic1_onkey.
 
+config INPUT_MACSMC_HID
+	tristate "Apple Mac SMC lid/buttons"
+	depends on MFD_MACSMC
+	help
+	  Say Y here if you want to use the input events delivered via the
+	  SMC controller on Apple Mac machines using the macsmc driver.
+	  This includes lid open/close and the power button.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called macsmc-hid.
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index d468c8140b93da5bb537e8a3baea2b90e7bb4229..95b8ebbb9ebbe6f3afc9db724d2ebeba1d75d1a6 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -51,6 +51,7 @@ obj-$(CONFIG_INPUT_IQS7222)		+= iqs7222.o
 obj-$(CONFIG_INPUT_KEYSPAN_REMOTE)	+= keyspan_remote.o
 obj-$(CONFIG_INPUT_KXTJ9)		+= kxtj9.o
 obj-$(CONFIG_INPUT_M68K_BEEP)		+= m68kspkr.o
+obj-$(CONFIG_INPUT_MACSMC_HID)		+= macsmc-hid.o
 obj-$(CONFIG_INPUT_MAX77650_ONKEY)	+= max77650-onkey.o
 obj-$(CONFIG_INPUT_MAX77693_HAPTIC)	+= max77693-haptic.o
 obj-$(CONFIG_INPUT_MAX8925_ONKEY)	+= max8925_onkey.o
diff --git a/drivers/input/misc/macsmc-hid.c b/drivers/input/misc/macsmc-hid.c
new file mode 100644
index 0000000000000000000000000000000000000000..cdeef6c4797f82f464e3a6a760dbaaf186e1c58d
--- /dev/null
+++ b/drivers/input/misc/macsmc-hid.c
@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * Apple SMC input event driver
+ * Copyright The Asahi Linux Contributors
+ *
+ * This driver exposes HID events from the SMC as an input device.
+ * This includes the lid open/close and power button notifications.
+ */
+
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/macsmc.h>
+#include <linux/module.h>
+#include <linux/reboot.h>
+
+
+/**
+ * struct macsmc_hid
+ * @dev: Underlying struct device for the HID sub-device
+ * @smc: Pointer to apple_smc struct of the mfd parent
+ * @input: Allocated input_dev; devres managed
+ * @nb: Notifier block used for incoming events from SMC (e.g. button pressed down)
+ * @wakeup_mode: Set to true when system is suspended and power button events should wake it
+ */
+struct macsmc_hid {
+	struct device *dev;
+	struct apple_smc *smc;
+	struct input_dev *input;
+	struct notifier_block nb;
+	bool wakeup_mode;
+};
+
+#define SMC_EV_BTN 0x7201
+#define SMC_EV_LID 0x7203
+
+#define BTN_POWER		0x01 /* power button on e.g. Mac Mini chasis pressed */
+#define BTN_TOUCHID		0x06 /* combined TouchID / power button on MacBooks pressed */
+#define BTN_POWER_HELD_SHORT	0xfe /* power button briefly held down */
+#define BTN_POWER_HELD_LONG	0x00 /* power button held down; sent just before forced poweroff */
+
+static void macsmc_hid_event_button(struct macsmc_hid *smchid, unsigned long event)
+{
+	u8 button = (event >> 8) & 0xff;
+	u8 state = !!(event & 0xff);
+
+	switch (button) {
+	case BTN_POWER:
+	case BTN_TOUCHID:
+		if (smchid->wakeup_mode) {
+			if (state)
+				pm_wakeup_hard_event(smchid->dev);
+		} else {
+			input_report_key(smchid->input, KEY_POWER, state);
+			input_sync(smchid->input);
+		}
+		break;
+	case BTN_POWER_HELD_SHORT: /* power button held down; ignore */
+		break;
+	case BTN_POWER_HELD_LONG:
+		/*
+		 * If we get here the power button has been held down for a while and
+		 * we have about 4 seconds before forced power-off is triggered by SMC.
+		 * Try to do an emergency shutdown to make sure the NVMe cache is
+		 * flushed. macOS actually does this by panicing (!)...
+		 */
+		if (state) {
+			dev_crit(smchid->dev, "Triggering forced shutdown!\n");
+			if (kernel_can_power_off())
+				kernel_power_off();
+			else /* Missing macsmc-reboot driver? */
+				kernel_restart("SMC power button triggered restart");
+		}
+		break;
+	default:
+		dev_warn(smchid->dev, "Unknown SMC button event: %04lx\n", event & 0xffff);
+	}
+}
+
+static void macsmc_hid_event_lid(struct macsmc_hid *smchid, unsigned long event)
+{
+	u8 lid_state = !!((event >> 8) & 0xff);
+
+	if (smchid->wakeup_mode && !lid_state)
+		pm_wakeup_hard_event(smchid->dev);
+
+	input_report_switch(smchid->input, SW_LID, lid_state);
+	input_sync(smchid->input);
+}
+
+static int macsmc_hid_event(struct notifier_block *nb, unsigned long event, void *data)
+{
+	struct macsmc_hid *smchid = container_of(nb, struct macsmc_hid, nb);
+	u16 type = event >> 16;
+
+	switch (type) {
+	case SMC_EV_BTN:
+		macsmc_hid_event_button(smchid, event);
+		return NOTIFY_OK;
+	case SMC_EV_LID:
+		macsmc_hid_event_lid(smchid, event);
+		return NOTIFY_OK;
+	default:
+		/* SMC event meant for another driver */
+		return NOTIFY_DONE;
+	}
+}
+
+static int macsmc_hid_probe(struct platform_device *pdev)
+{
+	struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
+	struct macsmc_hid *smchid;
+	bool have_lid, have_power;
+	int ret;
+
+	/* Bail early if this SMC neither supports power button nor lid events */
+	have_lid = apple_smc_key_exists(smc, SMC_KEY(MSLD));
+	have_power = apple_smc_key_exists(smc, SMC_KEY(bHLD));
+	if (!have_lid && !have_power)
+		return -ENODEV;
+
+	smchid = devm_kzalloc(&pdev->dev, sizeof(*smchid), GFP_KERNEL);
+	if (!smchid)
+		return -ENOMEM;
+
+	smchid->dev = &pdev->dev;
+	smchid->smc = smc;
+	platform_set_drvdata(pdev, smchid);
+
+	smchid->input = devm_input_allocate_device(&pdev->dev);
+	if (!smchid->input)
+		return -ENOMEM;
+
+	smchid->input->phys = "macsmc-hid (0)";
+	smchid->input->name = "Apple SMC power/lid events";
+
+	if (have_lid)
+		input_set_capability(smchid->input, EV_SW, SW_LID);
+	if (have_power)
+		input_set_capability(smchid->input, EV_KEY, KEY_POWER);
+
+	ret = input_register_device(smchid->input);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register input device: %d\n", ret);
+		return ret;
+	}
+
+	if (have_lid) {
+		u8 val;
+
+		ret = apple_smc_read_u8(smc, SMC_KEY(MSLD), &val);
+		if (ret < 0)
+			dev_warn(&pdev->dev, "Failed to read initial lid state\n");
+		else
+			input_report_switch(smchid->input, SW_LID, val);
+	}
+
+	if (have_power) {
+		u32 val;
+
+		ret = apple_smc_read_u32(smc, SMC_KEY(bHLD), &val);
+		if (ret < 0)
+			dev_warn(&pdev->dev, "Failed to read initial power button state\n");
+		else
+			input_report_key(smchid->input, KEY_POWER, val & 1);
+	}
+
+	input_sync(smchid->input);
+
+	smchid->nb.notifier_call = macsmc_hid_event;
+	blocking_notifier_chain_register(&smc->event_handlers, &smchid->nb);
+
+	device_init_wakeup(&pdev->dev, 1);
+
+	return 0;
+}
+
+static int macsmc_hid_pm_prepare(struct device *dev)
+{
+	struct macsmc_hid *smchid = dev_get_drvdata(dev);
+
+	smchid->wakeup_mode = true;
+	return 0;
+}
+
+static void macsmc_hid_pm_complete(struct device *dev)
+{
+	struct macsmc_hid *smchid = dev_get_drvdata(dev);
+
+	smchid->wakeup_mode = false;
+}
+
+static const struct dev_pm_ops macsmc_hid_pm_ops = {
+	.prepare = macsmc_hid_pm_prepare,
+	.complete = macsmc_hid_pm_complete,
+};
+
+static struct platform_driver macsmc_hid_driver = {
+	.driver = {
+		.name = "macsmc-hid",
+		.pm = &macsmc_hid_pm_ops,
+	},
+	.probe = macsmc_hid_probe,
+};
+module_platform_driver(macsmc_hid_driver);
+
+MODULE_AUTHOR("Hector Martin <marcan@marcan.st>");
+MODULE_LICENSE("Dual MIT/GPL");
+MODULE_DESCRIPTION("Apple SMC HID driver");
+MODULE_ALIAS("platform:macsmc-hid");
diff --git a/drivers/mfd/macsmc.c b/drivers/mfd/macsmc.c
index bc4adf2fcfdce6c5ecbc51ced0e5985cbd36f54d..6a5ac449e23ddda0ebe4980614c49fe6c3bae50d 100644
--- a/drivers/mfd/macsmc.c
+++ b/drivers/mfd/macsmc.c
@@ -45,6 +45,7 @@
 #define SMC_TIMEOUT_MS		500
 
 static const struct mfd_cell apple_smc_devs[] = {
+	MFD_CELL_NAME("macsmc-hid"),
 	MFD_CELL_OF("macsmc-gpio", NULL, NULL, 0, 0, "apple,smc-gpio"),
 	MFD_CELL_OF("macsmc-reboot", NULL, NULL, 0, 0, "apple,smc-reboot"),
 	MFD_CELL_OF("macsmc-rtc", NULL, NULL, 0, 0, "apple,smc-rtc"),

-- 
2.50.1

[PATCH 6/8] arm64: dts: apple: t8103,t600x,t8112: Add SMC RTC node

[James Calligeros]

From: Sven Peter <sven@kernel.org>

The System Manager Controller of all M1/M2 SoCs supports the RTC
sub-device.

Reviewed-by: Neal Gompa <neal@gompa.dev>
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 .../arm64/boot/dts/apple/t600x-die0.dtsi | 6 ++++++
 arch/arm64/boot/dts/apple/t8103.dtsi     | 6 ++++++
 arch/arm64/boot/dts/apple/t8112.dtsi     | 6 ++++++
 3 files changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/apple/t600x-die0.dtsi b/arch/arm64/boot/dts/apple/t600x-die0.dtsi
index 3603b276a2abcfa6a730f58d5c6b9914f22f00b0..f715b19efd1679e5cd384a967d6e2a7c261ee679 100644
--- a/arch/arm64/boot/dts/apple/t600x-die0.dtsi
+++ b/arch/arm64/boot/dts/apple/t600x-die0.dtsi
@@ -44,6 +44,12 @@ smc_reboot: reboot {
 			nvmem-cell-names = "shutdown_flag", "boot_stage",
 				"boot_error_count", "panic_count";
 		};
+
+		rtc {
+			compatible = "apple,smc-rtc";
+			nvmem-cells = <&rtc_offset>;
+			nvmem-cell-names = "rtc_offset";
+		};
 	};
 
 	smc_mbox: mbox@290408000 {
diff --git a/arch/arm64/boot/dts/apple/t8103.dtsi b/arch/arm64/boot/dts/apple/t8103.dtsi
index 8b7b27887968741b745651e5133dffa7d8d20f6d..59f2678639cf47f469dc699c0ecb5b9e50a45ab1 100644
--- a/arch/arm64/boot/dts/apple/t8103.dtsi
+++ b/arch/arm64/boot/dts/apple/t8103.dtsi
@@ -916,6 +916,12 @@ smc_reboot: reboot {
 				nvmem-cell-names = "shutdown_flag", "boot_stage",
 					"boot_error_count", "panic_count";
 			};
+
+			rtc {
+				compatible = "apple,smc-rtc";
+				nvmem-cells = <&rtc_offset>;
+				nvmem-cell-names = "rtc_offset";
+			};
 		};
 
 		smc_mbox: mbox@23e408000 {
diff --git a/arch/arm64/boot/dts/apple/t8112.dtsi b/arch/arm64/boot/dts/apple/t8112.dtsi
index 3f79878b25af1f7760088aa552589494d67347fb..6bc3f58b06f703ed79578e89a030929a18d57796 100644
--- a/arch/arm64/boot/dts/apple/t8112.dtsi
+++ b/arch/arm64/boot/dts/apple/t8112.dtsi
@@ -919,6 +919,12 @@ smc_reboot: reboot {
 				nvmem-cell-names = "shutdown_flag", "boot_stage",
 					"boot_error_count", "panic_count";
 			};
+
+			rtc {
+				compatible = "apple,smc-rtc";
+				nvmem-cells = <&rtc_offset>;
+				nvmem-cell-names = "rtc_offset";
+			};
 		};
 
 		smc_mbox: mbox@23e408000 {

-- 
2.50.1

[PATCH 7/8] arm64: dts: apple: add common hwmon sensors and fans

[James Calligeros]

Each Apple Silicon device exposes a unique set of sensors and fans,
however some have been found to be reliably common across devices.

Add these as .dtsi files so that they can be combined with any
device-specific sensors without excessive repetition.

Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 .../boot/dts/apple/hwmon-common.dtsi     | 46 +++++++++++++++++++++++++
 .../boot/dts/apple/hwmon-fan-dual.dtsi   | 27 +++++++++++++++
 arch/arm64/boot/dts/apple/hwmon-fan.dtsi | 21 +++++++++++
 .../boot/dts/apple/hwmon-laptop.dtsi     | 43 +++++++++++++++++++++++
 .../boot/dts/apple/hwmon-mac-mini.dtsi   | 19 ++++++++++
 5 files changed, 156 insertions(+)

diff --git a/arch/arm64/boot/dts/apple/hwmon-common.dtsi b/arch/arm64/boot/dts/apple/hwmon-common.dtsi
new file mode 100644
index 0000000000000000000000000000000000000000..b8da8951194d1bce6e55f3558a2f4af8b3bbb45c
--- /dev/null
+++ b/arch/arm64/boot/dts/apple/hwmon-common.dtsi
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Hardawre monitoring sensors expected to be found on all Apple Silicon devices
+ *
+ * Copyright The Asahi Linux Contributors
+ */
+
+&smc {
+	hwmon {
+		current {
+			current-ID0R {
+				apple,key-id = "ID0R";
+				label = "AC Input Current";
+			};
+		};
+
+		power {
+			power-PSTR {
+				apple,key-id = "PSTR";
+				label = "Total System Power";
+			};
+			power-PDTR {
+				apple,key-id = "PDTR";
+				label = "AC Input Power";
+			};
+			power-PMVR {
+				apple,key-id = "PMVR";
+				label = "3.8 V Rail Power";
+			};
+		};
+
+		temperature {
+			temperature-TH0x {
+				apple,key-id = "TH0x";
+				label = "NAND Flash Temperature";
+			};
+		};
+		voltage {
+			voltage-VD0R {
+				apple,key-id = "VD0R";
+				label = "AC Input Voltage";
+			};
+		};
+
+	};
+};
diff --git a/arch/arm64/boot/dts/apple/hwmon-fan-dual.dtsi b/arch/arm64/boot/dts/apple/hwmon-fan-dual.dtsi
new file mode 100644
index 0000000000000000000000000000000000000000..2897f3b7a6a32ad3d31566816a1b237f27dc281d
--- /dev/null
+++ b/arch/arm64/boot/dts/apple/hwmon-fan-dual.dtsi
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * SMC hwmon fan keys for Apple Silicon desktops/laptops with two fans
+ *
+ * Copyright The Asahi Linux Contributors
+ */
+
+#include "hwmon-fan.dtsi"
+
+&smc {
+	hwmon {
+		fan{
+			fan-F0Ac {
+				label = "Fan 1";
+			};
+
+			fan-F1Ac {
+				apple,key-id = "F1Ac";
+				label = "Fan 2";
+				apple,fan-minimum = "F1Mn";
+				apple,fan-maximum = "F1Mx";
+				apple,fan-target = "F1Tg";
+				apple,fan-mode = "F1Md";
+			};
+		};
+	};
+};
diff --git a/arch/arm64/boot/dts/apple/hwmon-fan.dtsi b/arch/arm64/boot/dts/apple/hwmon-fan.dtsi
new file mode 100644
index 0000000000000000000000000000000000000000..6d307abd64adef9dda1a64351070968167cc9f3a
--- /dev/null
+++ b/arch/arm64/boot/dts/apple/hwmon-fan.dtsi
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * hwmon fan keys for Apple Silicon desktops/laptops with a single fan.
+ *
+ *  Copyright The Asahi Linux Contributors
+ */
+
+&smc {
+	hwmon {
+		fan {
+			fan-F0Ac {
+				apple,key-id = "F0Ac";
+				label = "Fan";
+				apple,fan-minimum = "F0Mn";
+				apple,fan-maximum = "F0Mx";
+				apple,fan-target = "F0Tg";
+				apple,fan-mode = "F0Md";
+			};
+		};
+	};
+};
diff --git a/arch/arm64/boot/dts/apple/hwmon-laptop.dtsi b/arch/arm64/boot/dts/apple/hwmon-laptop.dtsi
new file mode 100644
index 0000000000000000000000000000000000000000..f2d473c26526cff89d497b4702323996764973de
--- /dev/null
+++ b/arch/arm64/boot/dts/apple/hwmon-laptop.dtsi
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Hardware monitoring sensors expected on all Apple Silicon laptops
+ *
+ * Copyright The Asahi Linux Contributors
+ */
+
+&smc {
+	hwmon {
+		power {
+			power-PHPC {
+				apple,key-id = "PHPC";
+				label = "Heatpipe Power";
+			};
+		};
+
+		temperature {
+			temperature-TB0T {
+				apple,key-id = "TB0T";
+				label = "Battery Hotspot Temperature";
+			};
+			temperature-TCHP {
+				apple,key-id = "TCHP";
+				label = "Charge Regulator Temperature";
+			};
+			temperature-TW0P {
+				apple,key-id = "TW0P";
+				label = "WiFi/BT Module Temperature";
+			};
+		};
+
+		voltage {
+			voltage-SBAV {
+				apple,key-id = "SBAV";
+				label = "Battery Voltage";
+			};
+			voltage-VD0R {
+				apple,key-id = "VD0R";
+				label = "Charger Input Voltage";
+			};
+		};
+	};
+};
diff --git a/arch/arm64/boot/dts/apple/hwmon-mac-mini.dtsi b/arch/arm64/boot/dts/apple/hwmon-mac-mini.dtsi
new file mode 100644
index 0000000000000000000000000000000000000000..f65a3011372bd2bc37f84fad3abcd6ca59e88549
--- /dev/null
+++ b/arch/arm64/boot/dts/apple/hwmon-mac-mini.dtsi
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * hwmon sensors expected on all Mac mini models
+ *
+ * Copyright The Asahi Linux Contributors
+ */
+
+#include "hwmon-fan.dtsi"
+
+&smc {
+	hwmon {
+		temperature {
+			temperature-TW0P {
+				apple,key-id = "TW0P";
+				label = "WiFi/BT Module Temperature";
+			};
+		};
+	};
+};

-- 
2.50.1

[PATCH 8/8] arm64: dts: apple: t8103, t600x, t8112: add common hwmon nodes to devices

[James Calligeros]

Add the known, common hwmon-related SMC keys to the DTs for the devices
they pertain to.

Co-developed-by: Janne Grunau <j@jannau.net>
Signed-off-by: Janne Grunau <j@jannau.net>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 .../arm64/boot/dts/apple/t6001-j375c.dts | 2 ++
 .../arm64/boot/dts/apple/t6002-j375d.dts | 2 ++
 .../boot/dts/apple/t600x-j314-j316.dtsi  | 4 ++++
 .../arm64/boot/dts/apple/t600x-j375.dtsi | 2 ++
 arch/arm64/boot/dts/apple/t8103-j274.dts | 2 ++
 arch/arm64/boot/dts/apple/t8103-j293.dts | 3 +++
 arch/arm64/boot/dts/apple/t8103-j313.dts | 2 ++
 arch/arm64/boot/dts/apple/t8103-j456.dts | 2 ++
 arch/arm64/boot/dts/apple/t8103-j457.dts | 2 ++
 .../arm64/boot/dts/apple/t8103-jxxx.dtsi | 2 ++
 arch/arm64/boot/dts/apple/t8112-j413.dts | 2 ++
 arch/arm64/boot/dts/apple/t8112-j473.dts | 2 ++
 arch/arm64/boot/dts/apple/t8112-j493.dts | 3 +++
 .../arm64/boot/dts/apple/t8112-jxxx.dtsi | 2 ++
 14 files changed, 32 insertions(+)

diff --git a/arch/arm64/boot/dts/apple/t6001-j375c.dts b/arch/arm64/boot/dts/apple/t6001-j375c.dts
index 62ea437b58b25ca649e20b1072b4d835bbc17d3a..9e8c4107e65d8dc47ad2ad99af2f436613c11d8f 100644
--- a/arch/arm64/boot/dts/apple/t6001-j375c.dts
+++ b/arch/arm64/boot/dts/apple/t6001-j375c.dts
@@ -16,3 +16,5 @@ / {
 	compatible = "apple,j375c", "apple,t6001", "apple,arm-platform";
 	model = "Apple Mac Studio (M1 Max, 2022)";
 };
+
+#include "hwmon-fan-dual.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t6002-j375d.dts b/arch/arm64/boot/dts/apple/t6002-j375d.dts
index 3365429bdc8be90b63c8051822243d897854ab27..b62cf16d6b73e74c3d9116730b44596be2a89ea0 100644
--- a/arch/arm64/boot/dts/apple/t6002-j375d.dts
+++ b/arch/arm64/boot/dts/apple/t6002-j375d.dts
@@ -48,3 +48,5 @@ hpm5: usb-pd@3a {
 
 /delete-node/ &ps_disp0_cpu0_die1;
 /delete-node/ &ps_disp0_fe_die1;
+
+#include "hwmon-fan-dual.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi b/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
index 22ebc78e120bf8f0f71fd532e9dce4dcd117bbc6..2cb38861c3855e31c9b8ab66fe69b818c381c604 100644
--- a/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
+++ b/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
@@ -121,3 +121,7 @@ &fpwm0 {
 };
 
 #include "spi1-nvram.dtsi"
+
+#include "hwmon-common.dtsi"
+#include "hwmon-laptop.dtsi"
+#include "hwmon-fan-dual.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t600x-j375.dtsi b/arch/arm64/boot/dts/apple/t600x-j375.dtsi
index d5b985ad567936111ee5cccc9ca9fc23d01d9edf..7e551e8660c9c2b51f021b1188680c5cce9d2951 100644
--- a/arch/arm64/boot/dts/apple/t600x-j375.dtsi
+++ b/arch/arm64/boot/dts/apple/t600x-j375.dtsi
@@ -128,3 +128,5 @@ &pcie0_dart_3 {
 };
 
 #include "spi1-nvram.dtsi"
+
+#include "hwmon-common.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8103-j274.dts b/arch/arm64/boot/dts/apple/t8103-j274.dts
index 1c3e37f86d46d7b5d733717b47c4b57dc55e1201..f5b8cc087882d6bd2b0f4f2711844d2a0fa8b604 100644
--- a/arch/arm64/boot/dts/apple/t8103-j274.dts
+++ b/arch/arm64/boot/dts/apple/t8103-j274.dts
@@ -61,3 +61,5 @@ &pcie0_dart_2 {
 &i2c2 {
 	status = "okay";
 };
+
+#include "hwmon-mac-mini.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8103-j293.dts b/arch/arm64/boot/dts/apple/t8103-j293.dts
index 5b3c42e9f0e6776241bf746d3458766e44e3639a..abb88391635fa048c196d0631d90405519ddd178 100644
--- a/arch/arm64/boot/dts/apple/t8103-j293.dts
+++ b/arch/arm64/boot/dts/apple/t8103-j293.dts
@@ -119,3 +119,6 @@ dfr_panel_in: endpoint {
 &displaydfr_dart {
 	status = "okay";
 };
+
+#include "hwmon-laptop.dtsi"
+#include "hwmon-fan.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8103-j313.dts b/arch/arm64/boot/dts/apple/t8103-j313.dts
index 97a4344d8dca685708aff136af92a1b316f3c3dd..491ead016b2193f123f4ded9dadf85ebf37cdc7e 100644
--- a/arch/arm64/boot/dts/apple/t8103-j313.dts
+++ b/arch/arm64/boot/dts/apple/t8103-j313.dts
@@ -41,3 +41,5 @@ &wifi0 {
 &fpwm1 {
 	status = "okay";
 };
+
+#include "hwmon-laptop.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8103-j456.dts b/arch/arm64/boot/dts/apple/t8103-j456.dts
index 58c8e43789b4861544e20c717124ede3327be010..c2ec6fbb633cc6aeec2322f295c054998fbf08cc 100644
--- a/arch/arm64/boot/dts/apple/t8103-j456.dts
+++ b/arch/arm64/boot/dts/apple/t8103-j456.dts
@@ -75,3 +75,5 @@ &pcie0_dart_1 {
 &pcie0_dart_2 {
 	status = "okay";
 };
+
+#include "hwmon-fan-dual.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8103-j457.dts b/arch/arm64/boot/dts/apple/t8103-j457.dts
index 152f95fd49a2118093396838fbd8b6bd1b518f81..20e85612ae70c116cab788f2c03f5a7d76cb19be 100644
--- a/arch/arm64/boot/dts/apple/t8103-j457.dts
+++ b/arch/arm64/boot/dts/apple/t8103-j457.dts
@@ -48,3 +48,5 @@ ethernet0: ethernet@0,0 {
 &pcie0_dart_2 {
 	status = "okay";
 };
+
+#include "hwmon-fan.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8103-jxxx.dtsi b/arch/arm64/boot/dts/apple/t8103-jxxx.dtsi
index 0c8206156bfefda8a32c869787b2e0c8e67a9d17..f711af410a2edf7587ff091e195c5cf243dc8a34 100644
--- a/arch/arm64/boot/dts/apple/t8103-jxxx.dtsi
+++ b/arch/arm64/boot/dts/apple/t8103-jxxx.dtsi
@@ -92,3 +92,5 @@ &nco_clkref {
 };
 
 #include "spi1-nvram.dtsi"
+
+#include "hwmon-common.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8112-j413.dts b/arch/arm64/boot/dts/apple/t8112-j413.dts
index 6f69658623bf89ce73e3486bce504f1f5f8003f3..500dcdf2d4b5da698ee0798f37f624ff70e7b9f0 100644
--- a/arch/arm64/boot/dts/apple/t8112-j413.dts
+++ b/arch/arm64/boot/dts/apple/t8112-j413.dts
@@ -78,3 +78,5 @@ &i2c4 {
 &fpwm1 {
 	status = "okay";
 };
+
+#include "hwmon-laptop.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8112-j473.dts b/arch/arm64/boot/dts/apple/t8112-j473.dts
index 06fe257f08be498ace6906b936012e01084da702..11db6a92493f367cfa64be5e844c80e99bdd325b 100644
--- a/arch/arm64/boot/dts/apple/t8112-j473.dts
+++ b/arch/arm64/boot/dts/apple/t8112-j473.dts
@@ -52,3 +52,5 @@ &pcie1_dart {
 &pcie2_dart {
 	status = "okay";
 };
+
+#include "hwmon-mac-mini.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8112-j493.dts b/arch/arm64/boot/dts/apple/t8112-j493.dts
index fb8ad7d4c65a8fe7966f5541f24f03a379143cfb..a0da02c00f157a0e667b26aebef9157636b14ecf 100644
--- a/arch/arm64/boot/dts/apple/t8112-j493.dts
+++ b/arch/arm64/boot/dts/apple/t8112-j493.dts
@@ -133,3 +133,6 @@ touchbar0: touchbar@0 {
 		touchscreen-inverted-y;
 	};
 };
+
+#include "hwmon-laptop.dtsi"
+#include "hwmon-fan.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8112-jxxx.dtsi b/arch/arm64/boot/dts/apple/t8112-jxxx.dtsi
index 6da35496a4c88dbaba125ebbe8c5a4a428c647c3..6e54c1fb097e8f72cb4fb37e491893a7e3d7e6c2 100644
--- a/arch/arm64/boot/dts/apple/t8112-jxxx.dtsi
+++ b/arch/arm64/boot/dts/apple/t8112-jxxx.dtsi
@@ -81,3 +81,5 @@ &nco_clkref {
 };
 
 #include "spi1-nvram.dtsi"
+
+#include "hwmon-common.dtsi"

-- 
2.50.1

Re: [PATCH 5/8] input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid

[Lee Jones]

On Tue, 19 Aug 2025, James Calligeros wrote:

> From: Hector Martin <marcan@marcan.st>
> 
> This driver implements power button and lid switch support for Apple Mac
> devices using SMC controllers driven by the macsmc driver.
> 
> In addition to basic input support, this also responds to the final
> shutdown warning (when the power button is held down long enough) by
> doing an emergency kernel poweroff. This allows the NVMe controller to
> be cleanly shut down, which prevents data loss for in-cache data.
> 
> Signed-off-by: Hector Martin <marcan@marcan.st>
> Co-developed-by: Sven Peter <sven@kernel.org>
> Signed-off-by: Sven Peter <sven@kernel.org>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
>  MAINTAINERS                     |   1 +
>  drivers/input/misc/Kconfig      |  11 ++
>  drivers/input/misc/Makefile     |   1 +
>  drivers/input/misc/macsmc-hid.c | 210 +++++++++++++++++++++++++

>  drivers/mfd/macsmc.c            |   1 +

Separate patch please.

>  5 files changed, 224 insertions(+)

-- 
Lee Jones [李琼斯]

Re: [PATCH 4/8] hwmon: Add Apple Silicon SMC hwmon driver

[Lee Jones]

On Tue, 19 Aug 2025, James Calligeros wrote:

> The System Management Controller on Apple Silicon devices is responsible
> for integrating and exposing the data reported by the vast array of
> hardware monitoring sensors present on these devices. It is also
> responsible for fan control, and allows users to manually set fan
> speeds if they so desire. Add a hwmon driver to expose current,
> power, temperature, and voltage monitoring sensors, as well as
> fan speed monitoring and control via the SMC on Apple Silicon devices.
> 
> The SMC firmware has no consistency between devices, even when they
> share an SoC. The FourCC keys used to access sensors are almost
> random. An M1 Mac mini will have different FourCCs for its CPU core
> temperature sensors to an M1 MacBook Pro, for example. For this
> reason, the valid sensors for a given device are specified in a
> child of the SMC Devicetree node. The driver uses this information
> to determine which sensors to make available at runtime.
> 
> Co-developed-by: Janne Grunau <j@jannau.net>
> Signed-off-by: Janne Grunau <j@jannau.net>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
>  MAINTAINERS                  |   2 +
>  drivers/hwmon/Kconfig        |  12 +
>  drivers/hwmon/Makefile       |   1 +
>  drivers/hwmon/macsmc_hwmon.c | 858 +++++++++++++++++++++++++
>  drivers/mfd/macsmc.c         |   1 +

And here.  And everywhere else.

>  5 files changed, 874 insertions(+)

-- 
Lee Jones [李琼斯]

Re: [PATCH 5/8] input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid

[Sasha Finkelstein]

On Tue, 19 Aug 2025 at 14:39, Lee Jones <lee@kernel.org> wrote:
> Separate patch please.
>

Per the discussion in the thread linked from the cover letter, the dt
maintainers have requested the bindings for all subdevices to be added
together. Do you want a separate series with just the dt bindings and dts
changes and the actual drivers in separate series or how do you prefer it?

Re: [PATCH 5/8] input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid

[Janne Grunau]

On Tue, Aug 19, 2025 at 02:49:49PM +0200, Sasha Finkelstein wrote:
> On Tue, 19 Aug 2025 at 14:39, Lee Jones <lee@kernel.org> wrote:
> > Separate patch please.
> >
> 
> Per the discussion in the thread linked from the cover letter, the dt
> maintainers have requested the bindings for all subdevices to be added
> together. Do you want a separate series with just the dt bindings and dts
> changes and the actual drivers in separate series or how do you prefer it?

I think it's asking for patches with the drivera and then separate
single line patches to wire up the drivers in drivers/mfd/macsmc.c

Janne

Re: [PATCH 5/8] input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid

[Lee Jones]

On Tue, 19 Aug 2025, Sasha Finkelstein wrote:

> On Tue, 19 Aug 2025 at 14:39, Lee Jones <lee@kernel.org> wrote:
> > Separate patch please.
> >
> 
> Per the discussion in the thread linked from the cover letter, the dt
> maintainers have requested the bindings for all subdevices to be added
> together. Do you want a separate series with just the dt bindings and dts
> changes and the actual drivers in separate series or how do you prefer it?

I'm only concerned with the part I highlighted:

>  drivers/mfd/macsmc.c            |   1 +

-- 
Lee Jones [李琼斯]

Re: [PATCH 4/8] hwmon: Add Apple Silicon SMC hwmon driver

[Guenter Roeck]

On 8/19/25 04:47, James Calligeros wrote:
> The System Management Controller on Apple Silicon devices is responsible
> for integrating and exposing the data reported by the vast array of
> hardware monitoring sensors present on these devices. It is also
> responsible for fan control, and allows users to manually set fan
> speeds if they so desire. Add a hwmon driver to expose current,
> power, temperature, and voltage monitoring sensors, as well as
> fan speed monitoring and control via the SMC on Apple Silicon devices.
> 
> The SMC firmware has no consistency between devices, even when they
> share an SoC. The FourCC keys used to access sensors are almost
> random. An M1 Mac mini will have different FourCCs for its CPU core
> temperature sensors to an M1 MacBook Pro, for example. For this
> reason, the valid sensors for a given device are specified in a
> child of the SMC Devicetree node. The driver uses this information
> to determine which sensors to make available at runtime.
> 
> Co-developed-by: Janne Grunau <j@jannau.net>
> Signed-off-by: Janne Grunau <j@jannau.net>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>

Checkpatch says:

total: 0 errors, 0 warnings, 35 checks, 904 lines checked

There are lots of alignment issues, making me stumble over them.

Some comments below, but this is not a complete review. The unnecessary
variable initializations, error messages displayed only to be ignored,
and similar problems make it all but impossible for me to really review
the actual code.

All of that is a perfect example for "I can't see the forest because of
all the trees". Please fix.

> ---
>   MAINTAINERS                  |   2 +
>   drivers/hwmon/Kconfig        |  12 +
>   drivers/hwmon/Makefile       |   1 +
>   drivers/hwmon/macsmc_hwmon.c | 858 +++++++++++++++++++++++++
>   drivers/mfd/macsmc.c         |   1 +
>   5 files changed, 874 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 029117b921ea97d1276f5496ea06a3f918929b20..2eb23522654dd050262eb06e077587030cc335aa 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2380,6 +2380,7 @@ F:	Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
>   F:	Documentation/devicetree/bindings/dma/apple,admac.yaml
>   F:	Documentation/devicetree/bindings/gpio/apple,smc-gpio.yaml
>   F:	Documentation/devicetree/bindings/gpu/apple,agx.yaml
> +F:	Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
>   F:	Documentation/devicetree/bindings/i2c/apple,i2c.yaml
>   F:	Documentation/devicetree/bindings/input/touchscreen/apple,z2-multitouch.yaml
>   F:	Documentation/devicetree/bindings/interrupt-controller/apple,*
> @@ -2407,6 +2408,7 @@ F:	drivers/clk/clk-apple-nco.c
>   F:	drivers/cpufreq/apple-soc-cpufreq.c
>   F:	drivers/dma/apple-admac.c
>   F:	drivers/gpio/gpio-macsmc.c
> +F:	drivers/hwmon/macsmc_hwmon.c
>   F:	drivers/pmdomain/apple/
>   F:	drivers/i2c/busses/i2c-pasemi-core.c
>   F:	drivers/i2c/busses/i2c-pasemi-platform.c
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 9d28fcf7cd2a6f9e2f54694a717bd85ff4047b46..1ca6db71e4d9da32717fd14487c10944433ada41 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1164,6 +1164,18 @@ config SENSORS_LTQ_CPUTEMP
>   	  If you say yes here you get support for the temperature
>   	  sensor inside your CPU.
>   
> +config SENSORS_MACSMC_HWMON
> +	tristate "Apple SMC (Apple Silicon)"
> +	depends on MFD_MACSMC && OF
> +	help
> +	  This driver enables hwmon support for current, power, temperature,
> +	  and voltage sensors, as well as fan speed reporting and control
> +	  on Apple Silicon devices. Say Y here if you have an Apple Silicon
> +	  device.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called macsmc_hwmon.
> +
>   config SENSORS_MAX1111
>   	tristate "Maxim MAX1111 Serial 8-bit ADC chip and compatibles"
>   	depends on SPI_MASTER
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index cd8bc4752b4dbf015c6eb46157626f4e8f87dfae..80fc8447aff15b3b8e8583dc755c8accb3b6a93e 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -147,6 +147,7 @@ obj-$(CONFIG_SENSORS_LTC4260)	+= ltc4260.o
>   obj-$(CONFIG_SENSORS_LTC4261)	+= ltc4261.o
>   obj-$(CONFIG_SENSORS_LTC4282)	+= ltc4282.o
>   obj-$(CONFIG_SENSORS_LTQ_CPUTEMP) += ltq-cputemp.o
> +obj-$(CONFIG_SENSORS_MACSMC_HWMON)	+= macsmc_hwmon.o
>   obj-$(CONFIG_SENSORS_MAX1111)	+= max1111.o
>   obj-$(CONFIG_SENSORS_MAX127)	+= max127.o
>   obj-$(CONFIG_SENSORS_MAX16065)	+= max16065.o
> diff --git a/drivers/hwmon/macsmc_hwmon.c b/drivers/hwmon/macsmc_hwmon.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..543a1ab50fc3587cc88625ec703d92a7e7db0825
> --- /dev/null
> +++ b/drivers/hwmon/macsmc_hwmon.c
> @@ -0,0 +1,858 @@
> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> +/*
> + * Apple SMC hwmon driver for Apple Silicon platforms
> + *
> + * The System Management Controller on Apple Silicon devices is responsible for
> + * measuring data from sensors across the SoC and machine. These include power,
> + * temperature, voltage and current sensors. Some "sensors" actually expose
> + * derived values. An example of this is the key PHPC, which is an estimate
> + * of the heat energy being dissipated by the SoC.
> + *
> + * While each SoC only has one SMC variant, each platform exposes a different
> + * set of sensors. For example, M1 MacBooks expose battery telemetry sensors
> + * which are not present on the M1 Mac mini. For this reason, the available
> + * sensors for a given platform are described in the device tree in a child
> + * node of the SMC device. We must walk this list of available sensors and
> + * populate the required hwmon data structures at runtime.
> + *
> + * Originally based on a concept by Jean-Francois Bortolotti <jeff@borto.fr>
> + *
> + * Copyright The Asahi Linux Contributors
> + */
> +
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/mfd/macsmc.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#define MAX_LABEL_LENGTH 32
> +#define NUM_SENSOR_TYPES 5 /* temp, volt, current, power, fan */
> +
> +#define FLT_EXP_BIAS 127
> +#define FLT_EXP_MASK GENMASK(30, 23)
> +#define FLT_MANT_BIAS 23
> +#define FLT_MANT_MASK GENMASK(22, 0)
> +#define FLT_SIGN_MASK BIT(31)

#define<space>NAME<tab>value

> +
> +static bool melt_my_mac;
> +module_param_unsafe(melt_my_mac, bool, 0644);
> +MODULE_PARM_DESC(
> +	melt_my_mac,
> +	"Override the SMC to set your own fan speeds on supported machines");

melt_my_mac ? Please consider a less catchy name.

> +
> +struct macsmc_hwmon_sensor {
> +	struct apple_smc_key_info info;
> +	smc_key macsmc_key;
> +	char label[MAX_LABEL_LENGTH];
> +};
> +
> +struct macsmc_hwmon_fan {
> +	struct macsmc_hwmon_sensor now;
> +	struct macsmc_hwmon_sensor min;
> +	struct macsmc_hwmon_sensor max;
> +	struct macsmc_hwmon_sensor set;
> +	struct macsmc_hwmon_sensor mode;
> +	char label[MAX_LABEL_LENGTH];
> +	u32 attrs;
> +	bool manual;
> +};
> +
> +struct macsmc_hwmon_sensors {
> +	struct hwmon_channel_info channel_info;
> +	struct macsmc_hwmon_sensor *sensors;
> +	u32 n_sensors;
> +};
> +
> +struct macsmc_hwmon_fans {
> +	struct hwmon_channel_info channel_info;
> +	struct macsmc_hwmon_fan *fans;
> +	u32 n_fans;
> +};
> +
> +struct macsmc_hwmon {
> +	struct device *dev;
> +	struct apple_smc *smc;
> +	struct device *hwmon_dev;
> +	struct hwmon_chip_info chip_info;
> +	/* Chip + sensor types + NULL */
> +	const struct hwmon_channel_info *channel_infos[1 + NUM_SENSOR_TYPES + 1];
> +	struct macsmc_hwmon_sensors temp;
> +	struct macsmc_hwmon_sensors volt;
> +	struct macsmc_hwmon_sensors curr;
> +	struct macsmc_hwmon_sensors power;
> +	struct macsmc_hwmon_fans fan;
> +};
> +
> +static int macsmc_hwmon_read_label(struct device *dev,
> +				   enum hwmon_sensor_types type, u32 attr,
> +				   int channel, const char **str)
> +{
> +	struct macsmc_hwmon *hwmon = dev_get_drvdata(dev);
> +
> +	switch (type) {
> +	case hwmon_temp:
> +		if (channel >= hwmon->temp.n_sensors)
> +			return -EINVAL;
> +		*str = hwmon->temp.sensors[channel].label;
> +		break;
> +	case hwmon_in:
> +		if (channel >= hwmon->volt.n_sensors)
> +			return -EINVAL;
> +		*str = hwmon->volt.sensors[channel].label;
> +		break;
> +	case hwmon_curr:
> +		if (channel >= hwmon->curr.n_sensors)
> +			return -EINVAL;
> +		*str = hwmon->curr.sensors[channel].label;
> +		break;
> +	case hwmon_power:
> +		if (channel >= hwmon->power.n_sensors)
> +			return -EINVAL;
> +		*str = hwmon->power.sensors[channel].label;
> +		break;
> +	case hwmon_fan:
> +		if (channel >= hwmon->fan.n_fans)
> +			return -EINVAL;
> +		*str = hwmon->fan.fans[channel].label;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * A number of sensors report data in a 48.16 fixed-point decimal format that is
> + * not used by any other function of the SMC.
> + */
> +static int macsmc_hwmon_read_ioft_scaled(struct apple_smc *smc, smc_key key,
> +					 u64 *p, int scale)
> +{
> +	u64 val;
> +	int ret;
> +
> +	ret = apple_smc_read_u64(smc, key, &val);
> +	if (ret < 0)
> +		return ret;
> +
> +	*p = mult_frac(val, scale, 65536);
> +
> +	return 0;
> +}
> +
> +/*
> + * Many sensors report their data as IEEE-754 floats. No other SMC function uses
> + * them.
> + */
> +static int macsmc_hwmon_read_f32_scaled(struct apple_smc *smc, smc_key key,
> +					int *p, int scale)
> +{
> +	u32 fval;
> +	u64 val;
> +	int ret, exp;
> +
> +	ret = apple_smc_read_u32(smc, key, &fval);
> +	if (ret < 0)
> +		return ret;
> +
> +	val = ((u64)((fval & FLT_MANT_MASK) | BIT(23)));
> +	exp = ((fval >> 23) & 0xff) - FLT_EXP_BIAS - FLT_MANT_BIAS;
> +	if (scale < 0) {
> +		val <<= 32;
> +		exp -= 32;
> +		val /= -scale;

I am quiter sure that this doesn't compile on 32 bit builds.

> +	} else {
> +		val *= scale;
> +	}
> +
> +	if (exp > 63)
> +		val = U64_MAX;
> +	else if (exp < -63)
> +		val = 0;
> +	else if (exp < 0)
> +		val >>= -exp;
> +	else if (exp != 0 && (val & ~((1UL << (64 - exp)) - 1))) /* overflow */
> +		val = U64_MAX;
> +	else
> +		val <<= exp;
> +
> +	if (fval & FLT_SIGN_MASK) {
> +		if (val > (-(s64)INT_MIN))
> +			*p = INT_MIN;
> +		else
> +			*p = -val;
> +	} else {
> +		if (val > INT_MAX)
> +			*p = INT_MAX;
> +		else
> +			*p = val;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * The SMC has keys of multiple types, denoted by a FourCC of the same format
> + * as the key ID. We don't know what data type a key encodes until we poke at it.
> + */
> +static int macsmc_hwmon_read_key(struct apple_smc *smc,
> +				 struct macsmc_hwmon_sensor *sensor, int scale,
> +				 long *val)
> +{
> +	int ret = 0;
> +
> +	switch (sensor->info.type_code) {
> +	/* 32-bit IEEE 754 float */
> +	case _SMC_KEY("flt "): {
> +		u32 flt_ = 0;
> +
> +		ret = macsmc_hwmon_read_f32_scaled(smc, sensor->macsmc_key,
> +						   &flt_, scale);
> +		*val = flt_;
> +		break;
> +	}
> +	/* 48.16 fixed point decimal */
> +	case _SMC_KEY("ioft"): {
> +		u64 ioft = 0;
> +
> +		ret = macsmc_hwmon_read_ioft_scaled(smc, sensor->macsmc_key,
> +						    &ioft, scale);
> +		*val = (long)ioft;
> +		break;
> +	}
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (ret)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int macsmc_hwmon_write_f32_scaled(struct apple_smc *smc, smc_key key,
> +					 int value, int scale)
> +{
> +	u64 val;
> +	u32 fval = 0;
> +	int exp = 0, neg;
> +
> +	val = abs(value);
> +	neg = val != value;
> +
> +	if (scale > 1) {
> +		val <<= 32;
> +		exp = 32;
> +		val /= scale;
> +	} else if (scale < 1) {
> +		val *= -scale;
> +	}
> +
> +	if (val) {
> +		int msb = __fls(val) - exp;
> +
> +		if (msb > 23) {
> +			val >>= msb - 23;
> +			exp -= msb - 23;
> +		} else if (msb < 23) {
> +			val <<= 23 - msb;
> +			exp += msb;
> +		}
> +
> +		fval = FIELD_PREP(FLT_SIGN_MASK, neg) |
> +		       FIELD_PREP(FLT_EXP_MASK, exp + FLT_EXP_BIAS) |
> +		       FIELD_PREP(FLT_MANT_MASK, val);
> +	}
> +
> +	return apple_smc_write_u32(smc, key, fval);
> +}
> +
> +static int macsmc_hwmon_write_key(struct apple_smc *smc,
> +				  struct macsmc_hwmon_sensor *sensor, long val,
> +				  int scale)
> +{
> +	switch (sensor->info.type_code) {
> +	/* 32-bit IEEE 754 float */
> +	case _SMC_KEY("flt "):
> +		return macsmc_hwmon_write_f32_scaled(smc, sensor->macsmc_key,
> +						     val, scale);
> +	/* unsigned 8-bit integer */
> +	case _SMC_KEY("ui8 "):
> +		return apple_smc_write_u8(smc, sensor->macsmc_key, val);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int macsmc_hwmon_read_fan(struct macsmc_hwmon *hwmon, u32 attr, int chan,
> +				 long *val)
> +{
> +	if (!(hwmon->fan.fans[chan].attrs & BIT(attr)))
> +		return -EINVAL;
> +
> +	switch (attr) {
> +	case hwmon_fan_input:
> +		return macsmc_hwmon_read_key(
> +			hwmon->smc, &hwmon->fan.fans[chan].now, 1, val);
> +	case hwmon_fan_min:
> +		return macsmc_hwmon_read_key(
> +			hwmon->smc, &hwmon->fan.fans[chan].min, 1, val);
> +	case hwmon_fan_max:
> +		return macsmc_hwmon_read_key(
> +			hwmon->smc, &hwmon->fan.fans[chan].max, 1, val);
> +	case hwmon_fan_target:
> +		return macsmc_hwmon_read_key(
> +			hwmon->smc, &hwmon->fan.fans[chan].set, 1, val);
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int macsmc_hwmon_write_fan(struct device *dev, u32 attr, int channel,
> +				  long val)
> +{
> +	struct macsmc_hwmon *hwmon = dev_get_drvdata(dev);
> +	int ret = 0;
> +	long min = 0;
> +	long max = 0;
> +
> +	if (!melt_my_mac || hwmon->fan.fans[channel].mode.macsmc_key == 0)
> +		return -EOPNOTSUPP;
> +
> +	if ((channel >= hwmon->fan.n_fans) ||
> +	    !(hwmon->fan.fans[channel].attrs & BIT(attr)) ||
> +	    (attr != hwmon_fan_target))
> +		return -EINVAL;
> +
> +	/*
> +	 * The SMC does no sanity checks on requested fan speeds, so we need to.
> +	 */
> +	ret = macsmc_hwmon_read_key(hwmon->smc, &hwmon->fan.fans[channel].min,
> +				    1, &min);
> +	if (ret)
> +		return ret;
> +	ret = macsmc_hwmon_read_key(hwmon->smc, &hwmon->fan.fans[channel].max,
> +				    1, &max);
> +	if (ret)
> +		return ret;
> +
> +	if (val >= min && val <= max) {
> +		if (!hwmon->fan.fans[channel].manual) {
> +			/* Write 1 to mode key for manual control */
> +			ret = macsmc_hwmon_write_key(
> +				hwmon->smc, &hwmon->fan.fans[channel].mode, 1,
> +				1);
> +			if (ret < 0)
> +				return ret;
> +
> +			hwmon->fan.fans[channel].manual = true;
> +			dev_info(
> +				dev,
> +				"Fan %d now under manual control! Set target speed to 0 for automatic control.\n",
> +				channel + 1);

No such noise please. Make it debug if you want to keep such messages,
but sysfs attribute writes must not result in clogging the log.

> +		}
> +		return macsmc_hwmon_write_key(
> +			hwmon->smc, &hwmon->fan.fans[channel].set, val, 1);
> +	} else if (!val) {
> +		if (hwmon->fan.fans[channel].manual) {
> +			dev_info(dev, "Returning control of fan %d to SMC.\n",
> +				 channel + 1);
> +			ret = macsmc_hwmon_write_key(
> +				hwmon->smc, &hwmon->fan.fans[channel].mode, 0,
> +				1);
> +			if (ret < 0)
> +				return ret;
> +
> +			hwmon->fan.fans[channel].manual = false;
> +		}
> +	} else {
> +		dev_err(dev, "Requested fan speed %ld out of range [%ld, %ld]",
> +			val, min, max);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int macsmc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> +			     u32 attr, int channel, long *val)
> +{
> +	struct macsmc_hwmon *hwmon = dev_get_drvdata(dev);
> +	int ret = 0;
> +
> +	switch (type) {
> +	case hwmon_temp:
> +		ret = macsmc_hwmon_read_key(
> +			hwmon->smc, &hwmon->temp.sensors[channel], 1000, val);
> +		break;
> +	case hwmon_in:
> +		ret = macsmc_hwmon_read_key(
> +			hwmon->smc, &hwmon->volt.sensors[channel], 1000, val);
> +		break;
> +	case hwmon_curr:
> +		ret = macsmc_hwmon_read_key(
> +			hwmon->smc, &hwmon->curr.sensors[channel], 1000, val);
> +		break;
> +	case hwmon_power:
> +		/* SMC returns power in Watts with acceptable precision to scale to uW */
> +		ret = macsmc_hwmon_read_key(hwmon->smc,
> +					    &hwmon->power.sensors[channel],
> +					    1000000, val);
> +		break;
> +	case hwmon_fan:
> +		ret = macsmc_hwmon_read_fan(hwmon, attr, channel, val);
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return ret;
> +}
> +
> +static int macsmc_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> +			      u32 attr, int channel, long val)
> +{
> +	switch (type) {
> +	case hwmon_fan:
> +		return macsmc_hwmon_write_fan(dev, attr, channel, val);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static umode_t macsmc_hwmon_fan_is_visible(const void *data, u32 attr,
> +					   int channel)
> +{
> +	const struct macsmc_hwmon *hwmon = data;
> +
> +	if (channel >= hwmon->fan.n_fans)
> +		return -EINVAL;

That is not a valid value for umode_t.

> +
> +	if (melt_my_mac && attr == hwmon_fan_target &&
> +	    hwmon->fan.fans[channel].mode.macsmc_key != 0)
> +		return 0644;
> +
> +	return 0444;
> +}
> +
> +static umode_t macsmc_hwmon_is_visible(const void *data,
> +				       enum hwmon_sensor_types type, u32 attr,
> +				       int channel)
> +{
> +	switch (type) {
> +	case hwmon_fan:
> +		return macsmc_hwmon_fan_is_visible(data, attr, channel);
> +	default:
> +		break;
> +	}
> +
> +	return 0444;
> +}
> +
> +static const struct hwmon_ops macsmc_hwmon_ops = {
> +	.is_visible = macsmc_hwmon_is_visible,
> +	.read = macsmc_hwmon_read,
> +	.read_string = macsmc_hwmon_read_label,
> +	.write = macsmc_hwmon_write,
> +};
> +
> +/*
> + * Get the key metadata, including key data type, from the SMC.
> + */
> +static int macsmc_hwmon_parse_key(struct device *dev, struct apple_smc *smc,
> +				  struct macsmc_hwmon_sensor *sensor,
> +				  const char *key)
> +{
> +	int ret = 0;
> +
> +	ret = apple_smc_get_key_info(smc, _SMC_KEY(key), &sensor->info);
> +	if (ret) {
> +		dev_err(dev, "Failed to retrieve key info for %s\n", key);
> +		return ret;
> +	}
> +	sensor->macsmc_key = _SMC_KEY(key);
> +
> +	return 0;
> +}
> +
> +/*
> + * A sensor is a single key-value pair as made available by the SMC.
> + * The devicetree gives us the SMC key ID and a friendly name where the
> + * purpose of the sensor is known.
> + */
> +static int macsmc_hwmon_create_sensor(struct device *dev, struct apple_smc *smc,
> +				      struct device_node *sensor_node,
> +				      struct macsmc_hwmon_sensor *sensor)
> +{
> +	const char *key, *label;
> +	int ret = 0;
> +
> +	ret = of_property_read_string(sensor_node, "apple,key-id", &key);
> +	if (ret) {
> +		dev_err(dev, "Could not find apple,key-id in sensor node");
> +		return ret;
> +	}
> +
> +	ret = macsmc_hwmon_parse_key(dev, smc, sensor, key);
> +	if (ret)
> +		return ret;
> +
> +	if (!of_property_read_string(sensor_node, "label", &label))
> +		strscpy_pad(sensor->label, label, sizeof(sensor->label));
> +	else
> +		strscpy_pad(sensor->label, key, sizeof(sensor->label));
> +
> +	return 0;
> +}
> +
> +/*
> + * Fan data is exposed by the SMC as multiple sensors.
> + *
> + * The devicetree schema reuses apple,key-id for the actual fan speed sensor.
> + * Mix, max and target keys do not need labels, so we can reuse label
> + * for naming the entire fan.
> + */
> +static int macsmc_hwmon_create_fan(struct device *dev, struct apple_smc *smc,
> +				   struct device_node *fan_node,
> +				   struct macsmc_hwmon_fan *fan)
> +{
> +	const char *label;
> +	const char *now;
> +	const char *min;
> +	const char *max;
> +	const char *set;
> +	const char *mode;
> +	int ret = 0;

Unnecessary inititialization. This is not the only place. Drop.

> +
> +	ret = of_property_read_string(fan_node, "apple,key-id", &now);
> +	if (ret) {
> +		dev_err(dev, "apple,key-id not found in fan node!");
> +		return -EINVAL;
> +	}
> +
> +	ret = macsmc_hwmon_parse_key(dev, smc, &fan->now, now);
> +	if (ret)
> +		return ret;
> +
> +	if (!of_property_read_string(fan_node, "label", &label))
> +		strscpy_pad(fan->label, label, sizeof(fan->label));
> +	else
> +		strscpy_pad(fan->label, now, sizeof(fan->label));
> +
> +	fan->attrs = HWMON_F_LABEL | HWMON_F_INPUT;
> +
> +	ret = of_property_read_string(fan_node, "apple,fan-minimum", &min);
> +	if (ret)
> +		dev_warn(dev, "No minimum fan speed key for %s", fan->label);
> +	else {
> +		if (!macsmc_hwmon_parse_key(dev, smc, &fan->min, min))
> +			fan->attrs |= HWMON_F_MIN;

Above the error from macsmc_hwmon_parse_key() results in an abort,
here the error is logged in the function and ignored.

Either it is an error or it isn't. Ignoring errors is not acceptable.
Dumping error messages and ignoring the error is even less acceptable.

> +	}
> +
> +	ret = of_property_read_string(fan_node, "apple,fan-maximum", &max);
> +	if (ret)
> +		dev_warn(dev, "No maximum fan speed key for %s", fan->label);
> +	else {
> +		if (!macsmc_hwmon_parse_key(dev, smc, &fan->max, max))
> +			fan->attrs |= HWMON_F_MAX;
> +	}
> +
> +	ret = of_property_read_string(fan_node, "apple,fan-target", &set);
> +	if (ret)
> +		dev_warn(dev, "No target fan speed key for %s", fan->label);
> +	else {
> +		if (!macsmc_hwmon_parse_key(dev, smc, &fan->set, set))
> +			fan->attrs |= HWMON_F_TARGET;
> +	}
> +
> +	ret = of_property_read_string(fan_node, "apple,fan-mode", &mode);
> +	if (ret)
> +		dev_warn(dev, "No fan mode key for %s", fan->label);
> +	else {
> +		ret = macsmc_hwmon_parse_key(dev, smc, &fan->mode, mode);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Initialise fan control mode to automatic */
> +	fan->manual = false;
> +
> +	return 0;
> +}
> +
> +static int macsmc_hwmon_populate_sensors(struct macsmc_hwmon *hwmon,
> +					 struct device_node *hwmon_node)
> +{
> +	struct device_node *group_node = NULL;
> +
> +	for_each_child_of_node(hwmon_node, group_node) {
> +		struct device_node *key_node = NULL;
> +		struct macsmc_hwmon_sensors *sensor_group = NULL;
> +		struct macsmc_hwmon_fans *fan_group = NULL;
> +		u32 n_keys = 0;
> +		int i = 0;
> +
> +		n_keys = of_get_child_count(group_node);
> +		if (!n_keys) {
> +			dev_err(hwmon->dev, "No keys found in %s!\n",
> +				group_node->name);
> +			continue;
> +		}
> +
> +		if (strcmp(group_node->name, "temperature") == 0)
> +			sensor_group = &hwmon->temp;
> +		else if (strcmp(group_node->name, "voltage") == 0)
> +			sensor_group = &hwmon->volt;
> +		else if (strcmp(group_node->name, "current") == 0)
> +			sensor_group = &hwmon->curr;
> +		else if (strcmp(group_node->name, "power") == 0)
> +			sensor_group = &hwmon->power;
> +		else if (strcmp(group_node->name, "fan") == 0)
> +			fan_group = &hwmon->fan;
> +		else {
> +			dev_err(hwmon->dev, "Invalid group node: %s",
> +				group_node->name);

Same here and elsewhere: If it is an error, abort. If it isn't an error,
do not use dev_err().

> +			continue;
> +		}
> +
> +		if (sensor_group) {
> +			sensor_group->sensors = devm_kzalloc(
> +				hwmon->dev,
> +				sizeof(struct macsmc_hwmon_sensor) * n_keys,
> +				GFP_KERNEL);
> +			if (!sensor_group->sensors) {
> +				of_node_put(group_node);
> +				return -ENOMEM;
> +			}
> +
> +			for_each_child_of_node(group_node, key_node) {
> +				if (!macsmc_hwmon_create_sensor(
> +					    hwmon->dev, hwmon->smc, key_node,
> +					    &sensor_group->sensors[i]))
> +					i++;
> +			}
> +
> +			sensor_group->n_sensors = i;
> +			if (!sensor_group->n_sensors) {
> +				dev_err(hwmon->dev,
> +					"No valid sensor keys found in %s\n",
> +					group_node->name);
> +				continue;
> +			}
> +		} else if (fan_group) {
> +			fan_group->fans = devm_kzalloc(
> +				hwmon->dev,
> +				sizeof(struct macsmc_hwmon_fan) * n_keys,
> +				GFP_KERNEL);
> +
> +			if (!fan_group->fans) {
> +				of_node_put(group_node);
> +				return -ENOMEM;
> +			}
> +
> +			for_each_child_of_node(group_node, key_node) {
> +				if (!macsmc_hwmon_create_fan(
> +					    hwmon->dev, hwmon->smc, key_node,
> +					    &fan_group->fans[i]))
> +					i++;
> +			}
> +
> +			fan_group->n_fans = i;
> +			if (!fan_group->n_fans) {
> +				dev_err(hwmon->dev,
> +					"No valid sensor fans found in %s\n",
> +					group_node->name);
> +				continue;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/* Create NULL-terminated config arrays */
> +static void macsmc_hwmon_populate_configs(u32 *configs, u32 num_keys, u32 flags)
> +{
> +	int idx = 0;

Yet another obviously unnecessary initialization. I am not reporting all of those.
Again, the forest story: Such code only distracts from the real problems. Please
keep that in mind.

> +
> +	for (idx = 0; idx < num_keys; idx++)
> +		configs[idx] = flags;
> +
> +	configs[idx + 1] = 0;
> +}
> +
> +static void macsmc_hwmon_populate_fan_configs(u32 *configs, u32 num_keys,
> +					      struct macsmc_hwmon_fans *fans)
> +{
> +	int idx = 0;
> +
> +	for (idx = 0; idx < num_keys; idx++)
> +		configs[idx] = fans->fans[idx].attrs;
> +
> +	configs[idx + 1] = 0;

When would configs[idx + 1] ever be != 0 ?

> +}
> +
> +static const struct hwmon_channel_info *const macsmc_chip_channel_info =
> +	HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ);
> +
> +static int macsmc_hwmon_create_infos(struct macsmc_hwmon *hwmon)
> +{
> +	int i = 0;
> +	struct hwmon_channel_info *channel_info;
> +
> +	/* chip */
> +	hwmon->channel_infos[i++] = macsmc_chip_channel_info;
> +
> +	if (hwmon->temp.n_sensors) {
> +		channel_info = &hwmon->temp.channel_info;
> +		channel_info->type = hwmon_temp;
> +		channel_info->config = devm_kzalloc(
> +			hwmon->dev, sizeof(u32) * hwmon->temp.n_sensors + 1,
> +			GFP_KERNEL);

Consider using devm_kcalloc() instead of re-implementing it.

> +		if (!channel_info->config)
> +			return -ENOMEM;
> +
> +		macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> +					      hwmon->temp.n_sensors,
> +					      (HWMON_T_INPUT | HWMON_T_LABEL));
> +		hwmon->channel_infos[i++] = channel_info;
> +	}
> +
> +	if (hwmon->volt.n_sensors) {
> +		channel_info = &hwmon->volt.channel_info;
> +		channel_info->type = hwmon_in;
> +		channel_info->config = devm_kzalloc(
> +			hwmon->dev, sizeof(u32) * hwmon->volt.n_sensors + 1,
> +			GFP_KERNEL);
> +		if (!channel_info->config)
> +			return -ENOMEM;
> +
> +		macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> +					      hwmon->volt.n_sensors,
> +					      (HWMON_I_INPUT | HWMON_I_LABEL));
> +		hwmon->channel_infos[i++] = channel_info;
> +	}
> +
> +	if (hwmon->curr.n_sensors) {
> +		channel_info = &hwmon->curr.channel_info;
> +		channel_info->type = hwmon_curr;
> +		channel_info->config = devm_kzalloc(
> +			hwmon->dev, sizeof(u32) * hwmon->curr.n_sensors + 1,
> +			GFP_KERNEL);
> +		if (!channel_info->config)
> +			return -ENOMEM;
> +
> +		macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> +					      hwmon->curr.n_sensors,
> +					      (HWMON_C_INPUT | HWMON_C_LABEL));
> +		hwmon->channel_infos[i++] = channel_info;
> +	}
> +
> +	if (hwmon->power.n_sensors) {
> +		channel_info = &hwmon->power.channel_info;
> +		channel_info->type = hwmon_power;
> +		channel_info->config = devm_kzalloc(
> +			hwmon->dev, sizeof(u32) * hwmon->power.n_sensors + 1,
> +			GFP_KERNEL);
> +		if (!channel_info->config)
> +			return -ENOMEM;
> +
> +		macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> +					      hwmon->power.n_sensors,
> +					      (HWMON_P_INPUT | HWMON_P_LABEL));
> +		hwmon->channel_infos[i++] = channel_info;
> +	}
> +
> +	if (hwmon->fan.n_fans) {
> +		channel_info = &hwmon->fan.channel_info;
> +		channel_info->type = hwmon_fan;
> +		channel_info->config = devm_kzalloc(
> +			hwmon->dev, sizeof(u32) * hwmon->fan.n_fans + 1,
> +			GFP_KERNEL);
> +		if (!channel_info->config)
> +			return -ENOMEM;
> +
> +		macsmc_hwmon_populate_fan_configs((u32 *)channel_info->config,
> +						  hwmon->fan.n_fans,
> +						  &hwmon->fan);
> +		hwmon->channel_infos[i++] = channel_info;
> +	}
> +
> +	return 0;
> +}
> +
> +static int macsmc_hwmon_probe(struct platform_device *pdev)
> +{
> +	struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
> +	struct macsmc_hwmon *hwmon;
> +	struct device_node *hwmon_node;
> +	int ret = 0;
> +
> +	hwmon = devm_kzalloc(&pdev->dev, sizeof(struct macsmc_hwmon),
> +			     GFP_KERNEL);

sizeof(*hwmon) is preferred in situations like this to ensure that
the variable types match.

> +	if (!hwmon)
> +		return -ENOMEM;
> +
> +	hwmon->dev = &pdev->dev;
> +	hwmon->smc = smc;
> +
> +	hwmon_node = of_get_child_by_name(pdev->dev.parent->of_node, "hwmon");
> +	if (!hwmon_node) {
> +		dev_err(hwmon->dev, "SMC hwmon node not found in Devicetree\n");
> +		return -ENODEV;
> +	}
> +
> +	ret = macsmc_hwmon_populate_sensors(hwmon, hwmon_node);
> +	if (ret)
> +		dev_info(hwmon->dev, "Could not populate keys!\n");

The only error returned from that function is -ENOMEM. Ignoring that is ok ?
Really ?

And then the function generates many dev_err() but ignores the actual errors.
Sorry, that is just not acceptable.

> +
> +	of_node_put(hwmon_node);
> +
> +	if (!hwmon->temp.n_sensors && !hwmon->volt.n_sensors &&
> +	    !hwmon->curr.n_sensors && !hwmon->power.n_sensors &&
> +	    !hwmon->fan.n_fans) {
> +		dev_err(hwmon->dev,
> +			"No valid keys found of any supported type");
> +		return -ENODEV;
> +	}
> +
> +	ret = macsmc_hwmon_create_infos(hwmon);
> +	if (ret)
> +		return ret;
> +
> +	hwmon->chip_info.ops = &macsmc_hwmon_ops;
> +	hwmon->chip_info.info =
> +		(const struct hwmon_channel_info *const *)&hwmon->channel_infos;
> +
> +	hwmon->hwmon_dev = devm_hwmon_device_register_with_info(
> +		&pdev->dev, "macsmc_hwmon", hwmon, &hwmon->chip_info, NULL);
> +	if (IS_ERR(hwmon->hwmon_dev))
> +		return dev_err_probe(hwmon->dev, PTR_ERR(hwmon->hwmon_dev),
> +				     "Probing SMC hwmon device failed\n");
> +
> +	dev_info(hwmon->dev, "Registered SMC hwmon device. Sensors:");
> +	dev_info(
> +		hwmon->dev,
> +		"Temperature: %d, Voltage: %d, Current: %d, Power: %d, Fans: %d",
> +		hwmon->temp.n_sensors, hwmon->volt.n_sensors,
> +		hwmon->curr.n_sensors, hwmon->power.n_sensors,
> +		hwmon->fan.n_fans);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver macsmc_hwmon_driver = {
> +	.probe = macsmc_hwmon_probe,
> +	.driver = {
> +		.name = "macsmc_hwmon",
> +	},
> +};
> +module_platform_driver(macsmc_hwmon_driver);
> +
> +MODULE_DESCRIPTION("Apple Silicon SMC hwmon driver");
> +MODULE_AUTHOR("James Calligeros <jcalligeros99@gmail.com>");
> +MODULE_LICENSE("Dual MIT/GPL");
> +MODULE_ALIAS("platform:macsmc_hwmon");
> diff --git a/drivers/mfd/macsmc.c b/drivers/mfd/macsmc.c
> index 59be894460d33afa754927630881532b548b7ad8..bc4adf2fcfdce6c5ecbc51ced0e5985cbd36f54d 100644
> --- a/drivers/mfd/macsmc.c
> +++ b/drivers/mfd/macsmc.c
> @@ -48,6 +48,7 @@ static const struct mfd_cell apple_smc_devs[] = {
>   	MFD_CELL_OF("macsmc-gpio", NULL, NULL, 0, 0, "apple,smc-gpio"),
>   	MFD_CELL_OF("macsmc-reboot", NULL, NULL, 0, 0, "apple,smc-reboot"),
>   	MFD_CELL_OF("macsmc-rtc", NULL, NULL, 0, 0, "apple,smc-rtc"),
> +	MFD_CELL_OF("macsmc_hwmon", NULL, NULL, 0, 0, "apple,smc-hwmon"),
>   };
>   
>   static int apple_smc_cmd_locked(struct apple_smc *smc, u64 cmd, u64 arg,
> 

Re: [PATCH 2/8] dt-bindings: hwmon: add Apple System Management Controller hwmon schema

[Rob Herring]

On Tue, Aug 19, 2025 at 09:47:54PM +1000, James Calligeros wrote:
> Apple Silicon devices integrate a vast array of sensors, monitoring
> current, power, temperature, and voltage across almost every part of
> the system. The sensors themselves are all connected to the System
> Management Controller (SMC). The SMC firmware exposes the data
> reported by these sensors via its standard FourCC-based key-value
> API. The SMC is also responsible for monitoring and controlling any
> fans connected to the system, exposing them in the same way.
> 
> For reasons known only to Apple, each device exposes its sensors with
> an almost totally unique set of keys. This is true even for devices
> which share an SoC. An M1 Mac mini, for example, will report its core
> temperatures on different keys to an M1 MacBook Pro. Worse still, the
> SMC does not provide a way to enumerate the available keys at runtime,
> nor do the keys follow any sort of reasonable or consistent naming
> rules that could be used to deduce their purpose. We must therefore
> know which keys are present on any given device, and which function
> they serve, ahead of time.

I'm confused because you say this, but then the .dtsi files are common.

> 
> Add a schema so that we can describe the available sensors for a given
> Apple Silicon device in the Devicetree.
> 
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
>  .../bindings/hwmon/apple,smc-hwmon.yaml  | 148 +++++++++++++++++++++++++
>  .../bindings/mfd/apple,smc.yaml          |  45 ++++++++
>  2 files changed, 193 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..3ebc0463be4e1ce54005418feaa87ec7254dab6e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
> @@ -0,0 +1,148 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/apple,smc-hwmon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple SMC Hardware Monitoring
> +
> +description:
> +  Apple's System Management Controller (SMC) exposes a vast array of
> +  hardware monitoring sensors, including temperature probes, current and
> +  voltage sense, power meters, and fan speeds. It also provides endpoints
> +  to manually control the speed of each fan individually. Each Apple
> +  Silicon device exposes a different set of endpoints via SMC keys. This
> +  is true even when two machines share an SoC. The CPU core temperature
> +  sensor keys on an M1 Mac mini are different to those on an M1 MacBook
> +  Pro, for example.
> +
> +maintainers:
> +  - James Calligeros <jcalligeros99@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: apple,smc-hwmon
> +
> +  current:

I don't see any need to group these and I would remove the intermediate 
node. We have an iterator to iterate over a matching node name prefix if 
that was the reasoning.

> +    description: SMC current sense endpoints
> +    type: object
> +    additionalProperties: false

blank line

> +    patternProperties:
> +      "^current-[A-Za-z0-9]{4}":

Missing a '$' anchor on the end.

> +        type: object
> +        additionalProperties: false

blank line.

> +        required:
> +          - apple,key-id

'required' goes after 'properties'. blank lines in between.

> +        properties:
> +          apple,key-id:
> +            $ref: /schemas/types.yaml#/definitions/string
> +            pattern: "^[A-Za-z0-9]{4}"
> +            description: The SMC FourCC key of the desired current sensor.
> +              Should match the node's suffix, but doesn't have to.

blank line

> +          label:
> +            $ref: /schemas/types.yaml#/definitions/string

Already has a type, don't need to re-define it.

> +            description: Human-readable name for the sensor

Instead of duplicating these properties, You can do it once under a 
'$defs' key and then reference it here.

> +
> +  fan:
> +    description: SMC fan control endpoints. A fan is made up of five
> +      SMC keys - the fan's current speed, its minimum speed, its maximum
> +      speed, a writeable target speed, and a writeable mode. The SMC will
> +      automatically manage system fans unless a 1 is written to the fan's
> +      mode key.
> +    type: object
> +    additionalProperties: false

blank line. And so on...

> +    patternProperties:
> +      "^fan-[A-Za-z0-9]{4}":
> +        type: object
> +        additionalProperties: false
> +        required:
> +          - apple,key-id
> +        properties:
> +          apple,key-id:
> +            $ref: /schemas/types.yaml#/definitions/string
> +            pattern: "^[A-Za-z0-9]{4}"
> +            description: The SMC FourCC key of the desired fan. This is the
> +              main key, which reports the fan's current speed. Sould match

typo

> +              the node's suffix, but doesn't have to.

Why can't we require that they match? (Other than we can't express that 
in schema?)

> +          apple,fan-minimum:
> +            $ref: /schemas/types.yaml#/definitions/string
> +            pattern: "^[A-Za-z0-9]{4}"
> +            description: The minimum speed the current fan can run at

This is not the speed, but the identifier key to retrieve the min speed, 
right? That's not clear. It's a bit odd that everything is a key id, but 
one property has that in the name and the others don't. I don't have any 
better suggestion though...

> +          apple,fan-maximum:
> +            $ref: /schemas/types.yaml#/definitions/string
> +            pattern: "^[A-Za-z0-9]{4}"
> +            description: The maximum speed the current fan can run at
> +          apple,fan-target:
> +            $ref: /schemas/types.yaml#/definitions/string
> +            pattern: "^[A-Za-z0-9]{4}"
> +            description: Writeable endpoint for setting desired fan speed
> +          apple,fan-mode:
> +            $ref: /schemas/types.yaml#/definitions/string
> +            pattern: "^[A-Za-z0-9]{4}"
> +            description: Writeable endpoint to enable/disable manual fan
> +              control
> +          label:
> +            $ref: /schemas/types.yaml#/definitions/string
> +            description: Human-readable name for the sensor

Surely more than apple,key-id is required? How would it be useful with 
only that? You can know how many fans you have, but have no info or 
control over them?

Rob

Re: [PATCH 1/8] dt-bindings: rtc: Add Apple SMC RTC

[Rob Herring (Arm)]

On Tue, 19 Aug 2025 21:47:53 +1000, James Calligeros wrote:
> From: Sven Peter <sven@kernel.org>
> 
> Apple Silicon Macs (M1, etc.) have an RTC that is part of the PMU IC,
> but most of the PMU functionality is abstracted out by the SMC.
> An additional RTC offset stored inside NVMEM is required to compute
> the current date/time.
> 
> Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
> Reviewed-by: Neal Gompa <neal@gompa.dev>
> Signed-off-by: Sven Peter <sven@kernel.org>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
>  .../bindings/mfd/apple,smc.yaml          |  9 +++++++
>  .../bindings/rtc/apple,smc-rtc.yaml      | 35 +++++++++++++++++++++++++
>  MAINTAINERS                              |  1 +
>  3 files changed, 45 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH 2/8] dt-bindings: hwmon: add Apple System Management Controller hwmon schema

[James Calligeros]

Hi Rob,

On Wed, Aug 20, 2025 at 6:15 AM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Aug 19, 2025 at 09:47:54PM +1000, James Calligeros wrote:
> > Apple Silicon devices integrate a vast array of sensors, monitoring
> > current, power, temperature, and voltage across almost every part of
> > the system. The sensors themselves are all connected to the System
> > Management Controller (SMC). The SMC firmware exposes the data
> > reported by these sensors via its standard FourCC-based key-value
> > API. The SMC is also responsible for monitoring and controlling any
> > fans connected to the system, exposing them in the same way.
> >
> > For reasons known only to Apple, each device exposes its sensors with
> > an almost totally unique set of keys. This is true even for devices
> > which share an SoC. An M1 Mac mini, for example, will report its core
> > temperatures on different keys to an M1 MacBook Pro. Worse still, the
> > SMC does not provide a way to enumerate the available keys at runtime,
> > nor do the keys follow any sort of reasonable or consistent naming
> > rules that could be used to deduce their purpose. We must therefore
> > know which keys are present on any given device, and which function
> > they serve, ahead of time.
>
> I'm confused because you say this, but then the .dtsi files are common.

The SMC exposes dozens of sensors, and figuring out which one is which
when all we have to go by are cryptic FourCCs is proving very time consuming.
This is made worse by the fact that even sensors which you'd think should
be consistent across devices with a given SoC are often not. For example,
the M1 Mac mini exposes the application core temperature sensors on different
keys to the M1 MacBook Pro. We have only included the minimal subset of
sensors/fans that we know are common to most devices to validate our approach
with and make the driver itself useful.

> > +    patternProperties:
> > +      "^fan-[A-Za-z0-9]{4}":
> > +        type: object
> > +        additionalProperties: false
> > +        required:
> > +          - apple,key-id
> > +        properties:
> > +          apple,key-id:
> > +            $ref: /schemas/types.yaml#/definitions/string
> > +            pattern: "^[A-Za-z0-9]{4}"
> > +            description: The SMC FourCC key of the desired fan. This is the
> > +              main key, which reports the fan's current speed. Sould match
>
> typo
>
> > +              the node's suffix, but doesn't have to.
>
> Why can't we require that they match? (Other than we can't express that
> in schema?)

I made this optional mostly because these subnode names are inconsequential.
It's the apple,key-id property that matters. If it makes more sense
to say that the node name and property 'must' match instead of 'should' then
there's no reason we can't do that. Another option is to just have a
numbered sequence, e.g. fan-01, temperature-01, etc.

> > +          apple,fan-minimum:
> > +            $ref: /schemas/types.yaml#/definitions/string
> > +            pattern: "^[A-Za-z0-9]{4}"
> > +            description: The minimum speed the current fan can run at
>
> This is not the speed, but the identifier key to retrieve the min speed,
> right? That's not clear. It's a bit odd that everything is a key id, but
> one property has that in the name and the others don't. I don't have any
> better suggestion though...

Would it make sense to append '-key' to all of the optional fan properties
to make this clearer?

> > +          apple,fan-maximum:
> > +            $ref: /schemas/types.yaml#/definitions/string
> > +            pattern: "^[A-Za-z0-9]{4}"
> > +            description: The maximum speed the current fan can run at
> > +          apple,fan-target:
> > +            $ref: /schemas/types.yaml#/definitions/string
> > +            pattern: "^[A-Za-z0-9]{4}"
> > +            description: Writeable endpoint for setting desired fan speed
> > +          apple,fan-mode:
> > +            $ref: /schemas/types.yaml#/definitions/string
> > +            pattern: "^[A-Za-z0-9]{4}"
> > +            description: Writeable endpoint to enable/disable manual fan
> > +              control
> > +          label:
> > +            $ref: /schemas/types.yaml#/definitions/string
> > +            description: Human-readable name for the sensor
>
> Surely more than apple,key-id is required? How would it be useful with
> only that? You can know how many fans you have, but have no info or
> control over them?

The key specified in apple,key-id is the fan's current speed, which is the
only key strictly required to enumerate the presence of a fan in the system.
All of the other keys are optional information that are only really useful
when implementing manual fan control, which is itself optional as the platform
really expects the SMC firmware to have control over fan speeds at all times.

> Rob

Regards,

James

Re: [PATCH 2/8] dt-bindings: hwmon: add Apple System Management Controller hwmon schema

[Sven Peter]

On 20.08.25 01:22, James Calligeros wrote:
> Hi Rob,
> 
> On Wed, Aug 20, 2025 at 6:15 AM Rob Herring <robh@kernel.org> wrote:
>>

[...]

>>> +          apple,fan-maximum:
>>> +            $ref: /schemas/types.yaml#/definitions/string
>>> +            pattern: "^[A-Za-z0-9]{4}"
>>> +            description: The maximum speed the current fan can run at
>>> +          apple,fan-target:
>>> +            $ref: /schemas/types.yaml#/definitions/string
>>> +            pattern: "^[A-Za-z0-9]{4}"
>>> +            description: Writeable endpoint for setting desired fan speed
>>> +          apple,fan-mode:
>>> +            $ref: /schemas/types.yaml#/definitions/string
>>> +            pattern: "^[A-Za-z0-9]{4}"
>>> +            description: Writeable endpoint to enable/disable manual fan
>>> +              control
>>> +          label:
>>> +            $ref: /schemas/types.yaml#/definitions/string
>>> +            description: Human-readable name for the sensor
>>
>> Surely more than apple,key-id is required? How would it be useful with
>> only that? You can know how many fans you have, but have no info or
>> control over them?
> 
> The key specified in apple,key-id is the fan's current speed, which is the
> only key strictly required to enumerate the presence of a fan in the system.
> All of the other keys are optional information that are only really useful
> when implementing manual fan control, which is itself optional as the platform
> really expects the SMC firmware to have control over fan speeds at all times.


Can we at least also require the label? Then we have the SMC key and a 
human readable representation which is already useful.


Sven

Re: [PATCH 4/8] hwmon: Add Apple Silicon SMC hwmon driver

[James Calligeros]

Hi Guenter,

On Wednesday, 20 August 2025 2:02:58 am Australian Eastern Standard Time Guenter Roeck wrote:
> On 8/19/25 04:47, James Calligeros wrote:
> > +/*
> > + * Many sensors report their data as IEEE-754 floats. No other SMC
> > function uses + * them.
> > + */
> > +static int macsmc_hwmon_read_f32_scaled(struct apple_smc *smc, smc_key
> > key, +					int *p, int scale)
> > +{
> > +	u32 fval;
> > +	u64 val;
> > +	int ret, exp;
> > +
> > +	ret = apple_smc_read_u32(smc, key, &fval);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	val = ((u64)((fval & FLT_MANT_MASK) | BIT(23)));
> > +	exp = ((fval >> 23) & 0xff) - FLT_EXP_BIAS - FLT_MANT_BIAS;
> > +	if (scale < 0) {
> > +		val <<= 32;
> > +		exp -= 32;
> > +		val /= -scale;
> 
> I am quiter sure that this doesn't compile on 32 bit builds.
> 
I don't see why not. We're not doing any 64-bit math on pointers, so we should
be safe here. Regardless, this driver depends on MFD_MACSMC, which depends on
ARCH_APPLE, which is an ARM64 platform, so this driver shouldn't be compiled
during a 32-bit build anyway.


> > +
> > +	ret = of_property_read_string(fan_node, "apple,key-id", &now);
> > +	if (ret) {
> > +		dev_err(dev, "apple,key-id not found in fan node!");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = macsmc_hwmon_parse_key(dev, smc, &fan->now, now);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (!of_property_read_string(fan_node, "label", &label))
> > +		strscpy_pad(fan->label, label, sizeof(fan->label));
> > +	else
> > +		strscpy_pad(fan->label, now, sizeof(fan->label));
> > +
> > +	fan->attrs = HWMON_F_LABEL | HWMON_F_INPUT;
> > +
> > +	ret = of_property_read_string(fan_node, "apple,fan-minimum", &min);
> > +	if (ret)
> > +		dev_warn(dev, "No minimum fan speed key for %s", fan->label);
> > +	else {
> > +		if (!macsmc_hwmon_parse_key(dev, smc, &fan->min, min))
> > +			fan->attrs |= HWMON_F_MIN;
> 
> Above the error from macsmc_hwmon_parse_key() results in an abort,
> here the error is logged in the function and ignored.
> 
> Either it is an error or it isn't. Ignoring errors is not acceptable.
> Dumping error messages and ignoring the error is even less acceptable.
> 
The only strictly required key for fan speed monitoring is apple,key-id,
which is why it is the only one that causes an early return when parsing
it fails. If we don't have keys in the DT for min, max, target and mode,
then all that means is we can't enable manual fan speed control. I don't
see how making a failure to read these keys non-blocking is unacceptable
in this context. If this is about the dev_err print in parse_key, then
I can just get rid of that and have the parse_key callers do it when it's
actually a blocking error.

Regards,
James

Re: [PATCH 4/8] hwmon: Add Apple Silicon SMC hwmon driver

[Guenter Roeck]

On 8/22/25 20:33, James Calligeros wrote:
> Hi Guenter,
> 
> On Wednesday, 20 August 2025 2:02:58 am Australian Eastern Standard Time Guenter Roeck wrote:
>> On 8/19/25 04:47, James Calligeros wrote:
>>> +/*
>>> + * Many sensors report their data as IEEE-754 floats. No other SMC
>>> function uses + * them.
>>> + */
>>> +static int macsmc_hwmon_read_f32_scaled(struct apple_smc *smc, smc_key
>>> key, +					int *p, int scale)
>>> +{
>>> +	u32 fval;
>>> +	u64 val;
>>> +	int ret, exp;
>>> +
>>> +	ret = apple_smc_read_u32(smc, key, &fval);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	val = ((u64)((fval & FLT_MANT_MASK) | BIT(23)));
>>> +	exp = ((fval >> 23) & 0xff) - FLT_EXP_BIAS - FLT_MANT_BIAS;
>>> +	if (scale < 0) {
>>> +		val <<= 32;
>>> +		exp -= 32;
>>> +		val /= -scale;
>>
>> I am quiter sure that this doesn't compile on 32 bit builds.
>>
> I don't see why not. We're not doing any 64-bit math on pointers, so we should

Odd (and wrong) answer. What do pointers have to do with 64-bit math ? Nothing.
val is a 64-bit variable, so "val /= -scale" is a 64-bit math operation.

> be safe here. Regardless, this driver depends on MFD_MACSMC, which depends on
> ARCH_APPLE, which is an ARM64 platform, so this driver shouldn't be compiled
> during a 32-bit build anyway.

Right answer.

> 
> 
>>> +
>>> +	ret = of_property_read_string(fan_node, "apple,key-id", &now);
>>> +	if (ret) {
>>> +		dev_err(dev, "apple,key-id not found in fan node!");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	ret = macsmc_hwmon_parse_key(dev, smc, &fan->now, now);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	if (!of_property_read_string(fan_node, "label", &label))
>>> +		strscpy_pad(fan->label, label, sizeof(fan->label));
>>> +	else
>>> +		strscpy_pad(fan->label, now, sizeof(fan->label));
>>> +
>>> +	fan->attrs = HWMON_F_LABEL | HWMON_F_INPUT;
>>> +
>>> +	ret = of_property_read_string(fan_node, "apple,fan-minimum", &min);
>>> +	if (ret)
>>> +		dev_warn(dev, "No minimum fan speed key for %s", fan->label);
>>> +	else {
>>> +		if (!macsmc_hwmon_parse_key(dev, smc, &fan->min, min))
>>> +			fan->attrs |= HWMON_F_MIN;
>>
>> Above the error from macsmc_hwmon_parse_key() results in an abort,
>> here the error is logged in the function and ignored.
>>
>> Either it is an error or it isn't. Ignoring errors is not acceptable.
>> Dumping error messages and ignoring the error is even less acceptable.
>>
> The only strictly required key for fan speed monitoring is apple,key-id,
> which is why it is the only one that causes an early return when parsing
> it fails. If we don't have keys in the DT for min, max, target and mode,
> then all that means is we can't enable manual fan speed control. I don't
> see how making a failure to read these keys non-blocking is unacceptable
> in this context. If this is about the dev_err print in parse_key, then
> I can just get rid of that and have the parse_key callers do it when it's
> actually a blocking error.

dev_err -> it is an error. Don't ignore it. If some of the properties are
optional, they should be defined as such in the devicetree description,
there should be neither an error nor a warning message. Plus, the above
should be explained in a comment so future developers do not wonder and
don't add error handling.

Guenter