[RFC PATCH] pinctrl: tegra: detect unknown/invalid pin/func configurations

[Svyatoslav Ryhel]

нд, 30 бер. 2025 р. о 22:12 Artur Kowalski <arturkow2000 at gmail.com> пише:
>
> Tegra20 driver doesn't know about some pin configurations and even about
> some pins. In case when pin configuration is unknown the pin would be
> muxed to whatever is under function 0, in case when pin itself is
> unknown, it could cause out-of-bounds array access in pinmux_set_func
> and pinmux_set_pullupdown.
>
> Signed-off-by: Artur Kowalski <arturkow2000 at gmail.com>
> ---
> This allows to detect some unsupported/partially supported pinmux functions.
> Without this patch, U-Boot boots "normally" on Transformer T20, unless
> we enable debug by defining DEBUG in include/log.h, then we will hit an
> assertion in pinmux_set_func: pmux_func_isvalid.
>
> With this patch I get following warnings:
>
> warning: not muxing i2cp(18) to i2cp(64): invalid pin or function

i2cp function for t20 pinmux never existed in the U-Boot

> warning: not muxing ld0(64) to displaya(64): invalid pin or function
> warning: not muxing ld1(65) to displaya(64): invalid pin or function
> warning: not muxing ld2(66) to displaya(64): invalid pin or function
> warning: not muxing ld3(67) to displaya(64): invalid pin or function
> warning: not muxing ld4(68) to displaya(64): invalid pin or function
> warning: not muxing ld5(69) to displaya(64): invalid pin or function
> warning: not muxing ld6(70) to displaya(64): invalid pin or function
> warning: not muxing ld7(71) to displaya(64): invalid pin or function
> warning: not muxing ld8(72) to displaya(64): invalid pin or function
> warning: not muxing ld9(73) to displaya(64): invalid pin or function
> warning: not muxing ld10(74) to displaya(64): invalid pin or function
> warning: not muxing ld11(75) to displaya(64): invalid pin or function
> warning: not muxing ld12(76) to displaya(64): invalid pin or function
> warning: not muxing ld13(77) to displaya(64): invalid pin or function
> warning: not muxing ld14(78) to displaya(64): invalid pin or function
> warning: not muxing ld15(79) to displaya(64): invalid pin or function
> warning: not muxing ld16(80) to displaya(64): invalid pin or function
> warning: not muxing ld17(81) to displaya(64): invalid pin or function
> warning: not muxing ldi(102) to displaya(64): invalid pin or function
> warning: not muxing lhp0(82) to displaya(64): invalid pin or function
> warning: not muxing lhp1(83) to displaya(64): invalid pin or function
> warning: not muxing lhp2(84) to displaya(64): invalid pin or function
> warning: not muxing lhs(103) to displaya(64): invalid pin or function
> warning: not muxing lpp(104) to displaya(64): invalid pin or function
> warning: not muxing lpw0(99) to displaya(64): invalid pin or function
> warning: not muxing lpw2(101) to displaya(64): invalid pin or function
> warning: not muxing lsc0(91) to displaya(64): invalid pin or function
> warning: not muxing lsc1(92) to displaya(64): invalid pin or function
> warning: not muxing lsck(93) to displaya(64): invalid pin or function
> warning: not muxing lsda(97) to displaya(64): invalid pin or function
> warning: not muxing lspi(96) to displaya(64): invalid pin or function
> warning: not muxing lvp1(86) to displaya(64): invalid pin or function
> warning: not muxing lvs(90) to displaya(64): invalid pin or function
> warning: not muxing rm(25) to i2c1(64): invalid pin or function

These are caused by wrong function naming, and was fixed by
https://source.denx.de/u-boot/u-boot/-/commit/6494be8c722aa5e1d7bc9ea8d9e0b29d6dfe9b04

Always use the latest master/next branch or staging branch of tegra
custodian tree for patches you are submitting.

> warning: not muxing uad(53) to irda(64): invalid pin or function

irda function for t20 pinmux never existed in the U-Boot

>
> and pins:
> warning: not configuring pull/tristate on lc(121): invalid pin
> warning: not configuring pull/tristate on ls(121): invalid pin
> warning: not configuring pull/tristate on ld17_0(121): invalid pin
> warning: not configuring pull/tristate on ld19_18(121): invalid pin
> warning: not configuring pull/tristate on ld21_20(121): invalid pin
> warning: not configuring pull/tristate on ld23_22(121): invalid pin
>

These pins are software groups in Linux driver they do not resemble an
actual configurations, check T20 TRM. If you are willing to implement
their configuration I would not mind.

> Going back to what was before the patch, here is what happens in release build:
> tegra_pinctrl_set_func iterates over tegra_pinctrl_to_func to find
> matching func_id
>
> if (function)
>         for (i = 0; i < PMUX_FUNC_COUNT; i++)
>                 if (tegra_pinctrl_to_func[i])
>                         if (!strcmp(function, tegra_pinctrl_to_func[i]))
>                                 break;
>
> If there is no match loop exits with i = PMUX_FUNC_COUNT which is then
> assigned to func_id (if function were NULL, func_id would be
> uninitialized). Similar logic applies to the loop below, looking for
> pin_id, and to loop in tegra_pinctrl_set_pin. All these loops are fixed
> by this commit.
>
> But, going back to what was before: pinmux_set_func is called with
> PMUX_FUNC_COUNT, in effect we end up here:
>
> if (func >= PMUX_FUNC_RSVD1) {
>         mux = (func - PMUX_FUNC_RSVD1) & 3;
> }
>
> As PMUX_FUNC_COUNT = 64 and PMUX_FUNC_RSVD1 = 60 this becomes:
>
> mux = (64 - 60) & 3 = 0
>
> In effect U-Boot sets some pins (look at logs above with (64) after
> function name) to whatever is under function 0. In case of LD* pins
> this is displaya, exactly what we needed, so we still have working
> display. This seems to be the case for other pins too, however I still
> need to validate that.
>
> In the case of pull mode, PULL_SHIFT macro internal accesses
> tegra_soc_pingroups, here causing out-of-bounds access.
>
> For now, this patch only add warnings and some IFs to prevent undefined
> behaviour but doesn't address missing functionality yet. I'd like to fix
> that in another patch/series in following weeks.
>
> Device still doesn't boot in debug due to some other issues which
> require further investigation:
>
> clk_set_defaults(regulator-3v3)
> clk_set_default_parents: could not read assigned-clock-parents for 3f33a518
> ofnode_read_prop: assigned-clock-rates: <not found>
> regulator_common_set_enable: dev='regulator-3v3', enable=1, delay=0, has_gpio=0
> regulator-3v3 at vdd_3v3_vs: set 3300000 uV; enabling
> initcall: 00112c70 (relocated to 3f73cc70)
> Osc = 12000000
> CLKM = 12000000
> PLLC = 600000000
> PLLM = 600000000
> PLLP = 216000000
> PLLU = 960000000
> PLLD = 1000000
> PLLX = 1000000000
> arch/arm/mach-tegra/tegra20/clock.c:492: get_periph_clock_source: Assertion `!err' failed.
> resetting ...
>
>  drivers/pinctrl/tegra/pinctrl-tegra20.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra20.c b/drivers/pinctrl/tegra/pinctrl-tegra20.c
> index d59b3ec7b5d..aa73ada92ba 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra20.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra20.c
> @@ -37,6 +37,12 @@ static void tegra_pinctrl_set_pin(struct udevice *config)
>                                 if (!strcmp(pins[i], tegra_pinctrl_to_pingrp[pin_id]))
>                                         break;
>
> +               if (pin_id == PMUX_PINGRP_COUNT) {
> +                       debug("warning: not configuring pull/tristate on %s(%d): invalid pin\n",
> +                             pins[i], pin_id);
> +                       continue;
> +               }
> +
>                 if (pull >= 0)
>                         pinmux_set_pullupdown(pin_id, pull);
>
> @@ -58,13 +64,16 @@ static void tegra_pinctrl_set_func(struct udevice *config)
>         const char **pins;
>
>         function = dev_read_string(config, "nvidia,function");
> -       if (function)
> +       if (function) {
>                 for (i = 0; i < PMUX_FUNC_COUNT; i++)
>                         if (tegra_pinctrl_to_func[i])
>                                 if (!strcmp(function, tegra_pinctrl_to_func[i]))
>                                         break;
>
> -       func_id = i;
> +               func_id = i;
> +       } else {
> +               func_id = PMUX_FUNC_COUNT;
> +       }
>
>         count = dev_read_string_list(config, "nvidia,pins", &pins);
>         if (count < 0) {
> @@ -78,6 +87,12 @@ static void tegra_pinctrl_set_func(struct udevice *config)
>                                 if (!strcmp(pins[i], tegra_pinctrl_to_pingrp[pin_id]))
>                                         break;
>
> +               if (func_id == PMUX_FUNC_COUNT || pin_id == PMUX_PINGRP_COUNT) {
> +                       debug("warning: not muxing %s(%d) to %s(%d): invalid pin or function\n",
> +                             pins[i], pin_id, function, func_id);
> +                       continue;
> +               }
> +
>                 debug("%s(%d) muxed to %s(%d)\n", pins[i], pin_id, function, func_id);
>
>                 pinmux_set_func(pin_id, func_id);
> --
> 2.48.1
>

I see your point here. If you wish, you may submit a patch which will
add checks for pin/function correctness. That would be nice.