wip

Author: robtaylor

chipflow_lib/config_models.py

@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: BSD-2-Clause
 import re
+from pprint import pformat
 from typing import Dict, Optional, Literal, Any
 
 from pydantic import BaseModel, model_validator, ValidationInfo, field_validator
@@ -21,6 +22,9 @@ def validate_loc_format(self):
 
     @classmethod
     def validate_pad_dict(cls, v: dict, info: ValidationInfo):
+        print(f"validate_pad_dict: info:\n{pformat(info)}")
+        print(f"info.context:\n{pformat(info.context)}")
+        print(f"info.data:\n{pformat(info.data)}")
         """Custom validation for pad dicts from TOML that may not have all fields."""
         if isinstance(v, dict):
             # Handle legacy format - if 'type' is missing but should be inferred from context

chipflow_lib/platforms/utils.py

@@ -557,7 +557,7 @@ def add_pad(self, name: str, defn: dict):
             case {"type": "power", "loc": loc}:
                 self.power[name] = Port(type="power", pins=[loc], port_name=name)
             case {"type": "ground", "loc": loc}:
-                self.power[name] = Port(type="ground", pins=[loc])
+                self.power[name] = Port(type="ground", pins=[loc], port_name=name)
             case {"type": "power", "name": name, "voltage": voltage}:
                 # Support for new power pin format
                 # First, get the default pin from the package type

docs/chipflow-toml-guide.rst

@@ -105,7 +105,8 @@ The logic that synchronizes the reset signal with the clock will be generated au
    package = "pga144"
 
 
-The ``silicon`` section sets the Foundry ``process`` (i.e. PDK) that we are targeting for manufacturing, and the physical ``package`` (pad ring) we want to place our design inside.
+The ``silicon`` section sets the Foundry ``process`` (i.e. PDK) that we are targeting for manufacturing, and the physical ``package`` (including pad ring) we want to place our design inside.
+
 You'll choose the ``process`` and ``package`` based in the requirements of your design.
 
 Available processes
@@ -124,8 +125,8 @@ Available processes
 | ihp_sg13g2 | pga144     | IHP SG13G2 130nm SiGe     |
 +------------+------------+---------------------------+
 
-Available pad rings
--------------------
+Available Package Definitions
+-----------------------------
 
 +----------+-----------+--------------------+------------------------------------+
 | Pad ring | Pad count | Pad locations      | Notes                              |
@@ -139,19 +140,37 @@ Available pad rings
 +----------+-----------+--------------------+------------------------------------+
 
 
-``[silicon.pads]``
-------------------
+``[silicon.power]``
+-------------------
 
-The ``silicon.pads`` section lists special pads. In general you are unlikely to need to add to this.
-Each pad specified with the name used by the design and two parameters: :term:type and :term:`loc`.
+This section outlines the connection of pads to the power supply available for the selected process and package.
+These pads are declared with the :term:type parameter, along with a name and other optional information, like voltage.
 
-.. code-block:: TOML
+Note that in this context, the :term:type parameter can only be ``ground`` or ``power``.
+
+The package definition provides default locations for standard pins like power, ground, clocks, and resets. You only need to specify the name and properties.
+
+[chipflow.silicon.power]
+vdd = { type = "power", name = "vdd", voltage = "1.8V" }
+gnd = { type = "ground", name = "gnd" }
+```
+
+
+``[silicon.power]``
+-------------------
+
+This section outlines the connection of pads to the power supply available for the selected process and package.
+These pads are declared with the :term:type parameter, along with a name and other optional information, like voltage.
 
-   [chipflow.silicon.pads]
-   sys_clk   = { type = "clock", loc = "114" }
-   sys_rst_n = { type = "reset", loc = "115" }
+Note that in this context, the :term:type parameter can only be ``ground`` or ``power``.
+
+[chipflow.silicon.power]
+vdd = { type = "power", name = "vdd", voltage = "1.8V" }
+gnd = { type = "ground", name = "gnd" }
+```
+
+In the new format, the package definition provides default locations for standard pins like power, ground, clocks, and resets. You only need to specify the name and properties.
 
-In the above example two pads specified, ``sys_clk`` pad for clock input and ``sys_rst_n`` for reset.
 
 .. glossary::
 
@@ -168,13 +187,4 @@ In the above example two pads specified, ``sys_clk`` pad for clock input and ``s
        External reset input.
 
 
-``[silicon.power]``
--------------------
-
-This section outlines the connection of pads to the power supply available for the selected process and package.
-These pads are declared with the :term:type and :term:loc parameters, similar to the `[silicon.pads]`_ section.
-Note that in this context, the :term:type parameter can only be ``ground`` or ``power``.
-
-This is a work in progress, and currently you can use the defaults provided by customer support.
-
 .. _Caravel Harness: https://caravel-harness.readthedocs.io/en/latest/

docs/package_pins.md

@@ -7,36 +7,14 @@ This document describes the package pin interface in ChipFlow, introduced to pro
 The package pin interface provides definitions for various types of pins in a chip package:
 
 - Power and ground pins
-- Clock pins 
+- Clock pins
 - Reset pins
 - JTAG pins
 - Heartbeat pins
 
 Each package type (PGA, bare die, etc.) defines its own implementation of these pin types, with appropriate pin numbering and allocation strategies.
 
