[PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

The plan for "struct function_desc" is to make its "group_names"
/double/ const. That will allow drivers to use it with static const
data.

This imx change is required to avoid:
drivers/pinctrl/freescale/pinctrl-imx.c: In function 'imx_pinctrl_parse_functions':
drivers/pinctrl/freescale/pinctrl-imx.c:672:24: error: assignment of read-only location '*(func->group_names + (sizetype)(i * 4))'
  672 |   func->group_names[i] = child->name;
      |                        ^

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/pinctrl/freescale/pinctrl-imx.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index daf28bc5661d..47b2ab1a14d0 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -648,6 +648,7 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
 	struct device_node *child;
 	struct function_desc *func;
 	struct group_desc *grp;
+	const char **group_names;
 	u32 i = 0;
 
 	dev_dbg(pctl->dev, "parse function(%d): %pOFn\n", index, np);
@@ -663,14 +664,16 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
 		dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
 		return -EINVAL;
 	}
-	func->group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
-					 sizeof(char *), GFP_KERNEL);
+
+	group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
+				   sizeof(char *), GFP_KERNEL);
 	if (!func->group_names)
 		return -ENOMEM;
+	for_each_child_of_node(np, child)
+		group_names[i] = child->name;
+	func->group_names = group_names;
 
 	for_each_child_of_node(np, child) {
-		func->group_names[i] = child->name;
-
 		grp = devm_kzalloc(ipctl->dev, sizeof(struct group_desc),
 				   GFP_KERNEL);
 		if (!grp) {
-- 
2.31.1

[PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

The plan for "struct function_desc" is to make its "group_names"
/double/ const. That will allow drivers to use it with static const
data.

This imx change is required to avoid:
drivers/pinctrl/freescale/pinctrl-imx.c: In function 'imx_pinctrl_parse_functions':
drivers/pinctrl/freescale/pinctrl-imx.c:672:24: error: assignment of read-only location '*(func->group_names + (sizetype)(i * 4))'
  672 |   func->group_names[i] = child->name;
      |                        ^

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/pinctrl/freescale/pinctrl-imx.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index daf28bc5661d..47b2ab1a14d0 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -648,6 +648,7 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
 	struct device_node *child;
 	struct function_desc *func;
 	struct group_desc *grp;
+	const char **group_names;
 	u32 i = 0;
 
 	dev_dbg(pctl->dev, "parse function(%d): %pOFn\n", index, np);
@@ -663,14 +664,16 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
 		dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
 		return -EINVAL;
 	}
-	func->group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
-					 sizeof(char *), GFP_KERNEL);
+
+	group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
+				   sizeof(char *), GFP_KERNEL);
 	if (!func->group_names)
 		return -ENOMEM;
