Hi,

On Tue, Oct 04, 2016 at 10:28:18AM +0800, Chen-Yu Tsai wrote:

quoted

        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.

quoted

-               (*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.

quoted

        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