Fix output ports to not have _oe signals #63
Conversation
|
|
robtaylor
force-pushed
the
fix-output-port-oe
branch
from
352b4de to
8ce2ae9
Compare
|
reviewed ai generated parts, looks good |
| if hasattr(interface, 'o'): | ||
| m.d.comb += self.o.eq(interface.o) | ||
| if hasattr(interface, 'oe') and self._oe is not None: | ||
| m.d.comb += self.oe.eq(interface.oe) |
There was a problem hiding this comment.
the mixing of self._oe and self.oe is a bit confusing here
There was a problem hiding this comment.
any suggestions? SiliconPlatformPort is public, so i think it has to be self._oe, but could be wrong!
chipflow_lib/platforms/silicon.py
Outdated
| m.d.comb += self.port.o.eq(o_inv) | ||
| m.d.comb += self.port.oe.eq(self.oe) | ||
| # Only set oe for bidirectional ports | ||
| if self.direction is io.Direction.Bidir: |
There was a problem hiding this comment.
this if doesn't need to be nested in the above one
|
@robtaylor check |
robtaylor
force-pushed
the
fix-output-port-oe
branch
from
8ce2ae9 to
09960aa
Compare
- Modified SiliconPlatformPort.__init__ to not create _oe signals for output ports - Changed IOBuffer.elaborate to only set oe for bidirectional ports - Updated wire method to check if _oe is None before setting it - Improved oe property error message to be more specific - Updated tests to verify output ports don't have oe signals - Fixed test_wire_output to work without an oe signal - Removed trailing whitespace for better code quality This fix addresses the hardware design principle that output ports should only have _o signals, not _oe (output enable) signals. Only bidirectional ports need _oe signals to control when they are driving vs. receiving. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
robtaylor
force-pushed
the
fix-output-port-oe
branch
from
09960aa to
c7b68da
Compare
|
I am now confused about the background of this PR. An output port with output enable is an entirely valid thing (it's possible to tristate a port without it being bidirectional, if you don't care about the input value while it's Hi-Z). This is what Amaranth implements too: https://github.com/amaranth-lang/amaranth/blob/5689b6bc5a55fe1e634883e3fdebe8981ae3981b/amaranth/lib/io.py#L349-L350 |
|
totally superceded now anyhow! |
This fix addresses the hardware design principle that output ports should only
have _o signals, not _oe (output enable) signals. Only bidirectional ports
need _oe signals to control when they are driving vs. receiving.
🤖 Generated with Claude Code
Co-Authored-By: Claude [email protected]