| Message ID | 20200904154547.3836-16-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | gpio: mockup: support dynamically created and removed chips | expand |
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 >
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
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?
On Mon, Sep 7, 2020 at 1:50 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > 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? > But ida_destroy() and ida_free() are already well documented. It's clear that we remove all devices at exit and that every device removes its ID, there's really no point in mentioning it again. Bart
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;