pinctrl: core: Remove unnecessary test for desc->name
PATCH 1/2 pinctrl: core: Remove unnecessary test for desc->name
The implementation in pinctrl_register_one_pin() ensures pindesc->name is always not NULL before insert the pindesc to radix tree.
If the desc return from pin_desc_get is not NULL, desc->name is always not NULL.
--- drivers/pinctrl/core.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/pinctrl/core.c +1 -3
@@ -153,9 +153,7 @@ int pin_get_from_name(struct pinctrl_dev *pctldev, const char *name) pin = pctldev->desc->pins[i].number; desc =pin_desc_get (pctldev, pin);/* Pin space may be sparse */ -if (desc ==NULL ) -continue ; -if (desc->name && !strcmp (name, desc->name)) +if (desc && !strcmp (name, desc->name))return pin; }
Signature · 1 line
1.8.1.2
PATCH 2/2 pinctrl: core: Add proper mutex lock in pinctrl_request_gpio
This one is missed in commit 42fed7ba "pinctrl: move subsystem mutex to pinctrl_dev struct".
--- drivers/pinctrl/core.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/pinctrl/core.c +4 -0
@@ -562,11 +562,15 @@ int pinctrl_request_gpio(unsigned gpio)return ret; } +mutex_lock (&pctldev->mutex); +/* Convert to the pin controllers number space */ pin =gpio_to_pin (range, gpio); ret =pinmux_request_gpio (pctldev, range, pin, gpio); +mutex_unlock (&pctldev->mutex); +return ret; }EXPORT_SYMBOL_GPL (pinctrl_request_gpio);
Signature · 1 line
1.8.1.2
Re: [PATCH 2/2] pinctrl: core: Add proper mutex lock in pinctrl_request_gpio
Quoted 2 lines · Axel Lin
This one is missed in commit
42fed7ba"pinctrl: move subsystem mutex topinctrl_devstruct".
I think this was never there.
Quoted 13 lines
Signed-off-by Axel Lin <[email protected]>--- drivers/pinctrl/core.c | 4 ++++ 1 file changed, 4 insertions(+)drivers/pinctrl/core.c +0
-0@@ -562,11 +562,15 @@ int pinctrl_request_gpio(unsigned gpio)return ret; }
drivers/pinctrl/core.c int pinctrl_request_gpio(unsigned gpio) · line 574
Quoted 10 lines
drivers/pinctrl/core.c +4
-0+mutex_lock (&pctldev->mutex); +/* Convert to the pin controllers number space */ pin =gpio_to_pin (range, gpio); ret =pinmux_request_gpio (pctldev, range, pin, gpio); +mutex_unlock (&pctldev->mutex); +return ret;
What is this protecting against?
I'm not sure I follow this so better ask.
Yours,
Linus Walleij
Re: [PATCH 2/2] pinctrl: core: Add proper mutex lock in pinctrl_request_gpio
2013/8/22 Linus Walleij <[email protected]>:
Quoted 6 lines · Axel Lin
Quoted 3 lines
This one is missed in commit
42fed7ba"pinctrl: move subsystem mutex topinctrl_devstruct".I think this was never there.
I think it was protected by the global pinctrl_mutex in v3.9, you can check the source code here:
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/drivers/pinctrl/core.c?id=refs/tags/v3.9.11
However, commit 42fed7ba44 "pinctrl: move subsystem mutex to pinctrl_dev struct" then
adds mutex_lock(&pinctrldev_list_mutex); but forgot to add mutex_lock(&pctldev->mutex);
here although it adds mutex_lock(&pctldev->mutex) in pinctrl_free_gpio(). The mutex_lock(&pinctrldev_list_mutex) is then moved into pinctrl_get_device_gpio_range()
and pinctrl_ready_for_gpio_range() by commit 44d5f7b "pinctrl: sink pinctrldev_list_mutex".
Quoted 21 lines
Quoted 21 lines
Signed-off-by Axel Lin <[email protected]>--- drivers/pinctrl/core.c | 4 ++++ 1 file changed, 4 insertions(+)drivers/pinctrl/core.c +2
-0@@ -562,11 +562,15 @@ int pinctrl_request_gpio(unsigned gpio)return ret; } +mutex_lock (&pctldev->mutex); +/* Convert to the pin controllers number space */ pin =gpio_to_pin (range, gpio); ret =pinmux_request_gpio (pctldev, range, pin, gpio);
Quoted 7 lines
drivers/pinctrl/core.c int pinctrl_request_gpio(unsigned gpio) · line 574
Quoted 3 lines
drivers/pinctrl/core.c +2
-0+mutex_unlock (&pctldev->mutex); +return ret;What is this protecting against?
I'm not sure I follow this so better ask.
I think this fixes the race between pin_free() and pin_request() calls. (Well, I don't have a h/w to test at this moment.)
It protects accessing the members of pctldev->desc.
(e.g. update desc->mux_usecount, desc->gpio_owner, desc->mux_owner, etc) Current code grabs pctldev->mutex before calling pinmux_free_gpio(), but did not grab the mutex while calling pinmux_request_gpio().
Regards,
Axel
Re: [PATCH 2/2] pinctrl: core: Add proper mutex lock in pinctrl_request_gpio
Quoted 15 lines · Axel Lin
2013/8/22 Linus Walleij <[email protected]>:
Quoted 13 lines
drivers/pinctrl/core.c int pinctrl_request_gpio(unsigned gpio) · line 574
Quoted 10 lines
drivers/pinctrl/core.c +4
-0+mutex_lock (&pctldev->mutex); +/* Convert to the pin controllers number space */ pin =gpio_to_pin (range, gpio); ret =pinmux_request_gpio (pctldev, range, pin, gpio); +mutex_unlock (&pctldev->mutex); +return ret;What is this protecting against?
Quoted 8 lines
Quoted 1 line
I'm not sure I follow this so better ask.
I think this fixes the race between
pin_free()andpin_request()calls. (Well, I don't have a h/w to test at this moment.)
It protects accessing the members ofpctldev->desc.
(e.g. updatedesc->mux_usecount,desc->gpio_owner,desc->mux_owner, etc) Current code grabspctldev->mutexbefore callingpinmux_free_gpio(), but did not grab the mutex while callingpinmux_request_gpio().
OK you won me over, patch applied!
I also edited in some of the above text into the commit.
Yours,
Linus Walleij
Quoted 5 lines · Axel Lin
The implementation in
pinctrl_register_one_pin()ensurespindesc->nameis always not NULL before insert the pindesc to radix tree.
If the desc return frompin_desc_getis not NULL,desc->nameis always not NULL.Signed-off-by Axel Lin <[email protected]>
Good point. Patch applied.
Yours,
Linus Walleij