Messages in this thread | | | Date | Fri, 7 Oct 2016 11:02:53 +0200 | From | Maxime Ripard <> | Subject | Re: [PATCH 4/9] pinctrl: sunxi: Deal with configless pins |
| |
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 [unhandled content-type:application/pgp-signature] | |