dts: add include file required by iMX.RT pinctrl implementation

[danieldegrasse]

Add SOC level include file for RT1062 pinctrl implementation, which
defines all pin mux options

Signed-off-by: Daniel DeGrasse [email protected]

[mmahadevan108]

Is it possible to add a comment block describing what the 5 numbers in each macro.

[danieldegrasse]

@mmahadevan108 Yes, I updated the pinctrl dtsi file with a comment block describing the purpose of each register

[mmahadevan108]

Minor issue. Capital T for the.

[danieldegrasse]

Thanks, fixed now

[gmarull]

A few comments:

1. Some of these properties seem redundant here, DT bindings can provide defaults.
2. Node names should use_lowercase, by convention
3. Remember autogenerated nodes are only suitable for a single state (I suppose default here). How will, e.g. sleep state be solved in this case?
4. Node naming structure should be documented

[danieldegrasse]

1. The core issue I'm running into with DT binding defaults here is that not every pin on the SOC has the same set of default register values. Since there are many permutations of register values possible, several different DT bindings would need to be created to provide defaults. Another method I could use is to have the driver check if a property is actually present before configuring it, and otherwise leave it at the SOC default. Do you think that would work better?
2. Noted, I'll fix this in the next push.
3. That's a good question, and an issue with this approach I hadn't considered. I looked at ST's implementation and do not see how they solve it. Do you know what approach they are using?
4. Same as point 2, I'll fix this in the next push. Just want to wait to update this PR until we've worked through your 1st and 3rd points

[danieldegrasse]

@gmarull Just wanted to make you aware of this PR: zephyrproject-rtos/zephyr#42238. I opened a second PR primarily so that both the pin node and pin group approaches could be compared, rather than replacing the current PR with a pin group based approach.

In that PR, the pinctrl driver does not modify a given bitfield in the SOC pin configuration register unless the board level pinctrl node sets a property. The pin group based approach should solve the issue with states, since pinctrl nodes in different groups can select the same pinmux setting but use different pin configuration settings for something like a sleep state.

[gmarull]

1. The core issue I'm running into with DT binding defaults here is that not every pin on the SOC has the same set of default register values. Since there are many permutations of register values possible, several different DT bindings would need to be created to provide defaults. Another method I could use is to have the driver check if a property is actually present before configuring it, and otherwise leave it at the SOC default. Do you think that would work better?

I could probably provide a more informed advice if you can provide me with a datasheet (and concrete section) to look at. Do all settings have different defaults, for example slew-rate, speed, etc.?

3. That's a good question, and an issue with this approach I hadn't considered. I looked at ST's implementation and do not see how they solve it. Do you know what approach they are using?

In case of STM32 low power is achieved by configuring pin in analog mode, so they basically provide predefined nodes for analog config, see zephyrproject-rtos/hal_stm32#126 (ongoing).

pinctrl-0 = <&i2c0_scl_pa0 &i2c0_sda_pa1>;
pinctrl-1 = <&analog_pa0 &analog_pa1>;
pinctrl-names = "default", "sleep";

GD32 is a similar case but doesn't pre-define nodes, see https://github.com/zephyrproject-rtos/hal_gigadevice/blob/main/include/dt-bindings/pinctrl/gd32f450i(g-i-k)xx-pinctrl.h#L105-L122

&i2c0_default {
    group1 {
        pinmux = <I2C0_SCL_PA0>, <I2C0_SDA_PA1>;
        bias-open-drain;
    };
};

&i2c0_sleep {
    group1 {
        pinmux = <ANALOG_PA0>, <ANALOG_PA1>;
    };
};

...
pinctrl-0 = <&i2c0_default>;
pinctrl-1 = <&i2c0_sleep>;
pinctrl-names = "default", "sleep";

[danieldegrasse]

I could probably provide a more informed advice if you can provide me with a datasheet (and concrete section) to look at. Do all settings have different defaults, for example slew-rate, speed, etc.?

Here's a link to the reference manual for the RT1060 (NXP requires you to create a free account to download it). The specific section I'm looking at is 11.7, IOMUXC Memory Map/Register definition. You can see that the reset values for various SOC pin configurations (for example DSE in IOMUXC_SW_PAD_CTL_PAD_GPIO_AD_B0_05 and IOMUXC_SW_PAD_CTL_PAD_GPIO_AD_B0_06) differ.

Usually the only settings that will differ are the PUE, PUS and DSE bits, but I have only done this comparison on the RT1060, so ideally no register bit fields would be assigned a driver wide default, since other SOCs might have different default values for these bits.

[gmarull]

Thanks for the reference manual link. After looking in more detail, I think you have these options:

1. Use autogenerated nodes containing all register defaults. They can be overridden at the board level as needed. This sounds like the safest option in this particular case.
2. Autogenerate pinmux combinations, and force people to define all fields that have “no default”.
3. Autogenerate pinmux combinations, and have a special value to indicate user has not specified a certain property value, meaning it will be left as is. This may be dangerous because the current value may not always be register default (e.g. a bootloader tweaking things).

So in the end it's a vendor decision, take the option that you think your users will feel more comfortable, or are used to. Human factor is important: people will be reading and writing these files, so it has to be easy and non-error-prone. I don't have any preference, as soon as standard properties are used when available.

[danieldegrasse]

1. Use autogenerated nodes containing all register defaults. They can be overridden at the board level as needed. This sounds like the safest option in this particular case.

I agree that this is the safest option, but I have the following concerns:

• Defining a large number of states for a given peripheral's pins becomes challenging (for example pin states corresponding to bus speed for an SDHC)
• Our linux capable cores use the pin group implementation, as opposed to pin nodes.

I think we may end up using option 2. Forcing the user to define all fields feels a bit unfriendly, but it prevents issues like you mentioned in option 3. Do you have any issues if we proceed with that approach (outside of the comments you have left on that PR)?

[gmarull]

It's really a vendor decision, I have no preference.

[danieldegrasse]

Closing this PR in favor of #136, which enables pin groups