Fix some bugs in tl_broadcast module #1
Merged
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.
Hello there,
Recently, while working on my personal project using your TileLink IP to build a system with coherent caches, I encountered some issues with the tl_broadcast module in my use case. I've made fixes to address these problems:
The MaxSize parameter in the tl_broadcast module appears to be ineffective, as the module seemed to only support a MaxSize of 2^6 Bytes. I found this issue is related to some wiring issues. In this PR, I've addressed this issue with the first commit.
In certain multi-core access scenarios, the tl_broadcast module occasionally experiences hang-ups. After waveform inspection, I discovered that this was due to inconsistent handling of two types of ProbeAck: ProbeAck and ProbeAckData.
ProbeAck immediately asserts the probe_ack_complete signal to indicate that the probe has been completed, whereas ProbeAckData waits for the device side to write back data to the bus before asserting the probe_ack_data_complete signal.
This means that both probe_ack_complete and probe_ack_data_complete signals could be raised simultaneously. In such cases, it should be recognized as the completion of two probes rather than one. I've also addressed this issue in the second commit.
Thank you for your attention to these matters.
Best regards.