[PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[bjd]

From: Bauke Jan Douma <bjdouma@xs4all.nl>

Add two new ioctl's to have the input driver return actual current values for
EV_REP and EV_SND event codes.

Currently there is no ioctl to retrieve EV_REP values, even though they have
actually always been stored in dev->rep.  A new ioctl, EVIOCGREPCODE, retrieves
them.

The existing EVCGSND ioctl has never returned anything meaningful; the relevant
fragment in input.c was missing even a change_bit() call.
The actual EV_SND values are now written in dev->snd.  To make this work,
dev->snd had to be made an int array, and as a consequence the EVICGSND ioctl
became problematic.  I have removed it in this diff, but --even though it never
has returned anything meaningful-- I'm not quite sure that's the right thing to
do, so I would appreciate feedback on this.
Anyway, an EVIOCGSNDCODE ioctl was added to retrieve these values.

I have named the two ioctl's EVIOCGREPCODE and EVIOCGSNDCODE; I didn't want
to use EVIOCGSND for obvious reasons, and since the ioctl's retrieve the value
of an event _code_, I named them in this way, more or less analogue to
EVIOCGKEYCODE -- after all the input driver works with type, code, value, which
is nicely reflected in the naming and argument.  Feedback appreciated though.

The ioctl's btw. currently work analoguous to EVIOCKEYCODE; they must be given
an int[2] argument, where int[0] is exactly one event code (of the appropriate
event type) of which the value is queried; the value will be returned in int[1].


Signed-off-by: Bauke Jan Douma <bjdouma@xs4all.nl>

---

Patch is against 2.6.16 proper.


diff -uprN a/drivers/input/evdev.c b/drivers/input/evdev.c
--- ./linux/drivers/input/evdev.c.orig	2006-03-22 23:34:42.000000000 +0100
+++ ./linux/drivers/input/evdev.c	2006-04-22 21:31:43.000000000 +0200
@@ -407,6 +407,24 @@ static long evdev_ioctl_handler(struct f
 
 			return 0;
 
+		case EVIOCGREPCODE:
+			if (get_user(t, ip))
+				return -EFAULT;
+			if (t < 0 || t >= REP_MAX + 1)
+				return -EINVAL;
+			if (put_user(dev->rep[t], ip + 1))
+				return -EFAULT;
+			return 0;
+
+		case EVIOCGSNDCODE:
+			if (get_user(t, ip))
+				return -EFAULT;
+			if (t < 0 || t >= SND_MAX + 1)
+				return -EINVAL;
+			if (put_user(dev->snd[t], ip + 1))
+				return -EFAULT;
+			return 0;
+
 		case EVIOCGKEYCODE:
 			if (get_user(t, ip))
 				return -EFAULT;
@@ -513,10 +531,6 @@ static long evdev_ioctl_handler(struct f
 					return bits_to_user(dev->led, LED_MAX, _IOC_SIZE(cmd),
 							    p, compat_mode);
 
-				if (_IOC_NR(cmd) == _IOC_NR(EVIOCGSND(0)))
-					return bits_to_user(dev->snd, SND_MAX, _IOC_SIZE(cmd),
-							    p, compat_mode);
-
 				if (_IOC_NR(cmd) == _IOC_NR(EVIOCGSW(0)))
 					return bits_to_user(dev->sw, SW_MAX, _IOC_SIZE(cmd),
 							    p, compat_mode);
diff -uprN a/drivers/input/input.c b/drivers/input/input.c
--- ./linux/drivers/input/input.c.orig	2006-03-22 23:34:42.000000000 +0100
+++ ./linux/drivers/input/input.c	2006-04-22 21:31:43.000000000 +0200
@@ -153,6 +153,7 @@ void input_event(struct input_dev *dev, 
 			if (code > SND_MAX || !test_bit(code, dev->sndbit))
 				return;
 
+			dev->snd[code] = value;
 			if (dev->event) dev->event(dev, type, code, value);
 
 			break;
diff -uprN a/include/linux/input.h b/include/linux/input.h
--- ./linux/include/linux/input.h.orig	2006-03-22 23:34:50.000000000 +0100
+++ ./linux/include/linux/input.h	2006-04-22 21:31:43.000000000 +0200
@@ -59,6 +59,8 @@ struct input_absinfo {
 #define EVIOCGVERSION		_IOR('E', 0x01, int)			/* get driver version */
 #define EVIOCGID		_IOR('E', 0x02, struct input_id)	/* get device ID */
 #define EVIOCGKEYCODE		_IOR('E', 0x04, int[2])			/* get keycode */
+#define EVIOCGREPCODE		_IOR('E', 0x10, int[2])			/* get an EV_REP setting */
+#define EVIOCGSNDCODE		_IOR('E', 0x11, int[2])			/* get an EV_SND setting */
 #define EVIOCSKEYCODE		_IOW('E', 0x04, int[2])			/* set keycode */
 
 #define EVIOCGNAME(len)		_IOC(_IOC_READ, 'E', 0x06, len)		/* get device name */
@@ -67,7 +69,6 @@ struct input_absinfo {
 
 #define EVIOCGKEY(len)		_IOC(_IOC_READ, 'E', 0x18, len)		/* get global keystate */
 #define EVIOCGLED(len)		_IOC(_IOC_READ, 'E', 0x19, len)		/* get all LEDs */
-#define EVIOCGSND(len)		_IOC(_IOC_READ, 'E', 0x1a, len)		/* get all sounds status */
 #define EVIOCGSW(len)		_IOC(_IOC_READ, 'E', 0x1b, len)		/* get all switch states */
 
 #define EVIOCGBIT(ev,len)	_IOC(_IOC_READ, 'E', 0x20 + ev, len)	/* get event bits */
@@ -908,10 +909,10 @@ struct input_dev {
 
 	int abs[ABS_MAX + 1];
 	int rep[REP_MAX + 1];
+	int snd[SND_MAX + 1];
 
 	unsigned long key[NBITS(KEY_MAX)];
 	unsigned long led[NBITS(LED_MAX)];
-	unsigned long snd[NBITS(SND_MAX)];
 	unsigned long sw[NBITS(SW_MAX)];
 
 	int absmax[ABS_MAX + 1];

Re: [PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[Dmitry Torokhov]

On 4/22/06, bjd <bjdouma@xs4all.nl> wrote:
>
> From: Bauke Jan Douma <bjdouma@xs4all.nl>
>

Hi Bauke,

Thank you for your patch.

> Add two new ioctl's to have the input driver return actual current values for
> EV_REP and EV_SND event codes.
>
> Currently there is no ioctl to retrieve EV_REP values, even though they have
> actually always been stored in dev->rep.  A new ioctl, EVIOCGREPCODE,
> retrieves them.
>

EVIOCGREP and EVIOCSREP ioctls are present in 2.4 but they have been
removed during 2.6 development. If you need to get/set repeat delay
and period you need to use KDKBDREP ioctl; it will change the repeat
rate for all keyboards attached to the box.

Vojtech, could you remind me why EVIOC{G|S}REP were removed? Some
people want to have ability to separate keyboards (via grabbing); they
also might want to control repeat rate independently. Shoudl we
reinstate these ioctls?

> The existing EVCGSND ioctl has never returned anything meaningful; the relevant
> fragment in input.c was missing even a change_bit() call.
> The actual EV_SND values are now written in dev->snd.  To make this work,
> dev->snd had to be made an int array, and as a consequence the EVICGSND ioctl
> became problematic.  I have removed it in this diff, but --even though it never
> has returned anything meaningful-- I'm not quite sure that's the right thing to
> do, so I would appreciate feedback on this.
> Anyway, an EVIOCGSNDCODE ioctl was added to retrieve these values.

I think we should just fix EVCGSND and just allow userspace to query
which sound evvects are active fro device - IOW just return bitmap
like we do for keys and leds and switches. I don't think actuall
"value" of the SND_TONE is interesting to anyone.

--
Dmitry

Re: [PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[Vojtech Pavlik]

On Mon, Apr 24, 2006 at 10:31:39AM -0400, Dmitry Torokhov wrote:
> On 4/22/06, bjd <bjdouma@xs4all.nl> wrote:
> >
> > From: Bauke Jan Douma <bjdouma@xs4all.nl>
> >
> 
> Hi Bauke,
> 
> Thank you for your patch.
> 
> > Add two new ioctl's to have the input driver return actual current values for
> > EV_REP and EV_SND event codes.
> >
> > Currently there is no ioctl to retrieve EV_REP values, even though they have
> > actually always been stored in dev->rep.  A new ioctl, EVIOCGREPCODE,
> > retrieves them.
> >
> 
> EVIOCGREP and EVIOCSREP ioctls are present in 2.4 but they have been
> removed during 2.6 development. If you need to get/set repeat delay
> and period you need to use KDKBDREP ioctl; it will change the repeat
> rate for all keyboards attached to the box.
> 
> Vojtech, could you remind me why EVIOC{G|S}REP were removed? Some
> people want to have ability to separate keyboards (via grabbing); they
> also might want to control repeat rate independently. Shoudl we
> reinstate these ioctls?

I believe they were replaced by the ability to send EV_REP style events
to the device, setting the repeat rate.

> > The existing EVCGSND ioctl has never returned anything meaningful; the relevant
> > fragment in input.c was missing even a change_bit() call.
> > The actual EV_SND values are now written in dev->snd.  To make this work,
> > dev->snd had to be made an int array, and as a consequence the EVICGSND ioctl
> > became problematic.  I have removed it in this diff, but --even though it never
> > has returned anything meaningful-- I'm not quite sure that's the right thing to
> > do, so I would appreciate feedback on this.
> > Anyway, an EVIOCGSNDCODE ioctl was added to retrieve these values.
> 
> I think we should just fix EVCGSND and just allow userspace to query
> which sound evvects are active fro device - IOW just return bitmap
> like we do for keys and leds and switches. I don't think actuall
> "value" of the SND_TONE is interesting to anyone.

Agreed.

-- 
Vojtech Pavlik
Director SuSE Labs

Re: [PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[Dmitry Torokhov]

On 4/24/06, Vojtech Pavlik <vojtech@suse.cz> wrote:
> On Mon, Apr 24, 2006 at 10:31:39AM -0400, Dmitry Torokhov wrote:
> >
> > Vojtech, could you remind me why EVIOC{G|S}REP were removed? Some
> > people want to have ability to separate keyboards (via grabbing); they
> > also might want to control repeat rate independently. Shoudl we
> > reinstate these ioctls?
>
> I believe they were replaced by the ability to send EV_REP style events
> to the device, setting the repeat rate.
>

Argh, why am I always forgetting about ability to write events into devices?

--
Dmitry

Re: [PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[Dmitry Torokhov]

On 4/24/06, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On 4/24/06, Vojtech Pavlik <vojtech@suse.cz> wrote:
> > On Mon, Apr 24, 2006 at 10:31:39AM -0400, Dmitry Torokhov wrote:
> > >
> > > Vojtech, could you remind me why EVIOC{G|S}REP were removed? Some
> > > people want to have ability to separate keyboards (via grabbing); they
> > > also might want to control repeat rate independently. Shoudl we
> > > reinstate these ioctls?
> >
> > I believe they were replaced by the ability to send EV_REP style events
> > to the device, setting the repeat rate.
> >
>
> Argh, why am I always forgetting about ability to write events into devices?
>

Thinking about it some more - writing to the event device is an
elegant way to set repeat rate but how do you retrieve current repeat
rate for a given device?

--
Dmitry

Re: [PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[Vojtech Pavlik]

On Tue, Apr 25, 2006 at 09:19:42AM -0400, Dmitry Torokhov wrote:
> On 4/24/06, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > On 4/24/06, Vojtech Pavlik <vojtech@suse.cz> wrote:
> > > On Mon, Apr 24, 2006 at 10:31:39AM -0400, Dmitry Torokhov wrote:
> > > >
> > > > Vojtech, could you remind me why EVIOC{G|S}REP were removed? Some
> > > > people want to have ability to separate keyboards (via grabbing); they
> > > > also might want to control repeat rate independently. Shoudl we
> > > > reinstate these ioctls?
> > >
> > > I believe they were replaced by the ability to send EV_REP style events
> > > to the device, setting the repeat rate.
> > >
> >
> > Argh, why am I always forgetting about ability to write events into devices?
> 
> Thinking about it some more - writing to the event device is an
> elegant way to set repeat rate but how do you retrieve current repeat
> rate for a given device?
 
You can't. And that's likely a problem that needs fixing.

-- 
Vojtech Pavlik
Director SuSE Labs

Re: [PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[Dmitry Torokhov]

On 4/25/06, Vojtech Pavlik <vojtech@suse.cz> wrote:
> On Tue, Apr 25, 2006 at 09:19:42AM -0400, Dmitry Torokhov wrote:
> > On 4/24/06, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > On 4/24/06, Vojtech Pavlik <vojtech@suse.cz> wrote:
> > > > On Mon, Apr 24, 2006 at 10:31:39AM -0400, Dmitry Torokhov wrote:
> > > > >
> > > > > Vojtech, could you remind me why EVIOC{G|S}REP were removed? Some
> > > > > people want to have ability to separate keyboards (via grabbing); they
> > > > > also might want to control repeat rate independently. Shoudl we
> > > > > reinstate these ioctls?
> > > >
> > > > I believe they were replaced by the ability to send EV_REP style events
> > > > to the device, setting the repeat rate.
> > > >
> > >
> > > Argh, why am I always forgetting about ability to write events into devices?
> >
> > Thinking about it some more - writing to the event device is an
> > elegant way to set repeat rate but how do you retrieve current repeat
> > rate for a given device?
>
> You can't. And that's likely a problem that needs fixing.
>

So do you agree that we need to ressurect EVIOCGREP (and EVIOCSREP
just to complement the interface)?

--
Dmitry

Re: [PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[Vojtech Pavlik]

On Tue, Apr 25, 2006 at 11:23:02AM -0400, Dmitry Torokhov wrote:
> On 4/25/06, Vojtech Pavlik <vojtech@suse.cz> wrote:
> > On Tue, Apr 25, 2006 at 09:19:42AM -0400, Dmitry Torokhov wrote:
> > > On 4/24/06, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > > On 4/24/06, Vojtech Pavlik <vojtech@suse.cz> wrote:
> > > > > On Mon, Apr 24, 2006 at 10:31:39AM -0400, Dmitry Torokhov wrote:
> > > > > >
> > > > > > Vojtech, could you remind me why EVIOC{G|S}REP were removed? Some
> > > > > > people want to have ability to separate keyboards (via grabbing); they
> > > > > > also might want to control repeat rate independently. Shoudl we
> > > > > > reinstate these ioctls?
> > > > >
> > > > > I believe they were replaced by the ability to send EV_REP style events
> > > > > to the device, setting the repeat rate.
> > > > >
> > > >
> > > > Argh, why am I always forgetting about ability to write events into devices?
> > >
> > > Thinking about it some more - writing to the event device is an
> > > elegant way to set repeat rate but how do you retrieve current repeat
> > > rate for a given device?
> >
> > You can't. And that's likely a problem that needs fixing.
> >
> 
> So do you agree that we need to ressurect EVIOCGREP (and EVIOCSREP
> just to complement the interface)?
 
Yes. And EVIOCSREP should just generate the events needed to notify the
drivers to do the change.

-- 
Vojtech Pavlik
Director SuSE Labs

Re: [PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[Dmitry Torokhov]

On Tuesday 25 April 2006 11:26, Vojtech Pavlik wrote:
> On Tue, Apr 25, 2006 at 11:23:02AM -0400, Dmitry Torokhov wrote:
> > On 4/25/06, Vojtech Pavlik <vojtech@suse.cz> wrote:
> > > On Tue, Apr 25, 2006 at 09:19:42AM -0400, Dmitry Torokhov wrote:
> > > > On 4/24/06, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > > > > On 4/24/06, Vojtech Pavlik <vojtech@suse.cz> wrote:
> > > > > > On Mon, Apr 24, 2006 at 10:31:39AM -0400, Dmitry Torokhov wrote:
> > > > > > >
> > > > > > > Vojtech, could you remind me why EVIOC{G|S}REP were removed? Some
> > > > > > > people want to have ability to separate keyboards (via grabbing); they
> > > > > > > also might want to control repeat rate independently. Shoudl we
> > > > > > > reinstate these ioctls?
> > > > > >
> > > > > > I believe they were replaced by the ability to send EV_REP style events
> > > > > > to the device, setting the repeat rate.
> > > > > >
> > > > >
> > > > > Argh, why am I always forgetting about ability to write events into devices?
> > > >
> > > > Thinking about it some more - writing to the event device is an
> > > > elegant way to set repeat rate but how do you retrieve current repeat
> > > > rate for a given device?
> > >
> > > You can't. And that's likely a problem that needs fixing.
> > >
> > 
> > So do you agree that we need to ressurect EVIOCGREP (and EVIOCSREP
> > just to complement the interface)?
>  
> Yes. And EVIOCSREP should just generate the events needed to notify the
> drivers to do the change.
> 

What do you gius think about the patch below?

-- 
Dmitry

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/input/evdev.c |   21 +++++++++++++++++++++
 include/linux/input.h |    2 ++
 2 files changed, 23 insertions(+)

Index: work/drivers/input/evdev.c
===================================================================
--- work.orig/drivers/input/evdev.c
+++ work/drivers/input/evdev.c
@@ -403,6 +403,27 @@ static long evdev_ioctl_handler(struct f
 		case EVIOCGID:
 			if (copy_to_user(p, &dev->id, sizeof(struct input_id)))
 				return -EFAULT;
+			return 0;
+
+		case EVIOCGREP:
+			if (!test_bit(EV_REP, dev->evbit))
+				return -ENOSYS;
+			if (put_user(dev->rep[REP_DELAY], ip))
+				return -EFAULT;
+			if (put_user(dev->rep[REP_PERIOD], ip + 1))
+				return -EFAULT;
+			return 0;
+
+		case EVIOCSREP:
+			if (!test_bit(EV_REP, dev->evbit))
+				return -ENOSYS;
+			if (get_user(u, ip))
+				return -EFAULT;
+			if (get_user(v, ip + 1))
+				return -EFAULT;
+
+			input_event(dev, EV_REP, REP_DELAY, u);
+			input_event(dev, EV_REP, REP_PERIOD, v);
 
 			return 0;
 
Index: work/include/linux/input.h
===================================================================
--- work.orig/include/linux/input.h
+++ work/include/linux/input.h
@@ -56,6 +56,8 @@ struct input_absinfo {
 
 #define EVIOCGVERSION		_IOR('E', 0x01, int)			/* get driver version */
 #define EVIOCGID		_IOR('E', 0x02, struct input_id)	/* get device ID */
+#define EVIOCGREP		_IOR('E', 0x03, int[2])			/* get repeat settings */
+#define EVIOCSREP		_IOW('E', 0x03, int[2])			/* get repeat settings */
 #define EVIOCGKEYCODE		_IOR('E', 0x04, int[2])			/* get keycode */
 #define EVIOCSKEYCODE		_IOW('E', 0x04, int[2])			/* set keycode */
 

Re: [PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[Vojtech Pavlik]

On Wed, Apr 26, 2006 at 01:06:38AM -0400, Dmitry Torokhov wrote:

> > Yes. And EVIOCSREP should just generate the events needed to notify the
> > drivers to do the change.
> > 
> 
> What do you gius think about the patch below?

It looks fine to me.

> -- 
> Dmitry
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> ---
> 
>  drivers/input/evdev.c |   21 +++++++++++++++++++++
>  include/linux/input.h |    2 ++
>  2 files changed, 23 insertions(+)
> 
> Index: work/drivers/input/evdev.c
> ===================================================================
> --- work.orig/drivers/input/evdev.c
> +++ work/drivers/input/evdev.c
> @@ -403,6 +403,27 @@ static long evdev_ioctl_handler(struct f
>  		case EVIOCGID:
>  			if (copy_to_user(p, &dev->id, sizeof(struct input_id)))
>  				return -EFAULT;
> +			return 0;
> +
> +		case EVIOCGREP:
> +			if (!test_bit(EV_REP, dev->evbit))
> +				return -ENOSYS;
> +			if (put_user(dev->rep[REP_DELAY], ip))
> +				return -EFAULT;
> +			if (put_user(dev->rep[REP_PERIOD], ip + 1))
> +				return -EFAULT;
> +			return 0;
> +
> +		case EVIOCSREP:
> +			if (!test_bit(EV_REP, dev->evbit))
> +				return -ENOSYS;
> +			if (get_user(u, ip))
> +				return -EFAULT;
> +			if (get_user(v, ip + 1))
> +				return -EFAULT;
> +
> +			input_event(dev, EV_REP, REP_DELAY, u);
> +			input_event(dev, EV_REP, REP_PERIOD, v);
>  
>  			return 0;
>  
> Index: work/include/linux/input.h
> ===================================================================
> --- work.orig/include/linux/input.h
> +++ work/include/linux/input.h
> @@ -56,6 +56,8 @@ struct input_absinfo {
>  
>  #define EVIOCGVERSION		_IOR('E', 0x01, int)			/* get driver version */
>  #define EVIOCGID		_IOR('E', 0x02, struct input_id)	/* get device ID */
> +#define EVIOCGREP		_IOR('E', 0x03, int[2])			/* get repeat settings */
> +#define EVIOCSREP		_IOW('E', 0x03, int[2])			/* get repeat settings */
>  #define EVIOCGKEYCODE		_IOR('E', 0x04, int[2])			/* get keycode */
>  #define EVIOCSKEYCODE		_IOW('E', 0x04, int[2])			/* set keycode */
>  
> 
> 

-- 
Vojtech Pavlik
Director SuSE Labs

Re: [PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[bjdouma]

On Wed, Apr 26, 2006 at 01:06:38AM -0400, Dmitry Torokhov wrote:

> What do you gius think about the patch below?

Works like a charm.
Why is it though that we need EVIOCSREP, when I can set PERIOD and
DELAY through writing a struct input_event directly to the file
descriptor?  I've been doing that for quite some time, having
softrepeat=1 (I need a quick keyboard, DELAY=120, PERIOD=18).

One typo in the patch:
+#define EVIOCSREP		_IOW('E', 0x03, int[2])			/* get repeat settings */
should be:
+#define EVIOCSREP		_IOW('E', 0x03, int[2])			/* set repeat settings */

Now, the EV_SND bitmap is still broken.
I don't think it's simply a matter of adding change_bit(code,dev->snd)
in the EV_SND part of input.c.  During a quick test the bitmap
became confused, after setting both bell and tone through writing
a struct input_event to the file descriptor of the pcspkr's event
file in /dev/input/, then setting just bell to 0.

bjd

Re: [PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[Dmitry Torokhov]

[-- Attachment #1: Type: text/plain, Size: 1396 bytes --]

On 4/26/06, bjdouma <bjdouma@xs4all.nl> wrote:
> On Wed, Apr 26, 2006 at 01:06:38AM -0400, Dmitry Torokhov wrote:
>
> > What do you gius think about the patch below?
>
> Works like a charm.
> Why is it though that we need EVIOCSREP, when I can set PERIOD and
> DELAY through writing a struct input_event directly to the file
> descriptor?  I've been doing that for quite some time, having
> softrepeat=1 (I need a quick keyboard, DELAY=120, PERIOD=18).
>
> One typo in the patch:
> +#define EVIOCSREP              _IOW('E', 0x03, int[2])                 /* get repeat settings */
> should be:
> +#define EVIOCSREP              _IOW('E', 0x03, int[2])                 /* set repeat settings */
>

Ah, OK, thanks, I will fix that.

> Now, the EV_SND bitmap is still broken.
> I don't think it's simply a matter of adding change_bit(code,dev->snd)
> in the EV_SND part of input.c.  During a quick test the bitmap
> became confused, after setting both bell and tone through writing
> a struct input_event to the file descriptor of the pcspkr's event
> file in /dev/input/, then setting just bell to 0.
>

Are you saying that both bits were set to 0 or that you could not hear
the tone after killing bell? If latter then it is sort of pcspkr
problem as from input core POV tone is still active.

Btw, your patch - did it resemble something like attached?

--
Dmitry

[-- Attachment #2: input-fix-EVIOCGSND.patch --]
[-- Type: text/plain, Size: 619 bytes --]

Input: make EVIOCGSND return meaningful data

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---
 drivers/input/input.c |    3 +++
 1 file changed, 3 insertions(+)

Index: linux/drivers/input/input.c
===================================================================
--- linux.orig/drivers/input/input.c
+++ linux/drivers/input/input.c
@@ -155,6 +155,9 @@ void input_event(struct input_dev *dev, 
 			if (code > SND_MAX || !test_bit(code, dev->sndbit))
 				return;
 
+			if (!!test_bit(code, dev->snd) != !!value)
+				change_bit(code, dev->snd);
+
 			if (dev->event) dev->event(dev, type, code, value);
 
 			break;

Re: [PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[bjdouma]

On Wed, Apr 26, 2006 at 10:24:32AM -0400, Dmitry Torokhov wrote:

> Are you saying that both bits were set to 0 or that you could not hear
> the tone after killing bell? If latter then it is sort of pcspkr
> problem as from input core POV tone is still active.
> 
> Btw, your patch - did it resemble something like attached?

> Index: linux/drivers/input/input.c
> ===================================================================
> --- linux.orig/drivers/input/input.c
> +++ linux/drivers/input/input.c
> @@ -155,6 +155,9 @@ void input_event(struct input_dev *dev, 
>  			if (code > SND_MAX || !test_bit(code, dev->sndbit))
>  				return;
>  
> +			if (!!test_bit(code, dev->snd) != !!value)
> +				change_bit(code, dev->snd);
> +
>  			if (dev->event) dev->event(dev, type, code, value);
>  
>  			break;

Before I made the change of dev->snd to an int array, yes it did
exactly this same thing.

Basically what I'm saying is that when you query the input driver
for the state of EV_SND, it doesn't tell you much about what tone
is actually audible, if at all.

Let me give two examples.  I am using here my program inputcntrl
that I am working on -- basically a wrapper with a parser and
interpreter around some of the uinput driver's functions (if you
want a copy of the WIP tree, let me know).

Just regard both examples as a complete session, i.e. no other
commands influencing the pcspkr are interspersed in what you see
below (/dev/input/pcspkr happens to be a symlink to /dev/input/event1).

These examples are with your latest small patch in place, the one
doing the change_bit(code, dev->snd).



Example i.

$> inputcntrl -d /dev/input/pcspkr set-snd 'bell=0;tone=0'    # part 1; reset
$> inputcntrl -d /dev/input/pcspkr set-snd bell=1
$> inputcntrl -d /dev/input/pcspkr get-snd bell,tone
SND_BELL               1
SND_TONE               0
	tone 1000Hz, see pcspkr.c

$> inputcntrl -d /dev/input/pcspkr set-snd tone=1200
$> inputcntrl -d /dev/input/pcspkr get-snd bell,tone
SND_BELL               1
SND_TONE               1
	tone 1200Hz
	note that bell remains 1
	ok, let's conclude that if bell=1 then sound=1000Hz unless tone=1

$> inputcntrl -d /dev/input/pcspkr set-snd 'bell=0;tone0'    # part 2; reset
$> inputcntrl -d /dev/input/pcspkr set-snd tone=1200
$> inputcntrl -d /dev/input/pcspkr get-snd bell,tone
SND_BELL               0
SND_TONE               1
	tone 1200Hz

$> inputcntrl -d /dev/input/pcspkr set-snd bell=1
$> inputcntrl -d /dev/input/pcspkr get-snd bell,tone
SND_BELL               1
SND_TONE               1
	tone 1000Hz
	oddness: state of EV_SND (1,1) is same as in part 1,
	only now is sound a tone of 1000Hz.
	conclusion in part 1 apparently wrong.
	so when state is (1,1), actual sound can be anything



Example ii.

$> inputcntrl -d /dev/input/pcspkr set-snd 'bell=0;tone=0'    # part 3; reset
$> inputcntrl -d /dev/input/pcspkr set-snd tone=1200
inputcntrl -d /dev/input/pcspkr get-snd bell,tone
SND_BELL               0
SND_TONE               1
	tone 1200Hz

$> inputcntrl -d /dev/input/pcspkr set-snd bell=1
$> inputcntrl -d /dev/input/pcspkr get-snd bell,tone
SND_BELL               1
SND_TONE               1
	tone 1000Hz

$> inputcntrl -d /dev/input/pcspkr set-snd bell=0
$> inputcntrl -d /dev/input/pcspkr get-snd bell,tone
SND_BELL               0
SND_TONE               1
	silence!, but SND_TONE = 1!
	on top of that, state is same as in part 2, only then
	we heard a tone of 1200Hz, now we have silence


bjd

Re: [PATCH 001/001] INPUT: new ioctl's to retrieve values of EV_REP and EV_SND event codes

[Dmitry Torokhov]

On 4/26/06, bjdouma <bjdouma@xs4all.nl> wrote:
>
> Basically what I'm saying is that when you query the input driver
> for the state of EV_SND, it doesn't tell you much about what tone
> is actually audible, if at all.
>

I agree but I still questinon usefullness of that data.

> Let me give two examples.  I am using here my program inputcntrl
> that I am working on -- basically a wrapper with a parser and
> interpreter around some of the uinput driver's functions (if you
> want a copy of the WIP tree, let me know).
>
> Just regard both examples as a complete session, i.e. no other
> commands influencing the pcspkr are interspersed in what you see
> below (/dev/input/pcspkr happens to be a symlink to /dev/input/event1).
>
> These examples are with your latest small patch in place, the one
> doing the change_bit(code, dev->snd).
>

<..skipped..>

OK, so the first example illustrates that input core does not provide
"tone" data - no surprise here. The scond example proves that pcspkr
driver does not handle concurrent access very well. Still the input
core behaved exactly as it was supposed to, everythng is fine as far
as I can see.

--
Dmitry