lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130620080346.GA20594@balto.lan>
Date:	Thu, 20 Jun 2013 10:03:46 +0200
From:	Fabio Baltieri <fabio.baltieri@...aro.org>
To:	patrice.chotard.st@...il.com
Cc:	linux-kernel@...r.kernel.org,
	Linus Walleij <linus.walleij@...aro.org>,
	Olivier Clergeaud <olivier.clergeaud@...com>,
	Lee Jones <lee.jones@...aro.org>,
	Patrice Chotard <patrice.chotard@...com>,
	Gabriel Fernandez <gabriel.fernandez@...com>
Subject: Re: [PATCH 1/2] pinctrl: abx500: Add device tree support

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@...il.com wrote:
> From: Patrice Chotard <patrice.chotard@...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@...com>
> Signed-off-by: Patrice Chotard <patrice.chotard@...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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