Messages in this thread | | | Subject | Re: [PATCH v2 1/2] [media] rc-core: fix protocol chan ge regression in ir raw event register | Date | Tue, 28 Oct 2014 09:42:30 +0100 | From | David Härdeman <> |
| |
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/
| |