Unfortunately it doesn't seem to work, because the ST HAL defines constants like TIM2, USART2, SDIO etc.

I was trying to use the macro with sdcard.c, lines 153-158.

Oh, are the fn tokens being expanded before they get joined to the unit for you?

I did wonder if it would be better to just pass in the entire enum value STATIC_AF_FMC_SDCLK as teh function argument then at least autocomplete / code navigation has a chance of working, rather than anonymous tokens that are being joined later

Oh, are the fn tokens being expanded before they get joined to the unit for you?

Yes. I think with FMC it wasn't, but other periphs are.

I did wonder if it would be better to just pass in the entire enum value STATIC_AF_FMC_SDCLK

Maybe that is a better idea. Less macro magic, less things to go wrong. At the expense of more typing. But it's not much more typing.

Ok it's set up to pass the enum as the arg now, looks to be working well on my end.

I've updated&rebased the sdram pr to suit as well.

I was about to merge, and double checking it, but unfortunately it still has some problems!

The point is that the compiler doesn't know that pin objects have distinct addresses. So it can't optimise something like this:

&pin_A1_obj == &pin_A0_obj ? 1 :
&pin_A1_obj == &pin_A1_obj ? 2 :
-1

The first check may succeed if pin_A0_obj is aliased to pin_A1_obj. I'm not sure how to fix that, or if there is a way to work around it.

Doesn't having the pin_A1_obj declared as const mean the compiler should know it's fixed?
Have you got some way of testing that the compiler isn't optimising it?

ok, to test I've updated pin_static_af.h

#define mp_hal_pin_config_alt_static(pin_obj, mode, pull, fn_type) \
        mp_hal_pin_config(pin_obj, mode, pull, STATIC_AF(pin_obj, fn_type)); \
        MP_STATIC_ASSERT(STATIC_AF(pin_obj, fn_type) != -1);\
        MP_STATIC_ASSERT(__builtin_constant_p(STATIC_AF(pin_obj, fn_type)) == 1)

This still compiles, so I'm pretty sure __builtin_constant_p appears to be reporting that it is recognised as a constant.

Have you got some way of testing that the compiler isn't optimising it?

Two ways: 1) inspect the assembly output; 2) use _Static_assert instead of MP_STATIC_ASSERT to detect non-constant values.

I found this issue when converting sdcard.c to use the new macro, the SDMMC2 path. It only happens when the same AF is available on more than one pin.

Yep thanks, I've confirmed the issue.

Not yet sure how to resolve this one, I've tried a few games with casting / const'ing in the auto-generated table to no avail.... not sure I fully understand the root cause yet, need more investigation.

I reckon I've got it sorted. Turns out the compiler is happy to coimpile-time calculate strcmp.
As such stringify'ing the pin definition and checking that works well.

I've left the (#defined out) version of the macro/function used to confirm that it is statically constant and working, looks good for the sdcard on the 769disc target too.

I've lost track of the number of trials it took to get this far... geez I hate macro magic. Why can't they just bring constexpr from c++ back into c!

The autogenerated file however is now a much simpler format, similar to your other initial suggestion.
This is an improvement even without the stringify compare test; comparing the pin pointer objects worked better like this in that some would compile time calculate, but at least the ones that are runtime calc are much smaller and arguably efficient enough.

The stringify's are more efficient again, but at the expense of being more strict about the format of the pin definition being passed in. I don't see this being a problem though?

Thanks a lot for working on this, it does look a lot better now. Really glad you found a way to fix it!

But still one minor thing: the STATIC_AF_* macros seem to be too smart for the MP_STATIC_ASSERT() to work, and if you give it a pin that doesn't have the alt function it will happily pass through -1 as the argument for the alt func, and at the same time MP_STATIC_ASSERT() will pass!

This could possibly be fixed by making MP_STATIC_ASSERT() better, but that's a bit out of scope of what we are trying to do here. (Maybe define this macro to use _Static_assert if it's provided by the compiler.)

As a workaround, I tried changing the -1 to 0xffffffffffffffffULL so that it warns about an overflowing constant literal if it gets to that path in the macro. That seems to work OK for me, with both MP_STATIC_ASSERT and the _Static_assert variants in pin_static_af.h. I'd suggest going this route, to be sure you get some kind of warning if the AF is wrong.

The stringify's are more efficient again, but at the expense of being more strict about the format of the pin definition being passed in. I don't see this being a problem though?

It should be ok. It does mean though that you must have the correct number of parenthesis, so this won't work:

mp_hal_pin_config_alt_static(pyb_pin_SD_CK, MP_HAL_PIN_MODE_ALT, MP_HAL_PIN_PULL_UP, STATIC_AF_SDMMC2_CK);

it instead needs to be (not extra parenthesis):

mp_hal_pin_config_alt_static((pyb_pin_SD_CK), MP_HAL_PIN_MODE_ALT, MP_HAL_PIN_PULL_UP, STATIC_AF_SDMMC2_CK);

But I'd say that's acceptable. These pin definitions should anyway be specified in mpconfigboard.h where everything uses parenthesis. And it'll give an error if the parenthesis are wrong because it won't match any of the strings.

In summary: if you're happy changing -1 to 0xffffffffffffffffULL then it's good to go (and I can do this during merge if you like).

Ah, I forgot to test that it still worked with MP_STATIC_ASSERT, thanks for following that up!
I'm quite happy with an overflow to force the fail, yet another win for warnings-as-errors. With this the MP_STATIC_ASSERT isn't needed at all, the warning is thrown from the function argument.

Yeah I do feel a little uncomfortable about the somewhat-hidden restriction on pin label/defining format this this macro magic enforces. Perhaps it would be possible to spin the macro's more to be slightly more flexible on brackets, but nothing really comes to mind.

Do you have any objection to two strcmp's &'d together, to cover these two (most likely?) use cases?
Have a look at this update. I've moved the "error" comment too so it shows up in the gcc error message when the AF is wrong pin, although it's still a little hidden in the overflow error message it's better than nothing.

ps. thanks so much for building a platform with enforced no-warnings, it's fantastic to see that in an open-source project of this size!

Do you have any objection to two strcmp's &'d together, to cover these two (most likely?) use cases?

That's ok.

I checked the latest version of this PR and verified that it all works as expected now. And I've now merged it in 4343c93 . sdcard.c was switched to use this new function/macro in 4201f36

Thanks for the effort on this!

thanks so much for building a platform with enforced no-warnings, it's fantastic to see that in an open-source project of this size!

You're welcome!