| Message ID | 20170119094820.83595-3-mika.westerberg@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 2017-01-19 at 12:48 +0300, Mika Westerberg wrote: > When a GPIO driver is backed by a pinctrl driver the GPIO driver > sometimes needs to call the pinctrl driver to configure certain > things, > like whether the pin is used as input or output. In addition to this > there are other configurations applicable to GPIOs such as setting > debounce time of the GPIO. > > To support this we introduce a new function pinctrl_gpio_set_config() > that can be used by gpiolib based driver to pass configuration > requests > to the backing pinctrl driver. > + mutex_lock(&pctldev->mutex); > + pin = gpio_to_pin(range, gpio); > + ret = pinconf_set_config(pctldev, pin, configs, > ARRAY_SIZE(configs)); > + mutex_unlock(&pctldev->mutex); Does gpio_to_pin() require to be under lock?
On Thu, Jan 19, 2017 at 1:11 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Thu, 2017-01-19 at 12:48 +0300, Mika Westerberg wrote: >> When a GPIO driver is backed by a pinctrl driver the GPIO driver >> sometimes needs to call the pinctrl driver to configure certain >> things, >> like whether the pin is used as input or output. In addition to this >> there are other configurations applicable to GPIOs such as setting >> debounce time of the GPIO. >> >> To support this we introduce a new function pinctrl_gpio_set_config() >> that can be used by gpiolib based driver to pass configuration >> requests >> to the backing pinctrl driver. > > >> + mutex_lock(&pctldev->mutex); >> + pin = gpio_to_pin(range, gpio); >> + ret = pinconf_set_config(pctldev, pin, configs, >> ARRAY_SIZE(configs)); >> + mutex_unlock(&pctldev->mutex); > > Does gpio_to_pin() require to be under lock? All other callers do that because: commit 9b77ace409e1419c331209c4c8eb2c8bc990e9fd Author: Axel Lin <axel.lin@ingics.com> Date: Mon Aug 19 10:07:46 2013 +0800 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". I think this fixes the race between pin_free() and pin_request() calls. 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(). Signed-off-by: Axel Lin <axel.lin@ingics.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c index fb38e208f32d..8f6b2631c244 100644 --- a/drivers/pinctrl/core.c +++ b/drivers/pinctrl/core.c @@ -688,6 +688,34 @@ int pinctrl_gpio_direction_output(unsigned gpio) } EXPORT_SYMBOL_GPL(pinctrl_gpio_direction_output); +/** + * pinctrl_gpio_set_config() - Apply config to given GPIO pin + * @gpio: the GPIO pin number from the GPIO subsystem number space + * + * This function should *ONLY* be used from gpiolib-based GPIO drivers, if + * they need to call the underlying pin controller to change GPIO config + * (for example set debounce time). + */ +int pinctrl_gpio_set_config(unsigned gpio, unsigned long config) +{ + unsigned long configs[] = { config }; + struct pinctrl_gpio_range *range; + struct pinctrl_dev *pctldev; + int ret, pin; + + ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range); + if (ret) + return ret; + + mutex_lock(&pctldev->mutex); + pin = gpio_to_pin(range, gpio); + ret = pinconf_set_config(pctldev, pin, configs, ARRAY_SIZE(configs)); + mutex_unlock(&pctldev->mutex); + + return ret; +} +EXPORT_SYMBOL_GPL(pinctrl_gpio_set_config); + static struct pinctrl_state *find_state(struct pinctrl *p, const char *name) { diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c index 799048f3c8d4..c1c1ccc58267 100644 --- a/drivers/pinctrl/pinconf.c +++ b/drivers/pinctrl/pinconf.c @@ -200,6 +200,18 @@ int pinconf_apply_setting(struct pinctrl_setting const *setting) return 0; } +int pinconf_set_config(struct pinctrl_dev *pctldev, unsigned pin, + unsigned long *configs, size_t nconfigs) +{ + const struct pinconf_ops *ops; + + ops = pctldev->desc->confops; + if (!ops) + return -ENOTSUPP; + + return ops->pin_config_set(pctldev, pin, configs, nconfigs); +} + #ifdef CONFIG_DEBUG_FS static void pinconf_show_config(struct seq_file *s, struct pinctrl_dev *pctldev, diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h index 55c75780b3b2..bf8aff9abf32 100644 --- a/drivers/pinctrl/pinconf.h +++ b/drivers/pinctrl/pinconf.h @@ -20,6 +20,9 @@ int pinconf_map_to_setting(struct pinctrl_map const *map, void pinconf_free_setting(struct pinctrl_setting const *setting); int pinconf_apply_setting(struct pinctrl_setting const *setting); +int pinconf_set_config(struct pinctrl_dev *pctldev, unsigned pin, + unsigned long *configs, size_t nconfigs); + /* * You will only be interested in these if you're using PINCONF * so don't supply any stubs for these. @@ -56,6 +59,12 @@ static inline int pinconf_apply_setting(struct pinctrl_setting const *setting) return 0; } +static inline int pinconf_set_config(struct pinctrl_dev *pctldev, unsigned pin, + unsigned long *configs, size_t nconfigs) +{ + return -ENOTSUPP; +} + #endif #if defined(CONFIG_PINCONF) && defined(CONFIG_DEBUG_FS) diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h index d7e5d608faa7..a0f2aba72fa9 100644 --- a/include/linux/pinctrl/consumer.h +++ b/include/linux/pinctrl/consumer.h @@ -29,6 +29,7 @@ extern int pinctrl_request_gpio(unsigned gpio); extern void pinctrl_free_gpio(unsigned gpio); extern int pinctrl_gpio_direction_input(unsigned gpio); extern int pinctrl_gpio_direction_output(unsigned gpio); +extern int pinctrl_gpio_set_config(unsigned gpio, unsigned long config); extern struct pinctrl * __must_check pinctrl_get(struct device *dev); extern void pinctrl_put(struct pinctrl *p); @@ -80,6 +81,11 @@ static inline int pinctrl_gpio_direction_output(unsigned gpio) return 0; } +static inline int pinctrl_gpio_set_config(unsigned gpio, unsigned long config) +{ + return 0; +} + static inline struct pinctrl * __must_check pinctrl_get(struct device *dev) { return NULL;
When a GPIO driver is backed by a pinctrl driver the GPIO driver sometimes needs to call the pinctrl driver to configure certain things, like whether the pin is used as input or output. In addition to this there are other configurations applicable to GPIOs such as setting debounce time of the GPIO. To support this we introduce a new function pinctrl_gpio_set_config() that can be used by gpiolib based driver to pass configuration requests to the backing pinctrl driver. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/pinctrl/core.c | 28 ++++++++++++++++++++++++++++ drivers/pinctrl/pinconf.c | 12 ++++++++++++ drivers/pinctrl/pinconf.h | 9 +++++++++ include/linux/pinctrl/consumer.h | 6 ++++++ 4 files changed, 55 insertions(+)