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