Skip to content
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

[RFC] Allow DiffPairs and Pins to be inverted individually #510

Open
anuejn opened this issue Oct 20, 2020 · 15 comments
Open

[RFC] Allow DiffPairs and Pins to be inverted individually #510

anuejn opened this issue Oct 20, 2020 · 15 comments
Labels

Comments

@anuejn
Copy link
Contributor

anuejn commented Oct 20, 2020

Currently, DiffPairs and Pins can be either inverted completely or not at all.
This is a problem in corner cases where the hardware inverts some parts of a logical Bundle of ios (for example to ease routing in case of differential pairs; see #386).

I hereby propose to extend the Platform DSL in the following way:

  • Add a invert= kwarg to the constructor of DiffPairs
  • Allow Tuples of booleans of the width of the pin list to be passed to the invert= kwarg of both DiffPairs and Pins. Each bool in the tuple then controls the inversion of one pin / differential pair.
@whitequark
Copy link
Member

whitequark commented Oct 20, 2020

Right, this is the obvious solution. The main issue I see with it is that when you have a lot of diffpairs, it can be hard to correlate the booleans in the invert= tuple with the actual diffpairs.

@whitequark
Copy link
Member

whitequark commented Oct 20, 2020

As an alternative I was wondering if something like DiffPairs("A1 !A2", "B1 !B2") would work. The ! sigils would then have to match in both the P and the N pin lists. Of course, invert=True should still invert all of them, and conversely, it probably makes sense to accept ! for single-ended pins too.

@adamgreig
Copy link
Contributor

I like putting it in the pin list string(s), it's compact but I think also quite clear. Maybe "~A1" would be a better match to the nmigen negation syntax?

@whitequark
Copy link
Member

Sure, ~ is fine by me.

@anuejn
Copy link
Contributor Author

anuejn commented Oct 20, 2020

I also really like that idea :)

@whitequark
Copy link
Member

Sounds great. I'll bring this up on the next meeting but I think you can start implementing it as it seems unlikely anyone's going to be opposed.

@anuejn
Copy link
Contributor Author

anuejn commented Oct 20, 2020

sounds good :)

@anuejn
Copy link
Contributor Author

anuejn commented Oct 21, 2020

I think we should support similar syntax for the use with connectors (which can be routed through e.g. inverted level shifters or have crossed differential pairs). So I think it would be nice to have syntax elements in connectors that can say "invert this signal if it is a diff pair" or "invert this signal always".

I propose that we use ~ for always invert and ? for invert in differential pairs in connectors.

@whitequark
Copy link
Member

I would like to avoid introducing that functionality (and the associated syntactic burden) until someone actually needs it.

@anuejn
Copy link
Contributor Author

anuejn commented Oct 21, 2020

/me needs the latter (inverted differential pairs in connectors)

@whitequark
Copy link
Member

Okay. I still think it should be done separately because while this one is fairly straightforward (it's just a way to write a tuple that's less error prone than usual), what you request with connectors requires completely rethinking the way we do connectors.

@anuejn
Copy link
Contributor Author

anuejn commented Oct 21, 2020

Okay I see that.
So I will implement this RFC as-is and go with out of tree hacks for now.
If I really need this feature to become upstream I will write another RFC :)

@whitequark
Copy link
Member

I think we can do it upstream; I said earlier that I'd like to avoid introducing it because it sounded theoretical (which is usually bad because features done in abstract tend to not survive collision with reality). If you actually hit that problem I think we can certainly add it. Let's discuss the connector semantics on the next meeting?

@anuejn
Copy link
Contributor Author

anuejn commented Oct 21, 2020

Okay 👍

@whitequark
Copy link
Member

We discussed inversion of connector signals at the last meeting. Because inversion of single ended signals is rarely needed and can also complicate things even further (you have to keep track of two layers of inversion rather than one), we decided that we would implement inversion in connectors but only for diff pairs. Connecting a single-ended signal to an inverted connector pin would be a hard error. If it turns out to be worth the complexity in the future we can always accept it.

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

No branches or pull requests

3 participants