+	for_each_child_of_node(np, child)
+		group_names[i] = child->name;
+	func->group_names = group_names;
 
 	for_each_child_of_node(np, child) {
-		func->group_names[i] = child->name;
-
 		grp = devm_kzalloc(ipctl->dev, sizeof(struct group_desc),
 				   GFP_KERNEL);
 		if (!grp) {
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 2/4] pinctrl: keembay: comment process of building functions a bit

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This should make code a bit easier to follow. While at it use some "for"
loops to simplify array iteration loops.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/pinctrl/pinctrl-keembay.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-keembay.c b/drivers/pinctrl/pinctrl-keembay.c
index 2bce563d5b8b..9a602abad8df 100644
--- a/drivers/pinctrl/pinctrl-keembay.c
+++ b/drivers/pinctrl/pinctrl-keembay.c
@@ -1617,37 +1617,38 @@ static int keembay_build_functions(struct keembay_pinctrl *kpc)
 	struct function_desc *keembay_funcs, *new_funcs;
 	int i;
 
-	/* Allocate total number of functions */
+	/*
+	 * Allocate maximum possible number of functions. Assume every pin
+	 * being part of 8 (hw maximum) globally unique muxes.
+	 */
 	kpc->nfuncs = 0;
 	keembay_funcs = kcalloc(kpc->npins * 8, sizeof(*keembay_funcs), GFP_KERNEL);
 	if (!keembay_funcs)
 		return -ENOMEM;
 
-	/* Find total number of functions and each's properties */
+	/* Setup 1 function for each unique mux */
 	for (i = 0; i < kpc->npins; i++) {
 		const struct pinctrl_pin_desc *pdesc = keembay_pins + i;
-		struct keembay_mux_desc *mux = pdesc->drv_data;
+		struct keembay_mux_desc *mux;
 
-		while (mux->name) {
-			struct function_desc *fdesc = keembay_funcs;
+		for (mux = pdesc->drv_data; mux->name; mux++) {
+			struct function_desc *fdesc;
 
-			while (fdesc->name) {
+			/* Check if we already have function for this mux */
+			for (fdesc = keembay_funcs; fdesc->name; fdesc++) {
 				if (!strcmp(mux->name, fdesc->name)) {
 					fdesc->num_group_names++;
 					break;
 				}
-
-				fdesc++;
 			}
 
+			/* Setup new function for this mux we didn't see before */
 			if (!fdesc->name) {
 				fdesc->name = mux->name;
 				fdesc->num_group_names = 1;
 				fdesc->data = &mux->mode;
 				kpc->nfuncs++;
 			}
-
-			mux++;
 		}
 	}
 
-- 
2.31.1

[PATCH V2 2/4] pinctrl: keembay: comment process of building functions a bit

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This should make code a bit easier to follow. While at it use some "for"
loops to simplify array iteration loops.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/pinctrl/pinctrl-keembay.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-keembay.c b/drivers/pinctrl/pinctrl-keembay.c
index 2bce563d5b8b..9a602abad8df 100644
--- a/drivers/pinctrl/pinctrl-keembay.c
+++ b/drivers/pinctrl/pinctrl-keembay.c
@@ -1617,37 +1617,38 @@ static int keembay_build_functions(struct keembay_pinctrl *kpc)
 	struct function_desc *keembay_funcs, *new_funcs;
 	int i;
 
-	/* Allocate total number of functions */
+	/*
+	 * Allocate maximum possible number of functions. Assume every pin
+	 * being part of 8 (hw maximum) globally unique muxes.
+	 */
 	kpc->nfuncs = 0;
 	keembay_funcs = kcalloc(kpc->npins * 8, sizeof(*keembay_funcs), GFP_KERNEL);
 	if (!keembay_funcs)
 		return -ENOMEM;
 
-	/* Find total number of functions and each's properties */
+	/* Setup 1 function for each unique mux */
 	for (i = 0; i < kpc->npins; i++) {
 		const struct pinctrl_pin_desc *pdesc = keembay_pins + i;
-		struct keembay_mux_desc *mux = pdesc->drv_data;
+		struct keembay_mux_desc *mux;
 
-		while (mux->name) {
-			struct function_desc *fdesc = keembay_funcs;
+		for (mux = pdesc->drv_data; mux->name; mux++) {
+			struct function_desc *fdesc;
 
-			while (fdesc->name) {
+			/* Check if we already have function for this mux */
+			for (fdesc = keembay_funcs; fdesc->name; fdesc++) {
 				if (!strcmp(mux->name, fdesc->name)) {
 					fdesc->num_group_names++;
 					break;
 				}
-
-				fdesc++;
 			}
 
+			/* Setup new function for this mux we didn't see before */
 			if (!fdesc->name) {
 				fdesc->name = mux->name;
 				fdesc->num_group_names = 1;
 				fdesc->data = &mux->mode;
 				kpc->nfuncs++;
 			}
-
-			mux++;
 		}
 	}
 
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 3/4] pinctrl: keembay: rework loops looking for groups names

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Make the outer loop iterate over functions as that's the real subject.
This simplifies code (and reduces amount of lines of code) as allocating
memory for names doesn't require extra checks anymore.

While at it use local "group_names" variable. The plan for
"struct function_desc" is to make its "group_names" /double/ const. That
will allow drivers to use it with static const data.

This keembay "group_names" change is required to avoid:
drivers/pinctrl/pinctrl-keembay.c: In function 'keembay_add_functions':
drivers/pinctrl/pinctrl-keembay.c:1594:8: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
 1594 |    grp = func->group_names;
      |        ^

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
This has been runtime tested. I verified that output of
/sys/kernel/debug/pinctrl/*/pinmux-functions is the same without and
with my patches.
---
 drivers/pinctrl/pinctrl-keembay.c | 66 ++++++++++++-------------------
 1 file changed, 25 insertions(+), 41 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-keembay.c b/drivers/pinctrl/pinctrl-keembay.c
index 9a602abad8df..152c35bce8ec 100644
--- a/drivers/pinctrl/pinctrl-keembay.c
+++ b/drivers/pinctrl/pinctrl-keembay.c
@@ -1555,58 +1555,42 @@ static int keembay_pinctrl_reg(struct keembay_pinctrl *kpc,  struct device *dev)
 }
 
 static int keembay_add_functions(struct keembay_pinctrl *kpc,
-				 struct function_desc *function)
+				 struct function_desc *functions)
 {
 	unsigned int i;
 
 	/* Assign the groups for each function */
-	for (i = 0; i < kpc->npins; i++) {
-		const struct pinctrl_pin_desc *pdesc = keembay_pins + i;
-		struct keembay_mux_desc *mux = pdesc->drv_data;
-
-		while (mux->name) {
-			struct function_desc *func;
-			const char **grp;
-			size_t grp_size;
-			u32 j, grp_num;
-
-			for (j = 0; j < kpc->nfuncs; j++) {
-				if (!strcmp(mux->name, function[j].name))
-					break;
-			}
-
-			if (j == kpc->nfuncs)
-				return -EINVAL;
-
-			func = function + j;
-			grp_num = func->num_group_names;
-			grp_size = sizeof(*func->group_names);
-
-			if (!func->group_names) {
-				func->group_names = devm_kcalloc(kpc->dev,
-								 grp_num,
-								 grp_size,
-								 GFP_KERNEL);
-				if (!func->group_names)
-					return -ENOMEM;
+	for (i = 0; i < kpc->nfuncs; i++) {
+		struct function_desc *func = &functions[i];
+		const char **group_names;
+		unsigned int grp_idx = 0;
+		int j;
+
+		group_names = devm_kcalloc(kpc->dev, func->num_group_names,
+					   sizeof(*group_names), GFP_KERNEL);
+		if (!group_names)
+			return -ENOMEM;
+
+		for (j = 0; j < kpc->npins; j++) {
+			const struct pinctrl_pin_desc *pdesc = &keembay_pins[j];
+			struct keembay_mux_desc *mux;
+
+			for (mux = pdesc->drv_data; mux->name; mux++) {
+				if (!strcmp(mux->name, func->name))
+					group_names[grp_idx++] = pdesc->name;
 			}
-
-			grp = func->group_names;
-			while (*grp)
-				grp++;
-
-			*grp = pdesc->name;
-			mux++;
 		}
+
+		func->group_names = group_names;
 	}
 
 	/* Add all functions */
 	for (i = 0; i < kpc->nfuncs; i++) {
 		pinmux_generic_add_function(kpc->pctrl,
-					    function[i].name,
-					    function[i].group_names,
-					    function[i].num_group_names,
-					    function[i].data);
+					    functions[i].name,
+					    functions[i].group_names,
+					    functions[i].num_group_names,
+					    functions[i].data);
 	}
 
 	return 0;
-- 
2.31.1

[PATCH V2 3/4] pinctrl: keembay: rework loops looking for groups names

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Make the outer loop iterate over functions as that's the real subject.
This simplifies code (and reduces amount of lines of code) as allocating
memory for names doesn't require extra checks anymore.

While at it use local "group_names" variable. The plan for
"struct function_desc" is to make its "group_names" /double/ const. That
will allow drivers to use it with static const data.

This keembay "group_names" change is required to avoid:
drivers/pinctrl/pinctrl-keembay.c: In function 'keembay_add_functions':
drivers/pinctrl/pinctrl-keembay.c:1594:8: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
 1594 |    grp = func->group_names;
      |        ^

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
This has been runtime tested. I verified that output of
/sys/kernel/debug/pinctrl/*/pinmux-functions is the same without and
with my patches.
---
 drivers/pinctrl/pinctrl-keembay.c | 66 ++++++++++++-------------------
 1 file changed, 25 insertions(+), 41 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-keembay.c b/drivers/pinctrl/pinctrl-keembay.c
index 9a602abad8df..152c35bce8ec 100644
--- a/drivers/pinctrl/pinctrl-keembay.c
+++ b/drivers/pinctrl/pinctrl-keembay.c
@@ -1555,58 +1555,42 @@ static int keembay_pinctrl_reg(struct keembay_pinctrl *kpc,  struct device *dev)
 }
 
 static int keembay_add_functions(struct keembay_pinctrl *kpc,
-				 struct function_desc *function)
+				 struct function_desc *functions)
 {
 	unsigned int i;
 
 	/* Assign the groups for each function */
-	for (i = 0; i < kpc->npins; i++) {
-		const struct pinctrl_pin_desc *pdesc = keembay_pins + i;
-		struct keembay_mux_desc *mux = pdesc->drv_data;
-
-		while (mux->name) {
-			struct function_desc *func;
-			const char **grp;
-			size_t grp_size;
-			u32 j, grp_num;
-
-			for (j = 0; j < kpc->nfuncs; j++) {
-				if (!strcmp(mux->name, function[j].name))
-					break;
-			}
-
-			if (j == kpc->nfuncs)
-				return -EINVAL;
-
-			func = function + j;
-			grp_num = func->num_group_names;
-			grp_size = sizeof(*func->group_names);
-
-			if (!func->group_names) {
-				func->group_names = devm_kcalloc(kpc->dev,
-								 grp_num,
-								 grp_size,
-								 GFP_KERNEL);
-				if (!func->group_names)
-					return -ENOMEM;
+	for (i = 0; i < kpc->nfuncs; i++) {
+		struct function_desc *func = &functions[i];
+		const char **group_names;
+		unsigned int grp_idx = 0;
+		int j;
+
+		group_names = devm_kcalloc(kpc->dev, func->num_group_names,
+					   sizeof(*group_names), GFP_KERNEL);
+		if (!group_names)
+			return -ENOMEM;
+
+		for (j = 0; j < kpc->npins; j++) {
+			const struct pinctrl_pin_desc *pdesc = &keembay_pins[j];
+			struct keembay_mux_desc *mux;
+
+			for (mux = pdesc->drv_data; mux->name; mux++) {
+				if (!strcmp(mux->name, func->name))
+					group_names[grp_idx++] = pdesc->name;
 			}
-
-			grp = func->group_names;
-			while (*grp)
-				grp++;
-
-			*grp = pdesc->name;
-			mux++;
 		}
+
+		func->group_names = group_names;
 	}
 
 	/* Add all functions */
 	for (i = 0; i < kpc->nfuncs; i++) {
 		pinmux_generic_add_function(kpc->pctrl,
-					    function[i].name,
-					    function[i].group_names,
-					    function[i].num_group_names,
-					    function[i].data);
+					    functions[i].name,
+					    functions[i].group_names,
+					    functions[i].num_group_names,
+					    functions[i].data);
 	}
 
 	return 0;
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Generic code doesn't modify those strings and .get_function_groups
callback has that extra "const" as well. This allows more flexibility in
GENERIC_PINMUX_FUNCTIONS users.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/pinctrl/pinmux.c | 2 +-
 drivers/pinctrl/pinmux.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 6cdbd9ccf2f0..f94d43b082d9 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
  */
 int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
 				const char *name,
-				const char **groups,
+				const char * const *groups,
 				const unsigned int num_groups,
 				void *data)
 {
diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
index 78c3a31be882..72fcf03eaa43 100644
--- a/drivers/pinctrl/pinmux.h
+++ b/drivers/pinctrl/pinmux.h
@@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
  */
 struct function_desc {
 	const char *name;
-	const char **group_names;
+	const char * const *group_names;
 	int num_group_names;
 	void *data;
 };
@@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
 
 int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
 				const char *name,
-				const char **groups,
+				const char * const *groups,
 				unsigned const num_groups,
 				void *data);
 
-- 
2.31.1

[PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Generic code doesn't modify those strings and .get_function_groups
callback has that extra "const" as well. This allows more flexibility in
GENERIC_PINMUX_FUNCTIONS users.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 drivers/pinctrl/pinmux.c | 2 +-
 drivers/pinctrl/pinmux.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 6cdbd9ccf2f0..f94d43b082d9 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
  */
 int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
 				const char *name,
-				const char **groups,
+				const char * const *groups,
 				const unsigned int num_groups,
 				void *data)
 {
diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
index 78c3a31be882..72fcf03eaa43 100644
--- a/drivers/pinctrl/pinmux.h
+++ b/drivers/pinctrl/pinmux.h
@@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
  */
 struct function_desc {
 	const char *name;
-	const char **group_names;
+	const char * const *group_names;
 	int num_group_names;
 	void *data;
 };
@@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
 
 int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
 				const char *name,
-				const char **groups,
+				const char * const *groups,
 				unsigned const num_groups,
 				void *data);
 
-- 
2.31.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Nathan Chancellor]

Hi Rafał,

On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Generic code doesn't modify those strings and .get_function_groups
> callback has that extra "const" as well. This allows more flexibility in
> GENERIC_PINMUX_FUNCTIONS users.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  drivers/pinctrl/pinmux.c | 2 +-
>  drivers/pinctrl/pinmux.h | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> index 6cdbd9ccf2f0..f94d43b082d9 100644
> --- a/drivers/pinctrl/pinmux.c
> +++ b/drivers/pinctrl/pinmux.c
> @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
>   */
>  int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>  				const char *name,
> -				const char **groups,
> +				const char * const *groups,
>  				const unsigned int num_groups,
>  				void *data)
>  {
> diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
> index 78c3a31be882..72fcf03eaa43 100644
> --- a/drivers/pinctrl/pinmux.h
> +++ b/drivers/pinctrl/pinmux.h
> @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
>   */
>  struct function_desc {
>  	const char *name;
> -	const char **group_names;
> +	const char * const *group_names;
>  	int num_group_names;
>  	void *data;
>  };
> @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
>  
>  int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>  				const char *name,
> -				const char **groups,
> +				const char * const *groups,
>  				unsigned const num_groups,
>  				void *data);
>  
> -- 
> 2.31.1
> 
> 

I have not seen this reported yet, even though it has been broken for a
couple of weeks now. I see the following error in -next:

$ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
  815 |                         grp = func->group_names;
      |                             ^
cc1: all warnings being treated as errors

Looks like something like the third patch of the series is needed for
the Thunderbay driver, which it appears was in development at the same
time as this series.

Cheers,
Nathan

Re: [PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Nathan Chancellor]

Hi Rafał,

On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Generic code doesn't modify those strings and .get_function_groups
> callback has that extra "const" as well. This allows more flexibility in
> GENERIC_PINMUX_FUNCTIONS users.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  drivers/pinctrl/pinmux.c | 2 +-
>  drivers/pinctrl/pinmux.h | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> index 6cdbd9ccf2f0..f94d43b082d9 100644
> --- a/drivers/pinctrl/pinmux.c
> +++ b/drivers/pinctrl/pinmux.c
> @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
>   */
>  int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>  				const char *name,
> -				const char **groups,
> +				const char * const *groups,
>  				const unsigned int num_groups,
>  				void *data)
>  {
> diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
> index 78c3a31be882..72fcf03eaa43 100644
> --- a/drivers/pinctrl/pinmux.h
> +++ b/drivers/pinctrl/pinmux.h
> @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
>   */
>  struct function_desc {
>  	const char *name;
> -	const char **group_names;
> +	const char * const *group_names;
>  	int num_group_names;
>  	void *data;
>  };
> @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
>  
>  int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>  				const char *name,
> -				const char **groups,
> +				const char * const *groups,
>  				unsigned const num_groups,
>  				void *data);
>  
> -- 
> 2.31.1
> 
> 

I have not seen this reported yet, even though it has been broken for a
couple of weeks now. I see the following error in -next:

$ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
  815 |                         grp = func->group_names;
      |                             ^
cc1: all warnings being treated as errors

Looks like something like the third patch of the series is needed for
the Thunderbay driver, which it appears was in development at the same
time as this series.

Cheers,
Nathan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Rafał Miłecki]

On 11.01.2022 16:34, Nathan Chancellor wrote:
> On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Generic code doesn't modify those strings and .get_function_groups
>> callback has that extra "const" as well. This allows more flexibility in
>> GENERIC_PINMUX_FUNCTIONS users.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>   drivers/pinctrl/pinmux.c | 2 +-
>>   drivers/pinctrl/pinmux.h | 4 ++--
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
>> index 6cdbd9ccf2f0..f94d43b082d9 100644
>> --- a/drivers/pinctrl/pinmux.c
>> +++ b/drivers/pinctrl/pinmux.c
>> @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
>>    */
>>   int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>>   				const char *name,
>> -				const char **groups,
>> +				const char * const *groups,
>>   				const unsigned int num_groups,
>>   				void *data)
>>   {
>> diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
>> index 78c3a31be882..72fcf03eaa43 100644
>> --- a/drivers/pinctrl/pinmux.h
>> +++ b/drivers/pinctrl/pinmux.h
>> @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
>>    */
>>   struct function_desc {
>>   	const char *name;
>> -	const char **group_names;
>> +	const char * const *group_names;
>>   	int num_group_names;
>>   	void *data;
>>   };
>> @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
>>   
>>   int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>>   				const char *name,
>> -				const char **groups,
>> +				const char * const *groups,
>>   				unsigned const num_groups,
>>   				void *data);
>>   
>> -- 
>> 2.31.1
>>
>>
> 
> I have not seen this reported yet, even though it has been broken for a
> couple of weeks now. I see the following error in -next:
> 
> $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
> drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
> drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
>    815 |                         grp = func->group_names;
>        |                             ^
> cc1: all warnings being treated as errors
> 
> Looks like something like the third patch of the series is needed for
> the Thunderbay driver, which it appears was in development at the same
> time as this series.

Correct, this driver didn't exist in Linus's tree when I developed my changes.

Too bad thunderbay copies that old & complex logic that I just fixed in the keembay driver. I'll have to redo my changes for the thunderbay now.

I don't agree with the idea of reverting my patchset and working on V3 though. It's a relatively simple thing we need to fix, it just be just a follow-up commit.

Re: [PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Rafał Miłecki]

On 11.01.2022 16:34, Nathan Chancellor wrote:
> On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Generic code doesn't modify those strings and .get_function_groups
>> callback has that extra "const" as well. This allows more flexibility in
>> GENERIC_PINMUX_FUNCTIONS users.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>   drivers/pinctrl/pinmux.c | 2 +-
>>   drivers/pinctrl/pinmux.h | 4 ++--
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
>> index 6cdbd9ccf2f0..f94d43b082d9 100644
>> --- a/drivers/pinctrl/pinmux.c
>> +++ b/drivers/pinctrl/pinmux.c
>> @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
>>    */
>>   int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>>   				const char *name,
>> -				const char **groups,
>> +				const char * const *groups,
>>   				const unsigned int num_groups,
>>   				void *data)
>>   {
>> diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
>> index 78c3a31be882..72fcf03eaa43 100644
>> --- a/drivers/pinctrl/pinmux.h
>> +++ b/drivers/pinctrl/pinmux.h
>> @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
>>    */
>>   struct function_desc {
>>   	const char *name;
>> -	const char **group_names;
>> +	const char * const *group_names;
>>   	int num_group_names;
>>   	void *data;
>>   };
>> @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
>>   
>>   int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>>   				const char *name,
>> -				const char **groups,
>> +				const char * const *groups,
>>   				unsigned const num_groups,
>>   				void *data);
>>   
>> -- 
>> 2.31.1
>>
>>
> 
> I have not seen this reported yet, even though it has been broken for a
> couple of weeks now. I see the following error in -next:
> 
> $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
> drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
> drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
>    815 |                         grp = func->group_names;
>        |                             ^
> cc1: all warnings being treated as errors
> 
> Looks like something like the third patch of the series is needed for
> the Thunderbay driver, which it appears was in development at the same
> time as this series.

Correct, this driver didn't exist in Linus's tree when I developed my changes.

Too bad thunderbay copies that old & complex logic that I just fixed in the keembay driver. I'll have to redo my changes for the thunderbay now.

I don't agree with the idea of reverting my patchset and working on V3 though. It's a relatively simple thing we need to fix, it just be just a follow-up commit.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Linus Walleij]

