From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754139AbcBBIK3 (ORCPT ); Tue, 2 Feb 2016 03:10:29 -0500 Received: from mga03.intel.com ([134.134.136.65]:60246 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753843AbcBBIKT (ORCPT ); Tue, 2 Feb 2016 03:10:19 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,384,1449561600"; d="scan'208";a="903642482" Message-ID: <1454400524.23271.8.camel@linux.intel.com> Subject: Re: [PATCH v4 3/9] pinctrl: convert to use match_string() helper From: Andy Shevchenko To: Andrew Morton Cc: Tejun Heo , Linus Walleij , Dmitry Eremin-Solenikov , linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, "David S . Miller" , David Airlie , Rasmus Villemoes Date: Tue, 02 Feb 2016 10:08:44 +0200 In-Reply-To: <20160201220230.92810bde.akpm@linux-foundation.org> References: <1453986865-133572-1-git-send-email-andriy.shevchenko@linux.intel.com> <1453986865-133572-4-git-send-email-andriy.shevchenko@linux.intel.com> <20160201220230.92810bde.akpm@linux-foundation.org> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.3-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2016-02-01 at 22:02 -0800, Andrew Morton wrote: > On Thu, 28 Jan 2016 15:14:19 +0200 Andy 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 Intel Finland Oy