-
Notifications
You must be signed in to change notification settings - Fork 28
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
SPI multiple subs functionality. #189
base: main
Are you sure you want to change the base?
Conversation
@@ -33,6 +33,7 @@ class SpiMain extends Module { | |||
required Logic reset, | |||
required Logic start, | |||
required Logic busIn, | |||
Logic? css, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add documentation for what css
is
@@ -50,11 +55,15 @@ class SpiMain extends Module { | |||
..pairConnectIO(this, intf, PairRole.provider); | |||
|
|||
final isRunning = Logic(name: 'isRunning'); | |||
final active = Logic(name: 'enable'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you mean to call this "active"?
@@ -34,6 +34,7 @@ class SpiSub extends Module { | |||
{required SpiInterface intf, | |||
Logic? busIn, | |||
Logic? reset, | |||
bool triStateOutput = false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
document triStateOutput
asyncReset: true, | ||
resetValue: busIn?[-1]); | ||
// flop data from shift register dataOut. | ||
final flopDataOut = FlipFlop(~intf.sclk, shiftReg.dataOut, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: why not keep it as a flop
like it was?
@@ -14,26 +14,31 @@ class SpiInterface extends PairInterface { | |||
/// The data length for serial transmissions on this interface. | |||
final int dataLength; | |||
|
|||
/// The number of Chip Select signals in this interface. | |||
final int multiChipSelects; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why no numChipSelects
or something?
@@ -243,6 +243,167 @@ class SpiPairTest extends Test { | |||
} | |||
} | |||
|
|||
class SpiMultiSubTest extends Test { | |||
late final SpiInterface intfMain; | |||
late final SpiInterface intfSubA; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: might be better to make this a List<SpiInterface>
to reduce the copy-paste for A,B,C,D? And List<SpiSub>
etc.
@@ -38,7 +38,7 @@ class SpiMainDriver extends PendingClockedDriver<SpiPacket> { | |||
unawaited(super.run(phase)); | |||
|
|||
Simulator.injectAction(() { | |||
intf.csb.put(1); | |||
intf.csb[0].put(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is incomplete on the BFM side, right? we need ways to drive different CSB's, not just always 0
@@ -34,6 +34,7 @@ class SpiSub extends Module { | |||
{required SpiInterface intf, | |||
Logic? busIn, | |||
Logic? reset, | |||
bool triStateOutput = false, | |||
super.name = 'spiSub'}) { | |||
// SPI Interface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add validation that the interface always has 1 CS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add validation that the interface always has 1 CS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does the SPI checker need updating as well?
Description & Motivation
Add ability for main to have multiple chip selects and sub to tristate its outputs. Allowing for multidrop configurations in SPI.
Related Issue(s)
#158
Testing
Unit tests for multiple sub configurations.
Backwards-compatibility
Documentation