-## Configuration in TOML Files
-
-### Legacy Format (still supported)
-```toml
-[chipflow.silicon.power]
-vdd = { type = "power", loc = "1" }
-gnd = { type = "ground", loc = "2" }
-
-[chipflow.silicon.pads]
-reset = { type = "reset", loc = "3" }
-clock = { type = "clock", loc = "4" }
-```
-
-### New Format
-```toml
-[chipflow.silicon.power]
-vdd = { type = "power", name = "vdd", voltage = "1.8V" }
-gnd = { type = "ground", name = "gnd" }
-```
-
-In the new format, the package definition provides default locations for standard pins like power, ground, clocks, and resets. You only need to specify the name and properties.
-
-## Using the Package Pin Interface in Code
+# Using the Package Pin Interface in Code
 
 ### Getting Default Pins
 
@@ -100,4 +78,4 @@ Tests for the package pin interface can be run using:
 
 ```bash
 pdm run pytest tests/test_package_pins.py
-```
+```

tests/test_package_pins.py

@@ -3,7 +3,7 @@
 from unittest import mock
 
 from chipflow_lib.platforms.utils import (
-    _BareDiePackageDef, _QuadPackageDef,
+    _BareDiePackageDef, _PGAPackageDef,
     PowerType, JTAGWireName, Package
 )
 
@@ -49,7 +49,7 @@ def test_heartbeat_pins(self):
 
 class TestQuadPackage(unittest.TestCase):
     def setUp(self):
-        self.package = _QuadPackageDef(name="test_package", width=36, height=36)
+        self.package = _PGAPackageDef(name="test_package", width=36, height=36)
 
     def test_pins_property(self):
         """Test that pins property returns all pins"""

tests/test_utils_additional.py

@@ -16,7 +16,7 @@
     _Side,
     _BasePackageDef,
     _BareDiePackageDef,
-    _QuadPackageDef,
+    _PGAPackageDef,
     Package,
     Port,
     PortMap,
@@ -210,9 +210,9 @@ def test_portmap_methods(self):
 
 class TestPackageDef(unittest.TestCase):
     def test_quad_package_def(self):
-        """Test _QuadPackageDef class"""
+        """Test _PGAPackageDef class"""
         # Create instance
-        quad_pkg = _QuadPackageDef(name="test_quad", width=5, height=5)
+        quad_pkg = _PGAPackageDef(name="test_quad", width=5, height=5)
 
         # Check properties
         self.assertEqual(quad_pkg.name, "test_quad")
@@ -232,33 +232,6 @@ def test_quad_package_def(self):
         mock_sorted = sorted(test_pins, key=int)
         self.assertEqual(mock_sorted, ["1", "2", "3", "4", "5"])
 
-    def test_base_package_def_sortpins_bug(self):
-        """Test _BasePackageDef sortpins method - documenting the bug"""
-        # Create a minimal subclass of _BasePackageDef for testing
-        class TestPackageDef(_BasePackageDef):
-            @property
-            def pins(self):
-                return {"1", "2", "3"}
-
-            def allocate(self, available, width):
-                return list(available)[:width]
-
-        # Create an instance
-        pkg = TestPackageDef(name="test_pkg")
-
-        # Instead of using SiliconTop to test elaboratables, let's use a simple mock
-        # This avoids the need to import and use SiliconTop which generates warnings
-        elaboratable_mock = mock.MagicMock()
-        elaboratable_mock.elaborate = mock.MagicMock(return_value=mock.MagicMock())
-
-        # Test sortpins method - THIS IS EXPECTED TO FAIL because of a bug
-        # The method should return sorted(list(pins)) but actually returns None
-        # because list.sort() sorts in-place and returns None
-        result = pkg.sortpins(["3", "1", "2"])
-
-        # This test documents the bug - the method returns None instead of a sorted list
-        self.assertIsNone(result, "This documents a bug in sortpins! It should return a sorted list.")
-
     def test_bare_die_package_def(self):
         """Test _BareDiePackageDef class"""
         # Create instance
@@ -307,7 +280,7 @@ class TestPackage(unittest.TestCase):
     def test_package_init(self):
         """Test Package initialization"""
         # Create package type
-        package_type = _QuadPackageDef(name="test_package", width=10, height=10)
+        package_type = _PGAPackageDef(name="test_package", width=10, height=10)
 
         # Create package
         package = Package(package_type=package_type)
@@ -321,7 +294,7 @@ def test_package_init(self):
     def test_package_add_pad(self):
         """Test Package.add_pad method"""
         # Create package type
-        package_type = _QuadPackageDef(name="test_package", width=10, height=10)
+        package_type = _PGAPackageDef(name="test_package", width=10, height=10)
 
         # Create package
         package = Package(package_type=package_type)
@@ -354,7 +327,7 @@ def test_package_add_pad(self):
     def test_package_check_pad(self):
         """Test Package.check_pad method"""
         # Create package type
-        package_type = _QuadPackageDef(name="test_package", width=10, height=10)
+        package_type = _PGAPackageDef(name="test_package", width=10, height=10)
 
         # Create package
         package = Package(package_type=package_type)
@@ -371,7 +344,8 @@ def test_package_check_pad(self):
         self.assertEqual(clock_port.pins, ["1"])
 
         reset_port = package.check_pad("rst1", {"type": "reset"})
-        self.assertIsNone(reset_port)  # This is None due to a bug in the code
+        self.assertIsNotNone(reset_port)
+        self.assertEqual(reset_port.pins, ["2"])
 
         power_port = package.check_pad("vdd", {"type": "power"})
         self.assertIsNotNone(power_port)