Skip to content

Commit c7b68da

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 2ed9a3f commit c7b68da

File tree

2 files changed

+16
-15
lines changed

2 files changed

+16
-15
lines changed

chipflow_lib/platforms/silicon.py

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,45 +86,43 @@ def __init__(self,
8686
self._i = Signal(port.width, name=f"{component}_{name}__i")
8787
if self._direction in (io.Direction.Output, io.Direction.Bidir):
8888
self._o = Signal(port.width, name=f"{component}_{name}__o")
89+
# Only bidir ports have output-enable. Either each line has its own OE, or there's one OE for all the wires
8990
if self._direction is io.Direction.Bidir:
9091
if "all_have_oe" in self._options and self._options["all_have_oe"]:
9192
self._oe = Signal(port.width, name=f"{component}_{name}__oe", init=-1)
9293
else:
9394
self._oe = Signal(1, name=f"{component}_{name}__oe", init=-1)
94-
elif self._direction is io.Direction.Output:
95-
# Always create an _oe for output ports
96-
self._oe = Signal(1, name=f"{component}_{name}__oe", init=-1)
9795

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

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

108107
@property
109-
110108
def i(self):
111109
if self._i is None:
112110
raise AttributeError("SiliconPlatformPort with output direction does not have an "
113-
"input signal")
111+
"input signal")
114112
return self._i
115113

116114
@property
117115
def o(self):
118116
if self._o is None:
119117
raise AttributeError("SiliconPlatformPort with input direction does not have an "
120-
"output signal")
118+
"output signal")
121119
return self._o
122120

123121
@property
124122
def oe(self):
125123
if self._oe is None:
126-
raise AttributeError("SiliconPlatformPort with input direction does not have an "
127-
"output enable signal")
124+
raise AttributeError("SiliconPlatformPort with output or input direction does not have an "
125+
"output enable signal")
128126
return self._oe
129127

130128
@property
@@ -219,6 +217,9 @@ def elaborate(self, platform):
219217
m.d.comb += i_inv.eq(self.port.i)
220218
if self.direction in (io.Direction.Output, io.Direction.Bidir):
221219
m.d.comb += self.port.o.eq(o_inv)
220+
221+
# Only set oe for bidirectional ports
222+
if self.direction is io.Direction.Bidir:
222223
m.d.comb += self.port.oe.eq(self.oe)
223224

224225
return m

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)