linux-kernel - Re: [PATCH RESEND 1/5] pinctrl: allows not to define the get_group

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: <5347B431.5060506@gmail.com>
Date:	Fri, 11 Apr 2014 11:21:53 +0200
From:	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
To:	Antoine Ténart 
	<antoine.tenart@...e-electrons.com>, linus.walleij@...aro.org
CC:	alexandre.belloni@...e-electrons.com, zmxu@...vell.com,
	jszhang@...vell.com, linux-arm-kernel@...ts.infradead.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND 1/5] pinctrl: allows not to define the get_group_pins
 operation

On 04/10/2014 03:07 PM, Antoine Ténart wrote:
> When using a group only pinctrl driver, which does not have any
> information on the pins it is useless to define a get_group_pins
> always returning an empty list of pins.
>
> When not using get_group_pin[1], a driver must implement it so
> pins = NULL and num_pins = 0. This patch makes it the default
> behaviour if not defined in the pinctrl driver when used in
> pinmux enable and disable funtions and in pinctrl_groups_show.
>
> It also adds a check in pinctrl_get_group_pins and return -EINVAL if
> not defined. This function is called in the gpiolib when adding when
> pingroup range. It cannot be used if no group is defined, so this seams
> reasonable.

I don't have a strong opinion about making pins optional for pinctrl
drivers. I'll leave it for Linus to decide on it.

But have you checked all the other places pins are referred to?

One place that jumps into my mind would be the DEBUGFS files...

Sebastian

> [1] get_group_pin(struct pinctrl_dev *pctldev,
> 		  unsigned selector,
> 		  const unsigned **pins,
> 		  unsigned *num_pins);
>
> Signed-off-by: Antoine Ténart <antoine.tenart@...e-electrons.com>
> ---
>   drivers/pinctrl/core.c   | 17 ++++++++++-------
>   drivers/pinctrl/pinmux.c | 23 +++++++++++++----------
>   2 files changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
> index c0fe6091566a..e09474ecde23 100644
> --- a/drivers/pinctrl/core.c
> +++ b/drivers/pinctrl/core.c
> @@ -468,6 +468,9 @@ int pinctrl_get_group_pins(struct pinctrl_dev *pctldev, const char *pin_group,
>   	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
>   	int gs;
>
> +	if (!pctlops->get_group_pins)
> +		return -EINVAL;
> +
>   	gs = pinctrl_get_group_selector(pctldev, pin_group);
>   	if (gs < 0)
>   		return gs;
> @@ -1362,15 +1365,16 @@ static int pinctrl_groups_show(struct seq_file *s, void *what)
>
>   	seq_puts(s, "registered pin groups:\n");
>   	while (selector < ngroups) {
> -		const unsigned *pins;
> -		unsigned num_pins;
> +		const unsigned *pins = NULL;
> +		unsigned num_pins = 0;
>   		const char *gname = ops->get_group_name(pctldev, selector);
>   		const char *pname;
> -		int ret;
> +		int ret = 0;
>   		int i;
>
> -		ret = ops->get_group_pins(pctldev, selector,
> -					  &pins, &num_pins);
> +		if (ops->get_group_pins)
> +			ret = ops->get_group_pins(pctldev, selector,
> +						  &pins, &num_pins);
>   		if (ret)
>   			seq_printf(s, "%s [ERROR GETTING PINS]\n",
>   				   gname);
> @@ -1694,8 +1698,7 @@ static int pinctrl_check_ops(struct pinctrl_dev *pctldev)
>
>   	if (!ops ||
>   	    !ops->get_groups_count ||
> -	    !ops->get_group_name ||
> -	    !ops->get_group_pins)
> +	    !ops->get_group_name)
>   		return -EINVAL;
>
>   	if (ops->dt_node_to_map && !ops->dt_free_map)
> diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
> index 9248ce4efed4..051e8592990e 100644
> --- a/drivers/pinctrl/pinmux.c
> +++ b/drivers/pinctrl/pinmux.c
> @@ -391,14 +391,16 @@ int pinmux_enable_setting(struct pinctrl_setting const *setting)
>   	struct pinctrl_dev *pctldev = setting->pctldev;
>   	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
>   	const struct pinmux_ops *ops = pctldev->desc->pmxops;
> -	int ret;
> -	const unsigned *pins;
> -	unsigned num_pins;
> +	int ret = 0;
> +	const unsigned *pins = NULL;
> +	unsigned num_pins = 0;
>   	int i;
>   	struct pin_desc *desc;
>
> -	ret = pctlops->get_group_pins(pctldev, setting->data.mux.group,
> -				      &pins, &num_pins);
> +	if (pctlops->get_group_pins)
> +		ret = pctlops->get_group_pins(pctldev, setting->data.mux.group,
> +					      &pins, &num_pins);
> +
>   	if (ret) {
>   		const char *gname;
>
> @@ -470,14 +472,15 @@ void pinmux_disable_setting(struct pinctrl_setting const *setting)
>   	struct pinctrl_dev *pctldev = setting->pctldev;
>   	const struct pinctrl_ops *pctlops = pctldev->desc->pctlops;
>   	const struct pinmux_ops *ops = pctldev->desc->pmxops;
> -	int ret;
> -	const unsigned *pins;
> -	unsigned num_pins;
> +	int ret = 0;
> +	const unsigned *pins = NULL;
> +	unsigned num_pins = 0;
>   	int i;
>   	struct pin_desc *desc;
>
> -	ret = pctlops->get_group_pins(pctldev, setting->data.mux.group,
> -				      &pins, &num_pins);
> +	if (pctlops->get_group_pins)
> +		ret = pctlops->get_group_pins(pctldev, setting->data.mux.group,
> +					      &pins, &num_pins);
>   	if (ret) {
>   		const char *gname;
>
>

--
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/