| Message ID | 20230721095859.280864-1-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [5.10.y-cip] pinctrl: renesas: rzg2l: Handle non-unique subnode names | expand |
Hi! > 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. Looks okay to me, and I can apply it if it passes testing and there are no other comments. I believe we want this for 6.1, too. I can apply it there, too. There was quite a lot of patches recently; I believe we handled all of them, but let me know if we did not. If there's something in 5.10 we don't have in 6.1, we want to fix that, too. Best regards, Pavel
Hi Pavel, Thanks for the feedback. > -----Original Message----- > From: Pavel Machek <pavel@denx.de> > Sent: Saturday, July 22, 2023 12:31 PM > To: Biju Das <biju.das.jz@bp.renesas.com> > Cc: cip-dev@lists.cip-project.org; Nobuhiro Iwamatsu > <nobuhiro1.iwamatsu@toshiba.co.jp>; Pavel Machek <pavel@denx.de>; > Fabrizio Castro <fabrizio.castro.jz@renesas.com> > Subject: Re: [PATCH 5.10.y-cip] pinctrl: renesas: rzg2l: Handle non- > unique subnode names > > Hi! > > > 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. > > Looks okay to me, and I can apply it if it passes testing and there are > no other comments. > > I believe we want this for 6.1, too. I can apply it there, too. For 6.1, It will be taken care by stable, we just need to wait for it to hit 6.1 stable. > > There was quite a lot of patches recently; I believe we handled all of > them, but let me know if we did not. > > If there's something in 5.10 we don't have in 6.1, we want to fix that, > too. Sure. Cheers, Biju
Hi! > > I believe we want this for 6.1, too. I can apply it there, too. > > For 6.1, It will be taken care by stable, we just need to wait for it to hit 6.1 stable. Aha, ok, thanks for explanation. Anyway, I applied the patch. Best regards, Pavel
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c index 6593e2172d0a..af53a816ef85 100644 --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c @@ -235,6 +235,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) @@ -252,6 +253,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); @@ -335,8 +337,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; @@ -346,17 +359,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); @@ -403,7 +415,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); @@ -412,7 +424,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;
commit bfc374a145ae133613e05b9b89be561f169cb58d upstream. 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 <biju.das.jz@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Link: https://lore.kernel.org/r/20230704111858.215278-1-biju.das.jz@bp.renesas.com Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- drivers/pinctrl/renesas/pinctrl-rzg2l.c | 28 ++++++++++++++++++------- 1 file changed, 20 insertions(+), 8 deletions(-)