development question: pin abstraction layer and Teensy 4.x

[stapelberg]

Currently, in tmk_core/common/chibios/pin_defs.h, we have a bunch of defines like these:

#    define A0 PAL_LINE(GPIOA, 0)
#    define A1 PAL_LINE(GPIOA, 1)
#    define A2 PAL_LINE(GPIOA, 2)
#    define A3 PAL_LINE(GPIOA, 3)
#    define A4 PAL_LINE(GPIOA, 4)
#    define A5 PAL_LINE(GPIOA, 5)
#    define A6 PAL_LINE(GPIOA, 6)
#    define A7 PAL_LINE(GPIOA, 7)
#    define A8 PAL_LINE(GPIOA, 8)
#    define A9 PAL_LINE(GPIOA, 9)
#    define A10 PAL_LINE(GPIOA, 10)
#    define A11 PAL_LINE(GPIOA, 11)
#    define A12 PAL_LINE(GPIOA, 12)
#    define A13 PAL_LINE(GPIOA, 13)
#    define A14 PAL_LINE(GPIOA, 14)
#    define A15 PAL_LINE(GPIOA, 15)
#    define B0 PAL_LINE(GPIOB, 0)
[…]

These correspond to the pin names from the schematic (excerpt):

---

On the Teensy 4.x (NXP MIMXRT1062), however, the GPIO blocks are named GPIO1 to GPIO9 (numbers instead of letters).

Worse, the pin names no longer follow the PT… scheme, and seem all over the place:

Parts of QMK seem to require pin names to follow the <letter><digits> format, e.g.:

qmk_firmware/lib/python/qmk/info.py

Line 146 in 6bafefa

elif pin[0] in 'ABCDEFGHIJK' and pin[1].isdigit():

This requirement wasn’t as strict when I started development of my kint41 controller, but has gotten stricter over the time, to the point where I can’t rebase my changes onto current QMK develop because of the pin naming.

---

What is the desired solution for pin naming with the Teensy 4.x?

Thanks

[tzarc]

Just spitballing..... would it be worth adding a new platform under platforms/chibios/ for the 4.x's, with a config.h file specifically defining the pins as per the normal Teensy pinout? Rather than perpetuating the centralisation of the pins in config_common.h -- we can couple it tightly to the platform it runs on instead?

[tzarc]

Main rationale being things like the Proton-C being defined as the Pro Micro pinout when using CTPC=yes -- we have overrides in config_common.h specifically for it -- in hindsight is it the best place for that?

[tzarc]

Also, kinda dodgy and I don't recommend it... but #define T4_13 ..... would pass that check, no?

Errr, disregard, my brain totally skipped the 'ABCDEFGHIJK'.

[stapelberg]

Just spitballing..... would it be worth adding a new platform under platforms/chibios/ for the 4.x's, with a config.h file specifically defining the pins as per the normal Teensy pinout? Rather than perpetuating the centralisation of the pins in config_common.h -- we can couple it tightly to the platform it runs on instead?

Tightly coupling the pin definitions to the platform of the keyboard sounds good to me. That should give us enough flexibility to deal with all pin naming schemes.

There already is https://github.com/ChibiOS/ChibiOS-Contrib/blob/chibios-20.3.x/os/hal/boards/PJRC_TEENSY_4_1/board.h, which already defines the LINE_PIN0 .. LINE_PIN54 macros.

I’m assuming you are suggesting a new platforms/chibios/IC_TEENSY_4_1 (next to existing IC_TEENSY_3_1) so that we can add our own macros that follow a different naming scheme than LINE_PINxx, right?

What should this naming scheme be? T4_1 .. T4_54? Would it be okay to modify the allowed pin names as I did in kinx-project@791dc61, or do we want to avoid that?

I’m not super familar with all the different includes and build system configuration that QMK uses, so I appreciate your guidance on this! Thanks!

[stapelberg]

@tzarc friendly ping for the questions in my previous comment? :) Thanks!

[stapelberg]

So we now have the platforms/chibios/IC_TEENSY_4_1 subdirectory, but I’m still unclear on what naming scheme would be okay for the teensy pins. @tzarc any thoughts?

Thanks,

[tzarc]

Will ping @qmk/collaborators internally and see how we'd like to proceed. Sorry for the holdup.

[tzarc]

So after some discussion we're okay with allowing for LINE_PINxx.
No need for QMK-side definitions, obviously.

The link to the validation modifications is almost fine as-is, however the suggestion would be to split it out into a separate entry in the "oneOf" section, so that there's an explicit rule rather than piggybacking off the existing one.

[stapelberg]

Thanks for following up! PR #13247 sent