All of lore.kernel.org
 help / color / mirror / Atom feed
* Bug: decoders referenced in kernel rc-keymaps not loaded on boot
@ 2017-02-21 18:49 Matthias Reichl
  2017-02-21 19:34 ` Sean Young
  0 siblings, 1 reply; 6+ messages in thread
From: Matthias Reichl @ 2017-02-21 18:49 UTC (permalink / raw)
  To: Heiner Kallweit, Mauro Carvalho Chehab; +Cc: linux-media

There seems to be a bug in on-demand loading of IR protocol decoders.

After bootup the protocol referenced in the in-kernel rc keymap shows
up as enabled (in sysfs and ir-keytable) but the protocol decoder
is not loaded and thus no rc input events will be generated.

For example, RPi3 with kernel 4.10 and gpio-ir-recv configured to use
the rc-hauppauge keymap in devicetree:

# lsmod | grep '^\(ir\|rc_\)'
ir_lirc_codec           5590  0
rc_hauppauge            2422  0
rc_core                24320  5 rc_hauppauge,ir_lirc_codec,lirc_dev,gpio_ir_recv

# cat /sys/class/rc/rc0/protocols
other unknown [rc-5] nec rc-6 jvc sony rc-5-sz sanyo sharp mce_kbd xmp cec [lirc]

# dmesg | grep "IR "
[    4.506728] Registered IR keymap rc-hauppauge
[    4.554651] lirc_dev: IR Remote Control driver registered, major 242
[    4.576490] IR LIRC bridge handler initialized

The same happens with other IR receivers, eg the streamzap receiver,
which uses the rc-5-sz protocol / ir_rc5_decoder, on x86.

Reverting the on-demand-loading patches

[media] media: rc: remove unneeded code
commit c1500ba0b61e9abf95e0e7ecd3c4ad877f019abe

[media] media: rc: move check whether a protocol is enabled to the core
commit d80ca8bd71f0b01b2b12459189927cb3299cfab9

[media] media: rc: load decoder modules on-demand
commit acc1c3c688ed8cc862ddc007eab0dcef839f4ec8

restores the previous behaviour, all decoders are enabled and IR
events can be generated immediately after boot without having to
manually trigger loading of a protocol decoder.

so long,

Hias

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

* Re: Bug: decoders referenced in kernel rc-keymaps not loaded on boot
  2017-02-21 18:49 Bug: decoders referenced in kernel rc-keymaps not loaded on boot Matthias Reichl
@ 2017-02-21 19:34 ` Sean Young
  2017-02-21 22:52   ` Matthias Reichl
  0 siblings, 1 reply; 6+ messages in thread
From: Sean Young @ 2017-02-21 19:34 UTC (permalink / raw)
  To: Matthias Reichl; +Cc: Heiner Kallweit, Mauro Carvalho Chehab, linux-media

On Tue, Feb 21, 2017 at 07:49:29PM +0100, Matthias Reichl wrote:
> There seems to be a bug in on-demand loading of IR protocol decoders.
> 
> After bootup the protocol referenced in the in-kernel rc keymap shows
> up as enabled (in sysfs and ir-keytable) but the protocol decoder
> is not loaded and thus no rc input events will be generated.
> 
> For example, RPi3 with kernel 4.10 and gpio-ir-recv configured to use
> the rc-hauppauge keymap in devicetree:
> 
> # lsmod | grep '^\(ir\|rc_\)'
> ir_lirc_codec           5590  0
> rc_hauppauge            2422  0
> rc_core                24320  5 rc_hauppauge,ir_lirc_codec,lirc_dev,gpio_ir_recv
> 
> # cat /sys/class/rc/rc0/protocols
> other unknown [rc-5] nec rc-6 jvc sony rc-5-sz sanyo sharp mce_kbd xmp cec [lirc]
> 
> # dmesg | grep "IR "
> [    4.506728] Registered IR keymap rc-hauppauge
> [    4.554651] lirc_dev: IR Remote Control driver registered, major 242
> [    4.576490] IR LIRC bridge handler initialized
> 
> The same happens with other IR receivers, eg the streamzap receiver,
> which uses the rc-5-sz protocol / ir_rc5_decoder, on x86.
> 
> Reverting the on-demand-loading patches
> 
> [media] media: rc: remove unneeded code
> commit c1500ba0b61e9abf95e0e7ecd3c4ad877f019abe
> 
> [media] media: rc: move check whether a protocol is enabled to the core
> commit d80ca8bd71f0b01b2b12459189927cb3299cfab9
> 
> [media] media: rc: load decoder modules on-demand
> commit acc1c3c688ed8cc862ddc007eab0dcef839f4ec8
> 
> restores the previous behaviour, all decoders are enabled and IR
> events can be generated immediately after boot without having to
> manually trigger loading of a protocol decoder.

Hmm this seems to be working fine for me. If you write to the protocols
file, eg. "echo +nec > /sys/class/rc/rc0/protocols", is ir-nec-decoder
loaded and do you get any messages in dmesg (you should).

What's your config?

Thanks,
Sean

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

* Re: Bug: decoders referenced in kernel rc-keymaps not loaded on boot
  2017-02-21 19:34 ` Sean Young
@ 2017-02-21 22:52   ` Matthias Reichl
  2017-02-22 23:00     ` Sean Young
  0 siblings, 1 reply; 6+ messages in thread
From: Matthias Reichl @ 2017-02-21 22:52 UTC (permalink / raw)
  To: Sean Young; +Cc: Heiner Kallweit, Mauro Carvalho Chehab, linux-media

On Tue, Feb 21, 2017 at 07:34:39PM +0000, Sean Young wrote:
> On Tue, Feb 21, 2017 at 07:49:29PM +0100, Matthias Reichl wrote:
> > There seems to be a bug in on-demand loading of IR protocol decoders.
> > 
> > After bootup the protocol referenced in the in-kernel rc keymap shows
> > up as enabled (in sysfs and ir-keytable) but the protocol decoder
> > is not loaded and thus no rc input events will be generated.
> > 
> > For example, RPi3 with kernel 4.10 and gpio-ir-recv configured to use
> > the rc-hauppauge keymap in devicetree:
> > 
> > # lsmod | grep '^\(ir\|rc_\)'
> > ir_lirc_codec           5590  0
> > rc_hauppauge            2422  0
> > rc_core                24320  5 rc_hauppauge,ir_lirc_codec,lirc_dev,gpio_ir_recv
> > 
> > # cat /sys/class/rc/rc0/protocols
> > other unknown [rc-5] nec rc-6 jvc sony rc-5-sz sanyo sharp mce_kbd xmp cec [lirc]
> > 
> > # dmesg | grep "IR "
> > [    4.506728] Registered IR keymap rc-hauppauge
> > [    4.554651] lirc_dev: IR Remote Control driver registered, major 242
> > [    4.576490] IR LIRC bridge handler initialized
> > 
> > The same happens with other IR receivers, eg the streamzap receiver,
> > which uses the rc-5-sz protocol / ir_rc5_decoder, on x86.
> > 
> > Reverting the on-demand-loading patches
> > 
> > [media] media: rc: remove unneeded code
> > commit c1500ba0b61e9abf95e0e7ecd3c4ad877f019abe
> > 
> > [media] media: rc: move check whether a protocol is enabled to the core
> > commit d80ca8bd71f0b01b2b12459189927cb3299cfab9
> > 
> > [media] media: rc: load decoder modules on-demand
> > commit acc1c3c688ed8cc862ddc007eab0dcef839f4ec8
> > 
> > restores the previous behaviour, all decoders are enabled and IR
> > events can be generated immediately after boot without having to
> > manually trigger loading of a protocol decoder.
> 
> Hmm this seems to be working fine for me. If you write to the protocols
> file, eg. "echo +nec > /sys/class/rc/rc0/protocols", is ir-nec-decoder
> loaded and do you get any messages in dmesg (you should).
> 
> What's your config?

When I do an "echo +nec > /sys/class/rc/rc0/protocols" it triggers
the load of both rc5 and nec decoder modules:

root@rpi3:~# cat /sys/class/rc/rc0/protocols
other unknown [rc-5] nec rc-6 jvc sony rc-5-sz sanyo sharp mce_kbd xmp cec [lirc]
root@rpi3:~# echo +nec > /sys/class/rc/rc0/protocols
root@rpi3:~# cat /sys/class/rc/rc0/protocols
other unknown [rc-5] [nec] rc-6 jvc sony rc-5-sz sanyo sharp mce_kbd xmp cec [lirc]
root@rpi3:~# dmesg | grep "IR "
[    3.565061] Registered IR keymap rc-hauppauge
[    3.613031] lirc_dev: IR Remote Control driver registered, major 242
[    3.641423] IR LIRC bridge handler initialized
[   41.877263] IR RC5(x/sz) protocol handler initialized
[   41.931575] IR NEC protocol handler initialized

I'm currently testing with downstream RPi kernel 4.9 on Raspbian Jessie
(a Debian derivate).

Kernel config is here:
https://github.com/raspberrypi/linux/blob/rpi-4.9.y/arch/arm/configs/bcm2709_defconfig

To reproduce the issue it's important to disable the udev rule that
runs ir-keytable -a as that can trigger a load of the kernel keytable
via the userspace keymap/protocol.

We ran accross the issue via a bugreport from a LibreELEC user,
his streamzap remote wasn't working anymore on x86 in the beta
releases:
https://forum.libreelec.tv/thread-4873.html

Kernel-config for LibreELEC x86 is here:
https://github.com/LibreELEC/LibreELEC.tv/blob/libreelec-8.0/projects/Generic/linux/linux.x86_64.conf

Our analysis (I hope it's not completely off) is about this:

In the previous version (with kernel 4.4) it worked because
the kernel loaded the keymap and protocol decoders. The udev
rule probably failed as ir-keytable -a couldn't cope with the RC5_SZ
protocol - but that went unnoticed as everything was setup fine
by the kernel.

In current beta (with kernel 4.9) the kernel only loaded the
keymap but didn't enable the decoder. Since ir-keytable -a again
failed to setup the protocol the user was left with a non-functioning
remote.

I then could reproduce this on RPi with Raspbian and LibreELEC
using gpio-ir-recv. With udev/ir-keytable -a working the protocol
decoder is loaded, without that it isn't.

so long,

Hias

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

* Re: Bug: decoders referenced in kernel rc-keymaps not loaded on boot
  2017-02-21 22:52   ` Matthias Reichl
@ 2017-02-22 23:00     ` Sean Young
  2017-02-22 23:11       ` [PATCH] [media] rc: raw decoder for keymap protocol is not loaded on register Sean Young
  0 siblings, 1 reply; 6+ messages in thread
From: Sean Young @ 2017-02-22 23:00 UTC (permalink / raw)
  To: Matthias Reichl; +Cc: Heiner Kallweit, Mauro Carvalho Chehab, linux-media

On Tue, Feb 21, 2017 at 11:52:24PM +0100, Matthias Reichl wrote:
> On Tue, Feb 21, 2017 at 07:34:39PM +0000, Sean Young wrote:
> > On Tue, Feb 21, 2017 at 07:49:29PM +0100, Matthias Reichl wrote:
> > > There seems to be a bug in on-demand loading of IR protocol decoders.
> > > 
> > > After bootup the protocol referenced in the in-kernel rc keymap shows
> > > up as enabled (in sysfs and ir-keytable) but the protocol decoder
> > > is not loaded and thus no rc input events will be generated.
> > > 
> > > For example, RPi3 with kernel 4.10 and gpio-ir-recv configured to use
> > > the rc-hauppauge keymap in devicetree:
> > > 
> > > # lsmod | grep '^\(ir\|rc_\)'
> > > ir_lirc_codec           5590  0
> > > rc_hauppauge            2422  0
> > > rc_core                24320  5 rc_hauppauge,ir_lirc_codec,lirc_dev,gpio_ir_recv
> > > 
> > > # cat /sys/class/rc/rc0/protocols
> > > other unknown [rc-5] nec rc-6 jvc sony rc-5-sz sanyo sharp mce_kbd xmp cec [lirc]
> > > 
> > > # dmesg | grep "IR "
> > > [    4.506728] Registered IR keymap rc-hauppauge
> > > [    4.554651] lirc_dev: IR Remote Control driver registered, major 242
> > > [    4.576490] IR LIRC bridge handler initialized
> > > 
> > > The same happens with other IR receivers, eg the streamzap receiver,
> > > which uses the rc-5-sz protocol / ir_rc5_decoder, on x86.
> > > 
> > > Reverting the on-demand-loading patches
> > > 
> > > [media] media: rc: remove unneeded code
> > > commit c1500ba0b61e9abf95e0e7ecd3c4ad877f019abe
> > > 
> > > [media] media: rc: move check whether a protocol is enabled to the core
> > > commit d80ca8bd71f0b01b2b12459189927cb3299cfab9
> > > 
> > > [media] media: rc: load decoder modules on-demand
> > > commit acc1c3c688ed8cc862ddc007eab0dcef839f4ec8
> > > 
> > > restores the previous behaviour, all decoders are enabled and IR
> > > events can be generated immediately after boot without having to
> > > manually trigger loading of a protocol decoder.
> > 
> > Hmm this seems to be working fine for me. If you write to the protocols
> > file, eg. "echo +nec > /sys/class/rc/rc0/protocols", is ir-nec-decoder
> > loaded and do you get any messages in dmesg (you should).
> > 
> > What's your config?
> 
> When I do an "echo +nec > /sys/class/rc/rc0/protocols" it triggers
> the load of both rc5 and nec decoder modules:
> 
> root@rpi3:~# cat /sys/class/rc/rc0/protocols
> other unknown [rc-5] nec rc-6 jvc sony rc-5-sz sanyo sharp mce_kbd xmp cec [lirc]
> root@rpi3:~# echo +nec > /sys/class/rc/rc0/protocols
> root@rpi3:~# cat /sys/class/rc/rc0/protocols
> other unknown [rc-5] [nec] rc-6 jvc sony rc-5-sz sanyo sharp mce_kbd xmp cec [lirc]
> root@rpi3:~# dmesg | grep "IR "
> [    3.565061] Registered IR keymap rc-hauppauge
> [    3.613031] lirc_dev: IR Remote Control driver registered, major 242
> [    3.641423] IR LIRC bridge handler initialized
> [   41.877263] IR RC5(x/sz) protocol handler initialized
> [   41.931575] IR NEC protocol handler initialized
> 
> I'm currently testing with downstream RPi kernel 4.9 on Raspbian Jessie
> (a Debian derivate).
> 
> Kernel config is here:
> https://github.com/raspberrypi/linux/blob/rpi-4.9.y/arch/arm/configs/bcm2709_defconfig
> 
> To reproduce the issue it's important to disable the udev rule that
> runs ir-keytable -a as that can trigger a load of the kernel keytable
> via the userspace keymap/protocol.

Ah. Yes, this is broken. In commit "9f0bf36 [media] media: rc: preparation
for on-demand decoder module loading", code was added so that only the
required decoder module was loaded. This added it to the sysfs protocols
attribute handling, but not to the rc_register_device(), so no decoder
will be loaded when a device is first registered.

As you say the udev rule papers over the problem by executing ir-keytable,
so I never noticed the problem either.

I'll send a patch as a reply to this email.


Sean

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

* [PATCH] [media] rc: raw decoder for keymap protocol is not loaded on register
  2017-02-22 23:00     ` Sean Young
@ 2017-02-22 23:11       ` Sean Young
  2017-02-23 19:00         ` Matthias Reichl
  0 siblings, 1 reply; 6+ messages in thread
From: Sean Young @ 2017-02-22 23:11 UTC (permalink / raw)
  To: Matthias Reichl; +Cc: Heiner Kallweit, linux-media, stable

When the protocol is set via the sysfs protocols attribute, the
decoder is loaded. However, when it is not when a device is first
plugged in or registered.

Fixes: acc1c3c ("[media] media: rc: load decoder modules on-demand")

Signed-off-by: Sean Young <sean@mess.org>
Cc: <stable@kernel.org> # v4.5+
---
 drivers/media/rc/rc-main.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 2424946..a158b32 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1663,6 +1663,7 @@ static int rc_setup_rx_device(struct rc_dev *dev)
 {
 	int rc;
 	struct rc_map *rc_map;
+	u64 rc_type;
 
 	if (!dev->map_name)
 		return -EINVAL;
@@ -1677,15 +1678,18 @@ static int rc_setup_rx_device(struct rc_dev *dev)
 	if (rc)
 		return rc;
 
-	if (dev->change_protocol) {
-		u64 rc_type = (1ll << rc_map->rc_type);
+	rc_type = BIT_ULL(rc_map->rc_type);
 
+	if (dev->change_protocol) {
 		rc = dev->change_protocol(dev, &rc_type);
 		if (rc < 0)
 			goto out_table;
 		dev->enabled_protocols = rc_type;
 	}
 
+	if (dev->driver_type == RC_DRIVER_IR_RAW)
+		ir_raw_load_modules(&rc_type);
+
 	set_bit(EV_KEY, dev->input_dev->evbit);
 	set_bit(EV_REP, dev->input_dev->evbit);
 	set_bit(EV_MSC, dev->input_dev->evbit);
-- 
2.9.3

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

* Re: [PATCH] [media] rc: raw decoder for keymap protocol is not loaded on register
  2017-02-22 23:11       ` [PATCH] [media] rc: raw decoder for keymap protocol is not loaded on register Sean Young
@ 2017-02-23 19:00         ` Matthias Reichl
  0 siblings, 0 replies; 6+ messages in thread
From: Matthias Reichl @ 2017-02-23 19:00 UTC (permalink / raw)
  To: Sean Young; +Cc: Heiner Kallweit, linux-media, stable

On Wed, Feb 22, 2017 at 11:11:49PM +0000, Sean Young wrote:
> When the protocol is set via the sysfs protocols attribute, the
> decoder is loaded. However, when it is not when a device is first
> plugged in or registered.
> 
> Fixes: acc1c3c ("[media] media: rc: load decoder modules on-demand")
> 
> Signed-off-by: Sean Young <sean@mess.org>

Tested-by: Matthias Reichl <hias@horus.com>

I've tested the backported patch below successfully on RPi3 with
kernel 4.10 and decoder modules are loading fine again:

# dmesg | grep "IR "
[    3.526404] Registered IR keymap rc-hauppauge
[    3.590875] lirc_dev: IR Remote Control driver registered, major 242
[    3.600602] IR RC5(x/sz) protocol handler initialized
[    3.602111] IR LIRC bridge handler initialized

Thanks a lot for fixing this so quickly!

so long,

Hias

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index dedaf38..9a397da 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1441,6 +1441,7 @@ int rc_register_device(struct rc_dev *dev)
 	int attr = 0;
 	int minor;
 	int rc;
+	u64 rc_type;
 
 	if (!dev || !dev->map_name)
 		return -EINVAL;
@@ -1526,14 +1527,18 @@ int rc_register_device(struct rc_dev *dev)
 			goto out_input;
 	}
 
+	rc_type = BIT_ULL(rc_map->rc_type);
+
 	if (dev->change_protocol) {
-		u64 rc_type = (1ll << rc_map->rc_type);
 		rc = dev->change_protocol(dev, &rc_type);
 		if (rc < 0)
 			goto out_raw;
 		dev->enabled_protocols = rc_type;
 	}
 
+	if (dev->driver_type == RC_DRIVER_IR_RAW)
+		ir_raw_load_modules(&rc_type);
+
 	/* Allow the RC sysfs nodes to be accessible */
 	atomic_set(&dev->initialized, 1);
 

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

end of thread, other threads:[~2017-02-23 19:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-21 18:49 Bug: decoders referenced in kernel rc-keymaps not loaded on boot Matthias Reichl
2017-02-21 19:34 ` Sean Young
2017-02-21 22:52   ` Matthias Reichl
2017-02-22 23:00     ` Sean Young
2017-02-22 23:11       ` [PATCH] [media] rc: raw decoder for keymap protocol is not loaded on register Sean Young
2017-02-23 19:00         ` Matthias Reichl

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.