Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH] ARM: dts: imx53: enable PMIC RTC on imx53-qsrb
From: Shawn Guo @ 2025-10-27  5:50 UTC (permalink / raw)
  To: Alexander Kurz
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, devicetree,
	linux-input, linux-kernel
In-Reply-To: <20251011060251.13356-1-akurz@blala.de>

On Sat, Oct 11, 2025 at 06:02:51AM +0000, Alexander Kurz wrote:
> The RTC inside mc34708 is supported by RTC_DRV_MC13XXX since v3.6-rc1.
> Enable the PMIC RTC on the imx53-qsrb. Without a battery the RTC may be
> powered via the micro-USB connector when main 5V power is not available.
> 
> Signed-off-by: Alexander Kurz <akurz@blala.de>

Applied, thanks!


^ permalink raw reply

* Re: [PATCH] dt-bindings: Remove extra blank lines
From: Xu Yilun @ 2025-10-27  1:36 UTC (permalink / raw)
  To: Rob Herring (Arm)
  Cc: Krzysztof Kozlowski, Conor Dooley, Stephen Boyd, David Airlie,
	Simona Vetter, Maarten Lankhorst, Thomas Zimmermann,
	Andrzej Hajda, Robert Foss, Vinod Koul, Moritz Fischer, Xu Yilun,
	Bartosz Golaszewski, Guenter Roeck, Andi Shyti, Jonathan Cameron,
	Dmitry Torokhov, Georgi Djakov, Thomas Gleixner, Joerg Roedel,
	Jassi Brar, Mauro Carvalho Chehab, Lee Jones, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Johannes Berg, Krzysztof Wilczyński, Manivannan Sadhasivam,
	Bjorn Helgaas, Kishon Vijay Abraham I, Sebastian Reichel,
	Uwe Kleine-König, Mark Brown, Mathieu Poirier, Philipp Zabel,
	Olivia Mackall, Herbert Xu, Daniel Lezcano, Greg Kroah-Hartman,
	devicetree, linux-kernel, linux-clk, dri-devel, linux-fbdev,
	dmaengine, linux-fpga, linux-gpio, linux-hwmon, linux-i2c,
	linux-iio, linux-input, linux-pm, iommu, linux-media, linux-mtd,
	netdev, linux-wireless, linux-pci, linux-phy, linux-pwm,
	linux-remoteproc, linux-crypto, linux-sound, linux-usb
In-Reply-To: <20251023143957.2899600-1-robh@kernel.org>

On Thu, Oct 23, 2025 at 09:37:56AM -0500, Rob Herring (Arm) wrote:
> Generally at most 1 blank line is the standard style for DT schema
> files. Remove the few cases with more than 1 so that the yamllint check
> for this can be enabled.
> 
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>

[...]

>  Documentation/devicetree/bindings/fpga/fpga-region.yaml      | 5 -----

Acked-by: Xu Yilun <yilun.xu@intel.com>

^ permalink raw reply

* Re: [PATCH v4] HID: quirks: Change manufacturer for 4c4a:4155
From: Terry Junge @ 2025-10-26 19:58 UTC (permalink / raw)
  To: zhangheng, jikos, bentiss, staffan.melin
  Cc: linux-input, linux-kernel, 1114557, stable
In-Reply-To: <e765d91f-3c00-4dc5-ac24-68a5512a0c12@kylinos.cn>

for v4-0001-HID-quirks-Change-manufacturer-for-4c4a-4155.patch

Reviewed-by: Terry Junge <linuxhid@cosmicgizmosystems.com>

On 10/25/2025 8:34 PM, zhangheng wrote:

^ permalink raw reply

* Re: [PATCH] dt-bindings: Remove extra blank lines
From: Manivannan Sadhasivam @ 2025-10-26 15:33 UTC (permalink / raw)
  To: Rob Herring (Arm)
  Cc: Krzysztof Kozlowski, Conor Dooley, Stephen Boyd, David Airlie,
	Simona Vetter, Maarten Lankhorst, Thomas Zimmermann,
	Andrzej Hajda, Robert Foss, Vinod Koul, Moritz Fischer, Xu Yilun,
	Bartosz Golaszewski, Guenter Roeck, Andi Shyti, Jonathan Cameron,
	Dmitry Torokhov, Georgi Djakov, Thomas Gleixner, Joerg Roedel,
	Jassi Brar, Mauro Carvalho Chehab, Lee Jones, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Johannes Berg, Krzysztof Wilczyński, Bjorn Helgaas,
	Kishon Vijay Abraham I, Sebastian Reichel, Uwe Kleine-König,
	Mark Brown, Mathieu Poirier, Philipp Zabel, Olivia Mackall,
	Herbert Xu, Daniel Lezcano, Greg Kroah-Hartman, devicetree,
	linux-kernel, linux-clk, dri-devel, linux-fbdev, dmaengine,
	linux-fpga, linux-gpio, linux-hwmon, linux-i2c, linux-iio,
	linux-input, linux-pm, iommu, linux-media, linux-mtd, netdev,
	linux-wireless, linux-pci, linux-phy, linux-pwm, linux-remoteproc,
	linux-crypto, linux-sound, linux-usb
In-Reply-To: <20251023143957.2899600-1-robh@kernel.org>

On Thu, Oct 23, 2025 at 09:37:56AM -0500, Rob Herring (Arm) wrote:
> Generally at most 1 blank line is the standard style for DT schema
> files. Remove the few cases with more than 1 so that the yamllint check
> for this can be enabled.
> 
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
>  Documentation/devicetree/bindings/.yamllint                  | 2 +-
>  Documentation/devicetree/bindings/arm/psci.yaml              | 1 -
>  .../bindings/clock/allwinner,sun4i-a10-gates-clk.yaml        | 1 -
>  .../devicetree/bindings/clock/renesas,cpg-mssr.yaml          | 1 -
>  .../devicetree/bindings/clock/xlnx,clocking-wizard.yaml      | 1 -
>  .../display/allwinner,sun4i-a10-display-frontend.yaml        | 1 -
>  .../devicetree/bindings/display/allwinner,sun6i-a31-drc.yaml | 1 -
>  .../bindings/display/allwinner,sun8i-a83t-dw-hdmi.yaml       | 1 -
>  .../devicetree/bindings/display/amlogic,meson-vpu.yaml       | 1 -
>  .../devicetree/bindings/display/bridge/adi,adv7511.yaml      | 1 -
>  .../devicetree/bindings/display/bridge/lvds-codec.yaml       | 1 -
>  .../devicetree/bindings/display/bridge/toshiba,tc358767.yaml | 1 -
>  .../devicetree/bindings/display/ilitek,ili9486.yaml          | 1 -
>  Documentation/devicetree/bindings/display/msm/gpu.yaml       | 1 -
>  .../devicetree/bindings/display/panel/panel-timing.yaml      | 1 -
>  .../devicetree/bindings/display/panel/tpo,tpg110.yaml        | 1 -
>  .../devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml | 1 -
>  .../devicetree/bindings/display/simple-framebuffer.yaml      | 1 -
>  .../devicetree/bindings/dma/snps,dma-spear1340.yaml          | 1 -
>  Documentation/devicetree/bindings/dma/stericsson,dma40.yaml  | 1 -
>  .../devicetree/bindings/dma/stm32/st,stm32-dma.yaml          | 1 -
>  Documentation/devicetree/bindings/edac/apm,xgene-edac.yaml   | 1 -
>  .../devicetree/bindings/firmware/qemu,fw-cfg-mmio.yaml       | 1 -
>  Documentation/devicetree/bindings/fpga/fpga-region.yaml      | 5 -----
>  .../devicetree/bindings/gpio/brcm,xgs-iproc-gpio.yaml        | 1 -
>  .../devicetree/bindings/gpio/fairchild,74hc595.yaml          | 1 -
>  Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml     | 1 -
>  Documentation/devicetree/bindings/hwmon/adi,max31827.yaml    | 1 -
>  Documentation/devicetree/bindings/hwmon/national,lm90.yaml   | 1 -
>  Documentation/devicetree/bindings/hwmon/ti,tmp513.yaml       | 1 -
>  Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml     | 1 -
>  Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.yaml     | 1 -
>  .../devicetree/bindings/i2c/realtek,rtl9301-i2c.yaml         | 1 -
>  Documentation/devicetree/bindings/i2c/tsd,mule-i2c-mux.yaml  | 2 --
>  Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml    | 1 -
>  Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml    | 1 -
>  Documentation/devicetree/bindings/iio/adc/adi,ad7949.yaml    | 1 -
>  Documentation/devicetree/bindings/iio/adc/adi,ade9000.yaml   | 1 -
>  .../devicetree/bindings/iio/adc/cosmic,10001-adc.yaml        | 1 -
>  Documentation/devicetree/bindings/iio/adc/st,stm32-adc.yaml  | 1 -
>  .../devicetree/bindings/iio/adc/x-powers,axp209-adc.yaml     | 1 -
>  .../devicetree/bindings/iio/afe/voltage-divider.yaml         | 1 -
>  .../devicetree/bindings/iio/frequency/adi,admv4420.yaml      | 1 -
>  .../devicetree/bindings/iio/pressure/murata,zpa2326.yaml     | 1 -
>  .../devicetree/bindings/iio/proximity/semtech,sx9324.yaml    | 1 -
>  .../devicetree/bindings/iio/temperature/adi,ltc2983.yaml     | 1 -
>  Documentation/devicetree/bindings/input/ti,drv266x.yaml      | 1 -
>  .../devicetree/bindings/interconnect/qcom,rpmh.yaml          | 1 -
>  .../devicetree/bindings/interrupt-controller/arm,gic-v3.yaml | 1 -
>  .../bindings/interrupt-controller/aspeed,ast2700-intc.yaml   | 1 -
>  .../bindings/interrupt-controller/fsl,vf610-mscm-ir.yaml     | 1 -
>  .../bindings/interrupt-controller/loongson,liointc.yaml      | 1 -
>  .../bindings/interrupt-controller/mediatek,mtk-cirq.yaml     | 1 -
>  .../bindings/interrupt-controller/mscc,ocelot-icpu-intr.yaml | 1 -
>  Documentation/devicetree/bindings/iommu/arm,smmu.yaml        | 4 ----
>  Documentation/devicetree/bindings/mailbox/arm,mhu.yaml       | 1 -
>  Documentation/devicetree/bindings/mailbox/arm,mhuv2.yaml     | 1 -
>  Documentation/devicetree/bindings/mailbox/mtk,adsp-mbox.yaml | 1 -
>  Documentation/devicetree/bindings/media/amphion,vpu.yaml     | 1 -
>  Documentation/devicetree/bindings/media/i2c/adi,adv7604.yaml | 2 --
>  .../devicetree/bindings/media/i2c/techwell,tw9900.yaml       | 1 -
>  Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml   | 1 -
>  .../devicetree/bindings/media/qcom,sc8280xp-camss.yaml       | 1 -
>  .../bindings/media/samsung,exynos4212-fimc-is.yaml           | 1 -
>  .../devicetree/bindings/media/samsung,s5pv210-jpeg.yaml      | 1 -
>  Documentation/devicetree/bindings/media/st,stm32-dma2d.yaml  | 1 -
>  .../devicetree/bindings/media/video-interface-devices.yaml   | 4 ----
>  .../memory-controllers/qcom,ebi2-peripheral-props.yaml       | 1 -
>  Documentation/devicetree/bindings/mfd/stericsson,ab8500.yaml | 1 -
>  .../devicetree/bindings/mtd/amlogic,meson-nand.yaml          | 1 -
>  .../devicetree/bindings/mtd/marvell,nand-controller.yaml     | 1 -
>  Documentation/devicetree/bindings/mux/mux-controller.yaml    | 1 -
>  .../devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml   | 2 --
>  Documentation/devicetree/bindings/net/brcm,bcmgenet.yaml     | 1 -
>  .../devicetree/bindings/net/brcm,mdio-mux-iproc.yaml         | 1 -
>  .../devicetree/bindings/net/cortina,gemini-ethernet.yaml     | 1 -
>  Documentation/devicetree/bindings/net/fsl,gianfar.yaml       | 2 --
>  .../devicetree/bindings/net/mdio-mux-multiplexer.yaml        | 1 -
>  Documentation/devicetree/bindings/net/qcom,ipa.yaml          | 1 -
>  Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml    | 1 -
>  .../devicetree/bindings/net/wireless/ti,wlcore.yaml          | 1 -
>  .../devicetree/bindings/pci/altr,pcie-root-port.yaml         | 1 -
>  Documentation/devicetree/bindings/pci/loongson.yaml          | 1 -
>  Documentation/devicetree/bindings/pci/rockchip-dw-pcie.yaml  | 1 -
>  .../devicetree/bindings/pci/starfive,jh7110-pcie.yaml        | 1 -


Acked-by: Manivannan Sadhasivam <mani@kernel.org> # For PCI controller bindings

- Mani

-- 
மணிவண்ணன் சதாசிவம்

^ permalink raw reply

* Re: [PATCH v4] HID: quirks: Change manufacturer for 4c4a:4155
From: Staffan Melin @ 2025-10-26 10:45 UTC (permalink / raw)
  To: Terry Junge, jikos, bentiss, zhangheng
  Cc: linux-input, linux-kernel, 1114557, stable
In-Reply-To: <e765d91f-3c00-4dc5-ac24-68a5512a0c12@kylinos.cn>

Thank you,
 
this patch also works fine on my GPD DUO.
 
Tested-by: Staffan Melin, staffan.melin@oscillator.se
 
Best regards,
 
Staffan Melin
 
"zhangheng" zhangheng@kylinos.cn – 26 oktober 2025 kl. 04:35
> 
> 
>

^ permalink raw reply

* [PATCH v4] HID: quirks: Change manufacturer for 4c4a:4155
From: zhangheng @ 2025-10-26  3:34 UTC (permalink / raw)
  To: Terry Junge, jikos, bentiss, staffan.melin
  Cc: linux-input, linux-kernel, 1114557, stable
In-Reply-To: <c7aab08b-52fa-41ef-a7fb-118298bb93aa@cosmicgizmosystems.com>




[-- Attachment #2: v4-0001-HID-quirks-Change-manufacturer-for-4c4a-4155.patch --]
[-- Type: text/plain, Size: 2837 bytes --]

From 01be0e31b4fd991f955f77ec06d0c8b7a6d0fb13 Mon Sep 17 00:00:00 2001
From: Zhang Heng <zhangheng@kylinos.cn>
Date: Fri, 12 Sep 2025 20:38:18 +0800
Subject: [PATCH v4] HID: quirks: Change manufacturer for 4c4a:4155

Based on available evidence, the USB ID 4c4a:4155 used by multiple
devices has been attributed to Jieli. The commit 1a8953f4f774
("HID: Add IGNORE quirk for SMARTLINKTECHNOLOGY") affected touchscreen
functionality. Added checks for manufacturer and serial number to
maintain microphone compatibility, enabling both devices to function
properly.

Fixes: 1a8953f4f774 ("HID: Add IGNORE quirk for SMARTLINKTECHNOLOGY")
Cc: stable@vger.kernel.org
Tested-by: staffan.melin@oscillator.se
Signed-off-by: Zhang Heng <zhangheng@kylinos.cn>
---
Changes in v4:
-- add serial number. Since I saw JIELI using identical IDs for different
devices with distinct serial numbers, I worry SMARTLINKTECHNOLOGY might
reuse this ID for touchscreens. Adding serial number verification provides
extra insurance.
 drivers/hid/hid-ids.h    |  4 ++--
 drivers/hid/hid-quirks.c | 13 ++++++++++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 66f05d02cfca..5e6e87487205 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1546,7 +1546,7 @@
 #define USB_VENDOR_ID_SIGNOTEC			0x2133
 #define USB_DEVICE_ID_SIGNOTEC_VIEWSONIC_PD1011	0x0018
 
-#define USB_VENDOR_ID_SMARTLINKTECHNOLOGY              0x4c4a
-#define USB_DEVICE_ID_SMARTLINKTECHNOLOGY_4155         0x4155
+#define USB_VENDOR_ID_JIELI_SDK_DEFAULT		0x4c4a
+#define USB_DEVICE_ID_JIELI_SDK_4155		0x4155
 
 #endif
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index bcd4bccf1a7c..22760ac50f2d 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -915,7 +915,6 @@ static const struct hid_device_id hid_ignore_list[] = {
 #endif
 	{ HID_USB_DEVICE(USB_VENDOR_ID_YEALINK, USB_DEVICE_ID_YEALINK_P1K_P4K_B2K) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_HP_5MP_CAMERA_5473) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_SMARTLINKTECHNOLOGY, USB_DEVICE_ID_SMARTLINKTECHNOLOGY_4155) },
 	{ }
 };
 
