Re: [PATCH] rc-core: use an IDA rather than a bitmap
On Thu, May 14, 2015 at 05:29:29PM -0300, Mauro Carvalho Chehab wrote:
Em Thu, 02 Apr 2015 12:18:55 +0200
David Härdeman da...@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.
I didn't add it 'cause I wanted Stefan to test it first. Seems he's done
so...so I'll resubmit with my SOB and his Tested-by...
---
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);
+
Re: [PATCH] rc-core: use an IDA rather than a bitmap
Em Thu, 02 Apr 2015 12:18:55 +0200
David Härdeman da...@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
Re: [PATCH] rc-core: use an IDA rather than a bitmap
Hi On 2015-05-14, Mauro Carvalho Chehab wrote: Em Thu, 02 Apr 2015 12:18:55 +0200 David Härdeman da...@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. Just to follow this up, with this patch applied, I've never (for the last 6 weeks already) seen sysfs file conflicts again. So now I'm pretty confident that it really fixes my original problem. Feel free to add the tested-by tag, if you like. Tested-by: Stefan Lippers-Hollmann s@gmx.de Thanks a lot Stefan Lippers-Hollmann pgpSCzEChEoRA.pgp Description: Digitale Signatur von OpenPGP
Re: [PATCH] rc-core: use an IDA rather than a bitmap
Hi On 2015-04-02, David Härdeman wrote: 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? --- 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(-) Just some preliminary feedback after two weeks of testing. So far the problem with multiple rc_core devices fighting over the same sysfs file hasn't occured again, but it's a bit too early to be completely sure about it (probably 4-5 more weeks before I can be (relatively) confident that the problem is really gone). With this patch applied, everything continues to work fine for me; I haven't noticed any problems. Thanks a lot Stefan Lippers-Hollmann -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rc-core: use an IDA rather than a bitmap
Hi On 2015-04-02, David Härdeman wrote: 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? Thanks, I've applied this patch to my kernel and have started testing it. As the race conditions between both (well, three, the TeVii s480 v2.1 exposes two rc_core devices) doesn't trigger all the time, it might take a while before I can report back. Thanks a lot Stefan Lippers-Hollmann pgpoSevqeY_Li.pgp Description: Digitale Signatur von OpenPGP