On Tue, Jan 11, 2022 at 5:51 PM Rafał Miłecki <zajec5@gmail.com> wrote:

> > Looks like something like the third patch of the series is needed for
> > the Thunderbay driver, which it appears was in development at the same
> > time as this series.
>
> Correct, this driver didn't exist in Linus's tree when I developed my changes.
>
> Too bad thunderbay copies that old & complex logic that I just fixed in the keembay driver. I'll have to redo my changes for the thunderbay now.
>
> I don't agree with the idea of reverting my patchset and working on V3 though. It's a relatively simple thing we need to fix, it just be just a follow-up commit.

Yeah this kind of things happens, we just deal with it throughout the -rc:s
no big deal. Release candidates exist for a reason. Let's make a fixup patch.

Yours,
Linus Walleij

Re: [PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Linus Walleij]

On Tue, Jan 11, 2022 at 5:51 PM Rafał Miłecki <zajec5@gmail.com> wrote:

> > Looks like something like the third patch of the series is needed for
> > the Thunderbay driver, which it appears was in development at the same
> > time as this series.
>
> Correct, this driver didn't exist in Linus's tree when I developed my changes.
>
> Too bad thunderbay copies that old & complex logic that I just fixed in the keembay driver. I'll have to redo my changes for the thunderbay now.
>
> I don't agree with the idea of reverting my patchset and working on V3 though. It's a relatively simple thing we need to fix, it just be just a follow-up commit.

