i3c: scanning for i2c client devices on the network
Mukesh Savaliya
mukesh.savaliya at oss.qualcomm.com
Tue Sep 23 11:46:45 PDT 2025
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.
>>>> 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
>
More information about the linux-i3c
mailing list