[PATCH 0/2] pinctrl: ABX500: Add device tree support

[PATCH 0/2] pinctrl: ABX500: Add device tree support

[patrice.chotard.st]

From: Patrice Chotard <patrice.chotard@st.com>

This patchset replaces previously submitted patch regarding device tree
support for ABX500 Asic family. It now uses pin configuration generic 
parsing code proposed by Heiko Stuebner.

Patrice Chotard (2):
  pinctrl: abx500: Add device tree support
  pinctrl: abx500: fix abx500_pin_config_set()

 .../devicetree/bindings/pinctrl/ste,abx500.txt     |  352 ++++++++++++++++++++
 drivers/pinctrl/pinctrl-abx500.c                   |  215 +++++++++++-
 2 files changed, 561 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ste,abx500.txt

-- 
1.7.10

[PATCH 1/2] pinctrl: abx500: Add device tree support

[patrice.chotard.st]

From: Patrice Chotard <patrice.chotard@st.com>

We use the same way to define pin muxing and pin configuration 
than for nomadik. So pickup code from pinctrl_nomadik.c to be 
able to implement pin multiplexing and pin configuration using
the device tree. Pin configuration uses generic parsing code.

Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---
 .../devicetree/bindings/pinctrl/ste,abx500.txt     |  352 ++++++++++++++++++++
 drivers/pinctrl/pinctrl-abx500.c                   |  184 ++++++++++
 2 files changed, 536 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ste,abx500.txt

diff --git a/Documentation/devicetree/bindings/pinctrl/ste,abx500.txt b/Documentation/devicetree/bindings/pinctrl/ste,abx500.txt
new file mode 100644
index 0000000..e74a72d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/ste,abx500.txt
@@ -0,0 +1,352 @@
+ST Ericsson abx500 pinmux controller
+
+Required properties:
+- compatible: "stericsson,ab8500-gpio",  "stericsson,ab8540-gpio",
+	      "stericsson,ab8505-gpio", "stericsson,ab9540-gpio",
+
+Please refer to pinctrl-bindings.txt in this directory for details of the
+common pinctrl bindings used by client devices, including the meaning of the
+phrase "pin configuration node".
+
+ST Ericsson's pin configuration nodes act as a container for an arbitrary number of
+subnodes. Each of these subnodes represents some desired configuration for a
+pin, a group, or a list of pins or groups. This configuration can include the
+mux function to select on those pin(s)/group(s), and various pin configuration
+parameters, such as input, output, pull up, pull down...
+
+The name of each subnode is not important; all subnodes should be enumerated
+and processed purely based on their content.
+
+Required subnode-properties:
+- ste,pins : An array of strings. Each string contains the name of a pin or
+    group.
+
+Optional subnode-properties:
+- ste,function: A string containing the name of the function to mux to the
+  pin or group.
+
+- generic pin configuration option to use. Example :
+
+	default_cfg {
+		ste,pins = "GPIO1";
+		bias-disable;
+	};
+
+- ste,config: Handle of pin configuration node containing the generic
+  pinconfig options to use, as described in pinctrl-bindings.txt in
+  this directory. Example :
+
+	pcfg_bias_disable: pcfg_bias_disable {
+		bias-disable;
+	};
+
+	default_cfg {
+		ste,pins = "GPIO1";
+		ste.config = <&pcfg_bias_disable>;
+	};
+
+Example board file extract:
+
+&pinctrl_abx500 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&sysclkreq2_default_mode>, <&sysclkreq3_default_mode>, <&gpio3_default_mode>, <&sysclkreq6_default_mode>, <&pwmout1_default_mode>, <&pwmout2_default_mode>, <&pwmout3_default_mode>, <&adi1_default_mode>, <&dmic12_default_mode>, <&dmic34_default_mode>, <&dmic56_default_mode>, <&sysclkreq5_default_mode>, <&batremn_default_mode>, <&service_default_mode>, <&pwrctrl0_default_mode>, <&pwrctrl1_default_mode>, <&pwmextvibra1_default_mode>, <&pwmextvibra2_default_mode>, <&gpio51_default_mode>, <&gpio52_default_mode>, <&gpio53_default_mode>, <&gpio54_default_mode>, <&pdmclkdat_default_mode>;
+
+	sysclkreq2 {
+		sysclkreq2_default_mode: sysclkreq2_default {
+			default_mux {
+				ste,function = "sysclkreq";
+				ste,pins = "sysclkreq2_d_1";
+			};
+			default_cfg {
+				ste,pins = "GPIO1";
+				bias-disable;
+			};
+		};
+	};
+	sysclkreq3 {
+		sysclkreq3_default_mode: sysclkreq3_default {
+			default_mux {
+				ste,function = "sysclkreq";
+				ste,pins = "sysclkreq3_d_1";
+			};
+			default_cfg {
+				ste,pins = "GPIO2";
+				output-low;
+			};
+		};
+	};
+	gpio3 {
+		gpio3_default_mode: gpio3_default {
+			default_mux {
+				ste,function = "gpio";
+				ste,pins = "gpio3_a_1";
+			};
+			default_cfg {
+				ste,pins = "GPIO3";
+				output-low;
+			};
+		};
+	};
+	sysclkreq6 {
+		sysclkreq6_default_mode: sysclkreq6_default {
+			default_mux {
+				ste,function = "sysclkreq";
+				ste,pins = "sysclkreq6_d_1";
+			};
+			default_cfg {
+				ste,pins = "GPIO4";
+				bias-disable;
+			};
+		};
+	};
+	pwmout1 {
+		pwmout1_default_mode: pwmout1_default {
+			default_mux {
+				ste,function = "pwmout";
+				ste,pins = "pwmout1_d_1";
+			};
+			default_cfg {
+				ste,pins = "GPIO14";
+				output-low;
+			};
+		};
+	};
+	pwmout2 {
+		pwmout2_default_mode: pwmout2_default {
+			pwmout2_default_mux {
+				ste,function = "pwmout";
+				ste,pins = "pwmout2_d_1";
+			};
+			pwmout2_default_cfg {
+				ste,pins = "GPIO15";
+				output-low;
+			};
+		};
+	};
+	pwmout3 {
+		pwmout3_default_mode: pwmout3_default {
+			pwmout3_default_mux {
+				ste,function = "pwmout";
+				ste,pins = "pwmout3_d_1";
+			};
+			pwmout3_default_cfg {
+				ste,pins = "GPIO16";
+				output-low;
+			};
+		};
+	};
+	adi1 {{
+
+		adi1_default_mode: adi1_default {
+			adi1_default_mux {
+				ste,function = "adi1";
+				ste,pins = "adi1_d_1";
+			};
+			adi1_default_cfg1 {
+				ste,pins = "GPIO17","GPIO19","GPIO20";
+				bias-disable;
+			};
+			adi1_default_cfg2 {
+				ste,pins = "GPIO18";
+				output-low;
+			};
+		};
+	};
+	dmic12 {
+		dmic12_default_mode: dmic12_default {
+			dmic12_default_mux {
+				ste,function = "dmic";
+				ste,pins = "dmic12_d_1";
+			};
+			dmic12_default_cfg1 {
+				ste,pins = "GPIO27";
+				output-low;
+			};
+			dmic12_default_cfg2 {
+				ste,pins = "GPIO28";
+				bias-disable;
+			};
+		};
+	};
+	dmic34 {
+		dmic34_default_mode: dmic34_default {
+			dmic34_default_mux {
+				ste,function = "dmic";
+				ste,pins = "dmic34_d_1";
+			};
+			dmic34_default_cfg1 {
+				ste,pins = "GPIO29";
+				output-low;
+			};
+			dmic34_default_cfg2 {
+				ste,pins = "GPIO30";
+				bias-disable;{
+
+			};
+		};
+	};
+	dmic56 {
+		dmic56_default_mode: dmic56_default {
+			dmic56_default_mux {
+				ste,function = "dmic";
+				ste,pins = "dmic56_d_1";
+			};
+			dmic56_default_cfg1 {
+				ste,pins = "GPIO31";
+				output-low;
+			};
+			dmic56_default_cfg2 {
+				ste,pins = "GPIO32";
+				bias-disable;
+			};
+		};
+	};
+	sysclkreq5 {
+		sysclkreq5_default_mode: sysclkreq5_default {
+			sysclkreq5_default_mux {
+				ste,function = "sysclkreq";
+				ste,pins = "sysclkreq5_d_1";
+			};
+			sysclkreq5_default_cfg {
+				ste,pins = "GPIO42";
+				output-low;
+			};
+		};
+	};
+	batremn {
+		batremn_default_mode: batremn_default {
+			batremn_default_mux {
+				ste,function = "batremn";
+				ste,pins = "batremn_d_1";
+			};
+			batremn_default_cfg {
+				ste,pins = "GPIO43";
+				bias-disable;
+			};
+		};
+	};
+	service {
+		service_default_mode: service_default {
+			service_default_mux {
+				ste,function = "service";
+				ste,pins = "service_d_1";
+			};
+			service_default_cfg {
+				ste,pins = "GPIO44";
+				bias-disable;
+			};
+		};
+	};
+	pwrctrl0 {
+		pwrctrl0_default_mux: pwrctrl0_mux {
+			pwrctrl0_default_mux {
+				ste,function = "pwrctrl";
+				ste,pins = "pwrctrl0_d_1";
+			};
+		};
+		pwrctrl0_default_mode: pwrctrl0_default {
+			pwrctrl0_default_cfg {
+				ste,pins = "GPIO45";
+				bias-disable;
+			};
+		};
+	};
+	pwrctrl1 {
+		pwrctrl1_default_mux: pwrctrl1_mux {
+			pwrctrl1_default_mux {
+				ste,function = "pwrctrl";
+				ste,pins = "pwrctrl1_d_1";
+			};
+		};
+		pwrctrl1_default_mode: pwrctrl1_default {
+			pwrctrl1_default_cfg {
+				ste,pins = "GPIO46";
+				bias-disable;
+			};
+		};
+	};
+	pwmextvibra1 {
+		pwmextvibra1_default_mode: pwmextvibra1_default {
+			pwmextvibra1_default_mux {
+				ste,function = "pwmextvibra";
+				ste,pins = "pwmextvibra1_d_1";
+			};
+			pwmextvibra1_default_cfg {
+				ste,pins = "GPIO47";
+				bias-disable;
+			};
+		};
+	};
+	pwmextvibra2 {
+		pwmextvibra2_default_mode: pwmextvibra2_default {
+			pwmextvibra2_default_mux {
+				ste,function = "pwmextvibra";
+				ste,pins = "pwmextvibra2_d_1";
+			};
+			pwmextvibra1_default_cfg {
+				ste,pins = "GPIO48";
+				bias-disable;
+			};
+		};
+	};
+	gpio51 {
+		gpio51_default_mode: gpio51_default {
+				gpio51_default_mux {
+				ste,function = "gpio";
+				ste,pins = "gpio51_a_1";
+			};
+			gpio51_default_cfg {
+				ste,pins = "GPIO51";
+				output-low;
+			};
+		};
+	};
+	gpio52 {
+		gpio52_default_mode: gpio52_default {
+			gpio52_default_mux {
+				ste,function = "gpio";
+				ste,pins = "gpio52_a_1";
+			};
+			gpio52_default_cfg {
+				ste,pins = "GPIO52";
+				bias-pull-down;
+			};
+		};
+	};
+	gpio53 {
+		gpio53_default_mode: gpio53_default {
+			gpio53_default_mux {
+				ste,function = "gpio";
+				ste,pins = "gpio53_a_1";
+			};
+			gpio53_default_cfg {
+				ste,pins = "GPIO53";
+				bias-pull-down;
+		};
+	};
+	};
+	gpio54 {
+		gpio54_default_mode: gpio54_default {
+			gpio54_default_mux {
+				ste,function = "gpio";
+				ste,pins = "gpio54_a_1";
+			};
+			gpio54_default_cfg {
+				ste,pins = "GPIO54";
+				output-low;
+			};
+		};
+	};
+	pdmclkdat {
+		pdmclkdat_default_mode: pdmclkdat_default {
+			pdmclkdat_default_mux {
+				ste,function = "pdm";
+				ste,pins = "pdmclkdat_d_1";
+			};
+			pdmclkdat_default_cfg {
+				ste,pins = "GPIO55", "GPIO56";
+				bias-disable;
+			};
+		};
+	};
+};
diff --git a/drivers/pinctrl/pinctrl-abx500.c b/drivers/pinctrl/pinctrl-abx500.c
index 84b40f7..b5b5460 100644
--- a/drivers/pinctrl/pinctrl-abx500.c
+++ b/drivers/pinctrl/pinctrl-abx500.c
@@ -30,8 +30,10 @@
 #include <linux/pinctrl/pinmux.h>
 #include <linux/pinctrl/pinconf.h>
 #include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/machine.h>
 
 #include "pinctrl-abx500.h"
+#include "pinconf.h"
 
 /*
  * The AB9540 and AB8540 GPIO support are extended versions
@@ -757,11 +759,193 @@ static void abx500_pin_dbg_show(struct pinctrl_dev *pctldev,
 				 chip->base + offset - 1);
 }
 
+static void abx500_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_PIN)
+			kfree(map[i].data.configs.configs);
+	kfree(map);
+}
+
+static int abx500_dt_reserve_map(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)
+		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 abx500_dt_add_map_mux(struct pinctrl_map **map,
+		unsigned *reserved_maps,
+		unsigned *num_maps, const char *group,
+		const char *function)
+{
+	if (*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 abx500_dt_add_map_configs(struct pinctrl_map **map,
+		unsigned *reserved_maps,
+		unsigned *num_maps, const char *group,
+		unsigned long *configs, unsigned num_configs)
+{
+	unsigned long *dup_configs;
+
+	if (*num_maps == *reserved_maps)
+		return -ENOSPC;
+
+	dup_configs = kmemdup(configs, num_configs * sizeof(*dup_configs),
+			      GFP_KERNEL);
+	if (!dup_configs)
+		return -ENOMEM;
+
+	(*map)[*num_maps].type = PIN_MAP_TYPE_CONFIGS_PIN;
+
+	(*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 const char *abx500_find_pin_name(struct pinctrl_dev *pctldev,
+					const char *pin_name)
+{
+	int i, pin_number;
+	struct abx500_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
+
+	if (sscanf((char *)pin_name, "GPIO%d", &pin_number) == 1)
+		for (i = 0; i < npct->soc->npins; i++)
+			if (npct->soc->pins[i].number == pin_number)
+				return npct->soc->pins[i].name;
+	return NULL;
+}
+
+int abx500_dt_subnode_to_map(struct pinctrl_dev *pctldev,
+		struct device_node *np,
+		struct pinctrl_map **map,
+		unsigned *reserved_maps,
+		unsigned *num_maps)
+{
+	int ret;
+	const char *function = NULL;
+	unsigned long *configs;
+	unsigned int nconfigs = 0;
+	bool has_config = 0;
+	unsigned reserve = 0;
+	struct property *prop;
+	const char *group, *gpio_name;
+	struct device_node *np_config;
+
+	ret = of_property_read_string(np, "ste,function", &function);
+	if (ret >= 0)
+		reserve = 1;
+
+	ret = pinconf_generic_parse_dt_config(np, &configs, &nconfigs);
+	if (nconfigs)
+		has_config = 1;
+
+	np_config = of_parse_phandle(np, "ste,config", 0);
+	if (np_config) {
+		ret = pinconf_generic_parse_dt_config(np_config, &configs,
+				&nconfigs);
+		if (ret)
+			goto exit;
+		has_config |= nconfigs;
+	}
+
+	ret = of_property_count_strings(np, "ste,pins");
+	if (ret < 0)
+		goto exit;
+
+	if (has_config)
+		reserve++;
+
+	reserve *= ret;
+
+	ret = abx500_dt_reserve_map(map, reserved_maps, num_maps, reserve);
+	if (ret < 0)
+		goto exit;
+
+	of_property_for_each_string(np, "ste,pins", prop, group) {
+		if (function) {
+			ret = abx500_dt_add_map_mux(map, reserved_maps,
+					num_maps, group, function);
+			if (ret < 0)
+				goto exit;
+		}
+		if (has_config) {
+			gpio_name = abx500_find_pin_name(pctldev, group);
+
+			ret = abx500_dt_add_map_configs(map, reserved_maps,
+					num_maps, gpio_name, configs, 1);
+			if (ret < 0)
+				goto exit;
+		}
+
+	}
+exit:
+	return ret;
+}
+
+int abx500_dt_node_to_map(struct pinctrl_dev *pctldev,
+				 struct device_node *np_config,
+				 struct pinctrl_map **map, unsigned *num_maps)
+{
+	unsigned reserved_maps;
+	struct device_node *np;
+	int ret;
+
+	reserved_maps = 0;
+	*map = NULL;
+	*num_maps = 0;
+
+	for_each_child_of_node(np_config, np) {
+		ret = abx500_dt_subnode_to_map(pctldev, np, map,
+				&reserved_maps, num_maps);
+		if (ret < 0) {
+			abx500_dt_free_map(pctldev, *map, *num_maps);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 static const struct pinctrl_ops abx500_pinctrl_ops = {
 	.get_groups_count = abx500_get_groups_cnt,
 	.get_group_name = abx500_get_group_name,
 	.get_group_pins = abx500_get_group_pins,
 	.pin_dbg_show = abx500_pin_dbg_show,
+	.dt_node_to_map = abx500_dt_node_to_map,
+	.dt_free_map = abx500_dt_free_map,
 };
 
 static int abx500_pin_config_get(struct pinctrl_dev *pctldev,
-- 
1.7.10

Re: [PATCH 1/2] pinctrl: abx500: Add device tree support

[Fabio Baltieri]

Hi Patrice,

this looks good, just a couple of minor notes, check below...

On Thu, Jun 20, 2013 at 09:23:21AM +0200, patrice.chotard.st@gmail.com wrote:
> From: Patrice Chotard <patrice.chotard@st.com>
> 
> We use the same way to define pin muxing and pin configuration 
> than for nomadik. So pickup code from pinctrl_nomadik.c to be 
> able to implement pin multiplexing and pin configuration using
> the device tree. Pin configuration uses generic parsing code.
> 
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> ---
>  .../devicetree/bindings/pinctrl/ste,abx500.txt     |  352 ++++++++++++++++++++
>  drivers/pinctrl/pinctrl-abx500.c                   |  184 ++++++++++
>  2 files changed, 536 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/ste,abx500.txt
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/ste,abx500.txt b/Documentation/devicetree/bindings/pinctrl/ste,abx500.txt
> new file mode 100644
> index 0000000..e74a72d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/ste,abx500.txt
> @@ -0,0 +1,352 @@
> +ST Ericsson abx500 pinmux controller
> +
> +Required properties:
> +- compatible: "stericsson,ab8500-gpio",  "stericsson,ab8540-gpio",
> +	      "stericsson,ab8505-gpio", "stericsson,ab9540-gpio",
> +
> +Please refer to pinctrl-bindings.txt in this directory for details of the
> +common pinctrl bindings used by client devices, including the meaning of the
> +phrase "pin configuration node".
> +
> +ST Ericsson's pin configuration nodes act as a container for an arbitrary number of
> +subnodes. Each of these subnodes represents some desired configuration for a
> +pin, a group, or a list of pins or groups. This configuration can include the
> +mux function to select on those pin(s)/group(s), and various pin configuration
> +parameters, such as input, output, pull up, pull down...
> +
> +The name of each subnode is not important; all subnodes should be enumerated
> +and processed purely based on their content.
> +
> +Required subnode-properties:
> +- ste,pins : An array of strings. Each string contains the name of a pin or
> +    group.
> +
> +Optional subnode-properties:
> +- ste,function: A string containing the name of the function to mux to the
> +  pin or group.
> +
> +- generic pin configuration option to use. Example :
> +
> +	default_cfg {
> +		ste,pins = "GPIO1";
> +		bias-disable;
> +	};
> +
> +- ste,config: Handle of pin configuration node containing the generic
> +  pinconfig options to use, as described in pinctrl-bindings.txt in
> +  this directory. Example :
> +
> +	pcfg_bias_disable: pcfg_bias_disable {
> +		bias-disable;
> +	};
> +
> +	default_cfg {
> +		ste,pins = "GPIO1";
> +		ste.config = <&pcfg_bias_disable>;
> +	};
> +
> +Example board file extract:
> +
> +&pinctrl_abx500 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&sysclkreq2_default_mode>, <&sysclkreq3_default_mode>, <&gpio3_default_mode>, <&sysclkreq6_default_mode>, <&pwmout1_default_mode>, <&pwmout2_default_mode>, <&pwmout3_default_mode>, <&adi1_default_mode>, <&dmic12_default_mode>, <&dmic34_default_mode>, <&dmic56_default_mode>, <&sysclkreq5_default_mode>, <&batremn_default_mode>, <&service_default_mode>, <&pwrctrl0_default_mode>, <&pwrctrl1_default_mode>, <&pwmextvibra1_default_mode>, <&pwmextvibra2_default_mode>, <&gpio51_default_mode>, <&gpio52_default_mode>, <&gpio53_default_mode>, <&gpio54_default_mode>, <&pdmclkdat_default_mode>;
> +
> +	sysclkreq2 {
> +		sysclkreq2_default_mode: sysclkreq2_default {
> +			default_mux {
> +				ste,function = "sysclkreq";
> +				ste,pins = "sysclkreq2_d_1";
> +			};
> +			default_cfg {
> +				ste,pins = "GPIO1";
> +				bias-disable;
> +			};
> +		};
> +	};
> +	sysclkreq3 {
> +		sysclkreq3_default_mode: sysclkreq3_default {
> +			default_mux {
> +				ste,function = "sysclkreq";
> +				ste,pins = "sysclkreq3_d_1";
> +			};
> +			default_cfg {
> +				ste,pins = "GPIO2";
> +				output-low;
> +			};
> +		};
> +	};
> +	gpio3 {
> +		gpio3_default_mode: gpio3_default {
> +			default_mux {
> +				ste,function = "gpio";
> +				ste,pins = "gpio3_a_1";
> +			};
> +			default_cfg {
> +				ste,pins = "GPIO3";
> +				output-low;
> +			};
> +		};
> +	};
> +	sysclkreq6 {
> +		sysclkreq6_default_mode: sysclkreq6_default {
> +			default_mux {
> +				ste,function = "sysclkreq";
> +				ste,pins = "sysclkreq6_d_1";
> +			};
> +			default_cfg {
> +				ste,pins = "GPIO4";
> +				bias-disable;
> +			};
> +		};
> +	};
> +	pwmout1 {
> +		pwmout1_default_mode: pwmout1_default {
> +			default_mux {
> +				ste,function = "pwmout";
> +				ste,pins = "pwmout1_d_1";
> +			};
> +			default_cfg {
> +				ste,pins = "GPIO14";
> +				output-low;
> +			};
> +		};
> +	};
> +	pwmout2 {
> +		pwmout2_default_mode: pwmout2_default {
> +			pwmout2_default_mux {
> +				ste,function = "pwmout";
> +				ste,pins = "pwmout2_d_1";
> +			};
> +			pwmout2_default_cfg {
> +				ste,pins = "GPIO15";
> +				output-low;
> +			};
> +		};
> +	};
> +	pwmout3 {
> +		pwmout3_default_mode: pwmout3_default {
> +			pwmout3_default_mux {
> +				ste,function = "pwmout";
> +				ste,pins = "pwmout3_d_1";
> +			};
> +			pwmout3_default_cfg {
> +				ste,pins = "GPIO16";
> +				output-low;
> +			};
> +		};
> +	};
> +	adi1 {{

Two '{'? (I know that's just documentation but better get it right too)

> +
> +		adi1_default_mode: adi1_default {
> +			adi1_default_mux {
> +				ste,function = "adi1";
> +				ste,pins = "adi1_d_1";
> +			};
> +			adi1_default_cfg1 {
> +				ste,pins = "GPIO17","GPIO19","GPIO20";
> +				bias-disable;
> +			};
> +			adi1_default_cfg2 {
> +				ste,pins = "GPIO18";
> +				output-low;
> +			};
> +		};
> +	};
> +	dmic12 {
> +		dmic12_default_mode: dmic12_default {
> +			dmic12_default_mux {
> +				ste,function = "dmic";
> +				ste,pins = "dmic12_d_1";
> +			};
> +			dmic12_default_cfg1 {
> +				ste,pins = "GPIO27";
> +				output-low;
> +			};
> +			dmic12_default_cfg2 {
> +				ste,pins = "GPIO28";
> +				bias-disable;
> +			};
> +		};
> +	};
> +	dmic34 {
> +		dmic34_default_mode: dmic34_default {
> +			dmic34_default_mux {
> +				ste,function = "dmic";
> +				ste,pins = "dmic34_d_1";
> +			};
> +			dmic34_default_cfg1 {
> +				ste,pins = "GPIO29";
> +				output-low;
> +			};
> +			dmic34_default_cfg2 {
> +				ste,pins = "GPIO30";
> +				bias-disable;{
> +
> +			};
> +		};
> +	};
> +	dmic56 {
> +		dmic56_default_mode: dmic56_default {
> +			dmic56_default_mux {
> +				ste,function = "dmic";
> +				ste,pins = "dmic56_d_1";
> +			};
> +			dmic56_default_cfg1 {
> +				ste,pins = "GPIO31";
> +				output-low;
> +			};
> +			dmic56_default_cfg2 {
> +				ste,pins = "GPIO32";
> +				bias-disable;
> +			};
> +		};
> +	};
> +	sysclkreq5 {
> +		sysclkreq5_default_mode: sysclkreq5_default {
> +			sysclkreq5_default_mux {
> +				ste,function = "sysclkreq";
> +				ste,pins = "sysclkreq5_d_1";
> +			};
> +			sysclkreq5_default_cfg {
> +				ste,pins = "GPIO42";
> +				output-low;
> +			};
> +		};
> +	};
> +	batremn {
> +		batremn_default_mode: batremn_default {
> +			batremn_default_mux {
> +				ste,function = "batremn";
> +				ste,pins = "batremn_d_1";
> +			};
> +			batremn_default_cfg {
> +				ste,pins = "GPIO43";
> +				bias-disable;
> +			};
> +		};
> +	};
> +	service {
> +		service_default_mode: service_default {
> +			service_default_mux {
> +				ste,function = "service";
> +				ste,pins = "service_d_1";
> +			};
> +			service_default_cfg {
> +				ste,pins = "GPIO44";
> +				bias-disable;
> +			};
> +		};
> +	};
> +	pwrctrl0 {
> +		pwrctrl0_default_mux: pwrctrl0_mux {
> +			pwrctrl0_default_mux {
> +				ste,function = "pwrctrl";
> +				ste,pins = "pwrctrl0_d_1";
> +			};
> +		};
> +		pwrctrl0_default_mode: pwrctrl0_default {
> +			pwrctrl0_default_cfg {
> +				ste,pins = "GPIO45";
> +				bias-disable;
> +			};
> +		};
> +	};
> +	pwrctrl1 {
> +		pwrctrl1_default_mux: pwrctrl1_mux {
> +			pwrctrl1_default_mux {
> +				ste,function = "pwrctrl";
> +				ste,pins = "pwrctrl1_d_1";
> +			};
> +		};
> +		pwrctrl1_default_mode: pwrctrl1_default {
> +			pwrctrl1_default_cfg {
> +				ste,pins = "GPIO46";
> +				bias-disable;
> +			};
> +		};
> +	};
> +	pwmextvibra1 {
> +		pwmextvibra1_default_mode: pwmextvibra1_default {
> +			pwmextvibra1_default_mux {
> +				ste,function = "pwmextvibra";
> +				ste,pins = "pwmextvibra1_d_1";
> +			};
> +			pwmextvibra1_default_cfg {
> +				ste,pins = "GPIO47";
> +				bias-disable;
> +			};
> +		};
> +	};
> +	pwmextvibra2 {
> +		pwmextvibra2_default_mode: pwmextvibra2_default {
> +			pwmextvibra2_default_mux {
> +				ste,function = "pwmextvibra";
> +				ste,pins = "pwmextvibra2_d_1";
> +			};
> +			pwmextvibra1_default_cfg {
> +				ste,pins = "GPIO48";
> +				bias-disable;
> +			};
> +		};
> +	};
> +	gpio51 {
> +		gpio51_default_mode: gpio51_default {
> +				gpio51_default_mux {
> +				ste,function = "gpio";
> +				ste,pins = "gpio51_a_1";
> +			};
> +			gpio51_default_cfg {
> +				ste,pins = "GPIO51";
> +				output-low;
> +			};
> +		};
> +	};
> +	gpio52 {
> +		gpio52_default_mode: gpio52_default {
> +			gpio52_default_mux {
> +				ste,function = "gpio";
> +				ste,pins = "gpio52_a_1";
> +			};
> +			gpio52_default_cfg {
> +				ste,pins = "GPIO52";
> +				bias-pull-down;
> +			};
> +		};
> +	};
> +	gpio53 {
> +		gpio53_default_mode: gpio53_default {
> +			gpio53_default_mux {
> +				ste,function = "gpio";
> +				ste,pins = "gpio53_a_1";
> +			};
> +			gpio53_default_cfg {
> +				ste,pins = "GPIO53";
> +				bias-pull-down;
> +		};
> +	};

Wrong indentation here.

> +	};
> +	gpio54 {
> +		gpio54_default_mode: gpio54_default {
> +			gpio54_default_mux {
> +				ste,function = "gpio";
> +				ste,pins = "gpio54_a_1";
> +			};
> +			gpio54_default_cfg {
> +				ste,pins = "GPIO54";
> +				output-low;
> +			};
> +		};
> +	};
> +	pdmclkdat {
> +		pdmclkdat_default_mode: pdmclkdat_default {
> +			pdmclkdat_default_mux {
> +				ste,function = "pdm";
> +				ste,pins = "pdmclkdat_d_1";
> +			};
> +			pdmclkdat_default_cfg {
> +				ste,pins = "GPIO55", "GPIO56";
> +				bias-disable;
> +			};
> +		};
> +	};
> +};
> diff --git a/drivers/pinctrl/pinctrl-abx500.c b/drivers/pinctrl/pinctrl-abx500.c
> index 84b40f7..b5b5460 100644
> --- a/drivers/pinctrl/pinctrl-abx500.c
> +++ b/drivers/pinctrl/pinctrl-abx500.c
> @@ -30,8 +30,10 @@
>  #include <linux/pinctrl/pinmux.h>
>  #include <linux/pinctrl/pinconf.h>
>  #include <linux/pinctrl/pinconf-generic.h>
> +#include <linux/pinctrl/machine.h>
>  
>  #include "pinctrl-abx500.h"
> +#include "pinconf.h"
>  
>  /*
>   * The AB9540 and AB8540 GPIO support are extended versions
> @@ -757,11 +759,193 @@ static void abx500_pin_dbg_show(struct pinctrl_dev *pctldev,
>  				 chip->base + offset - 1);
>  }
>  
> +static void abx500_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_PIN)
> +			kfree(map[i].data.configs.configs);
> +	kfree(map);
> +}
> +
> +static int abx500_dt_reserve_map(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)
> +		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 abx500_dt_add_map_mux(struct pinctrl_map **map,
> +		unsigned *reserved_maps,
> +		unsigned *num_maps, const char *group,
> +		const char *function)
> +{
> +	if (*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 abx500_dt_add_map_configs(struct pinctrl_map **map,
> +		unsigned *reserved_maps,
> +		unsigned *num_maps, const char *group,
> +		unsigned long *configs, unsigned num_configs)
> +{
> +	unsigned long *dup_configs;
> +
> +	if (*num_maps == *reserved_maps)
> +		return -ENOSPC;
> +
> +	dup_configs = kmemdup(configs, num_configs * sizeof(*dup_configs),
> +			      GFP_KERNEL);
> +	if (!dup_configs)
> +		return -ENOMEM;
> +
> +	(*map)[*num_maps].type = PIN_MAP_TYPE_CONFIGS_PIN;
> +
> +	(*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 const char *abx500_find_pin_name(struct pinctrl_dev *pctldev,
> +					const char *pin_name)
> +{
> +	int i, pin_number;
> +	struct abx500_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
> +
> +	if (sscanf((char *)pin_name, "GPIO%d", &pin_number) == 1)
> +		for (i = 0; i < npct->soc->npins; i++)
> +			if (npct->soc->pins[i].number == pin_number)
> +				return npct->soc->pins[i].name;
> +	return NULL;
> +}
> +
> +int abx500_dt_subnode_to_map(struct pinctrl_dev *pctldev,

Missing static?

> +		struct device_node *np,
> +		struct pinctrl_map **map,
> +		unsigned *reserved_maps,
> +		unsigned *num_maps)
> +{
> +	int ret;
> +	const char *function = NULL;
> +	unsigned long *configs;
> +	unsigned int nconfigs = 0;
> +	bool has_config = 0;
> +	unsigned reserve = 0;
> +	struct property *prop;
> +	const char *group, *gpio_name;
> +	struct device_node *np_config;
> +
> +	ret = of_property_read_string(np, "ste,function", &function);
> +	if (ret >= 0)
> +		reserve = 1;
> +
> +	ret = pinconf_generic_parse_dt_config(np, &configs, &nconfigs);
> +	if (nconfigs)
> +		has_config = 1;
> +
> +	np_config = of_parse_phandle(np, "ste,config", 0);
> +	if (np_config) {
> +		ret = pinconf_generic_parse_dt_config(np_config, &configs,
> +				&nconfigs);
> +		if (ret)
> +			goto exit;
> +		has_config |= nconfigs;
> +	}
> +
> +	ret = of_property_count_strings(np, "ste,pins");
> +	if (ret < 0)
> +		goto exit;
> +
> +	if (has_config)
> +		reserve++;
> +
> +	reserve *= ret;
> +
> +	ret = abx500_dt_reserve_map(map, reserved_maps, num_maps, reserve);
> +	if (ret < 0)
> +		goto exit;
> +
> +	of_property_for_each_string(np, "ste,pins", prop, group) {
> +		if (function) {
> +			ret = abx500_dt_add_map_mux(map, reserved_maps,
> +					num_maps, group, function);
> +			if (ret < 0)
> +				goto exit;
> +		}
> +		if (has_config) {
> +			gpio_name = abx500_find_pin_name(pctldev, group);
> +
> +			ret = abx500_dt_add_map_configs(map, reserved_maps,
> +					num_maps, gpio_name, configs, 1);
> +			if (ret < 0)
> +				goto exit;
> +		}
> +
> +	}
> +exit:
> +	return ret;
> +}
> +
> +int abx500_dt_node_to_map(struct pinctrl_dev *pctldev,

Same here.

Fabio

> +				 struct device_node *np_config,
> +				 struct pinctrl_map **map, unsigned *num_maps)
> +{
> +	unsigned reserved_maps;
> +	struct device_node *np;
> +	int ret;
> +
> +	reserved_maps = 0;
> +	*map = NULL;
> +	*num_maps = 0;
> +
> +	for_each_child_of_node(np_config, np) {
> +		ret = abx500_dt_subnode_to_map(pctldev, np, map,
> +				&reserved_maps, num_maps);
> +		if (ret < 0) {
> +			abx500_dt_free_map(pctldev, *map, *num_maps);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct pinctrl_ops abx500_pinctrl_ops = {
>  	.get_groups_count = abx500_get_groups_cnt,
>  	.get_group_name = abx500_get_group_name,
>  	.get_group_pins = abx500_get_group_pins,
>  	.pin_dbg_show = abx500_pin_dbg_show,
> +	.dt_node_to_map = abx500_dt_node_to_map,
> +	.dt_free_map = abx500_dt_free_map,
>  };
>  
>  static int abx500_pin_config_get(struct pinctrl_dev *pctldev,
> -- 
> 1.7.10
> 

-- 
Fabio Baltieri

Re: [PATCH 1/2] pinctrl: abx500: Add device tree support

[Patrice Chotard]

Hi Fabio,

thanks for your remarks, i will submit a fix for all of them.

Patrice


On Thu, Jun 20, 2013 at 10:03 AM, Fabio Baltieri
<fabio.baltieri@linaro.org> wrote:
> Hi Patrice,
>
> this looks good, just a couple of minor notes, check below...
>
> On Thu, Jun 20, 2013 at 09:23:21AM +0200, patrice.chotard.st@gmail.com wrote:
>> From: Patrice Chotard <patrice.chotard@st.com>
>>
>> We use the same way to define pin muxing and pin configuration
>> than for nomadik. So pickup code from pinctrl_nomadik.c to be
>> able to implement pin multiplexing and pin configuration using
>> the device tree. Pin configuration uses generic parsing code.
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
>> ---
>>  .../devicetree/bindings/pinctrl/ste,abx500.txt     |  352 ++++++++++++++++++++
>>  drivers/pinctrl/pinctrl-abx500.c                   |  184 ++++++++++
>>  2 files changed, 536 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pinctrl/ste,abx500.txt
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/ste,abx500.txt b/Documentation/devicetree/bindings/pinctrl/ste,abx500.txt
>> new file mode 100644
>> index 0000000..e74a72d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/ste,abx500.txt
>> @@ -0,0 +1,352 @@
>> +ST Ericsson abx500 pinmux controller
>> +
>> +Required properties:
>> +- compatible: "stericsson,ab8500-gpio",  "stericsson,ab8540-gpio",
>> +           "stericsson,ab8505-gpio", "stericsson,ab9540-gpio",
>> +
>> +Please refer to pinctrl-bindings.txt in this directory for details of the
>> +common pinctrl bindings used by client devices, including the meaning of the
>> +phrase "pin configuration node".
>> +
>> +ST Ericsson's pin configuration nodes act as a container for an arbitrary number of
>> +subnodes. Each of these subnodes represents some desired configuration for a
>> +pin, a group, or a list of pins or groups. This configuration can include the
>> +mux function to select on those pin(s)/group(s), and various pin configuration
>> +parameters, such as input, output, pull up, pull down...
>> +
>> +The name of each subnode is not important; all subnodes should be enumerated
>> +and processed purely based on their content.
>> +
>> +Required subnode-properties:
>> +- ste,pins : An array of strings. Each string contains the name of a pin or
>> +    group.
>> +
>> +Optional subnode-properties:
>> +- ste,function: A string containing the name of the function to mux to the
>> +  pin or group.
>> +
>> +- generic pin configuration option to use. Example :
>> +
>> +     default_cfg {
>> +             ste,pins = "GPIO1";
>> +             bias-disable;
>> +     };
>> +
>> +- ste,config: Handle of pin configuration node containing the generic
>> +  pinconfig options to use, as described in pinctrl-bindings.txt in
>> +  this directory. Example :
>> +
>> +     pcfg_bias_disable: pcfg_bias_disable {
>> +             bias-disable;
>> +     };
>> +
>> +     default_cfg {
>> +             ste,pins = "GPIO1";
>> +             ste.config = <&pcfg_bias_disable>;
>> +     };
>> +
>> +Example board file extract:
>> +
>> +&pinctrl_abx500 {
>> +     pinctrl-names = "default";
>> +     pinctrl-0 = <&sysclkreq2_default_mode>, <&sysclkreq3_default_mode>, <&gpio3_default_mode>, <&sysclkreq6_default_mode>, <&pwmout1_default_mode>, <&pwmout2_default_mode>, <&pwmout3_default_mode>, <&adi1_default_mode>, <&dmic12_default_mode>, <&dmic34_default_mode>, <&dmic56_default_mode>, <&sysclkreq5_default_mode>, <&batremn_default_mode>, <&service_default_mode>, <&pwrctrl0_default_mode>, <&pwrctrl1_default_mode>, <&pwmextvibra1_default_mode>, <&pwmextvibra2_default_mode>, <&gpio51_default_mode>, <&gpio52_default_mode>, <&gpio53_default_mode>, <&gpio54_default_mode>, <&pdmclkdat_default_mode>;
>> +
>> +     sysclkreq2 {
>> +             sysclkreq2_default_mode: sysclkreq2_default {
>> +                     default_mux {
>> +                             ste,function = "sysclkreq";
>> +                             ste,pins = "sysclkreq2_d_1";
>> +                     };
>> +                     default_cfg {
>> +                             ste,pins = "GPIO1";
>> +                             bias-disable;
>> +                     };
>> +             };
>> +     };
>> +     sysclkreq3 {
>> +             sysclkreq3_default_mode: sysclkreq3_default {
>> +                     default_mux {
>> +                             ste,function = "sysclkreq";
>> +                             ste,pins = "sysclkreq3_d_1";
>> +                     };
>> +                     default_cfg {
>> +                             ste,pins = "GPIO2";
>> +                             output-low;
>> +                     };
>> +             };
>> +     };
>> +     gpio3 {
>> +             gpio3_default_mode: gpio3_default {
>> +                     default_mux {
>> +                             ste,function = "gpio";
>> +                             ste,pins = "gpio3_a_1";
>> +                     };
>> +                     default_cfg {
>> +                             ste,pins = "GPIO3";
>> +                             output-low;
>> +                     };
>> +             };
>> +     };
>> +     sysclkreq6 {
>> +             sysclkreq6_default_mode: sysclkreq6_default {
>> +                     default_mux {
>> +                             ste,function = "sysclkreq";
>> +                             ste,pins = "sysclkreq6_d_1";
>> +                     };
>> +                     default_cfg {
>> +                             ste,pins = "GPIO4";
>> +                             bias-disable;
>> +                     };
>> +             };
>> +     };
>> +     pwmout1 {
>> +             pwmout1_default_mode: pwmout1_default {
>> +                     default_mux {
>> +                             ste,function = "pwmout";
>> +                             ste,pins = "pwmout1_d_1";
>> +                     };
>> +                     default_cfg {
>> +                             ste,pins = "GPIO14";
>> +                             output-low;
>> +                     };
>> +             };
>> +     };
>> +     pwmout2 {
>> +             pwmout2_default_mode: pwmout2_default {
>> +                     pwmout2_default_mux {
>> +                             ste,function = "pwmout";
>> +                             ste,pins = "pwmout2_d_1";
>> +                     };
>> +                     pwmout2_default_cfg {
>> +                             ste,pins = "GPIO15";
>> +                             output-low;
>> +                     };
>> +             };
>> +     };
>> +     pwmout3 {
>> +             pwmout3_default_mode: pwmout3_default {
>> +                     pwmout3_default_mux {
>> +                             ste,function = "pwmout";
>> +                             ste,pins = "pwmout3_d_1";
>> +                     };
>> +                     pwmout3_default_cfg {
>> +                             ste,pins = "GPIO16";
>> +                             output-low;
>> +                     };
>> +             };
>> +     };
>> +     adi1 {{
>
> Two '{'? (I know that's just documentation but better get it right too)
>
>> +
>> +             adi1_default_mode: adi1_default {
>> +                     adi1_default_mux {
>> +                             ste,function = "adi1";
>> +                             ste,pins = "adi1_d_1";
>> +                     };
>> +                     adi1_default_cfg1 {
>> +                             ste,pins = "GPIO17","GPIO19","GPIO20";
>> +                             bias-disable;
>> +                     };
>> +                     adi1_default_cfg2 {
>> +                             ste,pins = "GPIO18";
>> +                             output-low;
>> +                     };
>> +             };
>> +     };
>> +     dmic12 {
>> +             dmic12_default_mode: dmic12_default {
>> +                     dmic12_default_mux {
>> +                             ste,function = "dmic";
>> +                             ste,pins = "dmic12_d_1";
>> +                     };
>> +                     dmic12_default_cfg1 {
>> +                             ste,pins = "GPIO27";
>> +                             output-low;
>> +                     };
>> +                     dmic12_default_cfg2 {
>> +                             ste,pins = "GPIO28";
>> +                             bias-disable;
>> +                     };
>> +             };
>> +     };
>> +     dmic34 {
>> +             dmic34_default_mode: dmic34_default {
>> +                     dmic34_default_mux {
>> +                             ste,function = "dmic";
>> +                             ste,pins = "dmic34_d_1";
>> +                     };
>> +                     dmic34_default_cfg1 {
>> +                             ste,pins = "GPIO29";
>> +                             output-low;
>> +                     };
>> +                     dmic34_default_cfg2 {
>> +                             ste,pins = "GPIO30";
>> +                             bias-disable;{
>> +
>> +                     };
>> +             };
>> +     };
>> +     dmic56 {
>> +             dmic56_default_mode: dmic56_default {
>> +                     dmic56_default_mux {
>> +                             ste,function = "dmic";
>> +                             ste,pins = "dmic56_d_1";
>> +                     };
>> +                     dmic56_default_cfg1 {
>> +                             ste,pins = "GPIO31";
>> +                             output-low;
>> +                     };
>> +                     dmic56_default_cfg2 {
>> +                             ste,pins = "GPIO32";
>> +                             bias-disable;
>> +                     };
>> +             };
>> +     };
>> +     sysclkreq5 {
>> +             sysclkreq5_default_mode: sysclkreq5_default {
>> +                     sysclkreq5_default_mux {
>> +                             ste,function = "sysclkreq";
>> +                             ste,pins = "sysclkreq5_d_1";
>> +                     };
>> +                     sysclkreq5_default_cfg {
>> +                             ste,pins = "GPIO42";
>> +                             output-low;
>> +                     };
>> +             };
>> +     };
>> +     batremn {
>> +             batremn_default_mode: batremn_default {
>> +                     batremn_default_mux {
>> +                             ste,function = "batremn";
>> +                             ste,pins = "batremn_d_1";
>> +                     };
>> +                     batremn_default_cfg {
>> +                             ste,pins = "GPIO43";
>> +                             bias-disable;
>> +                     };
>> +             };
>> +     };
>> +     service {
>> +             service_default_mode: service_default {
>> +                     service_default_mux {
>> +                             ste,function = "service";
>> +                             ste,pins = "service_d_1";
>> +                     };
>> +                     service_default_cfg {
>> +                             ste,pins = "GPIO44";
>> +                             bias-disable;
>> +                     };
>> +             };
>> +     };
>> +     pwrctrl0 {
>> +             pwrctrl0_default_mux: pwrctrl0_mux {
>> +                     pwrctrl0_default_mux {
>> +                             ste,function = "pwrctrl";
>> +                             ste,pins = "pwrctrl0_d_1";
>> +                     };
>> +             };
>> +             pwrctrl0_default_mode: pwrctrl0_default {
>> +                     pwrctrl0_default_cfg {
>> +                             ste,pins = "GPIO45";
>> +                             bias-disable;
>> +                     };
>> +             };
>> +     };
>> +     pwrctrl1 {
>> +             pwrctrl1_default_mux: pwrctrl1_mux {
>> +                     pwrctrl1_default_mux {
>> +                             ste,function = "pwrctrl";
>> +                             ste,pins = "pwrctrl1_d_1";
>> +                     };
>> +             };
>> +             pwrctrl1_default_mode: pwrctrl1_default {
>> +                     pwrctrl1_default_cfg {
>> +                             ste,pins = "GPIO46";
>> +                             bias-disable;
>> +                     };
>> +             };
>> +     };
>> +     pwmextvibra1 {
>> +             pwmextvibra1_default_mode: pwmextvibra1_default {
>> +                     pwmextvibra1_default_mux {
>> +                             ste,function = "pwmextvibra";
>> +                             ste,pins = "pwmextvibra1_d_1";
>> +                     };
>> +                     pwmextvibra1_default_cfg {
>> +                             ste,pins = "GPIO47";
>> +                             bias-disable;
>> +                     };
>> +             };
>> +     };
>> +     pwmextvibra2 {
>> +             pwmextvibra2_default_mode: pwmextvibra2_default {
>> +                     pwmextvibra2_default_mux {
>> +                             ste,function = "pwmextvibra";
>> +                             ste,pins = "pwmextvibra2_d_1";
>> +                     };
>> +                     pwmextvibra1_default_cfg {
>> +                             ste,pins = "GPIO48";
>> +                             bias-disable;
>> +                     };
>> +             };
>> +     };
>> +     gpio51 {
>> +             gpio51_default_mode: gpio51_default {
>> +                             gpio51_default_mux {
>> +                             ste,function = "gpio";
>> +                             ste,pins = "gpio51_a_1";
>> +                     };
>> +                     gpio51_default_cfg {
>> +                             ste,pins = "GPIO51";
>> +                             output-low;
>> +                     };
>> +             };
>> +     };
>> +     gpio52 {
>> +             gpio52_default_mode: gpio52_default {
>> +                     gpio52_default_mux {
>> +                             ste,function = "gpio";
>> +                             ste,pins = "gpio52_a_1";
>> +                     };
>> +                     gpio52_default_cfg {
>> +                             ste,pins = "GPIO52";
>> +                             bias-pull-down;
>> +                     };
>> +             };
>> +     };
>> +     gpio53 {
>> +             gpio53_default_mode: gpio53_default {
>> +                     gpio53_default_mux {
>> +                             ste,function = "gpio";
>> +                             ste,pins = "gpio53_a_1";
>> +                     };
>> +                     gpio53_default_cfg {
>> +                             ste,pins = "GPIO53";
>> +                             bias-pull-down;
>> +             };
>> +     };
>
> Wrong indentation here.
>
>> +     };
>> +     gpio54 {
>> +             gpio54_default_mode: gpio54_default {
>> +                     gpio54_default_mux {
>> +                             ste,function = "gpio";
>> +                             ste,pins = "gpio54_a_1";
>> +                     };
>> +                     gpio54_default_cfg {
>> +                             ste,pins = "GPIO54";
>> +                             output-low;
>> +                     };
>> +             };
>> +     };
>> +     pdmclkdat {
>> +             pdmclkdat_default_mode: pdmclkdat_default {
>> +                     pdmclkdat_default_mux {
>> +                             ste,function = "pdm";
>> +                             ste,pins = "pdmclkdat_d_1";
>> +                     };
>> +                     pdmclkdat_default_cfg {
>> +                             ste,pins = "GPIO55", "GPIO56";
>> +                             bias-disable;
>> +                     };
>> +             };
>> +     };
>> +};
>> diff --git a/drivers/pinctrl/pinctrl-abx500.c b/drivers/pinctrl/pinctrl-abx500.c
>> index 84b40f7..b5b5460 100644
>> --- a/drivers/pinctrl/pinctrl-abx500.c
>> +++ b/drivers/pinctrl/pinctrl-abx500.c
>> @@ -30,8 +30,10 @@
>>  #include <linux/pinctrl/pinmux.h>
>>  #include <linux/pinctrl/pinconf.h>
>>  #include <linux/pinctrl/pinconf-generic.h>
>> +#include <linux/pinctrl/machine.h>
>>
>>  #include "pinctrl-abx500.h"
>> +#include "pinconf.h"
>>
>>  /*
>>   * The AB9540 and AB8540 GPIO support are extended versions
>> @@ -757,11 +759,193 @@ static void abx500_pin_dbg_show(struct pinctrl_dev *pctldev,
>>                                chip->base + offset - 1);
>>  }
>>
>> +static void abx500_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_PIN)
>> +                     kfree(map[i].data.configs.configs);
>> +     kfree(map);
>> +}
>> +
>> +static int abx500_dt_reserve_map(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)
>> +             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 abx500_dt_add_map_mux(struct pinctrl_map **map,
>> +             unsigned *reserved_maps,
>> +             unsigned *num_maps, const char *group,
>> +             const char *function)
>> +{
>> +     if (*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 abx500_dt_add_map_configs(struct pinctrl_map **map,
>> +             unsigned *reserved_maps,
>> +             unsigned *num_maps, const char *group,
>> +             unsigned long *configs, unsigned num_configs)
>> +{
>> +     unsigned long *dup_configs;
>> +
>> +     if (*num_maps == *reserved_maps)
>> +             return -ENOSPC;
>> +
>> +     dup_configs = kmemdup(configs, num_configs * sizeof(*dup_configs),
>> +                           GFP_KERNEL);
>> +     if (!dup_configs)
>> +             return -ENOMEM;
>> +
>> +     (*map)[*num_maps].type = PIN_MAP_TYPE_CONFIGS_PIN;
>> +
>> +     (*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 const char *abx500_find_pin_name(struct pinctrl_dev *pctldev,
>> +                                     const char *pin_name)
>> +{
>> +     int i, pin_number;
>> +     struct abx500_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
>> +
>> +     if (sscanf((char *)pin_name, "GPIO%d", &pin_number) == 1)
>> +             for (i = 0; i < npct->soc->npins; i++)
>> +                     if (npct->soc->pins[i].number == pin_number)
>> +                             return npct->soc->pins[i].name;
>> +     return NULL;
>> +}
>> +
>> +int abx500_dt_subnode_to_map(struct pinctrl_dev *pctldev,
>
> Missing static?
>
>> +             struct device_node *np,
>> +             struct pinctrl_map **map,
>> +             unsigned *reserved_maps,
>> +             unsigned *num_maps)
>> +{
>> +     int ret;
>> +     const char *function = NULL;
>> +     unsigned long *configs;
>> +     unsigned int nconfigs = 0;
>> +     bool has_config = 0;
>> +     unsigned reserve = 0;
>> +     struct property *prop;
>> +     const char *group, *gpio_name;
>> +     struct device_node *np_config;
>> +
>> +     ret = of_property_read_string(np, "ste,function", &function);
>> +     if (ret >= 0)
>> +             reserve = 1;
>> +
>> +     ret = pinconf_generic_parse_dt_config(np, &configs, &nconfigs);
>> +     if (nconfigs)
>> +             has_config = 1;
>> +
>> +     np_config = of_parse_phandle(np, "ste,config", 0);
>> +     if (np_config) {
>> +             ret = pinconf_generic_parse_dt_config(np_config, &configs,
>> +                             &nconfigs);
>> +             if (ret)
>> +                     goto exit;
>> +             has_config |= nconfigs;
>> +     }
>> +
>> +     ret = of_property_count_strings(np, "ste,pins");
>> +     if (ret < 0)
>> +             goto exit;
>> +
>> +     if (has_config)
>> +             reserve++;
>> +
>> +     reserve *= ret;
>> +
>> +     ret = abx500_dt_reserve_map(map, reserved_maps, num_maps, reserve);
>> +     if (ret < 0)
>> +             goto exit;
>> +
>> +     of_property_for_each_string(np, "ste,pins", prop, group) {
>> +             if (function) {
>> +                     ret = abx500_dt_add_map_mux(map, reserved_maps,
>> +                                     num_maps, group, function);
>> +                     if (ret < 0)
>> +                             goto exit;
>> +             }
>> +             if (has_config) {
>> +                     gpio_name = abx500_find_pin_name(pctldev, group);
>> +
>> +                     ret = abx500_dt_add_map_configs(map, reserved_maps,
>> +                                     num_maps, gpio_name, configs, 1);
>> +                     if (ret < 0)
>> +                             goto exit;
>> +             }
>> +
>> +     }
>> +exit:
>> +     return ret;
>> +}
>> +
>> +int abx500_dt_node_to_map(struct pinctrl_dev *pctldev,
>
> Same here.
>
> Fabio
>
>> +                              struct device_node *np_config,
>> +                              struct pinctrl_map **map, unsigned *num_maps)
>> +{
>> +     unsigned reserved_maps;
>> +     struct device_node *np;
>> +     int ret;
>> +
>> +     reserved_maps = 0;
>> +     *map = NULL;
>> +     *num_maps = 0;
>> +
>> +     for_each_child_of_node(np_config, np) {
>> +             ret = abx500_dt_subnode_to_map(pctldev, np, map,
>> +                             &reserved_maps, num_maps);
>> +             if (ret < 0) {
>> +                     abx500_dt_free_map(pctldev, *map, *num_maps);
>> +                     return ret;
>> +             }
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>>  static const struct pinctrl_ops abx500_pinctrl_ops = {
>>       .get_groups_count = abx500_get_groups_cnt,
>>       .get_group_name = abx500_get_group_name,
>>       .get_group_pins = abx500_get_group_pins,
>>       .pin_dbg_show = abx500_pin_dbg_show,
>> +     .dt_node_to_map = abx500_dt_node_to_map,
>> +     .dt_free_map = abx500_dt_free_map,
>>  };
>>
>>  static int abx500_pin_config_get(struct pinctrl_dev *pctldev,
>> --
>> 1.7.10
>>
>
> --
> Fabio Baltieri

[PATCH 2/2] pinctrl: abx500: fix abx500_pin_config_set()

[patrice.chotard.st]

From: Patrice Chotard <patrice.chotard@st.com>

_ Update abx500_pin_config_set() in order to take in
account PIN_CONFIG_BIAS_DISABLE state to disable
pull up or pull down.

_ Rework error path.

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---
 drivers/pinctrl/pinctrl-abx500.c |   31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-abx500.c b/drivers/pinctrl/pinctrl-abx500.c
index b5b5460..14dc078 100644
--- a/drivers/pinctrl/pinctrl-abx500.c
+++ b/drivers/pinctrl/pinctrl-abx500.c
@@ -33,6 +33,7 @@
 #include <linux/pinctrl/machine.h>
 
 #include "pinctrl-abx500.h"
+#include "core.h"
 #include "pinconf.h"
 
 /*
@@ -963,7 +964,7 @@ static int abx500_pin_config_set(struct pinctrl_dev *pctldev,
 	struct pullud *pullud = pct->soc->pullud;
 	struct gpio_chip *chip = &pct->chip;
 	unsigned offset;
-	int ret = 0;
+	int ret = -EINVAL;
 	enum pin_config_param param = pinconf_to_config_param(config);
 	enum pin_config_param argument = pinconf_to_config_argument(config);
 
@@ -976,13 +977,32 @@ static int abx500_pin_config_set(struct pinctrl_dev *pctldev,
 	offset = pin - 1;
 
 	switch (param) {
-	case PIN_CONFIG_BIAS_PULL_DOWN:
+	case PIN_CONFIG_BIAS_DISABLE:
+		ret = abx500_gpio_direction_input(chip, offset);
 		/*
-		 * if argument = 1 set the pull down
-		 * else clear the pull down
+		 * Some chips only support pull down, while some actually
+		 * support both pull up and pull down. Such chips have
+		 * a "pullud" range specified for the pins that support
+		 * both features. If the pin is not within that range, we
+		 * fall back to the old bit set that only support pull down.
 		 */
+		if (pullud &&
+		    pin >= pullud->first_pin &&
+		    pin <= pullud->last_pin)
+			ret = abx500_set_pull_updown(pct,
+				pin,
+				ABX500_GPIO_PULL_NONE);
+		else
+			/* Chip only supports pull down */
+			ret = abx500_gpio_set_bits(chip, AB8500_GPIO_PUD1_REG,
+				offset, ABX500_GPIO_PULL_NONE);
+		break;
+
+	case PIN_CONFIG_BIAS_PULL_DOWN:
 		ret = abx500_gpio_direction_input(chip, offset);
 		/*
+		 * if argument = 1 set the pull down
+		 * else clear the pull down
 		 * Some chips only support pull down, while some actually
 		 * support both pull up and pull down. Such chips have
 		 * a "pullud" range specified for the pins that support
@@ -1002,6 +1022,7 @@ static int abx500_pin_config_set(struct pinctrl_dev *pctldev,
 		break;
 
 	case PIN_CONFIG_BIAS_PULL_UP:
+		ret = abx500_gpio_direction_input(chip, offset);
 		/*
 		 * if argument = 1 set the pull up
 		 * else clear the pull up
@@ -1030,8 +1051,6 @@ static int abx500_pin_config_set(struct pinctrl_dev *pctldev,
 
 	default:
 		dev_err(chip->dev, "illegal configuration requested\n");
-
-		return -EINVAL;
 	}
 
 	return ret;
-- 
1.7.10

Re: [PATCH 2/2] pinctrl: abx500: fix abx500_pin_config_set()

[Fabio Baltieri]

On Thu, Jun 20, 2013 at 09:23:22AM +0200, patrice.chotard.st@gmail.com wrote:
> From: Patrice Chotard <patrice.chotard@st.com>
> 
> _ Update abx500_pin_config_set() in order to take in
> account PIN_CONFIG_BIAS_DISABLE state to disable
> pull up or pull down.
> 
> _ Rework error path.
> 
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> ---
>  drivers/pinctrl/pinctrl-abx500.c |   31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-abx500.c b/drivers/pinctrl/pinctrl-abx500.c
> index b5b5460..14dc078 100644
> --- a/drivers/pinctrl/pinctrl-abx500.c
> +++ b/drivers/pinctrl/pinctrl-abx500.c
> @@ -33,6 +33,7 @@
>  #include <linux/pinctrl/machine.h>
>  
>  #include "pinctrl-abx500.h"
> +#include "core.h"
>  #include "pinconf.h"
>  
>  /*
> @@ -963,7 +964,7 @@ static int abx500_pin_config_set(struct pinctrl_dev *pctldev,
>  	struct pullud *pullud = pct->soc->pullud;
>  	struct gpio_chip *chip = &pct->chip;
>  	unsigned offset;
> -	int ret = 0;
> +	int ret = -EINVAL;
>  	enum pin_config_param param = pinconf_to_config_param(config);
>  	enum pin_config_param argument = pinconf_to_config_argument(config);
>  
> @@ -976,13 +977,32 @@ static int abx500_pin_config_set(struct pinctrl_dev *pctldev,
>  	offset = pin - 1;
>  
>  	switch (param) {
> -	case PIN_CONFIG_BIAS_PULL_DOWN:
> +	case PIN_CONFIG_BIAS_DISABLE:
> +		ret = abx500_gpio_direction_input(chip, offset);
>  		/*
> -		 * if argument = 1 set the pull down
> -		 * else clear the pull down
> +		 * Some chips only support pull down, while some actually
> +		 * support both pull up and pull down. Such chips have
> +		 * a "pullud" range specified for the pins that support
> +		 * both features. If the pin is not within that range, we
> +		 * fall back to the old bit set that only support pull down.
>  		 */
> +		if (pullud &&
> +		    pin >= pullud->first_pin &&
> +		    pin <= pullud->last_pin)

This multi-line check is replicated in all conditions, would it make
sense to move it on a dedicated function to improve readability?

> +			ret = abx500_set_pull_updown(pct,
> +				pin,
> +				ABX500_GPIO_PULL_NONE);
> +		else
> +			/* Chip only supports pull down */
> +			ret = abx500_gpio_set_bits(chip, AB8500_GPIO_PUD1_REG,
> +				offset, ABX500_GPIO_PULL_NONE);
> +		break;
> +
> +	case PIN_CONFIG_BIAS_PULL_DOWN:
>  		ret = abx500_gpio_direction_input(chip, offset);
>  		/*
> +		 * if argument = 1 set the pull down
> +		 * else clear the pull down
>  		 * Some chips only support pull down, while some actually
>  		 * support both pull up and pull down. Such chips have
>  		 * a "pullud" range specified for the pins that support
> @@ -1002,6 +1022,7 @@ static int abx500_pin_config_set(struct pinctrl_dev *pctldev,
>  		break;
>  
>  	case PIN_CONFIG_BIAS_PULL_UP:
> +		ret = abx500_gpio_direction_input(chip, offset);

Here the return value of abx500_gpio_direction_input is set but never
checked, and will be always overwritten by the next abx500_gpio_ call...
Would it make sense to add a pr_err for it?  On the other side, if it
never fails, you can just drop the return field altogether.

That's also done in other conditions in the same 'switch', it may make
sense to have a patch just for that.

Thanks,
Fabio

>  		/*
>  		 * if argument = 1 set the pull up
>  		 * else clear the pull up
> @@ -1030,8 +1051,6 @@ static int abx500_pin_config_set(struct pinctrl_dev *pctldev,
>  
>  	default:
>  		dev_err(chip->dev, "illegal configuration requested\n");
> -
> -		return -EINVAL;
>  	}
>  
>  	return ret;
> -- 
> 1.7.10
> 

-- 
Fabio Baltieri

Re: [PATCH 2/2] pinctrl: abx500: fix abx500_pin_config_set()

[Patrice Chotard]

On Thu, Jun 20, 2013 at 10:18 AM, Fabio Baltieri
<fabio.baltieri@linaro.org> wrote:
> On Thu, Jun 20, 2013 at 09:23:22AM +0200, patrice.chotard.st@gmail.com wrote:
>> From: Patrice Chotard <patrice.chotard@st.com>
>>
>> _ Update abx500_pin_config_set() in order to take in
>> account PIN_CONFIG_BIAS_DISABLE state to disable
>> pull up or pull down.
>>
>> _ Rework error path.
>>
>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
>> ---
>>  drivers/pinctrl/pinctrl-abx500.c |   31 +++++++++++++++++++++++++------
>>  1 file changed, 25 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-abx500.c b/drivers/pinctrl/pinctrl-abx500.c
>> index b5b5460..14dc078 100644
>> --- a/drivers/pinctrl/pinctrl-abx500.c
>> +++ b/drivers/pinctrl/pinctrl-abx500.c
>> @@ -33,6 +33,7 @@
>>  #include <linux/pinctrl/machine.h>
>>
>>  #include "pinctrl-abx500.h"
>> +#include "core.h"
>>  #include "pinconf.h"
>>
>>  /*
>> @@ -963,7 +964,7 @@ static int abx500_pin_config_set(struct pinctrl_dev *pctldev,
>>       struct pullud *pullud = pct->soc->pullud;
>>       struct gpio_chip *chip = &pct->chip;
>>       unsigned offset;
>> -     int ret = 0;
>> +     int ret = -EINVAL;
>>       enum pin_config_param param = pinconf_to_config_param(config);
>>       enum pin_config_param argument = pinconf_to_config_argument(config);
>>
>> @@ -976,13 +977,32 @@ static int abx500_pin_config_set(struct pinctrl_dev *pctldev,
>>       offset = pin - 1;
>>
>>       switch (param) {
>> -     case PIN_CONFIG_BIAS_PULL_DOWN:
>> +     case PIN_CONFIG_BIAS_DISABLE:
>> +             ret = abx500_gpio_direction_input(chip, offset);
>>               /*
>> -              * if argument = 1 set the pull down
>> -              * else clear the pull down
>> +              * Some chips only support pull down, while some actually
>> +              * support both pull up and pull down. Such chips have
>> +              * a "pullud" range specified for the pins that support
>> +              * both features. If the pin is not within that range, we
>> +              * fall back to the old bit set that only support pull down.
>>                */
>> +             if (pullud &&
>> +                 pin >= pullud->first_pin &&
>> +                 pin <= pullud->last_pin)
>
> This multi-line check is replicated in all conditions, would it make
> sense to move it on a dedicated function to improve readability?

Yes i will add a dedicated function.

>
>> +                     ret = abx500_set_pull_updown(pct,
>> +                             pin,
>> +                             ABX500_GPIO_PULL_NONE);
>> +             else
>> +                     /* Chip only supports pull down */
>> +                     ret = abx500_gpio_set_bits(chip, AB8500_GPIO_PUD1_REG,
>> +                             offset, ABX500_GPIO_PULL_NONE);
>> +             break;
>> +
>> +     case PIN_CONFIG_BIAS_PULL_DOWN:
>>               ret = abx500_gpio_direction_input(chip, offset);
>>               /*
>> +              * if argument = 1 set the pull down
>> +              * else clear the pull down
>>                * Some chips only support pull down, while some actually
>>                * support both pull up and pull down. Such chips have
>>                * a "pullud" range specified for the pins that support
>> @@ -1002,6 +1022,7 @@ static int abx500_pin_config_set(struct pinctrl_dev *pctldev,
>>               break;
>>
>>       case PIN_CONFIG_BIAS_PULL_UP:
>> +             ret = abx500_gpio_direction_input(chip, offset);
>
> Here the return value of abx500_gpio_direction_input is set but never
> checked, and will be always overwritten by the next abx500_gpio_ call...
> Would it make sense to add a pr_err for it?  On the other side, if it
> never fails, you can just drop the return field altogether.

I will rework this part and make a global review as i notice that same issue
appears on several other place in this file.
>
> That's also done in other conditions in the same 'switch', it may make
> sense to have a patch just for that.
>
> Thanks,
> Fabio
>
>>               /*
>>                * if argument = 1 set the pull up
>>                * else clear the pull up
>> @@ -1030,8 +1051,6 @@ static int abx500_pin_config_set(struct pinctrl_dev *pctldev,
>>
>>       default:
>>               dev_err(chip->dev, "illegal configuration requested\n");
>> -
>> -             return -EINVAL;
>>       }
>>
>>       return ret;
>> --
>> 1.7.10
>>
>
> --
> Fabio Baltieri