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

[Conor Dooley]

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@xxxxxxxxxx> 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().

Cheers,
Conor.

Also worth pointing out that this driver has match data but only one
supported device:
+static const struct of_device_id ur_pinctrl_of_match[] = {
+	{ .compatible = "ultrarisc,dp1000-pinctrl", .data = &ur_dp1000_match_data, },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ur_pinctrl_of_match);

Attachment: signature.asc
Description: PGP signature