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

Unexpected inversion when using inverted registered outputs #16

Closed
and3rson opened this issue Nov 23, 2023 · 5 comments
Closed

Unexpected inversion when using inverted registered outputs #16

and3rson opened this issue Nov 23, 2023 · 5 comments
Assignees

Comments

@and3rson
Copy link

and3rson commented Nov 23, 2023

Consider this example:

GAL22V10
PS2

CLK  A    B    NC   NC   NC   NC   NC   NC   NC   NC   GND
/CS  Q1   Q2   NC   NC   NC   NC   NC   NC   NC   NC   VCC

/Q1.R = A * B
/Q2.R = /Q1

DESCRIPTION
t

If I invert Q1 and Q2 in pin definitions as such:

< /CS  Q1   Q2   NC   NC   NC   NC   NC   NC   NC   NC   VCC
---
> /CS /Q1  /Q2   NC   NC   NC   NC   NC   NC   NC   NC   VCC

...I can observe the following difference in fuses:

132c132
< Pin 15 = Q2           S0 = 0   S1 = 0
---
> Pin 15 = /Q2          S0 = 1   S1 = 0
134c134
< 112  ---- ---- ---- ---- ---- ---- ---- ---- ---- --x- ----
---
> 112  ---- ---- ---- ---- ---- ---- ---- ---- ---- ---x ----
145c145
< Pin 14 = Q1           S0 = 0   S1 = 0
---
> Pin 14 = /Q1          S0 = 1   S1 = 0

I would expect only the S0 bit to change. However, the actual column is also getting changed (from true to inverted).

I checked the datasheet (http://web.mit.edu/6.115/www/document/gal22v10.pdf). On page 4 in the top section (S1=0) it's stated that the feedback originates from /Q while the pin itself is driven by Q, so pin inversion does not have any influence on the feedback since it's driven by a separate driver.
This means that S0=0,S1=0 and S0=1,S1=0 should only change the behavior of the physical output, not the matrix. In other words, inverting a pin in the definition section should only change the S0 flag.
Is this a bug or am I misunderstanding something?

@and3rson
Copy link
Author

and3rson commented Nov 24, 2023

I think I figured it out.

GALasm implicitly changes S0 of OLMC if polarities of pin definition and pin equation differ.

e.g.

/CS /Q ; ...

/Q.R = A * B

; Q: S0 = 1, S1 = 0
/CS /Q ; ...

Q.R = A * B

; Q: S0 = 0, S1 = 1

@and3rson
Copy link
Author

and3rson commented Nov 25, 2023

Another update. Consider the following equations:

/Q1.R = A * B
Q2.R = Q1

When assembled with galasm and then disassembled with jed2eqn, I get the following equations:

/Q1 := A * B
Q2 := /Q1

I expected Q1 & Q2 to be equal, but they are not. In other words - I expected /X = equation to be a different way of writing X = /equation (which helps when having multiple OR terms). I could also reproduce the same behavior with https://github.com/simon-frankau/galette.

@and3rson
Copy link
Author

and3rson commented Nov 26, 2023

I think the offending line is galasm:1605 which shouldn't be there, since it inverts the pin (inside equation) if it is active-low. But being active-low should not affect any equations (as per the datasheet).
Seems like this line is needed due to register feedback being from a negative flip-flop output. So the spurious inversion might be coming from a different place.

EDIT: Possible fix:

diff --git a/src/galasm.c b/src/galasm.c
index 052ad70..9788e91 100644
--- a/src/galasm.c
+++ b/src/galasm.c
@@ -1600,8 +1600,10 @@ void SetAND(int row, int pinnum, int negation, int gal_type)

                                     /* is it a registered OLMC pin?   */
                                     /* yes, then correct the negation */
-            if ((pinnum >= 14 && pinnum <= 23) && !Jedec.GALS1[23 - pinnum])
+            if ((pinnum >= 14 && pinnum <= 23) && !Jedec.GALS1[23 - pinnum] && Jedec.GALXOR[23 - pinnum])
             {
                 negation = negation ? 0 : 1;
             }

This prevents inversion of registered pin if it's active-low.

Tests after applying this fix:

; Source
A  B
Q1 Q2

/Q1.R = A * B
Q2.R = Q1

; Disassembled (before fix)
/Q1 := A * B
Q2 := /Q1    ; WRONG

; Disassembled (after fix)
/Q1 := A * B
Q2 := Q1
; Source
 A  B
/Q1 Q2

Q1.R = A * B
Q2.R = Q1

; Disassembled (before fix)
/Q1 := A * B
Q2 := Q1     ; WRONG

; Disassembled (after fix)
/Q1 := A * B
Q2 := /Q1

@pjdennis pjdennis self-assigned this Nov 29, 2023
@pjdennis
Copy link
Collaborator

I tried out the initial example on an ATF22V10C without the proposed fix and confirmed wrong behavior - Q2 is inverted from what it should be:

CLK  A    B    NC   NC   NC   NC   NC   NC   NC   NC   GND
/CS  Q1   Q2   NC   NC   NC   NC   NC   NC   NC   NC   VCC

/Q1.R = A * B
/Q2.R = /Q1

---

Start with A Low; B Low
Toggle CLK twice
Result: Q1 High; Q2 Low but should be Q1 High; Q2 High

With the proposed fix, this example works correctly, as does the modified example having Q1 and Q2 inverted in the pin definitions.

The proposed fix seems reasonable, based on the fact that feedback is unaffected by pin inversion in registered mode. I'll spend a bit more time reviewing before pushing the update.

Thanks @and3rson for the report and detailed info.

pjdennis added a commit that referenced this issue Nov 30, 2023
@pjdennis
Copy link
Collaborator

Fixed in commit c376d56

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

No branches or pull requests

2 participants