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

[rtl/core] fixing SLINK IRQ bug #199 #200

Closed
wants to merge 3 commits into from
Closed

Conversation

stnolting
Copy link
Owner

This PR aims to fix the bug in SLINK's interrupt signal logic from #199.

@LeFl0w

@stnolting stnolting added the HW Hardware-related label Nov 4, 2021
@stnolting stnolting self-assigned this Nov 4, 2021
@stnolting stnolting linked an issue Nov 4, 2021 that may be closed by this pull request
@stnolting
Copy link
Owner Author

@LeFl0w You're ok with the modifications?

@LeFl0w
Copy link

LeFl0w commented Nov 4, 2021

I'm not sure about changes for lines 263 and 277.
You come from an iteration over only the implemented Slink bits to an iteration over every possible links numbers (even unimplemented ones).
This will produce dead code in case of SLINK_NUM_ not equal to 8.

I think you could keep the previous code , as your vector is initialized entirely to a default value with rx_tmp_v := (others => '0') and then you iterate over only implemented slink bits.

With your new code I 'll have to analyze how irq_rx_mode(i), rx_fifo_half(i) and rx_fifo_avail(i) behave when i is not referring to an implemented slink number. In my point of view it is more complicated to analyze without simulation.

@stnolting
Copy link
Owner Author

I'm not sure about changes for lines 263 and 277. This will produce dead code in case of SLINK_NUM_ not equal to 8.

That's right, but the synthesis tool should trim these signals.

I noticed that the MSB-aligned bits of irq_rx_en and irq_tx_en might not be initialized (not written in the bus write access process) if less than 8 links are implemented. That's where those chances came from.

However, feel free to update this PR with another proposal 😉

@stnolting stnolting marked this pull request as draft November 5, 2021 20:34
@stnolting
Copy link
Owner Author

This is obsolete now. See #202.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HW Hardware-related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants