From: Bartosz Golaszewski <[email protected]>
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 <[email protected]>
---
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 04, 2020 at 05:45:39PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>
--
With Best Regards,
Andy Shevchenko
On Mon, Sep 07, 2020 at 01:04:29PM +0200, Bartosz Golaszewski wrote:
> On Fri, Sep 4, 2020 at 6:49 PM Andy Shevchenko
> <[email protected]> wrote:
> > On Fri, Sep 04, 2020 at 05:45:39PM +0200, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <[email protected]>
> > >
> > > 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?
--
With Best Regards,
Andy Shevchenko
On Mon, Sep 7, 2020 at 1:50 PM Andy Shevchenko
<[email protected]> 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
> > <[email protected]> wrote:
> > > On Fri, Sep 04, 2020 at 05:45:39PM +0200, Bartosz Golaszewski wrote:
> > > > From: Bartosz Golaszewski <[email protected]>
> > > >
> > > > 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
On Fri, Sep 4, 2020 at 6:49 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Fri, Sep 04, 2020 at 05:45:39PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <[email protected]>
> >
> > 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