Another minor comment which too can be addressed later
On 2/24/2026 6:12 PM, Jamin Lin wrote: > Adds an I3C bus and a target class. > The bus supports: > - I3C data transmission and reception > - CCCs (including ENTDAA) > - IBIs > - legacy I2C transactions > > General usage of the bus is similar to I2C. Users are expected to > initialize a bus via i3c_init_bus, and use the bus returned from the > init function to do transactions on the bus. > > In order to handle IBIs, the controller provides callbacks to handle > receiving an IBI from a target, receiving (optional) additional IBI > bytes from a target, and handling when a target is done with its IBI. > > Similarly, target creation is done via i3c_target_create_simple and > users use the provided I3CTarget to handle transactions. > The target has functions provided that it can use to invoke an IBI and > send additional bytes. > > Along with the send, recv, and event callbacks that are expected of an > I3C target, which are similar to I2C, there is a separate callback for > CCC handling. > This is to help encapsulate CCC handling and keep it separate from > target-specific read/write functionality. > > To avoid repition for required CCCs among I3C targets, there is some > class-level CCC handling added. The CCC is then passed to the target in > case it needs to handle it in some way. > > Signed-off-by: Joe Komlodi <[email protected]> > Reviewed-by: Patrick Venture <[email protected]> > Reviewed-by: Titus Rwantare <[email protected]> > Reviewed-by: Jamin Lin <[email protected]> > Signed-off-by: Jamin Lin <[email protected]> > --- > include/hw/i3c/i3c.h | 277 ++++++++++++++++++ > hw/i3c/core.c | 647 +++++++++++++++++++++++++++++++++++++++++++ > hw/i3c/meson.build | 1 + > hw/i3c/trace-events | 16 ++ > 4 files changed, 941 insertions(+) > create mode 100644 include/hw/i3c/i3c.h > create mode 100644 hw/i3c/core.c > ... > + > +int i3c_send_byte(I3CBus *bus, uint8_t data) > +{ > + /* > + * Ignored, the caller can determine how many were sent based on if this > was > + * ACKed/NACKed. > + */ > + uint32_t num_sent; num_sent is uninitialized here. Even though i3c_send_byte ignores it after the call, it gets passed by pointer into i3c_send() where it is used in the trace_i3c_send() call. If the send callback does not write *num_sent (as is the case with mock_i3c_target_tx, refer my prev comment for patch 18/22), the trace might show garbage. Initializing it to zero might reduce the confusion > + return i3c_send(bus, &data, 1, &num_sent); > +} > + > +int i3c_send(I3CBus *bus, const uint8_t *data, uint32_t num_to_send, > + uint32_t *num_sent) > +{ > + I3CTargetClass *tc; > + I3CTarget *t; > + I3CNode *node; > + int ret = 0; > + > + /* If this message is a broadcast and no CCC has been found, grab it. */ > + if (bus->broadcast && !bus->in_ccc) { > + bus->ccc = *data; > + bus->in_ccc = true; > + /* > + * We need to keep track if we're currently in ENTDAA. > + * On any other CCC, the CCC is over on a RESTART or STOP, but ENTDAA > + * is only over on a STOP. > + */ > + if (bus->ccc == I3C_CCC_ENTDAA) { > + bus->in_entdaa = true; > + } > + } > + > + QLIST_FOREACH(node, &bus->current_devs, next) { > + t = node->target; > + tc = I3C_TARGET_GET_CLASS(t); > + if (bus->in_ccc) { > + if (!tc->handle_ccc_write) { > + ret = -1; > + continue; > + } > + ret = i3c_target_handle_ccc_write(t, data, num_to_send, > num_sent); > + /* Targets should only NACK on a direct CCC. */ > + if (ret && !CCC_IS_DIRECT(bus->ccc)) { > + ret = 0; > + } > + } else { > + if (tc->send) { > + ret = ret || tc->send(t, data, num_to_send, num_sent); > + } else { > + ret = -1; > + } > + } > + } > + > + trace_i3c_send(*num_sent, num_to_send, ret == 0); This trace might print garbage if the target send callback doesnt set it > + > + return ret ? -1 : 0; > +} > + Thanks Jithu
