[PATCH v3] [media] rc-core: fix protocol_change regression in ir_raw_event_register

2014-10-28 Thread Tomas Melin 
IR receiver 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 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.

Enable lirc protocol by default for ir raw decoders to restore original 
behaviour.

Signed-off-by: Tomas Melin 
---
 drivers/media/rc/rc-ir-raw.c |1 -
 drivers/media/rc/rc-main.c   |2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index e8fff2a..b732ac6 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -262,7 +262,6 @@ int ir_raw_event_register(struct rc_dev *dev)
return -ENOMEM;
 
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,
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index a7991c7..8d3b74c 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1421,6 +1421,8 @@ int rc_register_device(struct rc_dev *dev)
 
if (dev->change_protocol) {
u64 rc_type = (1 << rc_map->rc_type);
+   if (dev->driver_type == RC_DRIVER_IR_RAW)
+   rc_type |= RC_BIT_LIRC;
rc = dev->change_protocol(dev, &rc_type);
if (rc < 0)
goto out_raw;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] [media] rc-core: fix protocol_change regression in ir_raw_event_register

2014-10-28 Thread Tomas Melin 
On Tue, Oct 28, 2014 at 10:42 AM, David Härdeman  wrote:
> On 2014-10-26 20:33, Tomas Melin wrote:
>>
>> Please let me know your preferences on which you prefer, and, if
>> needed, I'll make a new patch version.
>
>
> I'd prefer the above, minimal, approach. But it's Mauro who decides in the
> end.
Then let's just go with that approach and see if it's ok with Mauro.

Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] [media] rc-core: fix protocol_change regression in ir_raw_event_register

2014-10-26 Thread Tomas Melin 
On Sat, Oct 25, 2014 at 12:03 PM, David Härdeman  wrote:
> Wouldn't something like this be a simpler way of achieving the same
> result? (untested):

The idea was to remove the empty change_protocol function that had
been added in the breaking commit.
IMHO, it would be better to not have functions that don't do anything.
Actually, another problem with that empty function is that if the
driver first sets up a "real" change_protocol function and related
data, and then calls rc_register_device, the driver defined
change_protocol function would be overwritten.


> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index a7991c7..d521f20 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -1421,6 +1421,9 @@ int rc_register_device(struct rc_dev *dev)
>
> if (dev->change_protocol) {
> u64 rc_type = (1 << rc_map->rc_type);
> +   if (dev->driver_type == RC_DRIVER_IR_RAW)
> +   rc_type |= RC_BIT_LIRC;
> +
> rc = dev->change_protocol(dev, &rc_type);
> if (rc < 0)
> goto out_raw;

But otherwise yes, your suggestion could work, with the addition that
we still need to update enabled_protocols (and not init
enabled_protocols anymore in ir_raw_event_register() ).
+   dev->enabled_protocols = (rc_type | RC_BIT_LIRC);

Please let me know your preferences on which you prefer, and, if
needed, I'll make a new patch version.
Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/2] [media] rc-core: fix protocol_change regression in ir_raw_event_register

2014-10-21 Thread Tomas Melin 
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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] [media] rc-core: change enabled_protocol default setting

2014-10-21 Thread Tomas Melin 
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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] [media] rc-core: change enabled_protocol default setting

2014-10-21 Thread Tomas Melin 
On Mon, Oct 20, 2014 at 5:12 PM, David Härdeman  wrote:
> On 2014-10-19 12:21, Tomas Melin wrote:
>>
>> Change default setting for enabled protocols.
>> Instead of enabling all protocols, disable all except lirc during
>> registration.
>> Reduces overhead since all protocols not handled by default.
>> Protocol to use will be enabled when keycode table is written by
>> userspace.
>
>
> I can see the appeal in this, but now you've disabled automatic decoding for
> the protocol specified by the keymap for raw drivers? So this would also be
> a change, right?

Yes, you are right. Atleast it potentially still could change expected
behaviour.

>
> I agree with Mauro that the "proper" long-term fix would be to teach the
> LIRC userspace daemon to enable the lirc protocol as/when necessary, but
> something similar to the patch below (but lirc + keymap protocol...if that's
> possible to implement in a non-intrusive manner, I haven't checked TBH)
> might be a good idea as an interim measure?
>
I think it looks feasible to implement that way. I'll look in to it
and send a new patch series.

Tomas

>
>
>>
>> Signed-off-by: Tomas Melin 
>> ---
>>  drivers/media/rc/rc-ir-raw.c |3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
>> index a118539..63d23d0 100644
>> --- a/drivers/media/rc/rc-ir-raw.c
>> +++ b/drivers/media/rc/rc-ir-raw.c
>> @@ -256,7 +256,8 @@ int ir_raw_event_register(struct rc_dev *dev)
>> return -ENOMEM;
>>
>> dev->raw->dev = dev;
>> -   dev->enabled_protocols = ~0;
>> +   /* by default, disable all but lirc*/
>> +   dev->enabled_protocols = RC_BIT_LIRC;
>> rc = kfifo_alloc(&dev->raw->kfifo,
>>  sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE,
>>  GFP_KERNEL);
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] [media] rc-core: change enabled_protocol default setting

2014-10-19 Thread Tomas Melin 
Change default setting for enabled protocols.
Instead of enabling all protocols, disable all except lirc during registration.
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 
---
 drivers/media/rc/rc-ir-raw.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index a118539..63d23d0 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -256,7 +256,8 @@ int ir_raw_event_register(struct rc_dev *dev)
return -ENOMEM;
 
dev->raw->dev = dev;
-   dev->enabled_protocols = ~0;
+   /* by default, disable all but lirc*/
+   dev->enabled_protocols = RC_BIT_LIRC;
rc = kfifo_alloc(&dev->raw->kfifo,
 sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE,
 GFP_KERNEL);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] [media] rc-core: fix protocol_change regression in ir_raw_event_register

2014-10-19 Thread Tomas Melin 
IR reciever using nuvoton-cir and lirc was not working anymore 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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH resend] [media] rc-core: fix protocol_change regression in ir_raw_event_register

2014-10-19 Thread Tomas Melin 
On Sat, Oct 18, 2014 at 9:18 PM, Mauro Carvalho Chehab
 wrote:
>
> The right behavior here is to enable the protocol as soon as the
> new keycode table is written by userspace.
>
> Except for LIRC and the protocol of the current table enabled is
> not a good idea because:
>
> 1) It misread the code from some other IR;
> 2) It will be just spending power without need, running
>several tasks (one for each IR type) with no reason, as the
>keytable won't match the codes for other IRs (and if it is
>currently matching, then this is a bad behavior).
>
I agree, it sounds overkill to have all protocols enabled by default.
I'll make a new patch that enables lirc but disables other protocols
during registration.

Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH resend] [media] rc-core: fix protocol_change regression in ir_raw_event_register

2014-10-18 Thread Tomas Melin 
On Thu, Oct 16, 2014 at 11:49 PM, David Härdeman  wrote:
> I think this is already addressed in this thread:
> http://www.spinics.net/lists/linux-media/msg79865.html
The patch in that thread would have broken things since the
store_protocol function is not changed at the same time. The patch I
sent also takes that into account.

My concern is still that user space behaviour changes.
In my case, lirc simply does not work anymore. More generically,
anyone now using e.g. nuvoton-cir with anything other than RC6_MCE
will not get their devices working without first explictly enabling
the correct protocol from sysfs or with ir-keytable.

Correct me if I'm wrong but the change_protocol function in struct
rc_dev is meant for changing hardware decoder protocols which means
only a few drivers actually use it. So the added empty function
change_protocol into rc-ir-raw.c doesnt really make sense in the first
place.

Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH resend] [media] rc-core: fix protocol_change regression in ir_raw_event_register

2014-10-09 Thread Tomas Melin 
IR reciever using nuvoton-cir and lirc was not working anymore 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 function change_protocol in
ir-raw.c. During registration, ir_raw_event_register enables all protocols
by default. Later, rc_register_device also tests dev->change_protocol and
changes the enabled protocols based on rc_map type. However, rc_map type
only defines a single specific protocol, so in the case of a more generic
driver, this disables all protocols but the one defined by the map.

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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [media] rc-core: fix protocol_change regression in ir_raw_event_register

2014-10-06 Thread Tomas Melin 
IR reciever using nuvoton-cir and lirc was not working anymore 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 function change_protocol in
ir-raw.c. During registration, ir_raw_event_register enables all protocols
by default. Later, rc_register_device also tests dev->change_protocol and
changes the enabled protocols based on rc_map type. However, rc_map type
only defines a single specific protocol, so in the case of a more generic
driver, this disables all protocols but the one defined by the map.

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 
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [media] rc-main: fix lockdep splash for rc-main

2014-09-30 Thread Tomas Melin 
lockdep reports a potential circular dependecy deadlock when registering input 
device.

Unlock mutex rc_dev->lock prior to calling ir_raw_event_register to avoid the 
circular
dependency since that function also calls input_register_device and rc_open.

 ==
 [ INFO: possible circular locking dependency detected ]
 3.17.0-rc7+ #24 Not tainted
 ---
 modprobe/647 is trying to acquire lock:
  (input_mutex){+.+.+.}, at: [] 
input_register_device+0x2ba/0x381

 but task is already holding lock:
  (ir_raw_handler_lock){+.+.+.}, at: [] 
ir_raw_event_register+0x102/0x190

 which lock already depends on the new lock.

[cut text]

 other info that might help us debug this:

 Chain exists of:
   input_mutex --> &dev->lock --> ir_raw_handler_lock

  Possible unsafe locking scenario:

CPU0CPU1

   lock(ir_raw_handler_lock);
lock(&dev->lock);
lock(ir_raw_handler_lock);
   lock(input_mutex);

  *** DEADLOCK ***

 4 locks held by modprobe/647:
  #0:  (&dev->mutex){..}, at: [] device_lock+0xf/0x11
  #1:  (&dev->mutex){..}, at: [] device_lock+0xf/0x11
  #2:  (&dev->lock){+.+.+.}, at: [] 
rc_register_device+0x55d/0x58a
  #3:  (ir_raw_handler_lock){+.+.+.}, at: [] 
ir_raw_event_register+0x102/0x190

 stack backtrace:
 CPU: 0 PID: 647 Comm: modprobe Not tainted 3.17.0-rc7+ #24

 Call Trace:
  [] dump_stack+0x46/0x58
  [] print_circular_bug+0x1f8/0x209
  [] __lock_acquire+0xb54/0xeda
  [] ? console_unlock+0x34d/0x399
  [] lock_acquire+0xd9/0x111
  [] ? input_register_device+0x2ba/0x381
  [] mutex_lock_interruptible_nested+0x57/0x381
  [] ? input_register_device+0x2ba/0x381
  [] ? kfree+0x7c/0x96
  [] ? input_register_device+0x2ba/0x381
  [] ? trace_hardirqs_on+0xd/0xf
  [] input_register_device+0x2ba/0x381
  [] ir_mce_kbd_register+0x109/0x139
  [] ir_raw_event_register+0x13d/0x190
  [] rc_register_device+0x39e/0x58a
  [] ? trace_hardirqs_on+0xd/0xf
  [] nvt_probe+0x5ad/0xd52 [nuvoton_cir]
  [] ? nvt_resume+0x80/0x80 [nuvoton_cir]
  [] pnp_device_probe+0x8c/0xa9
  [] ? driver_sysfs_add+0x6e/0x93
  [] driver_probe_device+0xa1/0x1e3
  [] ? driver_probe_device+0x1e3/0x1e3
  [] __driver_attach+0x4e/0x6f
  [] bus_for_each_dev+0x5a/0x8c
  [] driver_attach+0x19/0x1b
  [] bus_add_driver+0xf1/0x1d6
  [] driver_register+0x87/0xbe
  [] ? 0xa012
  [] pnp_register_driver+0x1c/0x1e
  [] nvt_init+0x10/0x1000 [nuvoton_cir]
  [] do_one_initcall+0xea/0x18c
  [] ? __vunmap+0x9d/0xc7
  [] load_module+0x1c21/0x1f2c
  [] ? show_initstate+0x44/0x44
  [] SyS_init_module+0xa2/0xb1
  [] system_call_fastpath+0x16/0x1b

Signed-off-by: Tomas Melin 
---
 drivers/media/rc/rc-main.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index a7991c7..296de85 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1414,7 +1414,10 @@ int rc_register_device(struct rc_dev *dev)
ir_raw_init();
raw_init = true;
}
+   /* calls ir_register_device so unlock mutex here*/
+   mutex_unlock(&dev->lock);
rc = ir_raw_event_register(dev);
+   mutex_lock(&dev->lock);
if (rc < 0)
goto out_input;
}
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

regression nuvoton-cir: possible circular locking dependency

2014-08-30 Thread Tomas Melin 

Hi,

I get a lockdep splat with kernels 3.17-rc1 and newer for the 
nuvoton-cir driver on my Intel NUC D54250. The driver was working 
correctly with Linux 3.16, after upgrading it does not react to any 
input anymore although loading seems to be succesful.


If you have any ideas on this issue I can help out with debugging/testing.

Tomas


[4.962509] Registered IR keymap rc-rc6-mce
[4.963166] input: Nuvoton w836x7hg Infrared Remote Transceiver as 
/devices/pnp0/00:05/rc/rc0/input6
[4.963656] rc0: Nuvoton w836x7hg Infrared Remote Transceiver as 
/devices/pnp0/00:05/rc/rc0

[4.965433] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[4.966211] input: MCE IR Keyboard/Mouse (nuvoton-cir) as 
/devices/virtual/input/input7

[4.966222]
[4.966266] ==
[4.966317] [ INFO: possible circular locking dependency detected ]
[4.966371] 3.17.0-rc2 #12 Not tainted
[4.966417] ---
[4.966469] modprobe/642 is trying to acquire lock:
[4.966518]  (input_mutex){+.+.+.}, at: [] 
input_register_device+0x2ba/0x381

[4.966729]
[4.966729] but task is already holding lock:
[4.966779]  (ir_raw_handler_lock){+.+.+.}, at: [] 
ir_raw_event_register+0x102/0x190

[4.966990]
[4.966990] which lock already depends on the new lock.
[4.966990]
[4.967060]
[4.967060] the existing dependency chain (in reverse order) is:
[4.967144]
[4.967144] -> #3 (ir_raw_handler_lock){+.+.+.}:
[4.967386][] lock_acquire+0xd9/0x111
[4.967501][] mutex_lock_nested+0x57/0x37a
[4.967616][] ir_raw_event_register+0x102/0x190
[4.967731][] rc_register_device+0x39e/0x58a
[4.967846][] nvt_probe+0x5ad/0xd52 
[nuvoton_cir]

[4.967963][] pnp_device_probe+0x8c/0xa9
[4.968079][] driver_probe_device+0xa1/0x1e3
[4.968195][] __driver_attach+0x4e/0x6f
[4.968309][] bus_for_each_dev+0x5a/0x8c
[4.968425][] driver_attach+0x19/0x1b
[4.968529][] bus_add_driver+0xf1/0x1d6
[4.968633][] driver_register+0x87/0xbe
[4.968737][] pnp_register_driver+0x1c/0x1e
[4.968842][] nvt_init+0x10/0x1000 
[nuvoton_cir]

[4.968948][] do_one_initcall+0xea/0x18c
[4.969054][] load_module+0x1c21/0x1f2c
[4.969159][] SyS_init_module+0xa2/0xb1
[4.969263][] system_call_fastpath+0x16/0x1b
[4.969384]
[4.969384] -> #2 (&dev->lock){+.+.+.}:
[4.969625][] lock_acquire+0xd9/0x111
[4.969739][] mutex_lock_nested+0x57/0x37a
[4.969853][] rc_open+0x24/0x70
[4.969966][] ir_open+0x10/0x12
[4.970078][] input_open_device+0x64/0x91
[4.970193][] kbd_connect+0x55/0x87
[4.970307][] input_attach_handler+0x1aa/0x1dc
[4.970425][] input_register_device+0x2ff/0x381
[4.970530][] rc_register_device+0x550/0x58a
[4.970635][] nvt_probe+0x5ad/0xd52 
[nuvoton_cir]

[4.970742][] pnp_device_probe+0x8c/0xa9
[4.970846][] driver_probe_device+0xa1/0x1e3
[4.970951][] __driver_attach+0x4e/0x6f
[4.971055][] bus_for_each_dev+0x5a/0x8c
[4.971159][] driver_attach+0x19/0x1b
[4.971263][] bus_add_driver+0xf1/0x1d6
[4.971378][] driver_register+0x87/0xbe
[4.971491][] pnp_register_driver+0x1c/0x1e
[4.971606][] nvt_init+0x10/0x1000 
[nuvoton_cir]

[4.971722][] do_one_initcall+0xea/0x18c
[4.971837][] load_module+0x1c21/0x1f2c
[4.971950][] SyS_init_module+0xa2/0xb1
[4.972063][] system_call_fastpath+0x16/0x1b
[4.972178]
[4.972178] -> #1 (&dev->mutex#2){+.+...}:
[4.972462][] lock_acquire+0xd9/0x111
[4.972566][] 
mutex_lock_interruptible_nested+0x57/0x381

[4.972688][] input_register_handle+0x2c/0xbf
[4.972794][] mousedev_create+0x244/0x2c7
[4.972898][] mousedev_connect+0x1b/0xd2
[4.973003][] input_attach_handler+0x1aa/0x1dc
[4.973107][] input_register_device+0x2ff/0x381
[4.973213][] hidinput_connect+0x30ec/0x316c
[4.973319][] hid_connect+0x6e/0x26f
[4.973440][] hid_device_probe+0xd5/0x14b
[4.973554][] driver_probe_device+0xa1/0x1e3
[4.973669][] __device_attach+0x25/0x38
[4.973782][] bus_for_each_drv+0x52/0x8e
[4.973896][] device_attach+0x69/0x8a
[4.974009][] bus_probe_device+0x33/0xb5
[4.974123][] device_add+0x38a/0x532
[4.974235][] hid_add_device+0x1c2/0x1fe
[4.974349][] usbhid_probe+0x374/0x3bd [usbhid]
[4.974468][] 
usb_probe_interface+0x143/0x1da [usbcore]

[4.974598][] driver_probe_device+0xa1/0x1e3
[4.974703][] __driver_att