[PATCH v4 0/3] i3c: Introduce i3c device userspace interface

[PATCH v4 0/3] i3c: Introduce i3c device userspace interface

[Sam Agazaryan]

This series revives the I3C userspace character device interface (i3cdev),
originally proposed by Vitor Soares in 2019/2020 [1][2].

1. Motivation & Production Use Case:
Previously, one of the reservations against introducing an i3cdev module
was the lack of a standardized production userspace use case beyond
controller development and bringup.

We now have a standardized industry use case: the OCP Secure Firmware
Recovery Specification (v1.1) and Open-Source Silicon Root of Trust
(Caliptra) recovery flows over I3C. When an I3C target device (SoC, ASIC,
or SRoT) is held in ROM or bootloader recovery mode, no functional
in-kernel driver is bound to the target. A userspace recovery daemon on
the BMC must perform private SDR transfers to interact with Recovery
Control & Status Registers (CSRs) and stream recovery firmware images.

Exposing an interface modeled after i2c-dev/spidev allows userspace
recovery tools to operate directly on unbound I3C devices without
requiring rigid or proprietary in-kernel recovery drivers.

2. Changes since v3 (Feb 2020):
- Rebased onto upstream i3c/next and adapted to the unified i3c_xfer API
  (replacing deprecated i3c_priv_xfer / i3c_device_do_priv_xfers with
  i3c_xfer / i3c_device_do_xfers(..., I3C_SDR)).
- Addressed UAPI structure feedback from Greg KH and Arnd Bergmann:
  * Used explicit __u64 for user data buffer addresses.
  * Added fixed-width explicit padding to ensure consistent 32-bit /
    64-bit ABI alignment.
- Wired up .compat_ioctl = compat_ptr_ioctl in file_operations.
- Fixed device lifecycle and concurrency:
  * Switched to cdev_device_add() and cdev_device_del() with an embedded
    struct device and a release callback, preventing use-after-free and
    races on driver unbind/detach (incorporating fix from Oleksandr
    Shulzhenko).
  * Cleared i3cdev->i3c under xfer_lock on detach so concurrent/subsequent
    file operations safely return -ENODEV.
- Fixed read transfer buffer allocation:
  * Replaced unconditional memdup_user() with kzalloc() for read
    transfers (rnw == true), avoiding copying uninitialized userspace
    memory into the kernel buffer.
- Fixed mutex leak in i3cdev_read() and i3cdev_write() error paths.
- Switched to static const struct class with class_register().
- Switched minor number allocation to the IDA allocator.
- Added include/uapi/linux/i3c/ to MAINTAINERS under I3C SUBSYSTEM.

[1] https://lore.kernel.org/all/cover.1575977795.git.vitor.soares@synopsys.com/
[2] https://github.com/vitor-soares-snps/i3c-tools

Vitor Soares (3):
  i3c: master: export i3c_masterdev_type
  i3c: master: add i3c_for_each_dev helper
  i3c: add i3cdev module to expose i3c dev in /dev

 MAINTAINERS                     |   1 +
 drivers/i3c/Kconfig             |  11 +
 drivers/i3c/Makefile            |   1 +
 drivers/i3c/i3cdev.c            | 445 ++++++++++++++++++++++++++++++++
 drivers/i3c/internals.h         |   4 +
 drivers/i3c/master.c            |  15 +-
 include/uapi/linux/i3c/i3cdev.h |  37 +++
 7 files changed, 513 insertions(+), 1 deletion(-)
 create mode 100644 drivers/i3c/i3cdev.c
 create mode 100644 include/uapi/linux/i3c/i3cdev.h

-- 
2.55.0.979.g7e5102b832-goog

[PATCH v4 1/3] i3c: master: export i3c_masterdev_type

[Sam Agazaryan]

From: Vitor Soares <vitor.soares@toradex.com>

Export i3c_masterdev_type so i3cdev module can verify if an i3c device
is a master.

Signed-off-by: Vitor Soares <vitor.soares@toradex.com>
Signed-off-by: Sam Agazaryan <samagazaryan@google.com>
---
 drivers/i3c/internals.h | 2 ++
 drivers/i3c/master.c    | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
index 86a36b951e0d..cbc00c862104 100644
--- a/drivers/i3c/internals.h
+++ b/drivers/i3c/internals.h
@@ -11,6 +11,8 @@
 #include <linux/i3c/master.h>
 #include <linux/io.h>
 
+extern const struct device_type i3c_masterdev_type;
+
 int __must_check i3c_bus_rpm_get(struct i3c_bus *bus);
 void i3c_bus_rpm_put(struct i3c_bus *bus);
 bool i3c_bus_rpm_ibi_allowed(struct i3c_bus *bus);
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index afcd7a21a3e6..e8a9f8c3041e 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -864,9 +864,10 @@ static void i3c_masterdev_release(struct device *dev)
 	i3c_master_free_i3c_dev(master->this);
 }
 
-static const struct device_type i3c_masterdev_type = {
+const struct device_type i3c_masterdev_type = {
 	.groups	= i3c_masterdev_groups,
 };
+EXPORT_SYMBOL_GPL(i3c_masterdev_type);
 
 static void i3c_master_shutdown(struct i3c_master_controller *master)
 {
-- 
2.55.0.979.g7e5102b832-goog

[PATCH v4 2/3] i3c: master: add i3c_for_each_dev helper

[Sam Agazaryan]

From: Vitor Soares <vitor.soares@toradex.com>

Introduce i3c_for_each_dev(), an i3c device iterator for use by i3cdev.

Signed-off-by: Vitor Soares <vitor.soares@toradex.com>
Signed-off-by: Sam Agazaryan <samagazaryan@google.com>
---
 drivers/i3c/internals.h |  2 ++
 drivers/i3c/master.c    | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
index cbc00c862104..3c53a5108b81 100644
--- a/drivers/i3c/internals.h
+++ b/drivers/i3c/internals.h
@@ -79,4 +79,6 @@ static inline struct i3c_master_controller *i3c_bus_to_i3c_master(struct i3c_bus
 	return container_of(i3cbus, struct i3c_master_controller, bus);
 }
 
+int i3c_for_each_dev(void *data, int (*fn)(struct device *, void *));
+
 #endif /* I3C_INTERNAL_H */
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index e8a9f8c3041e..dd5071ca9195 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -4011,6 +4011,18 @@ void i3c_dev_free_ibi_locked(struct i3c_dev_desc *dev)
 }
 EXPORT_SYMBOL_GPL(i3c_dev_free_ibi_locked);
 
+int i3c_for_each_dev(void *data, int (*fn)(struct device *, void *))
+{
+	int res;
+
+	mutex_lock(&i3c_core_lock);
+	res = bus_for_each_dev(&i3c_bus_type, NULL, data, fn);
+	mutex_unlock(&i3c_core_lock);
+
+	return res;
+}
+EXPORT_SYMBOL_GPL(i3c_for_each_dev);
+
 static int __init i3c_init(void)
 {
 	int res;
-- 
2.55.0.979.g7e5102b832-goog

[PATCH v4 3/3] i3c: add i3cdev module to expose i3c dev in /dev

[Sam Agazaryan]

From: Vitor Soares <vitor.soares@toradex.com>

This patch adds userspace character device support for I3C SDR private
transfers via /dev.

The module allows userspace programs to interact directly with I3C
targets that do not have a kernel driver bound to them, such as devices
in ROM/bootloader recovery mode (e.g. OCP Secure Firmware Recovery v1.1
and Caliptra Silicon Root of Trust recovery flows).

Features:
  - Dynamically exposes /dev/bus/i3c/<device> character devices for I3C
    devices when unbound from kernel drivers.
  - Dynamically allocates character device minor numbers using the IDA
    allocator.
  - Implements private SDR read/write transfers via I3C_IOC_PRIV_XFER ioctl
    with 64-bit aligned UAPI data structures.
  - Supports compat_ptr_ioctl for 32-bit userspace on 64-bit kernels.
  - Uses cdev_device_add/cdev_device_del with device refcounting to ensure
    safe lifecycle management and prevent use-after-free on driver detach.

Signed-off-by: Vitor Soares <vitor.soares@toradex.com>
Co-developed-by: Oleksandr Shulzhenko <oleksandr.shulzhenko.viktorovych@intel.com>
Signed-off-by: Oleksandr Shulzhenko <oleksandr.shulzhenko.viktorovych@intel.com>
Co-developed-by: Sam Agazaryan <samagazaryan@google.com>
Signed-off-by: Sam Agazaryan <samagazaryan@google.com>
---
 MAINTAINERS                     |   1 +
 drivers/i3c/Kconfig             |  11 +
 drivers/i3c/Makefile            |   1 +
 drivers/i3c/i3cdev.c            | 445 ++++++++++++++++++++++++++++++++
 include/uapi/linux/i3c/i3cdev.h |  37 +++
 5 files changed, 495 insertions(+)
 create mode 100644 drivers/i3c/i3cdev.c
 create mode 100644 include/uapi/linux/i3c/i3cdev.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 81a9a02c919d..30a5cb12c4f0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12364,6 +12364,7 @@ F:	Documentation/driver-api/i3c
 F:	drivers/i3c/
 F:	include/dt-bindings/i3c/
 F:	include/linux/i3c/
+F:	include/uapi/linux/i3c/
 
 IBM Operation Panel Input Driver
 M:	Eddie James <eajames@linux.ibm.com>
diff --git a/drivers/i3c/Kconfig b/drivers/i3c/Kconfig
index 626c54b386d5..166875837ec6 100644
--- a/drivers/i3c/Kconfig
+++ b/drivers/i3c/Kconfig
@@ -20,6 +20,17 @@ menuconfig I3C
 	  will be called i3c.
 
 if I3C
+
+config I3CDEV
+	tristate "I3C device interface"
+	help
+	  Say Y here to use i3c-* device files, usually found in the /dev
+	  directory on your system.  They make it possible to have user-space
+	  programs use the I3C devices.
+
+	  This support is also available as a module.  If so, the module
+	  will be called i3cdev.
+
 source "drivers/i3c/master/Kconfig"
 endif # I3C
 
diff --git a/drivers/i3c/Makefile b/drivers/i3c/Makefile
index 11982efbc6d9..606d422841b2 100644
--- a/drivers/i3c/Makefile
+++ b/drivers/i3c/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 i3c-y				:= device.o master.o
 obj-$(CONFIG_I3C)		+= i3c.o
+obj-$(CONFIG_I3CDEV)		+= i3cdev.o
 obj-$(CONFIG_I3C)		+= master/
diff --git a/drivers/i3c/i3cdev.c b/drivers/i3c/i3cdev.c
new file mode 100644
index 000000000000..904ebfd48769
--- /dev/null
+++ b/drivers/i3c/i3cdev.c
@@ -0,0 +1,445 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 Synopsys, Inc. and/or its affiliates.
+ *
+ * Author: Vitor Soares <soares@synopsys.com>
+ */
+
+#include <linux/cdev.h>
+#include <linux/compat.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+
+#include <linux/i3c/i3cdev.h>
+
+#include "internals.h"
+
+struct i3cdev_data {
+	struct i3c_device *i3c;
+	struct device dev;
+	struct mutex xfer_lock; /* prevent detach while transferring */
+	struct cdev cdev;
+	int id;
+};
+
+static DEFINE_IDA(i3cdev_ida);
+static dev_t i3cdev_number;
+#define I3C_MINORS (MINORMASK + 1)
+
+static void i3cdev_dev_release(struct device *dev)
+{
+	struct i3cdev_data *i3cdev = container_of(dev, struct i3cdev_data, dev);
+
+	ida_free(&i3cdev_ida, i3cdev->id);
+	kfree(i3cdev);
+}
+
+static struct i3cdev_data *get_free_i3cdev(struct i3c_device *i3c)
+{
+	struct i3cdev_data *i3cdev;
+	int id;
+
+	id = ida_alloc(&i3cdev_ida, GFP_KERNEL);
+	if (id < 0) {
+		pr_err("i3cdev: no minor number available!\n");
+		return ERR_PTR(id);
+	}
+
+	i3cdev = kzalloc(sizeof(*i3cdev), GFP_KERNEL);
+	if (!i3cdev) {
+		ida_free(&i3cdev_ida, id);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	i3cdev->i3c = i3c;
+	i3cdev->id = id;
+	i3cdev_set_drvdata(i3c, i3cdev);
+
+	return i3cdev;
+}
+
+static ssize_t
+i3cdev_read(struct file *file, char __user *buf, size_t count, loff_t *f_pos)
+{
+	struct i3cdev_data *i3cdev = file->private_data;
+	struct i3c_device *i3c;
+	struct i3c_xfer xfers = {
+		.rnw = true,
+		.len = count,
+	};
+	int ret = -ENODEV;
+	char *tmp;
+
+	mutex_lock(&i3cdev->xfer_lock);
+	i3c = i3cdev->i3c;
+	if (!i3c || i3c->dev.driver)
+		goto err_out;
+
+	tmp = kzalloc(count, GFP_KERNEL);
+	if (!tmp) {
+		ret = -ENOMEM;
+		goto err_out;
+	}
+
+	xfers.data.in = tmp;
+
+	dev_dbg(&i3c->dev, "Reading %zu bytes.\n", count);
+
+	ret = i3c_device_do_xfers(i3c, &xfers, 1, I3C_SDR);
+	if (!ret)
+		ret = copy_to_user(buf, tmp, xfers.len) ? -EFAULT : xfers.len;
+
+	kfree(tmp);
+
+err_out:
+	mutex_unlock(&i3cdev->xfer_lock);
+	return ret;
+}
+
+static ssize_t
+i3cdev_write(struct file *file, const char __user *buf, size_t count,
+	     loff_t *f_pos)
+{
+	struct i3cdev_data *i3cdev = file->private_data;
+	struct i3c_device *i3c;
+	struct i3c_xfer xfers = {
+		.rnw = false,
+		.len = count,
+	};
+	int ret = -ENODEV;
+	char *tmp;
+
+	mutex_lock(&i3cdev->xfer_lock);
+	i3c = i3cdev->i3c;
+	if (!i3c || i3c->dev.driver)
+		goto err_out;
+
+	tmp = memdup_user(buf, count);
+	if (IS_ERR(tmp)) {
+		ret = PTR_ERR(tmp);
+		goto err_out;
+	}
+
+	xfers.data.out = tmp;
+
+	dev_dbg(&i3c->dev, "Writing %zu bytes.\n", count);
+
+	ret = i3c_device_do_xfers(i3c, &xfers, 1, I3C_SDR);
+	kfree(tmp);
+
+err_out:
+	mutex_unlock(&i3cdev->xfer_lock);
+	return (!ret) ? count : ret;
+}
+
+static int
+i3cdev_do_priv_xfer(struct i3c_device *dev, struct i3c_ioc_priv_xfer *xfers,
+		    unsigned int nxfers)
+{
+	struct i3c_xfer *k_xfers;
+	u8 **data_ptrs;
+	int i, j, ret = 0;
+
+	/* Since we have nxfers we may allocate k_xfer + *data_ptrs together */
+	k_xfers = kcalloc(nxfers, sizeof(*k_xfers) + sizeof(*data_ptrs),
+			  GFP_KERNEL);
+	if (!k_xfers)
+		return -ENOMEM;
+
+	/* set data_ptrs to be after nxfers * i3c_xfer */
+	data_ptrs = (void *)k_xfers + (nxfers * sizeof(*k_xfers));
+
+	for (i = 0; i < nxfers; i++) {
+		if (xfers[i].rnw) {
+			data_ptrs[i] = kzalloc(xfers[i].len, GFP_KERNEL);
+			if (!data_ptrs[i]) {
+				ret = -ENOMEM;
+				break;
+			}
+			k_xfers[i].rnw = true;
+			k_xfers[i].data.in = data_ptrs[i];
+		} else {
+			data_ptrs[i] = memdup_user(u64_to_user_ptr(xfers[i].data),
+						   xfers[i].len);
+			if (IS_ERR(data_ptrs[i])) {
+				ret = PTR_ERR(data_ptrs[i]);
+				break;
+			}
+			k_xfers[i].rnw = false;
+			k_xfers[i].data.out = data_ptrs[i];
+		}
+
+		k_xfers[i].len = xfers[i].len;
+	}
+
+	if (ret < 0)
+		goto err_free_mem;
+
+	ret = i3c_device_do_xfers(dev, k_xfers, nxfers, I3C_SDR);
+	if (ret)
+		goto err_free_mem;
+
+	for (i = 0; i < nxfers; i++) {
+		if (xfers[i].rnw) {
+			if (copy_to_user(u64_to_user_ptr(xfers[i].data),
+					 data_ptrs[i], xfers[i].len))
+				ret = -EFAULT;
+		}
+	}
+
+err_free_mem:
+	for (j = 0; j < i; j++)
+		kfree(data_ptrs[j]);
+	kfree(k_xfers);
+	return ret;
+}
+
+static struct i3c_ioc_priv_xfer *
+i3cdev_get_ioc_priv_xfer(unsigned int cmd, struct i3c_ioc_priv_xfer *u_xfers,
+			 unsigned int *nxfers)
+{
+	u32 tmp = _IOC_SIZE(cmd);
+
+	if ((tmp % sizeof(struct i3c_ioc_priv_xfer)) != 0)
+		return ERR_PTR(-EINVAL);
+
+	*nxfers = tmp / sizeof(struct i3c_ioc_priv_xfer);
+	if (*nxfers == 0)
+		return ERR_PTR(-EINVAL);
+
+	return memdup_user(u_xfers, tmp);
+}
+
+static int
+i3cdev_ioc_priv_xfer(struct i3c_device *i3c, unsigned int cmd,
+		     struct i3c_ioc_priv_xfer *u_xfers)
+{
+	struct i3c_ioc_priv_xfer *k_xfers;
+	unsigned int nxfers;
+	int ret;
+
+	k_xfers = i3cdev_get_ioc_priv_xfer(cmd, u_xfers, &nxfers);
+	if (IS_ERR(k_xfers))
+		return PTR_ERR(k_xfers);
+
+	ret = i3cdev_do_priv_xfer(i3c, k_xfers, nxfers);
+
+	kfree(k_xfers);
+
+	return ret;
+}
+
+static long
+i3cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	struct i3cdev_data *i3cdev = file->private_data;
+	struct i3c_device *i3c;
+	int ret = -ENODEV;
+
+	if (_IOC_TYPE(cmd) != I3C_DEV_IOC_MAGIC)
+		return -ENOTTY;
+
+	/* Use the xfer_lock to prevent device detach during ioctl call */
+	mutex_lock(&i3cdev->xfer_lock);
+	i3c = i3cdev->i3c;
+	if (!i3c || i3c->dev.driver)
+		goto err_no_dev;
+
+	dev_dbg(&i3c->dev, "ioctl, cmd=0x%02x, arg=0x%02lx\n", cmd, arg);
+
+	/* Check command number and direction */
+	if (_IOC_NR(cmd) == _IOC_NR(I3C_IOC_PRIV_XFER(0)) &&
+	    _IOC_DIR(cmd) == (_IOC_READ | _IOC_WRITE))
+		ret = i3cdev_ioc_priv_xfer(i3c, cmd,
+					(struct i3c_ioc_priv_xfer __user *)arg);
+	else
+		ret = -ENOTTY;
+
+err_no_dev:
+	mutex_unlock(&i3cdev->xfer_lock);
+	return ret;
+}
+
+static int i3cdev_open(struct inode *inode, struct file *file)
+{
+	struct i3cdev_data *i3cdev = container_of(inode->i_cdev,
+						  struct i3cdev_data,
+						  cdev);
+	file->private_data = i3cdev;
+
+	return 0;
+}
+
+static int i3cdev_release(struct inode *inode, struct file *file)
+{
+	file->private_data = NULL;
+
+	return 0;
+}
+
+static const struct file_operations i3cdev_fops = {
+	.owner		= THIS_MODULE,
+	.read		= i3cdev_read,
+	.write		= i3cdev_write,
+	.unlocked_ioctl	= i3cdev_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
+	.open		= i3cdev_open,
+	.release	= i3cdev_release,
+};
+
+/* ------------------------------------------------------------------------- */
+
+static const struct class i3cdev_class = {
+	.name = "i3cdev",
+};
+
+static int i3cdev_attach(struct device *dev, void *dummy)
+{
+	struct i3cdev_data *i3cdev;
+	struct i3c_device *i3c;
+	int res;
+
+	if (dev->type == &i3c_masterdev_type || dev->driver)
+		return 0;
+
+	i3c = dev_to_i3cdev(dev);
+
+	/* Get a device */
+	i3cdev = get_free_i3cdev(i3c);
+	if (IS_ERR(i3cdev))
+		return PTR_ERR(i3cdev);
+
+	mutex_init(&i3cdev->xfer_lock);
+	cdev_init(&i3cdev->cdev, &i3cdev_fops);
+	i3cdev->cdev.owner = THIS_MODULE;
+
+	device_initialize(&i3cdev->dev);
+	i3cdev->dev.devt = MKDEV(MAJOR(i3cdev_number), i3cdev->id);
+	i3cdev->dev.class = &i3cdev_class;
+	i3cdev->dev.parent = &i3c->dev;
+	i3cdev->dev.release = i3cdev_dev_release;
+
+	res = dev_set_name(&i3cdev->dev, "bus!i3c!%s", dev_name(&i3c->dev));
+	if (res)
+		goto error_put_dev;
+
+	res = cdev_device_add(&i3cdev->cdev, &i3cdev->dev);
+	if (res)
+		goto error_put_dev;
+
+	pr_debug("i3cdev: I3C device [%s] registered as minor %d\n",
+		 dev_name(&i3c->dev), i3cdev->id);
+	return 0;
+
+error_put_dev:
+	i3cdev_set_drvdata(i3c, NULL);
+	put_device(&i3cdev->dev);
+	return res;
+}
+
+static int i3cdev_detach(struct device *dev, void *dummy)
+{
+	struct i3cdev_data *i3cdev;
+	struct i3c_device *i3c;
+
+	if (dev->type == &i3c_masterdev_type)
+		return 0;
+
+	i3c = dev_to_i3cdev(dev);
+
+	i3cdev = i3cdev_get_drvdata(i3c);
+	if (!i3cdev)
+		return 0;
+
+	i3cdev_set_drvdata(i3c, NULL);
+
+	/* Prevent transfers while cdev removal */
+	mutex_lock(&i3cdev->xfer_lock);
+	i3cdev->i3c = NULL;
+	mutex_unlock(&i3cdev->xfer_lock);
+
+	cdev_device_del(&i3cdev->cdev, &i3cdev->dev);
+	put_device(&i3cdev->dev);
+
+	pr_debug("i3cdev: device [%s] unregistered\n", dev_name(&i3c->dev));
+
+	return 0;
+}
+
+static int i3cdev_notifier_call(struct notifier_block *nb,
+				unsigned long action,
+				void *data)
+{
+	struct device *dev = data;
+
+	switch (action) {
+	case BUS_NOTIFY_ADD_DEVICE:
+	case BUS_NOTIFY_UNBOUND_DRIVER:
+		return i3cdev_attach(dev, NULL);
+	case BUS_NOTIFY_DEL_DEVICE:
+	case BUS_NOTIFY_REMOVED_DEVICE:
+	case BUS_NOTIFY_BIND_DRIVER:
+		return i3cdev_detach(dev, NULL);
+	}
+
+	return 0;
+}
+
+static struct notifier_block i3cdev_notifier = {
+	.notifier_call = i3cdev_notifier_call,
+};
+
+static int __init i3cdev_init(void)
+{
+	int res;
+
+	/* Dynamically request unused major number */
+	res = alloc_chrdev_region(&i3cdev_number, 0, I3C_MINORS, "i3c");
+	if (res)
+		goto out;
+
+	/* Register device class to populate sysfs entries */
+	res = class_register(&i3cdev_class);
+	if (res)
+		goto out_unreg_chrdev;
+
+	/* Keep track of busses which have devices to add or remove later */
+	res = bus_register_notifier(&i3c_bus_type, &i3cdev_notifier);
+	if (res)
+		goto out_unreg_class;
+
+	/* Bind to already existing device without driver right away */
+	i3c_for_each_dev(NULL, i3cdev_attach);
+
+	return 0;
+
+out_unreg_class:
+	class_unregister(&i3cdev_class);
+out_unreg_chrdev:
+	unregister_chrdev_region(i3cdev_number, I3C_MINORS);
+out:
+	pr_err("%s: Driver Initialisation failed\n", __FILE__);
+	return res;
+}
+
+static void __exit i3cdev_exit(void)
+{
+	bus_unregister_notifier(&i3c_bus_type, &i3cdev_notifier);
+	i3c_for_each_dev(NULL, i3cdev_detach);
+	class_unregister(&i3cdev_class);
+	unregister_chrdev_region(i3cdev_number, I3C_MINORS);
+}
+
+MODULE_AUTHOR("Vitor Soares <soares@synopsys.com>");
+MODULE_DESCRIPTION("I3C /dev entries driver");
+MODULE_LICENSE("GPL");
+
+module_init(i3cdev_init);
+module_exit(i3cdev_exit);
diff --git a/include/uapi/linux/i3c/i3cdev.h b/include/uapi/linux/i3c/i3cdev.h
new file mode 100644
index 000000000000..5adc1e3e7c4f
--- /dev/null
+++ b/include/uapi/linux/i3c/i3cdev.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (c) 2020 Synopsys, Inc. and/or its affiliates.
+ *
+ * Author: Vitor Soares <vitor.soares@synopsys.com>
+ */
+
+#ifndef _UAPI_I3C_DEV_H_
+#define _UAPI_I3C_DEV_H_
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+/* IOCTL commands */
+#define I3C_DEV_IOC_MAGIC	0x07
+
+/**
+ * struct i3c_ioc_priv_xfer - I3C SDR ioctl private transfer
+ * @data: Holds pointer to userspace buffer with transmit data.
+ * @len: Length of data buffer buffers, in bytes.
+ * @rnw: encodes the transfer direction. true for a read, false for a write
+ */
+struct i3c_ioc_priv_xfer {
+	__u64 data;
+	__u16 len;
+	__u8 rnw;
+	__u8 pad[5];
+};
+
+#define I3C_PRIV_XFER_SIZE(N)	\
+	((((sizeof(struct i3c_ioc_priv_xfer)) * (N)) < (1 << _IOC_SIZEBITS)) \
+	? ((sizeof(struct i3c_ioc_priv_xfer)) * (N)) : 0)
+
+#define I3C_IOC_PRIV_XFER(N)	\
+	_IOC(_IOC_READ|_IOC_WRITE, I3C_DEV_IOC_MAGIC, 30, I3C_PRIV_XFER_SIZE(N))
+
+#endif
-- 
2.55.0.979.g7e5102b832-goog

Re: [PATCH v4 3/3] i3c: add i3cdev module to expose i3c dev in /dev

[Greg Kroah-Hartman]

On Sun, Sep 06, 2026 at 08:27:47PM +0000, Sam Agazaryan wrote:
> +/**
> + * struct i3c_ioc_priv_xfer - I3C SDR ioctl private transfer
> + * @data: Holds pointer to userspace buffer with transmit data.
> + * @len: Length of data buffer buffers, in bytes.
> + * @rnw: encodes the transfer direction. true for a read, false for a write

You do not describe the "pad" variable.

And why not call "rnw" "direction"?

> + */
> +struct i3c_ioc_priv_xfer {
> +	__u64 data;
> +	__u16 len;
> +	__u8 rnw;
> +	__u8 pad[5];

You do not enforce that pad[5] MUST be set to all 0.

thanks,

greg k-h

Re: [PATCH v4 3/3] i3c: add i3cdev module to expose i3c dev in /dev

[Sam Agazaryan]

On Mon, Sep 7, 2026 at 7:42 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sun, Sep 06, 2026 at 08:27:47PM +0000, Sam Agazaryan wrote:
> > +/**
> > + * struct i3c_ioc_priv_xfer - I3C SDR ioctl private transfer
> > + * @data: Holds pointer to userspace buffer with transmit data.
> > + * @len: Length of data buffer buffers, in bytes.
> > + * @rnw: encodes the transfer direction. true for a read, false for a write
>
> You do not describe the "pad" variable.

I'll add a description for "pad".

> And why not call "rnw" "direction"?

I'll update "rnw" to "direction" since this is exposed to userland.

> > + */
> > +struct i3c_ioc_priv_xfer {
> > +     __u64 data;
> > +     __u16 len;
> > +     __u8 rnw;
> > +     __u8 pad[5];
>
> You do not enforce that pad[5] MUST be set to all 0.

I'll add a zero check against the padding to address this.

The next revision of the patchset will include these updates.

Thanks,
Sam

Re: [PATCH v4 3/3] i3c: add i3cdev module to expose i3c dev in /dev

[Frank Li]

On Sun, Sep 06, 2026 at 08:27:47PM +0000, Sam Agazaryan wrote:
> From: Vitor Soares <vitor.soares@toradex.com>
>
> This patch adds userspace character device support for I3C SDR private
> transfers via /dev.

Add userspace character device ...

Now frame already support HDR.

>
> The module allows userspace programs to interact directly with I3C
> targets that do not have a kernel driver bound to them, such as devices
> in ROM/bootloader recovery mode (e.g. OCP Secure Firmware Recovery v1.1
> and Caliptra Silicon Root of Trust recovery flows).
>
> Features:
>   - Dynamically exposes /dev/bus/i3c/<device> character devices for I3C
>     devices when unbound from kernel drivers.
>   - Dynamically allocates character device minor numbers using the IDA
>     allocator.
>   - Implements private SDR read/write transfers via I3C_IOC_PRIV_XFER ioctl
>     with 64-bit aligned UAPI data structures.

Consider PRIV for SDR, need unified HDR also,

I3C_IOC_XFER

>   - Supports compat_ptr_ioctl for 32-bit userspace on 64-bit kernels.
>   - Uses cdev_device_add/cdev_device_del with device refcounting to ensure
>     safe lifecycle management and prevent use-after-free on driver detach.
>
> Signed-off-by: Vitor Soares <vitor.soares@toradex.com>
> Co-developed-by: Oleksandr Shulzhenko <oleksandr.shulzhenko.viktorovych@intel.com>
> Signed-off-by: Oleksandr Shulzhenko <oleksandr.shulzhenko.viktorovych@intel.com>
> Co-developed-by: Sam Agazaryan <samagazaryan@google.com>
> Signed-off-by: Sam Agazaryan <samagazaryan@google.com>
> ---
>  MAINTAINERS                     |   1 +
>  drivers/i3c/Kconfig             |  11 +
>  drivers/i3c/Makefile            |   1 +
>  drivers/i3c/i3cdev.c            | 445 ++++++++++++++++++++++++++++++++
>  include/uapi/linux/i3c/i3cdev.h |  37 +++
>  5 files changed, 495 insertions(+)
>  create mode 100644 drivers/i3c/i3cdev.c
>  create mode 100644 include/uapi/linux/i3c/i3cdev.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 81a9a02c919d..30a5cb12c4f0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12364,6 +12364,7 @@ F:	Documentation/driver-api/i3c
>  F:	drivers/i3c/
>  F:	include/dt-bindings/i3c/
>  F:	include/linux/i3c/
> +F:	include/uapi/linux/i3c/
>
>  IBM Operation Panel Input Driver
>  M:	Eddie James <eajames@linux.ibm.com>
> diff --git a/drivers/i3c/Kconfig b/drivers/i3c/Kconfig
> index 626c54b386d5..166875837ec6 100644
> --- a/drivers/i3c/Kconfig
> +++ b/drivers/i3c/Kconfig
> @@ -20,6 +20,17 @@ menuconfig I3C
>  	  will be called i3c.
>
>  if I3C
> +
> +config I3CDEV
> +	tristate "I3C device interface"
> +	help
> +	  Say Y here to use i3c-* device files, usually found in the /dev
> +	  directory on your system.  They make it possible to have user-space
> +	  programs use the I3C devices.
> +
> +	  This support is also available as a module.  If so, the module
> +	  will be called i3cdev.
> +
>  source "drivers/i3c/master/Kconfig"
>  endif # I3C
>
> diff --git a/drivers/i3c/Makefile b/drivers/i3c/Makefile
> index 11982efbc6d9..606d422841b2 100644
> --- a/drivers/i3c/Makefile
> +++ b/drivers/i3c/Makefile
> @@ -1,4 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
>  i3c-y				:= device.o master.o
>  obj-$(CONFIG_I3C)		+= i3c.o
> +obj-$(CONFIG_I3CDEV)		+= i3cdev.o
>  obj-$(CONFIG_I3C)		+= master/
> diff --git a/drivers/i3c/i3cdev.c b/drivers/i3c/i3cdev.c
> new file mode 100644
> index 000000000000..904ebfd48769
> --- /dev/null
> +++ b/drivers/i3c/i3cdev.c
> @@ -0,0 +1,445 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2020 Synopsys, Inc. and/or its affiliates.

new post, should be 2026

> + *
> + * Author: Vitor Soares <soares@synopsys.com>
> + */
> +
> +#include <linux/cdev.h>
> +#include <linux/compat.h>
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/jiffies.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +
> +#include <linux/i3c/i3cdev.h>
> +
> +#include "internals.h"
> +
> +struct i3cdev_data {
> +	struct i3c_device *i3c;
> +	struct device dev;
> +	struct mutex xfer_lock; /* prevent detach while transferring */
> +	struct cdev cdev;
> +	int id;
> +};
> +
> +static DEFINE_IDA(i3cdev_ida);
> +static dev_t i3cdev_number;
> +#define I3C_MINORS (MINORMASK + 1)
> +
> +static void i3cdev_dev_release(struct device *dev)
> +{
> +	struct i3cdev_data *i3cdev = container_of(dev, struct i3cdev_data, dev);
> +
> +	ida_free(&i3cdev_ida, i3cdev->id);
> +	kfree(i3cdev);
> +}
> +
> +static struct i3cdev_data *get_free_i3cdev(struct i3c_device *i3c)
> +{
> +	struct i3cdev_data *i3cdev;
> +	int id;
> +
> +	id = ida_alloc(&i3cdev_ida, GFP_KERNEL);
> +	if (id < 0) {
> +		pr_err("i3cdev: no minor number available!\n");
> +		return ERR_PTR(id);
> +	}
> +
> +	i3cdev = kzalloc(sizeof(*i3cdev), GFP_KERNEL);

kzalloc_obj()

> +	if (!i3cdev) {
> +		ida_free(&i3cdev_ida, id);
> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	i3cdev->i3c = i3c;
> +	i3cdev->id = id;
> +	i3cdev_set_drvdata(i3c, i3cdev);
> +
> +	return i3cdev;
> +}
> +
> +static ssize_t
> +i3cdev_read(struct file *file, char __user *buf, size_t count, loff_t *f_pos)
> +{
> +	struct i3cdev_data *i3cdev = file->private_data;
> +	struct i3c_device *i3c;
> +	struct i3c_xfer xfers = {
> +		.rnw = true,
> +		.len = count,
> +	};
> +	int ret = -ENODEV;
> +	char *tmp;
> +
> +	mutex_lock(&i3cdev->xfer_lock);
> +	i3c = i3cdev->i3c;
> +	if (!i3c || i3c->dev.driver)
> +		goto err_out;
> +
> +	tmp = kzalloc(count, GFP_KERNEL);
> +	if (!tmp) {
> +		ret = -ENOMEM;
> +		goto err_out;
> +	}
> +
> +	xfers.data.in = tmp;
> +
> +	dev_dbg(&i3c->dev, "Reading %zu bytes.\n", count);
> +
> +	ret = i3c_device_do_xfers(i3c, &xfers, 1, I3C_SDR);

why force to SDR here.

> +	if (!ret)
> +		ret = copy_to_user(buf, tmp, xfers.len) ? -EFAULT : xfers.len;
> +
> +	kfree(tmp);
> +
> +err_out:
> +	mutex_unlock(&i3cdev->xfer_lock);
> +	return ret;
> +}
> +
> +static ssize_t
> +i3cdev_write(struct file *file, const char __user *buf, size_t count,
> +	     loff_t *f_pos)
> +{
> +	struct i3cdev_data *i3cdev = file->private_data;
> +	struct i3c_device *i3c;
> +	struct i3c_xfer xfers = {
> +		.rnw = false,
> +		.len = count,
> +	};
> +	int ret = -ENODEV;
> +	char *tmp;
> +
> +	mutex_lock(&i3cdev->xfer_lock);
> +	i3c = i3cdev->i3c;
> +	if (!i3c || i3c->dev.driver)
> +		goto err_out;
> +
> +	tmp = memdup_user(buf, count);
> +	if (IS_ERR(tmp)) {
> +		ret = PTR_ERR(tmp);
> +		goto err_out;
> +	}
> +
> +	xfers.data.out = tmp;
> +
> +	dev_dbg(&i3c->dev, "Writing %zu bytes.\n", count);
> +
> +	ret = i3c_device_do_xfers(i3c, &xfers, 1, I3C_SDR);
> +	kfree(tmp);
> +
> +err_out:
> +	mutex_unlock(&i3cdev->xfer_lock);

use cleanup guard()

> +	return (!ret) ? count : ret;
> +}
> +
> +static int
> +i3cdev_do_priv_xfer(struct i3c_device *dev, struct i3c_ioc_priv_xfer *xfers,
> +		    unsigned int nxfers)
> +{
> +	struct i3c_xfer *k_xfers;
> +	u8 **data_ptrs;
> +	int i, j, ret = 0;
> +
> +	/* Since we have nxfers we may allocate k_xfer + *data_ptrs together */
> +	k_xfers = kcalloc(nxfers, sizeof(*k_xfers) + sizeof(*data_ptrs),
> +			  GFP_KERNEL);
> +	if (!k_xfers)
> +		return -ENOMEM;
> +
> +	/* set data_ptrs to be after nxfers * i3c_xfer */
> +	data_ptrs = (void *)k_xfers + (nxfers * sizeof(*k_xfers));
> +
> +	for (i = 0; i < nxfers; i++) {
> +		if (xfers[i].rnw) {
> +			data_ptrs[i] = kzalloc(xfers[i].len, GFP_KERNEL);
> +			if (!data_ptrs[i]) {
> +				ret = -ENOMEM;
> +				break;
> +			}
> +			k_xfers[i].rnw = true;
> +			k_xfers[i].data.in = data_ptrs[i];
> +		} else {
> +			data_ptrs[i] = memdup_user(u64_to_user_ptr(xfers[i].data),
> +						   xfers[i].len);
> +			if (IS_ERR(data_ptrs[i])) {
> +				ret = PTR_ERR(data_ptrs[i]);
> +				break;
> +			}
> +			k_xfers[i].rnw = false;
> +			k_xfers[i].data.out = data_ptrs[i];
> +		}
> +
> +		k_xfers[i].len = xfers[i].len;
> +	}
> +
> +	if (ret < 0)
> +		goto err_free_mem;
> +
> +	ret = i3c_device_do_xfers(dev, k_xfers, nxfers, I3C_SDR);
> +	if (ret)
> +		goto err_free_mem;
> +
> +	for (i = 0; i < nxfers; i++) {
> +		if (xfers[i].rnw) {
> +			if (copy_to_user(u64_to_user_ptr(xfers[i].data),
> +					 data_ptrs[i], xfers[i].len))

Use actual_len,

> +				ret = -EFAULT;
> +		}
> +	}
> +
> +err_free_mem:
> +	for (j = 0; j < i; j++)
> +		kfree(data_ptrs[j]);
> +	kfree(k_xfers);


use clean up __free(kfree) when alloc.

> +	return ret;
> +}
> +
> +static struct i3c_ioc_priv_xfer *
> +i3cdev_get_ioc_priv_xfer(unsigned int cmd, struct i3c_ioc_priv_xfer *u_xfers,
> +			 unsigned int *nxfers)
> +{
> +	u32 tmp = _IOC_SIZE(cmd);
> +
> +	if ((tmp % sizeof(struct i3c_ioc_priv_xfer)) != 0)
> +		return ERR_PTR(-EINVAL);
> +
> +	*nxfers = tmp / sizeof(struct i3c_ioc_priv_xfer);
> +	if (*nxfers == 0)
> +		return ERR_PTR(-EINVAL);
> +
> +	return memdup_user(u_xfers, tmp);
> +}
> +
> +static int
> +i3cdev_ioc_priv_xfer(struct i3c_device *i3c, unsigned int cmd,
> +		     struct i3c_ioc_priv_xfer *u_xfers)
> +{
> +	struct i3c_ioc_priv_xfer *k_xfers;
> +	unsigned int nxfers;
> +	int ret;
> +
> +	k_xfers = i3cdev_get_ioc_priv_xfer(cmd, u_xfers, &nxfers);
> +	if (IS_ERR(k_xfers))
> +		return PTR_ERR(k_xfers);
> +
> +	ret = i3cdev_do_priv_xfer(i3c, k_xfers, nxfers);
> +
> +	kfree(k_xfers);
> +
> +	return ret;
> +}
> +
> +static long
> +i3cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	struct i3cdev_data *i3cdev = file->private_data;
> +	struct i3c_device *i3c;
> +	int ret = -ENODEV;
> +
> +	if (_IOC_TYPE(cmd) != I3C_DEV_IOC_MAGIC)
> +		return -ENOTTY;
> +
> +	/* Use the xfer_lock to prevent device detach during ioctl call */
> +	mutex_lock(&i3cdev->xfer_lock);
> +	i3c = i3cdev->i3c;
> +	if (!i3c || i3c->dev.driver)
> +		goto err_no_dev;
> +
> +	dev_dbg(&i3c->dev, "ioctl, cmd=0x%02x, arg=0x%02lx\n", cmd, arg);
> +
> +	/* Check command number and direction */
> +	if (_IOC_NR(cmd) == _IOC_NR(I3C_IOC_PRIV_XFER(0)) &&
> +	    _IOC_DIR(cmd) == (_IOC_READ | _IOC_WRITE))
> +		ret = i3cdev_ioc_priv_xfer(i3c, cmd,
> +					(struct i3c_ioc_priv_xfer __user *)arg);
> +	else
> +		ret = -ENOTTY;
> +
> +err_no_dev:
> +	mutex_unlock(&i3cdev->xfer_lock);
> +	return ret;
> +}
> +
> +static int i3cdev_open(struct inode *inode, struct file *file)
> +{
> +	struct i3cdev_data *i3cdev = container_of(inode->i_cdev,
> +						  struct i3cdev_data,
> +						  cdev);
> +	file->private_data = i3cdev;
> +
> +	return 0;
> +}
> +
> +static int i3cdev_release(struct inode *inode, struct file *file)
> +{
> +	file->private_data = NULL;
> +
> +	return 0;
> +}
> +
> +static const struct file_operations i3cdev_fops = {
> +	.owner		= THIS_MODULE,
> +	.read		= i3cdev_read,
> +	.write		= i3cdev_write,
> +	.unlocked_ioctl	= i3cdev_ioctl,
> +	.compat_ioctl	= compat_ptr_ioctl,
> +	.open		= i3cdev_open,
> +	.release	= i3cdev_release,
> +};
> +
> +/* ------------------------------------------------------------------------- */
> +
> +static const struct class i3cdev_class = {
> +	.name = "i3cdev",
> +};
> +
> +static int i3cdev_attach(struct device *dev, void *dummy)
> +{
> +	struct i3cdev_data *i3cdev;
> +	struct i3c_device *i3c;
> +	int res;
> +
> +	if (dev->type == &i3c_masterdev_type || dev->driver)
> +		return 0;
> +
> +	i3c = dev_to_i3cdev(dev);
> +
> +	/* Get a device */
> +	i3cdev = get_free_i3cdev(i3c);
> +	if (IS_ERR(i3cdev))
> +		return PTR_ERR(i3cdev);
> +
> +	mutex_init(&i3cdev->xfer_lock);
> +	cdev_init(&i3cdev->cdev, &i3cdev_fops);
> +	i3cdev->cdev.owner = THIS_MODULE;
> +
> +	device_initialize(&i3cdev->dev);
> +	i3cdev->dev.devt = MKDEV(MAJOR(i3cdev_number), i3cdev->id);
> +	i3cdev->dev.class = &i3cdev_class;
> +	i3cdev->dev.parent = &i3c->dev;
> +	i3cdev->dev.release = i3cdev_dev_release;
> +
> +	res = dev_set_name(&i3cdev->dev, "bus!i3c!%s", dev_name(&i3c->dev));
> +	if (res)
> +		goto error_put_dev;
> +
> +	res = cdev_device_add(&i3cdev->cdev, &i3cdev->dev);
> +	if (res)
> +		goto error_put_dev;
> +
> +	pr_debug("i3cdev: I3C device [%s] registered as minor %d\n",
> +		 dev_name(&i3c->dev), i3cdev->id);
> +	return 0;
> +
> +error_put_dev:
> +	i3cdev_set_drvdata(i3c, NULL);
> +	put_device(&i3cdev->dev);
> +	return res;
> +}
> +
> +static int i3cdev_detach(struct device *dev, void *dummy)
> +{
> +	struct i3cdev_data *i3cdev;
> +	struct i3c_device *i3c;
> +
> +	if (dev->type == &i3c_masterdev_type)
> +		return 0;
> +
> +	i3c = dev_to_i3cdev(dev);
> +
> +	i3cdev = i3cdev_get_drvdata(i3c);
> +	if (!i3cdev)
> +		return 0;
> +
> +	i3cdev_set_drvdata(i3c, NULL);
> +
> +	/* Prevent transfers while cdev removal */
> +	mutex_lock(&i3cdev->xfer_lock);
> +	i3cdev->i3c = NULL;
> +	mutex_unlock(&i3cdev->xfer_lock);
> +
> +	cdev_device_del(&i3cdev->cdev, &i3cdev->dev);
> +	put_device(&i3cdev->dev);
> +
> +	pr_debug("i3cdev: device [%s] unregistered\n", dev_name(&i3c->dev));
> +
> +	return 0;
> +}
> +
> +static int i3cdev_notifier_call(struct notifier_block *nb,
> +				unsigned long action,
> +				void *data)
> +{
> +	struct device *dev = data;
> +
> +	switch (action) {
> +	case BUS_NOTIFY_ADD_DEVICE:
> +	case BUS_NOTIFY_UNBOUND_DRIVER:
> +		return i3cdev_attach(dev, NULL);

Does all i3c devices attach this driver?

> +	case BUS_NOTIFY_DEL_DEVICE:
> +	case BUS_NOTIFY_REMOVED_DEVICE:
> +	case BUS_NOTIFY_BIND_DRIVER:
> +		return i3cdev_detach(dev, NULL);
> +	}
> +
> +	return 0;
> +}
> +
> +static struct notifier_block i3cdev_notifier = {
> +	.notifier_call = i3cdev_notifier_call,
> +};
> +
> +static int __init i3cdev_init(void)
> +{
> +	int res;
> +
> +	/* Dynamically request unused major number */
> +	res = alloc_chrdev_region(&i3cdev_number, 0, I3C_MINORS, "i3c");
> +	if (res)
> +		goto out;
> +
> +	/* Register device class to populate sysfs entries */
> +	res = class_register(&i3cdev_class);
> +	if (res)
> +		goto out_unreg_chrdev;
> +
> +	/* Keep track of busses which have devices to add or remove later */
> +	res = bus_register_notifier(&i3c_bus_type, &i3cdev_notifier);
> +	if (res)
> +		goto out_unreg_class;
> +
> +	/* Bind to already existing device without driver right away */
> +	i3c_for_each_dev(NULL, i3cdev_attach);
> +
> +	return 0;
> +
> +out_unreg_class:
> +	class_unregister(&i3cdev_class);
> +out_unreg_chrdev:
> +	unregister_chrdev_region(i3cdev_number, I3C_MINORS);
> +out:
> +	pr_err("%s: Driver Initialisation failed\n", __FILE__);
> +	return res;
> +}
> +
> +static void __exit i3cdev_exit(void)
> +{
> +	bus_unregister_notifier(&i3c_bus_type, &i3cdev_notifier);
> +	i3c_for_each_dev(NULL, i3cdev_detach);
> +	class_unregister(&i3cdev_class);
> +	unregister_chrdev_region(i3cdev_number, I3C_MINORS);
> +}
> +
> +MODULE_AUTHOR("Vitor Soares <soares@synopsys.com>");
> +MODULE_DESCRIPTION("I3C /dev entries driver");
> +MODULE_LICENSE("GPL");
> +
> +module_init(i3cdev_init);
> +module_exit(i3cdev_exit);
> diff --git a/include/uapi/linux/i3c/i3cdev.h b/include/uapi/linux/i3c/i3cdev.h
> new file mode 100644
> index 000000000000..5adc1e3e7c4f
> --- /dev/null
> +++ b/include/uapi/linux/i3c/i3cdev.h
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (c) 2020 Synopsys, Inc. and/or its affiliates.
> + *
> + * Author: Vitor Soares <vitor.soares@synopsys.com>
> + */
> +
> +#ifndef _UAPI_I3C_DEV_H_
> +#define _UAPI_I3C_DEV_H_
> +
> +#include <linux/types.h>
> +#include <linux/ioctl.h>
> +
> +/* IOCTL commands */
> +#define I3C_DEV_IOC_MAGIC	0x07
> +
> +/**
> + * struct i3c_ioc_priv_xfer - I3C SDR ioctl private transfer
> + * @data: Holds pointer to userspace buffer with transmit data.
> + * @len: Length of data buffer buffers, in bytes.
> + * @rnw: encodes the transfer direction. true for a read, false for a write
> + */
> +struct i3c_ioc_priv_xfer {
> +	__u64 data;
> +	__u16 len;
> +	__u8 rnw;

In HDR mode,  rnw is command.

> +	__u8 pad[5];
> +};
> +
> +#define I3C_PRIV_XFER_SIZE(N)	\
> +	((((sizeof(struct i3c_ioc_priv_xfer)) * (N)) < (1 << _IOC_SIZEBITS)) \
> +	? ((sizeof(struct i3c_ioc_priv_xfer)) * (N)) : 0)
> +
> +#define I3C_IOC_PRIV_XFER(N)	\
> +	_IOC(_IOC_READ|_IOC_WRITE, I3C_DEV_IOC_MAGIC, 30, I3C_PRIV_XFER_SIZE(N))

I3C_IOC_SDR_XFER, support DDR, TSP, TSL future.

Frank
> +
> +#endif
> --
> 2.55.0.979.g7e5102b832-goog
>

Re: [PATCH v4 3/3] i3c: add i3cdev module to expose i3c dev in /dev

[Sam Agazaryan]

On Wed Sep 9, 2026 at 9:51 PM UTC, Frank Li wrote:
> On Sun, Sep 06, 2026 at 08:27:47PM +0000, Sam Agazaryan wrote:
>> From: Vitor Soares <vitor.soares@toradex.com>
>>
>> This patch adds userspace character device support for I3C SDR private
>> transfers via /dev.
>
> Add userspace character device ...

Updated in v5.


>>
>>   - Dynamically exposes /dev/bus/i3c/<device> character devices for I3C
>>     devices when unbound from kernel drivers.
>>   - Dynamically allocates character device minor numbers using the IDA
>>     allocator.
>>   - Implements private SDR read/write transfers via I3C_IOC_PRIV_XFER ioctl
>>     with 64-bit aligned UAPI data structures.
>
> Consider PRIV for SDR, need unified HDR also,
>
> I3C_IOC_XFER
>

At this very moment the production use case we have (OCP Recovery) supports SDR mode. That being said
supporting HDR mode also makes sense.

>> + * Copyright (c) 2020 Synopsys, Inc. and/or its affiliates.
>
> new post, should be 2026

Updated in v5.

>> +
>> +	i3cdev = kzalloc(sizeof(*i3cdev), GFP_KERNEL);
>
> kzalloc_obj()
>

Updated in v5.

>> +	dev_dbg(&i3c->dev, "Reading %zu bytes.\n", count);
>> +
>> +	ret = i3c_device_do_xfers(i3c, &xfers, 1, I3C_SDR);
>
> why force to SDR here.
>

For reads and writes we don't really have an option of passing down a struct denoting cmd mode.
Is there a different default behavior you had in mind for read/write besides SDR mode?

>> +err_out:
>> +	mutex_unlock(&i3cdev->xfer_lock);
>
> use cleanup guard()
>

Updated in v5.

>> +	for (i = 0; i < nxfers; i++) {
>> +		if (xfers[i].rnw) {
>> +			if (copy_to_user(u64_to_user_ptr(xfers[i].data),
>> +					 data_ptrs[i], xfers[i].len))
>
> Use actual_len,
>

Updated in v5.

>> +				ret = -EFAULT;
>> +		}
>> +	}
>> +
>> +err_free_mem:
>> +	for (j = 0; j < i; j++)
>> +		kfree(data_ptrs[j]);
>> +	kfree(k_xfers);
>
>
> use clean up __free(kfree) when alloc.
>

Updated in v5.

>> +	case BUS_NOTIFY_ADD_DEVICE:
>> +	case BUS_NOTIFY_UNBOUND_DRIVER:
>> +		return i3cdev_attach(dev, NULL);
>
> Does all i3c devices attach this driver?
>

Just i3c target devices without a bound driver, the controller should not attach.
The controller itself is explicitly skipped:
        if (dev->type == &i3c_masterdev_type || dev->driver)
            return 0;

>> +struct i3c_ioc_priv_xfer {
>> +	__u64 data;
>> +	__u16 len;
>> +	__u8 rnw;
>
> In HDR mode,  rnw is command.
>

Addressing this in the final comment in this message alongside the rest of
the HDR suggestions.

>> +	__u8 pad[5];
>> +};
>> +
>> +#define I3C_PRIV_XFER_SIZE(N)	\
>> +	((((sizeof(struct i3c_ioc_priv_xfer)) * (N)) < (1 << _IOC_SIZEBITS)) \
>> +	? ((sizeof(struct i3c_ioc_priv_xfer)) * (N)) : 0)
>> +
>> +#define I3C_IOC_PRIV_XFER(N)	\
>> +	_IOC(_IOC_READ|_IOC_WRITE, I3C_DEV_IOC_MAGIC, 30, I3C_PRIV_XFER_SIZE(N))
>
> I3C_IOC_SDR_XFER, support DDR, TSP, TSL future.
>
> Frank


On HDR modes - I think it makes sense to ensure the UAPI supports HDR modes now that
they're integrated into the i3c core. Would you prefer a scheme where we pass
in a more unified struct to specify either: SDR mode r/w or an HDR mode r/w
with ioctl?

Drawing inspiration from i3c_xfer:

    struct i3c_ioc_xfer {
        __u64 data;
        __u16 len;
        __u16 actual_len;
        union {
            __u8 rnw;
            __u8 cmd;
        };
        __u8 mode;
        __u8 pad[2];
    };



Thanks,
Sam

Re: [PATCH v4 3/3] i3c: add i3cdev module to expose i3c dev in /dev

[Frank Li]

On Fri, Sep 11, 2026 at 03:44:40AM +0000, Sam Agazaryan wrote:
> On Wed Sep 9, 2026 at 9:51 PM UTC, Frank Li wrote:
> > On Sun, Sep 06, 2026 at 08:27:47PM +0000, Sam Agazaryan wrote:
> >> From: Vitor Soares <vitor.soares@toradex.com>
> >>
> >> This patch adds userspace character device support for I3C SDR private
> >> transfers via /dev.
> >
> > Add userspace character device ...
>
> Updated in v5.
>
>
> >>
> >>   - Dynamically exposes /dev/bus/i3c/<device> character devices for I3C
> >>     devices when unbound from kernel drivers.
> >>   - Dynamically allocates character device minor numbers using the IDA
> >>     allocator.
> >>   - Implements private SDR read/write transfers via I3C_IOC_PRIV_XFER ioctl
> >>     with 64-bit aligned UAPI data structures.
> >
> > Consider PRIV for SDR, need unified HDR also,
> >
> > I3C_IOC_XFER
> >
>
> At this very moment the production use case we have (OCP Recovery) supports SDR mode. That being said
> supporting HDR mode also makes sense.
>
> >> + * Copyright (c) 2020 Synopsys, Inc. and/or its affiliates.
> >
> > new post, should be 2026
>
> Updated in v5.
>
> >> +
> >> +	i3cdev = kzalloc(sizeof(*i3cdev), GFP_KERNEL);
> >
> > kzalloc_obj()
> >
>
> Updated in v5.
>
> >> +	dev_dbg(&i3c->dev, "Reading %zu bytes.\n", count);
> >> +
> >> +	ret = i3c_device_do_xfers(i3c, &xfers, 1, I3C_SDR);
> >
> > why force to SDR here.
> >
>
> For reads and writes we don't really have an option of passing down a struct denoting cmd mode.
> Is there a different default behavior you had in mind for read/write besides SDR mode?

this is problem, Maybe we can use ioctrl to set default cmd, hdr/ sdr(default).

>
> >> +err_out:
> >> +	mutex_unlock(&i3cdev->xfer_lock);
> >
> > use cleanup guard()
> >
>
> Updated in v5.
>
> >> +	for (i = 0; i < nxfers; i++) {
> >> +		if (xfers[i].rnw) {
> >> +			if (copy_to_user(u64_to_user_ptr(xfers[i].data),
> >> +					 data_ptrs[i], xfers[i].len))
> >
> > Use actual_len,
> >
>
> Updated in v5.
>
> >> +				ret = -EFAULT;
> >> +		}
> >> +	}
> >> +
> >> +err_free_mem:
> >> +	for (j = 0; j < i; j++)
> >> +		kfree(data_ptrs[j]);
> >> +	kfree(k_xfers);
> >
> >
> > use clean up __free(kfree) when alloc.
> >
>
> Updated in v5.
>
> >> +	case BUS_NOTIFY_ADD_DEVICE:
> >> +	case BUS_NOTIFY_UNBOUND_DRIVER:
> >> +		return i3cdev_attach(dev, NULL);
> >
> > Does all i3c devices attach this driver?
> >
>
> Just i3c target devices without a bound driver, the controller should not attach.
> The controller itself is explicitly skipped:
>         if (dev->type == &i3c_masterdev_type || dev->driver)
>             return 0;

How about this driver built-in and other devices build as modules, modules
load maybe later than this driver?

Frank
>
> >> +struct i3c_ioc_priv_xfer {
> >> +	__u64 data;
> >> +	__u16 len;
> >> +	__u8 rnw;
> >
> > In HDR mode,  rnw is command.
> >
>
> Addressing this in the final comment in this message alongside the rest of
> the HDR suggestions.
>
> >> +	__u8 pad[5];
> >> +};
> >> +
> >> +#define I3C_PRIV_XFER_SIZE(N)	\
> >> +	((((sizeof(struct i3c_ioc_priv_xfer)) * (N)) < (1 << _IOC_SIZEBITS)) \
> >> +	? ((sizeof(struct i3c_ioc_priv_xfer)) * (N)) : 0)
> >> +
> >> +#define I3C_IOC_PRIV_XFER(N)	\
> >> +	_IOC(_IOC_READ|_IOC_WRITE, I3C_DEV_IOC_MAGIC, 30, I3C_PRIV_XFER_SIZE(N))
> >
> > I3C_IOC_SDR_XFER, support DDR, TSP, TSL future.
> >
> > Frank
>
>
> On HDR modes - I think it makes sense to ensure the UAPI supports HDR modes now that
> they're integrated into the i3c core. Would you prefer a scheme where we pass
> in a more unified struct to specify either: SDR mode r/w or an HDR mode r/w
> with ioctl?
>
> Drawing inspiration from i3c_xfer:
>
>     struct i3c_ioc_xfer {
>         __u64 data;
>         __u16 len;
>         __u16 actual_len;
>         union {
>             __u8 rnw;
>             __u8 cmd;
>         };
>         __u8 mode;
>         __u8 pad[2];
>     };
>
>
>
> Thanks,
> Sam

Re: [PATCH v4 3/3] i3c: add i3cdev module to expose i3c dev in /dev

[Sam Agazaryan]

> this is problem, Maybe we can use ioctrl to set default cmd, hdr/ sdr(default).

That makes sense.

We can keep read()/write() defaulting to I3C_SDR out of the box, and add an
ioctl (e.g. I3C_IOC_SET_MODE) to allow configuring the default mode and
command byte for that file descriptor if a user wants to use read()/write()
with HDR. How does that sound?

>How about this driver built-in and other devices build as modules, modules
>load maybe later than this driver?

This scenario is handled automatically by our bus notifier
(i3cdev_notifier_call):

When i3cdev is built-in, it attaches to unbound target devices at boot. If a
device driver module is loaded later (e.g. via modprobe), the driver core
emits BUS_NOTIFY_BIND_DRIVER right before binding the new driver.

Our notifier listens for BUS_NOTIFY_BIND_DRIVER and immediately calls
i3cdev_detach(), which unregisters the device clears i3cdev->i3c.
That way, i3cdev automatically steps aside whenever a dedicated driver
loads. And if that module is ever unloaded later, BUS_NOTIFY_UNBOUND_DRIVER
triggers i3cdev_attach() to re-expose the /dev node.

Thanks,
Sam

Re: [PATCH v4 3/3] i3c: add i3cdev module to expose i3c dev in /dev

[Adrian Hunter]

On 11/09/2026 06:44, Sam Agazaryan wrote:
> On Wed Sep 9, 2026 at 9:51 PM UTC, Frank Li wrote:
>> On Sun, Sep 06, 2026 at 08:27:47PM +0000, Sam Agazaryan wrote:
>>> From: Vitor Soares <vitor.soares@toradex.com>
>>> +	for (i = 0; i < nxfers; i++) {
>>> +		if (xfers[i].rnw) {
>>> +			if (copy_to_user(u64_to_user_ptr(xfers[i].data),
>>> +					 data_ptrs[i], xfers[i].len))
>>
>> Use actual_len,
>
> Updated in v5.

actual_len is a problem.  Only 2 controller drivers support it.
No I3C device drivers support it.  mctp-i3c reads len not actual_len.
regmap-i3c ignores len and actual_len.  And its definition comment
is not exactly correct since it is only valid for reads.

Despite that, the additional issue here, is what to do about a short
read.  Seems like it should be in the uapi, but actual_len needs to
be fixed first.

Re: [PATCH v4 3/3] i3c: add i3cdev module to expose i3c dev in /dev

[Meagan Lloyd]

FYI, I have an alternative design option that's worth reviewing:
https://lore.kernel.org/linux-i3c/20260911210935.1353126-1-meaganlloyd@linux.microsoft.com/T/#mcec3e06c911d331d5cd2b2d0a7f7169b5f9f044b

It follows in the direction Boris was going in the 2020 thread:
https://lore.kernel.org/linux-i3c/20200222093844.2f5ed538@collabora.com/

The alternate binding solution is:

If i3cdev match id table is empty, the generic i3cdev driver can't bind
during the auto-loading that happens on boot. This means we won't step
on the toes of more specialized drivers. Once user-space sets
driver_override sysfs knob to 'i3cdev' and binds the device to i3cdev,
it behaves the same as any other typical driver model-based driver. We
can also use udev rules to automate setting the driver_override & doing
the bind for any i3c devices found by the core.

My series also implements actual_len in the controller drivers and uses
it as an output for user-space to consume for reads. It also accounts for
maintaining compatibility if i3cdev_xfer struct expands for HDR modes.

- Meagan

Re: [PATCH v4 3/3] i3c: add i3cdev module to expose i3c dev in /dev

[Sam Agazaryan]

Hey Meagan,

Thanks for sharing your patch series.

 The reason we have the bus notifier system here is
 1. Dynamic module loading is handled automatically: if i3cdev is built-in and
    a specialized driver module loads later, the bus notifier receives
    BUS_NOTIFY_BIND_DRIVER and immediately calls i3cdev_detach().
 2. No cross-subsystem changes needed. The bus notifier avoids touching other
    subsystems entirely.
 3. Boot-time recovery flows: For OCP Secure Firmware Recovery
    devices come up unbound and need /dev/bus/i3c/<device> available
    immediately without requiring udev rules or sysfs writes first.

Let me know what you think about that and if there's something I may be missing.

It looks like we're both going in the same direction for UAPI integration too.

If you're open to collaborating, I think we can combine and converge on a single
i3cdev driver - adopting all necessary fixes and ensuring the UAPI and features
cover both of our use cases so we have one unified series (and any
other use cases
or desires we may want out of an i3cdev driver).

I'd like to know what everyone thinks about taking that approach also
if there are any
other use cases we may be missing here, regardless of which patch set we choose.

I guess just to get things moving, if we go one way and choose to go
forward with
the bus notifier approach, Meagan would you be okay with me incorporating your
Patch 2/3 for actual_len for the i3c controller drivers? Of course
with your authorship.


Thanks,
Sam

Re: [PATCH v4 0/3] i3c: Introduce i3c device userspace interface

[Wolfram Sang]

> - Fixed device lifecycle and concurrency:
>   * Switched to cdev_device_add() and cdev_device_del() with an embedded
>     struct device and a release callback, preventing use-after-free and
>     races on driver unbind/detach (incorporating fix from Oleksandr
>     Shulzhenko).
>   * Cleared i3cdev->i3c under xfer_lock on detach so concurrent/subsequent
>     file operations safely return -ENODEV.

I haven't looked at the code (and won't have the time to do so), but if
the above paragraph had not been present, I would have nacked the series
right away. To say it positive: Kudos for working on these issues!

Re: [PATCH v4 0/3] i3c: Introduce i3c device userspace interface

[Wolfram Sang]

> [2] https://github.com/vitor-soares-snps/i3c-tools

An idea: because this project only contains 'i3ctransfer' at the moment,
it probably makes sense to provide this tool with the Kernel, placed in
the 'tools' directory? We can seperate it again if it becomes too big or
we want its own community there. But for now, I think it could make
sense unless you want to maintain this repo as well?

Happy hacking!