Yeah this kind of things happens, we just deal with it throughout the -rc:s
no big deal. Release candidates exist for a reason. Let's make a fixup patch.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 3787 bytes --]

On Tue, Jan 11, 2022 at 05:51:44PM +0100, Rafał Miłecki wrote:
> On 11.01.2022 16:34, Nathan Chancellor wrote:
> > On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
> > > From: Rafał Miłecki <rafal@milecki.pl>
> > > 
> > > Generic code doesn't modify those strings and .get_function_groups
> > > callback has that extra "const" as well. This allows more flexibility in
> > > GENERIC_PINMUX_FUNCTIONS users.
> > > 
> > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > > ---
> > >   drivers/pinctrl/pinmux.c | 2 +-
> > >   drivers/pinctrl/pinmux.h | 4 ++--
> > >   2 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> > > index 6cdbd9ccf2f0..f94d43b082d9 100644
> > > --- a/drivers/pinctrl/pinmux.c
> > > +++ b/drivers/pinctrl/pinmux.c
> > > @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
> > >    */
> > >   int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
> > >   				const char *name,
> > > -				const char **groups,
> > > +				const char * const *groups,
> > >   				const unsigned int num_groups,
> > >   				void *data)
> > >   {
> > > diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
> > > index 78c3a31be882..72fcf03eaa43 100644
> > > --- a/drivers/pinctrl/pinmux.h
> > > +++ b/drivers/pinctrl/pinmux.h
> > > @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
> > >    */
> > >   struct function_desc {
> > >   	const char *name;
> > > -	const char **group_names;
> > > +	const char * const *group_names;
> > >   	int num_group_names;
> > >   	void *data;
> > >   };
> > > @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
> > >   int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
> > >   				const char *name,
> > > -				const char **groups,
> > > +				const char * const *groups,
> > >   				unsigned const num_groups,
> > >   				void *data);
> > > -- 
> > > 2.31.1
> > > 
> > > 
> > 
> > I have not seen this reported yet, even though it has been broken for a
> > couple of weeks now. I see the following error in -next:
> > 
> > $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
> > drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
> > drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
> >    815 |                         grp = func->group_names;
> >        |                             ^
> > cc1: all warnings being treated as errors
> > 
> > Looks like something like the third patch of the series is needed for
> > the Thunderbay driver, which it appears was in development at the same
> > time as this series.
> 
> Correct, this driver didn't exist in Linus's tree when I developed my changes.

I stumbled above this issue, too. For the record, this patch fixes the
build issue:

diff --git a/drivers/pinctrl/pinctrl-thunderbay.c b/drivers/pinctrl/pinctrl-thunderbay.c
index b5b47f4dd774..a6a9a0cca6bf 100644
--- a/drivers/pinctrl/pinctrl-thunderbay.c
+++ b/drivers/pinctrl/pinctrl-thunderbay.c
@@ -812,7 +812,7 @@ static int thunderbay_add_functions(struct thunderbay_pinctrl *tpc, struct funct
 				}
 			}
 
-			grp = func->group_names;
+			grp = (void *)func->group_names;
 			while (*grp)
 				grp++;
 
(however it's probably not safe for runtime).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 3787 bytes --]

On Tue, Jan 11, 2022 at 05:51:44PM +0100, Rafał Miłecki wrote:
> On 11.01.2022 16:34, Nathan Chancellor wrote:
> > On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
> > > From: Rafał Miłecki <rafal@milecki.pl>
> > > 
> > > Generic code doesn't modify those strings and .get_function_groups
> > > callback has that extra "const" as well. This allows more flexibility in
> > > GENERIC_PINMUX_FUNCTIONS users.
> > > 
> > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > > ---
> > >   drivers/pinctrl/pinmux.c | 2 +-
> > >   drivers/pinctrl/pinmux.h | 4 ++--
> > >   2 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> > > index 6cdbd9ccf2f0..f94d43b082d9 100644
> > > --- a/drivers/pinctrl/pinmux.c
> > > +++ b/drivers/pinctrl/pinmux.c
> > > @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
> > >    */
> > >   int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
> > >   				const char *name,
> > > -				const char **groups,
> > > +				const char * const *groups,
> > >   				const unsigned int num_groups,
> > >   				void *data)
> > >   {
> > > diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
> > > index 78c3a31be882..72fcf03eaa43 100644
> > > --- a/drivers/pinctrl/pinmux.h
> > > +++ b/drivers/pinctrl/pinmux.h
> > > @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
> > >    */
> > >   struct function_desc {
> > >   	const char *name;
> > > -	const char **group_names;
> > > +	const char * const *group_names;
> > >   	int num_group_names;
> > >   	void *data;
> > >   };
> > > @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
> > >   int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
> > >   				const char *name,
> > > -				const char **groups,
> > > +				const char * const *groups,
> > >   				unsigned const num_groups,
> > >   				void *data);
> > > -- 
> > > 2.31.1
> > > 
> > > 
> > 
> > I have not seen this reported yet, even though it has been broken for a
> > couple of weeks now. I see the following error in -next:
> > 
> > $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
> > drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
> > drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
> >    815 |                         grp = func->group_names;
> >        |                             ^
> > cc1: all warnings being treated as errors
> > 
> > Looks like something like the third patch of the series is needed for
> > the Thunderbay driver, which it appears was in development at the same
> > time as this series.
> 
> Correct, this driver didn't exist in Linus's tree when I developed my changes.

