Skip to contentSkip to conversation

media: rc: improve lirc module detection

PATCH In review Heiner Kallweit 2 participants 3 messages Nov 21, 2015 – Dec 3, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 2/2 media: rc: improve lirc module detection

Improve detection whether lirc codec is loaded and avoid dependency on config symbols and having to check for a specific module.

This also fixes the bug that the current check checks for module lirc_dev instead of ir_lirc_codec (which depends on lirc_dev). If the ir_lirc_codec module is unloaded the check would still return OK.

---
 drivers/media/rc/ir-lirc-codec.c |  4 ++++
 drivers/media/rc/rc-main.c       | 19 +++++--------------
 2 files changed, 9 insertions(+), 14 deletions(-)
drivers/media/rc/ir-lirc-codec.c +4 -0
index a32659f..7fc5b3f 100644
@@ -22,6 +22,8 @@
 
 #define LIRCBUF_SIZE 256
 
+extern atomic_t ir_raw_lirc_available;
+
 /**
  * ir_lirc_decode() - Send raw IR data to lirc_dev to be relayed to the
  *		      lircd userspace daemon for decoding.
@@ -398,6 +400,7 @@ static int ir_lirc_register(struct rc_dev *dev)
 
 	dev->raw->lirc.drv = drv;
 	dev->raw->lirc.dev = dev;
+	atomic_set(&ir_raw_lirc_available, 1);
 	return 0;
 
 lirc_register_failed:
@@ -413,6 +416,7 @@ static int ir_lirc_unregister(struct rc_dev *dev)
 {
 	struct lirc_codec *lirc = &dev->raw->lirc;
 
+	atomic_set(&ir_raw_lirc_available, 0);
 	lirc_unregister_driver(lirc->drv->minor);
 	lirc_buffer_free(lirc->drv->rbuf);
 	kfree(lirc->drv);
drivers/media/rc/rc-main.c +5 -14
index 1042fa3..0bd11b4 100644
@@ -829,21 +829,12 @@ struct rc_filter_attribute {
 		.mask = (_mask),					\
 	}
 
-static bool lirc_is_present(void)
+atomic_t ir_raw_lirc_available = ATOMIC_INIT(0);
+EXPORT_SYMBOL(ir_raw_lirc_available);
+
+static inline bool lirc_is_present(void)
 {
-#if defined(CONFIG_LIRC_MODULE)
-	struct module *lirc;
-
-	mutex_lock(&module_mutex);
-	lirc = find_module("lirc_dev");
-	mutex_unlock(&module_mutex);
-
-	return lirc ? true : false;
-#elif defined(CONFIG_LIRC)
-	return true;
-#else
-	return false;
-#endif
+	return atomic_read(&ir_raw_lirc_available) != 0;
 }
 
 /**
Signature · 1 line
2.6.2
#1 lore raw

Em Sat, 21 Nov 2015 16:30:56 +0100
Heiner Kallweit <[email protected]> escreveu:

Quoted 19 lines

Improve detection whether lirc codec is loaded and avoid dependency on config symbols and having to check for a specific module.

This also fixes the bug that the current check checks for module lirc_dev instead of ir_lirc_codec (which depends on lirc_dev). If the ir_lirc_codec module is unloaded the check would still return OK.

---
 drivers/media/rc/ir-lirc-codec.c |  4 ++++
 drivers/media/rc/rc-main.c       | 19 +++++--------------
 2 files changed, 9 insertions(+), 14 deletions(-)
drivers/media/rc/ir-lirc-codec.c +0 -0
index a32659f..7fc5b3f 100644

drivers/media/rc/ir-lirc-codec.c line 26

Quoted 5 lines
drivers/media/rc/ir-lirc-codec.c +2 -0
@@ -22,6 +22,8 @@
 
 #define LIRCBUF_SIZE 256
 
+extern atomic_t ir_raw_lirc_available;
+

No, this is not good, due to several reasons:

- it lacks an EXPORT_SYMBOL_GPL(ir_raw_lirc_available);
- if lirc module is not compiled, the RC core will have a missed symbol.

The right thing to do is to have a var inside the RC core and a way for LIRC to tell the core that it was loaded.

I would prefer to not export a var. So, perhaps this could be done by calling some function.

Anyway, the way the patch is, it will break compilation, depending on the config options.

Regards,
Mauro

Quoted 50 lines
drivers/media/rc/ir-lirc-codec.c +2 -0
 /**
  * ir_lirc_decode() - Send raw IR data to lirc_dev to be relayed to the
  *		      lircd userspace daemon for decoding.
@@ -398,6 +400,7 @@ static int ir_lirc_register(struct rc_dev *dev)
 
 	dev->raw->lirc.drv = drv;
 	dev->raw->lirc.dev = dev;
+	atomic_set(&ir_raw_lirc_available, 1);
 	return 0;
 
 lirc_register_failed:
@@ -413,6 +416,7 @@ static int ir_lirc_unregister(struct rc_dev *dev)
 {
 	struct lirc_codec *lirc = &dev->raw->lirc;
 
+	atomic_set(&ir_raw_lirc_available, 0);
 	lirc_unregister_driver(lirc->drv->minor);
 	lirc_buffer_free(lirc->drv->rbuf);
 	kfree(lirc->drv);
drivers/media/rc/rc-main.c +5 -14
index 1042fa3..0bd11b4 100644
@@ -829,21 +829,12 @@ struct rc_filter_attribute {
 		.mask = (_mask),					\
 	}
 
-static bool lirc_is_present(void)
+atomic_t ir_raw_lirc_available = ATOMIC_INIT(0);
+EXPORT_SYMBOL(ir_raw_lirc_available);
+
+static inline bool lirc_is_present(void)
 {
-#if defined(CONFIG_LIRC_MODULE)
-	struct module *lirc;
-
-	mutex_lock(&module_mutex);
-	lirc = find_module("lirc_dev");
-	mutex_unlock(&module_mutex);
-
-	return lirc ? true : false;
-#elif defined(CONFIG_LIRC)
-	return true;
-#else
-	return false;
-#endif
+	return atomic_read(&ir_raw_lirc_available) != 0;
 }
 
 /**
#2 lore raw

Am 03.12.2015 um 18:08 schrieb Mauro Carvalho Chehab:

Quoted 30 lines

Em Sat, 21 Nov 2015 16:30:56 +0100
Heiner Kallweit <[email protected]> escreveu:

Quoted 19 lines

Improve detection whether lirc codec is loaded and avoid dependency on config symbols and having to check for a specific module.

This also fixes the bug that the current check checks for module lirc_dev instead of ir_lirc_codec (which depends on lirc_dev). If the ir_lirc_codec module is unloaded the check would still return OK.

---
 drivers/media/rc/ir-lirc-codec.c |  4 ++++
 drivers/media/rc/rc-main.c       | 19 +++++--------------
 2 files changed, 9 insertions(+), 14 deletions(-)
drivers/media/rc/ir-lirc-codec.c +0 -0
index a32659f..7fc5b3f 100644

drivers/media/rc/ir-lirc-codec.c line 26

Quoted 5 lines
drivers/media/rc/ir-lirc-codec.c +2 -0
@@ -22,6 +22,8 @@
 
 #define LIRCBUF_SIZE 256
 
+extern atomic_t ir_raw_lirc_available;
+

No, this is not good, due to several reasons:

Quoted 7 lines

- it lacks an EXPORT_SYMBOL_GPL(ir_raw_lirc_available);
- if lirc module is not compiled, the RC core will have a missed symbol.

The right thing to do is to have a var inside the RC core and a way for LIRC to tell the core that it was loaded.

IMHO that's actually the case.
The var is defined in rc_main.c and also exported from there. Your comment is at the position where the variable is declared as "extern" in the (potential) lirc module.

Quoted 3 lines

I would prefer to not export a var. So, perhaps this could be done by calling some function.

Sure, we could define the var static and add a public setter. Would you prefer this?

Quoted 6 lines

Anyway, the way the patch is, it will break compilation, depending on the config options.

Regards,
Mauro

Rgds, Heiner

Quoted 52 lines
Quoted 51 lines
drivers/media/rc/ir-lirc-codec.c +2 -0
 /**
  * ir_lirc_decode() - Send raw IR data to lirc_dev to be relayed to the
  *		      lircd userspace daemon for decoding.
@@ -398,6 +400,7 @@ static int ir_lirc_register(struct rc_dev *dev)
 
 	dev->raw->lirc.drv = drv;
 	dev->raw->lirc.dev = dev;
+	atomic_set(&ir_raw_lirc_available, 1);
 	return 0;
 
 lirc_register_failed:
@@ -413,6 +416,7 @@ static int ir_lirc_unregister(struct rc_dev *dev)
 {
 	struct lirc_codec *lirc = &dev->raw->lirc;
 
+	atomic_set(&ir_raw_lirc_available, 0);
 	lirc_unregister_driver(lirc->drv->minor);
 	lirc_buffer_free(lirc->drv->rbuf);
 	kfree(lirc->drv);
drivers/media/rc/rc-main.c +5 -14
index 1042fa3..0bd11b4 100644
@@ -829,21 +829,12 @@ struct rc_filter_attribute {
 		.mask = (_mask),					\
 	}
 
-static bool lirc_is_present(void)
+atomic_t ir_raw_lirc_available = ATOMIC_INIT(0);
+EXPORT_SYMBOL(ir_raw_lirc_available);
+
+static inline bool lirc_is_present(void)
 {
-#if defined(CONFIG_LIRC_MODULE)
-	struct module *lirc;
-
-	mutex_lock(&module_mutex);
-	lirc = find_module("lirc_dev");
-	mutex_unlock(&module_mutex);
-
-	return lirc ? true : false;
-#elif defined(CONFIG_LIRC)
-	return true;
-#else
-	return false;
-#endif
+	return atomic_read(&ir_raw_lirc_available) != 0;
 }
 
 /**