On 07/06/18 16:23, Richard Fitzgerald wrote:
On 07/06/18 15:09, Fabio Estevam wrote:
Hi Richard,
On Thu, Jun 7, 2018 at 7:14 AM, Richard Fitzgerald
<rf@xxxxxxxxxxxxxxxxxxxxx> wrote:
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
I applied the following change in the original 4.17 kernel:
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -122,6 +122,11 @@ static int dt_to_map_one_config(struct pinctrl *p,
/* OK let's just assume this will appear
later then */
return -EPROBE_DEFER;
}
+
+ if (np_pctldev == p->dev->of_node)
+ dev_info(p->dev, "***** Matched: %s\n",
p->dev->of_node->name);
+ else
+ dev_info(p->dev, "***** Not Matched: %s\n",
p->dev->of_node->name);
/* If we're creating a hog we can use the passed
pctldev */
if (pctldev && (np_pctldev == p->dev->of_node))
break;
and this is what I get:
[ 0.082643] imx53-pinctrl 53fa8000.iomuxc: ***** Not Matched: iomuxc
[ 0.082803] imx53-pinctrl 53fa8000.iomuxc: ***** Matched: iomuxc
[ 0.084386] imx53-pinctrl 53fa8000.iomuxc: ***** Not Matched: iomuxc
[ 0.084413] imx53-pinctrl 53fa8000.iomuxc: ***** Matched: iomuxc
[ 0.155791] imx-i2c 63fc4000.i2c: ***** Not Matched: i2c
[ 0.155828] imx-i2c 63fc4000.i2c: ***** Not Matched: i2c
[ 0.158001] imx-i2c 63fc8000.i2c: ***** Not Matched: i2c
[ 0.158033] imx-i2c 63fc8000.i2c: ***** Not Matched: i2c
[ 0.159389] mc13xxx 0-0008: ***** Not Matched: mc34708
[ 0.159440] mc13xxx 0-0008: ***** Not Matched: mc34708
[ 0.247099] imx-audmux 63fd0000.audmux: ***** Not Matched: audmux
[ 0.247130] imx-audmux 63fd0000.audmux: ***** Not Matched: audmux
[ 0.340774] imx-uart 53fbc000.serial: ***** Not Matched: serial
[ 0.340819] imx-uart 53fbc000.serial: ***** Not Matched: serial
[ 1.189378] imx-tve 63ff0000.tve: ***** Not Matched: tve
[ 1.194917] imx-tve 63ff0000.tve: ***** Not Matched: tve
[ 1.485778] fec 63fec000.ethernet: ***** Not Matched: ethernet
[ 1.491713] fec 63fec000.ethernet: ***** Not Matched: ethernet
[ 1.746872] sdhci-esdhc-imx 50004000.esdhc: ***** Not Matched: esdhc
[ 1.753489] sdhci-esdhc-imx 50004000.esdhc: ***** Not Matched: esdhc
[ 1.818500] sdhci-esdhc-imx 50020000.esdhc: ***** Not Matched: esdhc
[ 1.825480] sdhci-esdhc-imx 50020000.esdhc: ***** Not Matched: esdhc
[ 1.883043] leds-gpio leds: ***** Not Matched: leds
[ 1.887976] leds-gpio leds: ***** Not Matched: leds
I am running imx53-qdb.dtb which includes
arch/arm/boot/dts/imx53-qsb-common.dtsi.
The pinctrl driver is drivers/pinctrl/freescale/pinctrl-imx53.c
Any ideas?
Most likely the bug that my patch fixed was masking a different
problem with the iMX system.
In imx53-qsb-common.dtsi the iomuxc pinctrl driver node has some hogs
and also has many other pinctrl configurations that are not hogs and
are used by other drivers. What I think is happening is that because of
the bug in the original dt_to_map_one_config() all the child nodes under
iomux would have been treated as hogs, including the ones that are not
hogs. So when the iomuxc driver is probed all the possible pinctrl
configurations would be applied immediately and you would see correctly
configured pin muxes even though the way they got configured was wrong.
I assume your problem is that those settings which are not hogs are not
getting set up now? That indicates that for some reason when the
individual drivers call pinctrl_get() the mappings do not get applied.
So the interesting case to debug is what happens when
dt_to_map_one_config() is called with pctldev==NULL, why are the
pinctrl mappings requested by all those drivers not mapped to the
iomuxc driver?
Which of the pinctrl configuration are not applied? Are all missing?
Or only some?
In your log:
iomuxc - this is a pinctrl driver with hogs and the debug shows that
the hogs were matched on the 2nd try (they are 2 levels down
from the parent node)
It also contains many other pinctrl configurations used by
other drivers. These are _not_ hogs (because they are not used
by the iomux driver).
All the other drivers are not pinctrl drivers and they have their own DT
nodes. So it is expected that np_pctldev != p->dev->of_node.
np_pctldev is the pointer to the pinctrl definition node under &iomuxc
p->dev_of_node points to the client driver DT node, like &esdhc1
Another thing, you should be extremely careful about using "default"
state because this probably doesn't do what you think. What it does is
"Try to apply these dependencies before calling the driver probe() BUT
DO NOT ABORT PROBE IF THE PINCTRL MAPPING FAILS, GO AHEAD AND CALL
probe() ANYWAY".
Thus, even if the driver core failed to apply your pinctrl "default"
mapping, it will still probe your driver. You can't assume that
"default" will have been applied when your driver probes. It's much
safer to define your own state and explicitly request it from your
driver probe, so that you can EPROBE_DEFER if the pinctrl mapping fails.
You might be able to explicitly request "default" but I seem to recall
there was a reason that didn't work.
(We've had a look at making "default" behave as you might expect but it
looked difficult to do and likely to break so many things it's easier to
avoid it.)
--
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
