Refactor CCC tests for sim target - #197
Open
upadhyayulakiran wants to merge 7 commits into
Open
Conversation
mkj121
reviewed
Apr 30, 2026
mkj121
left a comment
mkj121
left a comment
Collaborator
There was a problem hiding this comment.
I've really only reviewed ccc_descriptors.py because it will require rework of the others. Once reworked then I am happy to review everything else.
Comment on lines
+475
to
+565
| async def do_ccc_read(i3c_controller, desc, addr, target=None, *, count=None): | ||
| """Send a directed read CCC to a single target. Returns (ack, data). | ||
|
|
||
| If `count` is None, uses descriptor.count(target) when available, else | ||
| descriptor.base_count. | ||
| """ | ||
| assert desc.kind == "read", f"do_ccc_read called with kind={desc.kind}" | ||
| if count is None: | ||
| count = descriptor_count(desc, target) | ||
| responses = await i3c_controller.i3c_ccc_read( | ||
| ccc=desc.code, addr=addr, count=count) | ||
| ack, data = responses[0] | ||
| cocotb.log.info( | ||
| f"{desc.name} addr=0x{addr:02X} ack={ack} -> {bytes(data).hex()}" | ||
| ) | ||
| return ack, data | ||
|
|
||
|
|
||
| async def do_ccc_read_multi(i3c_controller, desc, addr_list, target=None, | ||
| *, count=None): | ||
| """Send a directed read CCC across multiple targets in one frame. | ||
|
|
||
| Returns the raw list of (ack, data) tuples, one per address. | ||
| """ | ||
| assert desc.kind == "read", ( | ||
| f"do_ccc_read_multi called with kind={desc.kind}" | ||
| ) | ||
| if count is None: | ||
| count = descriptor_count(desc, target) | ||
| responses = await i3c_controller.i3c_ccc_read( | ||
| ccc=desc.code, addr=addr_list, count=count) | ||
| cocotb.log.info( | ||
| f"{desc.name} multi addr={['0x%02X' % a for a in addr_list]} " | ||
| f"-> {len(responses)} responses" | ||
| ) | ||
| return responses | ||
|
|
||
|
|
||
| async def do_ccc_write(i3c_controller, desc, addr, target=None): | ||
| """Send a directed write CCC. Returns True if ACKed. | ||
|
|
||
| If `desc.apply_write` is not None and a target is provided, the | ||
| side-effect is mirrored on the target so subsequent reads can verify | ||
| state. | ||
| """ | ||
| assert desc.kind == "write", f"do_ccc_write called with kind={desc.kind}" | ||
| defining_byte, data_bytes = desc.gen_data(target) | ||
| kwargs = {"directed_data": [(addr, list(data_bytes))]} | ||
| if defining_byte is not None: | ||
| kwargs["defining_byte"] = defining_byte | ||
| kwargs["stop"] = True | ||
| acks = await i3c_controller.i3c_ccc_write(ccc=desc.code, **kwargs) | ||
| ok = bool(acks and acks[0]) | ||
| cocotb.log.info( | ||
| f"{desc.name} write addr=0x{addr:02X} db={defining_byte} " | ||
| f"data={list(data_bytes)} ack={ok}" | ||
| ) | ||
| if ok and desc.apply_write is not None and target is not None: | ||
| desc.apply_write(target, list(data_bytes)) | ||
| return ok | ||
|
|
||
|
|
||
| async def do_ccc_bcast(i3c_controller, desc, target=None): | ||
| """Send a broadcast write CCC. Returns True (no per-target ACK).""" | ||
| assert desc.kind == "bcast_write", ( | ||
| f"do_ccc_bcast called with kind={desc.kind}" | ||
| ) | ||
| defining_byte, data_bytes = desc.gen_data(target) | ||
| kwargs = {"broadcast_data": list(data_bytes)} | ||
| if defining_byte is not None: | ||
| kwargs["defining_byte"] = defining_byte | ||
| await i3c_controller.i3c_ccc_write(ccc=desc.code, **kwargs) | ||
| cocotb.log.info( | ||
| f"{desc.name} bcast db={defining_byte} data={list(data_bytes)}" | ||
| ) | ||
| if desc.apply_write is not None and target is not None: | ||
| desc.apply_write(target, list(data_bytes)) | ||
| return True | ||
|
|
||
|
|
||
| async def do_ccc_read_verify(i3c_controller, desc, addr, target): | ||
| """Send a directed read CCC, assert ACK, verify the value. | ||
|
|
||
| Also runs post_read_check if the descriptor supplies one. | ||
| """ | ||
| ack, data = await do_ccc_read(i3c_controller, desc, addr, target) | ||
| assert ack, f"{desc.name} NACK addr=0x{addr:02X}" | ||
| desc.verify(target, data) | ||
| if desc.post_read_check is not None: | ||
| desc.post_read_check(target) | ||
| return data |
Collaborator
There was a problem hiding this comment.
I am not clear why many of these functions are being re-implemented. There are routines within i3c_controller_fixed.py that implement both CCC Read (i3c_ccc_read) and CCC Write (i3c_ccc_write).
The advantages that both of the existing routines over these routines are:
- You can chain CCC commands together, e.g. by setting
stop=False - Each routine can handle either a CCC command with single or multiple Target addresses
- The write routine handles broadcast data
I would suggest the code is re-worked to re-use these existing routines.
Collaborator
Author
There was a problem hiding this comment.
As I understand it, these functions are just wrappers that inherently use the functions from i3c_controller_fixed. They're there to help keep the tests smaller in terms of lines of code so it looks more readable, specially for tests that issue more than 1 CCC
Collaborator
There was a problem hiding this comment.
OK, let me take another look.
mkj121
reviewed
May 1, 2026
mkj121
left a comment
mkj121
left a comment
Collaborator
There was a problem hiding this comment.
Some additional comments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.