Re: usb hid: reset NumLock
On Thu, 5 Apr 2007 16:54:17 -0400, "Dmitry Torokhov" <[EMAIL PROTECTED]> wrote: > > On a certain keyboard, when BIOS sets NumLock LED on, it survives the > > takeover > > by Linux and thus confuses users. > > > > Eating of an increasibly scarce quirk bit is unfortunate. We do it for > > safety, > > given the history of nervous input devices which crash if anything unusual > > happens. > You know, I would not call turning leds on an off an unusual operation > for a keyboard. Right, but monkeying with them immediately upon initialization may be "unusual", meaning "device was tested with one version of Windows and shipped, everything else is unusual". -- Pete - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On 4/5/07, Pete Zaitcev <[EMAIL PROTECTED]> wrote: On a certain keyboard, when BIOS sets NumLock LED on, it survives the takeover by Linux and thus confuses users. Eating of an increasibly scarce quirk bit is unfortunate. We do it for safety, given the history of nervous input devices which crash if anything unusual happens. You know, I would not call turning leds on an off an unusual operation for a keyboard. -- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On a certain keyboard, when BIOS sets NumLock LED on, it survives the takeover
by Linux and thus confuses users.
Eating of an increasibly scarce quirk bit is unfortunate. We do it for safety,
given the history of nervous input devices which crash if anything unusual
happens.
Signed-off-by: Pete Zaitcev <[EMAIL PROTECTED]>
---
diff --git a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c
index 827a75a..23b1e70 100644
--- a/drivers/usb/input/hid-core.c
+++ b/drivers/usb/input/hid-core.c
@@ -545,6 +545,45 @@ void usbhid_init_reports(struct hid_device *hid)
warn("timeout initializing reports");
}
+/*
+ * Reset LEDs which BIOS might have left on. For now, just NumLock (0x01).
+ */
+
+static int hid_find_field_early(struct hid_device *hid, unsigned int page,
+unsigned int hid_code, struct hid_field **pfield)
+{
+ struct hid_report *report;
+ struct hid_field *field;
+ struct hid_usage *usage;
+ int i, j;
+
+ list_for_each_entry(report,
>report_enum[HID_OUTPUT_REPORT].report_list, list) {
+ for (i = 0; i < report->maxfield; i++) {
+ field = report->field[i];
+ for (j = 0; j < field->maxusage; j++) {
+ usage = >usage[j];
+ if ((usage->hid & HID_USAGE_PAGE) == page &&
+ (usage->hid & 0x) == hid_code) {
+ *pfield = field;
+ return j;
+ }
+ }
+ }
+ }
+ return -1;
+}
+
+static void usbhid_set_leds(struct hid_device *hid)
+{
+ struct hid_field *field;
+ int offset;
+
+ if ((offset = hid_find_field_early(hid, HID_UP_LED, 0x01, )) !=
-1) {
+ hid_set_field(field, offset, 0);
+ usbhid_submit_report(hid, field->report, USB_DIR_OUT);
+ }
+}
+
#define USB_VENDOR_ID_GTCO 0x078c
#define USB_DEVICE_ID_GTCO_90 0x0090
#define USB_DEVICE_ID_GTCO_100 0x0100
@@ -765,6 +804,9 @@ void usbhid_init_reports(struct hid_device *hid)
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
+#define USB_VENDOR_ID_DELL 0x413c
+#define USB_DEVICE_ID_DELL_W7658 0x2005
+
/*
* Alphabetically sorted blacklist by quirk type.
*/
@@ -947,6 +989,8 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_CIDC, 0x0103, HID_QUIRK_IGNORE },
+ { USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_W7658, HID_QUIRK_RESET_LEDS },
+
{ 0, 0 }
};
@@ -1334,6 +1378,8 @@ static int hid_probe(struct usb_interface *intf, const
struct usb_device_id *id)
usbhid_init_reports(hid);
hid_dump_device(hid);
+ if (hid->quirks & HID_QUIRK_RESET_LEDS)
+ usbhid_set_leds(hid);
if (!hidinput_connect(hid))
hid->claimed |= HID_CLAIMED_INPUT;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 8c97d4d..3e8dcb0 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -269,6 +269,7 @@ struct hid_item {
#define HID_QUIRK_SONY_PS3_CONTROLLER 0x0008
#define HID_QUIRK_LOGITECH_S510_DESCRIPTOR 0x0010
#define HID_QUIRK_DUPLICATE_USAGES 0x0020
+#define HID_QUIRK_RESET_LEDS 0x0040
/*
* This is the global environment of the parser. This information is
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Wed, 4 Apr 2007, Dmitry Torokhov wrote: > Unfortunately my patch is crap. We should not be sending events down > dev->event() until dev->open() has been called because many drivers > start hardware from there and not ready until then. So it is HID driver > responsibility to properly reset leds after all. Pete, could you please resend your patch, with proper metadata, we need to merge your one then. Thanks, -- Jiri Kosina - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On a certain keyboard, when BIOS sets NumLock LED on, it survives the takeover
by Linux and thus confuses users.
Eating of an increasibly scarce quirk bit is unfortunate. We do it for safety,
given the history of nervous input devices which crash if anything unusual
happens.
Signed-off-by: Pete Zaitcev [EMAIL PROTECTED]
---
diff --git a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c
index 827a75a..23b1e70 100644
--- a/drivers/usb/input/hid-core.c
+++ b/drivers/usb/input/hid-core.c
@@ -545,6 +545,45 @@ void usbhid_init_reports(struct hid_device *hid)
warn(timeout initializing reports);
}
+/*
+ * Reset LEDs which BIOS might have left on. For now, just NumLock (0x01).
+ */
+
+static int hid_find_field_early(struct hid_device *hid, unsigned int page,
+unsigned int hid_code, struct hid_field **pfield)
+{
+ struct hid_report *report;
+ struct hid_field *field;
+ struct hid_usage *usage;
+ int i, j;
+
+ list_for_each_entry(report,
hid-report_enum[HID_OUTPUT_REPORT].report_list, list) {
+ for (i = 0; i report-maxfield; i++) {
+ field = report-field[i];
+ for (j = 0; j field-maxusage; j++) {
+ usage = field-usage[j];
+ if ((usage-hid HID_USAGE_PAGE) == page
+ (usage-hid 0x) == hid_code) {
+ *pfield = field;
+ return j;
+ }
+ }
+ }
+ }
+ return -1;
+}
+
+static void usbhid_set_leds(struct hid_device *hid)
+{
+ struct hid_field *field;
+ int offset;
+
+ if ((offset = hid_find_field_early(hid, HID_UP_LED, 0x01, field)) !=
-1) {
+ hid_set_field(field, offset, 0);
+ usbhid_submit_report(hid, field-report, USB_DIR_OUT);
+ }
+}
+
#define USB_VENDOR_ID_GTCO 0x078c
#define USB_DEVICE_ID_GTCO_90 0x0090
#define USB_DEVICE_ID_GTCO_100 0x0100
@@ -765,6 +804,9 @@ void usbhid_init_reports(struct hid_device *hid)
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
+#define USB_VENDOR_ID_DELL 0x413c
+#define USB_DEVICE_ID_DELL_W7658 0x2005
+
/*
* Alphabetically sorted blacklist by quirk type.
*/
@@ -947,6 +989,8 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_CIDC, 0x0103, HID_QUIRK_IGNORE },
+ { USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_W7658, HID_QUIRK_RESET_LEDS },
+
{ 0, 0 }
};
@@ -1334,6 +1378,8 @@ static int hid_probe(struct usb_interface *intf, const
struct usb_device_id *id)
usbhid_init_reports(hid);
hid_dump_device(hid);
+ if (hid-quirks HID_QUIRK_RESET_LEDS)
+ usbhid_set_leds(hid);
if (!hidinput_connect(hid))
hid-claimed |= HID_CLAIMED_INPUT;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 8c97d4d..3e8dcb0 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -269,6 +269,7 @@ struct hid_item {
#define HID_QUIRK_SONY_PS3_CONTROLLER 0x0008
#define HID_QUIRK_LOGITECH_S510_DESCRIPTOR 0x0010
#define HID_QUIRK_DUPLICATE_USAGES 0x0020
+#define HID_QUIRK_RESET_LEDS 0x0040
/*
* This is the global environment of the parser. This information is
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On 4/5/07, Pete Zaitcev [EMAIL PROTECTED] wrote: On a certain keyboard, when BIOS sets NumLock LED on, it survives the takeover by Linux and thus confuses users. Eating of an increasibly scarce quirk bit is unfortunate. We do it for safety, given the history of nervous input devices which crash if anything unusual happens. You know, I would not call turning leds on an off an unusual operation for a keyboard. -- Dmitry - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Thu, 5 Apr 2007 16:54:17 -0400, Dmitry Torokhov [EMAIL PROTECTED] wrote: On a certain keyboard, when BIOS sets NumLock LED on, it survives the takeover by Linux and thus confuses users. Eating of an increasibly scarce quirk bit is unfortunate. We do it for safety, given the history of nervous input devices which crash if anything unusual happens. You know, I would not call turning leds on an off an unusual operation for a keyboard. Right, but monkeying with them immediately upon initialization may be unusual, meaning device was tested with one version of Windows and shipped, everything else is unusual. -- Pete - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Wed, 4 Apr 2007, Dmitry Torokhov wrote: Unfortunately my patch is crap. We should not be sending events down dev-event() until dev-open() has been called because many drivers start hardware from there and not ready until then. So it is HID driver responsibility to properly reset leds after all. Pete, could you please resend your patch, with proper metadata, we need to merge your one then. Thanks, -- Jiri Kosina - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Tuesday 03 April 2007 04:52, Jiri Kosina wrote: > On Mon, 2 Apr 2007, Pete Zaitcev wrote: > > > How about this? > > Looks quite fine to me. > > But in case that Dmitry's patch "Input: add generic suspend and resume for > uinput devices" fixes your issue too, I wouldn't merge it as it won't be > needed. Could you please let me know? Unfortunately my patch is crap. We should not be sending events down dev->event() until dev->open() has been called because many drivers start hardware from there and not ready until then. So it is HID driver responsibility to properly reset leds after all. -- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Tuesday 03 April 2007 04:52, Jiri Kosina wrote: On Mon, 2 Apr 2007, Pete Zaitcev wrote: How about this? Looks quite fine to me. But in case that Dmitry's patch Input: add generic suspend and resume for uinput devices fixes your issue too, I wouldn't merge it as it won't be needed. Could you please let me know? Unfortunately my patch is crap. We should not be sending events down dev-event() until dev-open() has been called because many drivers start hardware from there and not ready until then. So it is HID driver responsibility to properly reset leds after all. -- Dmitry - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Tue, 3 Apr 2007, Robert Marquardt wrote: > > As this quirk is only needed once in the initialization method, we > > could probably get rid of it and just put and explicit test for PID > > and VID there (with apropriate comment). We can't afford wasting quirk > > entries, as we are slowly running out of them. > Is it a problem of table size or number of table entries? For the number > of entries a simple change to PID ranges should help. It would also > allow to move Wacom to the blacklist. I have sent a patch for that > already, but it was rejected by Greg. The issue is that 32 bits of the quirk bitmask are going to be taken by the quirk entries (so no, it's not related to the size of the table). Thanks, -- Jiri Kosina - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb hid: reset NumLock
Jiri Kosina wrote: As this quirk is only needed once in the initialization method, we could probably get rid of it and just put and explicit test for PID and VID there (with apropriate comment). We can't afford wasting quirk entries, as we are slowly running out of them. Is it a problem of table size or number of table entries? For the number of entries a simple change to PID ranges should help. It would also allow to move Wacom to the blacklist. I have sent a patch for that already, but it was rejected by Greg. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Mon, 2 Apr 2007, Pete Zaitcev wrote:
> How about this?
Looks quite fine to me.
But in case that Dmitry's patch "Input: add generic suspend and resume for
uinput devices" fixes your issue too, I wouldn't merge it as it won't be
needed. Could you please let me know?
> @@ -947,6 +989,8 @@ static const struct hid_blacklist {
>
> { USB_VENDOR_ID_CIDC, 0x0103, HID_QUIRK_IGNORE },
>
> + { USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_W7658, HID_QUIRK_RESET_LEDS },
> +
As this quirk is only needed once in the initialization method, we could
probably get rid of it and just put and explicit test for PID and VID
there (with apropriate comment). We can't afford wasting quirk entries, as
we are slowly running out of them.
Depends on whether this is going to be needed for > 1 PID.
> I wasn't sure where to place the function, so I just put it above its
> user, to signify that it's special-casing. Also, it's unclear where to
> put the quirk entry and defines. There's a comment saying that they are
> sorted alphabetically by quirk, but apparently the order was violated
> with more recent additions.
Yep, the blacklist is ugly. I have a patch moving it to the proper place
and rearranging it to be sorted again, queued for 2.6.22.
Thanks,
--
Jiri Kosina
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Tue, 3 Apr 2007 01:04:05 -0400, Dmitry Torokhov <[EMAIL PROTECTED]> wrote: > What do you think? The patch looks sane, although I haven't yet tested it. I'll live with it until next quarterly update, then consider if I should take it or use my patch for RHEL 5 and RHEL 4. -- Pete - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Tue, 3 Apr 2007 01:04:05 -0400, Dmitry Torokhov [EMAIL PROTECTED] wrote: What do you think? The patch looks sane, although I haven't yet tested it. I'll live with it until next quarterly update, then consider if I should take it or use my patch for RHEL 5 and RHEL 4. -- Pete - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Mon, 2 Apr 2007, Pete Zaitcev wrote:
How about this?
Looks quite fine to me.
But in case that Dmitry's patch Input: add generic suspend and resume for
uinput devices fixes your issue too, I wouldn't merge it as it won't be
needed. Could you please let me know?
@@ -947,6 +989,8 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_CIDC, 0x0103, HID_QUIRK_IGNORE },
+ { USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_W7658, HID_QUIRK_RESET_LEDS },
+
As this quirk is only needed once in the initialization method, we could
probably get rid of it and just put and explicit test for PID and VID
there (with apropriate comment). We can't afford wasting quirk entries, as
we are slowly running out of them.
Depends on whether this is going to be needed for 1 PID.
I wasn't sure where to place the function, so I just put it above its
user, to signify that it's special-casing. Also, it's unclear where to
put the quirk entry and defines. There's a comment saying that they are
sorted alphabetically by quirk, but apparently the order was violated
with more recent additions.
Yep, the blacklist is ugly. I have a patch moving it to the proper place
and rearranging it to be sorted again, queued for 2.6.22.
Thanks,
--
Jiri Kosina
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [linux-usb-devel] usb hid: reset NumLock
Jiri Kosina wrote: As this quirk is only needed once in the initialization method, we could probably get rid of it and just put and explicit test for PID and VID there (with apropriate comment). We can't afford wasting quirk entries, as we are slowly running out of them. Is it a problem of table size or number of table entries? For the number of entries a simple change to PID ranges should help. It would also allow to move Wacom to the blacklist. I have sent a patch for that already, but it was rejected by Greg. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Tue, 3 Apr 2007, Robert Marquardt wrote: As this quirk is only needed once in the initialization method, we could probably get rid of it and just put and explicit test for PID and VID there (with apropriate comment). We can't afford wasting quirk entries, as we are slowly running out of them. Is it a problem of table size or number of table entries? For the number of entries a simple change to PID ranges should help. It would also allow to move Wacom to the blacklist. I have sent a patch for that already, but it was rejected by Greg. The issue is that 32 bits of the quirk bitmask are going to be taken by the quirk entries (so no, it's not related to the size of the table). Thanks, -- Jiri Kosina - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Monday 02 April 2007 19:12, Pete Zaitcev wrote:
> On Mon, 2 Apr 2007 16:48:24 +0200 (CEST), Jiri Kosina <[EMAIL PROTECTED]>
> wrote:
> > On Sun, 1 Apr 2007, Pete Zaitcev wrote:
>
> > could you please change the order of the two functions, so that you
> > don't have to put the forward declaration here?
> >[...]
> > I'd say this is a little bit overcommented.
> >[...]
> > So as soon as you have the VIDs and PIDs of the hardware which
> > requires this, could you please update the patch and send it to me again?
>
> How about this?
Actually I think I will be adding the patch below, but it has to wait
till 2.6.22 as it requires input core to struct device conversion
patch.
What do you think?
--
Dmitry
Input: add generic suspend and resume for uinput devices
Automatically turn off leds and sound effects as part of suspend
process and restore led state, sounds and repeat rate at resume.
Also synchronize hardware state with logical state at device
registration.
Signed-off-by: Dmitry Torokhov <[EMAIL PROTECTED]>
---
drivers/input/input.c | 80 ++
1 files changed, 80 insertions(+)
Index: work/drivers/input/input.c
===
--- work.orig/drivers/input/input.c
+++ work/drivers/input/input.c
@@ -997,10 +997,88 @@ static int input_dev_uevent(struct devic
return 0;
}
+static void input_dev_toggle(struct input_dev *dev,
+unsigned int type, unsigned int code,
+unsigned long *cap_bits, unsigned long *bits,
+int force_off)
+{
+ if (test_bit(code, cap_bits)) {
+ if (!force_off)
+ dev->event(dev, type, code, test_bit(code, bits));
+ else if (test_bit(code, bits))
+ dev->event(dev, type, code, 0);
+ }
+}
+
+static void input_dev_reset(struct input_dev *dev, int force_off)
+{
+ int i;
+
+ if (!dev->event)
+ return;
+
+ /* synchronize led state */
+ if (test_bit(EV_LED, dev->evbit))
+ for (i = 0; i <= LED_MAX; i++)
+ input_dev_toggle(dev, EV_LED, i,
+dev->ledbit, dev->led, force_off);
+
+ /* restore sound */
+ if (test_bit(EV_SND, dev->evbit))
+ for (i = 0; i <= SND_MAX; i++)
+ input_dev_toggle(dev, EV_SND, i,
+dev->sndbit, dev->snd, force_off);
+
+ if (!force_off && test_bit(EV_REP, dev->evbit)) {
+ dev->event(dev, EV_REP, REP_PERIOD, dev->rep[REP_PERIOD]);
+ dev->event(dev, EV_REP, REP_DELAY, dev->rep[REP_DELAY]);
+ }
+}
+
+#ifdef CONFIG_PM
+static int input_dev_suspend(struct device *dev, pm_message_t state)
+{
+ struct input_dev *input_dev = to_input_dev(dev);
+
+ mutex_lock(_dev->mutex);
+
+ if (dev->power.power_state.event != state.event) {
+ if (state.event == PM_EVENT_SUSPEND)
+ input_dev_reset(input_dev, 1);
+
+ dev->power.power_state = state;
+ }
+
+ mutex_unlock(_dev->mutex);
+
+ return 0;
+}
+
+static int input_dev_resume(struct device *dev)
+{
+ struct input_dev *input_dev = to_input_dev(dev);
+
+ mutex_lock(_dev->mutex);
+
+ if (dev->power.power_state.event != PM_EVENT_ON)
+ input_dev_reset(to_input_dev(dev), 0);
+
+ dev->power.power_state = PMSG_ON;
+
+ mutex_unlock(_dev->mutex);
+
+ return 0;
+}
+#endif /* CONFIG_PM */
+
static struct device_type input_dev_type = {
.groups = input_dev_attr_groups,
.release= input_dev_release,
.uevent = input_dev_uevent,
+#ifdef CONFIG_PM
+ .suspend= input_dev_suspend,
+ .resume = input_dev_resume,
+#endif
};
struct class input_class = {
@@ -1080,6 +1158,8 @@ int input_register_device(struct input_d
dev->rep[REP_PERIOD] = 33;
}
+ input_dev_reset(dev, 0);
+
if (!dev->getkeycode)
dev->getkeycode = input_default_getkeycode;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Mon, 2 Apr 2007 16:48:24 +0200 (CEST), Jiri Kosina <[EMAIL PROTECTED]> wrote:
> On Sun, 1 Apr 2007, Pete Zaitcev wrote:
> could you please change the order of the two functions, so that you
> don't have to put the forward declaration here?
>[...]
> I'd say this is a little bit overcommented.
>[...]
> So as soon as you have the VIDs and PIDs of the hardware which
> requires this, could you please update the patch and send it to me again?
How about this?
diff --git a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c
index 827a75a..23b1e70 100644
--- a/drivers/usb/input/hid-core.c
+++ b/drivers/usb/input/hid-core.c
@@ -545,6 +545,45 @@ void usbhid_init_reports(struct hid_device *hid)
warn("timeout initializing reports");
}
+/*
+ * Reset LEDs which BIOS might have left on. For now, just NumLock (0x01).
+ */
+
+static int hid_find_field_early(struct hid_device *hid, unsigned int page,
+unsigned int hid_code, struct hid_field **pfield)
+{
+ struct hid_report *report;
+ struct hid_field *field;
+ struct hid_usage *usage;
+ int i, j;
+
+ list_for_each_entry(report,
>report_enum[HID_OUTPUT_REPORT].report_list, list) {
+ for (i = 0; i < report->maxfield; i++) {
+ field = report->field[i];
+ for (j = 0; j < field->maxusage; j++) {
+ usage = >usage[j];
+ if ((usage->hid & HID_USAGE_PAGE) == page &&
+ (usage->hid & 0x) == hid_code) {
+ *pfield = field;
+ return j;
+ }
+ }
+ }
+ }
+ return -1;
+}
+
+static void usbhid_set_leds(struct hid_device *hid)
+{
+ struct hid_field *field;
+ int offset;
+
+ if ((offset = hid_find_field_early(hid, HID_UP_LED, 0x01, )) !=
-1) {
+ hid_set_field(field, offset, 0);
+ usbhid_submit_report(hid, field->report, USB_DIR_OUT);
+ }
+}
+
#define USB_VENDOR_ID_GTCO 0x078c
#define USB_DEVICE_ID_GTCO_90 0x0090
#define USB_DEVICE_ID_GTCO_100 0x0100
@@ -765,6 +804,9 @@ void usbhid_init_reports(struct hid_device *hid)
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
+#define USB_VENDOR_ID_DELL 0x413c
+#define USB_DEVICE_ID_DELL_W7658 0x2005
+
/*
* Alphabetically sorted blacklist by quirk type.
*/
@@ -947,6 +989,8 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_CIDC, 0x0103, HID_QUIRK_IGNORE },
+ { USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_W7658, HID_QUIRK_RESET_LEDS },
+
{ 0, 0 }
};
@@ -1334,6 +1378,8 @@ static int hid_probe(struct usb_interface *intf, const
struct usb_device_id *id)
usbhid_init_reports(hid);
hid_dump_device(hid);
+ if (hid->quirks & HID_QUIRK_RESET_LEDS)
+ usbhid_set_leds(hid);
if (!hidinput_connect(hid))
hid->claimed |= HID_CLAIMED_INPUT;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 8c97d4d..3e8dcb0 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -269,6 +269,7 @@ struct hid_item {
#define HID_QUIRK_SONY_PS3_CONTROLLER 0x0008
#define HID_QUIRK_LOGITECH_S510_DESCRIPTOR 0x0010
#define HID_QUIRK_DUPLICATE_USAGES 0x0020
+#define HID_QUIRK_RESET_LEDS 0x0040
/*
* This is the global environment of the parser. This information is
I wasn't sure where to place the function, so I just put it above its
user, to signify that it's special-casing. Also, it's unclear where
to put the quirk entry and defines. There's a comment saying that they
are sorted alphabetically by quirk, but apparently the order was violated
with more recent additions.
-- Pete
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Sun, 1 Apr 2007, Pete Zaitcev wrote:
> diff --git a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c
> index ef09952..7338e81 100644
> --- a/drivers/usb/input/hid-core.c
> +++ b/drivers/usb/input/hid-core.c
> @@ -548,6 +548,28 @@ void usbhid_init_reports(struct hid_device *hid)
> warn("timeout initializing reports");
> }
>
> +/*
> + * Reset LEDs which BIOS might have left on.
> + */
> +static int hid_find_field_early(struct hid_device *hid, unsigned int page,
> +unsigned int hid_code, struct hid_field **field);
Hi Pete,
could you please change the order of the two functions, so that you
don't have to put the forward declaration here?
> + /*
> + * Just reset Num Lock for now.
> + * This is called for non-keyboard devices too, so no printk if field
> + * is not found.
> + */
I'd say this is a little bit overcommented.
> The main reason is, I have a USB-to-PS/2 adapter where early acces to
> LEDs does not work. Apparently, it is still initializing the PS/2 part
> when it reports to USB that it's ready, and needs a delay. So, I figured
> that I may be breaking some odd devices. Some may crash or whatnot. This
> is why I asked Stuart to get me VID/PID for involved keyboards.
Fair enough, thanks.
Besides the trivial nitpicks above, I think that this fixed version is
fine. So as soon as you have the VIDs and PIDs of the hardware which
requires this, could you please update the patch and send it to me again?
Thanks,
--
Jiri Kosina
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Sun, 1 Apr 2007, Pete Zaitcev wrote: diff --git a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c index ef09952..7338e81 100644 --- a/drivers/usb/input/hid-core.c +++ b/drivers/usb/input/hid-core.c @@ -548,6 +548,28 @@ void usbhid_init_reports(struct hid_device *hid) warn(timeout initializing reports); } +/* + * Reset LEDs which BIOS might have left on. + */ +static int hid_find_field_early(struct hid_device *hid, unsigned int page, +unsigned int hid_code, struct hid_field **field); Hi Pete, could you please change the order of the two functions, so that you don't have to put the forward declaration here? + /* + * Just reset Num Lock for now. + * This is called for non-keyboard devices too, so no printk if field + * is not found. + */ I'd say this is a little bit overcommented. The main reason is, I have a USB-to-PS/2 adapter where early acces to LEDs does not work. Apparently, it is still initializing the PS/2 part when it reports to USB that it's ready, and needs a delay. So, I figured that I may be breaking some odd devices. Some may crash or whatnot. This is why I asked Stuart to get me VID/PID for involved keyboards. Fair enough, thanks. Besides the trivial nitpicks above, I think that this fixed version is fine. So as soon as you have the VIDs and PIDs of the hardware which requires this, could you please update the patch and send it to me again? Thanks, -- Jiri Kosina - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Mon, 2 Apr 2007 16:48:24 +0200 (CEST), Jiri Kosina [EMAIL PROTECTED] wrote:
On Sun, 1 Apr 2007, Pete Zaitcev wrote:
could you please change the order of the two functions, so that you
don't have to put the forward declaration here?
[...]
I'd say this is a little bit overcommented.
[...]
So as soon as you have the VIDs and PIDs of the hardware which
requires this, could you please update the patch and send it to me again?
How about this?
diff --git a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c
index 827a75a..23b1e70 100644
--- a/drivers/usb/input/hid-core.c
+++ b/drivers/usb/input/hid-core.c
@@ -545,6 +545,45 @@ void usbhid_init_reports(struct hid_device *hid)
warn(timeout initializing reports);
}
+/*
+ * Reset LEDs which BIOS might have left on. For now, just NumLock (0x01).
+ */
+
+static int hid_find_field_early(struct hid_device *hid, unsigned int page,
+unsigned int hid_code, struct hid_field **pfield)
+{
+ struct hid_report *report;
+ struct hid_field *field;
+ struct hid_usage *usage;
+ int i, j;
+
+ list_for_each_entry(report,
hid-report_enum[HID_OUTPUT_REPORT].report_list, list) {
+ for (i = 0; i report-maxfield; i++) {
+ field = report-field[i];
+ for (j = 0; j field-maxusage; j++) {
+ usage = field-usage[j];
+ if ((usage-hid HID_USAGE_PAGE) == page
+ (usage-hid 0x) == hid_code) {
+ *pfield = field;
+ return j;
+ }
+ }
+ }
+ }
+ return -1;
+}
+
+static void usbhid_set_leds(struct hid_device *hid)
+{
+ struct hid_field *field;
+ int offset;
+
+ if ((offset = hid_find_field_early(hid, HID_UP_LED, 0x01, field)) !=
-1) {
+ hid_set_field(field, offset, 0);
+ usbhid_submit_report(hid, field-report, USB_DIR_OUT);
+ }
+}
+
#define USB_VENDOR_ID_GTCO 0x078c
#define USB_DEVICE_ID_GTCO_90 0x0090
#define USB_DEVICE_ID_GTCO_100 0x0100
@@ -765,6 +804,9 @@ void usbhid_init_reports(struct hid_device *hid)
#define USB_VENDOR_ID_SONY 0x054c
#define USB_DEVICE_ID_SONY_PS3_CONTROLLER 0x0268
+#define USB_VENDOR_ID_DELL 0x413c
+#define USB_DEVICE_ID_DELL_W7658 0x2005
+
/*
* Alphabetically sorted blacklist by quirk type.
*/
@@ -947,6 +989,8 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_CIDC, 0x0103, HID_QUIRK_IGNORE },
+ { USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_W7658, HID_QUIRK_RESET_LEDS },
+
{ 0, 0 }
};
@@ -1334,6 +1378,8 @@ static int hid_probe(struct usb_interface *intf, const
struct usb_device_id *id)
usbhid_init_reports(hid);
hid_dump_device(hid);
+ if (hid-quirks HID_QUIRK_RESET_LEDS)
+ usbhid_set_leds(hid);
if (!hidinput_connect(hid))
hid-claimed |= HID_CLAIMED_INPUT;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 8c97d4d..3e8dcb0 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -269,6 +269,7 @@ struct hid_item {
#define HID_QUIRK_SONY_PS3_CONTROLLER 0x0008
#define HID_QUIRK_LOGITECH_S510_DESCRIPTOR 0x0010
#define HID_QUIRK_DUPLICATE_USAGES 0x0020
+#define HID_QUIRK_RESET_LEDS 0x0040
/*
* This is the global environment of the parser. This information is
I wasn't sure where to place the function, so I just put it above its
user, to signify that it's special-casing. Also, it's unclear where
to put the quirk entry and defines. There's a comment saying that they
are sorted alphabetically by quirk, but apparently the order was violated
with more recent additions.
-- Pete
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Monday 02 April 2007 19:12, Pete Zaitcev wrote:
On Mon, 2 Apr 2007 16:48:24 +0200 (CEST), Jiri Kosina [EMAIL PROTECTED]
wrote:
On Sun, 1 Apr 2007, Pete Zaitcev wrote:
could you please change the order of the two functions, so that you
don't have to put the forward declaration here?
[...]
I'd say this is a little bit overcommented.
[...]
So as soon as you have the VIDs and PIDs of the hardware which
requires this, could you please update the patch and send it to me again?
How about this?
Actually I think I will be adding the patch below, but it has to wait
till 2.6.22 as it requires input core to struct device conversion
patch.
What do you think?
--
Dmitry
Input: add generic suspend and resume for uinput devices
Automatically turn off leds and sound effects as part of suspend
process and restore led state, sounds and repeat rate at resume.
Also synchronize hardware state with logical state at device
registration.
Signed-off-by: Dmitry Torokhov [EMAIL PROTECTED]
---
drivers/input/input.c | 80 ++
1 files changed, 80 insertions(+)
Index: work/drivers/input/input.c
===
--- work.orig/drivers/input/input.c
+++ work/drivers/input/input.c
@@ -997,10 +997,88 @@ static int input_dev_uevent(struct devic
return 0;
}
+static void input_dev_toggle(struct input_dev *dev,
+unsigned int type, unsigned int code,
+unsigned long *cap_bits, unsigned long *bits,
+int force_off)
+{
+ if (test_bit(code, cap_bits)) {
+ if (!force_off)
+ dev-event(dev, type, code, test_bit(code, bits));
+ else if (test_bit(code, bits))
+ dev-event(dev, type, code, 0);
+ }
+}
+
+static void input_dev_reset(struct input_dev *dev, int force_off)
+{
+ int i;
+
+ if (!dev-event)
+ return;
+
+ /* synchronize led state */
+ if (test_bit(EV_LED, dev-evbit))
+ for (i = 0; i = LED_MAX; i++)
+ input_dev_toggle(dev, EV_LED, i,
+dev-ledbit, dev-led, force_off);
+
+ /* restore sound */
+ if (test_bit(EV_SND, dev-evbit))
+ for (i = 0; i = SND_MAX; i++)
+ input_dev_toggle(dev, EV_SND, i,
+dev-sndbit, dev-snd, force_off);
+
+ if (!force_off test_bit(EV_REP, dev-evbit)) {
+ dev-event(dev, EV_REP, REP_PERIOD, dev-rep[REP_PERIOD]);
+ dev-event(dev, EV_REP, REP_DELAY, dev-rep[REP_DELAY]);
+ }
+}
+
+#ifdef CONFIG_PM
+static int input_dev_suspend(struct device *dev, pm_message_t state)
+{
+ struct input_dev *input_dev = to_input_dev(dev);
+
+ mutex_lock(input_dev-mutex);
+
+ if (dev-power.power_state.event != state.event) {
+ if (state.event == PM_EVENT_SUSPEND)
+ input_dev_reset(input_dev, 1);
+
+ dev-power.power_state = state;
+ }
+
+ mutex_unlock(input_dev-mutex);
+
+ return 0;
+}
+
+static int input_dev_resume(struct device *dev)
+{
+ struct input_dev *input_dev = to_input_dev(dev);
+
+ mutex_lock(input_dev-mutex);
+
+ if (dev-power.power_state.event != PM_EVENT_ON)
+ input_dev_reset(to_input_dev(dev), 0);
+
+ dev-power.power_state = PMSG_ON;
+
+ mutex_unlock(input_dev-mutex);
+
+ return 0;
+}
+#endif /* CONFIG_PM */
+
static struct device_type input_dev_type = {
.groups = input_dev_attr_groups,
.release= input_dev_release,
.uevent = input_dev_uevent,
+#ifdef CONFIG_PM
+ .suspend= input_dev_suspend,
+ .resume = input_dev_resume,
+#endif
};
struct class input_class = {
@@ -1080,6 +1158,8 @@ int input_register_device(struct input_d
dev-rep[REP_PERIOD] = 33;
}
+ input_dev_reset(dev, 0);
+
if (!dev-getkeycode)
dev-getkeycode = input_default_getkeycode;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Sat, 31 Mar 2007 21:35:19 +0200 (CEST), Jiri Kosina <[EMAIL PROTECTED]>
wrote:
> I think I see an issue here. Imagine that you boot a system initially with
> one keyboard connected (usb, ps/2, doesn't matter), and after some time
> you connect second USB keyboard (the NumLock is 'on' on the first keyboard
> when you connect the second one).
OK. I toyed with a return to the Stuart's idea: defeat filtering
somehow, but it wasn't working well, too much duplication. So, I created
a clone of hidinput_find_field() instead. It's still an annoying
duplication, but a lesser one, I think.
diff --git a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c
index ef09952..7338e81 100644
--- a/drivers/usb/input/hid-core.c
+++ b/drivers/usb/input/hid-core.c
@@ -548,6 +548,28 @@ void usbhid_init_reports(struct hid_device *hid)
warn("timeout initializing reports");
}
+/*
+ * Reset LEDs which BIOS might have left on.
+ */
+static int hid_find_field_early(struct hid_device *hid, unsigned int page,
+unsigned int hid_code, struct hid_field **field);
+
+static void usbhid_set_leds(struct hid_device *hid)
+{
+ struct hid_field *field;
+ int offset;
+
+ /*
+* Just reset Num Lock for now.
+* This is called for non-keyboard devices too, so no printk if field
+* is not found.
+*/
+ if ((offset = hid_find_field_early(hid, HID_UP_LED, 0x01, )) !=
-1) {
+ hid_set_field(field, offset, 0);
+ usbhid_submit_report(hid, field->report, USB_DIR_OUT);
+ }
+}
+
#define USB_VENDOR_ID_GTCO 0x078c
#define USB_DEVICE_ID_GTCO_90 0x0090
#define USB_DEVICE_ID_GTCO_100 0x0100
@@ -971,6 +993,30 @@ static void hid_find_max_report(struct hid_device *hid,
unsigned int type, int *
}
}
+static int hid_find_field_early(struct hid_device *hid, unsigned int page,
+unsigned int hid_code, struct hid_field **pfield)
+{
+ struct hid_report *report;
+ struct hid_field *field;
+ struct hid_usage *usage;
+ int i, j;
+
+ list_for_each_entry(report,
>report_enum[HID_OUTPUT_REPORT].report_list, list) {
+ for (i = 0; i < report->maxfield; i++) {
+ field = report->field[i];
+ for (j = 0; j < field->maxusage; j++) {
+ usage = >usage[j];
+ if ((usage->hid & HID_USAGE_PAGE) == page &&
+ (usage->hid & 0x) == hid_code) {
+ *pfield = field;
+ return j;
+ }
+ }
+ }
+ }
+ return -1;
+}
+
static int hid_alloc_buffers(struct usb_device *dev, struct hid_device *hid)
{
struct usbhid_device *usbhid = hid->driver_data;
@@ -1314,6 +1360,8 @@ static int hid_probe(struct usb_interface *intf, const
struct usb_device_id *id)
usbhid_init_reports(hid);
hid_dump_device(hid);
+ /* if (hid->quirks & HID_QUIRK_RESET_LEDS) */
+ usbhid_set_leds(hid);
if (!hidinput_connect(hid))
hid->claimed |= HID_CLAIMED_INPUT;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d26b08f..f592f01 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -267,6 +267,7 @@ struct hid_item {
#define HID_QUIRK_SKIP_OUTPUT_REPORTS 0x0002
#define HID_QUIRK_IGNORE_MOUSE 0x0004
#define HID_QUIRK_SONY_PS3_CONTROLLER 0x0008
+#define HID_QUIRK_RESET_LEDS 0x0010
/*
* This is the global environment of the parser. This information is
> > +++ b/include/linux/hid.h
> > @@ -267,6 +267,7 @@ struct hid_item {
> > #define HID_QUIRK_SKIP_OUTPUT_REPORTS 0x0002
> > #define HID_QUIRK_IGNORE_MOUSE 0x0004
> > #define HID_QUIRK_SONY_PS3_CONTROLLER 0x0008
> > +#define HID_QUIRK_RESET_LEDS 0x0010
>
> I think this is not worth a quirk - when we get it working properly, why
> not do it unconditionally for all keyboards?
The main reason is, I have a USB-to-PS/2 adapter where early acces to
LEDs does not work. Apparently, it is still initializing the PS/2 part
when it reports to USB that it's ready, and needs a delay. So, I figured
that I may be breaking some odd devices. Some may crash or whatnot.
This is why I asked Stuart to get me VID/PID for involved keyboards.
> > URL with details, discussion, rejected patch to read BIOS byte at 0x417:
> > https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=228674
>
> "You are not authorized to access bug #228674. To see this bug, you must
> first log in to an account with the appropriate permissions."
Sorry. I missed that the bug has access flags set...
-- Pete
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a
Re: usb hid: reset NumLock
Hi Pekka, On Sunday 01 April 2007 07:49, Pekka Enberg wrote: > On 3/30/07, Pete Zaitcev <[EMAIL PROTECTED]> wrote: > > Dell people (Stuart and Charles) complained that on some USB keyboards, > > if BIOS enables NumLock, it stays on even after Linux has started. Since > > we always start with NumLock off, this confuses users. Quick double dab > > at NumLock fixes it, but it's not nice. > > What I am seeing on my Thinkpad is that when I boot _without_ an USB > keyboard NumLock is enabled. Switching to virtual console and back to > X fixes it which is why I have never bothered to debug it further. > Perhaps this is related? Should I give your patch a spin to see if it > fixes the problem? > Are you saying that NumLock LED is lit or that keyboard is in NumLock state? Does the same happen if you boot with init=/bin/bash? -- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Sun, 1 Apr 2007 14:49:59 +0300, "Pekka Enberg" <[EMAIL PROTECTED]> wrote: > On 3/30/07, Pete Zaitcev <[EMAIL PROTECTED]> wrote: > > Dell people (Stuart and Charles) complained that on some USB keyboards, > > if BIOS enables NumLock, it stays on even after Linux has started. Since > > we always start with NumLock off, this confuses users. Quick double dab > > at NumLock fixes it, but it's not nice. > > What I am seeing on my Thinkpad is that when I boot _without_ an USB > keyboard NumLock is enabled. Switching to virtual console and back to > X fixes it which is why I have never bothered to debug it further. > Perhaps this is related? Should I give your patch a spin to see if it > fixes the problem? It's related, but not the same problem. Perhaps the initialization in atkbd which Dmitry mentioned is not complete. Jiri found a fatal bug in my patch, but even assuming that it worked, it still would do nothing to you. -- Pete - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On 3/30/07, Pete Zaitcev <[EMAIL PROTECTED]> wrote: Dell people (Stuart and Charles) complained that on some USB keyboards, if BIOS enables NumLock, it stays on even after Linux has started. Since we always start with NumLock off, this confuses users. Quick double dab at NumLock fixes it, but it's not nice. What I am seeing on my Thinkpad is that when I boot _without_ an USB keyboard NumLock is enabled. Switching to virtual console and back to X fixes it which is why I have never bothered to debug it further. Perhaps this is related? Should I give your patch a spin to see if it fixes the problem? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On 3/30/07, Pete Zaitcev [EMAIL PROTECTED] wrote: Dell people (Stuart and Charles) complained that on some USB keyboards, if BIOS enables NumLock, it stays on even after Linux has started. Since we always start with NumLock off, this confuses users. Quick double dab at NumLock fixes it, but it's not nice. What I am seeing on my Thinkpad is that when I boot _without_ an USB keyboard NumLock is enabled. Switching to virtual console and back to X fixes it which is why I have never bothered to debug it further. Perhaps this is related? Should I give your patch a spin to see if it fixes the problem? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Sun, 1 Apr 2007 14:49:59 +0300, Pekka Enberg [EMAIL PROTECTED] wrote: On 3/30/07, Pete Zaitcev [EMAIL PROTECTED] wrote: Dell people (Stuart and Charles) complained that on some USB keyboards, if BIOS enables NumLock, it stays on even after Linux has started. Since we always start with NumLock off, this confuses users. Quick double dab at NumLock fixes it, but it's not nice. What I am seeing on my Thinkpad is that when I boot _without_ an USB keyboard NumLock is enabled. Switching to virtual console and back to X fixes it which is why I have never bothered to debug it further. Perhaps this is related? Should I give your patch a spin to see if it fixes the problem? It's related, but not the same problem. Perhaps the initialization in atkbd which Dmitry mentioned is not complete. Jiri found a fatal bug in my patch, but even assuming that it worked, it still would do nothing to you. -- Pete - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
Hi Pekka, On Sunday 01 April 2007 07:49, Pekka Enberg wrote: On 3/30/07, Pete Zaitcev [EMAIL PROTECTED] wrote: Dell people (Stuart and Charles) complained that on some USB keyboards, if BIOS enables NumLock, it stays on even after Linux has started. Since we always start with NumLock off, this confuses users. Quick double dab at NumLock fixes it, but it's not nice. What I am seeing on my Thinkpad is that when I boot _without_ an USB keyboard NumLock is enabled. Switching to virtual console and back to X fixes it which is why I have never bothered to debug it further. Perhaps this is related? Should I give your patch a spin to see if it fixes the problem? Are you saying that NumLock LED is lit or that keyboard is in NumLock state? Does the same happen if you boot with init=/bin/bash? -- Dmitry - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Sat, 31 Mar 2007 21:35:19 +0200 (CEST), Jiri Kosina [EMAIL PROTECTED]
wrote:
I think I see an issue here. Imagine that you boot a system initially with
one keyboard connected (usb, ps/2, doesn't matter), and after some time
you connect second USB keyboard (the NumLock is 'on' on the first keyboard
when you connect the second one).
OK. I toyed with a return to the Stuart's idea: defeat filtering
somehow, but it wasn't working well, too much duplication. So, I created
a clone of hidinput_find_field() instead. It's still an annoying
duplication, but a lesser one, I think.
diff --git a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c
index ef09952..7338e81 100644
--- a/drivers/usb/input/hid-core.c
+++ b/drivers/usb/input/hid-core.c
@@ -548,6 +548,28 @@ void usbhid_init_reports(struct hid_device *hid)
warn(timeout initializing reports);
}
+/*
+ * Reset LEDs which BIOS might have left on.
+ */
+static int hid_find_field_early(struct hid_device *hid, unsigned int page,
+unsigned int hid_code, struct hid_field **field);
+
+static void usbhid_set_leds(struct hid_device *hid)
+{
+ struct hid_field *field;
+ int offset;
+
+ /*
+* Just reset Num Lock for now.
+* This is called for non-keyboard devices too, so no printk if field
+* is not found.
+*/
+ if ((offset = hid_find_field_early(hid, HID_UP_LED, 0x01, field)) !=
-1) {
+ hid_set_field(field, offset, 0);
+ usbhid_submit_report(hid, field-report, USB_DIR_OUT);
+ }
+}
+
#define USB_VENDOR_ID_GTCO 0x078c
#define USB_DEVICE_ID_GTCO_90 0x0090
#define USB_DEVICE_ID_GTCO_100 0x0100
@@ -971,6 +993,30 @@ static void hid_find_max_report(struct hid_device *hid,
unsigned int type, int *
}
}
+static int hid_find_field_early(struct hid_device *hid, unsigned int page,
+unsigned int hid_code, struct hid_field **pfield)
+{
+ struct hid_report *report;
+ struct hid_field *field;
+ struct hid_usage *usage;
+ int i, j;
+
+ list_for_each_entry(report,
hid-report_enum[HID_OUTPUT_REPORT].report_list, list) {
+ for (i = 0; i report-maxfield; i++) {
+ field = report-field[i];
+ for (j = 0; j field-maxusage; j++) {
+ usage = field-usage[j];
+ if ((usage-hid HID_USAGE_PAGE) == page
+ (usage-hid 0x) == hid_code) {
+ *pfield = field;
+ return j;
+ }
+ }
+ }
+ }
+ return -1;
+}
+
static int hid_alloc_buffers(struct usb_device *dev, struct hid_device *hid)
{
struct usbhid_device *usbhid = hid-driver_data;
@@ -1314,6 +1360,8 @@ static int hid_probe(struct usb_interface *intf, const
struct usb_device_id *id)
usbhid_init_reports(hid);
hid_dump_device(hid);
+ /* if (hid-quirks HID_QUIRK_RESET_LEDS) */
+ usbhid_set_leds(hid);
if (!hidinput_connect(hid))
hid-claimed |= HID_CLAIMED_INPUT;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d26b08f..f592f01 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -267,6 +267,7 @@ struct hid_item {
#define HID_QUIRK_SKIP_OUTPUT_REPORTS 0x0002
#define HID_QUIRK_IGNORE_MOUSE 0x0004
#define HID_QUIRK_SONY_PS3_CONTROLLER 0x0008
+#define HID_QUIRK_RESET_LEDS 0x0010
/*
* This is the global environment of the parser. This information is
+++ b/include/linux/hid.h
@@ -267,6 +267,7 @@ struct hid_item {
#define HID_QUIRK_SKIP_OUTPUT_REPORTS 0x0002
#define HID_QUIRK_IGNORE_MOUSE 0x0004
#define HID_QUIRK_SONY_PS3_CONTROLLER 0x0008
+#define HID_QUIRK_RESET_LEDS 0x0010
I think this is not worth a quirk - when we get it working properly, why
not do it unconditionally for all keyboards?
The main reason is, I have a USB-to-PS/2 adapter where early acces to
LEDs does not work. Apparently, it is still initializing the PS/2 part
when it reports to USB that it's ready, and needs a delay. So, I figured
that I may be breaking some odd devices. Some may crash or whatnot.
This is why I asked Stuart to get me VID/PID for involved keyboards.
URL with details, discussion, rejected patch to read BIOS byte at 0x417:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=228674
You are not authorized to access bug #228674. To see this bug, you must
first log in to an account with the appropriate permissions.
Sorry. I missed that the bug has access flags set...
-- Pete
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More
Re: usb hid: reset NumLock
On Sat, 31 Mar 2007 21:35:19 +0200 (CEST), Jiri Kosina <[EMAIL PROTECTED]> wrote: > On Fri, 30 Mar 2007, Pete Zaitcev wrote: > I think I see an issue here. Imagine that you boot a system initially with > one keyboard connected (usb, ps/2, doesn't matter), and after some time > you connect second USB keyboard (the NumLock is 'on' on the first keyboard > when you connect the second one). > > Without your patch, the NumLock led will be OK on the second keyboard > immediately. With your patch, the NumLock will be forced to 'off' and it > will be out of sync with the first keyboard. The leds will get in sync > later when any change occurs. Oh, right, I missed it. The difficulty is how I rely on usages and fields being already mapped by hidinput_configure_usage(). Thanks for letting me know, I'll think about it. -- Pete - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Fri, 30 Mar 2007, Pete Zaitcev wrote:
> @@ -1328,9 +1340,18 @@ static int hid_probe(struct usb_interface *intf, const
> struct usb_device_id *id)
> return -ENODEV;
> }
>
> - if ((hid->claimed & HID_CLAIMED_INPUT))
> + if ((hid->claimed & HID_CLAIMED_INPUT)) {
> hid_ff_init(hid);
>
> + /*
> + * We do this only if input has claimed the device because
> + * we can only find fields after they were configured in
> + * hidinput_connect.
> + */
> + /* if (hid->quirks & HID_QUIRK_RESET_LEDS) */
> + usbhid_set_leds(hid, LED_NUML, 0);
> + }
> +
> if (hid->quirks & HID_QUIRK_SONY_PS3_CONTROLLER)
> hid_fixup_sony_ps3_controller(interface_to_usbdev(intf),
> intf->cur_altsetting->desc.bInterfaceNumber);
Hi Pete,
I think I see an issue here. Imagine that you boot a system initially with
one keyboard connected (usb, ps/2, doesn't matter), and after some time
you connect second USB keyboard (the NumLock is 'on' on the first keyboard
when you connect the second one).
Without your patch, the NumLock led will be OK on the second keyboard
immediately. With your patch, the NumLock will be forced to 'off' and it
will be out of sync with the first keyboard. The leds will get in sync
later when any change occurs.
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index d26b08f..f592f01 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -267,6 +267,7 @@ struct hid_item {
> #define HID_QUIRK_SKIP_OUTPUT_REPORTS0x0002
> #define HID_QUIRK_IGNORE_MOUSE 0x0004
> #define HID_QUIRK_SONY_PS3_CONTROLLER0x0008
> +#define HID_QUIRK_RESET_LEDS 0x0010
I think this is not worth a quirk - when we get it working properly, why
not do it unconditionally for all keyboards?
> URL with details, discussion, rejected patch to read BIOS byte at 0x417:
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=228674
"You are not authorized to access bug #228674. To see this bug, you must
first log in to an account with the appropriate permissions."
:)
Thanks,
--
Jiri Kosina
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Fri, 30 Mar 2007, Pete Zaitcev wrote:
@@ -1328,9 +1340,18 @@ static int hid_probe(struct usb_interface *intf, const
struct usb_device_id *id)
return -ENODEV;
}
- if ((hid-claimed HID_CLAIMED_INPUT))
+ if ((hid-claimed HID_CLAIMED_INPUT)) {
hid_ff_init(hid);
+ /*
+ * We do this only if input has claimed the device because
+ * we can only find fields after they were configured in
+ * hidinput_connect.
+ */
+ /* if (hid-quirks HID_QUIRK_RESET_LEDS) */
+ usbhid_set_leds(hid, LED_NUML, 0);
+ }
+
if (hid-quirks HID_QUIRK_SONY_PS3_CONTROLLER)
hid_fixup_sony_ps3_controller(interface_to_usbdev(intf),
intf-cur_altsetting-desc.bInterfaceNumber);
Hi Pete,
I think I see an issue here. Imagine that you boot a system initially with
one keyboard connected (usb, ps/2, doesn't matter), and after some time
you connect second USB keyboard (the NumLock is 'on' on the first keyboard
when you connect the second one).
Without your patch, the NumLock led will be OK on the second keyboard
immediately. With your patch, the NumLock will be forced to 'off' and it
will be out of sync with the first keyboard. The leds will get in sync
later when any change occurs.
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d26b08f..f592f01 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -267,6 +267,7 @@ struct hid_item {
#define HID_QUIRK_SKIP_OUTPUT_REPORTS0x0002
#define HID_QUIRK_IGNORE_MOUSE 0x0004
#define HID_QUIRK_SONY_PS3_CONTROLLER0x0008
+#define HID_QUIRK_RESET_LEDS 0x0010
I think this is not worth a quirk - when we get it working properly, why
not do it unconditionally for all keyboards?
URL with details, discussion, rejected patch to read BIOS byte at 0x417:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=228674
You are not authorized to access bug #228674. To see this bug, you must
first log in to an account with the appropriate permissions.
:)
Thanks,
--
Jiri Kosina
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Sat, 31 Mar 2007 21:35:19 +0200 (CEST), Jiri Kosina [EMAIL PROTECTED] wrote: On Fri, 30 Mar 2007, Pete Zaitcev wrote: I think I see an issue here. Imagine that you boot a system initially with one keyboard connected (usb, ps/2, doesn't matter), and after some time you connect second USB keyboard (the NumLock is 'on' on the first keyboard when you connect the second one). Without your patch, the NumLock led will be OK on the second keyboard immediately. With your patch, the NumLock will be forced to 'off' and it will be out of sync with the first keyboard. The leds will get in sync later when any change occurs. Oh, right, I missed it. The difficulty is how I rely on usages and fields being already mapped by hidinput_configure_usage(). Thanks for letting me know, I'll think about it. -- Pete - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On 3/30/07, Pete Zaitcev <[EMAIL PROTECTED]> wrote: On Fri, 30 Mar 2007 14:14:20 -0400, "Dmitry Torokhov" <[EMAIL PROTECTED]> wrote: > > I didn't like a) layering violation, and b) that they defeat filtering > > unconditionally. Why have any filtering then? > > > > Instead, I propose for USB HID driver to reset NumLock on probe. Like this: > > > > --- a/drivers/usb/input/hid-core.c > This is fine and that's what we do in atkbd probe but maybe we should > move that in input core and reset leds as part of > input_register_device()? Sure, as long as it works. I think (as much as I understand), that we already attempt to do this indirectly. input_register_device invokes ->start handlers, and the kbd_start attempts to reset LEDs, but fails because of the state filtering. Not exactly... Keyboard handler's start method tries to bring state of new keyboard in sync with the state of the rest of keyboards. The purpose of start() is not to complete hardware initialization but to adjust logical state of the device according to handler's requirements... But I am backpedaling on my statement about moving it into input core. The driver (HID in this case) should properly reset the device before registering it with input layer. -- Dmitry - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Fri, 30 Mar 2007 14:14:20 -0400, "Dmitry Torokhov" <[EMAIL PROTECTED]> wrote: > > I didn't like a) layering violation, and b) that they defeat filtering > > unconditionally. Why have any filtering then? > > > > Instead, I propose for USB HID driver to reset NumLock on probe. Like this: > > > > --- a/drivers/usb/input/hid-core.c > This is fine and that's what we do in atkbd probe but maybe we should > move that in input core and reset leds as part of > input_register_device()? Sure, as long as it works. I think (as much as I understand), that we already attempt to do this indirectly. input_register_device invokes ->start handlers, and the kbd_start attempts to reset LEDs, but fails because of the state filtering. -- Pete - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
Hi Pete,
On 3/30/07, Pete Zaitcev <[EMAIL PROTECTED]> wrote:
I didn't like a) layering violation, and b) that they defeat filtering
unconditionally. Why have any filtering then?
Instead, I propose for USB HID driver to reset NumLock on probe. Like this:
--- a/drivers/usb/input/hid-core.c
+++ b/drivers/usb/input/hid-core.c
@@ -458,6 +458,18 @@ static int usb_hidinput_input_event(struct input_dev *dev,
unsigned int type, un
return 0;
}
+static void usbhid_set_leds(struct hid_device *hid, unsigned int code, int val)
+{
+ struct hid_field *field;
+ int offset;
+
+ /* This is often called for the mouse half. */
+ if ((offset = hidinput_find_field(hid, EV_LED, code, )) != -1) {
+ hid_set_field(field, offset, val);
+ usbhid_submit_report(hid, field->report, USB_DIR_OUT);
+ }
+}
+
This is fine and that's what we do in atkbd probe but maybe we should
move that in input core and reset leds as part of
input_register_device()?
--
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
usb hid: reset NumLock
This is a patch for comments only, please do not apply (at least not as-is).
I haven't got the test results yet.
Dell people (Stuart and Charles) complained that on some USB keyboards,
if BIOS enables NumLock, it stays on even after Linux has started. Since
we always start with NumLock off, this confuses users. Quick double dab
at NumLock fixes it, but it's not nice.
The keyboard driver tries to reset LEDs at start, but this never works.
Since the bitmap dev->led is zero, the input_event() filters out any
attempts to switch LEDs off. So, the code in kbd_start() does nothing.
The Dell's solution looks like this (for 2.6.9 code base):
linux-2.6.9-42.0.3/drivers/char/keyboard.c:
@@ -931,6 +931,19 @@ void kbd_refresh_leds(struct input_handl
tasklet_disable(_tasklet);
if (leds != 0xff) {
+ /*
+* We can't be sure of the state of the LEDs on keyboards
+* (e.g., BIOS might have left LED_NUML on), so set dev->led
+* opposite of ledstate, to make sure input_event() actually
+* sends the command to the keyboard to set each LED.
+*/
+ if (test_bit(LED_SCROLLL, handle->dev->led) == !!(leds & 0x01))
+ change_bit(LED_SCROLLL, handle->dev->led);
+ if (test_bit(LED_NUML, handle->dev->led) == !!(leds & 0x02))
+ change_bit(LED_NUML, handle->dev->led);
+ if (test_bit(LED_CAPSL, handle->dev->led) == !!(leds & 0x04))
+ change_bit(LED_CAPSL, handle->dev->led);
+
input_event(handle->dev, EV_LED, LED_SCROLLL, !!(leds & 0x01));
input_event(handle->dev, EV_LED, LED_NUML,!!(leds & 0x02));
input_event(handle->dev, EV_LED, LED_CAPSL, !!(leds & 0x04));
I didn't like a) layering violation, and b) that they defeat filtering
unconditionally. Why have any filtering then?
Instead, I propose for USB HID driver to reset NumLock on probe. Like this:
--- a/drivers/usb/input/hid-core.c
+++ b/drivers/usb/input/hid-core.c
@@ -458,6 +458,18 @@ static int usb_hidinput_input_event(struct input_dev *dev,
unsigned int type, un
return 0;
}
+static void usbhid_set_leds(struct hid_device *hid, unsigned int code, int val)
+{
+ struct hid_field *field;
+ int offset;
+
+ /* This is often called for the mouse half. */
+ if ((offset = hidinput_find_field(hid, EV_LED, code, )) != -1) {
+ hid_set_field(field, offset, val);
+ usbhid_submit_report(hid, field->report, USB_DIR_OUT);
+ }
+}
+
int usbhid_wait_io(struct hid_device *hid)
{
struct usbhid_device *usbhid = hid->driver_data;
@@ -1328,9 +1340,18 @@ static int hid_probe(struct usb_interface *intf, const
struct usb_device_id *id)
return -ENODEV;
}
- if ((hid->claimed & HID_CLAIMED_INPUT))
+ if ((hid->claimed & HID_CLAIMED_INPUT)) {
hid_ff_init(hid);
+ /*
+* We do this only if input has claimed the device because
+* we can only find fields after they were configured in
+* hidinput_connect.
+*/
+ /* if (hid->quirks & HID_QUIRK_RESET_LEDS) */
+ usbhid_set_leds(hid, LED_NUML, 0);
+ }
+
if (hid->quirks & HID_QUIRK_SONY_PS3_CONTROLLER)
hid_fixup_sony_ps3_controller(interface_to_usbdev(intf),
intf->cur_altsetting->desc.bInterfaceNumber);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d26b08f..f592f01 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -267,6 +267,7 @@ struct hid_item {
#define HID_QUIRK_SKIP_OUTPUT_REPORTS 0x0002
#define HID_QUIRK_IGNORE_MOUSE 0x0004
#define HID_QUIRK_SONY_PS3_CONTROLLER 0x0008
+#define HID_QUIRK_RESET_LEDS 0x0010
/*
* This is the global environment of the parser. This information is
URL with details, discussion, rejected patch to read BIOS byte at 0x417:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=228674
Jiri, Dmitry, any opinions please?
-- Pete
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
usb hid: reset NumLock
This is a patch for comments only, please do not apply (at least not as-is).
I haven't got the test results yet.
Dell people (Stuart and Charles) complained that on some USB keyboards,
if BIOS enables NumLock, it stays on even after Linux has started. Since
we always start with NumLock off, this confuses users. Quick double dab
at NumLock fixes it, but it's not nice.
The keyboard driver tries to reset LEDs at start, but this never works.
Since the bitmap dev-led is zero, the input_event() filters out any
attempts to switch LEDs off. So, the code in kbd_start() does nothing.
The Dell's solution looks like this (for 2.6.9 code base):
linux-2.6.9-42.0.3/drivers/char/keyboard.c:
@@ -931,6 +931,19 @@ void kbd_refresh_leds(struct input_handl
tasklet_disable(keyboard_tasklet);
if (leds != 0xff) {
+ /*
+* We can't be sure of the state of the LEDs on keyboards
+* (e.g., BIOS might have left LED_NUML on), so set dev-led
+* opposite of ledstate, to make sure input_event() actually
+* sends the command to the keyboard to set each LED.
+*/
+ if (test_bit(LED_SCROLLL, handle-dev-led) == !!(leds 0x01))
+ change_bit(LED_SCROLLL, handle-dev-led);
+ if (test_bit(LED_NUML, handle-dev-led) == !!(leds 0x02))
+ change_bit(LED_NUML, handle-dev-led);
+ if (test_bit(LED_CAPSL, handle-dev-led) == !!(leds 0x04))
+ change_bit(LED_CAPSL, handle-dev-led);
+
input_event(handle-dev, EV_LED, LED_SCROLLL, !!(leds 0x01));
input_event(handle-dev, EV_LED, LED_NUML,!!(leds 0x02));
input_event(handle-dev, EV_LED, LED_CAPSL, !!(leds 0x04));
I didn't like a) layering violation, and b) that they defeat filtering
unconditionally. Why have any filtering then?
Instead, I propose for USB HID driver to reset NumLock on probe. Like this:
--- a/drivers/usb/input/hid-core.c
+++ b/drivers/usb/input/hid-core.c
@@ -458,6 +458,18 @@ static int usb_hidinput_input_event(struct input_dev *dev,
unsigned int type, un
return 0;
}
+static void usbhid_set_leds(struct hid_device *hid, unsigned int code, int val)
+{
+ struct hid_field *field;
+ int offset;
+
+ /* This is often called for the mouse half. */
+ if ((offset = hidinput_find_field(hid, EV_LED, code, field)) != -1) {
+ hid_set_field(field, offset, val);
+ usbhid_submit_report(hid, field-report, USB_DIR_OUT);
+ }
+}
+
int usbhid_wait_io(struct hid_device *hid)
{
struct usbhid_device *usbhid = hid-driver_data;
@@ -1328,9 +1340,18 @@ static int hid_probe(struct usb_interface *intf, const
struct usb_device_id *id)
return -ENODEV;
}
- if ((hid-claimed HID_CLAIMED_INPUT))
+ if ((hid-claimed HID_CLAIMED_INPUT)) {
hid_ff_init(hid);
+ /*
+* We do this only if input has claimed the device because
+* we can only find fields after they were configured in
+* hidinput_connect.
+*/
+ /* if (hid-quirks HID_QUIRK_RESET_LEDS) */
+ usbhid_set_leds(hid, LED_NUML, 0);
+ }
+
if (hid-quirks HID_QUIRK_SONY_PS3_CONTROLLER)
hid_fixup_sony_ps3_controller(interface_to_usbdev(intf),
intf-cur_altsetting-desc.bInterfaceNumber);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d26b08f..f592f01 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -267,6 +267,7 @@ struct hid_item {
#define HID_QUIRK_SKIP_OUTPUT_REPORTS 0x0002
#define HID_QUIRK_IGNORE_MOUSE 0x0004
#define HID_QUIRK_SONY_PS3_CONTROLLER 0x0008
+#define HID_QUIRK_RESET_LEDS 0x0010
/*
* This is the global environment of the parser. This information is
URL with details, discussion, rejected patch to read BIOS byte at 0x417:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=228674
Jiri, Dmitry, any opinions please?
-- Pete
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
Hi Pete,
On 3/30/07, Pete Zaitcev [EMAIL PROTECTED] wrote:
I didn't like a) layering violation, and b) that they defeat filtering
unconditionally. Why have any filtering then?
Instead, I propose for USB HID driver to reset NumLock on probe. Like this:
--- a/drivers/usb/input/hid-core.c
+++ b/drivers/usb/input/hid-core.c
@@ -458,6 +458,18 @@ static int usb_hidinput_input_event(struct input_dev *dev,
unsigned int type, un
return 0;
}
+static void usbhid_set_leds(struct hid_device *hid, unsigned int code, int val)
+{
+ struct hid_field *field;
+ int offset;
+
+ /* This is often called for the mouse half. */
+ if ((offset = hidinput_find_field(hid, EV_LED, code, field)) != -1) {
+ hid_set_field(field, offset, val);
+ usbhid_submit_report(hid, field-report, USB_DIR_OUT);
+ }
+}
+
This is fine and that's what we do in atkbd probe but maybe we should
move that in input core and reset leds as part of
input_register_device()?
--
Dmitry
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On Fri, 30 Mar 2007 14:14:20 -0400, Dmitry Torokhov [EMAIL PROTECTED] wrote: I didn't like a) layering violation, and b) that they defeat filtering unconditionally. Why have any filtering then? Instead, I propose for USB HID driver to reset NumLock on probe. Like this: --- a/drivers/usb/input/hid-core.c This is fine and that's what we do in atkbd probe but maybe we should move that in input core and reset leds as part of input_register_device()? Sure, as long as it works. I think (as much as I understand), that we already attempt to do this indirectly. input_register_device invokes -start handlers, and the kbd_start attempts to reset LEDs, but fails because of the state filtering. -- Pete - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: usb hid: reset NumLock
On 3/30/07, Pete Zaitcev [EMAIL PROTECTED] wrote: On Fri, 30 Mar 2007 14:14:20 -0400, Dmitry Torokhov [EMAIL PROTECTED] wrote: I didn't like a) layering violation, and b) that they defeat filtering unconditionally. Why have any filtering then? Instead, I propose for USB HID driver to reset NumLock on probe. Like this: --- a/drivers/usb/input/hid-core.c This is fine and that's what we do in atkbd probe but maybe we should move that in input core and reset leds as part of input_register_device()? Sure, as long as it works. I think (as much as I understand), that we already attempt to do this indirectly. input_register_device invokes -start handlers, and the kbd_start attempts to reset LEDs, but fails because of the state filtering. Not exactly... Keyboard handler's start method tries to bring state of new keyboard in sync with the state of the rest of keyboards. The purpose of start() is not to complete hardware initialization but to adjust logical state of the device according to handler's requirements... But I am backpedaling on my statement about moving it into input core. The driver (HID in this case) should properly reset the device before registering it with input layer. -- Dmitry - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
