2004-10-21 07:12:22

by Dmitry Torokhov

[permalink] [raw]
Subject: [PATCH 0/5] Sonypi driver model & PM changes

Hi,

I have been looking at the sysdevs in present in the kernel and noticed that
sonypi was registering itself as a system device. Surely it is possible to
suspend it with interrupyts enabled, so it better be converted to a platform
device. I course of convert I also did some additional changes:

01-sonypi-whitespace-fixes.patch
- get rid of extra whitespace and convert to the kernel cosing style:
... However, there is one special case, namely functions: they have
the opening brace at the beginning of the next line...

02-sonypi-module_param.patch
- convert sonypi from using MODULE_PARM and __setup to module_param.
The parameters are:
sonypi.camera
sonypi.compat
sonypi.fnkeyinit
sonypi.mask= - exported through sysfs, writeable
sonypi.minor=
sonypi.useinput
sonypi.verbose - exported through sysfs, writeable

03-sonypi-pm-changes.patch
- convert sonypi sysdev to platform device, drop old-style PM code
since APM does call device_suspend anyway so the new style handlers
will be called.

04-sonypi-misc-changes.patch
- switch sonypi_misc_read to use wake_event_interruptible instead of
a homemade copy, fix small race there, make sure that the device
is fully initialized before turning the interrupts on.

05-sonypi-pci_get_device.patch
- convert from pci_find_device which is obsolete to pci_get_device.

Warning: I do not have the hardware som while the code is compiles and I am
pretty sure it is correct it has not been tested.

Should apply to 2.6.9

--
Dmitry


2004-10-21 07:02:40

by Dmitry Torokhov

[permalink] [raw]
Subject: [PATCH 5/5] Sonypi: use pci_get_device


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


[email protected], 2004-10-21 01:48:41-05:00, [email protected]
Sonypi: switch from pci_find_device to pci_get_device.

Signed-off-by: Dmitry Torokhov <[email protected]>


sonypi.c | 15 +++++++++------
1 files changed, 9 insertions(+), 6 deletions(-)


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



diff -Nru a/drivers/char/sonypi.c b/drivers/char/sonypi.c
--- a/drivers/char/sonypi.c 2004-10-21 01:54:57 -05:00
+++ b/drivers/char/sonypi.c 2004-10-21 01:54:57 -05:00
@@ -750,11 +750,16 @@
.shutdown = sonypi_shutdown,
};

-static int __devinit sonypi_probe(struct pci_dev *pcidev)
+static int __devinit sonypi_probe(void)
{
int i, ret;
struct sonypi_ioport_list *ioport_list;
struct sonypi_irq_list *irq_list;
+ struct pci_dev *pcidev;
+
+ pcidev = pci_find_device(PCI_VENDOR_ID_INTEL,
+ PCI_DEVICE_ID_INTEL_82371AB_3,
+ NULL);

spin_lock_init(&sonypi_device.queue.s_lock);
init_waitqueue_head(&sonypi_device.proc_list);
@@ -881,6 +886,7 @@
out2:
misc_deregister(&sonypi_misc_device);
out1:
+ pci_dev_put(sonypi_device.dev);
return ret;
}

@@ -896,6 +902,7 @@
platform_device_unregister(sonypi_device.pdev);
free_irq(sonypi_device.irq, sonypi_irq);
release_region(sonypi_device.ioport1, sonypi_device.region_size);
+ pci_dev_put(sonypi_device.dev);
misc_deregister(&sonypi_misc_device);
printk(KERN_INFO "sonypi: removed.\n");
}
@@ -920,7 +927,6 @@

static int __init sonypi_init_module(void)
{
- struct pci_dev *pcidev;
int ret;

if (!dmi_check_system(sonypi_dmi_table))
@@ -930,10 +936,7 @@
if (ret)
return ret;

- pcidev = pci_find_device(PCI_VENDOR_ID_INTEL,
- PCI_DEVICE_ID_INTEL_82371AB_3,
- NULL);
- ret = sonypi_probe(pcidev);
+ ret = sonypi_probe();
if (ret)
driver_unregister(&sonypi_driver);

2004-10-21 07:06:37

by Dmitry Torokhov

[permalink] [raw]
Subject: [PATCH 2/5] Sonypi: switch to module_param


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


[email protected], 2004-10-21 01:44:25-05:00, [email protected]
Sonypi: convert to use new style of mocule parameters (module_param).
See Documentation/kernel-parameters.txt for new syntax.

Signed-off-by: Dmitry Torokhov <[email protected]>


Documentation/kernel-parameters.txt | 19 +++++
drivers/char/sonypi.c | 122 ++++++++++++++++++------------------
drivers/char/sonypi.h | 31 ---------
3 files changed, 81 insertions(+), 91 deletions(-)


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



diff -Nru a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
--- a/Documentation/kernel-parameters.txt 2004-10-21 01:54:11 -05:00
+++ b/Documentation/kernel-parameters.txt 2004-10-21 01:54:11 -05:00
@@ -1219,8 +1219,23 @@
sonycd535= [HW,CD]
Format: <io>[,<irq>]

- sonypi= [HW] Sony Programmable I/O Control Device driver
- Format: <minor>,<verbose>,<fnkeyinit>,<camera>,<compat>,<nojogdial>
+ sonypi.* [HW] Sony Programmable I/O Control Device driver
+ camera Activate control of MotionEye camera, default is 0 (off)
+ Format: <camera> - boolean, default is off
+ compat Trun on compatibility mode
+ Format: <compat> - boolean, default is off
+ fnkeyinit
+ Force initialization of FN-keys, default is 0 (off)
+ Format: <fnkeyinit> - boolean, default is off
+ mask= Event mask
+ Format: <minor> - integer, default is 0xffffffff (all)
+ minor= Minor number of the misc device
+ Format: <minor> - integer, default is -1 (auto)
+ useinput
+ Report events from jogdial through input core,
+ Format: <useinput> - boolean, default is on
+ verbose Verbose event reporting
+ Format: <verbose> - boolean, default is off

specialix= [HW,SERIAL] Specialix multi-serial port adapter
See Documentation/specialix.txt.
diff -Nru a/drivers/char/sonypi.c b/drivers/char/sonypi.c
--- a/drivers/char/sonypi.c 2004-10-21 01:54:11 -05:00
+++ b/drivers/char/sonypi.c 2004-10-21 01:54:11 -05:00
@@ -50,18 +50,46 @@
#include <asm/io.h>
#include <asm/system.h>

-static int verbose; /* = 0 */
-
#include "sonypi.h"
#include <linux/sonypi.h>

-static struct sonypi_device sonypi_device;
+MODULE_AUTHOR("Stelian Pop <[email protected]>");
+MODULE_DESCRIPTION("Sony Programmable I/O Control Device driver");
+MODULE_LICENSE("GPL");
+
static int minor = -1;
+module_param(minor, uint, 0);
+MODULE_PARM_DESC(minor, "minor number of the misc device, default is -1 (automatic)");
+
+static int verbose; /* = 0 */
+module_param(verbose, bool, 600);
+MODULE_PARM_DESC(verbose, "be verbose, default is 0 (no)");
+
static int fnkeyinit; /* = 0 */
+module_param(fnkeyinit, bool, 0);
+MODULE_PARM_DESC(fnkeyinit, "set this if your Fn keys do not generate any event");
+
static int camera; /* = 0 */
+module_param(camera, bool, 0);
+MODULE_PARM_DESC(camera, "set this if you have a MotionEye camera (PictureBook series)");
+
static int compat; /* = 0 */
-static int useinput = 1;
+module_param(compat, bool, 0);
+MODULE_PARM_DESC(compat, "set this if you want to enable backward compatibility mode");
+
static unsigned long mask = 0xffffffff;
+module_param(mask, ulong, 600);
+MODULE_PARM_DESC(mask, "set this to the mask of event you want to enable (see doc)");
+
+#ifdef SONYPI_USE_INPUT
+static int useinput = 1;
+module_param(useinput, bool, 0);
+MODULE_PARM_DESC(useinput, "if you have a jogdial, set this if you would like it to use the modern Linux Input Driver system");
+#else
+static int useinput; /* = 0 */
+#endif
+
+static struct sonypi_device sonypi_device;

/* Inits the queue */
static inline void sonypi_initq(void)
@@ -125,6 +153,38 @@
return result;
}

+static int sonypi_ec_write(u8 addr, u8 value)
+{
+#ifdef CONFIG_ACPI_EC
+ if (SONYPI_ACPI_ACTIVE)
+ return ec_write(addr, value);
+#endif
+ wait_on_command(1, inb_p(SONYPI_CST_IOPORT) & 3, ITERATIONS_LONG);
+ outb_p(0x81, SONYPI_CST_IOPORT);
+ wait_on_command(0, inb_p(SONYPI_CST_IOPORT) & 2, ITERATIONS_LONG);
+ outb_p(addr, SONYPI_DATA_IOPORT);
+ wait_on_command(0, inb_p(SONYPI_CST_IOPORT) & 2, ITERATIONS_LONG);
+ outb_p(value, SONYPI_DATA_IOPORT);
+ wait_on_command(0, inb_p(SONYPI_CST_IOPORT) & 2, ITERATIONS_LONG);
+ return 0;
+}
+
+static int sonypi_ec_read(u8 addr, u8 *value)
+{
+#ifdef CONFIG_ACPI_EC
+ if (SONYPI_ACPI_ACTIVE)
+ return ec_read(addr, value);
+#endif
+ wait_on_command(1, inb_p(SONYPI_CST_IOPORT) & 3, ITERATIONS_LONG);
+ outb_p(0x80, SONYPI_CST_IOPORT);
+ wait_on_command(0, inb_p(SONYPI_CST_IOPORT) & 2, ITERATIONS_LONG);
+ outb_p(addr, SONYPI_DATA_IOPORT);
+ wait_on_command(0, inb_p(SONYPI_CST_IOPORT) & 2, ITERATIONS_LONG);
+ *value = inb_p(SONYPI_DATA_IOPORT);
+ return 0;
+}
+
+
static int ec_read16(u8 addr, u16 *value)
{
u8 val_lb, val_hb;
@@ -932,62 +992,8 @@
sonypi_remove();
}

