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

Update UART Example so that acknowledging RX data sets RX ready low. #344

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

newhouseb
Copy link

The UART example code currently sets rx_rdy high when data has been received. I had assumed that setting rx_ack to high would clear this until data is ready again, but in the existing implementation rx_rdy only goes low when new data arrives. This small diff changes rx_ack to clear rx_rdy so that a user can acknowledge data after the first byte and then wait on rx_rdy for the next one. It also updates the simulation to verify that the right thing happens.

I suppose this could've been design with the intent that the user strobes out a signal when rx_rdy goes from low to high, but I'm not sure what the use of rx_ack is if this was the intended design. Sorry if I'm being oblivious, but if I am, feel free to close out this diff.

The UART example code currently sets `rx_rdy` high when data has been received. I had assumed that setting `rx_ack` to high would clear this until data is ready again, but in the existing implementation `rx_rdy` only goes low when new data arrives. This small diff changes `rx_ack` to clear `rx_rdy` so that a user can acknowledge data after the first byte and then wait on `rx_rdy` for the next one.

I suppose this could've been design with the intent that the user strobes out a signal when `rx_rdy` goes from low to high, but I'm not sure what the use of `rx_ack` is if this was the intended design. Sorry if I'm being oblivious, but if I am, feel free to close out this diff.
louiecaulfield pushed a commit to louiecaulfield/nmigen that referenced this pull request Apr 7, 2022
…hdl.verilog.convert and compat.run_simulation

Fixes m-labs#344
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.

1 participant