[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 02 Feb 2016 10:08:44 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Tejun Heo <tj@...nel.org>,
Linus Walleij <linus.walleij@...aro.org>,
Dmitry Eremin-Solenikov <dbaryshkov@...il.com>,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
"David S . Miller" <davem@...emloft.net>,
David Airlie <airlied@...ux.ie>,
Rasmus Villemoes <linux@...musvillemoes.dk>
Subject: Re: [PATCH v4 3/9] pinctrl: convert to use match_string() helper
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@...ux.intel.com>
Intel Finland Oy
Powered by blists - more mailing lists