-#ifndef MODULE
-static int __init sonypi_setup(char *str)
-{
- int ints[8];
-
- str = get_options(str, ARRAY_SIZE(ints), ints);
- if (ints[0] <= 0)
- goto out;
- minor = ints[1];
- if (ints[0] == 1)
- goto out;
- verbose = ints[2];
- if (ints[0] == 2)
- goto out;
- fnkeyinit = ints[3];
- if (ints[0] == 3)
- goto out;
- camera = ints[4];
- if (ints[0] == 4)
- goto out;
- compat = ints[5];
- if (ints[0] == 5)
- goto out;
- mask = ints[6];
- if (ints[0] == 6)
- goto out;
- useinput = ints[7];
-out:
- return 1;
-}
-
-__setup("sonypi=", sonypi_setup);
-#endif /* !MODULE */
-
/* Module entry points */
module_init(sonypi_init_module);
module_exit(sonypi_cleanup_module);
-
-MODULE_AUTHOR("Stelian Pop <[email protected]>");
-MODULE_DESCRIPTION("Sony Programmable I/O Control Device driver");
-MODULE_LICENSE("GPL");
-
-
-MODULE_PARM(minor,"i");
-MODULE_PARM_DESC(minor, "minor number of the misc device, default is -1 (automatic)");
-MODULE_PARM(verbose,"i");
-MODULE_PARM_DESC(verbose, "be verbose, default is 0 (no)");
-MODULE_PARM(fnkeyinit,"i");
-MODULE_PARM_DESC(fnkeyinit, "set this if your Fn keys do not generate any event");
-MODULE_PARM(camera,"i");
-MODULE_PARM_DESC(camera, "set this if you have a MotionEye camera (PictureBook series)");
-MODULE_PARM(compat,"i");
-MODULE_PARM_DESC(compat, "set this if you want to enable backward compatibility mode");
-MODULE_PARM(mask, "i");
-MODULE_PARM_DESC(mask, "set this to the mask of event you want to enable (see doc)");
-MODULE_PARM(useinput, "i");
-MODULE_PARM_DESC(useinput, "if you have a jogdial, set this if you would like it to use the modern Linux Input Driver system");

EXPORT_SYMBOL(sonypi_camera_command);
diff -Nru a/drivers/char/sonypi.h b/drivers/char/sonypi.h
--- a/drivers/char/sonypi.h 2004-10-21 01:54:10 -05:00
+++ b/drivers/char/sonypi.h 2004-10-21 01:54:10 -05:00
@@ -401,37 +401,6 @@
#define SONYPI_ACPI_ACTIVE 0
#endif /* CONFIG_ACPI */

-static inline int sonypi_ec_write(u8 addr, u8 value)
-{
-#ifdef CONFIG_ACPI_EC
- if (SONYPI_ACPI_ACTIVE)
- return ec_write(addr, value);
-#endif
- wait_on_command(1, inb_p(SONYPI_CST_IOPORT) & 3, ITERATIONS_LONG);
- outb_p(0x81, SONYPI_CST_IOPORT);
- wait_on_command(0, inb_p(SONYPI_CST_IOPORT) & 2, ITERATIONS_LONG);
- outb_p(addr, SONYPI_DATA_IOPORT);
- wait_on_command(0, inb_p(SONYPI_CST_IOPORT) & 2, ITERATIONS_LONG);
- outb_p(value, SONYPI_DATA_IOPORT);
- wait_on_command(0, inb_p(SONYPI_CST_IOPORT) & 2, ITERATIONS_LONG);
- return 0;
-}
-
-static inline int sonypi_ec_read(u8 addr, u8 *value)
-{
-#ifdef CONFIG_ACPI_EC
- if (SONYPI_ACPI_ACTIVE)
- return ec_read(addr, value);
-#endif
- wait_on_command(1, inb_p(SONYPI_CST_IOPORT) & 3, ITERATIONS_LONG);
- outb_p(0x80, SONYPI_CST_IOPORT);
- wait_on_command(0, inb_p(SONYPI_CST_IOPORT) & 2, ITERATIONS_LONG);
- outb_p(addr, SONYPI_DATA_IOPORT);
- wait_on_command(0, inb_p(SONYPI_CST_IOPORT) & 2, ITERATIONS_LONG);
- *value = inb_p(SONYPI_DATA_IOPORT);
- return 0;
-}
-
#endif /* __KERNEL__ */

#endif /* _SONYPI_PRIV_H_ */

2004-10-21 07:08:47

by Dmitry Torokhov

[permalink] [raw]
Subject: [PATCH 3/5] Sonypi: switch from sysdev to platform device, drop old-style PM code


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


[email protected], 2004-10-21 01:45:45-05:00, [email protected]
Sonypi: power management changes
- convert from system device to platform device, it surely
can suspend with interrupts enabled;
- drop old style PM implementation as APM calls device_suspend
anyway.

Signed-off-by: Dmitry Torokhov <[email protected]>


sonypi.c | 198 +++++++++++++++++++++++++--------------------------------------
sonypi.h | 5 -
2 files changed, 82 insertions(+), 121 deletions(-)


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



diff -Nru a/drivers/char/sonypi.c b/drivers/char/sonypi.c
--- a/drivers/char/sonypi.c 2004-10-21 01:54:27 -05:00
+++ b/drivers/char/sonypi.c 2004-10-21 01:54:27 -05:00
@@ -44,7 +44,7 @@
#include <linux/wait.h>
#include <linux/acpi.h>
#include <linux/dmi.h>
-#include <linux/sysdev.h>
+#include <linux/err.h>

#include <asm/uaccess.h>
#include <asm/io.h>
@@ -688,72 +688,78 @@
-1, "sonypi", &sonypi_misc_fops
};

-#ifdef CONFIG_PM
-static int old_camera_power;
+static void sonypi_enable(unsigned int camera_on)
+{
+ if (sonypi_device.model == SONYPI_DEVICE_MODEL_TYPE2)
+ sonypi_type2_srs();
+ else
+ sonypi_type1_srs();
+
+ sonypi_call1(0x82);
+ sonypi_call2(0x81, 0xff);
+ sonypi_call1(compat ? 0x92 : 0x82);
+
+ /* Enable ACPI mode to get Fn key events */
+ if (!SONYPI_ACPI_ACTIVE && fnkeyinit)
+ outb(0xf0, 0xb2);
+
+ if (camera && camera_on)
+ sonypi_camera_on();
+}

-static int sonypi_suspend(struct sys_device *dev, u32 state)
+static void sonypi_disable(void)
{
sonypi_call2(0x81, 0); /* make sure we don't get any more events */
- if (camera) {
- old_camera_power = sonypi_device.camera_power;
+
+ if (camera)
sonypi_camera_off();
- }
+
+ /* disable ACPI mode */
+ if (!SONYPI_ACPI_ACTIVE && fnkeyinit)
+ outb(0xf1, 0xb2);
+
if (sonypi_device.model == SONYPI_DEVICE_MODEL_TYPE2)
sonypi_type2_dis();
else
sonypi_type1_dis();
- /* disable ACPI mode */
- if (!SONYPI_ACPI_ACTIVE && fnkeyinit)
- outb(0xf1, 0xb2);
- return 0;
}

-static int sonypi_resume(struct sys_device *dev)
+#ifdef CONFIG_PM
+static int old_camera_power;
+
+static int sonypi_suspend(struct device *dev, u32 state, u32 level)
{
- /* Enable ACPI mode to get Fn key events */
- if (!SONYPI_ACPI_ACTIVE && fnkeyinit)
- outb(0xf0, 0xb2);
- if (sonypi_device.model == SONYPI_DEVICE_MODEL_TYPE2)
- sonypi_type2_srs();
- else
- sonypi_type1_srs();
- sonypi_call1(0x82);
- sonypi_call2(0x81, 0xff);
- if (compat)
- sonypi_call1(0x92);
- else
- sonypi_call1(0x82);
- if (camera && old_camera_power)
- sonypi_camera_on();
+ if (level == SUSPEND_DISABLE) {
+ old_camera_power = sonypi_device.camera_power;
+ sonypi_disable();
+ }
+
return 0;
}

-/* Old PM scheme */
-static int sonypi_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data)
+static int sonypi_resume(struct device *dev, u32 level)
{
- switch (rqst) {
- case PM_SUSPEND:
- sonypi_suspend(NULL, 0);
- break;
- case PM_RESUME:
- sonypi_resume(NULL);
- break;
- }
+ if (level == RESUME_ENABLE)
+ sonypi_enable(old_camera_power);
+
return 0;
}
+#endif

-/* New PM scheme (device model) */
-static struct sysdev_class sonypi_sysclass = {
- set_kset_name("sonypi"),
- .suspend = sonypi_suspend,
- .resume = sonypi_resume,
-};
+static void sonypi_shutdown(struct device *dev)
+{
+ sonypi_disable();
+}

-static struct sys_device sonypi_sysdev = {
- .id = 0,
- .cls = &sonypi_sysclass,
-};
+static struct device_driver sonypi_driver = {
+ .name = "sonypi",
+ .bus = &platform_bus_type,
+#ifdef CONFIG_PM
+ .suspend = sonypi_suspend,
+ .resume = sonypi_resume,
#endif
+ .shutdown = sonypi_shutdown,
+};

static int __devinit sonypi_probe(struct pci_dev *pcidev)
{
@@ -762,10 +768,8 @@
struct sonypi_irq_list *irq_list;

sonypi_device.dev = pcidev;
- if (pcidev)
- sonypi_device.model = SONYPI_DEVICE_MODEL_TYPE1;
- else
- sonypi_device.model = SONYPI_DEVICE_MODEL_TYPE2;
+ sonypi_device.model = pcidev ?
+ SONYPI_DEVICE_MODEL_TYPE1 : SONYPI_DEVICE_MODEL_TYPE2;
sonypi_initq();
init_MUTEX(&sonypi_device.lock);
sonypi_device.bluetooth_power = 0;
@@ -788,8 +792,7 @@
sonypi_device.region_size = SONYPI_TYPE2_REGION_SIZE;
sonypi_device.evtype_offset = SONYPI_TYPE2_EVTYPE_OFFSET;
irq_list = sonypi_type2_irq_list;
- }
- else {
+ } else {
ioport_list = sonypi_type1_ioport_list;
sonypi_device.region_size = SONYPI_TYPE1_REGION_SIZE;
sonypi_device.evtype_offset = SONYPI_TYPE1_EVTYPE_OFFSET;
@@ -806,6 +809,7 @@
break;
}
}
+
if (!sonypi_device.ioport1) {
printk(KERN_ERR "sonypi: request_region failed\n");
ret = -ENODEV;
@@ -817,29 +821,10 @@
sonypi_device.irq = irq_list[i].irq;
sonypi_device.bits = irq_list[i].bits;

- /* Enable sonypi IRQ settings */
- if (sonypi_device.model == SONYPI_DEVICE_MODEL_TYPE2)
- sonypi_type2_srs();
- else
- sonypi_type1_srs();
-
- sonypi_call1(0x82);
- sonypi_call2(0x81, 0xff);
- if (compat)
- sonypi_call1(0x92);
- else
- sonypi_call1(0x82);
-
/* Now try requesting the irq from the system */
if (!request_irq(sonypi_device.irq, sonypi_irq,
SA_SHIRQ, "sonypi", sonypi_irq))
break;
-
- /* If request_irq failed, disable sonypi IRQ settings */
- if (sonypi_device.model == SONYPI_DEVICE_MODEL_TYPE2)
- sonypi_type2_dis();
- else
- sonypi_type1_dis();
}

if (!irq_list[i].irq) {
@@ -848,24 +833,13 @@
goto out3;
}

-#ifdef CONFIG_PM
- sonypi_device.pm = pm_register(PM_PCI_DEV, 0, sonypi_pm_callback);
-
- if (sysdev_class_register(&sonypi_sysclass) != 0) {
- printk(KERN_ERR "sonypi: sysdev_class_register failed\n");
- ret = -ENODEV;
+ sonypi_device.pdev = platform_device_register_simple("sonypi", -1, NULL, 0);
+ if (IS_ERR(sonypi_device.pdev)) {
+ ret = PTR_ERR(sonypi_device.pdev);
goto out4;
}
- if (sysdev_register(&sonypi_sysdev) != 0) {
- printk(KERN_ERR "sonypi: sysdev_register failed\n");
- ret = -ENODEV;
- goto out5;
- }
-#endif

- /* Enable ACPI mode to get Fn key events */
- if (!SONYPI_ACPI_ACTIVE && fnkeyinit)
- outb(0xf0, 0xb2);
+ sonypi_enable(0);

printk(KERN_INFO "sonypi: Sony Programmable I/O Controller Driver v%d.%d.\n",
SONYPI_DRIVER_MAJORVERSION,
@@ -909,12 +883,8 @@

return 0;

-#ifdef CONFIG_PM
-out5:
- sysdev_class_unregister(&sonypi_sysclass);
out4:
free_irq(sonypi_device.irq, sonypi_irq);
-#endif
out3:
release_region(sonypi_device.ioport1, sonypi_device.region_size);
out2:
@@ -925,15 +895,6 @@

static void __devexit sonypi_remove(void)
{
-#ifdef CONFIG_PM
- pm_unregister(sonypi_device.pm);
-
- sysdev_unregister(&sonypi_sysdev);
- sysdev_class_unregister(&sonypi_sysclass);
-#endif
-
- sonypi_call2(0x81, 0); /* make sure we don't get any more events */
-
#ifdef SONYPI_USE_INPUT
if (useinput) {
input_unregister_device(&sonypi_device.jog_dev);
@@ -941,15 +902,7 @@
}
#endif /* SONYPI_USE_INPUT */

- if (camera)
- sonypi_camera_off();
- if (sonypi_device.model == SONYPI_DEVICE_MODEL_TYPE2)
- sonypi_type2_dis();
- else
- sonypi_type1_dis();
- /* disable ACPI mode */
- if (!SONYPI_ACPI_ACTIVE && fnkeyinit)
- outb(0xf1, 0xb2);
+ platform_device_unregister(sonypi_device.pdev);
free_irq(sonypi_device.irq, sonypi_irq);
release_region(sonypi_device.ioport1, sonypi_device.region_size);
misc_deregister(&sonypi_misc_device);
@@ -976,20 +929,31 @@

static int __init sonypi_init_module(void)
{
- struct pci_dev *pcidev = NULL;
- if (dmi_check_system(sonypi_dmi_table)) {
- pcidev = pci_find_device(PCI_VENDOR_ID_INTEL,
- PCI_DEVICE_ID_INTEL_82371AB_3,
- NULL);
- return sonypi_probe(pcidev);
- }
- else
+ struct pci_dev *pcidev;
+ int ret;
+
+ if (!dmi_check_system(sonypi_dmi_table))
return -ENODEV;
+
+ ret = driver_register(&sonypi_driver);
+ if (ret)
+ return ret;
+
+ pcidev = pci_find_device(PCI_VENDOR_ID_INTEL,
+ PCI_DEVICE_ID_INTEL_82371AB_3,
+ NULL);
+ ret = sonypi_probe(pcidev);
+ if (ret)
+ driver_unregister(&sonypi_driver);
+
+ return ret;
}

static void __exit sonypi_cleanup_module(void)
{
+ sonypi_disable();
sonypi_remove();
+ driver_unregister(&sonypi_driver);
}

/* Module entry points */
diff -Nru a/drivers/char/sonypi.h b/drivers/char/sonypi.h
--- a/drivers/char/sonypi.h 2004-10-21 01:54:27 -05:00
+++ b/drivers/char/sonypi.h 2004-10-21 01:54:27 -05:00
@@ -46,7 +46,6 @@
#include <linux/types.h>
#include <linux/pci.h>
#include <linux/input.h>
-#include <linux/pm.h>
#include <linux/acpi.h>
#include "linux/sonypi.h"

@@ -364,6 +363,7 @@

struct sonypi_device {
struct pci_dev *dev;
+ struct platform_device *pdev;
u16 irq;
u16 bits;
u16 ioport1;
@@ -378,9 +378,6 @@
int model;
#ifdef SONYPI_USE_INPUT
struct input_dev jog_dev;
-#endif
-#ifdef CONFIG_PM
- struct pm_dev *pm;
#endif
};

2004-10-21 07:12:23

by Dmitry Torokhov

[permalink] [raw]
Subject: [PATCH 1/5] Sonypi: whitespace fixes


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


[email protected], 2004-10-21 01:43:04-05:00, [email protected]
Sonypi: whitespace and coding style fixes.

Signed-off-by: Dmitry Torokhov <[email protected]>


drivers/char/sonypi.c | 153 +++++++++++++++++++++++++++++--------------------
drivers/char/sonypi.h | 18 +++--
include/linux/sonypi.h | 10 +--
3 files changed, 106 insertions(+), 75 deletions(-)


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



diff -Nru a/drivers/char/sonypi.c b/drivers/char/sonypi.c
--- a/drivers/char/sonypi.c 2004-10-21 01:53:49 -05:00
+++ b/drivers/char/sonypi.c 2004-10-21 01:53:49 -05:00
@@ -19,12 +19,12 @@
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
- *
+ *
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
- *
+ *
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
@@ -64,7 +64,8 @@
static unsigned long mask = 0xffffffff;

/* Inits the queue */
-static inline void sonypi_initq(void) {
+static inline void sonypi_initq(void)
+{
sonypi_device.queue.head = sonypi_device.queue.tail = 0;
sonypi_device.queue.len = 0;
sonypi_device.queue.s_lock = SPIN_LOCK_UNLOCKED;
@@ -72,7 +73,8 @@
}

/* Pulls an event from the queue */
-static inline unsigned char sonypi_pullq(void) {
+static inline unsigned char sonypi_pullq(void)
+{
unsigned char result;
unsigned long flags;

@@ -90,7 +92,8 @@
}

/* Pushes an event into the queue */
-static inline void sonypi_pushq(unsigned char event) {
+static inline void sonypi_pushq(unsigned char event)
+{
unsigned long flags;

spin_lock_irqsave(&sonypi_device.queue.s_lock, flags);
@@ -111,7 +114,8 @@
}

/* Tests if the queue is empty */
-static inline int sonypi_emptyq(void) {
+static inline int sonypi_emptyq(void)
+{
int result;
unsigned long flags;

@@ -121,7 +125,8 @@
return result;
}

-static int ec_read16(u8 addr, u16 *value) {
+static int ec_read16(u8 addr, u16 *value)
+{
u8 val_lb, val_hb;
if (sonypi_ec_read(addr, &val_lb))
return -1;
@@ -132,7 +137,8 @@
}

/* Initializes the device - this comes from the AML code in the ACPI bios */
-static void sonypi_type1_srs(void) {
+static void sonypi_type1_srs(void)
+{
u32 v;

pci_read_config_dword(sonypi_device.dev, SONYPI_G10A, &v);
@@ -140,7 +146,7 @@
pci_write_config_dword(sonypi_device.dev, SONYPI_G10A, v);

pci_read_config_dword(sonypi_device.dev, SONYPI_G10A, &v);
- v = (v & 0xFFF0FFFF) |
+ v = (v & 0xFFF0FFFF) |
(((u32)sonypi_device.ioport1 ^ sonypi_device.ioport2) << 16);
pci_write_config_dword(sonypi_device.dev, SONYPI_G10A, v);

@@ -154,7 +160,8 @@
pci_write_config_dword(sonypi_device.dev, SONYPI_G10A, v);
}

-static void sonypi_type2_srs(void) {
+static void sonypi_type2_srs(void)
+{
if (sonypi_ec_write(SONYPI_SHIB, (sonypi_device.ioport1 & 0xFF00) >> 8))
printk(KERN_WARNING "ec_write failed\n");
if (sonypi_ec_write(SONYPI_SLOB, sonypi_device.ioport1 & 0x00FF))
@@ -165,7 +172,8 @@
}

/* Disables the device - this comes from the AML code in the ACPI bios */
-static void sonypi_type1_dis(void) {
+static void sonypi_type1_dis(void)
+{
u32 v;

pci_read_config_dword(sonypi_device.dev, SONYPI_G10A, &v);
@@ -177,7 +185,8 @@
outl(v, SONYPI_IRQ_PORT);
}

-static void sonypi_type2_dis(void) {
+static void sonypi_type2_dis(void)
+{
if (sonypi_ec_write(SONYPI_SHIB, 0))
printk(KERN_WARNING "ec_write failed\n");
if (sonypi_ec_write(SONYPI_SLOB, 0))
@@ -186,7 +195,8 @@
printk(KERN_WARNING "ec_write failed\n");
}

-static u8 sonypi_call1(u8 dev) {
+static u8 sonypi_call1(u8 dev)
+{
u8 v1, v2;

wait_on_command(0, inb_p(sonypi_device.ioport2) & 2, ITERATIONS_LONG);
@@ -196,7 +206,8 @@
return v2;
}

-static u8 sonypi_call2(u8 dev, u8 fn) {
+static u8 sonypi_call2(u8 dev, u8 fn)
+{
u8 v1;

wait_on_command(0, inb_p(sonypi_device.ioport2) & 2, ITERATIONS_LONG);
@@ -207,7 +218,8 @@
return v1;
}

-static u8 sonypi_call3(u8 dev, u8 fn, u8 v) {
+static u8 sonypi_call3(u8 dev, u8 fn, u8 v)
+{
u8 v1;

wait_on_command(0, inb_p(sonypi_device.ioport2) & 2, ITERATIONS_LONG);
@@ -220,7 +232,8 @@
return v1;
}

-static u8 sonypi_read(u8 fn) {
+static u8 sonypi_read(u8 fn)
+{
u8 v1, v2;
int n = 100;

@@ -234,13 +247,14 @@
}

/* Set brightness, hue etc */
-static void sonypi_set(u8 fn, u8 v) {
-
+static void sonypi_set(u8 fn, u8 v)
+{
wait_on_command(0, sonypi_call3(0x90, fn, v), ITERATIONS_SHORT);
}

/* Tests if the camera is ready */
-static int sonypi_camera_ready(void) {
+static int sonypi_camera_ready(void)
+{
u8 v;

v = sonypi_call2(0x8f, SONYPI_CAMERA_STATUS);
@@ -248,19 +262,20 @@
}

/* Turns the camera off */
-static void sonypi_camera_off(void) {
-
+static void sonypi_camera_off(void)
+{
sonypi_set(SONYPI_CAMERA_PICTURE, SONYPI_CAMERA_MUTE_MASK);

if (!sonypi_device.camera_power)
return;

- sonypi_call2(0x91, 0);
+ sonypi_call2(0x91, 0);
sonypi_device.camera_power = 0;
}

/* Turns the camera on */
-static void sonypi_camera_on(void) {
+static void sonypi_camera_on(void)
+{
int i, j;

if (sonypi_device.camera_power)
@@ -283,7 +298,7 @@
if (i)
break;
}
-
+
if (j == 0) {
printk(KERN_WARNING "sonypi: failed to power on camera\n");
return;
@@ -294,19 +309,20 @@
}

/* sets the bluetooth subsystem power state */
-static void sonypi_setbluetoothpower(u8 state) {
-
+static void sonypi_setbluetoothpower(u8 state)
+{
state = !!state;
- if (sonypi_device.bluetooth_power == state)
+ if (sonypi_device.bluetooth_power == state)
return;
-
+
sonypi_call2(0x96, state);
sonypi_call1(0x82);
sonypi_device.bluetooth_power = state;
}

/* Interrupt handler: some event is available */
-static irqreturn_t sonypi_irq(int irq, void *dev_id, struct pt_regs *regs) {
+static irqreturn_t sonypi_irq(int irq, void *dev_id, struct pt_regs *regs)
+{
u8 v1, v2, event = 0;
int i, j;

@@ -329,16 +345,17 @@
}

if (verbose)
- printk(KERN_WARNING
+ printk(KERN_WARNING
"sonypi: unknown event port1=0x%02x,port2=0x%02x\n",v1,v2);
+
/* We need to return IRQ_HANDLED here because there *are*
- * events belonging to the sonypi device we don't know about,
+ * events belonging to the sonypi device we don't know about,
* but we still don't want those to pollute the logs... */
return IRQ_HANDLED;

found:
if (verbose > 1)
- printk(KERN_INFO
+ printk(KERN_INFO
"sonypi: event port1=0x%02x,port2=0x%02x\n", v1, v2);

#ifdef SONYPI_USE_INPUT
@@ -357,12 +374,14 @@
input_sync(jog_dev);
}
#endif /* SONYPI_USE_INPUT */
+
sonypi_pushq(event);
return IRQ_HANDLED;
}

/* External camera command (exported to the motion eye v4l driver) */
-u8 sonypi_camera_command(int command, u8 value) {
+u8 sonypi_camera_command(int command, u8 value)
+{
u8 ret = 0;

if (!camera)
@@ -437,7 +456,8 @@
return ret;
}

-static int sonypi_misc_fasync(int fd, struct file *filp, int on) {
+static int sonypi_misc_fasync(int fd, struct file *filp, int on)
+{
int retval;

retval = fasync_helper(fd, filp, on, &sonypi_device.queue.fasync);
@@ -446,7 +466,8 @@
return 0;
}

-static int sonypi_misc_release(struct inode * inode, struct file * file) {
+static int sonypi_misc_release(struct inode * inode, struct file * file)
+{
sonypi_misc_fasync(-1, file, 0);
down(&sonypi_device.lock);
sonypi_device.open_count--;
@@ -454,7 +475,8 @@
return 0;
}

-static int sonypi_misc_open(struct inode * inode, struct file * file) {
+static int sonypi_misc_open(struct inode * inode, struct file * file)
+{
down(&sonypi_device.lock);
/* Flush input queue on first open */
if (!sonypi_device.open_count)
@@ -498,15 +520,17 @@
return 0;
}

-static unsigned int sonypi_misc_poll(struct file *file, poll_table * wait) {
+static unsigned int sonypi_misc_poll(struct file *file, poll_table * wait)
+{
poll_wait(file, &sonypi_device.queue.proc_list, wait);
if (!sonypi_emptyq())
return POLLIN | POLLRDNORM;
return 0;
}

-static int sonypi_misc_ioctl(struct inode *ip, struct file *fp,
- unsigned int cmd, unsigned long arg) {
+static int sonypi_misc_ioctl(struct inode *ip, struct file *fp,
+ unsigned int cmd, unsigned long arg)
+{
int ret = 0;
void __user *argp = (void __user *)arg;
u8 val8;
@@ -607,7 +631,8 @@
#ifdef CONFIG_PM
static int old_camera_power;

-static int sonypi_suspend(struct sys_device *dev, u32 state) {
+static int sonypi_suspend(struct sys_device *dev, u32 state)
+{
sonypi_call2(0x81, 0); /* make sure we don't get any more events */
if (camera) {
old_camera_power = sonypi_device.camera_power;
@@ -623,7 +648,8 @@
return 0;
}

-static int sonypi_resume(struct sys_device *dev) {
+static int sonypi_resume(struct sys_device *dev)
+{
/* Enable ACPI mode to get Fn key events */
if (!SONYPI_ACPI_ACTIVE && fnkeyinit)
outb(0xf0, 0xb2);
@@ -634,7 +660,7 @@
sonypi_call1(0x82);
sonypi_call2(0x81, 0xff);
if (compat)
- sonypi_call1(0x92);
+ sonypi_call1(0x92);
else
sonypi_call1(0x82);
if (camera && old_camera_power)
@@ -643,8 +669,8 @@
}

/* Old PM scheme */
-static int sonypi_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data) {
-
+static int sonypi_pm_callback(struct pm_dev *dev, pm_request_t rqst, void *data)
+{
switch (rqst) {
case PM_SUSPEND:
sonypi_suspend(NULL, 0);
@@ -669,7 +695,8 @@
};
#endif

-static int __devinit sonypi_probe(struct pci_dev *pcidev) {
+static int __devinit sonypi_probe(struct pci_dev *pcidev)
+{
int i, ret;
struct sonypi_ioport_list *ioport_list;
struct sonypi_irq_list *irq_list;
@@ -682,14 +709,14 @@
sonypi_initq();
init_MUTEX(&sonypi_device.lock);
sonypi_device.bluetooth_power = 0;
-
+
if (pcidev && pci_enable_device(pcidev)) {
printk(KERN_ERR "sonypi: pci_enable_device failed\n");
ret = -EIO;
goto out1;
}

- sonypi_misc_device.minor = (minor == -1) ?
+ sonypi_misc_device.minor = (minor == -1) ?
MISC_DYNAMIC_MINOR : minor;
if ((ret = misc_register(&sonypi_misc_device))) {
printk(KERN_ERR "sonypi: misc_register failed\n");
@@ -710,8 +737,8 @@
}

for (i = 0; ioport_list[i].port1; i++) {
- if (request_region(ioport_list[i].port1,
- sonypi_device.region_size,
+ if (request_region(ioport_list[i].port1,
+ sonypi_device.region_size,
"Sony Programable I/O Device")) {
/* get the ioport */
sonypi_device.ioport1 = ioport_list[i].port1;
@@ -739,12 +766,12 @@
sonypi_call1(0x82);
sonypi_call2(0x81, 0xff);
if (compat)
- sonypi_call1(0x92);
+ sonypi_call1(0x92);
else
sonypi_call1(0x82);

/* Now try requesting the irq from the system */
- if (!request_irq(sonypi_device.irq, sonypi_irq,
+ if (!request_irq(sonypi_device.irq, sonypi_irq,
SA_SHIRQ, "sonypi", sonypi_irq))
break;

@@ -796,7 +823,7 @@
useinput ? "on" : "off",
SONYPI_ACPI_ACTIVE ? "on" : "off");
printk(KERN_INFO "sonypi: enabled at irq=%d, port1=0x%x, port2=0x%x\n",
- sonypi_device.irq,
+ sonypi_device.irq,
sonypi_device.ioport1, sonypi_device.ioport2);

if (minor == -1)
@@ -814,7 +841,7 @@
sprintf(sonypi_device.jog_dev.name, SONYPI_INPUTNAME);
sonypi_device.jog_dev.id.bustype = BUS_ISA;
sonypi_device.jog_dev.id.vendor = PCI_VENDOR_ID_SONY;
-
+
input_register_device(&sonypi_device.jog_dev);
printk(KERN_INFO "%s installed.\n", sonypi_device.jog_dev.name);
}
@@ -836,8 +863,8 @@
return ret;
}

-static void __devexit sonypi_remove(void) {
-
+static void __devexit sonypi_remove(void)
+{
#ifdef CONFIG_PM
pm_unregister(sonypi_device.pm);

@@ -846,7 +873,7 @@
#endif

sonypi_call2(0x81, 0); /* make sure we don't get any more events */
-
+
#ifdef SONYPI_USE_INPUT
if (useinput) {
input_unregister_device(&sonypi_device.jog_dev);
@@ -891,8 +918,8 @@
{
struct pci_dev *pcidev = NULL;
if (dmi_check_system(sonypi_dmi_table)) {
- pcidev = pci_find_device(PCI_VENDOR_ID_INTEL,
- PCI_DEVICE_ID_INTEL_82371AB_3,
+ pcidev = pci_find_device(PCI_VENDOR_ID_INTEL,
+ PCI_DEVICE_ID_INTEL_82371AB_3,
NULL);
return sonypi_probe(pcidev);
}
@@ -900,16 +927,18 @@
return -ENODEV;
}

-static void __exit sonypi_cleanup_module(void) {
+static void __exit sonypi_cleanup_module(void)
+{
sonypi_remove();
}

#ifndef MODULE
-static int __init sonypi_setup(char *str) {
+static int __init sonypi_setup(char *str)
+{
int ints[8];

str = get_options(str, ARRAY_SIZE(ints), ints);
- if (ints[0] <= 0)
+ if (ints[0] <= 0)
goto out;
minor = ints[1];
if (ints[0] == 1)
@@ -936,7 +965,7 @@

__setup("sonypi=", sonypi_setup);
#endif /* !MODULE */
-
+
/* Module entry points */
module_init(sonypi_init_module);
module_exit(sonypi_cleanup_module);
diff -Nru a/drivers/char/sonypi.h b/drivers/char/sonypi.h
--- a/drivers/char/sonypi.h 2004-10-21 01:53:49 -05:00
+++ b/drivers/char/sonypi.h 2004-10-21 01:53:49 -05:00
@@ -1,4 +1,4 @@
-/*
+/*
* Sony Programmable I/O Control Device driver for VAIO
*
* Copyright (C) 2001-2003 Stelian Pop <[email protected]>
@@ -14,24 +14,24 @@
* Copyright (C) 2000 Andrew Tridgell <[email protected]>
*
* Earlier work by Werner Almesberger, Paul `Rusty' Russell and Paul Mackerras.
- *
+ *
* 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, or
* (at your option) any later version.
- *
+ *
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
- *
+ *
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*
*/

-#ifndef _SONYPI_PRIV_H_
+#ifndef _SONYPI_PRIV_H_
#define _SONYPI_PRIV_H_

#ifdef __KERNEL__
@@ -350,7 +350,7 @@
unsigned char buf[SONYPI_BUF_SIZE];
};

-/* We enable input subsystem event forwarding if the input
+/* We enable input subsystem event forwarding if the input
* subsystem is compiled in, but only if sonypi is not into the
* kernel and input as a module... */
#if defined(CONFIG_INPUT) || defined(CONFIG_INPUT_MODULE)
@@ -401,7 +401,8 @@
#define SONYPI_ACPI_ACTIVE 0
#endif /* CONFIG_ACPI */

-static inline int sonypi_ec_write(u8 addr, u8 value) {
+static inline int sonypi_ec_write(u8 addr, u8 value)
+{
#ifdef CONFIG_ACPI_EC
if (SONYPI_ACPI_ACTIVE)
return ec_write(addr, value);
@@ -416,7 +417,8 @@
return 0;
}

-static inline int sonypi_ec_read(u8 addr, u8 *value) {
+static inline int sonypi_ec_read(u8 addr, u8 *value)
+{
#ifdef CONFIG_ACPI_EC
if (SONYPI_ACPI_ACTIVE)
return ec_read(addr, value);
diff -Nru a/include/linux/sonypi.h b/include/linux/sonypi.h
--- a/include/linux/sonypi.h 2004-10-21 01:53:49 -05:00
+++ b/include/linux/sonypi.h 2004-10-21 01:53:49 -05:00
@@ -1,4 +1,4 @@
-/*
+/*
* Sony Programmable I/O Control Device driver for VAIO
*
* Copyright (C) 2001-2003 Stelian Pop <[email protected]>
@@ -14,24 +14,24 @@
* Copyright (C) 2000 Andrew Tridgell <[email protected]>
*
* Earlier work by Werner Almesberger, Paul `Rusty' Russell and Paul Mackerras.
- *
+ *
* 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, or
* (at your option) any later version.
- *
+ *
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
- *
+ *
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*
*/

-#ifndef _SONYPI_H_
+#ifndef _SONYPI_H_
#define _SONYPI_H_

#include <linux/types.h>

2004-10-21 07:20:14

by Dmitry Torokhov

[permalink] [raw]
Subject: [PATCH 4/5] Sonypi: use wait_event_interruptible and other assorted changes


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


[email protected], 2004-10-21 01:47:57-05:00, [email protected]
Sonypi: miscellaneous fixes
- convert sonypi_misc_read to use wait_event_interruptible;
- fix race sonypi_misc_read when sonypi_emptyq reports queue
is not empty and other thread steals the byte before
sonypi_pullq is called;
- spin lock should be initialized at registration time, not
at open time because interrupt handler takes it and it may
come before misc device is opened;
- ensure that all parts of device is properly registered
before enabling it (used to register input device after
enabling interrupts from the hardware, could crash).

Signed-off-by: Dmitry Torokhov <[email protected]>


sonypi.c | 145 +++++++++++++++++++++++++++++----------------------------------
sonypi.h | 4 -
2 files changed, 70 insertions(+), 79 deletions(-)


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



diff -Nru a/drivers/char/sonypi.c b/drivers/char/sonypi.c
--- a/drivers/char/sonypi.c 2004-10-21 01:54:42 -05:00
+++ b/drivers/char/sonypi.c 2004-10-21 01:54:42 -05:00
@@ -94,40 +94,48 @@
/* Inits the queue */
static inline void sonypi_initq(void)
{
- sonypi_device.queue.head = sonypi_device.queue.tail = 0;
+ unsigned long flags;
+
+ spin_lock_irqsave(&sonypi_device.queue.s_lock, flags);
+ sonypi_device.queue.head = sonypi_device.queue.tail = 0;
sonypi_device.queue.len = 0;
- sonypi_device.queue.s_lock = SPIN_LOCK_UNLOCKED;
- init_waitqueue_head(&sonypi_device.queue.proc_list);
+ spin_unlock_irqrestore(&sonypi_device.queue.s_lock, flags);
+}
+
+/* Tests if the queue is empty */
+static inline int sonypi_emptyq(void)
+{
+ return sonypi_device.queue.len == 0;
}

/* Pulls an event from the queue */
-static inline unsigned char sonypi_pullq(void)
+static int sonypi_pullq(unsigned char *c)
{
- unsigned char result;
+ int have_data = 0;
unsigned long flags;

spin_lock_irqsave(&sonypi_device.queue.s_lock, flags);
- if (!sonypi_device.queue.len) {
- spin_unlock_irqrestore(&sonypi_device.queue.s_lock, flags);
- return 0;
+ if (!sonypi_emptyq()) {
+ *c = sonypi_device.queue.buf[sonypi_device.queue.head];
+ sonypi_device.queue.head++;
+ sonypi_device.queue.head &= (SONYPI_BUF_SIZE - 1);
+ sonypi_device.queue.len--;
+ have_data = 1;
}
- result = sonypi_device.queue.buf[sonypi_device.queue.head];
- sonypi_device.queue.head++;
- sonypi_device.queue.head &= (SONYPI_BUF_SIZE - 1);
- sonypi_device.queue.len--;
spin_unlock_irqrestore(&sonypi_device.queue.s_lock, flags);
- return result;
+
+ return have_data;
}

/* Pushes an event into the queue */
-static inline void sonypi_pushq(unsigned char event)
+static void sonypi_pushq(unsigned char event)
{
unsigned long flags;

spin_lock_irqsave(&sonypi_device.queue.s_lock, flags);
if (sonypi_device.queue.len == SONYPI_BUF_SIZE) {
/* remove the first element */
- sonypi_device.queue.head++;
+ sonypi_device.queue.head++;
sonypi_device.queue.head &= (SONYPI_BUF_SIZE - 1);
sonypi_device.queue.len--;
}
@@ -135,22 +143,10 @@
sonypi_device.queue.tail++;
sonypi_device.queue.tail &= (SONYPI_BUF_SIZE - 1);
sonypi_device.queue.len++;
-
- kill_fasync(&sonypi_device.queue.fasync, SIGIO, POLL_IN);
- wake_up_interruptible(&sonypi_device.queue.proc_list);
spin_unlock_irqrestore(&sonypi_device.queue.s_lock, flags);
-}
-
-/* Tests if the queue is empty */
-static inline int sonypi_emptyq(void)
-{
- int result;
- unsigned long flags;

- spin_lock_irqsave(&sonypi_device.queue.s_lock, flags);
- result = (sonypi_device.queue.len == 0);
- spin_unlock_irqrestore(&sonypi_device.queue.s_lock, flags);
- return result;
+ kill_fasync(&sonypi_device.fasync, SIGIO, POLL_IN);
+ wake_up_interruptible(&sonypi_device.proc_list);
}

static int sonypi_ec_write(u8 addr, u8 value)
@@ -520,7 +516,7 @@
{
int retval;

- retval = fasync_helper(fd, filp, on, &sonypi_device.queue.fasync);
+ retval = fasync_helper(fd, filp, on, &sonypi_device.fasync);
if (retval < 0)
return retval;
return 0;
@@ -549,40 +545,31 @@
static ssize_t sonypi_misc_read(struct file * file, char __user * buf,
size_t count, loff_t *pos)
{
- DECLARE_WAITQUEUE(wait, current);
- ssize_t i = count;
+ ssize_t ret;
unsigned char c;

- if (sonypi_emptyq()) {
- if (file->f_flags & O_NONBLOCK)
- return -EAGAIN;
- add_wait_queue(&sonypi_device.queue.proc_list, &wait);
-repeat:
- set_current_state(TASK_INTERRUPTIBLE);
- if (sonypi_emptyq() && !signal_pending(current)) {
- schedule();
- goto repeat;
- }
- current->state = TASK_RUNNING;
- remove_wait_queue(&sonypi_device.queue.proc_list, &wait);
- }
- while (i > 0 && !sonypi_emptyq()) {
- c = sonypi_pullq();
- put_user(c, buf++);
- i--;
+ if (sonypi_emptyq() && (file->f_flags & O_NONBLOCK))
+ return -EAGAIN;
+
+ ret = wait_event_interruptible(sonypi_device.proc_list, !sonypi_emptyq());
+ if (ret)
+ return ret;
+
+ while (ret < count && !sonypi_pullq(&c)) {
+ if (put_user(c, buf++))
+ return -EFAULT;
+ ret++;
}
- if (count - i) {
+
+ if (ret > 0)
file->f_dentry->d_inode->i_atime = CURRENT_TIME;
- return count-i;
- }
- if (signal_pending(current))
- return -ERESTARTSYS;
- return 0;
+
+ return ret;
}

static unsigned int sonypi_misc_poll(struct file *file, poll_table * wait)
{
- poll_wait(file, &sonypi_device.queue.proc_list, wait);
+ poll_wait(file, &sonypi_device.proc_list, wait);
if (!sonypi_emptyq())
return POLLIN | POLLRDNORM;
return 0;
@@ -685,7 +672,9 @@
};

struct miscdevice sonypi_misc_device = {
- -1, "sonypi", &sonypi_misc_fops
+ .minor = -1,
+ .name = "sonypi",
+ .fops = &sonypi_misc_fops,
};

static void sonypi_enable(unsigned int camera_on)
@@ -767,12 +756,14 @@
struct sonypi_ioport_list *ioport_list;
struct sonypi_irq_list *irq_list;

- sonypi_device.dev = pcidev;
- sonypi_device.model = pcidev ?
- SONYPI_DEVICE_MODEL_TYPE1 : SONYPI_DEVICE_MODEL_TYPE2;
+ spin_lock_init(&sonypi_device.queue.s_lock);
+ init_waitqueue_head(&sonypi_device.proc_list);
sonypi_initq();
init_MUTEX(&sonypi_device.lock);
sonypi_device.bluetooth_power = 0;
+ sonypi_device.dev = pcidev;
+ sonypi_device.model = pcidev ?
+ SONYPI_DEVICE_MODEL_TYPE1 : SONYPI_DEVICE_MODEL_TYPE2;

if (pcidev && pci_enable_device(pcidev)) {
printk(KERN_ERR "sonypi: pci_enable_device failed\n");
@@ -839,6 +830,23 @@
goto out4;
}

+#ifdef SONYPI_USE_INPUT
+ if (useinput) {
+ /* Initialize the Input Drivers: */
+ sonypi_device.jog_dev.evbit[0] = BIT(EV_KEY) | BIT(EV_REL);
+ sonypi_device.jog_dev.keybit[LONG(BTN_MOUSE)] = BIT(BTN_MIDDLE);
+ sonypi_device.jog_dev.relbit[0] = BIT(REL_WHEEL);
+ sonypi_device.jog_dev.name = (char *) kmalloc(
+ sizeof(SONYPI_INPUTNAME), GFP_KERNEL);
+ sprintf(sonypi_device.jog_dev.name, SONYPI_INPUTNAME);
+ sonypi_device.jog_dev.id.bustype = BUS_ISA;
+ sonypi_device.jog_dev.id.vendor = PCI_VENDOR_ID_SONY;
+
+ input_register_device(&sonypi_device.jog_dev);
+ printk(KERN_INFO "%s installed.\n", sonypi_device.jog_dev.name);
+ }
+#endif /* SONYPI_USE_INPUT */
+
sonypi_enable(0);

printk(KERN_INFO "sonypi: Sony Programmable I/O Controller Driver v%d.%d.\n",
@@ -863,23 +871,6 @@
if (minor == -1)
printk(KERN_INFO "sonypi: device allocated minor is %d\n",
sonypi_misc_device.minor);
-
-#ifdef SONYPI_USE_INPUT
- if (useinput) {
- /* Initialize the Input Drivers: */
- sonypi_device.jog_dev.evbit[0] = BIT(EV_KEY) | BIT(EV_REL);
- sonypi_device.jog_dev.keybit[LONG(BTN_MOUSE)] = BIT(BTN_MIDDLE);
- sonypi_device.jog_dev.relbit[0] = BIT(REL_WHEEL);
- sonypi_device.jog_dev.name = (char *) kmalloc(
- sizeof(SONYPI_INPUTNAME), GFP_KERNEL);
- sprintf(sonypi_device.jog_dev.name, SONYPI_INPUTNAME);
- sonypi_device.jog_dev.id.bustype = BUS_ISA;
- sonypi_device.jog_dev.id.vendor = PCI_VENDOR_ID_SONY;
-
- input_register_device(&sonypi_device.jog_dev);
- printk(KERN_INFO "%s installed.\n", sonypi_device.jog_dev.name);
- }
-#endif /* SONYPI_USE_INPUT */

return 0;

diff -Nru a/drivers/char/sonypi.h b/drivers/char/sonypi.h
--- a/drivers/char/sonypi.h 2004-10-21 01:54:42 -05:00
+++ b/drivers/char/sonypi.h 2004-10-21 01:54:42 -05:00
@@ -344,8 +344,6 @@
unsigned long tail;
unsigned long len;
spinlock_t s_lock;
- wait_queue_head_t proc_list;
- struct fasync_struct *fasync;
unsigned char buf[SONYPI_BUF_SIZE];
};

@@ -373,6 +371,8 @@
int camera_power;
int bluetooth_power;
struct semaphore lock;
+ wait_queue_head_t proc_list;
+ struct fasync_struct *fasync;
struct sonypi_queue queue;
int open_count;
int model;

2004-10-25 12:55:37

by Stelian Pop

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Thu, Oct 21, 2004 at 01:54:58AM -0500, Dmitry Torokhov wrote:

> I have been looking at the sysdevs in present in the kernel and noticed that
> sonypi was registering itself as a system device. Surely it is possible to
> suspend it with interrupyts enabled, so it better be converted to a platform
> device. I course of convert I also did some additional changes:
[...]

Thanks for those patches and sorry for the lack of response, I was out
of town for the last week.

I have quite a few changes in my tree already for the sonypi driver,
and I was delaying the submission because I need to solve a problem
with the integration with the input subsystem...

Some of your changes (those related to module_param(), wait_event()
use etc) were already in my tree, those related to whitespace cleanup,
platform instead of sysdev etc are new and I will integrate them.

Let me work a bit on those, and I will try to separate my changes
and resubmit them in the next days.

Stelian.
--
Stelian Pop <[email protected]>

2004-10-25 12:56:27

by Stelian Pop

[permalink] [raw]
Subject: Re: [PATCH 5/5] Sonypi: use pci_get_device

On Thu, Oct 21, 2004 at 01:59:35AM -0500, Dmitry Torokhov wrote:

> [email protected], 2004-10-21 01:48:41-05:00, [email protected]
> Sonypi: switch from pci_find_device to pci_get_device.
[...]
> + struct pci_dev *pcidev;
> +
> + pcidev = pci_find_device(PCI_VENDOR_ID_INTEL,
> + PCI_DEVICE_ID_INTEL_82371AB_3,
> + NULL);

I guess this is supposed to be pci_get_device no ? :)

Stelian.
--
Stelian Pop <[email protected]>

2004-10-25 13:25:10

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Monday 25 October 2004 07:56 am, Stelian Pop wrote:
> On Thu, Oct 21, 2004 at 01:54:58AM -0500, Dmitry Torokhov wrote:
>
> > I have been looking at the sysdevs in present in the kernel and noticed that
> > sonypi was registering itself as a system device. Surely it is possible to
> > suspend it with interrupyts enabled, so it better be converted to a platform
> > device. I course of convert I also did some additional changes:
> [...]
>
> Thanks for those patches and sorry for the lack of response, I was out
> of town for the last week.
>
> I have quite a few changes in my tree already for the sonypi driver,
> and I was delaying the submission because I need to solve a problem
> with the integration with the input subsystem...
>

If you need a hand - I am a bit familiar with the input system...

> Some of your changes (those related to module_param(), wait_event()
> use etc) were already in my tree, those related to whitespace cleanup,
> platform instead of sysdev etc are new and I will integrate them.
>

The change from sysdev to a platform device is the main reason I did
the change (and getting rid of old pm_register stuff which is useless
now) because swsusp2 (and seems that swsusp1 as well) have trouble
resuming system devices. The rest was just fluff really.

--
Dmitry

2004-10-25 13:27:13

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 5/5] Sonypi: use pci_get_device

On Monday 25 October 2004 07:57 am, Stelian Pop wrote:
> On Thu, Oct 21, 2004 at 01:59:35AM -0500, Dmitry Torokhov wrote:
>
> > [email protected], 2004-10-21 01:48:41-05:00, [email protected]
> > Sonypi: switch from pci_find_device to pci_get_device.
> [...]
> > + struct pci_dev *pcidev;
> > +
> > + pcidev = pci_find_device(PCI_VENDOR_ID_INTEL,
> > + PCI_DEVICE_ID_INTEL_82371AB_3,
> > + NULL);
>
> I guess this is supposed to be pci_get_device no ? :)
>

Ahem... well.. yes ;)

--
Dmitry

2004-10-25 13:52:40

by Stelian Pop

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Mon, Oct 25, 2004 at 02:56:29PM +0200, Stelian Pop wrote:

> I have quite a few changes in my tree already for the sonypi driver,
> and I was delaying the submission because I need to solve a problem
> with the integration with the input subsystem...

Speaking of this, Vojtech, how should I proceed ?

Let me remind you the problem: I am in the process of converting
the sonypi driver to (fully) use the input subsystem to forward
special key presses to X and the console instead of using a userspace
daemon which pushes the KeyPress events into the X queue.

The special keys are like KEY_BACK, KEY_HELP, KEY_ZOOM, KEY_CAMERA,
and a dozen FN + some key combinations.

I can integrate those events into the input layer in 2 different ways:

* allocate a new key event (in include/linux/input.h) for each
key *and* combination. This will make the keys and the combinations
work both on the console and in X.

Unfortunately only events under the 0xff limit seem to be
propagated to X, the other ones don't generate any X event (I haven't
looked at the problem but I suppose it somewhere into the X code).

showkey does corectly see the keys in raw mode.

* allocate a FN key event and let FN be a modifier.

This is much nicer (less events allocated in input.h), but I haven't
found a way (and I'm not sure there is one) to say to X that Fn is a
*new* modifier. Yes, I can say FN act like a Control, Meta or whatever
existing modifier, but this is useless since I already have a Control,
Alt, etc. key on my keyboard. The whole point is to add support for
a new key !

I also haven't looked yet at adding a new modifier in the console
mode...

Please advice on the recommended way to do this properly.

Stelian.
--
Stelian Pop <[email protected]>

2004-10-25 13:57:02

by Stelian Pop

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Mon, Oct 25, 2004 at 08:22:46AM -0500, Dmitry Torokhov wrote:

> If you need a hand - I am a bit familiar with the input system...

See the other mail I just send CC:'ed to Vojtech...

> > Some of your changes (those related to module_param(), wait_event()
> > use etc) were already in my tree, those related to whitespace cleanup,
> > platform instead of sysdev etc are new and I will integrate them.
> >
>
> The change from sysdev to a platform device is the main reason I did
> the change (and getting rid of old pm_register stuff which is useless
> now) because swsusp2 (and seems that swsusp1 as well) have trouble
> resuming system devices. The rest was just fluff really.

Ok. Suspending never really worked on my laptop so I'll have to assume
you're correct. :)

[ Just tried once again to do a suspend to ram, seems that there were
some enhancements in this area lately.

No luck. Machine suspends ok, but upon waking up, the power led goes
greek ok, the disk led lights up, but the keyboard is dead, the
network card is dead, the screen doesn't turn on...

Since this laptop has no serial port I don't see what else I can do,
except wait another 6 months and try again... :(
]

Stelian.
--
Stelian Pop <[email protected]>

2004-10-25 13:58:36

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Mon, Oct 25, 2004 at 03:50:36PM +0200, Stelian Pop wrote:

> The special keys are like KEY_BACK, KEY_HELP, KEY_ZOOM, KEY_CAMERA,
> and a dozen FN + some key combinations.
>
> I can integrate those events into the input layer in 2 different ways:
>
> * allocate a new key event (in include/linux/input.h) for each
> key *and* combination. This will make the keys and the combinations
> work both on the console and in X.
>
> Unfortunately only events under the 0xff limit seem to be
> propagated to X, the other ones don't generate any X event (I haven't
> looked at the problem but I suppose it somewhere into the X code).

The number is 240 and it's the number of possible PS/2 scancode
combinations, and since at this time X can only understand the PS/2
protocol (and not native Linux events), this is the only way how to pass
keypresses to X.

I believe that although this way may be easier, it leads to madness.

> showkey does corectly see the keys in raw mode.
>
> * allocate a FN key event and let FN be a modifier.
>
> This is much nicer (less events allocated in input.h), but I haven't
> found a way (and I'm not sure there is one) to say to X that Fn is a
> *new* modifier. Yes, I can say FN act like a Control, Meta or whatever
> existing modifier, but this is useless since I already have a Control,
> Alt, etc. key on my keyboard. The whole point is to add support for
> a new key !
>
> I also haven't looked yet at adding a new modifier in the console
> mode...

IIRC X has only 8 modifier keys and all are already defined and you
can't define any more. But I doubt you're using all of them on your
keyboard. It should be possible to assign Fn to one of them.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2004-10-25 15:27:46

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

Apologies for breaking the threading...

Stelian Pop wrote:
> On Mon, Oct 25, 2004 at 03:57:43PM +0200, Vojtech Pavlik wrote:
>
> > The number is 240 and it's the number of possible PS/2 scancode
> > combinations, and since at this time X can only understand the PS/2
> > protocol (and not native Linux events), this is the only way how to pass
> > keypresses to X.
> >
> > I believe that although this way may be easier, it leads to madness.
>
> It is also impossible for me to go this way because there is no way
> to put 20+ events between 226 and 240...
>

I'd say just allocate brand new events for all combinations and do not
worry that the default X keyboard drivers to not get them. There are
already patches in Gentoo adding both keyboard and mouse event support
to X [1] and it is only matter of time ofr other duistributions to pick
it up as well.

I think it is sensible for an supplemental driver (sonypi) to require
some additional support form userspace and not to force itself into
boundaries of a legacy protocol.

--
Dmitry

[1]
http://csociety-ftp.ecn.purdue.edu/pub/gentoo/distfiles/xorg-x11-6.8.0-patches-0.2.5.tar.bz2
Extract patches 9000, 9001 and 9002. Btw, these are not mine - I have
Not even tries them myself but I have read several success stories.

2004-10-25 19:30:10

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Mon, Oct 25, 2004 at 05:20:24PM +0200, Stelian Pop wrote:
> xev doesn't say anything about modifiers. It shows the modifier being
> pressed, the other key pressed, then released, then the modifier being
> released too.
>
> The trace shows the same kind of events in a working Control case or
> in a not working 'function' case:
>
> KeyPress event, serial 24, synthetic NO, window 0x2a00001,
> root 0x40, subw 0x2a00002, time 6566259, (37,47), root:(542,70),
> state 0x0, keycode 214 (keysym 0x8f6, function), same_screen YES,
> XLookupString gives 0 bytes:
> XmbLookupString gives 0 bytes:
> XFilterEvent returns: False
>
> KeyPress event, serial 24, synthetic NO, window 0x2a00001,
> root 0x40, subw 0x2a00002, time 6566259, (37,47), root:(542,70),
> state 0x20, keycode 67 (keysym 0xffbe, F1), same_screen YES,
> XLookupString gives 0 bytes:
> XmbLookupString gives 0 bytes:
> XFilterEvent returns: False
>
> KeyRelease event, serial 24, synthetic NO, window 0x2a00001,
> root 0x40, subw 0x2a00002, time 6566259, (37,47), root:(542,70),
> state 0x20, keycode 67 (keysym 0xffbe, F1), same_screen YES,
> XLookupString gives 0 bytes:
>
> KeyRelease event, serial 24, synthetic NO, window 0x2a00001,
> root 0x40, subw 0x2a00002, time 6566259, (37,47), root:(542,70),
> state 0x20, keycode 214 (keysym 0x8f6, function), same_screen YES,
> XLookupString gives 0 bytes:

Watch the "state" variable.

--
Vojtech Pavlik
SuSE Labs, SuSE CR

2004-10-25 21:47:38

by Stelian Pop

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Mon, Oct 25, 2004 at 05:11:22PM +0200, Vojtech Pavlik wrote:

> > > I believe that although this way may be easier, it leads to madness.
> >
> > It is also impossible for me to go this way because there is no way
> > to put 20+ events between 226 and 240...
>
> You could fix X to use either medium raw mode on the console or the
> event protocol. Medium raw has a limit of 2^14 and event protocol is
> limited by 2^16.

Well, letting X get the events directly on /dev/input/event* is clearly
the way to go, for a long term solution.

However, I would like a short term solution, available now, so users
of sonypi could just migrate towards the new way without having to
compile their own version of the X server...

> > That's what I thought too. However, it seems to work only when the
> > keysym associated with the modifier is a well known key (Control_L,
> > Control_R, Alt_L etc).
> >
> > If I do (214 is the keycode generated by my Fn key):
> > keycode 214 = Control_L
> > clear mod3
> > add mod3 = Control_L
> > then Fn + F1 will generate Mod3 + F1 (but Control_L will not work as
> > a Control modifier anymore).
> >
> > But if I do:
> > keycode 214 = function
> > clear mod3
> > add mod3 = function
> > then (at least) WindowMaker does not see the modifier anymore (only
> > a 'function' single key press is received).
>
> Does the same happen in 'xev'?

xev doesn't say anything about modifiers. It shows the modifier being
pressed, the other key pressed, then released, then the modifier being
released too.

The trace shows the same kind of events in a working Control case or
in a not working 'function' case:

KeyPress event, serial 24, synthetic NO, window 0x2a00001,
root 0x40, subw 0x2a00002, time 6566259, (37,47), root:(542,70),
state 0x0, keycode 214 (keysym 0x8f6, function), same_screen YES,
XLookupString gives 0 bytes:
XmbLookupString gives 0 bytes:
XFilterEvent returns: False

KeyPress event, serial 24, synthetic NO, window 0x2a00001,
root 0x40, subw 0x2a00002, time 6566259, (37,47), root:(542,70),
state 0x20, keycode 67 (keysym 0xffbe, F1), same_screen YES,
XLookupString gives 0 bytes:
XmbLookupString gives 0 bytes:
XFilterEvent returns: False

KeyRelease event, serial 24, synthetic NO, window 0x2a00001,
root 0x40, subw 0x2a00002, time 6566259, (37,47), root:(542,70),
state 0x20, keycode 67 (keysym 0xffbe, F1), same_screen YES,
XLookupString gives 0 bytes:

KeyRelease event, serial 24, synthetic NO, window 0x2a00001,
root 0x40, subw 0x2a00002, time 6566259, (37,47), root:(542,70),
state 0x20, keycode 214 (keysym 0x8f6, function), same_screen YES,
XLookupString gives 0 bytes:

Stelian.
--
Stelian Pop <[email protected]>

2004-10-25 22:17:51

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

Hi!

> * allocate a FN key event and let FN be a modifier.
>
> This is much nicer (less events allocated in input.h), but I haven't
> found a way (and I'm not sure there is one) to say to X that Fn is

I think this is *bad* idea. In such case, userland would see
Fn-F3. My notebook has "sleep" key on Fn-F3, but your notebook
probably has something else there. You'd need another mapping in
userspace...

I believe Fn-F3 on my machine is meant to be replacement for hardware
sleep button (and it has sleep label on it!), and we really should
generate sleep event for Fn-F3...
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

2004-10-25 22:26:36

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

Hi!

> > If you need a hand - I am a bit familiar with the input system...
>
> See the other mail I just send CC:'ed to Vojtech...
>
> > > Some of your changes (those related to module_param(), wait_event()
> > > use etc) were already in my tree, those related to whitespace cleanup,
> > > platform instead of sysdev etc are new and I will integrate them.
> > >
> >
> > The change from sysdev to a platform device is the main reason I did
> > the change (and getting rid of old pm_register stuff which is useless
> > now) because swsusp2 (and seems that swsusp1 as well) have trouble
> > resuming system devices. The rest was just fluff really.
>
> Ok. Suspending never really worked on my laptop so I'll have to assume
> you're correct. :)
>
> [ Just tried once again to do a suspend to ram, seems that there were
> some enhancements in this area lately.
>
> No luck. Machine suspends ok, but upon waking up, the power led goes
> greek ok, the disk led lights up, but the keyboard is dead, the
> network card is dead, the screen doesn't turn on...
>
> Since this laptop has no serial port I don't see what else I can do,
> except wait another 6 months and try again... :(

Debug using pc speaker... Or paralel port, or something like that.

Pavel

--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

2004-10-26 00:31:18

by Stelian Pop

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Mon, Oct 25, 2004 at 03:57:43PM +0200, Vojtech Pavlik wrote:

> The number is 240 and it's the number of possible PS/2 scancode
> combinations, and since at this time X can only understand the PS/2
> protocol (and not native Linux events), this is the only way how to pass
> keypresses to X.
>
> I believe that although this way may be easier, it leads to madness.

It is also impossible for me to go this way because there is no way
to put 20+ events between 226 and 240...

> > I also haven't looked yet at adding a new modifier in the console
> > mode...
>
> IIRC X has only 8 modifier keys and all are already defined and you
> can't define any more. But I doubt you're using all of them on your
> keyboard. It should be possible to assign Fn to one of them.

That's what I thought too. However, it seems to work only when the
keysym associated with the modifier is a well known key (Control_L,
Control_R, Alt_L etc).

If I do (214 is the keycode generated by my Fn key):
keycode 214 = Control_L
clear mod3
add mod3 = Control_L
then Fn + F1 will generate Mod3 + F1 (but Control_L will not work as
a Control modifier anymore).

But if I do:
keycode 214 = function
clear mod3
add mod3 = function
then (at least) WindowMaker does not see the modifier anymore (only
a 'function' single key press is received).

Stelian.
--
Stelian Pop <[email protected]>

2004-10-26 03:20:19

by Nigel Cunningham

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

Hi.

On Mon, 2004-10-25 at 23:22, Dmitry Torokhov wrote:
> The change from sysdev to a platform device is the main reason I did
> the change (and getting rid of old pm_register stuff which is useless
> now) because swsusp2 (and seems that swsusp1 as well) have trouble
> resuming system devices. The rest was just fluff really.

I'm not sure why we're not trying to resume system devices. I'll give it
a whirl and see if anything breaks :> Feel free to tell me if/when you
notice things like this in future; I try to be approachable and
responsive.

Regards,

Nigel
--
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

Everyone lives by faith. Some people just don't believe it.
Want proof? Try to prove that the theory of evolution is true.

2004-10-26 05:53:13

by Stelian Pop

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Tue, Oct 26, 2004 at 12:09:21AM +0200, Pavel Machek wrote:

> > Ok. Suspending never really worked on my laptop so I'll have to assume
> > you're correct. :)
> >
> > [ Just tried once again to do a suspend to ram, seems that there were
> > some enhancements in this area lately.
> >
> > No luck. Machine suspends ok, but upon waking up, the power led goes
> > greek ok, the disk led lights up, but the keyboard is dead, the
> > network card is dead, the screen doesn't turn on...
> >
> > Since this laptop has no serial port I don't see what else I can do,
> > except wait another 6 months and try again... :(
>
> Debug using pc speaker... Or paralel port, or something like that.

This is a laptop which has no serial or parallel port.

All the interfaces it has are all too high level to be used for
debugging (USB, firewire, pcmcia etc).

I think the speaker would probably work. I'll give it a try and
see if it helps next time...

Stelian.
--
Stelian Pop <[email protected]>

2004-10-26 06:06:22

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Tuesday 26 October 2004 12:55 am, Stelian Pop wrote:
> On Tue, Oct 26, 2004 at 12:09:21AM +0200, Pavel Machek wrote:
>
> > > Ok. Suspending never really worked on my laptop so I'll have to assume
> > > you're correct. :)
> > >
> > > [ Just tried once again to do a suspend to ram, seems that there were
> > > some enhancements in this area lately.
> > >
> > > No luck. Machine suspends ok, but upon waking up, the power led goes
> > > greek ok, the disk led lights up, but the keyboard is dead, the
> > > network card is dead, the screen doesn't turn on...
> > >
> > > Since this laptop has no serial port I don't see what else I can do,
> > > except wait another 6 months and try again... :(
> >
> > Debug using pc speaker... Or paralel port, or something like that.
>
> This is a laptop which has no serial or parallel port.
>
> All the interfaces it has are all too high level to be used for
> debugging (USB, firewire, pcmcia etc).
>
> I think the speaker would probably work. I'll give it a try and
> see if it helps next time...
>

I'd try to get suspend-to-disk working first.. ACPI S3 seems to be much
trickier.

--
Dmitry

2004-10-26 06:21:51

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Monday 25 October 2004 09:28 pm, Nigel Cunningham wrote:
> Hi.
>
> On Mon, 2004-10-25 at 23:22, Dmitry Torokhov wrote:
> > The change from sysdev to a platform device is the main reason I did
> > the change (and getting rid of old pm_register stuff which is useless
> > now) because swsusp2 (and seems that swsusp1 as well) have trouble
> > resuming system devices. The rest was just fluff really.
>
> I'm not sure why we're not trying to resume system devices. I'll give it
> a whirl and see if anything breaks :> Feel free to tell me if/when you
> notice things like this in future; I try to be approachable and
> responsive.
>

Hi Nigel,

System devices are resumed when you call device_power_up and I could
not find references to it in swsusp2 code, it goes straight to
device_resume_tree... Am I missng something?

--
Dmitry

2004-10-26 06:49:50

by Nigel Cunningham

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

Hi.

On Tue, 2004-10-26 at 16:21, Dmitry Torokhov wrote:
> On Monday 25 October 2004 09:28 pm, Nigel Cunningham wrote:
> > Hi.
> >
> > On Mon, 2004-10-25 at 23:22, Dmitry Torokhov wrote:
> > > The change from sysdev to a platform device is the main reason I did
> > > the change (and getting rid of old pm_register stuff which is useless
> > > now) because swsusp2 (and seems that swsusp1 as well) have trouble
> > > resuming system devices. The rest was just fluff really.
> >
> > I'm not sure why we're not trying to resume system devices. I'll give it
> > a whirl and see if anything breaks :> Feel free to tell me if/when you
> > notice things like this in future; I try to be approachable and
> > responsive.
> >
>
> Hi Nigel,
>
> System devices are resumed when you call device_power_up and I could
> not find references to it in swsusp2 code, it goes straight to
> device_resume_tree... Am I missng something?

No, you weren't. I was following the pattern of Patrick and Pavel. I've
changed it this afternoon and have it running fine, except that the PIC
timer code takes a few seconds (see separate message).

The changes will be in 2.1.1. I just have a couple more little fixes to
do before I release it.

Regards,

Nigel
--
Nigel Cunningham
Pastoral Worker
Christian Reformed Church of Tuggeranong
PO Box 1004, Tuggeranong, ACT 2901

Everyone lives by faith. Some people just don't believe it.
Want proof? Try to prove that the theory of evolution is true.

2004-10-26 08:33:24

by Karol Kozimor

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Tuesday 26 of October 2004 00:12, Pavel Machek wrote:
> > * allocate a FN key event and let FN be a modifier.
> >
> > This is much nicer (less events allocated in input.h), but I haven't
> > found a way (and I'm not sure there is one) to say to X that Fn is
>
> I think this is *bad* idea. In such case, userland would see
> Fn-F3. My notebook has "sleep" key on Fn-F3, but your notebook
> probably has something else there. You'd need another mapping in
> userspace...
>
> I believe Fn-F3 on my machine is meant to be replacement for hardware
> sleep button (and it has sleep label on it!), and we really should
> generate sleep event for Fn-F3...

Then map the button to invoke the suspend script in userspace. First we're
mapping ACPI events to input events, then the other way around? Sounds
fishy to me.
Best regards,

--
Karol Kozimor
[email protected]

2004-10-26 09:26:48

by Stelian Pop

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Mon, Oct 25, 2004 at 08:20:29AM -0700, Dmitry Torokhov wrote:

> I'd say just allocate brand new events for all combinations and do not
> worry that the default X keyboard drivers to not get them. There are
> already patches in Gentoo adding both keyboard and mouse event support
> to X [1] and it is only matter of time ofr other duistributions to pick
> it up as well.
>
> I think it is sensible for an supplemental driver (sonypi) to require
> some additional support form userspace and not to force itself into
> boundaries of a legacy protocol.
>
> --
> Dmitry
>
> [1]
> http://csociety-ftp.ecn.purdue.edu/pub/gentoo/distfiles/xorg-x11-6.8.0-patches-0.2.5.tar.bz2
> Extract patches 9000, 9001 and 9002. Btw, these are not mine - I have
> Not even tries them myself but I have read several success stories.

Got them and trying to build the new drivers right now. Thanks !

Stelian.
--
Stelian Pop <[email protected]>

2004-10-26 09:45:12

by Stelian Pop

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Mon, Oct 25, 2004 at 06:04:27PM +0200, Vojtech Pavlik wrote:

> > KeyRelease event, serial 24, synthetic NO, window 0x2a00001,
> > root 0x40, subw 0x2a00002, time 6566259, (37,47), root:(542,70),
> > state 0x20, keycode 214 (keysym 0x8f6, function), same_screen YES,
> > XLookupString gives 0 bytes:
>
> Watch the "state" variable.

Indeed, I didn't noticed it.

However, this still doesn't work as expected. I looked at WindowMaker's
code for grabbing a key and found out it uses the standard IsModifierKey
macro, which is defined in X11/Xutil.h as:

#define IsModifierKey(keysym) \
((((KeySym)(keysym) >= XK_Shift_L) && ((KeySym)(keysym) <= XK_Hyper_R)) \
|| (((KeySym)(keysym) >= XK_ISO_Lock) && \
((KeySym)(keysym) <= XK_ISO_Last_Group_Lock)) \
|| ((KeySym)(keysym) == XK_Mode_switch) \
|| ((KeySym)(keysym) == XK_Num_Lock))

So it clearly handles only some special modifier keys.

Moreover, the xkeycaps manpage (available at
http://wwwvms.mppmu.mpg.de/unix_man_pages/xkeycaps.html) goes a bit
more in-depth and says:

Modifier Bit
Modifier bits are attributes which certain
keysyms can have. Some modifier bits have pre?
defined semantics: Shift, Lock, and Control.
The remaining modifier bits (Mod1 through Mod5)
have semantics which are defined by the keys
with which they are associated.

So I'm back where I started, I cannot see how to define my new
'Fn' key to be interpreted as a 'ModX' modifier by X.

I will probably go now the new evdev based X driver, and will use
new, above 240, keycodes for all key combinations. Provided the
Gentoo patches build and work correctly.

Stelian.
--
Stelian Pop <[email protected]>

2004-10-26 15:29:56

by Stelian Pop

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Tue, Oct 26, 2004 at 11:28:02AM +0200, Stelian Pop wrote:

> > [1]
> > http://csociety-ftp.ecn.purdue.edu/pub/gentoo/distfiles/xorg-x11-6.8.0-patches-0.2.5.tar.bz2
> > Extract patches 9000, 9001 and 9002. Btw, these are not mine - I have
> > Not even tries them myself but I have read several success stories.
>
> Got them and trying to build the new drivers right now. Thanks !

Well, it kinda works, but there are still some rough edges (the kbd
driver maps all the unknown keys to KEY_UNKNOWN, making them all to
have the same keycode in X, making them unusable. After removing the
test it forwards the events ok).

There are also problems because my sonypi input device acts both like
a mouse and a keyboard, and the X event driver wants them to be separate.

Vojtech: should I generate two different input devices in my driver,
a mouse like and a keyboard like device, or should the userspace be
able to demultiplex the events ?

Stelian.
--
Stelian Pop <[email protected]>

2004-10-26 15:57:07

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes


And I am breaking the thread yet again....

Stelian pop wrote:
> On Tue, Oct 26, 2004 at 11:28:02AM +0200, Stelian Pop wrote:
>
> > > [1]
> > > http://csociety-ftp.ecn.purdue.edu/pub/gentoo/distfiles/xorg-x11-
> 6.8.0-patches-0.2.5.tar.bz2
> > > Extract patches 9000, 9001 and 9002. Btw, these are not mine - I have
> > > Not even tries them myself but I have read several success stories.
> >
> > Got them and trying to build the new drivers right now. Thanks !
>
> Well, it kinda works, but there are still some rough edges (the kbd
> driver maps all the unknown keys to KEY_UNKNOWN, making them all to
> have the same keycode in X, making them unusable. After removing the
> test it forwards the events ok).
>
> There are also problems because my sonypi input device acts both like
> a mouse and a keyboard, and the X event driver wants them to be separate.
>
> Vojtech: should I generate two different input devices in my driver,
> a mouse like and a keyboard like device, or should the userspace be
> able to demultiplex the events ?
>

If you want jogdial to continue generating BTN_MIDDLE and BTN_WHEEL
events then IMHO you should create 2 separate input devices - one
for jogdial and the other for FN-keys.

On the other hand I am not sure if it is handy as a ponter device -
I think scrolling is much more natural with the touchpad (but
remember I don't have the hardware) and it may be more convenient
to assign brand-new events to jogdial as well and then map it in
userspace (X) into something different, like volume control. In
this case you can continue having just one input device.

Btw, you should probably drop conditional support for input layer
and always compile it in.

--
Dmitry

2004-10-26 18:15:32

by Stelian Pop

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Tue, Oct 26, 2004 at 08:56:39AM -0700, Dmitry Torokhov wrote:

> If you want jogdial to continue generating BTN_MIDDLE and BTN_WHEEL
> events then IMHO you should create 2 separate input devices - one
> for jogdial and the other for FN-keys.

That's what I thought too...

> On the other hand I am not sure if it is handy as a ponter device -
> I think scrolling is much more natural with the touchpad (but
> remember I don't have the hardware)

I don't have a touchpad, just a stick. And I use the jogdial to
scroll quite often...

> and it may be more convenient
> to assign brand-new events to jogdial as well and then map it in
> userspace (X) into something different, like volume control. In
> this case you can continue having just one input device.
>
> Btw, you should probably drop conditional support for input layer
> and always compile it in.

Is CONFIG_INPUT now a requirement for the (at least i386) kernel ?
If this is the case, I'll drop the conditional.

Stelian.
--
Stelian Pop <[email protected]>

2004-10-27 02:57:07

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Tuesday 26 October 2004 01:09 pm, Stelian Pop wrote:
> > Btw, you should probably drop conditional support for input layer
> > and always compile it in.
>
> Is CONFIG_INPUT now a requirement for the (at least i386) kernel ?
> If this is the case, I'll drop the conditional.
>

While it can be disabled when one selects !EMBEDDED I doubt hi/she will
be interested in sonnypi in this case :). For all practical reasons input
layer is always present.

--
Dmitry

2004-10-27 03:14:23

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Tuesday 26 October 2004 01:09 pm, Stelian Pop wrote:
> > On the other hand I am not sure if it is handy as a ponter device -
> > I think scrolling is much more natural with the touchpad (but
> > remember I don't have the hardware)
>
> I don't have a touchpad, just a stick. And I use the jogdial to
> scroll quite often...
>

I wonder if the best option is to always have 2 devices and make one
corrsponding to jogdial switch between mouse-like and keyboad-like
events based on module parameter.

--
Dmitry

2004-10-27 08:04:33

by Stelian Pop

[permalink] [raw]
Subject: Re: [PATCH 0/5] Sonypi driver model & PM changes

On Tue, Oct 26, 2004 at 09:56:50PM -0500, Dmitry Torokhov wrote:

> On Tuesday 26 October 2004 01:09 pm, Stelian Pop wrote:
> > > Btw, you should probably drop conditional support for input layer
> > > and always compile it in.
> >
> > Is CONFIG_INPUT now a requirement for the (at least i386) kernel ?
> > If this is the case, I'll drop the conditional.
> >
>
> While it can be disabled when one selects !EMBEDDED I doubt hi/she will
> be interested in sonnypi in this case :). For all practical reasons input
> layer is always present.

Ok. I'll remove the ifdefs and make SONYPI depend on INPUT in Kconfig.

Stelian.
--
Stelian Pop <[email protected]>