Hi Andy, On Fri, 1 Sep 2023, Andy Shevchenko wrote:
Use macros defined in linux/cleanup.h to automate resource lifetime control in gpio-pca953x. Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Thanks for your patch, which is now commit 8e471b784a720f6f ("gpio: pca953x: Simplify code with cleanup helpers") in gpio/gpio/for-next.
--- a/drivers/gpio/gpio-pca953x.c +++ b/drivers/gpio/gpio-pca953x.c @@ -557,9 +554,8 @@ static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off) u32 reg_val; int ret; - mutex_lock(&chip->i2c_lock); - ret = regmap_read(chip->regmap, inreg, ®_val); - mutex_unlock(&chip->i2c_lock); + scoped_guard(mutex, &chip->i2c_lock) + ret = regmap_read(chip->regmap, inreg, ®_val);
I can't say I'm thrilled about the lack of curly braces. I was also surprised to discover that checkpatch nor gcc W=1 complain about the indentation change. I know we don't use curly braces in single-statement for_each_*() loops, but at least these have the familiar "for"-prefix. And having the scope is very important here, so using braces, this would stand out more. Hence can we please get curly braces, like scoped_guard(mutex, &chip->i2c_lock) { ret = regmap_read(chip->regmap, inreg, ®_val); } ? Thanks! ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds