2003-06-14 20:21:44

by Vojtech Pavlik

[permalink] [raw]
Subject: [patch] input: Implement device grabbing [1/13]

Hi!

This is a batch of already very much overdue fixes and needed
improvements. The most significant changes are Synaptics pad support
(although that code will still develop a bit), and fixes and
quirk additions in USB HID driver.

You can pull this changeset from:
bk://kernel.bkbits.net/vojtech/input

===================================================================

[email protected], 2003-06-09 13:45:46+02:00, [email protected]
input: Implement input device grabbing so that it is possible to steal
an input device from other handlers and have an exclusive access
to events.


drivers/input/evdev.c | 37 +++++++++++++++++++++++++++++++++++--
drivers/input/input.c | 27 ++++++++++++++++++++++++---
include/linux/input.h | 7 +++++++
3 files changed, 66 insertions(+), 5 deletions(-)

===================================================================

diff -Nru a/drivers/input/evdev.c b/drivers/input/evdev.c
--- a/drivers/input/evdev.c Sat Jun 14 22:21:24 2003
+++ b/drivers/input/evdev.c Sat Jun 14 22:21:24 2003
@@ -29,6 +29,7 @@
char name[16];
struct input_handle handle;
wait_queue_head_t wait;
+ struct evdev_list *grab;
struct list_head list;
};

@@ -48,7 +49,8 @@
struct evdev *evdev = handle->private;
struct evdev_list *list;

- list_for_each_entry(list, &evdev->list, node) {
+ if (evdev->grab) {
+ list = evdev->grab;

do_gettimeofday(&list->buffer[list->head].time);
list->buffer[list->head].type = type;
@@ -57,7 +59,17 @@
list->head = (list->head + 1) & (EVDEV_BUFFER_SIZE - 1);

kill_fasync(&list->fasync, SIGIO, POLL_IN);
- }
+ } else
+ list_for_each_entry(list, &evdev->list, node) {
+
+ do_gettimeofday(&list->buffer[list->head].time);
+ list->buffer[list->head].type = type;
+ list->buffer[list->head].code = code;
+ list->buffer[list->head].value = value;
+ list->head = (list->head + 1) & (EVDEV_BUFFER_SIZE - 1);
+
+ kill_fasync(&list->fasync, SIGIO, POLL_IN);
+ }

wake_up_interruptible(&evdev->wait);
}
@@ -88,6 +100,11 @@
{
struct evdev_list *list = file->private_data;

+ if (list->evdev->grab == list) {
+ input_release_device(&list->evdev->handle);
+ list->evdev->grab = NULL;
+ }
+
evdev_fasync(-1, file, 0);
list_del(&list->node);

@@ -256,6 +273,22 @@
if (put_user(dev->ff_effects_max, (int*) arg))
return -EFAULT;
return 0;
+
+ case EVIOCGRAB:
+ if (arg) {
+ if (evdev->grab)
+ return -EBUSY;
+ if (input_grab_device(&evdev->handle))
+ return -EBUSY;
+ evdev->grab = list;
+ return 0;
+ } else {
+ if (evdev->grab != list)
+ return -EINVAL;
+ input_release_device(&evdev->handle);
+ evdev->grab = NULL;
+ return 0;
+ }

default:

diff -Nru a/drivers/input/input.c b/drivers/input/input.c
--- a/drivers/input/input.c Sat Jun 14 22:21:24 2003
+++ b/drivers/input/input.c Sat Jun 14 22:21:24 2003
@@ -33,6 +33,8 @@
EXPORT_SYMBOL(input_unregister_device);
EXPORT_SYMBOL(input_register_handler);
EXPORT_SYMBOL(input_unregister_handler);
+EXPORT_SYMBOL(input_grab_device);
+EXPORT_SYMBOL(input_release_device);
EXPORT_SYMBOL(input_open_device);
EXPORT_SYMBOL(input_close_device);
EXPORT_SYMBOL(input_accept_process);
@@ -175,9 +177,12 @@
if (type != EV_SYN)
dev->sync = 0;

- list_for_each_entry(handle, &dev->h_list, d_node)
- if (handle->open)
- handle->handler->event(handle, type, code, value);
+ if (dev->grab)
+ dev->grab->handler->event(dev->grab, type, code, value);
+ else
+ list_for_each_entry(handle, &dev->h_list, d_node)
+ if (handle->open)
+ handle->handler->event(handle, type, code, value);
}

static void input_repeat_key(unsigned long data)
@@ -201,6 +206,21 @@
return 0;
}

+int input_grab_device(struct input_handle *handle)
+{
+ if (handle->dev->grab)
+ return -EBUSY;
+
+ handle->dev->grab = handle;
+ return 0;
+}
+
+void input_release_device(struct input_handle *handle)
+{
+ if (handle->dev->grab == handle)
+ handle->dev->grab = NULL;
+}
+
int input_open_device(struct input_handle *handle)
{
if (handle->dev->pm_dev)
@@ -221,6 +241,7 @@

void input_close_device(struct input_handle *handle)
{
+ input_release_device(handle);
if (handle->dev->pm_dev)
pm_dev_idle(handle->dev->pm_dev);
if (handle->dev->close)
diff -Nru a/include/linux/input.h b/include/linux/input.h
--- a/include/linux/input.h Sat Jun 14 22:21:24 2003
+++ b/include/linux/input.h Sat Jun 14 22:21:24 2003
@@ -77,6 +77,8 @@
#define EVIOCRMFF _IOW('E', 0x81, int) /* Erase a force effect */
#define EVIOCGEFFECTS _IOR('E', 0x84, int) /* Report number of effects playable at the same time */

+#define EVIOCGRAB _IOW('E', 0x90, int) /* Grab/Release device */
+
/*
* Event types
*/
@@ -798,6 +800,8 @@
int (*upload_effect)(struct input_dev *dev, struct ff_effect *effect);
int (*erase_effect)(struct input_dev *dev, int effect_id);

+ struct input_handle *grab;
+
struct list_head h_list;
struct list_head node;
};
@@ -887,6 +891,9 @@

void input_register_handler(struct input_handler *);
void input_unregister_handler(struct input_handler *);
+
+int input_grab_device(struct input_handle *);
+void input_release_device(struct input_handle *);

int input_open_device(struct input_handle *);
void input_close_device(struct input_handle *);


2003-06-14 20:22:51

by Vojtech Pavlik

[permalink] [raw]
Subject: [patch] input: Fix sunkbd keybit bitfield filling [2/13]


You can pull this changeset from:
bk://kernel.bkbits.net/vojtech/input

===================================================================

[email protected], 2003-06-09 13:48:38+02:00, [email protected]
input: fix sunkbd to properly set its bitfields up to key #127.


sunkbd.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)

===================================================================

diff -Nru a/drivers/input/keyboard/sunkbd.c b/drivers/input/keyboard/sunkbd.c
--- a/drivers/input/keyboard/sunkbd.c Sat Jun 14 22:21:43 2003
+++ b/drivers/input/keyboard/sunkbd.c Sat Jun 14 22:21:43 2003
@@ -271,7 +271,7 @@
sprintf(sunkbd->name, "Sun Type %d keyboard", sunkbd->type);

memcpy(sunkbd->keycode, sunkbd_keycode, sizeof(sunkbd->keycode));
- for (i = 0; i < 127; i++)
+ for (i = 0; i < 128; i++)
set_bit(sunkbd->keycode[i], sunkbd->dev.keybit);
clear_bit(0, sunkbd->dev.keybit);

2003-06-14 20:23:41

by Vojtech Pavlik

[permalink] [raw]
Subject: [patch] input: Implement HID quirk for A4Tech mice [3/13]


You can pull this changeset from:
bk://kernel.bkbits.net/vojtech/input

===================================================================

[email protected], 2003-06-09 13:52:46+02:00, [email protected]
input: Implement a HID quirk for 2-wheel A4Tech mice.


hid-core.c | 4 ++++
hid-input.c | 19 +++++++++++++++++++
hid.h | 16 +++++++++-------
3 files changed, 32 insertions(+), 7 deletions(-)

===================================================================

diff -Nru a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c
--- a/drivers/usb/input/hid-core.c Sat Jun 14 22:22:02 2003
+++ b/drivers/usb/input/hid-core.c Sat Jun 14 22:22:02 2003
@@ -1351,6 +1351,9 @@
#define USB_VENDOR_ID_ESSENTIAL_REALITY 0x0d7f
#define USB_DEVICE_ID_ESSENTIAL_REALITY_P5 0x0100

+#define USB_VENDOR_ID_A4TECH 0x09DA
+#define USB_DEVICE_ID_A4TECH_WCP32PU 0x0006
+
struct hid_blacklist {
__u16 idVendor;
__u16 idProduct;
@@ -1398,6 +1401,7 @@
{ USB_VENDOR_ID_ONTRAK, USB_DEVICE_ID_ONTRAK_ADU100 + 500, HID_QUIRK_IGNORE },
{ USB_VENDOR_ID_TANGTOP, USB_DEVICE_ID_TANGTOP_USBPS2, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_ESSENTIAL_REALITY, USB_DEVICE_ID_ESSENTIAL_REALITY_P5, HID_QUIRK_IGNORE },
+ { USB_VENDOR_ID_A4TECH, USB_DEVICE_ID_A4TECH_WCP32PU, HID_QUIRK_2WHEEL_MOUSE_HACK },
{ 0, 0 }
};

diff -Nru a/drivers/usb/input/hid-input.c b/drivers/usb/input/hid-input.c
--- a/drivers/usb/input/hid-input.c Sat Jun 14 22:22:02 2003
+++ b/drivers/usb/input/hid-input.c Sat Jun 14 22:22:02 2003
@@ -376,6 +376,11 @@
}

set_bit(usage->type, input->evbit);
+ if ((usage->type == EV_REL)
+ && (device->quirks & HID_QUIRK_2WHEEL_MOUSE_HACK)
+ && (usage->code == REL_WHEEL)) {
+ set_bit(REL_HWHEEL, bit);
+ }

while (usage->code <= max && test_and_set_bit(usage->code, bit)) {
usage->code = find_next_zero_bit(bit, max + 1, usage->code);
@@ -425,6 +430,20 @@
return;

input_regs(input, regs);
+
+ if ((hid->quirks & HID_QUIRK_2WHEEL_MOUSE_HACK)
+ && (usage->code == BTN_BACK)) {
+ if (value)
+ hid->quirks |= HID_QUIRK_2WHEEL_MOUSE_HACK_ON;
+ else
+ hid->quirks &= ~HID_QUIRK_2WHEEL_MOUSE_HACK_ON;
+ return;
+ }
+ if ((hid->quirks & HID_QUIRK_2WHEEL_MOUSE_HACK_ON)
+ && (usage->code == REL_WHEEL)) {
+ input_event(input, usage->type, REL_HWHEEL, value);
+ return;
+ }

if (usage->hat_min != usage->hat_max) {
value = (value - usage->hat_min) * 8 / (usage->hat_max - usage->hat_min + 1) + 1;
diff -Nru a/drivers/usb/input/hid.h b/drivers/usb/input/hid.h
--- a/drivers/usb/input/hid.h Sat Jun 14 22:22:01 2003
+++ b/drivers/usb/input/hid.h Sat Jun 14 22:22:01 2003
@@ -201,13 +201,15 @@
* HID device quirks.
*/

-#define HID_QUIRK_INVERT 0x01
-#define HID_QUIRK_NOTOUCH 0x02
-#define HID_QUIRK_IGNORE 0x04
-#define HID_QUIRK_NOGET 0x08
-#define HID_QUIRK_HIDDEV 0x10
-#define HID_QUIRK_BADPAD 0x20
-#define HID_QUIRK_MULTI_INPUT 0x40
+#define HID_QUIRK_INVERT 0x001
+#define HID_QUIRK_NOTOUCH 0x002
+#define HID_QUIRK_IGNORE 0x004
+#define HID_QUIRK_NOGET 0x008
+#define HID_QUIRK_HIDDEV 0x010
+#define HID_QUIRK_BADPAD 0x020
+#define HID_QUIRK_MULTI_INPUT 0x040
+#define HID_QUIRK_2WHEEL_MOUSE_HACK 0x080
+#define HID_QUIRK_2WHEEL_MOUSE_HACK_ON 0x100

/*
* This is the global environment of the parser. This information is

2003-06-14 20:26:05

by Vojtech Pavlik

[permalink] [raw]
Subject: [patch] input: Add hiragana/katakana keys to atkbd.c [4/13]


You can pull this changeset from:
bk://kernel.bkbits.net/vojtech/input

===================================================================

[email protected], 2003-06-09 13:55:51+02:00, [email protected]
input: Add default mapping for the hiragana/katakana key.


atkbd.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)

===================================================================

diff -Nru a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
--- a/drivers/input/keyboard/atkbd.c Sat Jun 14 22:22:20 2003
+++ b/drivers/input/keyboard/atkbd.c Sat Jun 14 22:22:20 2003
@@ -39,7 +39,7 @@

static unsigned char atkbd_set2_keycode[512] = {
0, 67, 65, 63, 61, 59, 60, 88, 0, 68, 66, 64, 62, 15, 41, 85,
- 0, 56, 42, 0, 29, 16, 2, 89, 0, 0, 44, 31, 30, 17, 3, 90,
+ 0, 56, 42,182, 29, 16, 2, 89, 0, 0, 44, 31, 30, 17, 3, 90,
0, 46, 45, 32, 18, 5, 4, 91, 0, 57, 47, 33, 20, 19, 6, 0,
0, 49, 48, 35, 34, 21, 7, 0, 0, 0, 50, 36, 22, 8, 9, 0,
0, 51, 37, 23, 24, 11, 10, 0, 0, 52, 53, 38, 39, 25, 12, 0,

2003-06-14 20:27:23

by Vojtech Pavlik

[permalink] [raw]
Subject: [patch] input: Add PCI PS/2 controller support [5/13]


You can pull this changeset from:
bk://kernel.bkbits.net/vojtech/input

===================================================================

[email protected], 2003-06-09 14:02:05+02:00, [email protected]
input: PCI PS/2 keyboard and mouse controller (Mobility Docking station)


Kconfig | 11 +++
Makefile | 1
pcips2.c | 230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 242 insertions(+)

===================================================================

diff -Nru a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
--- a/drivers/input/serio/Kconfig Sat Jun 14 22:22:39 2003
+++ b/drivers/input/serio/Kconfig Sat Jun 14 22:22:39 2003
@@ -119,3 +119,14 @@
The module will be called rpckbd.o. If you want to compile it as a
module, say M here and read <file:Documentation/modules.txt>.

+config SERIO_PCIPS2
+ tristate "PCI PS/2 keyboard and PS/2 mouse controller"
+ depends on PCI && SERIO
+ help
+ Say Y here if you have a Mobility Docking station with PS/2
+ keyboard and mice ports.
+
+ This driver is also available as a module ( = code which can be
+ inserted in and removed from the running kernel whenever you want).
+ The module will be called rpckbd. If you want to compile it as a
+ module, say M here and read <file:Documentation/modules.txt>.
diff -Nru a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
--- a/drivers/input/serio/Makefile Sat Jun 14 22:22:39 2003
+++ b/drivers/input/serio/Makefile Sat Jun 14 22:22:39 2003
@@ -14,3 +14,4 @@
obj-$(CONFIG_SERIO_AMBAKMI) += ambakmi.o
obj-$(CONFIG_SERIO_Q40KBD) += q40kbd.o
obj-$(CONFIG_SERIO_98KBD) += 98kbd-io.o
+obj-$(CONFIG_SERIO_PCIPS2) += pcips2.o
diff -Nru a/drivers/input/serio/pcips2.c b/drivers/input/serio/pcips2.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/input/serio/pcips2.c Sat Jun 14 22:22:39 2003
@@ -0,0 +1,230 @@
+/*
+ * linux/drivers/input/serio/pcips2.c
+ *
+ * Copyright (C) 2003 Russell King, All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * I'm not sure if this is a generic PS/2 PCI interface or specific to
+ * the Mobility Electronics docking station.
+ */
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/input.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+#include <linux/serio.h>
+#include <linux/delay.h>
+#include <asm/io.h>
+
+#define PS2_CTRL (0)
+#define PS2_STATUS (1)
+#define PS2_DATA (2)
+
+#define PS2_CTRL_CLK (1<<0)
+#define PS2_CTRL_DAT (1<<1)
+#define PS2_CTRL_TXIRQ (1<<2)
+#define PS2_CTRL_ENABLE (1<<3)
+#define PS2_CTRL_RXIRQ (1<<4)
+
+#define PS2_STAT_CLK (1<<0)
+#define PS2_STAT_DAT (1<<1)
+#define PS2_STAT_PARITY (1<<2)
+#define PS2_STAT_RXFULL (1<<5)
+#define PS2_STAT_TXBUSY (1<<6)
+#define PS2_STAT_TXEMPTY (1<<7)
+
+struct pcips2_data {
+ struct serio io;
+ unsigned int base;
+ struct pci_dev *dev;
+};
+
+static int pcips2_write(struct serio *io, unsigned char val)
+{
+ struct pcips2_data *ps2if = io->driver;
+ unsigned int stat;
+
+ do {
+ stat = inb(ps2if->base + PS2_STATUS);
+ cpu_relax();
+ } while (!(stat & PS2_STAT_TXEMPTY));
+
+ outb(val, ps2if->base + PS2_DATA);
+
+ return 0;
+}
+
+static void pcips2_interrupt(int irq, void *devid, struct pt_regs *regs)
+{
+ struct pcips2_data *ps2if = devid;
+ unsigned char status, scancode;
+
+ do {
+ unsigned int flag;
+
+ status = inb(ps2if->base + PS2_STATUS);
+ if (!(status & PS2_STAT_RXFULL))
+ break;
+ scancode = inb(ps2if->base + PS2_DATA);
+ if (status == 0xff && scancode == 0xff)
+ break;
+
+ flag = (status & PS2_STAT_PARITY) ? 0 : SERIO_PARITY;
+
+ if (hweight8(scancode) & 1)
+ flag ^= SERIO_PARITY;
+
+ serio_interrupt(&ps2if->io, scancode, flag, regs);
+ } while (1);
+}
+
+static void pcips2_flush_input(struct pcips2_data *ps2if)
+{
+ unsigned char status, scancode;
+
+ do {
+ status = inb(ps2if->base + PS2_STATUS);
+ if (!(status & PS2_STAT_RXFULL))
+ break;
+ scancode = inb(ps2if->base + PS2_DATA);
+ if (status == 0xff && scancode == 0xff)
+ break;
+ } while (1);
+}
+
+static int pcips2_open(struct serio *io)
+{
+ struct pcips2_data *ps2if = io->driver;
+ int ret, val = 0;
+
+ outb(PS2_CTRL_ENABLE, ps2if->base);
+ pcips2_flush_input(ps2if);
+
+ ret = request_irq(ps2if->dev->irq, pcips2_interrupt, SA_SHIRQ,
+ "pcips2", ps2if);
+ if (ret == 0)
+ val = PS2_CTRL_ENABLE | PS2_CTRL_RXIRQ;
+
+ outb(val, ps2if->base);
+
+ return ret;
+}
+
+static void pcips2_close(struct serio *io)
+{
+ struct pcips2_data *ps2if = io->driver;
+
+ outb(0, ps2if->base);
+
+ free_irq(ps2if->dev->irq, ps2if);
+}
+
+static int __devinit pcips2_probe(struct pci_dev *dev, const struct pci_device_id *id)
+{
+ struct pcips2_data *ps2if;
+ int ret;
+
+ ret = pci_enable_device(dev);
+ if (ret)
+ return ret;
+
+ if (!request_region(pci_resource_start(dev, 0),
+ pci_resource_len(dev, 0), "pcips2")) {
+ ret = -EBUSY;
+ goto disable;
+ }
+
+ ps2if = kmalloc(sizeof(struct pcips2_data), GFP_KERNEL);
+ if (!ps2if) {
+ ret = -ENOMEM;
+ goto release;
+ }
+
+ memset(ps2if, 0, sizeof(struct pcips2_data));
+
+ ps2if->io.type = SERIO_8042;
+ ps2if->io.write = pcips2_write;
+ ps2if->io.open = pcips2_open;
+ ps2if->io.close = pcips2_close;
+ ps2if->io.name = dev->dev.name;
+ ps2if->io.phys = dev->dev.bus_id;
+ ps2if->io.driver = ps2if;
+ ps2if->dev = dev;
+ ps2if->base = pci_resource_start(dev, 0);
+
+ pci_set_drvdata(dev, ps2if);
+
+ serio_register_port(&ps2if->io);
+ return 0;
+
+ release:
+ release_region(pci_resource_start(dev, 0),
+ pci_resource_len(dev, 0));
+ disable:
+ pci_disable_device(dev);
+ return ret;
+}
+
+static void __devexit pcips2_remove(struct pci_dev *dev)
+{
+ struct pcips2_data *ps2if = pci_get_drvdata(dev);
+
+ serio_unregister_port(&ps2if->io);
+ release_region(pci_resource_start(dev, 0),
+ pci_resource_len(dev, 0));
+ pci_set_drvdata(dev, NULL);
+ kfree(ps2if);
+ pci_disable_device(dev);
+}
+
+static struct pci_device_id pcips2_ids[] = {
+ {
+ .vendor = 0x14f2, /* MOBILITY */
+ .device = 0x0123, /* Keyboard */
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
+ .class = PCI_CLASS_INPUT_KEYBOARD << 8,
+ .class_mask = 0xffff00,
+ },
+ {
+ .vendor = 0x14f2, /* MOBILITY */
+ .device = 0x0124, /* Mouse */
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
+ .class = PCI_CLASS_INPUT_MOUSE << 8,
+ .class_mask = 0xffff00,
+ },
+ { 0, }
+};
+
+static struct pci_driver pcips2_driver = {
+ .name = "pcips2",
+ .id_table = pcips2_ids,
+ .probe = pcips2_probe,
+ .remove = __devexit_p(pcips2_remove),
+ .driver = {
+ .devclass = &input_devclass,
+ },
+};
+
+static int __init pcips2_init(void)
+{
+ return pci_module_init(&pcips2_driver);
+}
+
+static void __exit pcips2_exit(void)
+{
+ pci_unregister_driver(&pcips2_driver);
+}
+
+module_init(pcips2_init);
+module_exit(pcips2_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Russell King <[email protected]>");
+MODULE_DESCRIPTION("PCI PS/2 keyboard/mouse driver");
+MODULE_DEVICE_TABLE(pci, pcips2_ids);

2003-06-14 20:28:51

by Vojtech Pavlik

[permalink] [raw]
Subject: [patch] input: Turn numlock ON on HP HIL machines [6/13]


You can pull this changeset from:
bk://kernel.bkbits.net/vojtech/input

===================================================================

[email protected], 2003-06-09 14:06:54+02:00, [email protected]
input: Turn on the NumLock ON by default on PARISC HP-HIL machines.


keyboard.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)

===================================================================

diff -Nru a/drivers/char/keyboard.c b/drivers/char/keyboard.c
--- a/drivers/char/keyboard.c Sat Jun 14 22:22:58 2003
+++ b/drivers/char/keyboard.c Sat Jun 14 22:22:58 2003
@@ -52,11 +52,13 @@

/*
* Some laptops take the 789uiojklm,. keys as number pad when NumLock is on.
- * This seems a good reason to start with NumLock off. On PC9800 however there
- * is no NumLock key and everyone expects the keypad to be used for numbers.
+ * This seems a good reason to start with NumLock off. On PC9800 and HIL keyboards
+ * of PARISC machines however there is no NumLock key and everyone expects the keypad
+ * to be used for numbers.
*/

-#ifdef CONFIG_X86_PC9800
+#if defined(CONFIG_X86_PC9800) || \
+ defined(CONFIG_PARISC) && (defined(CONFIG_KEYBOARD_HIL) || defined(CONFIG_KEYBOARD_HIL_OLD))
#define KBD_DEFLEDS (1 << VC_NUMLOCK)
#else
#define KBD_DEFLEDS 0

2003-06-14 20:32:01

by Vojtech Pavlik

[permalink] [raw]
Subject: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]


You can pull this changeset from:
bk://kernel.bkbits.net/vojtech/input

===================================================================

[email protected], 2003-06-09 14:41:31+02:00, [email protected]
input: Change input/misc/pcspkr.c to use CLOCK_TICK_RATE instead of
a fixed value of 1193182. And change CLOCK_TICK_RATE and several
usages of a fixed value 1193180 to a slightly more correct value
of 1193182. (True freq is 1.193181818181...).


drivers/char/vt_ioctl.c | 4 ++--
drivers/input/gameport/gameport.c | 2 +-
drivers/input/joystick/analog.c | 2 +-
drivers/input/misc/pcspkr.c | 2 +-
include/asm-i386/timex.h | 2 +-
include/asm-x86_64/timex.h | 2 +-
6 files changed, 7 insertions(+), 7 deletions(-)

===================================================================

diff -Nru a/drivers/char/vt_ioctl.c b/drivers/char/vt_ioctl.c
--- a/drivers/char/vt_ioctl.c Sat Jun 14 22:23:32 2003
+++ b/drivers/char/vt_ioctl.c Sat Jun 14 22:23:32 2003
@@ -395,7 +395,7 @@
if (!perm)
return -EPERM;
if (arg)
- arg = 1193180 / arg;
+ arg = 1193182 / arg;
kd_mksound(arg, 0);
return 0;

@@ -412,7 +412,7 @@
ticks = HZ * ((arg >> 16) & 0xffff) / 1000;
count = ticks ? (arg & 0xffff) : 0;
if (count)
- count = 1193180 / count;
+ count = 1193182 / count;
kd_mksound(count, ticks);
return 0;
}
diff -Nru a/drivers/input/gameport/gameport.c b/drivers/input/gameport/gameport.c
--- a/drivers/input/gameport/gameport.c Sat Jun 14 22:23:32 2003
+++ b/drivers/input/gameport/gameport.c Sat Jun 14 22:23:32 2003
@@ -37,7 +37,7 @@

#ifdef __i386__

-#define DELTA(x,y) ((y)-(x)+((y)<(x)?1193180/HZ:0))
+#define DELTA(x,y) ((y)-(x)+((y)<(x)?1193182/HZ:0))
#define GET_TIME(x) do { x = get_time_pit(); } while (0)

static unsigned int get_time_pit(void)
diff -Nru a/drivers/input/joystick/analog.c b/drivers/input/joystick/analog.c
--- a/drivers/input/joystick/analog.c Sat Jun 14 22:23:32 2003
+++ b/drivers/input/joystick/analog.c Sat Jun 14 22:23:32 2003
@@ -138,7 +138,7 @@

#ifdef __i386__
#define GET_TIME(x) do { if (cpu_has_tsc) rdtscl(x); else x = get_time_pit(); } while (0)
-#define DELTA(x,y) (cpu_has_tsc?((y)-(x)):((x)-(y)+((x)<(y)?1193180L/HZ:0)))
+#define DELTA(x,y) (cpu_has_tsc?((y)-(x)):((x)-(y)+((x)<(y)?1193182L/HZ:0)))
#define TIME_NAME (cpu_has_tsc?"TSC":"PIT")
static unsigned int get_time_pit(void)
{
diff -Nru a/drivers/input/misc/pcspkr.c b/drivers/input/misc/pcspkr.c
--- a/drivers/input/misc/pcspkr.c Sat Jun 14 22:23:32 2003
+++ b/drivers/input/misc/pcspkr.c Sat Jun 14 22:23:32 2003
@@ -43,7 +43,7 @@
}

if (value > 20 && value < 32767)
- count = 1193182 / value;
+ count = CLOCK_TICK_RATE / value;

spin_lock_irqsave(&i8253_beep_lock, flags);

diff -Nru a/include/asm-i386/timex.h b/include/asm-i386/timex.h
--- a/include/asm-i386/timex.h Sat Jun 14 22:23:32 2003
+++ b/include/asm-i386/timex.h Sat Jun 14 22:23:32 2003
@@ -15,7 +15,7 @@
#ifdef CONFIG_MELAN
# define CLOCK_TICK_RATE 1189200 /* AMD Elan has different frequency! */
#else
-# define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
+# define CLOCK_TICK_RATE 1193182 /* Underlying HZ */
#endif
#endif

diff -Nru a/include/asm-x86_64/timex.h b/include/asm-x86_64/timex.h
--- a/include/asm-x86_64/timex.h Sat Jun 14 22:23:32 2003
+++ b/include/asm-x86_64/timex.h Sat Jun 14 22:23:32 2003
@@ -10,7 +10,7 @@
#include <asm/msr.h>
#include <asm/vsyscall.h>

-#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
+#define CLOCK_TICK_RATE 1193182 /* Underlying HZ */
#define CLOCK_TICK_FACTOR 20 /* Factor of both 1000000 and CLOCK_TICK_RATE */
#define FINETUNE ((((((int)LATCH * HZ - CLOCK_TICK_RATE) << SHIFT_HZ) * \
(1000000/CLOCK_TICK_FACTOR) / (CLOCK_TICK_RATE/CLOCK_TICK_FACTOR)) \

2003-06-14 20:28:55

by Vojtech Pavlik

[permalink] [raw]
Subject: [patch] input: Add keys for HP HIL [7/13]


You can pull this changeset from:
bk://kernel.bkbits.net/vojtech/input

===================================================================

[email protected], 2003-06-09 14:09:58+02:00, [email protected]
input: Add key definitions for HP-HIL keyboards.


input.h | 5 +++++
1 files changed, 5 insertions(+)

===================================================================

diff -Nru a/include/linux/input.h b/include/linux/input.h
--- a/include/linux/input.h Sat Jun 14 22:23:15 2003
+++ b/include/linux/input.h Sat Jun 14 22:23:15 2003
@@ -473,6 +473,11 @@
#define KEY_TEEN 0x19e
#define KEY_TWEN 0x19f

+#define KEY_DEL_EOL 0x1c0
+#define KEY_DEL_EOS 0x1c1
+#define KEY_INS_LINE 0x1c2
+#define KEY_DEL_LINE 0x1c3
+
#define KEY_MAX 0x1ff

/*

2003-06-14 20:32:57

by Vojtech Pavlik

[permalink] [raw]
Subject: [patch] input: Fix hiddev_ioctl() [11/13]


You can pull this changeset from:
bk://kernel.bkbits.net/vojtech/input

===================================================================

[email protected], 2003-06-11 16:58:41+02:00, [email protected]
hid: Add missing 'return 0's in hiddev ioctl handler.


hiddev.c | 10 ++++++++++
1 files changed, 10 insertions(+)

===================================================================

diff -Nru a/drivers/usb/input/hiddev.c b/drivers/usb/input/hiddev.c
--- a/drivers/usb/input/hiddev.c Sat Jun 14 22:24:10 2003
+++ b/drivers/usb/input/hiddev.c Sat Jun 14 22:24:10 2003
@@ -442,10 +442,14 @@
if (copy_to_user((void *) arg, &dinfo, sizeof(dinfo)))
return -EFAULT;

+ return 0;
+
case HIDIOCGFLAG:
if (put_user(list->flags, (int *) arg))
return -EFAULT;

+ return 0;
+
case HIDIOCSFLAG:
{
int newflags;
@@ -533,6 +537,8 @@
if (copy_to_user((void *) arg, &rinfo, sizeof(rinfo)))
return -EFAULT;

+ return 0;
+
case HIDIOCGFIELDINFO:
if (copy_from_user(&finfo, (void *) arg, sizeof(finfo)))
return -EFAULT;
@@ -564,6 +570,8 @@
if (copy_to_user((void *) arg, &finfo, sizeof(finfo)))
return -EFAULT;

+ return 0;
+
case HIDIOCGUCODE:
if (copy_from_user(&uref, (void *) arg, sizeof(uref)))
return -EFAULT;
@@ -584,6 +592,8 @@

if (copy_to_user((void *) arg, &uref, sizeof(uref)))
return -EFAULT;
+
+ return 0;

case HIDIOCGUSAGE:
case HIDIOCSUSAGE:

2003-06-14 20:32:58

by Vojtech Pavlik

[permalink] [raw]
Subject: [patch] input: Fix i8042 interrupts on I2000 ia64 machines [9/13]


You can pull this changeset from:
bk://kernel.bkbits.net/vojtech/input

===================================================================

[email protected], 2003-06-09 14:54:00+02:00, [email protected]
input: The appended fix is needed on I2000 machines, to map the
legacy ISA interrupt onto the actual interrupt provided. Otherwise
the mouse and keyboard won't work. Patch against 2.5.70.


i8042-io.h | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)

===================================================================

diff -Nru a/drivers/input/serio/i8042-io.h b/drivers/input/serio/i8042-io.h
--- a/drivers/input/serio/i8042-io.h Sat Jun 14 22:23:45 2003
+++ b/drivers/input/serio/i8042-io.h Sat Jun 14 22:23:45 2003
@@ -20,11 +20,14 @@
*/

#ifdef __alpha__
-#define I8042_KBD_IRQ 1
-#define I8042_AUX_IRQ (RTC_PORT(0) == 0x170 ? 9 : 12) /* Jensen is special */
+# define I8042_KBD_IRQ 1
+# define I8042_AUX_IRQ (RTC_PORT(0) == 0x170 ? 9 : 12) /* Jensen is special */
+#elif defined(__ia64__)
+# define I8042_KBD_IRQ isa_irq_to_vector(1)
+# define I8042_AUX_IRQ isa_irq_to_vector(12)
#else
-#define I8042_KBD_IRQ 1
-#define I8042_AUX_IRQ 12
+# define I8042_KBD_IRQ 1
+# define I8042_AUX_IRQ 12
#endif

/*

2003-06-14 20:35:55

by Vojtech Pavlik

[permalink] [raw]
Subject: [patch] input: Fix minor errors in input-programming.txt [12/13]


You can pull this changeset from:
bk://kernel.bkbits.net/vojtech/input

===================================================================

[email protected], 2003-06-14 14:18:59+02:00, [email protected]
input: fix some minor errors found in the input-programming.txt file


input-programming.txt | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)

===================================================================

diff -Nru a/Documentation/input/input-programming.txt b/Documentation/input/input-programming.txt
--- a/Documentation/input/input-programming.txt Sat Jun 14 22:24:22 2003
+++ b/Documentation/input/input-programming.txt Sat Jun 14 22:24:22 2003
@@ -146,7 +146,7 @@
Note the button_used variable - we have to track how many times the open
function was called to know when exactly our device stops being used.

-The open() callback should return a 0 in case of succes or any nonzero value
+The open() callback should return a 0 in case of success or any nonzero value
in case of failure. The close() callback (which is void) must always succeed.

1.3 Basic event types
@@ -178,7 +178,7 @@
function. Events are generated only for nonzero value.

However EV_ABS requires a little special care. Before calling
-input_register_devices, you have to fill additional fields in the input_dev
+input_register_device, you have to fill additional fields in the input_dev
struct for each absolute axis your device has. If our button device had also
the ABS_X axis:

@@ -207,11 +207,11 @@
1.5 NBITS(), LONG(), BIT()
~~~~~~~~~~~~~~~~~~~~~~~~~~

-These three macros frin input.h help some bitfield computations:
+These three macros from input.h help some bitfield computations:

NBITS(x) - returns the length of a bitfield array in longs for x bits
LONG(x) - returns the index in the array in longs for bit x
- BIT(x) - returns the indes in a long for bit x
+ BIT(x) - returns the index in a long for bit x

1.6 The number, id* and name fields
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -221,7 +221,7 @@
in system messages.

The dev->name should be set before registering the input device by the input
-device driver. It's a string like 'Generic button device' containing an
+device driver. It's a string like 'Generic button device' containing a
user friendly name of the device.

The id* fields contain the bus ID (PCI, USB, ...), vendor ID and device ID
@@ -237,7 +237,7 @@
1.7 The keycode, keycodemax, keycodesize fields
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

-These two fields will be used for any inpur devices that report their data
+These two fields will be used for any input devices that report their data
as scancodes. If not all scancodes can be known by autodetection, they may
need to be set by userland utilities. The keycode array then is an array
used to map from scancodes to input system keycodes. The keycode max will
@@ -258,7 +258,7 @@

The other event types up to now are:

-EV_LED - used for the keyboad LEDs.
+EV_LED - used for the keyboard LEDs.
EV_SND - used for keyboard beeps.

They are very similar to for example key events, but they go in the other
@@ -270,7 +270,7 @@

int button_event(struct input_dev *dev, unsigned int type, unsigned int code, int value);
{
- if (type == EV_SND && code == EV_BELL) {
+ if (type == EV_SND && code == SND_BELL) {
outb(value, BUTTON_BELL);
return 0;
}

2003-06-14 20:36:32

by Vojtech Pavlik

[permalink] [raw]
Subject: [patch] input: Add Synaptics touchpad support [13/13]


You can pull this changeset from:
bk://kernel.bkbits.net/vojtech/input

===================================================================

[email protected], 2003-06-14 18:33:19+02:00, [email protected]
input: Add Synaptics touchpad absolute mode support.


b/drivers/input/mouse/Kconfig | 13
b/drivers/input/mouse/Makefile | 5
b/drivers/input/mouse/psmouse-base.c | 661 +++++++++++++++++++++++++++++++++
b/drivers/input/mouse/psmouse.h | 49 ++
b/drivers/input/mouse/synaptics.c | 390 +++++++++++++++++++
b/drivers/input/mouse/synaptics.h | 105 +++++
b/include/linux/input.h | 1
drivers/input/mouse/psmouse.c | 689 -----------------------------------
8 files changed, 1224 insertions(+), 689 deletions(-)

===================================================================

diff -Nru a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
--- a/drivers/input/mouse/Kconfig Sat Jun 14 22:24:35 2003
+++ b/drivers/input/mouse/Kconfig Sat Jun 14 22:24:35 2003
@@ -28,6 +28,19 @@
The module will be called psmouse. If you want to compile it as a
module, say M here and read <file:Documentation/modules.txt>.

+config MOUSE_PS2_SYNAPTICS
+ bool "Synaptics TouchPad"
+ default n
+ depends on INPUT && INPUT_MOUSE && SERIO && MOUSE_PS2
+ ---help---
+ Say Y here if you have a Synaptics TouchPad connected to your system.
+ This touchpad is found on many modern laptop computers.
+ Note that you also need a user space driver to interpret the data
+ generated by the kernel. A compatible driver for XFree86 is available
+ from http://...
+
+ If unsure, say Y.
+
config MOUSE_SERIAL
tristate "Serial mouse"
depends on INPUT && INPUT_MOUSE && SERIO
diff -Nru a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
--- a/drivers/input/mouse/Makefile Sat Jun 14 22:24:35 2003
+++ b/drivers/input/mouse/Makefile Sat Jun 14 22:24:35 2003
@@ -13,3 +13,8 @@
obj-$(CONFIG_MOUSE_PC9800) += 98busmouse.o
obj-$(CONFIG_MOUSE_PS2) += psmouse.o
obj-$(CONFIG_MOUSE_SERIAL) += sermouse.o
+
+psmouse-objs := psmouse-base.o
+ifeq ($(CONFIG_MOUSE_PS2_SYNAPTICS),y)
+ psmouse-objs += synaptics.o
+endif
diff -Nru a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/input/mouse/psmouse-base.c Sat Jun 14 22:24:35 2003
[snip]
+ if (psmouse->pktcnt == 1 && psmouse->type == PSMOUSE_SYNAPTICS) {
+ /*
+ * The synaptics driver has its own resync logic,
+ * so it needs to receive all bytes one at a time.
+ */
+ synaptics_process_byte(psmouse, regs);
+ psmouse->pktcnt = 0;
+ goto out;
+ }
[snip]
+/*
+ * Try Synaptics TouchPad magic ID
+ */
+
+ param[0] = 0;
+ psmouse_command(psmouse, param, PSMOUSE_CMD_SETRES);
+ psmouse_command(psmouse, param, PSMOUSE_CMD_SETRES);
+ psmouse_command(psmouse, param, PSMOUSE_CMD_SETRES);
+ psmouse_command(psmouse, param, PSMOUSE_CMD_SETRES);
+ psmouse_command(psmouse, param, PSMOUSE_CMD_GETINFO);
+
+ if (param[1] == 0x47) {
+ psmouse->vendor = "Synaptics";
+ psmouse->name = "TouchPad";
+ if (!synaptics_init(psmouse))
+ return PSMOUSE_SYNAPTICS;
+ else
+ return PSMOUSE_PS2;
+ }
diff -Nru a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/input/mouse/psmouse.h Sat Jun 14 22:24:35 2003
@@ -0,0 +1,49 @@
+#ifndef _PSMOUSE_H
+#define _PSMOUSE_H
+
+#define PSMOUSE_CMD_SETSCALE11 0x00e6
+#define PSMOUSE_CMD_SETRES 0x10e8
+#define PSMOUSE_CMD_GETINFO 0x03e9
+#define PSMOUSE_CMD_SETSTREAM 0x00ea
+#define PSMOUSE_CMD_POLL 0x03eb
+#define PSMOUSE_CMD_GETID 0x02f2
+#define PSMOUSE_CMD_SETRATE 0x10f3
+#define PSMOUSE_CMD_ENABLE 0x00f4
+#define PSMOUSE_CMD_RESET_DIS 0x00f6
+#define PSMOUSE_CMD_RESET_BAT 0x02ff
+
+#define PSMOUSE_RET_BAT 0xaa
+#define PSMOUSE_RET_ACK 0xfa
+#define PSMOUSE_RET_NAK 0xfe
+
+struct psmouse {
+ void *private;
+ struct input_dev dev;
+ struct serio *serio;
+ char *vendor;
+ char *name;
+ unsigned char cmdbuf[8];
+ unsigned char packet[8];
+ unsigned char cmdcnt;
+ unsigned char pktcnt;
+ unsigned char type;
+ unsigned char model;
+ unsigned long last;
+ char acking;
+ volatile char ack;
+ char error;
+ char devname[64];
+ char phys[32];
+};
+
+#define PSMOUSE_PS2 1
+#define PSMOUSE_PS2PP 2
+#define PSMOUSE_PS2TPP 3
+#define PSMOUSE_GENPS 4
+#define PSMOUSE_IMPS 5
+#define PSMOUSE_IMEX 6
+#define PSMOUSE_SYNAPTICS 7
+
+int psmouse_command(struct psmouse *psmouse, unsigned char *param, int command);
+
+#endif /* _PSMOUSE_H */
diff -Nru a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/input/mouse/synaptics.c Sat Jun 14 22:24:35 2003
@@ -0,0 +1,390 @@
+/*
+ * Synaptics TouchPad PS/2 mouse driver
+ *
+ * 2003 Peter Osterlund <[email protected]>
+ * Ported to 2.5 input device infrastructure.
+ *
+ * Copyright (C) 2001 Stefan Gmeiner <[email protected]>
+ * start merging tpconfig and gpm code to a xfree-input module
+ * adding some changes and extensions (ex. 3rd and 4th button)
+ *
+ * Copyright (c) 1997 C. Scott Ananian <[email protected]>
+ * Copyright (c) 1998-2000 Bruce Kalk <[email protected]>
+ * code for the special synaptics commands (from the tpconfig-source)
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * Trademarks are the property of their respective owners.
+ */
+
+#include <linux/module.h>
+#include <linux/input.h>
+#include "psmouse.h"
+#include "synaptics.h"
+
+/*****************************************************************************
+ * Synaptics communications functions
+ ****************************************************************************/
+
+/*
+ * Use the Synaptics extended ps/2 syntax to write a special command byte.
+ * special command: 0xE8 rr 0xE8 ss 0xE8 tt 0xE8 uu where (rr*64)+(ss*16)+(tt*4)+uu
+ * is the command. A 0xF3 or 0xE9 must follow (see synaptics_send_cmd
+ * and synaptics_set_mode)
+ */
+static int synaptics_special_cmd(struct psmouse *psmouse, unsigned char command)
+{
+ int i;
+
+ if (psmouse_command(psmouse, NULL, PSMOUSE_CMD_SETSCALE11))
+ return -1;
+
+ for (i = 6; i >= 0; i -= 2) {
+ unsigned char d = (command >> i) & 3;
+ if (psmouse_command(psmouse, &d, PSMOUSE_CMD_SETRES))
+ return -1;
+ }
+
+ return 0;
+}
+
+/*
+ * Send a command to the synpatics touchpad by special commands
+ */
+static int synaptics_send_cmd(struct psmouse *psmouse, unsigned char c, unsigned char *param)
+{
+ if (synaptics_special_cmd(psmouse, c))
+ return -1;
+ if (psmouse_command(psmouse, param, PSMOUSE_CMD_GETINFO))
+ return -1;
+ return 0;
+}
+
+/*
+ * Set the synaptics touchpad mode byte by special commands
+ */
+static int synaptics_set_mode(struct psmouse *psmouse, unsigned char mode)
+{
+ unsigned char param[1];
+
+ if (synaptics_special_cmd(psmouse, mode))
+ return -1;
+ param[0] = 0x14;
+ if (psmouse_command(psmouse, param, PSMOUSE_CMD_SETRATE))
+ return -1;
+ return 0;
+}
+
+static int synaptics_reset(struct psmouse *psmouse)
+{
+ unsigned char r[2];
+
+ if (psmouse_command(psmouse, r, PSMOUSE_CMD_RESET_BAT))
+ return -1;
+ if (r[0] == 0xAA && r[1] == 0x00)
+ return 0;
+ return -1;
+}
+
+/*
+ * Read the model-id bytes from the touchpad
+ * see also SYN_MODEL_* macros
+ */
+static int synaptics_model_id(struct psmouse *psmouse, unsigned long int *model_id)
+{
+ unsigned char mi[3];
+
+ if (synaptics_send_cmd(psmouse, SYN_QUE_MODEL, mi))
+ return -1;
+ *model_id = (mi[0]<<16) | (mi[1]<<8) | mi[2];
+ return 0;
+}
+
+/*
+ * Read the capability-bits from the touchpad
+ * see also the SYN_CAP_* macros
+ */
+static int synaptics_capability(struct psmouse *psmouse, unsigned long int *capability)
+{
+ unsigned char cap[3];
+
+ if (synaptics_send_cmd(psmouse, SYN_QUE_CAPABILITIES, cap))
+ return -1;
+ *capability = (cap[0]<<16) | (cap[1]<<8) | cap[2];
+ if (SYN_CAP_VALID(*capability))
+ return 0;
+ return -1;
+}
+
+/*
+ * Identify Touchpad
+ * See also the SYN_ID_* macros
+ */
+static int synaptics_identify(struct psmouse *psmouse, unsigned long int *ident)
+{
+ unsigned char id[3];
+
+ if (synaptics_send_cmd(psmouse, SYN_QUE_IDENTIFY, id))
+ return -1;
+ *ident = (id[0]<<16) | (id[1]<<8) | id[2];
+ if (SYN_ID_IS_SYNAPTICS(*ident))
+ return 0;
+ return -1;
+}
+
+static int synaptics_enable_device(struct psmouse *psmouse)
+{
+ if (psmouse_command(psmouse, NULL, PSMOUSE_CMD_ENABLE))
+ return -1;
+ return 0;
+}
+
+static void print_ident(struct synaptics_data *priv)
+{
+ printk(KERN_INFO "Synaptics Touchpad, model: %ld\n", SYN_ID_MODEL(priv->identity));
+ printk(KERN_INFO " Firware: %ld.%ld\n", SYN_ID_MAJOR(priv->identity),
+ SYN_ID_MINOR(priv->identity));
+
+ if (SYN_MODEL_ROT180(priv->model_id))
+ printk(KERN_INFO " 180 degree mounted touchpad\n");
+ if (SYN_MODEL_PORTRAIT(priv->model_id))
+ printk(KERN_INFO " portrait touchpad\n");
+ printk(KERN_INFO " Sensor: %ld\n", SYN_MODEL_SENSOR(priv->model_id));
+ if (SYN_MODEL_NEWABS(priv->model_id))
+ printk(KERN_INFO " new absolute packet format\n");
+ if (SYN_MODEL_PEN(priv->model_id))
+ printk(KERN_INFO " pen detection\n");
+
+ if (SYN_CAP_EXTENDED(priv->capabilities)) {
+ printk(KERN_INFO " Touchpad has extended capability bits\n");
+ if (SYN_CAP_FOUR_BUTTON(priv->capabilities))
+ printk(KERN_INFO " -> four buttons\n");
+ if (SYN_CAP_MULTIFINGER(priv->capabilities))
+ printk(KERN_INFO " -> multifinger detection\n");
+ if (SYN_CAP_PALMDETECT(priv->capabilities))
+ printk(KERN_INFO " -> palm detection\n");
+ }
+}
+
+static int query_hardware(struct psmouse *psmouse)
+{
+ struct synaptics_data *priv = psmouse->private;
+ int retries = 3;
+
+ while ((retries++ <= 3) && synaptics_reset(psmouse))
+ printk(KERN_ERR "synaptics reset failed\n");
+
+ if (synaptics_identify(psmouse, &priv->identity))
+ return -1;
+ if (synaptics_model_id(psmouse, &priv->model_id))
+ return -1;
+ if (synaptics_capability(psmouse, &priv->capabilities))
+ return -1;
+ if (synaptics_set_mode(psmouse, (SYN_BIT_ABSOLUTE_MODE |
+ SYN_BIT_HIGH_RATE |
+ SYN_BIT_DISABLE_GESTURE |
+ SYN_BIT_W_MODE)))
+ return -1;
+
+ synaptics_enable_device(psmouse);
+
+ print_ident(priv);
+
+ return 0;
+}
+
+/*****************************************************************************
+ * Driver initialization/cleanup functions
+ ****************************************************************************/
+
+static inline void set_abs_params(struct input_dev *dev, int axis, int min, int max, int fuzz, int flat)
+{
+ dev->absmin[axis] = min;
+ dev->absmax[axis] = max;
+ dev->absfuzz[axis] = fuzz;
+ dev->absflat[axis] = flat;
+
+ set_bit(axis, dev->absbit);
+}
+
+int synaptics_init(struct psmouse *psmouse)
+{
+ struct synaptics_data *priv;
+
+ psmouse->private = priv = kmalloc(sizeof(struct synaptics_data), GFP_KERNEL);
+ if (!priv)
+ return -1;
+ memset(priv, 0, sizeof(struct synaptics_data));
+
+ priv->inSync = 1;
+
+ if (query_hardware(psmouse)) {
+ printk(KERN_ERR "Unable to query/initialize Synaptics hardware.\n");
+ goto init_fail;
+ }
+
+ /*
+ * The x/y limits are taken from the Synaptics TouchPad interfacing Guide,
+ * which says that they should be valid regardless of the actual size of
+ * the senser.
+ */
+ set_bit(EV_ABS, psmouse->dev.evbit);
+ set_abs_params(&psmouse->dev, ABS_X, 1472, 5472, 0, 0);
+ set_abs_params(&psmouse->dev, ABS_Y, 1408, 4448, 0, 0);
+ set_abs_params(&psmouse->dev, ABS_PRESSURE, 0, 255, 0, 0);
+
+ set_bit(EV_MSC, psmouse->dev.evbit);
+ set_bit(MSC_GESTURE, psmouse->dev.mscbit);
+
+ set_bit(EV_KEY, psmouse->dev.evbit);
+ set_bit(BTN_LEFT, psmouse->dev.keybit);
+ set_bit(BTN_RIGHT, psmouse->dev.keybit);
+ set_bit(BTN_FORWARD, psmouse->dev.keybit);
+ set_bit(BTN_BACK, psmouse->dev.keybit);
+
+ clear_bit(EV_REL, psmouse->dev.evbit);
+ clear_bit(REL_X, psmouse->dev.relbit);
+ clear_bit(REL_Y, psmouse->dev.relbit);
+
+ return 0;
+
+ init_fail:
+ kfree(priv);
+ return -1;
+}
+
+void synaptics_disconnect(struct psmouse *psmouse)
+{
+ struct synaptics_data *priv = psmouse->private;
+
+ kfree(priv);
+}
+
+/*****************************************************************************
+ * Functions to interpret the absolute mode packets
+ ****************************************************************************/
+
+static void synaptics_parse_hw_state(struct synaptics_data *priv,
+ struct synaptics_hw_state *hw)
+{
+ unsigned char *buf = priv->proto_buf;
+
+ hw->x = (((buf[3] & 0x10) << 8) |
+ ((buf[1] & 0x0f) << 8) |
+ buf[4]);
+ hw->y = (((buf[3] & 0x20) << 7) |
+ ((buf[1] & 0xf0) << 4) |
+ buf[5]);
+
+ hw->z = buf[2];
+ hw->w = (((buf[0] & 0x30) >> 2) |
+ ((buf[0] & 0x04) >> 1) |
+ ((buf[3] & 0x04) >> 2));
+
+ hw->left = (buf[0] & 0x01) ? 1 : 0;
+ hw->right = (buf[0] & 0x2) ? 1 : 0;
+ hw->up = 0;
+ hw->down = 0;
+
+ if (SYN_CAP_EXTENDED(priv->capabilities) &&
+ (SYN_CAP_FOUR_BUTTON(priv->capabilities))) {
+ hw->up = ((buf[3] & 0x01)) ? 1 : 0;
+ if (hw->left)
+ hw->up = !hw->up;
+ hw->down = ((buf[3] & 0x02)) ? 1 : 0;
+ if (hw->right)
+ hw->down = !hw->down;
+ }
+}
+
+/*
+ * called for each full received packet from the touchpad
+ */
+static void synaptics_process_packet(struct psmouse *psmouse)
+{
+ struct input_dev *dev = &psmouse->dev;
+ struct synaptics_data *priv = psmouse->private;
+ struct synaptics_hw_state hw;
+
+ synaptics_parse_hw_state(priv, &hw);
+
+ if (hw.z > 0) {
+ int w_ok = 0;
+ /*
+ * Use capability bits to decide if the w value is valid.
+ * If not, set it to 5, which corresponds to a finger of
+ * normal width.
+ */
+ if (SYN_CAP_EXTENDED(priv->capabilities)) {
+ switch (hw.w) {
+ case 0 ... 1:
+ w_ok = SYN_CAP_MULTIFINGER(priv->capabilities);
+ break;
+ case 2:
+ w_ok = SYN_MODEL_PEN(priv->model_id);
+ break;
+ case 4 ... 15:
+ w_ok = SYN_CAP_PALMDETECT(priv->capabilities);
+ break;
+ }
+ }
+ if (!w_ok)
+ hw.w = 5;
+ }
+
+ /* Post events */
+ input_report_abs(dev, ABS_X, hw.x);
+ input_report_abs(dev, ABS_Y, hw.y);
+ input_report_abs(dev, ABS_PRESSURE, hw.z);
+
+ if (hw.w != priv->old_w) {
+ input_event(dev, EV_MSC, MSC_GESTURE, hw.w);
+ priv->old_w = hw.w;
+ }
+
+ input_report_key(dev, BTN_LEFT, hw.left);
+ input_report_key(dev, BTN_RIGHT, hw.right);
+ input_report_key(dev, BTN_FORWARD, hw.up);
+ input_report_key(dev, BTN_BACK, hw.down);
+
+ input_sync(dev);
+}
+
+void synaptics_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
+{
+ struct input_dev *dev = &psmouse->dev;
+ struct synaptics_data *priv = psmouse->private;
+ unsigned char *pBuf = priv->proto_buf;
+ unsigned char u = psmouse->packet[0];
+
+ input_regs(dev, regs);
+
+ pBuf[priv->proto_buf_tail++] = u;
+
+ /* check first byte */
+ if ((priv->proto_buf_tail == 1) && ((u & 0xC8) != 0x80)) {
+ priv->inSync = 0;
+ priv->proto_buf_tail = 0;
+ printk(KERN_WARNING "Synaptics driver lost sync at 1st byte\n");
+ return;
+ }
+
+ /* check 4th byte */
+ if ((priv->proto_buf_tail == 4) && ((u & 0xc8) != 0xc0)) {
+ priv->inSync = 0;
+ priv->proto_buf_tail = 0;
+ printk(KERN_WARNING "Synaptics driver lost sync at 4th byte\n");
+ return;
+ }
+
+ if (priv->proto_buf_tail >= 6) { /* Full packet received */
+ if (!priv->inSync) {
+ priv->inSync = 1;
+ printk(KERN_NOTICE "Synaptics driver resynced.\n");
+ }
+ synaptics_process_packet(psmouse);
+ priv->proto_buf_tail = 0;
+ }
+}
diff -Nru a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
--- /dev/null Wed Dec 31 16:00:00 1969
+++ b/drivers/input/mouse/synaptics.h Sat Jun 14 22:24:35 2003
@@ -0,0 +1,105 @@
+/*
+ * Synaptics TouchPad PS/2 mouse driver
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef _SYNAPTICS_H
+#define _SYNAPTICS_H
+
+#ifdef CONFIG_MOUSE_PS2_SYNAPTICS
+
+extern void synaptics_process_byte(struct psmouse *psmouse, struct pt_regs *regs);
+extern int synaptics_init(struct psmouse *psmouse);
+extern void synaptics_disconnect(struct psmouse *psmouse);
+
+#else
+
+static inline void synaptics_process_byte(struct psmouse *psmouse, struct pt_regs *regs) {}
+static inline int synaptics_init(struct psmouse *psmouse) { return -1; }
+static inline void synaptics_disconnect(struct psmouse *psmouse) {}
+
+#endif
+
+
+/* synaptics queries */
+#define SYN_QUE_IDENTIFY 0x00
+#define SYN_QUE_MODES 0x01
+#define SYN_QUE_CAPABILITIES 0x02
+#define SYN_QUE_MODEL 0x03
+#define SYN_QUE_SERIAL_NUMBER_PREFIX 0x06
+#define SYN_QUE_SERIAL_NUMBER_SUFFIX 0x07
+#define SYN_QUE_RESOLUTION 0x08
+
+/* synatics modes */
+#define SYN_BIT_ABSOLUTE_MODE (1 << 7)
+#define SYN_BIT_HIGH_RATE (1 << 6)
+#define SYN_BIT_SLEEP_MODE (1 << 3)
+#define SYN_BIT_DISABLE_GESTURE (1 << 2)
+#define SYN_BIT_W_MODE (1 << 0)
+
+/* synaptics model ID bits */
+#define SYN_MODEL_ROT180(m) ((m) & (1 << 23))
+#define SYN_MODEL_PORTRAIT(m) ((m) & (1 << 22))
+#define SYN_MODEL_SENSOR(m) (((m) >> 16) & 0x3f)
+#define SYN_MODEL_HARDWARE(m) (((m) >> 9) & 0x7f)
+#define SYN_MODEL_NEWABS(m) ((m) & (1 << 7))
+#define SYN_MODEL_PEN(m) ((m) & (1 << 6))
+#define SYN_MODEL_SIMPLIC(m) ((m) & (1 << 5))
+#define SYN_MODEL_GEOMETRY(m) ((m) & 0x0f)
+
+/* synaptics capability bits */
+#define SYN_CAP_EXTENDED(c) ((c) & (1 << 23))
+#define SYN_CAP_SLEEP(c) ((c) & (1 << 4))
+#define SYN_CAP_FOUR_BUTTON(c) ((c) & (1 << 3))
+#define SYN_CAP_MULTIFINGER(c) ((c) & (1 << 1))
+#define SYN_CAP_PALMDETECT(c) ((c) & (1 << 0))
+#define SYN_CAP_VALID(c) ((((c) & 0x00ff00) >> 8) == 0x47)
+
+/* synaptics modes query bits */
+#define SYN_MODE_ABSOLUTE(m) ((m) & (1 << 7))
+#define SYN_MODE_RATE(m) ((m) & (1 << 6))
+#define SYN_MODE_BAUD_SLEEP(m) ((m) & (1 << 3))
+#define SYN_MODE_DISABLE_GESTURE(m) ((m) & (1 << 2))
+#define SYN_MODE_PACKSIZE(m) ((m) & (1 << 1))
+#define SYN_MODE_WMODE(m) ((m) & (1 << 0))
+
+/* synaptics identify query bits */
+#define SYN_ID_MODEL(i) (((i) >> 4) & 0x0f)
+#define SYN_ID_MAJOR(i) ((i) & 0x0f)
+#define SYN_ID_MINOR(i) (((i) >> 16) & 0xff)
+#define SYN_ID_IS_SYNAPTICS(i) ((((i) >> 8) & 0xff) == 0x47)
+
+/*
+ * A structure to describe the state of the touchpad hardware (buttons and pad)
+ */
+
+struct synaptics_hw_state {
+ int x;
+ int y;
+ int z;
+ int w;
+ int left;
+ int right;
+ int up;
+ int down;
+};
+
+struct synaptics_data {
+ /* Data read from the touchpad */
+ unsigned long int model_id; /* Model-ID */
+ unsigned long int capabilities; /* Capabilities */
+ unsigned long int identity; /* Identification */
+
+ /* Data for normal processing */
+ unsigned char proto_buf[6]; /* Buffer for Packet */
+ unsigned char last_byte; /* last received byte */
+ int inSync; /* Packets in sync */
+ int proto_buf_tail;
+
+ int old_w; /* Previous w value */
+};
+
+#endif /* _SYNAPTICS_H */
diff -Nru a/include/linux/input.h b/include/linux/input.h
--- a/include/linux/input.h Sat Jun 14 22:24:35 2003
+++ b/include/linux/input.h Sat Jun 14 22:24:35 2003
@@ -530,6 +530,7 @@

#define MSC_SERIAL 0x00
#define MSC_PULSELED 0x01
+#define MSC_GESTURE 0x02
#define MSC_MAX 0x07

