* Re: [PATCH] gpio: twl4030: Use guard(mutex)() over manual locking [not found] <20260502210354.160439-1-m32285159@gmail.com> @ 2026-05-04 12:49 ` Bartosz Golaszewski 2026-05-04 13:05 ` Maxwell Doose 2026-05-13 19:33 ` kernel test robot 1 sibling, 1 reply; 3+ messages in thread From: Bartosz Golaszewski @ 2026-05-04 12:49 UTC (permalink / raw) To: Maxwell Doose; +Cc: linux-gpio, linux-kernel, linusw, brgl On Sat, 2 May 2026 23:03:54 +0200, Maxwell Doose <m32285159@gmail.com> said: > Replace mutex_lock() and mutex_unlock() pairs with the RAII macro > guard(mutex)(). This keeps the driver up-to-date with the latest > function macros. > > Remove now-redundant gotos and goto labels which will maintain > readability. In addition, replace some gotos with direct returns where > appropriate. > > Update certain control paths to make them more suited to the new > locking. > > Signed-off-by: Maxwell Doose <m32285159@gmail.com> > --- > drivers/gpio/gpio-twl4030.c | 69 ++++++++++++++----------------------- > 1 file changed, 26 insertions(+), 43 deletions(-) > > diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c > index a851702befde..df17f9c08817 100644 > --- a/drivers/gpio/gpio-twl4030.c > +++ b/drivers/gpio/gpio-twl4030.c > @@ -23,6 +23,7 @@ > #include <linux/platform_device.h> > #include <linux/of.h> > #include <linux/irqdomain.h> > +#include <linux/cleanup.h> Maybe order the includes alphabetically if you're at it? > > #include <linux/mfd/twl.h> > > @@ -209,7 +210,7 @@ static int twl_request(struct gpio_chip *chip, unsigned offset) > struct gpio_twl4030_priv *priv = gpiochip_get_data(chip); > int status = 0; > > - mutex_lock(&priv->mutex); > + guard(mutex)(&priv->mutex); > > /* Support the two LED outputs as output-only GPIOs. */ > if (offset >= TWL4030_GPIO_MAX) { > @@ -227,30 +228,29 @@ static int twl_request(struct gpio_chip *chip, unsigned offset) > /* Configure PWM OFF register first */ > status = twl_i2c_write_u8(TWL4030_MODULE_LED, 0x7f, reg + 1); > if (status < 0) > - goto done; > + return status; > > /* Followed by PWM ON register */ > status = twl_i2c_write_u8(TWL4030_MODULE_LED, 0x7f, reg); > if (status < 0) > - goto done; > + return status; > > /* init LED to not-driven (high) */ > status = twl_i2c_read_u8(TWL4030_MODULE_LED, &cached_leden, > TWL4030_LED_LEDEN_REG); > if (status < 0) > - goto done; > + return status; > cached_leden &= ~ledclr_mask; > status = twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden, > TWL4030_LED_LEDEN_REG); > if (status < 0) > - goto done; > + return status; > > status = 0; > - goto done; > } > > /* on first use, turn GPIO module "on" */ > - if (!priv->usage_count) { > + else if (!priv->usage_count) { This is a functional change, I'm not sure you want to change the logic. > struct twl4030_gpio_platform_data *pdata; > u8 value = MASK_GPIO_CTRL_GPIO_ON; > > @@ -264,11 +264,9 @@ static int twl_request(struct gpio_chip *chip, unsigned offset) > status = gpio_twl4030_write(REG_GPIO_CTRL, value); > } > > -done: > if (!status) > priv->usage_count |= BIT(offset); > > - mutex_unlock(&priv->mutex); > return status; > } > > @@ -276,10 +274,10 @@ static void twl_free(struct gpio_chip *chip, unsigned offset) > { > struct gpio_twl4030_priv *priv = gpiochip_get_data(chip); > > - mutex_lock(&priv->mutex); > + guard(mutex)(&priv->mutex); > if (offset >= TWL4030_GPIO_MAX) { > WARN_ON_ONCE(twl4030_led_set_value(offset - TWL4030_GPIO_MAX, 1)); > - goto out; > + return; > } > > priv->usage_count &= ~BIT(offset); > @@ -287,9 +285,6 @@ static void twl_free(struct gpio_chip *chip, unsigned offset) > /* on last use, switch off GPIO module */ > if (!priv->usage_count) > gpio_twl4030_write(REG_GPIO_CTRL, 0x0); > - > -out: > - mutex_unlock(&priv->mutex); > } > > static int twl_direction_in(struct gpio_chip *chip, unsigned offset) > @@ -297,17 +292,15 @@ static int twl_direction_in(struct gpio_chip *chip, unsigned offset) > struct gpio_twl4030_priv *priv = gpiochip_get_data(chip); > int ret; > > - mutex_lock(&priv->mutex); > + guard(mutex)(&priv->mutex); > if (offset < TWL4030_GPIO_MAX) > ret = twl4030_set_gpio_direction(offset, 1); > else > - ret = -EINVAL; /* LED outputs can't be set as input */ > + return -EINVAL; /* LED outputs can't be set as input */ > > if (!ret) > priv->direction &= ~BIT(offset); > > - mutex_unlock(&priv->mutex); > - > return ret; > } > > @@ -317,10 +310,9 @@ static int twl_get(struct gpio_chip *chip, unsigned offset) > int ret; > int status = 0; > > - mutex_lock(&priv->mutex); > + guard(mutex)(&priv->mutex); > if (!(priv->usage_count & BIT(offset))) { > - ret = -EPERM; > - goto out; > + return -EPERM; > } > > if (priv->direction & BIT(offset)) > @@ -328,10 +320,7 @@ static int twl_get(struct gpio_chip *chip, unsigned offset) > else > status = twl4030_get_gpio_datain(offset); > > - ret = (status < 0) ? status : !!status; > -out: > - mutex_unlock(&priv->mutex); > - return ret; > + return (status < 0) ? status : !!status; > } > > static int twl_set(struct gpio_chip *chip, unsigned int offset, int value) > @@ -339,7 +328,7 @@ static int twl_set(struct gpio_chip *chip, unsigned int offset, int value) > struct gpio_twl4030_priv *priv = gpiochip_get_data(chip); > int ret; > > - mutex_lock(&priv->mutex); > + guard(mutex)(&priv->mutex); > if (offset < TWL4030_GPIO_MAX) > ret = twl4030_set_gpio_dataout(offset, value); > else > @@ -350,8 +339,6 @@ static int twl_set(struct gpio_chip *chip, unsigned int offset, int value) > else > priv->out_state &= ~BIT(offset); > > - mutex_unlock(&priv->mutex); > - > return ret; > } > > @@ -360,21 +347,19 @@ static int twl_direction_out(struct gpio_chip *chip, unsigned offset, int value) > struct gpio_twl4030_priv *priv = gpiochip_get_data(chip); > int ret = 0; > > - mutex_lock(&priv->mutex); > - if (offset < TWL4030_GPIO_MAX) { > - ret = twl4030_set_gpio_direction(offset, 0); > - if (ret) { > - mutex_unlock(&priv->mutex); > - return ret; > + scoped_guard(mutex, &priv->mutex) { > + if (offset < TWL4030_GPIO_MAX) { > + ret = twl4030_set_gpio_direction(offset, 0); > + if (ret) > + return ret; > } > - } > > - /* > - * LED gpios i.e. offset >= TWL4030_GPIO_MAX are always output > - */ > + /* > + * LED gpios i.e. offset >= TWL4030_GPIO_MAX are always output > + */ > > - priv->direction |= BIT(offset); > - mutex_unlock(&priv->mutex); > + priv->direction |= BIT(offset); > + } > > return twl_set(chip, offset, value); > } > @@ -388,15 +373,13 @@ static int twl_get_direction(struct gpio_chip *chip, unsigned offset) > */ > int ret = GPIO_LINE_DIRECTION_OUT; > > - mutex_lock(&priv->mutex); > + guard(mutex)(&priv->mutex); > if (offset < TWL4030_GPIO_MAX) { > ret = twl4030_get_gpio_direction(offset); > if (ret) { Drop the brackets if there's only one line left. Same elsewhere. > - mutex_unlock(&priv->mutex); > return ret; > } > } > - mutex_unlock(&priv->mutex); > > return ret; > } > -- > 2.53.0 > > Bart ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gpio: twl4030: Use guard(mutex)() over manual locking 2026-05-04 12:49 ` [PATCH] gpio: twl4030: Use guard(mutex)() over manual locking Bartosz Golaszewski @ 2026-05-04 13:05 ` Maxwell Doose 0 siblings, 0 replies; 3+ messages in thread From: Maxwell Doose @ 2026-05-04 13:05 UTC (permalink / raw) To: Bartosz Golaszewski; +Cc: linux-gpio, linux-kernel, linusw Hi Bartosz, On Mon, May 4, 2026 at 7:50 AM Bartosz Golaszewski <brgl@kernel.org> wrote: > > On Sat, 2 May 2026 23:03:54 +0200, Maxwell Doose <m32285159@gmail.com> said: > > Replace mutex_lock() and mutex_unlock() pairs with the RAII macro > > guard(mutex)(). This keeps the driver up-to-date with the latest > > function macros. > > > > Remove now-redundant gotos and goto labels which will maintain > > readability. In addition, replace some gotos with direct returns where > > appropriate. > > > > Update certain control paths to make them more suited to the new > > locking. > > > > Signed-off-by: Maxwell Doose <m32285159@gmail.com> > > --- > > drivers/gpio/gpio-twl4030.c | 69 ++++++++++++++----------------------- > > 1 file changed, 26 insertions(+), 43 deletions(-) > > > > diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c > > index a851702befde..df17f9c08817 100644 > > --- a/drivers/gpio/gpio-twl4030.c > > +++ b/drivers/gpio/gpio-twl4030.c > > @@ -23,6 +23,7 @@ > > #include <linux/platform_device.h> > > #include <linux/of.h> > > #include <linux/irqdomain.h> > > +#include <linux/cleanup.h> > > Maybe order the includes alphabetically if you're at it? > I'll make sure that happens in the v2. > > > > > #include <linux/mfd/twl.h> > > > > @@ -209,7 +210,7 @@ static int twl_request(struct gpio_chip *chip, unsigned offset) > > struct gpio_twl4030_priv *priv = gpiochip_get_data(chip); > > int status = 0; > > > > - mutex_lock(&priv->mutex); > > + guard(mutex)(&priv->mutex); > > > > /* Support the two LED outputs as output-only GPIOs. */ > > if (offset >= TWL4030_GPIO_MAX) { > > @@ -227,30 +228,29 @@ static int twl_request(struct gpio_chip *chip, unsigned offset) > > /* Configure PWM OFF register first */ > > status = twl_i2c_write_u8(TWL4030_MODULE_LED, 0x7f, reg + 1); > > if (status < 0) > > - goto done; > > + return status; > > > > /* Followed by PWM ON register */ > > status = twl_i2c_write_u8(TWL4030_MODULE_LED, 0x7f, reg); > > if (status < 0) > > - goto done; > > + return status; > > > > /* init LED to not-driven (high) */ > > status = twl_i2c_read_u8(TWL4030_MODULE_LED, &cached_leden, > > TWL4030_LED_LEDEN_REG); > > if (status < 0) > > - goto done; > > + return status; > > cached_leden &= ~ledclr_mask; > > status = twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden, > > TWL4030_LED_LEDEN_REG); > > if (status < 0) > > - goto done; > > + return status; > > > > status = 0; > > - goto done; > > } > > > > /* on first use, turn GPIO module "on" */ > > - if (!priv->usage_count) { > > + else if (!priv->usage_count) { > > This is a functional change, I'm not sure you want to change the logic. > The idea here was to drop the goto and just use "else if" instead, since I've found goto to bypass a lot of the semantics that __attribute__((cleanup())) enforces. The control paths that are taken are the same though. I can also see about finding a different solution but this one seemed like the best. > > > struct twl4030_gpio_platform_data *pdata; > > u8 value = MASK_GPIO_CTRL_GPIO_ON; > > > > @@ -264,11 +264,9 @@ static int twl_request(struct gpio_chip *chip, unsigned offset) > > status = gpio_twl4030_write(REG_GPIO_CTRL, value); > > } > > > > -done: > > if (!status) > > priv->usage_count |= BIT(offset); > > > > - mutex_unlock(&priv->mutex); > > return status; > > } > > > > @@ -276,10 +274,10 @@ static void twl_free(struct gpio_chip *chip, unsigned offset) > > { > > struct gpio_twl4030_priv *priv = gpiochip_get_data(chip); > > > > - mutex_lock(&priv->mutex); > > + guard(mutex)(&priv->mutex); > > if (offset >= TWL4030_GPIO_MAX) { > > WARN_ON_ONCE(twl4030_led_set_value(offset - TWL4030_GPIO_MAX, 1)); > > - goto out; > > + return; > > } > > > > priv->usage_count &= ~BIT(offset); > > @@ -287,9 +285,6 @@ static void twl_free(struct gpio_chip *chip, unsigned offset) > > /* on last use, switch off GPIO module */ > > if (!priv->usage_count) > > gpio_twl4030_write(REG_GPIO_CTRL, 0x0); > > - > > -out: > > - mutex_unlock(&priv->mutex); > > } > > > > static int twl_direction_in(struct gpio_chip *chip, unsigned offset) > > @@ -297,17 +292,15 @@ static int twl_direction_in(struct gpio_chip *chip, unsigned offset) > > struct gpio_twl4030_priv *priv = gpiochip_get_data(chip); > > int ret; > > > > - mutex_lock(&priv->mutex); > > + guard(mutex)(&priv->mutex); > > if (offset < TWL4030_GPIO_MAX) > > ret = twl4030_set_gpio_direction(offset, 1); > > else > > - ret = -EINVAL; /* LED outputs can't be set as input */ > > + return -EINVAL; /* LED outputs can't be set as input */ > > > > if (!ret) > > priv->direction &= ~BIT(offset); > > > > - mutex_unlock(&priv->mutex); > > - > > return ret; > > } > > > > @@ -317,10 +310,9 @@ static int twl_get(struct gpio_chip *chip, unsigned offset) > > int ret; > > int status = 0; > > > > - mutex_lock(&priv->mutex); > > + guard(mutex)(&priv->mutex); > > if (!(priv->usage_count & BIT(offset))) { > > - ret = -EPERM; > > - goto out; > > + return -EPERM; > > } > > > > if (priv->direction & BIT(offset)) > > @@ -328,10 +320,7 @@ static int twl_get(struct gpio_chip *chip, unsigned offset) > > else > > status = twl4030_get_gpio_datain(offset); > > > > - ret = (status < 0) ? status : !!status; > > -out: > > - mutex_unlock(&priv->mutex); > > - return ret; > > + return (status < 0) ? status : !!status; > > } > > > > static int twl_set(struct gpio_chip *chip, unsigned int offset, int value) > > @@ -339,7 +328,7 @@ static int twl_set(struct gpio_chip *chip, unsigned int offset, int value) > > struct gpio_twl4030_priv *priv = gpiochip_get_data(chip); > > int ret; > > > > - mutex_lock(&priv->mutex); > > + guard(mutex)(&priv->mutex); > > if (offset < TWL4030_GPIO_MAX) > > ret = twl4030_set_gpio_dataout(offset, value); > > else > > @@ -350,8 +339,6 @@ static int twl_set(struct gpio_chip *chip, unsigned int offset, int value) > > else > > priv->out_state &= ~BIT(offset); > > > > - mutex_unlock(&priv->mutex); > > - > > return ret; > > } > > > > @@ -360,21 +347,19 @@ static int twl_direction_out(struct gpio_chip *chip, unsigned offset, int value) > > struct gpio_twl4030_priv *priv = gpiochip_get_data(chip); > > int ret = 0; > > > > - mutex_lock(&priv->mutex); > > - if (offset < TWL4030_GPIO_MAX) { > > - ret = twl4030_set_gpio_direction(offset, 0); > > - if (ret) { > > - mutex_unlock(&priv->mutex); > > - return ret; > > + scoped_guard(mutex, &priv->mutex) { > > + if (offset < TWL4030_GPIO_MAX) { > > + ret = twl4030_set_gpio_direction(offset, 0); > > + if (ret) > > + return ret; > > } > > - } > > > > - /* > > - * LED gpios i.e. offset >= TWL4030_GPIO_MAX are always output > > - */ > > + /* > > + * LED gpios i.e. offset >= TWL4030_GPIO_MAX are always output > > + */ > > > > - priv->direction |= BIT(offset); > > - mutex_unlock(&priv->mutex); > > + priv->direction |= BIT(offset); > > + } > > > > return twl_set(chip, offset, value); > > } > > @@ -388,15 +373,13 @@ static int twl_get_direction(struct gpio_chip *chip, unsigned offset) > > */ > > int ret = GPIO_LINE_DIRECTION_OUT; > > > > - mutex_lock(&priv->mutex); > > + guard(mutex)(&priv->mutex); > > if (offset < TWL4030_GPIO_MAX) { > > ret = twl4030_get_gpio_direction(offset); > > if (ret) { > > Drop the brackets if there's only one line left. Same elsewhere. > Nice catch, I must've missed that in my final review. That'll be included in the v2. best regards, maxwell > > > > - mutex_unlock(&priv->mutex); > > return ret; > > } > > } > > - mutex_unlock(&priv->mutex); > > > > return ret; > > } > > -- > > 2.53.0 > > > > > > Bart ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gpio: twl4030: Use guard(mutex)() over manual locking [not found] <20260502210354.160439-1-m32285159@gmail.com> 2026-05-04 12:49 ` [PATCH] gpio: twl4030: Use guard(mutex)() over manual locking Bartosz Golaszewski @ 2026-05-13 19:33 ` kernel test robot 1 sibling, 0 replies; 3+ messages in thread From: kernel test robot @ 2026-05-13 19:33 UTC (permalink / raw) To: Maxwell Doose, linusw, brgl; +Cc: oe-kbuild-all, linux-gpio, linux-kernel Hi Maxwell, kernel test robot noticed the following build warnings: [auto build test WARNING on brgl/gpio/for-next] [also build test WARNING on linus/master v7.1-rc3 next-20260508] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Maxwell-Doose/gpio-twl4030-Use-guard-mutex-over-manual-locking/20260513-231548 base: https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git gpio/for-next patch link: https://lore.kernel.org/r/20260502210354.160439-1-m32285159%40gmail.com patch subject: [PATCH] gpio: twl4030: Use guard(mutex)() over manual locking config: arc-randconfig-002-20260514 (https://download.01.org/0day-ci/archive/20260514/202605140349.KZOdtI2I-lkp@intel.com/config) compiler: arc-linux-gcc (GCC) 8.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260514/202605140349.KZOdtI2I-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202605140349.KZOdtI2I-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/gpio/gpio-twl4030.c: In function 'twl_get': >> drivers/gpio/gpio-twl4030.c:310:6: warning: unused variable 'ret' [-Wunused-variable] int ret; ^~~ vim +/ret +310 drivers/gpio/gpio-twl4030.c e9d359471dfed51 drivers/gpio/twl4030-gpio.c David Brownell 2008-10-20 306 e9d359471dfed51 drivers/gpio/twl4030-gpio.c David Brownell 2008-10-20 307 static int twl_get(struct gpio_chip *chip, unsigned offset) e9d359471dfed51 drivers/gpio/twl4030-gpio.c David Brownell 2008-10-20 308 { 231a8680b78260f drivers/gpio/gpio-twl4030.c Linus Walleij 2015-12-07 309 struct gpio_twl4030_priv *priv = gpiochip_get_data(chip); c111feabe2e200b drivers/gpio/gpio-twl4030.c Peter Ujfalusi 2012-12-20 @310 int ret; e9d359471dfed51 drivers/gpio/twl4030-gpio.c David Brownell 2008-10-20 311 int status = 0; e9d359471dfed51 drivers/gpio/twl4030-gpio.c David Brownell 2008-10-20 312 b833f746820a65c drivers/gpio/gpio-twl4030.c Maxwell Doose 2026-05-02 313 guard(mutex)(&priv->mutex); c111feabe2e200b drivers/gpio/gpio-twl4030.c Peter Ujfalusi 2012-12-20 314 if (!(priv->usage_count & BIT(offset))) { b833f746820a65c drivers/gpio/gpio-twl4030.c Maxwell Doose 2026-05-02 315 return -EPERM; c111feabe2e200b drivers/gpio/gpio-twl4030.c Peter Ujfalusi 2012-12-20 316 } 72c7901ef00925c drivers/gpio/gpio-twl4030.c Peter Ujfalusi 2012-12-06 317 c111feabe2e200b drivers/gpio/gpio-twl4030.c Peter Ujfalusi 2012-12-20 318 if (priv->direction & BIT(offset)) c111feabe2e200b drivers/gpio/gpio-twl4030.c Peter Ujfalusi 2012-12-20 319 status = priv->out_state & BIT(offset); e9d359471dfed51 drivers/gpio/twl4030-gpio.c David Brownell 2008-10-20 320 else c111feabe2e200b drivers/gpio/gpio-twl4030.c Peter Ujfalusi 2012-12-20 321 status = twl4030_get_gpio_datain(offset); 72c7901ef00925c drivers/gpio/gpio-twl4030.c Peter Ujfalusi 2012-12-06 322 b833f746820a65c drivers/gpio/gpio-twl4030.c Maxwell Doose 2026-05-02 323 return (status < 0) ? status : !!status; e9d359471dfed51 drivers/gpio/twl4030-gpio.c David Brownell 2008-10-20 324 } e9d359471dfed51 drivers/gpio/twl4030-gpio.c David Brownell 2008-10-20 325 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-13 19:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260502210354.160439-1-m32285159@gmail.com>
2026-05-04 12:49 ` [PATCH] gpio: twl4030: Use guard(mutex)() over manual locking Bartosz Golaszewski
2026-05-04 13:05 ` Maxwell Doose
2026-05-13 19:33 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox