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

[Jia Wang]

On 2026-05-25 11:28 +0200, Linus Walleij wrote:
> Hi Jia,
> 
> thanks for your patch!
> 
> On Fri, May 15, 2026 at 3:18 AM Jia Wang via B4 Relay
> <devnull+wangjia.ultrarisc.com at kernel.org> wrote:
> 
> > From: Jia Wang <wangjia at ultrarisc.com>
> >
> > Add pinctrl driver for UltraRISC DP1000 pinctrl controller.
> >
> > Signed-off-by: Jia Wang <wangjia at ultrarisc.com>
> 
> Please write an elaborate commit message with some details about
> the hardware and it's history etc.
>

Thanks, will expand the commit message with more hardware details in v2.

> (...)
> 
> > +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"?
> 

The legacy parts will be dropped in v2.

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

Agreed. I've removed both wrappers and switched the driver to use the
generic pinconf DT helpers directly: pinconf_generic_dt_node_to_map_all
for mapping and pinconf_generic_dt_free_map for freeing the maps.

> Yours,
> Linus Walleij
> 

Best regards,
Jia Wang