pinctrl: mxs: warn if functions are not grouped by name
diff mbox

Message ID 1413884445-28347-1-git-send-email-u.kleine-koenig@pengutronix.de
State Not Applicable
Headers show

Commit Message

Uwe Kleine-König Oct. 21, 2014, 9:40 a.m. UTC
The mxs pinctrl driver cannot handle when functions are not grouped by
name (which IMO is a bug). This happens for example if a
imx28-somemachine.dts provides a function that has the same name as a
function defined in imx28.dtsi.

The proper way to fix that would be to check for duplicates in the loops
(which increases parsing time) or parse the groups first and sort the
resulting array.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
I tried to fix that, but it made my machine fail to boot without any
output and I currently don't have access to an i.MX28 with a working
debug UART, so I cannot use earlyprintk without effort.

I wrote this patch because I debugged this issue the second time now.

I admit that a fix would be better, but a warning is better as the
status quo. If someone is interested in a proper fix I can provide my
broken patch, maybe someone spots my error.

The current issue was with mac0 on an i.MX28 and I expected the fec
driver to fail to bind, but ethernet works just fine (relying on the
pinmuxing done by the bootloader).

Also something not catched is if for two functions with the same name,
the reg property is identical. This somehow breaks, too. Didn't debug
this one.

Best regards
Uwe
---
 drivers/pinctrl/freescale/pinctrl-mxs.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

Comments

Shawn Guo Oct. 25, 2014, 12:24 p.m. UTC | #1
On Tue, Oct 21, 2014 at 11:40:45AM +0200, Uwe Kleine-König wrote:
> The mxs pinctrl driver cannot handle when functions are not grouped by
> name (which IMO is a bug). This happens for example if a
> imx28-somemachine.dts provides a function that has the same name as a
> function defined in imx28.dtsi.
> 
> The proper way to fix that would be to check for duplicates in the loops
> (which increases parsing time) or parse the groups first and sort the
> resulting array.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Acked-by: Shawn Guo <shawn.guo@linaro.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Oct. 30, 2014, 2:53 p.m. UTC | #2
On Tue, Oct 21, 2014 at 11:40 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:

> The mxs pinctrl driver cannot handle when functions are not grouped by
> name (which IMO is a bug). This happens for example if a
> imx28-somemachine.dts provides a function that has the same name as a
> function defined in imx28.dtsi.

Patch applied with Shawn's ACK.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/pinctrl/freescale/pinctrl-mxs.c b/drivers/pinctrl/freescale/pinctrl-mxs.c
index f98c6bb0f769..646d5c244af1 100644
--- a/drivers/pinctrl/freescale/pinctrl-mxs.c
+++ b/drivers/pinctrl/freescale/pinctrl-mxs.c
@@ -445,6 +445,31 @@  static int mxs_pinctrl_probe_dt(struct platform_device *pdev,
 		if (of_property_read_u32(child, "reg", &val))
 			continue;
 		if (strcmp(fn, child->name)) {
+			struct device_node *child2;
+
+			/*
+			 * This reference is dropped by
+			 * of_get_next_child(np, * child)
+			 */
+			of_node_get(child);
+
+			/*
+			 * The logic parsing the functions from dt currently
+			 * doesn't handle if functions with the same name are
+			 * not grouped together. Only the first contiguous
+			 * cluster is usable for each function name. This is a
+			 * bug that is not trivial to fix, but at least warn
+			 * about it.
+			 */
+			for (child2 = of_get_next_child(np, child);
+			     child2 != NULL;
+			     child2 = of_get_next_child(np, child2)) {
+				if (!strcmp(child2->name, fn))
+					dev_warn(&pdev->dev,
+						 "function nodes must be grouped by name (failed for: %s)",
+						 fn);
+			}
+
 			f = &soc->functions[idxf++];
 			f->name = fn = child->name;
 		}