@@ -1064,6 +1063,18 @@ bool hid_ignore(struct hid_device *hdev)
 					     strlen(elan_acpi_id[i].id)))
 					return true;
 		break;
+	case USB_VENDOR_ID_JIELI_SDK_DEFAULT:
+		/*
+		 * Multiple USB devices with identical IDs (mic & touchscreen).
+		 * The touch screen requires hid core processing, but the
+		 * microphone does not. They can be distinguished by manufacturer
+		 * and serial number.
+		 */
+		if (hdev->product == USB_DEVICE_ID_JIELI_SDK_4155 &&
+		    strncmp(hdev->name, "SmartlinkTechnology", 19) == 0 &&
+		    strncmp(hdev->uniq, "20201111000001", 14) == 0)
+			return true;
+		break;
 	}
 
 	if (hdev->type == HID_TYPE_USBMOUSE &&
-- 
2.47.1


^ permalink raw reply related

* [PATCH AUTOSEL 6.17-6.6] HID: asus: add Z13 folio to generic group for multitouch to work
From: Sasha Levin @ 2025-10-25 16:01 UTC (permalink / raw)
  To: patches, stable
  Cc: Antheas Kapenekakis, Luke D. Jones, Jiri Kosina, Sasha Levin,
	jikos, bentiss, linux-input
In-Reply-To: <20251025160905.3857885-1-sashal@kernel.org>

From: Antheas Kapenekakis <lkml@antheas.dev>

[ Upstream commit b595974b4afe0e171dd707da570964ff642742e3 ]

The Asus Z13 folio has a multitouch touchpad that needs to bind
to the hid-multitouch driver in order to work properly. So bind
it to the HID_GROUP_GENERIC group to release the touchpad and
move it to the bottom so that the comment applies to it.

While at it, change the generic KEYBOARD3 name to Z13_FOLIO.

Reviewed-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Based on my comprehensive analysis, here is my recommendation:

## Backport Status: **YES**

### Summary of Analysis

This commit fixes a hardware functionality bug where the multitouch
touchpad on the ASUS ROG Z13 folio does not work. The fix is a textbook
example of a stable tree backport candidate.

### Technical Understanding

**What Changed:**
1. **drivers/hid/hid-asus.c (lines 1387-1421)**: The device entry for
   Z13 folio (device ID 0x1a30) was moved from the middle of the device
   table to the bottom, and crucially changed from:
  ```c
  HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
  USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3)
  ```
  to:
  ```c
  HID_DEVICE(BUS_USB, HID_GROUP_GENERIC, USB_VENDOR_ID_ASUSTEK,
  USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO)
  ```

2. **drivers/hid/hid-ids.h (line 226)**: Renamed the constant from
   `USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3` to the more descriptive
   `USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO`

**Why This Matters:**
- The Z13 folio is a composite USB device with both a keyboard and
  multitouch touchpad
- `HID_USB_DEVICE` expands to `.bus = BUS_USB` without specifying a
  group, so it matches **all HID groups**
- `HID_DEVICE(BUS_USB, HID_GROUP_GENERIC, ...)` explicitly restricts
  binding to only `HID_GROUP_GENERIC`
- The multitouch touchpad presents itself as `HID_GROUP_MULTITOUCH` or
  `HID_GROUP_MULTITOUCH_WIN_8`
- When hid-asus binds to all groups, it prevents hid-multitouch from
  binding to the touchpad
- By restricting hid-asus to `HID_GROUP_GENERIC` (keyboard), hid-
  multitouch can properly handle the touchpad

### Evidence of Correctness

1. **Established Pattern**: This exact fix was applied to the T101HA
   keyboard in 2021 (commit a94f66aecdaa4). The commit message for that
   fix explicitly states: *"The hid-multitouch hiddev has a group of
   HID_GROUP_MULTITOUCH_WIN_8, so the same result can be achieved by
   making the hid_device_id entry for the dock in the asus_devices[]
   table only match on HID_GROUP_GENERIC"*

2. **Wide Usage**: Verified that `HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
   ...)` pattern is used extensively in:
   - hid-asus.c (T101HA at drivers/hid/hid-asus.c:1422)
   - hid-google-hammer.c (9+ instances)
   - hid-ite.c (4+ instances)
   - hid-lenovo.c (5+ instances)

3. **Device History**: The Z13 keyboard was originally added in v6.5
   (commit 74e47b2c52ed4) using HID_USB_DEVICE, which inadvertently
   prevented the touchpad from working. This commit corrects that
   oversight.

4. **Author Credibility**: Antheas Kapenekakis has 15+ kernel
   contributions focused on platform/x86 and HID subsystems, including
   other Z13 fixes. The patch was reviewed by Luke D. Jones, the
   original author of Z13 support.

5. **Comment Alignment**: The commit moves the entry below an existing
   comment (lines 1415-1418) that explicitly explains: *"Note bind to
   the HID_GROUP_GENERIC group, so that we only bind to the keyboard
   part, while letting hid-multitouch.c handle the touchpad."* This
   comment now applies to both T101HA and Z13 folio.

### Risk Analysis

**Minimal Risk:**
- **Isolated Scope**: Only affects ASUS device ID 0x1a30 (Z13 folio)
- **No Dependencies**: Standalone fix with no prerequisites
- **No Architectural Changes**: Uses existing, proven pattern
- **Verified Pattern**: Multiple similar devices use this approach since
  2021 without issues
- **Clean Rename**: Old constant name completely removed, no lingering
  references found

**Regression Potential:**
- Extremely low - the change makes a non-working touchpad work
- Users without this fix have a broken touchpad; users with it have
  working touchpad
- The keyboard functionality is unaffected (same quirks applied)

### Stable Tree Criteria Assessment

✅ **Fixes important bug**: Multitouch touchpad completely non-functional
✅ **Small and contained**: 2 files changed, device ID rename + entry
repositioning
✅ **No architectural changes**: Uses established HID group filtering
pattern
✅ **Minimal regression risk**: Proven pattern, isolated to one device
✅ **Confined to subsystem**: HID subsystem only
✅ **Applies to stable**: Any stable kernel >= 6.5 (where Z13 support
exists)

**Note**: While the commit lacks a `Cc: stable@vger.kernel.org` tag or
`Fixes:` tag, it clearly meets stable tree criteria and should be
backported to all stable kernels that contain the original Z13 support
(6.5+).

### Conclusion

This is an **unambiguous YES** for backporting. The commit:
- Fixes real hardware (touchpad doesn't work without it)
- Uses a well-tested, 4+ year old pattern
- Has minimal risk and isolated impact
- Comes from a credible contributor
- Was reviewed by the original device maintainer

This should be backported to all stable trees from 6.5 onwards where the
Z13 folio device support exists.

 drivers/hid/hid-asus.c | 6 +++---
 drivers/hid/hid-ids.h  | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 8db9d4e7c3b0b..a444d41e53b6c 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1387,9 +1387,6 @@ static const struct hid_device_id asus_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
 	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
-	    USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3),
-	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 	    USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
 	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
@@ -1419,6 +1416,9 @@ static const struct hid_device_id asus_devices[] = {
 	 * Note bind to the HID_GROUP_GENERIC group, so that we only bind to the keyboard
 	 * part, while letting hid-multitouch.c handle the touchpad.
 	 */
+	{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
+		USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO),
+	  QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
 	{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
 		USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_T101HA_KEYBOARD) },
 	{ }
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index ded5348d190c5..5721b8414bbdf 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -223,7 +223,7 @@
 #define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD3 0x1822
 #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD	0x1866
 #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2	0x19b6
-#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3	0x1a30
+#define USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO		0x1a30
 #define USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR		0x18c6
 #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY		0x1abe
 #define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X		0x1b4c
-- 
2.51.0


^ permalink raw reply related

* [PATCH AUTOSEL 6.17-6.12] HID: pidff: Use direction fix only for conditional effects
From: Sasha Levin @ 2025-10-25 15:56 UTC (permalink / raw)
  To: patches, stable
  Cc: Tomasz Pakuła, Oleg Makarenko, Jiri Kosina, Sasha Levin,
	jikos, bentiss, linux-input, linux-usb
In-Reply-To: <20251025160905.3857885-1-sashal@kernel.org>

From: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>

[ Upstream commit f345a4798dab800159b09d088e7bdae0f16076c3 ]

The already fixed bug in SDL only affected conditional effects. This
should fix FFB in Forza Horizion 4/5 on Moza Devices as Forza Horizon
flips the constant force direction instead of using negative magnitude
values.

Changing the direction in the effect directly in pidff_upload_effect()
would affect it's value in further operations like comparing to the old
effect and/or just reading the effect values in the user application.

This, in turn, would lead to constant PID_SET_EFFECT spam as the effect
direction would constantly not match the value that's set by the
application.

This way, it's still transparent to any software/API.

Only affects conditional effects now so it's better for it to explicitly
state that in the name. If any HW ever needs fixed direction for other
effects, we'll add more quirks.

Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
Reviewed-by: Oleg Makarenko <oleg@makarenk.ooo>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

- What it fixes
  - The earlier quirk forced a fixed direction (0x4000) for all effect
    types on specific wheelbases to work around mis-set PID directions
    in some user stacks (dinput → wine → SDL). That broad forcing breaks
    games that legitimately use the direction field for non-conditional
    effects (e.g., Forza Horizon 4/5 uses direction flips for constant
    force instead of negative magnitudes). This patch narrows the quirk
    to conditional effects only (spring, damper, inertia, friction),
    matching where the SDL-side bug actually applied.

- How it changes behavior
  - Adds a helper to detect conditional effects and a wrapper to set the
    direction only when appropriate:
    - New logic: pidff_is_effect_conditional() and
      pidff_set_effect_direction() in drivers/hid/usbhid/hid-pidff.c
      (replaces unconditional forcing in Set Effect).
    - Set Effect now calls the helper instead of unconditionally forcing
      direction for all effects.
  - Renames the quirk to reflect scope:
    HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION →
    HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION, still using the same bit
    (BIT(3)).
  - Updates device ID table entries to use the new quirk name for Moza
    devices in drivers/hid/hid-universal-pidff.c.

- Why it’s appropriate for stable
  - User-visible bug/regression: Fixes incorrect or missing FFB in Forza
    Horizon 4/5 on Moza devices when the kernel overrode constant-force
    directions. The message explicitly states this resolves that case.
  - Small and contained: Limited to HID PID force-feedback code and
    quirk tables; no architectural changes.
  - Minimal risk: Only affects devices already marked with the quirk
    bit. Behavior is narrowed (less intrusive) by applying the fixed
    direction only to conditional effects; other effect types now honor
    the application-provided direction as intended.
  - No side effects on unrelated subsystems: Touches only HID FFB code
    paths.
  - Clear lineage: It logically corrects the earlier
    “FIX_WHEEL_DIRECTION” quirk (drivers/hid/usbhid/hid-pidff.c:397)
    that forced direction for all effect types.

- Specific code references
  - Current unconditional forcing (to be replaced by helper call):
    drivers/hid/usbhid/hid-pidff.c:397
  - Quirk bit definition that’s renamed but remains BIT(3):
    drivers/hid/usbhid/hid-pidff.h:20
  - Fixed direction constant (still used, but now applied
    conditionally): drivers/hid/usbhid/hid-pidff.c:151
  - Device entries using the quirk (updated to new name):
    drivers/hid/hid-universal-pidff.c:6

- Additional considerations
  - The commit avoids mutating effect->direction in
    pidff_upload_effect(), preventing mismatches with the application’s
    state and avoiding needless PID_SET_EFFECT churn. It keeps behavior
    transparent to user space.
  - If any future hardware requires fixed direction for non-conditional
    effects, the commit message notes that more targeted quirks will be
    added, which further limits regression risk.

Given it’s a targeted regression fix for a real-world breakage, small in
scope, and reduces the quirk’s blast radius, it’s a strong candidate for
stable backport (especially on branches that already carry the earlier
FIX_WHEEL_DIRECTION quirk).

 drivers/hid/hid-universal-pidff.c | 20 ++++++++++----------
 drivers/hid/usbhid/hid-pidff.c    | 28 +++++++++++++++++++++++-----
 drivers/hid/usbhid/hid-pidff.h    |  2 +-
 3 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/drivers/hid/hid-universal-pidff.c b/drivers/hid/hid-universal-pidff.c
index 554a6559aeb73..70fce0f88e825 100644
--- a/drivers/hid/hid-universal-pidff.c
+++ b/drivers/hid/hid-universal-pidff.c
@@ -144,25 +144,25 @@ static int universal_pidff_input_configured(struct hid_device *hdev,
 
 static const struct hid_device_id universal_pidff_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R3),
-		.driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+		.driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R3_2),
-		.driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+		.driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R5),
-		.driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+		.driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R5_2),
-		.driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+		.driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R9),
-		.driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+		.driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R9_2),
-		.driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+		.driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R12),
-		.driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+		.driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R12_2),
-		.driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+		.driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R16_R21),
-		.driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+		.driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R16_R21_2),
-		.driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+		.driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CAMMUS, USB_DEVICE_ID_CAMMUS_C5) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_CAMMUS, USB_DEVICE_ID_CAMMUS_C12) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_VRS, USB_DEVICE_ID_VRS_DFP),
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 614a20b620231..c6b4f61e535d5 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -205,6 +205,14 @@ struct pidff_device {
 	u8 effect_count;
 };
 
+static int pidff_is_effect_conditional(struct ff_effect *effect)
+{
+	return effect->type == FF_SPRING  ||
+	       effect->type == FF_DAMPER  ||
+	       effect->type == FF_INERTIA ||
+	       effect->type == FF_FRICTION;
+}
+
 /*
  * Clamp value for a given field
  */
@@ -294,6 +302,20 @@ static void pidff_set_duration(struct pidff_usage *usage, u16 duration)
 	pidff_set_time(usage, duration);
 }
 
+static void pidff_set_effect_direction(struct pidff_device *pidff,
+				       struct ff_effect *effect)
+{
+	u16 direction = effect->direction;
+
+	/* Use fixed direction if needed */
+	if (pidff->quirks & HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION &&
+	    pidff_is_effect_conditional(effect))
+		direction = PIDFF_FIXED_WHEEL_DIRECTION;
+
+	pidff->effect_direction->value[0] =
+		pidff_rescale(direction, U16_MAX, pidff->effect_direction);
+}
+
 /*
  * Send envelope report to the device
  */
