devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

  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).