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 <[email protected]>
---
drivers/pinctrl/tegra/pinctrl-tegra.c | 19 +++++++++++++++++--
drivers/pinctrl/tegra/pinctrl-tegra.h | 2 ++
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
index 4547cf66d03b..2752c914f628 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,func", 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;
@@ -620,6 +627,7 @@ static void tegra_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
s8 bank, bit, width;
s32 reg;
u32 val;
+ u8 idx;
g = &pmx->soc->groups[group];
@@ -633,8 +641,15 @@ 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) {
+ 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 {
--
2.17.1
On Wed, Jul 12, 2023 at 07:25:53PM +0530, Prathamesh Shete 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 <[email protected]>
> ---
> drivers/pinctrl/tegra/pinctrl-tegra.c | 19 +++++++++++++++++--
> drivers/pinctrl/tegra/pinctrl-tegra.h | 2 ++
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> index 4547cf66d03b..2752c914f628 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,func", TEGRA_PINCONF_PARAM_FUNCTION},
Device tree bindings define the "nvidia,function" property to contain
this information, so shouldn't we use the same name here?
Thierry
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 <[email protected]>
---
drivers/pinctrl/tegra/pinctrl-tegra.c | 19 +++++++++++++++++--
drivers/pinctrl/tegra/pinctrl-tegra.h | 2 ++
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
index 4547cf66d03b..4da8873942cf 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;
@@ -620,6 +627,7 @@ static void tegra_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
s8 bank, bit, width;
s32 reg;
u32 val;
+ u8 idx;
g = &pmx->soc->groups[group];
@@ -633,8 +641,15 @@ 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) {
+ 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 {
--
2.17.1
On Thu, Jul 13, 2023 at 01:51:06PM +0530, Prathamesh Shete 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 <[email protected]>
> ---
> drivers/pinctrl/tegra/pinctrl-tegra.c | 19 +++++++++++++++++--
> drivers/pinctrl/tegra/pinctrl-tegra.h | 2 ++
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> index 4547cf66d03b..4da8873942cf 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;
> @@ -620,6 +627,7 @@ static void tegra_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
> s8 bank, bit, width;
> s32 reg;
> u32 val;
> + u8 idx;
This declaration could be moved into the only conditional that uses this
variable...
>
> g = &pmx->soc->groups[group];
>
> @@ -633,8 +641,15 @@ 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) {
> + idx = pmx->soc->groups[group].funcs[val];
... so that this would become:
u8 idx = ...;
Either way:
Acked-by: Thierry Reding <[email protected]>
On 14/07/2023 10:47, Thierry Reding wrote:
> On Thu, Jul 13, 2023 at 01:51:06PM +0530, Prathamesh Shete 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 <[email protected]>
>> ---
>> drivers/pinctrl/tegra/pinctrl-tegra.c | 19 +++++++++++++++++--
>> drivers/pinctrl/tegra/pinctrl-tegra.h | 2 ++
>> 2 files changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
>> index 4547cf66d03b..4da8873942cf 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;
>> @@ -620,6 +627,7 @@ static void tegra_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
>> s8 bank, bit, width;
>> s32 reg;
>> u32 val;
>> + u8 idx;
>
> This declaration could be moved into the only conditional that uses this
> variable...
>
>>
>> g = &pmx->soc->groups[group];
>>
>> @@ -633,8 +641,15 @@ 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) {
>> + idx = pmx->soc->groups[group].funcs[val];
>
> ... so that this would become:
>
> u8 idx = ...;
>
> Either way:
>
> Acked-by: Thierry Reding <[email protected]>
Looks good to me ...
Reviewed-by: Jon Hunter <[email protected]>
Thanks!
Jon
--
nvpublic
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 <[email protected]>
Acked-by: Thierry Reding <[email protected]>
Reviewed-by: Jon Hunter <[email protected]>
---
drivers/pinctrl/tegra/pinctrl-tegra.c | 19 +++++++++++++++++--
drivers/pinctrl/tegra/pinctrl-tegra.h | 2 ++
2 files changed, 19 insertions(+), 2 deletions(-)
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 {
--
2.17.1
On Fri, Jul 14, 2023 at 1:36 PM Prathamesh Shete <[email protected]> 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 <[email protected]>
> Acked-by: Thierry Reding <[email protected]>
> Reviewed-by: Jon Hunter <[email protected]>
Patch applied.
Yours,
Linus Walleij
Hello Prathamesh Shete,
On Fri, 14 Jul 2023 17:05:47 +0530
Prathamesh Shete <[email protected]> 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 <[email protected]>
> Acked-by: Thierry Reding <[email protected]>
> Reviewed-by: Jon Hunter <[email protected]>
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
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Mon, Sep 25, 2023 at 6:30 PM Luca Ceresoli <[email protected]> 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
can test a new patch then we will merge that.
Yours,
Linus Walleij
Hello Linus, Prathamesh,
On Wed, 27 Sep 2023 10:54:15 +0200
Linus Walleij <[email protected]> wrote:
> On Mon, Sep 25, 2023 at 6:30 PM Luca Ceresoli <[email protected]> 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.
Best regards,
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
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 <[email protected]> wrote:
>
> > On Mon, Sep 25, 2023 at 6:30 PM Luca Ceresoli <[email protected]> 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