@@ -395,11 +417,7 @@ static void pidff_set_effect_report(struct pidff_device *pidff,
 		pidff->set_effect[PID_GAIN].field->logical_maximum;
 	pidff->set_effect[PID_DIRECTION_ENABLE].value[0] = 1;
 
-	/* Use fixed direction if needed */
-	pidff->effect_direction->value[0] = pidff_rescale(
-		pidff->quirks & HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION ?
-		PIDFF_FIXED_WHEEL_DIRECTION : effect->direction,
-		U16_MAX, pidff->effect_direction);
+	pidff_set_effect_direction(pidff, effect);
 
 	/* Omit setting delay field if it's missing */
 	if (!(pidff->quirks & HID_PIDFF_QUIRK_MISSING_DELAY))
diff --git a/drivers/hid/usbhid/hid-pidff.h b/drivers/hid/usbhid/hid-pidff.h
index a53a8b436baa6..f321f675e1318 100644
--- a/drivers/hid/usbhid/hid-pidff.h
+++ b/drivers/hid/usbhid/hid-pidff.h
@@ -16,7 +16,7 @@
 #define HID_PIDFF_QUIRK_PERMISSIVE_CONTROL	BIT(2)
 
 /* Use fixed 0x4000 direction during SET_EFFECT report upload */
-#define HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION	BIT(3)
+#define HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION	BIT(3)
 
 /* Force all periodic effects to be uploaded as SINE */
 #define HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY	BIT(4)
-- 
2.51.0


^ permalink raw reply related

* Re: [PATCH v7 1/9] HID: asus: simplify RGB init sequence
From: Antheas Kapenekakis @ 2025-10-25  7:20 UTC (permalink / raw)
  To: Denis Benato
  Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
	Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
	Ilpo Järvinen
In-Reply-To: <ee179dc9-e7ac-4b64-a58c-93da5f7f4057@gmail.com>

On Sat, 25 Oct 2025 at 03:25, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 10/24/25 23:20, Antheas Kapenekakis wrote:
> > On Fri, 24 Oct 2025 at 20:53, Denis Benato <benato.denis96@gmail.com> wrote:
> >>
> >> On 10/24/25 18:20, Antheas Kapenekakis wrote:
> >>> On Fri, 24 Oct 2025 at 01:25, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >>>> On Fri, 24 Oct 2025 at 00:53, Denis Benato <benato.denis96@gmail.com> wrote:
> >>>>> On 10/23/25 23:30, Antheas Kapenekakis wrote:
> >>>>>> On Thu, 23 Oct 2025 at 22:05, Denis Benato <benato.denis96@gmail.com> wrote:
> >>>>>>> On 10/23/25 20:06, Antheas Kapenekakis wrote:
> >>>>>>>> On Thu, 23 Oct 2025 at 19:38, Denis Benato <benato.denis96@gmail.com> wrote:
> >>>>>>>>> On 10/18/25 12:17, Antheas Kapenekakis wrote:
> >>>>>>>>>> Currently, RGB initialization forks depending on whether a device is
> >>>>>>>>>> NKEY. Then, NKEY devices are initialized using 0x5a, 0x5d, 0x5e
> >>>>>>>>>> endpoints, and non-NKEY devices with 0x5a and then a
> >>>>>>>>>> backlight check, which is omitted for NKEY devices.
> >>>>>>>>>>
> >>>>>>>>>> Remove the fork, using a common initialization sequence for both,
> >>>>>>>>>> where they are both only initialized with 0x5a, then checked for
> >>>>>>>>>> backlight support. This patch should not affect existing functionality.
> >>>>>>>>>>
> >>>>>>>>>> 0x5d and 0x5e endpoint initializations are performed by Windows
> >>>>>>>>>> userspace programs associated with different usages that reside under
> >>>>>>>>>> the vendor HID. Specifically, 0x5d is used by Armoury Crate, which
> >>>>>>>>>> controls RGB and 0x5e by an animation program for certain Asus laptops.
> >>>>>>>>>> Neither is used currently in the driver.
> >>>>>>>>> What benefits do we get from removing the unused initialization?
> >>>>>>>>>
> >>>>>>>>> If this has never caused any troubles I don't see the reason for removing
> >>>>>>>>> them. Moreover the lighting protocol is known and I might as well add
> >>>>>>>>> support for it in the near future,
> >>>>>>>> I already have a patch that adds RGB and delay inits that endpoint. It
> >>>>>>>> got removed to make this easier to merge. See [1].
> >>>>>>>>
> >>>>>>>> [1] https://lore.kernel.org/lkml/20250324210151.6042-10-lkml@antheas.dev/
> >>>>>>> I have to main concerns about this:
> >>>>>>>
> >>>>>>> 1. taking away initialization commands in one patchset to make it
> >>>>>>> easier to merge another unrelated patch doesn't seem the right thing
> >>>>>>> to do if the other patch it's not in the same series.
> >>>>>>>
> >>>>>>> I can see [1] has been removed from the set for a later moment in time,
> >>>>>>> it's fine if it needs more work, just send something that function in the
> >>>>>>> same way and do not remove initialization commands when unnecessary,
> >>>>>>> especially since there will be for sure future development.
> >>>>>> The initialization was removed as part of general cleanup. Not to make
> >>>>>> it easier to merge the RGB patch. In addition, the RGB patch only runs
> >>>>>> the init in a lazy fashion, so if nobody uses the RGB sysfs the init
> >>>>>> does not run and the behavior is the same.
> >>>>> There are a few problems here:
> >>>>> 1. sope creep: either do a cleanup or solve bugs. The fact that your flow z13
> >>>>> doesn't load hid-asus correctly has nothing to do with the initialization of anime.
> >>>>> The fact that hid-asus is driving leds instead of asus-wmi has nothing to do with
> >>>>> anime matrix initialization either.
> >>>>> 2. not sending the initialization can get hardware misbehave because it
> >>>>> is left in an uninitialized state.
> >>>>> 3. there are absolutely zero reasons to do that. There are even less reasons
> >>>>> as to do it as part of this patchset.
> >>>>>
> >>>>>>> 2. Your patchset resolves around keyboard backlight control and how
> >>>>>>> the keyboard device is exposed to userspace: it's fine but I do not see
> >>>>>>> the point in removing initialization commands that has nothing to do
> >>>>>>> with the issue we are trying to solve here.
> >>>>>>>
> >>>>>>> Please leave 0x5E and 0x5D initialization commands where they are now.
> >>>>>> I mean the second part of the patchset does that. The first part is a
> >>>>>> cleanup. What would be the reason for keeping 0x5E and 0x5D? They are
> >>>>>> only used when initializing those endpoints to write further commands
> >>>>>> to them and for identification. The current driver does not write
> >>>>>> commands to those endpoints and identifies itself over 0x5A.
> >>>>> There are no bugs opened that ties initialization of devices to bugs.
> >>>>> Quite the opposite: I can guarantee you that removing part of the
> >>>>> init will introduce regressions.
> >>>>>
> >>>>> The onus is on you to provide strong evidence that the removal is
> >>>>> a necessary act.
> >>>>>
> >>>>> Regardless it is not in the scope of this patchset: remove it.
> >>>>>> I do get that it is a bit risky as some laptops might be hardcoded to
> >>>>>> wait for 0x5D to turn on RGB. Which is why we had the last patch until
> >>>>>> V4. But we have yet to find a laptop that has this problem, so I find
> >>>>>> it difficult to justify keeping the init.
> >>>>> Yes it's risky to remove initialization sequences for a device that is
> >>>>> in every modern ASUS laptop and is tied to the EC.
> >>>>>> Do note that you might need to add the 0x5D init to your userspace
> >>>>>> program for certain laptops if you haven't already. But that is ok,
> >>>>>> since in doing so you are also validating you are speaking to an Asus
> >>>>>> device, which is important.
> >>>>> This doesn't make much sense: why would anyone remove
> >>>>> a command from the kernel, that can be very well essential to some models
> >>>>> (sleep can break, for example) just to add it back in a userspace program?
> >>>>>
> >>>>> What does it mean I have to validate I am speaking to an asus device?
> >>>>> Software selects devices by known attribute, one of them is the vid:pid....
> >>>>> Beside what does this have to do with the removal of initialization commands
> >>>>> from the kernel?
> >>>>>
> >>>>> Even late initializing devices can lead to problems. Windows doesn't do that:
> >>>>> as soon as asus drivers are loaded all relevant initialization sequences are
> >>>>> sent; Windows is the only officially supported OS: do not introduce commands
> >>>>> flow divergence without strong reasons backing it up.
> >>>> If you think keeping 0x5D init is that important, I can spin patch [1]
> >>>> into this series. But then this quirk will stay in the kernel forever.
> >>>> I can even add 0x5E since that does not affect newer devices, which I
> >>>> care for simplifying the sequence.
> >> Fully initializing the device tied to the EC in the same windows does
> >> is not a "quirk". Please stop calling it that.
> >>
> >> It will stay on the kernel until we have strong evidence that it is causing
> >> problems, at that point we simply avoid doing it for problematic laptops.
> >>
> >> If adding other commands doesn't introduce regressions or are otherwise
> >> easy to bisect and makes more hardware working please do.
> > It is not an init sequence. It is a handshake with the userspace
> > program that proves to the program it is talking with a genuine asus
> > device and to the device with the correct program. For all devices
> > that I have tested it seems to NOOP.
> The MCU doesn't distinguish between userspace or kernel space:
> "it is a handshake" => yeah handshakes are part of initialization procedures.
> "with the userspace program [...]" => MCU does not care where data is coming from.
>
> Anyway further discussion is useless. We understood you are against
> keeping commands that that you believe  are useless, but sometimes
> software is like life: you have to accept compromises.
> > 0x5a is the only one used for a driver and it does brightness control.
> > 0x5d/0x5e are used with userspace Windows programs. 0x5d does RGB.
> > Moreover, the application 0xff310076 only has a single report ID under
> > it, 0x5a. 0x5d and 0x5e belong to different hid applications that are
> > not currently checked by the driver (but when they exist they reside
> > under the same hid endpoint, with a multi-application collection that
> > bifurcates in Windows to multiple hid devices).
> The MCU works as a state machine where the status is updated
> on sleep, power on and power off: not sending initialization commands
> will confuse (some) hardware. If not now in a few years when some user will
> migrate away from whatever debian 12 they are running.
>
> Those commands are not simple commands to "just init the device" as you
> are depicting here. Stop doing that.
> > So it makes sense to remove the redundant handshakes. If some laptops
> > require 0x5d to enable shortcuts as Luke said, I have a patch that
> > does that and is straightforward to do. But since the shortcut
> > response comes from the 0x5a endpoint, I find it unlikely for it to
> > require a handshake over a different endpoint to init.
> So you want to remove some code, that has caused no troubles, on
> the assumption that such removal won't have any visible consequence,
> but you have a patch ready to restore the previous behavior in case
> something goes wrong? It doesn't matter if you find it unlikely or not:
> either there is a strong reason to remove it or there is not: you finding
> such removal "unlikely" to break anything down the line is not a strong
> reason to remove it. The fact that you don't like some code is not a strong
> reason to remove it either.
>
> Antheas... your flow z13 isn't loading correctly. Focus on the issue
> at hands. Please.
>

If that initialization means that much to you I will re-add the patch
that runs it on old laptops. No need to crash out

> >>>> Luke said these two pairs are the important ones to keep.
> >>>>
> >>>> I'm not sure what to do.
> >>> I was asked by a 2025 Asus Zenbook Duo user to add his IDs in [1]. In
> >>> doing so, I updated the rgb and legacy init patches for the new series
> >>> and added a quirk for early init of the duo keyboards.
> >> I will take a look when I can, but if you haven't removed anything
> >> that shouldn't pose any risk. None that I can think of at the moment anyway.
> >>> The series is 14 patches long, I don't think my email can take it :(
> >> linux.dev accounts for maintainers are provided free of charge
> >> and I had to ask for an account too. I suggest you do the same.
> >>> Should we merge the first part of this series with the legacy init,
> >>> then do the backlight refactor, and finally the new Duo stuff + rgb?
> >> I think so. My only doubt is about the per_app quirk. Other than
> >> that looks good and solves one problem while also better representing
> >> the hardware, so I can't think of any blockers.
> >>> Antheas
> >>>
> >> Thanks,
> >> Denis
> >>>> Antheas
> >>>>
> >>>> [1] https://lore.kernel.org/all/20250325184601.10990-12-lkml@antheas.dev/
> >>>>
> >>>>>> Antheas
> >>>>>>
> >>>>> Denis
> >>>>>>>>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> >>>>>>>>>> ---
> >>>>>>>>>>  drivers/hid/hid-asus.c | 56 ++++++++++++++----------------------------
> >>>>>>>>>>  1 file changed, 19 insertions(+), 37 deletions(-)
> >>>>>>>>>>
> >>>>>>>>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> >>>>>>>>>> index a444d41e53b6..7ea1037c3979 100644
> >>>>>>>>>> --- a/drivers/hid/hid-asus.c
> >>>>>>>>>> +++ b/drivers/hid/hid-asus.c
> >>>>>>>>>> @@ -638,50 +638,32 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
> >>>>>>>>>>       unsigned char kbd_func;
> >>>>>>>>>>       int ret;
> >>>>>>>>>>
> >>>>>>>>>> -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
> >>>>>>>>>> -             /* Initialize keyboard */
> >>>>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>>>>>>>>> -             if (ret < 0)
> >>>>>>>>>> -                     return ret;
> >>>>>>>>>> -
> >>>>>>>>>> -             /* The LED endpoint is initialised in two HID */
> >>>>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
> >>>>>>>>>> -             if (ret < 0)
> >>>>>>>>>> -                     return ret;
> >>>>>>>>>> -
> >>>>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
> >>>>>>>>>> -             if (ret < 0)
> >>>>>>>>>> -                     return ret;
> >>>>>>>>>> -
> >>>>>>>>>> -             if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> >>>>>>>>>> -                     ret = asus_kbd_disable_oobe(hdev);
> >>>>>>>>>> -                     if (ret < 0)
> >>>>>>>>>> -                             return ret;
> >>>>>>>>>> -             }
> >>>>>>>>>> -
> >>>>>>>>>> -             if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> >>>>>>>>>> -                     intf = to_usb_interface(hdev->dev.parent);
> >>>>>>>>>> -                     udev = interface_to_usbdev(intf);
> >>>>>>>>>> -                     validate_mcu_fw_version(hdev,
> >>>>>>>>>> -                             le16_to_cpu(udev->descriptor.idProduct));
> >>>>>>>>>> -             }
> >>>>>>>>>> +     ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>>>>>>>>> +     if (ret < 0)
> >>>>>>>>>> +             return ret;
> >>>>>>>>>>
> >>>>>>>>>> -     } else {
> >>>>>>>>>> -             /* Initialize keyboard */
> >>>>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
> >>>>>>>>>> -             if (ret < 0)
> >>>>>>>>>> -                     return ret;
> >>>>>>>>>> +     /* Get keyboard functions */
> >>>>>>>>>> +     ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> >>>>>>>>>> +     if (ret < 0)
> >>>>>>>>>> +             return ret;
> >>>>>>>>>>
> >>>>>>>>>> -             /* Get keyboard functions */
> >>>>>>>>>> -             ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
> >>>>>>>>>> +     if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
> >>>>>>>>>> +             ret = asus_kbd_disable_oobe(hdev);
> >>>>>>>>>>               if (ret < 0)
> >>>>>>>>>>                       return ret;
> >>>>>>>>>> +     }
> >>>>>>>>>>
> >>>>>>>>>> -             /* Check for backlight support */
> >>>>>>>>>> -             if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> >>>>>>>>>> -                     return -ENODEV;
> >>>>>>>>>> +     if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
> >>>>>>>>>> +             intf = to_usb_interface(hdev->dev.parent);
> >>>>>>>>>> +             udev = interface_to_usbdev(intf);
> >>>>>>>>>> +             validate_mcu_fw_version(
> >>>>>>>>>> +                     hdev, le16_to_cpu(udev->descriptor.idProduct));
> >>>>>>>>>>       }
> >>>>>>>>>>
> >>>>>>>>>> +     /* Check for backlight support */
> >>>>>>>>>> +     if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
> >>>>>>>>>> +             return -ENODEV;
> >>>>>>>>>> +
> >>>>>>>>>>       drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
> >>>>>>>>>>                                             sizeof(struct asus_kbd_leds),
> >>>>>>>>>>                                             GFP_KERNEL);
>


^ permalink raw reply

* Re: [PATCH 0/2] hid-alps docs heading cleanup
From: Randy Dunlap @ 2025-10-25  7:12 UTC (permalink / raw)
  To: Bagas Sanjaya, Linux Kernel Mailing List, Linux Documentation,
	Linux Input Devices
  Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Masaki Ota,
	George Anthony Vernon
In-Reply-To: <20251024103934.20019-1-bagasdotme@gmail.com>



On 10/24/25 3:39 AM, Bagas Sanjaya wrote:
> Hi,
> 
> Here are two section headings cleanup patches for Alps HID documentation.
> Enjoy!
> 
> Bagas Sanjaya (2):
>   Documentation: hid-alps: Fix packet format section headings
>   Documentation: hid-alps: Format DataByte* subsection headings
> 
>  Documentation/hid/hid-alps.rst | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Much nicer. Thanks.

for both patches:

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>

-- 
~Randy

^ permalink raw reply

* Re: [PATCH v7 1/9] HID: asus: simplify RGB init sequence
From: Denis Benato @ 2025-10-25  1:25 UTC (permalink / raw)
  To: Antheas Kapenekakis
  Cc: platform-driver-x86, linux-input, linux-kernel, Jiri Kosina,
	Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede,
	Ilpo Järvinen
In-Reply-To: <CAGwozwHyC8P4KzZFY7t=WF3ANiJ4q6HgbiAMUNAGHE899Jd6rQ@mail.gmail.com>


