Skip to contentSkip to conversation

gpio: pca9570: Ad-hoc cleanups

PATCH Superseded Andy Shevchenko 3 participants 9 messages Jan 13 GPIO

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Andy Shevchenko

author
#0 lore raw

cover gpio: pca9570: Ad-hoc cleanups

Looking at this driver for something else I took the opportunity to clean it up. Hence this mini-series.

Andy Shevchenko (3):
gpio: pca9570: Use devm_mutex_init() for mutex initialization
gpio: pca9570: Don't use "proxy" headers
gpio: pca9570: use lock guards

 drivers/gpio/gpio-pca9570.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
Signature · 1 line
2.50.1
author
#1 lore raw

PATCH 1/3 gpio: pca9570: Use devm_mutex_init() for mutex initialization

Replace mutex_init() with the devm_mutex_init(), to ensure proper mutex cleanup during probe failure and driver removal.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/gpio/gpio-pca9570.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/gpio/gpio-pca9570.c +5 -1
index c5a1287079a0..eae35135c71e 100644
@@ -115,7 +115,9 @@ static int pca9570_set(struct gpio_chip *chip, unsigned int offset, int value)
 
 static int pca9570_probe(struct i2c_client *client)
 {
+	struct device *dev = &client->dev;
 	struct pca9570 *gpio;
+	int ret;
 
 	gpio = devm_kzalloc(&client->dev, sizeof(*gpio), GFP_KERNEL);
 	if (!gpio)

@@ -132,7 +134,9 @@ static int pca9570_probe(struct i2c_client *client)
 	gpio->chip.ngpio = gpio->chip_data->ngpio;
 	gpio->chip.can_sleep = true;
 
-	mutex_init(&gpio->lock);
+	ret = devm_mutex_init(dev, &gpio->lock);
+	if (ret)
+		return ret;
 
 	/* Read the current output level */
 	pca9570_read(gpio, &gpio->out);
Signature · 1 line
2.50.1
maintainer
#3 lore raw

Re: [PATCH v1 1/3] gpio: pca9570: Use devm_mutex_init() for mutex initialization

In reply to Andy Shevchenko
Quoted 4 lines · Andy Shevchenko

Replace mutex_init() with the devm_mutex_init(), to ensure proper mutex cleanup during probe failure and driver removal.

Well, I can almost hear Johan Hovold yelling at you that this is not about mutex cleanup but merely a useless debugging feature.

Though personally, I'm fine with it. Just reword the commit message because there's no actual cleanup happening other than freeing the memory allocated for the devres entry. :)

Bartosz

author
#4 lore raw

Re: [PATCH v1 1/3] gpio: pca9570: Use devm_mutex_init() for mutex initialization

Quoted 6 lines · Bartosz Golaszewski
Quoted 3 lines · Andy Shevchenko

Replace mutex_init() with the devm_mutex_init(), to ensure proper mutex cleanup during probe failure and driver removal.

Quoted 6 lines

Well, I can almost hear Johan Hovold yelling at you that this is not about mutex cleanup but merely a useless debugging feature.

Though personally, I'm fine with it. Just reword the commit message because there's no actual cleanup happening other than freeing the memory allocated for the devres entry. :)

v2 is sent, thanks for review!

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#5 lore raw

PATCH 2/3 gpio: pca9570: Don't use "proxy" headers

In reply to Andy Shevchenko

Update header inclusions to follow IWYU (Include What You Use) principle.

Note that kernel.h is discouraged to be included as it's written at the top of that file.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/gpio/gpio-pca9570.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/gpio/gpio-pca9570.c +4 -0
index eae35135c71e..a41837f1201b 100644
@@ -9,11 +9,15 @@
  *	Andrew F. Davis <[email protected]>
  */
 
+#include <linux/bits.h>
+#include <linux/device/devres.h>
+#include <linux/errno.h>
 #include <linux/gpio/driver.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/property.h>
+#include <linux/types.h>
 
 #define SLG7XL45106_GPO_REG	0xDB
Signature · 1 line
2.50.1
author
#7 lore raw

PATCH 3/3 gpio: pca9570: use lock guards

In reply to Andy Shevchenko

Shrink the code by a couple lines and improve lock management by using lock guards from cleanup.h.

Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/gpio/gpio-pca9570.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/gpio/gpio-pca9570.c +4 -5
index a41837f1201b..4a368803fb03 100644
@@ -10,6 +10,7 @@
  */
 
 #include <linux/bits.h>
+#include <linux/cleanup.h>
 #include <linux/device/devres.h>
 #include <linux/errno.h>
 #include <linux/gpio/driver.h>

@@ -98,7 +99,7 @@ static int pca9570_set(struct gpio_chip *chip, unsigned int offset, int value)
 	u8 buffer;
 	int ret;
 
-	mutex_lock(&gpio->lock);
+	guard(mutex)(&gpio->lock);
 
 	buffer = gpio->out;
 	if (value)

@@ -108,13 +109,11 @@ static int pca9570_set(struct gpio_chip *chip, unsigned int offset, int value)
 
 	ret = pca9570_write(gpio, buffer);
 	if (ret)
-		goto out;
+		return ret;
 
 	gpio->out = buffer;
 
-out:
-	mutex_unlock(&gpio->lock);
-	return ret;
+	return 0;
 }
 
 static int pca9570_probe(struct i2c_client *client)
Signature · 1 line
2.50.1