visrealm / pico9918

A replacement for the classic TMS9918A/TMS9929A VDP, powered by a Raspberry Pi Pico
https://hackaday.io/project/196478-pico9918
MIT License
128 stars 7 forks source link

Comments in main.c are misleading vis a vis CD0-CD7 #12

Closed thorpej closed 2 months ago

thorpej commented 2 months ago

The comments in main.c about the CD0-CD7 pin mappings are misleading. TI's convention calls the LSB "CD7" and the MSB "CD0". The nomenclature is correct on the v0.3 schematic, but the comments in main.c are incorrect. A corrected comment block would be:

  * Pin  | GPIO | Name      | TMS9918A Pin
  * -----+------+-----------+-------------
  *  19  |  14  |  CD7      |  17         LEAST SIGNIFICANT BIT
  *  20  |  15  |  CD6      |  18
  *  21  |  16  |  CD5      |  19
  *  22  |  17  |  CD4      |  20
  *  24  |  18  |  CD3      |  21
  *  25  |  19  |  CD2      |  22
  *  26  |  20  |  CD1      |  23
  *  27  |  21  |  CD0      |  24         MOST SIGNIFICANT BIT
.
.
.

I also suggest renaming the constant GPIO_CD0 to GPIO_CD7.

visrealm commented 2 months ago

Thanks. You're right, this was left over from v0.2 which actually had this pinout. They were reversed like this resulting in any gpio operations on the CD0-7 pins needing to be reversed via a lookup table. It was a messy time. 😁

visrealm commented 2 months ago

Closed as fixed in dev