On 10/24/25 23:20, Antheas Kapenekakis wrote:
> On Fri, 24 Oct 2025 at 20:53, Denis Benato <benato.denis96@gmail.com> wrote:
>>
>> On 10/24/25 18:20, Antheas Kapenekakis wrote:
>>> On Fri, 24 Oct 2025 at 01:25, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>>>> On Fri, 24 Oct 2025 at 00:53, Denis Benato <benato.denis96@gmail.com> wrote:
>>>>> On 10/23/25 23:30, Antheas Kapenekakis wrote:
>>>>>> On Thu, 23 Oct 2025 at 22:05, Denis Benato <benato.denis96@gmail.com> wrote:
>>>>>>> On 10/23/25 20:06, Antheas Kapenekakis wrote:
>>>>>>>> On Thu, 23 Oct 2025 at 19:38, Denis Benato <benato.denis96@gmail.com> wrote:
>>>>>>>>> On 10/18/25 12:17, Antheas Kapenekakis wrote:
>>>>>>>>>> Currently, RGB initialization forks depending on whether a device is
>>>>>>>>>> NKEY. Then, NKEY devices are initialized using 0x5a, 0x5d, 0x5e
>>>>>>>>>> endpoints, and non-NKEY devices with 0x5a and then a
>>>>>>>>>> backlight check, which is omitted for NKEY devices.
>>>>>>>>>>
>>>>>>>>>> Remove the fork, using a common initialization sequence for both,
>>>>>>>>>> where they are both only initialized with 0x5a, then checked for
>>>>>>>>>> backlight support. This patch should not affect existing functionality.
>>>>>>>>>>
>>>>>>>>>> 0x5d and 0x5e endpoint initializations are performed by Windows
>>>>>>>>>> userspace programs associated with different usages that reside under
>>>>>>>>>> the vendor HID. Specifically, 0x5d is used by Armoury Crate, which
>>>>>>>>>> controls RGB and 0x5e by an animation program for certain Asus laptops.
>>>>>>>>>> Neither is used currently in the driver.
>>>>>>>>> What benefits do we get from removing the unused initialization?
>>>>>>>>>
>>>>>>>>> If this has never caused any troubles I don't see the reason for removing
>>>>>>>>> them. Moreover the lighting protocol is known and I might as well add
>>>>>>>>> support for it in the near future,
>>>>>>>> I already have a patch that adds RGB and delay inits that endpoint. It
>>>>>>>> got removed to make this easier to merge. See [1].
>>>>>>>>
>>>>>>>> [1] https://lore.kernel.org/lkml/20250324210151.6042-10-lkml@antheas.dev/
>>>>>>> I have to main concerns about this:
>>>>>>>
>>>>>>> 1. taking away initialization commands in one patchset to make it
>>>>>>> easier to merge another unrelated patch doesn't seem the right thing
>>>>>>> to do if the other patch it's not in the same series.
>>>>>>>
>>>>>>> I can see [1] has been removed from the set for a later moment in time,
>>>>>>> it's fine if it needs more work, just send something that function in the
>>>>>>> same way and do not remove initialization commands when unnecessary,
>>>>>>> especially since there will be for sure future development.
>>>>>> The initialization was removed as part of general cleanup. Not to make
>>>>>> it easier to merge the RGB patch. In addition, the RGB patch only runs
>>>>>> the init in a lazy fashion, so if nobody uses the RGB sysfs the init
>>>>>> does not run and the behavior is the same.
>>>>> There are a few problems here:
>>>>> 1. sope creep: either do a cleanup or solve bugs. The fact that your flow z13
>>>>> doesn't load hid-asus correctly has nothing to do with the initialization of anime.
>>>>> The fact that hid-asus is driving leds instead of asus-wmi has nothing to do with
>>>>> anime matrix initialization either.
>>>>> 2. not sending the initialization can get hardware misbehave because it
>>>>> is left in an uninitialized state.
>>>>> 3. there are absolutely zero reasons to do that. There are even less reasons
>>>>> as to do it as part of this patchset.
>>>>>
>>>>>>> 2. Your patchset resolves around keyboard backlight control and how
>>>>>>> the keyboard device is exposed to userspace: it's fine but I do not see
>>>>>>> the point in removing initialization commands that has nothing to do
>>>>>>> with the issue we are trying to solve here.
>>>>>>>
>>>>>>> Please leave 0x5E and 0x5D initialization commands where they are now.
>>>>>> I mean the second part of the patchset does that. The first part is a
>>>>>> cleanup. What would be the reason for keeping 0x5E and 0x5D? They are
>>>>>> only used when initializing those endpoints to write further commands
>>>>>> to them and for identification. The current driver does not write
>>>>>> commands to those endpoints and identifies itself over 0x5A.
>>>>> There are no bugs opened that ties initialization of devices to bugs.
>>>>> Quite the opposite: I can guarantee you that removing part of the
>>>>> init will introduce regressions.
>>>>>
>>>>> The onus is on you to provide strong evidence that the removal is
>>>>> a necessary act.
>>>>>
>>>>> Regardless it is not in the scope of this patchset: remove it.
>>>>>> I do get that it is a bit risky as some laptops might be hardcoded to
>>>>>> wait for 0x5D to turn on RGB. Which is why we had the last patch until
>>>>>> V4. But we have yet to find a laptop that has this problem, so I find
>>>>>> it difficult to justify keeping the init.
>>>>> Yes it's risky to remove initialization sequences for a device that is
>>>>> in every modern ASUS laptop and is tied to the EC.
>>>>>> Do note that you might need to add the 0x5D init to your userspace
>>>>>> program for certain laptops if you haven't already. But that is ok,
>>>>>> since in doing so you are also validating you are speaking to an Asus
>>>>>> device, which is important.
>>>>> This doesn't make much sense: why would anyone remove
>>>>> a command from the kernel, that can be very well essential to some models
>>>>> (sleep can break, for example) just to add it back in a userspace program?
>>>>>
>>>>> What does it mean I have to validate I am speaking to an asus device?
>>>>> Software selects devices by known attribute, one of them is the vid:pid....
>>>>> Beside what does this have to do with the removal of initialization commands
>>>>> from the kernel?
>>>>>
>>>>> Even late initializing devices can lead to problems. Windows doesn't do that:
>>>>> as soon as asus drivers are loaded all relevant initialization sequences are
>>>>> sent; Windows is the only officially supported OS: do not introduce commands
>>>>> flow divergence without strong reasons backing it up.
>>>> If you think keeping 0x5D init is that important, I can spin patch [1]
>>>> into this series. But then this quirk will stay in the kernel forever.
>>>> I can even add 0x5E since that does not affect newer devices, which I
>>>> care for simplifying the sequence.
>> Fully initializing the device tied to the EC in the same windows does
>> is not a "quirk". Please stop calling it that.
>>
>> It will stay on the kernel until we have strong evidence that it is causing
>> problems, at that point we simply avoid doing it for problematic laptops.
>>
>> If adding other commands doesn't introduce regressions or are otherwise
>> easy to bisect and makes more hardware working please do.
> It is not an init sequence. It is a handshake with the userspace
> program that proves to the program it is talking with a genuine asus
> device and to the device with the correct program. For all devices
> that I have tested it seems to NOOP.
The MCU doesn't distinguish between userspace or kernel space:
"it is a handshake" => yeah handshakes are part of initialization procedures.
"with the userspace program [...]" => MCU does not care where data is coming from.

Anyway further discussion is useless. We understood you are against
keeping commands that that you believe  are useless, but sometimes
software is like life: you have to accept compromises.
> 0x5a is the only one used for a driver and it does brightness control.
> 0x5d/0x5e are used with userspace Windows programs. 0x5d does RGB.
> Moreover, the application 0xff310076 only has a single report ID under
> it, 0x5a. 0x5d and 0x5e belong to different hid applications that are
> not currently checked by the driver (but when they exist they reside
> under the same hid endpoint, with a multi-application collection that
> bifurcates in Windows to multiple hid devices).
The MCU works as a state machine where the status is updated
on sleep, power on and power off: not sending initialization commands
will confuse (some) hardware. If not now in a few years when some user will
migrate away from whatever debian 12 they are running.

Those commands are not simple commands to "just init the device" as you
are depicting here. Stop doing that.
> So it makes sense to remove the redundant handshakes. If some laptops
> require 0x5d to enable shortcuts as Luke said, I have a patch that
> does that and is straightforward to do. But since the shortcut
> response comes from the 0x5a endpoint, I find it unlikely for it to
> require a handshake over a different endpoint to init.
So you want to remove some code, that has caused no troubles, on
the assumption that such removal won't have any visible consequence,
but you have a patch ready to restore the previous behavior in case
something goes wrong? It doesn't matter if you find it unlikely or not:
either there is a strong reason to remove it or there is not: you finding
such removal "unlikely" to break anything down the line is not a strong
reason to remove it. The fact that you don't like some code is not a strong
reason to remove it either.

Antheas... your flow z13 isn't loading correctly. Focus on the issue
at hands. Please.


>>>> Luke said these two pairs are the important ones to keep.
>>>>
>>>> I'm not sure what to do.
>>> I was asked by a 2025 Asus Zenbook Duo user to add his IDs in [1]. In
>>> doing so, I updated the rgb and legacy init patches for the new series
>>> and added a quirk for early init of the duo keyboards.
>> I will take a look when I can, but if you haven't removed anything
>> that shouldn't pose any risk. None that I can think of at the moment anyway.
>>> The series is 14 patches long, I don't think my email can take it :(
>> linux.dev accounts for maintainers are provided free of charge
>> and I had to ask for an account too. I suggest you do the same.
>>> Should we merge the first part of this series with the legacy init,
>>> then do the backlight refactor, and finally the new Duo stuff + rgb?
>> I think so. My only doubt is about the per_app quirk. Other than
>> that looks good and solves one problem while also better representing
>> the hardware, so I can't think of any blockers.
>>> Antheas
>>>
>> Thanks,
>> Denis
>>>> Antheas
>>>>
>>>> [1] https://lore.kernel.org/all/20250325184601.10990-12-lkml@antheas.dev/
>>>>
>>>>>> Antheas
>>>>>>
>>>>> Denis
>>>>>>>>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
>>>>>>>>>> ---
>>>>>>>>>>  drivers/hid/hid-asus.c | 56 ++++++++++++++----------------------------
>>>>>>>>>>  1 file changed, 19 insertions(+), 37 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
>>>>>>>>>> index a444d41e53b6..7ea1037c3979 100644
>>>>>>>>>> --- a/drivers/hid/hid-asus.c
>>>>>>>>>> +++ b/drivers/hid/hid-asus.c
>>>>>>>>>> @@ -638,50 +638,32 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
>>>>>>>>>>       unsigned char kbd_func;
>>>>>>>>>>       int ret;
>>>>>>>>>>
>>>>>>>>>> -     if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
>>>>>>>>>> -             /* Initialize keyboard */
>>>>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>>>>>>>>> -             if (ret < 0)
>>>>>>>>>> -                     return ret;
>>>>>>>>>> -
>>>>>>>>>> -             /* The LED endpoint is initialised in two HID */
>>>>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
>>>>>>>>>> -             if (ret < 0)
>>>>>>>>>> -                     return ret;
>>>>>>>>>> -
>>>>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
>>>>>>>>>> -             if (ret < 0)
>>>>>>>>>> -                     return ret;
>>>>>>>>>> -
>>>>>>>>>> -             if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
>>>>>>>>>> -                     ret = asus_kbd_disable_oobe(hdev);
>>>>>>>>>> -                     if (ret < 0)
>>>>>>>>>> -                             return ret;
>>>>>>>>>> -             }
>>>>>>>>>> -
>>>>>>>>>> -             if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
>>>>>>>>>> -                     intf = to_usb_interface(hdev->dev.parent);
>>>>>>>>>> -                     udev = interface_to_usbdev(intf);
>>>>>>>>>> -                     validate_mcu_fw_version(hdev,
>>>>>>>>>> -                             le16_to_cpu(udev->descriptor.idProduct));
>>>>>>>>>> -             }
>>>>>>>>>> +     ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>>>>>>>>> +     if (ret < 0)
>>>>>>>>>> +             return ret;
>>>>>>>>>>
>>>>>>>>>> -     } else {
>>>>>>>>>> -             /* Initialize keyboard */
>>>>>>>>>> -             ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
>>>>>>>>>> -             if (ret < 0)
>>>>>>>>>> -                     return ret;
>>>>>>>>>> +     /* Get keyboard functions */
>>>>>>>>>> +     ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
>>>>>>>>>> +     if (ret < 0)
>>>>>>>>>> +             return ret;
>>>>>>>>>>
>>>>>>>>>> -             /* Get keyboard functions */
>>>>>>>>>> -             ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
>>>>>>>>>> +     if (dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16")) {
>>>>>>>>>> +             ret = asus_kbd_disable_oobe(hdev);
>>>>>>>>>>               if (ret < 0)
>>>>>>>>>>                       return ret;
>>>>>>>>>> +     }
>>>>>>>>>>
>>>>>>>>>> -             /* Check for backlight support */
>>>>>>>>>> -             if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
>>>>>>>>>> -                     return -ENODEV;
>>>>>>>>>> +     if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
>>>>>>>>>> +             intf = to_usb_interface(hdev->dev.parent);
>>>>>>>>>> +             udev = interface_to_usbdev(intf);
>>>>>>>>>> +             validate_mcu_fw_version(
>>>>>>>>>> +                     hdev, le16_to_cpu(udev->descriptor.idProduct));
>>>>>>>>>>       }
>>>>>>>>>>
>>>>>>>>>> +     /* Check for backlight support */
>>>>>>>>>> +     if (!(kbd_func & SUPPORT_KBD_BACKLIGHT))
>>>>>>>>>> +             return -ENODEV;
>>>>>>>>>> +
>>>>>>>>>>       drvdata->kbd_backlight = devm_kzalloc(&hdev->dev,
>>>>>>>>>>                                             sizeof(struct asus_kbd_leds),
>>>>>>>>>>                                             GFP_KERNEL);

^ permalink raw reply

* [PATCH v4 11/11] arm64: dts: apple: t8103, t8112, t60xx: Add hwmon SMC subdevice
From: James Calligeros @ 2025-10-25  0:24 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
	Jonathan Corbet, James Calligeros
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, linux-doc
In-Reply-To: <20251025-macsmc-subdevs-v4-0-374d5c9eba0e@gmail.com>

Apple's System Management Controller integrates numerous sensors
that can be exposed via hwmon. Add the subdevice, compatible,
and some common sensors that are exposed on every currently
supported device as a starting point.

Reviewed-by: Neal Gompa <neal@gompa.dev>
Co-developed-by: Janne Grunau <j@jannau.net>
Signed-off-by: Janne Grunau <j@jannau.net>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 .../boot/dts/apple/hwmon-common.dtsi     | 33 +++++++++++++++++++++++++
 .../boot/dts/apple/hwmon-fan-dual.dtsi   | 22 +++++++++++++++++
 arch/arm64/boot/dts/apple/hwmon-fan.dtsi | 17 +++++++++++++
 .../boot/dts/apple/hwmon-laptop.dtsi     | 33 +++++++++++++++++++++++++
 .../boot/dts/apple/hwmon-mac-mini.dtsi   | 15 +++++++++++
 .../arm64/boot/dts/apple/t6001-j375c.dts |  2 ++
 arch/arm64/boot/dts/apple/t6001.dtsi     |  2 ++
 .../arm64/boot/dts/apple/t6002-j375d.dts |  2 ++
 .../arm64/boot/dts/apple/t600x-die0.dtsi |  4 +++
 .../boot/dts/apple/t600x-j314-j316.dtsi  |  3 +++
 .../arm64/boot/dts/apple/t602x-die0.dtsi |  4 +++
 arch/arm64/boot/dts/apple/t8103-j274.dts |  2 ++
 arch/arm64/boot/dts/apple/t8103-j293.dts |  3 +++
 arch/arm64/boot/dts/apple/t8103-j313.dts |  2 ++
 arch/arm64/boot/dts/apple/t8103-j456.dts |  2 ++
 arch/arm64/boot/dts/apple/t8103-j457.dts |  2 ++
 arch/arm64/boot/dts/apple/t8103.dtsi     |  5 ++++
 arch/arm64/boot/dts/apple/t8112-j413.dts |  2 ++
 arch/arm64/boot/dts/apple/t8112-j473.dts |  2 ++
 arch/arm64/boot/dts/apple/t8112-j493.dts |  3 +++
 arch/arm64/boot/dts/apple/t8112.dtsi     |  5 ++++
 21 files changed, 165 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 000000000000..b87021855fdf
