Re: [PATCH] pinctrl: aspeed-g5: Delay acquisition of regmaps

2019-07-28 Thread Linus Walleij
On Wed, Jul 24, 2019 at 10:02 AM Andrew Jeffery  wrote:

> While sorting out some devicetree issues I found that the pinctrl driver
> was failing to acquire its GFX regmap even though the phandle was
> present in the devicetree:
>
> [0.124190] aspeed-g5-pinctrl 1e6e2000.syscon:pinctrl: No GFX phandle 
> found, some mux configurations may fail
>
> Without access to the GFX regmap we fail to configure the mux for the
> VPO function:
>
> [1.548866] pinctrl core: add 1 pinctrl maps
> [1.549826] aspeed-g5-pinctrl 1e6e2000.syscon:pinctrl: found group 
> selector 164 for VPO
> [1.550638] aspeed-g5-pinctrl 1e6e2000.syscon:pinctrl: request pin 144 
> (V20) for 1e6e6000.display
> [1.551346] aspeed-g5-pinctrl 1e6e2000.syscon:pinctrl: request pin 145 
> (U19) for 1e6e6000.display
> ...
> [1.562057] aspeed-g5-pinctrl 1e6e2000.syscon:pinctrl: request pin 218 
> (T22) for 1e6e6000.display
> [1.562541] aspeed-g5-pinctrl 1e6e2000.syscon:pinctrl: request pin 219 
> (R20) for 1e6e6000.display
> [1.563113] Muxing pin 144 for VPO
> [1.563456] Want SCU8C[0x0001]=0x1, got 0x0 from 0x
> [1.564624] aspeed_gfx 1e6e6000.display: Error applying setting, 
> reverse things back
>
> This turned out to be a simple problem of timing: The ASPEED pinctrl
> driver is probed during arch_initcall(), while GFX is processed much
> later. As such the GFX syscon is not yet registered during the pinctrl
> probe() and we get an -EPROBE_DEFER when we try to look it up, however
> we must not defer probing the pinctrl driver for the inability to mux
> some GFX-related functions.
>
> Switch to lazily grabbing the regmaps when they're first required by the
> mux configuration. This generates a bit of noise in the patch as we have
> to drop the `const` qualifier on arguments for several function
> prototypes, but has the benefit of working.
>
> I've smoke tested this for the ast2500-evb under qemu with a dummy
> graphics device. We now succeed in our attempts to configure the SoC's
> VPO pinmux function.
>
> Fixes: 7d29ed88acbb ("pinctrl: aspeed: Read and write bits in LPC and GFX 
> controllers")
> Signed-off-by: Andrew Jeffery 

Patch applied for fixes. Good rootcausing!

Yours,
Linus Walleij


[PATCH] pinctrl: aspeed-g5: Delay acquisition of regmaps

2019-07-24 Thread Andrew Jeffery
While sorting out some devicetree issues I found that the pinctrl driver
was failing to acquire its GFX regmap even though the phandle was
present in the devicetree:

[0.124190] aspeed-g5-pinctrl 1e6e2000.syscon:pinctrl: No GFX phandle 
found, some mux configurations may fail

Without access to the GFX regmap we fail to configure the mux for the
VPO function:

[1.548866] pinctrl core: add 1 pinctrl maps
[1.549826] aspeed-g5-pinctrl 1e6e2000.syscon:pinctrl: found group 
selector 164 for VPO
[1.550638] aspeed-g5-pinctrl 1e6e2000.syscon:pinctrl: request pin 144 
(V20) for 1e6e6000.display
[1.551346] aspeed-g5-pinctrl 1e6e2000.syscon:pinctrl: request pin 145 
(U19) for 1e6e6000.display
...
[1.562057] aspeed-g5-pinctrl 1e6e2000.syscon:pinctrl: request pin 218 
(T22) for 1e6e6000.display
[1.562541] aspeed-g5-pinctrl 1e6e2000.syscon:pinctrl: request pin 219 
(R20) for 1e6e6000.display
[1.563113] Muxing pin 144 for VPO
[1.563456] Want SCU8C[0x0001]=0x1, got 0x0 from 0x
[1.564624] aspeed_gfx 1e6e6000.display: Error applying setting, reverse 
things back

This turned out to be a simple problem of timing: The ASPEED pinctrl
driver is probed during arch_initcall(), while GFX is processed much
later. As such the GFX syscon is not yet registered during the pinctrl
probe() and we get an -EPROBE_DEFER when we try to look it up, however
we must not defer probing the pinctrl driver for the inability to mux
some GFX-related functions.

Switch to lazily grabbing the regmaps when they're first required by the
mux configuration. This generates a bit of noise in the patch as we have
to drop the `const` qualifier on arguments for several function
prototypes, but has the benefit of working.

I've smoke tested this for the ast2500-evb under qemu with a dummy
graphics device. We now succeed in our attempts to configure the SoC's
VPO pinmux function.

Fixes: 7d29ed88acbb ("pinctrl: aspeed: Read and write bits in LPC and GFX 
controllers")
Signed-off-by: Andrew Jeffery 
---
 drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c |  2 +-
 drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 92 +++---
 drivers/pinctrl/aspeed/pinctrl-aspeed.c| 12 ++-
 drivers/pinctrl/aspeed/pinmux-aspeed.h |  5 +-
 4 files changed, 74 insertions(+), 37 deletions(-)

diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c 
b/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
index 384396cbb22d..22256576b69a 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
@@ -2412,7 +2412,7 @@ static const struct aspeed_pin_config aspeed_g4_configs[] 
= {
{ PIN_CONFIG_INPUT_DEBOUNCE, { C14, B14 }, SCUA8, 27 },
 };
 
-static int aspeed_g4_sig_expr_set(const struct aspeed_pinmux_data *ctx,
+static int aspeed_g4_sig_expr_set(struct aspeed_pinmux_data *ctx,
  const struct aspeed_sig_expr *expr,
  bool enable)
 {
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c 
b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
index 053101f795a2..ba6438ac4d72 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
@@ -2507,6 +2507,61 @@ static struct aspeed_pin_config aspeed_g5_configs[] = {
{ PIN_CONFIG_INPUT_DEBOUNCE, { A20, B19 }, SCUA8, 27 },
 };
 
+static struct regmap *aspeed_g5_acquire_regmap(struct aspeed_pinmux_data *ctx,
+  int ip)
+{
+   if (ip == ASPEED_IP_SCU) {
+   WARN(!ctx->maps[ip], "Missing SCU syscon!");
+   return ctx->maps[ip];
+   }
+
+   if (ip >= ASPEED_NR_PINMUX_IPS)
+   return ERR_PTR(-EINVAL);
+
+   if (likely(ctx->maps[ip]))
+   return ctx->maps[ip];
+
+   if (ip == ASPEED_IP_GFX) {
+   struct device_node *node;
+   struct regmap *map;
+
+   node = of_parse_phandle(ctx->dev->of_node,
+   "aspeed,external-nodes", 0);
+   if (node) {
+   map = syscon_node_to_regmap(node);
+   of_node_put(node);
+   if (IS_ERR(map))
+   return map;
+   } else
+   return ERR_PTR(-ENODEV);
+
+   ctx->maps[ASPEED_IP_GFX] = map;
+   dev_dbg(ctx->dev, "Acquired GFX regmap");
+   return map;
+   }
+
+   if (ip == ASPEED_IP_LPC) {
+   struct device_node *node;
+   struct regmap *map;
+
+   node = of_parse_phandle(ctx->dev->of_node,
+   "aspeed,external-nodes", 1);
+   if (node) {
+   map = syscon_node_to_regmap(node->parent);
+   of_node_put(node);
+   if (IS_ERR(map))
+   return map;
+