I stumbled above this issue, too. For the record, this patch fixes the
build issue:

diff --git a/drivers/pinctrl/pinctrl-thunderbay.c b/drivers/pinctrl/pinctrl-thunderbay.c
index b5b47f4dd774..a6a9a0cca6bf 100644
--- a/drivers/pinctrl/pinctrl-thunderbay.c
+++ b/drivers/pinctrl/pinctrl-thunderbay.c
@@ -812,7 +812,7 @@ static int thunderbay_add_functions(struct thunderbay_pinctrl *tpc, struct funct
 				}
 			}
 
-			grp = func->group_names;
+			grp = (void *)func->group_names;
 			while (*grp)
 				grp++;
 
(however it's probably not safe for runtime).

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Rafał Miłecki]

On 18.01.2022 08:02, Uwe Kleine-König wrote:
> On Tue, Jan 11, 2022 at 05:51:44PM +0100, Rafał Miłecki wrote:
>> On 11.01.2022 16:34, Nathan Chancellor wrote:
>>> On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> Generic code doesn't modify those strings and .get_function_groups
>>>> callback has that extra "const" as well. This allows more flexibility in
>>>> GENERIC_PINMUX_FUNCTIONS users.
>>>>
>>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>>> ---
>>>>    drivers/pinctrl/pinmux.c | 2 +-
>>>>    drivers/pinctrl/pinmux.h | 4 ++--
>>>>    2 files changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
>>>> index 6cdbd9ccf2f0..f94d43b082d9 100644
>>>> --- a/drivers/pinctrl/pinmux.c
>>>> +++ b/drivers/pinctrl/pinmux.c
>>>> @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
>>>>     */
>>>>    int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>>>>    				const char *name,
>>>> -				const char **groups,
>>>> +				const char * const *groups,
>>>>    				const unsigned int num_groups,
>>>>    				void *data)
>>>>    {
>>>> diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
>>>> index 78c3a31be882..72fcf03eaa43 100644
>>>> --- a/drivers/pinctrl/pinmux.h
>>>> +++ b/drivers/pinctrl/pinmux.h
>>>> @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
>>>>     */
>>>>    struct function_desc {
>>>>    	const char *name;
>>>> -	const char **group_names;
>>>> +	const char * const *group_names;
>>>>    	int num_group_names;
>>>>    	void *data;
>>>>    };
>>>> @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
>>>>    int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>>>>    				const char *name,
>>>> -				const char **groups,
>>>> +				const char * const *groups,
>>>>    				unsigned const num_groups,
>>>>    				void *data);
>>>> -- 
>>>> 2.31.1
>>>>
>>>>
>>>
>>> I have not seen this reported yet, even though it has been broken for a
>>> couple of weeks now. I see the following error in -next:
>>>
>>> $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
>>> drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
>>> drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
>>>     815 |                         grp = func->group_names;
>>>         |                             ^
>>> cc1: all warnings being treated as errors
>>>
>>> Looks like something like the third patch of the series is needed for
>>> the Thunderbay driver, which it appears was in development at the same
>>> time as this series.
>>
>> Correct, this driver didn't exist in Linus's tree when I developed my changes.
> 
> I stumbled above this issue, too. For the record, this patch fixes the
> build issue:

[PATCH 5.17 1/2] pinctrl: thunderbay: comment process of building functions a bit
[PATCH 5.17 2/2] pinctrl: thunderbay: rework loops looking for groups names
https://patchwork.ozlabs.org/project/linux-gpio/list/?series=280568

Patches already queued in the:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=fixes

Re: [PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Rafał Miłecki]

On 18.01.2022 08:02, Uwe Kleine-König wrote:
> On Tue, Jan 11, 2022 at 05:51:44PM +0100, Rafał Miłecki wrote:
>> On 11.01.2022 16:34, Nathan Chancellor wrote:
>>> On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> Generic code doesn't modify those strings and .get_function_groups
>>>> callback has that extra "const" as well. This allows more flexibility in
>>>> GENERIC_PINMUX_FUNCTIONS users.
>>>>
>>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>>> ---
>>>>    drivers/pinctrl/pinmux.c | 2 +-
>>>>    drivers/pinctrl/pinmux.h | 4 ++--
>>>>    2 files changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
>>>> index 6cdbd9ccf2f0..f94d43b082d9 100644
>>>> --- a/drivers/pinctrl/pinmux.c
>>>> +++ b/drivers/pinctrl/pinmux.c
>>>> @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
>>>>     */
>>>>    int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>>>>    				const char *name,
>>>> -				const char **groups,
>>>> +				const char * const *groups,
>>>>    				const unsigned int num_groups,
>>>>    				void *data)
>>>>    {
>>>> diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
>>>> index 78c3a31be882..72fcf03eaa43 100644
>>>> --- a/drivers/pinctrl/pinmux.h
>>>> +++ b/drivers/pinctrl/pinmux.h
>>>> @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
>>>>     */
>>>>    struct function_desc {
>>>>    	const char *name;
>>>> -	const char **group_names;
>>>> +	const char * const *group_names;
>>>>    	int num_group_names;
>>>>    	void *data;
>>>>    };
>>>> @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
>>>>    int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
>>>>    				const char *name,
>>>> -				const char **groups,
>>>> +				const char * const *groups,
>>>>    				unsigned const num_groups,
>>>>    				void *data);
>>>> -- 
>>>> 2.31.1
>>>>
>>>>
>>>
>>> I have not seen this reported yet, even though it has been broken for a
>>> couple of weeks now. I see the following error in -next:
>>>
>>> $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
>>> drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
>>> drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
>>>     815 |                         grp = func->group_names;
>>>         |                             ^
>>> cc1: all warnings being treated as errors
>>>
>>> Looks like something like the third patch of the series is needed for
>>> the Thunderbay driver, which it appears was in development at the same
>>> time as this series.
>>
>> Correct, this driver didn't exist in Linus's tree when I developed my changes.
> 
> I stumbled above this issue, too. For the record, this patch fixes the
> build issue:

[PATCH 5.17 1/2] pinctrl: thunderbay: comment process of building functions a bit
[PATCH 5.17 2/2] pinctrl: thunderbay: rework loops looking for groups names
https://patchwork.ozlabs.org/project/linux-gpio/list/?series=280568

Patches already queued in the:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=fixes

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 4243 bytes --]

