lkml.org 
[lkml]   [2014]   [Oct]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 1/2] [media] rc-core: fix protocol chan ge regression in ir raw event register
Date
From
On 2014-10-26 20:33, Tomas Melin wrote:
> On Sat, Oct 25, 2014 at 12:03 PM, David Härdeman <david@hardeman.nu>
> 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.

The empty function was added for a reason? The presence of a
change_protocol function implies that the receiver supports protocol
changing (whether it's via the raw IR decoding or in hardware).

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

change_protocol is only set if it's a driver that does in-kernel
decoding...meaning that it generates pulse/space timings...meaning that
hardware protocol changes aren't necessary?

>> 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() ).

First, enabled_protocols is already taken care of in the above patch
(the line after "goto out_raw" is "dev->enabled_protocols = rc_type;")?

Second, initializing enabled_protocols to some default in
ir_raw_event_register() might not be strictly necessary but it also
doesn't hurt since that happens before dev->change_protocol() is called
in rc_register_device()?

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

I'd prefer the above, minimal, approach. But it's Mauro who decides in
the end.

Regards,
David

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2014-10-28 10:21    [W:0.071 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site