[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251016215159.GB897177@robin.jannau.net>
Date: Thu, 16 Oct 2025 23:51:59 +0200
From: Janne Grunau <j@...nau.net>
To: James Calligeros <jcalligeros99@...il.com>
Cc: Sven Peter <sven@...nel.org>, Alyssa Rosenzweig <alyssa@...enzweig.io>,
Neal Gompa <neal@...pa.dev>, Lee Jones <lee@...nel.org>,
Rob Herring <robh@...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>,
Jonathan Corbet <corbet@....net>, 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,
linux-doc@...r.kernel.org
Subject: Re: [PATCH v3 06/13] hwmon: Add Apple Silicon SMC hwmon driver
On Tue, Oct 07, 2025 at 09:16:47PM +1000, 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.
>
> Reviewed-by: Neal Gompa <neal@...pa.dev>
> Co-developed-by: Janne Grunau <j@...nau.net>
> Signed-off-by: Janne Grunau <j@...nau.net>
> Signed-off-by: James Calligeros <jcalligeros99@...il.com>
> ---
> Documentation/hwmon/macsmc-hwmon.rst | 71 +++
> MAINTAINERS | 2 +
> drivers/hwmon/Kconfig | 12 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/macsmc-hwmon.c | 850 +++++++++++++++++++++++++
> 5 files changed, 936 insertions(+)
...
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -148,6 +148,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 000000000000..342fe3a5ff62
> --- /dev/null
> +++ b/drivers/hwmon/macsmc-hwmon.c
> @@ -0,0 +1,850 @@
> +// 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@...to.fr>
> + *
> + * Copyright The Asahi Linux Contributors
> + */
> +
missing linux/bitfield.h include as noted by kernel robot
> +#include <linux/hwmon.h>
> +#include <linux/mfd/macsmc.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
...
> +static int macsmc_hwmon_probe(struct platform_device *pdev)
> +{
> + struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
> + struct macsmc_hwmon *hwmon;
> + int ret;
> +
> + /*
> + * The MFD driver will try to probe us unconditionally. Some devices
> + * with the SMC do not have hwmon capabilities. Only probe if we have
> + * a hwmon node.
> + */
> + if (!pdev->dev.of_node)
> + return -ENODEV;
> +
> + hwmon = devm_kzalloc(&pdev->dev, sizeof(*hwmon),
> + GFP_KERNEL);
> + if (!hwmon)
> + return -ENOMEM;
> +
> + hwmon->dev = &pdev->dev;
> + hwmon->smc = smc;
> +
> + ret = macsmc_hwmon_populate_sensors(hwmon, hwmon->dev->of_node);
> + if (ret) {
> + dev_err(hwmon->dev, "Could not parse sensors\n");
> + return ret;
> + }
> +
> + if (!hwmon->curr.count && !hwmon->fan.count &&
> + !hwmon->power.count && !hwmon->temp.count &&
> + !hwmon->volt.count) {
> + dev_err(hwmon->dev,
> + "No valid sensors found of any supported type\n");
> + 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,
printing non-errors during probe is strongly discouraged. I also do not
see much value in this message outside of development so please change
to dev_dbg().
Janne
Powered by blists - more mailing lists