mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Meagan Lloyd <meaganlloyd@linux.microsoft.com>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Meagan Lloyd <meaganlloyd@linux.microsoft.com>,
	linux-i3c@lists.infradead.org, alexandre.belloni@bootlin.com,
	vitor.soares@toradex.com, samagazaryan@google.com,
	gregkh@linuxfoundation.org, arnd@arndb.de,
	boris.brezillon@collabora.com,
	oleksandr.shulzhenko.viktorovych@intel.com,
	tgopinath@linux.microsoft.com, corbet@lwn.net,
	skhan@linuxfoundation.org, linux@roeck-us.net, Frank.Li@nxp.com,
	jorge.marques@analog.com, pgaj@cadence.com,
	wsa+renesas@sang-engineering.com,
	tommaso.merciai.xr@bp.renesas.com, nuno.sa@analog.com,
	Michael.Hennerich@analog.com, jic23@kernel.org,
	dlechner@baylibre.com, andy@kernel.org, lorenzo@kernel.org,
	enelsonmoore@gmail.com, rppt@kernel.org, pratyush@kernel.org,
	giovanni.cabiddu@intel.com, gabewhigham@gmail.com,
	haren@linux.ibm.com, pasha.tatashin@soleen.com,
	jirislaby@kernel.org, adrian.ho.yin.ng@altera.com,
	ustc.gu@gmail.com, jszhang@kernel.org, adrian.hunter@intel.com,
	akhilrajeev@nvidia.com, tze.yee.ng@altera.com,
	manikanta.guntupalli@amd.com, shubhrajyoti.datta@amd.com,
	jarkko.nikula@linux.intel.com, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org,
	linux@analog.com, linux-iio@vger.kernel.org,
	andi.shyti@kernel.org
Subject: Re: [PATCH 3/3] i3c: add i3cdev character device module for user-space access
Date: Thu, 24 Sep 2026 16:26:47 -0700	[thread overview]
Message-ID: <20260924-86692413bdae0e78a12c1425@linux.microsoft.com> (raw)
In-Reply-To: <aqt_dCazxTDR4cpk@ashevche-desk.local>

On Thu, Sep 17, 2026 at 08:49:40AM +0300, Andy Shevchenko wrote:
> 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.
> 

(resending as my previous reply hit a snag and is probably marked as
spam for most people)

My take is that if you're using the i3cdev module, it's up to the
programmer to review schematics/datasheets to know how to program
the hardware devices and bus (in the case of hubs) correctly.

Note, this thread's driver_override approach was not favored by the i3c
subsystem reviewers, so please continue any further discussion in the
prevailing approach's thread:
https://lore.kernel.org/linux-i3c/20260921230603.2518652-1-samagazaryan@google.com/T/#mb42d9b2785f8508d39eaf782b3d29189226b8029

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

cc'ing Andi Shyti from I2C subsystem for context.

Andi, we're considering an I3C character device driver interface.
If you have any feedback from your experience with I2C, can you
please share it in:
https://lore.kernel.org/linux-i3c/20260921230603.2518652-1-samagazaryan@google.com/T/#mb42d9b2785f8508d39eaf782b3d29189226b8029

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

I'm supporting a kernel running on accelerator cards that plug into
various server SKUs where hubs/devices along the bus are not physically
on the card & may differ across SKUs. So, I'm looking to program hubs
flexibly and dynamically from userspace to setup bus hardware paths.

The hub drivers being posted to i3c subsystem require hard-coding in the
devicetree to setup the ports physically according to intended
bus/hardware design. As mentioned, these cards will not know what SKU it
will be plugged into, so the devicetree hard-coding cannot reasonably be
determined ahead of time. Theoretically, you may be able to add some
bootloader support to go query SKU information from other server
components and then populate the devicetree for the kernel, but that
will require heavy lifting and plumbing between other server firmware
components and the bootloader.

Further replies and discussion can be routed here:
https://lore.kernel.org/linux-i3c/20260921230603.2518652-1-samagazaryan@google.com/T/#mb42d9b2785f8508d39eaf782b3d29189226b8029

Thank you,
Meagan

> > > > 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
> 
> 
> 
> -- 
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

  parent reply	other threads:[~2026-09-24 23:27 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-11 21:09 [PATCH 0/3] I3C character device driver using driver_override Meagan Lloyd
2026-09-11 21:09 ` [PATCH 1/3] i3c: master: enable driver_override for I3C Meagan Lloyd
2026-09-11 21:36   ` Guenter Roeck
2026-09-16 18:29     ` Meagan Lloyd
2026-09-12 13:22   ` Andy Shevchenko
2026-09-16 18:39     ` Meagan Lloyd
2026-09-13  0:24   ` Jonathan Cameron
2026-09-16 18:53     ` Meagan Lloyd
2026-09-11 21:09 ` [PATCH 2/3] i3c: set i3c_xfer.actual_len in controller drivers Meagan Lloyd
2026-09-13  0:26   ` Jonathan Cameron
2026-09-16 19:11     ` Meagan Lloyd
2026-09-11 21:09 ` [PATCH 3/3] i3c: add i3cdev character device module for user-space access Meagan Lloyd
2026-09-11 23:29   ` Randy Dunlap
2026-09-16 18:37     ` Meagan Lloyd
2026-09-12 13:34   ` Andy Shevchenko
2026-09-16 22:57     ` Meagan Lloyd
2026-09-17  5:49       ` Andy Shevchenko
2026-09-22 20:53         ` Armin Wolf
2026-09-24 22:10         ` Meagan Lloyd
2026-09-24 23:26         ` Meagan Lloyd [this message]
2026-09-12 13:26 ` [PATCH 0/3] I3C character device driver using driver_override Andy Shevchenko
2026-09-16 19:28   ` Meagan Lloyd
2026-09-17  6:23     ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260924-86692413bdae0e78a12c1425@linux.microsoft.com \
    --to=meaganlloyd@linux.microsoft.com \
    --cc=Frank.Li@nxp.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=adrian.ho.yin.ng@altera.com \
    --cc=adrian.hunter@intel.com \
    --cc=akhilrajeev@nvidia.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andi.shyti@kernel.org \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=arnd@arndb.de \
    --cc=boris.brezillon@collabora.com \
    --cc=corbet@lwn.net \
    --cc=dlechner@baylibre.com \
    --cc=enelsonmoore@gmail.com \
    --cc=gabewhigham@gmail.com \
    --cc=giovanni.cabiddu@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=haren@linux.ibm.com \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=jic23@kernel.org \
    --cc=jirislaby@kernel.org \
    --cc=jorge.marques@analog.com \
    --cc=jszhang@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-i3c@lists.infradead.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@analog.com \
    --cc=linux@roeck-us.net \
    --cc=lorenzo@kernel.org \
    --cc=manikanta.guntupalli@amd.com \
    --cc=nuno.sa@analog.com \
    --cc=oleksandr.shulzhenko.viktorovych@intel.com \
    --cc=pasha.tatashin@soleen.com \
    --cc=pgaj@cadence.com \
    --cc=pratyush@kernel.org \
    --cc=rppt@kernel.org \
    --cc=samagazaryan@google.com \
    --cc=shubhrajyoti.datta@amd.com \
    --cc=skhan@linuxfoundation.org \
    --cc=tgopinath@linux.microsoft.com \
    --cc=tommaso.merciai.xr@bp.renesas.com \
    --cc=tze.yee.ng@altera.com \
    --cc=ustc.gu@gmail.com \
    --cc=vitor.soares@toradex.com \
    --cc=wsa+renesas@sang-engineering.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®