Skip to content

Commit 26a3c5a

Browse files
robtaylorclaude
andcommitted
Fix output ports to not have _oe signals
- 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]>
1 parent 3048db3 commit 26a3c5a

File tree

2 files changed

+13
-13
lines changed

2 files changed

+13
-13
lines changed

chipflow_lib/platforms/silicon.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -90,22 +90,20 @@ def __init__(self,
9090
self._oe = Signal(port.width, name=f"{component}_{name}__oe", init=-1)
9191
else:
9292
self._oe = Signal(1, name=f"{component}_{name}__oe", init=-1)
93-
elif self._direction is io.Direction.Output:
94-
# Always create an _oe for output ports
95-
self._oe = Signal(1, name=f"{component}_{name}__oe", init=-1)
93+
# Output ports don't have _oe signals
9694

9795
logger.debug(f"Created SiliconPlatformPort {name}, width={len(port.pins)},dir{self._direction}")
9896

9997
def wire(self, m: Module, interface: PureInterface):
10098
assert self._direction == interface.signature.direction
10199
if hasattr(interface, 'i'):
102100
m.d.comb += interface.i.eq(self.i)
103-
for d in ['o', 'oe']:
104-
if hasattr(interface, d):
105-
m.d.comb += getattr(self, d).eq(getattr(interface, d))
101+
if hasattr(interface, 'o'):
102+
m.d.comb += self.o.eq(interface.o)
103+
if hasattr(interface, 'oe') and self._oe is not None:
104+
m.d.comb += self.oe.eq(interface.oe)
106105

107106
@property
108-
109107
def i(self):
110108
if self._i is None:
111109
raise AttributeError("SiliconPlatformPort with output direction does not have an "
@@ -122,7 +120,7 @@ def o(self):
122120
@property
123121
def oe(self):
124122
if self._oe is None:
125-
raise AttributeError("SiliconPlatformPort with input direction does not have an "
123+
raise AttributeError("SiliconPlatformPort with output or input direction does not have an "
126124
"output enable signal")
127125
return self._oe
128126

@@ -217,7 +215,9 @@ def elaborate(self, platform):
217215
m.d.comb += i_inv.eq(self.port.i)
218216
if self.direction in (io.Direction.Output, io.Direction.Bidir):
219217
m.d.comb += self.port.o.eq(o_inv)
220-
m.d.comb += self.port.oe.eq(self.oe)
218+
# Only set oe for bidirectional ports
219+
if self.direction is io.Direction.Bidir:
220+
m.d.comb += self.port.oe.eq(self.oe)
221221

222222
return m
223223

tests/test_silicon_platform_port.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ def test_init_output_port(self):
4040

4141
# Test accessing properties
4242
_ = spp.o # Should not raise an error
43-
_ = spp.oe # Should not raise an error since we now always have an _oe for outputs
43+
with self.assertRaises(AttributeError):
44+
_ = spp.oe # Should raise an error since output ports don't have oe signals
4445
with self.assertRaises(AttributeError):
4546
_ = spp.i # Should raise an error for output port
4647

@@ -174,7 +175,7 @@ def __init__(self):
174175
spp.wire(m, interface)
175176

176177
def test_wire_output(self):
177-
# Test wire method with a mock output interface to cover line 105
178+
# Test wire method with a mock output interface
178179
port_obj = Port(type="output", pins=["1", "2"], port_name="test_output",
179180
direction="o", options={})
180181
spp = SiliconPlatformPort("comp", "test_output", port_obj)
@@ -193,7 +194,6 @@ class MockInterface(PureInterface):
193194
def __init__(self):
194195
self.signature = MockSignature()
195196
self.o = Signal(2)
196-
self.oe = Signal(1)
197197

198198
interface = MockInterface()
199199
m = Module()
@@ -247,4 +247,4 @@ def test_repr(self):
247247
self.assertIn("SiliconPlatformPort", repr_str)
248248
self.assertIn("direction", repr_str)
249249
self.assertIn("width=3", repr_str)
250-
self.assertIn("invert=False", repr_str)
250+
self.assertIn("invert=False", repr_str)

0 commit comments

Comments
 (0)