pinctrl: mcux_rt1xxx: add pinctrl support for rt1xxx series

[hakehuang]

pinctrl: nxp imxrt1xxx: add pinctrl support

demo on lpuart and i2s driver.

nxp rt1xxx series has different iomuxc so we add sepcial pincontrol bindings definitions.
and for gpr registers sets we rename former pinmux to gpr, and driver can access gpr directly demoed in i2s driver

tested on imxrt1060_evk board with hello_world

[hakehuang]

below warning in CI is a must for pinctrl

-:698: WARNING:NEW_TYPEDEFS: do not add new typedefs
#698: FILE: include/drivers/pinctrl/pinctrl_soc_mcux_rt_common.h:27:
+typedef struct pinctrl_soc_pin {

- total: 0 errors, 1 warnings, 658 lines checked

[danieldegrasse]

Thanks for getting this implemented @hakehuang! Just a note: I built a version of pinctrl for the RT1xxx based off this PR (here's a link to the fork), that uses pin nodes at the SOC level as opposed to pin groupings defined at the board level. After implementing that method of pinctrl, I think your implementation may be the cleaner method, but I'm interested to hear your thoughts.

[danieldegrasse]

Maybe move the inputOnfield parameter in the pinmux property to be a configurable value instead of part of the pin definition? My reasoning here is that the the inputOnfield property is usually given as an argument to IOMUXC_SetPinMux, where as the rest of the constants defined here are not

[hakehuang]

looks better.

[danieldegrasse]

Why is this file necessary?

[hakehuang]

yes it is not used, remove

[danieldegrasse]

Do we need address-cells and size-cells? The dts parser was giving me a warning that they were unused

[hakehuang]

yes, they are not used for now, remove it

[danieldegrasse]

These constants are defined in the device files in the HAL, see here

[hakehuang]

right, but dts can not include those header file. I can not find a way for it

[danieldegrasse]

does the DTS use these definitions directly? I thought it pulled from the yaml file for pinctrl, and that these constants would only be used in the C driver file

[hakehuang]

right, dts does not use this, update to use sdk macros.

[hakehuang]

Thanks for getting this implemented @hakehuang! Just a note: I built a version of pinctrl for the RT1xxx based off this PR (here's a link to the fork), that uses pin nodes at the SOC level as opposed to pin groupings defined at the board level. After implementing that method of pinctrl, I think your implementation may be the cleaner method, but I'm interested to hear your thoughts.

@danieldegrasse , I leave some comments to your commit directly, and I will close this PR, and wait for yours

[danieldegrasse]

@hakehuang To clarify- I think your approach is probably best here, I just wanted to make my change known in case we wanted to go that direction. Do you feel that the other approach would work better for the RT parts? I prefer your approach because it feels like it will lend itself better to auto generation of DTS files

[hakehuang]

@hakehuang To clarify- I think your approach is probably best here, I just wanted to make my change known in case we wanted to go that direction. Do you feel that the other approach would work better for the RT parts? I prefer your approach because it feels like it will lend itself better to auto generation of DTS files
@danieldegrasse
from the https://docs.zephyrproject.org/latest/guides/pinctrl/index.html , there are two options to support dts, you implement option 2, and I am using option 1.
option 1:
board dts+ board_pinctrl dtsi + soc.h
option 2:
board dts + board pinctrl dtsi + soc dtsi

so the differences are:

1. whether to generate soc.h(currently I put defines directly into pinctrl dtsi as there is no soc.h in hal for dts_root in our hal) or generate soc dtsi
2. goup pins maybe make the code easy to maintain

@dleach02, @mmahadevan108 do you have any comments on this?

btw: I add you as code-signer, thanks for the ideas on inoutput, and header

[danieldegrasse]

so the differences are:

1. whether to generate soc.h(currently I put defines directly into pinctrl dtsi as there is no soc.h in hal for dts_root in our hal) or generate soc dtsi
2. goup pins maybe make the code easy to maintain

I think that's correct, yeah. The primary differences are that the pins can no longer be grouped by peripheral, and that SOCs will have all pins defined in a dtsi file (as opposed to per board definitions). I agree that grouped pins are a big benefit to your method.

When I set out to implement my method, I felt it would better lend itself to auto generation of board level DTS files, but after comparing each approach I no longer believe that. An auto generation tool would likely be aware of the relevant registers used to configure each pin, but would not be aware of the specific DTS names we used for each pinmux node in my approach.

[hakehuang]

When I set out to implement my method, I felt it would better lend itself to auto generation of board level DTS files, but after comparing each approach I no longer believe that. An auto generation tool would likely be aware of the relevant registers used to configure each pin, but would not be aware of the specific DTS names we used for each pinmux node in my approach.

@danieldegrasse so you mean like the IOMUXC_GPIO_AD_B1_09_SAI1_MCLK is not consist over each soc? so the board dts will suffer to re-implement them, and if in this case use a soc.h file would be more concise, as we only need update the board pinctrl.dtsi. am I right?

[danieldegrasse]

@hakehuang Exactly, when moving between SOCs (or implementing a custom board) having all pinmux possibilities defined at the soc level in a dtsi file would simplify hand-coding new board pinctrl dts files.

However, if auto generation of the board level dts files became an option, then your method would work best. The reason I think that is because an autogeneration tool would always be aware of the SOC level registers used for configuring pinctrl, but would not necessarily be aware of the pinctrl node names in Zephyr. If the auto generation tool was made aware of the pinctrl node names defined at the soc level dtsi than my method would also work.

[hakehuang]

If the auto generation tool was made aware of the pinctrl node names defined at the soc level dtsi than my method would also work.

@danieldegrasse , you mean IOMUXC_GPIO_AD_B1_09_SAI1_MCLK? this name is generated by the config tools for device header file, I guess those can be aligned in auto tools.

[danieldegrasse]

@hakehuang yeah, values like IOMUXC_GPIO_AD_B1_09_SAI1_MCLK. I guess the core question is this: Are those names defined by SOC data the config tools use, or by the config tools themselves? If the names are defined by SOC data, then I think my approach will work without too much risk, since those names are unlikely to change. Otherwise, I think we should go with your method.

[hakehuang]

@hakehuang yeah, values like IOMUXC_GPIO_AD_B1_09_SAI1_MCLK. I guess the core question is this: Are those names defined by SOC data the config tools use, or by the config tools themselves? If the names are defined by SOC data, then I think my approach will work without too much risk, since those names are unlikely to change. Otherwise, I think we should go with your method.

@danieldegrasse those names are defined by config tools usually default value will not change. so I think you method will always work.

[danieldegrasse]

@hakehuang I've been working on this for a few days, and after some discussions with the config tools team I've opened #41810. The main differences are that a header file is used to define the IOMUXC constants for pinmux configuration, and the pinctrl node property names are changed to make future auto generation efforts of the board level DTSI file more viable.