From: Rob Herring <robh@kernel.org>
To: James Calligeros <jcalligeros99@gmail.com>
Cc: Sven Peter <sven@kernel.org>, Janne Grunau <j@jannau.net>,
Alyssa Rosenzweig <alyssa@rosenzweig.io>,
Neal Gompa <neal@gompa.dev>, Lee Jones <lee@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Alexandre Belloni <alexandre.belloni@bootlin.com>,
Jean Delvare <jdelvare@suse.com>,
Guenter Roeck <linux@roeck-us.net>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
asahi@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-rtc@vger.kernel.org, linux-hwmon@vger.kernel.org,
linux-input@vger.kernel.org
Subject: Re: [PATCH 2/8] dt-bindings: hwmon: add Apple System Management Controller hwmon schema
Date: Tue, 19 Aug 2025 15:15:37 -0500 [thread overview]
Message-ID: <20250819201537.GA1223169-robh@kernel.org> (raw)
In-Reply-To: <20250819-macsmc-subdevs-v1-2-57df6c3e5f19@gmail.com>
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
next prev parent reply other threads:[~2025-08-19 20:15 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-19 11:47 [PATCH 0/8] mfd: macsmc: add rtc, hwmon and hid subdevices James Calligeros
2025-08-19 11:47 ` [PATCH 1/8] dt-bindings: rtc: Add Apple SMC RTC James Calligeros
2025-08-19 20:17 ` Rob Herring (Arm)
2025-08-19 11:47 ` [PATCH 2/8] dt-bindings: hwmon: add Apple System Management Controller hwmon schema James Calligeros
2025-08-19 20:15 ` Rob Herring [this message]
2025-08-19 23:22 ` James Calligeros
2025-08-21 15:25 ` Sven Peter
2025-08-19 11:47 ` [PATCH 3/8] rtc: Add new rtc-macsmc driver for Apple Silicon Macs James Calligeros
2025-08-19 11:47 ` [PATCH 4/8] hwmon: Add Apple Silicon SMC hwmon driver James Calligeros
2025-08-19 12:39 ` Lee Jones
2025-08-19 16:02 ` Guenter Roeck
2025-08-23 3:33 ` James Calligeros
2025-08-23 5:13 ` Guenter Roeck
2025-08-19 11:47 ` [PATCH 5/8] input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid James Calligeros
2025-08-19 12:35 ` Lee Jones
2025-08-19 12:49 ` Sasha Finkelstein
2025-08-19 13:15 ` Janne Grunau
2025-08-19 13:35 ` Lee Jones
2025-08-19 11:47 ` [PATCH 6/8] arm64: dts: apple: t8103,t600x,t8112: Add SMC RTC node James Calligeros
2025-08-19 11:47 ` [PATCH 7/8] arm64: dts: apple: add common hwmon sensors and fans James Calligeros
2025-08-19 11:48 ` [PATCH 8/8] arm64: dts: apple: t8103, t600x, t8112: add common hwmon nodes to devices James Calligeros
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250819201537.GA1223169-robh@kernel.org \
--to=robh@kernel.org \
--cc=alexandre.belloni@bootlin.com \
--cc=alyssa@rosenzweig.io \
--cc=asahi@lists.linux.dev \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.torokhov@gmail.com \
--cc=j@jannau.net \
--cc=jcalligeros99@gmail.com \
--cc=jdelvare@suse.com \
--cc=krzk+dt@kernel.org \
--cc=lee@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rtc@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=neal@gompa.dev \
--cc=sven@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).