From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergei Shtylyov Subject: Re: [PATCH v2 1/4] sh-pfc: do not call sh_pfc_get_pin_index() when unnecessary Date: Tue, 23 Jun 2015 01:42:41 +0300 Message-ID: <55888F61.800@cogentembedded.com> References: <4326653.qPvIJDU6F2@wasted.cogentembedded.com> <10321666.EILxzFPKZf@wasted.cogentembedded.com> <1860613.5T71vLz0D7@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mail-lb0-f173.google.com ([209.85.217.173]:34737 "EHLO mail-lb0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751370AbbFVWmq (ORCPT ); Mon, 22 Jun 2015 18:42:46 -0400 Received: by lbnk3 with SMTP id k3so11636035lbn.1 for ; Mon, 22 Jun 2015 15:42:44 -0700 (PDT) In-Reply-To: <1860613.5T71vLz0D7@avalon> Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: Laurent Pinchart Cc: linus.walleij@linaro.org, linux-sh@vger.kernel.org, linux-gpio@vger.kernel.org Hello. On 06/18/2015 10:05 PM, Laurent Pinchart wrote: >> Calling sh_pfc_get_pin_index() to calculate a pin index based on the >> collected pin range data is unnecessary when we're dealing with >> 'pfc->info->pins' and 'chip->pins' arrays as those always reperesent the >> pins starting from index 0 sequentially. Being a mere optimization at >> this time, this change will become crucial when we'll allow the "holes" in >> those arrays... > Pin information is stored in per-SoC pins arrays (a.k.a. pfc->info->pins). The > driver support two models to number pins: > - The sequential model, in which pins are numbered sequentially starting at 0. > Pin numbers are equal to the index in the array. And I didn't touch this case. > - The explicit numbering model, in which each pin entry has an explicit number > (stored in struct sh_pfc_pin.pin). Pins numbers are not necessarily equal to > the index of the pin entry in the array. Ah... I was just looking at _GP_GPIO() which still assigns sequential pin #'s equal to the indices. > The sh_pfc_get_pin_index() function converts a pin number to the pin index in > the pins array. > Let's consider the sh_pfc_pinconf_validate() from which your patch removes the > call to sh_pfc_get_pin_index() and uses the pin number directly. The function > is called from the .pin_config_get() and .pin_config_set() handlers. One > possible call path is > pinconf_pins_show() -> pinconf_dump_pin() -> pinconf_dump_pins() -> > pinconf_generic_dump_pins() -> pinconf_generic_dump_one() -> > pin_config_get_for_pin() -> .pin_config_get() > The pin value passed to the .pin_config_get() function is pctldev->desc-> > pins[i].number, which is the pin number, not its index. It thus looks like > this patch introduces a bug. > There might be something obvious I'm not getting though, so please feel free > to prove me wrong. The bug seems more like theoretical one at this point (unless you have the examples with non-sequential pin #'s)... WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in