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

[Linus Walleij]

Hi Jia,

thanks for your patch!

On Fri, May 15, 2026 at 3:18 AM Jia Wang via B4 Relay
<devnull+wangjia.ultrarisc.com@xxxxxxxxxx> wrote:

> From: Jia Wang <wangjia@xxxxxxxxxxxxx>
>
> Add pinctrl driver for UltraRISC DP1000 pinctrl controller.
>
> Signed-off-by: Jia Wang <wangjia@xxxxxxxxxxxxx>

Please write an elaborate commit message with some details about
the hardware and it's history etc.

(...)

> +struct ur_legacy_prop_data {
> +       struct ur_pin_val *pin_vals;
> +       unsigned int *group_pins;
> +       unsigned int num_pins;
> +};

> +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"?

> +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.

> +static void ur_dt_free_map(struct pinctrl_dev *pctldev,
> +                          struct pinctrl_map *map,
> +                          unsigned int num_maps)
> +{
> +       pinctrl_utils_free_map(pctldev, map, num_maps);
> +}

Can't you just assign pinctrl_utils_free_map directly to ur_pinctrl_ops?

Yours,
Linus Walleij