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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Rob,

On Wed, Aug 20, 2025 at 6:15 AM Rob Herring <robh@xxxxxxxxxx> 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





[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux