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

[Andy Shevchenko]

On Wed, Sep 16, 2026 at 03:57:10PM -0700, Meagan Lloyd wrote:
> On Sat, Sep 12, 2026 at 04:34:01PM +0300, Andy Shevchenko wrote:
> > 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.
> 
> I can't see a reason that it'd be unsafe. To give additional confidence,
> it's already in-use in many bus_types:

This argument has nothing to do with i³c. Each bus is different on a physical
layer, electrical protocols and programming flow. Each of them has own
constraints.

> To answer why we need it:
> If we want to write i3cdev as a standard device driver, it can't
> actually match anything by default. This is because, some devices on the
> system may need specific drivers and i3cdev is generic and should
> technically match every device.

Yes, but I have seen no reason why we should expose i³c bus to the user
space. With i²c we already know very well that it was (and still is)
a bad idea. Why i³c is better (especially taking into account i²c
compatible mode and more complex programming flow)?

> Since the driver_override is default NULL and is set via sysfs, this
> allows any specific drivers on boot to be loaded up and would allow
> explicit control on what device i3cdev gets bound to.
> 
> This was my rational. I will refine the commit message with more details.

Put a real life example why the exposing i³c devices into user space is
absolutely necessary.

> > > 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>

...

> > > +	for (int i = 0; i < metadata->nxfers; i++) {
> > 
> > Why is 'i' signed?

> Mostly for readability and to make sure the line length on loop headers
> is kept below 80 chars. As a precaution, to make sure that 'i' can
> represent any metadata->nxfers value without overflow during loops, I
> check that metadata->nxfers is less than/equal to INT_MAX in
> get_metadata().

No need to add useless checks.

...

> > > +/** + * 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?
> 
> In i3cdev_ioctl_do_xfers, if i3c_device_do_xfers failed, I wanted to
> print out the first I3C controller error encountered. The controller
> drivers can set this in the i3c_xfer.err field. Hence this function.
> 
> It's to aid debugging and provide useful error information.
> I can certainly refine the wording on the print_i3c_err documentation
> header to make this more clear.

My point is about kernel-doc. Why do we need the return section for void?
Where it comes from?

> > > + */

...

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

> I think I gave you the wrong impression. The new contributions in this
> series were written and developed by me. I used AI for quality assurance
> and cross-referencing. Since I incorporated some AI-flagged suggestions,
> I tried to acknowledge that with the Assisted-by tag.

I see, then there is a room to improve the code. But the main question is
why do we even need this whole interface to begin with?

-- 
With Best Regards,
Andy Shevchenko