From f46a0b47cc0829acd050213194c5a77351e619b2 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Mon, 3 Jul 2023 17:07:06 +0200 Subject: [PATCH 1/4] pinctrl: renesas: rzv2m: Handle non-unique subnode names The eMMC and SDHI pin control configuration nodes in DT have subnodes with the same names ("data" and "ctrl"). As the RZ/V2M pin control driver considers only the names of the subnodes, this leads to conflicts: pinctrl-rzv2m b6250000.pinctrl: pin P8_2 already requested by 85000000.mmc; cannot claim for 85020000.mmc pinctrl-rzv2m b6250000.pinctrl: pin-130 (85020000.mmc) status -22 renesas_sdhi_internal_dmac 85020000.mmc: Error applying setting, reverse things back Fix this by constructing unique names from the node names of both the pin control configuration node and its child node, where appropriate. Reported by: Fabrizio Castro Fixes: 92a9b825257614af ("pinctrl: renesas: Add RZ/V2M pin and gpio controller driver") Signed-off-by: Geert Uytterhoeven Tested-by: Fabrizio Castro Link: https://lore.kernel.org/r/607bd6ab4905b0b1b119a06ef953fa1184505777.1688396717.git.geert+renesas@glider.be --- drivers/pinctrl/renesas/pinctrl-rzv2m.c | 28 ++++++++++++++++++------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/drivers/pinctrl/renesas/pinctrl-rzv2m.c b/drivers/pinctrl/renesas/pinctrl-rzv2m.c index e5472293bc7f..35b23c1a5684 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzv2m.c +++ b/drivers/pinctrl/renesas/pinctrl-rzv2m.c @@ -209,6 +209,7 @@ static int rzv2m_map_add_config(struct pinctrl_map *map, static int rzv2m_dt_subnode_to_map(struct pinctrl_dev *pctldev, struct device_node *np, + struct device_node *parent, struct pinctrl_map **map, unsigned int *num_maps, unsigned int *index) @@ -226,6 +227,7 @@ static int rzv2m_dt_subnode_to_map(struct pinctrl_dev *pctldev, struct property *prop; int ret, gsel, fsel; const char **pin_fn; + const char *name; const char *pin; pinmux = of_find_property(np, "pinmux", NULL); @@ -309,8 +311,19 @@ static int rzv2m_dt_subnode_to_map(struct pinctrl_dev *pctldev, psel_val[i] = MUX_FUNC(value); } + if (parent) { + name = devm_kasprintf(pctrl->dev, GFP_KERNEL, "%pOFn.%pOFn", + parent, np); + if (!name) { + ret = -ENOMEM; + goto done; + } + } else { + name = np->name; + } + /* Register a single pin group listing all the pins we read from DT */ - gsel = pinctrl_generic_add_group(pctldev, np->name, pins, num_pinmux, NULL); + gsel = pinctrl_generic_add_group(pctldev, name, pins, num_pinmux, NULL); if (gsel < 0) { ret = gsel; goto done; @@ -320,17 +333,16 @@ static int rzv2m_dt_subnode_to_map(struct pinctrl_dev *pctldev, * Register a single group function where the 'data' is an array PSEL * register values read from DT. */ - pin_fn[0] = np->name; - fsel = pinmux_generic_add_function(pctldev, np->name, pin_fn, 1, - psel_val); + pin_fn[0] = name; + fsel = pinmux_generic_add_function(pctldev, name, pin_fn, 1, psel_val); if (fsel < 0) { ret = fsel; goto remove_group; } maps[idx].type = PIN_MAP_TYPE_MUX_GROUP; - maps[idx].data.mux.group = np->name; - maps[idx].data.mux.function = np->name; + maps[idx].data.mux.group = name; + maps[idx].data.mux.function = name; idx++; dev_dbg(pctrl->dev, "Parsed %pOF with %d pins\n", np, num_pinmux); @@ -377,7 +389,7 @@ static int rzv2m_dt_node_to_map(struct pinctrl_dev *pctldev, index = 0; for_each_child_of_node(np, child) { - ret = rzv2m_dt_subnode_to_map(pctldev, child, map, + ret = rzv2m_dt_subnode_to_map(pctldev, child, np, map, num_maps, &index); if (ret < 0) { of_node_put(child); @@ -386,7 +398,7 @@ static int rzv2m_dt_node_to_map(struct pinctrl_dev *pctldev, } if (*num_maps == 0) { - ret = rzv2m_dt_subnode_to_map(pctldev, np, map, + ret = rzv2m_dt_subnode_to_map(pctldev, np, NULL, map, num_maps, &index); if (ret < 0) goto done; From bfc374a145ae133613e05b9b89be561f169cb58d Mon Sep 17 00:00:00 2001 From: Biju Das Date: Tue, 4 Jul 2023 12:18:58 +0100 Subject: [PATCH 2/4] pinctrl: renesas: rzg2l: Handle non-unique subnode names Currently, sd1 and sd0 have unique subnode names 'sd1_mux' and 'sd0_mux'. If we change these to non-unique subnode names such as 'mux' this can lead to the below conflict as the RZ/G2L pin control driver considers only the names of the subnodes. pinctrl-rzg2l 11030000.pinctrl: pin P47_0 already requested by 11c00000.mmc; cannot claim for 11c10000.mmc pinctrl-rzg2l 11030000.pinctrl: pin-376 (11c10000.mmc) status -22 pinctrl-rzg2l 11030000.pinctrl: could not request pin 376 (P47_0) from group mux on device pinctrl-rzg2l renesas_sdhi_internal_dmac 11c10000.mmc: Error applying setting, reverse things back Fix this by constructing unique names from the node names of both the pin control configuration node and its child node, where appropriate. Based on the work done by Geert for the RZ/V2M pinctrl driver. Fixes: c4c4637eb57f ("pinctrl: renesas: Add RZ/G2L pin and gpio controller driver") Signed-off-by: Biju Das Reviewed-by: Geert Uytterhoeven Link: https://lore.kernel.org/r/20230704111858.215278-1-biju.das.jz@bp.renesas.com Signed-off-by: Geert Uytterhoeven --- drivers/pinctrl/renesas/pinctrl-rzg2l.c | 28 ++++++++++++++++++------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c index 9511d920565e..b53d26167da5 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c @@ -249,6 +249,7 @@ static int rzg2l_map_add_config(struct pinctrl_map *map, static int rzg2l_dt_subnode_to_map(struct pinctrl_dev *pctldev, struct device_node *np, + struct device_node *parent, struct pinctrl_map **map, unsigned int *num_maps, unsigned int *index) @@ -266,6 +267,7 @@ static int rzg2l_dt_subnode_to_map(struct pinctrl_dev *pctldev, struct property *prop; int ret, gsel, fsel; const char **pin_fn; + const char *name; const char *pin; pinmux = of_find_property(np, "pinmux", NULL); @@ -349,8 +351,19 @@ static int rzg2l_dt_subnode_to_map(struct pinctrl_dev *pctldev, psel_val[i] = MUX_FUNC(value); } + if (parent) { + name = devm_kasprintf(pctrl->dev, GFP_KERNEL, "%pOFn.%pOFn", + parent, np); + if (!name) { + ret = -ENOMEM; + goto done; + } + } else { + name = np->name; + } + /* Register a single pin group listing all the pins we read from DT */ - gsel = pinctrl_generic_add_group(pctldev, np->name, pins, num_pinmux, NULL); + gsel = pinctrl_generic_add_group(pctldev, name, pins, num_pinmux, NULL); if (gsel < 0) { ret = gsel; goto done; @@ -360,17 +373,16 @@ static int rzg2l_dt_subnode_to_map(struct pinctrl_dev *pctldev, * Register a single group function where the 'data' is an array PSEL * register values read from DT. */ - pin_fn[0] = np->name; - fsel = pinmux_generic_add_function(pctldev, np->name, pin_fn, 1, - psel_val); + pin_fn[0] = name; + fsel = pinmux_generic_add_function(pctldev, name, pin_fn, 1, psel_val); if (fsel < 0) { ret = fsel; goto remove_group; } maps[idx].type = PIN_MAP_TYPE_MUX_GROUP; - maps[idx].data.mux.group = np->name; - maps[idx].data.mux.function = np->name; + maps[idx].data.mux.group = name; + maps[idx].data.mux.function = name; idx++; dev_dbg(pctrl->dev, "Parsed %pOF with %d pins\n", np, num_pinmux); @@ -417,7 +429,7 @@ static int rzg2l_dt_node_to_map(struct pinctrl_dev *pctldev, index = 0; for_each_child_of_node(np, child) { - ret = rzg2l_dt_subnode_to_map(pctldev, child, map, + ret = rzg2l_dt_subnode_to_map(pctldev, child, np, map, num_maps, &index); if (ret < 0) { of_node_put(child); @@ -426,7 +438,7 @@ static int rzg2l_dt_node_to_map(struct pinctrl_dev *pctldev, } if (*num_maps == 0) { - ret = rzg2l_dt_subnode_to_map(pctldev, np, map, + ret = rzg2l_dt_subnode_to_map(pctldev, np, NULL, map, num_maps, &index); if (ret < 0) goto done; From f4b2ce40fd99ddb5c2ab9f1193454080f6714dfe Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Mon, 3 Jul 2023 17:09:59 +0200 Subject: [PATCH 3/4] pinctrl: renesas: rzv2m: Use devm_clk_get_enabled() helper Simplify clock handling by using the devm_clk_get_enabled() helper, instead of open-coding the same operations. Move the clock pointer from the driver-private data to a local variable, as it is not needed outside the .probe() callback. Signed-off-by: Geert Uytterhoeven Link: https://lore.kernel.org/r/cca0b1795fd0335401bdf2be393ab84445e04095.1688396836.git.geert+renesas@glider.be --- drivers/pinctrl/renesas/pinctrl-rzv2m.c | 32 ++++--------------------- 1 file changed, 5 insertions(+), 27 deletions(-) diff --git a/drivers/pinctrl/renesas/pinctrl-rzv2m.c b/drivers/pinctrl/renesas/pinctrl-rzv2m.c index 35b23c1a5684..f0852296c1d3 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzv2m.c +++ b/drivers/pinctrl/renesas/pinctrl-rzv2m.c @@ -118,7 +118,6 @@ struct rzv2m_pinctrl { const struct rzv2m_pinctrl_data *data; void __iomem *base; struct device *dev; - struct clk *clk; struct gpio_chip gpio_chip; struct pinctrl_gpio_range gpio_range; @@ -1039,14 +1038,10 @@ static int rzv2m_pinctrl_register(struct rzv2m_pinctrl *pctrl) return 0; } -static void rzv2m_pinctrl_clk_disable(void *data) -{ - clk_disable_unprepare(data); -} - static int rzv2m_pinctrl_probe(struct platform_device *pdev) { struct rzv2m_pinctrl *pctrl; + struct clk *clk; int ret; pctrl = devm_kzalloc(&pdev->dev, sizeof(*pctrl), GFP_KERNEL); @@ -1063,32 +1058,15 @@ static int rzv2m_pinctrl_probe(struct platform_device *pdev) if (IS_ERR(pctrl->base)) return PTR_ERR(pctrl->base); - pctrl->clk = devm_clk_get(pctrl->dev, NULL); - if (IS_ERR(pctrl->clk)) { - ret = PTR_ERR(pctrl->clk); - dev_err(pctrl->dev, "failed to get GPIO clk : %i\n", ret); - return ret; - } + clk = devm_clk_get_enabled(pctrl->dev, NULL); + if (IS_ERR(clk)) + return dev_err_probe(pctrl->dev, PTR_ERR(clk), + "failed to enable GPIO clk\n"); spin_lock_init(&pctrl->lock); platform_set_drvdata(pdev, pctrl); - ret = clk_prepare_enable(pctrl->clk); - if (ret) { - dev_err(pctrl->dev, "failed to enable GPIO clk: %i\n", ret); - return ret; - } - - ret = devm_add_action_or_reset(&pdev->dev, rzv2m_pinctrl_clk_disable, - pctrl->clk); - if (ret) { - dev_err(pctrl->dev, - "failed to register GPIO clk disable action, %i\n", - ret); - return ret; - } - ret = rzv2m_pinctrl_register(pctrl); if (ret) return ret; From 95eb19869401850f069723b296170b8b3bd5be9e Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Sat, 22 Jul 2023 22:41:56 +0200 Subject: [PATCH 4/4] pinctrl: renesas: rzg2l: Use devm_clk_get_enabled() helper The devm_clk_get_enabled() helper: - calls devm_clk_get() - calls clk_prepare_enable() and registers what is needed in order to call clk_disable_unprepare() when needed, as a managed resource. This simplifies the code and avoids the need of a dedicated function used with devm_add_action_or_reset(). While at it, use dev_err_probe() which filters -EPROBE_DEFER. Signed-off-by: Christophe JAILLET Reviewed-by: Geert Uytterhoeven Link: https://lore.kernel.org/r/a4a586337d692f0ca396b80d275ba634eb419593.1690058500.git.christophe.jaillet@wanadoo.fr [geert: Make clk local to rzg2l_pinctrl_probe()] Signed-off-by: Geert Uytterhoeven --- drivers/pinctrl/renesas/pinctrl-rzg2l.c | 32 ++++--------------------- 1 file changed, 5 insertions(+), 27 deletions(-) diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c index b53d26167da5..0b8d27eb66c0 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c @@ -144,7 +144,6 @@ struct rzg2l_pinctrl { const struct rzg2l_pinctrl_data *data; void __iomem *base; struct device *dev; - struct clk *clk; struct gpio_chip gpio_chip; struct pinctrl_gpio_range gpio_range; @@ -1470,14 +1469,10 @@ static int rzg2l_pinctrl_register(struct rzg2l_pinctrl *pctrl) return 0; } -static void rzg2l_pinctrl_clk_disable(void *data) -{ - clk_disable_unprepare(data); -} - static int rzg2l_pinctrl_probe(struct platform_device *pdev) { struct rzg2l_pinctrl *pctrl; + struct clk *clk; int ret; BUILD_BUG_ON(ARRAY_SIZE(rzg2l_gpio_configs) * RZG2L_PINS_PER_PORT > @@ -1500,33 +1495,16 @@ static int rzg2l_pinctrl_probe(struct platform_device *pdev) if (IS_ERR(pctrl->base)) return PTR_ERR(pctrl->base); - pctrl->clk = devm_clk_get(pctrl->dev, NULL); - if (IS_ERR(pctrl->clk)) { - ret = PTR_ERR(pctrl->clk); - dev_err(pctrl->dev, "failed to get GPIO clk : %i\n", ret); - return ret; - } + clk = devm_clk_get_enabled(pctrl->dev, NULL); + if (IS_ERR(clk)) + return dev_err_probe(pctrl->dev, PTR_ERR(clk), + "failed to enable GPIO clk\n"); spin_lock_init(&pctrl->lock); spin_lock_init(&pctrl->bitmap_lock); platform_set_drvdata(pdev, pctrl); - ret = clk_prepare_enable(pctrl->clk); - if (ret) { - dev_err(pctrl->dev, "failed to enable GPIO clk: %i\n", ret); - return ret; - } - - ret = devm_add_action_or_reset(&pdev->dev, rzg2l_pinctrl_clk_disable, - pctrl->clk); - if (ret) { - dev_err(pctrl->dev, - "failed to register GPIO clk disable action, %i\n", - ret); - return ret; - } - ret = rzg2l_pinctrl_register(pctrl); if (ret) return ret;