All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media/rc: compile rc-cec.c into rc-core
@ 2021-02-18 16:13 Hans Verkuil
  2021-02-20 10:58 ` Sean Young
  2021-02-20 11:56 ` Hans de Goede
  0 siblings, 2 replies; 4+ messages in thread
From: Hans Verkuil @ 2021-02-18 16:13 UTC (permalink / raw)
  To: Sean Young, Linux Media Mailing List; +Cc: Hans de Goede

The rc-cec keymap is unusual in that it can't be built as a module,
instead it is registered directly in rc-main.c if CONFIG_MEDIA_CEC_RC
is set. This is because it can be called from drm_dp_cec_set_edid() via
cec_register_adapter() in an asynchronous context, and it is not
allowed to use request_module() to load rc-cec.ko in that case. Trying to
do so results in a 'WARN_ON_ONCE(wait && current_is_async())'.

Since this keymap is only used if CONFIG_MEDIA_CEC_RC is set, we
just compile this keymap into the rc-core module and never as a
separate module.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Fixes: 2c6d1fffa1d9 (drm: add support for DisplayPort CEC-Tunneling-over-AUX)
Reported-by: Hans de Goede <hdegoede@redhat.com>
---
Sean, I tested this both with RC_CORE=y and =m.

It looks reasonably clean to me, but I am not sure if this affects v4l-utils
as well. Let me know what you think.

Regards,

	Hans
---
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index 5bb2932ab119..ff6a8fc4c38e 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -5,6 +5,7 @@ obj-y += keymaps/
 obj-$(CONFIG_RC_CORE) += rc-core.o
 rc-core-y := rc-main.o rc-ir-raw.o
 rc-core-$(CONFIG_LIRC) += lirc_dev.o
+rc-core-$(CONFIG_MEDIA_CEC_RC) += keymaps/rc-cec.o
 rc-core-$(CONFIG_BPF_LIRC_MODE2) += bpf-lirc.o
 obj-$(CONFIG_IR_NEC_DECODER) += ir-nec-decoder.o
 obj-$(CONFIG_IR_RC5_DECODER) += ir-rc5-decoder.o
diff --git a/drivers/media/rc/keymaps/Makefile b/drivers/media/rc/keymaps/Makefile
index b252a1d2ebd6..cc6662e1903f 100644
--- a/drivers/media/rc/keymaps/Makefile
+++ b/drivers/media/rc/keymaps/Makefile
@@ -21,7 +21,6 @@ obj-$(CONFIG_RC_MAP) += rc-adstech-dvb-t-pci.o \
 			rc-behold.o \
 			rc-behold-columbus.o \
 			rc-budget-ci-old.o \
-			rc-cec.o \
 			rc-cinergy-1400.o \
 			rc-cinergy.o \
 			rc-d680-dmb.o \
diff --git a/drivers/media/rc/keymaps/rc-cec.c b/drivers/media/rc/keymaps/rc-cec.c
index 3e3bd11092b4..068e22aeac8c 100644
--- a/drivers/media/rc/keymaps/rc-cec.c
+++ b/drivers/media/rc/keymaps/rc-cec.c
@@ -1,5 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /* Keytable for the CEC remote control
+ *
+ * This keymap is unusual in that it can't be built as a module,
+ * instead it is registered directly in rc-main.c if CONFIG_MEDIA_CEC_RC
+ * is set. This is because it can be called from drm_dp_cec_set_edid() via
+ * cec_register_adapter() in an asynchronous context, and it is not
+ * allowed to use request_module() to load rc-cec.ko in that case.
+ *
+ * Since this keymap is only used if CONFIG_MEDIA_CEC_RC is set, we
+ * just compile this keymap into the rc-core module and never as a
+ * separate module.
  *
  * Copyright (c) 2015 by Kamil Debski
  */
@@ -152,7 +162,7 @@ static struct rc_map_table cec[] = {
 	/* 0x77-0xff: Reserved */
 };

-static struct rc_map_list cec_map = {
+struct rc_map_list cec_map = {
 	.map = {
 		.scan		= cec,
 		.size		= ARRAY_SIZE(cec),
@@ -160,19 +170,3 @@ static struct rc_map_list cec_map = {
 		.name		= RC_MAP_CEC,
 	}
 };
-
-static int __init init_rc_map_cec(void)
-{
-	return rc_map_register(&cec_map);
-}
-
-static void __exit exit_rc_map_cec(void)
-{
-	rc_map_unregister(&cec_map);
-}
-
-module_init(init_rc_map_cec);
-module_exit(exit_rc_map_cec);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Kamil Debski");
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 1fd62c1dac76..4768b1e9ffd1 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -163,6 +163,10 @@ static struct rc_map_list empty_map = {
 	}
 };

+#ifdef CONFIG_MEDIA_CEC_RC
+extern struct rc_map_list cec_map;
+#endif
+
 /**
  * scancode_to_u64() - converts scancode in &struct input_keymap_entry
  * @ke: keymap entry containing scancode to be converted.
@@ -2069,6 +2073,9 @@ static int __init rc_core_init(void)

 	led_trigger_register_simple("rc-feedback", &led_feedback);
 	rc_map_register(&empty_map);
+#ifdef CONFIG_MEDIA_CEC_RC
+	rc_map_register(&cec_map);
+#endif

 	return 0;
 }
@@ -2078,6 +2085,9 @@ static void __exit rc_core_exit(void)
 	lirc_dev_exit();
 	class_unregister(&rc_class);
 	led_trigger_unregister_simple(led_feedback);
+#ifdef CONFIG_MEDIA_CEC_RC
+	rc_map_register(&cec_map);
+#endif
 	rc_map_unregister(&empty_map);
 }


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] media/rc: compile rc-cec.c into rc-core
  2021-02-18 16:13 [PATCH] media/rc: compile rc-cec.c into rc-core Hans Verkuil
@ 2021-02-20 10:58 ` Sean Young
  2021-02-23 10:51   ` Hans Verkuil
  2021-02-20 11:56 ` Hans de Goede
  1 sibling, 1 reply; 4+ messages in thread
From: Sean Young @ 2021-02-20 10:58 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: Linux Media Mailing List, Hans de Goede

Hi Hans,

Thank you for writing this patch.

On Thu, Feb 18, 2021 at 05:13:08PM +0100, Hans Verkuil wrote:
> The rc-cec keymap is unusual in that it can't be built as a module,
> instead it is registered directly in rc-main.c if CONFIG_MEDIA_CEC_RC
> is set. This is because it can be called from drm_dp_cec_set_edid() via
> cec_register_adapter() in an asynchronous context, and it is not
> allowed to use request_module() to load rc-cec.ko in that case. Trying to
> do so results in a 'WARN_ON_ONCE(wait && current_is_async())'.
> 
> Since this keymap is only used if CONFIG_MEDIA_CEC_RC is set, we
> just compile this keymap into the rc-core module and never as a
> separate module.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Fixes: 2c6d1fffa1d9 (drm: add support for DisplayPort CEC-Tunneling-over-AUX)
> Reported-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Sean, I tested this both with RC_CORE=y and =m.
> 
> It looks reasonably clean to me, but I am not sure if this affects v4l-utils
> as well. Let me know what you think.
> 
> Regards,
> 
> 	Hans
> ---
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index 5bb2932ab119..ff6a8fc4c38e 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -5,6 +5,7 @@ obj-y += keymaps/
>  obj-$(CONFIG_RC_CORE) += rc-core.o
>  rc-core-y := rc-main.o rc-ir-raw.o
>  rc-core-$(CONFIG_LIRC) += lirc_dev.o
> +rc-core-$(CONFIG_MEDIA_CEC_RC) += keymaps/rc-cec.o
>  rc-core-$(CONFIG_BPF_LIRC_MODE2) += bpf-lirc.o
>  obj-$(CONFIG_IR_NEC_DECODER) += ir-nec-decoder.o
>  obj-$(CONFIG_IR_RC5_DECODER) += ir-rc5-decoder.o
> diff --git a/drivers/media/rc/keymaps/Makefile b/drivers/media/rc/keymaps/Makefile
> index b252a1d2ebd6..cc6662e1903f 100644
> --- a/drivers/media/rc/keymaps/Makefile
> +++ b/drivers/media/rc/keymaps/Makefile
> @@ -21,7 +21,6 @@ obj-$(CONFIG_RC_MAP) += rc-adstech-dvb-t-pci.o \
>  			rc-behold.o \
>  			rc-behold-columbus.o \
>  			rc-budget-ci-old.o \
> -			rc-cec.o \
>  			rc-cinergy-1400.o \
>  			rc-cinergy.o \
>  			rc-d680-dmb.o \

This is a great idea. If rc-core is built as a module, the cec keymap is 
included in the module, and if cec-rc is not enabled then it is not built
at all.

> diff --git a/drivers/media/rc/keymaps/rc-cec.c b/drivers/media/rc/keymaps/rc-cec.c
> index 3e3bd11092b4..068e22aeac8c 100644
> --- a/drivers/media/rc/keymaps/rc-cec.c
> +++ b/drivers/media/rc/keymaps/rc-cec.c
> @@ -1,5 +1,15 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /* Keytable for the CEC remote control
> + *
> + * This keymap is unusual in that it can't be built as a module,
> + * instead it is registered directly in rc-main.c if CONFIG_MEDIA_CEC_RC
> + * is set. This is because it can be called from drm_dp_cec_set_edid() via
> + * cec_register_adapter() in an asynchronous context, and it is not
> + * allowed to use request_module() to load rc-cec.ko in that case.
> + *
> + * Since this keymap is only used if CONFIG_MEDIA_CEC_RC is set, we
> + * just compile this keymap into the rc-core module and never as a
> + * separate module.
>   *
>   * Copyright (c) 2015 by Kamil Debski
>   */
> @@ -152,7 +162,7 @@ static struct rc_map_table cec[] = {
>  	/* 0x77-0xff: Reserved */
>  };
> 
> -static struct rc_map_list cec_map = {
> +struct rc_map_list cec_map = {
>  	.map = {
>  		.scan		= cec,
>  		.size		= ARRAY_SIZE(cec),
> @@ -160,19 +170,3 @@ static struct rc_map_list cec_map = {
>  		.name		= RC_MAP_CEC,
>  	}
>  };
> -
> -static int __init init_rc_map_cec(void)
> -{
> -	return rc_map_register(&cec_map);
> -}
> -
> -static void __exit exit_rc_map_cec(void)
> -{
> -	rc_map_unregister(&cec_map);
> -}
> -
> -module_init(init_rc_map_cec);
> -module_exit(exit_rc_map_cec);
> -
> -MODULE_LICENSE("GPL");
> -MODULE_AUTHOR("Kamil Debski");
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index 1fd62c1dac76..4768b1e9ffd1 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -163,6 +163,10 @@ static struct rc_map_list empty_map = {
>  	}
>  };
> 
> +#ifdef CONFIG_MEDIA_CEC_RC
> +extern struct rc_map_list cec_map;
> +#endif
> +
>  /**
>   * scancode_to_u64() - converts scancode in &struct input_keymap_entry
>   * @ke: keymap entry containing scancode to be converted.
> @@ -2069,6 +2073,9 @@ static int __init rc_core_init(void)
> 
>  	led_trigger_register_simple("rc-feedback", &led_feedback);
>  	rc_map_register(&empty_map);
> +#ifdef CONFIG_MEDIA_CEC_RC
> +	rc_map_register(&cec_map);
> +#endif
> 
>  	return 0;
>  }
> @@ -2078,6 +2085,9 @@ static void __exit rc_core_exit(void)
>  	lirc_dev_exit();
>  	class_unregister(&rc_class);
>  	led_trigger_unregister_simple(led_feedback);
> +#ifdef CONFIG_MEDIA_CEC_RC
> +	rc_map_register(&cec_map);
> +#endif
>  	rc_map_unregister(&empty_map);
>  }

Now I'm confused. Do we need these changes at all? I've experimented a little
bit (without the right hardware), and as far as I can make out, just the
changes to the Kconfig/Makefile should be enough.

Thanks,

Sean

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] media/rc: compile rc-cec.c into rc-core
  2021-02-18 16:13 [PATCH] media/rc: compile rc-cec.c into rc-core Hans Verkuil
  2021-02-20 10:58 ` Sean Young
@ 2021-02-20 11:56 ` Hans de Goede
  1 sibling, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2021-02-20 11:56 UTC (permalink / raw)
  To: Hans Verkuil, Sean Young, Linux Media Mailing List

Hi,

On 2/18/21 5:13 PM, Hans Verkuil wrote:
> The rc-cec keymap is unusual in that it can't be built as a module,
> instead it is registered directly in rc-main.c if CONFIG_MEDIA_CEC_RC
> is set. This is because it can be called from drm_dp_cec_set_edid() via
> cec_register_adapter() in an asynchronous context, and it is not
> allowed to use request_module() to load rc-cec.ko in that case. Trying to
> do so results in a 'WARN_ON_ONCE(wait && current_is_async())'.
> 
> Since this keymap is only used if CONFIG_MEDIA_CEC_RC is set, we
> just compile this keymap into the rc-core module and never as a
> separate module.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Fixes: 2c6d1fffa1d9 (drm: add support for DisplayPort CEC-Tunneling-over-AUX)
> Reported-by: Hans de Goede <hdegoede@redhat.com>

Thank you for writing this patch, this looks like a good solution to me.

Regards,

Hans





> ---
> Sean, I tested this both with RC_CORE=y and =m.
> 
> It looks reasonably clean to me, but I am not sure if this affects v4l-utils
> as well. Let me know what you think.
> 
> Regards,
> 
> 	Hans
> ---
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index 5bb2932ab119..ff6a8fc4c38e 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -5,6 +5,7 @@ obj-y += keymaps/
>  obj-$(CONFIG_RC_CORE) += rc-core.o
>  rc-core-y := rc-main.o rc-ir-raw.o
>  rc-core-$(CONFIG_LIRC) += lirc_dev.o
> +rc-core-$(CONFIG_MEDIA_CEC_RC) += keymaps/rc-cec.o
>  rc-core-$(CONFIG_BPF_LIRC_MODE2) += bpf-lirc.o
>  obj-$(CONFIG_IR_NEC_DECODER) += ir-nec-decoder.o
>  obj-$(CONFIG_IR_RC5_DECODER) += ir-rc5-decoder.o
> diff --git a/drivers/media/rc/keymaps/Makefile b/drivers/media/rc/keymaps/Makefile
> index b252a1d2ebd6..cc6662e1903f 100644
> --- a/drivers/media/rc/keymaps/Makefile
> +++ b/drivers/media/rc/keymaps/Makefile
> @@ -21,7 +21,6 @@ obj-$(CONFIG_RC_MAP) += rc-adstech-dvb-t-pci.o \
>  			rc-behold.o \
>  			rc-behold-columbus.o \
>  			rc-budget-ci-old.o \
> -			rc-cec.o \
>  			rc-cinergy-1400.o \
>  			rc-cinergy.o \
>  			rc-d680-dmb.o \
> diff --git a/drivers/media/rc/keymaps/rc-cec.c b/drivers/media/rc/keymaps/rc-cec.c
> index 3e3bd11092b4..068e22aeac8c 100644
> --- a/drivers/media/rc/keymaps/rc-cec.c
> +++ b/drivers/media/rc/keymaps/rc-cec.c
> @@ -1,5 +1,15 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  /* Keytable for the CEC remote control
> + *
> + * This keymap is unusual in that it can't be built as a module,
> + * instead it is registered directly in rc-main.c if CONFIG_MEDIA_CEC_RC
> + * is set. This is because it can be called from drm_dp_cec_set_edid() via
> + * cec_register_adapter() in an asynchronous context, and it is not
> + * allowed to use request_module() to load rc-cec.ko in that case.
> + *
> + * Since this keymap is only used if CONFIG_MEDIA_CEC_RC is set, we
> + * just compile this keymap into the rc-core module and never as a
> + * separate module.
>   *
>   * Copyright (c) 2015 by Kamil Debski
>   */
> @@ -152,7 +162,7 @@ static struct rc_map_table cec[] = {
>  	/* 0x77-0xff: Reserved */
>  };
> 
> -static struct rc_map_list cec_map = {
> +struct rc_map_list cec_map = {
>  	.map = {
>  		.scan		= cec,
>  		.size		= ARRAY_SIZE(cec),
> @@ -160,19 +170,3 @@ static struct rc_map_list cec_map = {
>  		.name		= RC_MAP_CEC,
>  	}
>  };
> -
> -static int __init init_rc_map_cec(void)
> -{
> -	return rc_map_register(&cec_map);
> -}
> -
> -static void __exit exit_rc_map_cec(void)
> -{
> -	rc_map_unregister(&cec_map);
> -}
> -
> -module_init(init_rc_map_cec);
> -module_exit(exit_rc_map_cec);
> -
> -MODULE_LICENSE("GPL");
> -MODULE_AUTHOR("Kamil Debski");
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index 1fd62c1dac76..4768b1e9ffd1 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -163,6 +163,10 @@ static struct rc_map_list empty_map = {
>  	}
>  };
> 
> +#ifdef CONFIG_MEDIA_CEC_RC
> +extern struct rc_map_list cec_map;
> +#endif
> +
>  /**
>   * scancode_to_u64() - converts scancode in &struct input_keymap_entry
>   * @ke: keymap entry containing scancode to be converted.
> @@ -2069,6 +2073,9 @@ static int __init rc_core_init(void)
> 
>  	led_trigger_register_simple("rc-feedback", &led_feedback);
>  	rc_map_register(&empty_map);
> +#ifdef CONFIG_MEDIA_CEC_RC
> +	rc_map_register(&cec_map);
> +#endif
> 
>  	return 0;
>  }
> @@ -2078,6 +2085,9 @@ static void __exit rc_core_exit(void)
>  	lirc_dev_exit();
>  	class_unregister(&rc_class);
>  	led_trigger_unregister_simple(led_feedback);
> +#ifdef CONFIG_MEDIA_CEC_RC
> +	rc_map_register(&cec_map);
> +#endif
>  	rc_map_unregister(&empty_map);
>  }
> 
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] media/rc: compile rc-cec.c into rc-core
  2021-02-20 10:58 ` Sean Young
@ 2021-02-23 10:51   ` Hans Verkuil
  0 siblings, 0 replies; 4+ messages in thread
From: Hans Verkuil @ 2021-02-23 10:51 UTC (permalink / raw)
  To: Sean Young; +Cc: Linux Media Mailing List, Hans de Goede

On 20/02/2021 11:58, Sean Young wrote:
> Hi Hans,
> 
> Thank you for writing this patch.
> 
> On Thu, Feb 18, 2021 at 05:13:08PM +0100, Hans Verkuil wrote:
>> The rc-cec keymap is unusual in that it can't be built as a module,
>> instead it is registered directly in rc-main.c if CONFIG_MEDIA_CEC_RC
>> is set. This is because it can be called from drm_dp_cec_set_edid() via
>> cec_register_adapter() in an asynchronous context, and it is not
>> allowed to use request_module() to load rc-cec.ko in that case. Trying to
>> do so results in a 'WARN_ON_ONCE(wait && current_is_async())'.
>>
>> Since this keymap is only used if CONFIG_MEDIA_CEC_RC is set, we
>> just compile this keymap into the rc-core module and never as a
>> separate module.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> Fixes: 2c6d1fffa1d9 (drm: add support for DisplayPort CEC-Tunneling-over-AUX)
>> Reported-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Sean, I tested this both with RC_CORE=y and =m.
>>
>> It looks reasonably clean to me, but I am not sure if this affects v4l-utils
>> as well. Let me know what you think.
>>
>> Regards,
>>
>> 	Hans
>> ---
>> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
>> index 5bb2932ab119..ff6a8fc4c38e 100644
>> --- a/drivers/media/rc/Makefile
>> +++ b/drivers/media/rc/Makefile
>> @@ -5,6 +5,7 @@ obj-y += keymaps/
>>  obj-$(CONFIG_RC_CORE) += rc-core.o
>>  rc-core-y := rc-main.o rc-ir-raw.o
>>  rc-core-$(CONFIG_LIRC) += lirc_dev.o
>> +rc-core-$(CONFIG_MEDIA_CEC_RC) += keymaps/rc-cec.o
>>  rc-core-$(CONFIG_BPF_LIRC_MODE2) += bpf-lirc.o
>>  obj-$(CONFIG_IR_NEC_DECODER) += ir-nec-decoder.o
>>  obj-$(CONFIG_IR_RC5_DECODER) += ir-rc5-decoder.o
>> diff --git a/drivers/media/rc/keymaps/Makefile b/drivers/media/rc/keymaps/Makefile
>> index b252a1d2ebd6..cc6662e1903f 100644
>> --- a/drivers/media/rc/keymaps/Makefile
>> +++ b/drivers/media/rc/keymaps/Makefile
>> @@ -21,7 +21,6 @@ obj-$(CONFIG_RC_MAP) += rc-adstech-dvb-t-pci.o \
>>  			rc-behold.o \
>>  			rc-behold-columbus.o \
>>  			rc-budget-ci-old.o \
>> -			rc-cec.o \
>>  			rc-cinergy-1400.o \
>>  			rc-cinergy.o \
>>  			rc-d680-dmb.o \
> 
> This is a great idea. If rc-core is built as a module, the cec keymap is 
> included in the module, and if cec-rc is not enabled then it is not built
> at all.
> 
>> diff --git a/drivers/media/rc/keymaps/rc-cec.c b/drivers/media/rc/keymaps/rc-cec.c
>> index 3e3bd11092b4..068e22aeac8c 100644
>> --- a/drivers/media/rc/keymaps/rc-cec.c
>> +++ b/drivers/media/rc/keymaps/rc-cec.c
>> @@ -1,5 +1,15 @@
>>  // SPDX-License-Identifier: GPL-2.0-or-later
>>  /* Keytable for the CEC remote control
>> + *
>> + * This keymap is unusual in that it can't be built as a module,
>> + * instead it is registered directly in rc-main.c if CONFIG_MEDIA_CEC_RC
>> + * is set. This is because it can be called from drm_dp_cec_set_edid() via
>> + * cec_register_adapter() in an asynchronous context, and it is not
>> + * allowed to use request_module() to load rc-cec.ko in that case.
>> + *
>> + * Since this keymap is only used if CONFIG_MEDIA_CEC_RC is set, we
>> + * just compile this keymap into the rc-core module and never as a
>> + * separate module.
>>   *
>>   * Copyright (c) 2015 by Kamil Debski
>>   */
>> @@ -152,7 +162,7 @@ static struct rc_map_table cec[] = {
>>  	/* 0x77-0xff: Reserved */
>>  };
>>
>> -static struct rc_map_list cec_map = {
>> +struct rc_map_list cec_map = {
>>  	.map = {
>>  		.scan		= cec,
>>  		.size		= ARRAY_SIZE(cec),
>> @@ -160,19 +170,3 @@ static struct rc_map_list cec_map = {
>>  		.name		= RC_MAP_CEC,
>>  	}
>>  };
>> -
>> -static int __init init_rc_map_cec(void)
>> -{
>> -	return rc_map_register(&cec_map);
>> -}
>> -
>> -static void __exit exit_rc_map_cec(void)
>> -{
>> -	rc_map_unregister(&cec_map);
>> -}
>> -
>> -module_init(init_rc_map_cec);
>> -module_exit(exit_rc_map_cec);
>> -
>> -MODULE_LICENSE("GPL");
>> -MODULE_AUTHOR("Kamil Debski");
>> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
>> index 1fd62c1dac76..4768b1e9ffd1 100644
>> --- a/drivers/media/rc/rc-main.c
>> +++ b/drivers/media/rc/rc-main.c
>> @@ -163,6 +163,10 @@ static struct rc_map_list empty_map = {
>>  	}
>>  };
>>
>> +#ifdef CONFIG_MEDIA_CEC_RC
>> +extern struct rc_map_list cec_map;
>> +#endif
>> +
>>  /**
>>   * scancode_to_u64() - converts scancode in &struct input_keymap_entry
>>   * @ke: keymap entry containing scancode to be converted.
>> @@ -2069,6 +2073,9 @@ static int __init rc_core_init(void)
>>
>>  	led_trigger_register_simple("rc-feedback", &led_feedback);
>>  	rc_map_register(&empty_map);
>> +#ifdef CONFIG_MEDIA_CEC_RC
>> +	rc_map_register(&cec_map);
>> +#endif
>>
>>  	return 0;
>>  }
>> @@ -2078,6 +2085,9 @@ static void __exit rc_core_exit(void)
>>  	lirc_dev_exit();
>>  	class_unregister(&rc_class);
>>  	led_trigger_unregister_simple(led_feedback);
>> +#ifdef CONFIG_MEDIA_CEC_RC
>> +	rc_map_register(&cec_map);

Oops, this should be rc_map_unregister()!

>> +#endif
>>  	rc_map_unregister(&empty_map);
>>  }
> 
> Now I'm confused. Do we need these changes at all? I've experimented a little
> bit (without the right hardware), and as far as I can make out, just the
> changes to the Kconfig/Makefile should be enough.

If I build with CONFIG_RC_CORE=m, then I get:

  CC [M]  drivers/media/rc/keymaps/rc-cec.o
  LD [M]  drivers/media/rc/rc-core.o
ld: drivers/media/rc/keymaps/rc-cec.o: in function `init_rc_map_cec':
/home/hans/work/src/v4l/media-git/drivers/media/rc/keymaps/rc-cec.c:165: multiple definition of `init_module';
drivers/media/rc/rc-main.o:/home/hans/work/src/v4l/media-git/drivers/media/rc/rc-main.c:2056: first defined here
ld: drivers/media/rc/keymaps/rc-cec.o: in function `exit_rc_map_cec':
/home/hans/work/src/v4l/media-git/drivers/media/rc/keymaps/rc-cec.c:171: multiple definition of `cleanup_module';
drivers/media/rc/rc-main.o:/home/hans/work/src/v4l/media-git/drivers/media/rc/rc-main.c:2078: first defined here

Which makes sense since now there are two module_init/exit entries.

It does work if CONFIG_RC_CORE=y, but not for =m.

Unless I am missing something, this is the right approach.

I'll post a v2 to fix the unregister issue.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-02-23 10:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-18 16:13 [PATCH] media/rc: compile rc-cec.c into rc-core Hans Verkuil
2021-02-20 10:58 ` Sean Young
2021-02-23 10:51   ` Hans Verkuil
2021-02-20 11:56 ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.