* [PATCH v2 0/6] misc: amd-sbi: Refactor SBTSI driver with I3C support and ioctl interface
@ 2026-05-15 13:45 Akshay Gupta
2026-05-15 13:45 ` [PATCH v2 1/6] hwmon/misc: amd-sbi: Move core sbtsi support from hwmon to misc Akshay Gupta
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Akshay Gupta @ 2026-05-15 13:45 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, akshay.gupta,
naveenkrishna.chatradhi, Prathima.Lk, Anand.Umarji, Kevin.Tung,
Akshay Gupta
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. 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.
5. 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.
6. 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 (6):
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: Add support for SB-TSI over I3C
misc: amd-sbi: Add SBTSI ioctl register transfer interface
docs: misc: amd-sbi: Document SBTSI userspace interface
Documentation/misc-devices/amd-sbi.rst | 64 ++++++++
drivers/hwmon/Kconfig | 2 +-
drivers/hwmon/sbtsi_temp.c | 158 ++++++++++----------
drivers/misc/amd-sbi/Kconfig | 13 ++
drivers/misc/amd-sbi/Makefile | 3 +
drivers/misc/amd-sbi/tsi-core.c | 148 ++++++++++++++++++
drivers/misc/amd-sbi/tsi-core.h | 15 ++
drivers/misc/amd-sbi/tsi.c | 198 +++++++++++++++++++++++++
include/linux/misc/tsi.h | 63 ++++++++
include/uapi/misc/amd-apml.h | 23 +++
10 files changed, 604 insertions(+), 83 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 [flat|nested] 12+ messages in thread
* [PATCH v2 1/6] hwmon/misc: amd-sbi: Move core sbtsi support from hwmon to misc
2026-05-15 13:45 [PATCH v2 0/6] misc: amd-sbi: Refactor SBTSI driver with I3C support and ioctl interface Akshay Gupta
@ 2026-05-15 13:45 ` Akshay Gupta
2026-05-15 14:22 ` sashiko-bot
2026-05-15 13:45 ` [PATCH v2 2/6] hwmon: sbtsi_temp: Refactor temperature register access into helpers Akshay Gupta
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Akshay Gupta @ 2026-05-15 13:45 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, akshay.gupta,
naveenkrishna.chatradhi, Prathima.Lk, Anand.Umarji, Kevin.Tung,
Akshay Gupta
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 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 | 73 ++++---------------
drivers/misc/amd-sbi/Kconfig | 13 ++++
drivers/misc/amd-sbi/Makefile | 3 +
drivers/misc/amd-sbi/tsi.c | 129 ++++++++++++++++++++++++++++++++++
include/linux/misc/tsi.h | 34 +++++++++
6 files changed, 194 insertions(+), 60 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 14e4cea48acc..6fa51e6ef6ff 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1939,7 +1939,7 @@ config SENSORS_SL28CPLD
config SENSORS_SBTSI
tristate "Emulated SB-TSI temperature sensor"
- depends on I2C
+ depends on 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 c5b2488c4c7f..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[] = {
- {"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..ee2216785550
--- /dev/null
+++ b/drivers/misc/amd-sbi/tsi.c
@@ -0,0 +1,129 @@
+// 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;
+ 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[] = {
+ {"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..6f7177edbcf5
--- /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_TSI_H_
+#define _LINUX_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_TSI_H_ */
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/6] hwmon: sbtsi_temp: Refactor temperature register access into helpers
2026-05-15 13:45 [PATCH v2 0/6] misc: amd-sbi: Refactor SBTSI driver with I3C support and ioctl interface Akshay Gupta
2026-05-15 13:45 ` [PATCH v2 1/6] hwmon/misc: amd-sbi: Move core sbtsi support from hwmon to misc Akshay Gupta
@ 2026-05-15 13:45 ` Akshay Gupta
2026-05-15 13:45 ` [PATCH v2 3/6] hwmon/misc: amd-sbi: Move sbtsi register transfer to core abstraction Akshay Gupta
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Akshay Gupta @ 2026-05-15 13:45 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, akshay.gupta,
naveenkrishna.chatradhi, Prathima.Lk, Anand.Umarji, Kevin.Tung,
Akshay Gupta
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 [flat|nested] 12+ messages in thread
* [PATCH v2 3/6] hwmon/misc: amd-sbi: Move sbtsi register transfer to core abstraction
2026-05-15 13:45 [PATCH v2 0/6] misc: amd-sbi: Refactor SBTSI driver with I3C support and ioctl interface Akshay Gupta
2026-05-15 13:45 ` [PATCH v2 1/6] hwmon/misc: amd-sbi: Move core sbtsi support from hwmon to misc Akshay Gupta
2026-05-15 13:45 ` [PATCH v2 2/6] hwmon: sbtsi_temp: Refactor temperature register access into helpers Akshay Gupta
@ 2026-05-15 13:45 ` Akshay Gupta
2026-05-15 13:45 ` [PATCH v2 4/6] misc: amd-sbi: Add support for SB-TSI over I3C Akshay Gupta
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Akshay Gupta @ 2026-05-15 13:45 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, akshay.gupta,
naveenkrishna.chatradhi, Prathima.Lk, Anand.Umarji, Kevin.Tung,
Akshay Gupta
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 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 6f7177edbcf5..8f8cb90c2023 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_TSI_H_ */
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/6] misc: amd-sbi: Add support for SB-TSI over I3C
2026-05-15 13:45 [PATCH v2 0/6] misc: amd-sbi: Refactor SBTSI driver with I3C support and ioctl interface Akshay Gupta
` (2 preceding siblings ...)
2026-05-15 13:45 ` [PATCH v2 3/6] hwmon/misc: amd-sbi: Move sbtsi register transfer to core abstraction Akshay Gupta
@ 2026-05-15 13:45 ` Akshay Gupta
2026-05-15 15:33 ` sashiko-bot
2026-05-15 13:45 ` [PATCH v2 5/6] misc: amd-sbi: Add SBTSI ioctl register transfer interface Akshay Gupta
2026-05-15 13:45 ` [PATCH v2 6/6] docs: misc: amd-sbi: Document SBTSI userspace interface Akshay Gupta
5 siblings, 1 reply; 12+ messages in thread
From: Akshay Gupta @ 2026-05-15 13:45 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, akshay.gupta,
naveenkrishna.chatradhi, Prathima.Lk, Anand.Umarji, Kevin.Tung,
Akshay Gupta
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 v1:
- Changes in accordance with usage of auxiliary device
drivers/misc/amd-sbi/Kconfig | 4 +--
drivers/misc/amd-sbi/tsi-core.c | 38 ++++++++++++++++++++
drivers/misc/amd-sbi/tsi.c | 61 +++++++++++++++++++++++++++++++--
include/linux/misc/tsi.h | 10 +++++-
4 files changed, 107 insertions(+), 6 deletions(-)
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..19388737b225 100644
--- a/drivers/misc/amd-sbi/tsi-core.c
+++ b/drivers/misc/amd-sbi/tsi-core.c
@@ -23,8 +23,46 @@ 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] = { };
+
+ /* Add Register data to read/write */
+ xfers[0].rnw = false;
+ xfers[0].len = 1;
+ xfers[0].data.out = ®
+
+ xfers[1].rnw = true;
+ xfers[1].len = 1;
+ xfers[1].data.in = val;
+
+ return i3c_device_do_xfers(data->i3cdev, xfers, 2, I3C_SDR);
+}
+
+/* I3C write transfer function */
+static int sbtsi_i3c_write(struct sbtsi_data *data, u8 reg, u8 val)
+{
+ u8 buf[2];
+ 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.c b/drivers/misc/amd-sbi/tsi.c
index ee2216785550..43bbac7faf08 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.
@@ -89,6 +89,7 @@ static int sbtsi_i2c_probe(struct i2c_client *client)
if (!data)
return -ENOMEM;
+ data->is_i3c = false;
data->client = client;
err = i2c_smbus_read_byte_data(data->client, SBTSI_REG_CONFIG);
if (err < 0)
@@ -123,7 +124,61 @@ 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 sbtsi_data *data;
+ int err;
+ u8 val;
+
+ /*
+ * 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.
+ */
+ if (I3C_PID_INSTANCE_ID(i3cdev->desc->info.pid) != 0)
+ return -ENXIO;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->i3cdev = i3cdev;
+ data->is_i3c = true;
+
+ 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, i3cdev->desc->info.dyn_addr);
+}
+
+static const struct i3c_device_id sbtsi_i3c_id[] = {
+ /* PID for AMD SBTSI device */
+ I3C_DEVICE_EXTRA_INFO(0x112, 0x0, 0x1, NULL),
+ 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 8f8cb90c2023..7ce689081427 100644
--- a/include/linux/misc/tsi.h
+++ b/include/linux/misc/tsi.h
@@ -9,18 +9,26 @@
#define _LINUX_TSI_H_
#include <linux/i2c.h>
+#include <linux/i3c/device.h>
+#include <linux/i3c/master.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)
* @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;
+ };
bool ext_range_mode;
bool read_order;
+ bool is_i3c;
};
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 5/6] misc: amd-sbi: Add SBTSI ioctl register transfer interface
2026-05-15 13:45 [PATCH v2 0/6] misc: amd-sbi: Refactor SBTSI driver with I3C support and ioctl interface Akshay Gupta
` (3 preceding siblings ...)
2026-05-15 13:45 ` [PATCH v2 4/6] misc: amd-sbi: Add support for SB-TSI over I3C Akshay Gupta
@ 2026-05-15 13:45 ` Akshay Gupta
2026-05-15 14:11 ` Guenter Roeck
2026-05-15 15:58 ` sashiko-bot
2026-05-15 13:45 ` [PATCH v2 6/6] docs: misc: amd-sbi: Document SBTSI userspace interface Akshay Gupta
5 siblings, 2 replies; 12+ messages in thread
From: Akshay Gupta @ 2026-05-15 13:45 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, akshay.gupta,
naveenkrishna.chatradhi, Prathima.Lk, Anand.Umarji, Kevin.Tung,
Akshay Gupta
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 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/hwmon/sbtsi_temp.c | 6 +++
drivers/misc/amd-sbi/tsi-core.c | 84 ++++++++++++++++++++++++++++++++-
drivers/misc/amd-sbi/tsi-core.h | 15 ++++++
drivers/misc/amd-sbi/tsi.c | 20 ++++++--
include/linux/misc/tsi.h | 8 ++++
include/uapi/misc/amd-apml.h | 23 +++++++++
6 files changed, 151 insertions(+), 5 deletions(-)
create mode 100644 drivers/misc/amd-sbi/tsi-core.h
diff --git a/drivers/hwmon/sbtsi_temp.c b/drivers/hwmon/sbtsi_temp.c
index d7ae986d824c..00e982f4c716 100644
--- a/drivers/hwmon/sbtsi_temp.c
+++ b/drivers/hwmon/sbtsi_temp.c
@@ -64,12 +64,15 @@ static inline void sbtsi_mc_to_reg(s32 temp, u8 *integer, u8 *decimal)
/*
* 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.
+ * The mutex protects against concurrent access to the shared I2C/I3C bus by
+ * the hwmon sysfs and a userspace ioctl
*/
static int sbtsi_temp_read(struct sbtsi_data *data, u8 reg1, u8 reg2,
u8 *val1, u8 *val2)
{
int ret;
+ guard(mutex)(&data->lock);
ret = sbtsi_xfer(data, reg1, val1, true);
if (!ret)
ret = sbtsi_xfer(data, reg2, val2, true);
@@ -78,12 +81,15 @@ static int sbtsi_temp_read(struct sbtsi_data *data, u8 reg1, u8 reg2,
/*
* Write integer and decimal parts of an SB-TSI temperature register pair.
+ * The mutex protects against concurrent access to the shared I2C/I3C bus by
+ * the hwmon sysfs and a userspace ioctl
*/
static int sbtsi_temp_write(struct sbtsi_data *data, u8 reg_int, u8 reg_dec,
u8 val_int, u8 val_dec)
{
int ret;
+ guard(mutex)(&data->lock);
ret = sbtsi_xfer(data, reg_int, &val_int, false);
if (!ret)
ret = sbtsi_xfer(data, reg_dec, &val_dec, false);
diff --git a/drivers/misc/amd-sbi/tsi-core.c b/drivers/misc/amd-sbi/tsi-core.c
index 19388737b225..c5bd60409d5b 100644
--- a/drivers/misc/amd-sbi/tsi-core.c
+++ b/drivers/misc/amd-sbi/tsi-core.c
@@ -6,8 +6,12 @@
* Copyright (C) 2026 Advanced Micro Devices, Inc.
*/
+#include <linux/fs.h>
+#include <linux/ioctl.h>
#include <linux/module.h>
-#include <linux/misc/tsi.h>
+#include <linux/uaccess.h>
+#include <uapi/misc/amd-apml.h>
+#include "tsi-core.h"
/* I2C transfer function */
static int sbtsi_i2c_xfer(struct sbtsi_data *data, u8 reg, u8 *val, bool is_read)
@@ -62,7 +66,83 @@ 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(mutex)(&data->lock);
+ 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 (copy_from_user(&msg, arg, sizeof(struct apml_tsi_xfer_msg)))
+ return -EFAULT;
+
+ 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 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,
+ .unlocked_ioctl = sbtsi_ioctl,
+ .compat_ioctl = compat_ptr_ioctl,
+};
+
+static void sbtsi_misc_deregister(void *data)
+{
+ misc_deregister((struct miscdevice *)data);
+}
+
+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 devm_add_action_or_reset(dev, sbtsi_misc_deregister,
+ &data->sbtsi_misc_dev);
+}
diff --git a/drivers/misc/amd-sbi/tsi-core.h b/drivers/misc/amd-sbi/tsi-core.h
new file mode 100644
index 000000000000..7bf967a09837
--- /dev/null
+++ b/drivers/misc/amd-sbi/tsi-core.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * AMD SBTSI misc tsi device .
+ *
+ * Copyright (C) 2026 Advanced Micro Devices, Inc.
+ */
+
+#ifndef _LINUX_TSI_CORE_H_
+#define _LINUX_TSI_CORE_H_
+
+#include <linux/misc/tsi.h>
+
+int create_misc_tsi_device(struct sbtsi_data *data, struct device *dev);
+
+#endif /* _LINUX_TSI_CORE_H_ */
diff --git a/drivers/misc/amd-sbi/tsi.c b/drivers/misc/amd-sbi/tsi.c
index 43bbac7faf08..6a9356740f4e 100644
--- a/drivers/misc/amd-sbi/tsi.c
+++ b/drivers/misc/amd-sbi/tsi.c
@@ -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 */
@@ -89,6 +89,9 @@ static int sbtsi_i2c_probe(struct i2c_client *client)
if (!data)
return -ENOMEM;
+ err = devm_mutex_init(dev, &data->lock);
+ if (err)
+ return err;
data->is_i3c = false;
data->client = client;
err = i2c_smbus_read_byte_data(data->client, SBTSI_REG_CONFIG);
@@ -98,7 +101,11 @@ static int sbtsi_i2c_probe(struct i2c_client *client)
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);
+ err = sbtsi_create_hwmon_adev(dev, client->addr);
+ if (err < 0)
+ return err;
+ data->dev_addr = client->addr;
+ return create_misc_tsi_device(data, dev);
}
static const struct i2c_device_id sbtsi_id[] = {
@@ -145,6 +152,9 @@ static int sbtsi_i3c_probe(struct i3c_device *i3cdev)
if (!data)
return -ENOMEM;
+ err = devm_mutex_init(dev, &data->lock);
+ if (err)
+ return err;
data->i3cdev = i3cdev;
data->is_i3c = true;
@@ -156,7 +166,11 @@ static int sbtsi_i3c_probe(struct i3c_device *i3cdev)
data->read_order = FIELD_GET(BIT(SBTSI_CONFIG_READ_ORDER_SHIFT), val);
dev_set_drvdata(dev, data);
- return sbtsi_create_hwmon_adev(dev, i3cdev->desc->info.dyn_addr);
+ err = sbtsi_create_hwmon_adev(dev, i3cdev->desc->info.dyn_addr);
+ if (err < 0)
+ return err;
+ data->dev_addr = i3cdev->desc->info.dyn_addr;
+ return create_misc_tsi_device(data, dev);
}
static const struct i3c_device_id sbtsi_i3c_id[] = {
diff --git a/include/linux/misc/tsi.h b/include/linux/misc/tsi.h
index 7ce689081427..184b1aa14f0a 100644
--- a/include/linux/misc/tsi.h
+++ b/include/linux/misc/tsi.h
@@ -11,12 +11,17 @@
#include <linux/i2c.h>
#include <linux/i3c/device.h>
#include <linux/i3c/master.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
+ * @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
@@ -26,6 +31,9 @@ struct sbtsi_data {
struct i2c_client *client;
struct i3c_device *i3cdev;
};
+ struct miscdevice sbtsi_misc_dev;
+ struct mutex lock; /* protects concurrent access to the device */
+ u8 dev_addr;
bool ext_range_mode;
bool read_order;
bool is_i3c;
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 [flat|nested] 12+ messages in thread
* [PATCH v2 6/6] docs: misc: amd-sbi: Document SBTSI userspace interface
2026-05-15 13:45 [PATCH v2 0/6] misc: amd-sbi: Refactor SBTSI driver with I3C support and ioctl interface Akshay Gupta
` (4 preceding siblings ...)
2026-05-15 13:45 ` [PATCH v2 5/6] misc: amd-sbi: Add SBTSI ioctl register transfer interface Akshay Gupta
@ 2026-05-15 13:45 ` Akshay Gupta
5 siblings, 0 replies; 12+ messages in thread
From: Akshay Gupta @ 2026-05-15 13:45 UTC (permalink / raw)
To: linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, linux, arnd, gregkh, akshay.gupta,
naveenkrishna.chatradhi, Prathima.Lk, Anand.Umarji, Kevin.Tung,
Akshay Gupta
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 v1:
- Elaborate the document
Documentation/misc-devices/amd-sbi.rst | 64 ++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/Documentation/misc-devices/amd-sbi.rst b/Documentation/misc-devices/amd-sbi.rst
index f91ddadefe48..6a6344439ef5 100644
--- a/Documentation/misc-devices/amd-sbi.rst
+++ b/Documentation/misc-devices/amd-sbi.rst
@@ -48,6 +48,56 @@ 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.
+
+.. 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 +113,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 +138,16 @@ Next thing, open the device file, as follows::
exit(1);
}
+To open SB-TSI device::
+
+ int file;
+
+ file = open("/dev/sbtsi-*", O_RDWR);
+ if (file < 0) {
+ /* ERROR HANDLING */
+ exit(1);
+ }
+
The following IOCTLs are defined:
``#define SB_BASE_IOCTL_NR 0xF9``
@@ -92,6 +155,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 [flat|nested] 12+ messages in thread
* Re: [PATCH v2 5/6] misc: amd-sbi: Add SBTSI ioctl register transfer interface
2026-05-15 13:45 ` [PATCH v2 5/6] misc: amd-sbi: Add SBTSI ioctl register transfer interface Akshay Gupta
@ 2026-05-15 14:11 ` Guenter Roeck
2026-05-15 15:58 ` sashiko-bot
1 sibling, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2026-05-15 14:11 UTC (permalink / raw)
To: Akshay Gupta, linux-doc, linux-kernel, linux-hwmon
Cc: corbet, skhan, arnd, gregkh, naveenkrishna.chatradhi, Prathima.Lk,
Anand.Umarji, Kevin.Tung
On 5/15/26 06:45, Akshay Gupta wrote:
> 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 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/hwmon/sbtsi_temp.c | 6 +++
> drivers/misc/amd-sbi/tsi-core.c | 84 ++++++++++++++++++++++++++++++++-
> drivers/misc/amd-sbi/tsi-core.h | 15 ++++++
> drivers/misc/amd-sbi/tsi.c | 20 ++++++--
> include/linux/misc/tsi.h | 8 ++++
> include/uapi/misc/amd-apml.h | 23 +++++++++
> 6 files changed, 151 insertions(+), 5 deletions(-)
> create mode 100644 drivers/misc/amd-sbi/tsi-core.h
>
> diff --git a/drivers/hwmon/sbtsi_temp.c b/drivers/hwmon/sbtsi_temp.c
> index d7ae986d824c..00e982f4c716 100644
> --- a/drivers/hwmon/sbtsi_temp.c
> +++ b/drivers/hwmon/sbtsi_temp.c
> @@ -64,12 +64,15 @@ static inline void sbtsi_mc_to_reg(s32 temp, u8 *integer, u8 *decimal)
> /*
> * 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.
> + * The mutex protects against concurrent access to the shared I2C/I3C bus by
> + * the hwmon sysfs and a userspace ioctl
> */
> static int sbtsi_temp_read(struct sbtsi_data *data, u8 reg1, u8 reg2,
> u8 *val1, u8 *val2)
> {
> int ret;
>
> + guard(mutex)(&data->lock);
I would suggest to hide this behind access functions such as sbtsi_lock(),
sbtsi_unlock(), and the matching guard functions. That can be done in a
separate patch; it should not be necessary to include hwmon in the patch
introducing the ioctl.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/6] hwmon/misc: amd-sbi: Move core sbtsi support from hwmon to misc
2026-05-15 13:45 ` [PATCH v2 1/6] hwmon/misc: amd-sbi: Move core sbtsi support from hwmon to misc Akshay Gupta
@ 2026-05-15 14:22 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-05-15 14:22 UTC (permalink / raw)
To: Akshay Gupta; +Cc: linux-hwmon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Auxiliary device ID collision prevents probe of multiple SB-TSI sensors
- [Medium] Kconfig dependency change causes silent loss of the hwmon driver on kernel upgrades
--
commit fe75a41fdaa2ec32654699f9128ef6e13b7c7140
Author: Prathima <Prathima.Lk@amd.com>
hwmon/misc: amd-sbi: Move core sbtsi support from hwmon to misc
This commit moves the core Side-Band Temperature Sensor Interface (SBTSI)
logic into drivers/misc/amd-sbi/tsi.c. It registers an auxiliary device per
target so that the hwmon driver can bind to it while allowing other consumers
to reuse the core functionality.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 14e4cea48acc4..6fa51e6ef6ff4 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1939,7 +1939,7 @@ config SENSORS_SL28CPLD
>
> config SENSORS_SBTSI
> tristate "Emulated SB-TSI temperature sensor"
> - depends on I2C
> + depends on AMD_SBTSI
Is there a risk that this dependency change could silently disable the
hwmon driver on kernel upgrades?
Because CONFIG_AMD_SBTSI has no default value, an unattended configuration
update like make olddefconfig might drop CONFIG_SENSORS_SBTSI entirely due
to unmet dependencies.
Could CONFIG_AMD_SBTSI be selected by CONFIG_SENSORS_SBTSI instead, or use
a default strategy to ensure existing user configurations are preserved?
[ ... ]
> diff --git a/drivers/misc/amd-sbi/tsi.c b/drivers/misc/amd-sbi/tsi.c
> new file mode 100644
> index 0000000000000..ee22167855505
> --- /dev/null
> +++ b/drivers/misc/amd-sbi/tsi.c
[ ... ]
> +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;
> + adev->id = dev_addr;
Could this assignment to adev->id cause a collision on multi-socket AMD
systems?
It looks like auxiliary device names are constructed globally as
modname.name.id by __auxiliary_device_add().
If multi-socket systems have identical sensors at the same I2C address
(for example, 0x4c) on different I2C adapters, probing the second sensor
might result in a duplicate sysfs name and cause device_add() to fail with
-EEXIST.
Should the driver allocate a globally unique instance ID for each
auxiliary device using an IDA, rather than relying solely on the I2C
address?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260515134506.397649-1-Akshay.Gupta@amd.com?part=1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/6] misc: amd-sbi: Add support for SB-TSI over I3C
2026-05-15 13:45 ` [PATCH v2 4/6] misc: amd-sbi: Add support for SB-TSI over I3C Akshay Gupta
@ 2026-05-15 15:33 ` sashiko-bot
2026-05-15 16:21 ` Guenter Roeck
0 siblings, 1 reply; 12+ messages in thread
From: sashiko-bot @ 2026-05-15 15:33 UTC (permalink / raw)
To: Akshay Gupta; +Cc: linux-hwmon
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Kconfig dependency uses a non-existent symbol (`I3C_OR_I2C`), causing silent build failures.
- [High] I3C transfer functions pass stack-allocated buffers for DMA transfers, risking memory corruption and panics.
- [Medium] The driver violates I3C subsystem API boundaries by including `<linux/i3c/master.h>` to access opaque internal structures.
--
commit fd51ed18b45ce179c5ee6ea15c970ec2d140c6c7
Author: Prathima <Prathima.Lk@amd.com>
misc: amd-sbi: Add support for SB-TSI over I3C
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 registers both I2C and I3C drivers using module_i3c_i2c_driver().
> diff --git a/drivers/misc/amd-sbi/Kconfig b/drivers/misc/amd-sbi/Kconfig
> --- 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
Is this a valid Kconfig symbol? It looks like it will evaluate as a single
config symbol named CONFIG_I3C_OR_I2C instead of a logical OR. Could this
cause the driver to be silently unbuildable? Perhaps it should be written
as depends on I3C || I2C?
> diff --git a/drivers/misc/amd-sbi/tsi-core.c b/drivers/misc/amd-sbi/tsi-core.c
> --- a/drivers/misc/amd-sbi/tsi-core.c
> +++ b/drivers/misc/amd-sbi/tsi-core.c
[ ... ]
> +/* I3C read transfer function */
> +static int sbtsi_i3c_read(struct sbtsi_data *data, u8 reg, u8 *val)
> +{
> + struct i3c_xfer xfers[2] = { };
> +
> + /* Add Register data to read/write */
> + xfers[0].rnw = false;
> + xfers[0].len = 1;
> + xfers[0].data.out = ®
> +
> + xfers[1].rnw = true;
> + xfers[1].len = 1;
> + xfers[1].data.in = val;
> +
> + return i3c_device_do_xfers(data->i3cdev, xfers, 2, I3C_SDR);
> +}
Does i3c_device_do_xfers() map the buffers for DMA? If so, passing stack
memory like ® and val could lead to memory corruption or kernel panics
when CONFIG_VMAP_STACK is enabled. Could these buffers be allocated on the
heap, for example by embedding them in the sbtsi_data structure?
> +/* I3C write transfer function */
> +static int sbtsi_i3c_write(struct sbtsi_data *data, u8 reg, u8 val)
> +{
> + u8 buf[2];
> + 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);
> +}
Similarly, does buf[2] on the stack present a DMA mapping issue here?
> diff --git a/drivers/misc/amd-sbi/tsi.c b/drivers/misc/amd-sbi/tsi.c
> --- a/drivers/misc/amd-sbi/tsi.c
> +++ b/drivers/misc/amd-sbi/tsi.c
[ ... ]
> +static int sbtsi_i3c_probe(struct i3c_device *i3cdev)
> +{
> + struct device *dev = i3cdev_to_dev(i3cdev);
> + struct sbtsi_data *data;
> + int err;
> + u8 val;
> +
> + /*
> + * 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.
> + */
> + if (I3C_PID_INSTANCE_ID(i3cdev->desc->info.pid) != 0)
> + return -ENXIO;
Is accessing i3cdev->desc->info.pid directly safe? The i3c_device struct is
typically treated as opaque to client drivers. Would it be better to use
i3c_device_get_info() to populate a struct i3c_device_info and read the
fields from there to ensure internal locking is respected?
[ ... ]
> + 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, i3cdev->desc->info.dyn_addr);
> +}
Similarly here, can this direct access to i3cdev->desc->info.dyn_addr be
replaced with the public i3c_device_get_info() API?
> diff --git a/include/linux/misc/tsi.h b/include/linux/misc/tsi.h
> --- a/include/linux/misc/tsi.h
> +++ b/include/linux/misc/tsi.h
> @@ -9,18 +9,26 @@
> #define _LINUX_TSI_H_
>
> #include <linux/i2c.h>
> +#include <linux/i3c/device.h>
> +#include <linux/i3c/master.h>
Does the driver need to include linux/i3c/master.h? Client drivers normally
only include linux/i3c/device.h, and relying on master.h might break the
I3C API boundaries.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260515134506.397649-1-Akshay.Gupta@amd.com?part=4
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 5/6] misc: amd-sbi: Add SBTSI ioctl register transfer interface
2026-05-15 13:45 ` [PATCH v2 5/6] misc: amd-sbi: Add SBTSI ioctl register transfer interface Akshay Gupta
2026-05-15 14:11 ` Guenter Roeck
@ 2026-05-15 15:58 ` sashiko-bot
1 sibling, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-05-15 15:58 UTC (permalink / raw)
To: Akshay Gupta; +Cc: linux-hwmon
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] Use-After-Free of `sbtsi_data` due to misaligned devres and VFS lifecycles
- [High] Unsafe DMA Buffer Allocation from stack variables
- [High] Subsystem Guideline Violation: Redundant custom lock instead of hwmon_lock()
- [High] sysfs name collision for miscdevice on multi-bus systems
- [Medium] Hardware state desynchronization via unrestricted IOCTL writes
- [Medium] UAPI Flaw: Unvalidated padding byte in IOCTL message
--
commit a076ef47ca989260af804712d32ac63c2aaf11cb
Author: Prathima <Prathima.Lk@amd.com>
misc: amd-sbi: Add SBTSI ioctl register transfer interface
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.
> diff --git a/drivers/hwmon/sbtsi_temp.c b/drivers/hwmon/sbtsi_temp.c
> index d7ae986d824c4..00e982f4c716c 100644
> --- a/drivers/hwmon/sbtsi_temp.c
> +++ b/drivers/hwmon/sbtsi_temp.c
> @@ -64,12 +64,15 @@ static inline void sbtsi_mc_to_reg(s32 temp, u8 *integer, u8 *decimal)
> /*
> * 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.
> + * The mutex protects against concurrent access to the shared I2C/I3C bus by
> + * the hwmon sysfs and a userspace ioctl
> */
> static int sbtsi_temp_read(struct sbtsi_data *data, u8 reg1, u8 reg2,
> u8 *val1, u8 *val2)
> {
> int ret;
>
> + guard(mutex)(&data->lock);
Does this custom lock bypass the standardized hwmon core synchronization
mechanism? The hwmon subsystem guidelines appear to mandate using
hwmon_lock() and hwmon_unlock() to serialize sysfs operations against
attributes registered by other means.
[ ... ]
> diff --git a/drivers/misc/amd-sbi/tsi-core.c b/drivers/misc/amd-sbi/tsi-core.c
> index 19388737b225e..c5bd60409d5bb 100644
> --- a/drivers/misc/amd-sbi/tsi-core.c
> +++ b/drivers/misc/amd-sbi/tsi-core.c
[ ... ]
> +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 (copy_from_user(&msg, arg, sizeof(struct apml_tsi_xfer_msg)))
> + return -EFAULT;
> +
> + ret = sbtsi_xfer_ioctl(data, msg.reg_addr, &msg.data_in_out, msg.rflag);
Could passing &msg.data_in_out to sbtsi_xfer_ioctl() eventually result in
a DMA transfer from stack memory?
If the transfer goes over I3C, sbtsi_i3c_read() uses this pointer directly:
static int sbtsi_i3c_read(struct sbtsi_data *data, u8 reg, u8 *val)
{
struct i3c_xfer xfers[2] = { };
xfers[0].rnw = false;
xfers[0].len = 1;
xfers[0].data.out = ®
xfers[1].rnw = true;
xfers[1].len = 1;
xfers[1].data.in = val;
return i3c_device_do_xfers(data->i3cdev, xfers, 2, I3C_SDR);
}
Since I3C host controllers commonly use DMA, and stack variables are not
cacheline aligned (and reside in virtually mapped memory with
CONFIG_VMAP_STACK enabled), this might violate the DMA API and lead to adjacent
stack corruption.
Furthermore, does this unrestricted IOCTL write capability allow userspace
to overwrite the SBTSI_REG_CONFIG register?
During probe, the driver caches hardware state (ext_range_mode and
read_order) from this register. If userspace modifies it without the
driver intercepting the write to update its cache, the hwmon interface
could report incorrect temperatures.
[ ... ]
> +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);
Can this lead to a use-after-free if the device is unbound?
The sbtsi_data structure is allocated with devm_kzalloc(), tying its
lifetime to the device's driver binding. When the device unbinds, devres
calls misc_deregister() and frees sbtsi_data.
Since misc_deregister() does not close or revoke existing file descriptors,
a process could retain its file descriptor and call ioctl() after the unbind,
extracting the freed sbtsi_data pointer and attempting to lock data->lock.
[ ... ]
> +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);
Will this naming scheme cause a sysfs name collision on systems with
multiple I2C/I3C buses?
If identically-addressed sensors exist on different buses, it looks like
misc_register() will attempt to create a duplicate sysfs entry
(e.g., /sys/class/misc/sbtsi-4c), fail with -EEXIST, and abort the probe.
[ ... ]
> diff --git a/include/uapi/misc/amd-apml.h b/include/uapi/misc/amd-apml.h
> index 745b3338fc06f..8a85f79b0938d 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 */
> +};
Should msg.pad be validated in apml_tsi_reg_xfer() to ensure it is zero?
Without this check, userspace can pass uninitialized garbage in the padding
field, which might prevent safely repurposing this field for new flags or
extensions in the future.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260515134506.397649-1-Akshay.Gupta@amd.com?part=5
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 4/6] misc: amd-sbi: Add support for SB-TSI over I3C
2026-05-15 15:33 ` sashiko-bot
@ 2026-05-15 16:21 ` Guenter Roeck
0 siblings, 0 replies; 12+ messages in thread
From: Guenter Roeck @ 2026-05-15 16:21 UTC (permalink / raw)
To: sashiko-reviews, Akshay Gupta; +Cc: linux-hwmon
On 5/15/26 08:33, sashiko-bot@kernel.org wrote:
...
>
>> diff --git a/drivers/misc/amd-sbi/Kconfig b/drivers/misc/amd-sbi/Kconfig
>> --- 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
>
> Is this a valid Kconfig symbol? It looks like it will evaluate as a single
> config symbol named CONFIG_I3C_OR_I2C instead of a logical OR. Could this
> cause the driver to be silently unbuildable? Perhaps it should be written
> as depends on I3C || I2C?
>
We'll need to teach Sashiko about this. I submitted a pull request.
Guenter
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-05-15 16:21 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-15 13:45 [PATCH v2 0/6] misc: amd-sbi: Refactor SBTSI driver with I3C support and ioctl interface Akshay Gupta
2026-05-15 13:45 ` [PATCH v2 1/6] hwmon/misc: amd-sbi: Move core sbtsi support from hwmon to misc Akshay Gupta
2026-05-15 14:22 ` sashiko-bot
2026-05-15 13:45 ` [PATCH v2 2/6] hwmon: sbtsi_temp: Refactor temperature register access into helpers Akshay Gupta
2026-05-15 13:45 ` [PATCH v2 3/6] hwmon/misc: amd-sbi: Move sbtsi register transfer to core abstraction Akshay Gupta
2026-05-15 13:45 ` [PATCH v2 4/6] misc: amd-sbi: Add support for SB-TSI over I3C Akshay Gupta
2026-05-15 15:33 ` sashiko-bot
2026-05-15 16:21 ` Guenter Roeck
2026-05-15 13:45 ` [PATCH v2 5/6] misc: amd-sbi: Add SBTSI ioctl register transfer interface Akshay Gupta
2026-05-15 14:11 ` Guenter Roeck
2026-05-15 15:58 ` sashiko-bot
2026-05-15 13:45 ` [PATCH v2 6/6] docs: misc: amd-sbi: Document SBTSI userspace interface Akshay Gupta
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox