i3c: scanning for i2c client devices on the network
Frank Li
Frank.li at nxp.com
Wed Sep 24 11:20:03 PDT 2025
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 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