* [PATCH v3 3/4] firmware: xilinx: Add support for Xilsem scan operations
From: Rama devi Veggalam @ 2026-06-24 21:25 UTC (permalink / raw)
To: bp, tony.luck, michal.simek, robh, krzk+dt, conor+dt
Cc: linux-kernel, linux-edac, devicetree, james.morse, mchehab, rric,
git, Rama devi Veggalam
In-Reply-To: <20260624212545.2850787-1-rama.devi.veggalam@amd.com>
Add the ATF EEMI call support for Xilsem scan operations.
Initialize, start, stop scan, error inject, read configuration,
status and register for software error events.
Add macros for XilSem correctable and uncorrectable error events.
Signed-off-by: Rama devi Veggalam <rama.devi.veggalam@amd.com>
---
Changes in v3:
- created separate file for Xilsem ATF EEMI calls.
Changes in v2:
- Patch created on top of dependent patch series
"enhance zynqmp_pm_get_family_info()"
- Removed non-relevant SOB names in error event header files
- Updated copyright information
- Merged Versal and Versal NET error event definitions to firmware
patch
---
drivers/firmware/xilinx/Makefile | 2 +-
drivers/firmware/xilinx/zynqmp-sem.c | 176 ++++++++++++++++++
drivers/soc/xilinx/xlnx_event_manager.c | 6 +-
.../linux/firmware/xlnx-versal-error-events.h | 43 +++++
include/linux/firmware/xlnx-zynqmp-sem.h | 69 +++++++
include/linux/firmware/xlnx-zynqmp.h | 1 +
6 files changed, 294 insertions(+), 3 deletions(-)
create mode 100644 drivers/firmware/xilinx/zynqmp-sem.c
create mode 100644 include/linux/firmware/xlnx-versal-error-events.h
create mode 100644 include/linux/firmware/xlnx-zynqmp-sem.h
diff --git a/drivers/firmware/xilinx/Makefile b/drivers/firmware/xilinx/Makefile
index 8db0e66b6b7e..f9380c8931ae 100644
--- a/drivers/firmware/xilinx/Makefile
+++ b/drivers/firmware/xilinx/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for Xilinx firmwares
-obj-$(CONFIG_ZYNQMP_FIRMWARE) += zynqmp.o zynqmp-ufs.o zynqmp-crypto.o
+obj-$(CONFIG_ZYNQMP_FIRMWARE) += zynqmp.o zynqmp-sem.o zynqmp-ufs.o zynqmp-crypto.o
obj-$(CONFIG_ZYNQMP_FIRMWARE_DEBUG) += zynqmp-debug.o
diff --git a/drivers/firmware/xilinx/zynqmp-sem.c b/drivers/firmware/xilinx/zynqmp-sem.c
new file mode 100644
index 000000000000..5cd399b165f5
--- /dev/null
+++ b/drivers/firmware/xilinx/zynqmp-sem.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Firmware layer for XilSEM APIs.
+ *
+ * Copyright (C), 2026 Advanced Micro Devices, Inc.
+ */
+
+#include <linux/export.h>
+#include <linux/firmware/xlnx-zynqmp.h>
+
+/**
+ * zynqmp_pm_xilsem_cntrl_ops - PM call to perform XilSEM operations
+ * @cmd: Command for XilSEM scan control operations
+ * @slrid: SLR id on which scan operation to be done
+ * @response: Output response (command header, error code or status, slr id)
+ *
+ * Return: Returns 0 on success or error value on failure.
+ */
+int zynqmp_pm_xilsem_cntrl_ops(u32 cmd, u32 slrid, u32 *const response)
+{
+ u32 ret_buf[PAYLOAD_ARG_CNT];
+ int ret;
+
+ ret = zynqmp_pm_invoke_fn(PM_XSEM_HEADER | cmd, ret_buf, 1, slrid);
+ response[0] = ret_buf[1];
+ response[1] = ret_buf[2];
+ response[2] = ret_buf[3];
+ response[3] = ret_buf[4];
+ response[4] = ret_buf[5];
+ response[5] = ret_buf[6];
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_xilsem_cntrl_ops);
+
+/**
+ * zynqmp_pm_xilsem_cram_errinj - PM call to perform CRAM error injection
+ * @slrid: SLR id to inject error in CRAM
+ * @frame: Frame number to be used for error injection
+ * @qword: Word number to be used for error injection
+ * @bit: Bit location to be used for error injection
+ * @row: CFRAME row number to be used for error injection
+ * @response: Output response (command header, error code or status, slr id)
+ *
+ * Return: Returns 0 on success or error value on failure.
+ */
+int zynqmp_pm_xilsem_cram_errinj(u32 slrid, u32 frame, u32 qword, u32 bit, u32 row,
+ u32 *const response)
+{
+ u32 ret_buf[PAYLOAD_ARG_CNT];
+ int ret;
+
+ ret = zynqmp_pm_invoke_fn(PM_XSEM_CRAM_ERRINJ, ret_buf, 5, slrid, frame, qword, bit, row);
+ response[0] = ret_buf[1];
+ response[1] = ret_buf[2];
+ response[2] = ret_buf[3];
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_xilsem_cram_errinj);
+
+/**
+ * zynqmp_pm_xilsem_cram_readecc - PM call to perform CFRAME ECC read
+ * @slrid: SLR id on which Frame ECC read to be done
+ * @frame: Frame number to be used for reading ECC
+ * @row: CFRAME row number to be used for reading ECC
+ * @response: Output response (Frame ecc header, ECC values, status)
+ *
+ * Return: Returns 0 on success or error value on failure.
+ */
+int zynqmp_pm_xilsem_cram_readecc(u32 slrid, u32 frame, u32 row, u32 *const response)
+{
+ u32 ret_buf[PAYLOAD_ARG_CNT];
+ int ret;
+
+ ret = zynqmp_pm_invoke_fn(PM_XSEM_CRAM_RD_ECC, ret_buf, 3, slrid, frame, row);
+ response[0] = ret_buf[1];
+ response[1] = ret_buf[2];
+ response[2] = ret_buf[3];
+ response[3] = ret_buf[4];
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_xilsem_cram_readecc);
+
+/**
+ * zynqmp_pm_xilsem_read_cfg - PM call to perform Xilsem configuration read
+ * @slrid: SLR id for which configuration to be read
+ * @response: Output response (config header, Xilsem config, status)
+ *
+ * Return: Returns 0 on success or error value on failure.
+ */
+int zynqmp_pm_xilsem_read_cfg(u32 slrid, u32 *const response)
+{
+ u32 ret_buf[PAYLOAD_ARG_CNT];
+ int ret;
+
+ ret = zynqmp_pm_invoke_fn(PM_XSEM_RD_CONFIG, ret_buf, 1, slrid);
+ response[0] = ret_buf[1];
+ response[1] = ret_buf[2];
+ response[2] = ret_buf[3];
+ response[3] = ret_buf[4];
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_xilsem_read_cfg);
+
+/**
+ * zynqmp_pm_xilsem_read_ssit_status - PM call to perform Xilsem SSIT status
+ * @slrid: SLR id for which ECC read to be done
+ * @bufaddr: Buffer address to get the status information
+ * @response: Output response (status read header, slr id)
+ *
+ * Return: Returns 0 on success or error value on failure.
+ */
+int zynqmp_pm_xilsem_read_ssit_status(u32 slrid, u32 bufaddr, u32 *const response)
+{
+ u32 ret_buf[PAYLOAD_ARG_CNT];
+ int ret;
+
+ ret = zynqmp_pm_invoke_fn(PM_XSEM_SSIT_RD_STS, ret_buf, 2, slrid, bufaddr);
+ response[0] = ret_buf[1];
+ response[1] = ret_buf[2];
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_xilsem_read_ssit_status);
+
+/**
+ * zynqmp_pm_xilsem_cram_getcrc - PM call to perform CRAM Row CRC read
+ * @slrid: SLR id for which CRC read to be done
+ * @rowindex: CFRAME row number to be used for reading CRC
+ * @response: Output response (Get CRC header, CRC values, status)
+ *
+ * Return: Returns 0 on success or error value on failure.
+ */
+int zynqmp_pm_xilsem_cram_getcrc(u32 slrid, u32 rowindex, u32 *const response)
+{
+ u32 ret_buf[PAYLOAD_ARG_CNT];
+ int ret;
+
+ ret = zynqmp_pm_invoke_fn(PM_XSEM_SSIT_GET_CRC, ret_buf, 2, slrid, rowindex);
+ response[0] = ret_buf[1];
+ response[1] = ret_buf[2];
+ response[2] = ret_buf[3];
+ response[3] = ret_buf[4];
+ response[4] = ret_buf[5];
+ response[5] = ret_buf[6];
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_xilsem_cram_getcrc);
+
+/**
+ * zynqmp_pm_xilsem_cram_ssit_totframes - PM call to perform total frames read
+ * @slrid: SLR id for which total frames read to be done
+ * @row: CFRAME row number to be used for reading ECC
+ * @framecnt: Buffer address to get toral frames data
+ * @response: Output response (Total frames header, slr id, row, status)
+ *
+ * Return: Returns 0 on success or error value on failure.
+ */
+int zynqmp_pm_xilsem_cram_ssit_totframes(u32 slrid, u32 row, u32 framecnt, u32 *const response)
+{
+ u32 ret_buf[PAYLOAD_ARG_CNT];
+ int ret;
+
+ ret = zynqmp_pm_invoke_fn(PM_XSEM_SSIT_GET_FRAMES, ret_buf, 3, slrid, row, framecnt);
+ response[0] = ret_buf[1];
+ response[1] = ret_buf[2];
+ response[2] = ret_buf[3];
+ response[3] = ret_buf[4];
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_xilsem_cram_ssit_totframes);
diff --git a/drivers/soc/xilinx/xlnx_event_manager.c b/drivers/soc/xilinx/xlnx_event_manager.c
index f733dc42b3b1..fb820fb15173 100644
--- a/drivers/soc/xilinx/xlnx_event_manager.c
+++ b/drivers/soc/xilinx/xlnx_event_manager.c
@@ -3,12 +3,13 @@
* Xilinx Event Management Driver
*
* Copyright (C) 2021 Xilinx, Inc.
- * Copyright (C) 2024 Advanced Micro Devices, Inc.
+ * Copyright (C) 2024-2026 Advanced Micro Devices, Inc.
*
* Abhyuday Godhasara <abhyuday.godhasara@xilinx.com>
*/
#include <linux/cpuhotplug.h>
+#include <linux/firmware/xlnx-versal-error-events.h>
#include <linux/firmware/xlnx-event-manager.h>
#include <linux/firmware/xlnx-zynqmp.h>
#include <linux/hashtable.h>
@@ -85,7 +86,8 @@ static bool xlnx_is_error_event(const u32 node_id)
if (node_id == VERSAL_EVENT_ERROR_PMC_ERR1 ||
node_id == VERSAL_EVENT_ERROR_PMC_ERR2 ||
node_id == VERSAL_EVENT_ERROR_PSM_ERR1 ||
- node_id == VERSAL_EVENT_ERROR_PSM_ERR2)
+ node_id == VERSAL_EVENT_ERROR_PSM_ERR2 ||
+ node_id == VERSAL_EVENT_ERROR_SW_ERR)
return true;
} else if (pm_family_code == PM_VERSAL_NET_FAMILY_CODE) {
if (node_id == VERSAL_NET_EVENT_ERROR_PMC_ERR1 ||
diff --git a/include/linux/firmware/xlnx-versal-error-events.h b/include/linux/firmware/xlnx-versal-error-events.h
new file mode 100644
index 000000000000..4767a23c9e4d
--- /dev/null
+++ b/include/linux/firmware/xlnx-versal-error-events.h
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Xilinx Versal Error Event Node IDs and Error Event Mask.
+ * Use with Xilinx Event Management Driver
+ *
+ * Copyright (C) 2021-2022 Xilinx
+ * Copyright (C) 2023-2026 Advanced Micro Devices, Inc.
+ *
+ */
+
+#ifndef _FIRMWARE_XLNX_VERSAL_ERROR_EVENTS_H_
+#define _FIRMWARE_XLNX_VERSAL_ERROR_EVENTS_H_
+
+/*
+ * Error Event Node Ids
+ */
+#define VERSAL_EVENT_ERROR_PMC_ERR1 (0x28100000U)
+#define VERSAL_EVENT_ERROR_PMC_ERR2 (0x28104000U)
+#define VERSAL_EVENT_ERROR_PSM_ERR1 (0x28108000U)
+#define VERSAL_EVENT_ERROR_PSM_ERR2 (0x2810C000U)
+#define VERSAL_EVENT_ERROR_SW_ERR (0x28110000U)
+
+/*
+ * XPM_VERSAL_EVENT_ERROR_MASK_XSEM_CRAM_CE_5: Error event mask for handling
+ * correctable error in Versal Configuration RAM which is reported by
+ * Soft Error Mitigation (XilSEM).
+ */
+#define XPM_VERSAL_EVENT_ERROR_MASK_XSEM_CRAM_CE_5 BIT(5)
+
+/**
+ * XPM_VERSAL_EVENT_ERROR_MASK_XSEM_CRAM_UE_6: Error event mask for handling
+ * uncorrectable error in Versal Configuration RAM which is reported by
+ * Soft Error Mitigation (XilSEM).
+ */
+#define XPM_VERSAL_EVENT_ERROR_MASK_XSEM_CRAM_UE_6 BIT(6)
+
+/**
+ * XPM_VERSAL_EVENT_ERROR_MASK_XSEM_NPI_UE_7: Error event mask for handling
+ * uncorrectable error in Versal NoC programming interface (NPI)
+ * register which is reported by Soft Error Mitigation (XilSEM).
+ */
+#define XPM_VERSAL_EVENT_ERROR_MASK_XSEM_NPI_UE_7 BIT(7)
+#endif /* _FIRMWARE_XLNX_VERSAL_ERROR_EVENTS_H_ */
diff --git a/include/linux/firmware/xlnx-zynqmp-sem.h b/include/linux/firmware/xlnx-zynqmp-sem.h
new file mode 100644
index 000000000000..722849cc6403
--- /dev/null
+++ b/include/linux/firmware/xlnx-zynqmp-sem.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Firmware layer for XilSEM APIs.
+ *
+ * Copyright (C), 2026 Advanced Micro Devices, Inc.
+ */
+
+#ifndef __FIRMWARE_ZYNQMP_SEM_H__
+#define __FIRMWARE_ZYNQMP_SEM_H__
+
+/* XilSEM commands */
+#define PM_XSEM_HEADER 0x300
+#define PM_XSEM_CRAM_ERRINJ 0x304
+#define PM_XSEM_RD_CONFIG 0x309
+#define PM_XSEM_CRAM_RD_ECC 0x30B
+#define PM_XSEM_SSIT_GET_CRC 0x30C
+#define PM_XSEM_SSIT_RD_STS 0x30D
+#define PM_XSEM_SSIT_GET_FRAMES 0x30E
+
+#if IS_REACHABLE(CONFIG_ZYNQMP_FIRMWARE)
+int zynqmp_pm_xilsem_cntrl_ops(u32 cmd, u32 slrid, u32 *const response);
+int zynqmp_pm_xilsem_cram_errinj(u32 slrid, u32 frame, u32 qword, u32 bit, u32 row,
+ u32 *const response);
+int zynqmp_pm_xilsem_cram_readecc(u32 slrid, u32 frame, u32 row, u32 *const response);
+int zynqmp_pm_xilsem_read_cfg(u32 slrid, u32 *const response);
+int zynqmp_pm_xilsem_read_ssit_status(u32 slrid, u32 bufaddr, u32 *const response);
+int zynqmp_pm_xilsem_cram_getcrc(u32 slrid, u32 rowindex, u32 *const response);
+int zynqmp_pm_xilsem_cram_ssit_totframes(u32 slrid, u32 row, u32 framecnt,
+ u32 *const response);
+#else
+static inline int zynqmp_pm_xilsem_cntrl_ops(u32 cmd, u32 slrid, u32 *const response)
+{
+ return -ENODEV;
+}
+
+static inline int zynqmp_pm_xilsem_cram_errinj(u32 slrid, u32 frame, u32 qword, u32 bit, u32 row,
+ u32 *const response)
+{
+ return -ENODEV;
+}
+
+static inline int zynqmp_pm_xilsem_cram_readecc(u32 slrid, u32 frame, u32 row, u32 *const response)
+{
+ return -ENODEV;
+}
+
+static inline int zynqmp_pm_xilsem_read_cfg(u32 slrid, u32 *const response)
+{
+ return -ENODEV;
+}
+
+static inline int zynqmp_pm_xilsem_read_ssit_status(u32 slrid, u32 bufaddr, u32 *const response)
+{
+ return -ENODEV;
+}
+
+static inline int zynqmp_pm_xilsem_cram_getcrc(u32 slrid, u32 rowindex, u32 *const response)
+{
+ return -ENODEV;
+}
+
+static inline int zynqmp_pm_xilsem_cram_ssit_totframes(u32 slrid, u32 row, u32 framecnt,
+ u32 *const response)
+{
+ return -ENODEV;
+}
+#endif
+
+#endif /* __FIRMWARE_ZYNQMP_SEM_H__ */
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 7e27b0f7bf7e..f5808f39c7a6 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -18,6 +18,7 @@
#include <linux/err.h>
#include <linux/firmware/xlnx-zynqmp-ufs.h>
#include <linux/firmware/xlnx-zynqmp-crypto.h>
+#include <linux/firmware/xlnx-zynqmp-sem.h>
#define ZYNQMP_PM_VERSION_MAJOR 1
#define ZYNQMP_PM_VERSION_MINOR 0
--
2.23.0
^ permalink raw reply related
* [PATCH v3 2/4] Documentation: ABI: Add ABI doc for versal edac sysfs
From: Rama devi Veggalam @ 2026-06-24 21:25 UTC (permalink / raw)
To: bp, tony.luck, michal.simek, robh, krzk+dt, conor+dt
Cc: linux-kernel, linux-edac, devicetree, james.morse, mchehab, rric,
git, Rama devi Veggalam
In-Reply-To: <20260624212545.2850787-1-rama.devi.veggalam@amd.com>
Add documentation for the sysfs entries created for
versal edac (XilSEM).
Signed-off-by: Rama devi Veggalam <rama.devi.veggalam@amd.com>
---
Changes in v3:
- Renamed file name from xilsem edac to versal edac
Changes in v2:
- Updated Date field in sysfs file
---
.../ABI/testing/sysfs-driver-versal-edac | 303 ++++++++++++++++++
1 file changed, 303 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-driver-versal-edac
diff --git a/Documentation/ABI/testing/sysfs-driver-versal-edac b/Documentation/ABI/testing/sysfs-driver-versal-edac
new file mode 100644
index 000000000000..c35d864fcbe2
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-versal-edac
@@ -0,0 +1,303 @@
+What: /sys/devices/system/edac/versal_xilsem/xsem_scan_control
+Date: What: /sys/devices/system/edac/mc/mc0/xsem_scan_control
+Date: June 2026
+Contact: rama.devi.veggalam@amd.com
+Description:
+ It is a read/write file.
+ Writing to this file causes the software to initiate a
+ request to the firmware for doing requested scan operation in
+ Configuration RAM (CRAM) or NPI of Versal mono/SSIT devices.
+ User needs to provide scan operation id (init, start, stop) and
+ SLR id (Versal SSIT device) details. For Versal mono devices,
+ the SLR id should be 0.
+ The scan operation id values are as given below:
+ 1 - Initialize the scan
+ 2 - Start CRAM scan
+ 3 - Stop CRAM scan
+ 5 - Start NPI scan
+ 6 - Stop NPI scan
+ 7 - Inject NPI error in first descriptor
+
+ When read, it shows the current scan status with error code.
+ The format is <0x1030 | operation Id> <error code> <Slr ID>
+ The different error codes are as given below:
+ ========== =====
+ Error Code Cause
+ ========== =====
+ 0x0 Scan operation success
+ 0x1 Failure in NPI scan
+ 0x80 Calibration timeout
+ 0x2000 Internal error
+ 0x500000 CRAM initialization not yet done
+ 0x600000 Start scan failed
+ 0x700000 Stop scan failed
+ 0xF00000 Active CRC/UE error
+ 0x1000000 ECC/CRC error detected during calibration
+ ========== =====
+
+What: /sys/devices/system/edac/mc/mc0/xsem_cram_injecterr
+Date: June 2026
+Contact: rama.devi.veggalam@amd.com
+Description:
+ It is a read/write file.
+ Writing to this file causes the software to initiate a
+ request to the firmware for doing error injection in
+ Configuration RAM (CRAM) of Versal mono/SSIT devices.
+ User needs to provide the location details of CRAM
+ (frame, qword, bit number, row number) and
+ SLR id (Versal SSIT device) details to inject the error.
+ For Versal mono devices, the SLR id should be 0..
+ When read, it shows the current error injection status. The
+ format is <header> <error code> <Slr ID>
+ Example: 0x10304 0
+ The different error codes are as given below:
+ ========== =====
+ Error Code Cause
+ ========== =====
+ 0x0 Error injection success
+ 0x2000 Internal NULL pointer error
+ 0x500000 CRAM initialization not yet done
+ 0x800000 Invalid row
+ 0x900000 Invalid qword
+ 0xA00000 Invalid bit
+ 0xB00000 Invalid frame address
+ 0xC00000 Unexpected bits flipped
+ 0xD00000 Masked bit
+ 0xE00000 Invalid block type
+ 0xF00000 Active CRC/UE error in CRAM
+ ========== =====
+
+What: /sys/devices/system/edac/mc/mc0/xsem_cram_framecc_read
+Date: June 2026
+Contact: rama.devi.veggalam@amd.com
+Description:
+ It is a read/write file.
+ Writing to this file causes the software to initiate a
+ request to the firmware for reading frame ECC values in
+ Configuration RAM (CRAM) of Versal devices. User needs
+ to provide the location details of CRAM
+ (frame, row number, SLR id in SSIT device) to read the ECC values.
+ For Versal mono devices, SLR id should be 0.
+ When read, it shows the ECC values for the requested frame.
+ The format is <header> <ECC_0> <ECC_1> <status>
+ Example:
+ Read Frame ECC Cmd: [0x3030a]
+ Frame ECC Word_0: [0x52f245]
+ Frame ECC Word_1: [0x7c5a6b]
+ Cmd Status: [0x0]
+
+What: /sys/devices/system/edac/mc/mc0/xsem_read_config
+Date: June 2026
+Contact: rama.devi.veggalam@amd.com
+Description:
+ It is a read/write file.
+ Writing to this file causes the software to initiate a
+ request to the firmware for reading Xilsem configuration.
+ When read, it shows the CRAM and NPI scan configuration.
+ The format is <status> <header> <CRAM config> <NPI config>
+ Example: Read Config Cmd: [0x30309]
+ CRAM Scan Config: [0x2e]
+ NPI Scan Config: [0x5016]
+ Cmd Status: [0x0]
+
+What: /sys/devices/system/edac/mc/mc0/xsem_read_status
+Date: June 2026
+Contact: rama.devi.veggalam@amd.com
+Description:
+ It is a read/write file.
+ Writing to this file causes the software to initiate a
+ request read the Xilsem status. User needs to provide
+ the module id for status. The module id values are as given below:
+ 1 - CRAM scan
+ 2 - NPI scan
+ When read, it shows the status of the requested module.
+ For CRAM: <status> <CE count>
+ Example: 0x10005 0
+ For NPI: <status> <scan count> <heartbeat count>
+ Example: 0xA01 0x10 0x1
+
+What: /sys/devices/system/edac/mc/mc0/xsem_cram_ssit_getcrc
+Date: June 2026
+Contact: rama.devi.veggalam@amd.com
+Description:
+ It is a read/write file.
+ Writing to this file causes the software to initiate a
+ request to read CRC of a row in CRAM. User needs to provide
+ the row number and SLR id (Versal SSIT device) for which CRC
+ to be read. For Versal mono devices, SLR id should be 0.
+ When read, it shows the CRC of the requested row.
+ The format is: <Header> <CRC Word 0 to 3> <status>
+ Example: Read CRC Cmd:[0x1030c]
+ CRC_Word 0:[0x0]
+ CRC_Word 1:[0x1f72d881]
+ CRC_Word 2:[0x0]
+ CRC_Word 3:[0x0]
+ Cmd status: [0x0]
+
+What: /sys/devices/system/edac/mc/mc0/xsem_read_ssit_status
+Date: June 2026
+Contact: rama.devi.veggalam@amd.com
+Description:
+ It is a read/write file.
+ Writing to this file causes the software to initiate a
+ request to read CRAM and NPI scan status information in a given SLR.
+ User needs to provide SLR id in Versal SSIT device for which
+ XilSEM status to be read. When read, it shows
+ the XilSEM status of the requested SLR.
+ The format is: <Header> <SLR id> <NPI status info> <CRAM status info>
+ Example: Read SLR Status Cmd:[0x1030d]
+ SLR ID:[0x3]
+ NPI status:[0xa01]
+ NPI scan count:[0x413]
+ NPI Heartbeat count:[0x1150c]
+ NPI scan skip count 0 :[0x0]
+ NPI scan skip count 1 :[0x0]
+ NPI scan skip count 2 :[0x0]
+ NPI scan skip count 3 :[0x0]
+ NPI scan skip count 4 :[0x0]
+ NPI scan skip count 5 :[0x0]
+ NPI scan skip count 6 :[0x0]
+ NPI scan skip count 7 :[0x0]
+ NPI error info 0 :[0x0]
+ NPI error info 1 :[0x0]
+ CRAM status:[0x10005]
+ Error Location High 0: [0x0]
+ Error Location Low 0: [0x0]
+ Error Location High 1: [0x0]
+ Error Location Low 1: [0x0]
+ Error Location High 2: [0x0]
+ Error Location Low 2: [0x0]
+ Error Location High 3: [0x0]
+ Error Location Low 3: [0x0]
+ Error Location High 4: [0x0]
+ Error Location Low 4: [0x0]
+ Error Location High 5: [0x0]
+ Error Location Low 5: [0x0]
+ Error Location High 6: [0x0]
+ Error Location Low 6: [0x0]
+ CRAM scan CE count:[0x0]
+
+What: /sys/devices/system/edac/mc/mc0/xsem_total_cframes_ssit
+Date: June 2026
+Contact: rama.devi.veggalam@amd.com
+Description:
+ It is a read/write file.
+ Writing to this file causes the software to initiate a
+ request to read total cframes in a row in CRAM.
+ User needs to provide row number and SLR id in
+ Versal SSIT device for which total frames to be read.
+ When read, it shows total number of Cframes for the requested SLR.
+ The format is: <Header> <SLR id> <row> <total frames> <Cmd status>
+ Example: Read Total Frames Cmd : [0x4030e]
+ SLR ID : [0x3]
+ Row Index: [0x0]
+ Type[0] frame count: [38752]
+ Type[1] frame count: [262144]
+ Type[2] frame count: [20480]
+ Type[3] frame count: [16]
+ Type[4] frame count: [8]
+ Type[5] frame count: [0]
+ Type[6] frame count: [83]
+ Cmd Status: [0x0]
+Contact: rama.devi.veggalam@amd.com
+Description:
+ It is a read/write file.
+ Writing to this file causes the software to initiate a
+ request to the firmware for doing requested scan operation in
+ Configuration RAM (CRAM) or NPI of Versal devices. User needs
+ to provide scan operation id (init, start, stop) details.
+ The scan operation id values are as given below:
+ 1 - Initialize the scan
+ 2 - Start CRAM scan
+ 3 - Stop CRAM scan
+ 5 - Start NPI scan
+ 6 - Stop NPI scan
+ 7 - Inject NPI error in first descriptor
+
+ When read, it shows the current scan status with error code.
+ The format is <0x1030 | operation Id> <error code>.
+ The different error codes are as given below:
+ ========== =====
+ Error Code Cause
+ ========== =====
+ 0x0 Scan operation success
+ 0x1 Failure in NPI scan
+ 0x80 Calibration timeout
+ 0x2000 Internal error
+ 0x500000 CRAM initialization not yet done
+ 0x600000 Start scan failed
+ 0x700000 Stop scan failed
+ 0xF00000 Active CRC/UE error
+ 0x1000000 ECC/CRC error detected during calibration
+ ========== =====
+
+What: /sys/devices/system/edac/versal_xilsem/xsem_cram_injecterr
+Date: July 2025
+Contact: rama.devi.veggalam@amd.com
+Description:
+ It is a read/write file.
+ Writing to this file causes the software to initiate a
+ request to the firmware for doing error injection in
+ Configuration RAM (CRAM) of Versal devices. User needs
+ to provide the location details of CRAM
+ (frame, qword, bit number, row number) to inject the error.
+ When read, it shows the current error injection status. The
+ format is <header> <error code>.
+ Example: 0x10304 0
+ The different error codes are as given below:
+ ========== =====
+ Error Code Cause
+ ========== =====
+ 0x0 Error injection success
+ 0x2000 Internal NULL pointer error
+ 0x500000 CRAM initialization not yet done
+ 0x800000 Invalid row
+ 0x900000 Invalid qword
+ 0xA00000 Invalid bit
+ 0xB00000 Invalid frame address
+ 0xC00000 Unexpected bits flipped
+ 0xD00000 Masked bit
+ 0xE00000 Invalid block type
+ 0xF00000 Active CRC/UE error in CRAM
+ ========== =====
+
+What: /sys/devices/system/edac/versal_xilsem/xsem_cram_framecc_read
+Date: July 2025
+Contact: rama.devi.veggalam@amd.com
+Description:
+ It is a read/write file.
+ Writing to this file causes the software to initiate a
+ request to the firmware for reading frame ECC values in
+ Configuration RAM (CRAM) of Versal devices. User needs
+ to provide the location details of CRAM
+ (frame, row number) to read the ECC values.
+ When read, it shows the ECC values for the requested frame.
+ The format is <status> <header> <ECC_0> <ECC_1>
+ Example: 0 0x1030A 0x363B1A 0x8A0200
+
+What: /sys/devices/system/edac/versal_xilsem/xsem_read_config
+Date: July 2025
+Contact: rama.devi.veggalam@amd.com
+Description:
+ It is a read/write file.
+ Writing to this file causes the software to initiate a
+ request to the firmware for reading Xilsem configuration.
+ When read, it shows the CRAM and NPI scan configuration.
+ The format is <status> <header> <CRAM config> <NPI config>
+ Example: 0 0x1030A 0x26 0x5016
+
+What: /sys/devices/system/edac/versal_xilsem/xsem_read_status
+Date: July 2025
+Contact: rama.devi.veggalam@amd.com
+Description:
+ It is a read/write file.
+ Writing to this file causes the software to initiate a
+ request read the Xilsem status. User needs to provide
+ the module id for status. The module id values are as given below:
+ 1 - CRAM scan
+ 2 - NPI scan
+ When read, it shows the status of the requested module.
+ For CRAM: <status> <CE count>
+ Example: 0x10005 0
+ For NPI: <status> <scan count> <heartbeat count>
+ Example: 0xA01 0x10 0x1
--
2.23.0
^ permalink raw reply related
* [PATCH v3 1/4] dt-bindings: edac: Add bindings for Xilinx Versal XilSEM
From: Rama devi Veggalam @ 2026-06-24 21:25 UTC (permalink / raw)
To: bp, tony.luck, michal.simek, robh, krzk+dt, conor+dt
Cc: linux-kernel, linux-edac, devicetree, james.morse, mchehab, rric,
git, Rama devi Veggalam
In-Reply-To: <20260624212545.2850787-1-rama.devi.veggalam@amd.com>
Update versal edac device tree bindings for
Versal Soft Error Mitigation (XilSEM).
Signed-off-by: Rama devi Veggalam <rama.devi.veggalam@amd.com>
---
Changes in v3:
- Merged XilSEM edac with Versal Edac
Changes in v2:
- Changed "xlnx,versal-xilsem-edac" to constant
- Removed "compatible: in required section
- Removed "|" in description
- Removed "items" in compatible
- Fixed indentation in examples
- Updated title and description
---
.../xlnx,versal-ddrmc-edac.yaml | 22 ++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/memory-controllers/xlnx,versal-ddrmc-edac.yaml b/Documentation/devicetree/bindings/memory-controllers/xlnx,versal-ddrmc-edac.yaml
index 12f8e9f350bc..568d2af7de81 100644
--- a/Documentation/devicetree/bindings/memory-controllers/xlnx,versal-ddrmc-edac.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/xlnx,versal-ddrmc-edac.yaml
@@ -4,17 +4,31 @@
$id: http://devicetree.org/schemas/memory-controllers/xlnx,versal-ddrmc-edac.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: Xilinx Versal DDRMC (Integrated DDR Memory Controller)
+title: Xilinx Versal DDRMC (Integrated DDR Memory Controller) and Soft Error Mitigation (XilSEM)
maintainers:
- Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
- Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
+ - Rama Devi Veggalam <rama.devi.veggalam@amd.com>
description:
The integrated DDR Memory Controllers (DDRMCs) support both DDR4 and LPDDR4/
4X memory interfaces. Versal DDR memory controller has an optional ECC support
which correct single bit ECC errors and detect double bit ECC errors.
+ Xilinx Versal Soft Error Mitigation (XilSEM) is part of the
+ Platform Loader and Manager (PLM) which runs on the
+ Platform Management Controller (PMC). XilSEM is responsible for reporting
+ and optionally correcting soft errors in Configuration Memory of Versal.
+ The Configuration Memory includes Configuration RAM and
+ Network on Chip (NoC) peripheral interconnect (NPI) Registers.
+
+ The memory is scanned by a hardware controller in the Versal Programmable
+ Logic (PL). During the scan, if the controller detects any error, be it
+ correctable or uncorrectable, it reports the error to PLM.
+ The XilSEM on PLM performs the error validation and notifies the errors to user application.
+
+
properties:
compatible:
const: xlnx,versal-ddrmc
@@ -23,11 +37,13 @@ properties:
items:
- description: DDR Memory Controller registers
- description: NOC registers corresponding to DDR Memory Controller
+ - description: SEM RTCA Controller registers
reg-names:
items:
- const: base
- const: noc
+ - const: semrtca
interrupts:
maxItems: 1
@@ -49,8 +65,8 @@ examples:
#size-cells = <2>;
memory-controller@f6150000 {
compatible = "xlnx,versal-ddrmc";
- reg = <0x0 0xf6150000 0x0 0x2000>, <0x0 0xf6070000 0x0 0x20000>;
- reg-names = "base", "noc";
+ reg = <0x0 0xf6150000 0x0 0x2000>, <0x0 0xf6070000 0x0 0x20000>, < 0x00 0xf2014050 0x00 0xc4>;
+ reg-names = "base", "noc" , "semrtca";
interrupt-parent = <&gic>;
interrupts = <GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>;
};
--
2.23.0
^ permalink raw reply related
* [PATCH v3 0/4] Add support for Versal Xilsem edac
From: Rama devi Veggalam @ 2026-06-24 21:25 UTC (permalink / raw)
To: bp, tony.luck, michal.simek, robh, krzk+dt, conor+dt
Cc: linux-kernel, linux-edac, devicetree, james.morse, mchehab, rric,
git, Rama devi Veggalam
Add sysfs interface for Xilsem scan operations initialize, start,
stop scan, error inject, read ECC, status and configuration values.
Handle correctable and uncorrectable xilsem error events.
Changes in V3:
- Merged XilSEM edac with Versal Edac driver to address
the review feedback.
Rama devi Veggalam (4):
dt-bindings: edac: Add bindings for Xilinx Versal XilSEM
Documentation: ABI: Add ABI doc for versal edac sysfs
firmware: xilinx: Add support for Xilsem scan operations
edac: xilinx: Add EDAC support for Versal XilSem
.../ABI/testing/sysfs-driver-versal-edac | 303 ++++
.../xlnx,versal-ddrmc-edac.yaml | 22 +-
MAINTAINERS | 1 +
drivers/edac/Kconfig | 4 +-
drivers/edac/versal_edac.c | 1348 ++++++++++++++++-
drivers/firmware/xilinx/Makefile | 2 +-
drivers/firmware/xilinx/zynqmp-sem.c | 176 +++
drivers/soc/xilinx/xlnx_event_manager.c | 6 +-
.../linux/firmware/xlnx-versal-error-events.h | 43 +
include/linux/firmware/xlnx-zynqmp-sem.h | 69 +
include/linux/firmware/xlnx-zynqmp.h | 1 +
11 files changed, 1965 insertions(+), 10 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-driver-versal-edac
create mode 100644 drivers/firmware/xilinx/zynqmp-sem.c
create mode 100644 include/linux/firmware/xlnx-versal-error-events.h
create mode 100644 include/linux/firmware/xlnx-zynqmp-sem.h
--
2.23.0
^ permalink raw reply
* Re: [PATCH v3] ASoC: dt-bindings: Convert cirrus,cs35l36 to DT schema
From: sashiko-bot @ 2026-06-24 21:24 UTC (permalink / raw)
To: David Heidelberg via B4 Relay; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260624-dt-cirrus-cs35l36-v3-1-ec451d5a2908@ixit.cz>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `cirrus,boost-ind-nanohenry` property was incorrectly omitted from the `required` list in the YAML schema.
--
commit 778c838f7518ecfc3c565efa00f1019533b91e2d
Author: David Heidelberg <david@ixit.cz>
ASoC: dt-bindings: Convert cirrus,cs35l36 to DT schema
Convert the CS35L36 Speaker Amplifier binding from text to YAML format,
updating the maintainer list and aligning the properties with the datasheet.
> diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs35l36.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs35l36.yaml
> new file mode 100644
> index 0000000000000..2a142b32acf50
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/cirrus,cs35l36.yaml
[ ... ]
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - VA-supply
> + - cirrus,boost-ctl-millivolt
> + - cirrus,boost-peak-milliamp
[Severity: Medium]
Should cirrus,boost-ind-nanohenry be included in this required list?
A device tree missing this property will pass schema validation, but the
driver will fail to probe. Looking at cs35l36_handle_of_data() in
sound/soc/codecs/cs35l36.c, the driver hard-fails if it's absent:
sound/soc/codecs/cs35l36.c:cs35l36_handle_of_data() {
...
if (of_property_read_u32(np, "cirrus,boost-ind-nanohenry", &val) >= 0) {
pdata->boost_ind = val;
} else {
dev_err(&i2c_client->dev, "Inductor not specified.\n");
return -EINVAL;
}
...
}
> +
> +unevaluatedProperties: false
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-dt-cirrus-cs35l36-v3-1-ec451d5a2908@ixit.cz?part=1
^ permalink raw reply
* Re: [PATCH v1 1/3] drivers/platform: lenovo-t14s-ec: Add hwmon support for temperatures and fan speed
From: sashiko-bot @ 2026-06-24 21:22 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: robh, conor+dt, devicetree, linux-hwmon
In-Reply-To: <20260624210825.264454-2-daniel.lezcano@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Direct calls to `t14s_ec_read` bypass the `regmap` mutex, violating the mandatory 10ms hardware recovery delay between EC commands.
- [Medium] Reading the fan RPM requires two separate I2C transfers for the LSB and MSB, allowing the hardware value to update in between and cause torn reads.
- [Low] Passing `int *` arguments to a function expecting `unsigned int *` for I2C register reads.
Pre-existing issues:
- [High] The `t14s_ec_read` function passes buffers allocated on the kernel stack to `__i2c_transfer()`, which violates DMA mapping rules and can cause panics.
--
commit d0870f0c919ef26da98287c26fc38bc327dcd76b
Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
drivers/platform: lenovo-t14s-ec: Add hwmon support for temperatures and fan speed
Expose the Lenovo ThinkPad T14s EC environmental sensors through
the hwmon subsystem.
> diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
> index 5590302a5694f..142464623f0e8 100644
> --- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
> +++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
[ ... ]
> @@ -555,6 +576,128 @@ static irqreturn_t t14s_ec_irq_handler(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static umode_t t14s_ec_hwmon_is_visible(const void *drvdata,
[ ... ]
> +static int t14s_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + struct t14s_ec *ec = dev_get_drvdata(dev);
> + unsigned int value;
> + int ret;
> +
> + switch (type) {
> + case hwmon_temp:
> + if (attr == hwmon_temp_input) {
> + ret = t14s_ec_read(ec, ec->ec_hwmon.sys_thermx[channel].reg, &value);
[Severity: High]
Does calling t14s_ec_read() directly here bypass the regmap mutex?
Because the embedded controller requires a 10ms hardware recovery delay between
commands, regmap serializes accesses and includes the sleep while holding the
lock. Calling t14s_ec_read() directly appears to bypass this locking, potentially
allowing concurrent sysfs reads to interleave with other subsystem I2C
transfers and violate the hardware constraint.
[Severity: High]
This isn't a bug introduced by this patch, but because this exposes new sysfs
attributes that userspace can trigger, does t14s_ec_read() pass stack-allocated
buffers to __i2c_transfer()?
Looking at t14s_ec_read():
static int t14s_ec_read(void *context, unsigned int reg,
unsigned int *val)
{
...
u8 buf[4] = {T14S_EC_CMD_ECRD, reg, 0x00, 0x01};
struct i2c_msg request, response;
u8 result;
...
request.buf = buf;
...
response.buf = &result;
...
ret = __i2c_transfer(client->adapter, &request, 1);
On ARM64 architectures with CONFIG_VMAP_STACK, passing stack memory to I2C
drivers using DMA (like Qualcomm GENI) can trigger a BUG() panic or corrupt
adjacent physical memory. Is it possible to use heap-allocated buffers here
instead?
> + if (ret)
> + return ret;
> + *val = value * 1000;
> +
> + return 0;
> + }
> + break;
> +
> + case hwmon_fan:
> + if (attr == hwmon_fan_input) {
> + int lsb, msb;
> + ret = t14s_ec_read(ec, T14S_EC_FAN_RPM_LSB, &lsb);
[Severity: Low]
Does this pass pointers of type int * to t14s_ec_read() which expects
unsigned int *?
Newer compilers like GCC 14 treat -Wincompatible-pointer-types as an
error by default, which might break the build here.
[Severity: Medium]
Is there a risk of torn reads when reading the lsb and msb sequentially?
Because t14s_ec_read() enforces a 10ms sleep after each I2C transfer, there is
a large race window between reading T14S_EC_FAN_RPM_LSB and
T14S_EC_FAN_RPM_MSB. If the fan RPM rolls over (e.g., from 0x01ff to 0x0200)
during that window, could we combine an old lsb with a new msb and report
a wildly incorrect fan speed?
> + if (ret)
> + return ret;
> +
> + ret = t14s_ec_read(ec, T14S_EC_FAN_RPM_MSB, &msb);
> + if (ret)
> + return ret;
> +
> + *val = 0;
> + *val = lsb + (msb << 8);
> +
> + return 0;
> + }
> + break;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624210825.264454-1-daniel.lezcano@oss.qualcomm.com?part=1
^ permalink raw reply
* Re: [PATCH v1 2/3] platform: arm64: thinkpad-t14s-ec: Wire EC thermal events to hwmon
From: sashiko-bot @ 2026-06-24 21:21 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: linux-hwmon, robh, conor+dt, devicetree
In-Reply-To: <20260624210825.264454-3-daniel.lezcano@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing HWMON_T_ALARM capability prevents sysfs notification from working.
--
commit 1242d377b6327e98b089989a953188982e0ca595
Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
platform: arm64: thinkpad-t14s-ec: Wire EC thermal events to hwmon
This patch wires up EC thermal zone status change notifications to the hwmon
notification framework. It associates hwmon temperature channels with EC
thermal events and emits hwmon_temp_alarm notifications to userspace.
> diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
> index 142464623f0e8..276bb51da33a3 100644
> --- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
> +++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
[ ... ]
> @@ -493,6 +496,20 @@ static int t14s_input_probe(struct t14s_ec *ec)
> return input_register_device(ec->inputdev);
> }
>
> +static void t14s_ec_hwmon_notify_event(struct t14s_ec *ec, u8 event)
> +{
> + for (int i = 0; i < ec->ec_hwmon.num_sys_thermx; i++) {
> + if (ec->ec_hwmon.sys_thermx[i].event != event)
> + continue;
> +
> + hwmon_notify_event(ec->ec_hwmon.dev, hwmon_temp,
> + hwmon_temp_alarm, i);
> +
> + dev_dbg(ec->dev, "Thermal Zone (%s) Status Change Event\n",
> + ec->ec_hwmon.sys_thermx[i].label);
> + }
> +}
> +
[ ... ]
> @@ -658,6 +671,7 @@ static const struct hwmon_ops t14s_ec_hwmon_ops = {
> };
>
> static const struct hwmon_channel_info *t14s_ec_hwmon_info[] = {
> + HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
> HWMON_CHANNEL_INFO(temp,
> HWMON_T_INPUT | HWMON_T_LABEL,
> HWMON_T_INPUT | HWMON_T_LABEL,
> HWMON_T_INPUT | HWMON_T_LABEL,
> HWMON_T_INPUT | HWMON_T_LABEL,
> HWMON_T_INPUT | HWMON_T_LABEL,
> HWMON_T_INPUT | HWMON_T_LABEL),
[Severity: Medium]
Will the hwmon_temp_alarm notifications successfully reach userspace?
The channel configuration array does not include the HWMON_T_ALARM capability
for any of the temperature channels. Without this, the hwmon core will not
create the corresponding tempX_alarm sysfs attributes.
When t14s_ec_hwmon_notify_event() calls hwmon_notify_event() for
hwmon_temp_alarm, it appears the sysfs notification will fail silently because
the attribute does not exist. The thermal subsystem still receives the event
via hwmon_thermal_notify(), but userspace monitoring tools will not be able to
read or poll the sysfs attributes.
Does HWMON_T_ALARM need to be added to the channel configurations here to make
the sysfs event notification work as described in the commit message?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624210825.264454-1-daniel.lezcano@oss.qualcomm.com?part=2
^ permalink raw reply
* Re: [PATCH v1 3/3] arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors
From: sashiko-bot @ 2026-06-24 21:18 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: linux-hwmon, robh, conor+dt, devicetree
In-Reply-To: <20260624210825.264454-4-daniel.lezcano@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message claims the charging thermal zone uses 'multiple passive trip points' to 'progressively throttle the different CPU clusters', but the implementation provides only a single passive trip point that throttles all CPUs simultaneously.
--
commit 165036bedb34a29493f4bd0a7af23dab152aff0c
Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors
Add thermal zone descriptions for the keyboard skin temperature and battery
charging circuitry sensors exposed by the embedded controller.
> - a charging circuitry temperature zone with multiple passive trip
> points and CPU frequency mitigation levels.
>
> The charging thermal zone progressively throttles the different CPU
> clusters as the charging circuitry temperature rises and triggers a hot
> condition at the highest trip point.
[Severity: Medium]
Does the implementation match this commit message description?
The commit message claims there are multiple passive trip points and
progressive throttling across different CPU clusters. Is a chunk of the patch
missing, or should the commit message be updated to reflect the final design
shown below?
> diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
> --- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
[ ... ]
> + ec-charging-thermal {
> + polling-delay = <5000>;
> + polling-delay-passive = <2000>;
> +
> + thermal-sensors = <&ec 3>;
> +
> + trips {
> + psv0: trip-point0 {
> + temperature = <55000>;
> + hysteresis = <0>;
> + type = "passive";
> + };
> +
> + alert0: trip-point1 {
> + temperature = <63000>;
> + hysteresis = <0>;
> + type = "hot";
> + };
> + };
[Severity: Medium]
Only a single passive trip point is defined here in ec-charging-thermal,
which doesn't match the "multiple passive trip points" mentioned in the
commit message.
> +
> + cooling-maps {
> + map0 {
> + trip = <&psv0>;
> + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu8 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu9 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu10 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu11 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +
> + };
> + };
[Severity: Medium]
Additionally, all 12 CPUs are tied uniformly to this single trip point.
Should this define progressive throttling across different CPU clusters as
described in the commit message?
> +
> + };
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624210825.264454-1-daniel.lezcano@oss.qualcomm.com?part=3
^ permalink raw reply
* Re: [PATCH V14 0/9] Add Invensense ICM42607
From: Chris Morgan @ 2026-06-24 21:18 UTC (permalink / raw)
To: Chris Morgan
Cc: linux-iio, andy, nuno.sa, dlechner, jic23, jean-baptiste.maneyrol,
linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
andriy.shevchenko
In-Reply-To: <20260624182350.50467-1-macroalpha82@gmail.com>
On Wed, Jun 24, 2026 at 01:23:39PM -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add support for the ICM42607 IMU. This sensor shares the same
> functionality but a different register layout with the existing
> ICM42600.
>
> This driver should work with the ICM42607 and ICM42607P over both I2C
> and SPI, however only the ICM42607P over I2C could be tested.
>
> Changes Since V1:
> - Instead of creating a new driver, merged with the existing inv_icm42600
> driver. This necessitated adding some code to the existing driver to
> permit using a different register layout for the same functionality.
> - Split changes up a bit more to decrease the size of the individual
> patches. Note that patch 0004 is still pretty hefty; if I need to split
> further I may need to create some temporary stub functions.
> - Used guard() and PM_RUNTIME_ACQUIRE_AUTOSUSPEND() on the new functions
> per Jonathan's recommendations.
>
> Changes Since V2:
> - Went back to using a new driver on advice from Invensense engineer.
> - Further split changes up into smaller chunks of functionality. Note
> still that the largest patch is approximately 900 lines, and that while
> the driver compiles cleanly at each commit it is not able to drive the
> hardware until the commit that adds the Interrupt (as it also adds the
> Makefile).
> - Change the error to a warning when the devicetree binding does not match
> the hardware ID.
> - Dropped the ack on the devicetree bindings, as I am creating a new file
> (for a new driver) instead of modifying the existing one.
>
> Changes Since V3:
> - Numerous small fixes (too many to list here). Thank you to everyone who
> provided feedback.
> - Split power management additions into an additional commit to break
> things up further.
> - Consolidated devicetree documentation in existing
> invensense,icm42600.yaml file.
> - Removed most of the FIELD_PREP from header file to c files to make code
> easier to read.
> - Changed scale values to 2D arrays for Gyro and Accelerometer.
> - Removed IIO_CHAN_INFO_CALIBBIAS attribute.
>
> Changes Since V4:
> - Additional numerous small fixes, thank you again for all the feedback.
> - Dropped power control API and instead run device in low noise mode.
> - Split devicetree bindings into two distinct changes.
> - Reordered adding of enums and structs to main header file so that they
> are only brought in when needed.
> - Stopped using enum for driver data and instead am using pointer to
> device specific driver data.
>
> Changes Since V5:
> - Corrected use of "dev_warn_probe" to just "dev_warn".
> - Fixed some return scenarios which would unconditionally return 0
> when an error was present.
> - Corrected use of max() to min() for bounds checking. max() was
> incorrect.
> - Fixed using "st->conf.accel.odr" in the gyroscope function. It
> should have been "st->conf.gyro.odr" which it now is.
> - Additional small fixes suggested by "sashiko.dev".
> - Added a regmap cache. I used the datasheet to try and determine
> which registers might change without explicit writes.
>
> Changes Since V6:
> - Corrected additional errors identified by sashiko.dev, mostly
> fixing potential deadlocks, missing calls for pm runtime, and
> potential overflow issues.
>
> Changes Since V7:
> - Dropped Wake on Movement patches, since some of the functionality
> was only available for a device on which I cannot test.
> - Dropped support for SPI 3-Wire mode, since it complicated the
> bus setup (and I lack the hardware to test such features anyway).
> - Fixed a few additional bugs identified by sashiko.dev bot.
>
> Changes Since V8:
> - Added back IRQ dropped accidentally when dropping wake on movement
> patches.
> - Dropped "Reviewed-By" tag on patch 2 because of substantial changes
> made to devicetree binding documentation.
> - Additional small fixes as suggested.
>
> Changes Since V9:
> - Removed interrupts (and buffers) from the driver. I previously was
> unable to detect deadlocks because it turns out my IRQ was not even
> wired correctly in my device.
> - Updated devicetree binding commits to make interrupts optional for
> users of the icm42607 driver.
>
> Changes Since V10:
> - Explicitly specified enum values in header file.
> - Removed additional dead code for buffer handling.
> - Cleaned up headers.
> - Added additional locks as requested by sashiko.dev bot.
>
> Changes Since V11:
> - Since driver has shrunk in size considerably, moved i2c bits into
> first code commit. This ensures that the very first commit with code
> can now be compiled. The commit after that adds SPI support as it
> was in the previous versions.
> - Used pahole to optimize inv_icm42607_state. Reordering elements
> reduced size in memory from 384 bytes to 256 bytes.
> - Added a map of all readable registers and all writeable registers
> according to the datasheet.
> - Added back some missing headers pointed out by the maintainers.
> - Added FIELD_PREP in a few more places to make the code more
> obvious on what it's doing.
> - Added a comment to the power management code to note that
> temperature sensor being enabled doesn't matter as the clocks
> are off by default when the gyro and accel channels are off.
> - Removed iio_device_claim_direct() calls since it was no longer
> needed.
> - Fixed shared_by_all attributes for temperature sensor.
> - Additional miscellanous fixes as requested.
>
> Changes Since V12:
> - Removed aligned buffer from inv_icm42607_state struct as we do not
> currently have the need for it.
> - Corrected the order of the odr values in the accel and gyro files
> as the values were out of order (the place in the array corresponds
> to the register value).
> - Stopped setting the clock value depending upon the temp config. The
> datasheet advised to keep using the default value.
> - Corrected logic when changing between states. We only need to pause
> when a sensor changes from off to an on state or when the gyro
> changes from an on state to off.
> - Added missing includes for several files.
>
> Changes Since V13:
> - Refactored inv_icm42607_set_accel_conf() and
> inv_icm42607_set_gyro_conf() into a single function.
> - Refactored inv_icm42607_accel_read_sensor() and
> inv_icm42607_gyro_read_sensor() into a single function.
> - Merged inv_icm42607_set_temp_conf() into initial init function
> since it only really needs to be called once.
> - Saved adding temp sensor for last and updated
> inv_icm42607_temp_read() to either confirm other sensors are already
> enabled or enable the accelerometer so it can get a reading.
> - Updated inv_icm42607_set_pwr_mgmt0() so that it does not update the
> sensor mode and forcibly keep the sensor enabled.
> - Added inv_icm42607_temp_filter_bw enums since it appears to use
> different values than the accel or gyro sensor.
> - Set the temp startup time from 77ms to 77us, as I previously misread
> the datasheet.
> - Additional minor fixes.
>
> Chris Morgan (9):
> dt-bindings: iio: imu: icm42600: Add mount-matrix
> dt-bindings: iio: imu: icm42600: Add icm42607
> iio: imu: inv_icm42607: Add inv_icm42607 Core Driver
> iio: imu: inv_icm42607: Add SPI For icm42607
> iio: imu: inv_icm42607: Add PM support for icm42607
> iio: imu: inv_icm42607: Add Accelerometer for icm42607
> iio: imu: inv_icm42607: Add Gyroscope to icm42607
> iio: imu: inv_icm42607: Add Temp Support in icm42607
> arm64: dts: rockchip: Add icm42607p IMU for RG-DS
>
> .../bindings/iio/imu/invensense,icm42600.yaml | 20 +-
> .../dts/rockchip/rk3568-anbernic-rg-ds.dts | 8 +-
> drivers/iio/imu/Kconfig | 1 +
> drivers/iio/imu/Makefile | 1 +
> drivers/iio/imu/inv_icm42607/Kconfig | 30 +
> drivers/iio/imu/inv_icm42607/Makefile | 13 +
> drivers/iio/imu/inv_icm42607/inv_icm42607.h | 423 ++++++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_accel.c | 317 +++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_core.c | 606 ++++++++++++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_gyro.c | 313 +++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_i2c.c | 98 +++
> .../iio/imu/inv_icm42607/inv_icm42607_spi.c | 108 ++++
> .../iio/imu/inv_icm42607/inv_icm42607_temp.c | 99 +++
> .../iio/imu/inv_icm42607/inv_icm42607_temp.h | 37 ++
> 14 files changed, 2072 insertions(+), 2 deletions(-)
> create mode 100644 drivers/iio/imu/inv_icm42607/Kconfig
> create mode 100644 drivers/iio/imu/inv_icm42607/Makefile
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607.h
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h
>
> --
> 2.43.0
>
I got sashiko.dev's review, and agree with everything except 2 points.
1) It asks "Is it appropriate to omit the interrupts property based on
driver behavior rather than hardware wiring?" I would say no, but in
my case the hardware is not wired for an interrupt so it's moot.
2) It asks "Does this code update the software state cache for the
sensor mode?" No, it doesn't. The goal is for the device to stay
powered up for up to 2 seconds until runtime PM turns it off if it
isn't active, but I don't mind something turning around and shutting
the hardware down if it's not in use before then. Anything that needs
to read the specific sensor in question will be holding the mutex so
that there shouldn't be a risk of another thread turning stuff off
while I'm trying to use it, I don't think. What I want to avoid is
calling to turn the hardware on for a one-shot read and then
immediately turning it off once I'm done. I can wait the two seconds
for runtime PM to do it when the alternative is to call the shutdown
routine every single time for one-shot reads and wait for the startup
and in the case of the gyro shutdown times otherwise.
Thoughts?
I can fix the remaining issues it points out and resubmit.
Thank you,
Chris
^ permalink raw reply
* Re: [PATCH 0/2] rtc: aspeed: Add AST2700 RTC support
From: Alexandre Belloni @ 2026-06-24 21:16 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley,
Andrew Jeffery, Tommy Huang
Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
linux-aspeed
In-Reply-To: <20260601-ast2700-rtc-v1-0-15d4ca46500a@aspeedtech.com>
On Mon, 01 Jun 2026 17:14:05 +0800, Tommy Huang wrote:
> This patch series adds support for the ASPEED AST2700 RTC define,
> includeing dt-binding in the Document and rtc-aspeed.c
Applied, thanks!
[1/2] dt-bindings: rtc: add ASPEED AST2700 compatible
https://git.kernel.org/abelloni/c/5e7f746bc106
[2/2] rtc: aspeed: add AST2700 compatible
https://git.kernel.org/abelloni/c/3319cfeeb8c4
Best regards,
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v5] dt-bindings: rtc: epson,rx6110: Convert to DT Schema
From: Alexandre Belloni @ 2026-06-24 21:15 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Udaya Kiran Challa
Cc: skhan, me, linux-rtc, devicetree, linux-kernel
In-Reply-To: <20260514173851.25088-1-challauday369@gmail.com>
On Thu, 14 May 2026 23:03:32 +0530, Udaya Kiran Challa wrote:
> Convert the Epson RX6110 Real Time Clock devicetree binding
> from the legacy text format to DT schema.
Applied, thanks!
[1/1] dt-bindings: rtc: epson,rx6110: Convert to DT Schema
https://git.kernel.org/abelloni/c/c7ab7504631d
Best regards,
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v3 0/6] rtc: m41t93: add new features alarm, clock out, watchdog
From: Alexandre Belloni @ 2026-06-24 21:15 UTC (permalink / raw)
To: krzk+dt, robh, conor+dt, Akhilesh Patil
Cc: skhan, linux-rtc, devicetree, linux-kernel, akhileshpatilvnit
In-Reply-To: <cover.1758379856.git.akhilesh@ee.iitb.ac.in>
On Sat, 20 Sep 2025 20:30:48 +0530, Akhilesh Patil wrote:
> This patch series adds following to m41t93 rtc driver.
>
> Functionalities:
> - Alarm support (support to configure alarm 1)
> - Square wave output support
> - Watchdog support
>
> [...]
Applied, thanks!
[1/6] dt-bindings: rtc: Add ST m41t93
https://git.kernel.org/abelloni/c/b70598b3769e
[2/6] rtc: m41t93: add device tree support
https://git.kernel.org/abelloni/c/081e3de28c76
[3/6] rtc: m41t93: migrate to regmap api for register access
https://git.kernel.org/abelloni/c/d86e8682593c
[4/6] rtc: m41t93: Add alarm support
https://git.kernel.org/abelloni/c/5b55ff7cace4
[5/6] rtc: m41t93: Add square wave clock provider support
https://git.kernel.org/abelloni/c/5cb6816cd81b
[6/6] rtc: m41t93: Add watchdog support
https://git.kernel.org/abelloni/c/15ec2ce19962
Best regards,
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* [PATCH v3] ASoC: dt-bindings: Convert cirrus,cs35l36 to DT schema
From: David Heidelberg via B4 Relay @ 2026-06-24 21:10 UTC (permalink / raw)
To: David Rhodes, Richard Fitzgerald, Liam Girdwood, Mark Brown,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, patches,
Bjorn Helgaas
Cc: David Rhodes, linux-sound, devicetree, linux-kernel, phone-devel,
David Rhodes, David Heidelberg
From: David Heidelberg <david@ixit.cz>
Convert CS35L36 Speaker Amplifier to yaml.
Changes:
- maintainers email to the generic Cirrus email
- Both the codec and downstream worked just fine without
VP-supply provided. Align with datasheet for similar models.
- add dai-common.yaml to cover for '#sound-dai-cells',
'sound-name-prefix'
- updated not yet implemented:
cirrus,weak-fet-delay -> cirrus,classh-wk-fet-delay-ms
(in both definition and example)
cirrus,weak-fet-thld -> cirrus,weak-fet-thld-millivolt
(only in the example)
- added two required properties:
cirrus,boost-ctl-millivolt
cirrus,boost-peak-milliamp
Assisted-by: OpenAI:gpt-4
Reviewed-by: David Rhodes <David.Rhodes@cirrus.com>
Co-developed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
Relevant for Pixel 3 / 3XL / 4.
---
Changes in v3:
- Corrected properties which has been unimplemented and also
conflicting between definition and the sample.
- Removed units from description where units are part of the property
name.
- Added sound into the $id path. (Mark)
- Readded James and removed Bjorn. (Rob)
- Made path to dai-common shorter.
- Added two required properties
cirrus,boost-ctl-millivolt and cirrus,boost-peak-milliamp.
- Use $defs for the cirrus,vpbr-config to prevent matching it against
vendor defined properties. The driver is already mainlined with
cirrus, vendor part, so we have to keep it compatible.
- Link to v2: https://lore.kernel.org/r/20260624-dt-cirrus-cs35l36-v2-1-74eccdbd8fe4@ixit.cz
Changes in v2:
- Rename the commit. (Mark)
- Link to v1: https://lore.kernel.org/r/20260618-dt-cirrus-cs35l36-v1-1-1a43515666ad@ixit.cz
---
.../devicetree/bindings/sound/cirrus,cs35l36.yaml | 240 +++++++++++++++++++++
.../devicetree/bindings/sound/cs35l36.txt | 168 ---------------
2 files changed, 240 insertions(+), 168 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs35l36.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs35l36.yaml
new file mode 100644
index 0000000000000..2a142b32acf50
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/cirrus,cs35l36.yaml
@@ -0,0 +1,240 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/cirrus,cs35l36.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus Logic CS35L36 Speaker Amplifier
+
+maintainers:
+ - David Rhodes <drhodes@opensource.cirrus.com>
+ - patches@opensource.cirrus.com
+
+description:
+ CS35L36 is a boosted mono Class D amplifier
+
+allOf:
+ - $ref: dai-common.yaml#
+
+properties:
+ compatible:
+ enum:
+ - cirrus,cs35l36
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ VA-supply:
+ description: Voltage regulator of analog internal section
+
+ VP-supply:
+ description: Voltage regulator of boost converter
+
+ reset-gpios:
+ maxItems: 1
+
+ cirrus,boost-ctl-millivolt:
+ description: Boost converter output voltage (step 50)
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 2550
+ maximum: 12000
+
+ cirrus,boost-peak-milliamp:
+ description: Boost-converter peak current limit (step 50)
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 4500
+ minimum: 1600
+ maximum: 4500
+
+ cirrus,boost-ind-nanohenry:
+ description: Initial inductor estimation reference value (1000=1μH, 1200=1.2μH)
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 1000
+
+ cirrus,multi-amp-mode:
+ description: Hi-Z ASP port when more than one amplifier in system
+ type: boolean
+
+ cirrus,boost-ctl-select:
+ description: Boost converter control source selection
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 1
+ enum:
+ - 0 # Control Port
+ - 1 # Class
+ - 2 # Sync
+
+ cirrus,amp-pcm-inv:
+ description: Invert incoming PCM data when true
+ type: boolean
+
+ cirrus,imon-pol-inv:
+ description: Invert polarity of outbound IMON feedback when true
+ type: boolean
+
+ cirrus,vmon-pol-inv:
+ description: Invert polarity of outbound VMON feedback when true
+ type: boolean
+
+ cirrus,dcm-mode-enable:
+ description: Enable boost converter automatic Discontinuous Conduction Mode
+ type: boolean
+
+ cirrus,weak-fet-disable:
+ description: Reduce output driver strength in Weak-FET Drive Mode when true
+ type: boolean
+
+ cirrus,classh-wk-fet-delay-ms:
+ description: Weak-FET entry delay
+ default: 100
+ enum: [0, 5, 10, 50, 100, 200, 500, 1000]
+
+ cirrus,classh-weak-fet-thld-millivolt:
+ description: Weak-FET drive threshold
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [50, 100, 150, 200, 250, 300, 350, 400, 450, 500, 550, 600, 650, 700]
+
+ cirrus,temp-warn-threshold:
+ description: Overtemperature warning threshold
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 2
+ enum:
+ - 0 # 105°C
+ - 1 # 115°C
+ - 2 # 125°C
+ - 3 # 135°C
+
+ cirrus,irq-drive-select:
+ description: Interrupt output driver type
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 1
+ enum:
+ - 0 # open-drain
+ - 1 # push-pull
+
+ cirrus,irq-gpio-select:
+ description: Programmable IRQ pin selection
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum:
+ - 0 # PDM_DATA/SWIRE_SD/INT
+ - 1 # GPIO
+
+ cirrus,vpbr-config:
+ $ref: "#/$defs/vpbr-config"
+
+$defs:
+ vpbr-config:
+ description: Brownout prevention configuration sub-node
+ type: object
+ additionalProperties: false
+
+ properties:
+ cirrus,vpbr-en:
+ description: VBST brownout prevention enable
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 0
+ enum:
+ - 0 # disabled
+ - 1 # enabled
+
+ cirrus,vpbr-thld:
+ description: Initial VPBR threshold voltage
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 2
+ maximum: 31
+
+ cirrus,vpbr-atk-rate:
+ description: Attenuation attack step rate
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 7
+
+ cirrus,vpbr-atk-vol:
+ description: VP brownout prevention step size
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 7
+
+ cirrus,vpbr-max-attn:
+ description: Maximum attenuation during VP brownout prevention (dB)
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 15
+
+ cirrus,vpbr-wait:
+ description: Delay between brownout clearance and attenuation release (ms)
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 1
+ enum:
+ - 0 # 10
+ - 1 # 100
+ - 2 # 250
+ - 3 # 500
+
+ cirrus,vpbr-rel-rate:
+ description: Attenuation release step rate
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 7
+
+ cirrus,vpbr-mute-en:
+ description: Mute audio if maximum attenuation reached
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - VA-supply
+ - cirrus,boost-ctl-millivolt
+ - cirrus,boost-peak-milliamp
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ codec@40 {
+ compatible = "cirrus,cs35l36";
+ reg = <0x40>;
+ VA-supply = <&dummy_vreg>;
+ VP-supply = <&dummy_vreg>;
+ reset-gpios = <&gpio0 54 GPIO_ACTIVE_HIGH>;
+ interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+ cirrus,boost-ind-nanohenry = <1000>;
+ cirrus,boost-ctl-millivolt = <10000>;
+ cirrus,boost-peak-milliamp = <4500>;
+ cirrus,boost-ctl-select = <0>;
+ cirrus,classh-wk-fet-delay-ms = <100>;
+ cirrus,classh-weak-fet-thld-millivolt = <100>;
+ cirrus,temp-warn-threshold = <1>;
+ cirrus,multi-amp-mode;
+ cirrus,irq-drive-select = <1>;
+ cirrus,irq-gpio-select = <1>;
+
+ cirrus,vpbr-config {
+ cirrus,vpbr-en = <0>;
+ cirrus,vpbr-thld = <5>;
+ cirrus,vpbr-atk-rate = <2>;
+ cirrus,vpbr-atk-vol = <1>;
+ cirrus,vpbr-max-attn = <9>;
+ cirrus,vpbr-wait = <1>;
+ cirrus,vpbr-rel-rate = <5>;
+ cirrus,vpbr-mute-en = <0>;
+ };
+ };
+ };
+
+...
diff --git a/Documentation/devicetree/bindings/sound/cs35l36.txt b/Documentation/devicetree/bindings/sound/cs35l36.txt
deleted file mode 100644
index d34117b8558e5..0000000000000
--- a/Documentation/devicetree/bindings/sound/cs35l36.txt
+++ /dev/null
@@ -1,168 +0,0 @@
-CS35L36 Speaker Amplifier
-
-Required properties:
-
- - compatible : "cirrus,cs35l36"
-
- - reg : the I2C address of the device for I2C
-
- - VA-supply, VP-supply : power supplies for the device,
- as covered in
- Documentation/devicetree/bindings/regulator/regulator.txt.
-
- - cirrus,boost-ctl-millivolt : Boost Voltage Value. Configures the boost
- converter's output voltage in mV. The range is from 2550mV to 12000mV with
- increments of 50mV.
- (Default) VP
-
- - cirrus,boost-peak-milliamp : Boost-converter peak current limit in mA.
- Configures the peak current by monitoring the current through the boost FET.
- Range starts at 1600mA and goes to a maximum of 4500mA with increments of
- 50mA.
- (Default) 4.50 Amps
-
- - cirrus,boost-ind-nanohenry : Inductor estimation LBST reference value.
- Seeds the digital boost converter's inductor estimation block with the initial
- inductance value to reference.
-
- 1000 = 1uH (Default)
- 1200 = 1.2uH
-
-Optional properties:
- - cirrus,multi-amp-mode : Boolean to determine if there are more than
- one amplifier in the system. If more than one it is best to Hi-Z the ASP
- port to prevent bus contention on the output signal
-
- - cirrus,boost-ctl-select : Boost converter control source selection.
- Selects the source of the BST_CTL target VBST voltage for the boost
- converter to generate.
- 0x00 - Control Port Value
- 0x01 - Class H Tracking (Default)
- 0x10 - MultiDevice Sync Value
-
- - cirrus,amp-pcm-inv : Boolean to determine Amplifier will invert incoming
- PCM data
-
- - cirrus,imon-pol-inv : Boolean to determine Amplifier will invert the
- polarity of outbound IMON feedback data
-
- - cirrus,vmon-pol-inv : Boolean to determine Amplifier will invert the
- polarity of outbound VMON feedback data
-
- - cirrus,dcm-mode-enable : Boost converter automatic DCM Mode enable.
- This enables the digital boost converter to operate in a low power
- (Discontinuous Conduction) mode during low loading conditions.
-
- - cirrus,weak-fet-disable : Boolean : The strength of the output drivers is
- reduced when operating in a Weak-FET Drive Mode and must not be used to drive
- a large load.
-
- - cirrus,classh-wk-fet-delay : Weak-FET entry delay. Controls the delay
- (in ms) before the Class H algorithm switches to the weak-FET voltage
- (after the audio falls and remains below the value specified in WKFET_AMP_THLD).
-
- 0 = 0ms
- 1 = 5ms
- 2 = 10ms
- 3 = 50ms
- 4 = 100ms (Default)
- 5 = 200ms
- 6 = 500ms
- 7 = 1000ms
-
- - cirrus,classh-weak-fet-thld-millivolt : Weak-FET amplifier drive threshold.
- Configures the signal threshold at which the PWM output stage enters
- weak-FET operation. The range is 50mV to 700mV in 50mV increments.
-
- - cirrus,temp-warn-threshold : Amplifier overtemperature warning threshold.
- Configures the threshold at which the overtemperature warning condition occurs.
- When the threshold is met, the overtemperature warning attenuation is applied
- and the TEMP_WARN_EINT interrupt status bit is set.
- If TEMP_WARN_MASK = 0, INTb is asserted.
-
- 0 = 105C
- 1 = 115C
- 2 = 125C (Default)
- 3 = 135C
-
- - cirrus,irq-drive-select : Selects the driver type of the selected interrupt
- output.
-
- 0 = Open-drain
- 1 = Push-pull (Default)
-
- - cirrus,irq-gpio-select : Selects the pin to serve as the programmable
- interrupt output.
-
- 0 = PDM_DATA / SWIRE_SD / INT (Default)
- 1 = GPIO
-
-Optional properties for the "cirrus,vpbr-config" Sub-node
-
- - cirrus,vpbr-en : VBST brownout prevention enable. Configures whether the
- VBST brownout prevention algorithm is enabled or disabled.
-
- 0 = VBST brownout prevention disabled (default)
- 1 = VBST brownout prevention enabled
-
- See Section 7.31.1 VPBR Config for configuration options & further details
-
- - cirrus,vpbr-thld : Initial VPBR threshold. Configures the VP brownout
- threshold voltage
-
- - cirrus,cirrus,vpbr-atk-rate : Attenuation attack step rate. Configures the
- amount delay between consecutive volume attenuation steps when a brownout
- condition is present and the VP brownout condition is in an attacking state.
-
- - cirrus,vpbr-atk-vol : VP brownout prevention step size. Configures the VP
- brownout prevention attacking attenuation step size when operating in either
- digital volume or analog gain modes.
-
- - cirrus,vpbr-max-attn : Maximum attenuation that the VP brownout prevention
- can apply to the audio signal.
-
- - cirrus,vpbr-wait : Configures the delay time between a brownout condition
- no longer being present and the VP brownout prevention entering an attenuation
- release state.
-
- - cirrus,vpbr-rel-rate : Attenuation release step rate. Configures the delay
- between consecutive volume attenuation release steps when a brownout condition
- is not longer present and the VP brownout is in an attenuation release state.
-
- - cirrus,vpbr-mute-en : During the attack state, if the vpbr-max-attn value
- is reached, the error condition still remains, and this bit is set, the audio
- is muted.
-
-Example:
-
-cs35l36: cs35l36@40 {
- compatible = "cirrus,cs35l36";
- reg = <0x40>;
- VA-supply = <&dummy_vreg>;
- VP-supply = <&dummy_vreg>;
- reset-gpios = <&gpio0 54 0>;
- interrupt-parent = <&gpio8>;
- interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
-
- cirrus,boost-ind-nanohenry = <1000>;
- cirrus,boost-ctl-millivolt = <10000>;
- cirrus,boost-peak-milliamp = <4500>;
- cirrus,boost-ctl-select = <0x00>;
- cirrus,weak-fet-delay = <0x04>;
- cirrus,weak-fet-thld = <0x01>;
- cirrus,temp-warn-threshold = <0x01>;
- cirrus,multi-amp-mode;
- cirrus,irq-drive-select = <0x01>;
- cirrus,irq-gpio-select = <0x01>;
-
- cirrus,vpbr-config {
- cirrus,vpbr-en = <0x00>;
- cirrus,vpbr-thld = <0x05>;
- cirrus,vpbr-atk-rate = <0x02>;
- cirrus,vpbr-atk-vol = <0x01>;
- cirrus,vpbr-max-attn = <0x09>;
- cirrus,vpbr-wait = <0x01>;
- cirrus,vpbr-rel-rate = <0x05>;
- cirrus,vpbr-mute-en = <0x00>;
- };
-};
---
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
change-id: 20260618-dt-cirrus-cs35l36-99c466fb13fd
Best regards,
--
David Heidelberg <david@ixit.cz>
^ permalink raw reply related
* [PATCH v1 3/3] arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors
From: Daniel Lezcano @ 2026-06-24 21:08 UTC (permalink / raw)
To: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
krzk+dt, conor+dt
Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
linux-arm-msm, devicetree
In-Reply-To: <20260624210825.264454-1-daniel.lezcano@oss.qualcomm.com>
Add thermal zone descriptions for the keyboard skin temperature and
battery charging circuitry sensors exposed by the embedded controller.
Expose the EC as a thermal sensor provider and define two thermal zones
using the temperatures reported by the EC:
- a keyboard skin temperature zone with passive and hot trip points,
- a charging circuitry temperature zone with multiple passive trip
points and CPU frequency mitigation levels.
The charging thermal zone progressively throttles the different CPU
clusters as the charging circuitry temperature rises and triggers a hot
condition at the highest trip point.
This provides thermal framework integration for the EC temperature
sensors and enables platform thermal management through standard thermal
zone definitions.
Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
---
.../qcom/x1e78100-lenovo-thinkpad-t14s.dtsi | 68 ++++++++++++++++++-
1 file changed, 67 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
index 5d49df41be02..e27533665c8d 100644
--- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
@@ -979,7 +979,7 @@ &i2c6 {
status = "okay";
- embedded-controller@28 {
+ ec: embedded-controller@28 {
compatible = "lenovo,thinkpad-t14s-ec";
reg = <0x28>;
@@ -988,6 +988,8 @@ embedded-controller@28 {
pinctrl-0 = <&ec_int_n_default>;
pinctrl-names = "default";
+ #thermal-sensor-cells = <1>;
+
wakeup-source;
};
};
@@ -1729,3 +1731,67 @@ &usb_mp_qmpphy1 {
status = "okay";
};
+
+&thermal_zones {
+ ec-keyboard-thermal {
+ polling-delay = <5000>;
+ polling-delay-passive = <1000>;
+
+ thermal-sensors = <&ec 1>;
+
+ trips {
+ trip-point0 {
+ temperature = <55000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ trip-point1 {
+ temperature = <62000>;
+ hysteresis = <0>;
+ type = "hot";
+ };
+ };
+ };
+
+ ec-charging-thermal {
+ polling-delay = <5000>;
+ polling-delay-passive = <2000>;
+
+ thermal-sensors = <&ec 3>;
+
+ trips {
+ psv0: trip-point0 {
+ temperature = <55000>;
+ hysteresis = <0>;
+ type = "passive";
+ };
+
+ alert0: trip-point1 {
+ temperature = <63000>;
+ hysteresis = <0>;
+ type = "hot";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&psv0>;
+ cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu8 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu9 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu10 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu11 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+
+ };
+ };
+
+ };
+};
--
2.53.0
^ permalink raw reply related
* [PATCH v1 2/3] platform: arm64: thinkpad-t14s-ec: Wire EC thermal events to hwmon
From: Daniel Lezcano @ 2026-06-24 21:08 UTC (permalink / raw)
To: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
krzk+dt, conor+dt
Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
linux-arm-msm, devicetree
In-Reply-To: <20260624210825.264454-1-daniel.lezcano@oss.qualcomm.com>
The EC generates thermal zone status change notifications for a subset
of the exposed temperature sensors. Wire these EC events to the hwmon
notification framework so userspace can be informed when a thermal alarm
state changes.
Associate each hwmon temperature channel with its corresponding EC
thermal event and emit hwmon_temp_alarm notifications through
hwmon_notify_event() when the EC reports a thermal zone status change.
Also register thermal zones in the hwmon chip capabilities and keep a
reference to the hwmon device to allow event propagation from the IRQ
handler.
This allows userspace monitoring tools to receive thermal alarm
updates without polling the sensors and gives the opportuniy to the
kernel to cool them down.
Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
---
drivers/platform/arm64/lenovo-thinkpad-t14s.c | 66 ++++++++++++++-----
1 file changed, 51 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
index 142464623f0e..276bb51da33a 100644
--- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
+++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
@@ -107,10 +107,13 @@ struct t14s_ec_led_classdev {
struct t14s_ec_hwmon_sys_thermx {
const char *label;
int reg;
+ u8 event;
};
struct t14s_ec_hwmon {
+ struct device *dev;
struct t14s_ec_hwmon_sys_thermx *sys_thermx;
+ size_t num_sys_thermx;
};
struct t14s_ec {
@@ -493,6 +496,20 @@ static int t14s_input_probe(struct t14s_ec *ec)
return input_register_device(ec->inputdev);
}
+static void t14s_ec_hwmon_notify_event(struct t14s_ec *ec, u8 event)
+{
+ for (int i = 0; i < ec->ec_hwmon.num_sys_thermx; i++) {
+ if (ec->ec_hwmon.sys_thermx[i].event != event)
+ continue;
+
+ hwmon_notify_event(ec->ec_hwmon.dev, hwmon_temp,
+ hwmon_temp_alarm, i);
+
+ dev_dbg(ec->dev, "Thermal Zone (%s) Status Change Event\n",
+ ec->ec_hwmon.sys_thermx[i].label);
+ }
+}
+
static irqreturn_t t14s_ec_irq_handler(int irq, void *data)
{
struct t14s_ec *ec = data;
@@ -542,13 +559,9 @@ static irqreturn_t t14s_ec_irq_handler(int irq, void *data)
dev_dbg(ec->dev, "LID closed\n");
break;
case T14S_EC_EVT_THERMAL_TZ40:
- dev_dbg(ec->dev, "Thermal Zone 40 Status Change Event (CPU/GPU)\n");
- break;
case T14S_EC_EVT_THERMAL_TZ42:
- dev_dbg(ec->dev, "Thermal Zone 42 Status Change Event (Battery)\n");
- break;
case T14S_EC_EVT_THERMAL_TZ39:
- dev_dbg(ec->dev, "Thermal Zone 39 Status Change Event (CPU/GPU)\n");
+ t14s_ec_hwmon_notify_event(ec, val);
break;
case T14S_EC_EVT_KEY_FN_G:
dev_dbg(ec->dev, "FN + G - toggle double-tapping\n");
@@ -658,6 +671,7 @@ static const struct hwmon_ops t14s_ec_hwmon_ops = {
};
static const struct hwmon_channel_info *t14s_ec_hwmon_info[] = {
+ HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
HWMON_CHANNEL_INFO(temp,
HWMON_T_INPUT | HWMON_T_LABEL,
HWMON_T_INPUT | HWMON_T_LABEL,
@@ -676,14 +690,34 @@ static const struct hwmon_chip_info t14s_ec_chip_info = {
static int t14s_ec_hwmon_probe(struct t14s_ec *ec)
{
- struct device *dev;
struct t14s_ec_hwmon_sys_thermx sys_thermx[] = {
- { T14S_EC_SYS_THERM0, "soc" },
- { T14S_EC_SYS_THERM1, "keyboard" },
- { T14S_EC_SYS_THERM2, "base" },
- { T14S_EC_SYS_THERM3, "pmbm" },
- { T14S_EC_SYS_THERM6, "qtm" },
- { T14S_EC_SYS_THERM7, "ssd" },
+ {
+ .label = "soc",
+ .reg = T14S_EC_SYS_THERM0,
+ .event = T14S_EC_EVT_THERMAL_TZ39
+ },
+ {
+ .label = "keyboard",
+ .reg = T14S_EC_SYS_THERM1,
+ .event = T14S_EC_EVT_THERMAL_TZ40
+ },
+ {
+ .label = "base",
+ .reg = T14S_EC_SYS_THERM2,
+ },
+ {
+ .label = "pmbm",
+ .reg = T14S_EC_SYS_THERM3,
+ .event = T14S_EC_EVT_THERMAL_TZ42
+ },
+ {
+ .label = "qtm",
+ .reg = T14S_EC_SYS_THERM6
+ },
+ {
+ .label = "ssd",
+ .reg = T14S_EC_SYS_THERM7
+ },
};
ec->ec_hwmon.sys_thermx = devm_kmemdup_array(ec->dev, sys_thermx,
@@ -692,10 +726,12 @@ static int t14s_ec_hwmon_probe(struct t14s_ec *ec)
if (!ec->ec_hwmon.sys_thermx)
return -ENOMEM;
- dev = devm_hwmon_device_register_with_info(ec->dev, "t14s_ec", ec,
- &t14s_ec_chip_info, NULL);
+ ec->ec_hwmon.num_sys_thermx = ARRAY_SIZE(sys_thermx);
+
+ ec->ec_hwmon.dev = devm_hwmon_device_register_with_info(ec->dev, "t14s_ec", ec,
+ &t14s_ec_chip_info, NULL);
- return PTR_ERR_OR_ZERO(dev);
+ return PTR_ERR_OR_ZERO(ec->ec_hwmon.dev);
}
static int t14s_ec_probe(struct i2c_client *client)
--
2.53.0
^ permalink raw reply related
* [PATCH v1 1/3] drivers/platform: lenovo-t14s-ec: Add hwmon support for temperatures and fan speed
From: Daniel Lezcano @ 2026-06-24 21:08 UTC (permalink / raw)
To: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
krzk+dt, conor+dt
Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
linux-arm-msm, devicetree
In-Reply-To: <20260624210825.264454-1-daniel.lezcano@oss.qualcomm.com>
Expose the Lenovo ThinkPad T14s EC environmental sensors through
the hwmon subsystem.
The driver now registers a hwmon device providing access to six EC
temperature sensors corresponding to the SoC, keyboard area, base
cover, PMIC/charging circuitry, QTM module and SSD. Sensor labels
are exported to allow user space to identify each measurement.
Additionally, expose the system fan speed by reading the fan RPM
registers from the embedded controller.
This allows standard monitoring tools such as lm-sensors to report
platform temperatures and fan speed.
Signed-off-by: Daniel Lezcano daniel.lezcano@oss.qualcomm.com
---
drivers/platform/arm64/lenovo-thinkpad-t14s.c | 147 ++++++++++++++++++
1 file changed, 147 insertions(+)
diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
index 5590302a5694..142464623f0e 100644
--- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
+++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
@@ -11,6 +11,7 @@
#include <linux/delay.h>
#include <linux/dev_printk.h>
#include <linux/err.h>
+#include <linux/hwmon.h>
#include <linux/i2c.h>
#include <linux/input.h>
#include <linux/input/sparse-keymap.h>
@@ -67,6 +68,16 @@
#define T14S_EC_EVT_KEY_FN_F11 0x7a
#define T14S_EC_EVT_KEY_FN_G 0x7e
+#define T14S_EC_SYS_THERM0 0x78 /* SoC (CPU+GPU) */
+#define T14S_EC_SYS_THERM1 0x79 /* Keyboard */
+#define T14S_EC_SYS_THERM2 0x7a /* Back cover */
+#define T14S_EC_SYS_THERM3 0x7b /* Charger / PMIC */
+#define T14S_EC_SYS_THERM6 0x7c /* QTM West */
+#define T14S_EC_SYS_THERM7 0x7d /* SSD */
+
+#define T14S_EC_FAN_RPM_LSB 0x84
+#define T14S_EC_FAN_RPM_MSB 0x85
+
/* Hardware LED blink rate is 1 Hz (500ms off, 500ms on) */
#define T14S_EC_BLINK_RATE_ON_OFF_MS 500
@@ -93,9 +104,19 @@ struct t14s_ec_led_classdev {
struct t14s_ec *ec;
};
+struct t14s_ec_hwmon_sys_thermx {
+ const char *label;
+ int reg;
+};
+
+struct t14s_ec_hwmon {
+ struct t14s_ec_hwmon_sys_thermx *sys_thermx;
+};
+
struct t14s_ec {
struct regmap *regmap;
struct device *dev;
+ struct t14s_ec_hwmon ec_hwmon;
struct t14s_ec_led_classdev led_pwr_btn;
struct t14s_ec_led_classdev led_chrg_orange;
struct t14s_ec_led_classdev led_chrg_white;
@@ -555,6 +576,128 @@ static irqreturn_t t14s_ec_irq_handler(int irq, void *data)
return IRQ_HANDLED;
}
+static umode_t t14s_ec_hwmon_is_visible(const void *drvdata,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ switch (type) {
+ case hwmon_temp:
+ return 0444;
+ case hwmon_fan:
+ return 0444;
+ default:
+ return 0;
+ }
+}
+
+static int t14s_ec_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, const char **str)
+{
+ struct t14s_ec *ec = dev_get_drvdata(dev);
+ switch (type) {
+ case hwmon_temp:
+ if (attr == hwmon_temp_label) {
+ *str = ec->ec_hwmon.sys_thermx[channel].label;
+ return 0;
+ }
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static int t14s_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ struct t14s_ec *ec = dev_get_drvdata(dev);
+ unsigned int value;
+ int ret;
+
+ switch (type) {
+ case hwmon_temp:
+ if (attr == hwmon_temp_input) {
+ ret = t14s_ec_read(ec, ec->ec_hwmon.sys_thermx[channel].reg, &value);
+ if (ret)
+ return ret;
+ *val = value * 1000;
+
+ return 0;
+ }
+ break;
+
+ case hwmon_fan:
+ if (attr == hwmon_fan_input) {
+ int lsb, msb;
+ ret = t14s_ec_read(ec, T14S_EC_FAN_RPM_LSB, &lsb);
+ if (ret)
+ return ret;
+
+ ret = t14s_ec_read(ec, T14S_EC_FAN_RPM_MSB, &msb);
+ if (ret)
+ return ret;
+
+ *val = 0;
+ *val = lsb + (msb << 8);
+
+ return 0;
+ }
+ break;
+ default:
+ break;
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static const struct hwmon_ops t14s_ec_hwmon_ops = {
+ .is_visible = t14s_ec_hwmon_is_visible,
+ .read = t14s_ec_hwmon_read,
+ .read_string = t14s_ec_hwmon_read_string,
+};
+
+static const struct hwmon_channel_info *t14s_ec_hwmon_info[] = {
+ HWMON_CHANNEL_INFO(temp,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL),
+ HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
+ NULL
+};
+
+static const struct hwmon_chip_info t14s_ec_chip_info = {
+ .ops = &t14s_ec_hwmon_ops,
+ .info = t14s_ec_hwmon_info,
+};
+
+static int t14s_ec_hwmon_probe(struct t14s_ec *ec)
+{
+ struct device *dev;
+ struct t14s_ec_hwmon_sys_thermx sys_thermx[] = {
+ { T14S_EC_SYS_THERM0, "soc" },
+ { T14S_EC_SYS_THERM1, "keyboard" },
+ { T14S_EC_SYS_THERM2, "base" },
+ { T14S_EC_SYS_THERM3, "pmbm" },
+ { T14S_EC_SYS_THERM6, "qtm" },
+ { T14S_EC_SYS_THERM7, "ssd" },
+ };
+
+ ec->ec_hwmon.sys_thermx = devm_kmemdup_array(ec->dev, sys_thermx,
+ ARRAY_SIZE(sys_thermx),
+ sizeof(sys_thermx[0]), GFP_KERNEL);
+ if (!ec->ec_hwmon.sys_thermx)
+ return -ENOMEM;
+
+ dev = devm_hwmon_device_register_with_info(ec->dev, "t14s_ec", ec,
+ &t14s_ec_chip_info, NULL);
+
+ return PTR_ERR_OR_ZERO(dev);
+}
+
static int t14s_ec_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
@@ -590,6 +733,10 @@ static int t14s_ec_probe(struct i2c_client *client)
if (ret < 0)
return ret;
+ ret = t14s_ec_hwmon_probe(ec);
+ if (ret < 0)
+ return ret;
+
ret = devm_request_threaded_irq(dev, client->irq, NULL,
t14s_ec_irq_handler,
IRQF_ONESHOT, dev_name(dev), ec);
--
2.53.0
^ permalink raw reply related
* [PATCH v1 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration
From: Daniel Lezcano @ 2026-06-24 21:08 UTC (permalink / raw)
To: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
krzk+dt, conor+dt
Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
linux-arm-msm, devicetree
Hi,
This series extends the Lenovo ThinkPad T14s embedded controller driver
with environmental monitoring capabilities and integrates the exposed
sensors into the Linux thermal framework.
The EC provides access to several platform temperature sensors covering
the SoC, keyboard area, bottom cover, charging circuitry, QTM module and
SSD, as well as fan speed information. These sensors are currently used
by the firmware for thermal management but are not exposed to Linux.
The first patch adds hwmon support for the EC temperature sensors and fan
RPM reporting.
The second patch wires EC thermal notifications into the hwmon event
framework, allowing userspace to receive alarm notifications when the EC
reports thermal zone state changes.
The last patch exposes the EC as a thermal sensor provider in the device
tree and defines thermal zones for the keyboard skin temperature and the
charging circuitry temperature. This allows the generic thermal
framework to react to EC-reported temperatures and apply standard Linux
thermal mitigation policies.
With the latest patch, this series fixes thermal issues happening on
this platform where a kernel compilation leads to a system reboot.
Tested on a Lenovo ThinkPad T14s Gen 6 (Snapdragon X Elite).
Thanks,
Daniel
Daniel Lezcano (3):
drivers/platform: lenovo-t14s-ec: Add hwmon support for temperatures
and fan speed
platform: arm64: thinkpad-t14s-ec: Wire EC thermal events to hwmon
arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin
and charging sensors
.../qcom/x1e78100-lenovo-thinkpad-t14s.dtsi | 68 +++++-
drivers/platform/arm64/lenovo-thinkpad-t14s.c | 193 +++++++++++++++++-
2 files changed, 255 insertions(+), 6 deletions(-)
--
2.53.0
^ permalink raw reply
* Re: [PATCH v6 5/9] media: synopsys: Use media_async_register_subdev() to simplify code
From: sashiko-bot @ 2026-06-24 21:07 UTC (permalink / raw)
To: Frank.Li; +Cc: devicetree, Frank.Li, robh, imx, conor+dt
In-Reply-To: <20260624-imx8qxp_pcam-v6-5-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
New issues:
- [Critical] Critical out-of-bounds heap write in v4l2_async_nf_parse_fwnode() due to mismatched endpoint counting.
- [High] Mixed goto-based error handling and __free() cleanup violates subsystem rules and leaves a dangling pointer.
- [High] Hardcoded array indexing for pads mismatches the dynamic Device Tree enumeration order.
- [High] Heap out-of-bounds access and routing validation failure on single-endpoint Device Trees.
- [High] Memory leak of `sd->active_state` on subdev registration failure.
- [High] Unconditional memory leak of `sd->subdev_notifier` during driver removal.
- [High] Leaked async connections and fwnode references on error path.
Pre-existing issues:
- [High] Lockless traversal of the media entity links list causes a race condition.
--
commit 892732bd4672cbe12751cfc2979a3f508c5cde6d
Author: Frank Li <Frank.Li@nxp.com>
Subject: media: synopsys: Use media_async_register_subdev() to simplify code
Use the media_async_register_subdev() to simplify the driver.
Replace the local subdev registration and media pad setup code with
media_async_register_subdev(). Reduce boilerplate code and aligns the
driver with the common pattern used by simple subdevices that each media
pad has one endpoint in fwnode.
> diff --git a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> index f51367409ff46..0fabc89a49b80 100644
> --- a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> +++ b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
[ ... ]
> @@ -360,9 +350,10 @@ dw_mipi_csi2rx_find_format(struct dw_mipi_csi2rx_device *csi2, u32 mbus_code)
>
> static int dw_mipi_csi2rx_start(struct dw_mipi_csi2rx_device *csi2)
> {
> + struct media_pad *sink_pad = &csi2->sd.entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
[Severity: High]
Does this hardcoded index match the dynamic pad allocation from the Device
Tree?
Because v4l2_async_nf_parse_fwnode() populates the pads array strictly in the
order endpoints appear in the Device Tree, accessing index
V4L2_SUBDEV_1TO1_PADS_SINK unconditionally here assumes the sink pad is always
at index 0.
If a platform's Device Tree lists the source endpoint before the sink
endpoint, will the driver access the wrong pad and potentially read incorrect
hardware parameters (like lane counts)?
> + u32 lanes = sink_pad->vep.bus.mipi_csi2.num_data_lanes;
> struct media_pad *source_pad;
[ ... ]
> @@ -549,12 +539,12 @@ static int dw_mipi_csi2rx_enable_streams(struct v4l2_subdev *sd,
> u64 mask;
> int ret;
>
> - sink_pad = &sd->entity.pads[DW_MIPI_CSI2RX_PAD_SINK];
> + sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
> remote_pad = media_pad_remote_pad_first(sink_pad);
[Severity: High]
This isn't a bug introduced by this patch, but does this lockless list
traversal race with userspace ioctls?
Calling media_pad_remote_pad_first() iterates the media entity links list:
drivers/media/mc/mc-entity.c:media_pad_remote_pad_first() {
...
for_each_media_entity_data_link(pad->entity, link) {
...
}
Since v4l2_subdev_enable_streams() does not acquire the global
mdev->graph_mutex, could this traversal race with userspace ioctls like
MEDIA_IOC_SETUP_LINK that mutate the graph structure, leading to linked list
corruption or a use-after-free?
> remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
[ ... ]
> @@ -649,9 +639,9 @@ static int dw_mipi_csi2rx_init_state(struct v4l2_subdev *sd,
> {
> struct v4l2_subdev_route routes[] = {
> {
> - .sink_pad = DW_MIPI_CSI2RX_PAD_SINK,
> + .sink_pad = V4L2_SUBDEV_1TO1_PADS_SINK,
> .sink_stream = 0,
> - .source_pad = DW_MIPI_CSI2RX_PAD_SRC,
> + .source_pad = V4L2_SUBDEV_1TO1_PADS_SOURCE,
[Severity: High]
Could this cause an out-of-bounds heap access if the Device Tree only defines
a single endpoint?
When __media_async_register_subdev() dynamically sizes the pads array based
on ep_count, a Device Tree with only a sink endpoint will result in an array
of size 1.
Since the driver unconditionally sets up active routes referring to
V4L2_SUBDEV_1TO1_PADS_SOURCE (index 1), won't this attempt to access pads[1]
on a size-1 array?
> .source_stream = 0,
> .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> },
[ ... ]
> @@ -764,45 +697,12 @@ static int dw_mipi_csi2rx_register(struct dw_mipi_csi2rx_device *csi2)
> snprintf(sd->name, sizeof(sd->name), "dw-mipi-csi2rx %s",
> dev_name(csi2->dev));
>
> - pads[DW_MIPI_CSI2RX_PAD_SINK].flags = MEDIA_PAD_FL_SINK |
> - MEDIA_PAD_FL_MUST_CONNECT;
> - pads[DW_MIPI_CSI2RX_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE;
> - ret = media_entity_pads_init(&sd->entity, DW_MIPI_CSI2RX_PAD_MAX, pads);
> + ret = media_async_register_subdev(&csi2->sd);
[Severity: High]
Does calling media_async_register_subdev() here expose a dangling pointer due
to mixed goto and __free() usage in __media_async_register_subdev()?
In drivers/media/v4l2-core/v4l2-fwnode.c:__media_async_register_subdev(), the
pads array is bound to scope-based cleanup and assigned to sd->entity.pads:
struct media_pad *pads __free(kfree) = kzalloc_objs(struct media_pad,
ep_count);
...
sd->entity.pads = pads;
If __v4l2_async_register_subdev() fails, the code jumps to out_unregister:
out_unregister:
v4l2_async_nf_unregister(notifier);
Because the function returns, pads is automatically freed, but sd->entity.pads
is not set to NULL. Does this leave a dangling pointer in the subdevice?
[Severity: High]
Could media_async_register_subdev() leak the active state if registration
fails?
In __media_async_register_subdev(), v4l2_subdev_init_finalize() dynamically
allocates sd->active_state. If the subsequent call to
__v4l2_async_register_subdev() fails:
ret = v4l2_subdev_init_finalize(sd);
if (ret)
goto out_unregister;
ret = __v4l2_async_register_subdev(sd, module);
if (ret < 0)
goto out_unregister;
The error path jumps to out_unregister and returns without calling
v4l2_subdev_cleanup(). Does this bypass freeing the active state?
[Severity: Critical]
Can this call trigger a heap out-of-bounds write if the Device Tree contains
disabled or disconnected endpoints?
In __media_async_register_subdev(), the pads array is allocated based on
ep_count, which only counts available connected endpoints (flags=0):
struct media_pad *pads __free(kfree) = kzalloc_objs(struct media_pad,
ep_count);
However, v4l2_async_nf_parse_fwnode() iterates over all endpoints without
filtering:
fwnode_graph_for_each_endpoint_scoped(dev_fwnode(dev), ep) {
...
ret = v4l2_fwnode_endpoint_parse(ep, &pad->vep);
If there are disconnected endpoints in the Device Tree, the loop will iterate
more times than ep_count. Will this increment the pad pointer beyond the
allocated array and overwrite adjacent memory?
[Severity: High]
Does media_async_register_subdev() leak async connections and fwnode
references if an error occurs during endpoint parsing?
In v4l2_async_nf_parse_fwnode(), newly allocated async connections are added
to the notifier. If an error occurs in a subsequent loop iteration:
if (IS_ERR(asd))
return dev_err_probe(dev, PTR_ERR(asd), "failed to add notifier\n");
The function returns directly. Then in __media_async_register_subdev():
ret = v4l2_async_nf_parse_fwnode(dev, pads, notifier);
if (ret < 0)
return ret;
This bypasses the out_cleanup label and fails to call v4l2_async_nf_cleanup(),
which seems to leak everything added to the notifier so far.
> if (ret)
> - goto err_notifier_unregister;
[ ... ]
> @@ -993,7 +895,7 @@ static void dw_mipi_csi2rx_remove(struct platform_device *pdev)
> {
> struct dw_mipi_csi2rx_device *csi2 = platform_get_drvdata(pdev);
>
> - dw_mipi_csi2rx_unregister(csi2);
> + media_async_subdev_cleanup(&csi2->sd);
[Severity: High]
Does media_async_subdev_cleanup() unconditionally leak the subdevice notifier?
The function __media_async_register_subdev() dynamically allocates the
notifier and assigns it to sd->subdev_notifier. During teardown here,
media_async_subdev_cleanup() calls v4l2_async_nf_cleanup() to clean the lists:
drivers/media/v4l2-core/v4l2-fwnode.c:media_async_subdev_cleanup() {
...
v4l2_async_nf_cleanup(sd->subdev_notifier);
...
}
However, it does not appear to call kfree(sd->subdev_notifier). Will this
leak the notifier structure every time the module is unbound or removed?
> phy_exit(csi2->phy);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=5
^ permalink raw reply
* Re: [PATCH v6 7/9] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)
From: sashiko-bot @ 2026-06-24 21:03 UTC (permalink / raw)
To: Frank.Li; +Cc: conor+dt, devicetree, imx, robh, Frank.Li
In-Reply-To: <20260624-imx8qxp_pcam-v6-7-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Hardcoded pad indices lead to a NULL pointer dereference in `imx_cpi_set_fmt()` if the device tree specifies fewer than 2 endpoints.
- [High] Hardcoded pad indices cause memory corruption and use of uninitialized flags if device tree endpoints are reordered.
- [High] Pending asynchronous PM runtime suspend is cancelled during device removal, permanently leaking hardware clocks.
- [Medium] The Kconfig symbol `VIDEO_IMX_PARALLEL_CPI` selects a non-existent symbol `V4L2_1TO1`.
- [Medium] Unchecked return value from `of_device_get_match_data()` leads to a NULL pointer dereference if bound via sysfs.
- [Medium] Incorrect pad arguments passed to `v4l2_subdev_state_xlate_streams()`.
- [Low] The V2 register macros for `CPI_CTRL_V2_REG1` overlap at bit 16 and exceed the 32-bit register width.
--
commit 547923ad3a1ab4157dbe156e0b9151600e5bb65f
Author: Alice Yuan <alice.yuan@nxp.com>
media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)
This commit adds a V4L2 sub-device driver for the CPI controller found
on i.MX8QXP, i.MX8QM, and i.MX93 SoCs. It supports parallel camera
sensors and enables image data capture through a parallel interface.
> diff --git a/drivers/media/platform/nxp/Kconfig b/drivers/media/platform/nxp/Kconfig
> index 40e3436669e21..90f7c792003f2 100644
> --- a/drivers/media/platform/nxp/Kconfig
> +++ b/drivers/media/platform/nxp/Kconfig
> @@ -39,6 +39,18 @@ config VIDEO_IMX_MIPI_CSIS
[ ... ]
> +config VIDEO_IMX_PARALLEL_CPI
> + tristate "NXP i.MX8/i.MX9 Parallel CPI Driver"
> + depends on ARCH_MXC || COMPILE_TEST
> + depends on VIDEO_DEV
> + select MEDIA_CONTROLLER
> + select V4L2_1TO1
[Severity: Medium]
Is this selecting a non-existent Kconfig symbol? V4L2_1TO1 does not appear
to exist in the kernel tree. Selecting a missing symbol can cause unmet
dependency warnings or silent build failures.
> + select V4L2_FWNODE
> + select VIDEO_V4L2_SUBDEV_API
> + help
> + Video4Linux2 sub-device driver for PARALLEL CPI receiver found
> + on some iMX8 and iMX9 SoCs.
> diff --git a/drivers/media/platform/nxp/imx-parallel-cpi.c b/drivers/media/platform/nxp/imx-parallel-cpi.c
> new file mode 100644
> index 0000000000000..0f74b51608715
> --- /dev/null
> +++ b/drivers/media/platform/nxp/imx-parallel-cpi.c
> @@ -0,0 +1,629 @@
[ ... ]
> +/* CPI INTERFACE CONTROL REG1 */
> +#define CPI_CTRL_REG1_PIXEL_WIDTH(v) FIELD_PREP(GENMASK(15, 0), (v))
> +#define CPI_CTRL_REG1_VSYNC_PULSE(v) FIELD_PREP(GENMASK(31, 16), (v))
> +
> +#define CPI_CTRL_V2_REG1_PIXEL_WIDTH(v) FIELD_PREP(GENMASK(16, 0), (v))
> +#define CPI_CTRL_V2_REG1_VSYNC_PULSE(v) FIELD_PREP(GENMASK(31, 16), (v))
[Severity: Low]
Do these V2 register macros overlap? The GENMASK(16, 0) and GENMASK(31, 16)
overlap at bit 16, resulting in a 33-bit span for what appears to be a
32-bit register. While currently unused, could this corrupt register
values if utilized in the future?
> +
> +/* Need match field DATA_TYPE_IN definition at CPI CTRL register */
[ ... ]
> +static void imx_cpi_hw_config(struct imx_cpi_device *pcpidev,
> + const struct imx_cpi_pix_format *pcpidev_fmt)
> +{
> + u32 flags = pcpidev->sd.entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK].vep.bus.parallel.flags;
[Severity: High]
Can hardcoding the pad index lead to reading uninitialized memory here?
The pads array is populated based on the order endpoints appear in the
Device Tree. If a device tree defines the source endpoint before the
sink endpoint, pads[0] will correspond to the source pad instead of
the sink pad.
> + const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> + bool hsync_pol = flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH;
[ ... ]
> +static int imx_cpi_set_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *sdformat)
> +{
> + struct imx_cpi_pix_format const *pcpidev_fmt;
> + struct v4l2_mbus_framefmt *fmt;
[ ... ]
> + /* Propagate the format from sink to source. */
> + fmt = v4l2_subdev_state_get_format(sd_state, V4L2_SUBDEV_1TO1_PADS_SOURCE);
> + *fmt = sdformat->format;
[Severity: High]
Can this result in a NULL pointer dereference?
If a misconfigured device tree only specifies a single endpoint,
media_async_register_subdev() will allocate exactly one pad, meaning
sd->entity.num_pads = 1.
When v4l2_subdev_state_get_format() is explicitly called with
V4L2_SUBDEV_1TO1_PADS_SOURCE (index 1), it will return NULL since the
index is out of bounds. The pointer is then blindly dereferenced.
> +
> + /* The format on the source pad might change due to unpacking. */
> + fmt->code = pcpidev_fmt->output;
> +
> + return 0;
> +}
[ ... ]
> +static int imx_cpi_enable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> +{
> + struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> + const struct imx_cpi_pix_format *pcpidev_fmt;
> + const struct v4l2_mbus_framefmt *format;
> + struct media_pad *sink_pad, *remote_pad;
> + struct device *dev = pcpidev->dev;
> + struct v4l2_subdev *remote_sd;
> + u64 mask;
> + int ret;
> +
> + sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
> + remote_pad = media_pad_remote_pad_first(sink_pad);
> + remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
[Severity: High]
If the device tree endpoints are reordered as mentioned previously, can
this cast the wrong entity type to a v4l2_subdev?
If pads[0] corresponds to the source pad, its remote entity will be a
video_device, not a v4l2_subdev. Calling media_entity_to_v4l2_subdev()
on a video_device produces an invalid pointer that will crash the kernel
when dereferenced.
> +
> + mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
> + V4L2_SUBDEV_1TO1_PADS_SOURCE,
> + &streams_mask);
[Severity: Medium]
Are the pad arguments reversed here?
Since v4l2_subdev_enable_streams() is called on the source pad,
streams_mask represents streams on the source pad. Passing
V4L2_SUBDEV_1TO1_PADS_SINK as the first argument informs the helper that
streams_mask belongs to the sink pad. Could this silently break routing
logic if asymmetrical stream IDs are ever used?
> +
> + format = v4l2_subdev_state_get_format(state, V4L2_SUBDEV_1TO1_PADS_SINK);
[ ... ]
> +static void imx_cpi_remove(struct platform_device *pdev)
> +{
> + struct imx_cpi_device *pcpidev = platform_get_drvdata(pdev);
> +
> + media_async_subdev_cleanup(&pcpidev->sd);
> +}
[Severity: High]
Could this leak hardware clocks if an asynchronous suspend is pending?
When streams are disabled, pm_runtime_put_autosuspend() is called to
queue an async suspend. If the driver is unbound before the async work
executes, imx_cpi_remove() will run.
Since the driver relies on devm_pm_runtime_enable() for PM cleanup,
pm_runtime_disable() will flush and cancel the pending async suspend,
which leaves the device permanently powered on. Should this call
pm_runtime_put_sync() or explicitly suspend the device if an async put
was previously used?
> +
> +static int imx_cpi_probe(struct platform_device *pdev)
> +{
> + struct imx_cpi_device *pcpidev;
> + struct device *dev = &pdev->dev;
> + struct v4l2_subdev *sd;
> + int ret;
> +
> + pcpidev = devm_kzalloc(dev, sizeof(*pcpidev), GFP_KERNEL);
> + if (!pcpidev)
> + return -ENOMEM;
> +
> + pcpidev->dev = dev;
> + platform_set_drvdata(pdev, pcpidev);
> +
> + pcpidev->pdata = of_device_get_match_data(dev);
[Severity: Medium]
Does this missing NULL check lead to a crash when binding via sysfs?
If an administrator manually binds the driver to a platform device without
an Open Firmware node via sysfs, of_device_get_match_data() returns NULL.
The driver will later unconditionally dereference pcpidev->pdata (such as
in imx_cpi_sw_reset()), leading to a kernel panic instead of gracefully
failing.
> + pcpidev->mode = PI_GATE_CLOCK_MODE;
> +
> + pcpidev->regs = devm_platform_ioremap_resource(pdev, 0);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=7
^ permalink raw reply
* Re: [PATCH v6 3/9] media: subdev: Add media_async_register_subdev() helper
From: sashiko-bot @ 2026-06-24 21:00 UTC (permalink / raw)
To: Frank.Li; +Cc: conor+dt, Frank.Li, robh, imx, devicetree
In-Reply-To: <20260624-imx8qxp_pcam-v6-3-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Heap Buffer Overflow during endpoint parsing
- [High] NULL Pointer Dereference of `sd->internal_ops`
- [Medium] Resource Leak / Subsystem Guideline Violation due to goto and scope-based cleanup mixing
- [Low] Code Quality / Misspelled diagnostic messages
--
commit 050e10af8571659357b0d6bb808eab4e58871636
Author: Frank Li <Frank.Li@nxp.com>
media: subdev: Add media_async_register_subdev() helper
This commit introduces media_async_register_subdev() to register a V4L2
sub-device with the asynchronous framework. It parses the firmware graph
and sets up common notifiers and media pads. This aims to reduce duplicated
code and simplify error handling in V4L2 sub-device drivers.
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 62a3a452f7884..169059654478f 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
[ ... ]
> @@ -1302,6 +1303,160 @@ int __v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *m
[ ... ]
> +static int
> +v4l2_async_nf_parse_fwnode(struct device *dev, struct media_pad *pads,
> + struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_subdev *sd = notifier->sd;
> + struct v4l2_async_connection *asd;
> + struct media_pad *pad;
> + int ret;
> +
> + if (!sd->internal_ops->set_pad_by_ep)
[Severity: High]
If a driver passes a subdevice without internal_ops initialized, will this
dereference a NULL pointer?
> + return dev_err_probe(dev, -EINVAL,
> + "Missed valiate_endpoint() callback\n");
[Severity: Low]
This isn't a bug, but there's a typo in "valiate" and it references the wrong
callback name (set_pad_by_ep).
> + pad = pads;
> +
> + fwnode_graph_for_each_endpoint_scoped(dev_fwnode(dev), ep) {
> + u32 flags;
> +
> + ret = v4l2_fwnode_endpoint_parse(ep, &pad->vep);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to parse endpoint\n");
> +
> + ret = sd->internal_ops->set_pad_by_ep(sd, pad);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Can support endponit\n");
[Severity: Low]
This isn't a bug, but "endponit" is misspelled, and the logic seems inverted
since this prints on an error path where the endpoint cannot be supported.
> +
> + flags = pad->flags;
> +
> + pad++;
[Severity: High]
Could this loop write out of bounds of the pads array?
In __media_async_register_subdev(), ep_count is calculated with:
ep_count = fwnode_graph_get_endpoint_count(dev_fwnode(dev), 0);
which only counts endpoints that have a remote connection, and the pads
array is allocated to exactly that size.
Since fwnode_graph_for_each_endpoint_scoped() unconditionally iterates over
all endpoints, if the firmware graph contains any unconnected endpoints, won't
this execute more times than ep_count and write into adjacent heap memory?
> +
> + if (flags & MEDIA_PAD_FL_SOURCE)
> + continue; /* Bypass source port */
> +
> + notifier->ops = &v4l2_common_notifier_ops;
> +
> + asd = v4l2_async_nf_add_fwnode_remote(notifier, ep,
> + struct v4l2_async_connection);
> + if (IS_ERR(asd))
> + return dev_err_probe(dev, PTR_ERR(asd),
> + "failed to add notifier\n");
> + }
> +
> + return 0;
> +}
[ ... ]
> +int __media_async_register_subdev(struct v4l2_subdev *sd, struct module *module)
> +{
> + struct device *dev = sd->dev;
> + u32 ep_count;
> + int ret;
> +
> + if (WARN_ON(!sd->dev))
> + return -ENODEV;
> +
> + struct v4l2_async_notifier *notifier __free(kfree) = kzalloc_obj(*notifier);
> + if (!notifier)
> + return -ENOMEM;
> +
> + v4l2_async_subdev_nf_init(notifier, sd);
> +
> + ep_count = fwnode_graph_get_endpoint_count(dev_fwnode(dev), 0);
> + if (!ep_count)
> + return dev_err_probe(dev, -EINVAL, "No connected endpoints\n");
> +
> + struct media_pad *pads __free(kfree) = kzalloc_objs(struct media_pad, ep_count);
> + if (!pads)
> + return -ENOMEM;
> +
> + ret = v4l2_async_nf_parse_fwnode(dev, pads, notifier);
> + if (ret < 0)
> + return ret;
[Severity: Medium]
If v4l2_async_nf_parse_fwnode() fails, does returning directly bypass
the v4l2_async_nf_cleanup() call that would normally happen via the
out_cleanup label?
The notifier itself is freed via cleanup helpers, but it seems the
v4l2_async_connection structures allocated in the parse loop and their fwnode
references will be leaked if we don't explicitly call v4l2_async_nf_cleanup().
> +
> + ret = media_entity_pads_init(&sd->entity, ep_count, pads);
> + if (ret)
> + goto out_cleanup;
> +
> + ret = v4l2_async_nf_register(notifier);
> + if (ret < 0)
> + goto out_cleanup;
> +
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret)
> + goto out_unregister;
> +
> + ret = __v4l2_async_register_subdev(sd, module);
> + if (ret < 0)
> + goto out_unregister;
[Severity: Medium]
If __v4l2_async_register_subdev() fails, we jump to out_unregister, which
cleans up the notifier and returns.
Does this leak the active_state allocated earlier by
v4l2_subdev_init_finalize(sd) since v4l2_subdev_cleanup() is bypassed?
> +
> + sd->subdev_notifier = no_free_ptr(notifier);
> + retain_and_null_ptr(pads);
> +
> + return 0;
> +
> +out_unregister:
> + v4l2_async_nf_unregister(notifier);
> +
> +out_cleanup:
> + v4l2_async_nf_cleanup(notifier);
> +
> + return ret;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=3
^ permalink raw reply
* Re: [PATCH v6 8/9] arm64: dts: imx8: add camera parallel interface (CPI) node
From: sashiko-bot @ 2026-06-24 21:00 UTC (permalink / raw)
To: Frank.Li; +Cc: conor+dt, imx, robh, devicetree, Frank.Li
In-Reply-To: <20260624-imx8qxp_pcam-v6-8-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Mismatch between DTS port assignment (`port@4`) and the dt-binding documentation which specifies `port@6` for the Parallel RX.
- [Low] Devicetree schema validation error: 'ports' is a strictly required property but is missing from the base `parallel_cpi` node.
--
commit 80a1591fa539edb7320ce7a2c3b3f4dda7b4baa1
Author: Frank Li <Frank.Li@nxp.com>
arm64: dts: imx8: add camera parallel interface (CPI) node
This commit adds the camera parallel interface (CPI) node for imx8. It
defines the base cpi node in imx8-ss-img.dtsi and configures the endpoint
connections in imx8qxp-ss-img.dtsi.
> diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> --- a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
[ ... ]
> @@ -222,6 +222,19 @@ irqsteer_parallel: irqsteer@58260000 {
> status = "disabled";
> };
>
> + parallel_cpi: cpi@58261000 {
> + compatible = "fsl,imx8qxp-pcif";
> + reg = <0x58261000 0x1000>;
[Severity: Low]
Does this base node need a ports property to pass dtbs_check validation?
The fsl,imx93-pcif.yaml schema strictly requires the ports property, and
dt-validate processes nodes regardless of whether status is disabled.
Will building devicetrees that include imx8-ss-img.dtsi without extending
this node trigger validation errors?
> + clocks = <&pi0_pxl_lpcg IMX_LPCG_CLK_0>,
> + <&pi0_ipg_lpcg IMX_LPCG_CLK_4>;
> + clock-names = "pixel", "ipg";
> + assigned-clocks = <&clk IMX_SC_R_PI_0 IMX_SC_PM_CLK_PER>;
> + assigned-clock-parents = <&clk IMX_SC_R_PI_0_PLL IMX_SC_PM_CLK_PLL>;
> + assigned-clock-rates = <160000000>;
> + power-domains = <&pd IMX_SC_R_PI_0>;
> + status = "disabled";
> + };
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> --- a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
[ ... ]
> @@ -62,6 +62,14 @@ isi_in_2: endpoint {
> remote-endpoint = <&mipi_csi0_out>;
> };
> };
> +
> + port@4 {
> + reg = <4>;
> +
> + isi_in_4: endpoint {
> + remote-endpoint = <¶llel_cpi_out>;
> + };
> + };
[Severity: Low]
Does port@4 match the devicetree binding documentation?
The Documentation/devicetree/bindings/media/fsl,imx8qxp-isi.yaml
documentation explicitly specifies port@6 as the CSI-2 Parallel RX.
Even though the driver configures the hardware correctly with num_ports = 5,
does adding port@4 here introduce an undocumented port and create a schema
mismatch?
> };
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=8
^ permalink raw reply
* Re: [PATCH v6 6/9] dt-bindings: media: add i.MX parallel CPI support
From: sashiko-bot @ 2026-06-24 20:57 UTC (permalink / raw)
To: Frank.Li; +Cc: robh, Frank.Li, conor+dt, imx, devicetree
In-Reply-To: <20260624-imx8qxp_pcam-v6-6-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unhandled resource leaks (memory and fwnode references) and double-free risk in `__media_async_register_subdev` and its cleanup path.
--
commit 050e10af8571659357b0d6bb808eab4e58871636
Author: Frank Li <Frank.Li@nxp.com>
media: subdev: Add media_async_register_subdev() helper
This commit adds media_async_register_subdev(), a helper to register a V4L2
sub-device with the asynchronous sub-device framework. It parses the firmware
graph, creates media pads for endpoints, and registers asynchronous notifiers.
This centralizes common initialization and link creation to reduce boilerplate.
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 62a3a452f7884..169059654478f 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -1302,6 +1303,160 @@ int __v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *m
> }
> EXPORT_SYMBOL_GPL(__v4l2_async_register_subdev_sensor);
>
[ ... ]
> +void media_async_subdev_cleanup(struct v4l2_subdev *sd)
> +{
> + v4l2_async_unregister_subdev(sd);
> + v4l2_subdev_cleanup(sd);
> + media_entity_cleanup(&sd->entity);
> + v4l2_async_nf_unregister(sd->subdev_notifier);
> + v4l2_async_nf_cleanup(sd->subdev_notifier);
> + kfree(sd->entity.pads);
> +}
> +EXPORT_SYMBOL_GPL(media_async_subdev_cleanup);
[Severity: High]
Does this cleanup function leak fwnode references?
v4l2_async_nf_parse_fwnode() parses endpoints directly into pad->vep, taking
an fwnode reference via fwnode_handle_get(). It appears this cleanup function
fails to iterate over the pads to call v4l2_fwnode_endpoint_free(&pad->vep) to
release these references.
Also, if a driver explicitly invokes this cleanup helper to recover after
__media_async_register_subdev() fails, will kfree(sd->entity.pads) cause a
double-free panic?
> +
> +int __media_async_register_subdev(struct v4l2_subdev *sd, struct module *module)
> +{
> + struct device *dev = sd->dev;
> + u32 ep_count;
> + int ret;
> +
[ ... ]
> + struct media_pad *pads __free(kfree) = kzalloc_objs(struct media_pad, ep_count);
> + if (!pads)
> + return -ENOMEM;
> +
> + ret = v4l2_async_nf_parse_fwnode(dev, pads, notifier);
> + if (ret < 0)
> + return ret;
> +
> + ret = media_entity_pads_init(&sd->entity, ep_count, pads);
> + if (ret)
> + goto out_cleanup;
> +
> + ret = v4l2_async_nf_register(notifier);
> + if (ret < 0)
> + goto out_cleanup;
> +
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret)
> + goto out_unregister;
> +
> + ret = __v4l2_async_register_subdev(sd, module);
> + if (ret < 0)
> + goto out_unregister;
> +
> + sd->subdev_notifier = no_free_ptr(notifier);
> + retain_and_null_ptr(pads);
> +
> + return 0;
> +
> +out_unregister:
> + v4l2_async_nf_unregister(notifier);
> +
> +out_cleanup:
> + v4l2_async_nf_cleanup(notifier);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(__media_async_register_subdev);
[Severity: High]
If __v4l2_async_register_subdev() fails, the error path returns without
calling v4l2_subdev_cleanup(sd) to free the active state allocated by
v4l2_subdev_init_finalize(). Can this leak sd->active_state?
Additionally, pads is allocated with __free(kfree) and linked to
sd->entity.pads. On a registration failure, pads is automatically freed when
the function returns, but sd->entity.pads is not cleared.
Could this leave a dangling pointer that causes a double-free if error
recovery is attempted?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=6
^ permalink raw reply
* Re: [PATCH v6 1/9] media: mc-entity: Store parsed V4L2 fwnode endpoint in media_pad
From: sashiko-bot @ 2026-06-24 20:56 UTC (permalink / raw)
To: Frank.Li; +Cc: Frank.Li, imx, conor+dt, devicetree, robh
In-Reply-To: <20260624-imx8qxp_pcam-v6-1-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Captured fwnode endpoint contains a dangling pointer to `local_fwnode` if the caller drops its reference after parsing.
- [Medium] Embedding a large, V4L2-specific `struct v4l2_fwnode_endpoint` into the generic Media Controller `struct media_pad` breaks subsystem abstraction and unconditionally bloats memory.
--
commit 7acfb56349845d6638c963825f0f6b087ebaa54d
Author: Frank Li <Frank.Li@nxp.com>
media: mc-entity: Store parsed V4L2 fwnode endpoint in media_pad
This commit captures the parsed v4l2 fwnode endpoint information directly
within struct media_pad to allow reuse by consumers without reparsing.
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index d9b72cd87d524..4a3785cd9f370 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -20,6 +20,8 @@
> #include <linux/minmax.h>
> #include <linux/types.h>
>
> +#include <media/v4l2-fwnode.h>
> +
[Severity: Medium]
Does this include break subsystem isolation? struct media_pad is a generic
structure used across several subsystems like alsa, dvb, drm, and iio.
Including a v4l2-specific header here appears to tie the generic media
controller to v4l2.
[ ... ]
> @@ -240,7 +243,7 @@ struct media_pad {
> u16 num_links;
> enum media_pad_signal_type sig_type;
> unsigned long flags;
> -
> + struct v4l2_fwnode_endpoint vep;
[Severity: Medium]
Is it expected to embed this v4l2-specific structure into every generic
media_pad? The v4l2_fwnode_endpoint struct is quite large, and
unconditionally embedding it adds memory overhead for all non-v4l2
entities that do not use fwnode endpoints.
[Severity: Critical]
Can this cause a use-after-free regression with the fwnode pointers?
When the endpoint is parsed, v4l2_fwnode_endpoint_parse() assigns the
fwnode to vep->base.local_fwnode without taking a reference count. This
was safe when vep was a transient stack variable, but now that it is
stored persistently in the media_pad, the caller might drop the
reference.
For example, if the caller iterates using a scoped iterator:
drivers/media/v4l2-core/v4l2-fwnode.c:v4l2_async_nf_parse_fwnode() {
...
fwnode_graph_for_each_endpoint_scoped(dev_fwnode(dev), ep) {
ret = v4l2_fwnode_endpoint_parse(ep, &pad->vep);
...
}
The scoped iterator automatically drops the reference to ep at the end
of the iteration, leaving pad->vep.base.local_fwnode as a dangling
pointer.
Later access or pointer comparisons, like in v4l2_common_notifier_bound(),
might dereference freed memory if the fwnode is deleted.
> /*
> * The fields below are private, and should only be accessed via
> * appropriate functions.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=1
^ permalink raw reply
* Re: [PATCH v6 2/9] media: subdev: Add set_pad_by_ep() callback to internal ops
From: sashiko-bot @ 2026-06-24 20:50 UTC (permalink / raw)
To: Frank.Li; +Cc: imx, Frank.Li, devicetree, robh, conor+dt
In-Reply-To: <20260624-imx8qxp_pcam-v6-2-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Severe layering violation by tightly coupling the core Media Controller framework with V4L2-specific firmware parsing.
- [Low] Typo in the callback documentation.
--
commit 3e6e9fb5a2a23ceb5cd559adeba28603dd1f9e0f
Author: Frank Li <Frank.Li@nxp.com>
media: subdev: Add set_pad_by_ep() callback to internal ops
This commit adds a set_pad_by_ep() callback to v4l2_subdev_internal_ops.
The callback is invoked while parsing firmware node endpoints and allows
subdevice drivers to configure media pad properties based on endpoint
information.
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index d256b7ec8f848..eb652eb76d33f 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -935,6 +935,10 @@ struct v4l2_subdev_ops {
> * the v4l2_subdev structure. It is almost certainly required for any
> * sub-device that sets the V4L2_SUBDEV_FL_HAS_DEVNODE flag.
> *
> + * @set_pad_by_ep: Set pad informaiton by fwnode endpoint, parsed fwnode already
[Severity: Low]
This isn't a bug, but there is a typo in information (informaiton).
> + * saved into pad->vep. return < 0 means can't support this type
> + * endpoint. Set pad->flags according to pad->vep information.
> + *
> * .. note::
> * Never call this from drivers, only the v4l2 framework can call
> * these ops.
> @@ -947,6 +951,7 @@ struct v4l2_subdev_internal_ops {
> int (*open)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh);
> int (*close)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh);
> void (*release)(struct v4l2_subdev *sd);
> + int (*set_pad_by_ep)(struct v4l2_subdev *sd, struct media_pad *pad);
[Severity: Medium]
Does this callback signature enforce an architectural layering violation?
By omitting the vep argument, the callback design expects the parsed V4L2
endpoint configuration to be passed via pad->vep. struct media_pad is part of
the generic Media Controller API, which is shared across various subsystems
like ALSA and DVB.
Embedding the V4L2-specific struct v4l2_fwnode_endpoint directly into the
generic struct media_pad creates an inappropriate subsystem dependency and
wastes memory for all non-V4L2 media pads.
Could the V4L2-specific endpoint be passed directly as an argument to the
callback, such as:
int (*set_pad_by_ep)(struct v4l2_subdev *sd, struct media_pad *pad,
struct v4l2_fwnode_endpoint *vep);
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=2
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: power: limits: Describe Qualcomm SPEL hardware
From: Daniel Lezcano @ 2026-06-24 20:41 UTC (permalink / raw)
To: Krzysztof Kozlowski, Manaf Meethalavalappu Pallikunhi
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Rafael J. Wysocki,
Bjorn Andersson, Konrad Dybcio, Gaurav Kohli, linux-arm-msm,
devicetree, linux-kernel, linux-pm
In-Reply-To: <d0528339-8be9-4895-a054-625df96ca926@kernel.org>
Hi Krzysztof,
Le 24/06/2026 à 12:45, Krzysztof Kozlowski a écrit :
> On 23/06/2026 11:47, Manaf Meethalavalappu Pallikunhi wrote:
>> Hi Krzysztof,
>>
>>
>> On 6/22/2026 5:58 PM, Krzysztof Kozlowski wrote:
>>> On Sat, Jun 20, 2026 at 02:09:08AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
>>>> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
>>>> based power monitoring and limiting capabilities for various domains.
>>>>
>>>> Add a DeviceTree binding to describe the SPEL block on Qualcomm's SoC.
>>>>
>>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>>> ---
>>>> .../bindings/power/limits/qcom,spel.yaml | 47 ++++++++++++++++++++++
>>>> MAINTAINERS | 6 +++
>>>> 2 files changed, 53 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml b/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
>>>
>>> What is "limits" directory for? What sort of class of devices fit there?
>>
>> Added for devices that integrate with the powercap framework (exposed
>> via sys/class/powercap). These devices are responsible for enforcing and
>
> That's a driver answer. I asked about class of devices. powercap
> framework is Linux thing, not a class of devices.
>
> Please describe hardware, not Linux frameworks.>>> monitoring power consumption limits across different domains, such as
>> the system, SoC, or specific subsystems. Any other better directory ?
>
> I don't know what is this hardware doing and commit msg is quite short
> on explanation. Power monitoring is usually hwmon, but probably this is
> not a hwmon.
Right, it is relatively new in the device tree bindings. There is the
Intel RAPL which is the same device but it does not belong to the DT
namespace.
RAPL : Running Average Power Limit
SPEL : SoC Power and Electrical Limits
It allows power capping and read the average power consumption for a
specific device (or/and an energy counter)
Basically you can set a power constraint (power limit) to a device and
this one won't consume more than that power (the power limitation
strategy is managed under the hood by the firmware depending on the
device - lower OPP, idle injection, modem weaker signal, etc ...).
The RAPL or the SPEL have a hierarchical power limitation. For example:
SoC
|
------------------------
| |
Cluster0 Cluster1
| |
----------------- -----------------
| | | | | | | |
CPU0 CPU1 CPU2 CPU3 CPU4 CPU5 CPU6 CPU7
If you specify a power limit to 'SoC', then the power consumption of
Cluster0 + Cluster1 <= SoC
If Cluster0 power consumption decreases, then Cluster1 is allowed to use
more power until Cluster0 + Cluster1 <= SoC
For me it sounds reasonable to put the device description under power/limits
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox