* [PATCH] rtc: fix error return in pm80x_rtc_set_time()
From: Alexandr Sapozhnkiov @ 2025-10-02 9:20 UTC (permalink / raw)
To: Alexandre Belloni, linux-rtc, linux-kernel
Cc: Alexandr Sapozhnikov, lvc-project
From: Alexandr Sapozhnikov <alsp705@gmail.com>
The regmap_raw_read() function may return an error.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Alexandr Sapozhnikov <alsp705@gmail.com>
---
drivers/rtc/rtc-88pm80x.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-88pm80x.c b/drivers/rtc/rtc-88pm80x.c
index f40cc06b0979..50266d2e078a 100644
--- a/drivers/rtc/rtc-88pm80x.c
+++ b/drivers/rtc/rtc-88pm80x.c
@@ -119,6 +119,7 @@ static int pm80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
static int pm80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
{
+ int ret;
struct pm80x_rtc_info *info = dev_get_drvdata(dev);
unsigned char buf[4];
unsigned long ticks, base, data;
@@ -126,7 +127,9 @@ static int pm80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
ticks = rtc_tm_to_time64(tm);
/* load 32-bit read-only counter */
- regmap_raw_read(info->map, PM800_RTC_COUNTER1, buf, 4);
+ ret = regmap_raw_read(info->map, PM800_RTC_COUNTER1, buf, 4);
+ if (ret)
+ return ret;
data = ((unsigned long)buf[3] << 24) | (buf[2] << 16) |
(buf[1] << 8) | buf[0];
base = ticks - data;
--
2.43.0
^ permalink raw reply related
* [PATCH] rtc: fix error return in pm80x_rtc_set_time()
From: Alexandr Sapozhnkiov @ 2025-10-02 9:19 UTC (permalink / raw)
To: Alexandre Belloni, linux-rtc, linux-kernel
Cc: Alexandr Sapozhnikov, lvc-project
From: Alexandr Sapozhnikov <alsp705@gmail.com>
The regmap_raw_write() function may return an error.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Alexandr Sapozhnikov <alsp705@gmail.com>
---
drivers/rtc/rtc-88pm80x.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-88pm80x.c b/drivers/rtc/rtc-88pm80x.c
index f40cc06b0979..71d4cc7e595c 100644
--- a/drivers/rtc/rtc-88pm80x.c
+++ b/drivers/rtc/rtc-88pm80x.c
@@ -136,9 +136,7 @@ static int pm80x_rtc_set_time(struct device *dev, struct rtc_time *tm)
buf[1] = (base >> 8) & 0xFF;
buf[2] = (base >> 16) & 0xFF;
buf[3] = (base >> 24) & 0xFF;
- regmap_raw_write(info->map, PM800_RTC_EXPIRE2_1, buf, 4);
-
- return 0;
+ return regmap_raw_write(info->map, PM800_RTC_EXPIRE2_1, buf, 4);
}
static int pm80x_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
--
2.43.0
^ permalink raw reply related
* [PATCH] rtc: fix error return in pm80x_rtc_read_alarm()
From: Alexandr Sapozhnkiov @ 2025-10-02 9:18 UTC (permalink / raw)
To: Alexandre Belloni, linux-rtc, linux-kernel
Cc: Alexandr Sapozhnikov, lvc-project
From: Alexandr Sapozhnikov <alsp705@gmail.com>
The regmap_raw_read() function may return an error.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Alexandr Sapozhnikov <alsp705@gmail.com>
---
drivers/rtc/rtc-88pm80x.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-88pm80x.c b/drivers/rtc/rtc-88pm80x.c
index f40cc06b0979..3651dc8436f0 100644
--- a/drivers/rtc/rtc-88pm80x.c
+++ b/drivers/rtc/rtc-88pm80x.c
@@ -153,7 +153,9 @@ static int pm80x_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
(buf[1] << 8) | buf[0];
dev_dbg(info->dev, "%x-%x-%x-%x\n", buf[0], buf[1], buf[2], buf[3]);
- regmap_raw_read(info->map, PM800_RTC_EXPIRE1_1, buf, 4);
+ ret = regmap_raw_read(info->map, PM800_RTC_EXPIRE1_1, buf, 4);
+ if (ret)
+ return ret;
data = ((unsigned long)buf[3] << 24) | (buf[2] << 16) |
(buf[1] << 8) | buf[0];
ticks = base + data;
--
2.43.0
^ permalink raw reply related
* [PATCH] rtc: fix error return in pm80x_rtc_read_alarm()
From: Alexandr Sapozhnkiov @ 2025-10-02 9:18 UTC (permalink / raw)
To: Alexandre Belloni, linux-rtc, linux-kernel
Cc: Alexandr Sapozhnikov, lvc-project
From: Alexandr Sapozhnikov <alsp705@gmail.com>
The regmap_raw_read() function may return an error.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Alexandr Sapozhnikov <alsp705@gmail.com>
---
drivers/rtc/rtc-88pm80x.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-88pm80x.c b/drivers/rtc/rtc-88pm80x.c
index f40cc06b0979..44d324c3101c 100644
--- a/drivers/rtc/rtc-88pm80x.c
+++ b/drivers/rtc/rtc-88pm80x.c
@@ -148,7 +148,9 @@ static int pm80x_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
unsigned long ticks, base, data;
int ret;
- regmap_raw_read(info->map, PM800_RTC_EXPIRE2_1, buf, 4);
+ ret = regmap_raw_read(info->map, PM800_RTC_EXPIRE2_1, buf, 4);
+ if (ret)
+ return ret;
base = ((unsigned long)buf[3] << 24) | (buf[2] << 16) |
(buf[1] << 8) | buf[0];
dev_dbg(info->dev, "%x-%x-%x-%x\n", buf[0], buf[1], buf[2], buf[3]);
--
2.43.0
^ permalink raw reply related
* [PATCH] rtc: fix error return in pm80x_rtc_set_alarm()
From: Alexandr Sapozhnkiov @ 2025-10-02 9:16 UTC (permalink / raw)
To: Alexandre Belloni, linux-rtc, linux-kernel
Cc: Alexandr Sapozhnikov, lvc-project
From: Alexandr Sapozhnikov <alsp705@gmail.com>
The regmap_raw_read() function may return an error.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Alexandr Sapozhnikov <alsp705@gmail.com>
---
drivers/rtc/rtc-88pm80x.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-88pm80x.c b/drivers/rtc/rtc-88pm80x.c
index f40cc06b0979..f26d8a1e827b 100644
--- a/drivers/rtc/rtc-88pm80x.c
+++ b/drivers/rtc/rtc-88pm80x.c
@@ -173,17 +173,21 @@ static int pm80x_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
struct rtc_time now_tm, alarm_tm;
unsigned long ticks, base, data;
unsigned char buf[4];
- int mask;
+ int mask, ret;
regmap_update_bits(info->map, PM800_RTC_CONTROL, PM800_ALARM1_EN, 0);
- regmap_raw_read(info->map, PM800_RTC_EXPIRE2_1, buf, 4);
+ ret = regmap_raw_read(info->map, PM800_RTC_EXPIRE2_1, buf, 4);
+ if (ret)
+ return ret;
base = ((unsigned long)buf[3] << 24) | (buf[2] << 16) |
(buf[1] << 8) | buf[0];
dev_dbg(info->dev, "%x-%x-%x-%x\n", buf[0], buf[1], buf[2], buf[3]);
/* load 32-bit read-only counter */
- regmap_raw_read(info->map, PM800_RTC_COUNTER1, buf, 4);
+ ret = regmap_raw_read(info->map, PM800_RTC_COUNTER1, buf, 4);
+ if (ret)
+ return ret;
data = ((unsigned long)buf[3] << 24) | (buf[2] << 16) |
(buf[1] << 8) | buf[0];
ticks = base + data;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] drivers rtc-pcf8523.c: add "clockout-disable" property
From: Rob Herring @ 2025-10-02 1:59 UTC (permalink / raw)
To: Rodolfo Giometti
Cc: linux-rtc, devicetree, Alexandre Belloni, Conor Dooley,
Sam Ravnborg
In-Reply-To: <20250926091038.2262571-1-giometti@enneenne.com>
On Fri, Sep 26, 2025 at 11:10:38AM +0200, Rodolfo Giometti wrote:
> Some systems may require disabling clock generation on the CLKOUT pin
> even if there is no IRQ management.
>
> Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>
> ---
> .../devicetree/bindings/rtc/nxp,pcf8523.yaml | 5 +++++
Bindings should be a separate patch.
> drivers/rtc/rtc-pcf8523.c | 13 +++++++++----
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf8523.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf8523.yaml
> index d11c8bc16bc0..d18c396c06cd 100644
> --- a/Documentation/devicetree/bindings/rtc/nxp,pcf8523.yaml
> +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf8523.yaml
> @@ -25,6 +25,11 @@ properties:
> enum: [ 7000, 12500 ]
> default: 12500
>
> + clockout-disable:
Needs a vendor prefix (nxp,)
> + type: boolean
> + description:
> + Disable the clock generation on CLKOUT pin.
> +
> required:
> - compatible
> - reg
^ permalink raw reply
* Re: [PATCH 03/25] dt-bindings: gpio: rda: Make interrupts optional
From: Linus Walleij @ 2025-10-01 6:24 UTC (permalink / raw)
To: Dang Huynh
Cc: Manivannan Sadhasivam, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bartosz Golaszewski, Alexandre Belloni,
Michael Turquette, Stephen Boyd, Philipp Zabel, Sebastian Reichel,
Vinod Koul, Kees Cook, Gustavo A. R. Silva, Ulf Hansson,
linux-arm-kernel, linux-unisoc, devicetree, linux-kernel,
linux-gpio, linux-rtc, linux-clk, linux-pm, dmaengine,
linux-hardening, linux-mmc
In-Reply-To: <20250917-rda8810pl-drivers-v1-3-74866def1fe3@mainlining.org>
On Tue, Sep 16, 2025 at 10:09 PM Dang Huynh <dang.huynh@mainlining.org> wrote:
> The GPIO controller from the modem does not have an interrupt.
>
> Signed-off-by: Dang Huynh <dang.huynh@mainlining.org>
Can you split out the GPIO patches (like the 3? of them?)
into it's own series and send them separately? They seem
to be possible to review and apply separately.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v2 02/11] dt-bindings: hwmon: Add Apple System Management Controller hwmon schema
From: Guenter Roeck @ 2025-09-28 2:09 UTC (permalink / raw)
To: James Calligeros, Janne Grunau, Rob Herring
Cc: Sven Peter, Alyssa Rosenzweig, Neal Gompa, Lee Jones,
Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Dmitry Torokhov, asahi, linux-arm-kernel,
devicetree, linux-kernel, linux-rtc, linux-hwmon, linux-input
In-Reply-To: <2537878.PYKUYFuaPT@setsuna>
On 9/27/25 17:36, James Calligeros wrote:
> Hi Rob,
>
> On Friday, 26 September 2025 7:43:23 am Australian Eastern Standard Time Rob
> Herring wrote:
>> On Thu, Sep 25, 2025 at 3:49 PM Janne Grunau <j@jannau.net> wrote:
>>> On Fri, Aug 29, 2025 at 11:40:57AM -0500, Rob Herring wrote:
>>>>
>>>> This should be something like this:
>>>>
>>>> "^current-[A-Za-z0-9]{4}$":
>>>> $ref: "#/$defs/sensor"
>>>> unevaluatedProperties: false
>>>>
>>>> With the $defs/sensor being:
>>>>
>>>> $defs:
>>>> sensor:
>>>> type: object
>>>>
>>>> properties:
>>>> apple,key-id:
>>>> $ref: /schemas/types.yaml#/definitions/string
>>>> pattern: "^[A-Za-z0-9]{4}$"
>>>>
>>>> description:
>>>> The SMC FourCC key of the desired sensor. Must match the
>>>> node's suffix.
>>>>
>>>> label:
>>>> description: Human-readable name for the sensor
>>>>
>>>> required:
>>>> - apple,key-id
>>>> - label
>>>>
>>>> Though in general, 'label' should never be required being just for human
>>>> convenience.
>>>
>>> That does not sound as it would be compatible with skipping nodes in the
>>> driver if the node misses label. The driver could of course fall back
>>> to create a hwmon sensors without labels.
>>
>> The driver absolutely should.
>
> The original submission (and our downstream version) do this, but I changed
> it for v2 per Sven's feedback [1]. Outside of development/experimentation,
> we will (should) never have a sensor in the Devicetree of uknown utility.
> If we know what a sensor is for, then we should have a label for it.
>
Label attributes are optional in hwmon drivers. Period.
Guenter
^ permalink raw reply
* Re: [PATCH v2 02/11] dt-bindings: hwmon: Add Apple System Management Controller hwmon schema
From: James Calligeros @ 2025-09-28 0:36 UTC (permalink / raw)
To: Janne Grunau, Rob Herring
Cc: Sven Peter, Alyssa Rosenzweig, Neal Gompa, Lee Jones,
Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, asahi,
linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input
In-Reply-To: <CAL_JsqK-9n3_H6vS80bZuZiSPi9UNuMzHEPFL_EzYTeyNS1cYg@mail.gmail.com>
Hi Rob,
On Friday, 26 September 2025 7:43:23 am Australian Eastern Standard Time Rob
Herring wrote:
> On Thu, Sep 25, 2025 at 3:49 PM Janne Grunau <j@jannau.net> wrote:
> > On Fri, Aug 29, 2025 at 11:40:57AM -0500, Rob Herring wrote:
> > >
> > > This should be something like this:
> > >
> > > "^current-[A-Za-z0-9]{4}$":
> > > $ref: "#/$defs/sensor"
> > > unevaluatedProperties: false
> > >
> > > With the $defs/sensor being:
> > >
> > > $defs:
> > > sensor:
> > > type: object
> > >
> > > properties:
> > > apple,key-id:
> > > $ref: /schemas/types.yaml#/definitions/string
> > > pattern: "^[A-Za-z0-9]{4}$"
> > >
> > > description:
> > > The SMC FourCC key of the desired sensor. Must match the
> > > node's suffix.
> > >
> > > label:
> > > description: Human-readable name for the sensor
> > >
> > > required:
> > > - apple,key-id
> > > - label
> > >
> > > Though in general, 'label' should never be required being just for human
> > > convenience.
> >
> > That does not sound as it would be compatible with skipping nodes in the
> > driver if the node misses label. The driver could of course fall back
> > to create a hwmon sensors without labels.
>
> The driver absolutely should.
The original submission (and our downstream version) do this, but I changed
it for v2 per Sven's feedback [1]. Outside of development/experimentation,
we will (should) never have a sensor in the Devicetree of uknown utility.
If we know what a sensor is for, then we should have a label for it.
> > I looks to me it would be a
> > stretch to call the presence of the labels human convenience.
>
> Then it is an abuse of 'label". "label" is supposed to be literally
> that. Matching a sticker on a port of a device.
>
> If you need to associate a sensor with some other piece of h/w, then
> that should be via a phandle or something.
I don't think doing so is particularly useful for this platform. Few of
the sensors that we know about are directly related to any one piece of
hardware.
It's pretty much just the CPU cores and Broadcom module. The rest are things
like fans, palm rest area temperature sensors, ammeters and voltmeters for
entire
rails, etc.
Even where we can reliably associate a sensor to a piece of hardware,
(e.g. the WiFi/BT board), doing so does not by itself do anything useful. We
still need to write a human-readable label for the sensor.
I was trying to avoid yet another vendor property, but would something like
'apple,sensor-label' work here?
> Rob
James
[1]: https://lore.kernel.org/asahi/4a95cbf3-b3ae-4b26-8db2-dd5cf14a4c0c@kernel.org/
^ permalink raw reply
* [PATCH] drivers rtc-pcf8523.c: add "clockout-disable" property
From: Rodolfo Giometti @ 2025-09-26 9:10 UTC (permalink / raw)
To: linux-rtc, devicetree
Cc: Alexandre Belloni, Rob Herring, Conor Dooley, Sam Ravnborg,
Rodolfo Giometti
Some systems may require disabling clock generation on the CLKOUT pin
even if there is no IRQ management.
Signed-off-by: Rodolfo Giometti <giometti@enneenne.com>
---
.../devicetree/bindings/rtc/nxp,pcf8523.yaml | 5 +++++
drivers/rtc/rtc-pcf8523.c | 13 +++++++++----
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf8523.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf8523.yaml
index d11c8bc16bc0..d18c396c06cd 100644
--- a/Documentation/devicetree/bindings/rtc/nxp,pcf8523.yaml
+++ b/Documentation/devicetree/bindings/rtc/nxp,pcf8523.yaml
@@ -25,6 +25,11 @@ properties:
enum: [ 7000, 12500 ]
default: 12500
+ clockout-disable:
+ type: boolean
+ description:
+ Disable the clock generation on CLKOUT pin.
+
required:
- compatible
- reg
diff --git a/drivers/rtc/rtc-pcf8523.c b/drivers/rtc/rtc-pcf8523.c
index 2c63c0ffd05a..7ecbee4f9c6b 100644
--- a/drivers/rtc/rtc-pcf8523.c
+++ b/drivers/rtc/rtc-pcf8523.c
@@ -418,6 +418,7 @@ static int pcf8523_probe(struct i2c_client *client)
struct pcf8523 *pcf8523;
struct rtc_device *rtc;
bool wakeup_source = false;
+ bool clockout_disable;
u32 value;
int err;
@@ -467,16 +468,20 @@ static int pcf8523_probe(struct i2c_client *client)
set_bit(RTC_FEATURE_ALARM_RES_MINUTE, rtc->features);
clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
+ clockout_disable = of_property_read_bool(client->dev.of_node,
+ "clockout-disable");
+ if (client->irq > 0 || clockout_disable) {
+ err = regmap_write(pcf8523->regmap,
+ PCF8523_TMR_CLKOUT_CTRL, 0x38);
+ if (err < 0)
+ return err;
+ }
if (client->irq > 0) {
unsigned long irqflags = IRQF_TRIGGER_LOW;
if (dev_fwnode(&client->dev))
irqflags = 0;
- err = regmap_write(pcf8523->regmap, PCF8523_TMR_CLKOUT_CTRL, 0x38);
- if (err < 0)
- return err;
-
err = devm_request_threaded_irq(&client->dev, client->irq,
NULL, pcf8523_irq,
IRQF_SHARED | IRQF_ONESHOT | irqflags,
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2 00/11] mfd: macsmc: add rtc, hwmon and hid subdevices
From: Janne Grunau @ 2025-09-25 22:13 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Alyssa Rosenzweig, Neal Gompa, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, asahi,
linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input, Mark Kettenis, Hector Martin
In-Reply-To: <20250827-macsmc-subdevs-v2-0-ce5e99d54c28@gmail.com>
On Wed, Aug 27, 2025 at 09:22:34PM +1000, James Calligeros wrote:
> Hi all,
>
> This series adds support for the remaining SMC subdevices. These are the
> RTC, hwmon, and HID devices. They are being submitted together as the RTC
> and hwmon drivers both require changes to the SMC DT schema.
I we would instantiate all sub devices via MFD_CELL_OF macsmc-hid (or
-input) and the omitted macsmc-power would require SMC dt-binding
changes as well. I'm conflicted on that since both use SMC to determine
if they are present.
Janne
^ permalink raw reply
* Re: [PATCH v2 10/11] arm64: dts: apple: Add common hwmon sensors and fans
From: Janne Grunau @ 2025-09-25 22:02 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Alyssa Rosenzweig, Neal Gompa, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, asahi,
linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input
In-Reply-To: <20250827-macsmc-subdevs-v2-10-ce5e99d54c28@gmail.com>
On Wed, Aug 27, 2025 at 09:22:44PM +1000, James Calligeros wrote:
> Each Apple Silicon device exposes a unique set of sensors and fans,
> however some have been found to be reliably common across devices.
>
> Add these as .dtsi files so that they can be combined with any
> device-specific sensors without excessive repetition.
>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
> .../boot/dts/apple/hwmon-common.dtsi | 37 +++++++++++++++++++++++++
> .../boot/dts/apple/hwmon-fan-dual.dtsi | 24 ++++++++++++++++
> arch/arm64/boot/dts/apple/hwmon-fan.dtsi | 19 +++++++++++++
> .../boot/dts/apple/hwmon-laptop.dtsi | 35 +++++++++++++++++++++++
> .../boot/dts/apple/hwmon-mac-mini.dtsi | 17 ++++++++++++
> 5 files changed, 132 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/apple/hwmon-common.dtsi b/arch/arm64/boot/dts/apple/hwmon-common.dtsi
> new file mode 100644
> index 0000000000000000000000000000000000000000..bac4e44d3f9892fe8ad04125e47dcccb2bcf57a0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/apple/hwmon-common.dtsi
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Hardawre monitoring sensors expected to be found on all Apple Silicon devices
typo: "Hardawre"
> + *
> + * Copyright The Asahi Linux Contributors
> + */
> +
> +&smc {
> + hwmon {
> + compatible = "apple,smc-hwmon";
I think it woiuld make more sense to add the hwmon child node with the
compatible in the SoC *.dtsi and then only add the sensor nodes to
"&smc_hwmon" in these hwmon-*.dtsi files
Janne
^ permalink raw reply
* Re: [PATCH v2 08/11] mfd: macsmc: Wire up Apple SMC HID subdevice
From: Janne Grunau @ 2025-09-25 21:55 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Alyssa Rosenzweig, Neal Gompa, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, asahi,
linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input
In-Reply-To: <20250827-macsmc-subdevs-v2-8-ce5e99d54c28@gmail.com>
On Wed, Aug 27, 2025 at 09:22:42PM +1000, James Calligeros wrote:
> Add the new SMC HID function to the mfd device
>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
> drivers/mfd/macsmc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mfd/macsmc.c b/drivers/mfd/macsmc.c
> index 286dc150aa6cfdd6d29f769094278daaddabe7c3..893fc47d62b2c956d966d1841895a3fa2b9a3005 100644
> --- a/drivers/mfd/macsmc.c
> +++ b/drivers/mfd/macsmc.c
> @@ -45,6 +45,7 @@
> #define SMC_TIMEOUT_MS 500
>
> static const struct mfd_cell apple_smc_devs[] = {
> + MFD_CELL_NAME("macsmc-hid"),
I think it would make sense to use MFD_CELL_OF and device nodes for all
macsmc even though it's in the case of macsmc-input (or -hid) not
strictly necessary
> MFD_CELL_OF("macsmc-gpio", NULL, NULL, 0, 0, "apple,smc-gpio"),
> MFD_CELL_OF("macsmc_hwmon", NULL, NULL, 0, 0, "apple,smc-hwmon"),
> MFD_CELL_OF("macsmc-reboot", NULL, NULL, 0, 0, "apple,smc-reboot"),
Janne
^ permalink raw reply
* Re: [PATCH v2 07/11] input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid
From: Janne Grunau @ 2025-09-25 21:49 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Alyssa Rosenzweig, Neal Gompa, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, asahi,
linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input, Hector Martin
In-Reply-To: <20250827-macsmc-subdevs-v2-7-ce5e99d54c28@gmail.com>
On Wed, Aug 27, 2025 at 09:22:41PM +1000, James Calligeros wrote:
> From: Hector Martin <marcan@marcan.st>
>
> This driver implements power button and lid switch support for Apple Mac
> devices using SMC controllers driven by the macsmc driver.
>
> In addition to basic input support, this also responds to the final
> shutdown warning (when the power button is held down long enough) by
> doing an emergency kernel poweroff. This allows the NVMe controller to
> be cleanly shut down, which prevents data loss for in-cache data.
>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> Co-developed-by: Sven Peter <sven@kernel.org>
> Signed-off-by: Sven Peter <sven@kernel.org>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
> MAINTAINERS | 1 +
> drivers/input/misc/Kconfig | 11 ++
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/macsmc-hid.c | 209 +++++++++++++++++++++++++
I would rename this driver to macsmc-input. I would expect hid driver to
live in drivers/hid/ and use HID report descriptors
> 4 files changed, 222 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2eb23522654dd050262eb06e077587030cc335aa..b3b5220fcd0d4bbef67613c8ee9afa880c0aa45d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2412,6 +2412,7 @@ F: drivers/hwmon/macsmc_hwmon.c
> F: drivers/pmdomain/apple/
> F: drivers/i2c/busses/i2c-pasemi-core.c
> F: drivers/i2c/busses/i2c-pasemi-platform.c
> +F: drivers/input/misc/macsmc-hid.c
> F: drivers/input/touchscreen/apple_z2.c
> F: drivers/iommu/apple-dart.c
> F: drivers/iommu/io-pgtable-dart.c
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 0fb21c99a5e3d1230d7f40f99e0c2d360bbe80e8..a430c50e7f63f245bba56bd526026ec7901cf821 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -961,4 +961,15 @@ config INPUT_STPMIC1_ONKEY
> To compile this driver as a module, choose M here: the
> module will be called stpmic1_onkey.
>
> +config INPUT_MACSMC_HID
> + tristate "Apple Mac SMC lid/buttons"
> + depends on MFD_MACSMC
> + help
> + Say Y here if you want to use the input events delivered via the
> + SMC controller on Apple Mac machines using the macsmc driver.
> + This includes lid open/close and the power button.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called macsmc-hid.
> +
> endif
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index d468c8140b93da5bb537e8a3baea2b90e7bb4229..95b8ebbb9ebbe6f3afc9db724d2ebeba1d75d1a6 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -51,6 +51,7 @@ obj-$(CONFIG_INPUT_IQS7222) += iqs7222.o
> obj-$(CONFIG_INPUT_KEYSPAN_REMOTE) += keyspan_remote.o
> obj-$(CONFIG_INPUT_KXTJ9) += kxtj9.o
> obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
> +obj-$(CONFIG_INPUT_MACSMC_HID) += macsmc-hid.o
> obj-$(CONFIG_INPUT_MAX77650_ONKEY) += max77650-onkey.o
> obj-$(CONFIG_INPUT_MAX77693_HAPTIC) += max77693-haptic.o
> obj-$(CONFIG_INPUT_MAX8925_ONKEY) += max8925_onkey.o
> diff --git a/drivers/input/misc/macsmc-hid.c b/drivers/input/misc/macsmc-hid.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..c7cd56e5c17f08936cc623f7d3690f27e05012a5
> --- /dev/null
> +++ b/drivers/input/misc/macsmc-hid.c
> @@ -0,0 +1,209 @@
> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> +/*
> + * Apple SMC input event driver
> + * Copyright The Asahi Linux Contributors
> + *
> + * This driver exposes HID events from the SMC as an input device.
> + * This includes the lid open/close and power button notifications.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/macsmc.h>
> +#include <linux/module.h>
> +#include <linux/reboot.h>
> +
> +/**
> + * struct macsmc_hid
> + * @dev: Underlying struct device for the HID sub-device
> + * @smc: Pointer to apple_smc struct of the mfd parent
> + * @input: Allocated input_dev; devres managed
> + * @nb: Notifier block used for incoming events from SMC (e.g. button pressed down)
> + * @wakeup_mode: Set to true when system is suspended and power button events should wake it
> + */
> +struct macsmc_hid {
> + struct device *dev;
> + struct apple_smc *smc;
> + struct input_dev *input;
> + struct notifier_block nb;
> + bool wakeup_mode;
> +};
> +
> +#define SMC_EV_BTN 0x7201
> +#define SMC_EV_LID 0x7203
> +
> +#define BTN_POWER 0x01 /* power button on e.g. Mac Mini chasis pressed */
> +#define BTN_TOUCHID 0x06 /* combined TouchID / power button on MacBooks pressed */
> +#define BTN_POWER_HELD_SHORT 0xfe /* power button briefly held down */
> +#define BTN_POWER_HELD_LONG 0x00 /* power button held down; sent just before forced poweroff */
> +
> +static void macsmc_hid_event_button(struct macsmc_hid *smchid, unsigned long event)
> +{
> + u8 button = (event >> 8) & 0xff;
> + u8 state = !!(event & 0xff);
> +
> + switch (button) {
> + case BTN_POWER:
> + case BTN_TOUCHID:
> + if (smchid->wakeup_mode) {
> + if (state)
> + pm_wakeup_hard_event(smchid->dev);
> + } else {
> + input_report_key(smchid->input, KEY_POWER, state);
> + input_sync(smchid->input);
> + }
> + break;
> + case BTN_POWER_HELD_SHORT: /* power button held down; ignore */
> + break;
> + case BTN_POWER_HELD_LONG:
> + /*
> + * If we get here the power button has been held down for a while and
> + * we have about 4 seconds before forced power-off is triggered by SMC.
> + * Try to do an emergency shutdown to make sure the NVMe cache is
> + * flushed. macOS actually does this by panicing (!)...
> + */
> + if (state) {
> + dev_crit(smchid->dev, "Triggering forced shutdown!\n");
> + if (kernel_can_power_off())
> + kernel_power_off();
> + else /* Missing macsmc-reboot driver? */
> + kernel_restart("SMC power button triggered restart");
> + }
> + break;
> + default:
> + dev_warn(smchid->dev, "Unknown SMC button event: %04lx\n", event & 0xffff);
> + }
> +}
> +
> +static void macsmc_hid_event_lid(struct macsmc_hid *smchid, unsigned long event)
> +{
> + u8 lid_state = !!((event >> 8) & 0xff);
> +
> + if (smchid->wakeup_mode && !lid_state)
> + pm_wakeup_hard_event(smchid->dev);
> +
> + input_report_switch(smchid->input, SW_LID, lid_state);
> + input_sync(smchid->input);
> +}
> +
> +static int macsmc_hid_event(struct notifier_block *nb, unsigned long event, void *data)
> +{
> + struct macsmc_hid *smchid = container_of(nb, struct macsmc_hid, nb);
> + u16 type = event >> 16;
> +
> + switch (type) {
> + case SMC_EV_BTN:
> + macsmc_hid_event_button(smchid, event);
> + return NOTIFY_OK;
> + case SMC_EV_LID:
> + macsmc_hid_event_lid(smchid, event);
> + return NOTIFY_OK;
> + default:
> + /* SMC event meant for another driver */
> + return NOTIFY_DONE;
> + }
> +}
> +
> +static int macsmc_hid_probe(struct platform_device *pdev)
> +{
> + struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
> + struct macsmc_hid *smchid;
> + bool have_lid, have_power;
> + int ret;
> +
> + /* Bail early if this SMC neither supports power button nor lid events */
> + have_lid = apple_smc_key_exists(smc, SMC_KEY(MSLD));
> + have_power = apple_smc_key_exists(smc, SMC_KEY(bHLD));
> + if (!have_lid && !have_power)
> + return -ENODEV;
> +
> + smchid = devm_kzalloc(&pdev->dev, sizeof(*smchid), GFP_KERNEL);
> + if (!smchid)
> + return -ENOMEM;
> +
> + smchid->dev = &pdev->dev;
> + smchid->smc = smc;
> + platform_set_drvdata(pdev, smchid);
> +
> + smchid->input = devm_input_allocate_device(&pdev->dev);
> + if (!smchid->input)
> + return -ENOMEM;
> +
> + smchid->input->phys = "macsmc-hid (0)";
> + smchid->input->name = "Apple SMC power/lid events";
> +
> + if (have_lid)
> + input_set_capability(smchid->input, EV_SW, SW_LID);
> + if (have_power)
> + input_set_capability(smchid->input, EV_KEY, KEY_POWER);
> +
> + ret = input_register_device(smchid->input);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to register input device: %d\n", ret);
> + return ret;
> + }
> +
> + if (have_lid) {
> + u8 val;
> +
> + ret = apple_smc_read_u8(smc, SMC_KEY(MSLD), &val);
> + if (ret < 0)
> + dev_warn(&pdev->dev, "Failed to read initial lid state\n");
> + else
> + input_report_switch(smchid->input, SW_LID, val);
> + }
> +
> + if (have_power) {
> + u32 val;
> +
> + ret = apple_smc_read_u32(smc, SMC_KEY(bHLD), &val);
> + if (ret < 0)
> + dev_warn(&pdev->dev, "Failed to read initial power button state\n");
> + else
> + input_report_key(smchid->input, KEY_POWER, val & 1);
> + }
> +
> + input_sync(smchid->input);
> +
> + smchid->nb.notifier_call = macsmc_hid_event;
> + blocking_notifier_chain_register(&smc->event_handlers, &smchid->nb);
> +
> + device_init_wakeup(&pdev->dev, 1);
> +
> + return 0;
> +}
> +
> +static int macsmc_hid_pm_prepare(struct device *dev)
> +{
> + struct macsmc_hid *smchid = dev_get_drvdata(dev);
> +
> + smchid->wakeup_mode = true;
> + return 0;
> +}
> +
> +static void macsmc_hid_pm_complete(struct device *dev)
> +{
> + struct macsmc_hid *smchid = dev_get_drvdata(dev);
> +
> + smchid->wakeup_mode = false;
> +}
> +
> +static const struct dev_pm_ops macsmc_hid_pm_ops = {
> + .prepare = macsmc_hid_pm_prepare,
> + .complete = macsmc_hid_pm_complete,
> +};
> +
> +static struct platform_driver macsmc_hid_driver = {
> + .driver = {
> + .name = "macsmc-hid",
> + .pm = &macsmc_hid_pm_ops,
> + },
> + .probe = macsmc_hid_probe,
> +};
> +module_platform_driver(macsmc_hid_driver);
> +
> +MODULE_AUTHOR("Hector Martin <marcan@marcan.st>");
> +MODULE_LICENSE("Dual MIT/GPL");
> +MODULE_DESCRIPTION("Apple SMC HID driver");
> +MODULE_ALIAS("platform:macsmc-hid");
The module alias should be removed.
Janne
^ permalink raw reply
* Re: [PATCH v2 02/11] dt-bindings: hwmon: Add Apple System Management Controller hwmon schema
From: Rob Herring @ 2025-09-25 21:43 UTC (permalink / raw)
To: Janne Grunau
Cc: James Calligeros, Sven Peter, Alyssa Rosenzweig, Neal Gompa,
Lee Jones, Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, asahi,
linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input
In-Reply-To: <20250925204925.GA637503@robin.jannau.net>
On Thu, Sep 25, 2025 at 3:49 PM Janne Grunau <j@jannau.net> wrote:
>
> On Fri, Aug 29, 2025 at 11:40:57AM -0500, Rob Herring wrote:
> > On Wed, Aug 27, 2025 at 09:22:36PM +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.
> > >
> > > 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 | 132 +++++++++++++++++++++++++
> > > .../bindings/mfd/apple,smc.yaml | 36 +++++++
> > > MAINTAINERS | 1 +
> > > 3 files changed, 169 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..08cc4f55f3a41ca8b3b428088f96240266fa42e8
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
> > > @@ -0,0 +1,132 @@
> >
> > This should be something like this:
> >
> > "^current-[A-Za-z0-9]{4}$":
> > $ref: "#/$defs/sensor"
> > unevaluatedProperties: false
> >
> > With the $defs/sensor being:
> >
> > $defs:
> > sensor:
> > type: object
> >
> > properties:
> > apple,key-id:
> > $ref: /schemas/types.yaml#/definitions/string
> > pattern: "^[A-Za-z0-9]{4}$"
> > description:
> > The SMC FourCC key of the desired sensor. Must match the
> > node's suffix.
> >
> > label:
> > description: Human-readable name for the sensor
> >
> > required:
> > - apple,key-id
> > - label
> >
> > Though in general, 'label' should never be required being just for human
> > convenience.
>
> That does not sound as it would be compatible with skipping nodes in the
> driver if the node misses label. The driver could of course fall back
> to create a hwmon sensors without labels.
The driver absolutely should.
> I looks to me it would be a
> stretch to call the presence of the labels human convenience.
Then it is an abuse of 'label". "label" is supposed to be literally
that. Matching a sticker on a port of a device.
If you need to associate a sensor with some other piece of h/w, then
that should be via a phandle or something.
Rob
^ permalink raw reply
* Re: [PATCH v2 05/11] hwmon: Add Apple Silicon SMC hwmon driver
From: Janne Grunau @ 2025-09-25 21:33 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Alyssa Rosenzweig, Neal Gompa, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, asahi,
linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input
In-Reply-To: <20250827-macsmc-subdevs-v2-5-ce5e99d54c28@gmail.com>
On Wed, Aug 27, 2025 at 09:22:39PM +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.
>
> Co-developed-by: Janne Grunau <j@jannau.net>
> Signed-off-by: Janne Grunau <j@jannau.net>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
> MAINTAINERS | 1 +
> drivers/hwmon/Kconfig | 12 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/macsmc_hwmon.c | 848 +++++++++++++++++++++++++
Please use macsmc-hwmon.c as name. All other macsmc sub device drivers
use '-' and it's more common in 'drivers/hwmon/*.c'
> 4 files changed, 862 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5d53d243dc9abdf1db5865f8e6bcddbac3eafebe..2eb23522654dd050262eb06e077587030cc335aa 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2408,6 +2408,7 @@ F: drivers/clk/clk-apple-nco.c
> F: drivers/cpufreq/apple-soc-cpufreq.c
> F: drivers/dma/apple-admac.c
> F: drivers/gpio/gpio-macsmc.c
> +F: drivers/hwmon/macsmc_hwmon.c
> F: drivers/pmdomain/apple/
> F: drivers/i2c/busses/i2c-pasemi-core.c
> F: drivers/i2c/busses/i2c-pasemi-platform.c
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 9d28fcf7cd2a6f9e2f54694a717bd85ff4047b46..1ca6db71e4d9da32717fd14487c10944433ada41 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1164,6 +1164,18 @@ config SENSORS_LTQ_CPUTEMP
> If you say yes here you get support for the temperature
> sensor inside your CPU.
>
> +config SENSORS_MACSMC_HWMON
> + tristate "Apple SMC (Apple Silicon)"
> + depends on MFD_MACSMC && OF
> + help
> + This driver enables hwmon support for current, power, temperature,
> + and voltage sensors, as well as fan speed reporting and control
> + on Apple Silicon devices. Say Y here if you have an Apple Silicon
> + device.
> +
> + This driver can also be built as a module. If so, the module will
> + be called macsmc_hwmon.
macsmc-hwmon
> +
> config SENSORS_MAX1111
> tristate "Maxim MAX1111 Serial 8-bit ADC chip and compatibles"
> depends on SPI_MASTER
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index cd8bc4752b4dbf015c6eb46157626f4e8f87dfae..80fc8447aff15b3b8e8583dc755c8accb3b6a93e 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -147,6 +147,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 0000000000000000000000000000000000000000..e44ea357870238aa0b38d7b674a6c456271fdf0c
> --- /dev/null
> +++ b/drivers/hwmon/macsmc_hwmon.c
> @@ -0,0 +1,848 @@
> +// 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@borto.fr>
> + *
> + * Copyright The Asahi Linux Contributors
> + */
> +
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/mfd/macsmc.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#define MAX_LABEL_LENGTH 32
> +#define NUM_SENSOR_TYPES 5 /* temp, volt, current, power, fan */
I would write voltage and temperature out. At least in comments.
Shortening "voltage" to volt seems strange when "current" and "power"
are used otherwise.
> +
> +#define FLT_EXP_BIAS 127
> +#define FLT_EXP_MASK GENMASK(30, 23)
> +#define FLT_MANT_BIAS 23
> +#define FLT_MANT_MASK GENMASK(22, 0)
> +#define FLT_SIGN_MASK BIT(31)
> +
> +static bool fan_control;
> +module_param_unsafe(fan_control, bool, 0644);
> +MODULE_PARM_DESC(fan_control,
> + "Override the SMC to set your own fan speeds on supported machines");
> +
> +struct macsmc_hwmon_sensor {
> + struct apple_smc_key_info info;
> + smc_key macsmc_key;
> + char label[MAX_LABEL_LENGTH];
> +};
> +
> +struct macsmc_hwmon_fan {
> + struct macsmc_hwmon_sensor now;
> + struct macsmc_hwmon_sensor min;
> + struct macsmc_hwmon_sensor max;
> + struct macsmc_hwmon_sensor set;
> + struct macsmc_hwmon_sensor mode;
> + char label[MAX_LABEL_LENGTH];
> + u32 attrs;
> + bool manual;
> +};
> +
> +struct macsmc_hwmon_sensors {
> + struct hwmon_channel_info channel_info;
> + struct macsmc_hwmon_sensor *sensors;
> + u32 count;
> +};
> +
> +struct macsmc_hwmon_fans {
> + struct hwmon_channel_info channel_info;
> + struct macsmc_hwmon_fan *fans;
> + u32 count;
> +};
> +
> +struct macsmc_hwmon {
> + struct device *dev;
> + struct apple_smc *smc;
> + struct device *hwmon_dev;
> + struct hwmon_chip_info chip_info;
> + /* Chip + sensor types + NULL */
> + const struct hwmon_channel_info *channel_infos[1 + NUM_SENSOR_TYPES + 1];
> + struct macsmc_hwmon_sensors temp;
> + struct macsmc_hwmon_sensors volt;
> + struct macsmc_hwmon_sensors curr;
matching the "hwmon_*" names is a convincing argument not sure what to
do about the voltage sensors though
> + struct macsmc_hwmon_sensors power;
> + struct macsmc_hwmon_fans fan;
> +};
> +
> +static int macsmc_hwmon_read_label(struct device *dev,
> + enum hwmon_sensor_types type, u32 attr,
> + int channel, const char **str)
> +{
> + struct macsmc_hwmon *hwmon = dev_get_drvdata(dev);
> +
> + switch (type) {
> + case hwmon_temp:
> + if (channel >= hwmon->temp.count)
> + return -EINVAL;
> + *str = hwmon->temp.sensors[channel].label;
> + break;
> + case hwmon_in:
> + if (channel >= hwmon->volt.count)
> + return -EINVAL;
> + *str = hwmon->volt.sensors[channel].label;
> + break;
> + case hwmon_curr:
> + if (channel >= hwmon->curr.count)
> + return -EINVAL;
> + *str = hwmon->curr.sensors[channel].label;
> + break;
> + case hwmon_power:
> + if (channel >= hwmon->power.count)
> + return -EINVAL;
> + *str = hwmon->power.sensors[channel].label;
> + break;
> + case hwmon_fan:
> + if (channel >= hwmon->fan.count)
> + return -EINVAL;
> + *str = hwmon->fan.fans[channel].label;
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * A number of sensors report data in a 48.16 fixed-point decimal format that is
> + * not used by any other function of the SMC.
> + */
> +static int macsmc_hwmon_read_ioft_scaled(struct apple_smc *smc, smc_key key,
> + u64 *p, int scale)
> +{
> + u64 val;
> + int ret;
> +
> + ret = apple_smc_read_u64(smc, key, &val);
> + if (ret < 0)
> + return ret;
> +
> + *p = mult_frac(val, scale, 65536);
> +
> + return 0;
> +}
> +
> +/*
> + * Many sensors report their data as IEEE-754 floats. No other SMC function uses
> + * them.
> + */
> +static int macsmc_hwmon_read_f32_scaled(struct apple_smc *smc, smc_key key,
> + int *p, int scale)
> +{
> + u32 fval;
> + u64 val;
> + int ret, exp;
> +
> + ret = apple_smc_read_u32(smc, key, &fval);
> + if (ret < 0)
> + return ret;
> +
> + val = ((u64)((fval & FLT_MANT_MASK) | BIT(23)));
> + exp = ((fval >> 23) & 0xff) - FLT_EXP_BIAS - FLT_MANT_BIAS;
> + if (scale < 0) {
> + val <<= 32;
> + exp -= 32;
> + val /= -scale;
64-bit division is not universally available. The easiest solution for
Apple silicon would be to depend on CONFIG_64BIT. However so far
macsmc_hwmon_read_key() is only called with positive scale values so it
would make sense to just not support negative scale values.
> + } else {
> + val *= scale;
> + }
> +
> + if (exp > 63)
> + val = U64_MAX;
> + else if (exp < -63)
> + val = 0;
> + else if (exp < 0)
> + val >>= -exp;
> + else if (exp != 0 && (val & ~((1UL << (64 - exp)) - 1))) /* overflow */
> + val = U64_MAX;
> + else
> + val <<= exp;
> +
> + if (fval & FLT_SIGN_MASK) {
> + if (val > (-(s64)INT_MIN))
> + *p = INT_MIN;
> + else
> + *p = -val;
> + } else {
> + if (val > INT_MAX)
> + *p = INT_MAX;
> + else
> + *p = val;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * The SMC has keys of multiple types, denoted by a FourCC of the same format
> + * as the key ID. We don't know what data type a key encodes until we poke at it.
> + */
> +static int macsmc_hwmon_read_key(struct apple_smc *smc,
> + struct macsmc_hwmon_sensor *sensor, int scale,
> + long *val)
> +{
> + int ret;
> +
> + switch (sensor->info.type_code) {
> + /* 32-bit IEEE 754 float */
> + case _SMC_KEY("flt "): {
This fails here with gcc (GCC) 15.2.1 20250808 (Red Hat 15.2.1-1)"
"error: case label does not reduce to an integer constant"
The same for the other "case _SMC_KEY()" statements
> + u32 flt_ = 0;
> +
> + ret = macsmc_hwmon_read_f32_scaled(smc, sensor->macsmc_key,
> + &flt_, scale);
> + *val = flt_;
> + break;
> + }
> + /* 48.16 fixed point decimal */
> + case _SMC_KEY("ioft"): {
> + u64 ioft = 0;
> +
> + ret = macsmc_hwmon_read_ioft_scaled(smc, sensor->macsmc_key,
> + &ioft, scale);
> + *val = (long)ioft;
> + break;
> + }
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + if (ret)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int macsmc_hwmon_write_f32_scaled(struct apple_smc *smc, smc_key key,
> + int value, int scale)
> +{
> + u64 val;
> + u32 fval = 0;
> + int exp, neg;
> +
> + val = abs(value);
> + neg = val != value;
> +
> + if (scale > 1) {
> + val <<= 32;
> + exp = 32;
> + val /= scale;
Similar here. macsmc_hwmon_write_key() is only called with scale == 1
> + } else if (scale < 1) {
> + val *= -scale;
> + }
> +
> + if (val) {
> + int msb = __fls(val) - exp;
> +
> + if (msb > 23) {
> + val >>= msb - 23;
> + exp -= msb - 23;
> + } else if (msb < 23) {
> + val <<= 23 - msb;
> + exp += msb;
> + }
> +
> + fval = FIELD_PREP(FLT_SIGN_MASK, neg) |
> + FIELD_PREP(FLT_EXP_MASK, exp + FLT_EXP_BIAS) |
> + FIELD_PREP(FLT_MANT_MASK, val);
> + }
> +
> + return apple_smc_write_u32(smc, key, fval);
> +}
> +
> +static int macsmc_hwmon_write_key(struct apple_smc *smc,
> + struct macsmc_hwmon_sensor *sensor, long val,
> + int scale)
> +{
> + switch (sensor->info.type_code) {
> + /* 32-bit IEEE 754 float */
> + case _SMC_KEY("flt "):
> + return macsmc_hwmon_write_f32_scaled(smc, sensor->macsmc_key,
> + val, scale);
> + /* unsigned 8-bit integer */
> + case _SMC_KEY("ui8 "):
> + return apple_smc_write_u8(smc, sensor->macsmc_key, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int macsmc_hwmon_read_fan(struct macsmc_hwmon *hwmon, u32 attr, int chan,
> + long *val)
> +{
> + if (!(hwmon->fan.fans[chan].attrs & BIT(attr)))
> + return -EINVAL;
> +
> + switch (attr) {
> + case hwmon_fan_input:
> + return macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->fan.fans[chan].now, 1, val);
> + case hwmon_fan_min:
> + return macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->fan.fans[chan].min, 1, val);
> + case hwmon_fan_max:
> + return macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->fan.fans[chan].max, 1, val);
> + case hwmon_fan_target:
> + return macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->fan.fans[chan].set, 1, val);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int macsmc_hwmon_write_fan(struct device *dev, u32 attr, int channel,
> + long val)
> +{
> + struct macsmc_hwmon *hwmon = dev_get_drvdata(dev);
> + long min, max;
> + int ret;
> +
> + if (!fan_control || hwmon->fan.fans[channel].mode.macsmc_key == 0)
> + return -EOPNOTSUPP;
> +
> + if (channel >= hwmon->fan.count ||
> + !(hwmon->fan.fans[channel].attrs & BIT(attr)) ||
> + attr != hwmon_fan_target)
> + return -EINVAL;
> +
> + /*
> + * The SMC does no sanity checks on requested fan speeds, so we need to.
> + */
> + ret = macsmc_hwmon_read_key(hwmon->smc, &hwmon->fan.fans[channel].min,
> + 1, &min);
> + if (ret)
> + return ret;
> +
> + ret = macsmc_hwmon_read_key(hwmon->smc, &hwmon->fan.fans[channel].max,
> + 1, &max);
> + if (ret)
> + return ret;
> +
> + if (val >= min && val <= max) {
> + if (!hwmon->fan.fans[channel].manual) {
> + /* Write 1 to mode key for manual control */
> + ret = macsmc_hwmon_write_key(hwmon->smc,
> + &hwmon->fan.fans[channel].mode, 1,
> + 1);
> + if (ret < 0)
> + return ret;
> +
> + hwmon->fan.fans[channel].manual = true;
> + }
> + return macsmc_hwmon_write_key(hwmon->smc,
> + &hwmon->fan.fans[channel].set, val, 1);
> + } else if (!val) {
> + if (hwmon->fan.fans[channel].manual) {
> + ret = macsmc_hwmon_write_key(hwmon->smc,
> + &hwmon->fan.fans[channel].mode, 0, 1);
> + if (ret < 0)
> + return ret;
> +
> + hwmon->fan.fans[channel].manual = false;
> + }
> + } else {
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int macsmc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + struct macsmc_hwmon *hwmon = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + switch (type) {
> + case hwmon_temp:
> + ret = macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->temp.sensors[channel], 1000, val);
> + break;
> + case hwmon_in:
> + ret = macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->volt.sensors[channel], 1000, val);
> + break;
> + case hwmon_curr:
> + ret = macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->curr.sensors[channel], 1000, val);
> + break;
> + case hwmon_power:
> + /* SMC returns power in Watts with acceptable precision to scale to uW */
> + ret = macsmc_hwmon_read_key(hwmon->smc,
> + &hwmon->power.sensors[channel],
> + 1000000, val);
> + break;
> + case hwmon_fan:
> + ret = macsmc_hwmon_read_fan(hwmon, attr, channel, val);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + return ret;
> +}
> +
> +static int macsmc_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long val)
> +{
> + switch (type) {
> + case hwmon_fan:
> + return macsmc_hwmon_write_fan(dev, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static umode_t macsmc_hwmon_fan_is_visible(const void *data, u32 attr,
> + int channel)
> +{
> + const struct macsmc_hwmon *hwmon = data;
> +
> + if (fan_control && attr == hwmon_fan_target &&
> + hwmon->fan.fans[channel].mode.macsmc_key != 0)
> + return 0644;
> +
> + return 0444;
> +}
> +
> +static umode_t macsmc_hwmon_is_visible(const void *data,
> + enum hwmon_sensor_types type, u32 attr,
> + int channel)
> +{
> + switch (type) {
> + case hwmon_fan:
> + return macsmc_hwmon_fan_is_visible(data, attr, channel);
> + default:
> + break;
> + }
> +
> + return 0444;
> +}
> +
> +static const struct hwmon_ops macsmc_hwmon_ops = {
> + .is_visible = macsmc_hwmon_is_visible,
> + .read = macsmc_hwmon_read,
> + .read_string = macsmc_hwmon_read_label,
> + .write = macsmc_hwmon_write,
> +};
> +
> +/*
> + * Get the key metadata, including key data type, from the SMC.
> + */
> +static int macsmc_hwmon_parse_key(struct device *dev, struct apple_smc *smc,
> + struct macsmc_hwmon_sensor *sensor,
> + const char *key)
> +{
> + int ret;
> +
> + ret = apple_smc_get_key_info(smc, _SMC_KEY(key), &sensor->info);
> + if (ret) {
> + dev_err(dev, "Failed to retrieve key info for %s\n", key);
> + return ret;
> + }
> +
> + sensor->macsmc_key = _SMC_KEY(key);
> +
> + return 0;
> +}
> +
> +/*
> + * A sensor is a single key-value pair as made available by the SMC.
> + * The devicetree gives us the SMC key ID and a friendly name where the
> + * purpose of the sensor is known.
> + */
> +static int macsmc_hwmon_create_sensor(struct device *dev, struct apple_smc *smc,
> + struct device_node *sensor_node,
> + struct macsmc_hwmon_sensor *sensor)
> +{
> + const char *key, *label;
> + int ret;
> +
> + ret = of_property_read_string(sensor_node, "apple,key-id", &key);
> + if (ret) {
> + dev_err(dev, "Could not find apple,key-id in sensor node\n");
> + return ret;
> + }
> +
> + ret = macsmc_hwmon_parse_key(dev, smc, sensor, key);
> + if (ret)
> + return ret;
> +
> + ret = of_property_read_string(sensor_node, "label", &label);
> + if (ret) {
> + dev_err(dev, "No label found for sensor %s\n", key);
see discussion in the dt-bindings patch, if the label is just for human
convenience this should not fail but use the key-id as fallback.
> + return ret;
> + }
> +
> + strscpy_pad(sensor->label, label, sizeof(sensor->label));
> +
> + return 0;
> +}
> +
> +/*
> + * Fan data is exposed by the SMC as multiple sensors.
> + *
> + * The devicetree schema reuses apple,key-id for the actual fan speed sensor.
> + * Min, max and target keys do not need labels, so we can reuse label
> + * for naming the entire fan.
> + */
> +static int macsmc_hwmon_create_fan(struct device *dev, struct apple_smc *smc,
> + struct device_node *fan_node,
> + struct macsmc_hwmon_fan *fan)
> +{
> + const char *label, *now, *min, *max, *set, *mode;
> + int ret;
> +
> + ret = of_property_read_string(fan_node, "apple,key-id", &now);
> + if (ret) {
> + dev_err(dev, "apple,key-id not found in fan node!\n");
> + return -EINVAL;
> + }
> +
> + ret = macsmc_hwmon_parse_key(dev, smc, &fan->now, now);
> + if (ret)
> + return ret;
> +
> + ret = of_property_read_string(fan_node, "label", &label);
> + if (ret) {
> + dev_err(dev, "No label found for fan %s\n", now);
> + return ret;
> + }
> +
> + strscpy_pad(fan->label, label, sizeof(fan->label));
> +
> + fan->attrs = HWMON_F_LABEL | HWMON_F_INPUT;
> +
> + /* The following keys are not required to simply monitor fan speed */
> + if (!of_property_read_string(fan_node, "apple,fan-minimum", &min)) {
> + ret = macsmc_hwmon_parse_key(dev, smc, &fan->min, min);
> + if (ret)
> + return ret;
> +
> + fan->attrs |= HWMON_F_MIN;
> + }
> +
> + if (!of_property_read_string(fan_node, "apple,fan-maximum", &max)) {
> + ret = macsmc_hwmon_parse_key(dev, smc, &fan->max, max);
> + if (ret)
> + return ret;
> +
> + fan->attrs |= HWMON_F_MAX;
> + }
> +
> + if (!of_property_read_string(fan_node, "apple,fan-target", &set)) {
> + ret = macsmc_hwmon_parse_key(dev, smc, &fan->set, set);
> + if (ret)
> + return ret;
> +
> + fan->attrs |= HWMON_F_TARGET;
> + }
> +
> + if (!of_property_read_string(fan_node, "apple,fan-mode", &mode)) {
> + ret = macsmc_hwmon_parse_key(dev, smc, &fan->mode, mode);
> + if (ret)
> + return ret;
> + }
> +
> + /* Initialise fan control mode to automatic */
> + fan->manual = false;
> +
> + return 0;
> +}
> +
> +static int macsmc_hwmon_populate_sensors(struct macsmc_hwmon *hwmon,
> + struct device_node *hwmon_node)
> +{
> + struct device_node *key_node __maybe_unused;
> + u32 n_current = 0, n_fan = 0, n_power = 0, n_temperature = 0, n_voltage = 0;
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "current-") {
> + n_current++;
> + }
> +
> + if (n_current) {
> + hwmon->curr.sensors = devm_kcalloc(hwmon->dev, n_current,
> + sizeof(struct macsmc_hwmon_sensor), GFP_KERNEL);
> + if (!hwmon->curr.sensors)
> + return -ENOMEM;
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "current-") {
> + if (!macsmc_hwmon_create_sensor(hwmon->dev, hwmon->smc, key_node,
> + &hwmon->curr.sensors[hwmon->curr.count]))
> + hwmon->curr.count++;
> + }
> + }
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "fan-") {
> + n_fan++;
> + }
> +
> + if (n_fan) {
> + hwmon->fan.fans = devm_kcalloc(hwmon->dev, n_fan,
> + sizeof(struct macsmc_hwmon_fan), GFP_KERNEL);
> + if (!hwmon->fan.fans)
> + return -ENOMEM;
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "fan-") {
> + if (!macsmc_hwmon_create_fan(hwmon->dev, hwmon->smc, key_node,
> + &hwmon->fan.fans[hwmon->fan.count]))
> + hwmon->fan.count++;
> + }
> + }
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "power-") {
> + n_power++;
> + }
> +
> + if (n_power) {
> + hwmon->power.sensors = devm_kcalloc(hwmon->dev, n_power,
> + sizeof(struct macsmc_hwmon_sensor), GFP_KERNEL);
> + if (!hwmon->power.sensors)
> + return -ENOMEM;
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "power-") {
> + if (!macsmc_hwmon_create_sensor(hwmon->dev, hwmon->smc, key_node,
> + &hwmon->power.sensors[hwmon->power.count]))
> + hwmon->power.count++;
> + }
> + }
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "temperature-") {
> + n_temperature++;
> + }
> +
> + if (n_temperature) {
> + hwmon->temp.sensors = devm_kcalloc(hwmon->dev, n_temperature,
> + sizeof(struct macsmc_hwmon_sensor), GFP_KERNEL);
> + if (!hwmon->temp.sensors)
> + return -ENOMEM;
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "temperature-") {
> + if (!macsmc_hwmon_create_sensor(hwmon->dev, hwmon->smc, key_node,
> + &hwmon->temp.sensors[hwmon->temp.count]))
> + hwmon->temp.count++;
> + }
> + }
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "voltage-") {
> + n_voltage++;
> + }
> +
> + if (n_voltage) {
> + hwmon->volt.sensors = devm_kcalloc(hwmon->dev, n_voltage,
> + sizeof(struct macsmc_hwmon_sensor), GFP_KERNEL);
> + if (!hwmon->volt.sensors)
> + return -ENOMEM;
> +
> + for_each_child_of_node_with_prefix(hwmon_node, key_node, "volt-") {
> + if (!macsmc_hwmon_create_sensor(hwmon->dev, hwmon->smc, key_node,
> + &hwmon->volt.sensors[hwmon->volt.count]))
> + hwmon->volt.count++;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/* Create NULL-terminated config arrays */
> +static void macsmc_hwmon_populate_configs(u32 *configs, u32 num_keys, u32 flags)
> +{
> + int idx;
> +
> + for (idx = 0; idx < num_keys; idx++)
> + configs[idx] = flags;
> +}
> +
> +static void macsmc_hwmon_populate_fan_configs(u32 *configs, u32 num_keys,
> + struct macsmc_hwmon_fans *fans)
> +{
> + int idx;
> +
> + for (idx = 0; idx < num_keys; idx++)
> + configs[idx] = fans->fans[idx].attrs;
> +}
> +
> +static const struct hwmon_channel_info *const macsmc_chip_channel_info =
> + HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ);
> +
> +static int macsmc_hwmon_create_infos(struct macsmc_hwmon *hwmon)
> +{
> + struct hwmon_channel_info *channel_info;
> + int i = 0;
> +
> + /* chip */
> + hwmon->channel_infos[i++] = macsmc_chip_channel_info;
> +
> + if (hwmon->curr.count) {
> + channel_info = &hwmon->curr.channel_info;
> + channel_info->type = hwmon_curr;
> + channel_info->config = devm_kcalloc(hwmon->dev, hwmon->curr.count + 1,
> + sizeof(u32), GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> + hwmon->curr.count,
> + (HWMON_C_INPUT | HWMON_C_LABEL));
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + if (hwmon->fan.count) {
> + channel_info = &hwmon->fan.channel_info;
> + channel_info->type = hwmon_fan;
> + channel_info->config = devm_kcalloc(hwmon->dev, hwmon->fan.count + 1,
> + sizeof(u32), GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_fan_configs((u32 *)channel_info->config,
> + hwmon->fan.count,
> + &hwmon->fan);
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + if (hwmon->power.count) {
> + channel_info = &hwmon->power.channel_info;
> + channel_info->type = hwmon_power;
> + channel_info->config = devm_kcalloc(hwmon->dev, hwmon->power.count + 1,
> + sizeof(u32), GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> + hwmon->power.count,
> + (HWMON_P_INPUT | HWMON_P_LABEL));
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + if (hwmon->temp.count) {
> + channel_info = &hwmon->temp.channel_info;
> + channel_info->type = hwmon_temp;
> + channel_info->config = devm_kcalloc(hwmon->dev, hwmon->temp.count + 1,
> + sizeof(u32), GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> + hwmon->temp.count,
> + (HWMON_T_INPUT | HWMON_T_LABEL));
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + if (hwmon->volt.count) {
> + channel_info = &hwmon->volt.channel_info;
> + channel_info->type = hwmon_in;
> + channel_info->config = devm_kcalloc(hwmon->dev, hwmon->volt.count + 1,
> + sizeof(u32), GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> + hwmon->volt.count,
> + (HWMON_I_INPUT | HWMON_I_LABEL));
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + return 0;
> +}
> +
> +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 populate keys!\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 keys found of any supported type");
> + 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,
> + "Current: %d, Fans: %d, Power: %d, Temperature: %d, Voltage: %d",
> + hwmon->curr.count, hwmon->fan.count,
> + hwmon->power.count, hwmon->temp.count,
> + hwmon->volt.count);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id macsmc_hwmon_of_table[] = {
> + { .compatible = "apple,smc-hwmon" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, macsmc_hwmon_of_table);
> +
> +static struct platform_driver macsmc_hwmon_driver = {
> + .probe = macsmc_hwmon_probe,
> + .driver = {
> + .name = "macsmc_hwmon",
macsmc-hwmon
> + .of_match_table = macsmc_hwmon_of_table,
> + },
> +};
> +module_platform_driver(macsmc_hwmon_driver);
> +
> +MODULE_DESCRIPTION("Apple Silicon SMC hwmon driver");
> +MODULE_AUTHOR("James Calligeros <jcalligeros99@gmail.com>");
> +MODULE_LICENSE("Dual MIT/GPL");
> +MODULE_ALIAS("platform:macsmc_hwmon");
The module alias should be removed when using MFD_CELL_OF in macsmc.c
Janne
^ permalink raw reply
* Re: [PATCH v2 02/11] dt-bindings: hwmon: Add Apple System Management Controller hwmon schema
From: Janne Grunau @ 2025-09-25 20:49 UTC (permalink / raw)
To: Rob Herring
Cc: James Calligeros, Sven Peter, Alyssa Rosenzweig, Neal Gompa,
Lee Jones, Krzysztof Kozlowski, Conor Dooley, Alexandre Belloni,
Jean Delvare, Guenter Roeck, Dmitry Torokhov, asahi,
linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input
In-Reply-To: <20250829164057.GA976361-robh@kernel.org>
On Fri, Aug 29, 2025 at 11:40:57AM -0500, Rob Herring wrote:
> On Wed, Aug 27, 2025 at 09:22:36PM +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.
> >
> > 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 | 132 +++++++++++++++++++++++++
> > .../bindings/mfd/apple,smc.yaml | 36 +++++++
> > MAINTAINERS | 1 +
> > 3 files changed, 169 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..08cc4f55f3a41ca8b3b428088f96240266fa42e8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
> > @@ -0,0 +1,132 @@
>
> This should be something like this:
>
> "^current-[A-Za-z0-9]{4}$":
> $ref: "#/$defs/sensor"
> unevaluatedProperties: false
>
> With the $defs/sensor being:
>
> $defs:
> sensor:
> type: object
>
> properties:
> apple,key-id:
> $ref: /schemas/types.yaml#/definitions/string
> pattern: "^[A-Za-z0-9]{4}$"
> description:
> The SMC FourCC key of the desired sensor. Must match the
> node's suffix.
>
> label:
> description: Human-readable name for the sensor
>
> required:
> - apple,key-id
> - label
>
> Though in general, 'label' should never be required being just for human
> convenience.
That does not sound as it would be compatible with skipping nodes in the
driver if the node misses label. The driver could of course fall back
to create a hwmon sensors without labels. I looks to me it would be a
stretch to call the presence of the labels human convenience.
Janne
^ permalink raw reply
* RE: [PATCH V2] rtc: zynqmp: Restore alarm functionality after kexec transition
From: T, Harini @ 2025-09-25 6:04 UTC (permalink / raw)
To: alexandre.belloni@bootlin.com, Simek, Michal
Cc: linux-rtc@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, git (AMD-Xilinx)
In-Reply-To: <DS7PR12MB606995B08A344C01711C7BCC9201A@DS7PR12MB6069.namprd12.prod.outlook.com>
[Public]
Ping!
> -----Original Message-----
> From: T, Harini
> Sent: Wednesday, September 3, 2025 8:53 PM
> To: Harini T <harini.t@amd.com>; alexandre.belloni@bootlin.com; Simek,
> Michal <michal.simek@amd.com>
> Cc: linux-rtc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>
> Subject: RE: [PATCH V2] rtc: zynqmp: Restore alarm functionality after kexec
> transition
>
> Hi Alexandre,
>
> > -----Original Message-----
> > From: Harini T <harini.t@amd.com>
> > Sent: Wednesday, July 30, 2025 7:51 PM
> > To: alexandre.belloni@bootlin.com; Simek, Michal
> > <michal.simek@amd.com>
> > Cc: linux-rtc@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > linux- kernel@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>; T,
> > Harini <Harini.T@amd.com>
> > Subject: [PATCH V2] rtc: zynqmp: Restore alarm functionality after
> > kexec transition
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > During kexec reboots, RTC alarms that are fired during the kernel
> > transition experience delayed execution. The new kernel would
> > eventually honor these alarms, but the interrupt handlers would only
> > execute after the driver probe is completed rather than at the intended alarm
> time.
> >
> > This is because pending alarm interrupt status from the previous
> > kernel is not properly cleared during driver initialization, causing
> > timing discrepancies in alarm delivery.
> >
> > To ensure precise alarm timing across kexec transitions, enhance the
> > probe function to:
> > 1. Clear any pending alarm interrupt status from previous boot.
> > 2. Detect existing valid alarms and preserve their state.
> > 3. Re-enable alarm interrupts for future alarms.
> >
> > Signed-off-by: Harini T <harini.t@amd.com>
> > ---
> > Changes in V2:
> > - Remove shutdown handler to prevent alarm interrupts from being
> > disabled during kexec transitions.
> > - Add alarm state detection and restoration in probe.
> >
> > V1 link: https://lore.kernel.org/linux-rtc/20250724170517.974356-1-
> > harini.t@amd.com/T/#u
> > ---
> > drivers/rtc/rtc-zynqmp.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c index
> > f39102b66eac..3baa2b481d9f 100644
> > --- a/drivers/rtc/rtc-zynqmp.c
> > +++ b/drivers/rtc/rtc-zynqmp.c
> > @@ -277,6 +277,10 @@ static irqreturn_t xlnx_rtc_interrupt(int irq,
> > void *id) static int xlnx_rtc_probe(struct platform_device *pdev) {
> > struct xlnx_rtc_dev *xrtcdev;
> > + bool is_alarm_set = false;
> > + u32 pending_alrm_irq;
> > + u32 current_time;
> > + u32 alarm_time;
> > int ret;
> >
> > xrtcdev = devm_kzalloc(&pdev->dev, sizeof(*xrtcdev),
> > GFP_KERNEL); @@
> > -296,6 +300,17 @@ static int xlnx_rtc_probe(struct platform_device *pdev)
> > if (IS_ERR(xrtcdev->reg_base))
> > return PTR_ERR(xrtcdev->reg_base);
> >
> > + /* Clear any pending alarm interrupts from previous kernel/boot */
> > + pending_alrm_irq = readl(xrtcdev->reg_base + RTC_INT_STS) &
> > RTC_INT_ALRM;
> > + if (pending_alrm_irq)
> > + writel(pending_alrm_irq, xrtcdev->reg_base +
> > + RTC_INT_STS);
> > +
> > + /* Check if a valid alarm is already set from previous kernel/boot */
> > + alarm_time = readl(xrtcdev->reg_base + RTC_ALRM);
> > + current_time = readl(xrtcdev->reg_base + RTC_CUR_TM);
> > + if (alarm_time > current_time && alarm_time != 0)
> > + is_alarm_set = true;
> > +
> > xrtcdev->alarm_irq = platform_get_irq_byname(pdev, "alarm");
> > if (xrtcdev->alarm_irq < 0)
> > return xrtcdev->alarm_irq; @@ -337,6 +352,10 @@ static
> > int xlnx_rtc_probe(struct platform_device *pdev)
> >
> > xlnx_init_rtc(xrtcdev);
> >
> > + /* Re-enable alarm interrupt if a valid alarm was found */
> > + if (is_alarm_set)
> > + writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_EN);
> > +
> > device_init_wakeup(&pdev->dev, true);
> >
> > return devm_rtc_register_device(xrtcdev->rtc);
> > --
> > 2.43.0
> >
>
> Kindly following up on this patch. I would appreciate it if you could review it
> when convenient and let me know if any updates are needed.
>
> Thank you for your time.
>
> Best regards,
> Harini T
^ permalink raw reply
* Re: [PATCH v6 1/4] dt-bindings: rtc: Document NVIDIA VRS RTC
From: Shubhi Garg @ 2025-09-25 5:36 UTC (permalink / raw)
To: Jon Hunter, Rob Herring
Cc: Lee Jones, Krzysztof Kozlowski, Conor Dooley, Catalin Marinas,
Will Deacon, Alexandre Belloni, devicetree, linux-arm-kernel,
linux-rtc, linux-tegra
In-Reply-To: <a1d6955c-ac43-4cd3-bb7a-9b627aecc47c@nvidia.com>
On 23/09/25 1:38 am, Jon Hunter wrote:
>
> On 22/09/2025 20:53, Rob Herring wrote:
>> On Fri, Sep 19, 2025 at 01:59:50PM +0000, Shubhi Garg wrote:
>>> Add device tree bindings for NVIDIA VRS (Voltage Regulator
>>> Specification)
>>> RTC device. NVIDIA VRS is a Power Management IC (PMIC) that implements a
>>> power sequencing solution with I2C interface. The device includes RTC
>>> which provides functionality to get/set system time, retain system
>>> time across boot, wake system from suspend and shutdown state.
>>
>> I only see this patch. The threading of this is broken.
>
> Yep I see the same. I am not sure what happened here, but let me work
> with Shubhi to get this resent.
Yes, patches got sent individually so threading is broken.
I am fixing it and will send V7 version with more fixes.
--
Regards,
Shubhi
^ permalink raw reply
* [PATCH] dt-bindings: rtc: Convert apm,xgene-rtc to DT schema
From: Rob Herring (Arm) @ 2025-09-24 22:28 UTC (permalink / raw)
To: Alexandre Belloni, Krzysztof Kozlowski, Conor Dooley, Khuong Dinh
Cc: linux-rtc, devicetree, linux-kernel
Convert the APM XGene RTC binding to DT schema format. It's a
straight-forward conversion.
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
.../bindings/rtc/apm,xgene-rtc.yaml | 45 +++++++++++++++++++
.../devicetree/bindings/rtc/xgene-rtc.txt | 28 ------------
2 files changed, 45 insertions(+), 28 deletions(-)
create mode 100644 Documentation/devicetree/bindings/rtc/apm,xgene-rtc.yaml
delete mode 100644 Documentation/devicetree/bindings/rtc/xgene-rtc.txt
diff --git a/Documentation/devicetree/bindings/rtc/apm,xgene-rtc.yaml b/Documentation/devicetree/bindings/rtc/apm,xgene-rtc.yaml
new file mode 100644
index 000000000000..b8f46536fd5a
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/apm,xgene-rtc.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/apm,xgene-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: APM X-Gene Real Time Clock
+
+maintainers:
+ - Khuong Dinh <khuong@os.amperecomputing.com>
+
+properties:
+ compatible:
+ const: apm,xgene-rtc
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ '#clock-cells':
+ const: 1
+
+ clocks:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - '#clock-cells'
+ - clocks
+
+additionalProperties: false
+
+examples:
+ - |
+ rtc@10510000 {
+ compatible = "apm,xgene-rtc";
+ reg = <0x10510000 0x400>;
+ interrupts = <0x0 0x46 0x4>;
+ #clock-cells = <1>;
+ clocks = <&rtcclk 0>;
+ };
diff --git a/Documentation/devicetree/bindings/rtc/xgene-rtc.txt b/Documentation/devicetree/bindings/rtc/xgene-rtc.txt
deleted file mode 100644
index fd195c358446..000000000000
--- a/Documentation/devicetree/bindings/rtc/xgene-rtc.txt
+++ /dev/null
@@ -1,28 +0,0 @@
-* APM X-Gene Real Time Clock
-
-RTC controller for the APM X-Gene Real Time Clock
-
-Required properties:
-- compatible : Should be "apm,xgene-rtc"
-- reg: physical base address of the controller and length of memory mapped
- region.
-- interrupts: IRQ line for the RTC.
-- #clock-cells: Should be 1.
-- clocks: Reference to the clock entry.
-
-Example:
-
-rtcclk: rtcclk {
- compatible = "fixed-clock";
- #clock-cells = <1>;
- clock-frequency = <100000000>;
- clock-output-names = "rtcclk";
-};
-
-rtc: rtc@10510000 {
- compatible = "apm,xgene-rtc";
- reg = <0x0 0x10510000 0x0 0x400>;
- interrupts = <0x0 0x46 0x4>;
- #clock-cells = <1>;
- clocks = <&rtcclk 0>;
-};
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v1 3/7] dt-bindings: clock: samsung,s2mps11: document the S2MPS16 compatible
From: Rob Herring (Arm) @ 2025-09-24 15:13 UTC (permalink / raw)
To: Ivaylo Ivanov
Cc: Conor Dooley, Alexandre Belloni, Alim Akhtar, Krzysztof Kozlowski,
Sylwester Nawrocki, Liam Girdwood, André Draszik,
linux-samsung-soc, Michael Turquette, linux-kernel, linux-rtc,
Stephen Boyd, Mark Brown, linux-clk, Chanwoo Choi, Lee Jones,
devicetree
In-Reply-To: <20250914124227.2619925-4-ivo.ivanov.ivanov1@gmail.com>
On Sun, 14 Sep 2025 15:42:23 +0300, Ivaylo Ivanov wrote:
> The S2MPS16 PMIC, alongside regulators and an rtc, provides 3 clock
> outputs, just like most of the other S2MPS PMICs. Document the S2MPS16
> clock compatible.
>
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
> Documentation/devicetree/bindings/clock/samsung,s2mps11.yaml | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v1 2/7] dt-bindings: mfd: samsung,s2mps11: add compatible for s2mps16-pmic
From: Rob Herring (Arm) @ 2025-09-24 15:13 UTC (permalink / raw)
To: Ivaylo Ivanov
Cc: Sylwester Nawrocki, Chanwoo Choi, Lee Jones, Krzysztof Kozlowski,
Stephen Boyd, André Draszik, devicetree, Alexandre Belloni,
Conor Dooley, linux-samsung-soc, Alim Akhtar, linux-kernel,
Liam Girdwood, Mark Brown, linux-rtc, linux-clk,
Michael Turquette
In-Reply-To: <20250914124227.2619925-3-ivo.ivanov.ivanov1@gmail.com>
On Sun, 14 Sep 2025 15:42:22 +0300, Ivaylo Ivanov wrote:
> S2MPS16 is a PMIC present in Samsung's exynos8890 devices. It houses
> voltage regulators (38 LDOs and 11 BUCKs), an RTC and a clocks module.
> Add the compatible string "samsung,s2mps16-pmic" to the PMIC.
>
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
> .../bindings/mfd/samsung,s2mps11.yaml | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v1 1/7] regulator: dt-bindings: add documentation for s2mps16-pmic regulators
From: Rob Herring @ 2025-09-24 15:12 UTC (permalink / raw)
To: Ivaylo Ivanov
Cc: Krzysztof Kozlowski, André Draszik, Sylwester Nawrocki,
Chanwoo Choi, Alim Akhtar, Michael Turquette, Stephen Boyd,
Conor Dooley, Lee Jones, Liam Girdwood, Mark Brown,
Alexandre Belloni, linux-samsung-soc, devicetree, linux-clk,
linux-rtc, linux-kernel
In-Reply-To: <20250914124227.2619925-2-ivo.ivanov.ivanov1@gmail.com>
On Sun, Sep 14, 2025 at 03:42:21PM +0300, Ivaylo Ivanov wrote:
> The S2MPS16 is a PMIC found in exynos8890 devices, which controls voltage
> regulators - 38 LDOs, of which 11 are used for CP, and 11 BUCKs, of which
> 1 is used for CP. Provide documentation for devicetree definitions,
> regulator naming patterns, etc.
Also, drop 'documentation for' in the subject.
>
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
> .../bindings/regulator/samsung,s2mps16.yaml | 50 +++++++++++++++++++
> 1 file changed, 50 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/samsung,s2mps16.yaml
>
> diff --git a/Documentation/devicetree/bindings/regulator/samsung,s2mps16.yaml b/Documentation/devicetree/bindings/regulator/samsung,s2mps16.yaml
> new file mode 100644
> index 000000000..ede87d3b9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/samsung,s2mps16.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/samsung,s2mps16.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung S2MPS16 Power Management IC regulators
> +
> +maintainers:
> + - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> +
> +description: |
> + This is a part of device tree bindings for S2M and S5M family of Power
> + Management IC (PMIC).
> +
> + The S2MPS16 provides buck and LDO regulators.
> +
> + See also Documentation/devicetree/bindings/mfd/samsung,s2mps11.yaml for
> + additional information and example.
> +
> +patternProperties:
> + # 27 LDOs
> + "^ldo([1-9]|1[0-3]|2[5-9]|3[0-8])$":
> + type: object
> + $ref: regulator.yaml#
> + unevaluatedProperties: false
> + description:
> + Properties for single LDO regulator.
> +
> + LDOs 14-24 are used for CP, and they're left unimplemented due to lack
> + of documentation on them.
> +
> + required:
> + - regulator-name
> +
> + # 10 bucks
> + "^buck([1-9]|1[0-1])$":
> + type: object
> + $ref: regulator.yaml#
> + unevaluatedProperties: false
> + description:
> + Properties for single BUCK regulator.
> +
> + BUCK 10 is used for CP, and it's left unimplemented due to lack of
> + documentation on it.
> +
> + required:
> + - regulator-name
> +
> +additionalProperties: false
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v1 1/7] regulator: dt-bindings: add documentation for s2mps16-pmic regulators
From: Rob Herring @ 2025-09-24 15:11 UTC (permalink / raw)
To: Ivaylo Ivanov
Cc: Krzysztof Kozlowski, André Draszik, Sylwester Nawrocki,
Chanwoo Choi, Alim Akhtar, Michael Turquette, Stephen Boyd,
Conor Dooley, Lee Jones, Liam Girdwood, Mark Brown,
Alexandre Belloni, linux-samsung-soc, devicetree, linux-clk,
linux-rtc, linux-kernel
In-Reply-To: <20250914124227.2619925-2-ivo.ivanov.ivanov1@gmail.com>
On Sun, Sep 14, 2025 at 03:42:21PM +0300, Ivaylo Ivanov wrote:
> The S2MPS16 is a PMIC found in exynos8890 devices, which controls voltage
> regulators - 38 LDOs, of which 11 are used for CP, and 11 BUCKs, of which
> 1 is used for CP. Provide documentation for devicetree definitions,
> regulator naming patterns, etc.
>
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> ---
> .../bindings/regulator/samsung,s2mps16.yaml | 50 +++++++++++++++++++
> 1 file changed, 50 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/samsung,s2mps16.yaml
>
> diff --git a/Documentation/devicetree/bindings/regulator/samsung,s2mps16.yaml b/Documentation/devicetree/bindings/regulator/samsung,s2mps16.yaml
> new file mode 100644
> index 000000000..ede87d3b9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/samsung,s2mps16.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/samsung,s2mps16.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung S2MPS16 Power Management IC regulators
> +
> +maintainers:
> + - Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>
> +
> +description: |
> + This is a part of device tree bindings for S2M and S5M family of Power
> + Management IC (PMIC).
> +
> + The S2MPS16 provides buck and LDO regulators.
> +
> + See also Documentation/devicetree/bindings/mfd/samsung,s2mps11.yaml for
> + additional information and example.
> +
> +patternProperties:
> + # 27 LDOs
> + "^ldo([1-9]|1[0-3]|2[5-9]|3[0-8])$":
> + type: object
> + $ref: regulator.yaml#
> + unevaluatedProperties: false
> + description:
You need '>' modifier to preserve paragraphs.
> + Properties for single LDO regulator.
> +
> + LDOs 14-24 are used for CP, and they're left unimplemented due to lack
> + of documentation on them.
What does unimplemented mean? In some driver? If so, that's specific to
a driver and not relevant to the binding.
> +
> + required:
> + - regulator-name
> +
> + # 10 bucks
> + "^buck([1-9]|1[0-1])$":
> + type: object
> + $ref: regulator.yaml#
> + unevaluatedProperties: false
> + description:
> + Properties for single BUCK regulator.
> +
> + BUCK 10 is used for CP, and it's left unimplemented due to lack of
> + documentation on it.
> +
> + required:
> + - regulator-name
> +
> +additionalProperties: false
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: rtc: Add pcf85053 support
From: Conor Dooley @ 2025-09-23 18:57 UTC (permalink / raw)
To: Lakshay Piplani
Cc: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree, pankit.garg, vikash.bansal, priyanka.jain,
shashank.rebbapragada
In-Reply-To: <20250923113441.555284-1-lakshay.piplani@nxp.com>
[-- Attachment #1: Type: text/plain, Size: 5975 bytes --]
On Tue, Sep 23, 2025 at 05:04:40PM +0530, Lakshay Piplani wrote:
> Add device tree bindings for NXP PCF85053 RTC chip.
>
> Signed-off-by: Pankit Garg <pankit.garg@nxp.com>
> Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
> ---
> V3 -> V4: Add dedicated nxp,pcf85053.yaml.
> Remove entry from trivial-rtc.yaml.
> V2 -> V3: Moved MAINTAINERS file changes to the driver patch
> V1 -> V2: Handled dt-bindings by trivial-rtc.yaml
>
> .../devicetree/bindings/rtc/nxp,pcf85053.yaml | 128 ++++++++++++++++++
> 1 file changed, 128 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml
>
> diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml
> new file mode 100644
> index 000000000000..6b1c97358486
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml
> @@ -0,0 +1,128 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2025 NXP
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/nxp,pcf85053.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP PCF85053 Real Time Clock
> +
> +maintainers:
> + - Pankit Garg <pankit.garg@nxp.com>
> + - Lakshay Piplani <lakshay.piplani@nxp.com>
> +
> +properties:
> + compatible:
> + enum:
> + - nxp,pcf85053
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + nxp,interface:
> + $ref: /schemas/types.yaml#/definitions/string
> + enum: [ primary, secondary ]
> + description: |
> + Identifies this host's logical role in a multi-host topology for the
> + PCF85053 RTC. The device exposes a "TWO" ownership bit in the CTRL
> + register that gates which host may write time/alarm registers.
> + - "primary": Designated host that *may* claim write ownership (set
> + CTRL.TWO=1) **if** write-access is explicitly requested.
> + - "secondary": Peer host that writes only when CTRL.TWO=0 (default).
> +
> + nxp,write-access:
> + type: boolean
> + description: |
> + Request the driver to claim write ownership at probe time by setting
> + CTRL.TWO=1. This property is only valid when nxp,interface="primary".
> + The driver will not modify any other CTRL bits (HF/DM/etc.) and will not
> + clear any status/interrupt flags at probe.
> +
> +required:
> + - compatible
> + - reg
> + - nxp,interface
> +
> +additionalProperties: false
> +
> +# Schema constraints matching driver:
> +# 1) If nxp,write-access is present, nxp,interface must be "primary".
> +# Rationale: only the primary may claim ownership; driver will set TWO=1.
> +# 2) If nxp,interface is "secondary", nxp,write-access must not be present.
> +# Rationale: secondary never claims ownership and cannot write CTRL/ST/alarm.
> +#
> +# Practical effect:
> +# - Primary without 'nxp,write-access'; primary is read only; secondary may
> +# write time registers.
> +# - Primary with 'nxp,write-access'; primary owns writes, secondary is read only.
> +allOf:
> + - $ref: rtc.yaml#
> + - oneOf:
> + # Case 1: primary with write-access
> + - required: [ "nxp,write-access" ]
> + properties:
> + nxp,interface:
> + const: primary
> +
> + # Case 2: primary without write-access
> + - properties:
> + nxp,interface:
> + const: primary
> + not:
> + required: [ "nxp,write-access" ]
Aren't case 1 and case 2 here redundant? All you need to do is block
interface == secondary when nxp,write-access is present, which your case
3 should be able to be modified to do via
if:
properties:
nxp,interface:
const: secondary
then:
properties:
nxp,write-access: false
I think your description for nxp,write-access gets the point across
about when it can be used, and the additional commentary is not really
helpful.
> +
> + # Case 3: secondary (must not have write-access)
> + - properties:
> + nxp,interface:
> + const: secondary
> + not:
> + required: [ "nxp,write-access" ]
> +
> +examples:
> + # Single host example.
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + rtc@6f {
> + compatible = "nxp,pcf85053";
> + reg = <0x6f>;
> + nxp,interface = "primary";
> + nxp,write-access;
> + interrupt-parent = <&gpio2>;
> + interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
> + };
> + };
> +
> + # Dual-host example: one primary that claims writes; one secondary that never claims writes.
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + i2c0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + rtc@6f {
> + compatible = "nxp,pcf85053";
> + reg = <0x6f>;
> + nxp,interface = "primary";
> + nxp,write-access;
> + interrupt-parent = <&gpio2>;
> + interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
> + };
> + };
> +
> + i2c1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + rtc@6f {
> + compatible = "nxp,pcf85053";
> + reg = <0x6f>;
> + nxp,interface = "secondary";
Maybe a silly question, but if you have a system that wants to have two
pairs of RTCs, how would you determine which primary a secondary belongs
to? I notice you have no link between these devices in dt so I am
curious. Would it be better to eschew nxp,interface and have a phandle
from the secondary to the primary?
I don't know anything about your use case or features, so maybe knowing
the relationship just is not relevant at all, or it can be determined at
runtime.
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox