* 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