Am 03.12.2015 um 18:08 schrieb Mauro Carvalho Chehab:
> Em Sat, 21 Nov 2015 16:30:56 +0100
> Heiner Kallweit <hkallweit1@xxxxxxxxx> escreveu:
>
>> 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.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
>> ---
>> drivers/media/rc/ir-lirc-codec.c | 4 ++++
>> drivers/media/rc/rc-main.c | 19 +++++--------------
>> 2 files changed, 9 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
>> index a32659f..7fc5b3f 100644
>> --- a/drivers/media/rc/ir-lirc-codec.c
>> +++ b/drivers/media/rc/ir-lirc-codec.c
>> @@ -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.
>
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.
> 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?
> Anyway, the way the patch is, it will break compilation, depending
> on the config options.
>
> Regards,
> Mauro
>
Rgds, Heiner
>
>> /**
>> * 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);
>> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
>> index 1042fa3..0bd11b4 100644
>> --- a/drivers/media/rc/rc-main.c
>> +++ b/drivers/media/rc/rc-main.c
>> @@ -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;
>> }
>>
>> /**
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
