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]>

Author: robtaylor

chipflow_lib/platforms/silicon.py

@@ -74,12 +74,11 @@ def __init__(self,
         self._invert = invert
         self._options = port.options
         self._pins = port.pins
-        
+
         # Initialize signal attributes to None
         self._i = None
         self._o = None
         self._oe = None
-        
         # Create signals based on direction
         if self._direction in (io.Direction.Input, io.Direction.Bidir):
             self._i = Signal(port.width, name=f"{component}_{name}__i")
@@ -90,19 +89,18 @@ def __init__(self,
                 self._oe = Signal(port.width, name=f"{component}_{name}__oe", init=-1)
             else:
                 self._oe = Signal(1, name=f"{component}_{name}__oe", init=-1)
-        elif self._direction is io.Direction.Output:
-            # Always create an _oe for output ports
-            self._oe = Signal(1, name=f"{component}_{name}__oe", init=-1)
+        # Output ports don't have _oe signals
 
         logger.debug(f"Created SiliconPlatformPort {name}, width={len(port.pins)},dir{self._direction}")
 
     def wire(self, m: Module, interface: PureInterface):
         assert self._direction == interface.signature.direction
         if hasattr(interface, 'i'):
             m.d.comb += interface.i.eq(self.i)
-        for d in ['o', 'oe']:
-            if hasattr(interface, d):
-                m.d.comb += getattr(self, d).eq(getattr(interface, d))
+        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)
 
     @property
 
@@ -122,7 +120,7 @@ def o(self):
     @property
     def oe(self):
         if self._oe is None:
-            raise AttributeError("SiliconPlatformPort with input direction does not have an "
+            raise AttributeError("SiliconPlatformPort with output or input direction does not have an "
                                "output enable signal")
         return self._oe
 
@@ -217,7 +215,9 @@ def elaborate(self, platform):
             m.d.comb += i_inv.eq(self.port.i)
         if self.direction in (io.Direction.Output, io.Direction.Bidir):
             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:
+                m.d.comb += self.port.oe.eq(self.oe)
 
         return m
 

tests/test_silicon_platform_port.py

@@ -14,14 +14,14 @@
 class TestSiliconPlatformPort(unittest.TestCase):
     def test_init_input_port(self):
         # Test initialization with input direction
-        port_obj = Port(type="input", pins=["1", "2", "3"], port_name="test_input", 
+        port_obj = Port(type="input", pins=["1", "2", "3"], port_name="test_input",
                          direction="i", options={})
         spp = SiliconPlatformPort("comp", "test_input", port_obj)
-        
+
         self.assertEqual(spp.direction, io.Direction.Input)
         self.assertEqual(len(spp), 3)  # Should match the port width
         self.assertFalse(spp.invert)
-        
+
         # Test accessing properties
         _ = spp.i  # Should not raise an error
         with self.assertRaises(AttributeError):
@@ -31,183 +31,184 @@ def test_init_input_port(self):
 
     def test_init_output_port(self):
         # Test initialization with output direction
-        port_obj = Port(type="output", pins=["1", "2"], port_name="test_output", 
+        port_obj = Port(type="output", pins=["1", "2"], port_name="test_output",
                          direction="o", options={})
         spp = SiliconPlatformPort("comp", "test_output", port_obj)
-        
+
         self.assertEqual(spp.direction, io.Direction.Output)
         self.assertEqual(len(spp), 2)  # Should match the port width
         self.assertFalse(spp.invert)
-        
+
         # Test accessing properties
         _ = spp.o  # Should not raise an error
-        _ = spp.oe  # Should not raise an error since we now always have an _oe for outputs
+        with self.assertRaises(AttributeError):
+            _ = spp.oe  # Should raise an error since output ports don't have oe signals
         with self.assertRaises(AttributeError):
             _ = spp.i  # Should raise an error for output port
 
     def test_init_bidir_port(self):
         # Test initialization with bidirectional direction
-        port_obj = Port(type="bidir", pins=["1", "2", "3", "4"], port_name="test_bidir", 
+        port_obj = Port(type="bidir", pins=["1", "2", "3", "4"], port_name="test_bidir",
                          direction="io", options={"all_have_oe": False})
         spp = SiliconPlatformPort("comp", "test_bidir", port_obj)
-        
+
         self.assertEqual(spp.direction, io.Direction.Bidir)
         self.assertEqual(len(spp), 4)  # Should match the port width
         self.assertFalse(spp.invert)
-        
+
         # Check the signals have the correct widths
         self.assertEqual(len(spp.i), 4)
         self.assertEqual(len(spp.o), 4)
         self.assertEqual(len(spp.oe), 1)  # Single OE for all pins
-        
+
         # Test accessing properties
         _ = spp.i  # Should not raise an error
         _ = spp.o  # Should not raise an error
         _ = spp.oe  # Should not raise an error
 
     def test_init_bidir_port_all_have_oe(self):
         # Test initialization with bidirectional direction and all_have_oe=True
-        port_obj = Port(type="bidir", pins=["1", "2", "3"], port_name="test_bidir", 
+        port_obj = Port(type="bidir", pins=["1", "2", "3"], port_name="test_bidir",
                          direction="io", options={"all_have_oe": True})
         spp = SiliconPlatformPort("comp", "test_bidir", port_obj)
-        
+
         self.assertEqual(spp.direction, io.Direction.Bidir)
         self.assertEqual(len(spp), 3)  # Should match the port width
         self.assertFalse(spp.invert)
-        
+
         # Check the signals have the correct widths
         self.assertEqual(len(spp.i), 3)
         self.assertEqual(len(spp.o), 3)
         self.assertEqual(len(spp.oe), 3)  # One OE per pin
-        
+
     def test_len_input_port(self):
         # Test __len__ with input direction
-        port_obj = Port(type="input", pins=["1", "2", "3"], port_name="test_input", 
+        port_obj = Port(type="input", pins=["1", "2", "3"], port_name="test_input",
                          direction="i", options={})
         spp = SiliconPlatformPort("comp", "test_input", port_obj)
-        
+
         self.assertEqual(len(spp), 3)  # Should match the port width
-        
+
     def test_len_output_port(self):
         # Test __len__ with output direction
-        port_obj = Port(type="output", pins=["1", "2"], port_name="test_output", 
+        port_obj = Port(type="output", pins=["1", "2"], port_name="test_output",
                          direction="o", options={})
         spp = SiliconPlatformPort("comp", "test_output", port_obj)
-        
+
         self.assertEqual(len(spp), 2)  # Should match the port width
-        
+
     def test_len_bidir_port(self):
         # Test __len__ with bidirectional direction
-        port_obj = Port(type="bidir", pins=["1", "2", "3", "4"], port_name="test_bidir", 
+        port_obj = Port(type="bidir", pins=["1", "2", "3", "4"], port_name="test_bidir",
                          direction="io", options={"all_have_oe": False})
         spp = SiliconPlatformPort("comp", "test_bidir", port_obj)
-        
+
         self.assertEqual(len(spp), 4)  # Should match the port width
-        
+
     def test_len_bidir_port_all_have_oe(self):
         # Test __len__ with bidirectional direction and all_have_oe=True
-        port_obj = Port(type="bidir", pins=["1", "2", "3"], port_name="test_bidir", 
+        port_obj = Port(type="bidir", pins=["1", "2", "3"], port_name="test_bidir",
                          direction="io", options={"all_have_oe": True})
         spp = SiliconPlatformPort("comp", "test_bidir", port_obj)
-        
+
         self.assertEqual(len(spp), 3)  # Should match the port width
 
     def test_getitem(self):
         # Test __getitem__
-        port_obj = Port(type="bidir", pins=["1", "2", "3"], port_name="test_bidir", 
+        port_obj = Port(type="bidir", pins=["1", "2", "3"], port_name="test_bidir",
                          direction="io", options={"all_have_oe": True})
         spp = SiliconPlatformPort("comp", "test_bidir", port_obj)
-        
+
         # Get a slice of the port
         slice_port = spp[1]
         self.assertEqual(spp.direction, slice_port.direction)
         self.assertEqual(spp.invert, slice_port.invert)
-        
+
     def test_invert(self):
         # Test __invert__ for a bidirectional port since it has all signal types
-        port_obj = Port(type="bidir", pins=["1", "2", "3"], port_name="test_bidir", 
+        port_obj = Port(type="bidir", pins=["1", "2", "3"], port_name="test_bidir",
                          direction="io", options={"all_have_oe": True})
         spp = SiliconPlatformPort("comp", "test_bidir", port_obj)
-        
+
         inverted_port = ~spp
         self.assertEqual(spp.direction, inverted_port.direction)
         self.assertNotEqual(spp.invert, inverted_port.invert)
         self.assertTrue(inverted_port.invert)
-        
+
     def test_add(self):
         # Test __add__
-        port_obj1 = Port(type="input", pins=["1", "2"], port_name="test_input1", 
+        port_obj1 = Port(type="input", pins=["1", "2"], port_name="test_input1",
                           direction="i", options={})
-        port_obj2 = Port(type="input", pins=["3", "4"], port_name="test_input2", 
+        port_obj2 = Port(type="input", pins=["3", "4"], port_name="test_input2",
                           direction="i", options={})
         spp1 = SiliconPlatformPort("comp", "test_input1", port_obj1)
         spp2 = SiliconPlatformPort("comp", "test_input2", port_obj2)
-        
+
         combined_port = spp1 + spp2
         self.assertEqual(spp1.direction, combined_port.direction)
         self.assertEqual(len(combined_port), len(spp1) + len(spp2))
-        
+
     def test_wire_input(self):
         # Test wire method with a mock input interface
-        port_obj = Port(type="input", pins=["1", "2", "3"], port_name="test_input", 
+        port_obj = Port(type="input", pins=["1", "2", "3"], port_name="test_input",
                          direction="i", options={})
         spp = SiliconPlatformPort("comp", "test_input", port_obj)
-        
+
         # Create a mock interface
         class MockSignature(wiring.Signature):
             def __init__(self):
                 super().__init__({"i": wiring.In(3)})
                 self._direction = io.Direction.Input
-                
+
             @property
             def direction(self):
                 return self._direction
-                
+
         class MockInterface(PureInterface):
             def __init__(self):
                 self.signature = MockSignature()
                 self.i = Signal(3)
-                
+
         interface = MockInterface()
         m = Module()
-        
+
         # Wire should not raise an exception
         spp.wire(m, interface)
-        
+
     def test_wire_output(self):
-        # Test wire method with a mock output interface to cover line 105
-        port_obj = Port(type="output", pins=["1", "2"], port_name="test_output", 
+        # Test wire method with a mock output interface to cover line 102
+        port_obj = Port(type="output", pins=["1", "2"], port_name="test_output",
                          direction="o", options={})
         spp = SiliconPlatformPort("comp", "test_output", port_obj)
-        
+
         # Create a mock interface
         class MockSignature(wiring.Signature):
             def __init__(self):
                 super().__init__({"o": wiring.Out(2)})
                 self._direction = io.Direction.Output
-                
+
             @property
             def direction(self):
                 return self._direction
-                
+
         class MockInterface(PureInterface):
             def __init__(self):
                 self.signature = MockSignature()
                 self.o = Signal(2)
-                self.oe = Signal(1)
-                
+                # No oe signal for output ports
+
         interface = MockInterface()
         m = Module()
-        
+
         # Wire should not raise an exception
         spp.wire(m, interface)
-        
+
     def test_wire_bidir(self):
         # Test wire method with a mock bidirectional interface to cover both cases
-        port_obj = Port(type="bidir", pins=["1", "2", "3"], port_name="test_bidir", 
+        port_obj = Port(type="bidir", pins=["1", "2", "3"], port_name="test_bidir",
                          direction="io", options={"all_have_oe": True})
         spp = SiliconPlatformPort("comp", "test_bidir", port_obj)
-        
+
         # Create a mock interface
         class MockSignature(wiring.Signature):
             def __init__(self):
@@ -217,33 +218,33 @@ def __init__(self):
                     "oe": wiring.Out(3),
                 })
                 self._direction = io.Direction.Bidir
-                
+
             @property
             def direction(self):
                 return self._direction
-                
+
         class MockInterface(PureInterface):
             def __init__(self):
                 self.signature = MockSignature()
                 self.i = Signal(3)
                 self.o = Signal(3)
                 self.oe = Signal(3)
-                
+
         interface = MockInterface()
         m = Module()
-        
+
         # Wire should not raise an exception
         spp.wire(m, interface)
-        
+
     def test_repr(self):
         # Test the __repr__ method for a bidirectional port
-        port_obj = Port(type="bidir", pins=["1", "2", "3"], port_name="test_bidir", 
+        port_obj = Port(type="bidir", pins=["1", "2", "3"], port_name="test_bidir",
                          direction="io", options={"all_have_oe": True})
         spp = SiliconPlatformPort("comp", "test_bidir", port_obj)
-        
+
         # Get the representation
         repr_str = repr(spp)
-        
+
         # Check that it contains expected elements
         self.assertIn("SiliconPlatformPort", repr_str)
         self.assertIn("direction", repr_str)