diff mbox series

gpio: sim: add lockdep asserts

Message ID 20240214104506.16771-1-brgl@bgdev.pl
State New
Headers show
Series gpio: sim: add lockdep asserts | expand

Commit Message

Bartosz Golaszewski Feb. 14, 2024, 10:45 a.m. UTC 
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

We have three functions in gpio-sim that are called with the device lock
already held. We use the "_unlocked" suffix in their names to indicate
that. This has proven to be confusing though as the naming convention in
the kernel varies between using "_locked" or "_unlocked" for this
purpose. Naming convention also doesn't enforce anything. Let's remove
the suffix and add lockdep annotation at the top of these functions.

This makes it clear the function requires a lock to be held (and which
one specifically!) as well as results in a warning if it's not the case.
The only place where the information is lost is the place where the
function is called but the caller doesn't care about that information
anyway.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpio-sim.c | 41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

Comments

Bartosz Golaszewski Feb. 23, 2024, 8:52 a.m. UTC | #1 
On Wed, Feb 14, 2024 at 11:45 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> We have three functions in gpio-sim that are called with the device lock
> already held. We use the "_unlocked" suffix in their names to indicate
> that. This has proven to be confusing though as the naming convention in
> the kernel varies between using "_locked" or "_unlocked" for this
> purpose. Naming convention also doesn't enforce anything. Let's remove
> the suffix and add lockdep annotation at the top of these functions.
>
> This makes it clear the function requires a lock to be held (and which
> one specifically!) as well as results in a warning if it's not the case.
> The only place where the information is lost is the place where the
> function is called but the caller doesn't care about that information
> anyway.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---

Patch queued for next.

Bart
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c
index c4106e37e6db..db42dc5616e4 100644
--- a/drivers/gpio/gpio-sim.c
+++ b/drivers/gpio/gpio-sim.c
@@ -22,6 +22,7 @@ 
 #include <linux/irq_sim.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
+#include <linux/lockdep.h>
 #include <linux/minmax.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
@@ -697,8 +698,10 @@  static struct gpio_sim_device *gpio_sim_hog_get_device(struct gpio_sim_hog *hog)
 	return gpio_sim_line_get_device(line);
 }
 
-static bool gpio_sim_device_is_live_unlocked(struct gpio_sim_device *dev)
+static bool gpio_sim_device_is_live(struct gpio_sim_device *dev)
 {
+	lockdep_assert_held(&dev->lock);
+
 	return !!dev->pdev;
 }
 
@@ -737,7 +740,7 @@  gpio_sim_device_config_live_show(struct config_item *item, char *page)
 	bool live;
 
 	scoped_guard(mutex, &dev->lock)
-		live = gpio_sim_device_is_live_unlocked(dev);
+		live = gpio_sim_device_is_live(dev);
 
 	return sprintf(page, "%c\n", live ? '1' : '0');
 }
@@ -926,7 +929,7 @@  static bool gpio_sim_bank_labels_non_unique(struct gpio_sim_device *dev)
 	return false;
 }
 
-static int gpio_sim_device_activate_unlocked(struct gpio_sim_device *dev)
+static int gpio_sim_device_activate(struct gpio_sim_device *dev)
 {
 	struct platform_device_info pdevinfo;
 	struct fwnode_handle *swnode;
@@ -934,6 +937,8 @@  static int gpio_sim_device_activate_unlocked(struct gpio_sim_device *dev)
 	struct gpio_sim_bank *bank;
 	int ret;
 
+	lockdep_assert_held(&dev->lock);
+
 	if (list_empty(&dev->bank_list))
 		return -ENODATA;
 
@@ -998,10 +1003,12 @@  static int gpio_sim_device_activate_unlocked(struct gpio_sim_device *dev)
 	return 0;
 }
 
-static void gpio_sim_device_deactivate_unlocked(struct gpio_sim_device *dev)
+static void gpio_sim_device_deactivate(struct gpio_sim_device *dev)
 {
 	struct fwnode_handle *swnode;
 
+	lockdep_assert_held(&dev->lock);
+
 	swnode = dev_fwnode(&dev->pdev->dev);
 	platform_device_unregister(dev->pdev);
 	gpio_sim_remove_hogs(dev);
@@ -1023,12 +1030,12 @@  gpio_sim_device_config_live_store(struct config_item *item,
 
 	guard(mutex)(&dev->lock);
 
-	if (live == gpio_sim_device_is_live_unlocked(dev))
+	if (live == gpio_sim_device_is_live(dev))
 		ret = -EPERM;
 	else if (live)
-		ret = gpio_sim_device_activate_unlocked(dev);
+		ret = gpio_sim_device_activate(dev);
 	else
-		gpio_sim_device_deactivate_unlocked(dev);
+		gpio_sim_device_deactivate(dev);
 
 	return ret ?: count;
 }
@@ -1069,7 +1076,7 @@  static ssize_t gpio_sim_bank_config_chip_name_show(struct config_item *item,
 
 	guard(mutex)(&dev->lock);
 
-	if (gpio_sim_device_is_live_unlocked(dev))
+	if (gpio_sim_device_is_live(dev))
 		return device_for_each_child(&dev->pdev->dev, &ctx,
 					     gpio_sim_emit_chip_name);
 
@@ -1098,7 +1105,7 @@  static ssize_t gpio_sim_bank_config_label_store(struct config_item *item,
 
 	guard(mutex)(&dev->lock);
 
-	if (gpio_sim_device_is_live_unlocked(dev))
+	if (gpio_sim_device_is_live(dev))
 		return -EBUSY;
 
 	trimmed = gpio_sim_strdup_trimmed(page, count);
@@ -1142,7 +1149,7 @@  gpio_sim_bank_config_num_lines_store(struct config_item *item,
 
 	guard(mutex)(&dev->lock);
 
-	if (gpio_sim_device_is_live_unlocked(dev))
+	if (gpio_sim_device_is_live(dev))
 		return -EBUSY;
 
 	bank->num_lines = num_lines;
@@ -1179,7 +1186,7 @@  static ssize_t gpio_sim_line_config_name_store(struct config_item *item,
 
 	guard(mutex)(&dev->lock);
 
-	if (gpio_sim_device_is_live_unlocked(dev))
+	if (gpio_sim_device_is_live(dev))
 		return -EBUSY;
 
 	trimmed = gpio_sim_strdup_trimmed(page, count);
@@ -1219,7 +1226,7 @@  static ssize_t gpio_sim_hog_config_name_store(struct config_item *item,
 
 	guard(mutex)(&dev->lock);
 
-	if (gpio_sim_device_is_live_unlocked(dev))
+	if (gpio_sim_device_is_live(dev))
 		return -EBUSY;
 
 	trimmed = gpio_sim_strdup_trimmed(page, count);
@@ -1274,7 +1281,7 @@  gpio_sim_hog_config_direction_store(struct config_item *item,
 
 	guard(mutex)(&dev->lock);
 
-	if (gpio_sim_device_is_live_unlocked(dev))
+	if (gpio_sim_device_is_live(dev))
 		return -EBUSY;
 
 	if (sysfs_streq(page, "input"))
@@ -1392,7 +1399,7 @@  gpio_sim_bank_config_make_line_group(struct config_group *group,
 
 	guard(mutex)(&dev->lock);
 
-	if (gpio_sim_device_is_live_unlocked(dev))
+	if (gpio_sim_device_is_live(dev))
 		return ERR_PTR(-EBUSY);
 
 	line = kzalloc(sizeof(*line), GFP_KERNEL);
@@ -1445,7 +1452,7 @@  gpio_sim_device_config_make_bank_group(struct config_group *group,
 
 	guard(mutex)(&dev->lock);
 
-	if (gpio_sim_device_is_live_unlocked(dev))
+	if (gpio_sim_device_is_live(dev))
 		return ERR_PTR(-EBUSY);
 
 	bank = kzalloc(sizeof(*bank), GFP_KERNEL);
@@ -1467,8 +1474,8 @@  static void gpio_sim_device_config_group_release(struct config_item *item)
 	struct gpio_sim_device *dev = to_gpio_sim_device(item);
 
 	scoped_guard(mutex, &dev->lock) {
-		if (gpio_sim_device_is_live_unlocked(dev))
-			gpio_sim_device_deactivate_unlocked(dev);
+		if (gpio_sim_device_is_live(dev))
+			gpio_sim_device_deactivate(dev);
 	}
 
 	mutex_destroy(&dev->lock);