Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:  -292 -0.080% TEENSY40[incl -248(data)]
        rp2:   -88 -0.027% RPI_PICO
       samd:  -188 -0.072% ADAFRUIT_ITSYBITSY_M4_EXPRESS

Yes, this make_pins stuff is pretty feral.
I have a unpublished PR for SAMD which swaps the fields in pins.csv, which is different ATM to the other ports. I can push that now, such that it is at least visible.

Codecov Report

Merging #12211 (6ef9b29) into master (aa329d1) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #12211   +/-   ##
=======================================
  Coverage   98.39%   98.39%           
=======================================
  Files         158      158           
  Lines       20973    20973           
=======================================
  Hits        20637    20637           
  Misses        336      336           

@jimmo. I added a commit to PR #12212 to match the argument names of the make-pins.py equivalent to your PR here, and also renamed that script to make-pins.py.

@jimmo. I added a commit to PR #12212 to match the argument names of the make-pins.py equivalent to your PR here, and also renamed that script to make-pins.py.

Thanks for that @robert-hh , I will integrate that into this PR.

One question with SAMD, I notice the convention in this port for cpu pins up to 10 appears to be PA01 whereas on STM32 that would be PA1 (i.e. samd is adding the leading zero).

In general I think we should match the style of what the vendor calls them, but also I would like where possible for things to be as consistent as possible across ports, and of course I'm hesitant to break compatibility here. What should we do?

I will integrate that into this PR.

This commit must be used with the first commit in the PR, which swaps the columns of the pins.csv files. Otherwise the builds will fail.

I notice the convention in this port for cpu pins up to 10 appears to be PA01

MicroChip uses consistently the 2-digit numbering for pins in their documents and library. See e.g. chapter 32.5.1 of the SAMD51 data sheet:
"The I/O lines of the PORT are mapped to pins of the physical device. The following naming scheme is used:
Each line bundle with up to 32 lines is assigned an identifier 'xy', with letter x=A, B, C… and two-digit number y=00, 01, …31. Examples: A24, C03.
PORT pins are labeled 'Pxy' accordingly, for example PA24, PC03. This identifies each pin in the device uniquely."
The board vendors do the same in their schematics, when they denote MCU pins. So I think we should stick to it.

So I think we should stick to it.

OK, makes sense to me! Thanks @robert-hh

@robert-hh I've updated this PR with samd and mimxrt using boardgen.py now.

Getting lost between 2 PRs. What I wanted to say:

You digged up quite a bit of the code, which was grown over time and may have needed some clean-up. From a distance, things look sometimes more clear. I made a trial build for a SAMD51 & SAMD21 board and noticed, that the the element name in machine_pin_obj_t is not properly assigned any more. It used to be the board pin name or '-', if no name is assigned. Now it is the CPU pin name, meaning that the function pin_name() returns the wrong value.

Teensy 4.1, SEEED_ARCH_MIX and MIMXRT1176_EVK do not build. And OLIMEX_RT1010 and ADAFRUIT_METRO_M7 still have the UART RTS/CTS bug (not your playground).

Edit: I see. The respective Pin definitions with the leading dash are missing in pins.csv

the element name in machine_pin_obj_t is not properly assigned any more. It used to be the board pin name or '-', if no name is assigned. Now it is the CPU pin name, meaning that the function pin_name() returns the wrong value.

That was intentional, and it's how it behaves on other ports. e.g. on stm32:

>>> machine.Pin.board.LED_RED
Pin(Pin.cpu.B1, mode=Pin.OUT)

I've updated the samd printing to match the same style. FYI you can use %q as a format specifier with mp_printf to directly print a qstr, so the pin_name function converting to char* wasn't necessary.

I agree it's neat that the pin knew its board name, but I'd rather make this consistent.

Teensy 4.1, SEEED_ARCH_MIX and MIMXRT1176_EVK do not build.

The way pins.csv works on other ports is that unless a pin is in pins.csv it doesn't get generated. (With the exception of rp2 and esp32 which always generate the fixed set of numeric cpu pins).

