i3c: scanning for i2c client devices on the network
Boris Staletic
bstaletic at axiado.com
Thu Sep 25 08:42:40 PDT 2025
On Wed, Sep 24, 2025 at 02:20:03PM -0400, Frank Li wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> On Wed, Sep 24, 2025 at 12:16:45AM +0530, Mukesh Savaliya wrote:
> > Hi Boris, Frank,
> >
> > On 9/22/2025 5:13 PM, Boris Staletic wrote:
> > > On Thu, Sep 18, 2025 at 04:15:17PM +0200, Boris Staletic wrote:
> > > > On Tue, Sep 16, 2025 at 01:08:44PM -0400, Frank Li wrote:
> > > > > On Wed, Sep 10, 2025 at 02:01:33PM +0000, Boris Staletic wrote:
> > > > > > Hello,
> > > > > >
> > > > > > We have an I3C bus where the user may ultimately attach an I2C slave with an arbitrary address, with the idea
> > > > > > of using i2c-utils to communicate with the I2C slave from userland.
> > > > > >
> > > > > > With the current kernel, I2C slaves need to be known statically and thus i2cdetect fails to detect any slaves.
> > > > > > All communication attempts get rejected early, because i3c_master_find_i2c_dev_by_addr returns NULL and then i3c_master_i2c_adapter_xfer exits with ENOENT.
> > > > > > I3c_master_find_i2c_dev_by_addr returns NULL because bus->devs.i2c ends up being empty.
> > > > > >
> >
> > Checked both .master_xfer() function registered for pure i2c controller and
> > for i3c controller.
> >
> > I2C controller provides full implementation of .master_xfer() function from
> > vendor driver, whereas for i3c controller, framework first validates if any
> > i2c devices with the passed address is already registered or not ?
> >
> > To suggest at i3c_master_i2c_adapter_xfer() - what's wrong if
> > i3c_master_find_i2c_dev_by_addr() returns NULL ? can we not continue instead
> > of returning -ENOENT ? If slave is not present, than we get NACK else
> > continue to perform master->ops->i2c_xfers() ?
> >
> > This is just my thought, I may be wrong. Please point me if i am missing any
> > fundamental from I3C specs.
>
> master.c search i2c_dev_desc to pass to controller driver.
>
> static int dw_i3c_master_i2c_xfers(struct i2c_dev_desc *dev,
> struct i2c_msg *i2c_xfers,
> int i2c_nxfers)
>
> look like below is more reasonable.
>
> static int dw_i3c_master_i2c_xfers(
> struct i3c_master_controller *master,
> struct i2c_msg *i2c_xfers,
> int i2c_nxfers)
I have tried this change already. Two things to note:
1. Yes, passing `struct i3c_master_controller *` instead of `struct
i2c_dev_desc *` makes sense, but isn't strictly necessary.
2. The Cadence i3c master still won't work without a call to
`i3c_master_attach_i2c_dev()`.
Let me elaborate on the latter.
The cadence i3c master has 10 "retaining registers" (11, but one is
referring to the master device itself).
If communication is attempted with a device which is not in a retaining
register, the command register's error field will just report "data
access error".
The simplest solution that I have found is:
1. Try to find a known device with `i3c_master_find_i2c_dev_by_addr`
2. If that returns NULL, allocate a new device with
`i3c_master_allocate_i2c_dev` and attach it with
`i3c_master_attach_i2c_dev`.
3. Now attempt communication.
4. If errored, detach and deallocate.
Of the four steps, current linux performs only step 1.
I did omit error handling in step 2, but only for brevity.
Regards,
Boris Staletic
>
> I am not sure why not use addr information in i2c_xfers, like other i2c
> adaptor driver.
>
> So framework needn't scan known dev for specific addr.
>
> It looks like not worth to update all drivers and frameworks only for
> i2cdetect function, which generally used for debug only.
>
> And actually, i3c transfer protocol is difference i2c although address phase
> is the same. but data transfer phase is difference.
>
> Frank
>
> >
> > > > > > Since the I2C drivers have i2c_detect and i3c_master_controller has its own i2c_adater, I looked into what needs to be done for networks
> > > > > > with I3C masters to use the existing i2c_detect mechanism. That lead me to the following list of changes:
> > > > > >
> > > > > > - i3c_master_controllr's i2c_adapter needs to have its class member set.
> > > > > > - In i2c_detect_address, temp_client->dev needs to be populated and registered before the call to i2c_default_probe.
> > > > > > - Otherwise the attempt to probe gets rejected early, for the same reason described above (i3c_master_find_i2c_dev_by_addr returning NULL).
> > > > > >
> > > > > > That much was enough to get i2c_detect to work with an I3C master.
> > > > > > To complete the use case I also needed a registered i2c_driver that implements a simple i2c_driver.detect function.
> > > > > > In my test, the detect function simply checked whether i2c_smbus_read_byte_data(client, 0) returns a non-error value.
> > > > > >
> > > > > > Could anyone comment whether this is the right approach for implementing I2C slave detection with an I3C master?
> > > > >
> > > > > Generally, it is the same as I2C. just send out address, and check if target
> > > > > ACK/NACK this address.
> > > >
> > > > Are you referring here to i2c_new_scanned_device? If so, that also, in
> > > > my testing, couldn't detect any devices without modificatioins.
> > > > The failure to scan happens because:
> > > >
> > > > - i2c_new_scanned_device calls i2c_default_probe, which calls
> > > > i2c_smbus_xfer.
> > > > - that eventually gets to i3c_master_i2c_adapter_xfer, which looks up
> > > > existing devices by calling i3c_master_find_i2c_dev_by_addr.
> > > > - i3c_master_find_i2c_dev_by_addr returns NULL
> > > > - i3c_master_i2c_adapter_xfer then returns -ENOENT, ven before trying to
> > > > communicate over the wire.
> > > >
> > > > I am probably missing something, because, as far as I can tell, neither
> > > > i2c_detect, nor i2c_new_scanned_device can currently be used with an I3C
> > > > master.
> > > >
> > > > Would you mind elaborating more on the suggested approach?
> > > >
> > >
> > > Replying here to post the diff of what I have tried (and attempted to
> > > explain the opening email). Hopefully it will make it clearer what
> > > is our need and (more so) what my attempted solution does.
> > >
> > > -------------------------------------------------------------
> > >
> > > This change allows I3C master drivers to trigger I2C slave detection
> > > by i2c slave drivers, leveraging the existing i2c_detect mechanism.
> > > ---
> > > drivers/i2c/busses/Makefile | 1 +
> > > drivers/i2c/busses/i2c-axiado.c | 53 +++++++++++++++++++++++++++++++++
> > > drivers/i2c/i2c-core-base.c | 12 +++++++-
> > > drivers/i3c/master.c | 1 +
> > > 4 files changed, 66 insertions(+), 1 deletion(-)
> > > create mode 100644 drivers/i2c/busses/i2c-axiado.c
> > >
> > > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> > > index 04db855fdfd6..7b51df1433c4 100644
> > > --- a/drivers/i2c/busses/Makefile
> > > +++ b/drivers/i2c/busses/Makefile
> > > @@ -42,6 +42,7 @@ obj-$(CONFIG_I2C_AT91) += i2c-at91.o
> > > i2c-at91-y := i2c-at91-core.o i2c-at91-master.o
> > > i2c-at91-$(CONFIG_I2C_AT91_SLAVE_EXPERIMENTAL) += i2c-at91-slave.o
> > > obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o
> > > +obj-y += i2c-axiado.o
> > > obj-$(CONFIG_I2C_AXXIA) += i2c-axxia.o
> > > obj-$(CONFIG_I2C_BCM2835) += i2c-bcm2835.o
> > > obj-$(CONFIG_I2C_BCM_IPROC) += i2c-bcm-iproc.o
> > > diff --git a/drivers/i2c/busses/i2c-axiado.c b/drivers/i2c/busses/i2c-axiado.c
> > > new file mode 100644
> > > index 000000000000..d5006e250a3d
> > > --- /dev/null
> > > +++ b/drivers/i2c/busses/i2c-axiado.c
> > > @@ -0,0 +1,53 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + * I2C bus driver for the Cadence I2C controller.
> > > + *
> > > + * Copyright (C) 2009 - 2014 Xilinx, Inc.
> > > + */
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/i2c.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/io.h>
> > > +#include <linux/iopoll.h>
> > > +#include <linux/module.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/of.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/pinctrl/consumer.h>
> > > +#include <linux/reset.h>
> > > +
> > > +static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x50, 0x51, 0x52, 0x53, I2C_CLIENT_END };
> > > +
> > > +static int ax_i2c_detect(struct i2c_client *client, struct i2c_board_info *info)
> > > +{
> > > + int byte = i2c_smbus_read_byte_data(client, 0);
> > > + if(byte < 0)
> > > + return -ENODEV;
> > > + strscpy(info->type, "axiado-client", I2C_NAME_SIZE);
> > > + return 0;
> > > +}
> > > +
> > > +static const struct of_device_id ax_i2c_of_match[] = {
> > > + { .compatible = "axiado,ax300",},
> > > + { /* end of table */ }
> > > +};
> > > +MODULE_DEVICE_TABLE(of, ax_i2c_of_match);
> > > +
> > > +
> > > +static struct i2c_driver ax_i2c_drv = {
> > > + .driver = {
> > > + .name = "axiado",
> > > + .of_match_table = ax_i2c_of_match,
> > > + },
> > > + .detect = ax_i2c_detect,
> > > + .address_list = normal_i2c,
> > > + .class = I2C_CLASS_HWMON
> > > +};
> > > +
> > > +module_i2c_driver(ax_i2c_drv);
> > > +
> > > +MODULE_AUTHOR("Boris Staletic axiado-2532 at axiado.com");
> > > +MODULE_DESCRIPTION("Axiado I2C bus driver");
> > > +MODULE_LICENSE("GPL");
> > > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> > > index ecca8c006b02..231c43cf657d 100644
> > > --- a/drivers/i2c/i2c-core-base.c
> > > +++ b/drivers/i2c/i2c-core-base.c
> > > @@ -2467,13 +2467,22 @@ static int i2c_detect_address(struct i2c_client *temp_client,
> > > return 0;
> > >
> > > /* Make sure there is something at this address */
> > > - if (!i2c_default_probe(adapter, addr))
> > > + temp_client->dev.bus = &i2c_bus_type;
> > > + temp_client->dev.type = &i2c_client_type;
> > > + temp_client->dev.parent = &temp_client->adapter->dev;
> > > + dev_set_name(&temp_client->dev, "i2c-temp-client");
> > > + int err3 = device_register(&temp_client->dev);
> > > + int err2 = i2c_default_probe(adapter, addr);
> > > + if (!err2) {
> > > + device_unregister(&temp_client->dev);
> > > return 0;
> > > + }
> > >
> > > /* Finally call the custom detection function */
> > > memset(&info, 0, sizeof(struct i2c_board_info));
> > > info.addr = addr;
> > > err = driver->detect(temp_client, &info);
> > > + device_unregister(&temp_client->dev);
> > > if (err) {
> > > /* -ENODEV is returned if the detection fails. We catch it
> > > here as this isn't an error. */
> > > @@ -2542,6 +2551,7 @@ static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver)
> > > dev_dbg(&adapter->dev,
> > > "found normal entry for adapter %d, addr 0x%02x\n",
> > > i2c_adapter_id(adapter), address_list[i]);
> > > + memset(&temp_client->dev, 0, sizeof(temp_client->dev));
> > > temp_client->addr = address_list[i];
> > > err = i2c_detect_address(temp_client, driver);
> > > if (unlikely(err))
> > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > > index 2ef898a8fd80..5fabba14de20 100644
> > > --- a/drivers/i3c/master.c
> > > +++ b/drivers/i3c/master.c
> > > @@ -2494,6 +2494,7 @@ static int i3c_master_i2c_adapter_init(struct i3c_master_controller *master)
> > > /* FIXME: Should we allow i3c masters to override these values? */
> > > adap->timeout = 1000;
> > > adap->retries = 3;
> > > + adap->class = 1;
> > >
> > > id = of_alias_get_id(master->dev.of_node, "i2c");
> > > if (id >= 0) {
> > > > Thanks in advance,
> > > > Boris Staletic
> > > >
> > > > >
> > > > > Address 7E, REPEAT START, scan's address. ACK/NACK STOP.
> > > > >
> > > > > Check ACK/NACK. But you don't know if it is i3c device or i2c device by this
> > > > > way.
> > > > >
> > > > > send 7E to avoid IBI during you send out address.
> > > > >
> > > > > Fank
> > > > >
> > > > > > The most iffy part of this approach is the need for such a dummy driver, just so the I3C master knows that "something exists".
> > > > > > I'd welcome any sort of feedback.
> > > > > >
> > > > > > Thanks,
> > > > > > Boris Staletic
> > > > > > --
> > > > > > linux-i3c mailing list
> > > > > > linux-i3c at lists.infradead.org
> > > > > > http://lists.infradead.org/mailman/listinfo/linux-i3c
> > >
> >
> >
> > --
> > linux-i3c mailing list
> > linux-i3c at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-i3c
More information about the linux-i3c
mailing list