drivers: i3c: unable to send CCC command/retrieve assigned dynamic address for I3C devices not in device tree after ENTDAA CCC

[albertofloyd]

Describe the bug If I3C device is described in device tree, dynamic address can be retrieved via i3c_device_find API But if an I3C device is not known at build time but is present in the bus, cannot determined its I3C address to be discovered using a dynamic approach.

What is the Zephyr I3C API sequence intended to discover I3C devices not defined in device tree

To Reproduce

Simple helloworld app with the following I3C API sequence :

 #define TGT_DEV1_ID                  0x208006c100b
 struct i3c_device_id id = {.pid = TGT_DEV1_ID};
 static struct i3c_device_desc tgt_desc = {.bus = DEVICE_DT_GET(I3C_DEV),
                   .dev = DEVICE_DT_GET(I3C_DEV)};

  i3c_do_daa((i3c_dev)
  i3c_device_find(i3c_dev, &id); 
  /* Won't find the descriptor since PID provided is not in the device tree */
  if (desc_ptr == NULL) {
    i3c_determine_default_addr(&tgt_desc, &addr);
    LOG_WRN("Retrieved dynamyc addr %x", addr);
  }

This results in successful dynamic address operation but incorrect dynamic address reported by i3c_determine_default_addr All subsequent CCC commands attempted with said descriptor fail

Expected behavior There should be a method via Zephyr I3C API to obtained the descriptor from the discovered I3C device after DAA

Impact If cannot enumerate or at least search a device using PID, means it need to describe all I3C devices in device tree which defeats the hot-join I3C spec feature

Logs and console output [00:01:27.762,000] i3c_m: Perform DAA [00:01:27.762,000] npcx_i3c: npcx_i3c_do_daa: npcx_i3c_do_daa [00:01:27.762,000] npcx_i3c: npcx_i3c_do_daa: DAA: ENTDAA [00:01:27.762,000] npcx_i3c: npcx_i3c_request_daa: npcx_i3c_request_daa [00:01:27.762,000] npcx_i3c: npcx_i3c_send_request: npcx_i3c_send_request 44 [00:01:27.762,000] npcx_i3c: npcx_i3c_do_daa: DAA: Rcvd PID 0x0208006c100b [00:01:27.762,000] npcx_i3c: i3c@400f4000: PID 0x0208006c100b is not in registered device list, given dynamic address 0x09 [00:01:27.762,000] npcx_i3c: npcx_i3c_request_daa: npcx_i3c_request_daa [00:01:27.762,000] npcx_i3c: npcx_i3c_send_request: npcx_i3c_send_request 44 [00:01:27.762,000] npcx_i3c: npcx_i3c_do_daa: PID 0x0208006c100b assigned dynamic address 0x09 [00:01:27.762,000] i3c_m: Find I3C device using PID [00:01:27.762,000] i3c_m: Device not found [00:01:27.762,000] i3c_m: Retrieved dynamic addr a

Environment (please complete the following information): OS: Linux Toolchain: Zephyr SDK, Additional context

[henrikbrixandersen]

Please use our bug template when reporting bugs. You need to edit this issue to include the information requested in https://github.com/zephyrproject-rtos/zephyr/blob/main/.github/ISSUE_TEMPLATE/001_bug_report.md

[albertofloyd]

Please use our bug template when reporting bugs. You need to edit this issue to include the information requested in https://github.com/zephyrproject-rtos/zephyr/blob/main/.github/ISSUE_TEMPLATE/001_bug_report.md

Updated

[XenuIsWatching]

I did have similar issue like this, where I did a hacky workaround and created a 'spoofy' device in the device tree along with a yaml so it would attach in to the list.... I also think that code sample you gave would not work as I see the same dev pointer for the bus and dev, which should not be the same

This is something that I have been thinking about recently with trying to implement controller handoff (when I get time). The i3c_device_find only looks at devices that are within it's list which comes from devicetree only, if it finds a device during ENTDAA, then it just arbitrarly assigns it a dynamic address without coordinating it to a i3c_device_desc. I can't really see this working with the current implementation, you may need to modify some code to get that working.

What I'm thinking is that a mem_slab pool of i3c_device_desc would have to added in to the i3c_common layer where it a allocs from if it finds a device that is not in it's list and attach it to it's slist of attached_devices. i3c_dev_list_find may also have to be modified to search in that pool'ed list of mem_slab'ed i3c_device_desc or for those i3c_device_desc that were statically declared outside of device tree and attached to the bus.

i3c_determine_default_addr is only intended to help with the first attachment before attach_i3c_device is done.

[albertofloyd]

@XenuIsWatching looks there is

I did have similar issue like this, where I did a hacky workaround and created a 'spoofy' device in the device tree along with a yaml so it would attach in to the list.... I also think that code sample you gave would not work as I see the same dev pointer for the bus and dev, which should not be the same

Is this part of current limitation if the i3C device is not part of the device, should this be NULL instead?

This is something that I have been thinking about recently with trying to implement controller handoff (when I get time). The i3c_device_find only looks at devices that are within it's list which comes from devicetree only, if it finds a device during ENTDAA, then it just arbitrarly assigns it a dynamic address without coordinating it to a i3c_device_desc. I can't really see this working with the current implementation, you may need to modify some code to get that working.

Understood

What I'm thinking is that a mem_slab pool of i3c_device_desc would have to added in to the i3c_common layer where it a allocs from if it finds a device that is not in it's list and attach it to it's slist of attached_devices. i3c_dev_list_find may also have to be modified to search in that pool'ed list of mem_slab'ed i3c_device_desc or for those i3c_device_desc that were statically declared outside of device tree and attached to the bus. Got it, looking forward to this enhancement.

i3c_determine_default_addr is only intended to help with the first attachment before attach_i3c_device is done. Thanks for clarifying this.

[dcpleung]

The original design has always been that all I3C must be declared in devicetree, even if they do not appear at boot. This is to avoid allocating memory at all since we may run out of memory. So this is not exactly a bug, but an enhancement. So marking it as such.