But to keep "pin to board" mapping centralised, this is also supported by pins.csv even for pins that will not be made available to Python. This is why the stm32 pins.csv (and now everything using boardgen.py) supports "hidden" pins. The idea is that you can add, e.g.

-ENET_RESET,-GPIO_LPSR_12

to pins.csv to make pin_ENET_RESET available (pointing to pin_GPIO_LPSR_12_obj), but neither will be in Pin.board/Pin.cpu. So this means eth.c can use pin_ENET_RESET, removing the defines in mpconfigboard.h.

Anyway, I had already fixed this for the other boards, but missed TEENSY41 and missed the 1G eth pins for MIMXRT1176_EVK.

Similarly for the USDHC1 pins, these also needed to be added to pins.csv.

I've patched in the changes from the RTS/CTS PR and now all boards are compiling.

That was intentional, and it's how it behaves on other ports. e.g. on stm32:

I do not agree to that change. The name field is intended to show the board pin name, so that the Pin print shows both the board name and the CPU name. Otherwise the field itself is obsolete, because the cpu name and the pin id are directly related. And since it's more likely that people use the board name for pins, it should be shown when devices properties are printed.

The changes for the MIMXRT boards seems fine. The three boards that did not build before build and Ethernet works. The info about the hidden pins is useful.

I agree it's neat that the pin knew its board name

From a usage perspective it's essential, not neat. People should be able to tell the pin name that is shown by the a board API without looking into information which exists somewhere.

People should be able to tell the pin name that is shown by the a board API without looking into information which exists somewhere.

I agree this is useful, but if we're going to do this then I think we need to agree to do it on all ports, and currently none of samd, stm32, rp2, esp32, nrf show the board name. There are some other minor considerations, for example it is technically possible for a cpu pin to have multiple board aliases.

currently none of samd, stm32, rp2, esp32, nrf show the board name.

samd does.
Anyhow. I agree that it is required to have a common baseline between ports. But the mapping between a pins cpu name and the board name is a one way road. People look at the board and see the board name. So there should be a way back to get the board name by a pin object or pin id. For the SAMD port there is the samd.pininfo() method, which returns the board name and the pin af vector and which is used by the pininfo.py script to print a pin assignment table. People found that very useful when actually using the board. For that, a way is needed to return the board name of a pin e.g. from the pin.board dictionary.

Another thing that is missing: when testing with the SAMD port i noticed, that pins which are not assigned to a board name are missing in the Pin.cpu list. At the moment they are present and it is useful to have them.

Same for MIMXRT.

Pin.cpu must contain all Pins of the MCU. The respective pin_af.csv tables list all possible pins. So that must be used for creating Pin.cpu.

Pin.cpu must contain all Pins of the MCU. The respective pin_af.csv tables list all possible pins. So that must be used for creating Pin.cpu.

This change is also for compatibility with the other ports. I realise that there isn't an easily definable "common behavior" because there is so much variation in each of the make-pins.py implementations, so to be clear, wherever there's a discrepancy I'm using stm32 as the reference implementation.

The idea is that if a pin isn't in the board pins.csv, then it's not "on the board" and therefore there would be no way to access it and no reason to make it available. "on the board" doesn't just mean "available on a header" though, it's more that the pin is routed somewhere on the board and has some function.

And when it comes to making it available, the "hidden" pins feature allows it to be just made available to C (e.g. the ETH_RESET pins discussed earlier are a good example). CPU pins also don't have to be given board aliases -- e.g. ,PA1 is a valid row in pins.csv.

One obvious reason for doing this is code size -- there's no point generating machine_pin_obj_t instances in ROM for every possible CPU pin if we know it's not used anywhere.

So with the example you ran into with SAMD testing, the intention is that the pin should have been in pins.csv (possibly without a board name). Is that reasonable? Or is there another scenario I'm missing?

