[PATCH 1/2] pinctrl: utils : add support to pass config type in generic util APIs

[PATCH 1/2] pinctrl: utils : add support to pass config type in generic util APIs

[Laxman Dewangan]

Add support to pass the config type like GROUP or PIN when using
the utils or generic pin configuration APIs. This will make the
APIs more generic.

Added additional inline APIs such that it can be use directly as
callback for the pinctrl_ops.

Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/pinctrl/pinconf-generic.c       |   12 +++++++-----
 drivers/pinctrl/pinctrl-palmas.c        |    4 ++--
 drivers/pinctrl/pinctrl-utils.c         |    9 +++++----
 drivers/pinctrl/pinctrl-utils.h         |   23 +++++++++++++++++++++--
 include/linux/pinctrl/pinconf-generic.h |   22 ++++++++++++++++++++--
 5 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index d9536ca..a278c32 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -240,7 +240,8 @@ out:
 
 int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 		struct device_node *np, struct pinctrl_map **map,
-		unsigned *reserved_maps, unsigned *num_maps)
+		unsigned *reserved_maps, unsigned *num_maps,
+		enum pinctrl_map_type type)
 {
 	int ret;
 	const char *function;
@@ -295,7 +296,7 @@ int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 		if (num_configs) {
 			ret = pinctrl_utils_add_map_configs(pctldev, map,
 					reserved_maps, num_maps, group, configs,
-					num_configs, PIN_MAP_TYPE_CONFIGS_PIN);
+					num_configs, type);
 			if (ret < 0)
 				goto exit;
 		}
