diff mbox series

[1/2] pinctrl: tegra: Duplicate pinmux functions table

Message ID 20230530105308.1292852-1-thierry.reding@gmail.com
State Accepted
Headers show
Series [1/2] pinctrl: tegra: Duplicate pinmux functions table | expand

Commit Message

Thierry Reding May 30, 2023, 10:53 a.m. UTC 
From: Thierry Reding <treding@nvidia.com>

The function table is filled with group information based on other
instance-specific data at runtime. However, the function table can be
shared between multiple instances, causing the ->probe() function for
one instance to overwrite the table of a previously probed instance.

Fix this by sharing only the function names and allocating a separate
function table for each instance.

Fixes: 5a0047360743 ("pinctrl: tegra: Separate Tegra194 instances")
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/pinctrl/tegra/pinctrl-tegra.c    | 15 +++++++++++----
 drivers/pinctrl/tegra/pinctrl-tegra.h    |  3 ++-
 drivers/pinctrl/tegra/pinctrl-tegra114.c |  7 ++-----
 drivers/pinctrl/tegra/pinctrl-tegra124.c |  7 ++-----
 drivers/pinctrl/tegra/pinctrl-tegra194.c |  7 ++-----
 drivers/pinctrl/tegra/pinctrl-tegra20.c  |  7 ++-----
 drivers/pinctrl/tegra/pinctrl-tegra210.c |  7 ++-----
 drivers/pinctrl/tegra/pinctrl-tegra30.c  |  7 ++-----
 8 files changed, 25 insertions(+), 35 deletions(-)

Comments

Linus Walleij June 1, 2023, 11:19 a.m. UTC | #1 
On Tue, May 30, 2023 at 12:53 PM Thierry Reding
<thierry.reding@gmail.com> wrote:

> From: Thierry Reding <treding@nvidia.com>
>
> The function table is filled with group information based on other
> instance-specific data at runtime. However, the function table can be
> shared between multiple instances, causing the ->probe() function for
> one instance to overwrite the table of a previously probed instance.
>
> Fix this by sharing only the function names and allocating a separate
> function table for each instance.
>
> Fixes: 5a0047360743 ("pinctrl: tegra: Separate Tegra194 instances")
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Both patches applied!

I can't figure out if the problem is urgent or just wasting memory, so
I applied it as non-urgent fix for now, tell me if this needs to go
upstream pronto.

Yours,
Linus Walleij
Thierry Reding June 1, 2023, 12:27 p.m. UTC | #2 
On Thu, Jun 01, 2023 at 01:19:45PM +0200, Linus Walleij wrote:
> On Tue, May 30, 2023 at 12:53 PM Thierry Reding
> <thierry.reding@gmail.com> wrote:
> 
> > From: Thierry Reding <treding@nvidia.com>
> >
> > The function table is filled with group information based on other
> > instance-specific data at runtime. However, the function table can be
> > shared between multiple instances, causing the ->probe() function for
> > one instance to overwrite the table of a previously probed instance.
> >
> > Fix this by sharing only the function names and allocating a separate
> > function table for each instance.
> >
> > Fixes: 5a0047360743 ("pinctrl: tegra: Separate Tegra194 instances")
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> 
> Both patches applied!
> 
> I can't figure out if the problem is urgent or just wasting memory, so
> I applied it as non-urgent fix for now, tell me if this needs to go
> upstream pronto.

I think you might be able to write a device tree that triggers this, but
so far I've only seen it cause an WARN splat when accessing the debugfs
attribute that reads out the pinmux functions. I think Prathamesh had
mentioned it could also hang the system when you access debugfs.

Overall I don't think this is very urgent. We only observed this as part
of testing the Tegra234 patches that are under review, so it's probably
uncommon for people to run the problematic code paths.

No matter what you decide, the Fixes: tag should make sure it goes into
stable releases which is probably good enough.

Thierry
diff mbox series

Patch

diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
index 802801dbeb18..10c28534da48 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
@@ -236,7 +236,7 @@  static const char *tegra_pinctrl_get_func_name(struct pinctrl_dev *pctldev,
 {
 	struct tegra_pmx *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	return pmx->soc->functions[function].name;
+	return pmx->functions[function].name;
 }
 
 static int tegra_pinctrl_get_func_groups(struct pinctrl_dev *pctldev,
@@ -246,8 +246,8 @@  static int tegra_pinctrl_get_func_groups(struct pinctrl_dev *pctldev,
 {
 	struct tegra_pmx *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	*groups = pmx->soc->functions[function].groups;
-	*num_groups = pmx->soc->functions[function].ngroups;
+	*groups = pmx->functions[function].groups;
+	*num_groups = pmx->functions[function].ngroups;
 
 	return 0;
 }
@@ -798,10 +798,17 @@  int tegra_pinctrl_probe(struct platform_device *pdev,
 	if (!pmx->group_pins)
 		return -ENOMEM;
 
+	pmx->functions = devm_kcalloc(&pdev->dev, pmx->soc->nfunctions,
+				      sizeof(*pmx->functions), GFP_KERNEL);
+	if (!pmx->functions)
+		return -ENOMEM;
+
 	group_pins = pmx->group_pins;
+
 	for (fn = 0; fn < soc_data->nfunctions; fn++) {
-		struct tegra_function *func = &soc_data->functions[fn];
+		struct tegra_function *func = &pmx->functions[fn];
 
+		func->name = pmx->soc->functions[fn];
 		func->groups = group_pins;
 
 		for (gn = 0; gn < soc_data->ngroups; gn++) {
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h
index 6130cba7cce5..b3289bdf727d 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.h
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.h
@@ -13,6 +13,7 @@  struct tegra_pmx {
 	struct pinctrl_dev *pctl;
 
 	const struct tegra_pinctrl_soc_data *soc;
+	struct tegra_function *functions;
 	const char **group_pins;
 
 	struct pinctrl_gpio_range gpio_range;
@@ -191,7 +192,7 @@  struct tegra_pinctrl_soc_data {
 	const char *gpio_compatible;
 	const struct pinctrl_pin_desc *pins;
 	unsigned npins;
-	struct tegra_function *functions;
+	const char * const *functions;
 	unsigned nfunctions;
 	const struct tegra_pingroup *groups;
 	unsigned ngroups;
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra114.c b/drivers/pinctrl/tegra/pinctrl-tegra114.c
index e72ab1eb2398..3d425b2018e7 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra114.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra114.c
@@ -1452,12 +1452,9 @@  enum tegra_mux {
 	TEGRA_MUX_VI_ALT3,
 };
 
-#define FUNCTION(fname)					\
-	{						\
-		.name = #fname,				\
-	}
+#define FUNCTION(fname) #fname
 
-static struct tegra_function tegra114_functions[] = {
+static const char * const tegra114_functions[] = {
 	FUNCTION(blink),
 	FUNCTION(cec),
 	FUNCTION(cldvfs),
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra124.c b/drivers/pinctrl/tegra/pinctrl-tegra124.c
index 26096c6b967e..2a50c5c7516c 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra124.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra124.c
@@ -1611,12 +1611,9 @@  enum tegra_mux {
 	TEGRA_MUX_VIMCLK2_ALT,
 };
 
-#define FUNCTION(fname)					\
-	{						\
-		.name = #fname,				\
-	}
+#define FUNCTION(fname) #fname
 
-static struct tegra_function tegra124_functions[] = {
+static const char * const tegra124_functions[] = {
 	FUNCTION(blink),
 	FUNCTION(ccla),
 	FUNCTION(cec),
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra194.c b/drivers/pinctrl/tegra/pinctrl-tegra194.c
index 277973c88434..69f58df62897 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra194.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra194.c
@@ -1189,12 +1189,9 @@  enum tegra_mux_dt {
 };
 
 /* Make list of each function name */
-#define TEGRA_PIN_FUNCTION(lid)			\
-	{					\
-		.name = #lid,			\
-	}
+#define TEGRA_PIN_FUNCTION(lid) #lid
 
-static struct tegra_function tegra194_functions[] = {
+static const char * const tegra194_functions[] = {
 	TEGRA_PIN_FUNCTION(rsvd0),
 	TEGRA_PIN_FUNCTION(rsvd1),
 	TEGRA_PIN_FUNCTION(rsvd2),
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra20.c b/drivers/pinctrl/tegra/pinctrl-tegra20.c
index 0dc2cf0d05b1..737fc2000f66 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra20.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra20.c
@@ -1889,12 +1889,9 @@  enum tegra_mux {
 	TEGRA_MUX_XIO,
 };
 
-#define FUNCTION(fname)					\
-	{						\
-		.name = #fname,				\
-	}
+#define FUNCTION(fname) #fname
 
-static struct tegra_function tegra20_functions[] = {
+static const char * const tegra20_functions[] = {
 	FUNCTION(ahb_clk),
 	FUNCTION(apb_clk),
 	FUNCTION(audio_sync),
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra210.c b/drivers/pinctrl/tegra/pinctrl-tegra210.c
index b480f607fa16..9bb29146dfff 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra210.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra210.c
@@ -1185,12 +1185,9 @@  enum tegra_mux {
 	TEGRA_MUX_VIMCLK2,
 };
 
-#define FUNCTION(fname)					\
-	{						\
-		.name = #fname,				\
-	}
+#define FUNCTION(fname) #fname
 
-static struct tegra_function tegra210_functions[] = {
+static const char * const tegra210_functions[] = {
 	FUNCTION(aud),
 	FUNCTION(bcl),
 	FUNCTION(blink),
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra30.c b/drivers/pinctrl/tegra/pinctrl-tegra30.c
index 7299a371827f..de5aa2d4d28d 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra30.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra30.c
@@ -2010,12 +2010,9 @@  enum tegra_mux {
 	TEGRA_MUX_VI_ALT3,
 };
 
-#define FUNCTION(fname)					\
-	{						\
-		.name = #fname,				\
-	}
+#define FUNCTION(fname) #fname
 
-static struct tegra_function tegra30_functions[] = {
+static const char * const tegra30_functions[] = {
 	FUNCTION(blink),
 	FUNCTION(cec),
 	FUNCTION(clk_12m_out),