FWIW, on stm32, not all board definitions get this right, so some clean-up here is in order too. For example there are many boards with pins.csv that just list every possible pin as PAxy,PAxy, which is pointless (adding the CPU names to Pin.board). Although many boards do list the CPU name on the silkscreen...so you could argue this is correct in some cases (but I think when a pin is clearly the CPU name, it should not be in Pin.board).

There are two exceptions to this on existing ports -- esp32 and rp2, where we added named pin support after board definition support, and so we have boards that haven't been updated with pins.csv files. So for now all CPU pins are enabled. But this should be fixed once we have pins.csv for all boards. ESP32 also doesn't have the Pin.cpu dict at all.

I really feel that consistency is the most important thing. Once everything is using boardgen.py, we can make changes as necessary in one place that apply to all ports.

"on the board" doesn't just mean "available on a header" though, it's more that the pin is routed somewhere on the board and has some function.

I verified for the SAMD and MIMXRT port that one can simply add all pins from the pin_af.csv file to pins.csv and get back a complete list of usable CPU pins into Pin.cpu. Usable CPU pins could both be pins that are routed to the board, or pins that are just internal to the MCU package but have a function, like those connected to an in-package peripheral. Adding that to each pins.csv file is possible, but lacks elegance. An option would be an include mechanism or a addition cpu_pins.csv file for common subsets.

there's no point generating machine_pin_obj_t instances in ROM for every possible CPU pin if we know it's not used anywhere.

How can you know, what users will use, how users will use boards? We should not patronize users.

wherever there's a discrepancy I'm using stm32 as the reference implementation.

The STM32 port has many excellent features implemented. But it carries with it a lot of legacy and history, that may not all be useful for getting a new common structure. I prefer to use the RP2 port as a reference. It is way more consistent and clear. Obviously it's for a single MCU only, which makes it easier to read and maintain.

Once everything is using boardgen.py, we can make changes as necessary in one place that apply to all ports.

I do not like to have a state which breaks things that worked before, just to eventually bring them back sometime later, if a PR for it is made and if it will be processed.

there's no point generating machine_pin_obj_t instances in ROM for every possible CPU pin if we know it's not used anywhere.

How can you know, what users will use, how users will use boards? We should not patronize users.

In my understanding of this there's a difference between what a CPU can provide and what a user can physically get access to.

A solid example that comes to mind is the Seeed Wio-E5 Wireless Module, from a micropython point of view this is just a STM32WL chip and SX126X Lora radio. It's a sealed can module, with only a small subset of the STM pins routed to the outside world and internal lora transceiver.
I definitely think it makes sense on a module (or board using this module) to only expose pins that are routed out of the module / used internally for radio, any others are just consfusing noise taking up flash space.

On a more generic front, if a board only has some CPU pins routed from the chip to peripherals / headers / test-points with the rest not even having tracks laid, I'd prefer to not have those blank pins included by default.
If I'm going to take a board like that and dead-bug wire some other parts onto those cpu pins, well I'm also happy to custom compile my firmware as it's no longer matching that board definition.

In my understanding of this there's a difference between what a CPU can provide and what a user can physically get access to.

Physical access is not the only aspect. It should include the aspect of what user code can use, even if a CPU pin is not routed on or to the board. Two examples:

• At the SEEED XIAO SAMD21 the PA03 is not routed to the board edge. PA03 is the AREF pin, and kept floating. That makes using the ADC a little bit tricky. But since it is available as CPU pin, one set set it to high level and have a AREF voltage applied at the pin.
• The MIMXRT PWM allows to synchronize PWM channels to another specific channel of a PWM module. These specific channels may not be routed to a board connector, but that's not required to use the sync mechanism. They must just be available to the code as Pin object.
  The may be more examples of situation, where a CPU pin can be used even if not routed to the board connectors or used on the board. Not all people a willing or able to build their own firmware in case they miss a pin definition. So why make things more complicated and restrictive? Pin.board may list only the labelled pins and dedicated pins on the board. But Pin.cpu should have all that can be used by code.

Let's keep the discussion about the ADCs in #12706. I've replicated @iabdalkader's comment above there.

Merged!

Any remaining inconsistencies/regressions can be dealt with separately.