Re: [PATCH 6/9] pinctrl: ultrarisc: Add UltraRISC DP1000 pinctrl driver

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4685 bytes --]

On Thu, May 28, 2026 at 03:46:25PM +0800, Jia Wang wrote:
> On 2026-05-25 11:10 +0100, Conor Dooley wrote:
> > On Mon, May 25, 2026 at 11:28:51AM +0200, Linus Walleij wrote:
> > > On Fri, May 15, 2026 at 3:18 AM Jia Wang via B4 Relay
> > > <devnull+wangjia.ultrarisc.com@kernel.org> wrote:
> > 
> > > > +static int ur_legacy_parse_prop(struct pinctrl_dev *pctldev,
> > > > +                               struct device_node *np,
> > > > +                               const char *propname,
> > > > +                               struct ur_legacy_prop_data *prop)
> > > > +static const char *ur_legacy_get_function_name(const struct ur_pinctrl_match_data *match_data,
> > > > +                                              u32 mode)
> > > > +static int ur_legacy_conf_to_configs(struct pinctrl_dev *pctldev, u32 conf,
> > > > +                                    unsigned long **configs,
> > > > +                                    unsigned int *num_configs)
> > > > +static int ur_legacy_add_pinconf_maps(struct pinctrl_dev *pctldev,
> > > > +                                     struct pinctrl_map **map,
> > > > +                                     unsigned int *reserved_maps,
> > > > +                                     unsigned int *num_maps,
> > > > +                                     const struct ur_legacy_prop_data *prop)
> > > > +static int ur_legacy_dt_node_to_map(struct pinctrl_dev *pctldev,
> > > > +                                   struct device_node *np,
> > > > +                                   struct pinctrl_map **map,
> > > > +                                   unsigned int *num_maps)
> > > 
> > > What's up with all this legacy stuff?
> > > 
> > > What is this a legacy of?
> > > 
> > > I thought this was a *new* driver so how can it be "legacy"?
> > 
> > Jia already agreed to drop this stuff :)
> > 
> > > 
> > > > +static int ur_generic_dt_node_to_map(struct pinctrl_dev *pctldev,
> > > > +                                    struct device_node *np_config,
> > > > +                                    struct pinctrl_map **map,
> > > > +                                    unsigned int *num_maps)
> > > > +{
> > > > +       return pinconf_generic_dt_node_to_map(pctldev, np_config, map, num_maps,
> > > > +                                             PIN_MAP_TYPE_INVALID);
> > > > +}
> > > 
> > > Hm I think Conor has new helpers for this so you don't need to wrap
> > > it like this.
> > 
> > Yeah, although "more importantly" the new helpers mean that drivers
> > shouldn't need to do things like calling these home-rolled functions in
> > probe, just so that they can try to use pinconf_generic_dt_node_to_map():
> > | static int ur_add_pin_groups(struct ur_pinctrl *pctldata)
> > | {
> > | 	for (u32 i = 0; i < pctldata->match_data->npins; i++) {
> > | 		int ret;
> > | 
> > | 		pctldata->group_names[i] = pctldata->match_data->pins[i].name;
> > | 		pctldata->group_pins[i] = pctldata->match_data->pins[i].number;
> > | 
> > | 		ret = pinctrl_generic_add_group(pctldata->pctl_dev, pctldata->group_names[i],
> > | 						&pctldata->group_pins[i], 1, NULL);
> > | 		if (ret < 0)
> > | 			return dev_err_probe(pctldata->dev, ret,
> > | 					     "failed to add pin group %s\n",
> > | 					     pctldata->group_names[i]);
> > | 	}
> > | 
> > | 	return 0;
> > | }
> > | 
> > | static int ur_add_functions(struct ur_pinctrl *pctldata)
> > | {
> > | 	for (u32 i = 0; i < pctldata->match_data->num_functions; i++) {
> > | 		const struct ur_function_desc *desc = &pctldata->match_data->functions[i];
> > | 		struct pinfunction func = desc->gpio ?
> > | 			PINCTRL_GPIO_PINFUNCTION(desc->name, pctldata->group_names,
> > | 						 pctldata->match_data->npins) :
> > | 			PINCTRL_PINFUNCTION(desc->name, pctldata->group_names,
> > | 					    pctldata->match_data->npins);
> > | 		int ret;
> > | 
> > | 		ret = pinmux_generic_add_pinfunction(pctldata->pctl_dev, &func, (void *)desc);
> > | 		if (ret < 0)
> > | 			return dev_err_probe(pctldata->dev, ret,
> > | 					     "failed to add function %s\n",
> > | 					     desc->name);
> > | 	}
> > | 
> > | 	return 0;
> > | }
> > (If I had more time, I would probably go looking to see if there are
> > more candidates for conversion)
> > 
> > Jia, the helper in question is pinctrl_generic_pins_functions_dt_node_to_map().
> >
> 
> I found pinctrl_generic_pins_function_dt_node_to_map() in mainline, but
> there is no pinctrl_generic_pins_functions_dt_node_to_map() as mentioned.
> Did you mean the singular version, and is that the one I should use?

I did, the s was a typo.

Sorry about that,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]