applet.interface.ps2_host: convert to nmigen #232
Conversation
attie
force-pushed
the
ps2-nmigen
branch
2 times, most recently
from
2fb701c
to
81acf3e
Compare
| self.in_fifo.w_en.eq(1), | ||
| self.in_fifo.w_data.eq(error), | ||
| ] | ||
| with m.If(self.in_fifo.w_rdy): |
There was a problem hiding this comment.
I've added this check on w_rdy before advancing state... it wasn't in before, but I figured we'd stand a chance of missing the error counter if it was left out.
If that's wrong, I'll remove it.
There was a problem hiding this comment.
At the very least it should be in a separate commit--there's nothing worse than a "no functional change" commit turning out to introduce a functional change.
There was a problem hiding this comment.
Okay, I went through the logic again and I think you aren't really fixing it--none of the other FIFO writes check readiness either. IIRC this choice is deliberate on my part (but is definitely sketchy); if you wanted to fix it, you could add a new FSM state (reached when the FIFO is full) that hangs the FSM while inhibiting any bus activity.
There was a problem hiding this comment.
none of the other FIFO writes check readiness either
You're quite right... whoops.
I've had a look, and I'm not sure I know enough about the protocol to cleanly implement something safer (e.g: does inhibiting the bus kill off the current request / response, or will it continue after releasing the bus).
I'll drop this patch for the moment, and it can be re-visited if it causes an issue later.
|
Thanks! |
Addresses
interface.ps2_hostitem from #150.