Message ID | 20230704111858.215278-1-biju.das.jz@bp.renesas.com (mailing list archive) |
---|---|
State | Mainlined |
Commit | bfc374a145ae133613e05b9b89be561f169cb58d |
Delegated to: | Geert Uytterhoeven |
Headers | show |
Series | pinctrl: renesas: rzg2l: Handle non-unique subnode names | expand |
Tue, Jul 04, 2023 at 12:18:58PM +0100, Biju Das kirjoitti: > Currently, sd1 and sd0 have unique subnode names 'sd1_mux' and 'sd0_mux'. > If we change it to a non-unique subnode name such as 'mux' this can lead > to the below conflicts 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 RZ/V2M pinctrl driver. ... > + if (parent) { > + name = devm_kasprintf(pctrl->dev, GFP_KERNEL, "%pOFn.%pOFn", > + parent, np); Is devm_*() usage appropriate here? > + if (!name) { > + ret = -ENOMEM; > + goto done; > + } > + } else { > + name = np->name; > + }
On Tue, Jul 4, 2023 at 1:19 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > Currently, sd1 and sd0 have unique subnode names 'sd1_mux' and 'sd0_mux'. > If we change it to a non-unique subnode name such as 'mux' this can lead > to the below conflicts 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 RZ/V2M pinctrl driver. > > Fixes: c4c4637eb57f ("pinctrl: renesas: Add RZ/G2L pin and gpio controller driver") > Cc: stable <stable@kernel.org> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> i.e. will queue as a fix in renesas-pinctrl-for-v6.5. Gr{oetje,eeting}s, Geert
Hi Andy, On Wed, Jul 5, 2023 at 11:41 AM <andy.shevchenko@gmail.com> wrote: > Tue, Jul 04, 2023 at 12:18:58PM +0100, Biju Das kirjoitti: > > Currently, sd1 and sd0 have unique subnode names 'sd1_mux' and 'sd0_mux'. > > If we change it to a non-unique subnode name such as 'mux' this can lead > > to the below conflicts 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 RZ/V2M pinctrl driver. > > ... > > > + if (parent) { > > + name = devm_kasprintf(pctrl->dev, GFP_KERNEL, "%pOFn.%pOFn", > > + parent, np); > > Is devm_*() usage appropriate here? I think so, as this is tied to the lifetime of the pin control driver, which you cannot really remove/unbind without causing havoc. Note that several other allocations already use devm_*(), too. > > > + if (!name) { > > + ret = -ENOMEM; > > + goto done; > > + } > > + } else { > > + name = np->name; > > + } Gr{oetje,eeting}s, Geert
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;