[PATCH 5/6] Input: ati-remote2 - switch to using new keycode interface
Switch the code to use new style of getkeycode and setkeycode
methods to allow retrieving and setting keycodes not only by
their scancodes but also by index.
Signed-off-by: Dmitry Torokhov
---
drivers/input/misc/ati_remote2.c | 93 +++---
1 files changed, 65 insertions(+), 28 deletions(-)
diff --git a/drivers/input/misc/ati_remote2.c b/drivers/input/misc/ati_remote2.c
index 2325765..b2e0d82 100644
--- a/drivers/input/misc/ati_remote2.c
+++ b/drivers/input/misc/ati_remote2.c
@@ -483,51 +483,88 @@ static void ati_remote2_complete_key(struct urb *urb)
}
static int ati_remote2_getkeycode(struct input_dev *idev,
- unsigned int scancode, unsigned int *keycode)
+ struct input_keymap_entry *ke)
{
struct ati_remote2 *ar2 = input_get_drvdata(idev);
unsigned int mode;
- int index;
+ int offset;
+ unsigned int index;
+ unsigned int scancode;
+
+ if (ke->flags & INPUT_KEYMAP_BY_INDEX) {
+ index = ke->index;
+ if (index >= (ATI_REMOTE2_MODES - 1) *
+ ARRAY_SIZE(ati_remote2_key_table))
+ return -EINVAL;
+
+ mode = ke->index / ARRAY_SIZE(ati_remote2_key_table);
+ offset = ke->index % ARRAY_SIZE(ati_remote2_key_table);
+ scancode = (mode << 8) + ati_remote2_key_table[offset].hw_code;
+ } else {
+ if (input_scancode_to_scalar(ke, &scancode))
+ return -EINVAL;
+
+ mode = scancode >> 8;
+ if (mode > ATI_REMOTE2_PC)
+ return -EINVAL;
+
+ offset = ati_remote2_lookup(scancode & 0xff);
+ if (offset < 0)
+ return -EINVAL;
+
+ index = mode * ARRAY_SIZE(ati_remote2_key_table) + offset;
+ }
- mode = scancode >> 8;
- if (mode > ATI_REMOTE2_PC || !((1 << mode) & ar2->mode_mask))
- return -EINVAL;
+ ke->keycode = ar2->keycode[mode][offset];
+ ke->len = sizeof(scancode);
+ memcpy(&ke->scancode, &scancode, sizeof(scancode));
+ ke->index = index;
- index = ati_remote2_lookup(scancode & 0xFF);
- if (index < 0)
- return -EINVAL;
-
- *keycode = ar2->keycode[mode][index];
return 0;
}
static int ati_remote2_setkeycode(struct input_dev *idev,
- unsigned int scancode, unsigned int keycode)
+ const struct input_keymap_entry *ke,
+ unsigned int *old_keycode)
{
struct ati_remote2 *ar2 = input_get_drvdata(idev);
- unsigned int mode, old_keycode;
- int index;
-
- mode = scancode >> 8;
- if (mode > ATI_REMOTE2_PC || !((1 << mode) & ar2->mode_mask))
- return -EINVAL;
-
- index = ati_remote2_lookup(scancode & 0xFF);
- if (index < 0)
- return -EINVAL;
+ unsigned int mode;
+ int offset;
+ unsigned int index;
+ unsigned int scancode;
+
+ if (ke->flags & INPUT_KEYMAP_BY_INDEX) {
+ if (ke->index >= (ATI_REMOTE2_MODES - 1) *
+ ARRAY_SIZE(ati_remote2_key_table))
+ return -EINVAL;
+
+ mode = ke->index / ARRAY_SIZE(ati_remote2_key_table);
+ offset = ke->index % ARRAY_SIZE(ati_remote2_key_table);
+ } else {
+ if (input_scancode_to_scalar(ke, &scancode))
+ return -EINVAL;
+
+ mode = scancode >> 8;
+ if (mode > ATI_REMOTE2_PC)
+ return -EINVAL;
+
+ offset = ati_remote2_lookup(scancode & 0xff);
+ if (offset < 0)
+ return -EINVAL;
+ }
- old_keycode = ar2->keycode[mode][index];
- ar2->keycode[mode][index] = keycode;
- __set_bit(keycode, idev->keybit);
+ *old_keycode = ar2->keycode[mode][offset];
+ ar2->keycode[mode][offset] = ke->keycode;
+ __set_bit(ke->keycode, idev->keybit);
for (mode = 0; mode < ATI_REMOTE2_MODES; mode++) {
for (index = 0; index < ARRAY_SIZE(ati_remote2_key_table);
index++) {
- if (ar2->keycode[mode][index] == old_keycode)
+ if (ar2->keycode[mode][index] == *old_keycode)
return 0;
}
}
- __clear_bit(old_keycode, idev->keybit);
+ __clear_bit(*old_keycode, idev->keybit);
return 0;
}
@@ -575,8 +612,8 @@ static int ati_remote2_input_init(struct ati_remote2 *ar2)
idev->open = ati_remote2_open;
idev->close = ati_remote2_close;
- idev->getkeycode = ati_remote2_getkeycode;
- idev->setkeycode = ati_remote2_setkeycode;
+ idev->getkeycode_new = ati_remote2_getkeycode;
+ idev
Re: [PATCH 5/6] Input: ati-remote2 - switch to using new keycode interface
On Wed, Sep 08, 2010 at 12:42:05AM -0700, Dmitry Torokhov wrote:
> Switch the code to use new style of getkeycode and setkeycode
> methods to allow retrieving and setting keycodes not only by
> their scancodes but also by index.
>
> Signed-off-by: Dmitry Torokhov
> ---
>
> drivers/input/misc/ati_remote2.c | 93
> +++---
> 1 files changed, 65 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/input/misc/ati_remote2.c
> b/drivers/input/misc/ati_remote2.c
> index 2325765..b2e0d82 100644
> --- a/drivers/input/misc/ati_remote2.c
> +++ b/drivers/input/misc/ati_remote2.c
> @@ -483,51 +483,88 @@ static void ati_remote2_complete_key(struct urb *urb)
> }
>
> static int ati_remote2_getkeycode(struct input_dev *idev,
> - unsigned int scancode, unsigned int *keycode)
> + struct input_keymap_entry *ke)
> {
> struct ati_remote2 *ar2 = input_get_drvdata(idev);
> unsigned int mode;
> - int index;
> + int offset;
> + unsigned int index;
> + unsigned int scancode;
> +
> + if (ke->flags & INPUT_KEYMAP_BY_INDEX) {
> + index = ke->index;
> + if (index >= (ATI_REMOTE2_MODES - 1) *
That -1 looks wrong. Same in setkeycode().
> + ARRAY_SIZE(ati_remote2_key_table))
> + return -EINVAL;
> +
> + mode = ke->index / ARRAY_SIZE(ati_remote2_key_table);
> + offset = ke->index % ARRAY_SIZE(ati_remote2_key_table);
> + scancode = (mode << 8) + ati_remote2_key_table[offset].hw_code;
> + } else {
> + if (input_scancode_to_scalar(ke, &scancode))
> + return -EINVAL;
> +
> + mode = scancode >> 8;
> + if (mode > ATI_REMOTE2_PC)
> + return -EINVAL;
> +
> + offset = ati_remote2_lookup(scancode & 0xff);
> + if (offset < 0)
> + return -EINVAL;
> +
> + index = mode * ARRAY_SIZE(ati_remote2_key_table) + offset;
> + }
>
> - mode = scancode >> 8;
> - if (mode > ATI_REMOTE2_PC || !((1 << mode) & ar2->mode_mask))
> - return -EINVAL;
You're removing the mode_mask check here, but I think that's fine. I
don't see why the keymap shouldn't be allowed to be queried/modified for
the unused modes.
> + ke->keycode = ar2->keycode[mode][offset];
> + ke->len = sizeof(scancode);
> + memcpy(&ke->scancode, &scancode, sizeof(scancode));
The scancodes fit into two bytes each. Does it matter that you're
using 4 bytes here?
> + ke->index = index;
>
> - index = ati_remote2_lookup(scancode & 0xFF);
> - if (index < 0)
> - return -EINVAL;
> -
> - *keycode = ar2->keycode[mode][index];
> return 0;
> }
>
> static int ati_remote2_setkeycode(struct input_dev *idev,
> - unsigned int scancode, unsigned int keycode)
> + const struct input_keymap_entry *ke,
> + unsigned int *old_keycode)
> {
> struct ati_remote2 *ar2 = input_get_drvdata(idev);
> - unsigned int mode, old_keycode;
> - int index;
> -
> - mode = scancode >> 8;
> - if (mode > ATI_REMOTE2_PC || !((1 << mode) & ar2->mode_mask))
> - return -EINVAL;
> -
> - index = ati_remote2_lookup(scancode & 0xFF);
> - if (index < 0)
> - return -EINVAL;
> + unsigned int mode;
> + int offset;
> + unsigned int index;
> + unsigned int scancode;
> +
> + if (ke->flags & INPUT_KEYMAP_BY_INDEX) {
> + if (ke->index >= (ATI_REMOTE2_MODES - 1) *
> + ARRAY_SIZE(ati_remote2_key_table))
> + return -EINVAL;
> +
> + mode = ke->index / ARRAY_SIZE(ati_remote2_key_table);
> + offset = ke->index % ARRAY_SIZE(ati_remote2_key_table);
> + } else {
> + if (input_scancode_to_scalar(ke, &scancode))
> + return -EINVAL;
> +
> + mode = scancode >> 8;
> + if (mode > ATI_REMOTE2_PC)
> + return -EINVAL;
> +
> + offset = ati_remote2_lookup(scancode & 0xff);
> + if (offset < 0)
> + return -EINVAL;
> + }
>
> - old_keycode = ar2->keycode[mode][index];
> - ar2->keycode[mode][index] = keycode;
> - __set_bit(keycode, idev->keybit);
> + *old_keycode = ar2->keycode[mode][offset];
> + ar2->keycode[mode][offset] = ke->keycode;
> + __set_bit(ke->keycode, idev->keybit);
>
> for (mode = 0; mode < ATI_REMOTE2_MODES; mode++) {
> for (index = 0; index < ARRAY_SIZE(ati_remote2_key_table);
> index++) {
> - if (ar2->keycode[mode][index] == old_keycode)
> + if (ar2->keycode[mode][index] == *old_keycode)
>
Re: [PATCH 5/6] Input: ati-remote2 - switch to using new keycode interface
On Thu, Sep 09, 2010 at 03:40:04PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 08, 2010 at 12:42:05AM -0700, Dmitry Torokhov wrote:
> > Switch the code to use new style of getkeycode and setkeycode
> > methods to allow retrieving and setting keycodes not only by
> > their scancodes but also by index.
> >
> > Signed-off-by: Dmitry Torokhov
> > ---
> >
> > drivers/input/misc/ati_remote2.c | 93
> > +++---
> > 1 files changed, 65 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/input/misc/ati_remote2.c
> > b/drivers/input/misc/ati_remote2.c
> > index 2325765..b2e0d82 100644
> > --- a/drivers/input/misc/ati_remote2.c
> > +++ b/drivers/input/misc/ati_remote2.c
> > @@ -483,51 +483,88 @@ static void ati_remote2_complete_key(struct urb *urb)
> > }
> >
> > static int ati_remote2_getkeycode(struct input_dev *idev,
> > - unsigned int scancode, unsigned int *keycode)
> > + struct input_keymap_entry *ke)
> > {
> > struct ati_remote2 *ar2 = input_get_drvdata(idev);
> > unsigned int mode;
> > - int index;
> > + int offset;
> > + unsigned int index;
> > + unsigned int scancode;
> > +
> > + if (ke->flags & INPUT_KEYMAP_BY_INDEX) {
> > + index = ke->index;
> > + if (index >= (ATI_REMOTE2_MODES - 1) *
>
> That -1 looks wrong. Same in setkeycode().
>
Yes, indeed. Thanks for noticing.
> > + ARRAY_SIZE(ati_remote2_key_table))
> > + return -EINVAL;
> > +
> > + mode = ke->index / ARRAY_SIZE(ati_remote2_key_table);
> > + offset = ke->index % ARRAY_SIZE(ati_remote2_key_table);
> > + scancode = (mode << 8) + ati_remote2_key_table[offset].hw_code;
> > + } else {
> > + if (input_scancode_to_scalar(ke, &scancode))
> > + return -EINVAL;
> > +
> > + mode = scancode >> 8;
> > + if (mode > ATI_REMOTE2_PC)
> > + return -EINVAL;
> > +
> > + offset = ati_remote2_lookup(scancode & 0xff);
> > + if (offset < 0)
> > + return -EINVAL;
> > +
> > + index = mode * ARRAY_SIZE(ati_remote2_key_table) + offset;
> > + }
> >
> > - mode = scancode >> 8;
> > - if (mode > ATI_REMOTE2_PC || !((1 << mode) & ar2->mode_mask))
> > - return -EINVAL;
>
> You're removing the mode_mask check here, but I think that's fine. I
> don't see why the keymap shouldn't be allowed to be queried/modified for
> the unused modes.
Rigth, that was my justification for removal of the check.
>
> > + ke->keycode = ar2->keycode[mode][offset];
> > + ke->len = sizeof(scancode);
> > + memcpy(&ke->scancode, &scancode, sizeof(scancode));
>
> The scancodes fit into two bytes each. Does it matter that you're
> using 4 bytes here?
The old interface used 4 bytes and I think it just easier this way. I
think userspace will default to 4-byte scancodes unless they know they
need to handle bigger ones.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] Input: ati-remote2 - switch to using new keycode interface
On Mon, Sep 13, 2010 at 09:28:07AM -0700, Dmitry Torokhov wrote:
> On Thu, Sep 09, 2010 at 03:40:04PM +0300, Ville Syrjälä wrote:
> > On Wed, Sep 08, 2010 at 12:42:05AM -0700, Dmitry Torokhov wrote:
> > > Switch the code to use new style of getkeycode and setkeycode
> > > methods to allow retrieving and setting keycodes not only by
> > > their scancodes but also by index.
> > >
> > > Signed-off-by: Dmitry Torokhov
> > > ---
> > >
> > > drivers/input/misc/ati_remote2.c | 93
> > > +++---
> > > 1 files changed, 65 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/drivers/input/misc/ati_remote2.c
> > > b/drivers/input/misc/ati_remote2.c
> > > index 2325765..b2e0d82 100644
> > > --- a/drivers/input/misc/ati_remote2.c
> > > +++ b/drivers/input/misc/ati_remote2.c
> > > @@ -483,51 +483,88 @@ static void ati_remote2_complete_key(struct urb
> > > *urb)
> > > }
> > >
> > > static int ati_remote2_getkeycode(struct input_dev *idev,
> > > - unsigned int scancode, unsigned int *keycode)
> > > + struct input_keymap_entry *ke)
> > > {
> > > struct ati_remote2 *ar2 = input_get_drvdata(idev);
> > > unsigned int mode;
> > > - int index;
> > > + int offset;
> > > + unsigned int index;
> > > + unsigned int scancode;
> > > +
> > > + if (ke->flags & INPUT_KEYMAP_BY_INDEX) {
> > > + index = ke->index;
> > > + if (index >= (ATI_REMOTE2_MODES - 1) *
> >
> > That -1 looks wrong. Same in setkeycode().
> >
>
> Yes, indeed. Thanks for noticing.
I fixed this bug locally and gave this a short whirl with my RWII.
I tried both the old and new style keycode ioctls. Everything
worked as expected.
So if you want more tags feel free to add my Acked-by and Tested-by
for this (assuming the off-by-one fix is included) and you can add my
Tested-by for patch 1/6 as well.
--
Ville Syrjälä
syrj...@sci.fi
http://www.sci.fi/~syrjala/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] Input: ati-remote2 - switch to using new keycode interface
On Thu, Sep 16, 2010 at 12:04:19AM +0300, Ville Syrjälä wrote:
> On Mon, Sep 13, 2010 at 09:28:07AM -0700, Dmitry Torokhov wrote:
> > On Thu, Sep 09, 2010 at 03:40:04PM +0300, Ville Syrjälä wrote:
> > > On Wed, Sep 08, 2010 at 12:42:05AM -0700, Dmitry Torokhov wrote:
> > > > Switch the code to use new style of getkeycode and setkeycode
> > > > methods to allow retrieving and setting keycodes not only by
> > > > their scancodes but also by index.
> > > >
> > > > Signed-off-by: Dmitry Torokhov
> > > > ---
> > > >
> > > > drivers/input/misc/ati_remote2.c | 93
> > > > +++---
> > > > 1 files changed, 65 insertions(+), 28 deletions(-)
> > > >
> > > > diff --git a/drivers/input/misc/ati_remote2.c
> > > > b/drivers/input/misc/ati_remote2.c
> > > > index 2325765..b2e0d82 100644
> > > > --- a/drivers/input/misc/ati_remote2.c
> > > > +++ b/drivers/input/misc/ati_remote2.c
> > > > @@ -483,51 +483,88 @@ static void ati_remote2_complete_key(struct urb
> > > > *urb)
> > > > }
> > > >
> > > > static int ati_remote2_getkeycode(struct input_dev *idev,
> > > > - unsigned int scancode, unsigned int
> > > > *keycode)
> > > > + struct input_keymap_entry *ke)
> > > > {
> > > > struct ati_remote2 *ar2 = input_get_drvdata(idev);
> > > > unsigned int mode;
> > > > - int index;
> > > > + int offset;
> > > > + unsigned int index;
> > > > + unsigned int scancode;
> > > > +
> > > > + if (ke->flags & INPUT_KEYMAP_BY_INDEX) {
> > > > + index = ke->index;
> > > > + if (index >= (ATI_REMOTE2_MODES - 1) *
> > >
> > > That -1 looks wrong. Same in setkeycode().
> > >
> >
> > Yes, indeed. Thanks for noticing.
>
> I fixed this bug locally and gave this a short whirl with my RWII.
> I tried both the old and new style keycode ioctls. Everything
> worked as expected.
>
> So if you want more tags feel free to add my Acked-by and Tested-by
> for this (assuming the off-by-one fix is included)
Thank you very much for reviewing and testing it Ville, I will surely
add the tags.
and you can add my
> Tested-by for patch 1/6 as well.
>
This one is already in public branch; I prefer not to rewind unless
there are compile or other major issues
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
