Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1323881

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

Show all headers | View raw

On Mon, 2016-02-01 at 22:02 -0800, Andrew Morton wrote:
> On Thu, 28 Jan 2016 15:14:19 +0200 Andy Shevchenko <andriy.shevchenko
> @linux.intel.com> 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.

Yeah, Al is concerned about this as well [1]. That's why I emphasized
this in cover letter.

>   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".

Yeah, my arguments still the same, our error reporting from 70s sucks.

Since few people are concerned about EINVAL vs. ENODATA, I will re-do
patches 1 and 2 to follow this default instead of ENODATA.

[1] http://www.spinics.net/lists/kernel/msg2157541.html

> >  		}
> >  	} else {
> >  		group = groups[0];

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v4 0/9] lib/string: introduce match_string() helper Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-28 14:20 +0100
  [PATCH v4 4/9] drm/edid: convert to use match_string() helper Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-28 14:20 +0100
  [PATCH v4 3/9] pinctrl: convert to use match_string() helper Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-28 14:20 +0100
    Re: [PATCH v4 3/9] pinctrl: convert to use match_string() helper Andrew Morton <akpm@linux-foundation.org> - 2016-02-02 07:10 +0100
      Re: [PATCH v4 3/9] pinctrl: convert to use match_string() helper Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-02-02 09:20 +0100
  [PATCH v4 8/9] ide: hpt366: convert to use match_string() helper Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-28 14:20 +0100
  [PATCH v4 5/9] power: charger_manager: convert to use match_string() helper Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-28 14:20 +0100
  [PATCH v4 6/9] power: ab8500: convert to use match_string() helper Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-28 14:20 +0100
  [PATCH v4 1/9] lib/string: introduce match_string() helper Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-28 14:20 +0100
    Re: [PATCH v4 1/9] lib/string: introduce match_string() helper Andrew Morton <akpm@linux-foundation.org> - 2016-02-02 07:10 +0100
      Re: [PATCH v4 1/9] lib/string: introduce match_string() helper Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-02-02 09:00 +0100
      Re: [PATCH v4 1/9] lib/string: introduce match_string() helper Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2016-02-02 21:00 +0100
  [PATCH v4 7/9] ata: hpt366: convert to use match_string() helper Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2016-01-28 14:20 +0100

csiph-web