On Tue, Jan 18, 2022 at 08:21:55AM +0100, Rafał Miłecki wrote:
> On 18.01.2022 08:02, Uwe Kleine-König wrote:
> > On Tue, Jan 11, 2022 at 05:51:44PM +0100, Rafał Miłecki wrote:
> > > On 11.01.2022 16:34, Nathan Chancellor wrote:
> > > > On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
> > > > > From: Rafał Miłecki <rafal@milecki.pl>
> > > > > 
> > > > > Generic code doesn't modify those strings and .get_function_groups
> > > > > callback has that extra "const" as well. This allows more flexibility in
> > > > > GENERIC_PINMUX_FUNCTIONS users.
> > > > > 
> > > > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > > > > ---
> > > > >    drivers/pinctrl/pinmux.c | 2 +-
> > > > >    drivers/pinctrl/pinmux.h | 4 ++--
> > > > >    2 files changed, 3 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> > > > > index 6cdbd9ccf2f0..f94d43b082d9 100644
> > > > > --- a/drivers/pinctrl/pinmux.c
> > > > > +++ b/drivers/pinctrl/pinmux.c
> > > > > @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
> > > > >     */
> > > > >    int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
> > > > >    				const char *name,
> > > > > -				const char **groups,
> > > > > +				const char * const *groups,
> > > > >    				const unsigned int num_groups,
> > > > >    				void *data)
> > > > >    {
> > > > > diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
> > > > > index 78c3a31be882..72fcf03eaa43 100644
> > > > > --- a/drivers/pinctrl/pinmux.h
> > > > > +++ b/drivers/pinctrl/pinmux.h
> > > > > @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
> > > > >     */
> > > > >    struct function_desc {
> > > > >    	const char *name;
> > > > > -	const char **group_names;
> > > > > +	const char * const *group_names;
> > > > >    	int num_group_names;
> > > > >    	void *data;
> > > > >    };
> > > > > @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
> > > > >    int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
> > > > >    				const char *name,
> > > > > -				const char **groups,
> > > > > +				const char * const *groups,
> > > > >    				unsigned const num_groups,
> > > > >    				void *data);
> > > > > -- 
> > > > > 2.31.1
> > > > > 
> > > > > 
> > > > 
> > > > I have not seen this reported yet, even though it has been broken for a
> > > > couple of weeks now. I see the following error in -next:
> > > > 
> > > > $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
> > > > drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
> > > > drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
> > > >     815 |                         grp = func->group_names;
> > > >         |                             ^
> > > > cc1: all warnings being treated as errors
> > > > 
> > > > Looks like something like the third patch of the series is needed for
> > > > the Thunderbay driver, which it appears was in development at the same
> > > > time as this series.
> > > 
> > > Correct, this driver didn't exist in Linus's tree when I developed my changes.
> > 
> > I stumbled above this issue, too. For the record, this patch fixes the
> > build issue:
> 
> [PATCH 5.17 1/2] pinctrl: thunderbay: comment process of building functions a bit
> [PATCH 5.17 2/2] pinctrl: thunderbay: rework loops looking for groups names
> https://patchwork.ozlabs.org/project/linux-gpio/list/?series=280568
> 
> Patches already queued in the:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=fixes

Ah great. I hope this makes it into Linus Torvald's tree before -rc1 is
tagged. Having allmodconfig build broken in release candidates is really
annoying.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH V2 4/4] pinctrl: add one more "const" for generic function groups

[Uwe Kleine-König]

[-- Attachment #1.1: Type: text/plain, Size: 4243 bytes --]

On Tue, Jan 18, 2022 at 08:21:55AM +0100, Rafał Miłecki wrote:
> On 18.01.2022 08:02, Uwe Kleine-König wrote:
> > On Tue, Jan 11, 2022 at 05:51:44PM +0100, Rafał Miłecki wrote:
> > > On 11.01.2022 16:34, Nathan Chancellor wrote:
> > > > On Thu, Dec 16, 2021 at 05:22:06PM +0100, Rafał Miłecki wrote:
> > > > > From: Rafał Miłecki <rafal@milecki.pl>
> > > > > 
> > > > > Generic code doesn't modify those strings and .get_function_groups
> > > > > callback has that extra "const" as well. This allows more flexibility in
> > > > > GENERIC_PINMUX_FUNCTIONS users.
> > > > > 
> > > > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > > > > ---
> > > > >    drivers/pinctrl/pinmux.c | 2 +-
> > > > >    drivers/pinctrl/pinmux.h | 4 ++--
> > > > >    2 files changed, 3 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> > > > > index 6cdbd9ccf2f0..f94d43b082d9 100644
> > > > > --- a/drivers/pinctrl/pinmux.c
> > > > > +++ b/drivers/pinctrl/pinmux.c
> > > > > @@ -875,7 +875,7 @@ EXPORT_SYMBOL_GPL(pinmux_generic_get_function);
> > > > >     */
> > > > >    int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
> > > > >    				const char *name,
> > > > > -				const char **groups,
> > > > > +				const char * const *groups,
> > > > >    				const unsigned int num_groups,
> > > > >    				void *data)
> > > > >    {
> > > > > diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
> > > > > index 78c3a31be882..72fcf03eaa43 100644
> > > > > --- a/drivers/pinctrl/pinmux.h
> > > > > +++ b/drivers/pinctrl/pinmux.h
> > > > > @@ -129,7 +129,7 @@ static inline void pinmux_init_device_debugfs(struct dentry *devroot,
> > > > >     */
> > > > >    struct function_desc {
> > > > >    	const char *name;
> > > > > -	const char **group_names;
> > > > > +	const char * const *group_names;
> > > > >    	int num_group_names;
> > > > >    	void *data;
> > > > >    };
> > > > > @@ -150,7 +150,7 @@ struct function_desc *pinmux_generic_get_function(struct pinctrl_dev *pctldev,
> > > > >    int pinmux_generic_add_function(struct pinctrl_dev *pctldev,
> > > > >    				const char *name,
> > > > > -				const char **groups,
> > > > > +				const char * const *groups,
> > > > >    				unsigned const num_groups,
> > > > >    				void *data);
> > > > > -- 
> > > > > 2.31.1
> > > > > 
> > > > > 
> > > > 
> > > > I have not seen this reported yet, even though it has been broken for a
> > > > couple of weeks now. I see the following error in -next:
> > > > 
> > > > $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- allmodconfig drivers/pinctrl/pinctrl-thunderbay.o
> > > > drivers/pinctrl/pinctrl-thunderbay.c: In function ‘thunderbay_add_functions’:
> > > > drivers/pinctrl/pinctrl-thunderbay.c:815:29: error: assignment discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
> > > >     815 |                         grp = func->group_names;
> > > >         |                             ^
> > > > cc1: all warnings being treated as errors
> > > > 
> > > > Looks like something like the third patch of the series is needed for
> > > > the Thunderbay driver, which it appears was in development at the same
> > > > time as this series.
> > > 
> > > Correct, this driver didn't exist in Linus's tree when I developed my changes.
> > 
> > I stumbled above this issue, too. For the record, this patch fixes the
> > build issue:
> 
> [PATCH 5.17 1/2] pinctrl: thunderbay: comment process of building functions a bit
> [PATCH 5.17 2/2] pinctrl: thunderbay: rework loops looking for groups names
> https://patchwork.ozlabs.org/project/linux-gpio/list/?series=280568
> 
> Patches already queued in the:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=fixes

Ah great. I hope this makes it into Linus Torvald's tree before -rc1 is
tagged. Having allmodconfig build broken in release candidates is really
annoying.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Andy Shevchenko]

On Fri, Dec 17, 2021 at 1:54 AM Rafał Miłecki <zajec5@gmail.com> wrote:
>
> From: Rafał Miłecki <rafal@milecki.pl>
>
> The plan for "struct function_desc" is to make its "group_names"
> /double/ const. That will allow drivers to use it with static const
> data.

Good plan, I support it!

> This imx change is required to avoid:
> drivers/pinctrl/freescale/pinctrl-imx.c: In function 'imx_pinctrl_parse_functions':
> drivers/pinctrl/freescale/pinctrl-imx.c:672:24: error: assignment of read-only location '*(func->group_names + (sizetype)(i * 4))'
>   672 |   func->group_names[i] = child->name;
>       |                        ^

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Andy Shevchenko]

On Fri, Dec 17, 2021 at 1:54 AM Rafał Miłecki <zajec5@gmail.com> wrote:
>
> From: Rafał Miłecki <rafal@milecki.pl>
>
> The plan for "struct function_desc" is to make its "group_names"
> /double/ const. That will allow drivers to use it with static const
> data.

Good plan, I support it!

> This imx change is required to avoid:
> drivers/pinctrl/freescale/pinctrl-imx.c: In function 'imx_pinctrl_parse_functions':
> drivers/pinctrl/freescale/pinctrl-imx.c:672:24: error: assignment of read-only location '*(func->group_names + (sizetype)(i * 4))'
>   672 |   func->group_names[i] = child->name;
>       |                        ^

-- 
With Best Regards,
Andy Shevchenko

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Linus Walleij]

