Re: [PATCH] rc-core: use an IDA rather than a bitmap

Mauro Carvalho Chehab
2015-05-14T20:29:38Z

Em Thu, 02 Apr 2015 12:18:55 +0200
David Härdeman <david@hardeman.nu> escreveu:

> This patch changes rc-core to use the kernel facilities that are already
> available for handling unique numbers instead of rolling its own bitmap
> stuff.
>
> Stefan, this should apply cleanly to the media git tree...could you test it?

Patch looks good to me but...

you forgot to add your SOB on it.

> ---
> drivers/media/rc/rc-ir-raw.c | 2 +-
> drivers/media/rc/rc-main.c | 40 ++++++++++++++++++++--------------------
> include/media/rc-core.h | 4 ++--
> 3 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
> index b732ac6..ad26052 100644
> --- a/drivers/media/rc/rc-ir-raw.c
> +++ b/drivers/media/rc/rc-ir-raw.c
> @@ -271,7 +271,7 @@ int ir_raw_event_register(struct rc_dev *dev)
>
> spin_lock_init(&dev->raw->lock);
> dev->raw->thread = kthread_run(ir_raw_event_thread, dev->raw,
> - "rc%ld", dev->devno);
> + "rc%u", dev->minor);
>
> if (IS_ERR(dev->raw->thread)) {
> rc = PTR_ERR(dev->raw->thread);
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index f8c5e47..d068c4e 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -18,17 +18,15 @@
> #include <linux/input.h>
> #include <linux/leds.h>
> #include <linux/slab.h>
> +#include <linux/idr.h>
> #include <linux/device.h>
> #include <linux/module.h>
> #include "rc-core-priv.h"
>
> -/* Bitmap to store allocated device numbers from 0 to IRRCV_NUM_DEVICES - 1 */
> -#define IRRCV_NUM_DEVICES 256
> -static DECLARE_BITMAP(ir_core_dev_number, IRRCV_NUM_DEVICES);
> -
> /* Sizes are in bytes, 256 bytes allows for 32 entries on x64 */
> #define IR_TAB_MIN_SIZE 256
> #define IR_TAB_MAX_SIZE 8192
> +#define RC_DEV_MAX 256
>
> /* FIXME: IR_KEYPRESS_TIMEOUT should be protocol specific */
> #define IR_KEYPRESS_TIMEOUT 250
> @@ -38,6 +36,9 @@ static LIST_HEAD(rc_map_list);
> static DEFINE_SPINLOCK(rc_map_lock);
> static struct led_trigger *led_feedback;
>
> +/* Used to keep track of rc devices */
> +static DEFINE_IDA(rc_ida);
> +
> static struct rc_map_list *seek_rc_map(const char *name)
> {
> struct rc_map_list *map = NULL;
> @@ -1312,7 +1313,9 @@ int rc_register_device(struct rc_dev *dev)
> static bool raw_init = false; /* raw decoders loaded? */
> struct rc_map *rc_map;
> const char *path;
> - int rc, devno, attr = 0;
> + int attr = 0;
> + int minor;
> + int rc;
>
> if (!dev || !dev->map_name)
> return -EINVAL;
> @@ -1332,13 +1335,13 @@ int rc_register_device(struct rc_dev *dev)
> if (dev->close)
> dev->input_dev->close = ir_close;
>
> - do {
> - devno = find_first_zero_bit(ir_core_dev_number,
> - IRRCV_NUM_DEVICES);
> - /* No free device slots */
> - if (devno >= IRRCV_NUM_DEVICES)
> - return -ENOMEM;
> - } while (test_and_set_bit(devno, ir_core_dev_number));
> + minor = ida_simple_get(&rc_ida, 0, RC_DEV_MAX, GFP_KERNEL);
> + if (minor < 0)
> + return minor;
> +
> + dev->minor = minor;
> + dev_set_name(&dev->dev, "rc%u", dev->minor);
> + dev_set_drvdata(&dev->dev, dev);
>
> dev->dev.groups = dev->sysfs_groups;
> dev->sysfs_groups[attr++] = &rc_dev_protocol_attr_grp;
> @@ -1358,9 +1361,6 @@ int rc_register_device(struct rc_dev *dev)
> */
> mutex_lock(&dev->lock);
>
> - dev->devno = devno;
> - dev_set_name(&dev->dev, "rc%ld", dev->devno);
> - dev_set_drvdata(&dev->dev, dev);
> rc = device_add(&dev->dev);
> if (rc)
> goto out_unlock;
> @@ -1433,8 +1433,8 @@ int rc_register_device(struct rc_dev *dev)
>
> mutex_unlock(&dev->lock);
>
> - IR_dprintk(1, "Registered rc%ld (driver: %s, remote: %s, mode %s)\n",
> - dev->devno,
> + IR_dprintk(1, "Registered rc%u (driver: %s, remote: %s, mode %s)\n",
> + dev->minor,
> dev->driver_name ? dev->driver_name : "unknown",
> rc_map->name ? rc_map->name : "unknown",
> dev->driver_type == RC_DRIVER_IR_RAW ? "raw" : "cooked");
> @@ -1453,7 +1453,7 @@ out_dev:
> device_del(&dev->dev);
> out_unlock:
> mutex_unlock(&dev->lock);
> - clear_bit(dev->devno, ir_core_dev_number);
> + ida_simple_remove(&rc_ida, minor);
> return rc;
> }
> EXPORT_SYMBOL_GPL(rc_register_device);
> @@ -1465,8 +1465,6 @@ void rc_unregister_device(struct rc_dev *dev)
>
> del_timer_sync(&dev->timer_keyup);
>
> - clear_bit(dev->devno, ir_core_dev_number);
> -
> if (dev->driver_type == RC_DRIVER_IR_RAW)
> ir_raw_event_unregister(dev);
>
> @@ -1479,6 +1477,8 @@ void rc_unregister_device(struct rc_dev *dev)
>
> device_del(&dev->dev);
>
> + ida_simple_remove(&rc_ida, dev->minor);
> +
> rc_free_device(dev);
> }
>
> diff --git a/include/media/rc-core.h b/include/media/rc-core.h
> index 2c7fbca..6b4400c 100644
> --- a/include/media/rc-core.h
> +++ b/include/media/rc-core.h
> @@ -69,7 +69,7 @@ enum rc_filter_type {
> * @rc_map: current scan/key table
> * @lock: used to ensure we've filled in all protocol details before
> * anyone can call show_protocols or store_protocols
> - * @devno: unique remote control device number
> + * @minor: unique minor remote control device number
> * @raw: additional data for raw pulse/space devices
> * @input_dev: the input child device used to communicate events to userspace
> * @driver_type: specifies if protocol decoding is done in hardware or software
> @@ -129,7 +129,7 @@ struct rc_dev {
> const char *map_name;
> struct rc_map rc_map;
> struct mutex lock;
> - unsigned long devno;
> + unsigned int minor;
> struct ir_raw_event_ctrl *raw;
> struct input_dev *input_dev;
> enum rc_driver_type driver_type;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html