linux-kernel - Re: [patch 2/2] Add a driver for the Winbond WPCD376I Consumer IR hardware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 13 Aug 2009 19:58:43 +0200
From:	David Härdeman <david@...deman.nu>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:	linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
	jbarnes@...tuousgeek.org, akpm@...ux-foundation.org
Subject: Re: [patch 2/2] Add a driver for the Winbond WPCD376I Consumer IR
 hardware

On Thu, Aug 13, 2009 at 08:56:37AM -0700, Dmitry Torokhov wrote:
>On Thu, Aug 13, 2009 at 11:34:44AM +0200, David Härdeman wrote:
>> The main problem right now is that getkeycode is practically useless since
>> you can't blindly guess at a full range of 2^32 different scancodes to get
>> the complete keymap. Perhaps a index-based getkeycode would make sense...
>
>The drivers that have such sparce keymaps are expected to issue
>EV_MSC/MSC_SCAN events to aid userspace in identifying the "scancodes"
>that are emitted by the device.

Ok, I've added EV_MSC/MSC_SCAN support.

>>>> +static struct device_attribute dev_attr_last_scancode = {
>>>> +	.attr = {
>>>> +		.name = "last_scancode",
>>>> +		.mode = 0444,
>>>> +	},
>>>> +	.show = wbcir_show_last_scancode,
>>>> +	.store = NULL,
>>>> +
>>>> +};
>>>
>>> Why is this needed? And if this is needed we have a nice macro
>>> for that.
>>
>> I hope I've explained it wrt. EV_IR in my other mail. It's for building
>> keymaps of unknown remotes. And it'll go away once EV_IR is supported so I
>> don't think there's much point in fiddling with it now?
>>
>
>Because once the driver is in mainline it becomes part of userspace ABI
>and has to stay for a looong time.

I've removed the sysfs attribute as EV_MSC/MSC_SCAN provides the same 
functionality.

>> Thanks for the review. Are you willing to push the driver upstream through
>> the input tree once I've implemented your suggested fixes?
>>
>
>I'd need to take a look at your EV_IR patyches and see how they will
>affect this driver. I do nt want to merge something that will stay one
>way for half a release and then will switch to completely new interface.

The EV_IR functionality is intended to be used in addition to regular 
key up/down/repeat events. Much like EV_MSC/MSC_SCAN but more 
descriptive and specific to IR protocols. Then advanced user-space apps 
can choose whether to use the in-kernel keymap or map remote codes 
directly to suitable events (and it allows remote keymaps to be built 
easily in user-space).

As the future EV_IR functionality is additional to the current 
functionality, I hope the driver still can be merged before I have EV_IR 
patches ready for review. I'll post an updated patch shortly.

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