[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250819201537.GA1223169-robh@kernel.org>
Date: Tue, 19 Aug 2025 15:15:37 -0500
From: Rob Herring <robh@...nel.org>
To: James Calligeros <jcalligeros99@...il.com>
Cc: Sven Peter <sven@...nel.org>, Janne Grunau <j@...nau.net>,
Alyssa Rosenzweig <alyssa@...enzweig.io>,
Neal Gompa <neal@...pa.dev>, Lee Jones <lee@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Jean Delvare <jdelvare@...e.com>,
Guenter Roeck <linux@...ck-us.net>,
Dmitry Torokhov <dmitry.torokhov@...il.com>, asahi@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-rtc@...r.kernel.org,
linux-hwmon@...r.kernel.org, linux-input@...r.kernel.org
Subject: Re: [PATCH 2/8] dt-bindings: hwmon: add Apple System Management
Controller hwmon schema
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@...il.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@...il.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
Powered by blists - more mailing lists