On Thu, Dec 16, 2021 at 5:22 PM Rafał Miłecki <zajec5@gmail.com> wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
>
> The plan for "struct function_desc" is to make its "group_names"
> /double/ const. That will allow drivers to use it with static const
> data.
>
> This imx change is required to avoid:
> drivers/pinctrl/freescale/pinctrl-imx.c: In function 'imx_pinctrl_parse_functions':
> drivers/pinctrl/freescale/pinctrl-imx.c:672:24: error: assignment of read-only location '*(func->group_names + (sizetype)(i * 4))'
>   672 |   func->group_names[i] = child->name;
>       |                        ^
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Patches applied, let's see what happens!

Yours,
Linus Walleij

Re: [PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Linus Walleij]

On Thu, Dec 16, 2021 at 5:22 PM Rafał Miłecki <zajec5@gmail.com> wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
>
> The plan for "struct function_desc" is to make its "group_names"
> /double/ const. That will allow drivers to use it with static const
> data.
>
> This imx change is required to avoid:
> drivers/pinctrl/freescale/pinctrl-imx.c: In function 'imx_pinctrl_parse_functions':
> drivers/pinctrl/freescale/pinctrl-imx.c:672:24: error: assignment of read-only location '*(func->group_names + (sizetype)(i * 4))'
>   672 |   func->group_names[i] = child->name;
>       |                        ^
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Patches applied, let's see what happens!

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Abel Vesa]

On 21-12-16 17:22:03, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> The plan for "struct function_desc" is to make its "group_names"
> /double/ const. That will allow drivers to use it with static const
> data.
> 
> This imx change is required to avoid:
> drivers/pinctrl/freescale/pinctrl-imx.c: In function 'imx_pinctrl_parse_functions':
> drivers/pinctrl/freescale/pinctrl-imx.c:672:24: error: assignment of read-only location '*(func->group_names + (sizetype)(i * 4))'
>   672 |   func->group_names[i] = child->name;
>       |                        ^
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  drivers/pinctrl/freescale/pinctrl-imx.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
> index daf28bc5661d..47b2ab1a14d0 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> @@ -648,6 +648,7 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
>  	struct device_node *child;
>  	struct function_desc *func;
>  	struct group_desc *grp;
> +	const char **group_names;
>  	u32 i = 0;
>  
>  	dev_dbg(pctl->dev, "parse function(%d): %pOFn\n", index, np);
> @@ -663,14 +664,16 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
>  		dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
>  		return -EINVAL;
>  	}
> -	func->group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
> -					 sizeof(char *), GFP_KERNEL);
> +
> +	group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
> +				   sizeof(char *), GFP_KERNEL);
>  	if (!func->group_names)
This line needs to be:
  	if (!group_names)

Otherwise, the driver never probes successufully.

Linus, maybe you can squashed this fix in your tree without a resend.

>  		return -ENOMEM;
> +	for_each_child_of_node(np, child)
> +		group_names[i] = child->name;
> +	func->group_names = group_names;
>  
>  	for_each_child_of_node(np, child) {
> -		func->group_names[i] = child->name;
> -
>  		grp = devm_kzalloc(ipctl->dev, sizeof(struct group_desc),
>  				   GFP_KERNEL);
>  		if (!grp) {
> -- 
> 2.31.1
>

Re: [PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Abel Vesa]

On 21-12-16 17:22:03, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> The plan for "struct function_desc" is to make its "group_names"
> /double/ const. That will allow drivers to use it with static const
> data.
> 
> This imx change is required to avoid:
> drivers/pinctrl/freescale/pinctrl-imx.c: In function 'imx_pinctrl_parse_functions':
> drivers/pinctrl/freescale/pinctrl-imx.c:672:24: error: assignment of read-only location '*(func->group_names + (sizetype)(i * 4))'
>   672 |   func->group_names[i] = child->name;
>       |                        ^
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  drivers/pinctrl/freescale/pinctrl-imx.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
> index daf28bc5661d..47b2ab1a14d0 100644
> --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> @@ -648,6 +648,7 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
>  	struct device_node *child;
>  	struct function_desc *func;
>  	struct group_desc *grp;
> +	const char **group_names;
>  	u32 i = 0;
>  
>  	dev_dbg(pctl->dev, "parse function(%d): %pOFn\n", index, np);
> @@ -663,14 +664,16 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
>  		dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
>  		return -EINVAL;
>  	}
> -	func->group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
> -					 sizeof(char *), GFP_KERNEL);
> +
> +	group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
> +				   sizeof(char *), GFP_KERNEL);
>  	if (!func->group_names)
This line needs to be:
  	if (!group_names)

Otherwise, the driver never probes successufully.

Linus, maybe you can squashed this fix in your tree without a resend.

>  		return -ENOMEM;
> +	for_each_child_of_node(np, child)
> +		group_names[i] = child->name;
> +	func->group_names = group_names;
>  
>  	for_each_child_of_node(np, child) {
> -		func->group_names[i] = child->name;
> -
>  		grp = devm_kzalloc(ipctl->dev, sizeof(struct group_desc),
>  				   GFP_KERNEL);
>  		if (!grp) {
> -- 
> 2.31.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Marcel Ziswiler]

On Wed, 2021-12-22 at 22:24 +0200, Abel Vesa wrote:
> On 21-12-16 17:22:03, Rafał Miłecki wrote:
> > From: Rafał Miłecki <rafal@milecki.pl>
> > 
> > The plan for "struct function_desc" is to make its "group_names"
> > /double/ const. That will allow drivers to use it with static const
> > data.
> > 
> > This imx change is required to avoid:
> > drivers/pinctrl/freescale/pinctrl-imx.c: In function 'imx_pinctrl_parse_functions':
> > drivers/pinctrl/freescale/pinctrl-imx.c:672:24: error: assignment of read-only location '*(func-
> > >group_names + (sizetype)(i * 4))'
> >   672 |   func->group_names[i] = child->name;
> >       |                        ^
> > 
> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > ---
> >  drivers/pinctrl/freescale/pinctrl-imx.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
> > index daf28bc5661d..47b2ab1a14d0 100644
> > --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> > +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> > @@ -648,6 +648,7 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
> >         struct device_node *child;
> >         struct function_desc *func;
> >         struct group_desc *grp;
> > +       const char **group_names;
> >         u32 i = 0;
> >  
> >         dev_dbg(pctl->dev, "parse function(%d): %pOFn\n", index, np);
> > @@ -663,14 +664,16 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
> >                 dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
> >                 return -EINVAL;
> >         }
> > -       func->group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
> > -                                        sizeof(char *), GFP_KERNEL);
> > +
> > +       group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
> > +                                  sizeof(char *), GFP_KERNEL);
> >         if (!func->group_names)
> This line needs to be:
>         if (!group_names)
> 
> Otherwise, the driver never probes successufully.

After my i.MX 8M Mini target running latest -next just hang early boot I bisected it to this commit.

I can confirm that this fixes it. Thanks!

> Linus, maybe you can squashed this fix in your tree without a resend.
> 
> >                 return -ENOMEM;
> > +       for_each_child_of_node(np, child)
> > +               group_names[i] = child->name;
> > +       func->group_names = group_names;
> >  
> >         for_each_child_of_node(np, child) {
> > -               func->group_names[i] = child->name;
> > -
> >                 grp = devm_kzalloc(ipctl->dev, sizeof(struct group_desc),
> >                                    GFP_KERNEL);
> >                 if (!grp) {
> > -- 
> > 2.31.1
-- 
Best regards - Mit freundlichen Grüssen - Meilleures salutations

Marcel Ziswiler
Software Team Lead - Embedded Linux BSP

Toradex AG
Ebenaustrasse 10 | 6048 Horw/Luzern | Switzerland | T: +41 41 500 4800

Re: [PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Marcel Ziswiler]

On Wed, 2021-12-22 at 22:24 +0200, Abel Vesa wrote:
> On 21-12-16 17:22:03, Rafał Miłecki wrote:
> > From: Rafał Miłecki <rafal@milecki.pl>
> > 
> > The plan for "struct function_desc" is to make its "group_names"
> > /double/ const. That will allow drivers to use it with static const
> > data.
> > 
> > This imx change is required to avoid:
> > drivers/pinctrl/freescale/pinctrl-imx.c: In function 'imx_pinctrl_parse_functions':
> > drivers/pinctrl/freescale/pinctrl-imx.c:672:24: error: assignment of read-only location '*(func-
> > >group_names + (sizetype)(i * 4))'
> >   672 |   func->group_names[i] = child->name;
> >       |                        ^
> > 
> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > ---
> >  drivers/pinctrl/freescale/pinctrl-imx.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
> > index daf28bc5661d..47b2ab1a14d0 100644
> > --- a/drivers/pinctrl/freescale/pinctrl-imx.c
> > +++ b/drivers/pinctrl/freescale/pinctrl-imx.c
> > @@ -648,6 +648,7 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
> >         struct device_node *child;
> >         struct function_desc *func;
> >         struct group_desc *grp;
> > +       const char **group_names;
> >         u32 i = 0;
> >  
> >         dev_dbg(pctl->dev, "parse function(%d): %pOFn\n", index, np);
> > @@ -663,14 +664,16 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
> >                 dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
> >                 return -EINVAL;
> >         }
> > -       func->group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
> > -                                        sizeof(char *), GFP_KERNEL);
> > +
> > +       group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
> > +                                  sizeof(char *), GFP_KERNEL);
> >         if (!func->group_names)
> This line needs to be:
>         if (!group_names)
> 
> Otherwise, the driver never probes successufully.

After my i.MX 8M Mini target running latest -next just hang early boot I bisected it to this commit.

I can confirm that this fixes it. Thanks!

> Linus, maybe you can squashed this fix in your tree without a resend.
> 
> >                 return -ENOMEM;
> > +       for_each_child_of_node(np, child)
> > +               group_names[i] = child->name;
> > +       func->group_names = group_names;
> >  
> >         for_each_child_of_node(np, child) {
> > -               func->group_names[i] = child->name;
> > -
> >                 grp = devm_kzalloc(ipctl->dev, sizeof(struct group_desc),
> >                                    GFP_KERNEL);
> >                 if (!grp) {
> > -- 
> > 2.31.1
-- 
Best regards - Mit freundlichen Grüssen - Meilleures salutations

Marcel Ziswiler
Software Team Lead - Embedded Linux BSP

Toradex AG
Ebenaustrasse 10 | 6048 Horw/Luzern | Switzerland | T: +41 41 500 4800
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Rafał Miłecki]

On 2.01.2022 00:55, Marcel Ziswiler wrote:
> On Wed, 2021-12-22 at 22:24 +0200, Abel Vesa wrote:
>>> @@ -663,14 +664,16 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
>>>                  dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
>>>                  return -EINVAL;
>>>          }
>>> -       func->group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
>>> -                                        sizeof(char *), GFP_KERNEL);
>>> +
>>> +       group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
>>> +                                  sizeof(char *), GFP_KERNEL);
>>>          if (!func->group_names)
>> This line needs to be:
>>          if (!group_names)
>>
>> Otherwise, the driver never probes successufully.
> 
> After my i.MX 8M Mini target running latest -next just hang early boot I bisected it to this commit.
> 
> I can confirm that this fixes it. Thanks!

Please note there is one more pending fix. Please apply both:
[PATCH] pinctrl: imx: fix allocation result check
[PATCH] pinctrl: imx: fix assigning groups names

Re: [PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Rafał Miłecki]

On 2.01.2022 00:55, Marcel Ziswiler wrote:
> On Wed, 2021-12-22 at 22:24 +0200, Abel Vesa wrote:
>>> @@ -663,14 +664,16 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
>>>                  dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
>>>                  return -EINVAL;
>>>          }
>>> -       func->group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
>>> -                                        sizeof(char *), GFP_KERNEL);
>>> +
>>> +       group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
>>> +                                  sizeof(char *), GFP_KERNEL);
>>>          if (!func->group_names)
>> This line needs to be:
>>          if (!group_names)
>>
>> Otherwise, the driver never probes successufully.
> 
> After my i.MX 8M Mini target running latest -next just hang early boot I bisected it to this commit.
> 
> I can confirm that this fixes it. Thanks!

Please note there is one more pending fix. Please apply both:
[PATCH] pinctrl: imx: fix allocation result check
[PATCH] pinctrl: imx: fix assigning groups names

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Marcel Ziswiler]

On Sun, 2022-01-02 at 00:58 +0100, Rafał Miłecki wrote:
> On 2.01.2022 00:55, Marcel Ziswiler wrote:
> > On Wed, 2021-12-22 at 22:24 +0200, Abel Vesa wrote:
> > > > @@ -663,14 +664,16 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
> > > >                  dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
> > > >                  return -EINVAL;
> > > >          }
> > > > -       func->group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
> > > > -                                        sizeof(char *), GFP_KERNEL);
> > > > +
> > > > +       group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
> > > > +                                  sizeof(char *), GFP_KERNEL);
> > > >          if (!func->group_names)
> > > This line needs to be:
> > >          if (!group_names)
> > > 
> > > Otherwise, the driver never probes successufully.
> > 
> > After my i.MX 8M Mini target running latest -next just hang early boot I bisected it to this commit.
> > 
> > I can confirm that this fixes it. Thanks!
> 
> Please note there is one more pending fix. Please apply both:
> [PATCH] pinctrl: imx: fix allocation result check
> [PATCH] pinctrl: imx: fix assigning groups names

Yep, also just noticed that. Thanks!

Re: [PATCH V2 1/4] pinctrl: imx: prepare for making "group_names" in "function_desc" const

[Marcel Ziswiler]

On Sun, 2022-01-02 at 00:58 +0100, Rafał Miłecki wrote:
> On 2.01.2022 00:55, Marcel Ziswiler wrote:
> > On Wed, 2021-12-22 at 22:24 +0200, Abel Vesa wrote:
> > > > @@ -663,14 +664,16 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
> > > >                  dev_err(ipctl->dev, "no groups defined in %pOF\n", np);
> > > >                  return -EINVAL;
> > > >          }
> > > > -       func->group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
> > > > -                                        sizeof(char *), GFP_KERNEL);
> > > > +
> > > > +       group_names = devm_kcalloc(ipctl->dev, func->num_group_names,
> > > > +                                  sizeof(char *), GFP_KERNEL);
> > > >          if (!func->group_names)
> > > This line needs to be:
> > >          if (!group_names)
> > > 
> > > Otherwise, the driver never probes successufully.
> > 
> > After my i.MX 8M Mini target running latest -next just hang early boot I bisected it to this commit.
> > 
> > I can confirm that this fixes it. Thanks!
> 
> Please note there is one more pending fix. Please apply both:
> [PATCH] pinctrl: imx: fix allocation result check
> [PATCH] pinctrl: imx: fix assigning groups names

Yep, also just noticed that. Thanks!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel