Re: [PATCH v2 1/4] sh-pfc: do not call sh_pfc_get_pin_index() when unnecessary

[Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>]

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