media: rc: improve lirc module detection
Subsystems (2)
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
@@ -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
@@ -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
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_devinstead ofir_lirc_codec(which depends onlirc_dev). If their_lirc_codecmodule is unloaded the check would still return OK.Signed-off-by Heiner Kallweit <[email protected]>--- 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
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@@ -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 ; }/**
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_devinstead ofir_lirc_codec(which depends onlirc_dev). If their_lirc_codecmodule is unloaded the check would still return OK.Signed-off-by Heiner Kallweit <[email protected]>--- 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
-0drivers/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@@ -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 ; }/**