Re: [PATCH 4/9] pinctrl: sunxi: Deal with configless pins

[Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]

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

Hi,

On Tue, Oct 04, 2016 at 10:28:18AM +0800, Chen-Yu Tsai wrote:
> >         if (sunxi_pctrl_has_drive_prop(node)) {
> >                 int drive = sunxi_pctrl_parse_drive_prop(node);
> > -               if (drive < 0)
> > +               if (drive < 0) {
> > +                       ret = -EINVAL;
> 
> Why not just pass the error code returned from the parse function?

Yep, I'll change that.

> > -               (*map)[i].type = PIN_MAP_TYPE_CONFIGS_GROUP;
> > -               (*map)[i].data.configs.group_or_pin = group;
> > -               (*map)[i].data.configs.configs = pinconfig;
> > -               (*map)[i].data.configs.num_configs = configlen;
> > -
> > -               i++;
> > +               if (pinconfig) {
> > +                       (*map)[i].type = PIN_MAP_TYPE_CONFIGS_GROUP;
> > +                       (*map)[i].data.configs.group_or_pin = group;
> > +                       (*map)[i].data.configs.configs = pinconfig;
> > +                       (*map)[i].data.configs.num_configs = configlen;
> > +                       i++;
> > +               }
> >         }
> >
> > -       *num_maps = nmaps;
> > +       *num_maps = i;
> 
> Thought: should we do a krealloc to shrink the array?

Yep, I'll make an additional patch to fix that.

> 
> >
> >         return 0;
> >
> > @@ -342,8 +357,13 @@ static void sunxi_pctrl_dt_free_map(struct pinctrl_dev *pctldev,
> >                                     struct pinctrl_map *map,
> >                                     unsigned num_maps)
> >  {
> > +       unsigned long *pinconfig;
> > +
> >         /* All the maps have the same pin config, free only the first one */
> > -       kfree(map[0].data.configs.configs);
> > +       pinconfig = map[0].data.configs.configs;
> > +       if (pinconfig)
> > +               kfree(pinconfig);
> 
> Passing NULL to kfree is allowed. (It becomes a no-op.)
> So you could leave this function alone.

And I'll change that as well.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

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