mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Sam Agazaryan <samagazaryan@google.com>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>,
	linux-i3c@lists.infradead.org, Frank Li <Frank.Li@nxp.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Meagan Lloyd <meaganlloyd@linux.microsoft.com>,
	Vitor Soares <vitor.soares@toradex.com>,
	Oleksandr Shulzhenko <oleksandr.shulzhenko.viktorovych@intel.com>,
	Boris Brezillon <boris.brezillon@collabora.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 0/5] i3c: add i3cdev module to expose i3c dev in /dev
Date: Wed, 21 Oct 2026 16:03:03 +0300	[thread overview]
Message-ID: <ati4B33tnKsCD4c_@ashevche-desk.local> (raw)
In-Reply-To: <202610210957562f3f6511@mail.local>

On Wed, Oct 21, 2026 at 11:57:56AM +0200, Alexandre Belloni wrote:
> On 20/10/2026 14:20:16-0700, Sam Agazaryan wrote:
> > > Did I miss this in the initial explanation? If not, please add it next time.
> > 
> > That is just how the existing I3C core enumerates targets during DAA,
> > not something added by this series. I will add a sentence to the
> > cover letter and Patch 4 commit message in v6 contrasting it with
> > i2c-dev to make that context clear.
> > 
> > > Do we have such use cases IRL? Do we have tooling support (at least pending)
> > > somewhere? Why does it require user space interaction in _this_ way (meaning
> > > IOCTL and /dev node)?
> > 
> > Yes, this is used in real systems today:
> > 
> > 1. Tooling & production users:
> > 
> > - i3ctransfer (Patch 5/5 of this series, and carried out-of-tree in
> > AspeedTech-BMC/i3c-tools and vendor OpenBMC BSPs for several
> > years).
> > 
> > - OCP Secure Firmware Recovery (DCR 0xBD) [1] and Caliptra Root-of-Trust
> > recovery agents on OpenBMC/gBMC platforms (e.g., AST2700 / Caliptra
> > MCU recovery tooling and BMC recovery daemons). Because Vitor's
> > original i3cdev series was not merged in 2019, multiple silicon
> > vendors and OpenBMC trees have been carrying out-of-tree forks of i3cdev;
> > upstreaming consolidates those diverging implementations.
> > 
> > 2. Why an ioctl is required over a read and write:
> > 
> > - In the OCP Secure Firmware Recovery v1.1 specification (Section
> > 8.4, "Recovery I3C Interface Block Read via Private Write and
> > Private Read"), every recovery command read is a combined transaction
> > consisting of a Private Write with no STOP, followed by a Repeated START,
> > followed by a Private Read, and finally a STOP.
> > 
> > - Because write() and read() only submit a single transfer at a time
> > (nxfers = 1), the controller emits a STOP condition on the wire
> > at the end of write() and drops the bus lock. Submitting multiple
> > i3c_ioc_xfer descriptors in a single I3C_IOC_XFER(N) ioctl is
> > required so i3c_device_do_xfers() queues them together under one
> > bus lock acquisition and the controller emits a Repeated START
> > between messages (as well as passing HDR mode/cmd parameters).
> > 
> > [1] https://www.opencompute.org/documents/ocp-recovery-document-1p1-final-pdf
> > 
> > > So, imagine that someone submits a kernel driver for a simple device, while
> > > another person already uses the driver for multi-functional device where the
> > > former one is driving only part of it. Strictly speaking the kernel driver
> > > submission becomes a user space regression. Which effectively means that whenever
> > > you open i3cdev, no other kernel drivers may appear. Yes, I understand that
> > > a poor victim of such an unlikely course of events may recompile the kernel,
> > > disable driver, et cetera, but this still raises the same question, why do
> > > we do that to the mere users?
> > 
> > Note that a user does not need to recompile the kernel if a kernel
> > driver binds to a device they wish to access via i3cdev. Because i3cdev
> > handles BUS_NOTIFY_UNBOUND_DRIVER, unbinding the kernel driver at
> > runtime via sysfs:
> > 
> > echo <dev> > /sys/bus/i3c/devices/<dev>/driver/unbind
> > 
> > automatically triggers i3cdev_attach() and exposes /dev/bus/i3c/<dev>
> > without rebuilding the kernel. Also, I3C drivers match against a
> > target's MIPI-assigned DCR or 48-bit PID (Manufacturer ID + Part ID),
> > so a driver for a simple device will not accidentally match a different
> > chip's PID or a dedicated DCR like 0xBD.
> > 
> > > P.S> Still not convinced, that this has to be an ABI. For debugfs and something
> > > like this, no objections, but ABI.
> > 
> > Production BMC recovery daemons cannot rely on debugfs, which is
> > optional and restricted or unmounted on production builds. A character
> > device in /dev/bus/i3c/* provides the stable ioctl those tools need and
> > integrates with udev so non-root daemons can be granted access to
> > specific target nodes (e.g. matching on DCR 0xBD), similar to spidev
> > and hidraw.
> > 
> > 
> > > Forgot to ask, but do they interfere?
> > 
> > Is this in reference to i2c devices on the i3c bus? No, I3C transfers
> > and legacy
> > I2C transfers on a mixed-mode bus do not interfere:
> > 
> > 1. Both i3c_device_do_xfers() and i3c_master_i2c_adapter_xfer() acquire
> > i3c_bus_normaluse_lock(&master->bus), so I3C and I2C transactions on
> > the same bus are serialized by the I3C core.
> > 
> > 2. Attached I2C client addresses are marked as I3C_ADDR_SLOT_I2C_DEV in
> > the bus address slot table, preventing DAA from assigning an I2C
> > device's address to an I3C target.
> 
> On my side, I believe i3cdev can be added to the kernel and that it is
> way safer than i2cdev but I agree that the ABI has to be properly
> designed, maybe we need to version it or have a way to probe for
> supported features so we can update safely later on. It is failry simple
> right now but I feel like we are going to see people with more needs
> than initiating transfers.

Yeah, I really think that i2cdev was a pure mistake (as to be an ABI).
I don't want to repeat it now, when we learnt about that case a lot.
If we ever going to have i3cdev (which justification I also put in doubts)
it needs to be very strict of what can be done and how.

> Also, I think this should not bind to any devices by default and make
> binding explicit.

Yes, this is quite dangerous "feature".

-- 
With Best Regards,
Andy Shevchenko



  reply	other threads:[~2026-10-21 13:03 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-21 23:05 Sam Agazaryan
2026-09-21 23:05 ` [PATCH v5 1/5] i3c: master: export i3c_masterdev_type Sam Agazaryan
2026-09-21 23:06 ` [PATCH v5 2/5] i3c: master: add i3c_for_each_dev helper Sam Agazaryan
2026-09-21 23:06 ` [PATCH v5 3/5] i3c: use actual_len for read transfers Sam Agazaryan
2026-09-24 11:53   ` Adrian Hunter
2026-09-25 15:25   ` Frank Li
2026-09-28 10:42     ` Adrian Hunter
2026-10-12 21:49       ` Sam Agazaryan
2026-09-21 23:06 ` [PATCH v5 4/5] i3c: add i3cdev module to expose i3c dev in /dev Sam Agazaryan
2026-09-25 15:38   ` Frank Li
2026-10-12 22:02     ` Sam Agazaryan
2026-09-21 23:06 ` [PATCH v5 5/5] tools: i3c: add i3ctransfer utility Sam Agazaryan
2026-09-25  9:08 ` [PATCH v5 0/5] i3c: add i3cdev module to expose i3c dev in /dev Andy Shevchenko
2026-09-25 10:57   ` Wolfram Sang
2026-10-10  4:05     ` Sam Agazaryan
2026-10-13 14:15       ` Andy Shevchenko
2026-10-18 20:28         ` Sam Agazaryan
2026-10-19  9:45           ` Andy Shevchenko
2026-10-19  9:47             ` Andy Shevchenko
2026-10-20 21:20               ` Sam Agazaryan
2026-10-21  9:57                 ` Alexandre Belloni
2026-10-21 13:03                   ` Andy Shevchenko [this message]
2026-10-21 13:00                 ` 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=ati4B33tnKsCD4c_@ashevche-desk.local \
    --to=andriy.shevchenko@intel.com \
    --cc=Frank.Li@nxp.com \
    --cc=adrian.hunter@intel.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=arnd@arndb.de \
    --cc=boris.brezillon@collabora.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-i3c@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=meaganlloyd@linux.microsoft.com \
    --cc=oleksandr.shulzhenko.viktorovych@intel.com \
    --cc=samagazaryan@google.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®