Prev: X86: Optimise fls(), ffs() and fls64()
Next: ext3: call blkdev_issue_flush on fsync
From: David Härdeman on 30 Mar 2010 07:10
On Sun, Mar 28, 2010 at 09:51:17PM -0300, Mauro Carvalho Chehab wrote:
>
> I spoke too soon... removing the index causes a problem at the read ioctl: there's no way
> to retrieve just the non-sparsed values.
>
> There's one solution that would allow both read/write and compat to work nicely,
> but the API would become somewhat asymmetrical:
>
> At get (EVIOCGKEYCODEBIG):
> use index/len as input and keycode/scancode as output;
>
> At set (EVIOCSKEYCODEBIG):
> use scancode/keycode/len as input (and, optionally, index as output).
>

This was exactly the approach I had in mind when I suggested using
indexes.

> Having it asymmetrical doesn't sound good, but, on the other hand,
> using index for
> the set function also doesn't seem good, as the driver may reorder the entries after
> setting, for example to work with a binary tree or with hashes.

I don't think the assymetry is really a problem. As I see it, there are
basically two user cases:

1) Userspace wants scancode X to generate keypress Y
(In which case userspace doesn't care one iota what the index is)

2) Userspace wants to get the current keytable from the kernel
(In which case a loop with an index from 0 to n is appropriate)

and, possibly:

3) Userspace wants to know what keycode (if any) scancode X generates
(In which case approach 2 will work just as well, but this usecase
seems a bit contrived anyway...)

--
David H�rdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Härdeman on 30 Mar 2010 07:20
On Sun, Mar 28, 2010 at 08:22:31PM -0300, Mauro Carvalho Chehab wrote:
> I also noticed another problem: kernel should have some way to report
> the expected
> size of the scancode to userspace, especially if we want to have the compatibility
> code (since, with compat, a scancode maximum size need to be 32 bits, otherwise
> the code won't work).
>
> I'll likely adding another control that returns the size of the scancode.

And perhaps the interface should explicitly define that for the case
where userspace sends an undersized scancode, the real scancode will be
generated by zero-extending the undersized scancode into its expected
size.

That way the interface will be binary-forwards-compatible even if
scancode sizes are increased at some later date.

--
David H�rdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Mauro Carvalho Chehab on 30 Mar 2010 08:50
David H�rdeman wrote:
> On Sun, Mar 28, 2010 at 08:22:31PM -0300, Mauro Carvalho Chehab wrote:
>> I also noticed another problem: kernel should have some way to report
>> the expected
>> size of the scancode to userspace, especially if we want to have the compatibility
>> code (since, with compat, a scancode maximum size need to be 32 bits, otherwise
>> the code won't work).
>>
>> I'll likely adding another control that returns the size of the scancode.
>
> And perhaps the interface should explicitly define that for the case
> where userspace sends an undersized scancode, the real scancode will be
> generated by zero-extending the undersized scancode into its expected
> size.
>
> That way the interface will be binary-forwards-compatible even if
> scancode sizes are increased at some later date.

Makes sense. Padding an undersized scancode is endian-dependent. So, we'll
likely need to add some padding functions. The better seems to add the logic
at include/linux/byteorder/generic.h.


--

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Mauro Carvalho Chehab on 31 Mar 2010 02:10
David H�rdeman wrote:
> On Sun, Mar 28, 2010 at 09:51:17PM -0300, Mauro Carvalho Chehab wrote:
>> I spoke too soon... removing the index causes a problem at the read ioctl: there's no way
>> to retrieve just the non-sparsed values.
>>
>> There's one solution that would allow both read/write and compat to work nicely,
>> but the API would become somewhat asymmetrical:
>>
>> At get (EVIOCGKEYCODEBIG):
>> use index/len as input and keycode/scancode as output;
>>
>> At set (EVIOCSKEYCODEBIG):
>> use scancode/keycode/len as input (and, optionally, index as output).
>>
>
> This was exactly the approach I had in mind when I suggested using
> indexes.

Doesn't work perfectly. The asymmetry has a side effect on the internal logic:

EVIOCGKEYCODEBIG should be implemented with a pseudo-code like:
kt_entry = getkeycodebig_from_index(index);

EVIOCSKEYCODEBIG should be implemented with a pseudo-code like:
kt_entry = getkeycodebig_from_scan(scan, len);
old_key = kt_entry->keycode;

kt_entry->keycode = newkey;
if (setkeycodebig(kt_entry) == 0)
keyup(old_key);

As you see, the input parameters for the getkeycodebig*() are different.

So, this approach requires 3 ops instead of 2. Yet, as scancode->keycode is
needed anyway, this doesn't actually hurts.

I just added the patches that implement those two ioctls on my IR development tree.
I tested only the original EVIOCGKEYCODE/EVIOSGKEYCODE and calling a clear_table
function using EVIOCSKEYCODEBIG via emulation.

My next step is to test the remaining ir-keytable functions via emulation, and then
implement the *BIG ioctls at ir-core, for testing.

I haven't test yet the *keycode*default methods.

After having it fully tested, I'll submit the complete input ioctl patch via ML.

--

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  | 
Pages: 1 2
Prev: X86: Optimise fls(), ffs() and fls64()
Next: ext3: call blkdev_issue_flush on fsync