@@ -310,7 +311,7 @@ EXPORT_SYMBOL_GPL(pinconf_generic_dt_subnode_to_map);
 
 int pinconf_generic_dt_node_to_map(struct pinctrl_dev *pctldev,
 		struct device_node *np_config, struct pinctrl_map **map,
-		unsigned *num_maps)
+		unsigned *num_maps, enum pinctrl_map_type type)
 {
 	unsigned reserved_maps;
 	struct device_node *np;
@@ -322,9 +323,10 @@ int pinconf_generic_dt_node_to_map(struct pinctrl_dev *pctldev,
 
 	for_each_child_of_node(np_config, np) {
 		ret = pinconf_generic_dt_subnode_to_map(pctldev, np, map,
-						&reserved_maps, num_maps);
+					&reserved_maps, num_maps, type);
 		if (ret < 0) {
-			pinctrl_utils_dt_free_map(pctldev, *map, *num_maps);
+			pinctrl_utils_dt_free_map_config(pctldev, *map,
+				*num_maps, type);
 			return ret;
 		}
 	}
diff --git a/drivers/pinctrl/pinctrl-palmas.c b/drivers/pinctrl/pinctrl-palmas.c
index 2697c2e..599c0c7 100644
--- a/drivers/pinctrl/pinctrl-palmas.c
+++ b/drivers/pinctrl/pinctrl-palmas.c
@@ -655,8 +655,8 @@ static const struct pinctrl_ops palmas_pinctrl_ops = {
 	.get_groups_count = palmas_pinctrl_get_groups_count,
 	.get_group_name = palmas_pinctrl_get_group_name,
 	.get_group_pins = palmas_pinctrl_get_group_pins,
-	.dt_node_to_map = pinconf_generic_dt_node_to_map,
-	.dt_free_map = pinctrl_utils_dt_free_map,
+	.dt_node_to_map = pinconf_generic_dt_node_to_map_pin,
+	.dt_free_map = pinctrl_utils_dt_free_map_configs_pin,
 };
 
 static int palmas_pinctrl_get_funcs_count(struct pinctrl_dev *pctldev)
diff --git a/drivers/pinctrl/pinctrl-utils.c b/drivers/pinctrl/pinctrl-utils.c
index b7ac646..102e2d5 100644
--- a/drivers/pinctrl/pinctrl-utils.c
+++ b/drivers/pinctrl/pinctrl-utils.c
@@ -121,15 +121,16 @@ int pinctrl_utils_add_config(struct pinctrl_dev *pctldev,
 }
 EXPORT_SYMBOL_GPL(pinctrl_utils_add_config);
 
-void pinctrl_utils_dt_free_map(struct pinctrl_dev *pctldev,
-	      struct pinctrl_map *map, unsigned num_maps)
+void pinctrl_utils_dt_free_map_config(struct pinctrl_dev *pctldev,
+		struct pinctrl_map *map, unsigned num_maps,
+		enum pinctrl_map_type type)
 {
 	int i;
 
 	for (i = 0; i < num_maps; i++)
-		if (map[i].type == PIN_MAP_TYPE_CONFIGS_GROUP)
+		if (map[i].type == type)
 			kfree(map[i].data.configs.configs);
 
 	kfree(map);
 }
-EXPORT_SYMBOL_GPL(pinctrl_utils_dt_free_map);
+EXPORT_SYMBOL_GPL(pinctrl_utils_dt_free_map_config);
diff --git a/drivers/pinctrl/pinctrl-utils.h b/drivers/pinctrl/pinctrl-utils.h
index d0ffe1c..6ca6ea1 100644
--- a/drivers/pinctrl/pinctrl-utils.h
+++ b/drivers/pinctrl/pinctrl-utils.h
@@ -22,6 +22,8 @@
 #ifndef __PINCTRL_UTILS_H__
 #define __PINCTRL_UTILS_H__
 
+#include <linux/pinctrl/machine.h>
+
 int pinctrl_utils_reserve_map(struct pinctrl_dev *pctldev,
 		struct pinctrl_map **map, unsigned *reserved_maps,
 		unsigned *num_maps, unsigned reserve);
@@ -37,7 +39,24 @@ int pinctrl_utils_add_map_configs(struct pinctrl_dev *pctldev,
 int pinctrl_utils_add_config(struct pinctrl_dev *pctldev,
 		unsigned long **configs, unsigned *num_configs,
 		unsigned long config);
-void pinctrl_utils_dt_free_map(struct pinctrl_dev *pctldev,
-		struct pinctrl_map *map, unsigned num_maps);
+void pinctrl_utils_dt_free_map_config(struct pinctrl_dev *pctldev,
+		struct pinctrl_map *map, unsigned num_maps,
+		enum pinctrl_map_type type);
+
+static inline void pinctrl_utils_dt_free_map_configs_group(
+	struct pinctrl_dev *pctldev,
+	struct pinctrl_map *map, unsigned num_maps)
+{
+	pinctrl_utils_dt_free_map_config(pctldev, map, num_maps,
+		PIN_MAP_TYPE_CONFIGS_GROUP);
+}
+
+static inline void pinctrl_utils_dt_free_map_configs_pin(
+	struct pinctrl_dev *pctldev,
+	struct pinctrl_map *map, unsigned num_maps)
+{
+	pinctrl_utils_dt_free_map_config(pctldev, map, num_maps,
+		PIN_MAP_TYPE_CONFIGS_PIN);
+}
 
 #endif /* __PINCTRL_UTILS_H__ */
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index 83f5179..fb90ef5 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -140,15 +140,33 @@ static inline unsigned long pinconf_to_config_packed(enum pin_config_param param
 #ifdef CONFIG_OF
 
 #include <linux/device.h>
+#include <linux/pinctrl/machine.h>
 struct pinctrl_dev;
 struct pinctrl_map;
 
 int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 		struct device_node *np, struct pinctrl_map **map,
-		unsigned *reserved_maps, unsigned *num_maps);
+		unsigned *reserved_maps, unsigned *num_maps,
+		enum pinctrl_map_type type);
 int pinconf_generic_dt_node_to_map(struct pinctrl_dev *pctldev,
 		struct device_node *np_config, struct pinctrl_map **map,
-		unsigned *num_maps);
+		unsigned *num_maps, enum pinctrl_map_type type);
+
+static inline int pinconf_generic_dt_node_to_map_group(
+		struct pinctrl_dev *pctldev, struct device_node *np_config,
+		struct pinctrl_map **map, unsigned *num_maps)
+{
+	return pinconf_generic_dt_node_to_map(pctldev, np_config, map, num_maps,
+			PIN_MAP_TYPE_CONFIGS_GROUP);
+}
+
+static inline int pinconf_generic_dt_node_to_map_pin(
+		struct pinctrl_dev *pctldev, struct device_node *np_config,
+		struct pinctrl_map **map, unsigned *num_maps)
+{
+	return pinconf_generic_dt_node_to_map(pctldev, np_config, map, num_maps,
+			PIN_MAP_TYPE_CONFIGS_PIN);
+}
 
 #endif
 