/*

2003-06-14 20:32:58

by Vojtech Pavlik

[permalink] [raw]
Subject: [patch] input: Fix sending reports in USB HID [10/13]


You can pull this changeset from:
bk://kernel.bkbits.net/vojtech/input

===================================================================

[email protected], 2003-06-11 16:57:39+02:00, [email protected]
hid: fix HID feature/output report writing to devices. This should
fix most problems with UPS shutdown.


hid-core.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)

===================================================================

diff -Nru a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c
--- a/drivers/usb/input/hid-core.c Sat Jun 14 22:23:57 2003
+++ b/drivers/usb/input/hid-core.c Sat Jun 14 22:23:57 2003
@@ -957,6 +957,10 @@
void hid_output_report(struct hid_report *report, __u8 *data)
{
unsigned n;
+
+ if (report->id > 0)
+ *data++ = report->id;
+
for (n = 0; n < report->maxfield; n++)
hid_output_field(report->field[n], data);
}
@@ -1051,7 +1055,7 @@
report = hid->out[hid->outtail];

hid_output_report(report, hid->outbuf);
- hid->urbout->transfer_buffer_length = ((report->size - 1) >> 3) + 1;
+ hid->urbout->transfer_buffer_length = ((report->size - 1) >> 3) + 1 + (report->id > 0);
hid->urbout->dev = hid->dev;

dbg("submitting out urb");
@@ -1075,7 +1079,7 @@
if (dir == USB_DIR_OUT)
hid_output_report(report, hid->ctrlbuf);

- hid->urbctrl->transfer_buffer_length = ((report->size - 1) >> 3) + 1 + ((report->id > 0) && (dir != USB_DIR_OUT));
+ hid->urbctrl->transfer_buffer_length = ((report->size - 1) >> 3) + 1 + (report->id > 0);
hid->urbctrl->pipe = (dir == USB_DIR_OUT) ? usb_sndctrlpipe(hid->dev, 0) : usb_rcvctrlpipe(hid->dev, 0);
hid->urbctrl->dev = hid->dev;

2003-06-14 20:39:44

by Oliver Neukum

[permalink] [raw]
Subject: Re: [patch] input: Add PCI PS/2 controller support [5/13]


> +static int pcips2_write(struct serio *io, unsigned char val)
> +{
> + struct pcips2_data *ps2if = io->driver;
> + unsigned int stat;
> +
> + do {
> + stat = inb(ps2if->base + PS2_STATUS);
> + cpu_relax();
> + } while (!(stat & PS2_STAT_TXEMPTY));

What will happen if somebody unplugs the base station while this
is running?

Regards
Oliver

2003-06-14 20:50:38

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [patch] input: Add PCI PS/2 controller support [5/13]

On Sat, Jun 14, 2003 at 10:51:51PM +0200, Oliver Neukum wrote:

> > +static int pcips2_write(struct serio *io, unsigned char val)
> > +{
> > + struct pcips2_data *ps2if = io->driver;
> > + unsigned int stat;
> > +
> > + do {
> > + stat = inb(ps2if->base + PS2_STATUS);
> > + cpu_relax();
> > + } while (!(stat & PS2_STAT_TXEMPTY));
>
> What will happen if somebody unplugs the base station while this
> is running?

I suppose it will wait until you put the base station back. Russell, is
there any notification that the base is getting removed or do all the
loops need checking? I'd consider it not hotpluggable for now.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-06-14 20:50:32

by Russell King

[permalink] [raw]
Subject: Re: [patch] input: Add PCI PS/2 controller support [5/13]

On Sat, Jun 14, 2003 at 10:51:51PM +0200, Oliver Neukum wrote:
> > +static int pcips2_write(struct serio *io, unsigned char val)
> > +{
> > + struct pcips2_data *ps2if = io->driver;
> > + unsigned int stat;
> > +
> > + do {
> > + stat = inb(ps2if->base + PS2_STATUS);
> > + cpu_relax();
> > + } while (!(stat & PS2_STAT_TXEMPTY));
>
> What will happen if somebody unplugs the base station while this
> is running?

PCI guarantees that we'll read 0xff, which means we will not loop.

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2003-06-14 20:53:10

by Riley Williams

[permalink] [raw]
Subject: RE: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

Hi.

> [email protected], 2003-06-09 14:41:31+02:00, [email protected]
> input: Change input/misc/pcspkr.c to use CLOCK_TICK_RATE instead of
> a fixed value of 1193182. And change CLOCK_TICK_RATE and several
> usages of a fixed value 1193180 to a slightly more correct value
> of 1193182. (True freq is 1.193181818181...).

Is there any reason why you used CLOCK_TICK_RATE in some places and
1193182 in others ??? I can understand your using the number in the
definition of CLOCK_TICK_RATE but not in the other cases.

If I'm reading it correctly, the result is a collection of bugs on the
AMD ELAN system as that uses a different frequency (at least, according
to the last but one hunk in your patch)...

Best wishes from Riley.
---
* Nothing as pretty as a smile, nothing as ugly as a frown.



> drivers/char/vt_ioctl.c | 4 ++--
> drivers/input/gameport/gameport.c | 2 +-
> drivers/input/joystick/analog.c | 2 +-
> drivers/input/misc/pcspkr.c | 2 +-
> include/asm-i386/timex.h | 2 +-
> include/asm-x86_64/timex.h | 2 +-
> 6 files changed, 7 insertions(+), 7 deletions(-)
>
> ===================================================================
>
> diff -Nru a/drivers/char/vt_ioctl.c b/drivers/char/vt_ioctl.c
> --- a/drivers/char/vt_ioctl.c Sat Jun 14 22:23:32 2003
> +++ b/drivers/char/vt_ioctl.c Sat Jun 14 22:23:32 2003
> @@ -395,7 +395,7 @@
> if (!perm)
> return -EPERM;
> if (arg)
> - arg = 1193180 / arg;
> + arg = 1193182 / arg;
> kd_mksound(arg, 0);
> return 0;
>
> @@ -412,7 +412,7 @@
> ticks = HZ * ((arg >> 16) & 0xffff) / 1000;
> count = ticks ? (arg & 0xffff) : 0;
> if (count)
> - count = 1193180 / count;
> + count = 1193182 / count;
> kd_mksound(count, ticks);
> return 0;
> }
> diff -Nru a/drivers/input/gameport/gameport.c
> b/drivers/input/gameport/gameport.c
> --- a/drivers/input/gameport/gameport.c Sat Jun 14 22:23:32 2003
> +++ b/drivers/input/gameport/gameport.c Sat Jun 14 22:23:32 2003
> @@ -37,7 +37,7 @@
>
> #ifdef __i386__
>
> -#define DELTA(x,y) ((y)-(x)+((y)<(x)?1193180/HZ:0))
> +#define DELTA(x,y) ((y)-(x)+((y)<(x)?1193182/HZ:0))
> #define GET_TIME(x) do { x = get_time_pit(); } while (0)
>
> static unsigned int get_time_pit(void)
> diff -Nru a/drivers/input/joystick/analog.c
> b/drivers/input/joystick/analog.c
> --- a/drivers/input/joystick/analog.c Sat Jun 14 22:23:32 2003
> +++ b/drivers/input/joystick/analog.c Sat Jun 14 22:23:32 2003
> @@ -138,7 +138,7 @@
>
> #ifdef __i386__
> #define GET_TIME(x) do { if (cpu_has_tsc) rdtscl(x); else x =
get_time_pit(); } while (0)
> -#define DELTA(x,y)
(cpu_has_tsc?((y)-(x)):((x)-(y)+((x)<(y)?1193180L/HZ:0)))
> +#define DELTA(x,y)
(cpu_has_tsc?((y)-(x)):((x)-(y)+((x)<(y)?1193182L/HZ:0)))
> #define TIME_NAME (cpu_has_tsc?"TSC":"PIT")
> static unsigned int get_time_pit(void)
> {
> diff -Nru a/drivers/input/misc/pcspkr.c b/drivers/input/misc/pcspkr.c
> --- a/drivers/input/misc/pcspkr.c Sat Jun 14 22:23:32 2003
> +++ b/drivers/input/misc/pcspkr.c Sat Jun 14 22:23:32 2003
> @@ -43,7 +43,7 @@
> }
>
> if (value > 20 && value < 32767)
> - count = 1193182 / value;
> + count = CLOCK_TICK_RATE / value;
>
> spin_lock_irqsave(&i8253_beep_lock, flags);
>
> diff -Nru a/include/asm-i386/timex.h b/include/asm-i386/timex.h
> --- a/include/asm-i386/timex.h Sat Jun 14 22:23:32 2003
> +++ b/include/asm-i386/timex.h Sat Jun 14 22:23:32 2003
> @@ -15,7 +15,7 @@
> #ifdef CONFIG_MELAN
> # define CLOCK_TICK_RATE 1189200 /* AMD Elan has different frequency!
*/
> #else
> -# define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
> +# define CLOCK_TICK_RATE 1193182 /* Underlying HZ */
> #endif
> #endif
>
> diff -Nru a/include/asm-x86_64/timex.h b/include/asm-x86_64/timex.h
> --- a/include/asm-x86_64/timex.h Sat Jun 14 22:23:32 2003
> +++ b/include/asm-x86_64/timex.h Sat Jun 14 22:23:32 2003
> @@ -10,7 +10,7 @@
> #include <asm/msr.h>
> #include <asm/vsyscall.h>
>
> -#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
> +#define CLOCK_TICK_RATE 1193182 /* Underlying HZ */
> #define CLOCK_TICK_FACTOR 20 /* Factor of both 1000000 and
CLOCK_TICK_RATE */
> #define FINETUNE ((((((int)LATCH * HZ - CLOCK_TICK_RATE) << SHIFT_HZ) *
\
> (1000000/CLOCK_TICK_FACTOR) / (CLOCK_TICK_RATE/CLOCK_TICK_FACTOR)) \

---
Outgoing mail is certified Virus Free.
Checked by AVG anti-virus system (http://www.grisoft.com).
Version: 6.0.489 / Virus Database: 288 - Release Date: 10-Jun-2003

2003-06-14 21:01:16

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

On Sat, Jun 14, 2003 at 10:05:24PM +0100, Riley Williams wrote:
> Hi.
>
> > [email protected], 2003-06-09 14:41:31+02:00, [email protected]
> > input: Change input/misc/pcspkr.c to use CLOCK_TICK_RATE instead of
> > a fixed value of 1193182. And change CLOCK_TICK_RATE and several
> > usages of a fixed value 1193180 to a slightly more correct value
> > of 1193182. (True freq is 1.193181818181...).
>
> Is there any reason why you used CLOCK_TICK_RATE in some places and
> 1193182 in others ??? I can understand your using the number in the
> definition of CLOCK_TICK_RATE but not in the other cases.

I only changed the numbers from 1193180 to 1193182 in the patch.
The presence of the number instead of CLOCK_TICK_RATE in many drivers
is most likely a bug by itself, but that'll need to be addressed in a
different patch.

The only one place where I fixed it for now is the pcspkr.c driver,
since that is the one that actually started the whole thing.

> If I'm reading it correctly, the result is a collection of bugs on the
> AMD ELAN system as that uses a different frequency (at least, according
> to the last but one hunk in your patch)...

Care to send me a patch to fix this all completely and for once?

Anyone disagrees with changing all the instances of 1193180/1193182 to
CLOCK_TICK_RATE?

> Best wishes from Riley.
> ---
> * Nothing as pretty as a smile, nothing as ugly as a frown.
>
>
>
> > drivers/char/vt_ioctl.c | 4 ++--
> > drivers/input/gameport/gameport.c | 2 +-
> > drivers/input/joystick/analog.c | 2 +-
> > drivers/input/misc/pcspkr.c | 2 +-
> > include/asm-i386/timex.h | 2 +-
> > include/asm-x86_64/timex.h | 2 +-
> > 6 files changed, 7 insertions(+), 7 deletions(-)
> >
> > ===================================================================
> >
> > diff -Nru a/drivers/char/vt_ioctl.c b/drivers/char/vt_ioctl.c
> > --- a/drivers/char/vt_ioctl.c Sat Jun 14 22:23:32 2003
> > +++ b/drivers/char/vt_ioctl.c Sat Jun 14 22:23:32 2003
> > @@ -395,7 +395,7 @@
> > if (!perm)
> > return -EPERM;
> > if (arg)
> > - arg = 1193180 / arg;
> > + arg = 1193182 / arg;
> > kd_mksound(arg, 0);
> > return 0;
> >
> > @@ -412,7 +412,7 @@
> > ticks = HZ * ((arg >> 16) & 0xffff) / 1000;
> > count = ticks ? (arg & 0xffff) : 0;
> > if (count)
> > - count = 1193180 / count;
> > + count = 1193182 / count;
> > kd_mksound(count, ticks);
> > return 0;
> > }
> > diff -Nru a/drivers/input/gameport/gameport.c
> > b/drivers/input/gameport/gameport.c
> > --- a/drivers/input/gameport/gameport.c Sat Jun 14 22:23:32 2003
> > +++ b/drivers/input/gameport/gameport.c Sat Jun 14 22:23:32 2003
> > @@ -37,7 +37,7 @@
> >
> > #ifdef __i386__
> >
> > -#define DELTA(x,y) ((y)-(x)+((y)<(x)?1193180/HZ:0))
> > +#define DELTA(x,y) ((y)-(x)+((y)<(x)?1193182/HZ:0))
> > #define GET_TIME(x) do { x = get_time_pit(); } while (0)
> >
> > static unsigned int get_time_pit(void)
> > diff -Nru a/drivers/input/joystick/analog.c
> > b/drivers/input/joystick/analog.c
> > --- a/drivers/input/joystick/analog.c Sat Jun 14 22:23:32 2003
> > +++ b/drivers/input/joystick/analog.c Sat Jun 14 22:23:32 2003
> > @@ -138,7 +138,7 @@
> >
> > #ifdef __i386__
> > #define GET_TIME(x) do { if (cpu_has_tsc) rdtscl(x); else x =
> get_time_pit(); } while (0)
> > -#define DELTA(x,y)
> (cpu_has_tsc?((y)-(x)):((x)-(y)+((x)<(y)?1193180L/HZ:0)))
> > +#define DELTA(x,y)
> (cpu_has_tsc?((y)-(x)):((x)-(y)+((x)<(y)?1193182L/HZ:0)))
> > #define TIME_NAME (cpu_has_tsc?"TSC":"PIT")
> > static unsigned int get_time_pit(void)
> > {
> > diff -Nru a/drivers/input/misc/pcspkr.c b/drivers/input/misc/pcspkr.c
> > --- a/drivers/input/misc/pcspkr.c Sat Jun 14 22:23:32 2003
> > +++ b/drivers/input/misc/pcspkr.c Sat Jun 14 22:23:32 2003
> > @@ -43,7 +43,7 @@
> > }
> >
> > if (value > 20 && value < 32767)
> > - count = 1193182 / value;
> > + count = CLOCK_TICK_RATE / value;
> >
> > spin_lock_irqsave(&i8253_beep_lock, flags);
> >
> > diff -Nru a/include/asm-i386/timex.h b/include/asm-i386/timex.h
> > --- a/include/asm-i386/timex.h Sat Jun 14 22:23:32 2003
> > +++ b/include/asm-i386/timex.h Sat Jun 14 22:23:32 2003
> > @@ -15,7 +15,7 @@
> > #ifdef CONFIG_MELAN
> > # define CLOCK_TICK_RATE 1189200 /* AMD Elan has different frequency!
> */
> > #else
> > -# define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
> > +# define CLOCK_TICK_RATE 1193182 /* Underlying HZ */
> > #endif
> > #endif
> >
> > diff -Nru a/include/asm-x86_64/timex.h b/include/asm-x86_64/timex.h
> > --- a/include/asm-x86_64/timex.h Sat Jun 14 22:23:32 2003
> > +++ b/include/asm-x86_64/timex.h Sat Jun 14 22:23:32 2003
> > @@ -10,7 +10,7 @@
> > #include <asm/msr.h>
> > #include <asm/vsyscall.h>
> >
> > -#define CLOCK_TICK_RATE 1193180 /* Underlying HZ */
> > +#define CLOCK_TICK_RATE 1193182 /* Underlying HZ */
> > #define CLOCK_TICK_FACTOR 20 /* Factor of both 1000000 and
> CLOCK_TICK_RATE */
> > #define FINETUNE ((((((int)LATCH * HZ - CLOCK_TICK_RATE) << SHIFT_HZ) *
> \
> > (1000000/CLOCK_TICK_FACTOR) / (CLOCK_TICK_RATE/CLOCK_TICK_FACTOR)) \
>
> ---
> Outgoing mail is certified Virus Free.
> Checked by AVG anti-virus system (http://www.grisoft.com).
> Version: 6.0.489 / Virus Database: 288 - Release Date: 10-Jun-2003
>

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-06-15 10:37:15

by Riley Williams

[permalink] [raw]
Subject: RE: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

Hi.

I've taken Linus out of the CC list as he'll not want to see this until
it's all sorted out...

>>> [email protected], 2003-06-09 14:41:31+02:00, [email protected]
>>> input: Change input/misc/pcspkr.c to use CLOCK_TICK_RATE instead of
>>> a fixed value of 1193182. And change CLOCK_TICK_RATE and several
>>> usages of a fixed value 1193180 to a slightly more correct value
>>> of 1193182. (True freq is 1.193181818181...).

>> Is there any reason why you used CLOCK_TICK_RATE in some places and
>> 1193182 in others ??? I can understand your using the number in the
>> definition of CLOCK_TICK_RATE but not in the other cases.

> I only changed the numbers from 1193180 to 1193182 in the patch.
> The presence of the number instead of CLOCK_TICK_RATE in many drivers
> is most likely a bug by itself, but that'll need to be addressed in a
> different patch.
>
> The only one place where I fixed it for now is the pcspkr.c driver,
> since that is the one that actually started the whole thing.

>> If I'm reading it correctly, the result is a collection of bugs on the
>> AMD ELAN system as that uses a different frequency (at least, according
>> to the last but one hunk in your patch)...

> Care to send me a patch to fix this all completely and for once?

I'm not sure whether your patch was for the 2.4 or 2.5 kernels. Linus has
just released the 2.5.71 kernel which I haven't yet downloaded, but when
UI have, I'll produce a patch for that as well. Enclosed is the relevant
patch against the 2.4.21 raw kernel tree with comments here:

1. The asm-arm version of timex.h includes an arm-subarch header that
is presumably supposed to define the relevant CLOCK_TICK_RATE for
each sub-arch. However, some don't. I've included a catch-all in
timex.h that defines CLOCK_TICK_RATE as being the standard value
you've used if it isn't defined otherwise.

Note that with the exception of the catch-all I've introduced, the
various arm sub-arches all use values other than 1193182 here, so
this architecture may need further work.

2. The IA64 arch didn't define CLOCK_TICK_RATE at all, but then used the
1193182 value as a magic value in several files. I've inserted that
as the definition thereof in timex.h for that arch.

3. The PARISC version of timex.h didn't define CLOCK_TICK_RATE at all.
Other than the magic values in several generic files, it apparently
didn't use it either. I've defined it with the 1193182 value here.

This patch defines CLOCK_TICK_RATE for all architectures as far as I can
tell, so the result should compile fine across them all. I can only test
it for the ix86 arch though as that's all I have.

> Anyone disagrees with changing all the instances of 1193180/1193182 to
> CLOCK_TICK_RATE?

Other than the ARM architecture, that appears to be the value used for
all of the currently supported architectures in the 2.4 kernel series...

Best wishes from Riley.
---
* Nothing as pretty as a smile, nothing as ugly as a frown.

---
Outgoing mail is certified Virus Free.
Checked by AVG anti-virus system (http://www.grisoft.com).
Version: 6.0.489 / Virus Database: 288 - Release Date: 10-Jun-2003


Attachments:
CLOCK_TICK_RATE.diff.bz2 (4.20 kB)

2003-06-16 18:46:44

by David Mosberger

[permalink] [raw]
Subject: RE: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

>>>>> On Sun, 15 Jun 2003 11:51:00 +0100, "Riley Williams" <[email protected]> said:

Riley> 2. The IA64 arch didn't define CLOCK_TICK_RATE at all, but
Riley> then used the 1193182 value as a magic value in several
Riley> files. I've inserted that as the definition thereof in
Riley> timex.h for that arch.

AFAIK, on ia64, it makes absolutely no sense at all to define this
magic CLOCK_TICK_RATE in timex.h. There simply is nothing in the ia64
architecture that requires any timer to operate at 1193182 Hz.

If there are drivers that rely on the frequency, those drivers should
be fixed instead.

Please do not add CLOCK_TICK_RATE to the ia64 timex.h header file.

--david

2003-06-17 21:57:44

by Riley Williams

[permalink] [raw]
Subject: RE: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

Hi David.

> Riley> 2. The IA64 arch didn't define CLOCK_TICK_RATE at all, but
> Riley> then used the 1193182 value as a magic value in several
> Riley> files. I've inserted that as the definition thereof in
> Riley> timex.h for that arch.

> AFAIK, on ia64, it makes absolutely no sense at all to define this
> magic CLOCK_TICK_RATE in timex.h. There simply is nothing in the
> ia64 architecture that requires any timer to operate at 1193182 Hz.

I think we're talking at cross-purposes here. The kernel includes a
timer that, amongst other things, measures how long it's been up, and
on most architectures, this is based on a hardware timer that runs at
a particular frequency. This define states what frequency that timer
runs at, nothing more nor less than that.

On most architectures, the said timer runs at 1,193,181.818181818 Hz.
However, there is absolutely nothing that states that it has to run at
that frequency. Indeed, some of the other architectures run at wildly
different frequencies from that one - varying from 25,000 Hz right up
to 40,000,000 Hz.

> If there are drivers that rely on the frequency, those drivers
> should be fixed instead.

There are generic drivers that rely on knowing the frequency of the
kernel timer, and those are almost certainly currently bug-ridden in
any architecture where the kernel timer doesn't run at the above
frequency simply because they currently assume it runs at that
frequency. However, ANY bugfix involves each architecture declaring
the frequency its particular kernel timer runs at, and thus requires
the CLOCK_TICK_RATE macro to be defined.

> Please do not add CLOCK_TICK_RATE to the ia64 timex.h header file.

It needs to be declared there. The only question is regarding the
value it is defined to, and it would have to be somebody with better
knowledge of the ia64 than me who decides that. All I can do is to
post a reasonable default until such decision is made.

Best wishes from Riley.
---
* Nothing as pretty as a smile, nothing as ugly as a frown.

---
Outgoing mail is certified Virus Free.
Checked by AVG anti-virus system (http://www.grisoft.com).
Version: 6.0.490 / Virus Database: 289 - Release Date: 16-Jun-2003

2003-06-17 22:06:05

by David Mosberger

[permalink] [raw]
Subject: RE: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

>>>>> On Tue, 17 Jun 2003 23:11:46 +0100, "Riley Williams" <[email protected]> said:

Riley> [CLOCK_TICK_RATE] needs to be declared there. The only
Riley> question is regarding the value it is defined to, and it
Riley> would have to be somebody with better knowledge of the ia64
Riley> than me who decides that. All I can do is to post a
Riley> reasonable default until such decision is made.

The ia64 platform architecture does not provide for such a timer and
hence there is no reasonable value that the macro could be set to.

Thanks,

--david

2003-06-17 22:07:30

by Russell King

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

On Tue, Jun 17, 2003 at 11:11:46PM +0100, Riley Williams wrote:
> On most architectures, the said timer runs at 1,193,181.818181818 Hz.

Wow. That's more accurate than a highly expensive Caesium standard.
And there's one inside most architectures? Wow, we're got a great
deal there, haven't we? 8)

> > Please do not add CLOCK_TICK_RATE to the ia64 timex.h header file.
>
> It needs to be declared there. The only question is regarding the
> value it is defined to, and it would have to be somebody with better
> knowledge of the ia64 than me who decides that. All I can do is to
> post a reasonable default until such decision is made.

If this is the case, we have a dilema on ARM. CLOCK_TICK_RATE has
been, and currently remains (at Georges distaste) a variable on
some platforms. I shudder to think what this is doing to some of
the maths in Georges new time keeping and timer code.

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2003-06-17 22:08:13

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

On Tue, Jun 17, 2003 at 03:19:57PM -0700, David Mosberger wrote:

> >>>>> On Tue, 17 Jun 2003 23:11:46 +0100, "Riley Williams" <[email protected]> said:
>
> Riley> [CLOCK_TICK_RATE] needs to be declared there. The only
> Riley> question is regarding the value it is defined to, and it
> Riley> would have to be somebody with better knowledge of the ia64
> Riley> than me who decides that. All I can do is to post a
> Riley> reasonable default until such decision is made.
>
> The ia64 platform architecture does not provide for such a timer and
> hence there is no reasonable value that the macro could be set to.

It seems to be used for making beeps, even on that architecture.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-06-17 22:22:25

by David Mosberger

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

>>>>> On Wed, 18 Jun 2003 00:21:46 +0200, Vojtech Pavlik <[email protected]> said:

Vojtech> It seems to be used for making beeps, even on that
Vojtech> architecture.

Don't confuse architecture and implementation. There are _some_
machines (implementations) which have legacy support. But the
architecture is explicitly designed to allow for legacy-free machines,
as is the case for the hp zx1-based machines, for example.

It seems to me that input/misc/pcspkr.c is doing the Right Thing:

count = 1193182 / value;

spin_lock_irqsave(&i8253_beep_lock, flags);

if (count) {
/* enable counter 2 */
outb_p(inb_p(0x61) | 3, 0x61);
/* set command for counter 2, 2 byte write */
outb_p(0xB6, 0x43);
/* select desired HZ */
outb_p(count & 0xff, 0x42);
outb((count >> 8) & 0xff, 0x42);

so if a legacy speaker is present, it assumes a particular frequency.
In other words: it's a driver issue. On ia64, this frequency
certainly has nothing to do with time-keeping and therefore doesn't
belong in timex.h.

--david

2003-06-17 22:25:09

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

On Tue, Jun 17, 2003 at 11:21:13PM +0100, Russell King wrote:

> On Tue, Jun 17, 2003 at 11:11:46PM +0100, Riley Williams wrote:
> > On most architectures, the said timer runs at 1,193,181.818181818 Hz.
>
> Wow. That's more accurate than a highly expensive Caesium standard.
> And there's one inside most architectures? Wow, we're got a great
> deal there, haven't we? 8)

Well, it's unfortunately up to 400ppm off on most systems. Nevertheless
this is the 'official' frequency, actually it's a NTSC dotclock (14.3181818)
divided by 12.

> > > Please do not add CLOCK_TICK_RATE to the ia64 timex.h header file.
> >
> > It needs to be declared there. The only question is regarding the
> > value it is defined to, and it would have to be somebody with better
> > knowledge of the ia64 than me who decides that. All I can do is to
> > post a reasonable default until such decision is made.
>
> If this is the case, we have a dilema on ARM. CLOCK_TICK_RATE has
> been, and currently remains (at Georges distaste) a variable on
> some platforms. I shudder to think what this is doing to some of
> the maths in Georges new time keeping and timer code.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-06-17 22:28:51

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

On Tue, Jun 17, 2003 at 03:34:24PM -0700, David Mosberger wrote:
> >>>>> On Wed, 18 Jun 2003 00:21:46 +0200, Vojtech Pavlik <[email protected]> said:
>
> Vojtech> It seems to be used for making beeps, even on that
> Vojtech> architecture.
>
> Don't confuse architecture and implementation. There are _some_
> machines (implementations) which have legacy support. But the
> architecture is explicitly designed to allow for legacy-free machines,
> as is the case for the hp zx1-based machines, for example.
>
> It seems to me that input/misc/pcspkr.c is doing the Right Thing:
>
> count = 1193182 / value;
>
> spin_lock_irqsave(&i8253_beep_lock, flags);
>
> if (count) {
> /* enable counter 2 */
> outb_p(inb_p(0x61) | 3, 0x61);
> /* set command for counter 2, 2 byte write */
> outb_p(0xB6, 0x43);
> /* select desired HZ */
> outb_p(count & 0xff, 0x42);
> outb((count >> 8) & 0xff, 0x42);
>
> so if a legacy speaker is present, it assumes a particular frequency.
> In other words: it's a driver issue. On ia64, this frequency
> certainly has nothing to do with time-keeping and therefore doesn't
> belong in timex.h.

I'm quite fine with that. However, different (sub)archs, for example the
AMD Elan CPUs have a slightly different base frequency. So it looks like
an arch-dependent #define is needed. I don't care about the location
(timex.h indeed seems inappropriate, maybe the right location is
pcspkr.c ...), or the name, but something needs to be done so that the
beeps have the same sound the same on all archs.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-06-17 22:34:35

by Russell King

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

On Wed, Jun 18, 2003 at 12:42:33AM +0200, Vojtech Pavlik wrote:
> an arch-dependent #define is needed. I don't care about the location
> (timex.h indeed seems inappropriate, maybe the right location is
> pcspkr.c ...), or the name, but something needs to be done so that the
> beeps have the same sound the same on all archs.

This may be something to aspire to, but I don't think its achievable
given the nature of PC hardware. Some "PC speakers" are actually
buzzers in some cases rather than real loudspeakers which give a
squark rather than a beep.

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2003-06-17 22:39:47

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

On Tue, Jun 17, 2003 at 11:48:27PM +0100, Russell King wrote:
> On Wed, Jun 18, 2003 at 12:42:33AM +0200, Vojtech Pavlik wrote:
> > an arch-dependent #define is needed. I don't care about the location
> > (timex.h indeed seems inappropriate, maybe the right location is
> > pcspkr.c ...), or the name, but something needs to be done so that the
> > beeps have the same sound the same on all archs.
>
> This may be something to aspire to, but I don't think its achievable
> given the nature of PC hardware. Some "PC speakers" are actually
> buzzers in some cases rather than real loudspeakers which give a
> squark rather than a beep.

Ok, you're right. But at least we should try to program them to the same
beep frequency.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-06-17 22:54:39

by David Mosberger

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

>>>>> On Wed, 18 Jun 2003 00:42:33 +0200, Vojtech Pavlik <[email protected]> said:

>> so if a legacy speaker is present, it assumes a particular
>> frequency. In other words: it's a driver issue. On ia64, this
>> frequency certainly has nothing to do with time-keeping and
>> therefore doesn't belong in timex.h.

Vojtech> I'm quite fine with that. However, different (sub)archs,
Vojtech> for example the AMD Elan CPUs have a slightly different
Vojtech> base frequency. So it looks like an arch-dependent #define
Vojtech> is needed. I don't care about the location (timex.h indeed
Vojtech> seems inappropriate, maybe the right location is pcspkr.c
Vojtech> ...), or the name, but something needs to be done so that
Vojtech> the beeps have the same sound the same on all archs.

Sounds much better to me. Wouldn't something along the lines of this
make the most sense:

#ifdef __ARCH_PIT_FREQ
# define PIT_FREQ __ARCH_PIT_FREQ
#else
# define PIT_FREQ 1193182
#endif

After all, it seems like the vast majority of legacy-compatible
hardware _do_ use the standard frequency.

--david

2003-06-17 23:00:23

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

On Tue, Jun 17, 2003 at 04:08:32PM -0700, David Mosberger wrote:

> >>>>> On Wed, 18 Jun 2003 00:42:33 +0200, Vojtech Pavlik <[email protected]> said:
>
> >> so if a legacy speaker is present, it assumes a particular
> >> frequency. In other words: it's a driver issue. On ia64, this
> >> frequency certainly has nothing to do with time-keeping and
> >> therefore doesn't belong in timex.h.
>
> Vojtech> I'm quite fine with that. However, different (sub)archs,
> Vojtech> for example the AMD Elan CPUs have a slightly different
> Vojtech> base frequency. So it looks like an arch-dependent #define
> Vojtech> is needed. I don't care about the location (timex.h indeed
> Vojtech> seems inappropriate, maybe the right location is pcspkr.c
> Vojtech> ...), or the name, but something needs to be done so that
> Vojtech> the beeps have the same sound the same on all archs.
>
> Sounds much better to me. Wouldn't something along the lines of this
> make the most sense:
>
> #ifdef __ARCH_PIT_FREQ
> # define PIT_FREQ __ARCH_PIT_FREQ
> #else
> # define PIT_FREQ 1193182
> #endif
>
> After all, it seems like the vast majority of legacy-compatible
> hardware _do_ use the standard frequency.

Now, if this was in some nice include file, along with the definition of
the i8253 PIT spinlock, that'd be great. Because not just the beeper
driver uses the PIT, also some joystick code uses it if it is available.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-06-17 23:10:14

by David Mosberger

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

>>>>> On Wed, 18 Jun 2003 01:14:11 +0200, Vojtech Pavlik <[email protected]> said:

>> Sounds much better to me. Wouldn't something along the lines of
>> this make the most sense:

>> #ifdef __ARCH_PIT_FREQ # define PIT_FREQ __ARCH_PIT_FREQ #else #
>> define PIT_FREQ 1193182 #endif

>> After all, it seems like the vast majority of legacy-compatible
>> hardware _do_ use the standard frequency.

Vojtech> Now, if this was in some nice include file, along with the
Vojtech> definition of the i8253 PIT spinlock, that'd be
Vojtech> great. Because not just the beeper driver uses the PIT,
Vojtech> also some joystick code uses it if it is available.

ftape, too. The LATCH() macro should also be moved to such a header
file, I think. How about just creating asm/pit.h? Only platforms
that need to (potentially) support legacy hardware would need to
define it. E.g., on ia64, we could do:

#ifndef _ASM_IA64_PIT_H
#define _ASM_IA64_PIT_H

#include <linux/config.h>

#ifdef CONFIG_LEGACY_HW
# define PIT_FREQ 1193182
# define LATCH ((CLOCK_TICK_RATE + HZ/2) / HZ)
#endif

#endif /* _ASM_IA64_PIT_H */

This way, machines that support legacy hardware can define
CONFIG_LEGACY_HW and on others, the macro can be left undefined, so
that any attempt to compile drivers requiring legacy hw would fail to
compile upfront (much better than accessing random ports!).

--david

2003-06-17 23:17:32

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

On Tue, Jun 17, 2003 at 04:24:04PM -0700, David Mosberger wrote:
> >>>>> On Wed, 18 Jun 2003 01:14:11 +0200, Vojtech Pavlik <[email protected]> said:
>
> >> Sounds much better to me. Wouldn't something along the lines of
> >> this make the most sense:
>
> >> #ifdef __ARCH_PIT_FREQ # define PIT_FREQ __ARCH_PIT_FREQ #else #
> >> define PIT_FREQ 1193182 #endif
>
> >> After all, it seems like the vast majority of legacy-compatible
> >> hardware _do_ use the standard frequency.
>
> Vojtech> Now, if this was in some nice include file, along with the
> Vojtech> definition of the i8253 PIT spinlock, that'd be
> Vojtech> great. Because not just the beeper driver uses the PIT,
> Vojtech> also some joystick code uses it if it is available.
>
> ftape, too. The LATCH() macro should also be moved to such a header
> file, I think. How about just creating asm/pit.h? Only platforms
> that need to (potentially) support legacy hardware would need to
> define it. E.g., on ia64, we could do:
>
> #ifndef _ASM_IA64_PIT_H
> #define _ASM_IA64_PIT_H
>
> #include <linux/config.h>
>
> #ifdef CONFIG_LEGACY_HW
> # define PIT_FREQ 1193182
> # define LATCH ((CLOCK_TICK_RATE + HZ/2) / HZ)
> #endif
>
> #endif /* _ASM_IA64_PIT_H */
>
> This way, machines that support legacy hardware can define
> CONFIG_LEGACY_HW and on others, the macro can be left undefined, so
> that any attempt to compile drivers requiring legacy hw would fail to
> compile upfront (much better than accessing random ports!).

Yes, that looks very good indeed. Riley?

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-06-18 00:32:48

by George Anzinger

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

Vojtech Pavlik wrote:
> On Tue, Jun 17, 2003 at 11:21:13PM +0100, Russell King wrote:
>
>
>>On Tue, Jun 17, 2003 at 11:11:46PM +0100, Riley Williams wrote:
>>
>>>On most architectures, the said timer runs at 1,193,181.818181818 Hz.
>>
>>Wow. That's more accurate than a highly expensive Caesium standard.
>>And there's one inside most architectures? Wow, we're got a great
>>deal there, haven't we? 8)
>
>
> Well, it's unfortunately up to 400ppm off on most systems. Nevertheless
> this is the 'official' frequency, actually it's a NTSC dotclock (14.3181818)
> divided by 12.
>
>
>>> > Please do not add CLOCK_TICK_RATE to the ia64 timex.h header file.
>>>
>>>It needs to be declared there. The only question is regarding the
>>>value it is defined to, and it would have to be somebody with better
>>>knowledge of the ia64 than me who decides that. All I can do is to
>>>post a reasonable default until such decision is made.
>>
>>If this is the case, we have a dilema on ARM. CLOCK_TICK_RATE has
>>been, and currently remains (at Georges distaste) a variable on
>>some platforms. I shudder to think what this is doing to some of
>>the maths in Georges new time keeping and timer code.

So do I :)
>
>

--
George Anzinger [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

2003-06-18 00:34:23

by George Anzinger

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

Vojtech Pavlik wrote:
> On Tue, Jun 17, 2003 at 04:24:04PM -0700, David Mosberger wrote:
>
>>>>>>>On Wed, 18 Jun 2003 01:14:11 +0200, Vojtech Pavlik <[email protected]> said:
>>
>> >> Sounds much better to me. Wouldn't something along the lines of
>> >> this make the most sense:
>>
>> >> #ifdef __ARCH_PIT_FREQ # define PIT_FREQ __ARCH_PIT_FREQ #else #
>> >> define PIT_FREQ 1193182 #endif
>>
>> >> After all, it seems like the vast majority of legacy-compatible
>> >> hardware _do_ use the standard frequency.
>>
>> Vojtech> Now, if this was in some nice include file, along with the
>> Vojtech> definition of the i8253 PIT spinlock, that'd be
>> Vojtech> great. Because not just the beeper driver uses the PIT,
>> Vojtech> also some joystick code uses it if it is available.
>>
>>ftape, too. The LATCH() macro should also be moved to such a header
>>file, I think. How about just creating asm/pit.h? Only platforms
>>that need to (potentially) support legacy hardware would need to
>>define it. E.g., on ia64, we could do:
>>
>> #ifndef _ASM_IA64_PIT_H
>> #define _ASM_IA64_PIT_H
>>
>> #include <linux/config.h>
>>
>> #ifdef CONFIG_LEGACY_HW
>> # define PIT_FREQ 1193182
>> # define LATCH ((CLOCK_TICK_RATE + HZ/2) / HZ)
----------------------------------^^^^^^^^^^^^^^^
should be PIT_FREQ me thinks :)

-g
>> #endif
>>
>> #endif /* _ASM_IA64_PIT_H */
>>
>>This way, machines that support legacy hardware can define
>>CONFIG_LEGACY_HW and on others, the macro can be left undefined, so
>>that any attempt to compile drivers requiring legacy hw would fail to
>>compile upfront (much better than accessing random ports!).
>
>
> Yes, that looks very good indeed. Riley?
>

--
George Anzinger [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

2003-06-18 00:47:33

by George Anzinger

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

Russell King wrote:
> On Tue, Jun 17, 2003 at 11:11:46PM +0100, Riley Williams wrote:
>
>>On most architectures, the said timer runs at 1,193,181.818181818 Hz.
>
>
> Wow. That's more accurate than a highly expensive Caesium standard.
> And there's one inside most architectures? Wow, we're got a great
> deal there, haven't we? 8)
>
>
>> > Please do not add CLOCK_TICK_RATE to the ia64 timex.h header file.
>>
>>It needs to be declared there. The only question is regarding the
>>value it is defined to, and it would have to be somebody with better
>>knowledge of the ia64 than me who decides that. All I can do is to
>>post a reasonable default until such decision is made.
>
>
> If this is the case, we have a dilema on ARM. CLOCK_TICK_RATE has
> been, and currently remains (at Georges distaste) a variable on
> some platforms. I shudder to think what this is doing to some of
> the maths in Georges new time keeping and timer code.
>
So just what is it used for? On the x86, the math on it is used
mostly (aside from LATCH) to figure out the actual value of 1/HZ.
This is then used to compute a more correct TICK_NSEC which is added
to xtime each tick. This usage of CLOCK_TICK_RATE just "beats it up"
to see how close the hardware can get to the requested rate of 1/HZ.
Since this code is in time.h and timex.h, it is shared with all the archs.

I submit that if it is not used to actually compute a LATCH value for
the 1/HZ tick, it should just be some rather large value that more or
less represents the granularity of the hardwares ability to generate
1/HZ ticks. Once it gets above about 100MHZ, I think it actually
drops out of the calculations. The last thing we want to do at this
point is make it a variable. (Nor do you want to put a -E in your cc
command and take a look at what is produced for the conversion constants.)

If it is not possible to make it a constant, I think we need to
revisit the timer conversion code as we would not only have a LOT of
code bloat, but it would add far too much time to the conversions.

--
George Anzinger [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml

2003-06-18 14:33:56

by Hollis Blanchard

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

On Tuesday, Jun 17, 2003, at 18:24 US/Central, David Mosberger wrote:
> #ifdef CONFIG_LEGACY_HW
> # define PIT_FREQ 1193182
> # define LATCH ((CLOCK_TICK_RATE + HZ/2) / HZ)
> #endif
>
> This way, machines that support legacy hardware can define
> CONFIG_LEGACY_HW and on others, the macro can be left undefined, so
> that any attempt to compile drivers requiring legacy hw would fail to
> compile upfront (much better than accessing random ports!).

Is "having legacy hardware" an all-or-nothing condition for you? If
not, a CONFIG_LEGACY_PIT might be more appropriate...

--
Hollis Blanchard
IBM Linux Technology Center

2003-06-18 18:36:42

by David Mosberger

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

>>>>> On Wed, 18 Jun 2003 09:47:44 -0500, Hollis Blanchard <[email protected]> said:

Hollis> On Tuesday, Jun 17, 2003, at 18:24 US/Central, David Mosberger wrote:
>> #ifdef CONFIG_LEGACY_HW
>> # define PIT_FREQ 1193182
>> # define LATCH ((CLOCK_TICK_RATE + HZ/2) / HZ)
>> #endif

>> This way, machines that support legacy hardware can define
>> CONFIG_LEGACY_HW and on others, the macro can be left undefined, so
>> that any attempt to compile drivers requiring legacy hw would fail to
>> compile upfront (much better than accessing random ports!).

Hollis> Is "having legacy hardware" an all-or-nothing condition for you? If
Hollis> not, a CONFIG_LEGACY_PIT might be more appropriate...

I believe it is, though I'm not entirely certain about Intel's Tiger
platform. If more fine-grained distinction really is needed, I
suspect we'd rather want something called CONFIG_8259_PIT. Might be a
good idea to do this for all legacy devices.

--david

2003-06-19 11:59:53

by David Woodhouse

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

On Tue, 2003-06-17 at 23:48, Russell King wrote:
> On Wed, Jun 18, 2003 at 12:42:33AM +0200, Vojtech Pavlik wrote:
> > an arch-dependent #define is needed. I don't care about the location
> > (timex.h indeed seems inappropriate, maybe the right location is
> > pcspkr.c ...), or the name, but something needs to be done so that the
> > beeps have the same sound the same on all archs.
>
> This may be something to aspire to, but I don't think its achievable
> given the nature of PC hardware. Some "PC speakers" are actually
> buzzers in some cases rather than real loudspeakers which give a
> squark rather than a beep.

They're not _that_ bad. Even on most recent hardware, mp3s played
through the PC speaker are relatively recognisable :)

--
dwmw2

2003-06-19 14:05:57

by Russell King

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

On Thu, Jun 19, 2003 at 01:13:28PM +0100, David Woodhouse wrote:
> They're not _that_ bad. Even on most recent hardware, mp3s played
> through the PC speaker are relatively recognisable :)

Maybe you've just been fortunate not to come across any.

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2003-06-25 07:49:10

by Riley Williams

[permalink] [raw]
Subject: RE: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

Hi all.

I have no objection to anything along these lines. The basic scenario
is simply this:

1. On ALL architectures except for IA64 and ARM there is a SINGLE
value for CLOCK_TICK_RATE that is used by several GENERIC drivers.
Currently, that value is used as a MAGIC NUMBER that corresponds
to the value in the Ix86 case, which is clearly wrong.

2. According to the IA64 people, those GENERIC drivers are apparently
irrelevant for that architecture, so making the CORRECT change of
replacing those magic numbers in the GENERIC drivers with the
CLOCK_TICK_RATE value will make no difference to IA64.

3. The ARM architecture has lots of sub-architectures, as was stated
here. The ARM version of timex.h includes a sub-arch-specific
header file which, for MOST of the sub-arch's, already defines
CLOCK_TICK_RATE to what appears to be an appropriate value. The
only change I made was to apply a catch-all to cover all of the
sub-arch's that didn't.

The CLOCK_TICK_RATE value should be defined as a result of doing...

#include <asm/timex.h>

...but I see absolutely nothing wrong with having that file do...

#include <asm/arch/timex.h>

...and having that in turn include various other include files that
are specific to that architecture, as per your proposal.

Best wishes from Riley.
---
* Nothing as pretty as a smile, nothing as ugly as a frown.


> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]]On Behalf Of Vojtech Pavlik
> Sent: Wednesday, June 18, 2003 12:31 AM
> To: [email protected]
> Cc: Vojtech Pavlik; Riley Williams; [email protected]
> Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]
>
>>>> Sounds much better to me. Wouldn't something along the lines of
>>>> this make the most sense:
>>
>>>> #ifdef __ARCH_PIT_FREQ # define PIT_FREQ __ARCH_PIT_FREQ #else #
>>>> define PIT_FREQ 1193182 #endif
>>
>>>> After all, it seems like the vast majority of legacy-compatible
>>>> hardware _do_ use the standard frequency.
>>
>>> Now, if this was in some nice include file, along with the
>>> definition of the i8253 PIT spinlock, that'd be
>>> great. Because not just the beeper driver uses the PIT,
>>> also some joystick code uses it if it is available.
>>
>> ftape, too. The LATCH() macro should also be moved to such a header
>> file, I think. How about just creating asm/pit.h? Only platforms
>> that need to (potentially) support legacy hardware would need to
>> define it. E.g., on ia64, we could do:
>>
>> #ifndef _ASM_IA64_PIT_H
>> #define _ASM_IA64_PIT_H
>>
>> #include <linux/config.h>
>>
>> #ifdef CONFIG_LEGACY_HW
>> # define PIT_FREQ 1193182
>> # define LATCH ((CLOCK_TICK_RATE + HZ/2) / HZ)
>> #endif
>>
>> #endif /* _ASM_IA64_PIT_H */
>>
>> This way, machines that support legacy hardware can define
>> CONFIG_LEGACY_HW and on others, the macro can be left undefined, so
>> that any attempt to compile drivers requiring legacy hw would fail to
>> compile upfront (much better than accessing random ports!).
>
> Yes, that looks very good indeed. Riley?

---
Outgoing mail is certified Virus Free.
Checked by AVG anti-virus system (http://www.grisoft.com).
Version: 6.0.491 / Virus Database: 290 - Release Date: 18-Jun-2003

2003-06-25 17:07:49

by David Mosberger

[permalink] [raw]
Subject: RE: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

>>>>> On Wed, 25 Jun 2003 09:03:34 +0100, "Riley Williams" <[email protected]> said:

Riley> Hi all.
Riley> I have no objection to anything along these lines. The basic scenario
Riley> is simply this:

Riley> 1. On ALL architectures except for IA64 and ARM there is a SINGLE
Riley> value for CLOCK_TICK_RATE that is used by several GENERIC drivers.
Riley> Currently, that value is used as a MAGIC NUMBER that corresponds
Riley> to the value in the Ix86 case, which is clearly wrong.

What do you mean be "generic"? AFAIK, the drivers you're talking
about all depend on there being an 8259-style PIT. As such, they
depend on the 8259 and are not generic. This dependency should be
made explicit.

BTW: I didn't think Alpha derived its clock-tick from the PIT either,
but I could be misremembering. Could someone more familiar with Alpha
confirm or deny?

Riley> 2. According to the IA64 people, those GENERIC drivers are apparently
Riley> irrelevant for that architecture, so making the CORRECT change of
Riley> replacing those magic numbers in the GENERIC drivers with the
Riley> CLOCK_TICK_RATE value will make no difference to IA64.

That's not precise: _some_ ia64 machies do have legacy hardware and
those should be able to use 8259-dependent drivers if they choose to
do so.

Moreover, the current drivers would compile just fine on ia64, even
though they could not possibly work correctly with the current use of
CLOCK_TICK_RATE. With a separate header file (and a config option),
these dependencies would be made explicit and that would improve
overall cleanliness.

In other words, I still think the right way to go about this is to
have <asm/pit.h>. On x86, this could be:

--
#include <asm/timex.h>

#define PIT_FREQ CLOCK_TICK_RATE
#define PIT_LATCH ((PIT_FREQ + HZ/2) / HZ)
--

If you insist, you could even put this in asm-generic, though
personally I don't think that's terribly elegant.

On ia64, <asm/pit.h> could be:

#ifdef CONFIG_PIT
# define PIT_FREQ 1193182
# define PIT_LATCH ((PIT_FREQ + HZ/2) / HZ)
#endif

--david

2003-06-25 17:42:25

by Riley Williams

[permalink] [raw]
Subject: RE: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

Hi David.

>> I have no objection to anything along these lines. The
>> basic scenario is simply this:
>>
>> 1. On ALL architectures except for IA64 and ARM there is
>> a SINGLE value for CLOCK_TICK_RATE that is used by
>> several GENERIC drivers. Currently, that value is used
>> as a MAGIC NUMBER that corresponds to the value in the
>> Ix86 case, which is clearly wrong.

> What do you mean be "generic"?

Not specific to any particular architecture as far as the
location of the file containing the driver code is concerned.
More simply, not located under linux/arch in the kernel tree.

> AFAIK, the drivers you're talking about all depend on there
> being an 8259-style PIT. As such, they depend on the 8259
> and are not generic. This dependency should be made explicit.

In that case, ALL of the drivers in question need to be moved
under the linux/arch tree. Very few are currently there.

> BTW: I didn't think Alpha derived its clock-tick from the
> PIT either, but I could be misremembering. Could someone
> more familiar with Alpha confirm or deny?

I know ZILCH about ALPHA as I've never had or seen one.

>> 2. According to the IA64 people, those GENERIC drivers
>> are apparently irrelevant for that architecture, so
>> making the CORRECT change of replacing those magic
>> numbers in the GENERIC drivers with the CLOCK_TICK_RATE
>> value will make no difference to IA64.

> That's not precise: _some_ ia64 machines do have legacy hardware
> and those should be able to use 8259-dependent drivers if they
> choose to do so.

My comment as quoted above is a summary of the comments made by
the IA64 developers in this thread. I know ZILCH about the IA64
architecture because, as with the ALPHA architecture, I've never
even seen one. I thus have to assume that comments made by the
IA64 maintainers are accurate.

> Moreover, the current drivers would compile just fine on ia64,
> even though they could not possibly work correctly with the
> current use of CLOCK_TICK_RATE. With a separate header file
> (and a config option), these dependencies would be made
> explicit and that would improve overall cleanliness.

I agree that the dependencies need to be made explicit. However,
I'm not convinced that a separate header file is justified, much
less needed.

> In other words, I still think the right way to go about this
> is to have <asm/pit.h>. On x86, this could be:
>
> --
> #include <asm/timex.h>
>
> #define PIT_FREQ CLOCK_TICK_RATE
> #define PIT_LATCH ((PIT_FREQ + HZ/2) / HZ)
> --
>
> If you insist, you could even put this in asm-generic, though
> personally I don't think that's terribly elegant.

The important details are...

1. The relevant values are in a known header file.

2. That header file is referenced and the values therein
are used rather than just using magic numbers.

Personally, I have no problem with which header files are used.
What matters is that inclusion of a specified header file always
defines the relevant values.

> On ia64, <asm/pit.h> could be:
>
> #ifdef CONFIG_PIT
> # define PIT_FREQ 1193182
> # define PIT_LATCH ((PIT_FREQ + HZ/2) / HZ)
> #endif

You would then need to ensure that if CONFIG_PIT was not defined,
no reference was ever made to either PIT_FREQ or PIT_LATCH which
can easily become ugly code that the maintainers won't touch.

Best wishes from Riley.
---
* Nothing as pretty as a smile, nothing as ugly as a frown.

---
Outgoing mail is certified Virus Free.
Checked by AVG anti-virus system (http://www.grisoft.com).
Version: 6.0.491 / Virus Database: 290 - Release Date: 18-Jun-2003

2003-06-25 18:35:35

by David Mosberger

[permalink] [raw]
Subject: RE: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

>>>>> On Wed, 25 Jun 2003 18:56:43 +0100, "Riley Williams" <[email protected]> said:

>> AFAIK, the drivers you're talking about all depend on there
>> being an 8259-style PIT. As such, they depend on the 8259
>> and are not generic. This dependency should be made explicit.

Riley> In that case, ALL of the drivers in question need to be moved
Riley> under the linux/arch tree. Very few are currently there.

Not at all. It's completely standard to have drivers in
linux/drivers/ which may never be enabled for certain platforms. Or
what's the last time an x86 PC used an Sun SBUS driver?

>>> 2. According to the IA64 people, those GENERIC drivers
>>> are apparently irrelevant for that architecture, so
>>> making the CORRECT change of replacing those magic
>>> numbers in the GENERIC drivers with the CLOCK_TICK_RATE
>>> value will make no difference to IA64.

>> That's not precise: _some_ ia64 machines do have legacy hardware
>> and those should be able to use 8259-dependent drivers if they
>> choose to do so.

Riley> My comment as quoted above is a summary of the comments made by
Riley> the IA64 developers in this thread. I know ZILCH about the IA64
Riley> architecture because, as with the ALPHA architecture, I've never
Riley> even seen one. I thus have to assume that comments made by the
Riley> IA64 maintainers are accurate.

You seem to fail to realize that I _am_ the ia64 linux tree maintainer.
What does it take for you to believe me?

>> Moreover, the current drivers would compile just fine on ia64,
>> even though they could not possibly work correctly with the
>> current use of CLOCK_TICK_RATE. With a separate header file
>> (and a config option), these dependencies would be made
>> explicit and that would improve overall cleanliness.

Riley> I agree that the dependencies need to be made explicit.

Good.

Riley> However, I'm not convinced that a separate header file is
Riley> justified, much less needed.

timex.h definitely is the wrong place. If you know of a better place
(other than <asm/pit.h>), I'm all ears.

>> In other words, I still think the right way to go about this
>> is to have <asm/pit.h>. On x86, this could be:
>>
>> --
>> #include <asm/timex.h>
>>
>> #define PIT_FREQ CLOCK_TICK_RATE
>> #define PIT_LATCH ((PIT_FREQ + HZ/2) / HZ)
>> --
>>
>> If you insist, you could even put this in asm-generic, though
>> personally I don't think that's terribly elegant.

Riley> The important details are...

Riley> 1. The relevant values are in a known header file.

Riley> 2. That header file is referenced and the values therein
Riley> are used rather than just using magic numbers.

I have no problem with that.

Riley> Personally, I have no problem with which header files are used.
Riley> What matters is that inclusion of a specified header file always
Riley> defines the relevant values.

Can we then agree to just create <asm/pit.h> and be done with it?

>> On ia64, <asm/pit.h> could be:

>> #ifdef CONFIG_PIT
>> # define PIT_FREQ 1193182
>> # define PIT_LATCH ((PIT_FREQ + HZ/2) / HZ)
>> #endif

Riley> You would then need to ensure that if CONFIG_PIT was not defined,
Riley> no reference was ever made to either PIT_FREQ or PIT_LATCH which
Riley> can easily become ugly code that the maintainers won't touch.

Thanks to the new Kbuild, it's very easy: just add a "depends on PIT"
for drivers that depend on the PIT (I think that's primarily ftape and
the drivers/input/misc/{pc,98}spkr.c.

--david

2003-06-25 19:45:13

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

On Wed, Jun 25, 2003 at 10:20:59AM -0700, David Mosberger wrote:

> Moreover, the current drivers would compile just fine on ia64, even
> though they could not possibly work correctly with the current use of
> CLOCK_TICK_RATE. With a separate header file (and a config option),
> these dependencies would be made explicit and that would improve
> overall cleanliness.
>
> In other words, I still think the right way to go about this is to
> have <asm/pit.h>. On x86, this could be:
>
> --
> #include <asm/timex.h>
>
> #define PIT_FREQ CLOCK_TICK_RATE
> #define PIT_LATCH ((PIT_FREQ + HZ/2) / HZ)
> --

Actually, I think it should be the other way around:

asm-i386/pit.h:

#define PIT_FREQ 1193182
#define PIT_LATCH ((PIT_FREQ + HZ/2) / HZ)

asm-i386/timex.h:

#include <asm/pit.h>
#define CLOCK_TICK_RATE PIT_FREQ

> If you insist, you could even put this in asm-generic, though
> personally I don't think that's terribly elegant.
>
> On ia64, <asm/pit.h> could be:
>
> #ifdef CONFIG_PIT
> # define PIT_FREQ 1193182
> # define PIT_LATCH ((PIT_FREQ + HZ/2) / HZ)
> #endif
>
> --david

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2003-06-25 19:55:25

by David Mosberger

[permalink] [raw]
Subject: Re: [patch] input: Fix CLOCK_TICK_RATE usage ... [8/13]

>>>>> On Wed, 25 Jun 2003 21:58:47 +0200, Vojtech Pavlik <[email protected]> said:

Vojtech> Actually, I think it should be the other way around:

Ah, yes, that looks better for x86.

--david

2003-06-25 20:11:59

by J.C. Wren

[permalink] [raw]
Subject: Assorted warnings while building 2.5.73

Perhaps I over estimate, but some of these seem of mild concern, primarily the forced conditions.
Does the LKML want to see these, or should I report them via some other mechanism?

fs/fat/inode.c: In function `fat_fill_super':
fs/fat/inode.c:806: warning: comparison is always true due to limited range of data type
fs/ntfs/super.c: In function `is_boot_sector_ntfs':
fs/ntfs/super.c:375: warning: integer constant is too large for "long" type
fs/smbfs/ioctl.c: In function `smb_ioctl':
fs/smbfs/ioctl.c:36: warning: comparison is always false due to limited range of data type
fs/smbfs/ioctl.c:36: warning: comparison is always false due to limited range of data type
fs/smbfs/ioctl.c:36: warning: comparison is always false due to limited range of data type
fs/smbfs/ioctl.c:36: warning: comparison is always false due to limited range of data type
drivers/char/vt_ioctl.c: In function `do_kdsk_ioctl':
drivers/char/vt_ioctl.c:85: warning: comparison is always false due to limited range of data type
drivers/char/vt_ioctl.c:85: warning: comparison is always false due to limited range of data type
drivers/char/vt_ioctl.c: In function `do_kdgkb_ioctl':
drivers/char/vt_ioctl.c:211: warning: comparison is always false due to limited range of data type
drivers/char/keyboard.c: In function `k_fn':
drivers/char/keyboard.c:665: warning: comparison is always true due to limited range of data type
drivers/ide/ide-probe.c: In function `hwif_check_region':
drivers/ide/ide-probe.c:644: warning: `check_region' is deprecated (declared at include/linux/ioport.h:116)
drivers/serial/8250.c: In function `serial8250_set_termios':
drivers/serial/8250.c:1428: warning: comparison is always false due to limited range of data type
drivers/video/matrox/matroxfb_g450.c: In function `g450_compute_bwlevel':
drivers/video/matrox/matroxfb_g450.c:129: warning: duplicate `const'
drivers/video/matrox/matroxfb_g450.c:130: warning: duplicate `const'
net/ipv4/igmp.c: In function `igmp_rcv':
net/ipv4/igmp.c:851: warning: `skb_linearize' is deprecated (declared at include/linux/skbuff.h:1129)
arch/i386/boot/setup.S: Assembler messages:
arch/i386/boot/setup.S:165: Warning: value 0x37ffffff truncated to 0x37ffffff
drivers/i2c/i2c-sensor.c: In function `i2c_detect':
drivers/i2c/i2c-sensor.c:54: warning: `check_region' is deprecated (declared at include/linux/ioport.h:116)
drivers/ieee1394/raw1394.c: In function `arm_register':
drivers/ieee1394/raw1394.c:1569: warning: integer constant is too large for "long" type
drivers/ieee1394/raw1394.c:1570: warning: integer constant is too large for "long" type

Reading specs from /usr/lib/gcc-lib/i686-pc-linux-gnu/3.3/specs
Configured with: /var/tmp/portage/gcc-3.3/work/gcc-3.3/configure --prefix=/usr --bindir=/usr/i686-pc-linux-gnu/gcc-bin/3.3 --includedir=/usr/lib/gcc-lib/i686-pc-linux-gnu/3.3/include --datadir=/usr/share/gcc-data/i686-pc-linux-gnu/3.3 --mandir=/usr/share/gcc-data/i686-pc-linux-gnu/3.3/man --infodir=/usr/share/gcc-data/i686-pc-linux-gnu/3.3/info --enable-shared --host=i686-pc-linux-gnu --target=i686-pc-linux-gnu --with-system-zlib --enable-languages=c,c++,ada,f77,objc,java --enable-threads=posix --enable-long-long --disable-checking --enable-cstdio=stdio --enable-clocale=generic --enable-__cxa_atexit --enable-version-specific-runtime-libs --with-gxx-include-dir=/usr/lib/gcc-lib/i686-pc-linux-gnu/3.3/include/g++-v3 --with-local-prefix=/usr/local --enable-shared --enable-nls --without-included-gettext --x-includes=/usr/X11R6/include --x-libraries=/usr/X11R6/lib --enable-interpreter --enable-java-awt=xlib --with-x
Thread model: posix
gcc version 3.3 (Gentoo Linux 1.4, PVR 3.3)

2003-06-25 21:11:47

by Petr Vandrovec

[permalink] [raw]
Subject: Re: Assorted warnings while building 2.5.73

On 25 Jun 03 at 16:25, J.C. Wren wrote:
> drivers/video/matrox/matroxfb_g450.c: In function `g450_compute_bwlevel':
> drivers/video/matrox/matroxfb_g450.c:129: warning: duplicate `const'
> drivers/video/matrox/matroxfb_g450.c:130: warning: duplicate `const'

Fix min/max macros and/or learn gcc that "const typeof(x)" where x
is already const type is OK. Or I can code it with simple if(), but
why we have min/max macros then?

Is there some __attribute__((-Wno-duplicate-const)) ?
Petr Vandrovec
[email protected]


2003-06-25 21:59:26

by Bob Miller

[permalink] [raw]
Subject: Re: Assorted warnings while building 2.5.73

On Wed, Jun 25, 2003 at 11:25:35PM +0200, Petr Vandrovec wrote:
> On 25 Jun 03 at 16:25, J.C. Wren wrote:
> > drivers/video/matrox/matroxfb_g450.c: In function `g450_compute_bwlevel':
> > drivers/video/matrox/matroxfb_g450.c:129: warning: duplicate `const'
> > drivers/video/matrox/matroxfb_g450.c:130: warning: duplicate `const'
>
> Fix min/max macros and/or learn gcc that "const typeof(x)" where x
> is already const type is OK. Or I can code it with simple if(), but
> why we have min/max macros then?
>
> Is there some __attribute__((-Wno-duplicate-const)) ?
> Petr Vandrovec
> [email protected]
>
>
Or use a newer compiler that has this fixed, or use min_t()/max_t()
instead.

--
Bob Miller Email: [email protected]
Open Source Development Lab Phone: 503.626.2455 Ext. 17