Re: [PATCH v4 3/9] pinctrl: convert to use match_string() helper

[Andrew Morton]

On Thu, 28 Jan 2016 15:14:19 +0200 Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:

> The new helper returns index of the mathing string in an array. We would use it
> here.
> 
> --- a/drivers/pinctrl/pinmux.c
> +++ b/drivers/pinctrl/pinmux.c
> @@ -334,7 +334,6 @@ int pinmux_map_to_setting(struct pinctrl_map const *map,
>  	unsigned num_groups;
>  	int ret;
>  	const char *group;
> -	int i;
>  
>  	if (!pmxops) {
>  		dev_err(pctldev->dev, "does not support mux function\n");
> @@ -363,19 +362,13 @@ int pinmux_map_to_setting(struct pinctrl_map const *map,
>  		return -EINVAL;
>  	}
>  	if (map->data.mux.group) {
> -		bool found = false;
>  		group = map->data.mux.group;
> -		for (i = 0; i < num_groups; i++) {
> -			if (!strcmp(group, groups[i])) {
> -				found = true;
> -				break;
> -			}
> -		}
> -		if (!found) {
> +		ret = match_string(groups, num_groups, group);
> +		if (ret < 0) {
>  			dev_err(pctldev->dev,
>  				"invalid group \"%s\" for function \"%s\"\n",
>  				group, map->data.mux.function);
> -			return -EINVAL;
> +			return ret;

Changes the return value from -EINVAL to -ENODATA.  I'm not reeeeeealy
sure what ENODATA means - it seems mostly a networking thing?  People
use it in various places because it kinda sounds like whatever it is
that just went wrong.

But the question is: what will the end user think when this error comes
out of the kernel?  Given that he/she has just tried to misconfigure
the pinctrl system, ENODATA will cause much head-scratching.  EINVAL is
more appropriate?  "You tried to do something invalid".

>  		}
>  	} else {
>  		group = groups[0];
> -- 
> 2.7.0.rc3