| Message ID | 20230119235501.53294-1-joel@jms.id.au |
|---|---|
| State | New |
| Headers | show |
| Series | pinctrl: aspeed-g5: Bypass clock check when fetching regmap | expand |
On Fri, 20 Jan 2023, at 10:25, Joel Stanley wrote: > A recent commit cf517fef601b ("pinctrl: aspeed: Force to disable the > function's signal") exposed a problem with fetching the regmap for > reading the GFX register. > > The Romulus machine the device tree contains a gpio hog for GPIO S7. > With the patch applied: > > Muxing pin 151 for GPIO > Disabling signal VPOB9 for VPO > aspeed-g5-pinctrl 1e6e2080.pinctrl: Failed to acquire regmap for IP block 1 > aspeed-g5-pinctrl 1e6e2080.pinctrl: request() failed for pin 151 > > The code path is aspeed-gpio -> pinmux-g5 -> regmap -> clk, and the > of_clock code returns an error as it doesn't have a valid struct clk_hw > pointer. The regmap call happens because pinmux wants to check the GFX > node (IP block 1) to query bits there. > > For reference, before the offending patch: > > Muxing pin 151 for GPIO > Disabling signal VPOB9 for VPO > Want SCU8C[0x00000080]=0x1, got 0x0 from 0x00000000 > Disabling signal VPOB9 for VPOOFF1 > Want SCU8C[0x00000080]=0x1, got 0x0 from 0x00000000 > Disabling signal VPOB9 for VPOOFF2 > Want SCU8C[0x00000080]=0x1, got 0x0 from 0x00000000 > Enabling signal GPIOS7 for GPIOS7 > Muxed pin 151 as GPIOS7 > gpio-943 (seq_cont): hogged as output/low > > As a workaround, skip the clock check to allow pinmux to proceed. We'd want the clock on and and the device out of reset before this makes sense though. We're just assuming the IP is in an operational state? Was this just accidentally working because reading the register in a bad state is producing 0 instead of other undefined garbage? Andrew
On Fri, Jan 20, 2023 at 3:35 AM Andrew Jeffery <andrew@aj.id.au> wrote: > On Fri, 20 Jan 2023, at 10:25, Joel Stanley wrote: > > A recent commit cf517fef601b ("pinctrl: aspeed: Force to disable the > > function's signal") exposed a problem with fetching the regmap for > > reading the GFX register. > > > > The Romulus machine the device tree contains a gpio hog for GPIO S7. > > With the patch applied: > > > > Muxing pin 151 for GPIO > > Disabling signal VPOB9 for VPO > > aspeed-g5-pinctrl 1e6e2080.pinctrl: Failed to acquire regmap for IP block 1 > > aspeed-g5-pinctrl 1e6e2080.pinctrl: request() failed for pin 151 > > > > The code path is aspeed-gpio -> pinmux-g5 -> regmap -> clk, and the > > of_clock code returns an error as it doesn't have a valid struct clk_hw > > pointer. The regmap call happens because pinmux wants to check the GFX > > node (IP block 1) to query bits there. > > > > For reference, before the offending patch: > > > > Muxing pin 151 for GPIO > > Disabling signal VPOB9 for VPO > > Want SCU8C[0x00000080]=0x1, got 0x0 from 0x00000000 > > Disabling signal VPOB9 for VPOOFF1 > > Want SCU8C[0x00000080]=0x1, got 0x0 from 0x00000000 > > Disabling signal VPOB9 for VPOOFF2 > > Want SCU8C[0x00000080]=0x1, got 0x0 from 0x00000000 > > Enabling signal GPIOS7 for GPIOS7 > > Muxed pin 151 as GPIOS7 > > gpio-943 (seq_cont): hogged as output/low > > > > As a workaround, skip the clock check to allow pinmux to proceed. > > We'd want the clock on and and the device out of reset before this > makes sense though. We're just assuming the IP is in an operational > state? Was this just accidentally working because reading the register > in a bad state is producing 0 instead of other undefined garbage? This makes sense, can we come up with a resonable patch for this problem or should this one be merged as an intermediate remedy? Yours, Linus Walleij
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c index 7ee0c473ad70..f714fe40e400 100644 --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c @@ -2635,7 +2635,7 @@ static struct regmap *aspeed_g5_acquire_regmap(struct aspeed_pinmux_data *ctx, node = of_parse_phandle(ctx->dev->of_node, "aspeed,external-nodes", 0); if (node) { - map = syscon_node_to_regmap(node); + map = device_node_to_regmap(node); of_node_put(node); if (IS_ERR(map)) return map;
A recent commit cf517fef601b ("pinctrl: aspeed: Force to disable the function's signal") exposed a problem with fetching the regmap for reading the GFX register. The Romulus machine the device tree contains a gpio hog for GPIO S7. With the patch applied: Muxing pin 151 for GPIO Disabling signal VPOB9 for VPO aspeed-g5-pinctrl 1e6e2080.pinctrl: Failed to acquire regmap for IP block 1 aspeed-g5-pinctrl 1e6e2080.pinctrl: request() failed for pin 151 The code path is aspeed-gpio -> pinmux-g5 -> regmap -> clk, and the of_clock code returns an error as it doesn't have a valid struct clk_hw pointer. The regmap call happens because pinmux wants to check the GFX node (IP block 1) to query bits there. For reference, before the offending patch: Muxing pin 151 for GPIO Disabling signal VPOB9 for VPO Want SCU8C[0x00000080]=0x1, got 0x0 from 0x00000000 Disabling signal VPOB9 for VPOOFF1 Want SCU8C[0x00000080]=0x1, got 0x0 from 0x00000000 Disabling signal VPOB9 for VPOOFF2 Want SCU8C[0x00000080]=0x1, got 0x0 from 0x00000000 Enabling signal GPIOS7 for GPIOS7 Muxed pin 151 as GPIOS7 gpio-943 (seq_cont): hogged as output/low As a workaround, skip the clock check to allow pinmux to proceed. Fixes: cf517fef601b ("pinctrl: aspeed: Force to disable the function's signal") Signed-off-by: Joel Stanley <joel@jms.id.au> --- drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)