boards: pinmux: Don't throw compiler warnings on using custom sercom use

[KubaFYI]

In order to use it's serial peripheral SERCOMs SAMD socs require their
pins to be configured with pinmux. Currently pins are muxed only for the
boards' default serial interfaces. The pinmux code is written to throw
a compiler error if a user tries to use any sercom in a way it wasn't
pre-defined to be used.

This commit changes compiler errors to warnings so that user can provide
custom pinmuxing code in their app.

Fixes #23133

Signed-off-by: Kuba Sanak contact@kuba.fyi

[stephanosio]

The pinmux code is written to throw a compiler error if a user tries to use any sercom in a way it wasn't pre-defined to be used.

I think that is quite self-explanatory of why this should be an error (i.e. you are trying to use the pin mappings that are not supported/should not be used on the board).

If you encounter these errors, you are probably using a different board with the same SoC or doing something not originally intended with the board- in which case, you should probably create a new board definition.

If the board can (and should) actually support such pin mappings yet pinmux.c disallows it, then the proper pin configurations using pinmux_pin_set for such mappings should be added instead.

[KubaFYI]

I think that is quite self-explanatory of why this should be an error (i.e. you are trying to use the pin mappings that are not supported/should not be used on the board).

If you encounter these errors, you are probably using a different board with the same SoC or doing something not originally intended with the board- in which case, you should probably create a new board definition.

Except that this might not necessarily be the case. Some of those boards (e.g. the Adafruit Feather and Trinket) are prototyping boards and have misc pins brought out to be used freely, instead of having their purpose set in stone. The way pinmux throws a fuss prevents any of those supposedly general use pins available on the boards to be used with the HW serial capability of the SOCs - which goes against what prototyping boards are intended for.

I suppose that if you really wanted to do make sure that no incorrect muxing happens that would need to be done in the pinmux driver itself. Currently as it stands the user can still set pins to something silly in their app. Trying to prevent incorrect pin configuration by stopping users from having custom SERCOMs is like trying to prevent unlicensed drivers from driving by banning cars.

[stephanosio]

I suppose that if you really wanted to do make sure that no incorrect muxing happens that would need to be done in the pinmux driver itself. Currently as it stands the user can still set pins to something silly in their app. Trying to prevent incorrect pin configuration by stopping users from having custom SERCOMs is like trying to prevent unlicensed drivers from driving by banning cars.

I am not suggesting that we block users from using/customising the board in the way they want.

What I am saying is that, if they want to do that, they should do it properly- i.e. in pinmux.c.

This commit changes compiler errors to warnings so that user can provide custom pinmuxing code in their app.

This is not something that should be encouraged, as it can create unnecessary clutters and introduce potential bugs.

[KubaFYI]

What I am saying is that, if they want to do that, they should do it properly- i.e. in pinmux.c.

I assumed that the design philosophy for zephyr is that users should be just able to do everything their board supports from the app, without modifying zephyr code. If that's not the case then I yield. If it is the case - I guess my argument is that prototyping boards like Adafruit Feather, with their pins brought out to be used for whatever, should support the use of one of the extra SERCOMs with them.

This is not something that should be encouraged, as it can create unnecessary clutters and introduce potential bugs.

I understand the clutter argument (though it would be only a single warning message per custom SERCOM used, so not exactly tons of warnings), but how is that likely to introduce bugs?

[henrikbrixandersen]

I think it is okay to reduce this to a warning.

[stephanosio]

I suppose this is okay given that the CI treats all warnings as errors and we won't ever see any bad practices being merged in the future.

[KubaFYI]

@galak & @MaureenHelm is there anything else I need to do with this?

[nandojve]

I'm usually use a board referenced on Zephyr with .conf and .overlay. This way I can enable/disable DT nodes and extend to my own app and implement necessary pinmux complement.

There are some of this on using SAM series: https://github.com/zephyrproject-rtos/zephyr/tree/master/samples/net/sockets/echo_client/boards using.

I was wondering if will it works in general cases like presented here with pinmux.c defined WEAK?

[KubaFYI]

Hey @nandojve could you please elaborate what you mean. Do you mean you reimplement the pinmux.c file present in the board's directory?

There are some of this on using SAM series: https://github.com/zephyrproject-rtos/zephyr/tree/master/samples/net/sockets/echo_client/boards using.

Those files don't really seem to mention pinmux at all.

[nandojve]

Hey @nandojve could you please elaborate what you mean. Do you mean you reimplement the pinmux.c file present in the board's directory?

There are some of this on using SAM series: https://github.com/zephyrproject-rtos/zephyr/tree/master/samples/net/sockets/echo_client/boards using.

Those files don't really seem to mention pinmux at all.

I mean, we update original file with weak attribute

master/boards/arm/atsamr21_xpro/pinmux.c
static int board_pinmux_init(struct device *dev) attribute((weak))

than when appropriated use <board>,config and <board>.overlay to get you project configured.
With that, you simple add *int board_pinmux_init(struct device dev) in your project. This new method with same signature will replace the original one at link time.

It is just another idea.

[KubaFYI]

Ok thanks for the explanation. This makes sense though we'd also need to remove the #errors from the original pinmux anyway as they would be resolved and stop the build before gcc scraps the weakly defined function.

As a meagre occasional contributor though I don't think I should make a decision on how exactly to handle this. @henrikbrixandersen @stephanosio Do you have any thoughts on this?