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

wishbone.bus.Decoder: Only assert stb when slave is selected #31

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

Conversation

antonblanchard
Copy link
Contributor

While a slave should check both cyc and stb are asserted, it is a bit
odd that we assert stb on all slaves even if they are not selected.

This changes stb to behave like the cyc signal, and adds a few tests.

While a slave should check both cyc and stb are asserted, it is a bit
odd that we assert stb on all slaves even if they are not selected.

This changes stb to behave like the cyc signal, and adds a few tests.
@antonblanchard
Copy link
Contributor Author

The wishbone spec mentions this about the stb signal:

The strobe input [STB_I], when asserted, indicates that the SLAVE is
selected. A SLAVE shall respond to other WISHBONE signals only when this
[STB_I] is asserted (except for the [RST_I] signal which should always
be responded to). The SLAVE asserts either the [ACK_O], [ERR_O] or
[RTY_O] signals in response to every assertion of the [STB_I] signal

@jfng
Copy link
Member

jfng commented Feb 6, 2023

Sorry for this very late response.

The Wishbone spec also says that interface signals are only valid if cyc is asserted:

3.1.2 Transfer Cycle initiation
MASTER interfaces initiate a transfer cycle by asserting [CYC_O]. When [CYC_O] is
negated, all other MASTER signals are invalid.
SLAVE interfaces respond to other SLAVE signals only when [CYC_I] is asserted.
SYSCON signals and responses to SYSCON signals are not affected.

AFAIU, this allows stb to be broadcast to all decoder subordinates to save some resources.

If this behavior is incompatible with other implementations, I don't mind changing it. Could you rebase this PR to main ?

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

Successfully merging this pull request may close these issues.

2 participants