[PATCH 0/3] I3C character device driver using driver_override

[PATCH 0/3] I3C character device driver using driver_override

[Meagan Lloyd]

This is a rework and revival option for Vitor Soares' I3C character
device driver patch series from 2020 [1] that I've been exploring for a
few months.  Recently there was a revival posted to the list [2], so I
wanted to share this design option as well.

In [1] and [2], the i3cdev driver automatically attaches and detaches
depending whether another driver has attached/not. In [1], Boris was
suggesting we explore a more straightforward and traditional binding
method aligning with the Linux driver model. At the time, there wasn't
a way to auto-bind while keeping manual binding possible as they shared
the same match() hook. Now with the new driver_override feature, the
auto-binding of i3cdev on boot can be avoided if the i3cdev driver has
an empty match ID table. After boot, where specialized drivers would have
already bound, user-space can explicitly opt-in by setting the
driver_override sysfs file with 'i3cdev' and manually binding via sysfs
(or by simply loading the driver if it's loadable). This can also be
easily automated with udev rules that run whenever the I3C core exposes
a new device.

One downside of the automatic attach/de-attach is that if a different
driver is loaded later, the first driver could have altered something
on the device, breaking any assumptions of the subsequent driver.

My series builds on [1] through:
0. Addressing code review feedback in [1] from Greg, Boris, and Randy.
1. Using actual_len for accurate read response reporting. The kernel
will report actual_len received from the core to user-space via the
uapi i3cdev_xfer struct.
2. Placing limits on the number of transfers and bytes in requests to
prevent unlimited-sized transfers or kernel memory allocation
3. Checking inputs and descriptive return codes as guard-rails
for user-space and to ease use of the i3cdev driver
4. Checking on MWL to ensure that we respect device limits
5. Proper lifetime management of i3cdev_data and underlying device
6. Addressing dangling fops in the event we have an open file descriptor
when a device gets unbound.
7. Fast-path locking to ensure transfers complete before a device is
unbound.
8. Allowing only one file descriptor per I3C device to avoid bugs
around multiple processes interacting with the device and altering
the device underneath the other. For example, without this, one process
could change the device's page or address pointer register underneath
the other process.
9. copy_struct_from_user to ensure struct i3cdev_xfer could be extended
in a compatible way. This is to be forward-looking towards potential
HDR mode expansion and code reuse.
10. Reserving the IOCTL number formally
11. Updating the Documentation to be a syntax correct example program
template.
12. Preserving /dev/bus/i3c/<bus id>-<Provisional ID> naming while
allowing sysfs path to be neatly named i3cdev-<minor>. This avoids
repeated <bus id>-<Provisional ID> in the sysfs paths which can be
confusing/circular-looking.
    e.g. /sys/bus/i3c/devices/0-deadbeef001/i3cdev/0-deadbeef001 ->
         /sys/bus/i3c/devices/0-deadbeef001/i3cdev/i3cdev-0
13. Updating all naming references related to i3c_priv_xfer to align
with new i3c_xfer struct
14. Updating the MAINTAINERS file for the new pieces of code

Note that i3c-tools [3] or a fork of it will need small updates:
1. Update include/uapi/linux/i3c/i3cdev.h to match updated uapi structs
2. In i3ctransfer.c, use actual_len for reads

I've added MODULE_VERSION("1.0.0") in the i3cdev driver, so i3c-tools
could use that to determine whether to use the old out-of-tree uapi or this one.

[1] https://lore.kernel.org/linux-i3c/cover.1582069402.git.vitor.soares@synopsys.com/
[2] https://lore.kernel.org/linux-i3c/ap_1-gFF7S821xJT@ninjato/T/#m9107c1785a4a16b1b3cb84c3269d17fac35819c8
[3] https://github.com/vitor-soares-snps/i3c-tools

Meagan Lloyd (3):
  i3c: master: enable driver_override for I3C
  i3c: set i3c_xfer.actual_len in controller drivers
  i3c: add i3cdev character device module for user-space access

 Documentation/userspace-api/i3c/i3cdev.rst    | 152 ++++
 .../userspace-api/ioctl/ioctl-number.rst      |   1 +
 MAINTAINERS                                   |   7 +
 drivers/hwmon/lm75.c                          |   7 +-
 drivers/hwmon/tmp108.c                        |   2 +
 drivers/i3c/Kconfig                           |  12 +
 drivers/i3c/Makefile                          |   1 +
 drivers/i3c/i3cdev.c                          | 730 ++++++++++++++++++
 drivers/i3c/master.c                          |   6 +
 drivers/i3c/master/adi-i3c-master.c           |   5 +-
 drivers/i3c/master/dw-i3c-master.c            |   4 +-
 drivers/i3c/master/i3c-master-cdns.c          |   5 +-
 drivers/i3c/master/mipi-i3c-hci/core.c        |   5 +-
 drivers/i3c/master/renesas-i3c.c              |   3 +
 drivers/iio/adc/ad4062.c                      |  10 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c   |   3 +
 include/uapi/linux/i3c/i3cdev.h               |  70 ++
 17 files changed, 1016 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/userspace-api/i3c/i3cdev.rst
 create mode 100644 drivers/i3c/i3cdev.c
 create mode 100644 include/uapi/linux/i3c/i3cdev.h


base-commit: cab40cfc9e116acd4d60f95b4b1264cab78f3803
-- 
2.49.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 1/3] i3c: master: enable driver_override for I3C

[Meagan Lloyd]

Opt-in to driver_override sysfs functionality for I3C subsystem.

Update some I3C drivers that are assuming that if their .probe is
invoked that the bus match function already checked the ID table. Go
ahead and verify in .probe that i3c_device_match_id() returns a valid
match before using the i3c_device_id pointer.

Signed-off-by: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
Assisted-by: Claude:claude-opus-4.8 copilot
---
 drivers/hwmon/lm75.c                        |  7 ++++++-
 drivers/hwmon/tmp108.c                      |  2 ++
 drivers/i3c/master.c                        |  6 ++++++
 drivers/iio/adc/ad4062.c                    | 10 ++++++++--
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c |  3 +++
 5 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 104149a03bad..0d3c843d1823 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -888,6 +888,7 @@ MODULE_DEVICE_TABLE(i3c, lm75_i3c_ids);
 static int lm75_i3c_probe(struct i3c_device *i3cdev)
 {
 	struct device *dev = i3cdev_to_dev(i3cdev);
+	const struct i3c_device_id *id;
 	const struct lm75_i3c_device *id_data;
 	struct regmap *regmap;
 
@@ -895,7 +896,11 @@ static int lm75_i3c_probe(struct i3c_device *i3cdev)
 	if (IS_ERR(regmap))
 		return PTR_ERR(regmap);
 
-	id_data = i3c_device_match_id(i3cdev, lm75_i3c_ids)->data;
+	id = i3c_device_match_id(i3cdev, lm75_i3c_ids);
+	if (!id)
+		return -ENODEV;
+
+	id_data = id->data;
 
 	return lm75_generic_probe(dev, id_data->name, id_data->type, 0, regmap);
 }
diff --git a/drivers/hwmon/tmp108.c b/drivers/hwmon/tmp108.c
index 1c4a58855e2d..98076ae8dce8 100644
--- a/drivers/hwmon/tmp108.c
+++ b/drivers/hwmon/tmp108.c
@@ -581,6 +581,8 @@ static int p3t1085_i3c_probe(struct i3c_device *i3cdev)
 				     "Failed to register i3c regmap\n");
 
 	id = i3c_device_match_id(i3cdev, p3t1085_i3c_ids);
+	if (!id)
+		return -ENODEV;
 
 	return tmp108_common_probe(dev, regmap, "p3t1085_i3c", id->data);
 }
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index afcd7a21a3e6..42bc9a923d81 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -340,10 +340,15 @@ static int i3c_device_match(struct device *dev, const struct device_driver *drv)
 	struct i3c_device *i3cdev;
 	const struct i3c_driver *i3cdrv;
 	u8 static_addr_method = 0;
+	int ret;
 
 	if (dev->type != &i3c_device_type)
 		return 0;
 
+	ret = device_match_driver_override(dev, drv);
+	if (ret >= 0)
+		return ret;
+
 	i3cdev = dev_to_i3cdev(dev);
 	i3cdrv = drv_to_i3cdrv(drv);
 
@@ -890,6 +895,7 @@ const struct bus_type i3c_bus_type = {
 	.probe = i3c_device_probe,
 	.remove = i3c_device_remove,
 	.shutdown = i3c_device_shutdown,
+	.driver_override = true,
 };
 EXPORT_SYMBOL_GPL(i3c_bus_type);
 
diff --git a/drivers/iio/adc/ad4062.c b/drivers/iio/adc/ad4062.c
index 8e5984055b15..f28b503d16eb 100644
--- a/drivers/iio/adc/ad4062.c
+++ b/drivers/iio/adc/ad4062.c
@@ -1482,14 +1482,20 @@ MODULE_DEVICE_TABLE(i3c, ad4062_id_table);
 
 static int ad4062_probe(struct i3c_device *i3cdev)
 {
-	const struct i3c_device_id *id = i3c_device_match_id(i3cdev, ad4062_id_table);
-	const struct ad4062_chip_info *chip = id->data;
+	const struct i3c_device_id *id;
+	const struct ad4062_chip_info *chip;
 	struct device *dev = &i3cdev->dev;
 	struct iio_dev *indio_dev;
 	struct ad4062_state *st;
 	bool ref_sel;
 	int ret;
 
+	id = i3c_device_match_id(i3cdev, ad4062_id_table);
+	if (!id)
+		return -ENODEV;
+
+	chip = id->data;
+
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
 	if (!indio_dev)
 		return -ENOMEM;
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
index cb5c5d7e1f3d..153a03e06801 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
@@ -32,6 +32,9 @@ static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
 	struct device *dev = i3cdev_to_dev(i3cdev);
 	struct regmap *regmap;
 
+	if (!id)
+		return -ENODEV;
+
 	regmap = devm_regmap_init_i3c(i3cdev, &st_lsm6dsx_i3c_regmap_config);
 	if (IS_ERR(regmap)) {
 		dev_err(dev, "Failed to register i3c regmap %ld\n", PTR_ERR(regmap));
-- 
2.49.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 1/3] i3c: master: enable driver_override for I3C

[sashiko-bot]

> Opt-in to driver_override sysfs functionality for I3C subsystem.
> 
> Update some I3C drivers that are assuming that if their .probe is
> invoked that the bus match function already checked the ID table. Go
> ahead and verify in .probe that i3c_device_match_id() returns a valid
> match before using the i3c_device_id pointer.
> 
> Signed-off-by: Meagan Lloyd <meaganlloyd@linux.microsoft.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911210935.1353126-1-meaganlloyd@linux.microsoft.com?part=1


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 1/3] i3c: master: enable driver_override for I3C

[Guenter Roeck]

On Fri, Sep 11, 2026 at 02:09:33PM -0700, Meagan Lloyd wrote:
> Opt-in to driver_override sysfs functionality for I3C subsystem.
> 
> Update some I3C drivers that are assuming that if their .probe is
> invoked that the bus match function already checked the ID table. Go
> ahead and verify in .probe that i3c_device_match_id() returns a valid
> match before using the i3c_device_id pointer.
> 
> Signed-off-by: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
> Assisted-by: Claude:claude-opus-4.8 copilot
> ---
>  drivers/hwmon/lm75.c                        |  7 ++++++-
>  drivers/hwmon/tmp108.c                      |  2 ++

Separate patches for each hwmon driver, please. Each patch should use the
standard prefix used in hwmon drivers.

Thanks,
Guenter

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 1/3] i3c: master: enable driver_override for I3C

[Andy Shevchenko]

On Fri, Sep 11, 2026 at 02:09:33PM -0700, Meagan Lloyd wrote:
> Opt-in to driver_override sysfs functionality for I3C subsystem.
> 
> Update some I3C drivers that are assuming that if their .probe is
> invoked that the bus match function already checked the ID table. Go
> ahead and verify in .probe that i3c_device_match_id() returns a valid
> match before using the i3c_device_id pointer.
> 
> Signed-off-by: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
> Assisted-by: Claude:claude-opus-4.8 copilot

Assisted-by: LLM

...

>  drivers/hwmon/lm75.c                        |  7 ++++++-
>  drivers/hwmon/tmp108.c                      |  2 ++
>  drivers/i3c/master.c                        |  6 ++++++
>  drivers/iio/adc/ad4062.c                    | 10 ++++++++--
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c |  3 +++

No, each IIO driver should have its own patch.

-- 
With Best Regards,
Andy Shevchenko



-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 1/3] i3c: master: enable driver_override for I3C

[Jonathan Cameron]

On Fri, 11 Sep 2026 14:09:33 -0700
Meagan Lloyd <meaganlloyd@linux.microsoft.com> wrote:

> Opt-in to driver_override sysfs functionality for I3C subsystem.
> 
> Update some I3C drivers that are assuming that if their .probe is
> invoked that the bus match function already checked the ID table. Go
> ahead and verify in .probe that i3c_device_match_id() returns a valid
> match before using the i3c_device_id pointer.
> 
> Signed-off-by: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
> Assisted-by: Claude:claude-opus-4.8 copilot

Along with splitting these up as this is the sort of change that
will sometimes need backporting in individual drivers because we are
layering something on top, I'd look at adding some helpers to skip
the id as that isn't much used and get the specific fields people
want.

> diff --git a/drivers/hwmon/tmp108.c b/drivers/hwmon/tmp108.c
> index 1c4a58855e2d..98076ae8dce8 100644
> --- a/drivers/hwmon/tmp108.c
> +++ b/drivers/hwmon/tmp108.c
> @@ -581,6 +581,8 @@ static int p3t1085_i3c_probe(struct i3c_device *i3cdev)
>  				     "Failed to register i3c regmap\n");
>  
>  	id = i3c_device_match_id(i3cdev, p3t1085_i3c_ids);
> +	if (!id)
> +		return -ENODEV;
>  
>  	return tmp108_common_probe(dev, regmap, "p3t1085_i3c", id->data);

Similar to below, jumping directly to an i3c_device_match_data() helper
in this series would avoid getting and id that these drivers don't
need access to.

>  }

> diff --git a/drivers/iio/adc/ad4062.c b/drivers/iio/adc/ad4062.c
> index 8e5984055b15..f28b503d16eb 100644
> --- a/drivers/iio/adc/ad4062.c
> +++ b/drivers/iio/adc/ad4062.c
> @@ -1482,14 +1482,20 @@ MODULE_DEVICE_TABLE(i3c, ad4062_id_table);
>  
>  static int ad4062_probe(struct i3c_device *i3cdev)
>  {
> -	const struct i3c_device_id *id = i3c_device_match_id(i3cdev, ad4062_id_table);
> -	const struct ad4062_chip_info *chip = id->data;
> +	const struct i3c_device_id *id;
> +	const struct ad4062_chip_info *chip;
>  	struct device *dev = &i3cdev->dev;
>  	struct iio_dev *indio_dev;
>  	struct ad4062_state *st;
>  	bool ref_sel;
>  	int ret;
>  
> +	id = i3c_device_match_id(i3cdev, ad4062_id_table);
> +	if (!id)
> +		return -ENODEV;
> +
> +	chip = id->data;

This might be a good opportunity to introduce

i3c_device_match_data() as in many drivers we don't actually care about
access to the id itself.

> +
>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
>  	if (!indio_dev)
>  		return -ENOMEM;
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> index cb5c5d7e1f3d..153a03e06801 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i3c.c
> @@ -32,6 +32,9 @@ static int st_lsm6dsx_i3c_probe(struct i3c_device *i3cdev)
>  	struct device *dev = i3cdev_to_dev(i3cdev);
>  	struct regmap *regmap;
>  

Please bring the assignment down to above the check so

	id = i3c_device_match_id(i3cdev,st_lsm6dsx_i3c_ids);

Which is what you did in the previous driver...
> +	if (!id)
> +		return -ENODEV;
> +
>  	regmap = devm_regmap_init_i3c(i3cdev, &st_lsm6dsx_i3c_regmap_config);
>  	if (IS_ERR(regmap)) {
>  		dev_err(dev, "Failed to register i3c regmap %ld\n", PTR_ERR(regmap));


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 2/3] i3c: set i3c_xfer.actual_len in controller drivers

[Meagan Lloyd]

Set i3c_xfer.actual_len for reads across the I3C Controller drivers.

actual_len provides a consistent location for device drivers to know the
read response bytes without overriding the len field.

Signed-off-by: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
---
 drivers/i3c/master/adi-i3c-master.c    | 5 ++++-
 drivers/i3c/master/dw-i3c-master.c     | 4 +++-
 drivers/i3c/master/i3c-master-cdns.c   | 5 ++++-
 drivers/i3c/master/mipi-i3c-hci/core.c | 5 ++++-
 drivers/i3c/master/renesas-i3c.c       | 3 +++
 5 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/i3c/master/adi-i3c-master.c b/drivers/i3c/master/adi-i3c-master.c
index b35386260350..d033bfd4ecc2 100644
--- a/drivers/i3c/master/adi-i3c-master.c
+++ b/drivers/i3c/master/adi-i3c-master.c
@@ -412,8 +412,11 @@ static int adi_i3c_master_i3c_xfers(struct i3c_dev_desc *dev,
 
 	ret = xfer->ret;
 
-	for (i = 0; i < nxfers; i++)
+	for (i = 0; i < nxfers; i++) {
 		xfers[i].err = adi_i3c_cmd_get_err(&xfer->cmds[i]);
+		if (xfers[i].rnw)
+			xfers[i].actual_len = xfer->cmds[i].rx_len;
+	}
 
 	return ret;
 }
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 4563d8761ba0..54406e31c870 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -1028,8 +1028,10 @@ static int dw_i3c_master_i3c_xfers(struct i3c_dev_desc *dev,
 	for (i = 0; i < i3c_nxfers; i++) {
 		struct dw_i3c_cmd *cmd = &xfer->cmds[i];
 
-		if (i3c_xfers[i].rnw)
+		if (i3c_xfers[i].rnw) {
 			i3c_xfers[i].len = cmd->rx_len;
+			i3c_xfers[i].actual_len = cmd->rx_len;
+		}
 	}
 
 	ret = xfer->ret;
diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
index 2d98c1ce9b12..331062c10b31 100644
--- a/drivers/i3c/master/i3c-master-cdns.c
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -795,8 +795,11 @@ static int cdns_i3c_master_i3c_xfers(struct i3c_dev_desc *dev,
 
 	ret = cdns_xfer->ret;
 
-	for (i = 0; i < nxfers; i++)
+	for (i = 0; i < nxfers; i++) {
 		xfers[i].err = cdns_i3c_cmd_get_err(&cdns_xfer->cmds[i]);
+		if (xfers[i].rnw)
+			xfers[i].actual_len = cdns_xfer->cmds[i].rx_len;
+	}
 
 	cdns_i3c_master_free_xfer(cdns_xfer);
 
diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index dadf049bd4b5..29fa68e29484 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -508,8 +508,11 @@ static int i3c_hci_i3c_xfers(struct i3c_dev_desc *dev,
 	if (ret)
 		goto out;
 	for (i = 0; i < nxfers; i++) {
-		if (i3c_xfers[i].rnw)
+		if (i3c_xfers[i].rnw) {
 			i3c_xfers[i].len = RESP_DATA_LENGTH(xfer[i].response);
+			i3c_xfers[i].actual_len = RESP_DATA_LENGTH(xfer[i].response);
+		}
+
 		if (RESP_STATUS(xfer[i].response) != RESP_SUCCESS) {
 			ret = -EIO;
 			goto out;
diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
index 28c0927a0179..a3dcb4cd9b0e 100644
--- a/drivers/i3c/master/renesas-i3c.c
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -918,6 +918,9 @@ static int renesas_i3c_i3c_xfers(struct i3c_dev_desc *dev, struct i3c_xfer *i3c_
 		time_left = renesas_i3c_wait_xfer(i3c, xfer);
 		if (!time_left)
 			xfer_failed = true;
+
+		if (i3c_xfers[i].rnw)
+			i3c_xfers[i].actual_len = cmd->rx_count;
 	}
 
 	if (xfer_failed)
-- 
2.49.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 2/3] i3c: set i3c_xfer.actual_len in controller drivers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The Renesas I3C controller silently ignores all transfer errors and timeouts, always returning success.
--

commit e28c1ae3a51db6eef5fcba16ca850acda1a087d2
Author: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
Subject: i3c: set i3c_xfer.actual_len in controller drivers

This commit updates various I3C controller drivers to set actual_len for
read transfers. It provides a consistent location for device drivers to know
the read response bytes without overriding the length field.

> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> --- a/drivers/i3c/master/renesas-i3c.c
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -918,6 +918,9 @@ static int renesas_i3c_i3c_xfers(struct i3c_dev_desc *dev, struct i3c_xfer *i3c_
>  		time_left = renesas_i3c_wait_xfer(i3c, xfer);
>  		if (!time_left)
>  			xfer_failed = true;
> +
> +		if (i3c_xfers[i].rnw)
> +			i3c_xfers[i].actual_len = cmd->rx_count;
>  	}
>  
>  	if (xfer_failed)
>  		renesas_i3c_irqs_mask_and_clear(i3c);
>  
>  	return 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but is a critical pre-existing
issue. Does renesas_i3c_i3c_xfers() silently ignore transfer errors?

If an I3C hardware error occurs during the transfer, the ISR populates
xfer->ret with a negative error code like -EIO, but the loop here never
checks xfer->ret. Furthermore, if a timeout occurs, xfer_failed is set,
but the loop does not break early, and the function unconditionally returns
0 at the end.

Since the controller ignores the error and returns success, callers will
proceed as if the data was successfully read or written. Could this lead
to I3C device drivers consuming uninitialized memory or encountering silent
state machine failures when a hardware NACK, CRC error, or timeout occurs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911210935.1353126-1-meaganlloyd@linux.microsoft.com?part=2

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 2/3] i3c: set i3c_xfer.actual_len in controller drivers

[Jonathan Cameron]

On Fri, 11 Sep 2026 14:09:34 -0700
Meagan Lloyd <meaganlloyd@linux.microsoft.com> wrote:

> Set i3c_xfer.actual_len for reads across the I3C Controller drivers.
> 
> actual_len provides a consistent location for device drivers to know the
> read response bytes without overriding the len field.

Hi Meagan,

Why is that useful in this series?  Currently this sounds like
an unrelated change.

Jonathan


> 
> Signed-off-by: Meagan Lloyd <meaganlloyd@linux.microsoft.com>

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 3/3] i3c: add i3cdev character device module for user-space access

[Meagan Lloyd]

The i3cdev driver is a character device driver that allows user-space
to control and interact with I3C devices.

Currently, it has the ability to perform Single Data Rate (SDR)
transfers - basic reads/writes.

With the addition of sysfs driver_override, there is now a
straightforward and direct way to match the i3cdev driver to any i3c
device without stepping on the toes of more specialized drivers that are
loaded automatically.

This is accomplished by the i3cdev driver not having any entries in the
i3c_device_id table. After boot, simply set the driver_override to
"i3cdev" and bind the device manually via the sysfs bind knob. This can
also be automated with udev rules as well.

The character device interface will be exposed at:
/dev/bus/i3c/<bus id>-<Provisional ID>

Signed-off-by: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
Assisted-by: Claude:claude-opus-4.8 copilot
---
 Documentation/userspace-api/i3c/i3cdev.rst    | 152 ++++
 .../userspace-api/ioctl/ioctl-number.rst      |   1 +
 MAINTAINERS                                   |   7 +
 drivers/i3c/Kconfig                           |  12 +
 drivers/i3c/Makefile                          |   1 +
 drivers/i3c/i3cdev.c                          | 730 ++++++++++++++++++
 include/uapi/linux/i3c/i3cdev.h               |  70 ++
 7 files changed, 973 insertions(+)
 create mode 100644 Documentation/userspace-api/i3c/i3cdev.rst
 create mode 100644 drivers/i3c/i3cdev.c
 create mode 100644 include/uapi/linux/i3c/i3cdev.h

diff --git a/Documentation/userspace-api/i3c/i3cdev.rst b/Documentation/userspace-api/i3c/i3cdev.rst
new file mode 100644
index 000000000000..ad246bf28919
--- /dev/null
+++ b/Documentation/userspace-api/i3c/i3cdev.rst
@@ -0,0 +1,152 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+====================
+I3C Device Interface
+====================
+
+This interface allows access to I3C devices from userspace.
+
+As I3C devices adhere to the I3C protocol, simple transfers can be accomplished
+using a generic driver. Currently, this interface supports private Single Data
+Rate (SDR) read and write transfers.
+
+The i3cdev module will not auto-bind to devices. Userspace needs to explicitly
+bind the device to the driver. This is to avoid interfering with the binding of
+specialized drivers.
+
+Once bound, a character device interface will be created at:
+/dev/bus/i3c/<bus id>-<Provisional ID>.
+
+====================
+Usage
+====================
+
+Any discovered I3C devices by the I3C subsystem will have device folders under /sys/bus/i3c/devices/<bus id>-<Provisional ID>.
+To allow binding of a device with i3cdev driver, set the driver_override:
+
+::
+
+    # echo "i3cdev" > /sys/bus/i3c/devices/<bus id>-<Provisional ID>/driver_override
+
+If the i3cdev driver is not yet loaded, load it and it will cause the driver to bind
+to any devices with the override in place.
+
+If the i3cdev driver is already loaded, go ahead and perform a manual bind:
+
+::
+
+    # echo "<bus id>-<Provisional ID>" > /sys/bus/i3c/drivers/i3cdev/bind
+
+Set driver override
+::
+
+    # echo "i3cdev" > /sys/bus/i3c/devices/0-deadbeef001/driver_override
+    # echo "i3cdev" > /sys/bus/i3c/devices/0-deadbeef002/driver_override
+
+Bind the device to the driver
+::
+
+    # echo "0-deadbeef001" > /sys/bus/i3c/drivers/i3cdev/bind
+    # echo "0-deadbeef002" > /sys/bus/i3c/drivers/i3cdev/bind
+
+Observe the resulting character device files under /dev/bus/i3c/
+::
+
+    # ls -ltr /dev/bus/i3c/
+    crw-------    1 root     root      235,   1 Jun 30 17:49 0-deadbeef002
+    crw-------    1 root     root      235,   0 Jun 30 17:49 0-deadbeef001
+
+BASIC CHARACTER DEVICE API
+===============================
+The API supports private Single Data Rate (SDR) read and write transfers.
+Those transaction can be achieved by the following:
+
+``read(file, buffer, sizeof(buffer))``
+  The standard read() operation will work as a simple transaction of private
+  SDR read data followed a stop.
+  Return the number of bytes read on success, and a negative error otherwise.
+
+``write(file, buffer, sizeof(buffer))``
+  The standard write() operation will work as a simple transaction of private
+  SDR write data followed a stop.
+  Return the number of bytes written on success, and a negative error otherwise.
+
+``ioctl(file, I3CDEV_XFER, struct i3cdev_xfers *xfers)``
+  It combines read/write transactions without a stop in between.
+  Return 0 on success, and a negative error otherwise.
+
+C EXAMPLE (PSEUDO CODE)
+=======================
+You need to open (and get a file descriptor) to /dev/bus/i3c/<bus id>-<Provisional ID>,
+do your operations (read, write, ioctl), and then close it.
+
+The following header files should be included in an I3C program::
+
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/ioctl.h>
+#include <i3c/i3cdev.h>
+#include <stdint.h>
+
+These additionally are used by this example::
+
+#include <stdlib.h>
+#include <assert.h>
+
+To work with an I3C device, one must call open and get a file descriptor::
+
+	int fd;
+
+	fd = open("/dev/bus/i3c/0-deadbeef001", O_RDWR);
+	if (fd < 0)
+		exit(EXIT_FAILURE);
+
+Now that the file is open, we can do some operations::
+
+	int ret;
+
+	/* Write function */
+	uint8_t  buf[] = {0x00, 0xde, 0xad, 0xbe, 0xef};
+	ret = write(fd, buf, 5);
+	if (ret != 5) {
+		/* ERROR HANDLING: I3C transaction failed */
+	}
+
+	/*  Read function */
+	ret = read(fd, buf, 4);
+	if (ret < 0) {
+		/* ERROR HANDLING: I3C transaction failed */
+	} else {
+		/* Iterate over buf[] to get the read data */
+	}
+
+	/* IOCTL function */
+	struct i3cdev_xfer xfers[2] = {0}; /* Must zero out for compatibility */
+	struct i3cdev_xfers xfers_metadata = { .nxfers = 2,
+					       .xfers = (uintptr_t) xfers,
+					       .xfer_size = sizeof(struct i3cdev_xfer)
+					     };
+
+	uint8_t tx_buf[] = {0x00, 0xde, 0xad, 0xbe, 0xef};
+	uint8_t rx_buf[10];
+
+	xfers[0].data = (uintptr_t) tx_buf;
+	xfers[0].len = 5;
+	xfers[0].rnw = 0;
+	xfers[1].data = (uintptr_t) rx_buf;
+	xfers[1].len = 10;
+	xfers[1].rnw = 1;
+
+	ret = ioctl(fd, I3CDEV_XFER, (uintptr_t) &xfers_metadata);
+	if (ret < 0) {
+		/* ERROR HANDLING: I3C transaction failed */
+	} else {
+		/* For reads, optionally verify that the response matches expectations */
+		assert(xfers[1].len == xfers[1].actual_len);
+
+		/* For reads, iterate through response data using xfers[i].actual_len */
+	}
+
+The device can be closed when the open file descriptor is no longer required::
+
+	close(fd);
\ No newline at end of file
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 3f0ef1e27eb0..b40f3b3f63a6 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -81,6 +81,7 @@ Code  Seq#    Include File                                             Comments
 0x03  all    linux/hdreg.h
 0x04  D2-DC  linux/umsdos_fs.h                                         Dead since 2.6.11, but don't reuse these.
 0x06  all    linux/lp.h
+0x07  00-9E  linux/i3c/i3cdev.h                                        i3cdev driver for I3C subystem
 0x07  9F-D0  linux/vmw_vmci_defs.h, uapi/linux/vm_sockets.h
 0x09  all    linux/raid/md_u.h
 0x10  00-0F  drivers/char/s390/vmcp.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 81a9a02c919d..5c75a3585d23 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12350,6 +12350,13 @@ S:	Orphan
 F:	Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
 F:	drivers/i3c/master/dw*
 
+I3C CHARACTER DEVICE DRIVER
+M:	Meagan Lloyd <meaganlloyd@linux.microsoft.com>
+S:	Supported
+F:	Documentation/userspace-api/i3c/i3cdev.rst
+F:	drivers/i3c/i3cdev.c
+F:	include/uapi/linux/i3c/i3cdev.h
+
 I3C SUBSYSTEM
 M:	Alexandre Belloni <alexandre.belloni@bootlin.com>
 R:	Frank Li <Frank.Li@nxp.com>
diff --git a/drivers/i3c/Kconfig b/drivers/i3c/Kconfig
index 626c54b386d5..630dc776c464 100644
--- a/drivers/i3c/Kconfig
+++ b/drivers/i3c/Kconfig
@@ -20,6 +20,18 @@ menuconfig I3C
 	  will be called i3c.
 
 if I3C
+
+config I3CDEV
+	tristate "I3C device interface"
+	depends on I3C
+	help
+	  Say Y here to include the i3cdev driver. This allows user-space to
+	  explicitly bind I3C devices to the driver in order to expose a
+	  character device interface for the I3C device under /dev/bus/i3c/.
+
+	  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..da0e6910a4f7
--- /dev/null
+++ b/drivers/i3c/i3cdev.c
@@ -0,0 +1,730 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2026 Microsoft Corporation
+ *
+ * Author: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
+ *
+ * Based on code from:
+ *     Copyright (c) 2020 Synopsys, Inc. and/or its affiliates.
+ *     Author: Vitor Soares <vitor.soares@synopsys.com>
+ *
+ *     Author: Boris Brezillon <boris.brezillon@bootlin.com>
+ */
+
+#include <linux/cdev.h>
+#include <linux/compat.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/i3c/device.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/master.h>
+#include <linux/cleanup.h>
+
+#include <uapi/linux/i3c/i3cdev.h>
+
+#define DRV_VERSION                         "1.0.0"
+#define SYSFS_PREFIX                        "i3cdev-"
+#define SYSFS_FORMAT                        SYSFS_PREFIX "%u"
+#define MAX_I3CDEV_DEVS                     256
+#define MAX_XFERS                           256
+#define MAX_TOTAL_DATA_BYTES                256
+
+struct i3cdev_data {
+	struct i3c_device *i3c;
+	struct device dev;
+	struct cdev cdev;
+	dev_t devt;
+	/* serializes in-flight transfers, set-up, and tear-down */
+	struct mutex lock;
+	bool bound;
+	u16 mwl;
+	atomic_t open_fd;
+};
+
+enum i3c_xfer_rnw {
+	I3C_WRITE = 0,
+	I3C_READ = 1,
+};
+
+static DEFINE_IDA(i3cdev_ida);
+static dev_t base_dev_t;
+
+/* Set-up character device path */
+static char *i3cdev_devnode(const struct device *dev, umode_t *mode)
+{
+	const struct i3cdev_data *i3cdev;
+	const char *name;
+
+	i3cdev = container_of_const(dev, struct i3cdev_data, dev);
+	name = dev_name(i3cdev_to_dev(i3cdev->i3c));
+
+	return kasprintf(GFP_KERNEL, "bus/i3c/%s", name);
+}
+
+static const struct class i3cdev_class = {
+	.name = "i3cdev",
+	.devnode = i3cdev_devnode,
+};
+
+/**
+ * exceeds_mwl() - Check if a write exceeds a specified Max Write Length (MWL)
+ * @i3cdev: Pointer to i3cdev_data
+ * @xfer: Pointer to a given i3c_xfer
+ *
+ * Returns: true if @xfer length exceeds MWL, false otherwise.
+ */
+static bool exceeds_mwl(struct i3cdev_data *i3cdev, struct i3c_xfer *xfer)
+{
+	if (i3cdev->mwl && xfer->len > i3cdev->mwl) {
+		dev_dbg(&i3cdev->dev, "Requested len exceeds MWL\n");
+		return true;
+	}
+
+	return false;
+}
+
+static inline bool is_write(struct i3c_xfer *x) { return x->rnw == I3C_WRITE; }
+
+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 = i3cdev->i3c;
+	struct i3c_xfer xfer = {
+		.rnw = I3C_READ
+	};
+	size_t len;
+	char *tmp;
+	int ret;
+
+	if (!i3cdev->bound)
+		return -ENXIO;
+
+	/* File system operation of 0 bytes is effectively a valid, no-op */
+	if (!count)
+		return 0;
+
+	/* Clamp transfer length within driver limits */
+	len = min(count, MAX_TOTAL_DATA_BYTES);
+	xfer.len = len;
+
+	tmp = kzalloc(len, GFP_KERNEL);
+	if (!tmp)
+		return -ENOMEM;
+
+	xfer.data.in = tmp;
+
+	dev_dbg(&i3cdev->dev, "Reading %zu bytes\n", len);
+
+	scoped_guard(mutex, &i3cdev->lock) {
+		/* .remove was called so don't mess with the device */
+		if (!i3cdev->bound) {
+			ret = -ENXIO;
+			goto out_free_kbuf;
+		}
+
+		ret = i3c_device_do_xfers(i3c, &xfer, 1, I3C_SDR);
+		if (ret)
+			goto out_free_kbuf;
+	}
+
+	dev_dbg(&i3cdev->dev, "Received %u bytes\n", xfer.actual_len);
+
+	/* Guard against a buggy controller driver */
+	if (xfer.actual_len > len) {
+		ret = -EIO;
+		goto out_free_kbuf;
+	}
+
+	ret = copy_to_user(buf, tmp, xfer.actual_len) ? -EFAULT : xfer.actual_len;
+
+out_free_kbuf:
+	kfree(tmp);
+
+	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 = i3cdev->i3c;
+	struct i3c_xfer xfer = {
+		.rnw = I3C_WRITE
+	};
+	size_t len;
+	char *tmp;
+	int ret;
+
+	if (!i3cdev->bound)
+		return -ENXIO;
+
+	/* File system operation of 0 bytes is effectively a valid, no-op */
+	if (!count)
+		return 0;
+
+	if (exceeds_mwl(i3cdev, &xfer))
+		return -ENXIO;
+
+	/* Clamp transfer length within driver limits */
+	len = min(count, MAX_TOTAL_DATA_BYTES);
+	xfer.len = len;
+
+	tmp = memdup_user(buf, len);
+	if (IS_ERR(tmp))
+		return PTR_ERR(tmp);
+
+	xfer.data.out = tmp;
+
+	dev_dbg(&i3cdev->dev, "Writing %zu bytes\n", len);
+
+	scoped_guard(mutex, &i3cdev->lock) {
+		/* .remove was called so don't mess with the device */
+		if (!i3cdev->bound) {
+			ret = -ENXIO;
+			goto out_free_kbuf;
+		}
+
+		ret = i3c_device_do_xfers(i3c, &xfer, 1, I3C_SDR);
+		if (ret)
+			goto out_free_kbuf;
+	}
+
+out_free_kbuf:
+	kfree(tmp);
+
+	return !ret ? len : ret;
+}
+
+/* IOCTL Helper Functions */
+
+/**
+ * get_metadata() - Copy i3cdev_xfers (I3CDEV_XFER ioctl input) from user-space
+ * @i3cdev: i3cdev_data object
+ * @uxfers: User-space i3cdev_xfers object
+ * @metadata: Kernel i3cdev_xfers object into which @uxfers will be copied
+ *
+ * Returns: 0 on success, a negative error code otherwise
+ */
+static int get_metadata(struct i3cdev_data *i3cdev,
+			struct i3cdev_xfers __user *uxfers,
+			struct i3cdev_xfers *metadata)
+{
+	if (copy_from_user(metadata, uxfers, sizeof(*metadata)))
+		return -EFAULT;
+
+	if (!metadata->nxfers)
+		return -EINVAL;
+
+	/* Limit and ensure nxfers fits in an int (for i3cdev and the core) */
+	if (metadata->nxfers > MAX_XFERS || metadata->nxfers > INT_MAX) {
+		dev_dbg(&i3cdev->dev,
+			"Number of transfers exceeds driver limit\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/**
+ * get_user_xfers() - Copy array of i3cdev_xfer objects from user-space
+ * @metadata: Kernel i3cdev_xfers object (I3CDEV_XFER ioctl input)
+ *
+ * Allocates kernel memory and copies the user-space array of i3cdev_xfer
+ * objects into it. On success, the caller must free the memory.
+ *
+ * Returns: a pointer to the kernel's copy of the i3cdev_xfer array on
+ * success, an ERR_PTR otherwise.
+ */
+static struct i3cdev_xfer *get_user_xfers(struct i3cdev_xfers *metadata)
+{
+	struct i3cdev_xfer *k_uxfers, *k_uxfer;
+	u8 *uxfer;
+	int ret;
+
+	k_uxfers = kcalloc(metadata->nxfers, sizeof(*k_uxfers), GFP_KERNEL);
+	if (!k_uxfers)
+		return ERR_PTR(-ENOMEM);
+
+	k_uxfer = k_uxfers;
+	uxfer = u64_to_user_ptr(metadata->xfers);
+	for (int i = 0; i < metadata->nxfers; i++) {
+		ret = copy_struct_from_user(k_uxfer,
+					    sizeof(*k_uxfers),
+					    uxfer,
+					    metadata->xfer_size);
+		if (ret)
+			goto out_free_k_uxfers;
+
+		/* Enforce that padding must be zero */
+		if (memchr_inv(k_uxfer->pad, 0, sizeof(k_uxfer->pad))) {
+			ret = -EINVAL;
+			goto out_free_k_uxfers;
+		}
+
+		uxfer += metadata->xfer_size; /* u8 pointer so use xfer_size */
+		k_uxfer++;		      /* struct i3cdev_xfer pointer */
+	}
+
+	return k_uxfers;
+
+out_free_k_uxfers:
+	kfree(k_uxfers);
+	return ERR_PTR(ret);
+}
+
+/**
+ * ioctl_i3c_xfer_input_checks() - Checks that an anticipated transfer is valid
+ * by itself or in the context of an array of i3c_xfer objects.
+ * @i3cdev: i3cdev_data object
+ * @xfer: i3c_xfer object
+ * @prior_bytes: Number of bytes in the i3c_xfer array prior to this transfer
+ *
+ * Returns: 0 on success, a negative error code otherwise
+ */
+static int
+ioctl_i3c_xfer_input_checks(struct i3cdev_data *i3cdev,
+			    struct i3c_xfer *xfer,
+			    unsigned long prior_bytes)
+{
+	/* I3C core will error on a transfer of 0 bytes */
+	if (!xfer->len) {
+		dev_dbg(&i3cdev->dev, "Invalid transfer of zero bytes\n");
+		return -EINVAL;
+	}
+
+	if (xfer->rnw > 1) {
+		dev_dbg(&i3cdev->dev, "Invalid rnw encoding\n");
+		return -EINVAL;
+	}
+
+	if ((prior_bytes + xfer->len) > MAX_TOTAL_DATA_BYTES) {
+		dev_dbg(&i3cdev->dev, "Byte count exceeds driver limit\n");
+		return -EINVAL;
+	}
+
+	if (is_write(xfer) && exceeds_mwl(i3cdev, xfer))
+		return -ENXIO;
+
+	return 0;
+}
+
+/**
+ * i3cdev_prepare_xfers_from_user() - Prepare the i3c_xfer array
+ * @i3cdev: i3cdev_data object
+ * @metadata: Kernel's copy of i3cdev_xfers (ioctl I3CDEV_XFER input)
+ * @k_uxfers: Kernel's copy of the i3cdev_xfer array
+ * @i3c_xfers: i3c_xfer array that will be sent to the I3C core
+ * @nbufs: In/out variable representing the number of successfully allocated
+ * i3c_xfer data buffers. Use this to walk the array when freeing the memory.
+ *
+ * Returns: 0 on success, a negative error code otherwise
+ */
+static int
+i3cdev_prepare_xfers_from_user(struct i3cdev_data *i3cdev,
+			       struct i3cdev_xfers *metadata,
+			       struct i3cdev_xfer *k_uxfers,
+			       struct i3c_xfer *i3c_xfers,
+			       int *nbufs)
+{
+	int ret;
+	unsigned long total_bytes = 0;
+	void __user *udata;
+	void *data;
+
+	*nbufs = 0;
+
+	/* Prepare i3c_xfer objs to send via the I3C core */
+	for (int i = 0; i < metadata->nxfers; i++) {
+		/* Copy fields from i3cdev_xfer -> i3c_xfer */
+		i3c_xfers[i].rnw = k_uxfers[i].rnw;
+		i3c_xfers[i].len = k_uxfers[i].len;
+
+		ret = ioctl_i3c_xfer_input_checks(i3cdev, &i3c_xfers[i],
+						  total_bytes);
+		if (!ret)
+			total_bytes += i3c_xfers[i].len;
+		else
+			return ret;
+
+		if (is_write(&i3c_xfers[i])) {
+			/* Copy the data to transmit to kernel-space */
+			udata = u64_to_user_ptr(k_uxfers[i].data);
+			data = memdup_user(udata, i3c_xfers[i].len);
+		} else {
+			/* Prepare a buffer for the resulting read data */
+			data = kzalloc(i3c_xfers[i].len, GFP_KERNEL);
+			if (!data)
+				data = ERR_PTR(-ENOMEM);
+		}
+
+		if (IS_ERR(data))
+			return PTR_ERR(data);
+
+		/* Track allocated data buffers for freeing */
+		(*nbufs)++;
+
+		if (is_write(&i3c_xfers[i]))
+			i3c_xfers[i].data.out = data;
+		else
+			i3c_xfers[i].data.in = data;
+	}
+
+	return 0;
+}
+
+/**
+ * print_i3c_err() - Prints the I3C error encountered during the prior
+ * call to the core's transfer function.
+ * @i3cdev: i3cdev_data object
+ * @metadata: Kernel's copy of i3cdev_xfers (ioctl I3CDEV_XFER input)
+ * @i3c_xfers: i3c_xfer array that was sent to the I3C core
+ *
+ * Returns: void
+ */
+static void print_i3c_err(struct i3cdev_data *i3cdev,
+			  struct i3cdev_xfers *metadata,
+			  struct i3c_xfer *i3c_xfers)
+{
+	for (int i = 0; i < metadata->nxfers; i++) {
+		/* Valid errors, e.g. M0 (now called C0) start at 1 */
+		if (i3c_xfers[i].err > 0) {
+			dev_warn(&i3cdev->dev,
+				 "I3C error encountered: C%u\n",
+				 i3c_xfers[i].err - 1);
+			return;
+		}
+	}
+}
+
+/**
+ * i3cdev_copy_results_to_user() - Copy results to user-space
+ * @metadata: Kernel's copy of i3cdev_xfers (ioctl I3CDEV_XFER input)
+ * @k_uxfers: Kernel's copy of i3cdev_xfer array
+ * @i3c_xfers: i3c_xfer array that was sent to the I3C core
+ *
+ * Returns: 0 on success, a negative error code otherwise
+ */
+static int
+i3cdev_copy_results_to_user(struct i3cdev_xfers *metadata,
+			    struct i3cdev_xfer *k_uxfers,
+			    struct i3c_xfer *i3c_xfers)
+{
+	u8 __user *uxfer, *uactual_len;
+	void __user *udata;
+	__u16 nbytes;
+
+	uxfer = u64_to_user_ptr(metadata->xfers);
+	for (int i = 0; i < metadata->nxfers; i++, uxfer += metadata->xfer_size) {
+		if (is_write(&i3c_xfers[i])) {
+			continue;
+		} else {
+			udata = u64_to_user_ptr(k_uxfers[i].data);
+			nbytes = i3c_xfers[i].actual_len;
+
+			/* Guard against a buggy controller driver */
+			if (nbytes > k_uxfers[i].len)
+				return -EIO;
+
+			/* Copy over the read response data */
+			if (copy_to_user(udata, i3c_xfers[i].data.in, nbytes))
+				return -EFAULT;
+
+			/* Copy over actual_len */
+			uactual_len = uxfer + offsetof(struct i3cdev_xfer, actual_len);
+			if (copy_to_user(uactual_len, &nbytes, sizeof(__u16)))
+				return -EFAULT;
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * i3cdev_ioctl_do_xfers() - Implementing function of the I3CDEV_XFER IOCTL
+ * @i3cdev: i3cdev_data object
+ * @uxfers: User-space pointer to I3CDEV_XFER IOCTL input (struct i3cdev_xfers)
+ *
+ * Performs the requested SDR transfers and copies the results to user-space.
+ *
+ * Returns: 0 on success, negative error code otherwise.
+ */
+static int
+i3cdev_ioctl_do_xfers(struct i3cdev_data *i3cdev,
+		      struct i3cdev_xfers __user *uxfers)
+{
+	struct i3c_device *i3c = i3cdev->i3c;
+	struct i3cdev_xfers metadata;
+	struct i3cdev_xfer *k_uxfers;
+	struct i3c_xfer *i3c_xfers;
+	int nbufs;
+	int ret;
+
+	ret = get_metadata(i3cdev, uxfers, &metadata);
+	if (ret)
+		return ret;
+
+	k_uxfers = get_user_xfers(&metadata);
+	if (IS_ERR(k_uxfers))
+		return PTR_ERR(k_uxfers);
+
+	i3c_xfers = kcalloc(metadata.nxfers, sizeof(*i3c_xfers), GFP_KERNEL);
+	if (!i3c_xfers) {
+		ret = -ENOMEM;
+		goto out_free_k_uxfers;
+	}
+
+	/* Prepare i3c_xfer objs to send via the I3C core */
+	ret = i3cdev_prepare_xfers_from_user(i3cdev, &metadata, k_uxfers,
+					     i3c_xfers, &nbufs);
+	if (ret)
+		goto out_free_i3c_xfers;
+
+	scoped_guard(mutex, &i3cdev->lock) {
+		/* .remove was called so don't mess with the device */
+		if (!i3cdev->bound) {
+			ret = -ENXIO;
+			goto out_free_i3c_xfers;
+		}
+		ret = i3c_device_do_xfers(i3c, i3c_xfers,
+					  metadata.nxfers, I3C_SDR);
+		if (ret) {
+			print_i3c_err(i3cdev, &metadata, i3c_xfers);
+			goto out_free_i3c_xfers;
+		}
+	}
+
+	ret = i3cdev_copy_results_to_user(&metadata, k_uxfers, i3c_xfers);
+
+out_free_i3c_xfers:
+	for (int i = 0; i < nbufs; i++)
+		kfree(i3c_xfers[i].data.in);
+	kfree(i3c_xfers);
+
+out_free_k_uxfers:
+	kfree(k_uxfers);
+
+	return ret;
+}
+
+static long
+i3cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	struct i3cdev_data *i3cdev = file->private_data;
+	void __user *udata = (void __user *)arg;
+	int ret;
+
+	if (!i3cdev->bound)
+		return -ENXIO;
+
+	dev_dbg(&i3cdev->dev, "ioctl, cmd=0x%02x, arg=0x%02lx\n", cmd, arg);
+
+	switch (cmd) {
+	case I3CDEV_XFER:
+		ret = i3cdev_ioctl_do_xfers(i3cdev, udata);
+		break;
+	default:
+		return -ENOTTY;
+	}
+
+	return ret;
+}
+
+static int i3cdev_open(struct inode *inode, struct file *file)
+{
+	struct i3cdev_data *i3cdev;
+
+	i3cdev = container_of_const(inode->i_cdev, struct i3cdev_data, cdev);
+
+	/* Let probe finish cdev_device_add */
+	scoped_guard(mutex, &i3cdev->lock)
+		if (!i3cdev->bound)
+			return -ENXIO;
+
+	/*
+	 * Some devices have pointer or page registers where programming
+	 * it can change what an address refers to, avoid this by allowing
+	 * only one process to interact with the device.
+	 */
+	if (atomic_cmpxchg_relaxed(&i3cdev->open_fd, 0, 1))
+		return -EBUSY;
+
+	file->private_data = i3cdev;
+
+	return 0;
+}
+
+static int i3cdev_release(struct inode *inode, struct file *file)
+{
+	struct i3cdev_data *i3cdev = file->private_data;
+
+	atomic_set(&i3cdev->open_fd, 0);
+
+	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 void free_i3cdev_data(struct device *d)
+{
+	struct i3cdev_data *i3cdev = container_of(d, struct i3cdev_data, dev);
+	struct device *i3c_device_dev = i3cdev_to_dev(i3cdev->i3c);
+
+	kfree(i3cdev);
+	/* Release the reference to the underlying device */
+	put_device(i3c_device_dev);
+}
+
+static int i3cdev_probe(struct i3c_device *i3c)
+{
+	struct i3cdev_data *i3cdev;
+	struct device *i3c_device_dev = i3cdev_to_dev(i3c);
+	int minor, ret;
+	struct i3c_device_info info;
+
+	i3cdev = kzalloc_obj(*i3cdev);
+	if (!i3cdev)
+		return -ENOMEM;
+
+	minor = ida_alloc_range(&i3cdev_ida, MINOR(base_dev_t),
+				MAX_I3CDEV_DEVS - 1, GFP_KERNEL);
+	if (minor < 0) {
+		dev_err(i3c_device_dev, "Not able to reserve a minor\n");
+		kfree(i3cdev);
+		return minor;
+	}
+
+	i3cdev->devt = MKDEV(MAJOR(base_dev_t), minor);
+
+	i3cdev->i3c = i3c;
+	i3cdev_set_drvdata(i3c, i3cdev);
+	/* Pin the underlying device as long as i3cdev lives */
+	get_device(i3c_device_dev);
+
+	i3c_device_get_info(i3c, &info);
+	i3cdev->mwl = info.max_write_len;
+
+	mutex_init(&i3cdev->lock);
+	atomic_set(&i3cdev->open_fd, 0);
+
+	i3cdev->dev.parent = i3c_device_dev;
+	i3cdev->dev.devt = i3cdev->devt;
+	i3cdev->dev.class = &i3cdev_class;
+	i3cdev->dev.release = free_i3cdev_data;
+	ret = dev_set_name(&i3cdev->dev, SYSFS_FORMAT, MINOR(i3cdev->devt));
+	if (ret)
+		goto error_free_ida;
+
+	device_initialize(&i3cdev->dev);
+
+	cdev_init(&i3cdev->cdev, &i3cdev_fops);
+	i3cdev->cdev.owner = THIS_MODULE;
+
+	scoped_guard(mutex, &i3cdev->lock) {
+		ret = cdev_device_add(&i3cdev->cdev, &i3cdev->dev);
+		if (ret)
+			goto error_cleanup;
+
+		i3cdev->bound = true;
+	}
+
+	return 0;
+
+error_cleanup:
+	put_device(&i3cdev->dev);
+
+error_free_ida:
+	ida_free(&i3cdev_ida, minor);
+
+	return ret;
+}
+
+static void i3cdev_remove(struct i3c_device *i3c)
+{
+	struct i3cdev_data *i3cdev;
+
+	i3cdev = i3cdev_get_drvdata(i3c);
+
+	/* via the lock, allow any work impacting the system to complete */
+	scoped_guard(mutex, &i3cdev->lock) {
+		/* signal to fops that the device is no longer managed */
+		i3cdev->bound = false;
+		cdev_device_del(&i3cdev->cdev, &i3cdev->dev);
+	}
+
+	ida_free(&i3cdev_ida, MINOR(i3cdev->devt));
+	put_device(&i3cdev->dev);
+}
+
+static const struct i3c_device_id i3cdev_ids[] = {
+	{ /* Sentinel */ },
+};
+
+static struct i3c_driver i3cdev_driver = {
+	.probe = i3cdev_probe,
+	.remove = i3cdev_remove,
+	.id_table = i3cdev_ids,
+	.driver = {
+		.name = "i3cdev",
+	}
+};
+
+static int __init i3cdev_init(void)
+{
+	int ret;
+
+	ret = alloc_chrdev_region(&base_dev_t, 0,
+				  MAX_I3CDEV_DEVS, "i3cdev");
+	if (ret)
+		return ret;
+
+	ret = class_register(&i3cdev_class);
+	if (ret)
+		goto err_unreg_chrdev_region;
+
+	ret = i3c_driver_register(&i3cdev_driver);
+	if (ret)
+		goto err_unregister_class;
+
+	return 0;
+
+err_unregister_class:
+	class_unregister(&i3cdev_class);
+
+err_unreg_chrdev_region:
+	unregister_chrdev_region(base_dev_t, MAX_I3CDEV_DEVS);
+
+	return ret;
+}
+module_init(i3cdev_init);
+
+static void __exit i3cdev_exit(void)
+{
+	i3c_driver_unregister(&i3cdev_driver);
+	class_unregister(&i3cdev_class);
+	unregister_chrdev_region(base_dev_t, MAX_I3CDEV_DEVS);
+	ida_destroy(&i3cdev_ida);
+}
+module_exit(i3cdev_exit);
+
+MODULE_AUTHOR("Meagan Lloyd <meaganlloyd@linux.microsoft.com>");
+MODULE_DESCRIPTION("I3C Character Device Driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRV_VERSION);
diff --git a/include/uapi/linux/i3c/i3cdev.h b/include/uapi/linux/i3c/i3cdev.h
new file mode 100644
index 000000000000..ed72d7455f31
--- /dev/null
+++ b/include/uapi/linux/i3c/i3cdev.h
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (c) 2026 Microsoft Corporation
+ *
+ * Author: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
+ *
+ * Based on code from:
+ *     Copyright (c) 2020 Synopsys, Inc. and/or its affiliates.
+ *     Author: Vitor Soares <vitor.soares@synopsys.com>
+ *
+ *     Author: Boris Brezillon <boris.brezillon@bootlin.com>
+ */
+
+#ifndef _UAPI_I3C_DEV_H_
+#define _UAPI_I3C_DEV_H_
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+/**
+ * Reserved ioctl encoding for i3cdev
+ * Code 0x7, Seq# 0x0-0x9E
+ */
+#define I3CDEV_IOCTL_ID	0x07
+
+/**
+ * struct i3cdev_xfer - I3C transfer (read/write)
+ *
+ * This struct more-or-less mirrors the kernel's 'struct i3c_xfer'.
+ * Currently, only Single Data Rate (SDR) transfers are supported.
+ *
+ * @data: Pointer to userspace buffer. For writes, this will be the
+ * bytes to send to the Target. For reads, this buffer will be
+ * populated with the read response from the Target.
+ * @actual_len: Where the kernel will report the number of processed
+ * bytes. For reads, this reflects the number of response bytes in the
+ * @data buffer. For SDR writes, the user shouldn't use this member
+ * as it's neither supported nor useful.
+ * @len: Length of input @data buffer in bytes.
+ * @rnw: Transfer direction. 1 for a read, 0 for a write
+ * @pad: Used to eliminate implicit, undefined-value padding. Zero
+ * these bytes without referencing this field (for compatibility).
+ */
+struct i3cdev_xfer {
+	__u64 data;
+	__u16 actual_len; /* output */
+	__u16 len;
+	union {
+		__u8 rnw; /* SDR */
+		__u8 cmd; /* Not currently supported (HDR) */
+	};
+	__u8 pad[3];
+};
+
+/**
+ * struct i3cdev_xfers - I3C transfers
+ * @nxfers: Number of i3cdev_xfer objs in @xfers
+ * @xfers: Pointer to an array of i3cdev_xfer objs
+ * @xfer_size: sizeof(struct i3cdev_xfer)
+ */
+struct i3cdev_xfers {
+	__u64 nxfers;
+	__u64 xfers;
+	__u64 xfer_size;
+};
+
+#define I3CDEV_XFER \
+	_IOWR(I3CDEV_IOCTL_ID, 0, struct i3cdev_xfers)
+
+#endif
-- 
2.49.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 3/3] i3c: add i3cdev character device module for user-space access

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] Max Write Length (MWL) check is bypassed in `i3cdev_write` due to uninitialized length field.
- [Medium] Mixing goto-based error handling and `scoped_guard()` declarations in the same function violates the cleanup subsystem guidelines.
- [Medium] Memory and reference count leak on the `dev_set_name()` error path in `i3cdev_probe()`.
- [Medium] Un-ratelimited kernel logging in `print_i3c_err` allows unprivileged users to flood dmesg.
--

commit 691a30342eecac1b5bf88181bfab4ade443f2923
Author: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
Subject: i3c: add i3cdev character device module for user-space access

This commit introduces a character device driver for I3C devices. It allows
user-space applications to perform basic SDR reads and writes by binding
devices manually via sysfs driver_override.

> diff --git a/drivers/i3c/i3cdev.c b/drivers/i3c/i3cdev.c
> new file mode 100644
> index 0000000000000..da0e6910a4f72
> --- /dev/null
> +++ b/drivers/i3c/i3cdev.c

[ ... ]

> +static ssize_t
> +i3cdev_read(struct file *file, char __user *buf, size_t count, loff_t *f_pos)
> +{

[ ... ]

> +	scoped_guard(mutex, &i3cdev->lock) {
> +		/* .remove was called so don't mess with the device */
> +		if (!i3cdev->bound) {
> +			ret = -ENXIO;
> +			goto out_free_kbuf;
> +		}
> +
> +		ret = i3c_device_do_xfers(i3c, &xfer, 1, I3C_SDR);
> +		if (ret)
> +			goto out_free_kbuf;
> +	}

[Severity: Medium]
Does this mix goto-based error handling with scoped_guard() in the same
function?

The VFS/cleanup guidelines indicate that mixing goto-based error labels with
scope-based cleanups like scoped_guard() creates confusing ownership semantics
and increases the risk of double-free or resource leaks. Since the function
uses a goto label for freeing the memory buffer out_free_kbuf, could we
either convert the buffer to use a scope-based __free() or use traditional
mutex lock/unlock operations?

[ ... ]

> +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 = i3cdev->i3c;
> +	struct i3c_xfer xfer = {
> +		.rnw = I3C_WRITE
> +	};
> +	size_t len;
> +	char *tmp;
> +	int ret;
> +
> +	if (!i3cdev->bound)
> +		return -ENXIO;
> +
> +	/* File system operation of 0 bytes is effectively a valid, no-op */
> +	if (!count)
> +		return 0;
> +
> +	if (exceeds_mwl(i3cdev, &xfer))
> +		return -ENXIO;
> +
> +	/* Clamp transfer length within driver limits */
> +	len = min(count, MAX_TOTAL_DATA_BYTES);
> +	xfer.len = len;

[Severity: Medium]
Is the Max Write Length (MWL) check bypassed here due to an uninitialized
length field?

At the point where exceeds_mwl() is called, xfer.len is 0 because only .rnw
was initialized in the struct declaration. The actual xfer.len is assigned
after the check, meaning the MWL validation will always pass regardless of the
requested size. This could allow hardware to receive writes exceeding its
supported maximum length. Should the length clamp and assignment happen before
the validation check?

[ ... ]

> +	scoped_guard(mutex, &i3cdev->lock) {
> +		/* .remove was called so don't mess with the device */
> +		if (!i3cdev->bound) {
> +			ret = -ENXIO;
> +			goto out_free_kbuf;
> +		}
> +
> +		ret = i3c_device_do_xfers(i3c, &xfer, 1, I3C_SDR);
> +		if (ret)
> +			goto out_free_kbuf;
> +	}

[Severity: Medium]
Similar to i3cdev_read(), does this mix goto-based error handling with
scoped_guard() in the write path?

[ ... ]

> +static void print_i3c_err(struct i3cdev_data *i3cdev,
> +			  struct i3cdev_xfers *metadata,
> +			  struct i3c_xfer *i3c_xfers)
> +{
> +	for (int i = 0; i < metadata->nxfers; i++) {
> +		/* Valid errors, e.g. M0 (now called C0) start at 1 */
> +		if (i3c_xfers[i].err > 0) {
> +			dev_warn(&i3cdev->dev,
> +				 "I3C error encountered: C%u\n",
> +				 i3c_xfers[i].err - 1);
> +			return;
> +		}
> +	}
> +}

[Severity: Medium]
Could this un-ratelimited kernel logging allow unprivileged users to flood
the dmesg log?

Since this error is triggered directly by user-supplied transfers via the
ioctl interface, a user-space application could repeatedly trigger hardware
errors to intentionally spam the kernel logs. Could we use a rate-limited
variant like dev_warn_ratelimited() here to prevent a potential local DoS?

[ ... ]

> +static int
> +i3cdev_ioctl_do_xfers(struct i3cdev_data *i3cdev,
> +		      struct i3cdev_xfers __user *uxfers)
> +{

[ ... ]

> +	scoped_guard(mutex, &i3cdev->lock) {
> +		/* .remove was called so don't mess with the device */
> +		if (!i3cdev->bound) {
> +			ret = -ENXIO;
> +			goto out_free_i3c_xfers;
> +		}
> +		ret = i3c_device_do_xfers(i3c, i3c_xfers,
> +					  metadata.nxfers, I3C_SDR);
> +		if (ret) {
> +			print_i3c_err(i3cdev, &metadata, i3c_xfers);
> +			goto out_free_i3c_xfers;
> +		}
> +	}

[Severity: Medium]
As in the read/write paths, does this also improperly mix scoped_guard()
with goto labels in the ioctl handler?

[ ... ]

> +static int i3cdev_probe(struct i3c_device *i3c)
> +{

[ ... ]

> +	i3cdev->dev.parent = i3c_device_dev;
> +	i3cdev->dev.devt = i3cdev->devt;
> +	i3cdev->dev.class = &i3cdev_class;
> +	i3cdev->dev.release = free_i3cdev_data;
> +	ret = dev_set_name(&i3cdev->dev, SYSFS_FORMAT, MINOR(i3cdev->devt));
> +	if (ret)
> +		goto error_free_ida;

[Severity: Medium]
Does this error path leak the i3cdev memory structure and leave an
unbalanced reference count on the underlying I3C device?

If dev_set_name() fails, execution jumps directly to error_free_ida, skipping
the release of i3cdev memory and the device reference acquired earlier in
probe. Should this instead call put_device(&i3cdev->dev) so that
free_i3cdev_data is executed to handle the cleanup properly?

[ ... ]

> +	scoped_guard(mutex, &i3cdev->lock) {
> +		ret = cdev_device_add(&i3cdev->cdev, &i3cdev->dev);
> +		if (ret)
> +			goto error_cleanup;
> +
> +		i3cdev->bound = true;
> +	}

[Severity: Medium]
Does this sequence mix scoped_guard() with a goto error cleanup label?

As with the other functions in this patch, this mixes scoped cleanup with
goto statements, which goes against the VFS/cleanup guidelines.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911210935.1353126-1-meaganlloyd@linux.microsoft.com?part=3

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 3/3] i3c: add i3cdev character device module for user-space access

[Randy Dunlap]

On 9/11/26 2:09 PM, Meagan Lloyd wrote:
> The i3cdev driver is a character device driver that allows user-space
> to control and interact with I3C devices.
> 
> Currently, it has the ability to perform Single Data Rate (SDR)
> transfers - basic reads/writes.
> 
> With the addition of sysfs driver_override, there is now a
> straightforward and direct way to match the i3cdev driver to any i3c
> device without stepping on the toes of more specialized drivers that are
> loaded automatically.
> 
> This is accomplished by the i3cdev driver not having any entries in the
> i3c_device_id table. After boot, simply set the driver_override to
> "i3cdev" and bind the device manually via the sysfs bind knob. This can
> also be automated with udev rules as well.
> 
> The character device interface will be exposed at:
> /dev/bus/i3c/<bus id>-<Provisional ID>
> 
> Signed-off-by: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
> Assisted-by: Claude:claude-opus-4.8 copilot
> ---
>  Documentation/userspace-api/i3c/i3cdev.rst    | 152 ++++
>  .../userspace-api/ioctl/ioctl-number.rst      |   1 +
>  MAINTAINERS                                   |   7 +
>  drivers/i3c/Kconfig                           |  12 +
>  drivers/i3c/Makefile                          |   1 +
>  drivers/i3c/i3cdev.c                          | 730 ++++++++++++++++++
>  include/uapi/linux/i3c/i3cdev.h               |  70 ++
>  7 files changed, 973 insertions(+)
>  create mode 100644 Documentation/userspace-api/i3c/i3cdev.rst
>  create mode 100644 drivers/i3c/i3cdev.c
>  create mode 100644 include/uapi/linux/i3c/i3cdev.h
> 
> diff --git a/Documentation/userspace-api/i3c/i3cdev.rst b/Documentation/userspace-api/i3c/i3cdev.rst
> new file mode 100644
> index 000000000000..ad246bf28919
> --- /dev/null
> +++ b/Documentation/userspace-api/i3c/i3cdev.rst
> @@ -0,0 +1,152 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +====================
> +I3C Device Interface
> +====================
> +
> +This interface allows access to I3C devices from userspace.
> +
> +As I3C devices adhere to the I3C protocol, simple transfers can be accomplished
> +using a generic driver. Currently, this interface supports private Single Data
> +Rate (SDR) read and write transfers.
> +
> +The i3cdev module will not auto-bind to devices. Userspace needs to explicitly
> +bind the device to the driver. This is to avoid interfering with the binding of
> +specialized drivers.
> +
> +Once bound, a character device interface will be created at:
> +/dev/bus/i3c/<bus id>-<Provisional ID>.
> +
> +====================
> +Usage
> +====================
> +
> +Any discovered I3C devices by the I3C subsystem will have device folders under /sys/bus/i3c/devices/<bus id>-<Provisional ID>.
> +To allow binding of a device with i3cdev driver, set the driver_override:
> +
> +::
> +
> +    # echo "i3cdev" > /sys/bus/i3c/devices/<bus id>-<Provisional ID>/driver_override
> +
> +If the i3cdev driver is not yet loaded, load it and it will cause the driver to bind
> +to any devices with the override in place.
> +
> +If the i3cdev driver is already loaded, go ahead and perform a manual bind:
> +
> +::
> +
> +    # echo "<bus id>-<Provisional ID>" > /sys/bus/i3c/drivers/i3cdev/bind
> +
> +Set driver override
> +::
> +
> +    # echo "i3cdev" > /sys/bus/i3c/devices/0-deadbeef001/driver_override
> +    # echo "i3cdev" > /sys/bus/i3c/devices/0-deadbeef002/driver_override
> +
> +Bind the device to the driver
> +::
> +
> +    # echo "0-deadbeef001" > /sys/bus/i3c/drivers/i3cdev/bind
> +    # echo "0-deadbeef002" > /sys/bus/i3c/drivers/i3cdev/bind
> +
> +Observe the resulting character device files under /dev/bus/i3c/
> +::
> +
> +    # ls -ltr /dev/bus/i3c/
> +    crw-------    1 root     root      235,   1 Jun 30 17:49 0-deadbeef002
> +    crw-------    1 root     root      235,   0 Jun 30 17:49 0-deadbeef001
> +
> +BASIC CHARACTER DEVICE API
> +===============================
> +The API supports private Single Data Rate (SDR) read and write transfers.
> +Those transaction can be achieved by the following:
> +
> +``read(file, buffer, sizeof(buffer))``
> +  The standard read() operation will work as a simple transaction of private
> +  SDR read data followed a stop.
> +  Return the number of bytes read on success, and a negative error otherwise.
> +
> +``write(file, buffer, sizeof(buffer))``
> +  The standard write() operation will work as a simple transaction of private
> +  SDR write data followed a stop.
> +  Return the number of bytes written on success, and a negative error otherwise.
> +
> +``ioctl(file, I3CDEV_XFER, struct i3cdev_xfers *xfers)``
> +  It combines read/write transactions without a stop in between.
> +  Return 0 on success, and a negative error otherwise.
> +
> +C EXAMPLE (PSEUDO CODE)
> +=======================
> +You need to open (and get a file descriptor) to /dev/bus/i3c/<bus id>-<Provisional ID>,
> +do your operations (read, write, ioctl), and then close it.
> +
> +The following header files should be included in an I3C program::
> +
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <sys/ioctl.h>
> +#include <i3c/i3cdev.h>
> +#include <stdint.h>
> +
> +These additionally are used by this example::
> +
> +#include <stdlib.h>
> +#include <assert.h>
> +
> +To work with an I3C device, one must call open and get a file descriptor::
> +
> +	int fd;
> +
> +	fd = open("/dev/bus/i3c/0-deadbeef001", O_RDWR);
> +	if (fd < 0)
> +		exit(EXIT_FAILURE);
> +
> +Now that the file is open, we can do some operations::
> +
> +	int ret;
> +
> +	/* Write function */
> +	uint8_t  buf[] = {0x00, 0xde, 0xad, 0xbe, 0xef};
> +	ret = write(fd, buf, 5);
> +	if (ret != 5) {
> +		/* ERROR HANDLING: I3C transaction failed */
> +	}
> +
> +	/*  Read function */
> +	ret = read(fd, buf, 4);
> +	if (ret < 0) {
> +		/* ERROR HANDLING: I3C transaction failed */
> +	} else {
> +		/* Iterate over buf[] to get the read data */
> +	}
> +
> +	/* IOCTL function */
> +	struct i3cdev_xfer xfers[2] = {0}; /* Must zero out for compatibility */
> +	struct i3cdev_xfers xfers_metadata = { .nxfers = 2,
> +					       .xfers = (uintptr_t) xfers,
> +					       .xfer_size = sizeof(struct i3cdev_xfer)
> +					     };
> +
> +	uint8_t tx_buf[] = {0x00, 0xde, 0xad, 0xbe, 0xef};
> +	uint8_t rx_buf[10];
> +
> +	xfers[0].data = (uintptr_t) tx_buf;
> +	xfers[0].len = 5;
> +	xfers[0].rnw = 0;
> +	xfers[1].data = (uintptr_t) rx_buf;
> +	xfers[1].len = 10;
> +	xfers[1].rnw = 1;
> +
> +	ret = ioctl(fd, I3CDEV_XFER, (uintptr_t) &xfers_metadata);
> +	if (ret < 0) {
> +		/* ERROR HANDLING: I3C transaction failed */
> +	} else {
> +		/* For reads, optionally verify that the response matches expectations */
> +		assert(xfers[1].len == xfers[1].actual_len);
> +
> +		/* For reads, iterate through response data using xfers[i].actual_len */
> +	}
> +
> +The device can be closed when the open file descriptor is no longer required::
> +
> +	close(fd);
> \ No newline at end of file

Fix the warning above, please.
Also:
Documentation/userspace-api/i3c/i3cdev.rst: WARNING: document isn't included in any toctree [toc.not_included]

add that file to the table of contents (somewhere).

thanks.
-- 
~Randy


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 3/3] i3c: add i3cdev character device module for user-space access

[Andy Shevchenko]

On Fri, Sep 11, 2026 at 02:09:35PM -0700, Meagan Lloyd wrote:
> The i3cdev driver is a character device driver that allows user-space
> to control and interact with I3C devices.

> Currently, it has the ability to perform Single Data Rate (SDR)
> transfers - basic reads/writes.
> 
> With the addition of sysfs driver_override, there is now a
> straightforward and direct way to match the i3cdev driver to any i3c
> device without stepping on the toes of more specialized drivers that are
> loaded automatically.

Is it safe? Why on the earth do we need this? The commit message has not enough
information.

> This is accomplished by the i3cdev driver not having any entries in the
> i3c_device_id table. After boot, simply set the driver_override to
> "i3cdev" and bind the device manually via the sysfs bind knob. This can
> also be automated with udev rules as well.
> 
> The character device interface will be exposed at:
> /dev/bus/i3c/<bus id>-<Provisional ID>

...

> +	struct i3c_xfer xfer = {
> +		.rnw = I3C_WRITE

In such cases always leave a trailing comma. It will reduce possible churn in
the future.

> +	};

...

> +	return !ret ? len : ret;

My gosh, wouldn't Elvis just work naturally?

	return ret ?: len;

...

> +	for (int i = 0; i < metadata->nxfers; i++) {

Why is 'i' signed?

> +		ret = copy_struct_from_user(k_uxfer,
> +					    sizeof(*k_uxfers),
> +					    uxfer,
> +					    metadata->xfer_size);
> +		if (ret)
> +			goto out_free_k_uxfers;
> +
> +		/* Enforce that padding must be zero */
> +		if (memchr_inv(k_uxfer->pad, 0, sizeof(k_uxfer->pad))) {
> +			ret = -EINVAL;
> +			goto out_free_k_uxfers;
> +		}
> +
> +		uxfer += metadata->xfer_size; /* u8 pointer so use xfer_size */
> +		k_uxfer++;		      /* struct i3cdev_xfer pointer */
> +	}

...

> +		if (!ret)
> +			total_bytes += i3c_xfers[i].len;
> +		else
> +			return ret;

Yeah, you really need to reconsider patterns you use in the code. Here 'else'
is redundant. Homework to understand how (#easy).

...

> +/**
> + * print_i3c_err() - Prints the I3C error encountered during the prior
> + * call to the core's transfer function.
> + * @i3cdev: i3cdev_data object
> + * @metadata: Kernel's copy of i3cdev_xfers (ioctl I3CDEV_XFER input)
> + * @i3c_xfers: i3c_xfer array that was sent to the I3C core

> + * Returns: void

Huh?! Where is this coming from?

> + */

...

Please, rely less on AI and more on the common sense and proof-reading.

-- 
With Best Regards,
Andy Shevchenko



-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 0/3] I3C character device driver using driver_override

[Andy Shevchenko]

On Fri, Sep 11, 2026 at 02:09:32PM -0700, Meagan Lloyd wrote:
> This is a rework and revival option for Vitor Soares' I3C character
> device driver patch series from 2020 [1] that I've been exploring for a
> few months.  Recently there was a revival posted to the list [2], so I
> wanted to share this design option as well.
> 
> In [1] and [2], the i3cdev driver automatically attaches and detaches
> depending whether another driver has attached/not. In [1], Boris was
> suggesting we explore a more straightforward and traditional binding
> method aligning with the Linux driver model. At the time, there wasn't
> a way to auto-bind while keeping manual binding possible as they shared
> the same match() hook. Now with the new driver_override feature,

Where is it new? It's quite an old mechanism in the driver core...

> the auto-binding of i3cdev on boot can be avoided if the i3cdev driver has
> an empty match ID table. After boot, where specialized drivers would have
> already bound, user-space can explicitly opt-in by setting the
> driver_override sysfs file with 'i3cdev' and manually binding via sysfs
> (or by simply loading the driver if it's loadable). This can also be
> easily automated with udev rules that run whenever the I3C core exposes
> a new device.
> 
> One downside of the automatic attach/de-attach is that if a different
> driver is loaded later, the first driver could have altered something
> on the device, breaking any assumptions of the subsequent driver.
> 
> My series builds on [1] through:
> 0. Addressing code review feedback in [1] from Greg, Boris, and Randy.
> 1. Using actual_len for accurate read response reporting. The kernel
> will report actual_len received from the core to user-space via the
> uapi i3cdev_xfer struct.
> 2. Placing limits on the number of transfers and bytes in requests to
> prevent unlimited-sized transfers or kernel memory allocation
> 3. Checking inputs and descriptive return codes as guard-rails
> for user-space and to ease use of the i3cdev driver
> 4. Checking on MWL to ensure that we respect device limits
> 5. Proper lifetime management of i3cdev_data and underlying device
> 6. Addressing dangling fops in the event we have an open file descriptor
> when a device gets unbound.
> 7. Fast-path locking to ensure transfers complete before a device is
> unbound.
> 8. Allowing only one file descriptor per I3C device to avoid bugs
> around multiple processes interacting with the device and altering
> the device underneath the other. For example, without this, one process
> could change the device's page or address pointer register underneath
> the other process.
> 9. copy_struct_from_user to ensure struct i3cdev_xfer could be extended
> in a compatible way. This is to be forward-looking towards potential
> HDR mode expansion and code reuse.
> 10. Reserving the IOCTL number formally
> 11. Updating the Documentation to be a syntax correct example program
> template.
> 12. Preserving /dev/bus/i3c/<bus id>-<Provisional ID> naming while
> allowing sysfs path to be neatly named i3cdev-<minor>. This avoids
> repeated <bus id>-<Provisional ID> in the sysfs paths which can be
> confusing/circular-looking.
>     e.g. /sys/bus/i3c/devices/0-deadbeef001/i3cdev/0-deadbeef001 ->
>          /sys/bus/i3c/devices/0-deadbeef001/i3cdev/i3cdev-0
> 13. Updating all naming references related to i3c_priv_xfer to align
> with new i3c_xfer struct
> 14. Updating the MAINTAINERS file for the new pieces of code
> 
> Note that i3c-tools [3] or a fork of it will need small updates:
> 1. Update include/uapi/linux/i3c/i3cdev.h to match updated uapi structs
> 2. In i3ctransfer.c, use actual_len for reads

> I've added MODULE_VERSION("1.0.0") in the i3cdev driver, so i3c-tools
> could use that to determine whether to use the old out-of-tree uapi or this one.

Absolutely no. This is legacy macro which has no need since Git era. In Git
the module version is the Git SHA hash of the tip of the used tree. Nobody will
understand what 1.0.0 means and how it maps to the applied patches (if any of
them affects the behaviour of the feature in question).

On top of that, upstream has no clue what and how many possible custom ABIs /
UAPIs exists, and we do not care, to be honest.

> [1] https://lore.kernel.org/linux-i3c/cover.1582069402.git.vitor.soares@synopsys.com/
> [2] https://lore.kernel.org/linux-i3c/ap_1-gFF7S821xJT@ninjato/T/#m9107c1785a4a16b1b3cb84c3269d17fac35819c8
> [3] https://github.com/vitor-soares-snps/i3c-tools

-- 
With Best Regards,
Andy Shevchenko



-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c