Re: Handling of large keycodes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Em 02-08-2010 05:02, Dmitry Torokhov escreveu:
> On Sat, Jul 31, 2010 at 10:23:45AM -0300, Mauro Carvalho Chehab wrote:
>> Hi Dmitry,
>>
>> Em 31-07-2010 06:19, Dmitry Torokhov escreveu:
>>> Hi Mauro,
>>>
>>> I finally got a chance to review the patches adding handling of large
>>> scancodes to input core and there are a few things with this approach
>>> that I do not like.
>>
>> Thanks for the review!
>>
>>> First of all I do not think that we should be working with scancode via
>>> a pointer as it requires additional compat handling when running 32-bit
>>> userspace on 64-bit kernel. We can use a static buffer of sufficient
>>> size (lets say 32 bytes) to move scancode around and simply increase its
>>> size if we come upon device that uses even bigger scancodes. As long as
>>> buffer is at the end we can handle this in a compatible way.
>>
>> Yes, this is the downside of using a pointer. I'm not aware of a Remote
>> Controller protocol using more than 256 bits for scancode, so 32 bits
>> should be ok.
>>
>>> The other issue is that interface is notsymmetrical, setting is done by
>>> scancode but retrieval is done by index. I think we should be able to
>>> use both scancode and index in both operations.
>>
>> Yes, this also bothered me. I was thinking to do something similar to your
>> approach of having a bool to select between them. This change is welcome.
>>
>>> The usefulnes of reserved data elements in the structure is doubtful,
>>> since we do not seem to require them being set to a particular value and
>>> so we'll be unable to distinguish betwee legacy and newer users.
>>
>> David proposed some parameters that we rejected on our discussions. As we
>> might need to add something similar, I decided to keep it on my approach,
>> since a set of reserved fields wouldn't hurt (and removing it on our discussions
>> would be easy), but I'm ok on removing them.
>>
>>> I also concerned about the code very messy with regard to using old/new
>>> style interfaces instea dof converting old ones to use new
>>> insfrastructure,
>>
>> Good cleanup at the code!
>>
>>> I below is something that addresses these issues and seems to be working
>>> for me. It is on top of your patches and it also depends on a few
>>> changes in my tree that I have not publushed yet but plan on doing that
>>> tomorrow. I am also attaching patches converting sparse keymap and hid
>>> to the new style of getkeycode and setkeycode as examples.
>>>
>>> Please take a look and let me know if I missed something important.
>>
>> It seems to work for me. After you add the patches on your git tree, I'll 
>> work on porting the RC core to the new approach, and change the ir-keycode
>> userspace program to work with, in order to be 100% sure that it will work, 
>> but I can't foresee any missing part on it.
>>
>> Currently, I'm not using my input patches, as I was waiting for your
>> review. I just patched the userspace application, in order to test the legacy
>> mode.
>>
> 
> OK, great.
> 
> I want to fold all the patches from your tree plus this one into one and
> apply in one shpt (mentioning Jarod and Dan as authors of fixup patches
> in the changelog) - I do not believe we shoudl expose intermediate
> versions in the code that will go to Linus. Are you OK with this?

I'm OK. If you want, you can add my ack on your patch:

Acked-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>

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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux