Discussion:
[PATCH v2 1/2] [media] rc-core: fix protocol_change regression in ir_raw_event_register
(too old to reply)
Tomas Melin
2014-10-21 18:30:17 UTC
Permalink
Raw Message
IR reciever using nuvoton-cir and lirc required additional configuration
steps after upgrade from kernel 3.16 to 3.17-rcX.
Bisected regression to commit da6e162d6a4607362f8478c715c797d84d449f8b
("[media] rc-core: simplify sysfs code").

The regression comes from adding empty function change_protocol in
ir-raw.c. It changes behaviour so that only the protocol enabled by driver's
map_name will be active after registration. This breaks user space behaviour,
lirc does not get key press signals anymore.

Changed back to original behaviour by removing empty function
change_protocol in ir-raw.c. Instead only calling change_protocol for
drivers that actually have the function set up.

Signed-off-by: Tomas Melin <***@iki.fi>
---
drivers/media/rc/rc-ir-raw.c | 7 -------
drivers/media/rc/rc-main.c | 19 ++++++++-----------
2 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index e8fff2a..a118539 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -240,12 +240,6 @@ ir_raw_get_allowed_protocols(void)
return protocols;
}

-static int change_protocol(struct rc_dev *dev, u64 *rc_type)
-{
- /* the caller will update dev->enabled_protocols */
- return 0;
-}
-
/*
* Used to (un)register raw event clients
*/
@@ -263,7 +257,6 @@ int ir_raw_event_register(struct rc_dev *dev)

dev->raw->dev = dev;
dev->enabled_protocols = ~0;
- dev->change_protocol = change_protocol;
rc = kfifo_alloc(&dev->raw->kfifo,
sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE,
GFP_KERNEL);
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index a7991c7..633c682 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1001,11 +1001,6 @@ static ssize_t store_protocols(struct device *device,
set_filter = dev->s_wakeup_filter;
}

- if (!change_protocol) {
- IR_dprintk(1, "Protocol switching not supported\n");
- return -EINVAL;
- }
-
mutex_lock(&dev->lock);

old_protocols = *current_protocols;
@@ -1013,12 +1008,14 @@ static ssize_t store_protocols(struct device *device,
rc = parse_protocol_change(&new_protocols, buf);
if (rc < 0)
goto out;
-
- rc = change_protocol(dev, &new_protocols);
- if (rc < 0) {
- IR_dprintk(1, "Error setting protocols to 0x%llx\n",
- (long long)new_protocols);
- goto out;
+ /* Only if protocol change set up in driver */
+ if (change_protocol) {
+ rc = change_protocol(dev, &new_protocols);
+ if (rc < 0) {
+ IR_dprintk(1, "Error setting protocols to 0x%llx\n",
+ (long long)new_protocols);
+ goto out;
+ }
}

if (new_protocols == old_protocols) {
--
1.7.10.4
Tomas Melin
2014-10-21 18:30:18 UTC
Permalink
Raw Message
Change default setting for enabled protocols.
Instead of enabling all protocols during registration,
disable all except default keymap and lirc.
Reduces overhead since all protocols not handled by default.
Protocol to use will be enabled when keycode table is written by userspace.

Signed-off-by: Tomas Melin <***@iki.fi>
---
drivers/media/rc/rc-ir-raw.c | 1 -
drivers/media/rc/rc-main.c | 6 ++++--
2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index a118539..d3b1e76 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -256,7 +256,6 @@ int ir_raw_event_register(struct rc_dev *dev)
return -ENOMEM;

dev->raw->dev = dev;
- dev->enabled_protocols = ~0;
rc = kfifo_alloc(&dev->raw->kfifo,
sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE,
GFP_KERNEL);
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 633c682..dcdf4cd 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1320,6 +1320,8 @@ int rc_register_device(struct rc_dev *dev)
rc_map = rc_map_get(RC_MAP_EMPTY);
if (!rc_map || !rc_map->scan || rc_map->size == 0)
return -EINVAL;
+ /* get default keymap type */
+ u64 rc_type = (1 << rc_map->rc_type);

set_bit(EV_KEY, dev->input_dev->evbit);
set_bit(EV_REP, dev->input_dev->evbit);
@@ -1412,16 +1414,16 @@ int rc_register_device(struct rc_dev *dev)
raw_init = true;
}
rc = ir_raw_event_register(dev);
+ dev->enabled_protocols = (rc_type | RC_BIT_LIRC);
if (rc < 0)
goto out_input;
}

if (dev->change_protocol) {
- u64 rc_type = (1 << rc_map->rc_type);
rc = dev->change_protocol(dev, &rc_type);
if (rc < 0)
goto out_raw;
- dev->enabled_protocols = rc_type;
+ dev->enabled_protocols = (rc_type | RC_BIT_LIRC);
}

mutex_unlock(&dev->lock);
--
1.7.10.4
Loading...