* Re: [PATCH 1/3] Input: gpio_keys - fall back to platform_get_irq() for interrupt-only keys
From: Andy Shevchenko @ 2025-08-20 13:37 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Matti Vaittinen, Lee Jones, Arnd Bergmann, Bartosz Golaszewski,
Linus Walleij, linux-input, linux-kernel
In-Reply-To: <20250817224731.1911207-1-dmitry.torokhov@gmail.com>
On Sun, Aug 17, 2025 at 03:47:26PM -0700, Dmitry Torokhov wrote:
> To allow transitioning away from gpio-keys platform data attempt to
> retrieve IRQ for interrupt-only keys using platform_get_irq_optional()
> if interrupt is not specified in platform data.
...
> + irq = platform_get_irq_optional(pdev, idx);
> + if (irq < 0) {
> + error = irq;
> + return dev_err_probe(dev, error,
Assigning error is redundant.
> + "Unable to determine IRQ# for button #%d",
> + idx);
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 1/6] dt-bindings: mfd: twl: Add missing sub-nodes for TWL4030 & TWL603x
From: Jihed Chaibi @ 2025-08-20 13:15 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andreas Kemnade, linux-kernel, peter.ujfalusi, dmitry.torokhov,
robh, krzk+dt, lgirdwood, tiwai, conor+dt, lee, ukleinek, broonie,
gregkh, linus.walleij, brgl, aaro.koskinen, khilman, rogerq, tony,
linux-gpio, linux-input, devicetree, linux-pwm, linux-sound,
linux-usb, linux-omap, shuah
In-Reply-To: <e0bec141-6aef-475f-b997-60fdf8234b82@kernel.org>
On Wed, Aug 20, 2025 at 7:57 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 19/08/2025 22:31, Andreas Kemnade wrote:
> >>
> >>> + type: object
> >>> + $ref: /schemas/usb/ti,twlxxxx-usb.yaml#
> >>
> >> Are you sure your patchset is bsiectable? Apply this patch and test. You
> >> will see errors and you must fix these. Even after fixing you have
> >> strict dependencies so your cover letter must explain these (or merging
> >> constraints)...
> >>
> > what are the rules here regarding bisectability? non-existing files
>
> dt_binding_check.
>
>
> Best regards,
> Krzysztof
Hello Krzysztof and Andreas,
Thanks again for your feedback,
You were right that my series had strict dependencies. Testing again I
found that a
local 'dt_binding_check' -only including this patch- passed, but the
dependencies
(non-existent yaml files in $ref) caused errors during a full 'dtbs_check'.
I managed to fix those dtbs_check warnings/errors thanks to
'additionalProperties: true'
to break the hard dependencies on the other new YAML files, and by
adding optional
definitions for the 'clocks' and 'clock-names' properties, which I
found are used by several
OMAP dts files.
Please let me know what you think of this, and whether I should send the new,
corrected v4 of this MFD (single) patch.
I have already sent the independent v4 patches for the other
subsystems (ASoC, USB, etc.).
in order to reach a much better orthogonality and independence between
the patches.
Thanks again for your guidance.
Best regards,
Jihed
^ permalink raw reply
* [PATCH v2 2/2] HID: i2c-hid: elan: Add parade-tc3408 timing
From: Langyan Ye @ 2025-08-20 12:25 UTC (permalink / raw)
To: dmitry.torokhov, robh, krzk+dt, conor+dt, jikos, bentiss
Cc: dianders, linux-input, devicetree, linux-kernel, Langyan Ye
In-Reply-To: <20250820122520.3356738-1-yelangyan@huaqin.corp-partner.google.com>
Parade-tc3408 requires reset to pull down time greater than 10ms,
so the configuration post_power_delay_ms is 10, and the chipset
initial time is required to be greater than 300ms,
so the post_gpio_reset_on_delay_ms is set to 300.
Signed-off-by: Langyan Ye <yelangyan@huaqin.corp-partner.google.com>
---
drivers/hid/i2c-hid/i2c-hid-of-elan.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
index 3fcff6daa0d3..23826cb808b7 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
@@ -179,11 +179,19 @@ static const struct elan_i2c_hid_chip_data ilitek_ili2901_chip_data = {
.main_supply_name = "vcc33",
};
+static const struct elan_i2c_hid_chip_data parade_tc3408_chip_data = {
+ .post_power_delay_ms = 10,
+ .post_gpio_reset_on_delay_ms = 100,
+ .hid_descriptor_address = 0x0001,
+ .main_supply_name = "vcc33",
+};
+
static const struct of_device_id elan_i2c_hid_of_match[] = {
{ .compatible = "elan,ekth6915", .data = &elan_ekth6915_chip_data },
{ .compatible = "elan,ekth6a12nay", .data = &elan_ekth6a12nay_chip_data },
{ .compatible = "ilitek,ili9882t", .data = &ilitek_ili9882t_chip_data },
{ .compatible = "ilitek,ili2901", .data = &ilitek_ili2901_chip_data },
+ { .compatible = "parade,tc3408", .data = ¶de_tc3408_chip_data },
{ }
};
MODULE_DEVICE_TABLE(of, elan_i2c_hid_of_match);
--
2.34.1
^ permalink raw reply related
* [PATCH v2 1/2] dt-bindings: input: Add Parade TC3408 touchscreen controller
From: Langyan Ye @ 2025-08-20 12:25 UTC (permalink / raw)
To: dmitry.torokhov, robh, krzk+dt, conor+dt, jikos, bentiss
Cc: dianders, linux-input, devicetree, linux-kernel, Langyan Ye
In-Reply-To: <20250820122520.3356738-1-yelangyan@huaqin.corp-partner.google.com>
The tc3408 touch screen chip same as Elan eKTH6915 controller
has a reset gpio. The difference is that they have different
post_power_delay_ms.
Signed-off-by: Langyan Ye <yelangyan@huaqin.corp-partner.google.com>
---
.../bindings/input/parade,tc3408.yaml | 68 +++++++++++++++++++
1 file changed, 68 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/parade,tc3408.yaml
diff --git a/Documentation/devicetree/bindings/input/parade,tc3408.yaml b/Documentation/devicetree/bindings/input/parade,tc3408.yaml
new file mode 100644
index 000000000000..30ffefb96c68
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/parade,tc3408.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/parade,tc3408.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Parade TC3408 touchscreen controller
+
+maintainers:
+ - Langyan Ye <yelangyan@huaqin.corp-partner.google.com>
+
+description: |
+ Parade TC3408 is a touchscreen controller supporting the I2C-HID protocol.
+ It requires a reset GPIO and two power supplies (3.3V and 1.8V).
+
+allOf:
+ - $ref: /schemas/input/touchscreen/touchscreen.yaml#
+
+properties:
+ compatible:
+ const: parade,tc3408
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ vcc33-supply:
+ description: The 3.3V supply to the touchscreen.
+
+ vccio-supply:
+ description: The 1.8V supply to the touchscreen.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - reset-gpios
+ - vcc33-supply
+ - vccio-supply
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen: touchscreen@24 {
+ compatible = "parade,tc3408";
+ reg = <0x24>;
+
+ interrupt-parent = <&pio>;
+ interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
+
+ reset-gpios = <&pio 126 GPIO_ACTIVE_LOW>;
+ vcc33-supply = <&pp3300_tchscr_x>;
+ vccio-supply = <&pp1800_tchscr_report_disable>;
+ };
+ };
--
2.34.1
^ permalink raw reply related
* [PATCH v2 0/2] Add tc3408 bindings and timing
From: Langyan Ye @ 2025-08-20 12:25 UTC (permalink / raw)
To: dmitry.torokhov, robh, krzk+dt, conor+dt, jikos, bentiss
Cc: dianders, linux-input, devicetree, linux-kernel, Langyan Ye
The tc3408 touch screen chip same as Elan eKTH6915 controller
has a reset gpio. The difference is that they have different
post_power_delay_ms.
According to the Parade TC3408 datasheet, the reset pin requires a
pull-down duration longer than 10 ms, therefore post_power_delay_ms
is set to 10. In addition, the chipset requires an initialization
time greater than 300 ms after reset, so post_gpio_reset_on_delay_ms
is configured as 300.
Changes in v2:
- PATCH 1/2: Drop redundant "bindings for" from subject
- PATCH 1/2: Improve description (describe hardware instead of bindings)
- PATCH 1/2: Drop "panel: true" property
- PATCH 1/2: Drop redundant description for reset-gpios
- PATCH 1/2: Use unevaluatedProperties: false instead of additionalProperties
- Link to v1: https://lore.kernel.org/all/20250819034852.1230264-1-yelangyan@huaqin.corp-partner.google.com/
Langyan Ye (2):
dt-bindings: input: Add Parade TC3408 touchscreen controller
HID: i2c-hid: elan: Add parade-tc3408 timing
.../bindings/input/parade,tc3408.yaml | 68 +++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid-of-elan.c | 8 +++
2 files changed, 76 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/parade,tc3408.yaml
--
2.34.1
^ permalink raw reply
* Re: [PATCH] HID: logitech: Add ids for G PRO 2 LIGHTSPEED
From: Bastien Nocera @ 2025-08-20 8:43 UTC (permalink / raw)
To: Matt Coffin, linux-input
Cc: Jiri Kosina, Benjamin Tissoires, Filipe Laíns, linux-kernel
In-Reply-To: <20250820074951.1120397-1-mcoffin13@gmail.com>
On Wed, 2025-08-20 at 01:49 -0600, Matt Coffin wrote:
> Adds support for the G PRO 2 LIGHTSPEED Wireless via it's nano receiver
"its"
> or directly. This nano receiver appears to work identically to the 1_1
> receiver for the case I've verified, which is the battery status through
> lg-hidpp.
>
> The same appears to be the case wired, sharing much with the Pro X
> Superlight 2; differences seemed to lie in userland configuration rather
> than in interfaces used by hid_logitech_hidpp on the kernel side.
>
> I verified the sysfs interface for battery charge/discharge status, and
> capacity read to be working on my 910-007290 device (white).
>
> Signed-off-by: Matt Coffin <mcoffin13@gmail.com>
Otherwise LGTM
Reviewed-by: Bastien Nocera <hadess@hadess.net>
> ---
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-logitech-dj.c | 4 ++++
> drivers/hid/hid-logitech-hidpp.c | 2 ++
> 3 files changed, 7 insertions(+)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 5a1096283855..37dc42380373 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -907,6 +907,7 @@
> #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_2 0xc534
> #define
> USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1 0xc539
> #define
> USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1 0xc53f
> +#define
> USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_2 0xc543
> #define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_POWERPLAY 0xc53a
> #define USB_DEVICE_ID_LOGITECH_BOLT_RECEIVER 0xc548
> #define USB_DEVICE_ID_SPACETRAVELLER 0xc623
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-
> logitech-dj.c
> index 34fa71ceec2b..cce54dd9884a 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -1983,6 +1983,10 @@ static const struct hid_device_id
> logi_dj_receivers[] = {
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1)
> ,
> .driver_data = recvr_type_gaming_hidpp},
> + { /* Logitech lightspeed receiver (0xc543) */
> + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> + USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_2)
> ,
> + .driver_data = recvr_type_gaming_hidpp},
>
> { /* Logitech 27 MHz HID++ 1.0 receiver (0xc513) */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
> USB_DEVICE_ID_MX3000_RECEIVER),
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
> logitech-hidpp.c
> index 10a3bc5f931b..aaef405a717e 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4596,6 +4596,8 @@ static const struct hid_device_id
> hidpp_devices[] = {
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC094) },
> { /* Logitech G Pro X Superlight 2 Gaming Mouse over USB */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC09b) },
> + { /* Logitech G PRO 2 LIGHTSPEED Wireless Mouse over USB */
> + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xc09a) },
>
> { /* G935 Gaming Headset */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0a87),
^ permalink raw reply
* [PATCH] HID: logitech: Add ids for G PRO 2 LIGHTSPEED
From: Matt Coffin @ 2025-08-20 7:49 UTC (permalink / raw)
To: linux-input
Cc: Jiri Kosina, Benjamin Tissoires, Filipe Laíns,
Bastien Nocera, linux-kernel, Matt Coffin
Adds support for the G PRO 2 LIGHTSPEED Wireless via it's nano receiver
or directly. This nano receiver appears to work identically to the 1_1
receiver for the case I've verified, which is the battery status through
lg-hidpp.
The same appears to be the case wired, sharing much with the Pro X
Superlight 2; differences seemed to lie in userland configuration rather
than in interfaces used by hid_logitech_hidpp on the kernel side.
I verified the sysfs interface for battery charge/discharge status, and
capacity read to be working on my 910-007290 device (white).
Signed-off-by: Matt Coffin <mcoffin13@gmail.com>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-logitech-dj.c | 4 ++++
drivers/hid/hid-logitech-hidpp.c | 2 ++
3 files changed, 7 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5a1096283855..37dc42380373 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -907,6 +907,7 @@
#define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_2 0xc534
#define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1 0xc539
#define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1 0xc53f
+#define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_2 0xc543
#define USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_POWERPLAY 0xc53a
#define USB_DEVICE_ID_LOGITECH_BOLT_RECEIVER 0xc548
#define USB_DEVICE_ID_SPACETRAVELLER 0xc623
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 34fa71ceec2b..cce54dd9884a 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -1983,6 +1983,10 @@ static const struct hid_device_id logi_dj_receivers[] = {
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_1),
.driver_data = recvr_type_gaming_hidpp},
+ { /* Logitech lightspeed receiver (0xc543) */
+ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH,
+ USB_DEVICE_ID_LOGITECH_NANO_RECEIVER_LIGHTSPEED_1_2),
+ .driver_data = recvr_type_gaming_hidpp},
{ /* Logitech 27 MHz HID++ 1.0 receiver (0xc513) */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER),
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 10a3bc5f931b..aaef405a717e 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4596,6 +4596,8 @@ static const struct hid_device_id hidpp_devices[] = {
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC094) },
{ /* Logitech G Pro X Superlight 2 Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC09b) },
+ { /* Logitech G PRO 2 LIGHTSPEED Wireless Mouse over USB */
+ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xc09a) },
{ /* G935 Gaming Headset */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0a87),
--
2.50.1
^ permalink raw reply related
* Re: [PATCH 0/3] Powerbutton driver and powerdown request for TPS65224 PMIC
From: Michael Walle @ 2025-08-20 7:06 UTC (permalink / raw)
To: Jon Cormier
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Julien Panis, Dmitry Torokhov, devicetree, linux-kernel,
linux-input
In-Reply-To: <CADL8D3Y0hRgJ6P59YvUKYzYBADjCXr7kevHNUDoqZXT41Ka2Yw@mail.gmail.com>
Hi,
On Tue Aug 19, 2025 at 6:22 PM CEST, Jon Cormier wrote:
> On Tue, Aug 19, 2025 at 7:30 AM Michael Walle <mwalle@kernel.org> wrote:
>>
>> On Tue Aug 19, 2025 at 1:27 PM CEST, Michael Walle wrote:
>> > Hi Job,
>> >
>> > > The following patches were created to get the tps65224 PMIC
>> > > powerbutton driver and power off request working on the
>> > > MitySOM-AM62PX. The patches are as follows:
>> >
>> > Are there any news on this series? Do you plan to post a new
>> > version soon, or do you want me to take over? I'd like to get
>> > support for this into the kernel :)
>>
>> FWIW, jsava@criticallink.com is bouncing.
> Yeah sorry, Job has gone back to school, he was on an internship.
>
> I'm not entirely sure how best to move forward with this change.
> There have been several suggestions thrown out and I'm a little lost
> on what's best/easiest.
>
> If you want to take over and add us as co authors that would be
> greatly appreciated. I can get his personal email if that makes
> sense. I'm not sure how the kernel normally deals with short term
> emails (interns).
I've asked around and Krzysztof said I should replace the mail with
his new address - or if I can't find it, use the old one. So yes,
you could ask him for his new (or private) address and if he's fine
with having that mentioned in the commit message.
> Note we are currently using the driver as is, on an Android prototype
> and for some reason Android isn't able to detect the power button
> hold, and open the power menu. Single push works fine to turn off the
> screen. I haven't looked into it yet, to see if this is an Android
> issue or if we are using the POWER key events wrong. Just thought I'd
> mention it.
Thanks, FWIW I'm using it with (normal) linux and it's working fine.
I'll double check if the press and release event is actually working.
-michael
^ permalink raw reply
* Re: [PATCH v3 1/6] dt-bindings: mfd: twl: Add missing sub-nodes for TWL4030 & TWL603x
From: Krzysztof Kozlowski @ 2025-08-20 5:57 UTC (permalink / raw)
To: Andreas Kemnade
Cc: Jihed Chaibi, linux-kernel, peter.ujfalusi, dmitry.torokhov, robh,
krzk+dt, lgirdwood, tiwai, conor+dt, lee, ukleinek, broonie,
gregkh, linus.walleij, brgl, aaro.koskinen, khilman, rogerq, tony,
linux-gpio, linux-input, devicetree, linux-pwm, linux-sound,
linux-usb, linux-omap, shuah
In-Reply-To: <20250819223157.0b271c74@akair>
On 19/08/2025 22:31, Andreas Kemnade wrote:
>>
>>> + type: object
>>> + $ref: /schemas/usb/ti,twlxxxx-usb.yaml#
>>
>> Are you sure your patchset is bsiectable? Apply this patch and test. You
>> will see errors and you must fix these. Even after fixing you have
>> strict dependencies so your cover letter must explain these (or merging
>> constraints)...
>>
> what are the rules here regarding bisectability? non-existing files
dt_binding_check.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] HID: axff: add cleanup allocated struct axff_device heap
From: Jeongjun Park @ 2025-08-20 4:43 UTC (permalink / raw)
To: dtor; +Cc: aha310510, bentiss, jikos, linux-input, linux-kernel, x0r
In-Reply-To: <fv32i2ejsupm64mpadzsfudaeuzkavkazpmnn2e4ijqeobl4gs@porr2zpxn4aj>
Hello Dmitry,
Dmitry Torokhov wrote:
> Hi Jeongjun,
>
> On Tue, Aug 19, 2025 at 12:43:02AM +0900, Jeongjun Park wrote:
> > Currently, acrux hid driver allocates heap memory equal to
> > sizeof(struct axff_device) to support force feedback, but there's no code
> > to free this memory except when initialization fails. This causes the
> > allocated memory to not be freed even if the driver is detached.
> >
> > Therefore, to properly clean up and safely manage the allocated heap,
> > must be modified to use devm_kzalloc().
>
> You have not tested this, have you? The private data that is passed to
> input_ff_create_memless() is freed by ml_ff_destroy() which is invoked
> when input core calls input_ff_destroy() as part of input device
> teardown. Your change introduces double-free.
Oops, I checked again and realized my patch was a complete mess.
Thanks for letting me know!
Regards,
Jeongjun Park
^ permalink raw reply
* Re: [PATCH v4 2/2] platform/x86: asus-wmi: Refactor Ally suspend/resume
From: Jiri Slaby @ 2025-08-20 3:47 UTC (permalink / raw)
To: Luke Jones, linux-kernel
Cc: hdegoede, ilpo.jarvinen, platform-driver-x86, linux-input,
bentiss, jikos, mario.limonciello, lkml
In-Reply-To: <20250323023421.78012-3-luke@ljones.dev>
On 23. 03. 25, 3:34, Luke Jones wrote:
> From: "Luke D. Jones" <luke@ljones.dev>
>
> Adjust how the CSEE direct call hack is used.
>
> The results of months of testing combined with help from ASUS to
> determine the actual cause of suspend issues has resulted in this
> refactoring which immensely improves the reliability for devices which
> do not have the following minimum MCU FW version:
> - ROG Ally X: 313
> - ROG Ally 1: 319
...
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
...
> @@ -5010,6 +5060,10 @@ static int asus_wmi_probe(struct platform_device *pdev)
> return ret;
> }
>
> + ret = acpi_register_lps0_dev(&asus_ally_s2idle_dev_ops);
This likely causes crashes:
https://bugzilla.suse.com/show_bug.cgi?id=1246924
> asus-nb-wmi asus-nb-wmi: Detected ATK, not ASUSWMI, use DSTS
> list_add double add: new=ffffffffc2094460, prev=ffffffffafa5cd20,
next=ffffffffc2094460.
> ------------[ cut here ]------------
> kernel BUG at lib/list_debug.c:35!
...
> acpi_register_lps0_dev.cold (include/linux/list.h:88
include/linux/list.h:150 include/linux/list.h:169
drivers/acpi/x86/s2idle.c:663)
> asus_wmi_probe (drivers/platform/x86/asus-wmi.c:5045 (discriminator
1) drivers/platform/x86/asus-wmi.c:5089 (discriminator 1)) asus_wmi
> platform_probe (drivers/base/platform.c:1404)
> really_probe (drivers/base/dd.c:579)
It looks like acpi_register_lps0_dev() is called twice with those ops...
Should the above be registered only if it is this RC71 device?
thanks,
--
js
suse labs
^ permalink raw reply
* Re: [PATCH 2/8] dt-bindings: hwmon: add Apple System Management Controller hwmon schema
From: James Calligeros @ 2025-08-19 23:22 UTC (permalink / raw)
To: Rob Herring
Cc: Sven Peter, Janne Grunau, 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: <20250819201537.GA1223169-robh@kernel.org>
Hi Rob,
On Wed, Aug 20, 2025 at 6:15 AM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Aug 19, 2025 at 09:47:54PM +1000, James Calligeros wrote:
> > Apple Silicon devices integrate a vast array of sensors, monitoring
> > current, power, temperature, and voltage across almost every part of
> > the system. The sensors themselves are all connected to the System
> > Management Controller (SMC). The SMC firmware exposes the data
> > reported by these sensors via its standard FourCC-based key-value
> > API. The SMC is also responsible for monitoring and controlling any
> > fans connected to the system, exposing them in the same way.
> >
> > For reasons known only to Apple, each device exposes its sensors with
> > an almost totally unique set of keys. This is true even for devices
> > which share an SoC. An M1 Mac mini, for example, will report its core
> > temperatures on different keys to an M1 MacBook Pro. Worse still, the
> > SMC does not provide a way to enumerate the available keys at runtime,
> > nor do the keys follow any sort of reasonable or consistent naming
> > rules that could be used to deduce their purpose. We must therefore
> > know which keys are present on any given device, and which function
> > they serve, ahead of time.
>
> I'm confused because you say this, but then the .dtsi files are common.
The SMC exposes dozens of sensors, and figuring out which one is which
when all we have to go by are cryptic FourCCs is proving very time consuming.
This is made worse by the fact that even sensors which you'd think should
be consistent across devices with a given SoC are often not. For example,
the M1 Mac mini exposes the application core temperature sensors on different
keys to the M1 MacBook Pro. We have only included the minimal subset of
sensors/fans that we know are common to most devices to validate our approach
with and make the driver itself useful.
> > + patternProperties:
> > + "^fan-[A-Za-z0-9]{4}":
> > + type: object
> > + additionalProperties: false
> > + required:
> > + - apple,key-id
> > + properties:
> > + apple,key-id:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + pattern: "^[A-Za-z0-9]{4}"
> > + description: The SMC FourCC key of the desired fan. This is the
> > + main key, which reports the fan's current speed. Sould match
>
> typo
>
> > + the node's suffix, but doesn't have to.
>
> Why can't we require that they match? (Other than we can't express that
> in schema?)
I made this optional mostly because these subnode names are inconsequential.
It's the apple,key-id property that matters. If it makes more sense
to say that the node name and property 'must' match instead of 'should' then
there's no reason we can't do that. Another option is to just have a
numbered sequence, e.g. fan-01, temperature-01, etc.
> > + apple,fan-minimum:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + pattern: "^[A-Za-z0-9]{4}"
> > + description: The minimum speed the current fan can run at
>
> This is not the speed, but the identifier key to retrieve the min speed,
> right? That's not clear. It's a bit odd that everything is a key id, but
> one property has that in the name and the others don't. I don't have any
> better suggestion though...
Would it make sense to append '-key' to all of the optional fan properties
to make this clearer?
> > + apple,fan-maximum:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + pattern: "^[A-Za-z0-9]{4}"
> > + description: The maximum speed the current fan can run at
> > + apple,fan-target:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + pattern: "^[A-Za-z0-9]{4}"
> > + description: Writeable endpoint for setting desired fan speed
> > + apple,fan-mode:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + pattern: "^[A-Za-z0-9]{4}"
> > + description: Writeable endpoint to enable/disable manual fan
> > + control
> > + label:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + description: Human-readable name for the sensor
>
> Surely more than apple,key-id is required? How would it be useful with
> only that? You can know how many fans you have, but have no info or
> control over them?
The key specified in apple,key-id is the fan's current speed, which is the
only key strictly required to enumerate the presence of a fan in the system.
All of the other keys are optional information that are only really useful
when implementing manual fan control, which is itself optional as the platform
really expects the SMC firmware to have control over fan speeds at all times.
> Rob
Regards,
James
^ permalink raw reply
* [PATCH v4] input: dt-bindings: ti,twl4030-keypad: convert to DT schema
From: Jihed Chaibi @ 2025-08-19 22:28 UTC (permalink / raw)
To: dmitry.torokhov
Cc: krzk+dt, robh, conor+dt, peter.ujfalusi, devicetree, linux-input,
linux-kernel, shuah, jihed.chaibi.dev
Convert the legacy TXT binding for the TWL4030 keypad module
to the modern YAML DT schema format. This adds formal validation
and improves documentation by inheriting from the matrix-keymap schema.
Signed-off-by: Jihed Chaibi <jihed.chaibi.dev@gmail.com>
---
Changes in v4:
- No changes.
- This patch is split from larger series per maintainer feedback.
- v3 link:
https://lore.kernel.org/all/20250816021523.167049-1-jihed.chaibi.dev@gmail.com/
Changes in v3:
- Made 'linux,keymap' a required property as suggested by the reviewer.
Changes in v2:
- Simplified the description field by removing redundant '|'
as it does not affect formatting in this context.
---
.../bindings/input/ti,twl4030-keypad.yaml | 59 +++++++++++++++++++
.../bindings/input/twl4030-keypad.txt | 27 ---------
2 files changed, 59 insertions(+), 27 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/ti,twl4030-keypad.yaml
delete mode 100644 Documentation/devicetree/bindings/input/twl4030-keypad.txt
diff --git a/Documentation/devicetree/bindings/input/ti,twl4030-keypad.yaml b/Documentation/devicetree/bindings/input/ti,twl4030-keypad.yaml
new file mode 100644
index 000000000..c69aa7f5c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/ti,twl4030-keypad.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/ti,twl4030-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments TWL4030-family Keypad Controller
+
+maintainers:
+ - Peter Ujfalusi <peter.ujfalusi@gmail.com>
+
+description:
+ TWL4030's Keypad controller is used to interface a SoC with a matrix-type
+ keypad device. The keypad controller supports multiple row and column lines.
+ A key can be placed at each intersection of a unique row and a unique column.
+ The keypad controller can sense a key-press and key-release and report the
+ event using a interrupt to the cpu.
+
+allOf:
+ - $ref: matrix-keymap.yaml#
+
+properties:
+ compatible:
+ const: ti,twl4030-keypad
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - interrupts
+ - keypad,num-rows
+ - keypad,num-columns
+ - linux,keymap
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/input/input.h>
+
+ keypad {
+ compatible = "ti,twl4030-keypad";
+ interrupts = <1>;
+ keypad,num-rows = <8>;
+ keypad,num-columns = <8>;
+ linux,keymap = <
+ /* row 0 */
+ MATRIX_KEY(0, 0, KEY_1)
+ MATRIX_KEY(0, 1, KEY_2)
+ MATRIX_KEY(0, 2, KEY_3)
+
+ /* ...and so on for a full 8x8 matrix... */
+
+ /* row 7 */
+ MATRIX_KEY(7, 6, KEY_Y)
+ MATRIX_KEY(7, 7, KEY_Z)
+ >;
+ };
diff --git a/Documentation/devicetree/bindings/input/twl4030-keypad.txt b/Documentation/devicetree/bindings/input/twl4030-keypad.txt
deleted file mode 100644
index e4be2f76a..000000000
--- a/Documentation/devicetree/bindings/input/twl4030-keypad.txt
+++ /dev/null
@@ -1,27 +0,0 @@
-* TWL4030's Keypad Controller device tree bindings
-
-TWL4030's Keypad controller is used to interface a SoC with a matrix-type
-keypad device. The keypad controller supports multiple row and column lines.
-A key can be placed at each intersection of a unique row and a unique column.
-The keypad controller can sense a key-press and key-release and report the
-event using a interrupt to the cpu.
-
-This binding is based on the matrix-keymap binding with the following
-changes:
-
- * keypad,num-rows and keypad,num-columns are required.
-
-Required SoC Specific Properties:
-- compatible: should be one of the following
- - "ti,twl4030-keypad": For controllers compatible with twl4030 keypad
- controller.
-- interrupt: should be one of the following
- - <1>: For controllers compatible with twl4030 keypad controller.
-
-Example:
- twl_keypad: keypad {
- compatible = "ti,twl4030-keypad";
- interrupts = <1>;
- keypad,num-rows = <8>;
- keypad,num-columns = <8>;
- };
--
2.39.5
^ permalink raw reply related
* [PATCH] hid-asus: add support for missing PX series fn keys
From: Amit Chaudhari @ 2025-08-19 21:49 UTC (permalink / raw)
To: jikos, bentiss; +Cc: linux-input, Amit Chaudhari
Add support for missing hotkey keycodes affecting Asus PX13 and PX16 families so userspace can use them.
Signed-off-by: Amit Chaudhari <amitchaudhari@mac.com>
---
drivers/hid/hid-asus.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 4b45e31f0bab..a30627bd32a5 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -974,7 +974,10 @@ static int asus_input_mapping(struct hid_device *hdev,
case 0xc4: asus_map_key_clear(KEY_KBDILLUMUP); break;
case 0xc5: asus_map_key_clear(KEY_KBDILLUMDOWN); break;
case 0xc7: asus_map_key_clear(KEY_KBDILLUMTOGGLE); break;
+ case 0x4e: asus_map_key_clear(KEY_FN_ESC); break;
+ case 0x7e: asus_map_key_clear(KEY_EMOJI_PICKER); break;
+ case 0x8b: asus_map_key_clear(KEY_PROG1); break; /* ProArt Creator Hub key */
case 0x6b: asus_map_key_clear(KEY_F21); break; /* ASUS touchpad toggle */
case 0x38: asus_map_key_clear(KEY_PROG1); break; /* ROG key */
case 0xba: asus_map_key_clear(KEY_PROG2); break; /* Fn+C ASUS Splendid */
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v3 1/6] dt-bindings: mfd: twl: Add missing sub-nodes for TWL4030 & TWL603x
From: Andreas Kemnade @ 2025-08-19 20:31 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Jihed Chaibi, linux-kernel, peter.ujfalusi, dmitry.torokhov, robh,
krzk+dt, lgirdwood, tiwai, conor+dt, lee, ukleinek, broonie,
gregkh, linus.walleij, brgl, aaro.koskinen, khilman, rogerq, tony,
linux-gpio, linux-input, devicetree, linux-pwm, linux-sound,
linux-usb, linux-omap, shuah
In-Reply-To: <20250819-humongous-muscular-curassow-5accd5@kuoka>
Am Tue, 19 Aug 2025 10:13:39 +0200
schrieb Krzysztof Kozlowski <krzk@kernel.org>:
> On Sat, Aug 16, 2025 at 04:15:18AM +0200, Jihed Chaibi wrote:
> > Update the TI TWL family Device Tree binding to include additional
> > subnodes for TWL4030, TWL6030, and TWL6032 devices.
> >
> > The simple power and PWM bindings (ti,twl4030-power, ti,twl-pwm, and
> > ti,twl-pwmled) are now defined directly within this binding.
> >
> > Other child node definitions (audio, gpio, keypad, usb, etc.) are also
> > added to the schema. These additions fix 'unevaluated properties'
> > errors found during dtbs_check for boards like the omap3-beagle
> > and improve the binding's overall completeness.
> >
> > Signed-off-by: Jihed Chaibi <jihed.chaibi.dev@gmail.com>
> >
> > ---
> > Changes in v3:
> > - New patch to consolidate simple bindings (power, pwm) and add
> > definitions for all child nodes to fix dtbs_check validation
> > errors found in v2.
> > ---
> > .../devicetree/bindings/mfd/ti,twl.yaml | 191 ++++++++++++++++++
> > .../devicetree/bindings/mfd/twl4030-power.txt | 48 -----
> > .../devicetree/bindings/pwm/ti,twl-pwm.txt | 17 --
> > .../devicetree/bindings/pwm/ti,twl-pwmled.txt | 17 --
> > 4 files changed, 191 insertions(+), 82 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/mfd/twl4030-power.txt
> > delete mode 100644 Documentation/devicetree/bindings/pwm/ti,twl-pwm.txt
> > delete mode 100644 Documentation/devicetree/bindings/pwm/ti,twl-pwmled.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> > index f162ab60c..b0f1cb7b5 100644
> > --- a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> > @@ -76,6 +76,98 @@ allOf:
> > properties:
> > compatible:
> > const: ti,twl4030-wdt
> > +
> > + audio:
> > + type: object
> > + $ref: /schemas/sound/ti,twl4030-audio.yaml#
> > + unevaluatedProperties: false
> > +
> > + keypad:
> > + type: object
> > + $ref: /schemas/input/ti,twl4030-keypad.yaml#
> > + unevaluatedProperties: false
> > +
> > + pwm:
> > + type: object
> > + $ref: /schemas/pwm/pwm.yaml#
> > + unevaluatedProperties: false
> > + description: |
> > + TWL4030 series: PWMA and PWMB (connected to LEDA and LEDB terminals)
> > + properties:
> > + compatible:
> > + enum:
> > + - ti,twl4030-pwm
> > + '#pwm-cells':
> > + const: 2
> > + required:
> > + - compatible
> > + - '#pwm-cells'
> > +
> > + pwmled:
> > + type: object
> > + $ref: /schemas/pwm/pwm.yaml#
> > + unevaluatedProperties: false
> > + description: |
> > + TWL4030 series: PWMA and PWMB (connected to LEDA and LEDB terminals)
> > + properties:
> > + compatible:
> > + enum:
> > + - ti,twl4030-pwmled
> > + '#pwm-cells':
> > + const: 2
> > + required:
> > + - compatible
> > + - '#pwm-cells'
> > +
> > + 'twl4030-usb':
>
> No need for quotes.
>
> > + type: object
> > + $ref: /schemas/usb/ti,twlxxxx-usb.yaml#
>
> Are you sure your patchset is bsiectable? Apply this patch and test. You
> will see errors and you must fix these. Even after fixing you have
> strict dependencies so your cover letter must explain these (or merging
> constraints)...
>
what are the rules here regarding bisectability? non-existing files
in $ref are probably bad. Are then unveiled errors in dts also a
problem? I would not expect too much fixing effort needed here. I have
not run dtbs_check yet.
I have expected this would all go via Lee's usual immutable branches.
Regards,
Andreas
^ permalink raw reply
* Re: [PATCH 1/8] dt-bindings: rtc: Add Apple SMC RTC
From: Rob Herring (Arm) @ 2025-08-19 20:17 UTC (permalink / raw)
To: James Calligeros
Cc: linux-arm-kernel, asahi, Lee Jones, Alexandre Belloni,
Conor Dooley, Mark Kettenis, Janne Grunau, linux-input,
Krzysztof Kozlowski, Sven Peter, Dmitry Torokhov,
Alyssa Rosenzweig, linux-rtc, Guenter Roeck, linux-kernel,
Jean Delvare, linux-hwmon, devicetree, Neal Gompa
In-Reply-To: <20250819-macsmc-subdevs-v1-1-57df6c3e5f19@gmail.com>
On Tue, 19 Aug 2025 21:47:53 +1000, James Calligeros wrote:
> From: Sven Peter <sven@kernel.org>
>
> Apple Silicon Macs (M1, etc.) have an RTC that is part of the PMU IC,
> but most of the PMU functionality is abstracted out by the SMC.
> An additional RTC offset stored inside NVMEM is required to compute
> the current date/time.
>
> Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
> Reviewed-by: Neal Gompa <neal@gompa.dev>
> Signed-off-by: Sven Peter <sven@kernel.org>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
> .../bindings/mfd/apple,smc.yaml | 9 +++++++
> .../bindings/rtc/apple,smc-rtc.yaml | 35 +++++++++++++++++++++++++
> MAINTAINERS | 1 +
> 3 files changed, 45 insertions(+)
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 2/8] dt-bindings: hwmon: add Apple System Management Controller hwmon schema
From: Rob Herring @ 2025-08-19 20:15 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Janne Grunau, 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: <20250819-macsmc-subdevs-v1-2-57df6c3e5f19@gmail.com>
On Tue, Aug 19, 2025 at 09:47:54PM +1000, James Calligeros wrote:
> Apple Silicon devices integrate a vast array of sensors, monitoring
> current, power, temperature, and voltage across almost every part of
> the system. The sensors themselves are all connected to the System
> Management Controller (SMC). The SMC firmware exposes the data
> reported by these sensors via its standard FourCC-based key-value
> API. The SMC is also responsible for monitoring and controlling any
> fans connected to the system, exposing them in the same way.
>
> For reasons known only to Apple, each device exposes its sensors with
> an almost totally unique set of keys. This is true even for devices
> which share an SoC. An M1 Mac mini, for example, will report its core
> temperatures on different keys to an M1 MacBook Pro. Worse still, the
> SMC does not provide a way to enumerate the available keys at runtime,
> nor do the keys follow any sort of reasonable or consistent naming
> rules that could be used to deduce their purpose. We must therefore
> know which keys are present on any given device, and which function
> they serve, ahead of time.
I'm confused because you say this, but then the .dtsi files are common.
>
> Add a schema so that we can describe the available sensors for a given
> Apple Silicon device in the Devicetree.
>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
> .../bindings/hwmon/apple,smc-hwmon.yaml | 148 +++++++++++++++++++++++++
> .../bindings/mfd/apple,smc.yaml | 45 ++++++++
> 2 files changed, 193 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..3ebc0463be4e1ce54005418feaa87ec7254dab6e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
> @@ -0,0 +1,148 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/apple,smc-hwmon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple SMC Hardware Monitoring
> +
> +description:
> + Apple's System Management Controller (SMC) exposes a vast array of
> + hardware monitoring sensors, including temperature probes, current and
> + voltage sense, power meters, and fan speeds. It also provides endpoints
> + to manually control the speed of each fan individually. Each Apple
> + Silicon device exposes a different set of endpoints via SMC keys. This
> + is true even when two machines share an SoC. The CPU core temperature
> + sensor keys on an M1 Mac mini are different to those on an M1 MacBook
> + Pro, for example.
> +
> +maintainers:
> + - James Calligeros <jcalligeros99@gmail.com>
> +
> +properties:
> + compatible:
> + const: apple,smc-hwmon
> +
> + current:
I don't see any need to group these and I would remove the intermediate
node. We have an iterator to iterate over a matching node name prefix if
that was the reasoning.
> + description: SMC current sense endpoints
> + type: object
> + additionalProperties: false
blank line
> + patternProperties:
> + "^current-[A-Za-z0-9]{4}":
Missing a '$' anchor on the end.
> + type: object
> + additionalProperties: false
blank line.
> + required:
> + - apple,key-id
'required' goes after 'properties'. blank lines in between.
> + properties:
> + apple,key-id:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}"
> + description: The SMC FourCC key of the desired current sensor.
> + Should match the node's suffix, but doesn't have to.
blank line
> + label:
> + $ref: /schemas/types.yaml#/definitions/string
Already has a type, don't need to re-define it.
> + description: Human-readable name for the sensor
Instead of duplicating these properties, You can do it once under a
'$defs' key and then reference it here.
> +
> + fan:
> + description: SMC fan control endpoints. A fan is made up of five
> + SMC keys - the fan's current speed, its minimum speed, its maximum
> + speed, a writeable target speed, and a writeable mode. The SMC will
> + automatically manage system fans unless a 1 is written to the fan's
> + mode key.
> + type: object
> + additionalProperties: false
blank line. And so on...
> + patternProperties:
> + "^fan-[A-Za-z0-9]{4}":
> + type: object
> + additionalProperties: false
> + required:
> + - apple,key-id
> + properties:
> + apple,key-id:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}"
> + description: The SMC FourCC key of the desired fan. This is the
> + main key, which reports the fan's current speed. Sould match
typo
> + the node's suffix, but doesn't have to.
Why can't we require that they match? (Other than we can't express that
in schema?)
> + apple,fan-minimum:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}"
> + description: The minimum speed the current fan can run at
This is not the speed, but the identifier key to retrieve the min speed,
right? That's not clear. It's a bit odd that everything is a key id, but
one property has that in the name and the others don't. I don't have any
better suggestion though...
> + apple,fan-maximum:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}"
> + description: The maximum speed the current fan can run at
> + apple,fan-target:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}"
> + description: Writeable endpoint for setting desired fan speed
> + apple,fan-mode:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}"
> + description: Writeable endpoint to enable/disable manual fan
> + control
> + label:
> + $ref: /schemas/types.yaml#/definitions/string
> + description: Human-readable name for the sensor
Surely more than apple,key-id is required? How would it be useful with
only that? You can know how many fans you have, but have no info or
control over them?
Rob
^ permalink raw reply
* Re: [PATCH 0/3] Powerbutton driver and powerdown request for TPS65224 PMIC
From: Jon Cormier @ 2025-08-19 16:22 UTC (permalink / raw)
To: Michael Walle
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Julien Panis, Dmitry Torokhov, devicetree, linux-kernel,
linux-input
In-Reply-To: <DC6DBZ4HHGOF.11HI37DX56PH0@kernel.org>
On Tue, Aug 19, 2025 at 7:30 AM Michael Walle <mwalle@kernel.org> wrote:
>
> On Tue Aug 19, 2025 at 1:27 PM CEST, Michael Walle wrote:
> > Hi Job,
> >
> > > The following patches were created to get the tps65224 PMIC
> > > powerbutton driver and power off request working on the
> > > MitySOM-AM62PX. The patches are as follows:
> >
> > Are there any news on this series? Do you plan to post a new
> > version soon, or do you want me to take over? I'd like to get
> > support for this into the kernel :)
>
> FWIW, jsava@criticallink.com is bouncing.
Yeah sorry, Job has gone back to school, he was on an internship.
I'm not entirely sure how best to move forward with this change.
There have been several suggestions thrown out and I'm a little lost
on what's best/easiest.
If you want to take over and add us as co authors that would be
greatly appreciated. I can get his personal email if that makes
sense. I'm not sure how the kernel normally deals with short term
emails (interns).
Note we are currently using the driver as is, on an Android prototype
and for some reason Android isn't able to detect the power button
hold, and open the power menu. Single push works fine to turn off the
screen. I haven't looked into it yet, to see if this is an Android
issue or if we are using the POWER key events wrong. Just thought I'd
mention it.
--
Jonathan Cormier
Senior Software Engineer
Voice: 315.425.4045 x222
http://www.CriticalLink.com
6712 Brooklawn Parkway, Syracuse, NY 13211
^ permalink raw reply
* Re: [PATCH 4/8] hwmon: Add Apple Silicon SMC hwmon driver
From: Guenter Roeck @ 2025-08-19 16:02 UTC (permalink / raw)
To: James Calligeros, Sven Peter, Janne Grunau, Alyssa Rosenzweig,
Neal Gompa, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Alexandre Belloni, Jean Delvare, Dmitry Torokhov
Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input
In-Reply-To: <20250819-macsmc-subdevs-v1-4-57df6c3e5f19@gmail.com>
On 8/19/25 04:47, 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>
Checkpatch says:
total: 0 errors, 0 warnings, 35 checks, 904 lines checked
There are lots of alignment issues, making me stumble over them.
Some comments below, but this is not a complete review. The unnecessary
variable initializations, error messages displayed only to be ignored,
and similar problems make it all but impossible for me to really review
the actual code.
All of that is a perfect example for "I can't see the forest because of
all the trees". Please fix.
> ---
> MAINTAINERS | 2 +
> drivers/hwmon/Kconfig | 12 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/macsmc_hwmon.c | 858 +++++++++++++++++++++++++
> drivers/mfd/macsmc.c | 1 +
> 5 files changed, 874 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 029117b921ea97d1276f5496ea06a3f918929b20..2eb23522654dd050262eb06e077587030cc335aa 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2380,6 +2380,7 @@ F: Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
> F: Documentation/devicetree/bindings/dma/apple,admac.yaml
> F: Documentation/devicetree/bindings/gpio/apple,smc-gpio.yaml
> F: Documentation/devicetree/bindings/gpu/apple,agx.yaml
> +F: Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
> F: Documentation/devicetree/bindings/i2c/apple,i2c.yaml
> F: Documentation/devicetree/bindings/input/touchscreen/apple,z2-multitouch.yaml
> F: Documentation/devicetree/bindings/interrupt-controller/apple,*
> @@ -2407,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.
> +
> 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..543a1ab50fc3587cc88625ec703d92a7e7db0825
> --- /dev/null
> +++ b/drivers/hwmon/macsmc_hwmon.c
> @@ -0,0 +1,858 @@
> +// 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 */
> +
> +#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)
#define<space>NAME<tab>value
> +
> +static bool melt_my_mac;
> +module_param_unsafe(melt_my_mac, bool, 0644);
> +MODULE_PARM_DESC(
> + melt_my_mac,
> + "Override the SMC to set your own fan speeds on supported machines");
melt_my_mac ? Please consider a less catchy name.
> +
> +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 n_sensors;
> +};
> +
> +struct macsmc_hwmon_fans {
> + struct hwmon_channel_info channel_info;
> + struct macsmc_hwmon_fan *fans;
> + u32 n_fans;
> +};
> +
> +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;
> + 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.n_sensors)
> + return -EINVAL;
> + *str = hwmon->temp.sensors[channel].label;
> + break;
> + case hwmon_in:
> + if (channel >= hwmon->volt.n_sensors)
> + return -EINVAL;
> + *str = hwmon->volt.sensors[channel].label;
> + break;
> + case hwmon_curr:
> + if (channel >= hwmon->curr.n_sensors)
> + return -EINVAL;
> + *str = hwmon->curr.sensors[channel].label;
> + break;
> + case hwmon_power:
> + if (channel >= hwmon->power.n_sensors)
> + return -EINVAL;
> + *str = hwmon->power.sensors[channel].label;
> + break;
> + case hwmon_fan:
> + if (channel >= hwmon->fan.n_fans)
> + 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;
I am quiter sure that this doesn't compile on 32 bit builds.
> + } 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 = 0;
> +
> + switch (sensor->info.type_code) {
> + /* 32-bit IEEE 754 float */
> + case _SMC_KEY("flt "): {
> + 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 = 0, neg;
> +
> + val = abs(value);
> + neg = val != value;
> +
> + if (scale > 1) {
> + val <<= 32;
> + exp = 32;
> + val /= scale;
> + } 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);
> + int ret = 0;
> + long min = 0;
> + long max = 0;
> +
> + if (!melt_my_mac || hwmon->fan.fans[channel].mode.macsmc_key == 0)
> + return -EOPNOTSUPP;
> +
> + if ((channel >= hwmon->fan.n_fans) ||
> + !(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;
> + dev_info(
> + dev,
> + "Fan %d now under manual control! Set target speed to 0 for automatic control.\n",
> + channel + 1);
No such noise please. Make it debug if you want to keep such messages,
but sysfs attribute writes must not result in clogging the log.
> + }
> + return macsmc_hwmon_write_key(
> + hwmon->smc, &hwmon->fan.fans[channel].set, val, 1);
> + } else if (!val) {
> + if (hwmon->fan.fans[channel].manual) {
> + dev_info(dev, "Returning control of fan %d to SMC.\n",
> + channel + 1);
> + 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 {
> + dev_err(dev, "Requested fan speed %ld out of range [%ld, %ld]",
> + val, min, max);
> + 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 (channel >= hwmon->fan.n_fans)
> + return -EINVAL;
That is not a valid value for umode_t.
> +
> + if (melt_my_mac && 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 = 0;
> +
> + 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 = 0;
> +
> + 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");
> + return ret;
> + }
> +
> + ret = macsmc_hwmon_parse_key(dev, smc, sensor, key);
> + if (ret)
> + return ret;
> +
> + if (!of_property_read_string(sensor_node, "label", &label))
> + strscpy_pad(sensor->label, label, sizeof(sensor->label));
> + else
> + strscpy_pad(sensor->label, key, 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.
> + * Mix, 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;
> + const char *now;
> + const char *min;
> + const char *max;
> + const char *set;
> + const char *mode;
> + int ret = 0;
Unnecessary inititialization. This is not the only place. Drop.
> +
> + ret = of_property_read_string(fan_node, "apple,key-id", &now);
> + if (ret) {
> + dev_err(dev, "apple,key-id not found in fan node!");
> + return -EINVAL;
> + }
> +
> + ret = macsmc_hwmon_parse_key(dev, smc, &fan->now, now);
> + if (ret)
> + return ret;
> +
> + if (!of_property_read_string(fan_node, "label", &label))
> + strscpy_pad(fan->label, label, sizeof(fan->label));
> + else
> + strscpy_pad(fan->label, now, sizeof(fan->label));
> +
> + fan->attrs = HWMON_F_LABEL | HWMON_F_INPUT;
> +
> + ret = of_property_read_string(fan_node, "apple,fan-minimum", &min);
> + if (ret)
> + dev_warn(dev, "No minimum fan speed key for %s", fan->label);
> + else {
> + if (!macsmc_hwmon_parse_key(dev, smc, &fan->min, min))
> + fan->attrs |= HWMON_F_MIN;
Above the error from macsmc_hwmon_parse_key() results in an abort,
here the error is logged in the function and ignored.
Either it is an error or it isn't. Ignoring errors is not acceptable.
Dumping error messages and ignoring the error is even less acceptable.
> + }
> +
> + ret = of_property_read_string(fan_node, "apple,fan-maximum", &max);
> + if (ret)
> + dev_warn(dev, "No maximum fan speed key for %s", fan->label);
> + else {
> + if (!macsmc_hwmon_parse_key(dev, smc, &fan->max, max))
> + fan->attrs |= HWMON_F_MAX;
> + }
> +
> + ret = of_property_read_string(fan_node, "apple,fan-target", &set);
> + if (ret)
> + dev_warn(dev, "No target fan speed key for %s", fan->label);
> + else {
> + if (!macsmc_hwmon_parse_key(dev, smc, &fan->set, set))
> + fan->attrs |= HWMON_F_TARGET;
> + }
> +
> + ret = of_property_read_string(fan_node, "apple,fan-mode", &mode);
> + if (ret)
> + dev_warn(dev, "No fan mode key for %s", fan->label);
> + else {
> + 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 *group_node = NULL;
> +
> + for_each_child_of_node(hwmon_node, group_node) {
> + struct device_node *key_node = NULL;
> + struct macsmc_hwmon_sensors *sensor_group = NULL;
> + struct macsmc_hwmon_fans *fan_group = NULL;
> + u32 n_keys = 0;
> + int i = 0;
> +
> + n_keys = of_get_child_count(group_node);
> + if (!n_keys) {
> + dev_err(hwmon->dev, "No keys found in %s!\n",
> + group_node->name);
> + continue;
> + }
> +
> + if (strcmp(group_node->name, "temperature") == 0)
> + sensor_group = &hwmon->temp;
> + else if (strcmp(group_node->name, "voltage") == 0)
> + sensor_group = &hwmon->volt;
> + else if (strcmp(group_node->name, "current") == 0)
> + sensor_group = &hwmon->curr;
> + else if (strcmp(group_node->name, "power") == 0)
> + sensor_group = &hwmon->power;
> + else if (strcmp(group_node->name, "fan") == 0)
> + fan_group = &hwmon->fan;
> + else {
> + dev_err(hwmon->dev, "Invalid group node: %s",
> + group_node->name);
Same here and elsewhere: If it is an error, abort. If it isn't an error,
do not use dev_err().
> + continue;
> + }
> +
> + if (sensor_group) {
> + sensor_group->sensors = devm_kzalloc(
> + hwmon->dev,
> + sizeof(struct macsmc_hwmon_sensor) * n_keys,
> + GFP_KERNEL);
> + if (!sensor_group->sensors) {
> + of_node_put(group_node);
> + return -ENOMEM;
> + }
> +
> + for_each_child_of_node(group_node, key_node) {
> + if (!macsmc_hwmon_create_sensor(
> + hwmon->dev, hwmon->smc, key_node,
> + &sensor_group->sensors[i]))
> + i++;
> + }
> +
> + sensor_group->n_sensors = i;
> + if (!sensor_group->n_sensors) {
> + dev_err(hwmon->dev,
> + "No valid sensor keys found in %s\n",
> + group_node->name);
> + continue;
> + }
> + } else if (fan_group) {
> + fan_group->fans = devm_kzalloc(
> + hwmon->dev,
> + sizeof(struct macsmc_hwmon_fan) * n_keys,
> + GFP_KERNEL);
> +
> + if (!fan_group->fans) {
> + of_node_put(group_node);
> + return -ENOMEM;
> + }
> +
> + for_each_child_of_node(group_node, key_node) {
> + if (!macsmc_hwmon_create_fan(
> + hwmon->dev, hwmon->smc, key_node,
> + &fan_group->fans[i]))
> + i++;
> + }
> +
> + fan_group->n_fans = i;
> + if (!fan_group->n_fans) {
> + dev_err(hwmon->dev,
> + "No valid sensor fans found in %s\n",
> + group_node->name);
> + continue;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> +/* Create NULL-terminated config arrays */
> +static void macsmc_hwmon_populate_configs(u32 *configs, u32 num_keys, u32 flags)
> +{
> + int idx = 0;
Yet another obviously unnecessary initialization. I am not reporting all of those.
Again, the forest story: Such code only distracts from the real problems. Please
keep that in mind.
> +
> + for (idx = 0; idx < num_keys; idx++)
> + configs[idx] = flags;
> +
> + configs[idx + 1] = 0;
> +}
> +
> +static void macsmc_hwmon_populate_fan_configs(u32 *configs, u32 num_keys,
> + struct macsmc_hwmon_fans *fans)
> +{
> + int idx = 0;
> +
> + for (idx = 0; idx < num_keys; idx++)
> + configs[idx] = fans->fans[idx].attrs;
> +
> + configs[idx + 1] = 0;
When would configs[idx + 1] ever be != 0 ?
> +}
> +
> +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)
> +{
> + int i = 0;
> + struct hwmon_channel_info *channel_info;
> +
> + /* chip */
> + hwmon->channel_infos[i++] = macsmc_chip_channel_info;
> +
> + if (hwmon->temp.n_sensors) {
> + channel_info = &hwmon->temp.channel_info;
> + channel_info->type = hwmon_temp;
> + channel_info->config = devm_kzalloc(
> + hwmon->dev, sizeof(u32) * hwmon->temp.n_sensors + 1,
> + GFP_KERNEL);
Consider using devm_kcalloc() instead of re-implementing it.
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> + hwmon->temp.n_sensors,
> + (HWMON_T_INPUT | HWMON_T_LABEL));
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + if (hwmon->volt.n_sensors) {
> + channel_info = &hwmon->volt.channel_info;
> + channel_info->type = hwmon_in;
> + channel_info->config = devm_kzalloc(
> + hwmon->dev, sizeof(u32) * hwmon->volt.n_sensors + 1,
> + GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> + hwmon->volt.n_sensors,
> + (HWMON_I_INPUT | HWMON_I_LABEL));
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + if (hwmon->curr.n_sensors) {
> + channel_info = &hwmon->curr.channel_info;
> + channel_info->type = hwmon_curr;
> + channel_info->config = devm_kzalloc(
> + hwmon->dev, sizeof(u32) * hwmon->curr.n_sensors + 1,
> + GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> + hwmon->curr.n_sensors,
> + (HWMON_C_INPUT | HWMON_C_LABEL));
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + if (hwmon->power.n_sensors) {
> + channel_info = &hwmon->power.channel_info;
> + channel_info->type = hwmon_power;
> + channel_info->config = devm_kzalloc(
> + hwmon->dev, sizeof(u32) * hwmon->power.n_sensors + 1,
> + GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_configs((u32 *)channel_info->config,
> + hwmon->power.n_sensors,
> + (HWMON_P_INPUT | HWMON_P_LABEL));
> + hwmon->channel_infos[i++] = channel_info;
> + }
> +
> + if (hwmon->fan.n_fans) {
> + channel_info = &hwmon->fan.channel_info;
> + channel_info->type = hwmon_fan;
> + channel_info->config = devm_kzalloc(
> + hwmon->dev, sizeof(u32) * hwmon->fan.n_fans + 1,
> + GFP_KERNEL);
> + if (!channel_info->config)
> + return -ENOMEM;
> +
> + macsmc_hwmon_populate_fan_configs((u32 *)channel_info->config,
> + hwmon->fan.n_fans,
> + &hwmon->fan);
> + 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;
> + struct device_node *hwmon_node;
> + int ret = 0;
> +
> + hwmon = devm_kzalloc(&pdev->dev, sizeof(struct macsmc_hwmon),
> + GFP_KERNEL);
sizeof(*hwmon) is preferred in situations like this to ensure that
the variable types match.
> + if (!hwmon)
> + return -ENOMEM;
> +
> + hwmon->dev = &pdev->dev;
> + hwmon->smc = smc;
> +
> + hwmon_node = of_get_child_by_name(pdev->dev.parent->of_node, "hwmon");
> + if (!hwmon_node) {
> + dev_err(hwmon->dev, "SMC hwmon node not found in Devicetree\n");
> + return -ENODEV;
> + }
> +
> + ret = macsmc_hwmon_populate_sensors(hwmon, hwmon_node);
> + if (ret)
> + dev_info(hwmon->dev, "Could not populate keys!\n");
The only error returned from that function is -ENOMEM. Ignoring that is ok ?
Really ?
And then the function generates many dev_err() but ignores the actual errors.
Sorry, that is just not acceptable.
> +
> + of_node_put(hwmon_node);
> +
> + if (!hwmon->temp.n_sensors && !hwmon->volt.n_sensors &&
> + !hwmon->curr.n_sensors && !hwmon->power.n_sensors &&
> + !hwmon->fan.n_fans) {
> + 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,
> + "Temperature: %d, Voltage: %d, Current: %d, Power: %d, Fans: %d",
> + hwmon->temp.n_sensors, hwmon->volt.n_sensors,
> + hwmon->curr.n_sensors, hwmon->power.n_sensors,
> + hwmon->fan.n_fans);
> +
> + return 0;
> +}
> +
> +static struct platform_driver macsmc_hwmon_driver = {
> + .probe = macsmc_hwmon_probe,
> + .driver = {
> + .name = "macsmc_hwmon",
> + },
> +};
> +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");
> diff --git a/drivers/mfd/macsmc.c b/drivers/mfd/macsmc.c
> index 59be894460d33afa754927630881532b548b7ad8..bc4adf2fcfdce6c5ecbc51ced0e5985cbd36f54d 100644
> --- a/drivers/mfd/macsmc.c
> +++ b/drivers/mfd/macsmc.c
> @@ -48,6 +48,7 @@ static const struct mfd_cell apple_smc_devs[] = {
> MFD_CELL_OF("macsmc-gpio", NULL, NULL, 0, 0, "apple,smc-gpio"),
> MFD_CELL_OF("macsmc-reboot", NULL, NULL, 0, 0, "apple,smc-reboot"),
> MFD_CELL_OF("macsmc-rtc", NULL, NULL, 0, 0, "apple,smc-rtc"),
> + MFD_CELL_OF("macsmc_hwmon", NULL, NULL, 0, 0, "apple,smc-hwmon"),
> };
>
> static int apple_smc_cmd_locked(struct apple_smc *smc, u64 cmd, u64 arg,
>
^ permalink raw reply
* Re: [PATCH v3 1/6] dt-bindings: mfd: twl: Add missing sub-nodes for TWL4030 & TWL603x
From: Jihed Chaibi @ 2025-08-19 14:51 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-kernel, andreas, peter.ujfalusi, dmitry.torokhov, robh,
krzk+dt, lgirdwood, tiwai, conor+dt, lee, ukleinek, broonie,
gregkh, linus.walleij, brgl, aaro.koskinen, khilman, rogerq, tony,
linux-gpio, linux-input, devicetree, linux-pwm, linux-sound,
linux-usb, linux-omap, shuah
In-Reply-To: <20250819-humongous-muscular-curassow-5accd5@kuoka>
On Tue, Aug 19, 2025 at 10:13 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Sat, Aug 16, 2025 at 04:15:18AM +0200, Jihed Chaibi wrote:
> > Update the TI TWL family Device Tree binding to include additional
> > subnodes for TWL4030, TWL6030, and TWL6032 devices.
> >
> > The simple power and PWM bindings (ti,twl4030-power, ti,twl-pwm, and
> > ti,twl-pwmled) are now defined directly within this binding.
> >
> > Other child node definitions (audio, gpio, keypad, usb, etc.) are also
> > added to the schema. These additions fix 'unevaluated properties'
> > errors found during dtbs_check for boards like the omap3-beagle
> > and improve the binding's overall completeness.
> >
> > Signed-off-by: Jihed Chaibi <jihed.chaibi.dev@gmail.com>
> >
> > ---
> > Changes in v3:
> > - New patch to consolidate simple bindings (power, pwm) and add
> > definitions for all child nodes to fix dtbs_check validation
> > errors found in v2.
> > ---
> > .../devicetree/bindings/mfd/ti,twl.yaml | 191 ++++++++++++++++++
> > .../devicetree/bindings/mfd/twl4030-power.txt | 48 -----
> > .../devicetree/bindings/pwm/ti,twl-pwm.txt | 17 --
> > .../devicetree/bindings/pwm/ti,twl-pwmled.txt | 17 --
> > 4 files changed, 191 insertions(+), 82 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/mfd/twl4030-power.txt
> > delete mode 100644 Documentation/devicetree/bindings/pwm/ti,twl-pwm.txt
> > delete mode 100644 Documentation/devicetree/bindings/pwm/ti,twl-pwmled.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> > index f162ab60c..b0f1cb7b5 100644
> > --- a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> > @@ -76,6 +76,98 @@ allOf:
> > properties:
> > compatible:
> > const: ti,twl4030-wdt
> > +
> > + audio:
> > + type: object
> > + $ref: /schemas/sound/ti,twl4030-audio.yaml#
> > + unevaluatedProperties: false
> > +
> > + keypad:
> > + type: object
> > + $ref: /schemas/input/ti,twl4030-keypad.yaml#
> > + unevaluatedProperties: false
> > +
> > + pwm:
> > + type: object
> > + $ref: /schemas/pwm/pwm.yaml#
> > + unevaluatedProperties: false
> > + description: |
> > + TWL4030 series: PWMA and PWMB (connected to LEDA and LEDB terminals)
> > + properties:
> > + compatible:
> > + enum:
> > + - ti,twl4030-pwm
> > + '#pwm-cells':
> > + const: 2
> > + required:
> > + - compatible
> > + - '#pwm-cells'
> > +
> > + pwmled:
> > + type: object
> > + $ref: /schemas/pwm/pwm.yaml#
> > + unevaluatedProperties: false
> > + description: |
> > + TWL4030 series: PWMA and PWMB (connected to LEDA and LEDB terminals)
> > + properties:
> > + compatible:
> > + enum:
> > + - ti,twl4030-pwmled
> > + '#pwm-cells':
> > + const: 2
> > + required:
> > + - compatible
> > + - '#pwm-cells'
> > +
> > + 'twl4030-usb':
>
> No need for quotes.
>
> > + type: object
> > + $ref: /schemas/usb/ti,twlxxxx-usb.yaml#
>
> Are you sure your patchset is bsiectable? Apply this patch and test. You
> will see errors and you must fix these. Even after fixing you have
> strict dependencies so your cover letter must explain these (or merging
> constraints)...
>
> I suggest listing here only compatible with additionalProperties:true
> and splitting entire patchset per different maintainers.
Hi Krzysztof,
Thank you for the review. That additionalProperties
suggestion is definitely helpful in this scenario.
I will split (in v4) the patchset into smaller & independent
series for each subsystem as recommended.
Thanks again for your guidance.
Jihed
^ permalink raw reply
* Re: [PATCH 5/8] input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid
From: Lee Jones @ 2025-08-19 13:35 UTC (permalink / raw)
To: Sasha Finkelstein
Cc: James Calligeros, Sven Peter, Janne Grunau, Alyssa Rosenzweig,
Neal Gompa, 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: <CAMT+MTQvMnv4Zj3A8hskU1JW3zys0diKxWUzK5ScerxeSmgPjw@mail.gmail.com>
On Tue, 19 Aug 2025, Sasha Finkelstein wrote:
> On Tue, 19 Aug 2025 at 14:39, Lee Jones <lee@kernel.org> wrote:
> > Separate patch please.
> >
>
> Per the discussion in the thread linked from the cover letter, the dt
> maintainers have requested the bindings for all subdevices to be added
> together. Do you want a separate series with just the dt bindings and dts
> changes and the actual drivers in separate series or how do you prefer it?
I'm only concerned with the part I highlighted:
> drivers/mfd/macsmc.c | 1 +
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH 0/8] dt-bindings: Convert TWL4030/6040 family binding to DT schema
From: Jihed Chaibi @ 2025-08-19 13:19 UTC (permalink / raw)
To: Uwe Kleine-König, peter.ujfalusi
Cc: linux-kernel, dmitry.torokhov, robh, krzk+dt, lgirdwood, tiwai,
conor+dt, lee, broonie, gregkh, linux-input, devicetree,
linux-pwm, linux-sound, linux-usb, shuah
In-Reply-To: <564lqfhskoiivxziptyhrkajpblrrnk7nmfe73fzsd6ov2vwlh@n6ydqbrxh2my>
> > The following eight patches are included in this series, covering the
> > audio, keypad, power, PWM, and USB sub-modules.
>
> Thanks for doing that.
>
> I think the patches are all orthogonal and can/should be applied by the
> respective subsystem maintainers once they are considered ready?
>
Hello Uwe, Peter, and everyone,
Thank you for the feedback, I am now working on a new v4 patch series
for each subsystem with few improvements and will make sure of their
orthogonality.
While working on this, I had a question about the 'maintainers:' property for
the new YAML bindings. I see from the MAINTAINERS file that Peter Ujfalusi
is the maintainer for the TWL4030 codec driver. I initially marked him as
maintainer for every new twl4030 related binding.
For those new YAML binding files I am creating, should I list only the driver
maintainer (Peter, once he confirms this), or should I add myself as
a co-maintainer since I am authoring these new binding files?
Pinging Peter for his thoughts on this as well.
Thanks again for your guidance.
Jihed
^ permalink raw reply
* Re: [PATCH 5/8] input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid
From: Janne Grunau @ 2025-08-19 13:15 UTC (permalink / raw)
To: Sasha Finkelstein
Cc: Lee Jones, James Calligeros, Sven Peter, Alyssa Rosenzweig,
Neal Gompa, 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: <CAMT+MTQvMnv4Zj3A8hskU1JW3zys0diKxWUzK5ScerxeSmgPjw@mail.gmail.com>
On Tue, Aug 19, 2025 at 02:49:49PM +0200, Sasha Finkelstein wrote:
> On Tue, 19 Aug 2025 at 14:39, Lee Jones <lee@kernel.org> wrote:
> > Separate patch please.
> >
>
> Per the discussion in the thread linked from the cover letter, the dt
> maintainers have requested the bindings for all subdevices to be added
> together. Do you want a separate series with just the dt bindings and dts
> changes and the actual drivers in separate series or how do you prefer it?
I think it's asking for patches with the drivera and then separate
single line patches to wire up the drivers in drivers/mfd/macsmc.c
Janne
^ permalink raw reply
* Re: [PATCH 5/8] input: macsmc-hid: New driver to handle the Apple Mac SMC buttons/lid
From: Sasha Finkelstein @ 2025-08-19 12:49 UTC (permalink / raw)
To: Lee Jones
Cc: James Calligeros, Sven Peter, Janne Grunau, Alyssa Rosenzweig,
Neal Gompa, 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: <20250819123505.GC7508@google.com>
On Tue, 19 Aug 2025 at 14:39, Lee Jones <lee@kernel.org> wrote:
> Separate patch please.
>
Per the discussion in the thread linked from the cover letter, the dt
maintainers have requested the bindings for all subdevices to be added
together. Do you want a separate series with just the dt bindings and dts
changes and the actual drivers in separate series or how do you prefer it?
^ permalink raw reply
* Re: [PATCH 4/8] hwmon: Add Apple Silicon SMC hwmon driver
From: Lee Jones @ 2025-08-19 12:39 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
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: <20250819-macsmc-subdevs-v1-4-57df6c3e5f19@gmail.com>
On Tue, 19 Aug 2025, 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 | 2 +
> drivers/hwmon/Kconfig | 12 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/macsmc_hwmon.c | 858 +++++++++++++++++++++++++
> drivers/mfd/macsmc.c | 1 +
And here. And everywhere else.
> 5 files changed, 874 insertions(+)
--
Lee Jones [李琼斯]
^ 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