* [PATCH v3 8/8] docs: misc: amd-sbi: Document SBTSI userspace interface
From: Akshay Gupta @ 2026-06-22 13:58 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, NaveenKrishna.Chatradhi,
Anand.Umarji, Akshay.Gupta, Prathima.Lk
In-Reply-To: <20260622135821.2190260-1-Akshay.Gupta@amd.com>
From: Prathima <Prathima.Lk@amd.com>
- Document AMD sideband IOCTL description defined
for SBTSI and its usage.
User space C-APIs are made available by esmi_oob_library [1],
which is provided by the E-SMS project [2].
Link: https://github.com/amd/esmi_oob_library [1]
Link: https://www.amd.com/en/developer/e-sms.html [2]
Include a user-space open example for /dev/sbtsi-* and list auxiliary
bus sysfs paths.
Reviewed-by: Akshay Gupta <Akshay.Gupta@amd.com>
Signed-off-by: Prathima <Prathima.Lk@amd.com>
---
Changes since v2:
- Update misc node names info as per socket
Changes since v1:
- Elaborate the document
Documentation/misc-devices/amd-sbi.rst | 68 ++++++++++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/Documentation/misc-devices/amd-sbi.rst b/Documentation/misc-devices/amd-sbi.rst
index f91ddadefe48..fbbbc504119f 100644
--- a/Documentation/misc-devices/amd-sbi.rst
+++ b/Documentation/misc-devices/amd-sbi.rst
@@ -48,6 +48,60 @@ Access restrictions:
* APML Mailbox messages and Register xfer access are read-write,
* CPUID and MCA_MSR access is read-only.
+SBTSI device
+============
+
+sbtsi driver under the drivers/misc/amd-sbi creates miscdevice
+/dev/sbtsi-* to let user space programs run APML TSI register xfer
+commands.
+
+The driver supports both I2C and I3C transports for SB-TSI targets.
+The transport is selected by the bus where the device is enumerated.
+
+Misc device:
+ * In 1P socket 0: /dev/sbtsi-4c
+ * In 2P socket 0: /dev/sbtsi-4c, socket 1: /dev/sbtsi-48
+
+.. code-block:: bash
+
+ $ ls -al /dev/sbtsi-4c
+ crw------- 1 root root 10, 116 Apr 2 05:22 /dev/sbtsi-4c
+
+
+Access restrictions:
+ * Only root user is allowed to open the file.
+ * APML TSI Register xfer access is read-write.
+
+SBTSI hwmon interface
+=====================
+
+The sbtsi_temp auxiliary driver binds to the auxiliary device published
+by the core sbtsi driver on the auxiliary bus. The auxiliary device is
+named amd-sbtsi.temp-sensor.<addr> where <addr> is the device's dynamic
+address.
+
+It registers a hwmon device, providing a standard Linux hwmon interface
+for reading CPU temperature and managing temperature limits.
+
+The hwmon device appears under ``/sys/class/hwmon/`` when both ``sbtsi.ko``
+and ``sbtsi_temp.ko`` are loaded.
+
+Verify auxiliary bus device::
+
+ ls /sys/bus/auxiliary/devices/
+ # e.g. amd-sbtsi.temp-sensor.X
+
+Example usage::
+
+ # Read current temperature
+ cat /sys/class/hwmon/hwmon<N>/temp1_input
+
+ # Set high temperature limit to 70 °C
+ echo 70000 > /sys/class/hwmon/hwmon<N>/temp1_max
+
+ # Verify
+ cat /sys/class/hwmon/hwmon<N>/temp1_max
+
Driver IOCTLs
=============
@@ -63,6 +117,9 @@ Driver IOCTLs
.. c:macro:: SBRMI_IOCTL_REG_XFER_CMD
.. kernel-doc:: include/uapi/misc/amd-apml.h
:doc: SBRMI_IOCTL_REG_XFER_CMD
+.. c:macro:: SBTSI_IOCTL_REG_XFER_CMD
+.. kernel-doc:: include/uapi/misc/amd-apml.h
+ :doc: SBTSI_IOCTL_REG_XFER_CMD
User-space usage
================
@@ -85,6 +142,16 @@ Next thing, open the device file, as follows::
exit(1);
}
+To open SB-TSI device::
+
+ int file;
+
+ file = open("/dev/sbtsi-4c", O_RDWR);
+ if (file < 0) {
+ /* ERROR HANDLING */
+ exit(1);
+ }
+
The following IOCTLs are defined:
``#define SB_BASE_IOCTL_NR 0xF9``
@@ -92,6 +159,7 @@ The following IOCTLs are defined:
``#define SBRMI_IOCTL_CPUID_CMD _IOWR(SB_BASE_IOCTL_NR, 1, struct apml_cpuid_msg)``
``#define SBRMI_IOCTL_MCAMSR_CMD _IOWR(SB_BASE_IOCTL_NR, 2, struct apml_mcamsr_msg)``
``#define SBRMI_IOCTL_REG_XFER_CMD _IOWR(SB_BASE_IOCTL_NR, 3, struct apml_reg_xfer_msg)``
+``#define SBTSI_IOCTL_REG_XFER_CMD _IOWR(SB_BASE_IOCTL_NR, 4, struct apml_tsi_xfer_msg)``
User space C-APIs are made available by esmi_oob_library, hosted at
--
2.34.1
^ permalink raw reply related
* [PATCH v3 5/8] misc: amd-sbi: Add support for SB-TSI over I3C
From: Akshay Gupta @ 2026-06-22 13:58 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, NaveenKrishna.Chatradhi,
Anand.Umarji, Akshay.Gupta, Prathima.Lk
In-Reply-To: <20260622135821.2190260-1-Akshay.Gupta@amd.com>
From: Prathima <Prathima.Lk@amd.com>
AMD SB-TSI temperature sensors can be accessed over both
I2C and I3C buses depending on the platform configuration.
Extend the SB-TSI driver to support both I2C and I3C bus interfaces
by selecting the appropriate transport based on the probed bus type.
The driver maintains backward compatibility with existing I2C
deployments while enabling support for systems using the I3C bus.
Register both I2C and I3C drivers using module_i3c_i2c_driver() and
update the Kconfig dependency from I2C to I3C_OR_I2C.
Reviewed-by: Akshay Gupta <Akshay.Gupta@amd.com>
Signed-off-by: Prathima <Prathima.Lk@amd.com>
---
Changes since v2:
- Fix DMA mapping issue to prevent memory corruption or kernel panics
when CONFIG_VMAP_STACK is enabled
- Use i3c_device_get_info() to populate i3c_device_info structure fields
- Remove unused header file inclusion from "include/linux/misc/tsi.h"
Changes since v1:
- Changes in accordance with usage of auxiliary device
drivers/misc/amd-sbi/Kconfig | 4 +--
drivers/misc/amd-sbi/tsi-core.c | 58 ++++++++++++++++++++++++++++++++-
drivers/misc/amd-sbi/tsi-core.h | 23 +++++++++++++
drivers/misc/amd-sbi/tsi.c | 58 ++++++++++++++++++++++++++++++---
include/linux/misc/tsi.h | 11 +++++--
5 files changed, 145 insertions(+), 9 deletions(-)
create mode 100644 drivers/misc/amd-sbi/tsi-core.h
diff --git a/drivers/misc/amd-sbi/Kconfig b/drivers/misc/amd-sbi/Kconfig
index 512251690e0e..1a96b71f8506 100644
--- a/drivers/misc/amd-sbi/Kconfig
+++ b/drivers/misc/amd-sbi/Kconfig
@@ -23,13 +23,13 @@ config AMD_SBRMI_HWMON
config AMD_SBTSI
tristate "AMD side band TSI support"
- depends on I2C
+ depends on I3C_OR_I2C
depends on ARM || ARM64 || COMPILE_TEST
select AUXILIARY_BUS
help
Enables support for the AMD SB-TSI (Side Band Temperature Sensor
Interface) driver, which provides access to emulated CPU temperature
- sensors on AMD SoCs via an I2C connected BMC device.
+ sensors on AMD SoCs via an I2C/I3C connected BMC device.
This driver can also be built as a module. If so, the module will
be called sbtsi.
diff --git a/drivers/misc/amd-sbi/tsi-core.c b/drivers/misc/amd-sbi/tsi-core.c
index 6ef1831515bb..9278d06d8e5f 100644
--- a/drivers/misc/amd-sbi/tsi-core.c
+++ b/drivers/misc/amd-sbi/tsi-core.c
@@ -7,7 +7,17 @@
*/
#include <linux/module.h>
-#include <linux/misc/tsi.h>
+#include "tsi-core.h"
+
+static inline struct sbtsi_i3c_priv *to_sbtsi_i3c_priv(struct sbtsi_data *data)
+{
+ return container_of(data, struct sbtsi_i3c_priv, data);
+}
+
+static u8 *sbtsi_i3c_buf(struct sbtsi_data *data)
+{
+ return to_sbtsi_i3c_priv(data)->buf;
+}
/* I2C transfer function */
static int sbtsi_i2c_xfer(struct sbtsi_data *data, u8 reg, u8 *val, bool is_read)
@@ -23,8 +33,54 @@ static int sbtsi_i2c_xfer(struct sbtsi_data *data, u8 reg, u8 *val, bool is_read
return i2c_smbus_write_byte_data(data->client, reg, *val);
}
+/* I3C read transfer function */
+static int sbtsi_i3c_read(struct sbtsi_data *data, u8 reg, u8 *val)
+{
+ struct i3c_xfer xfers[2] = { };
+ u8 *buf = sbtsi_i3c_buf(data);
+ int ret;
+
+ buf[0] = reg;
+ /* Add Register data to read/write */
+ xfers[0].rnw = false;
+ xfers[0].len = 1;
+ xfers[0].data.out = &buf[0];
+
+ xfers[1].rnw = true;
+ xfers[1].len = 1;
+ xfers[1].data.in = &buf[1];
+
+ ret = i3c_device_do_xfers(data->i3cdev, xfers, 2, I3C_SDR);
+ if (ret)
+ return ret;
+
+ *val = buf[1];
+ return ret;
+}
+
+/* I3C write transfer function */
+static int sbtsi_i3c_write(struct sbtsi_data *data, u8 reg, u8 val)
+{
+ u8 *buf = sbtsi_i3c_buf(data);
+ struct i3c_xfer xfers = {
+ .rnw = false,
+ .len = 2,
+ .data.out = buf,
+ };
+
+ buf[0] = reg;
+ buf[1] = val;
+
+ return i3c_device_do_xfers(data->i3cdev, &xfers, 1, I3C_SDR);
+}
+
+/* Unified transfer function for I2C and I3C access */
int sbtsi_xfer(struct sbtsi_data *data, u8 reg, u8 *val, bool is_read)
{
+ if (data->is_i3c)
+ return is_read ? sbtsi_i3c_read(data, reg, val)
+ : sbtsi_i3c_write(data, reg, *val);
+
return sbtsi_i2c_xfer(data, reg, val, is_read);
}
EXPORT_SYMBOL_GPL(sbtsi_xfer);
diff --git a/drivers/misc/amd-sbi/tsi-core.h b/drivers/misc/amd-sbi/tsi-core.h
new file mode 100644
index 000000000000..7dde040caf30
--- /dev/null
+++ b/drivers/misc/amd-sbi/tsi-core.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * AMD SBTSI core driver private definitions.
+ *
+ * Copyright (C) 2026 Advanced Micro Devices, Inc.
+ */
+
+#ifndef _LINUX_TSI_CORE_H_
+#define _LINUX_TSI_CORE_H_
+
+#include <linux/misc/tsi.h>
+
+/**
+ * struct sbtsi_i3c_priv - per-device state for I3C SBTSI (includes DMA-safe buffers)
+ * @data: public device state exposed via dev_set_drvdata()
+ * @buf: I3C transfer buffer; [0] register address, [1] data byte
+ */
+struct sbtsi_i3c_priv {
+ struct sbtsi_data data;
+ u8 buf[2];
+};
+
+#endif /* _LINUX_TSI_CORE_H_ */
diff --git a/drivers/misc/amd-sbi/tsi.c b/drivers/misc/amd-sbi/tsi.c
index a4c7e1be5624..8fb17ccab73d 100644
--- a/drivers/misc/amd-sbi/tsi.c
+++ b/drivers/misc/amd-sbi/tsi.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
- * tsi.c - AMD SBTSI I2C core driver. Probes the SBTSI device over I2C
+ * tsi.c - AMD SBTSI I2C/I3C core driver. Probes the SBTSI device over I2C/I3C
* and publishes an auxiliary device on the auxiliary bus.
*
* Copyright (C) 2026 Advanced Micro Devices, Inc.
@@ -10,8 +10,8 @@
#include <linux/bitfield.h>
#include <linux/module.h>
#include <linux/of.h>
-#include <linux/misc/tsi.h>
#include <linux/slab.h>
+#include "tsi-core.h"
#define SBTSI_REG_CONFIG 0x03 /* RO */
@@ -109,6 +109,7 @@ static int sbtsi_i2c_probe(struct i2c_client *client)
if (!data)
return -ENOMEM;
+ data->is_i3c = false;
data->client = client;
data->dev_addr = client->addr;
@@ -138,7 +139,56 @@ static struct i2c_driver sbtsi_driver = {
.id_table = sbtsi_id,
};
-module_i2c_driver(sbtsi_driver);
+static int sbtsi_i3c_probe(struct i3c_device *i3cdev)
+{
+ struct device *dev = i3cdev_to_dev(i3cdev);
+ struct i3c_device_info devinfo;
+ struct sbtsi_i3c_priv *i3c_priv;
+ struct sbtsi_data *data;
+
+ /*
+ * AMD OOB devices differ on basis of Instance ID,
+ * for SBTSI, instance ID is 0.
+ * As the device Id match is not on basis of Instance ID,
+ * add the below check to probe the SBTSI device only and
+ * not other OOB devices.
+ */
+ i3c_device_get_info(i3cdev, &devinfo);
+ if (I3C_PID_INSTANCE_ID(devinfo.pid) != 0)
+ return -ENXIO;
+
+ i3c_priv = devm_kzalloc(dev, sizeof(*i3c_priv), GFP_KERNEL);
+ if (!i3c_priv)
+ return -ENOMEM;
+
+ data = &i3c_priv->data;
+ data->i3cdev = i3cdev;
+ data->is_i3c = true;
+ data->dev_addr = devinfo.dyn_addr;
+
+ return sbtsi_probe_common(dev, data);
+}
+
+static const struct i3c_device_id sbtsi_i3c_id[] = {
+ /* PID for AMD SBTSI device */
+ I3C_DEVICE_EXTRA_INFO(0x112, 0x0, 0x1, NULL), /* Socket:0, Turin and Genoa */
+ I3C_DEVICE_EXTRA_INFO(0x0, 0x0, 0x118, NULL), /* Socket:0, Venice */
+ I3C_DEVICE_EXTRA_INFO(0x0, 0x100, 0x118, NULL), /* Socket:1, Venice */
+ I3C_DEVICE_EXTRA_INFO(0x112, 0x0, 0x119, NULL), /* Socket:0, Venice */
+ I3C_DEVICE_EXTRA_INFO(0x112, 0x100, 0x119, NULL), /* Socket:1, Venice */
+ {}
+};
+MODULE_DEVICE_TABLE(i3c, sbtsi_i3c_id);
+
+static struct i3c_driver sbtsi_i3c_driver = {
+ .driver = {
+ .name = "sbtsi-i3c",
+ },
+ .probe = sbtsi_i3c_probe,
+ .id_table = sbtsi_i3c_id,
+};
+
+module_i3c_i2c_driver(sbtsi_i3c_driver, &sbtsi_driver);
-MODULE_DESCRIPTION("AMD SB-TSI I2C core driver");
+MODULE_DESCRIPTION("AMD SB-TSI I2C/I3C core driver");
MODULE_LICENSE("GPL");
diff --git a/include/linux/misc/tsi.h b/include/linux/misc/tsi.h
index 55ee7e42a65d..02c90ec285ec 100644
--- a/include/linux/misc/tsi.h
+++ b/include/linux/misc/tsi.h
@@ -9,20 +9,27 @@
#define _LINUX_MISC_TSI_H_
#include <linux/i2c.h>
+#include <linux/i3c/device.h>
#include <linux/types.h>
/**
* struct sbtsi_data - driver private data for an AMD SB-TSI device
* @client: underlying I2C client
- * @dev_addr: I2C device address, used to name the misc device node
+ * @i3cdev: underlying I3C device (when using I3C bus)
+ * @dev_addr: I2C/I3C device address, used to name the misc device node
* @ext_range_mode: sensor uses extended temperature range
* @read_order: if set, decimal part must be read before integer part
+ * @is_i3c: true when the device is accessed over I3C
*/
struct sbtsi_data {
- struct i2c_client *client;
+ union {
+ struct i2c_client *client;
+ struct i3c_device *i3cdev;
+ };
u8 dev_addr;
bool ext_range_mode;
bool read_order;
+ bool is_i3c;
};
/*
--
2.34.1
^ permalink raw reply related
* [PATCH v3 7/8] hwmon: Add mutex protecting for sbtsi read/write through hwmon
From: Akshay Gupta @ 2026-06-22 13:58 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, NaveenKrishna.Chatradhi,
Anand.Umarji, Akshay.Gupta, Prathima.Lk
In-Reply-To: <20260622135821.2190260-1-Akshay.Gupta@amd.com>
From: Prathima <Prathima.Lk@amd.com>
Add a mutex and take it around SBTSI read/write paths so that only
one transaction runs at a time. The lock is held only for the
duration of the bus transfer and associated driver bookkeeping, not
across blocking work unrelated to SBTSI.
This is a concurrency hardening fix.
Reviewed-by: Akshay Gupta <Akshay.Gupta@amd.com>
Signed-off-by: Prathima <Prathima.Lk@amd.com>
---
Changes since v2:
- New patch
drivers/hwmon/sbtsi_temp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hwmon/sbtsi_temp.c b/drivers/hwmon/sbtsi_temp.c
index d7ae986d824c..11c8108d69b2 100644
--- a/drivers/hwmon/sbtsi_temp.c
+++ b/drivers/hwmon/sbtsi_temp.c
@@ -70,6 +70,7 @@ static int sbtsi_temp_read(struct sbtsi_data *data, u8 reg1, u8 reg2,
{
int ret;
+ guard(sbtsi)(data);
ret = sbtsi_xfer(data, reg1, val1, true);
if (!ret)
ret = sbtsi_xfer(data, reg2, val2, true);
@@ -84,6 +85,7 @@ static int sbtsi_temp_write(struct sbtsi_data *data, u8 reg_int, u8 reg_dec,
{
int ret;
+ guard(sbtsi)(data);
ret = sbtsi_xfer(data, reg_int, &val_int, false);
if (!ret)
ret = sbtsi_xfer(data, reg_dec, &val_dec, false);
--
2.34.1
^ permalink raw reply related
* [PATCH v3 6/8] misc: amd-sbi: Add SBTSI ioctl register transfer interface
From: Akshay Gupta @ 2026-06-22 13:58 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, NaveenKrishna.Chatradhi,
Anand.Umarji, Akshay.Gupta, Prathima.Lk
In-Reply-To: <20260622135821.2190260-1-Akshay.Gupta@amd.com>
From: Prathima <Prathima.Lk@amd.com>
Implement IOCTL interface for SB-TSI driver to enable userspace access
to TSI register read/write operations through the AMD Advanced Platform
Management Link (APML) protocol.
Add an ioctl command (SBTSI_IOCTL_REG_XFER_CMD) that accepts a register
address, data byte, and direction flag. Serialize access with a mutex
shared between the hwmon and ioctl paths to prevent concurrent bus
transactions from corrupting register state.
Reviewed-by: Akshay Gupta <Akshay.Gupta@amd.com>
Signed-off-by: Prathima <Prathima.Lk@amd.com>
---
Changes since v2:
- Address feedback to hide mutex with common guard function
- Separate out mutex patch in hwmon file to next patch, 7/8
- Use file operation, open/release and kref to prevent use-after-free
if the device is unbound in IOCTL
- Add check for msg.pad
Changes since v1:
- Use of devm_mutex_init in place of mutex_init
- Use of guard_mutex in place of mutex_lock()/mutex_unlock()
- Use of devm_add_action_or_reset() for clean removal
drivers/misc/amd-sbi/tsi-core.c | 118 +++++++++++++++++++++++++++++++-
drivers/misc/amd-sbi/tsi-core.h | 3 +
drivers/misc/amd-sbi/tsi.c | 36 +++++++++-
include/linux/misc/tsi.h | 15 ++++
include/uapi/misc/amd-apml.h | 23 +++++++
5 files changed, 191 insertions(+), 4 deletions(-)
diff --git a/drivers/misc/amd-sbi/tsi-core.c b/drivers/misc/amd-sbi/tsi-core.c
index 9278d06d8e5f..688b9221868f 100644
--- a/drivers/misc/amd-sbi/tsi-core.c
+++ b/drivers/misc/amd-sbi/tsi-core.c
@@ -6,7 +6,11 @@
* Copyright (C) 2026 Advanced Micro Devices, Inc.
*/
+#include <linux/fs.h>
+#include <linux/ioctl.h>
#include <linux/module.h>
+#include <linux/uaccess.h>
+#include <uapi/misc/amd-apml.h>
#include "tsi-core.h"
static inline struct sbtsi_i3c_priv *to_sbtsi_i3c_priv(struct sbtsi_data *data)
@@ -19,6 +23,17 @@ static u8 *sbtsi_i3c_buf(struct sbtsi_data *data)
return to_sbtsi_i3c_priv(data)->buf;
}
+void sbtsi_data_release(struct kref *kref)
+{
+ struct sbtsi_data *data = container_of(kref, struct sbtsi_data, kref);
+
+ mutex_destroy(&data->lock);
+ if (data->is_i3c)
+ kfree(to_sbtsi_i3c_priv(data));
+ else
+ kfree(data);
+}
+
/* I2C transfer function */
static int sbtsi_i2c_xfer(struct sbtsi_data *data, u8 reg, u8 *val, bool is_read)
{
@@ -80,7 +95,108 @@ int sbtsi_xfer(struct sbtsi_data *data, u8 reg, u8 *val, bool is_read)
if (data->is_i3c)
return is_read ? sbtsi_i3c_read(data, reg, val)
: sbtsi_i3c_write(data, reg, *val);
-
return sbtsi_i2c_xfer(data, reg, val, is_read);
}
EXPORT_SYMBOL_GPL(sbtsi_xfer);
+
+/*
+ * The mutex protects against concurrent access to the shared I2C/I3C bus by
+ * the hwmon sysfs and a userspace ioctl.
+ */
+static int sbtsi_xfer_ioctl(struct sbtsi_data *data, u8 reg, u8 *val, bool is_read)
+{
+ guard(sbtsi)(data);
+ return sbtsi_xfer(data, reg, val, is_read);
+}
+
+static int apml_tsi_reg_xfer(struct sbtsi_data *data,
+ struct apml_tsi_xfer_msg __user *arg)
+{
+ struct apml_tsi_xfer_msg msg = { 0 };
+ int ret;
+
+ if (data->detached)
+ return -ENODEV;
+
+ if (copy_from_user(&msg, arg, sizeof(struct apml_tsi_xfer_msg)))
+ return -EFAULT;
+
+ if (msg.pad)
+ return -EINVAL;
+
+ ret = sbtsi_xfer_ioctl(data, msg.reg_addr, &msg.data_in_out, msg.rflag);
+
+ if (msg.rflag && !ret) {
+ if (copy_to_user(arg, &msg, sizeof(struct apml_tsi_xfer_msg)))
+ return -EFAULT;
+ }
+ return ret;
+}
+
+static int sbtsi_open(struct inode *inode, struct file *fp)
+{
+ struct sbtsi_data *data;
+
+ data = container_of(fp->private_data, struct sbtsi_data, sbtsi_misc_dev);
+ if (data->detached)
+ return -ENODEV;
+
+ kref_get(&data->kref);
+
+ return 0;
+}
+
+static int sbtsi_release(struct inode *inode, struct file *fp)
+{
+ struct sbtsi_data *data;
+
+ data = container_of(fp->private_data, struct sbtsi_data, sbtsi_misc_dev);
+ kref_put(&data->kref, sbtsi_data_release);
+ return 0;
+}
+
+static long sbtsi_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
+{
+ void __user *argp = (void __user *)arg;
+ struct sbtsi_data *data;
+
+ data = container_of(fp->private_data, struct sbtsi_data, sbtsi_misc_dev);
+ switch (cmd) {
+ case SBTSI_IOCTL_REG_XFER_CMD:
+ return apml_tsi_reg_xfer(data, argp);
+ default:
+ return -ENOTTY;
+ }
+}
+
+static const struct file_operations sbtsi_fops = {
+ .owner = THIS_MODULE,
+ .open = sbtsi_open,
+ .release = sbtsi_release,
+ .unlocked_ioctl = sbtsi_ioctl,
+ .compat_ioctl = compat_ptr_ioctl,
+};
+
+int create_misc_tsi_device(struct sbtsi_data *data, struct device *dev)
+{
+ int ret;
+
+ data->sbtsi_misc_dev.name = devm_kasprintf(dev, GFP_KERNEL,
+ "sbtsi-%x", data->dev_addr);
+ if (!data->sbtsi_misc_dev.name)
+ return -ENOMEM;
+ data->sbtsi_misc_dev.minor = MISC_DYNAMIC_MINOR;
+ data->sbtsi_misc_dev.fops = &sbtsi_fops;
+ data->sbtsi_misc_dev.parent = dev;
+ data->sbtsi_misc_dev.nodename = devm_kasprintf(dev, GFP_KERNEL,
+ "sbtsi-%x", data->dev_addr);
+ if (!data->sbtsi_misc_dev.nodename)
+ return -ENOMEM;
+ data->sbtsi_misc_dev.mode = 0600;
+
+ ret = misc_register(&data->sbtsi_misc_dev);
+ if (ret)
+ return ret;
+
+ return 0;
+}
diff --git a/drivers/misc/amd-sbi/tsi-core.h b/drivers/misc/amd-sbi/tsi-core.h
index 7dde040caf30..c0849c704c7b 100644
--- a/drivers/misc/amd-sbi/tsi-core.h
+++ b/drivers/misc/amd-sbi/tsi-core.h
@@ -20,4 +20,7 @@ struct sbtsi_i3c_priv {
u8 buf[2];
};
+int create_misc_tsi_device(struct sbtsi_data *data, struct device *dev);
+
+void sbtsi_data_release(struct kref *kref);
#endif /* _LINUX_TSI_CORE_H_ */
diff --git a/drivers/misc/amd-sbi/tsi.c b/drivers/misc/amd-sbi/tsi.c
index 8fb17ccab73d..6649cd8cdf85 100644
--- a/drivers/misc/amd-sbi/tsi.c
+++ b/drivers/misc/amd-sbi/tsi.c
@@ -42,6 +42,21 @@ static void sbtsi_unregister_hwmon_adev(void *_adev)
auxiliary_device_uninit(adev);
}
+static void sbtsi_misc_unregister(void *arg)
+{
+ struct sbtsi_data *data = arg;
+
+ misc_deregister(&data->sbtsi_misc_dev);
+ data->detached = true;
+}
+
+static void sbtsi_driver_unref(void *arg)
+{
+ struct sbtsi_data *data = arg;
+
+ kref_put(&data->kref, sbtsi_data_release);
+}
+
/*
* Create and publish an auxiliary device. The hwmon driver in
* drivers/hwmon/sbtsi_temp.c binds to this device.
@@ -89,6 +104,13 @@ static int sbtsi_probe_common(struct device *dev, struct sbtsi_data *data)
u8 val;
int err;
+ mutex_init(&data->lock);
+ kref_init(&data->kref);
+
+ err = devm_add_action_or_reset(dev, sbtsi_driver_unref, data);
+ if (err)
+ return err;
+
err = sbtsi_xfer(data, SBTSI_REG_CONFIG, &val, true);
if (err)
return err;
@@ -97,7 +119,15 @@ static int sbtsi_probe_common(struct device *dev, struct sbtsi_data *data)
data->read_order = FIELD_GET(BIT(SBTSI_CONFIG_READ_ORDER_SHIFT), val);
dev_set_drvdata(dev, data);
- return sbtsi_create_hwmon_adev(dev, data->dev_addr);
+ err = sbtsi_create_hwmon_adev(dev, data->dev_addr);
+ if (err < 0)
+ return err;
+
+ err = create_misc_tsi_device(data, dev);
+ if (err)
+ return err;
+
+ return devm_add_action(dev, sbtsi_misc_unregister, data);
}
static int sbtsi_i2c_probe(struct i2c_client *client)
@@ -105,7 +135,7 @@ static int sbtsi_i2c_probe(struct i2c_client *client)
struct device *dev = &client->dev;
struct sbtsi_data *data;
- data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ data = kzalloc_obj(*data);
if (!data)
return -ENOMEM;
@@ -157,7 +187,7 @@ static int sbtsi_i3c_probe(struct i3c_device *i3cdev)
if (I3C_PID_INSTANCE_ID(devinfo.pid) != 0)
return -ENXIO;
- i3c_priv = devm_kzalloc(dev, sizeof(*i3c_priv), GFP_KERNEL);
+ i3c_priv = kzalloc_obj(*i3c_priv);
if (!i3c_priv)
return -ENOMEM;
diff --git a/include/linux/misc/tsi.h b/include/linux/misc/tsi.h
index 02c90ec285ec..3ce3770d1ad5 100644
--- a/include/linux/misc/tsi.h
+++ b/include/linux/misc/tsi.h
@@ -8,30 +8,45 @@
#ifndef _LINUX_MISC_TSI_H_
#define _LINUX_MISC_TSI_H_
+#include <linux/cleanup.h>
#include <linux/i2c.h>
#include <linux/i3c/device.h>
+#include <linux/kref.h>
+#include <linux/miscdevice.h>
+#include <linux/mutex.h>
#include <linux/types.h>
/**
* struct sbtsi_data - driver private data for an AMD SB-TSI device
* @client: underlying I2C client
* @i3cdev: underlying I3C device (when using I3C bus)
+ * @sbtsi_misc_dev: miscdevice exposing ioctl interface at /dev/sbtsi-<addr>
+ * @lock: mutex protecting concurrent access to the device
+ * @kref: reference count; keeps @sbtsi_data alive while misc fds are open
* @dev_addr: I2C/I3C device address, used to name the misc device node
* @ext_range_mode: sensor uses extended temperature range
* @read_order: if set, decimal part must be read before integer part
* @is_i3c: true when the device is accessed over I3C
+ * @detached: set on driver unbind; open/ioctl return -ENODEV afterward
*/
struct sbtsi_data {
union {
struct i2c_client *client;
struct i3c_device *i3cdev;
};
+ struct miscdevice sbtsi_misc_dev;
+ struct mutex lock; /* protects concurrent access to the device */
+ struct kref kref;
u8 dev_addr;
bool ext_range_mode;
bool read_order;
bool is_i3c;
+ bool detached;
};
+DEFINE_GUARD(sbtsi, struct sbtsi_data *, mutex_lock(&_T->lock),
+ mutex_unlock(&_T->lock))
+
/*
* Name of the auxiliary device published on the auxiliary bus by the core
* driver. The full device name is "amd-sbtsi.temp-sensor.<id>". where
diff --git a/include/uapi/misc/amd-apml.h b/include/uapi/misc/amd-apml.h
index 745b3338fc06..8a85f79b0938 100644
--- a/include/uapi/misc/amd-apml.h
+++ b/include/uapi/misc/amd-apml.h
@@ -73,6 +73,13 @@ struct apml_reg_xfer_msg {
__u8 rflag;
};
+struct apml_tsi_xfer_msg {
+ __u8 reg_addr; /* TSI register address offset */
+ __u8 data_in_out; /* Register data for read/write */
+ __u8 rflag; /* Register read or write */
+ __u8 pad; /* Explicit padding */
+};
+
/*
* AMD sideband interface base IOCTL
*/
@@ -149,4 +156,20 @@ struct apml_reg_xfer_msg {
*/
#define SBRMI_IOCTL_REG_XFER_CMD _IOWR(SB_BASE_IOCTL_NR, 3, struct apml_reg_xfer_msg)
+/**
+ * DOC: SBTSI_IOCTL_REG_XFER_CMD
+ *
+ * @Parameters
+ *
+ * @struct apml_tsi_xfer_msg
+ * Pointer to the &struct apml_tsi_xfer_msg that will contain the protocol
+ * information
+ *
+ * @Description
+ * IOCTL command for APML TSI messages using generic _IOWR
+ * The IOCTL provides userspace access to AMD sideband TSI register xfer protocol
+ * - TSI protocol to read/write temperature sensor registers
+ */
+#define SBTSI_IOCTL_REG_XFER_CMD _IOWR(SB_BASE_IOCTL_NR, 4, struct apml_tsi_xfer_msg)
+
#endif /*_AMD_APML_H_*/
--
2.34.1
^ permalink raw reply related
* [PATCH v3 4/8] misc: amd-sbi: Consolidate Common SBTSI Probe Path for I2C and I3C
From: Akshay Gupta @ 2026-06-22 13:58 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, NaveenKrishna.Chatradhi,
Anand.Umarji, Akshay.Gupta, Prathima.Lk
In-Reply-To: <20260622135821.2190260-1-Akshay.Gupta@amd.com>
From: Prathima <Prathima.Lk@amd.com>
Refactor shared probe procedures into sbtsi_probe_common() to ensure
that I2C and I3C probes focus solely on bus-specific allocation and
device configuration.
The utility function reads the configuration register via sbtsi_xfer(),
initializes ext_range_mode and read_order, assigns the driver data,
and registers the hwmon auxiliary device.
Utilizing sbtsi_xfer() for both buses eliminates the need to duplicate
the I2C SMBus and I3C transfer paths within tsi.c.
Reviewed-by: Akshay Gupta <Akshay.Gupta@amd.com>
Signed-off-by: Prathima <Prathima.Lk@amd.com>
---
Changes since v2:
- New patch to refactor SBTSI common probe
drivers/misc/amd-sbi/tsi.c | 26 ++++++++++++++++++--------
include/linux/misc/tsi.h | 2 ++
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/misc/amd-sbi/tsi.c b/drivers/misc/amd-sbi/tsi.c
index dfdd730b906a..a4c7e1be5624 100644
--- a/drivers/misc/amd-sbi/tsi.c
+++ b/drivers/misc/amd-sbi/tsi.c
@@ -84,25 +84,35 @@ static int sbtsi_create_hwmon_adev(struct device *dev, u8 dev_addr)
return devm_add_action_or_reset(dev, sbtsi_unregister_hwmon_adev, adev);
}
+static int sbtsi_probe_common(struct device *dev, struct sbtsi_data *data)
+{
+ u8 val;
+ int err;
+
+ err = sbtsi_xfer(data, SBTSI_REG_CONFIG, &val, true);
+ if (err)
+ return err;
+
+ data->ext_range_mode = FIELD_GET(BIT(SBTSI_CONFIG_EXT_RANGE_SHIFT), val);
+ data->read_order = FIELD_GET(BIT(SBTSI_CONFIG_READ_ORDER_SHIFT), val);
+
+ dev_set_drvdata(dev, data);
+ return sbtsi_create_hwmon_adev(dev, data->dev_addr);
+}
+
static int sbtsi_i2c_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
struct sbtsi_data *data;
- int err;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
data->client = client;
- err = i2c_smbus_read_byte_data(data->client, SBTSI_REG_CONFIG);
- if (err < 0)
- return err;
- data->ext_range_mode = FIELD_GET(BIT(SBTSI_CONFIG_EXT_RANGE_SHIFT), err);
- data->read_order = FIELD_GET(BIT(SBTSI_CONFIG_READ_ORDER_SHIFT), err);
+ data->dev_addr = client->addr;
- dev_set_drvdata(dev, data);
- return sbtsi_create_hwmon_adev(dev, client->addr);
+ return sbtsi_probe_common(dev, data);
}
static const struct i2c_device_id sbtsi_id[] = {
diff --git a/include/linux/misc/tsi.h b/include/linux/misc/tsi.h
index 2d2709f1ff32..55ee7e42a65d 100644
--- a/include/linux/misc/tsi.h
+++ b/include/linux/misc/tsi.h
@@ -14,11 +14,13 @@
/**
* struct sbtsi_data - driver private data for an AMD SB-TSI device
* @client: underlying I2C client
+ * @dev_addr: I2C device address, used to name the misc device node
* @ext_range_mode: sensor uses extended temperature range
* @read_order: if set, decimal part must be read before integer part
*/
struct sbtsi_data {
struct i2c_client *client;
+ u8 dev_addr;
bool ext_range_mode;
bool read_order;
};
--
2.34.1
^ permalink raw reply related
* [PATCH v3 3/8] hwmon/misc: amd-sbi: Move sbtsi register transfer to core abstraction
From: Akshay Gupta @ 2026-06-22 13:58 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, NaveenKrishna.Chatradhi,
Anand.Umarji, Akshay.Gupta, Prathima.Lk
In-Reply-To: <20260622135821.2190260-1-Akshay.Gupta@amd.com>
From: Prathima <Prathima.Lk@amd.com>
Move the I2C read/write byte operations from the sbtsi hwmon driver into
a common sbtsi_xfer() function in tsi-core.c.
This decouples the hwmon sensor driver from the underlying bus transport,
preparing for I3C support in a subsequent patch.
This patch does not introduce any functional changes. The updates are
limited to code organization/cleanup and should not affect the runtime
behavior of the driver
Reviewed-by: Akshay Gupta <Akshay.Gupta@amd.com>
Signed-off-by: Prathima <Prathima.Lk@amd.com>
---
Changes since v2:
- Name change for header inclusion guard to misc specific
Changes since v1:
- New patch
drivers/hwmon/sbtsi_temp.c | 17 ++++++-----------
drivers/misc/amd-sbi/Makefile | 2 +-
drivers/misc/amd-sbi/tsi-core.c | 30 ++++++++++++++++++++++++++++++
include/linux/misc/tsi.h | 13 +++++++++++++
4 files changed, 50 insertions(+), 12 deletions(-)
create mode 100644 drivers/misc/amd-sbi/tsi-core.c
diff --git a/drivers/hwmon/sbtsi_temp.c b/drivers/hwmon/sbtsi_temp.c
index 078f4ab25bde..d7ae986d824c 100644
--- a/drivers/hwmon/sbtsi_temp.c
+++ b/drivers/hwmon/sbtsi_temp.c
@@ -70,15 +70,10 @@ static int sbtsi_temp_read(struct sbtsi_data *data, u8 reg1, u8 reg2,
{
int ret;
- ret = i2c_smbus_read_byte_data(data->client, reg1);
- if (ret < 0)
- return ret;
- *val1 = ret;
- ret = i2c_smbus_read_byte_data(data->client, reg2);
- if (ret < 0)
- return ret;
- *val2 = ret;
- return 0;
+ ret = sbtsi_xfer(data, reg1, val1, true);
+ if (!ret)
+ ret = sbtsi_xfer(data, reg2, val2, true);
+ return ret;
}
/*
@@ -89,9 +84,9 @@ static int sbtsi_temp_write(struct sbtsi_data *data, u8 reg_int, u8 reg_dec,
{
int ret;
- ret = i2c_smbus_write_byte_data(data->client, reg_int, val_int);
+ ret = sbtsi_xfer(data, reg_int, &val_int, false);
if (!ret)
- ret = i2c_smbus_write_byte_data(data->client, reg_dec, val_dec);
+ ret = sbtsi_xfer(data, reg_dec, &val_dec, false);
return ret;
}
diff --git a/drivers/misc/amd-sbi/Makefile b/drivers/misc/amd-sbi/Makefile
index 28f95b9e204f..ce9321f5c601 100644
--- a/drivers/misc/amd-sbi/Makefile
+++ b/drivers/misc/amd-sbi/Makefile
@@ -3,5 +3,5 @@ sbrmi-i2c-objs += rmi-i2c.o rmi-core.o
sbrmi-i2c-$(CONFIG_AMD_SBRMI_HWMON) += rmi-hwmon.o
obj-$(CONFIG_AMD_SBRMI_I2C) += sbrmi-i2c.o
# SBTSI Configuration
-sbtsi-objs += tsi.o
+sbtsi-objs += tsi.o tsi-core.o
obj-$(CONFIG_AMD_SBTSI) += sbtsi.o
diff --git a/drivers/misc/amd-sbi/tsi-core.c b/drivers/misc/amd-sbi/tsi-core.c
new file mode 100644
index 000000000000..6ef1831515bb
--- /dev/null
+++ b/drivers/misc/amd-sbi/tsi-core.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * tsi-core.c - file defining SB-TSI protocols compliant
+ * AMD SoC device.
+ *
+ * Copyright (C) 2026 Advanced Micro Devices, Inc.
+ */
+
+#include <linux/module.h>
+#include <linux/misc/tsi.h>
+
+/* I2C transfer function */
+static int sbtsi_i2c_xfer(struct sbtsi_data *data, u8 reg, u8 *val, bool is_read)
+{
+ if (is_read) {
+ int ret = i2c_smbus_read_byte_data(data->client, reg);
+
+ if (ret < 0)
+ return ret;
+ *val = ret;
+ return 0;
+ }
+ return i2c_smbus_write_byte_data(data->client, reg, *val);
+}
+
+int sbtsi_xfer(struct sbtsi_data *data, u8 reg, u8 *val, bool is_read)
+{
+ return sbtsi_i2c_xfer(data, reg, val, is_read);
+}
+EXPORT_SYMBOL_GPL(sbtsi_xfer);
diff --git a/include/linux/misc/tsi.h b/include/linux/misc/tsi.h
index befdc2d14160..2d2709f1ff32 100644
--- a/include/linux/misc/tsi.h
+++ b/include/linux/misc/tsi.h
@@ -31,4 +31,17 @@ struct sbtsi_data {
#define AMD_SBTSI_ADEV "amd-sbtsi"
#define AMD_SBTSI_AUX_HWMON "temp-sensor"
+/**
+ * sbtsi_xfer - Perform a register read or write transfer on an AMD SB-TSI device.
+ *
+ * @data: Pointer to the sbtsi_data structure containing the device context
+ * @reg: Register address to access.
+ * @val: Pointer to the value to read into or write from.
+ * @is_read: If true, performs a read transfer and stores the result in @val.
+ * If false, performs a write transfer using the value in @val.
+ *
+ * Returns 0 on success, or a negative error code on failure.
+ */
+int sbtsi_xfer(struct sbtsi_data *data, u8 reg, u8 *val, bool is_read);
+
#endif /* _LINUX_MISC_TSI_H_ */
--
2.34.1
^ permalink raw reply related
* [PATCH v3 2/8] hwmon: sbtsi_temp: Refactor temperature register access into helpers
From: Akshay Gupta @ 2026-06-22 13:58 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, NaveenKrishna.Chatradhi,
Anand.Umarji, Akshay.Gupta, Prathima.Lk
In-Reply-To: <20260622135821.2190260-1-Akshay.Gupta@amd.com>
From: Prathima <Prathima.Lk@amd.com>
Extract the paired integer/decimal register reads and writes from the
hwmon read/write callbacks into sbtsi_temp_read() and sbtsi_temp_write()
helpers. This consolidates error handling and respects the ReadOrder bit
for atomic temperature latching.
This keeps register access independent while preserving existing hwmon
functionality.
Reviewed-by: Akshay Gupta <Akshay.Gupta@amd.com>
Signed-off-by: Prathima <Prathima.Lk@amd.com>
---
Changes since v1:
- New patch
drivers/hwmon/sbtsi_temp.c | 84 +++++++++++++++++++++++++++-----------
1 file changed, 61 insertions(+), 23 deletions(-)
diff --git a/drivers/hwmon/sbtsi_temp.c b/drivers/hwmon/sbtsi_temp.c
index 28258bf49922..078f4ab25bde 100644
--- a/drivers/hwmon/sbtsi_temp.c
+++ b/drivers/hwmon/sbtsi_temp.c
@@ -61,40 +61,82 @@ static inline void sbtsi_mc_to_reg(s32 temp, u8 *integer, u8 *decimal)
*decimal = (temp & 0x7) << 5;
}
+/*
+ * Read integer and decimal parts of an SB-TSI temperature register pair
+ * The read order is determined by the ReadOrder bit to ensure atomic latching.
+ */
+static int sbtsi_temp_read(struct sbtsi_data *data, u8 reg1, u8 reg2,
+ u8 *val1, u8 *val2)
+{
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(data->client, reg1);
+ if (ret < 0)
+ return ret;
+ *val1 = ret;
+ ret = i2c_smbus_read_byte_data(data->client, reg2);
+ if (ret < 0)
+ return ret;
+ *val2 = ret;
+ return 0;
+}
+
+/*
+ * Write integer and decimal parts of an SB-TSI temperature register pair.
+ */
+static int sbtsi_temp_write(struct sbtsi_data *data, u8 reg_int, u8 reg_dec,
+ u8 val_int, u8 val_dec)
+{
+ int ret;
+
+ ret = i2c_smbus_write_byte_data(data->client, reg_int, val_int);
+ if (!ret)
+ ret = i2c_smbus_write_byte_data(data->client, reg_dec, val_dec);
+ return ret;
+}
+
static int sbtsi_read(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long *val)
{
struct sbtsi_data *data = dev_get_drvdata(dev);
s32 temp_int, temp_dec;
+ int err;
+ u8 val_int, val_dec;
switch (attr) {
case hwmon_temp_input:
- if (data->read_order) {
- temp_dec = i2c_smbus_read_byte_data(data->client, SBTSI_REG_TEMP_DEC);
- temp_int = i2c_smbus_read_byte_data(data->client, SBTSI_REG_TEMP_INT);
- } else {
- temp_int = i2c_smbus_read_byte_data(data->client, SBTSI_REG_TEMP_INT);
- temp_dec = i2c_smbus_read_byte_data(data->client, SBTSI_REG_TEMP_DEC);
- }
+ if (data->read_order)
+ err = sbtsi_temp_read(data,
+ SBTSI_REG_TEMP_DEC, SBTSI_REG_TEMP_INT,
+ &val_dec, &val_int);
+ else
+ err = sbtsi_temp_read(data,
+ SBTSI_REG_TEMP_INT, SBTSI_REG_TEMP_DEC,
+ &val_int, &val_dec);
+ if (err < 0)
+ return err;
break;
case hwmon_temp_max:
- temp_int = i2c_smbus_read_byte_data(data->client, SBTSI_REG_TEMP_HIGH_INT);
- temp_dec = i2c_smbus_read_byte_data(data->client, SBTSI_REG_TEMP_HIGH_DEC);
+ err = sbtsi_temp_read(data,
+ SBTSI_REG_TEMP_HIGH_INT, SBTSI_REG_TEMP_HIGH_DEC,
+ &val_int, &val_dec);
+ if (err < 0)
+ return err;
break;
case hwmon_temp_min:
- temp_int = i2c_smbus_read_byte_data(data->client, SBTSI_REG_TEMP_LOW_INT);
- temp_dec = i2c_smbus_read_byte_data(data->client, SBTSI_REG_TEMP_LOW_DEC);
+ err = sbtsi_temp_read(data,
+ SBTSI_REG_TEMP_LOW_INT, SBTSI_REG_TEMP_LOW_DEC,
+ &val_int, &val_dec);
+
+ if (err < 0)
+ return err;
break;
default:
return -EINVAL;
}
-
- if (temp_int < 0)
- return temp_int;
- if (temp_dec < 0)
- return temp_dec;
-
+ temp_int = val_int;
+ temp_dec = val_dec;
*val = sbtsi_reg_to_mc(temp_int, temp_dec);
if (data->ext_range_mode)
*val -= SBTSI_TEMP_EXT_RANGE_ADJ;
@@ -106,7 +148,7 @@ static int sbtsi_write(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long val)
{
struct sbtsi_data *data = dev_get_drvdata(dev);
- int reg_int, reg_dec, err;
+ int reg_int, reg_dec;
u8 temp_int, temp_dec;
switch (attr) {
@@ -127,11 +169,7 @@ static int sbtsi_write(struct device *dev, enum hwmon_sensor_types type,
val = clamp_val(val, SBTSI_TEMP_MIN, SBTSI_TEMP_MAX);
sbtsi_mc_to_reg(val, &temp_int, &temp_dec);
- err = i2c_smbus_write_byte_data(data->client, reg_int, temp_int);
- if (err)
- return err;
-
- return i2c_smbus_write_byte_data(data->client, reg_dec, temp_dec);
+ return sbtsi_temp_write(data, reg_int, reg_dec, temp_int, temp_dec);
}
static umode_t sbtsi_is_visible(const void *data,
--
2.34.1
^ permalink raw reply related
* [PATCH v3 1/8] hwmon/misc: amd-sbi: Move core sbtsi support from hwmon to misc
From: Akshay Gupta @ 2026-06-22 13:58 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, NaveenKrishna.Chatradhi,
Anand.Umarji, Akshay.Gupta, Prathima.Lk
In-Reply-To: <20260622135821.2190260-1-Akshay.Gupta@amd.com>
From: Prathima <Prathima.Lk@amd.com>
Move SBTSI(Side-Band Temperature Sensor Interface) core functionality out
of the hwmon-only path and into drivers/misc/amd-sbi so it can be reused
by non-hwmon consumers.
I2C probe parsing is moved from drivers/hwmon/sbtsi_temp.c
into drivers/misc/amd-sbi/tsi.c under CONFIG_AMD_SBTSI. The core driver
stores struct sbtsi_data on the bus device and registers an auxiliary
device amd-sbtsi.temp-sensor.<addr> per target.
This split prepares the driver for additional interfaces while keeping
hwmon support in hwmon subsystem on top of common SBTSI core logic.
Reviewed-by: Akshay Gupta <Akshay.Gupta@amd.com>
Signed-off-by: Prathima <Prathima.Lk@amd.com>
---
Changes since v2:
- Change hwmon config symbol dependency, "depends on" to "select"
as "depends on" could silently disable the hwmon driver
Changes since v1:
- Use auxiliary device to probe hwmon sensor instead of moving
the hwmon functionality to misc subsystem. This change is as
per feedback.
drivers/hwmon/Kconfig | 2 +-
drivers/hwmon/sbtsi_temp.c | 71 ++++--------------
drivers/misc/amd-sbi/Kconfig | 13 ++++
drivers/misc/amd-sbi/Makefile | 3 +
drivers/misc/amd-sbi/tsi.c | 134 ++++++++++++++++++++++++++++++++++
include/linux/misc/tsi.h | 34 +++++++++
6 files changed, 198 insertions(+), 59 deletions(-)
create mode 100644 drivers/misc/amd-sbi/tsi.c
create mode 100644 include/linux/misc/tsi.h
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index e4c4f2b09732..8f204cf49b6e 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1963,7 +1963,7 @@ config SENSORS_SL28CPLD
config SENSORS_SBTSI
tristate "Emulated SB-TSI temperature sensor"
- depends on I2C
+ select AMD_SBTSI
help
If you say yes here you get support for emulated temperature
sensors on AMD SoCs with SB-TSI interface connected to a BMC device.
diff --git a/drivers/hwmon/sbtsi_temp.c b/drivers/hwmon/sbtsi_temp.c
index c28f8625cd3a..28258bf49922 100644
--- a/drivers/hwmon/sbtsi_temp.c
+++ b/drivers/hwmon/sbtsi_temp.c
@@ -7,13 +7,12 @@
* Copyright (c) 2020, Kun Yi <kunyi@google.com>
*/
+#include <linux/auxiliary_bus.h>
#include <linux/err.h>
-#include <linux/i2c.h>
-#include <linux/init.h>
#include <linux/hwmon.h>
+#include <linux/init.h>
#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/bitfield.h>
+#include <linux/misc/tsi.h>
/*
* SB-TSI registers only support SMBus byte data access. "_INT" registers are
@@ -22,39 +21,17 @@
*/
#define SBTSI_REG_TEMP_INT 0x01 /* RO */
#define SBTSI_REG_STATUS 0x02 /* RO */
-#define SBTSI_REG_CONFIG 0x03 /* RO */
#define SBTSI_REG_TEMP_HIGH_INT 0x07 /* RW */
#define SBTSI_REG_TEMP_LOW_INT 0x08 /* RW */
#define SBTSI_REG_TEMP_DEC 0x10 /* RW */
#define SBTSI_REG_TEMP_HIGH_DEC 0x13 /* RW */
#define SBTSI_REG_TEMP_LOW_DEC 0x14 /* RW */
-/*
- * Bit for reporting value with temperature measurement range.
- * bit == 0: Use default temperature range (0C to 255.875C).
- * bit == 1: Use extended temperature range (-49C to +206.875C).
- */
-#define SBTSI_CONFIG_EXT_RANGE_SHIFT 2
-/*
- * ReadOrder bit specifies the reading order of integer and decimal part of
- * CPU temperature for atomic reads. If bit == 0, reading integer part triggers
- * latching of the decimal part, so integer part should be read first.
- * If bit == 1, read order should be reversed.
- */
-#define SBTSI_CONFIG_READ_ORDER_SHIFT 5
-
#define SBTSI_TEMP_EXT_RANGE_ADJ 49000
#define SBTSI_TEMP_MIN 0
#define SBTSI_TEMP_MAX 255875
-/* Each client has this additional data */
-struct sbtsi_data {
- struct i2c_client *client;
- bool ext_range_mode;
- bool read_order;
-};
-
/*
* From SB-TSI spec: CPU temperature readings and limit registers encode the
* temperature in increments of 0.125 from 0 to 255.875. The "high byte"
@@ -195,55 +172,33 @@ static const struct hwmon_chip_info sbtsi_chip_info = {
.info = sbtsi_info,
};
-static int sbtsi_probe(struct i2c_client *client)
+static int sbtsi_probe(struct auxiliary_device *adev,
+ const struct auxiliary_device_id *id)
{
- struct device *dev = &client->dev;
+ struct sbtsi_data *data = dev_get_drvdata(adev->dev.parent);
+ struct device *dev = &adev->dev;
struct device *hwmon_dev;
- struct sbtsi_data *data;
- int err;
- data = devm_kzalloc(dev, sizeof(struct sbtsi_data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
-
- data->client = client;
-
- err = i2c_smbus_read_byte_data(data->client, SBTSI_REG_CONFIG);
- if (err < 0)
- return err;
- data->ext_range_mode = FIELD_GET(BIT(SBTSI_CONFIG_EXT_RANGE_SHIFT), err);
- data->read_order = FIELD_GET(BIT(SBTSI_CONFIG_READ_ORDER_SHIFT), err);
-
- hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name, data,
+ hwmon_dev = devm_hwmon_device_register_with_info(dev, "sbtsi", data,
&sbtsi_chip_info, NULL);
return PTR_ERR_OR_ZERO(hwmon_dev);
}
-static const struct i2c_device_id sbtsi_id[] = {
- { .name = "sbtsi" },
+static const struct auxiliary_device_id sbtsi_id[] = {
+ { .name = AMD_SBTSI_ADEV "." AMD_SBTSI_AUX_HWMON },
{ }
};
-MODULE_DEVICE_TABLE(i2c, sbtsi_id);
+MODULE_DEVICE_TABLE(auxiliary, sbtsi_id);
-static const struct of_device_id __maybe_unused sbtsi_of_match[] = {
- {
- .compatible = "amd,sbtsi",
- },
- { },
-};
-MODULE_DEVICE_TABLE(of, sbtsi_of_match);
-
-static struct i2c_driver sbtsi_driver = {
+static struct auxiliary_driver sbtsi_driver = {
.driver = {
.name = "sbtsi",
- .of_match_table = of_match_ptr(sbtsi_of_match),
},
.probe = sbtsi_probe,
.id_table = sbtsi_id,
};
-
-module_i2c_driver(sbtsi_driver);
+module_auxiliary_driver(sbtsi_driver);
MODULE_AUTHOR("Kun Yi <kunyi@google.com>");
MODULE_DESCRIPTION("Hwmon driver for AMD SB-TSI emulated sensor");
diff --git a/drivers/misc/amd-sbi/Kconfig b/drivers/misc/amd-sbi/Kconfig
index 30e7fad7356c..512251690e0e 100644
--- a/drivers/misc/amd-sbi/Kconfig
+++ b/drivers/misc/amd-sbi/Kconfig
@@ -20,3 +20,16 @@ config AMD_SBRMI_HWMON
This provides support for RMI device hardware monitoring. If enabled,
a hardware monitoring device will be created for each socket in
the system.
+
+config AMD_SBTSI
+ tristate "AMD side band TSI support"
+ depends on I2C
+ depends on ARM || ARM64 || COMPILE_TEST
+ select AUXILIARY_BUS
+ help
+ Enables support for the AMD SB-TSI (Side Band Temperature Sensor
+ Interface) driver, which provides access to emulated CPU temperature
+ sensors on AMD SoCs via an I2C connected BMC device.
+
+ This driver can also be built as a module. If so, the module will
+ be called sbtsi.
diff --git a/drivers/misc/amd-sbi/Makefile b/drivers/misc/amd-sbi/Makefile
index 38eaaa651fd9..28f95b9e204f 100644
--- a/drivers/misc/amd-sbi/Makefile
+++ b/drivers/misc/amd-sbi/Makefile
@@ -2,3 +2,6 @@
sbrmi-i2c-objs += rmi-i2c.o rmi-core.o
sbrmi-i2c-$(CONFIG_AMD_SBRMI_HWMON) += rmi-hwmon.o
obj-$(CONFIG_AMD_SBRMI_I2C) += sbrmi-i2c.o
+# SBTSI Configuration
+sbtsi-objs += tsi.o
+obj-$(CONFIG_AMD_SBTSI) += sbtsi.o
diff --git a/drivers/misc/amd-sbi/tsi.c b/drivers/misc/amd-sbi/tsi.c
new file mode 100644
index 000000000000..dfdd730b906a
--- /dev/null
+++ b/drivers/misc/amd-sbi/tsi.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * tsi.c - AMD SBTSI I2C core driver. Probes the SBTSI device over I2C
+ * and publishes an auxiliary device on the auxiliary bus.
+ *
+ * Copyright (C) 2026 Advanced Micro Devices, Inc.
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/bitfield.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/misc/tsi.h>
+#include <linux/slab.h>
+
+#define SBTSI_REG_CONFIG 0x03 /* RO */
+
+/*
+ * Bit for reporting value with temperature measurement range.
+ * bit == 0: Use default temperature range (0C to 255.875C).
+ * bit == 1: Use extended temperature range (-49C to +206.875C).
+ */
+#define SBTSI_CONFIG_EXT_RANGE_SHIFT 2
+
+/*
+ * ReadOrder bit specifies the reading order of integer and decimal part of
+ * CPU temperature for atomic reads. If bit == 0, reading integer part triggers
+ * latching of the decimal part, so integer part should be read first.
+ */
+#define SBTSI_CONFIG_READ_ORDER_SHIFT 5
+
+static void sbtsi_adev_release(struct device *dev)
+{
+ kfree(to_auxiliary_dev(dev));
+}
+
+static void sbtsi_unregister_hwmon_adev(void *_adev)
+{
+ struct auxiliary_device *adev = _adev;
+
+ auxiliary_device_delete(adev);
+ auxiliary_device_uninit(adev);
+}
+
+/*
+ * Create and publish an auxiliary device. The hwmon driver in
+ * drivers/hwmon/sbtsi_temp.c binds to this device.
+ *
+ * @dev: I2C device (parent of the auxiliary device)
+ * @dev_addr: I2C address — used as the auxiliary device instance ID so that
+ * each socket gets a unique name.
+ */
+static int sbtsi_create_hwmon_adev(struct device *dev, u8 dev_addr)
+{
+ struct auxiliary_device *adev;
+ int ret;
+
+ adev = kzalloc_obj(*adev);
+ if (!adev)
+ return -ENOMEM;
+
+ adev->name = AMD_SBTSI_AUX_HWMON;
+ /*
+ * In a multi-socket system, otherwise identical devices do not
+ * share the same static address; each instance has its own address,
+ * which must be supplied via the device tree (DTS).
+ */
+ adev->id = dev_addr;
+ adev->dev.parent = dev;
+ adev->dev.release = sbtsi_adev_release;
+
+ ret = auxiliary_device_init(adev);
+ if (ret) {
+ kfree(adev);
+ return ret;
+ }
+
+ ret = __auxiliary_device_add(adev, AMD_SBTSI_ADEV);
+ if (ret) {
+ auxiliary_device_uninit(adev);
+ return ret;
+ }
+
+ return devm_add_action_or_reset(dev, sbtsi_unregister_hwmon_adev, adev);
+}
+
+static int sbtsi_i2c_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct sbtsi_data *data;
+ int err;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->client = client;
+ err = i2c_smbus_read_byte_data(data->client, SBTSI_REG_CONFIG);
+ if (err < 0)
+ return err;
+ data->ext_range_mode = FIELD_GET(BIT(SBTSI_CONFIG_EXT_RANGE_SHIFT), err);
+ data->read_order = FIELD_GET(BIT(SBTSI_CONFIG_READ_ORDER_SHIFT), err);
+
+ dev_set_drvdata(dev, data);
+ return sbtsi_create_hwmon_adev(dev, client->addr);
+}
+
+static const struct i2c_device_id sbtsi_id[] = {
+ { .name = "sbtsi" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sbtsi_id);
+
+static const struct of_device_id __maybe_unused sbtsi_of_match[] = {
+ {
+ .compatible = "amd,sbtsi",
+ },
+ { },
+};
+MODULE_DEVICE_TABLE(of, sbtsi_of_match);
+
+static struct i2c_driver sbtsi_driver = {
+ .driver = {
+ .name = "sbtsi-i2c",
+ .of_match_table = of_match_ptr(sbtsi_of_match),
+ },
+ .probe = sbtsi_i2c_probe,
+ .id_table = sbtsi_id,
+};
+
+module_i2c_driver(sbtsi_driver);
+
+MODULE_DESCRIPTION("AMD SB-TSI I2C core driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/misc/tsi.h b/include/linux/misc/tsi.h
new file mode 100644
index 000000000000..befdc2d14160
--- /dev/null
+++ b/include/linux/misc/tsi.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * AMD SBTSI shared data structure and auxiliary bus definitions.
+ *
+ * Copyright (C) 2026 Advanced Micro Devices, Inc.
+ */
+
+#ifndef _LINUX_MISC_TSI_H_
+#define _LINUX_MISC_TSI_H_
+
+#include <linux/i2c.h>
+#include <linux/types.h>
+
+/**
+ * struct sbtsi_data - driver private data for an AMD SB-TSI device
+ * @client: underlying I2C client
+ * @ext_range_mode: sensor uses extended temperature range
+ * @read_order: if set, decimal part must be read before integer part
+ */
+struct sbtsi_data {
+ struct i2c_client *client;
+ bool ext_range_mode;
+ bool read_order;
+};
+
+/*
+ * Name of the auxiliary device published on the auxiliary bus by the core
+ * driver. The full device name is "amd-sbtsi.temp-sensor.<id>". where
+ * <id> is the auxiliary device instance id.
+ */
+#define AMD_SBTSI_ADEV "amd-sbtsi"
+#define AMD_SBTSI_AUX_HWMON "temp-sensor"
+
+#endif /* _LINUX_MISC_TSI_H_ */
--
2.34.1
^ permalink raw reply related
* [PATCH v3 0/8] misc: amd-sbi: Refactor SBTSI driver with I3C support and ioctl interface
From: Akshay Gupta @ 2026-06-22 13:58 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, NaveenKrishna.Chatradhi,
Anand.Umarji, Akshay.Gupta, Prathima.Lk
This series refactors the AMD SB-TSI (Side-Band Temperature Sensor
Interface) driver by moving the core from the hwmon subsystem into the
drivers/misc/amd-sbi framework, alongside the existing SB-RMI driver.
Registers an auxiliary device keeping hwmon sensors functionality intact.
Background:
The SB-TSI driver currently lives under drivers/hwmon/sbtsi_temp.c and
is limited to exposing temperature readings via the hwmon interface.
As AMD platforms evolve, SB-TSI access is required from multiple
consumers (hwmon, userspace via ioctl, I3C-attached devices), making
the hwmon-only placement insufficient.
This series restructures the driver into a layered design:
- tsi-core.c : core register access and ioctl/miscdevice support
- tsi.c : I2C/I3C probe and glue
- sbtsi_temp.c : hwmon sensor layer built on top of the core using aux device
Changes in this series:
1. Move core SBTSI driver probe from drivers/hwmon into drivers/misc/amd-sbi,
and registering an auxiliary device in core for hwmon subsystem probing
2. Register order follows the device ReadOrder bit so both parts latch atomically;
limit registers (temp / temp1_max / temp1_min) use the same helpers instead of
separate SMBus calls.
3. Move sbtsi register transfer to core abstraction to decouple the hwmon sensor
driver from the underlying bus transport. Preparing for I3C support in a
subsequent patch
4. Refactor I2C probe common functionality to new common helper function to prepare
for I3C support
5. Extend the driver to support SB-TSI over I3C in addition to I2C.
Both buses share the same core read/write path via sbtsi_xfer();
the is_i3c flag selects the underlying transport at probe time.
Backward compatibility with existing I2C deployments is maintained.
6. Add a miscdevice (/dev/sbtsi-<addr>) and an ioctl interface
(SBTSI_IOCTL_REG_XFER_CMD) that allows root userspace to perform
SB-TSI register read/write operations through the APML protocol,
consistent with the existing SBRMI ioctl interface.
7. Add mutex for SBTSI read/write in hwmon to prevent race condition with IOCTL.
8. Document the new SBTSI miscdevice and its ioctl in
Documentation/misc-devices/amd-sbi.rst.
Testing:
Tested on AMD Genoa/Turin/Venice BMC platforms with both I2C and I3C-attached
SB-TSI targets. hwmon sysfs attributes (tempX_input, tempX_max, etc.)
and ioctl register transfers verified against hardware.
Prathima (8):
hwmon/misc: amd-sbi: Move core sbtsi support from hwmon to misc
hwmon: sbtsi_temp: Refactor temperature register access into helpers
hwmon/misc: amd-sbi: Move sbtsi register transfer to core abstraction
misc: amd-sbi: Consolidate Common SBTSI Probe Path for I2C and I3C
misc: amd-sbi: Add support for SB-TSI over I3C
misc: amd-sbi: Add SBTSI ioctl register transfer interface
hwmon: Add mutex protecting for sbtsi read/write through hwmon
docs: misc: amd-sbi: Document SBTSI userspace interface
Documentation/misc-devices/amd-sbi.rst | 68 ++++++++
drivers/hwmon/Kconfig | 2 +-
drivers/hwmon/sbtsi_temp.c | 152 ++++++++---------
drivers/misc/amd-sbi/Kconfig | 13 ++
drivers/misc/amd-sbi/Makefile | 3 +
drivers/misc/amd-sbi/tsi-core.c | 202 ++++++++++++++++++++++
drivers/misc/amd-sbi/tsi-core.h | 26 +++
drivers/misc/amd-sbi/tsi.c | 224 +++++++++++++++++++++++++
include/linux/misc/tsi.h | 71 ++++++++
include/uapi/misc/amd-apml.h | 23 +++
10 files changed, 702 insertions(+), 82 deletions(-)
create mode 100644 drivers/misc/amd-sbi/tsi-core.c
create mode 100644 drivers/misc/amd-sbi/tsi-core.h
create mode 100644 drivers/misc/amd-sbi/tsi.c
create mode 100644 include/linux/misc/tsi.h
--
2.34.1
^ permalink raw reply
* Re: [PATCH 0/2] tracing: Move trace_printk.h out of kernel.h
From: Yury Norov @ 2026-06-22 13:11 UTC (permalink / raw)
To: Steven Rostedt
Cc: Christophe Leroy (CS GROUP), linux-kernel, linux-trace-kernel,
Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Andrew Morton,
Linus Torvalds, Sebastian Andrzej Siewior, John Ogness,
Thomas Gleixner, Peter Zijlstra, Julia Lawall, Yury Norov,
linux-doc, linux-kbuild, linuxppc-dev, dri-devel, linux-stm32,
linux-arm-kernel, linux-rdma, linux-usb, linux-ext4, linux-nfs,
kvm, intel-gfx
In-Reply-To: <20260622090826.20efadb3@fedora>
On Mon, Jun 22, 2026 at 09:08:26AM -0400, Steven Rostedt wrote:
> On Mon, 22 Jun 2026 10:05:13 +0200
> "Christophe Leroy (CS GROUP)" <chleroy@kernel.org> wrote:
>
> > > There's been complaints about trace_printk() being defined in kernel.h as it
> > > can increase the compilation time. As it is only used by some developers for
> > > debugging purposes, it should not be in kernel.h causing lots of wasted CPU
> > > cycles for those that do not ever care about it.
> >
> > Do we have a measurement of the increased compilation time ?
>
> I believe Yury does.
I re-run compilation is a more strict environment, and the difference
is negligible.
^ permalink raw reply
* Re: [PATCH] Documentation: admin-guide: fix bracelets and translation issue
From: Björn Persson @ 2026-06-22 13:10 UTC (permalink / raw)
To: Manuel Ebner
Cc: Jonathan Corbet, Shuah Khan, Mauro Carvalho Chehab, linux-media,
linux-doc, linux-kernel
In-Reply-To: <20260611075513.124994-2-manuelebner@mailbox.org>
[-- Attachment #1: Type: text/plain, Size: 268 bytes --]
Manuel Ebner wrote:
> -- Galaxis plug.in S [neuer Name: Galaxis DVB Card S CI
> +- Galaxis plug.in S [new Name: Galaxis DVB Card S CI]
If it's not German anymore, should it still have the capital N? It
doesn't look like "Name" is a name here.
Björn Persson
[-- Attachment #2: OpenPGP digital signatur --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 0/2] tracing: Move trace_printk.h out of kernel.h
From: Steven Rostedt @ 2026-06-22 13:08 UTC (permalink / raw)
To: Christophe Leroy (CS GROUP)
Cc: linux-kernel, linux-trace-kernel, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers, Andrew Morton, Linus Torvalds,
Sebastian Andrzej Siewior, John Ogness, Thomas Gleixner,
Peter Zijlstra, Julia Lawall, Yury Norov, linux-doc, linux-kbuild,
linuxppc-dev, dri-devel, linux-stm32, linux-arm-kernel,
linux-rdma, linux-usb, linux-ext4, linux-nfs, kvm, intel-gfx
In-Reply-To: <dbb5915e-6587-4de9-87f3-76bea5024da8@kernel.org>
On Mon, 22 Jun 2026 10:05:13 +0200
"Christophe Leroy (CS GROUP)" <chleroy@kernel.org> wrote:
> > There's been complaints about trace_printk() being defined in kernel.h as it
> > can increase the compilation time. As it is only used by some developers for
> > debugging purposes, it should not be in kernel.h causing lots of wasted CPU
> > cycles for those that do not ever care about it.
>
> Do we have a measurement of the increased compilation time ?
I believe Yury does.
-- Steve
^ permalink raw reply
* Re: [PATCH] docs: leds: uleds: Make the documentation match the code.
From: Björn Persson @ 2026-06-22 12:54 UTC (permalink / raw)
To: Lee Jones
Cc: Pavel Machek, Jonathan Corbet, Shuah Khan, linux-leds, linux-doc,
linux-kernel
In-Reply-To: <20260513134505.GZ305027@google.com>
[-- Attachment #1: Type: text/plain, Size: 273 bytes --]
Lee Jones wrote:
> On Sun, 10 May 2026, Björn Persson wrote:
> > If API documentation isn't allowed to name a type, then I withdraw the
> > patch.
>
> It's not that it's "not allowed".
In that case there may be a chance. I'm sending version 2.
Björn Persson
[-- Attachment #2: OpenPGP digital signatur --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 0/2] kasan: hw_tags: Add option to tag only at allocation time
From: Harry Yoo @ 2026-06-22 12:56 UTC (permalink / raw)
To: Catalin Marinas
Cc: Dev Jain, ryabinin.a.a, akpm, corbet, glider, andreyknvl, dvyukov,
vincenzo.frascino, kasan-dev, linux-mm, linux-kernel, skhan,
workflows, linux-doc, linux-arm-kernel, ryan.roberts,
anshuman.khandual, kaleshsingh, 21cnbao, david, will
In-Reply-To: <ajU-b32dmwS7XOg4@arm.com>
[-- Attachment #1.1: Type: text/plain, Size: 1103 bytes --]
On 6/19/26 10:04 PM, Catalin Marinas wrote:
> On Thu, Jun 18, 2026 at 11:05:43PM +0900, Harry Yoo wrote:
>> On 6/18/26 10:35 PM, Harry Yoo wrote:
>>> On 6/12/26 1:44 PM, Dev Jain wrote:
>>>> Introduce a boot option to tag only at allocation time of the objects. This
>>>> reduces KASAN MTE overhead, the tradeoff being reduced ability of
>>>> catching bugs.
>>>
>>> I think most of overhead when enabling MTE comes from loading and
>>> validing tags for every memory access (either in SYNC or ASYNC mode),
>>> rather than from storing tags.
>>
>> Is there any reason not to use STGM instead of STG + DC GVA when
>> setting/clearing tags for large sizes when we know they are properly
>> aligned?
>
> STGM is intended for copying tags when paired with LDGM. Have you seen
> hardware where STGM is faster than STG or DC GVA?
No, I haven't. It was a question I had after learning that there are
multiple ways to store tags ;)
> For properly aligned
> buffers, I'd expect DC GVA to behave at least on par with STGM.
Thanks for answering!
--
Cheers,
Harry / Hyeonggon
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH v2] docs: leds: uleds: Make the documentation match the code.
From: Björn Persson @ 2026-06-22 11:18 UTC (permalink / raw)
To: Lee Jones, Pavel Machek
Cc: Jonathan Corbet, Shuah Khan, linux-leds, linux-doc, linux-kernel
From: Björn Persson <Bjorn@Rombobjörn.se>
The description in uleds.rst omits the field max_brightness and claims
falsely that the maximum brightness is always 255. Leaving max_brightness
uninitialized or omitting it when writing to /dev/uleds won't work. It
must be given a value, and that value becomes the maximum brightness.
The document is also wrong about the type of brightness values. It says
that a single byte shall be read at a time. That's actually not allowed.
Then the word "unsigned" gives the impression that the type is unsigned.
In fact a signed type is used even though the values are never negative.
Change the document to describe the true API.
Signed-off-by: Björn Persson <Bjorn@Rombobjörn.se>
---
Changes in v2:
Replaced "given" with "specified" to prevent misinterpretation of "given name", avoided mentioning a type name outside of C code fragments, and rewrote the commit message to read more like speech, as requested.
Documentation/leds/uleds.rst | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/Documentation/leds/uleds.rst b/Documentation/leds/uleds.rst
index 83221098009c..f985048c641f 100644
--- a/Documentation/leds/uleds.rst
+++ b/Documentation/leds/uleds.rst
@@ -17,16 +17,23 @@ structure to it (found in kernel public header file linux/uleds.h)::
struct uleds_user_dev {
char name[LED_MAX_NAME_SIZE];
+ int max_brightness;
};
-A new LED class device will be created with the name given. The name can be
-any valid sysfs device node name, but consider using the LED class naming
-convention of "devicename:color:function".
+A new LED class device will be created with the specified name and maximum
+brightness. The name can be any valid sysfs device node name, but consider
+using the LED class naming convention of "devicename:color:function".
-The current brightness is found by reading a single byte from the character
-device. Values are unsigned: 0 to 255. Reading will block until the brightness
-changes. The device node can also be polled to notify when the brightness value
-changes.
+Although max_brightness is signed, only positive values are valid: 1 to INT_MAX.
+
+The current brightness shall be read from the character device like so::
+
+ int brightness;
+ result = read(file, &brightness, sizeof(brightness));
+
+The possible values are 0 to max_brightness. Reading will block until the
+brightness changes. The device node can also be polled to notify when the
+brightness value changes.
The LED class device will be removed when the open file handle to /dev/uleds
is closed.
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Conor Dooley @ 2026-06-22 12:50 UTC (permalink / raw)
To: Janani Sunil
Cc: Nuno Sá, Jonathan Cameron, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
In-Reply-To: <013aba24-c30c-44a8-8511-96278edb3f4a@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2977 bytes --]
On Mon, Jun 22, 2026 at 02:39:21PM +0200, Janani Sunil wrote:
>
> On 6/22/26 14:14, Conor Dooley wrote:
> > On Mon, Jun 22, 2026 at 01:54:25PM +0200, Janani Sunil wrote:
> > > > > > > Why do you think the microchip devices won't work? Does the spi core
> > > > > > > reject multiple devices with the same chip select being registered or
> > > > > > > something like that?
> > > > > > Not sure how things work atm. But I'm fairly sure it used to be like
> > > > > > that. SPI would reject devices on the same controller and CS. Now that
> > > > > > we support more than one CS per controller, not sure how things work.
> > > > > We always supported more than one per CS per controller. I guess you mean
> > > > > per device.
> > > > Obviously :)
> > > > > > Janani, maybe you can give it a try?
> > > > > I think we'd need to get it to work with shared gpio proxy which maybe
> > > > > will just get set up under the hood. This used to be opt in, but seems
> > > > > that changed fairly recently so maybe some of us are working with out
> > > > > of date knowledge! I haven't played with it yet, so might not be
> > > > > that simple.
> > > > >
> > > > What I meant for Janani was basically testing two devices on the same CS
> > > > as in my pseudo DT. For the GPIO, you mean having a way to select
> > > > between devices on the same CS?
> > > >
> > > > For these devices the pin id numbers get's setted up as part of the spi message
> > > > so my assumption is that all of them will receive the message but only one acks it.
> > > >
> > > > - Nuno Sá
> > > Hi Everyone,
> > >
> > > I tested the case where there are two devices on the same CS. The SPI core does reject it at spi_dev_check_cs():
> > > https://github.com/torvalds/linux/blob/master/drivers/spi/spi.c#L631
> >
> > Can you try again, but delete that check and allow the code to continue?
> > Worth knowing if the problem is policy (which makes sense for 99.99% of
> > devices that cannot share a chip select) or actually not supported by
> > the spi core code.
>
> Hi Conor,
>
> The CS conflict check is only a part of the problem. Even after removing it, the second device fails at the sysfs layer.
> The device naming in spi_dev_set_name() produces spi{bus}.{cs}. Both devices register as spi0.0 here, making it a duplicate directory.
That doesn't seem insurmountable, since these devices would really need
to be registered with a flag that notes sharing the cs is okay to solve
the problem in spi_dev_check_cs() which could be re-employed in
spi_dev_set_name() to append something.
Something could very well be the top bits of the address field used for
differentiation for spi{bus}.{cs}.{addr7..6}.
Whatever about this being the correct approach for your devices, there's
existing devices for which this would be needed to fully support, and
that doesn't seem like all that much work to do, if that's all that
prevents it.
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 0/2] kasan: hw_tags: Add option to tag only at allocation time
From: Harry Yoo @ 2026-06-22 12:42 UTC (permalink / raw)
To: Catalin Marinas
Cc: Dev Jain, ryabinin.a.a, akpm, corbet, glider, andreyknvl, dvyukov,
vincenzo.frascino, kasan-dev, linux-mm, linux-kernel, skhan,
workflows, linux-doc, linux-arm-kernel, ryan.roberts,
anshuman.khandual, kaleshsingh, 21cnbao, david, will
In-Reply-To: <ajVByfkLbetzA8bB@arm.com>
[-- Attachment #1.1: Type: text/plain, Size: 2477 bytes --]
Hi Catalin,
On 6/19/26 10:19 PM, Catalin Marinas wrote:
> On Thu, Jun 18, 2026 at 10:35:15PM +0900, Harry Yoo wrote:
>> On 6/12/26 1:44 PM, Dev Jain wrote:
>>> Introduce a boot option to tag only at allocation time of the objects. This
>>> reduces KASAN MTE overhead, the tradeoff being reduced ability of
>>> catching bugs.
>>
>> I think most of overhead when enabling MTE comes from loading and
>> validing tags for every memory access (either in SYNC or ASYNC mode),
>> rather than from storing tags.
>
> I guess it depends on the workload. Lots of allocations for short-lived
> buffers (e.g. network traffic) may notice the additional tagging more
> than the actual tag checking.
Agreed. Likely depends on lifetime and size of objects.
> Of course, it would be nice to get some numbers from those who have
> access to MTE capable hardware.
Agreed! (I don't have one, unfortunately. It's pretty new hardware
feature)
>>> Now, when a memory object will be freed, it will retain the random tag it
>>> had at allocation time. This compromises on catching UAF bugs, till the
>>> time the object is not reallocated, at which point it will have a new
>>> random tag.
>>>
>>> Hence, not catching "use-after-free-before-reallocation" and not catching
>>> "double-free" will be the compromise for reduced KASAN overhead.
>>
>> I doubt users who care about security enough to enable HW_TAGS KASAN
>> are willing to compromise on security just to save a few instructions
>> to store tags in the free path.
>>
>> To me, it looks like too much of a compromise on security for little
>> performance gain.
>
> I don't think there's much compromise on security for use-after-free.
I think it depends... OH, WAIT! I see what you mean.
You mean use-after-free before reallocation does not lead to much
compromise on security because objects are initialized after allocation?
You're probably right.
Hmm, but stores to e.g.) free pointer, fields initialized by
constructor or accessed by SLAB_TYPESAFE_BY_RCU semantics after free
will be undiscovered if they happen before reallocation.
Not sure what are security implications of that,
but sounds worth discussing.
> The buffer will be re-tagged later so use-after-realloc should be
> caught, especially if we ensure that a different tag will be used (I
> don't think Dev's patches do this).
Agreed that it'll be nice to ensure that.
--
Cheers,
Harry / Hyeonggon
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Janani Sunil @ 2026-06-22 12:39 UTC (permalink / raw)
To: Conor Dooley
Cc: Nuno Sá, Jonathan Cameron, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
In-Reply-To: <20260622-overbid-yonder-3fdfee9eda7a@spud>
On 6/22/26 14:14, Conor Dooley wrote:
> On Mon, Jun 22, 2026 at 01:54:25PM +0200, Janani Sunil wrote:
>>>>>> Why do you think the microchip devices won't work? Does the spi core
>>>>>> reject multiple devices with the same chip select being registered or
>>>>>> something like that?
>>>>> Not sure how things work atm. But I'm fairly sure it used to be like
>>>>> that. SPI would reject devices on the same controller and CS. Now that
>>>>> we support more than one CS per controller, not sure how things work.
>>>> We always supported more than one per CS per controller. I guess you mean
>>>> per device.
>>> Obviously :)
>>>>> Janani, maybe you can give it a try?
>>>> I think we'd need to get it to work with shared gpio proxy which maybe
>>>> will just get set up under the hood. This used to be opt in, but seems
>>>> that changed fairly recently so maybe some of us are working with out
>>>> of date knowledge! I haven't played with it yet, so might not be
>>>> that simple.
>>>>
>>> What I meant for Janani was basically testing two devices on the same CS
>>> as in my pseudo DT. For the GPIO, you mean having a way to select
>>> between devices on the same CS?
>>>
>>> For these devices the pin id numbers get's setted up as part of the spi message
>>> so my assumption is that all of them will receive the message but only one acks it.
>>>
>>> - Nuno Sá
>> Hi Everyone,
>>
>> I tested the case where there are two devices on the same CS. The SPI core does reject it at spi_dev_check_cs():
>> https://github.com/torvalds/linux/blob/master/drivers/spi/spi.c#L631
>
> Can you try again, but delete that check and allow the code to continue?
> Worth knowing if the problem is policy (which makes sense for 99.99% of
> devices that cannot share a chip select) or actually not supported by
> the spi core code.
Hi Conor,
The CS conflict check is only a part of the problem. Even after removing it, the second device fails at the sysfs layer.
The device naming in spi_dev_set_name() produces spi{bus}.{cs}. Both devices register as spi0.0 here, making it a duplicate directory.
- Janani Sunil
^ permalink raw reply
* [PATCH 2/2] hwmon: (chipcap2) Add support for label
From: Flaviu Nistor @ 2026-06-22 12:22 UTC (permalink / raw)
To: Guenter Roeck, Javier Carrasco, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Corbet, Shuah Khan
Cc: Flaviu Nistor, linux-hwmon, linux-kernel, devicetree, linux-doc
In-Reply-To: <20260622122200.14245-1-flaviu.nistor@gmail.com>
Add support for label sysfs attribute similar to other hwmon devices.
This is particularly useful for systems with multiple sensors on the
same board, where identifying individual sensors is much easier since
labels can be defined via device tree.
Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
---
Documentation/hwmon/chipcap2.rst | 2 ++
drivers/hwmon/chipcap2.c | 25 +++++++++++++++++++++++--
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/Documentation/hwmon/chipcap2.rst b/Documentation/hwmon/chipcap2.rst
index dc165becc64c..c38d87b91b69 100644
--- a/Documentation/hwmon/chipcap2.rst
+++ b/Documentation/hwmon/chipcap2.rst
@@ -70,4 +70,6 @@ humidity1_min_hyst: RW humidity low hystersis
humidity1_max_hyst: RW humidity high hystersis
humidity1_min_alarm: RO humidity low alarm indicator
humidity1_max_alarm: RO humidity high alarm indicator
+humidity1_label: RO descriptive name for the sensor
+temp1_label: RO descriptive name for the sensor
=============================== ======= ========================================
diff --git a/drivers/hwmon/chipcap2.c b/drivers/hwmon/chipcap2.c
index 4aecf463180f..086571d556b7 100644
--- a/drivers/hwmon/chipcap2.c
+++ b/drivers/hwmon/chipcap2.c
@@ -22,6 +22,8 @@
#include <linux/irq.h>
#include <linux/module.h>
#include <linux/regulator/consumer.h>
+#include <linux/mod_devicetable.h>
+#include <linux/property.h>
#define CC2_START_CM 0xA0
#define CC2_START_NOM 0x80
@@ -83,6 +85,7 @@ struct cc2_data {
struct i2c_client *client;
struct regulator *regulator;
const char *name;
+ const char *label;
int irq_ready;
int irq_low;
int irq_high;
@@ -449,6 +452,8 @@ static umode_t cc2_is_visible(const void *data, enum hwmon_sensor_types type,
switch (attr) {
case hwmon_humidity_input:
return 0444;
+ case hwmon_humidity_label:
+ return cc2->label ? 0444 : 0;
case hwmon_humidity_min_alarm:
return cc2->rh_alarm.low_alarm_visible ? 0444 : 0;
case hwmon_humidity_max_alarm:
@@ -466,6 +471,8 @@ static umode_t cc2_is_visible(const void *data, enum hwmon_sensor_types type,
switch (attr) {
case hwmon_temp_input:
return 0444;
+ case hwmon_temp_label:
+ return cc2->label ? 0444 : 0;
default:
return 0;
}
@@ -552,6 +559,16 @@ static int cc2_humidity_max_alarm_status(struct cc2_data *data, long *val)
return 0;
}
+static int cc2_read_string(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, const char **str)
+{
+ struct cc2_data *data = dev_get_drvdata(dev);
+
+ *str = data->label;
+
+ return 0;
+}
+
static int cc2_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
int channel, long *val)
{
@@ -670,8 +687,9 @@ static int cc2_request_alarm_irqs(struct cc2_data *data, struct device *dev)
}
static const struct hwmon_channel_info *cc2_info[] = {
- HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
- HWMON_CHANNEL_INFO(humidity, HWMON_H_INPUT | HWMON_H_MIN | HWMON_H_MAX |
+ HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT | HWMON_T_LABEL),
+ HWMON_CHANNEL_INFO(humidity, HWMON_H_INPUT | HWMON_H_LABEL |
+ HWMON_H_MIN | HWMON_H_MAX |
HWMON_H_MIN_HYST | HWMON_H_MAX_HYST |
HWMON_H_MIN_ALARM | HWMON_H_MAX_ALARM),
NULL
@@ -680,6 +698,7 @@ static const struct hwmon_channel_info *cc2_info[] = {
static const struct hwmon_ops cc2_hwmon_ops = {
.is_visible = cc2_is_visible,
.read = cc2_read,
+ .read_string = cc2_read_string,
.write = cc2_write,
};
@@ -710,6 +729,8 @@ static int cc2_probe(struct i2c_client *client)
return dev_err_probe(dev, PTR_ERR(data->regulator),
"Failed to get regulator\n");
+ device_property_read_string(dev, "label", &data->label);
+
ret = cc2_request_ready_irq(data, dev);
if (ret)
return dev_err_probe(dev, ret, "Failed to request ready irq\n");
--
2.34.1
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: hwmon: chipcap2: Add label property
From: Flaviu Nistor @ 2026-06-22 12:21 UTC (permalink / raw)
To: Guenter Roeck, Javier Carrasco, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jonathan Corbet, Shuah Khan
Cc: Flaviu Nistor, linux-hwmon, linux-kernel, devicetree, linux-doc
Add support for an optional label property similar to other hwmon devices.
This allows, in case of boards with multiple CHIPCAP2 sensors, to assign
distinct names to each instance.
Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
---
.../devicetree/bindings/hwmon/amphenol,chipcap2.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
index 17351fdbefce..f00b5a4b14dd 100644
--- a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
+++ b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
@@ -33,6 +33,10 @@ properties:
reg:
maxItems: 1
+ label:
+ description:
+ A descriptive name for this channel, like "ambient" or "psu".
+
interrupts:
items:
- description: measurement ready indicator
@@ -72,6 +76,7 @@ examples:
<5 IRQ_TYPE_EDGE_RISING>,
<6 IRQ_TYPE_EDGE_RISING>;
interrupt-names = "ready", "low", "high";
+ label = "somelabel";
vdd-supply = <®_vdd>;
};
};
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v4 00/31] Introduce SCMI Telemetry FS support
From: David Hildenbrand (Arm) @ 2026-06-22 12:20 UTC (permalink / raw)
To: Cristian Marussi
Cc: Christian Brauner, linux-kernel, linux-arm-kernel, arm-scmi,
linux-fsdevel, linux-doc, sudeep.holla, james.quinlan, f.fainelli,
vincent.guittot, etienne.carriere, peng.fan, michal.simek, d-gole,
jic23, elif.topuz, lukasz.luba, philip.radford,
souvik.chakravarty, leitao, kas, puranjay, usama.arif,
kernel-team
In-Reply-To: <ajU7UqwPZBlwRGkf@pluto>
>>>
>>> There is more stuff that indeed is configurable per the SCMI spec
>>> but these additional params are hidden into the SCMI Telemetry protocol
>>> layer (the initial patches in this series) and NOT made available to
>>> the driver/users of the protocol (like the SCMI FS driver that sits on
>>> top)
>>
>> Do you assume that there will get significantly more config options added in the
>> future for user space to configure?
>
> No, I dont think so...the only planned extensions were to support more
> performant read access mechanisms, i.e. direct mmap'ability of FW/Kernel
> SCMI Telemetry shared memory areas...BUT that will immediately dump all
> the bulk of the lower layer protocol work into the tools domain...and
Right. I guess you would hide that in library code, and provide a stable API
towards tools such that they won't have to worry about the implementation
details regarding how the values are obtained exactly.
[...]
>>>
>>> ...most of the other entries in the tree are simply RO properties of the DEs
>>> that have been discovered at enumeration time.
>>
>> Is this bulk-reading relevant for performance or just a "nice to have" ?
>>
>
> I suppose depends on your usage pattern: it is definitely relevant
> because the main collection mechanism are shared memory areas (SHMTIs)
> between the platform firmware and the Kernel: such areas being accessed
> from 2 differnt worlds concurrently come with a SCMI-specified
> synchro/consistency mechanism based simply on a pair of sequence numbers
> placed at the start and at the end of the SHMTI, so that the FW increases
> such magic numbers in a well-known way before and after updating the SHMTI
> values, so that the kernel can detect (without any interlocking mechanism)
> if a platform write happened in the middle of its reads...
Okay, so sequence counters to detect concurrent changes and retry.
>
> ...so if you read one single DE 64bit value, under the hood the kernel
> would have had to really perform at leats 3 reads from the SHMTI to check
> the consistecy of that single read...
>
> ... while if you do a bulk_read the overhead due to the consistecy
> checks gets 'spread' across a number of DEs because the kernel will snapshot
> the whole SHMTIs (potentially KBytes) between the 2 consistency reads
That makes sense.
I guess a user space library could implement some kind of caching (bulk-read,
then provide clean access to individual DEs) to hide the details from the tools?
>
> ...the good side effect of all of this is that I can leverage such
> sequence number to optimize reads..i.e. do NOT even try to read anything
> if the new sequnce number is unchanged from the last one I cached on the
> last successfull read of this value...
Right.
>
> So at the end I would say it is NOT simply a nice to have BUT it is
> certainly only the first step towards a more performant alternative access
> (like with mmaps)...it depends on the usage pattern...I am not sure what
> mechanism is used by our tools more...
Okay.
>>
>> Yes. How high-priority is the fs side? Or would a tool using a library to access
>> this information also work in the first step?
>>
>
> I have to sync with tools on this...because they are stiil probably
> using currently the FS, but it was already planned for the future to move to
> a more low level access (ioctl/mmap)...
>
> ...my aim would be, at this point, to favour this transition without sudden
> breaking their current world (and have to expatriate :P)
>
> ..from my personal point of view, I would certainly like to still have the
> FUSE layer for ease of testing and verification on my side...but it is just
> a nice to have...
Okay, what I thought. I assume the most important part is to be able to grab
telemetry data in an efficient way on a system to then share it with some
monitoring agent. Manually digging through the FS to inspect data (debugging,
..) is probably less relevant for the target use case.
>> Okay, so a single writer (admin) changing stuff could get picked up my possibly
>> many concurrent readers?
>
> Mmm...not sure what you mean here...
>
> If you configure your Telemetry as you desire and start collecting data via
> readers, BUT then some other process changes configs under your belt, that is
> allowed as said, and so your analisys could be impacted...(something turned off
> as an example, or update interval changed)...
I was rather wondering about "turning on more" concurrently. But yeah, makes sense.
(it's the same situation other system-wide settings. If one app enables KSM and
the another one disables KSM, inconsistency is unavoidable)
>>
>
> Thanks a lot, David !
Let's hope for some guidance regarding the FS side soon.
But yeah, avoiding the in-kernel FS sounds completely reasonable at this point.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Nuno Sá @ 2026-06-22 12:20 UTC (permalink / raw)
To: Rodrigo Alencar
Cc: Jonathan Cameron, Conor Dooley, Janani Sunil, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
In-Reply-To: <pifhwgj3cp2vc7ia4m6penh52iekzjljrp75y5b7j57vvtooad@32wfqruiqqjl>
On Mon, Jun 22, 2026 at 12:51:20PM +0100, Rodrigo Alencar wrote:
> On 22/06/26 11:29, Nuno Sá wrote:
> > On Mon, Jun 22, 2026 at 10:24:05AM +0100, Rodrigo Alencar wrote:
> > > On 21/06/26 15:33, Jonathan Cameron wrote:
> > > > On Fri, 19 Jun 2026 16:54:11 +0100
> > > > Nuno Sá <noname.nuno@gmail.com> wrote:
> > > >
> > > > > On Fri, Jun 19, 2026 at 03:12:07PM +0100, Conor Dooley wrote:
> > > > > > On Fri, Jun 19, 2026 at 02:01:08PM +0100, Nuno Sá wrote:
> > > > > > > On Fri, Jun 19, 2026 at 12:40:54PM +0100, Conor Dooley wrote:
> > > > > > > > On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:
> > > > > > > > > On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
> > > > > > > > > >
> > > > > > > > > > On 6/14/26 21:44, Jonathan Cameron wrote:
> > > > > > > > > > > On Tue, 9 Jun 2026 16:47:23 +0200
> > > > > > > > > > > Janani Sunil <jan.sun97@gmail.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > > On 5/26/26 15:11, Rodrigo Alencar wrote:
> > > > > > > > > > > > > On 26/05/19 05:42PM, Janani Sunil wrote:
> > > > > > > > > > > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > > > > > > > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > > > > > > > > > > integrated precision reference.
> > > > > > > > > > > > > ...
> > > > > > > > > > > > > Probably others may comment on that, but...
> > > > > > > > > > > > >
> > > > > > > > > > > > > This parent node may support device addressing for multi-device support through
> > > > > > > > > > > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > > > > > > > > > > other resources like the toggle pins or reset and enable.
> > > > > > > > > > > > >
> > > > > > > > > > > > > That way I suppose that an example would look like...
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > +patternProperties:
> > > > > > > > > > > > > > + "^channel@([0-9]|1[0-5])$":
> > > > > > > > > > > > > > + type: object
> > > > > > > > > > > > > > + description: Child nodes for individual channel configuration
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + properties:
> > > > > > > > > > > > > > + reg:
> > > > > > > > > > > > > > + description: Channel number.
> > > > > > > > > > > > > > + minimum: 0
> > > > > > > > > > > > > > + maximum: 15
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + adi,output-range-microvolt:
> > > > > > > > > > > > > > + description: |
> > > > > > > > > > > > > > + Output voltage range for this channel as [min, max] in microvolts.
> > > > > > > > > > > > > > + If not specified, defaults to 0V to 5V range.
> > > > > > > > > > > > > > + oneOf:
> > > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > > + - const: 0
> > > > > > > > > > > > > > + - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > > + - const: -5000000
> > > > > > > > > > > > > > + - const: 5000000
> > > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > > + - const: -10000000
> > > > > > > > > > > > > > + - const: 10000000
> > > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > > + - const: -15000000
> > > > > > > > > > > > > > + - const: 15000000
> > > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > > + - const: -20000000
> > > > > > > > > > > > > > + - const: 20000000
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + required:
> > > > > > > > > > > > > > + - reg
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + additionalProperties: false
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > +required:
> > > > > > > > > > > > > > + - compatible
> > > > > > > > > > > > > > + - reg
> > > > > > > > > > > > > > + - vdd-supply
> > > > > > > > > > > > > > + - avdd-supply
> > > > > > > > > > > > > > + - hvdd-supply
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > +dependencies:
> > > > > > > > > > > > > > + spi-cpha: [ spi-cpol ]
> > > > > > > > > > > > > > + spi-cpol: [ spi-cpha ]
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > +allOf:
> > > > > > > > > > > > > > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > +unevaluatedProperties: false
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > +examples:
> > > > > > > > > > > > > > + - |
> > > > > > > > > > > > > > + #include <dt-bindings/gpio/gpio.h>
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + spi {
> > > > > > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + dac@0 {
> > > > > > > > > > > > > > + compatible = "adi,ad5529r-16";
> > > > > > > > > > > > > > + reg = <0>;
> > > > > > > > > > > > > > + spi-max-frequency = <25000000>;
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > > > + avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > > > + hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > > > + hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + channel@0 {
> > > > > > > > > > > > > > + reg = <0>;
> > > > > > > > > > > > > > + adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > > > + };
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + channel@1 {
> > > > > > > > > > > > > > + reg = <1>;
> > > > > > > > > > > > > > + adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > > > + };
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > + channel@2 {
> > > > > > > > > > > > > > + reg = <2>;
> > > > > > > > > > > > > > + adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > > > > + };
> > > > > > > > > > > > > > + };
> > > > > > > > > > > > > > + };
> > > > > > > > > > > > > ...
> > > > > > > > > > > > >
> > > > > > > > > > > > > spi {
> > > > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > > > >
> > > > > > > > > > > > > multi-dac@0 {
> > > > > > > > > > > > > compatible = "adi,ad5529r-16";
> > > > > > > > > > > > > reg = <0>;
> > > > > > > > > > > > > spi-max-frequency = <25000000>;
> > > > > > > > > > > > >
> > > > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > > > >
> > > > > > > > > > > > > dac@0 {
> > > > > > > > > > > > > reg = <0>;
> > > > > > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > > >
> > > > > > > > > > > > > reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > > >
> > > > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > > > >
> > > > > > > > > > > > > channel@0 {
> > > > > > > > > > > > > reg = <0>;
> > > > > > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > > };
> > > > > > > > > > > > >
> > > > > > > > > > > > > channel@1 {
> > > > > > > > > > > > > reg = <1>;
> > > > > > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > > };
> > > > > > > > > > > > >
> > > > > > > > > > > > > channel@2 {
> > > > > > > > > > > > > reg = <2>;
> > > > > > > > > > > > > adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > > > };
> > > > > > > > > > > > > }
> > > > > > > > > > > > >
> > > > > > > > > > > > > dac@1 {
> > > > > > > > > > > > > reg = <1>;
> > > > > > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > > >
> > > > > > > > > > > > > reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > > >
> > > > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > > > >
> > > > > > > > > > > > > channel@0 {
> > > > > > > > > > > > > reg = <0>;
> > > > > > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > > };
> > > > > > > > > > > > >
> > > > > > > > > > > > > channel@1 {
> > > > > > > > > > > > > reg = <1>;
> > > > > > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > > };
> > > > > > > > > > > > > }
> > > > > > > > > > > > > };
> > > > > > > > > > > > > };
> > > > > > > > > > > > >
> > > > > > > > > > > > > then you might need something like:
> > > > > > > > > > > > >
> > > > > > > > > > > > > patternProperties:
> > > > > > > > > > > > > "^dac@[0-3]$":
> > > > > > > > > > > > >
> > > > > > > > > > > > > and put most of the things under this node pattern.
> > > > > > > > > > > > >
> > > > > > > > > > > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > > > > > > > > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > > > > > > > > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > > > > > > > > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
> > > > > > > > > > > > Hi Rodrigo,
> > > > > > > > > > > >
> > > > > > > > > > > > Thank you for looking at this.
> > > > > > > > > > > >
> > > > > > > > > > > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > > > > > > > > > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > > > > > > > > > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > > > > > > > > > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > > > > > > > > > > speculatively without a validating use case.
> > > > > > > > > > > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > > > > > > > > > > looking at it another way a kind of distributed SPI mux.
> > > > > > > > > > >
> > > > > > > > > > > Challenge of a binding is we need to anticipate the future. So I think we do need something
> > > > > > > > > > > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > > > > > > > > > > That would leave the path open to supporting the addressing at a later date.
> > > > > > > > > > > An alternative might be to look at it like a chained device setup. In those we pretend there
> > > > > > > > > > > is just one device with a lot of channels etc. The snag is that here things are more loosely
> > > > > > > > > > > coupled whereas for those devices it tends to be you have to read / write the same register
> > > > > > > > > > > in all devices in the chain as one big SPI message.
> > > > > > > > > > >
> > > > > > > > > > > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > > > > > > > > > > - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
> > > > > > > > > > > value that matches that or they are ignored. Thus a single bus + 1 chip select can
> > > > > > > > > > > be used to talk to 4 devices. Question is what that looks like in device tree + I guess
> > > > > > > > > > > longer term how to support it cleanly in SPI.
> > > > > > > > >
> > > > > > > > > I'd swear I have seen this before, from some Microchip devices. Let me
> > > > > > > > > see if I can find what I am thinking of...
> > > > > > > >
> > > > > > > >
> > > > > > > > microchip,mcp3911 and microchip,mcp3564 both seem to do this with
> > > > > > > > slightly different properties.
> > > > > > > >
> > > > > > > > microchip,device-addr:
> > > > > > > > description: Device address when multiple MCP3911 chips are present on the same SPI bus.
> > > > > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > enum: [0, 1, 2, 3]
> > > > > > > > default: 0
> > > > > > > >
> > > > > > > > and
> > > > > > > >
> > > > > > > >
> > > > > > > > microchip,hw-device-address:
> > > > > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > > minimum: 0
> > > > > > > > maximum: 3
> > > > > > > > description:
> > > > > > > > The address is set on a per-device basis by fuses in the factory,
> > > > > > > > configured on request. If not requested, the fuses are set for 0x1.
> > > > > > > > The device address is part of the device markings to avoid
> > > > > > > > potential confusion. This address is coded on two bits, so four possible
> > > > > > > > addresses are available when multiple devices are present on the same
> > > > > > > > SPI bus with only one Chip Select line for all devices.
> > > > > > > > Each device communication starts by a CS falling edge, followed by the
> > > > > > > > clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> > > > > > > > which is first one on the wire).
> > > > > > > >
> > > > > > > > This sounds exactly like the sort of feature that you're dealing with
> > > > > > > > here?
> > > > > > > >
> > > > > > >
> > > > > > > The core idea yes but for this chip, things are a bit more annoying (but
> > > > > > > Janani can correct me if I'm wrong). Here, each device can, in theory,
> > > > > > > have it's own supplies, pins and at the very least, channels with maybe
> > > > > > > different scales. That is why Janani is proposing dac nodes. Given I
> > > > > > > honestly don't like much of that "adi,ad5529r-bus" compatible I wondered
> > > > > > > about solving this at the spi level.
> > > > > > >
> > > > > > > Ah and to make it more annoying, we can also mix 12 and 16 bits variants
> > > > > > > together in the same bus.
> > > > > >
> > > > > > I'm definitely missing something, because that property for the
> > > > > > microchip devices is not impacted what else is on the bus. AFAICT, you
> > > > > > could have an mcp3911 and an mcp3564 on the same bus even though both
> > > > > > are completely different devices with different drivers. They have
> > > > > > individual device nodes and their own supplies etc etc. These aren't
> > > > > > per-channel properties on an adc or dac, they're per child device on a
> > > > > > spi bus.
> > > > >
> > > > > Maybe I'm the one missing something :). IIRC, spi would not allow two
> > > > > devices on the same CS right? Because for this chip we would need
> > > > > something like:
> > > > >
> > > > > spi {
> > > > > dac@0 {
> > > > > reg = <0>;
> > > > > adi,pin-id = <0>;
> > > > > };
> > > > >
> > > > > dac@1 {
> > > > > reg = <0>; // which seems already problematic?
> > > > > adi,pin-id <1>;
> > > > > };
> > > > >
> > > > > ...
> > > > >
> > > > > //up to 4
> > > > > };
> > > > Yeah. It's not clear to me how that works for the microchip devices
> > > > (I suspect it doesn't!)
> > > >
> > > > Just thinking as I type, but could we do something a bit nasty with
> > > > a gpio mux that doesn't actually switch but represents the GPIO being
> > > > shared? Given this is all tied to the spi bus that should all happen
> > > > under serializing locks.
> > > >
> > > > Agreed though that this would be nicer as an SPI thing that let
> > > > us specify that a single CS is share by multiple devices and their
> > > > is some other signal acting to select which one we are talking to.
> > > >
> > >
> > > If the device-addressing on the same chip-select is to be handled
> > > by the spi framework, wouldn't we lose device-specific features?
> > >
> > > I understand that this multi-device feature is there mostly to extend the
> > > channel count from 16 to 32, 48 or 64. I suppose the command:
> > >
> > > "MULTI DEVICE SW LDAC MODE"
> > >
> > > exists so that software can update channel values accross multiple devices.
> >
> > Right! You do have a point! I agree the main driver for a feature like
> > this is likely to extend the channel count and effectively "aggregate"
> > devices.
> >
> > But I would say that even with the spi solution the MULTI DEVICE stuff
> > should be doable (as we still need a sort of adi,pin-id property).
>
> I don't think we can have something like an IIO buffer shared by multiple
> devices. Synchronizing separate devices would be doable with proper hardware
> support for this (probably involving an FGPA).
True!
>
> > But yes, I do feel that the whole feature is for aggregation so seeing
> > one device with 32 channels is the expectation here? Rather than seeing
> > two devices with 16 channels.
>
> Yes, I think aggregation is the whole point there... so that the IIO driver
> is multi-device-aware.
Which makes me feel that different pins per device might be possible
from an HW point of view but does not make much sense. For example, for
the buffer example I would expect LDAC to be shared between all the
devices.
- Nuno Sá
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Conor Dooley @ 2026-06-22 12:14 UTC (permalink / raw)
To: Janani Sunil
Cc: Nuno Sá, Jonathan Cameron, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
In-Reply-To: <caa54d52-72db-4c58-ae3f-1d1343bd7845@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]
On Mon, Jun 22, 2026 at 01:54:25PM +0200, Janani Sunil wrote:
> > > > > Why do you think the microchip devices won't work? Does the spi core
> > > > > reject multiple devices with the same chip select being registered or
> > > > > something like that?
> > > > Not sure how things work atm. But I'm fairly sure it used to be like
> > > > that. SPI would reject devices on the same controller and CS. Now that
> > > > we support more than one CS per controller, not sure how things work.
> > > We always supported more than one per CS per controller. I guess you mean
> > > per device.
> > Obviously :)
> > > > Janani, maybe you can give it a try?
> > > I think we'd need to get it to work with shared gpio proxy which maybe
> > > will just get set up under the hood. This used to be opt in, but seems
> > > that changed fairly recently so maybe some of us are working with out
> > > of date knowledge! I haven't played with it yet, so might not be
> > > that simple.
> > >
> > What I meant for Janani was basically testing two devices on the same CS
> > as in my pseudo DT. For the GPIO, you mean having a way to select
> > between devices on the same CS?
> >
> > For these devices the pin id numbers get's setted up as part of the spi message
> > so my assumption is that all of them will receive the message but only one acks it.
> >
> > - Nuno Sá
>
> Hi Everyone,
>
> I tested the case where there are two devices on the same CS. The SPI core does reject it at spi_dev_check_cs():
> https://github.com/torvalds/linux/blob/master/drivers/spi/spi.c#L631
Can you try again, but delete that check and allow the code to continue?
Worth knowing if the problem is policy (which makes sense for 99.99% of
devices that cannot share a chip select) or actually not supported by
the spi core code.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Janani Sunil @ 2026-06-22 11:54 UTC (permalink / raw)
To: Nuno Sá, Jonathan Cameron
Cc: Conor Dooley, Rodrigo Alencar, Janani Sunil, Lars-Peter Clausen,
Michael Hennerich, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Jonathan Corbet, Shuah Khan, linux-iio, devicetree, linux-kernel,
linux-doc, Mark Brown
In-Reply-To: <ajkILRPq_g24g4dH@nsa>
On 6/22/26 12:17, Nuno Sá wrote:
> On Mon, Jun 22, 2026 at 10:27:22AM +0100, Jonathan Cameron wrote:
>> On Mon, 22 Jun 2026 10:07:01 +0100
>> Nuno Sá <noname.nuno@gmail.com> wrote:
>>
>>> On Sun, Jun 21, 2026 at 07:35:42PM +0100, Conor Dooley wrote:
>>>> On Sun, Jun 21, 2026 at 03:33:40PM +0100, Jonathan Cameron wrote:
>>>>> On Fri, 19 Jun 2026 16:54:11 +0100
>>>>> Nuno Sá <noname.nuno@gmail.com> wrote:
>>>>>
>>>>>> On Fri, Jun 19, 2026 at 03:12:07PM +0100, Conor Dooley wrote:
>>>>>>> On Fri, Jun 19, 2026 at 02:01:08PM +0100, Nuno Sá wrote:
>>>>>>>> On Fri, Jun 19, 2026 at 12:40:54PM +0100, Conor Dooley wrote:
>>>>>>>>> On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:
>>>>>>>>>> On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
>>>>>>>>>>> On 6/14/26 21:44, Jonathan Cameron wrote:
>>>>>>>>>>>> On Tue, 9 Jun 2026 16:47:23 +0200
>>>>>>>>>>>> Janani Sunil <jan.sun97@gmail.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> On 5/26/26 15:11, Rodrigo Alencar wrote:
>>>>>>>>>>>>>> On 26/05/19 05:42PM, Janani Sunil wrote:
>>>>>>>>>>>>>>> Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
>>>>>>>>>>>>>>> buffered voltage output digital-to-analog converter (DAC) with an
>>>>>>>>>>>>>>> integrated precision reference.
>>>>>>>>>>>>>> ...
>>>>>>>>>>>>>> Probably others may comment on that, but...
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This parent node may support device addressing for multi-device support through
>>>>>>>>>>>>>> those ID pins. I suppose that each device may have its own power supplies or
>>>>>>>>>>>>>> other resources like the toggle pins or reset and enable.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> That way I suppose that an example would look like...
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> +patternProperties:
>>>>>>>>>>>>>>> + "^channel@([0-9]|1[0-5])$":
>>>>>>>>>>>>>>> + type: object
>>>>>>>>>>>>>>> + description: Child nodes for individual channel configuration
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + properties:
>>>>>>>>>>>>>>> + reg:
>>>>>>>>>>>>>>> + description: Channel number.
>>>>>>>>>>>>>>> + minimum: 0
>>>>>>>>>>>>>>> + maximum: 15
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + adi,output-range-microvolt:
>>>>>>>>>>>>>>> + description: |
>>>>>>>>>>>>>>> + Output voltage range for this channel as [min, max] in microvolts.
>>>>>>>>>>>>>>> + If not specified, defaults to 0V to 5V range.
>>>>>>>>>>>>>>> + oneOf:
>>>>>>>>>>>>>>> + - items:
>>>>>>>>>>>>>>> + - const: 0
>>>>>>>>>>>>>>> + - enum: [5000000, 10000000, 20000000, 40000000]
>>>>>>>>>>>>>>> + - items:
>>>>>>>>>>>>>>> + - const: -5000000
>>>>>>>>>>>>>>> + - const: 5000000
>>>>>>>>>>>>>>> + - items:
>>>>>>>>>>>>>>> + - const: -10000000
>>>>>>>>>>>>>>> + - const: 10000000
>>>>>>>>>>>>>>> + - items:
>>>>>>>>>>>>>>> + - const: -15000000
>>>>>>>>>>>>>>> + - const: 15000000
>>>>>>>>>>>>>>> + - items:
>>>>>>>>>>>>>>> + - const: -20000000
>>>>>>>>>>>>>>> + - const: 20000000
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + required:
>>>>>>>>>>>>>>> + - reg
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + additionalProperties: false
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> +required:
>>>>>>>>>>>>>>> + - compatible
>>>>>>>>>>>>>>> + - reg
>>>>>>>>>>>>>>> + - vdd-supply
>>>>>>>>>>>>>>> + - avdd-supply
>>>>>>>>>>>>>>> + - hvdd-supply
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> +dependencies:
>>>>>>>>>>>>>>> + spi-cpha: [ spi-cpol ]
>>>>>>>>>>>>>>> + spi-cpol: [ spi-cpha ]
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> +allOf:
>>>>>>>>>>>>>>> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> +unevaluatedProperties: false
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> +examples:
>>>>>>>>>>>>>>> + - |
>>>>>>>>>>>>>>> + #include <dt-bindings/gpio/gpio.h>
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + spi {
>>>>>>>>>>>>>>> + #address-cells = <1>;
>>>>>>>>>>>>>>> + #size-cells = <0>;
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + dac@0 {
>>>>>>>>>>>>>>> + compatible = "adi,ad5529r-16";
>>>>>>>>>>>>>>> + reg = <0>;
>>>>>>>>>>>>>>> + spi-max-frequency = <25000000>;
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + vdd-supply = <&vdd_regulator>;
>>>>>>>>>>>>>>> + avdd-supply = <&avdd_regulator>;
>>>>>>>>>>>>>>> + hvdd-supply = <&hvdd_regulator>;
>>>>>>>>>>>>>>> + hvss-supply = <&hvss_regulator>;
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + #address-cells = <1>;
>>>>>>>>>>>>>>> + #size-cells = <0>;
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + channel@0 {
>>>>>>>>>>>>>>> + reg = <0>;
>>>>>>>>>>>>>>> + adi,output-range-microvolt = <0 5000000>;
>>>>>>>>>>>>>>> + };
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + channel@1 {
>>>>>>>>>>>>>>> + reg = <1>;
>>>>>>>>>>>>>>> + adi,output-range-microvolt = <(-10000000) 10000000>;
>>>>>>>>>>>>>>> + };
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + channel@2 {
>>>>>>>>>>>>>>> + reg = <2>;
>>>>>>>>>>>>>>> + adi,output-range-microvolt = <0 40000000>;
>>>>>>>>>>>>>>> + };
>>>>>>>>>>>>>>> + };
>>>>>>>>>>>>>>> + };
>>>>>>>>>>>>>> ...
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> spi {
>>>>>>>>>>>>>> #address-cells = <1>;
>>>>>>>>>>>>>> #size-cells = <0>;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> multi-dac@0 {
>>>>>>>>>>>>>> compatible = "adi,ad5529r-16";
>>>>>>>>>>>>>> reg = <0>;
>>>>>>>>>>>>>> spi-max-frequency = <25000000>;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #address-cells = <1>;
>>>>>>>>>>>>>> #size-cells = <0>;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> dac@0 {
>>>>>>>>>>>>>> reg = <0>;
>>>>>>>>>>>>>> vdd-supply = <&vdd_regulator>;
>>>>>>>>>>>>>> avdd-supply = <&avdd_regulator>;
>>>>>>>>>>>>>> hvdd-supply = <&hvdd_regulator>;
>>>>>>>>>>>>>> hvss-supply = <&hvss_regulator>;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #address-cells = <1>;
>>>>>>>>>>>>>> #size-cells = <0>;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> channel@0 {
>>>>>>>>>>>>>> reg = <0>;
>>>>>>>>>>>>>> adi,output-range-microvolt = <0 5000000>;
>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> channel@1 {
>>>>>>>>>>>>>> reg = <1>;
>>>>>>>>>>>>>> adi,output-range-microvolt = <(-10000000) 10000000>;
>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> channel@2 {
>>>>>>>>>>>>>> reg = <2>;
>>>>>>>>>>>>>> adi,output-range-microvolt = <0 40000000>;
>>>>>>>>>>>>>> };
>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> dac@1 {
>>>>>>>>>>>>>> reg = <1>;
>>>>>>>>>>>>>> vdd-supply = <&vdd_regulator>;
>>>>>>>>>>>>>> avdd-supply = <&avdd_regulator>;
>>>>>>>>>>>>>> hvdd-supply = <&hvdd_regulator>;
>>>>>>>>>>>>>> hvss-supply = <&hvss_regulator>;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> #address-cells = <1>;
>>>>>>>>>>>>>> #size-cells = <0>;
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> channel@0 {
>>>>>>>>>>>>>> reg = <0>;
>>>>>>>>>>>>>> adi,output-range-microvolt = <0 5000000>;
>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> channel@1 {
>>>>>>>>>>>>>> reg = <1>;
>>>>>>>>>>>>>> adi,output-range-microvolt = <(-10000000) 10000000>;
>>>>>>>>>>>>>> };
>>>>>>>>>>>>>> }
>>>>>>>>>>>>>> };
>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> then you might need something like:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> patternProperties:
>>>>>>>>>>>>>> "^dac@[0-3]$":
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> and put most of the things under this node pattern.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> So the main driver that you're putting together might need to handle up to four instances.
>>>>>>>>>>>>>> Even if your current driver cannot handle this, the dt-bindings might need cover that.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Need to double check if each dac node needs a separate compatible, so you would maybe populate
>>>>>>>>>>>>>> a platform data to be shared with the child nodes, which would be a separate driver.
>>>>>>>>>>>>>> (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
>>>>>>>>>>>>> Hi Rodrigo,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thank you for looking at this.
>>>>>>>>>>>>>
>>>>>>>>>>>>> For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
>>>>>>>>>>>>> hardware/use case we have only needs one device node and the driver is written around that model as well.
>>>>>>>>>>>>> While the device addressing pins could allow multi-device topology, we do not have an actual platform using
>>>>>>>>>>>>> that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
>>>>>>>>>>>>> speculatively without a validating use case.
>>>>>>>>>>>> Interesting feature - kind of similar to address control on a typical i2c bus device, or
>>>>>>>>>>>> looking at it another way a kind of distributed SPI mux.
>>>>>>>>>>>>
>>>>>>>>>>>> Challenge of a binding is we need to anticipate the future. So I think we do need something
>>>>>>>>>>>> like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
>>>>>>>>>>>> That would leave the path open to supporting the addressing at a later date.
>>>>>>>>>>>> An alternative might be to look at it like a chained device setup. In those we pretend there
>>>>>>>>>>>> is just one device with a lot of channels etc. The snag is that here things are more loosely
>>>>>>>>>>>> coupled whereas for those devices it tends to be you have to read / write the same register
>>>>>>>>>>>> in all devices in the chain as one big SPI message.
>>>>>>>>>>>>
>>>>>>>>>>>> +CC Mark Brown as he may know of some precedence for this feature. For his reference..
>>>>>>>>>>>> - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
>>>>>>>>>>>> value that matches that or they are ignored. Thus a single bus + 1 chip select can
>>>>>>>>>>>> be used to talk to 4 devices. Question is what that looks like in device tree + I guess
>>>>>>>>>>>> longer term how to support it cleanly in SPI.
>>>>>>>>>> I'd swear I have seen this before, from some Microchip devices. Let me
>>>>>>>>>> see if I can find what I am thinking of...
>>>>>>>>>
>>>>>>>>> microchip,mcp3911 and microchip,mcp3564 both seem to do this with
>>>>>>>>> slightly different properties.
>>>>>>>>>
>>>>>>>>> microchip,device-addr:
>>>>>>>>> description: Device address when multiple MCP3911 chips are present on the same SPI bus.
>>>>>>>>> $ref: /schemas/types.yaml#/definitions/uint32
>>>>>>>>> enum: [0, 1, 2, 3]
>>>>>>>>> default: 0
>>>>>>>>>
>>>>>>>>> and
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> microchip,hw-device-address:
>>>>>>>>> $ref: /schemas/types.yaml#/definitions/uint32
>>>>>>>>> minimum: 0
>>>>>>>>> maximum: 3
>>>>>>>>> description:
>>>>>>>>> The address is set on a per-device basis by fuses in the factory,
>>>>>>>>> configured on request. If not requested, the fuses are set for 0x1.
>>>>>>>>> The device address is part of the device markings to avoid
>>>>>>>>> potential confusion. This address is coded on two bits, so four possible
>>>>>>>>> addresses are available when multiple devices are present on the same
>>>>>>>>> SPI bus with only one Chip Select line for all devices.
>>>>>>>>> Each device communication starts by a CS falling edge, followed by the
>>>>>>>>> clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
>>>>>>>>> which is first one on the wire).
>>>>>>>>>
>>>>>>>>> This sounds exactly like the sort of feature that you're dealing with
>>>>>>>>> here?
>>>>>>>>>
>>>>>>>> The core idea yes but for this chip, things are a bit more annoying (but
>>>>>>>> Janani can correct me if I'm wrong). Here, each device can, in theory,
>>>>>>>> have it's own supplies, pins and at the very least, channels with maybe
>>>>>>>> different scales. That is why Janani is proposing dac nodes. Given I
>>>>>>>> honestly don't like much of that "adi,ad5529r-bus" compatible I wondered
>>>>>>>> about solving this at the spi level.
>>>>>>>>
>>>>>>>> Ah and to make it more annoying, we can also mix 12 and 16 bits variants
>>>>>>>> together in the same bus.
>>>>>>> I'm definitely missing something, because that property for the
>>>>>>> microchip devices is not impacted what else is on the bus. AFAICT, you
>>>>>>> could have an mcp3911 and an mcp3564 on the same bus even though both
>>>>>>> are completely different devices with different drivers. They have
>>>>>>> individual device nodes and their own supplies etc etc. These aren't
>>>>>>> per-channel properties on an adc or dac, they're per child device on a
>>>>>>> spi bus.
>>>>>> Maybe I'm the one missing something :). IIRC, spi would not allow two
>>>>>> devices on the same CS right? Because for this chip we would need
>>>>>> something like:
>>>>>>
>>>>>> spi {
>>>>>> dac@0 {
>>>>>> reg = <0>;
>>>>>> adi,pin-id = <0>;
>>>>>> };
>>>>>>
>>>>>> dac@1 {
>>>>>> reg = <0>; // which seems already problematic?
>>>>>> adi,pin-id <1>;
>>>>>> };
>>>>>>
>>>>>> ...
>>>>>>
>>>>>> //up to 4
>>>>>> };
>>>>> Yeah. It's not clear to me how that works for the microchip devices
>>>>> (I suspect it doesn't!)
>>>>>
>>>>> Just thinking as I type, but could we do something a bit nasty with
>>>>> a gpio mux that doesn't actually switch but represents the GPIO being
>>>>> shared? Given this is all tied to the spi bus that should all happen
>>>>> under serializing locks.
>>>>>
>>>>> Agreed though that this would be nicer as an SPI thing that let
>>>>> us specify that a single CS is share by multiple devices and their
>>>>> is some other signal acting to select which one we are talking to.
>>>> Whether it works or not, I think it is the more correct approach. Messing
>>>> with gpio muxes seems completely wrong, given the chip select may not be
>>>> a gpio at all.
>>>>
>>>> Why do you think the microchip devices won't work? Does the spi core
>>>> reject multiple devices with the same chip select being registered or
>>>> something like that?
>>> Not sure how things work atm. But I'm fairly sure it used to be like
>>> that. SPI would reject devices on the same controller and CS. Now that
>>> we support more than one CS per controller, not sure how things work.
>> We always supported more than one per CS per controller. I guess you mean
>> per device.
> Obviously :)
>>> Janani, maybe you can give it a try?
>> I think we'd need to get it to work with shared gpio proxy which maybe
>> will just get set up under the hood. This used to be opt in, but seems
>> that changed fairly recently so maybe some of us are working with out
>> of date knowledge! I haven't played with it yet, so might not be
>> that simple.
>>
> What I meant for Janani was basically testing two devices on the same CS
> as in my pseudo DT. For the GPIO, you mean having a way to select
> between devices on the same CS?
>
> For these devices the pin id numbers get's setted up as part of the spi message
> so my assumption is that all of them will receive the message but only one acks it.
>
> - Nuno Sá
Hi Everyone,
I tested the case where there are two devices on the same CS. The SPI core does reject it at spi_dev_check_cs():
https://github.com/torvalds/linux/blob/master/drivers/spi/spi.c#L631
-Janani Sunil
>
>> Jonathan
>>
>>> - Nuno Sá
>>>
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Rodrigo Alencar @ 2026-06-22 11:51 UTC (permalink / raw)
To: Nuno Sá, Rodrigo Alencar
Cc: Jonathan Cameron, Conor Dooley, Janani Sunil, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
In-Reply-To: <ajkMBh-R_7pYaoAn@nsa>
On 22/06/26 11:29, Nuno Sá wrote:
> On Mon, Jun 22, 2026 at 10:24:05AM +0100, Rodrigo Alencar wrote:
> > On 21/06/26 15:33, Jonathan Cameron wrote:
> > > On Fri, 19 Jun 2026 16:54:11 +0100
> > > Nuno Sá <noname.nuno@gmail.com> wrote:
> > >
> > > > On Fri, Jun 19, 2026 at 03:12:07PM +0100, Conor Dooley wrote:
> > > > > On Fri, Jun 19, 2026 at 02:01:08PM +0100, Nuno Sá wrote:
> > > > > > On Fri, Jun 19, 2026 at 12:40:54PM +0100, Conor Dooley wrote:
> > > > > > > On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:
> > > > > > > > On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
> > > > > > > > >
> > > > > > > > > On 6/14/26 21:44, Jonathan Cameron wrote:
> > > > > > > > > > On Tue, 9 Jun 2026 16:47:23 +0200
> > > > > > > > > > Janani Sunil <jan.sun97@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > > On 5/26/26 15:11, Rodrigo Alencar wrote:
> > > > > > > > > > > > On 26/05/19 05:42PM, Janani Sunil wrote:
> > > > > > > > > > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > > > > > > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > > > > > > > > > integrated precision reference.
> > > > > > > > > > > > ...
> > > > > > > > > > > > Probably others may comment on that, but...
> > > > > > > > > > > >
> > > > > > > > > > > > This parent node may support device addressing for multi-device support through
> > > > > > > > > > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > > > > > > > > > other resources like the toggle pins or reset and enable.
> > > > > > > > > > > >
> > > > > > > > > > > > That way I suppose that an example would look like...
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +patternProperties:
> > > > > > > > > > > > > + "^channel@([0-9]|1[0-5])$":
> > > > > > > > > > > > > + type: object
> > > > > > > > > > > > > + description: Child nodes for individual channel configuration
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + properties:
> > > > > > > > > > > > > + reg:
> > > > > > > > > > > > > + description: Channel number.
> > > > > > > > > > > > > + minimum: 0
> > > > > > > > > > > > > + maximum: 15
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + adi,output-range-microvolt:
> > > > > > > > > > > > > + description: |
> > > > > > > > > > > > > + Output voltage range for this channel as [min, max] in microvolts.
> > > > > > > > > > > > > + If not specified, defaults to 0V to 5V range.
> > > > > > > > > > > > > + oneOf:
> > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > + - const: 0
> > > > > > > > > > > > > + - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > + - const: -5000000
> > > > > > > > > > > > > + - const: 5000000
> > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > + - const: -10000000
> > > > > > > > > > > > > + - const: 10000000
> > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > + - const: -15000000
> > > > > > > > > > > > > + - const: 15000000
> > > > > > > > > > > > > + - items:
> > > > > > > > > > > > > + - const: -20000000
> > > > > > > > > > > > > + - const: 20000000
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + required:
> > > > > > > > > > > > > + - reg
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + additionalProperties: false
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +required:
> > > > > > > > > > > > > + - compatible
> > > > > > > > > > > > > + - reg
> > > > > > > > > > > > > + - vdd-supply
> > > > > > > > > > > > > + - avdd-supply
> > > > > > > > > > > > > + - hvdd-supply
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +dependencies:
> > > > > > > > > > > > > + spi-cpha: [ spi-cpol ]
> > > > > > > > > > > > > + spi-cpol: [ spi-cpha ]
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +allOf:
> > > > > > > > > > > > > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +unevaluatedProperties: false
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +examples:
> > > > > > > > > > > > > + - |
> > > > > > > > > > > > > + #include <dt-bindings/gpio/gpio.h>
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + spi {
> > > > > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + dac@0 {
> > > > > > > > > > > > > + compatible = "adi,ad5529r-16";
> > > > > > > > > > > > > + reg = <0>;
> > > > > > > > > > > > > + spi-max-frequency = <25000000>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > > + avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > > + hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > > + hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + channel@0 {
> > > > > > > > > > > > > + reg = <0>;
> > > > > > > > > > > > > + adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > > + };
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + channel@1 {
> > > > > > > > > > > > > + reg = <1>;
> > > > > > > > > > > > > + adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > > + };
> > > > > > > > > > > > > +
> > > > > > > > > > > > > + channel@2 {
> > > > > > > > > > > > > + reg = <2>;
> > > > > > > > > > > > > + adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > > > + };
> > > > > > > > > > > > > + };
> > > > > > > > > > > > > + };
> > > > > > > > > > > > ...
> > > > > > > > > > > >
> > > > > > > > > > > > spi {
> > > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > > >
> > > > > > > > > > > > multi-dac@0 {
> > > > > > > > > > > > compatible = "adi,ad5529r-16";
> > > > > > > > > > > > reg = <0>;
> > > > > > > > > > > > spi-max-frequency = <25000000>;
> > > > > > > > > > > >
> > > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > > >
> > > > > > > > > > > > dac@0 {
> > > > > > > > > > > > reg = <0>;
> > > > > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > >
> > > > > > > > > > > > reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > >
> > > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > > >
> > > > > > > > > > > > channel@0 {
> > > > > > > > > > > > reg = <0>;
> > > > > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > };
> > > > > > > > > > > >
> > > > > > > > > > > > channel@1 {
> > > > > > > > > > > > reg = <1>;
> > > > > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > };
> > > > > > > > > > > >
> > > > > > > > > > > > channel@2 {
> > > > > > > > > > > > reg = <2>;
> > > > > > > > > > > > adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > > };
> > > > > > > > > > > > }
> > > > > > > > > > > >
> > > > > > > > > > > > dac@1 {
> > > > > > > > > > > > reg = <1>;
> > > > > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > >
> > > > > > > > > > > > reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > >
> > > > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > > > #size-cells = <0>;
> > > > > > > > > > > >
> > > > > > > > > > > > channel@0 {
> > > > > > > > > > > > reg = <0>;
> > > > > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > > };
> > > > > > > > > > > >
> > > > > > > > > > > > channel@1 {
> > > > > > > > > > > > reg = <1>;
> > > > > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > > };
> > > > > > > > > > > > }
> > > > > > > > > > > > };
> > > > > > > > > > > > };
> > > > > > > > > > > >
> > > > > > > > > > > > then you might need something like:
> > > > > > > > > > > >
> > > > > > > > > > > > patternProperties:
> > > > > > > > > > > > "^dac@[0-3]$":
> > > > > > > > > > > >
> > > > > > > > > > > > and put most of the things under this node pattern.
> > > > > > > > > > > >
> > > > > > > > > > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > > > > > > > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > > > > > > > > > >
> > > > > > > > > > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > > > > > > > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > > > > > > > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
> > > > > > > > > > > Hi Rodrigo,
> > > > > > > > > > >
> > > > > > > > > > > Thank you for looking at this.
> > > > > > > > > > >
> > > > > > > > > > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > > > > > > > > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > > > > > > > > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > > > > > > > > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > > > > > > > > > speculatively without a validating use case.
> > > > > > > > > > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > > > > > > > > > looking at it another way a kind of distributed SPI mux.
> > > > > > > > > >
> > > > > > > > > > Challenge of a binding is we need to anticipate the future. So I think we do need something
> > > > > > > > > > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > > > > > > > > > That would leave the path open to supporting the addressing at a later date.
> > > > > > > > > > An alternative might be to look at it like a chained device setup. In those we pretend there
> > > > > > > > > > is just one device with a lot of channels etc. The snag is that here things are more loosely
> > > > > > > > > > coupled whereas for those devices it tends to be you have to read / write the same register
> > > > > > > > > > in all devices in the chain as one big SPI message.
> > > > > > > > > >
> > > > > > > > > > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > > > > > > > > > - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
> > > > > > > > > > value that matches that or they are ignored. Thus a single bus + 1 chip select can
> > > > > > > > > > be used to talk to 4 devices. Question is what that looks like in device tree + I guess
> > > > > > > > > > longer term how to support it cleanly in SPI.
> > > > > > > >
> > > > > > > > I'd swear I have seen this before, from some Microchip devices. Let me
> > > > > > > > see if I can find what I am thinking of...
> > > > > > >
> > > > > > >
> > > > > > > microchip,mcp3911 and microchip,mcp3564 both seem to do this with
> > > > > > > slightly different properties.
> > > > > > >
> > > > > > > microchip,device-addr:
> > > > > > > description: Device address when multiple MCP3911 chips are present on the same SPI bus.
> > > > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > enum: [0, 1, 2, 3]
> > > > > > > default: 0
> > > > > > >
> > > > > > > and
> > > > > > >
> > > > > > >
> > > > > > > microchip,hw-device-address:
> > > > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > > > > minimum: 0
> > > > > > > maximum: 3
> > > > > > > description:
> > > > > > > The address is set on a per-device basis by fuses in the factory,
> > > > > > > configured on request. If not requested, the fuses are set for 0x1.
> > > > > > > The device address is part of the device markings to avoid
> > > > > > > potential confusion. This address is coded on two bits, so four possible
> > > > > > > addresses are available when multiple devices are present on the same
> > > > > > > SPI bus with only one Chip Select line for all devices.
> > > > > > > Each device communication starts by a CS falling edge, followed by the
> > > > > > > clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> > > > > > > which is first one on the wire).
> > > > > > >
> > > > > > > This sounds exactly like the sort of feature that you're dealing with
> > > > > > > here?
> > > > > > >
> > > > > >
> > > > > > The core idea yes but for this chip, things are a bit more annoying (but
> > > > > > Janani can correct me if I'm wrong). Here, each device can, in theory,
> > > > > > have it's own supplies, pins and at the very least, channels with maybe
> > > > > > different scales. That is why Janani is proposing dac nodes. Given I
> > > > > > honestly don't like much of that "adi,ad5529r-bus" compatible I wondered
> > > > > > about solving this at the spi level.
> > > > > >
> > > > > > Ah and to make it more annoying, we can also mix 12 and 16 bits variants
> > > > > > together in the same bus.
> > > > >
> > > > > I'm definitely missing something, because that property for the
> > > > > microchip devices is not impacted what else is on the bus. AFAICT, you
> > > > > could have an mcp3911 and an mcp3564 on the same bus even though both
> > > > > are completely different devices with different drivers. They have
> > > > > individual device nodes and their own supplies etc etc. These aren't
> > > > > per-channel properties on an adc or dac, they're per child device on a
> > > > > spi bus.
> > > >
> > > > Maybe I'm the one missing something :). IIRC, spi would not allow two
> > > > devices on the same CS right? Because for this chip we would need
> > > > something like:
> > > >
> > > > spi {
> > > > dac@0 {
> > > > reg = <0>;
> > > > adi,pin-id = <0>;
> > > > };
> > > >
> > > > dac@1 {
> > > > reg = <0>; // which seems already problematic?
> > > > adi,pin-id <1>;
> > > > };
> > > >
> > > > ...
> > > >
> > > > //up to 4
> > > > };
> > > Yeah. It's not clear to me how that works for the microchip devices
> > > (I suspect it doesn't!)
> > >
> > > Just thinking as I type, but could we do something a bit nasty with
> > > a gpio mux that doesn't actually switch but represents the GPIO being
> > > shared? Given this is all tied to the spi bus that should all happen
> > > under serializing locks.
> > >
> > > Agreed though that this would be nicer as an SPI thing that let
> > > us specify that a single CS is share by multiple devices and their
> > > is some other signal acting to select which one we are talking to.
> > >
> >
> > If the device-addressing on the same chip-select is to be handled
> > by the spi framework, wouldn't we lose device-specific features?
> >
> > I understand that this multi-device feature is there mostly to extend the
> > channel count from 16 to 32, 48 or 64. I suppose the command:
> >
> > "MULTI DEVICE SW LDAC MODE"
> >
> > exists so that software can update channel values accross multiple devices.
>
> Right! You do have a point! I agree the main driver for a feature like
> this is likely to extend the channel count and effectively "aggregate"
> devices.
>
> But I would say that even with the spi solution the MULTI DEVICE stuff
> should be doable (as we still need a sort of adi,pin-id property).
I don't think we can have something like an IIO buffer shared by multiple
devices. Synchronizing separate devices would be doable with proper hardware
support for this (probably involving an FGPA).
> But yes, I do feel that the whole feature is for aggregation so seeing
> one device with 32 channels is the expectation here? Rather than seeing
> two devices with 16 channels.
Yes, I think aggregation is the whole point there... so that the IIO driver
is multi-device-aware.
--
Kind regards,
Rodrigo Alencar
^ 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