| Message ID | 20230714113547.15384-1-pshete@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] pinctrl: tegra: Add support to display pin function | expand |
On Fri, Jul 14, 2023 at 1:36 PM Prathamesh Shete <pshete@nvidia.com> wrote: > The current function for a given pin is not displayed via the debugfs. > Add support to display the current function that is set for each pin. > > Signed-off-by: Prathamesh Shete <pshete@nvidia.com> > Acked-by: Thierry Reding <treding@nvidia.com> > Reviewed-by: Jon Hunter <jonathanh@nvidia.com> Patch applied. Yours, Linus Walleij
Hello Prathamesh Shete, On Fri, 14 Jul 2023 17:05:47 +0530 Prathamesh Shete <pshete@nvidia.com> wrote: > The current function for a given pin is not displayed via the debugfs. > Add support to display the current function that is set for each pin. > > Signed-off-by: Prathamesh Shete <pshete@nvidia.com> > Acked-by: Thierry Reding <treding@nvidia.com> > Reviewed-by: Jon Hunter <jonathanh@nvidia.com> While testing a Tegra20-based custom board I found a regression which according to my bisecting appears starting with this patch (commit d1cd5b51bc91 upstream). The symptom is that i2c3 is not working anymore, the I2C lines being always high. No other known issues at the moment. The board is built around an Avionic Design Tamonten SOM: arch/arm/boot/dts/nvidia/tegra20-tamonten.dtsi, which has in the &state_default node: dtf { nvidia,pins = "dtf"; nvidia,function = "i2c3"; }; But on top of that the board dts has: &state_default { dtf { nvidia,pins = "dtf"; nvidia,function = "i2c3"; nvidia,pull = <TEGRA_PIN_PULL_NONE>; nvidia,tristate = <TEGRA_PIN_DISABLE>; }; }; > diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c > index 4547cf66d03b..cb1d67239cd0 100644 > --- a/drivers/pinctrl/tegra/pinctrl-tegra.c > +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c > @@ -96,6 +96,7 @@ static const struct cfg_param { > {"nvidia,slew-rate-falling", TEGRA_PINCONF_PARAM_SLEW_RATE_FALLING}, > {"nvidia,slew-rate-rising", TEGRA_PINCONF_PARAM_SLEW_RATE_RISING}, > {"nvidia,drive-type", TEGRA_PINCONF_PARAM_DRIVE_TYPE}, > + {"nvidia,function", TEGRA_PINCONF_PARAM_FUNCTION}, FYI, I reduced your patch to only this line plus the one in the pinctrl-tegra.h and the problem appears as well. This is all the info I have at the moment. Can you provide more info on what could be going on or how to investigate? I am available to share more info as needed. Best regards, Luca
On Thu, Sep 28, 2023 at 08:53:19AM +0200, Luca Ceresoli wrote: > Hello Linus, Prathamesh, > > On Wed, 27 Sep 2023 10:54:15 +0200 > Linus Walleij <linus.walleij@linaro.org> wrote: > > > On Mon, Sep 25, 2023 at 6:30 PM Luca Ceresoli <luca.ceresoli@bootlin.com> wrote: > > > > > The symptom is that i2c3 is not working anymore, the I2C lines being > > > always high. No other known issues at the moment. > > > > Hm.... > > > > >> + {"nvidia,function", TEGRA_PINCONF_PARAM_FUNCTION}, > > > > > > FYI, I reduced your patch to only this line plus the one in the > > > pinctrl-tegra.h and the problem appears as well. > > > > I think there is a conflict now, that the pinconf is "stealing" the function > > assignment from the pinmux call. > > > > It's just a debugprint, I will revert the patch, Luca can investigate and you > > Thanks for the quick revert Linus. > > > can test a new patch then we will merge that. > > Prathamesh, if you send a new patch it would be great if you can Cc: me so I > can test it. I was able to reproduce this on tegra20-trimslice, and it looks indeed that the "shortcut" of supporting this through the pinconf "framework" doesn't work. In addition to the pinmux now no longer getting applied (exactly why that is I don't think I understand), it also leads to weird things in other parts of the debugfs output. For example the code now ends up trying to read a u32 from the nvidia,function property, which is actually a string. That seems to be fine, but obviously it doesn't yield the value that is expected for a function and messes up the config param which will then later on fail to properly display. Anyway, turns out the correct implementation is even shorter. I've sent out a patch. Thierry
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c index 4547cf66d03b..cb1d67239cd0 100644 --- a/drivers/pinctrl/tegra/pinctrl-tegra.c +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c @@ -96,6 +96,7 @@ static const struct cfg_param { {"nvidia,slew-rate-falling", TEGRA_PINCONF_PARAM_SLEW_RATE_FALLING}, {"nvidia,slew-rate-rising", TEGRA_PINCONF_PARAM_SLEW_RATE_RISING}, {"nvidia,drive-type", TEGRA_PINCONF_PARAM_DRIVE_TYPE}, + {"nvidia,function", TEGRA_PINCONF_PARAM_FUNCTION}, }; static int tegra_pinctrl_dt_subnode_to_map(struct pinctrl_dev *pctldev, @@ -470,6 +471,12 @@ static int tegra_pinconf_reg(struct tegra_pmx *pmx, *bit = g->drvtype_bit; *width = 2; break; + case TEGRA_PINCONF_PARAM_FUNCTION: + *bank = g->mux_bank; + *reg = g->mux_reg; + *bit = g->mux_bit; + *width = 2; + break; default: dev_err(pmx->dev, "Invalid config param %04x\n", param); return -ENOTSUPP; @@ -633,8 +640,16 @@ static void tegra_pinconf_group_dbg_show(struct pinctrl_dev *pctldev, val >>= bit; val &= (1 << width) - 1; - seq_printf(s, "\n\t%s=%u", - strip_prefix(cfg_params[i].property), val); + if (cfg_params[i].param == TEGRA_PINCONF_PARAM_FUNCTION) { + u8 idx = pmx->soc->groups[group].funcs[val]; + + seq_printf(s, "\n\t%s=%s", + strip_prefix(cfg_params[i].property), + pmx->functions[idx].name); + } else { + seq_printf(s, "\n\t%s=%u", + strip_prefix(cfg_params[i].property), val); + } } } diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h index b3289bdf727d..e728efeaa4de 100644 --- a/drivers/pinctrl/tegra/pinctrl-tegra.h +++ b/drivers/pinctrl/tegra/pinctrl-tegra.h @@ -54,6 +54,8 @@ enum tegra_pinconf_param { TEGRA_PINCONF_PARAM_SLEW_RATE_RISING, /* argument: Integer, range is HW-dependant */ TEGRA_PINCONF_PARAM_DRIVE_TYPE, + /* argument: pinmux settings */ + TEGRA_PINCONF_PARAM_FUNCTION, }; enum tegra_pinconf_pull {