-- 
1.7.1.1

[PATCH 2/2] pinctrl: tegra: use pinctrl-utils APIs for mapping

[Laxman Dewangan]

Pin control utility functions provides the function for creating
map lists.

In place of implementing APIs locally in Tegra pin control driver
for creating map lists, use the utility functions. This reduces
the code size and avoid duplication.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/pinctrl/pinctrl-tegra.c |  130 ++++++---------------------------------
 1 files changed, 18 insertions(+), 112 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-tegra.c b/drivers/pinctrl/pinctrl-tegra.c
index 2fa9bc6..b7ceef1 100644
--- a/drivers/pinctrl/pinctrl-tegra.c
+++ b/drivers/pinctrl/pinctrl-tegra.c
@@ -32,6 +32,7 @@
 
 #include "core.h"
 #include "pinctrl-tegra.h"
+#include "pinctrl-utils.h"
 
 struct tegra_pmx {
 	struct device *dev;
@@ -90,107 +91,6 @@ static void tegra_pinctrl_pin_dbg_show(struct pinctrl_dev *pctldev,
 }
 #endif
 
-static int reserve_map(struct device *dev, struct pinctrl_map **map,
-		       unsigned *reserved_maps, unsigned *num_maps,
-		       unsigned reserve)
-{
-	unsigned old_num = *reserved_maps;
-	unsigned new_num = *num_maps + reserve;
-	struct pinctrl_map *new_map;
-
-	if (old_num >= new_num)
-		return 0;
-
-	new_map = krealloc(*map, sizeof(*new_map) * new_num, GFP_KERNEL);
-	if (!new_map) {
-		dev_err(dev, "krealloc(map) failed\n");
-		return -ENOMEM;
-	}
-
-	memset(new_map + old_num, 0, (new_num - old_num) * sizeof(*new_map));
-
-	*map = new_map;
-	*reserved_maps = new_num;
-
-	return 0;
-}
-
-static int add_map_mux(struct pinctrl_map **map, unsigned *reserved_maps,
-		       unsigned *num_maps, const char *group,
-		       const char *function)
-{
-	if (WARN_ON(*num_maps == *reserved_maps))
-		return -ENOSPC;
-
-	(*map)[*num_maps].type = PIN_MAP_TYPE_MUX_GROUP;
-	(*map)[*num_maps].data.mux.group = group;
-	(*map)[*num_maps].data.mux.function = function;
-	(*num_maps)++;
-
-	return 0;
-}
-
-static int add_map_configs(struct device *dev, struct pinctrl_map **map,
-			   unsigned *reserved_maps, unsigned *num_maps,
-			   const char *group, unsigned long *configs,
-			   unsigned num_configs)
-{
-	unsigned long *dup_configs;
-
-	if (WARN_ON(*num_maps == *reserved_maps))
-		return -ENOSPC;
-
-	dup_configs = kmemdup(configs, num_configs * sizeof(*dup_configs),
-			      GFP_KERNEL);
-	if (!dup_configs) {
-		dev_err(dev, "kmemdup(configs) failed\n");
-		return -ENOMEM;
-	}
-
-	(*map)[*num_maps].type = PIN_MAP_TYPE_CONFIGS_GROUP;
-	(*map)[*num_maps].data.configs.group_or_pin = group;
-	(*map)[*num_maps].data.configs.configs = dup_configs;
-	(*map)[*num_maps].data.configs.num_configs = num_configs;
-	(*num_maps)++;
-
-	return 0;
-}
-
-static int add_config(struct device *dev, unsigned long **configs,
-		      unsigned *num_configs, unsigned long config)
-{
-	unsigned old_num = *num_configs;
-	unsigned new_num = old_num + 1;
-	unsigned long *new_configs;
-
-	new_configs = krealloc(*configs, sizeof(*new_configs) * new_num,
-			       GFP_KERNEL);
-	if (!new_configs) {
-		dev_err(dev, "krealloc(configs) failed\n");
-		return -ENOMEM;
-	}
-
-	new_configs[old_num] = config;
-
-	*configs = new_configs;
-	*num_configs = new_num;
-
-	return 0;
-}
-
-static void tegra_pinctrl_dt_free_map(struct pinctrl_dev *pctldev,
-				      struct pinctrl_map *map,
-				      unsigned num_maps)
-{
-	int i;
-
-	for (i = 0; i < num_maps; i++)
-		if (map[i].type == PIN_MAP_TYPE_CONFIGS_GROUP)
-			kfree(map[i].data.configs.configs);
-
-	kfree(map);
-}
-
 static const struct cfg_param {
 	const char *property;
 	enum tegra_pinconf_param param;
@@ -212,12 +112,13 @@ static const struct cfg_param {
 	{"nvidia,drive-type",		TEGRA_PINCONF_PARAM_DRIVE_TYPE},
 };
 
-static int tegra_pinctrl_dt_subnode_to_map(struct device *dev,
+static int tegra_pinctrl_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 					   struct device_node *np,
 					   struct pinctrl_map **map,
 					   unsigned *reserved_maps,
 					   unsigned *num_maps)
 {
+	struct device *dev = pctldev->dev;
 	int ret, i;
 	const char *function;
 	u32 val;
@@ -241,7 +142,8 @@ static int tegra_pinctrl_dt_subnode_to_map(struct device *dev,
 		ret = of_property_read_u32(np, cfg_params[i].property, &val);
 		if (!ret) {
 			config = TEGRA_PINCONF_PACK(cfg_params[i].param, val);
-			ret = add_config(dev, &configs, &num_configs, config);
+			ret = pinctrl_utils_add_config(pctldev, &configs,
+					&num_configs, config);
 			if (ret < 0)
 				goto exit;
 		/* EINVAL=missing, which is fine since it's optional */
@@ -263,22 +165,25 @@ static int tegra_pinctrl_dt_subnode_to_map(struct device *dev,
 	}
 	reserve *= ret;
 
-	ret = reserve_map(dev, map, reserved_maps, num_maps, reserve);
+	ret = pinctrl_utils_reserve_map(pctldev, map, reserved_maps,
+					num_maps, reserve);
 	if (ret < 0)
 		goto exit;
 
 	of_property_for_each_string(np, "nvidia,pins", prop, group) {
 		if (function) {
-			ret = add_map_mux(map, reserved_maps, num_maps,
-					  group, function);
+			ret = pinctrl_utils_add_map_mux(pctldev, map,
+					reserved_maps, num_maps, group,
+					function);
 			if (ret < 0)
 				goto exit;
 		}
 
 		if (num_configs) {
-			ret = add_map_configs(dev, map, reserved_maps,
-					      num_maps, group, configs,
-					      num_configs);
+			ret = pinctrl_utils_add_map_configs(pctldev, map,
+					reserved_maps, num_maps, group,
+					configs, num_configs,
+					PIN_MAP_TYPE_CONFIGS_GROUP);
 			if (ret < 0)
 				goto exit;
 		}
@@ -305,10 +210,11 @@ static int tegra_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
 	*num_maps = 0;
 
 	for_each_child_of_node(np_config, np) {
-		ret = tegra_pinctrl_dt_subnode_to_map(pctldev->dev, np, map,
+		ret = tegra_pinctrl_dt_subnode_to_map(pctldev, np, map,
 						      &reserved_maps, num_maps);
 		if (ret < 0) {
-			tegra_pinctrl_dt_free_map(pctldev, *map, *num_maps);
+			pinctrl_utils_dt_free_map_configs_group(pctldev, *map,
+				*num_maps);
 			return ret;
 		}
 	}
@@ -324,7 +230,7 @@ static const struct pinctrl_ops tegra_pinctrl_ops = {
 	.pin_dbg_show = tegra_pinctrl_pin_dbg_show,
 #endif
 	.dt_node_to_map = tegra_pinctrl_dt_node_to_map,
-	.dt_free_map = tegra_pinctrl_dt_free_map,
+	.dt_free_map = pinctrl_utils_dt_free_map_configs_group,
 };
 
 static int tegra_pinctrl_get_funcs_count(struct pinctrl_dev *pctldev)
-- 
1.7.1.1

Re: [PATCH 1/2] pinctrl: utils : add support to pass config type in generic util APIs

[Stephen Warren]

On 08/20/2013 06:12 AM, Laxman Dewangan wrote:
> Add support to pass the config type like GROUP or PIN when using
> the utils or generic pin configuration APIs. This will make the
> APIs more generic.

I think passing in the type parameter to the *_to_map APIs makes sense,
but I don't think you need to have separate free functions; the way to
free any kind of map entry is always known, so you don't have to limit
the code to only freeing one type.

> diff --git a/drivers/pinctrl/pinctrl-utils.c b/drivers/pinctrl/pinctrl-utils.c

> -void pinctrl_utils_dt_free_map(struct pinctrl_dev *pctldev,
> -	      struct pinctrl_map *map, unsigned num_maps)
> +void pinctrl_utils_dt_free_map_config(struct pinctrl_dev *pctldev,
> +		struct pinctrl_map *map, unsigned num_maps,
> +		enum pinctrl_map_type type)
>  {
>  	int i;
>  
>  	for (i = 0; i < num_maps; i++)
> -		if (map[i].type == PIN_MAP_TYPE_CONFIGS_GROUP)
> +		if (map[i].type == type)
>  			kfree(map[i].data.configs.configs);

In other words, replace that if with:

switch (maps[i].type) {
case PIN_MAP_TYPE_CONFIGS_PIN:
case PIN_MAP_TYPE_CONFIGS_GROUP:
	kfree(map[i].data.configs.configs);
	break;
default:
	break;
}

Re: [PATCH 2/2] pinctrl: tegra: use pinctrl-utils APIs for mapping

[Stephen Warren]

On 08/20/2013 06:12 AM, Laxman Dewangan wrote:
> Pin control utility functions provides the function for creating
> map lists.
> 
> In place of implementing APIs locally in Tegra pin control driver
> for creating map lists, use the utility functions. This reduces
> the code size and avoid duplication.

This patch looks fine, so assuming the issue in patch 1/2 is fixed, this
patch,
Acked-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

One thought though:

> diff --git a/drivers/pinctrl/pinctrl-tegra.c b/drivers/pinctrl/pinctrl-tegra.c

>  static const struct cfg_param {
>  	const char *property;
>  	enum tegra_pinconf_param param;
> @@ -212,12 +112,13 @@ static const struct cfg_param {
>  	{"nvidia,drive-type",		TEGRA_PINCONF_PARAM_DRIVE_TYPE},
>  };

If you add a new field to that struct that's a function pointer, which
parses the content of the property and gives back an unsigned log or
struct pinctrl_map, you could probably make most of
tegra_pinctrl_dt_subnode_to_map() be generic too...

But, that's certainly something for a separate patch if you want.