Re: [PATCH] Revert "pinctrl: devicetree: Fix dt_to_map_one_config handling of hogs"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/06/18 22:40, Fabio Estevam wrote:
From: Fabio Estevam <fabio.estevam@xxxxxxx>

This reverts commit b89405b6102fcc3746f43697b826028caa94c823.

commit b89405b6102f ("pinctrl: devicetree: Fix dt_to_map_one_config
handling of hogs") causes all the pinctrl hog pins to be ignored
leaving them with the IOMUX settings untouched.

This causes several regressions on i.MX such as:


I don't mind you reverting it if you are also providing an alternative
fix for the bug in the original code.

Also by the way I'm not convinced that this patch "causes all the
pinctrl hog pins to be ignored" - it doesn't on our pin controller using
the generic pinctrl DT functionality. So there's something more specific
to the iMX configuration going on.

- OV5640 camera driver can not be probed anymore on imx6qdl-sabresd
because the camera clock pin is in a pinctrl_hog group and since
its pinctrl initialization is skipped, the camera clock is kept
in GPIO functionality instead of CLK_CKO function.


Have you determined why the iMX driver's hogs fail the check
(np_pctldev == p->dev->of_node)? That would seem to be the reason
why it's not working for you and an important question to answer. It
implies that either your hog is not in its parent pinctrl DT node (so
how can it be a hog?) or I've missed other possible of_node
relationships that a hog is allowed to have to its parent pinctrl driver
node, in which case it would be better to do a patch to fix my if
statement rather than reintroduce the bug the iMX code relies on. My
understanding was that a "hog" is when a pinctrl entry is a child node
of the pinctrl driver that supplies it, therefore np_pctldev ==
p->dev->of_node.

- Audio stopped working on imx6qdl-wandboard and imx53-qsb for
the same reason.

Reported-by: Steve Longerbeam <slongerbeam@xxxxxxxxx>
Signed-off-by: Fabio Estevam <fabio.estevam@xxxxxxx>
---
  drivers/pinctrl/devicetree.c | 6 ++----
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index b601039..1ff6c3573 100644
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -122,10 +122,8 @@ static int dt_to_map_one_config(struct pinctrl *p,
  			/* OK let's just assume this will appear later then */
  			return -EPROBE_DEFER;
  		}
-		/* If we're creating a hog we can use the passed pctldev */
-		if (pctldev && (np_pctldev == p->dev->of_node))
-			break;
-		pctldev = get_pinctrl_dev_from_of_node(np_pctldev);
+		if (!pctldev)
+			pctldev = get_pinctrl_dev_from_of_node(np_pctldev);

However these two lines you are putting back will apply any pinctrl
entry to pctldev even if it doesn't refer to pctldev. There could
be dependencies on other pin controllers and those are invariant
regardless of the value of pctldev passed into this function.

pinctrl_dt_to_map() walks through all phandles in the "pinctrl-n"
property and calls dt_to_map_one_config() on each. Those phandles could
point to nodes in other pin controllers. The original code would apply
all of them to pctldev because pctldev!=NULL.

  		if (pctldev)
  			break;
  		/* Do not defer probing of hogs (circular loop) */


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



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux