[15/23] gpio: mockup: use dynamic device IDs

Message ID 20200904154547.3836-16-brgl@bgdev.pl
State New
Headers show
Series
  • gpio: mockup: support dynamically created and removed chips
Related show

Commit Message

Bartosz Golaszewski Sept. 4, 2020, 3:45 p.m.
From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

We're currently creating chips at module init time only so using a
static index for dummy devices is fine. We want to support dynamically
created chips however so we need to switch to dynamic device IDs.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/gpio/gpio-mockup.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Andy Shevchenko Sept. 4, 2020, 4:49 p.m. | #1
On Fri, Sep 04, 2020 at 05:45:39PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> We're currently creating chips at module init time only so using a
> static index for dummy devices is fine. We want to support dynamically
> created chips however so we need to switch to dynamic device IDs.

It misses ida_destroy().

What about XArray API?

> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  drivers/gpio/gpio-mockup.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
> index 96976ba66598..995e37fef9c5 100644
> --- a/drivers/gpio/gpio-mockup.c
> +++ b/drivers/gpio/gpio-mockup.c
> @@ -9,6 +9,7 @@
>  
>  #include <linux/debugfs.h>
>  #include <linux/gpio/driver.h>
> +#include <linux/idr.h>
>  #include <linux/interrupt.h>
>  #include <linux/irq.h>
>  #include <linux/irq_sim.h>
> @@ -70,6 +71,8 @@ module_param_named(gpio_mockup_named_lines,
>  
>  static struct dentry *gpio_mockup_dbg_dir;
>  
> +static DEFINE_IDA(gpio_mockup_ida);
> +
>  static int gpio_mockup_range_base(unsigned int index)
>  {
>  	return gpio_mockup_ranges[index * 2];
> @@ -480,8 +483,12 @@ static LIST_HEAD(gpio_mockup_devices);
>  
>  static void gpio_mockup_unregister_one_device(struct gpio_mockup_device *dev)
>  {
> +	int id;
> +
>  	list_del(&dev->list);
> +	id = dev->pdev->id;
>  	platform_device_unregister(dev->pdev);
> +	ida_free(&gpio_mockup_ida, id);
>  	kfree(dev);
>  }
>  
> @@ -587,12 +594,19 @@ static int __init gpio_mockup_init(void)
>  		}
>  
>  		pdevinfo.name = "gpio-mockup";
> -		pdevinfo.id = i;
>  		pdevinfo.properties = properties;
>  
> +		pdevinfo.id = ida_alloc(&gpio_mockup_ida, GFP_KERNEL);
> +		if (pdevinfo.id < 0) {
> +			kfree_strarray(line_names, ngpio);
> +			err = pdevinfo.id;
> +			goto err_out;
> +		}
> +
>  		mockup_dev = kzalloc(sizeof(*mockup_dev), GFP_KERNEL);
>  		if (!mockup_dev) {
>  			kfree_strarray(line_names, ngpio);
> +			ida_free(&gpio_mockup_ida, pdevinfo.id);
>  			err = -ENOMEM;
>  			goto err_out;
>  		}
> @@ -601,6 +615,7 @@ static int __init gpio_mockup_init(void)
>  		kfree_strarray(line_names, ngpio);
>  		if (IS_ERR(mockup_dev->pdev)) {
>  			pr_err("error registering device");
> +			ida_free(&gpio_mockup_ida, pdevinfo.id);
>  			kfree(mockup_dev);
>  			err = PTR_ERR(mockup_dev->pdev);
>  			goto err_out;
> -- 
> 2.26.1
>
Bartosz Golaszewski Sept. 7, 2020, 11:04 a.m. | #2
On Fri, Sep 4, 2020 at 6:49 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Sep 04, 2020 at 05:45:39PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > We're currently creating chips at module init time only so using a
> > static index for dummy devices is fine. We want to support dynamically
> > created chips however so we need to switch to dynamic device IDs.
>
> It misses ida_destroy().
>

No, we always call ida_free() for separate IDs when removing devices
and we remove all devices at module exit so no need to call
ida_destroy().

> What about XArray API?
>

Answered that somewhere - xarray is already used internally by IDA.

Bart
Andy Shevchenko Sept. 7, 2020, 11:50 a.m. | #3
On Mon, Sep 07, 2020 at 01:04:29PM +0200, Bartosz Golaszewski wrote:
> On Fri, Sep 4, 2020 at 6:49 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Fri, Sep 04, 2020 at 05:45:39PM +0200, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > We're currently creating chips at module init time only so using a
> > > static index for dummy devices is fine. We want to support dynamically
> > > created chips however so we need to switch to dynamic device IDs.
> >
> > It misses ida_destroy().
> 
> No, we always call ida_free() for separate IDs when removing devices
> and we remove all devices at module exit so no need to call
> ida_destroy().

Perhaps couple of words about this in the commit message?

Patch

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index 96976ba66598..995e37fef9c5 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -9,6 +9,7 @@ 
 
 #include <linux/debugfs.h>
 #include <linux/gpio/driver.h>
+#include <linux/idr.h>
 #include <linux/interrupt.h>
 #include <linux/irq.h>
 #include <linux/irq_sim.h>
@@ -70,6 +71,8 @@  module_param_named(gpio_mockup_named_lines,
 
 static struct dentry *gpio_mockup_dbg_dir;
 
+static DEFINE_IDA(gpio_mockup_ida);
+
 static int gpio_mockup_range_base(unsigned int index)
 {
 	return gpio_mockup_ranges[index * 2];
@@ -480,8 +483,12 @@  static LIST_HEAD(gpio_mockup_devices);
 
 static void gpio_mockup_unregister_one_device(struct gpio_mockup_device *dev)
 {
+	int id;
+
 	list_del(&dev->list);
+	id = dev->pdev->id;
 	platform_device_unregister(dev->pdev);
+	ida_free(&gpio_mockup_ida, id);
 	kfree(dev);
 }
 
@@ -587,12 +594,19 @@  static int __init gpio_mockup_init(void)
 		}
 
 		pdevinfo.name = "gpio-mockup";
-		pdevinfo.id = i;
 		pdevinfo.properties = properties;
 
+		pdevinfo.id = ida_alloc(&gpio_mockup_ida, GFP_KERNEL);
+		if (pdevinfo.id < 0) {
+			kfree_strarray(line_names, ngpio);
+			err = pdevinfo.id;
+			goto err_out;
+		}
+
 		mockup_dev = kzalloc(sizeof(*mockup_dev), GFP_KERNEL);
 		if (!mockup_dev) {
 			kfree_strarray(line_names, ngpio);
+			ida_free(&gpio_mockup_ida, pdevinfo.id);
 			err = -ENOMEM;
 			goto err_out;
 		}
@@ -601,6 +615,7 @@  static int __init gpio_mockup_init(void)
 		kfree_strarray(line_names, ngpio);
 		if (IS_ERR(mockup_dev->pdev)) {
 			pr_err("error registering device");
+			ida_free(&gpio_mockup_ida, pdevinfo.id);
 			kfree(mockup_dev);
 			err = PTR_ERR(mockup_dev->pdev);
 			goto err_out;