Re: [PATCH 1/3] pinctrl: core: create unlocked version of pinctrl_find_gpio_range_from_pin

[Joachim Eastwood <manabian@xxxxxxxxx>]

On 25 February 2016 at 10:23, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> On Thu, Feb 25, 2016 at 10:21 AM, Linus Walleij
> <linus.walleij@xxxxxxxxxx> wrote:
>> On Sat, Feb 20, 2016 at 11:51 PM, Joachim Eastwood <manabian@xxxxxxxxx> wrote:
>>
>>> pinctrl_find_gpio_range_from_pin takes the pctldev->mutex but so
>>> does pinconf_pins_show and this will cause a deadlock if
>>> pinctrl_find_gpio_range_from_pin is used in .pin_config_get
>>> callback.
>>>
>>> Create an unlocked version of pinctrl_find_gpio_range_from_pin to
>>> allow pin to gpio lookup to be used from pinconf_pins_show.
>>>
>>> Signed-off-by: Joachim Eastwood <manabian@xxxxxxxxx>
>>
>> I understand that the function is needed and it's semantically
>> OK.
>>
>>> +EXPORT_SYMBOL_GPL(__pinctrl_find_gpio_range_from_pin);
>> (...)
>>> +extern struct pinctrl_gpio_range *
>>> +__pinctrl_find_gpio_range_from_pin(struct pinctrl_dev *pctldev,
>>> +                                  unsigned int pin);
>>> +
>>
>> This function name is NOT OK.
>>
>> Rename it pinctrl_fund_gpio_range_from_pin_unlocked(),
>
> Or rather, pinctrl_fund_gpio_range_from_pin_locked(),
> indicating that you're already holding the necessary lock
> when calling the function. Now I'm even confusing myself,
> sorry :(

Shouldn't the function name indicate what the function does with the lock?

pinctrl_fund_gpio_range_from_pin_unlocked() would indicate to me that
it does not acquire a lock and it is your responsibility as a caller
to ensure that the correct lock is held before calling.

But I am fine with either pinctrl_fund_gpio_range_from_pin_unlocked()
or pinctrl_fund_gpio_range_from_pin_locked(). It's all up to you.


regards,
Joachim Eastwood
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html