Skip to content

Interface for determining package pins #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

robtaylor
Copy link
Contributor

@robtaylor robtaylor commented Feb 12, 2025

Package Pin Interface

This PR implements a structured interface for determining package pins in ChipFlow.

Key Changes

  1. Added abstract methods to define standard pin types in package definitions:

    • Power and ground pins
    • Clock pins
    • Reset pins
    • JTAG pins
    • Heartbeat pins
  2. Implemented these methods in both package types:

    • BareDiePackageDef
    • QuadPackageDef
  3. Enhanced the Package class to:

    • Support new pin types
    • Initialize pins from package definitions
    • Support both legacy and new pin formats
  4. Added comprehensive tests in test_package_pins.py

  5. Created documentation in docs/package_pins.md

Backward Compatibility

The implementation maintains backward compatibility with the existing TOML configuration format while introducing a new, more flexible format.

Ready for Review

All tests are passing, and the implementation is ready for review.

Copy link

github-actions bot commented Feb 12, 2025

PR Preview Action v1.6.1

🚀 View preview at
https://chipflow-lib.docs.chipflow-infra.com/pr-preview/pr-38/

Built to branch gh-pages at 2025-05-01 17:35 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@@ -114,6 +114,16 @@ def BidirPinSignature(width, **kwargs):
PinList = List[Pin]
Pins = Union[PinSet, PinList]

class PowerPins(enum.Enum):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably at least want to split IO vs core here

Copy link
Contributor Author

@robtaylor robtaylor Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. what sort of information does the IO power pins need?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is what Serge brought up on Element already a few days ago. Basically right now we have all the IO in one bank with no breaker cells used, so we just have IOVdd/IOVss that apply to all IO pins. But eventually we want to have multiple groups of IO pins (potentially including analog) in seperate banks, each associated with its own one or more IO power and ground pins.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lanserge @gatecat I'm thinking how best to do this. is it a case that a port should have a request for an io power bank? and the pin allocation then should be able to have rules for where on the padring to allocate that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we introduce the idea of a power domain? Might be too much to resolve properly at this point without concrete use cases :/

Copy link
Contributor

@lanserge lanserge Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Power domain could be good idea, but domains could have relations, or even connected inside. I think better to start with two related to each other power domains, with specifying one for the Core and another for IO.

Copy link

@stafverhaegen-chipflow stafverhaegen-chipflow Feb 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this has to be further split up in different domains for core and different domains for IO. You will have breaker IO cells that will break the core and/or the IO supply/ground rings in the IO ring.
Multiple core power domains will also need support from Amaranth to know which logic has to be in which power domain.
An anlog input does not necessarily need to be in separate IO power domain but typically one wants to have a separate power domain for the analog block connected to the analog input to isolate analog block from the digital or IO switching noise on the vdd/gnd or iovdd/iognd signals.


@property
@abc.abstractmethod
def heartbeat(self) -> Dict(int, Pin):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clock in some cases comes as two pins close to each other to be connected to XTAL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, good point. Is that a config option to generate a oscillator?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we have nothing to specify use of XTAL, this needs to be added somehow.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clock in some cases comes as two pins close to each other to be connected to XTAL.

If you have external crystal you will need a crystal oscillator analog block on the chip. The actual clock will then be an output of this oscillator.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What also can happen is that clock is delivered as differential signal; but this can also be for digital signals. Likely only to add when use case is there.

@robtaylor robtaylor linked an issue Feb 14, 2025 that may be closed by this pull request
@robtaylor robtaylor marked this pull request as ready for review March 18, 2025 13:28
@robtaylor
Copy link
Contributor Author

I've updated this PR to implement a complete interface for package pins as discussed.

Key enhancements:

  1. Implemented all abstract methods in both package types with sensible defaults
  2. Added support for new power pin format while maintaining backward compatibility
  3. Created comprehensive tests with 76% code coverage
  4. Added documentation explaining the new interface

All tests are passing. Ready for review!

Copy link

github-actions bot commented May 1, 2025

Tests Skipped Failures Errors Time
2 0 💤 0 ❌ 2 🔥 1.327s ⏱️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add power, clock, jtag and heartbeat pins to package definitions
4 participants