ole00 / afterburner

GAL chip programmer for Arduino
157 stars 44 forks source link

Support 26cv12 and 26v12 #42

Closed rhgndf closed 10 months ago

rhgndf commented 11 months ago

This is done with the GALmate adapter. Reading and writing works.

img1 img2

ole00 commented 10 months ago

I'm wondering whether you had time to look at my comments related to PINOUT* as set in the 'galinfo' array? The pinout values do not seem to be correct for UNKNOWN, GAL16V8, GAL18V10 and GAL20V8. Also, the change in afterburner.c (if ((i % 32 != 1) && fuseSet) ...) does not seem to be an obvious bug - could you explain? Thanks.

ole00 commented 10 months ago

Related to the last change in afterburner.c: I think both the original code and your new fix is incorrect. Original code has a bug if there is any fuse set and the total fuse length is 32 fuses. In such case (i%32) is 0 because the 'i' is incremented to 32 at the end of the 'for' loop - therefore the modulo is 0 and the fuses would not be sent. Your new code would not work if there is only 1 fuse set to 1 (or there is 33 fuses and the last fuse is set to 1, etc.). In such case the last fuse would not be sent because the modulo would be 1. I think the proper fix is to only check 'fuseSet' - if it is set then send the fuses.

rhgndf commented 10 months ago

I'm wondering whether you had time to look at my comments related to PINOUT* as set in the 'galinfo' array? The pinout values do not seem to be correct for UNKNOWN, GAL16V8, GAL18V10 and GAL20V8. Also, the change in afterburner.c (if ((i % 32 != 1) && fuseSet) ...) does not seem to be an obvious bug - could you explain? Thanks.

Oops! I think something went wrong when pushing the code, I thought I had fixed it in the 'Fix pinout' commit, which might be me rebasing it to master incorrectly, as seen in the previous commit where it is changed: ff0665e47740f0048a4361904c87e96db95df455. Also, if you mean github code review comments, I didn't notice it since probably due to the rebase.

if ((i % 32 != 1)

Actually yeah this might be wrong, the if statement should just be only checking for fuseSet, since for sendLine to run there is at least an additional bit that is put inside buf. Here is the bug I encountered, the last few bits are not sent to the arduino when uploading 26CV12. I think this happens when totalFuses is a multiple of 32. If totalFuses = 32:

  1. The loop executes 4 times, since in each iteration i is incremented 8 times. The sendLine function is never called
  2. i is 32 after breaking out of the loop
  3. i % 32 is now 0, i % 32 evaluates to false, so the if statement is not executed, so sendLine is not called.
rhgndf commented 10 months ago

Ignore the second part 😅, I just saw your next comment, so I'm just going to push the fix.

ole00 commented 10 months ago

Thanks, there is no rush.