--- /dev/null
+++ b/arch/arm64/boot/dts/apple/hwmon-common.dtsi
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Hardware monitoring sensors expected to be found on all Apple Silicon devices
+ *
+ * Copyright The Asahi Linux Contributors
+ */
+
+&smc_hwmon {
+	current-ID0R {
+		apple,key-id = "ID0R";
+		label = "AC Input Current";
+	};
+	power-PSTR {
+		apple,key-id = "PSTR";
+		label = "Total System Power";
+	};
+	power-PDTR {
+		apple,key-id = "PDTR";
+		label = "AC Input Power";
+	};
+	power-PMVR {
+		apple,key-id = "PMVR";
+		label = "3.8 V Rail Power";
+	};
+	temperature-TH0x {
+		apple,key-id = "TH0x";
+		label = "NAND Flash Temperature";
+	};
+	voltage-VD0R {
+		apple,key-id = "VD0R";
+		label = "AC Input Voltage";
+	};
+};
diff --git a/arch/arm64/boot/dts/apple/hwmon-fan-dual.dtsi b/arch/arm64/boot/dts/apple/hwmon-fan-dual.dtsi
new file mode 100644
index 000000000000..3eef0721bcca
--- /dev/null
+++ b/arch/arm64/boot/dts/apple/hwmon-fan-dual.dtsi
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * SMC hwmon fan keys for Apple Silicon desktops/laptops with two fans
+ *
+ * Copyright The Asahi Linux Contributors
+ */
+
+#include "hwmon-fan.dtsi"
+
+&smc_hwmon {
+	fan-F0Ac {
+		label = "Fan 1";
+	};
+	fan-F1Ac {
+		apple,key-id = "F1Ac";
+		label = "Fan 2";
+		apple,fan-minimum = "F1Mn";
+		apple,fan-maximum = "F1Mx";
+		apple,fan-target = "F1Tg";
+		apple,fan-mode = "F1Md";
+	};
+};
diff --git a/arch/arm64/boot/dts/apple/hwmon-fan.dtsi b/arch/arm64/boot/dts/apple/hwmon-fan.dtsi
new file mode 100644
index 000000000000..fba9faf38f4b
--- /dev/null
+++ b/arch/arm64/boot/dts/apple/hwmon-fan.dtsi
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * hwmon fan keys for Apple Silicon desktops/laptops with a single fan.
+ *
+ *  Copyright The Asahi Linux Contributors
+ */
+
+&smc_hwmon {
+	fan-F0Ac {
+		apple,key-id = "F0Ac";
+		label = "Fan";
+		apple,fan-minimum = "F0Mn";
+		apple,fan-maximum = "F0Mx";
+		apple,fan-target = "F0Tg";
+		apple,fan-mode = "F0Md";
+	};
+};
diff --git a/arch/arm64/boot/dts/apple/hwmon-laptop.dtsi b/arch/arm64/boot/dts/apple/hwmon-laptop.dtsi
new file mode 100644
index 000000000000..0c4666282a5c
--- /dev/null
+++ b/arch/arm64/boot/dts/apple/hwmon-laptop.dtsi
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Hardware monitoring sensors expected on all Apple Silicon laptops
+ *
+ * Copyright The Asahi Linux Contributors
+ */
+
+&smc_hwmon {
+	power-PHPC {
+		apple,key-id = "PHPC";
+		label = "Heatpipe Power";
+	};
+	temperature-TB0T {
+		apple,key-id = "TB0T";
+		label = "Battery Hotspot Temperature";
+	};
+	temperature-TCHP {
+		apple,key-id = "TCHP";
+		label = "Charge Regulator Temperature";
+	};
+	temperature-TW0P {
+		apple,key-id = "TW0P";
+		label = "WiFi/BT Module Temperature";
+	};
+	voltage-SBAV {
+		apple,key-id = "SBAV";
+		label = "Battery Voltage";
+	};
+	voltage-VD0R {
+		apple,key-id = "VD0R";
+		label = "Charger Input Voltage";
+	};
+};
diff --git a/arch/arm64/boot/dts/apple/hwmon-mac-mini.dtsi b/arch/arm64/boot/dts/apple/hwmon-mac-mini.dtsi
new file mode 100644
index 000000000000..f32627336ae7
--- /dev/null
+++ b/arch/arm64/boot/dts/apple/hwmon-mac-mini.dtsi
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * hwmon sensors expected on all Mac mini models
+ *
+ * Copyright The Asahi Linux Contributors
+ */
+
+#include "hwmon-fan.dtsi"
+
+&smc_hwmon {
+	temperature-TW0P {
+		apple,key-id = "TW0P";
+		label = "WiFi/BT Module Temperature";
+	};
+};
diff --git a/arch/arm64/boot/dts/apple/t6001-j375c.dts b/arch/arm64/boot/dts/apple/t6001-j375c.dts
index 2e7c23714d4d..08276114c1d8 100644
--- a/arch/arm64/boot/dts/apple/t6001-j375c.dts
+++ b/arch/arm64/boot/dts/apple/t6001-j375c.dts
@@ -24,3 +24,5 @@ &wifi0 {
 &bluetooth0 {
 	brcm,board-type = "apple,okinawa";
 };
+
+#include "hwmon-fan-dual.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t6001.dtsi b/arch/arm64/boot/dts/apple/t6001.dtsi
index ffbe823b71bc..264df90f07d8 100644
--- a/arch/arm64/boot/dts/apple/t6001.dtsi
+++ b/arch/arm64/boot/dts/apple/t6001.dtsi
@@ -66,3 +66,5 @@ p-core-pmu-affinity {
 &gpu {
 	compatible = "apple,agx-g13c", "apple,agx-g13s";
 };
+
+#include "hwmon-common.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t6002-j375d.dts b/arch/arm64/boot/dts/apple/t6002-j375d.dts
index 2b7f80119618..d12c0ae418f7 100644
--- a/arch/arm64/boot/dts/apple/t6002-j375d.dts
+++ b/arch/arm64/boot/dts/apple/t6002-j375d.dts
@@ -56,3 +56,5 @@ &bluetooth0 {
 
 /delete-node/ &ps_disp0_cpu0_die1;
 /delete-node/ &ps_disp0_fe_die1;
+
+#include "hwmon-fan-dual.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t600x-die0.dtsi b/arch/arm64/boot/dts/apple/t600x-die0.dtsi
index f715b19efd16..e6647c1a9173 100644
--- a/arch/arm64/boot/dts/apple/t600x-die0.dtsi
+++ b/arch/arm64/boot/dts/apple/t600x-die0.dtsi
@@ -37,6 +37,10 @@ smc_gpio: gpio {
 			#gpio-cells = <2>;
 		};
 
+		smc_hwmon: hwmon {
+			compatible = "apple,smc-hwmon";
+		};
+
 		smc_reboot: reboot {
 			compatible = "apple,smc-reboot";
 			nvmem-cells = <&shutdown_flag>, <&boot_stage>,
diff --git a/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi b/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
index c0aac59a6fae..127814a9dfa4 100644
--- a/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
+++ b/arch/arm64/boot/dts/apple/t600x-j314-j316.dtsi
@@ -131,3 +131,6 @@ &fpwm0 {
 };
 
 #include "spi1-nvram.dtsi"
+
+#include "hwmon-laptop.dtsi"
+#include "hwmon-fan-dual.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t602x-die0.dtsi b/arch/arm64/boot/dts/apple/t602x-die0.dtsi
index 8622ddea7b44..680c103c1c0f 100644
--- a/arch/arm64/boot/dts/apple/t602x-die0.dtsi
+++ b/arch/arm64/boot/dts/apple/t602x-die0.dtsi
@@ -114,6 +114,10 @@ smc_gpio: gpio {
 			#gpio-cells = <2>;
 		};
 
+		smc_hwmon: hwmon {
+			compatible = "apple,smc-hwmon";
+		};
+
 		smc_reboot: reboot {
 			compatible = "apple,smc-reboot";
 			nvmem-cells = <&shutdown_flag>, <&boot_stage>,
diff --git a/arch/arm64/boot/dts/apple/t8103-j274.dts b/arch/arm64/boot/dts/apple/t8103-j274.dts
index 1c3e37f86d46..f5b8cc087882 100644
--- a/arch/arm64/boot/dts/apple/t8103-j274.dts
+++ b/arch/arm64/boot/dts/apple/t8103-j274.dts
@@ -61,3 +61,5 @@ &pcie0_dart_2 {
 &i2c2 {
 	status = "okay";
 };
+
+#include "hwmon-mac-mini.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8103-j293.dts b/arch/arm64/boot/dts/apple/t8103-j293.dts
index 5b3c42e9f0e6..abb88391635f 100644
--- a/arch/arm64/boot/dts/apple/t8103-j293.dts
+++ b/arch/arm64/boot/dts/apple/t8103-j293.dts
@@ -119,3 +119,6 @@ dfr_panel_in: endpoint {
 &displaydfr_dart {
 	status = "okay";
 };
+
+#include "hwmon-laptop.dtsi"
+#include "hwmon-fan.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8103-j313.dts b/arch/arm64/boot/dts/apple/t8103-j313.dts
index 97a4344d8dca..491ead016b21 100644
--- a/arch/arm64/boot/dts/apple/t8103-j313.dts
+++ b/arch/arm64/boot/dts/apple/t8103-j313.dts
@@ -41,3 +41,5 @@ &wifi0 {
 &fpwm1 {
 	status = "okay";
 };
+
+#include "hwmon-laptop.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8103-j456.dts b/arch/arm64/boot/dts/apple/t8103-j456.dts
index 58c8e43789b4..c2ec6fbb633c 100644
--- a/arch/arm64/boot/dts/apple/t8103-j456.dts
+++ b/arch/arm64/boot/dts/apple/t8103-j456.dts
@@ -75,3 +75,5 @@ &pcie0_dart_1 {
 &pcie0_dart_2 {
 	status = "okay";
 };
+
+#include "hwmon-fan-dual.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8103-j457.dts b/arch/arm64/boot/dts/apple/t8103-j457.dts
index 7089ccf3ce55..aeaab2482d54 100644
--- a/arch/arm64/boot/dts/apple/t8103-j457.dts
+++ b/arch/arm64/boot/dts/apple/t8103-j457.dts
@@ -56,3 +56,5 @@ ethernet0: ethernet@0,0 {
 &pcie0_dart_2 {
 	status = "okay";
 };
+
+#include "hwmon-fan.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8103.dtsi b/arch/arm64/boot/dts/apple/t8103.dtsi
index 59f2678639cf..f1820bdc0910 100644
--- a/arch/arm64/boot/dts/apple/t8103.dtsi
+++ b/arch/arm64/boot/dts/apple/t8103.dtsi
@@ -909,6 +909,10 @@ smc_gpio: gpio {
 				#gpio-cells = <2>;
 			};
 
+			smc_hwmon: hwmon {
+				compatible = "apple,smc-hwmon";
+			};
+
 			smc_reboot: reboot {
 				compatible = "apple,smc-reboot";
 				nvmem-cells = <&shutdown_flag>, <&boot_stage>,
@@ -1141,3 +1145,4 @@ port02: pci@2,0 {
 };
 
 #include "t8103-pmgr.dtsi"
+#include "hwmon-common.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8112-j413.dts b/arch/arm64/boot/dts/apple/t8112-j413.dts
index 6f69658623bf..500dcdf2d4b5 100644
--- a/arch/arm64/boot/dts/apple/t8112-j413.dts
+++ b/arch/arm64/boot/dts/apple/t8112-j413.dts
@@ -78,3 +78,5 @@ &i2c4 {
 &fpwm1 {
 	status = "okay";
 };
+
+#include "hwmon-laptop.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8112-j473.dts b/arch/arm64/boot/dts/apple/t8112-j473.dts
index 06fe257f08be..11db6a92493f 100644
--- a/arch/arm64/boot/dts/apple/t8112-j473.dts
+++ b/arch/arm64/boot/dts/apple/t8112-j473.dts
@@ -52,3 +52,5 @@ &pcie1_dart {
 &pcie2_dart {
 	status = "okay";
 };
+
+#include "hwmon-mac-mini.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8112-j493.dts b/arch/arm64/boot/dts/apple/t8112-j493.dts
index fb8ad7d4c65a..a0da02c00f15 100644
--- a/arch/arm64/boot/dts/apple/t8112-j493.dts
+++ b/arch/arm64/boot/dts/apple/t8112-j493.dts
@@ -133,3 +133,6 @@ touchbar0: touchbar@0 {
 		touchscreen-inverted-y;
 	};
 };
+
+#include "hwmon-laptop.dtsi"
+#include "hwmon-fan.dtsi"
diff --git a/arch/arm64/boot/dts/apple/t8112.dtsi b/arch/arm64/boot/dts/apple/t8112.dtsi
index 6bc3f58b06f7..c4d1e5ffaee9 100644
--- a/arch/arm64/boot/dts/apple/t8112.dtsi
+++ b/arch/arm64/boot/dts/apple/t8112.dtsi
@@ -912,6 +912,10 @@ smc_gpio: gpio {
 				#gpio-cells = <2>;
 			};
 
+			smc_hwmon: hwmon {
+				compatible = "apple,smc-hwmon";
+			};
+
 			smc_reboot: reboot {
 				compatible = "apple,smc-reboot";
 				nvmem-cells = <&shutdown_flag>, <&boot_stage>,
@@ -1180,3 +1184,4 @@ port03: pci@3,0 {
 };
 
 #include "t8112-pmgr.dtsi"
+#include "hwmon-common.dtsi"

-- 
2.51.0


^ permalink raw reply related

* [PATCH v4 10/11] arm64: dts: apple: t8103,t60xx,t8112: Add SMC RTC node
From: James Calligeros @ 2025-10-25  0:24 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
	Jonathan Corbet, James Calligeros
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, linux-doc
In-Reply-To: <20251025-macsmc-subdevs-v4-0-374d5c9eba0e@gmail.com>

From: Sven Peter <sven@kernel.org>

The System Manager Controller of all M1/M2 SoCs supports the RTC
sub-device.

Reviewed-by: Neal Gompa <neal@gompa.dev>
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 .../arm64/boot/dts/apple/t600x-die0.dtsi | 6 ++++++
 .../arm64/boot/dts/apple/t602x-die0.dtsi | 6 ++++++
 arch/arm64/boot/dts/apple/t8103.dtsi     | 6 ++++++
 arch/arm64/boot/dts/apple/t8112.dtsi     | 6 ++++++
 4 files changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/apple/t600x-die0.dtsi b/arch/arm64/boot/dts/apple/t600x-die0.dtsi
index 3603b276a2ab..f715b19efd16 100644
--- a/arch/arm64/boot/dts/apple/t600x-die0.dtsi
+++ b/arch/arm64/boot/dts/apple/t600x-die0.dtsi
@@ -44,6 +44,12 @@ smc_reboot: reboot {
 			nvmem-cell-names = "shutdown_flag", "boot_stage",
 				"boot_error_count", "panic_count";
 		};
+
+		rtc {
+			compatible = "apple,smc-rtc";
+			nvmem-cells = <&rtc_offset>;
+			nvmem-cell-names = "rtc_offset";
+		};
 	};
 
 	smc_mbox: mbox@290408000 {
diff --git a/arch/arm64/boot/dts/apple/t602x-die0.dtsi b/arch/arm64/boot/dts/apple/t602x-die0.dtsi
index 2e7d2bf08ddc..8622ddea7b44 100644
--- a/arch/arm64/boot/dts/apple/t602x-die0.dtsi
+++ b/arch/arm64/boot/dts/apple/t602x-die0.dtsi
@@ -121,6 +121,12 @@ smc_reboot: reboot {
 			nvmem-cell-names = "shutdown_flag", "boot_stage",
 				"boot_error_count", "panic_count";
 		};
+
+		rtc {
+			compatible = "apple,smc-rtc";
+			nvmem-cells = <&rtc_offset>;
+			nvmem-cell-names = "rtc_offset";
+		};
 	};
 
 	pinctrl_smc: pinctrl@2a2820000 {
diff --git a/arch/arm64/boot/dts/apple/t8103.dtsi b/arch/arm64/boot/dts/apple/t8103.dtsi
index 8b7b27887968..59f2678639cf 100644
--- a/arch/arm64/boot/dts/apple/t8103.dtsi
+++ b/arch/arm64/boot/dts/apple/t8103.dtsi
@@ -916,6 +916,12 @@ smc_reboot: reboot {
 				nvmem-cell-names = "shutdown_flag", "boot_stage",
 					"boot_error_count", "panic_count";
 			};
+
+			rtc {
+				compatible = "apple,smc-rtc";
+				nvmem-cells = <&rtc_offset>;
+				nvmem-cell-names = "rtc_offset";
+			};
 		};
 
 		smc_mbox: mbox@23e408000 {
diff --git a/arch/arm64/boot/dts/apple/t8112.dtsi b/arch/arm64/boot/dts/apple/t8112.dtsi
index 3f79878b25af..6bc3f58b06f7 100644
--- a/arch/arm64/boot/dts/apple/t8112.dtsi
+++ b/arch/arm64/boot/dts/apple/t8112.dtsi
@@ -919,6 +919,12 @@ smc_reboot: reboot {
 				nvmem-cell-names = "shutdown_flag", "boot_stage",
 					"boot_error_count", "panic_count";
 			};
+
+			rtc {
+				compatible = "apple,smc-rtc";
+				nvmem-cells = <&rtc_offset>;
+				nvmem-cell-names = "rtc_offset";
+			};
 		};
 
 		smc_mbox: mbox@23e408000 {

-- 
2.51.0


^ permalink raw reply related

* [PATCH v4 09/11] mfd: macsmc: Wire up Apple SMC input subdevice
From: James Calligeros @ 2025-10-25  0:24 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
	Jonathan Corbet, James Calligeros
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, linux-doc
In-Reply-To: <20251025-macsmc-subdevs-v4-0-374d5c9eba0e@gmail.com>

Add the new SMC input function to the mfd device

Reviewed-by: Neal Gompa <neal@gompa.dev>
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 51dd667d3b5f..3b69eb6d032a 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-input"),
 	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"),

-- 
2.51.0


^ permalink raw reply related

* [PATCH v4 08/11] input: macsmc-input: New driver to handle the Apple Mac SMC buttons/lid
From: James Calligeros @ 2025-10-25  0:24 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
	Jonathan Corbet, James Calligeros
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, linux-doc, Hector Martin
In-Reply-To: <20251025-macsmc-subdevs-v4-0-374d5c9eba0e@gmail.com>

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.

Reviewed-by: Neal Gompa <neal@gompa.dev>
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-input.c | 208 +++++++++++++++++++++++++
 4 files changed, 221 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 79b9f40224a9..e8283f127f11 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2451,6 +2451,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-input.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 0e6b49fb54bc..109660a1a5d2 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -981,4 +981,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
+	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-input.
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index ae857c24f48e..480a0d08d4ae 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_INPUT)	+= macsmc-input.o
 obj-$(CONFIG_INPUT_MAX7360_ROTARY)	+= max7360-rotary.o
 obj-$(CONFIG_INPUT_MAX77650_ONKEY)	+= max77650-onkey.o
 obj-$(CONFIG_INPUT_MAX77693_HAPTIC)	+= max77693-haptic.o
diff --git a/drivers/input/misc/macsmc-input.c b/drivers/input/misc/macsmc-input.c
new file mode 100644
index 000000000000..d35322856526
--- /dev/null
+++ b/drivers/input/misc/macsmc-input.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * Apple SMC input event driver
+ * Copyright The Asahi Linux Contributors
+ *
+ * This driver exposes certain 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_input
+ * @dev: Underlying struct device for the input 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_input {
+	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_input_event_button(struct macsmc_input *smcin, unsigned long event)
+{
+	u8 button = (event >> 8) & 0xff;
+	u8 state = !!(event & 0xff);
+
+	switch (button) {
+	case BTN_POWER:
+	case BTN_TOUCHID:
+		pm_wakeup_dev_event(smcin->dev, 0, (smcin->wakeup_mode && state));
+
+		/* Suppress KEY_POWER event to prevent immediate shutdown on wake */
+		if (smcin->wakeup_mode)
+			return;
+
+		input_report_key(smcin->input, KEY_POWER, state);
+		input_sync(smcin->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(smcin->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(smcin->dev, "Unknown SMC button event: %04lx\n", event & 0xffff);
+	}
+}
+
+static void macsmc_input_event_lid(struct macsmc_input *smcin, unsigned long event)
+{
+	u8 lid_state = !!((event >> 8) & 0xff);
+
+	pm_wakeup_dev_event(smcin->dev, 0, (smcin->wakeup_mode && !lid_state));
+	input_report_switch(smcin->input, SW_LID, lid_state);
+	input_sync(smcin->input);
+}
+
+static int macsmc_input_event(struct notifier_block *nb, unsigned long event, void *data)
+{
+	struct macsmc_input *smcin = container_of(nb, struct macsmc_input, nb);
+	u16 type = event >> 16;
+
+	switch (type) {
+	case SMC_EV_BTN:
+		macsmc_input_event_button(smcin, event);
+		return NOTIFY_OK;
+	case SMC_EV_LID:
+		macsmc_input_event_lid(smcin, event);
+		return NOTIFY_OK;
+	default:
+		/* SMC event meant for another driver */
+		return NOTIFY_DONE;
+	}
+}
+
+static int macsmc_input_probe(struct platform_device *pdev)
+{
+	struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
+	struct macsmc_input *smcin;
+	bool have_lid, have_power;
+	int error;
+
+	/* 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;
+
+	smcin = devm_kzalloc(&pdev->dev, sizeof(*smcin), GFP_KERNEL);
+	if (!smcin)
+		return -ENOMEM;
+
+	smcin->dev = &pdev->dev;
+	smcin->smc = smc;
+	platform_set_drvdata(pdev, smcin);
+
+	smcin->input = devm_input_allocate_device(&pdev->dev);
+	if (!smcin->input)
+		return -ENOMEM;
+
+	smcin->input->phys = "macsmc-input (0)";
+	smcin->input->name = "Apple SMC power/lid events";
+
+	if (have_lid)
+		input_set_capability(smcin->input, EV_SW, SW_LID);
+	if (have_power)
+		input_set_capability(smcin->input, EV_KEY, KEY_POWER);
+
+	if (have_lid) {
+		u8 val;
+
+		error = apple_smc_read_u8(smc, SMC_KEY(MSLD), &val);
+		if (error < 0)
+			dev_warn(&pdev->dev, "Failed to read initial lid state\n");
+		else
+			input_report_switch(smcin->input, SW_LID, val);
+	}
+
+	if (have_power) {
+		u32 val;
+
+		error = apple_smc_read_u32(smc, SMC_KEY(bHLD), &val);
+		if (error < 0)
+			dev_warn(&pdev->dev, "Failed to read initial power button state\n");
+		else
+			input_report_key(smcin->input, KEY_POWER, val & 1);
+	}
+
+	error = input_register_device(smcin->input);
+	if (error) {
+		dev_err(&pdev->dev, "Failed to register input device: %d\n", error);
+		return error;
+	}
+
+	input_sync(smcin->input);
+
+	smcin->nb.notifier_call = macsmc_input_event;
+	blocking_notifier_chain_register(&smc->event_handlers, &smcin->nb);
+
+	device_init_wakeup(&pdev->dev, true);
+
+	return 0;
+}
+
+static int macsmc_input_pm_prepare(struct device *dev)
+{
+	struct macsmc_input *smcin = dev_get_drvdata(dev);
+
+	smcin->wakeup_mode = true;
+	return 0;
+}
+
+static void macsmc_input_pm_complete(struct device *dev)
+{
+	struct macsmc_input *smcin = dev_get_drvdata(dev);
+
+	smcin->wakeup_mode = false;
+}
+
+static const struct dev_pm_ops macsmc_input_pm_ops = {
+	.prepare = macsmc_input_pm_prepare,
+	.complete = macsmc_input_pm_complete,
+};
+
+static struct platform_driver macsmc_input_driver = {
+	.driver = {
+		.name = "macsmc-input",
+		.pm = &macsmc_input_pm_ops,
+	},
+	.probe = macsmc_input_probe,
+};
+module_platform_driver(macsmc_input_driver);
+
+MODULE_AUTHOR("Hector Martin <marcan@marcan.st>");
+MODULE_LICENSE("Dual MIT/GPL");
+MODULE_DESCRIPTION("Apple SMC input driver");
+MODULE_ALIAS("platform:macsmc-input");

-- 
2.51.0


^ permalink raw reply related

* [PATCH v4 07/11] mfd: macsmc: Wire up Apple SMC hwmon subdevice
From: James Calligeros @ 2025-10-25  0:24 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
	Jonathan Corbet, James Calligeros
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, linux-doc
In-Reply-To: <20251025-macsmc-subdevs-v4-0-374d5c9eba0e@gmail.com>

Add the SMC hwmon functionality to the mfd device

Reviewed-by: Neal Gompa <neal@gompa.dev>
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 500395bb48da..51dd667d3b5f 100644
--- a/drivers/mfd/macsmc.c
+++ b/drivers/mfd/macsmc.c
@@ -46,6 +46,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-hwmon", NULL, NULL, 0, 0, "apple,smc-hwmon"),
 	MFD_CELL_OF("macsmc-reboot", NULL, NULL, 0, 0, "apple,smc-reboot"),
 	MFD_CELL_OF("macsmc-rtc", NULL, NULL, 0, 0, "apple,smc-rtc"),
 };

-- 
2.51.0


^ permalink raw reply related

* [PATCH v4 06/11] hwmon: Add Apple Silicon SMC hwmon driver
From: James Calligeros @ 2025-10-25  0:24 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
	Jonathan Corbet, James Calligeros
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, linux-doc
In-Reply-To: <20251025-macsmc-subdevs-v4-0-374d5c9eba0e@gmail.com>

The System Management Controller on Apple Silicon devices is responsible
for integrating and exposing the data reported by the vast array of
hardware monitoring sensors present on these devices. It is also
responsible for fan control, and allows users to manually set fan
speeds if they so desire. Add a hwmon driver to expose current,
power, temperature, and voltage monitoring sensors, as well as
fan speed monitoring and control via the SMC on Apple Silicon devices.

The SMC firmware has no consistency between devices, even when they
share an SoC. The FourCC keys used to access sensors are almost
random. An M1 Mac mini will have different FourCCs for its CPU core
temperature sensors to an M1 MacBook Pro, for example. For this
reason, the valid sensors for a given device are specified in a
child of the SMC Devicetree node. The driver uses this information
to determine which sensors to make available at runtime.

Reviewed-by: Neal Gompa <neal@gompa.dev>
Acked-by: Guenter Roeck <linux@roeck-us.net>
Co-developed-by: Janne Grunau <j@jannau.net>
Signed-off-by: Janne Grunau <j@jannau.net>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 Documentation/hwmon/macsmc-hwmon.rst |  71 +++
 MAINTAINERS                          |   2 +
 drivers/hwmon/Kconfig                |  12 +
 drivers/hwmon/Makefile               |   1 +
 drivers/hwmon/macsmc-hwmon.c         | 851 +++++++++++++++++++++++++
 5 files changed, 937 insertions(+)

diff --git a/Documentation/hwmon/macsmc-hwmon.rst b/Documentation/hwmon/macsmc-hwmon.rst
new file mode 100644
index 000000000000..6903f76df62b
--- /dev/null
+++ b/Documentation/hwmon/macsmc-hwmon.rst
@@ -0,0 +1,71 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Kernel driver macsmc-hwmon
+==========================
+
+Supported hardware
+
+    * Apple Silicon Macs (M1 and up)
+
+Author: James Calligeros <jcalligeros99@gmail.com>
+
+Description
+-----------
+
+macsmc-hwmon exposes the Apple System Management controller's
+temperature, voltage, current and power sensors, as well as
+fan speed and control capabilities, via hwmon.
+
+Because each Apple Silicon Mac exposes a different set of sensors
+(e.g. the MacBooks expose battery telemetry that is not present on
+the desktop Macs), sensors present on any given machine are described
+via Devicetree. The driver picks these up and registers them with
+hwmon when probed.
+
+Manual fan speed is supported via the fan_control module parameter. This
+is disabled by default and marked as unsafe, as it cannot be proven that
+the system will fail safe if overheating due to manual fan control being
+used.
+
+sysfs interface
+---------------
+
+currX_input
+    Ammeter value
+
+currX_label
+    Ammeter label
+
+fanX_input
+    Current fan speed
+
+fanX_label
+    Fan label
+
+fanX_min
+    Minimum possible fan speed
+
+fanX_max
+    Maximum possible fan speed
+
+fanX_target
+    Current fan setpoint
+
+inX_input
+    Voltmeter value
+
+inX_label
+    Voltmeter label
+
+powerX_input
+    Power meter value
+
+powerX_label
+    Power meter label
+
+tempX_input
+    Temperature sensor value
+
+tempX_label
+    Temperature sensor label
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 3c6322872dd1..79b9f40224a9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2440,12 +2440,14 @@ F:	Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml
 F:	Documentation/devicetree/bindings/spi/apple,spi.yaml
 F:	Documentation/devicetree/bindings/spmi/apple,spmi.yaml
 F:	Documentation/devicetree/bindings/watchdog/apple,wdt.yaml
+F:	Documentation/hwmon/macsmc-hwmon.rst
 F:	arch/arm64/boot/dts/apple/
 F:	drivers/bluetooth/hci_bcm4377.c
 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 2760feb9f83b..d6e9e39d2762 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1174,6 +1174,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 73b2abdcc6dd..f9c049ce9124 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -148,6 +148,7 @@ obj-$(CONFIG_SENSORS_LTC4260)	+= ltc4260.o
 obj-$(CONFIG_SENSORS_LTC4261)	+= ltc4261.o
 obj-$(CONFIG_SENSORS_LTC4282)	+= ltc4282.o
 obj-$(CONFIG_SENSORS_LTQ_CPUTEMP) += ltq-cputemp.o
+obj-$(CONFIG_SENSORS_MACSMC_HWMON)	+= macsmc-hwmon.o
 obj-$(CONFIG_SENSORS_MAX1111)	+= max1111.o
 obj-$(CONFIG_SENSORS_MAX127)	+= max127.o
 obj-$(CONFIG_SENSORS_MAX16065)	+= max16065.o
diff --git a/drivers/hwmon/macsmc-hwmon.c b/drivers/hwmon/macsmc-hwmon.c
new file mode 100644
index 000000000000..1c0bbec7e8eb
--- /dev/null
+++ b/drivers/hwmon/macsmc-hwmon.c
@@ -0,0 +1,851 @@
+// 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/bitfield.h>
+#include <linux/hwmon.h>
+#include <linux/mfd/macsmc.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#define MAX_LABEL_LENGTH	32
+
+/* Temperature, voltage, current, power, fan(s) */
+#define NUM_SENSOR_TYPES	5
+
+#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];
+	u32 attrs;
+};
+
+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;
+	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:
+		*str = hwmon->temp.sensors[channel].label;
+		break;
+	case hwmon_in:
+		*str = hwmon->volt.sensors[channel].label;
+		break;
+	case hwmon_curr:
+		*str = hwmon->curr.sensors[channel].label;
+		break;
+	case hwmon_power:
+		*str = hwmon->power.sensors[channel].label;
+		break;
+	case hwmon_fan:
+		*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;
+
+	/* We never have negatively scaled SMC floats */
+	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('f', 'l', 't', ' '): {
+		u32 flt_ = 0;
+
+		ret = macsmc_hwmon_read_f32_scaled(smc, sensor->macsmc_key,
+						   &flt_, scale);
+		if (ret)
+			return ret;
+
+		*val = flt_;
+		break;
+	}
+	/* 48.16 fixed point decimal */
+	case __SMC_KEY('i', 'o', 'f', 't'): {
+		u64 ioft = 0;
+
+		ret = macsmc_hwmon_read_ioft_scaled(smc, sensor->macsmc_key,
+						    &ioft, scale);
+		if (ret)
+			return ret;
+
+		*val = (long)ioft;
+		break;
+	}
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static int macsmc_hwmon_write_f32(struct apple_smc *smc, smc_key key, int value)
+{
+	u64 val;
+	u32 fval = 0;
+	int exp = 0, neg;
+
+	val = abs(value);
+	neg = val != value;
+
+	if (val) {
+		int msb = __fls(val) - exp;
+
+		if (msb > 23) {
+			val >>= msb - FLT_MANT_BIAS;
+			exp -= msb - FLT_MANT_BIAS;
+		} else if (msb < 23) {
+			val <<= FLT_MANT_BIAS - 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)
+{
+	switch (sensor->info.type_code) {
+	/* 32-bit IEEE 754 float */
+	case __SMC_KEY('f', 'l', 't', ' '):
+		return macsmc_hwmon_write_f32(smc, sensor->macsmc_key, val);
+	/* unsigned 8-bit integer */
+	case __SMC_KEY('u', 'i', '8', ' '):
+		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)
+{
+	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 -EOPNOTSUPP;
+	}
+}
+
+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;
+
+	/*
+	 * 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);
+			if (ret < 0)
+				return ret;
+
+			hwmon->fan.fans[channel].manual = true;
+		}
+		return macsmc_hwmon_write_key(hwmon->smc,
+					      &hwmon->fan.fans[channel].set, val);
+	} else if (!val) {
+		if (hwmon->fan.fans[channel].manual) {
+			ret = macsmc_hwmon_write_key(hwmon->smc,
+						     &hwmon->fan.fans[channel].mode, 0);
+			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 struct macsmc_hwmon_fan *fan,
+					   u32 attr)
+{
+	if (fan->attrs & BIT(attr)) {
+		if (attr == hwmon_fan_target && fan_control && fan->mode.macsmc_key)
+			return 0644;
+
+		return 0444;
+	}
+
+	return 0;
+}
+
+static umode_t macsmc_hwmon_is_visible(const void *data,
+				       enum hwmon_sensor_types type, u32 attr,
+				       int channel)
+{
+	const struct macsmc_hwmon *hwmon = data;
+	struct macsmc_hwmon_sensor *sensor;
+
+	switch (type) {
+	case hwmon_in:
+		sensor = &hwmon->volt.sensors[channel];
+		break;
+	case hwmon_curr:
+		sensor = &hwmon->curr.sensors[channel];
+		break;
+	case hwmon_power:
+		sensor = &hwmon->power.sensors[channel];
+		break;
+	case hwmon_temp:
+		sensor = &hwmon->temp.sensors[channel];
+		break;
+	case hwmon_fan:
+		return macsmc_hwmon_fan_is_visible(&hwmon->fan.fans[channel], attr);
+	default:
+		return 0;
+	}
+
+	/* Sensors only register ro attributes */
+	if (sensor->attrs & BIT(attr))
+		return 0444;
+
+	return 0;
+}
+
+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_dbg(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_dbg(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_dbg(dev, "No label found for sensor %s\n", key);
+	else
+		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 ret;
+	}
+
+	ret = macsmc_hwmon_parse_key(dev, smc, &fan->now, now);
+	if (ret)
+		return ret;
+
+	fan->attrs = HWMON_F_INPUT;
+
+	ret = of_property_read_string(fan_node, "label", &label);
+	if (ret) {
+		dev_dbg(dev, "No label found for fan %s\n", now);
+	} else {
+		strscpy_pad(fan->label, label, sizeof(fan->label));
+		fan->attrs |= HWMON_F_LABEL;
+	}
+
+	/* 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;
+	struct macsmc_hwmon_sensor *sensor;
+	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-") {
+			sensor = &hwmon->curr.sensors[hwmon->curr.count];
+			if (!macsmc_hwmon_create_sensor(hwmon->dev, hwmon->smc, key_node, sensor)) {
+				sensor->attrs = HWMON_C_INPUT;
+
+				if (*sensor->label)
+					sensor->attrs |= HWMON_C_LABEL;
+
+				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-") {
+			sensor = &hwmon->power.sensors[hwmon->power.count];
+			if (!macsmc_hwmon_create_sensor(hwmon->dev, hwmon->smc, key_node, sensor)) {
+				sensor->attrs = HWMON_P_INPUT;
+
+				if (*sensor->label)
+					sensor->attrs |= HWMON_P_LABEL;
+
+				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-") {
+			sensor = &hwmon->temp.sensors[hwmon->temp.count];
+			if (!macsmc_hwmon_create_sensor(hwmon->dev, hwmon->smc, key_node, sensor)) {
+				sensor->attrs = HWMON_T_INPUT;
+
+				if (*sensor->label)
+					sensor->attrs |= HWMON_T_LABEL;
+
+				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-") {
+			sensor = &hwmon->temp.sensors[hwmon->temp.count];
+			if (!macsmc_hwmon_create_sensor(hwmon->dev, hwmon->smc, key_node, sensor)) {
+				sensor->attrs = HWMON_I_INPUT;
+
+				if (*sensor->label)
+					sensor->attrs |= HWMON_I_LABEL;
+
+				hwmon->volt.count++;
+			}
+		}
+	}
+
+	return 0;
+}
+
+/* Create NULL-terminated config arrays */
+static void macsmc_hwmon_populate_configs(u32 *configs, const struct macsmc_hwmon_sensors *sensors)
+{
+	int idx;
+
+	for (idx = 0; idx < sensors->count; idx++)
+		configs[idx] = sensors->sensors[idx].attrs;
+}
+
+static void macsmc_hwmon_populate_fan_configs(u32 *configs, const struct macsmc_hwmon_fans *fans)
+{
+	int idx;
+
+	for (idx = 0; idx < fans->count; 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);
+		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);
+		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);
+		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);
+		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);
+		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 parse sensors\n");
+		return ret;
+	}
+
+	if (!hwmon->curr.count && !hwmon->fan.count &&
+	    !hwmon->power.count && !hwmon->temp.count &&
+	    !hwmon->volt.count) {
+		dev_err(hwmon->dev,
+			"No valid sensors found of any supported type\n");
+		return -ENODEV;
+	}
+
+	ret = macsmc_hwmon_create_infos(hwmon);
+	if (ret)
+		return ret;
+
+	hwmon->chip_info.ops = &macsmc_hwmon_ops;
+	hwmon->chip_info.info =
+		(const struct hwmon_channel_info *const *)&hwmon->channel_infos;
+
+	hwmon->hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev,
+								"macsmc_hwmon", hwmon,
+								&hwmon->chip_info, NULL);
+	if (IS_ERR(hwmon->hwmon_dev))
+		return dev_err_probe(hwmon->dev, PTR_ERR(hwmon->hwmon_dev),
+				     "Probing SMC hwmon device failed\n");
+
+	dev_dbg(hwmon->dev, "Registered SMC hwmon device. Sensors:\n");
+	dev_dbg(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",
+		.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");

-- 
2.51.0


^ permalink raw reply related

* [PATCH v4 05/11] mfd: macsmc: Add new __SMC_KEY macro
From: James Calligeros @ 2025-10-25  0:24 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
	Jonathan Corbet, James Calligeros
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, linux-doc
In-Reply-To: <20251025-macsmc-subdevs-v4-0-374d5c9eba0e@gmail.com>

When using the _SMC_KEY macro in switch/case statements, GCC 15.2.1 errors
out with 'case label does not reduce to an integer constant'. Introduce
a new __SMC_KEY macro that can be used instead.

Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 include/linux/mfd/macsmc.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/mfd/macsmc.h b/include/linux/mfd/macsmc.h
index 6b13f01a8592..f6f80c33b5cf 100644
--- a/include/linux/mfd/macsmc.h
+++ b/include/linux/mfd/macsmc.h
@@ -41,6 +41,7 @@ typedef u32 smc_key;
  */
 #define SMC_KEY(s) (smc_key)(_SMC_KEY(#s))
 #define _SMC_KEY(s) (((s)[0] << 24) | ((s)[1] << 16) | ((s)[2] << 8) | (s)[3])
+#define __SMC_KEY(a, b, c, d) (((u32)(a) << 24) | ((u32)(b) << 16) | ((u32)(c) << 8) | ((u32)(d)))
 
 #define APPLE_SMC_READABLE BIT(7)
 #define APPLE_SMC_WRITABLE BIT(6)

-- 
2.51.0


^ permalink raw reply related

* [PATCH v4 04/11] mfd: macsmc: Wire up Apple SMC RTC subdevice
From: James Calligeros @ 2025-10-25  0:24 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
	Jonathan Corbet, James Calligeros
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, linux-doc
In-Reply-To: <20251025-macsmc-subdevs-v4-0-374d5c9eba0e@gmail.com>

Add the new SMC RTC function to the mfd device

Reviewed-by: Neal Gompa <neal@gompa.dev>
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 e6cdae221f1d..500395bb48da 100644
--- a/drivers/mfd/macsmc.c
+++ b/drivers/mfd/macsmc.c
@@ -47,6 +47,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"),
 };
 
 static int apple_smc_cmd_locked(struct apple_smc *smc, u64 cmd, u64 arg,

-- 
2.51.0


^ permalink raw reply related

* [PATCH v4 03/11] rtc: Add new rtc-macsmc driver for Apple Silicon Macs
From: James Calligeros @ 2025-10-25  0:24 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
	Jonathan Corbet, James Calligeros
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, linux-doc, Hector Martin
In-Reply-To: <20251025-macsmc-subdevs-v4-0-374d5c9eba0e@gmail.com>

From: Hector Martin <marcan@marcan.st>

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.
On T600x machines, the RTC counter must be accessed via the SMC to
get full functionality, and it seems likely that future machines
will move towards making SMC handle all RTC functionality.

The SMC RTC counter access is implemented on all current machines
as of the time of this writing, on firmware 12.x. However, the RTC
offset (needed to set the time) is still only accessible via direct
PMU access. To handle this, we expose the RTC offset as an NVMEM
cell from the SPMI PMU device node, and this driver consumes that
cell and uses it to compute/set the current time.

Reviewed-by: Neal Gompa <neal@gompa.dev>
Signed-off-by: Hector Martin <marcan@marcan.st>
Signed-off-by: Sven Peter <sven@kernel.org>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 MAINTAINERS              |   1 +
 drivers/rtc/Kconfig      |  11 ++
 drivers/rtc/Makefile     |   1 +
 drivers/rtc/rtc-macsmc.c | 141 +++++++++++++++++++++++++
 4 files changed, 154 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 10f4c0034b5e..3c6322872dd1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2460,6 +2460,7 @@ F:	drivers/nvmem/apple-spmi-nvmem.c
 F:	drivers/pinctrl/pinctrl-apple-gpio.c
 F:	drivers/power/reset/macsmc-reboot.c
 F:	drivers/pwm/pwm-apple.c
+F:	drivers/rtc/rtc-macsmc.c
 F:	drivers/soc/apple/*
 F:	drivers/spi/spi-apple.c
 F:	drivers/spmi/spmi-apple-controller.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 4a8dc8d0a4b7..e165301d4abb 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -2078,6 +2078,17 @@ config RTC_DRV_WILCO_EC
 	  This can also be built as a module. If so, the module will
 	  be named "rtc_wilco_ec".
 
+config RTC_DRV_MACSMC
+	tristate "Apple Mac System Management Controller RTC"
+	depends on MFD_MACSMC
+	help
+	  If you say yes here you get support for RTC functions
+	  inside Apple SPMI PMUs accessed through the SoC's
+	  System Management Controller
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rtc-macsmc.
+
 config RTC_DRV_MSC313
 	tristate "MStar MSC313 RTC"
         depends on ARCH_MSTARV7 || COMPILE_TEST
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 610a9ee5fd33..32083bd5bb81 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_RTC_DRV_M48T35)	+= rtc-m48t35.o
 obj-$(CONFIG_RTC_DRV_M48T59)	+= rtc-m48t59.o
 obj-$(CONFIG_RTC_DRV_M48T86)	+= rtc-m48t86.o
 obj-$(CONFIG_RTC_DRV_MA35D1)	+= rtc-ma35d1.o
+obj-$(CONFIG_RTC_DRV_MACSMC)	+= rtc-macsmc.o
 obj-$(CONFIG_RTC_DRV_MAX31335)	+= rtc-max31335.o
 obj-$(CONFIG_RTC_DRV_MAX6900)	+= rtc-max6900.o
 obj-$(CONFIG_RTC_DRV_MAX6902)	+= rtc-max6902.o
diff --git a/drivers/rtc/rtc-macsmc.c b/drivers/rtc/rtc-macsmc.c
new file mode 100644
index 000000000000..05e360277f63
--- /dev/null
+++ b/drivers/rtc/rtc-macsmc.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * Apple SMC RTC driver
+ * Copyright The Asahi Linux Contributors
+ */
+
+#include <linux/bitops.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/macsmc.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+
+/* 48-bit RTC */
+#define RTC_BYTES 6
+#define RTC_BITS (8 * RTC_BYTES)
+
+/* 32768 Hz clock */
+#define RTC_SEC_SHIFT 15
+
+struct macsmc_rtc {
+	struct device *dev;
+	struct apple_smc *smc;
+	struct rtc_device *rtc_dev;
+	struct nvmem_cell *rtc_offset;
+};
+
+static int macsmc_rtc_get_time(struct device *dev, struct rtc_time *tm)
+{
+	struct macsmc_rtc *rtc = dev_get_drvdata(dev);
+	u64 ctr = 0, off = 0;
+	time64_t now;
+	void *p_off;
+	size_t len;
+	int ret;
+
+	ret = apple_smc_read(rtc->smc, SMC_KEY(CLKM), &ctr, RTC_BYTES);
+	if (ret < 0)
+		return ret;
+	if (ret != RTC_BYTES)
+		return -EIO;
+
+	p_off = nvmem_cell_read(rtc->rtc_offset, &len);
+	if (IS_ERR(p_off))
+		return PTR_ERR(p_off);
+	if (len < RTC_BYTES) {
+		kfree(p_off);
+		return -EIO;
+	}
+
+	memcpy(&off, p_off, RTC_BYTES);
+	kfree(p_off);
+
+	/* Sign extend from 48 to 64 bits, then arithmetic shift right 15 bits to get seconds */
+	now = sign_extend64(ctr + off, RTC_BITS - 1) >> RTC_SEC_SHIFT;
+	rtc_time64_to_tm(now, tm);
+
+	return ret;
+}
+
+static int macsmc_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct macsmc_rtc *rtc = dev_get_drvdata(dev);
+	u64 ctr = 0, off = 0;
+	int ret;
+
+	ret = apple_smc_read(rtc->smc, SMC_KEY(CLKM), &ctr, RTC_BYTES);
+	if (ret < 0)
+		return ret;
+	if (ret != RTC_BYTES)
+		return -EIO;
+
+	/* This sets the offset such that the set second begins now */
+	off = (rtc_tm_to_time64(tm) << RTC_SEC_SHIFT) - ctr;
+	return nvmem_cell_write(rtc->rtc_offset, &off, RTC_BYTES);
+}
+
+static const struct rtc_class_ops macsmc_rtc_ops = {
+	.read_time = macsmc_rtc_get_time,
+	.set_time = macsmc_rtc_set_time,
+};
+
+static int macsmc_rtc_probe(struct platform_device *pdev)
+{
+	struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
+	struct macsmc_rtc *rtc;
+
+	/*
+	 * MFD will probe this device even without a node in the device tree,
+	 * thus bail out early if the SMC on the current machines does not
+	 * support RTC and has no node in the device tree.
+	 */
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
+	if (!rtc)
+		return -ENOMEM;
+
+	rtc->dev = &pdev->dev;
+	rtc->smc = smc;
+
+	rtc->rtc_offset = devm_nvmem_cell_get(&pdev->dev, "rtc_offset");
+	if (IS_ERR(rtc->rtc_offset))
+		return dev_err_probe(&pdev->dev, PTR_ERR(rtc->rtc_offset),
+				     "Failed to get rtc_offset NVMEM cell\n");
+
+	rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev);
+	if (IS_ERR(rtc->rtc_dev))
+		return PTR_ERR(rtc->rtc_dev);
+
+	rtc->rtc_dev->ops = &macsmc_rtc_ops;
+	rtc->rtc_dev->range_min = S64_MIN >> (RTC_SEC_SHIFT + (64 - RTC_BITS));
+	rtc->rtc_dev->range_max = S64_MAX >> (RTC_SEC_SHIFT + (64 - RTC_BITS));
+
+	platform_set_drvdata(pdev, rtc);
+
+	return devm_rtc_register_device(rtc->rtc_dev);
+}
+
+static const struct of_device_id macsmc_rtc_of_table[] = {
+	{ .compatible = "apple,smc-rtc", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, macsmc_rtc_of_table);
+
+static struct platform_driver macsmc_rtc_driver = {
+	.driver = {
+		.name = "macsmc-rtc",
+		.of_match_table = macsmc_rtc_of_table,
+	},
+	.probe = macsmc_rtc_probe,
+};
+module_platform_driver(macsmc_rtc_driver);
+
+MODULE_LICENSE("Dual MIT/GPL");
+MODULE_DESCRIPTION("Apple SMC RTC driver");
+MODULE_AUTHOR("Hector Martin <marcan@marcan.st>");

-- 
2.51.0


^ permalink raw reply related

* [PATCH v4 02/11] dt-bindings: hwmon: Add Apple System Management Controller hwmon schema
From: James Calligeros @ 2025-10-25  0:24 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
	Jonathan Corbet, James Calligeros
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, linux-doc
In-Reply-To: <20251025-macsmc-subdevs-v4-0-374d5c9eba0e@gmail.com>

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.

Reviewed-by: Neal Gompa <neal@gompa.dev>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
---
 .../bindings/hwmon/apple,smc-hwmon.yaml  | 86 +++++++++++++++++++++++++
 .../bindings/mfd/apple,smc.yaml          | 36 +++++++++++
 MAINTAINERS                              |  1 +
 3 files changed, 123 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 000000000000..2eec317bc4b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
@@ -0,0 +1,86 @@
+# 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>
+
+$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
+
+properties:
+  compatible:
+    const: apple,smc-hwmon
+
+patternProperties:
+  "^current-[A-Za-z0-9]{4}$":
+    $ref: "#/$defs/sensor"
+    unevaluatedProperties: false
+
+  "^fan-[A-Za-z0-9]{4}$":
+    $ref: "#/$defs/sensor"
+    unevaluatedProperties: false
+
+    properties:
+      apple,fan-minimum:
+        $ref: /schemas/types.yaml#/definitions/string
+        pattern: "^[A-Za-z0-9]{4}$"
+        description: SMC key containing the fan's minimum speed
+
+      apple,fan-maximum:
+        $ref: /schemas/types.yaml#/definitions/string
+        pattern: "^[A-Za-z0-9]{4}$"
+        description: SMC key containing the fan's maximum speed
+
+      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 key to enable/disable manual fan control
+
+
+  "^power-[A-Za-z0-9]{4}$":
+    $ref: "#/$defs/sensor"
+    unevaluatedProperties: false
+
+  "^temperature-[A-Za-z0-9]{4}$":
+    $ref: "#/$defs/sensor"
+    unevaluatedProperties: false
+
+  "^voltage-[A-Za-z0-9]{4}$":
+    $ref: "#/$defs/sensor"
+    unevaluatedProperties: false
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
index 0410e712c900..34ce048619f5 100644
--- a/Documentation/devicetree/bindings/mfd/apple,smc.yaml
+++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
@@ -49,6 +49,9 @@ properties:
   rtc:
     $ref: /schemas/rtc/apple,smc-rtc.yaml
 
+  hwmon:
+    $ref: /schemas/hwmon/apple,smc-hwmon.yaml
+
 additionalProperties: false
 
 required:
@@ -89,5 +92,38 @@ examples:
           nvmem-cells = <&rtc_offset>;
           nvmem-cell-names = "rtc_offset";
        };
+
+        hwmon {
+          compatible = "apple,smc-hwmon";
+
+          current-ID0R {
+            apple,key-id = "ID0R";
+            label = "AC Input Current";
+          };
+
+          fan-F0Ac {
+            apple,key-id = "F0Ac";
+            apple,fan-minimum = "F0Mn";
+            apple,fan-maximum = "F0Mx";
+            apple,fan-target = "F0Tg";
+            apple,fan-mode = "F0Md";
+            label = "Fan 1";
+          };
+
+          power-PSTR {
+            apple,key-id = "PSTR";
+            label = "Total System Power";
+          };
+
+          temperature-TW0P {
+            apple,key-id = "TW0P";
+            label = "WiFi/BT Module Temperature";
+          };
+
+          voltage-VD0R {
+            apple,key-id = "VD0R";
+            label = "AC Input Voltage";
+          };
+        };
       };
     };
diff --git a/MAINTAINERS b/MAINTAINERS
index 745d85b66365..10f4c0034b5e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2418,6 +2418,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,*

-- 
2.51.0


^ permalink raw reply related

* [PATCH v4 01/11] dt-bindings: rtc: Add Apple SMC RTC
From: James Calligeros @ 2025-10-25  0:24 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
	Jonathan Corbet, James Calligeros
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, linux-doc, Mark Kettenis
In-Reply-To: <20251025-macsmc-subdevs-v4-0-374d5c9eba0e@gmail.com>

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>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
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(+)

diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
index 5429538f7e2e..0410e712c900 100644
--- a/Documentation/devicetree/bindings/mfd/apple,smc.yaml
+++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
@@ -46,6 +46,9 @@ properties:
   reboot:
     $ref: /schemas/power/reset/apple,smc-reboot.yaml
 
+  rtc:
+    $ref: /schemas/rtc/apple,smc-rtc.yaml
+
 additionalProperties: false
 
 required:
@@ -80,5 +83,11 @@ examples:
           nvmem-cell-names = "shutdown_flag", "boot_stage",
                              "boot_error_count", "panic_count";
         };
+
+        rtc {
+          compatible = "apple,smc-rtc";
+          nvmem-cells = <&rtc_offset>;
+          nvmem-cell-names = "rtc_offset";
+       };
       };
     };
diff --git a/Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml b/Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml
new file mode 100644
index 000000000000..607b610665a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/apple,smc-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple SMC RTC
+
+description:
+  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.
+
+maintainers:
+  - Sven Peter <sven@kernel.org>
+
+properties:
+  compatible:
+    const: apple,smc-rtc
+
+  nvmem-cells:
+    items:
+      - description: 48bit RTC offset, specified in 32768 (2^15) Hz clock ticks
+
+  nvmem-cell-names:
+    items:
+      - const: rtc_offset
+
+required:
+  - compatible
+  - nvmem-cells
+  - nvmem-cell-names
+
+additionalProperties: false
diff --git a/MAINTAINERS b/MAINTAINERS
index b45db73e55df..745d85b66365 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2435,6 +2435,7 @@ F:	Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
 F:	Documentation/devicetree/bindings/power/apple*
 F:	Documentation/devicetree/bindings/power/reset/apple,smc-reboot.yaml
 F:	Documentation/devicetree/bindings/pwm/apple,s5l-fpwm.yaml
+F:	Documentation/devicetree/bindings/rtc/apple,smc-rtc.yaml
 F:	Documentation/devicetree/bindings/spi/apple,spi.yaml
 F:	Documentation/devicetree/bindings/spmi/apple,spmi.yaml
 F:	Documentation/devicetree/bindings/watchdog/apple,wdt.yaml

-- 
2.51.0


^ permalink raw reply related

* [PATCH v4 00/11] mfd: macsmc: add rtc, hwmon and hid subdevices
From: James Calligeros @ 2025-10-25  0:24 UTC (permalink / raw)
  To: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
	Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Alexandre Belloni, Jean Delvare, Guenter Roeck, Dmitry Torokhov,
	Jonathan Corbet, James Calligeros
  Cc: asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
	linux-hwmon, linux-input, linux-doc, Mark Kettenis, Hector Martin

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.

The RTC driver is responsible for getting and setting the system clock,
and requires an NVMEM cell. This series replaces Sven's original RTC driver
submission [1].

The hwmon function is an interesting one. While each Apple Silicon device
exposes pretty similar sets of sensors, these all seem to be paired to
different SMC keys in the firmware interface. This is true even when the
sensors are on the SoC. For example, an M1 MacBook Pro will use different
keys to access the LITTLE core temperature sensors to an M1 Mac mini. This
necessitates describing which keys correspond to which sensors for each
device individually, and populating the hwmon structs at runtime. We do
this with a node in the device tree. This series includes only the keys
for sensors which we know to be common to all devices. The SMC is also
responsible for monitoring and controlling fan speeds on systems with fans,
which we expose via the hwmon driver.

The SMC also handles the hardware power button and lid switch. Power
button presses and lid opening/closing are emitted as HID events, so we
add an input subdevice to handle them.

Since there are no real dependencies between the components of this series,
it should be fine for each subsystem to take the relevant patches through
their trees. The mfd one-liners should be taken in order to avoid trivial
conflicts.

Regards,

James

[1] https://lore.kernel.org/asahi/CAEg-Je84XxLWH7vznQmPRfjf6GxWOu75ZetwN7AdseAwfMLLrQ@mail.gmail.com/T/#t

---
Changes in v4:
- Added Rob's R-b to hwmon Devicetree schema
- Added missing include to hwmon driver
- Dropped superfluous dev_info() from hwmon probe
- Added Guenter's A-b to hwmon driver
- Renamed INPUT_MACSMC_INPUT to INPUT_MACSMC
- Dropped leftover mention of HID in input driver
- Reinstated input driver MODULE_ALIAS
- Trigger a hard wakeup on power button/lid switch when coming out of
  s2idle
- Suppress KEY_POWER event on wakeup to prevent an immediate shutdown
  when waking up
- Squashed hwmon Devicetree commits into one
- Link to v3: https://lore.kernel.org/r/20251007-macsmc-subdevs-v3-0-d7d3bfd7ae02@gmail.com

Changes in v3:
- Renamed macsmc-hid to macsmc-input
- Switched to pm_wakeup_event in macsmc-input
- macsmc-input now configures its capabilities before registering the device
- Renamed macsmc_hwmon to macsmc-hwmon
- Dropped module aliases in macsmc-input and macsmc_hwmon
- Introduced new SMC FourCC macro to silence GCC errors
- Condensed hwmon binding using $defs
- Made label property optional for hwmon sensors
- Fixed incorrect hwmon is_visible implementation
- Dropped 64-bit math from SMC float ops
- Fixed incorrect use of error numbers in hwmon driver
- Replaced a number of non-fatal dev_errs with dev_dbgs in hwmon driver
- Added hwmon driver documentation
- Added hwmon subdevice directly to the DT SMC node
- Included "common" hwmon sensors in SoC .dtsi files
- Fixed typo in hwmon-common.dtsi
- Added Neal's R-b to series
- Added required nodes to t602x Devicetrees
- Link to v2: https://lore.kernel.org/r/20250827-macsmc-subdevs-v2-0-ce5e99d54c28@gmail.com

Changes in v2:
- Added Rob's R-b tag to RTC DT binding
- Removed redundant nesting from hwmon DT binding
- Dedpulicated property definitions in hwmon DT schema
- Made label a required property for hwmon DT nodes
- Clarified semantics in hwmon DT schema definitions
- Split mfd tree changes into separate commits
- Fixed numerous style errors in hwmon driver
- Removed log messages sysfs read/write functions in hwmon driver
- Removed ignored errors from hwmon driver
- Removed uses of dev_err for non-errors in hwmon driver
- Made it more obvious that a number of hwmon fan properties are optional
- Modified hwmon driver to reflect DT schema changes
- Added compatible property to hwmon node
- Link to v1: https://lore.kernel.org/r/20250819-macsmc-subdevs-v1-0-57df6c3e5f19@gmail.com

---
Hector Martin (2):
      rtc: Add new rtc-macsmc driver for Apple Silicon Macs
      input: macsmc-input: New driver to handle the Apple Mac SMC buttons/lid

James Calligeros (7):
      dt-bindings: hwmon: Add Apple System Management Controller hwmon schema
      mfd: macsmc: Wire up Apple SMC RTC subdevice
      mfd: macsmc: Add new __SMC_KEY macro
      hwmon: Add Apple Silicon SMC hwmon driver
      mfd: macsmc: Wire up Apple SMC hwmon subdevice
      mfd: macsmc: Wire up Apple SMC input subdevice
      arm64: dts: apple: t8103, t8112, t60xx: Add hwmon SMC subdevice

Sven Peter (2):
      dt-bindings: rtc: Add Apple SMC RTC
      arm64: dts: apple: t8103,t60xx,t8112: Add SMC RTC node

 .../bindings/hwmon/apple,smc-hwmon.yaml  |  86 +++
 .../bindings/mfd/apple,smc.yaml          |  45 ++
 .../bindings/rtc/apple,smc-rtc.yaml      |  35 +
 Documentation/hwmon/macsmc-hwmon.rst     |  71 +++
 MAINTAINERS                              |   6 +
 .../boot/dts/apple/hwmon-common.dtsi     |  33 +
 .../boot/dts/apple/hwmon-fan-dual.dtsi   |  22 +
 arch/arm64/boot/dts/apple/hwmon-fan.dtsi |  17 +
 .../boot/dts/apple/hwmon-laptop.dtsi     |  33 +
 .../boot/dts/apple/hwmon-mac-mini.dtsi   |  15 +
 .../arm64/boot/dts/apple/t6001-j375c.dts |   2 +
 arch/arm64/boot/dts/apple/t6001.dtsi     |   2 +
 .../arm64/boot/dts/apple/t6002-j375d.dts |   2 +
 .../arm64/boot/dts/apple/t600x-die0.dtsi |  10 +
 .../boot/dts/apple/t600x-j314-j316.dtsi  |   3 +
 .../arm64/boot/dts/apple/t602x-die0.dtsi |  10 +
 arch/arm64/boot/dts/apple/t8103-j274.dts |   2 +
 arch/arm64/boot/dts/apple/t8103-j293.dts |   3 +
 arch/arm64/boot/dts/apple/t8103-j313.dts |   2 +
 arch/arm64/boot/dts/apple/t8103-j456.dts |   2 +
 arch/arm64/boot/dts/apple/t8103-j457.dts |   2 +
 arch/arm64/boot/dts/apple/t8103.dtsi     |  11 +
 arch/arm64/boot/dts/apple/t8112-j413.dts |   2 +
 arch/arm64/boot/dts/apple/t8112-j473.dts |   2 +
 arch/arm64/boot/dts/apple/t8112-j493.dts |   3 +
 arch/arm64/boot/dts/apple/t8112.dtsi     |  11 +
 drivers/hwmon/Kconfig                    |  12 +
 drivers/hwmon/Makefile                   |   1 +
 drivers/hwmon/macsmc-hwmon.c             | 851 +++++++++++++++++++++++++
 drivers/input/misc/Kconfig               |  11 +
 drivers/input/misc/Makefile              |   1 +
 drivers/input/misc/macsmc-input.c        | 208 ++++++
 drivers/mfd/macsmc.c                     |   3 +
 drivers/rtc/Kconfig                      |  11 +
 drivers/rtc/Makefile                     |   1 +
 drivers/rtc/rtc-macsmc.c                 | 141 ++++
 include/linux/mfd/macsmc.h               |   1 +
 37 files changed, 1673 insertions(+)
---
base-commit: c746c3b5169831d7fb032a1051d8b45592ae8d78
change-id: 20250816-macsmc-subdevs-87032c017d0c

Best regards,
-- 
James Calligeros <jcalligeros99@gmail.com>


^ permalink raw reply

* Re: [PATCH] dt-bindings: Remove extra blank lines
From: Sebastian Reichel @ 2025-10-24 23:24 UTC (permalink / raw)
  To: Rob Herring (Arm)
  Cc: Krzysztof Kozlowski, Conor Dooley, Stephen Boyd, David Airlie,
	Simona Vetter, Maarten Lankhorst, Thomas Zimmermann,
	Andrzej Hajda, Robert Foss, Vinod Koul, Moritz Fischer, Xu Yilun,
	Bartosz Golaszewski, Guenter Roeck, Andi Shyti, Jonathan Cameron,
	Dmitry Torokhov, Georgi Djakov, Thomas Gleixner, Joerg Roedel,
	Jassi Brar, Mauro Carvalho Chehab, Lee Jones, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Johannes Berg, Krzysztof Wilczyński, Manivannan Sadhasivam,
	Bjorn Helgaas, Kishon Vijay Abraham I, Uwe Kleine-König,
	Mark Brown, Mathieu Poirier, Philipp Zabel, Olivia Mackall,
	Herbert Xu, Daniel Lezcano, Greg Kroah-Hartman, devicetree,
	linux-kernel, linux-clk, dri-devel, linux-fbdev, dmaengine,
	linux-fpga, linux-gpio, linux-hwmon, linux-i2c, linux-iio,
	linux-input, linux-pm, iommu, linux-media, linux-mtd, netdev,
	linux-wireless, linux-pci, linux-phy, linux-pwm, linux-remoteproc,
	linux-crypto, linux-sound, linux-usb
In-Reply-To: <20251023143957.2899600-1-robh@kernel.org>

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

Hi,

On Thu, Oct 23, 2025 at 09:37:56AM -0500, Rob Herring (Arm) wrote:
> Generally at most 1 blank line is the standard style for DT schema
> files. Remove the few cases with more than 1 so that the yamllint check
> for this can be enabled.
> 
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> ---
>  .../devicetree/bindings/power/supply/mt6360_charger.yaml     | 1 -
>  .../bindings/power/supply/stericsson,ab8500-charger.yaml     | 1 -

Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v2] HID: quirks: Add device descriptor for 4c4a:4155
From: Terry Junge @ 2025-10-24 22:28 UTC (permalink / raw)
  To: zhangheng, jikos, bentiss, staffan.melin
  Cc: linux-input, linux-kernel, 1114557, stable
In-Reply-To: <8f0155d4-72a7-45ec-a272-7892e783bbed@kylinos.cn>

Hi Zhang,

Since the manufacturer has verified that all the microphones have the 
same serial number you could add the test for that as well if you want.

The report descriptor is clearly for a keyboard/keypad and should be 
ignored.

In any case, the current v3 attached patch does resolve the conflict.

Reviewed-by: Terry Junge <linuxhid@cosmicgizmosystems.com>

On 10/23/2025 8:32 PM, zhangheng wrote:
> Hi Terry Junge,
> 
> I have made the changes as per your suggestion.
> mic.txt is the microphone report descriptor and is working properly.
> 
> 
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox