@samitouri / QOSamiQemu / commits / 04d249d1ef

hw/i3c/dw-i3c: Fix BCR/DCR extraction and PID assembly during ENTDAA

The target_info union in dw_i3c_addr_assign_cmd() declares pid, bcr, and dcr as separate union members, causing them to all alias b[0] rather than their correct positions in the ENTDAA response buffer. This results in dw_i3c_update_char_table() being called with BCR and DCR both read from b[0] instead of b[6] and b[7] respectively, corrupting the device characteristics table on every ENTDAA operation. Fix by replacing the broken members with uint64_t d and extracting fields per the I3C spec ENTDAA wire format. Additionally, dw_i3c_update_char_table() incorrectly splits PID across LOC1 and LOC2 at bit 32. Per the Linux kernel HCI driver (drivers/i3c/master/mipi-i3c-hci/dct_v1.c), the DCT layout requires LOC1 to hold pid[47:16] and LOC2 to hold pid[15:0]. Fix the split accordingly. Signed-off-by: Ashish Anand <ashish.a6@samsung.com> Reviewed-by: Jamin Lin <jamin_lin@aspeedtech.com> Link: https://lore.kernel.org/qemu-devel/20260505134002.509037-1-ashish.a6@samsung.com Signed-off-by: Cédric Le Goater <clg@redhat.com>

Ashish Anand committed May 5, 2026 at 19:10 UTC 04d249d1efbbecc74eb82d26558c78d5a3b43630
2 files changed +14 -9
hw/i3c/dw-i3c.c
+7 -9
@@ -1459,11 +1459,10 @@ static void dw_i3c_update_char_table(DWI3C *s, uint8_t offset, uint64_t pid,
1459 P_DEV_CHAR_TABLE_START_ADDR) /
1460 sizeof(uint32_t)) +
1461 (offset * sizeof(uint32_t));
1462 - s->regs[dev_index] = pid & 0xffffffff;
1463 - pid >>= 32;
1462 + s->regs[dev_index] = (pid >> 16) & 0xffffffff;
1463 s->regs[dev_index + 1] = FIELD_DP32(s->regs[dev_index + 1],
1464 DEVICE_CHARACTERISTIC_TABLE_LOC2,
1466 - MSB_PID, pid);
1465 + MSB_PID, pid & 0xffff);
1466 s->regs[dev_index + 2] = FIELD_DP32(s->regs[dev_index + 2],
1467 DEVICE_CHARACTERISTIC_TABLE_LOC3, DCR,
1468 dcr);
@@ -1507,10 +1506,9 @@ static void dw_i3c_addr_assign_cmd(DWI3C *s, DWI3CAddrAssignCmd cmd)
1506 for (i = 0; i < cmd.dev_count; i++) {
1507 uint8_t addr = dw_i3c_target_addr(s, cmd.dev_index + i);
1508 union {
1510 - uint64_t pid:48;
1511 - uint8_t bcr;
1512 - uint8_t dcr;
1509 + uint64_t d;
1510 uint32_t w[2];
1511 + /* Per I3C spec: b[0]=PID MSB, b[5]=PID LSB, b[6]=BCR, b[7]=DCR */
1512 uint8_t b[8];
1513 } target_info;
1514
@@ -1544,9 +1542,9 @@ static void dw_i3c_addr_assign_cmd(DWI3C *s, DWI3CAddrAssignCmd cmd)
1542 err = DW_I3C_RESP_QUEUE_ERR_DAA_NACK;
1543 break;
1544 }
1547 - dw_i3c_update_char_table(s, cmd.dev_index + i,
1548 - target_info.pid, target_info.bcr,
1549 - target_info.dcr, addr);
1545 + uint64_t pid = be64_to_cpu(target_info.d) >> 16;
1546 + dw_i3c_update_char_table(s, cmd.dev_index + i, pid, target_info.b[6],
1547 + target_info.b[7], addr);
1548
1549 /* Push the PID, BCR, and DCR to the RX queue. */
1550 dw_i3c_push_rx(s, target_info.w[0]);
include/hw/i3c/i3c.h
+7
@@ -138,6 +138,13 @@ struct I3CTarget {
138 uint8_t static_address;
139 uint8_t dcr;
140 uint8_t bcr;
141 + /*
142 + * Provisioned ID. Since core.c sends this LSB-first during ENTDAA
143 + * via (pid >> (offset * 8)) & 0xff, targets must store it
144 + * pre-reversed so that pid[47:40] goes on the wire first, as
145 + * required by the I3C spec.
146 + * e.g. for a device with pid 0xAABBCCDDEEFF, store 0xFFEEDDCCBBAA.
147 + */
148 uint64_t pid;
149
150 /* CCC State tracking. */