Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify axi_lite_slave and make it faster #139

Merged
merged 1 commit into from
Aug 2, 2023
Merged

Conversation

EmilioPeJu
Copy link
Contributor

The following states were removed:

  • State READ_START from the reading state machine
  • State WRITE_START from the writing state machine

This achieves a simpler state machine and it is 1 clock faster, because now, the strobe signal is asserted in the transition from idle

The following states were removed:
- State READ_START from the reading state machine
- State WRITE_START from the writing state machine

This achieves a simpler state machine and it is 1 clock faster, because
now, the strobe signal is asserted in the transition from idle
@EmilioPeJu
Copy link
Contributor Author

This has been tested in a real PandA, though it would still be nice having a test bench

@Araneidae
Copy link
Contributor

This is sound, but I do have one big reservation: this is a protocol change, and there may be some old code buried in there somewhere that makes use of the early presentation of the address.

To summarise: in the original code, the read or write address was being presented one tick before the corresponding strobe. I do remember that before the ack was plumbed into the design this extra tick was being used in some places to implement register reads.

I'm optimistic that there cannot be too many places where this kind of behaviour can be hiding, as most of the PandA registers are implemented in a very uniform way. I do think however that a more careful check of the more obscure register destinations is needed before this is merged. Specifically, look out for register reads where there is no delay on the ack.

@EmilioPeJu
Copy link
Contributor Author

We checked the special cases for register's access (mainly PCAP) and they are fine

@EmilioPeJu EmilioPeJu merged commit 9ad5512 into master Aug 2, 2023
@EmilioPeJu EmilioPeJu deleted the simpler-axil branch August 2, 2023 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants