[RFC 1/2] Input: ff, add FF_RAW effect

[RFC 1/2] Input: ff, add FF_RAW effect

[Jiri Slaby]

So fellows, what about these ones?

--

ff, add FF_RAW effect

Add new FF_RAW effect for devices such Phantom. The new model has up to 6DOF
torque force feedback independent on any 3d-or-so value.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

---
commit 759e7f172031959f49e5d3a7282379e7d73621b3
tree 906d51925ff7f95ebae8148ef2f1b2f252ef3f4e
parent bd99756ce7fb8f3e9105b076a71046b0d8ad1f8f
author Jiri Slaby <jirislaby@gmail.com> Tue, 17 Apr 2007 21:50:21 +0200
committer Jiri Slaby <jirislaby@gmail.com> Tue, 17 Apr 2007 21:50:21 +0200

 drivers/input/ff-memless.c |    4 ++++
 include/linux/input.h      |    4 +++-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index d226d93..3bbace9 100644
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -275,6 +275,10 @@ static void ml_combine_effects(struct ff_effect *effect,
 			min(i + effect->u.rumble.weak_magnitude, 0xffffU);
 		break;
 
+	case FF_RAW:
+		memcpy(effect->u.ff_raw, new->u.ff_raw, sizeof(new->u.ff_raw));
+		break;
+
 	default:
 		printk(KERN_ERR "ff-memless: invalid type in ml_combine_effects()\n");
 		break;
diff --git a/include/linux/input.h b/include/linux/input.h
index 2a23768..cc4ae1f 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -859,6 +859,7 @@ struct ff_effect {
 		struct ff_periodic_effect periodic;
 		struct ff_condition_effect condition[2]; /* One for each axis */
 		struct ff_rumble_effect rumble;
+		__u32 ff_raw[6];
 	} u;
 };
 
@@ -874,9 +875,10 @@ struct ff_effect {
 #define FF_DAMPER	0x55
 #define FF_INERTIA	0x56
 #define FF_RAMP		0x57
+#define FF_RAW		0x58
 
 #define FF_EFFECT_MIN	FF_RUMBLE
-#define FF_EFFECT_MAX	FF_RAMP
+#define FF_EFFECT_MAX	FF_RAW
 
 /*
  * Force feedback periodic effect types

[RFC 1/2] Input: ff, add FF_RAW effect

[Jiri Slaby]

So fellows, what about these ones?

--

ff, add FF_RAW effect

Add new FF_RAW effect for devices such Phantom. The new model has up to 6DOF
torque force feedback independent on any 3d-or-so value.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

---
commit 759e7f172031959f49e5d3a7282379e7d73621b3
tree 906d51925ff7f95ebae8148ef2f1b2f252ef3f4e
parent bd99756ce7fb8f3e9105b076a71046b0d8ad1f8f
author Jiri Slaby <jirislaby@gmail.com> Tue, 17 Apr 2007 21:50:21 +0200
committer Jiri Slaby <jirislaby@gmail.com> Tue, 17 Apr 2007 21:50:21 +0200

 drivers/input/ff-memless.c |    4 ++++
 include/linux/input.h      |    4 +++-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c
index d226d93..3bbace9 100644
--- a/drivers/input/ff-memless.c
+++ b/drivers/input/ff-memless.c
@@ -275,6 +275,10 @@ static void ml_combine_effects(struct ff_effect *effect,
 			min(i + effect->u.rumble.weak_magnitude, 0xffffU);
 		break;
 
+	case FF_RAW:
+		memcpy(effect->u.ff_raw, new->u.ff_raw, sizeof(new->u.ff_raw));
+		break;
+
 	default:
 		printk(KERN_ERR "ff-memless: invalid type in ml_combine_effects()\n");
 		break;
diff --git a/include/linux/input.h b/include/linux/input.h
index 2a23768..cc4ae1f 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -859,6 +859,7 @@ struct ff_effect {
 		struct ff_periodic_effect periodic;
 		struct ff_condition_effect condition[2]; /* One for each axis */
 		struct ff_rumble_effect rumble;
+		__u32 ff_raw[6];
 	} u;
 };
 
@@ -874,9 +875,10 @@ struct ff_effect {
 #define FF_DAMPER	0x55
 #define FF_INERTIA	0x56
 #define FF_RAMP		0x57
+#define FF_RAW		0x58
 
 #define FF_EFFECT_MIN	FF_RUMBLE
-#define FF_EFFECT_MAX	FF_RAMP
+#define FF_EFFECT_MAX	FF_RAW
 
 /*
  * Force feedback periodic effect types

[RFC 2/2] Input: phantom, add a new driver

[Jiri Slaby]

phantom, add a new driver

Sensable Phantom is a up to 7DOF force feedback (up to 6DOF FF) device. It's
atypical, so it's based on the new added FF_RAW effect.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

---
commit 73621a789a0482299242ea61c971af6b5f8b828a
tree 365ba8113af9aaf468415d67238378767efad092
parent 759e7f172031959f49e5d3a7282379e7d73621b3
author Jiri Slaby <jirislaby@gmail.com> Tue, 17 Apr 2007 21:58:27 +0200
committer Jiri Slaby <jirislaby@gmail.com> Tue, 17 Apr 2007 21:58:27 +0200

 MAINTAINERS                  |    5 +
 drivers/input/misc/Kconfig   |    9 +
 drivers/input/misc/Makefile  |    1 
 drivers/input/misc/phantom.c |  387 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 402 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8a68712..86a8417 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3112,6 +3112,11 @@ L: 	selinux@tycho.nsa.gov (subscribers-only, general discussion)
 W:	http://www.nsa.gov/selinux
 S:	Supported
 
+SENSABLE PHANTOM
+P:	Jiri Slaby
+M:	jirislaby@gmail.com
+S:	Maintained
+
 SERIAL ATA (SATA) SUBSYSTEM:
 P:	Jeff Garzik
 M:	jgarzik@pobox.com
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 5694115..6e432b0 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -98,4 +98,13 @@ config HP_SDC_RTC
 	  Say Y here if you want to support the built-in real time clock
 	  of the HP SDC controller.
 
+config PHANTOM
+	tristate "Sensable PHANToM"
+	depends on PCI && INPUT_FF_MEMLESS
+	help
+	  Say Y here if you want to build a driver for Sensable PHANToM device.
+
+	  If you choose to build module, its name will be phantom. If unsure,
+	  say N here.
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 9f08f27..3ab3cc2 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_INPUT_WISTRON_BTNS)	+= wistron_btns.o
 obj-$(CONFIG_INPUT_ATLAS_BTNS)		+= atlas_btns.o
 obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
 obj-$(CONFIG_INPUT_IXP4XX_BEEPER)	+= ixp4xx-beeper.o
+obj-$(CONFIG_PHANTOM)			+= phantom.o
diff --git a/drivers/input/misc/phantom.c b/drivers/input/misc/phantom.c
new file mode 100644
index 0000000..58f55cd
--- /dev/null
+++ b/drivers/input/misc/phantom.c
@@ -0,0 +1,387 @@
+/*
+ *  Copyright (C) 2005-2007 Jiri Slaby <jirislaby@gmail.com>
+ *
+ *  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.
+ *
+ *  You need an userspace library to cooperate with this driver. It (and other
+ *  info) may be obtained here:
+ *  http://www.fi.muni.cz/~xslaby/phantom.html
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/pci.h>
+#include <linux/fs.h>
+#include <linux/poll.h>
+#include <linux/interrupt.h>
+#include <linux/input.h>
+
+#include <asm/io.h>
+
+#define PHANTOM_VERSION		"n0.9.4"
+
+#define PHM_MAX_TORQUES		3
+
+#define PHN_CONTROL		0x6
+#define PHN_CTL_AMP		0x1
+#define PHN_CTL_BUT		0x2
+#define PHN_CTL_IRQ		0x10
+
+#define PHN_IRQCTL		0x4c
+
+#define PHN_ZERO_FORCE		2048
+
+#define PCI_ENCODER(dev, axis) ((0 - (int)ioread32((dev)->iaddr + (axis))) & \
+									0xffff)
+
+#define PHB_RUNNING		1
+#define PHB_RESET		2
+
+static struct PH_CLASSTYPE *phantom_class;
+
+struct phantom_device {
+	void __iomem *caddr;
+	u32 __iomem *iaddr;
+	u32 __iomem *oaddr;
+	u32 amp_bit;
+	s16 torques[PHM_MAX_TORQUES];
+	unsigned long status;
+
+	struct input_dev *idev;
+};
+
+static int phantom_status(struct phantom_device *dev, unsigned long newstat)
+{
+	pr_debug("phantom_status %lx %lx\n", dev->status, newstat);
+
+	if (!(dev->status & PHB_RUNNING) && (newstat & PHB_RUNNING)) {
+		iowrite32(PHN_CTL_IRQ | PHN_CTL_AMP, dev->iaddr + PHN_CONTROL);
+		dev->amp_bit = PHN_CTL_IRQ;
+		iowrite32(0x43, dev->caddr + PHN_IRQCTL);
+	} else if ((dev->status & PHB_RUNNING) && !(newstat & PHB_RUNNING))
+		iowrite32(0, dev->caddr + PHN_IRQCTL);
+
+	dev->status = newstat;
+
+	return 0;
+}
+
+static void phantom_close(struct input_dev *idev)
+{
+	struct phantom_device *dev = idev->private;
+
+	phantom_status(dev, dev->status & ~PHB_RUNNING);
+}
+
+static void phantom_reset(struct phantom_device *dev)
+{
+	pr_debug("resetting\n");
+
+	iowrite32(~0x1f, dev->iaddr);
+	wmb();
+	iowrite32(0x1f, dev->iaddr);
+	dev->status |= PHB_RESET;
+}
+
+static void phantom_autocenter(struct input_dev *idev, u16 magnitude)
+{
+	struct phantom_device *dev = idev->private;
+
+	phantom_reset(dev);
+}
+
+static int phantom_play(struct input_dev *idev, void *data,
+		struct ff_effect *eff)
+{
+	struct phantom_device *dev = idev->private;
+	unsigned int a;
+	int retval;
+
+	pr_debug("playing\n");
+
+	if (unlikely(!(dev->status & PHB_RUNNING))) {
+		retval = phantom_status(dev, dev->status | PHB_RUNNING);
+		if (retval)
+			return retval;
+	}
+
+	if (unlikely(!(dev->status & PHB_RESET)))
+		printk(KERN_INFO "phantom: phantom is used, but the device was"
+				"not reset since last module load\n");
+
+	for (a = 0; a < PHM_MAX_TORQUES; a++)
+		dev->torques[a] = eff->u.ff_raw[a];
+
+	return 0;
+}
+
+static irqreturn_t phantom_isr(int irq, void *data)
+{
+	struct phantom_device *dev = data;
+	struct input_dev *idev = dev->idev;
+	unsigned int a, hw_status;
+
+	hw_status = ioread32(dev->iaddr + PHN_CONTROL);
+	if (!(hw_status & PHN_CTL_IRQ))
+		return IRQ_NONE;
+
+	iowrite32(0, dev->iaddr);
+	wmb();
+	iowrite32(0xc0, dev->iaddr);
+
+	if (unlikely(idev == NULL))
+		return IRQ_HANDLED;
+
+	input_report_abs(idev, ABS_X, PCI_ENCODER(dev, 0));
+	input_report_abs(idev, ABS_Y, PCI_ENCODER(dev, 1));
+	input_report_abs(idev, ABS_Z, PCI_ENCODER(dev, 2));
+	input_report_abs(idev, ABS_RX, PCI_ENCODER(dev, 3));
+	input_report_abs(idev, ABS_RY, PCI_ENCODER(dev, 4));
+	input_report_abs(idev, ABS_RZ, PCI_ENCODER(dev, 5));
+	input_report_key(idev, BTN_0, !!(hw_status & PHN_CTL_BUT));
+	input_sync(idev);
+	input_event(idev, EV_SYN, SYN_CONFIG, 0);
+
+	for (a = 0; a < PHM_MAX_TORQUES; a++)
+		iowrite32(dev->torques[a] + PHN_ZERO_FORCE, dev->oaddr + a);
+	iowrite32(dev->amp_bit, dev->iaddr + PHN_CONTROL);
+	dev->amp_bit ^= PHN_CTL_AMP;
+
+	return IRQ_HANDLED;
+}
+
+/******************************************************************************
+ * Init and deinit driver
+ */
+static void __devinit phantom_init_idev(const struct pci_dev *pdev,
+		struct phantom_device *dev)
+{
+	struct input_dev *idev = dev->idev;
+
+	idev->private = dev;
+	idev->name = "Sensable Phantom";
+	idev->id.bustype = BUS_PCI;
+	idev->id.vendor = pdev->vendor;
+	idev->id.product = pdev->device;
+	idev->id.version = 1;
+	idev->close = phantom_close;
+
+	set_bit(BTN_0, idev->keybit);
+	input_set_abs_params(idev, ABS_X, -0x8000, 0x7fff, 4, 0);
+	input_set_abs_params(idev, ABS_Y, -0x8000, 0x7fff, 4, 0);
+	input_set_abs_params(idev, ABS_Z, -0x8000, 0x7fff, 4, 0);
+	input_set_abs_params(idev, ABS_RX, -0x8000, 0x7fff, 4, 0);
+	input_set_abs_params(idev, ABS_RY, -0x8000, 0x7fff, 4, 0);
+	input_set_abs_params(idev, ABS_RZ, -0x8000, 0x7fff, 4, 0);
+	set_bit(FF_RAW, idev->ffbit);
+
+	set_bit(EV_KEY, idev->evbit);
+	set_bit(EV_ABS, idev->evbit);
+	set_bit(EV_FF, idev->evbit);
+	set_bit(FF_AUTOCENTER, idev->ffbit); /* reset phantom */
+}
+
+static int __devinit phantom_probe(struct pci_dev *pdev,
+	const struct pci_device_id *pci_id)
+{
+	struct phantom_device *pht;
+	int retval;
+
+	retval = pci_enable_device(pdev);
+	if (retval)
+		goto err;
+
+	retval = pci_request_regions(pdev, "phantom");
+	if (retval)
+		goto err;
+
+	retval = -ENOMEM;
+	pht = kzalloc(sizeof(*pht), GFP_KERNEL);
+	if (pht == NULL) {
+		dev_err(&pdev->dev, "unable to allocate device\n");
+		goto err_reg;
+	}
+
+	pht->caddr = pci_iomap(pdev, 0, 0);
+	if (pht->caddr == NULL) {
+		dev_err(&pdev->dev, "can't remap conf space\n");
+		goto err_fr;
+	}
+	pht->iaddr = pci_iomap(pdev, 2, 0);
+	if (pht->iaddr == NULL) {
+		dev_err(&pdev->dev, "can't remap input space\n");
+		goto err_unmc;
+	}
+	pht->oaddr = pci_iomap(pdev, 3, 0);
+	if (pht->oaddr == NULL) {
+		dev_err(&pdev->dev, "can't remap output space\n");
+		goto err_unmi;
+	}
+
+	iowrite32(0, pht->caddr + PHN_IRQCTL);
+	retval = request_irq(pdev->irq, phantom_isr,
+			IRQF_SHARED | IRQF_DISABLED, "phantom", pht);
+	if (retval) {
+		dev_err(&pdev->dev, "can't establish ISR\n");
+		goto err_unmo;
+	}
+
+	pht->idev = input_allocate_device();
+	if (pht->idev == NULL) {
+		dev_err(&pdev->dev, "can't create input device\n");
+		retval = -ENOMEM;
+		goto err_irq;
+	}
+
+	phantom_init_idev(pdev, pht);
+
+	retval = input_ff_create_memless(pht->idev, NULL, phantom_play);
+	if (retval) {
+		dev_err(&pdev->dev, "can't create FF device\n");
+		goto err_idev;
+	}
+
+	pht->idev->ff->set_autocenter = phantom_autocenter;
+
+	retval = input_register_device(pht->idev);
+	if (retval) {
+		dev_err(&pdev->dev, "can't register input device\n");
+		goto err_idev;
+	}
+
+	pci_set_drvdata(pdev, pht);
+
+	return 0;
+err_idev:
+	input_free_device(pht->idev);
+err_irq:
+	free_irq(pdev->irq, pht);
+err_unmo:
+	pci_iounmap(pdev, pht->oaddr);
+err_unmi:
+	pci_iounmap(pdev, pht->iaddr);
+err_unmc:
+	pci_iounmap(pdev, pht->caddr);
+err_fr:
+	kfree(pht);
+err_reg:
+	pci_release_regions(pdev);
+err:
+	return retval;
+}
+
+static void __devexit phantom_remove(struct pci_dev *pdev)
+{
+	struct phantom_device *pht = pci_get_drvdata(pdev);
+
+	iowrite32(0, pht->caddr + PHN_IRQCTL);
+
+	input_unregister_device(pht->idev);
+
+	free_irq(pdev->irq, pht);
+
+	pci_iounmap(pdev, pht->oaddr);
+	pci_iounmap(pdev, pht->iaddr);
+	pci_iounmap(pdev, pht->caddr);
+
+	kfree(pht);
+
+	pci_release_regions(pdev);
+}
+
+static int phantom_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+	struct phantom_device *dev = pci_get_drvdata(pdev);
+
+	iowrite32(0, dev->caddr + PHN_IRQCTL);
+
+	return 0;
+}
+
+static int phantom_resume(struct pci_dev *pdev)
+{
+	struct phantom_device *dev = pci_get_drvdata(pdev);
+
+	iowrite32(0, dev->caddr + PHN_IRQCTL);
+
+	return 0;
+}
+
+static struct pci_device_id phantom_pci_tbl[] __devinitdata = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050),
+		.class = PCI_CLASS_BRIDGE_OTHER << 8, .class_mask = 0xffff00 },
+	{ 0, }
+};
+MODULE_DEVICE_TABLE(pci, phantom_pci_tbl);
+
+static struct pci_driver phantom_pci_driver = {
+	.name = "phantom",
+	.id_table = phantom_pci_tbl,
+	.probe = phantom_probe,
+	.remove = __devexit_p(phantom_remove),
+	.suspend = phantom_suspend,
+	.resume = phantom_resume
+};
+
+static ssize_t phantom_show_version(struct class *cls, char *buf)
+{
+	return sprintf(buf, PHANTOM_VERSION "\n");
+}
+
+static CLASS_ATTR(version, 0444, phantom_show_version, NULL);
+
+static int __init phantom_init(void)
+{
+	int retval;
+
+	phantom_class = class_create(THIS_MODULE, "phantom");
+	if (IS_ERR(phantom_class)) {
+		retval = PTR_ERR(phantom_class);
+		printk(KERN_ERR "phantom: can't register phantom class\n");
+		goto err;
+	}
+	retval = class_create_file(phantom_class, &class_attr_version);
+	if (retval) {
+		printk(KERN_ERR "phantom: can't create sysfs version file\n");
+		goto err_class;
+	}
+
+	retval = pci_register_driver(&phantom_pci_driver);
+	if (retval) {
+		printk(KERN_ERR "phantom: can't register pci driver\n");
+		goto err_attr;
+	}
+
+	printk(KERN_INFO "Phantom Linux Driver, version " PHANTOM_VERSION ", "
+			"init OK\n");
+
+	return 0;
+err_attr:
+	class_remove_file(phantom_class, &class_attr_version);
+err_class:
+	class_destroy(phantom_class);
+err:
+	return retval;
+}
+
+static void __exit phantom_exit(void)
+{
+	pci_unregister_driver(&phantom_pci_driver);
+
+	class_remove_file(phantom_class, &class_attr_version);
+	class_destroy(phantom_class);
+
+	pr_debug("phantom: module successfully removed\n");
+}
+
+module_init(phantom_init);
+module_exit(phantom_exit);
+
+MODULE_AUTHOR("Jiri Slaby <jirislaby@gmail.com>");
+MODULE_DESCRIPTION("Sensable Phantom driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(PHANTOM_VERSION);

[RFC 2/2] Input: phantom, add a new driver

[Jiri Slaby]

phantom, add a new driver

Sensable Phantom is a up to 7DOF force feedback (up to 6DOF FF) device. It's
atypical, so it's based on the new added FF_RAW effect.

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>

---
commit 73621a789a0482299242ea61c971af6b5f8b828a
tree 365ba8113af9aaf468415d67238378767efad092
parent 759e7f172031959f49e5d3a7282379e7d73621b3
author Jiri Slaby <jirislaby@gmail.com> Tue, 17 Apr 2007 21:58:27 +0200
committer Jiri Slaby <jirislaby@gmail.com> Tue, 17 Apr 2007 21:58:27 +0200

 MAINTAINERS                  |    5 +
 drivers/input/misc/Kconfig   |    9 +
 drivers/input/misc/Makefile  |    1 
 drivers/input/misc/phantom.c |  387 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 402 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8a68712..86a8417 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3112,6 +3112,11 @@ L: 	selinux@tycho.nsa.gov (subscribers-only, general discussion)
 W:	http://www.nsa.gov/selinux
 S:	Supported
 
+SENSABLE PHANTOM
+P:	Jiri Slaby
+M:	jirislaby@gmail.com
+S:	Maintained
+
 SERIAL ATA (SATA) SUBSYSTEM:
 P:	Jeff Garzik
 M:	jgarzik@pobox.com
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 5694115..6e432b0 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -98,4 +98,13 @@ config HP_SDC_RTC
 	  Say Y here if you want to support the built-in real time clock
 	  of the HP SDC controller.
 
+config PHANTOM
+	tristate "Sensable PHANToM"
+	depends on PCI && INPUT_FF_MEMLESS
+	help
+	  Say Y here if you want to build a driver for Sensable PHANToM device.
+
+	  If you choose to build module, its name will be phantom. If unsure,
+	  say N here.
+
 endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 9f08f27..3ab3cc2 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_INPUT_WISTRON_BTNS)	+= wistron_btns.o
 obj-$(CONFIG_INPUT_ATLAS_BTNS)		+= atlas_btns.o
 obj-$(CONFIG_HP_SDC_RTC)		+= hp_sdc_rtc.o
 obj-$(CONFIG_INPUT_IXP4XX_BEEPER)	+= ixp4xx-beeper.o
+obj-$(CONFIG_PHANTOM)			+= phantom.o
diff --git a/drivers/input/misc/phantom.c b/drivers/input/misc/phantom.c
new file mode 100644
index 0000000..58f55cd
--- /dev/null
+++ b/drivers/input/misc/phantom.c
@@ -0,0 +1,387 @@
+/*
+ *  Copyright (C) 2005-2007 Jiri Slaby <jirislaby@gmail.com>
+ *
+ *  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.
+ *
+ *  You need an userspace library to cooperate with this driver. It (and other
+ *  info) may be obtained here:
+ *  http://www.fi.muni.cz/~xslaby/phantom.html
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/pci.h>
+#include <linux/fs.h>
+#include <linux/poll.h>
+#include <linux/interrupt.h>
+#include <linux/input.h>
+
+#include <asm/io.h>
+
+#define PHANTOM_VERSION		"n0.9.4"
+
+#define PHM_MAX_TORQUES		3
+
+#define PHN_CONTROL		0x6
+#define PHN_CTL_AMP		0x1
+#define PHN_CTL_BUT		0x2
+#define PHN_CTL_IRQ		0x10
+
+#define PHN_IRQCTL		0x4c
+
+#define PHN_ZERO_FORCE		2048
+
+#define PCI_ENCODER(dev, axis) ((0 - (int)ioread32((dev)->iaddr + (axis))) & \
+									0xffff)
+
+#define PHB_RUNNING		1
+#define PHB_RESET		2
+
+static struct PH_CLASSTYPE *phantom_class;
+
+struct phantom_device {
+	void __iomem *caddr;
+	u32 __iomem *iaddr;
+	u32 __iomem *oaddr;
+	u32 amp_bit;
+	s16 torques[PHM_MAX_TORQUES];
+	unsigned long status;
+
+	struct input_dev *idev;
+};
+
+static int phantom_status(struct phantom_device *dev, unsigned long newstat)
+{
+	pr_debug("phantom_status %lx %lx\n", dev->status, newstat);
+
+	if (!(dev->status & PHB_RUNNING) && (newstat & PHB_RUNNING)) {
+		iowrite32(PHN_CTL_IRQ | PHN_CTL_AMP, dev->iaddr + PHN_CONTROL);
+		dev->amp_bit = PHN_CTL_IRQ;
+		iowrite32(0x43, dev->caddr + PHN_IRQCTL);
+	} else if ((dev->status & PHB_RUNNING) && !(newstat & PHB_RUNNING))
+		iowrite32(0, dev->caddr + PHN_IRQCTL);
+
+	dev->status = newstat;
+
+	return 0;
+}
+
+static void phantom_close(struct input_dev *idev)
+{
+	struct phantom_device *dev = idev->private;
+
+	phantom_status(dev, dev->status & ~PHB_RUNNING);
+}
+
+static void phantom_reset(struct phantom_device *dev)
+{
+	pr_debug("resetting\n");
+
+	iowrite32(~0x1f, dev->iaddr);
+	wmb();
+	iowrite32(0x1f, dev->iaddr);
+	dev->status |= PHB_RESET;
+}
+
+static void phantom_autocenter(struct input_dev *idev, u16 magnitude)
+{
+	struct phantom_device *dev = idev->private;
+
+	phantom_reset(dev);
+}
+
+static int phantom_play(struct input_dev *idev, void *data,
+		struct ff_effect *eff)
+{
+	struct phantom_device *dev = idev->private;
+	unsigned int a;
+	int retval;
+
+	pr_debug("playing\n");
+
+	if (unlikely(!(dev->status & PHB_RUNNING))) {
+		retval = phantom_status(dev, dev->status | PHB_RUNNING);
+		if (retval)
+			return retval;
+	}
+
+	if (unlikely(!(dev->status & PHB_RESET)))
+		printk(KERN_INFO "phantom: phantom is used, but the device was"
+				"not reset since last module load\n");
+
+	for (a = 0; a < PHM_MAX_TORQUES; a++)
+		dev->torques[a] = eff->u.ff_raw[a];
+
+	return 0;
+}
+
+static irqreturn_t phantom_isr(int irq, void *data)
+{
+	struct phantom_device *dev = data;
+	struct input_dev *idev = dev->idev;
+	unsigned int a, hw_status;
+
+	hw_status = ioread32(dev->iaddr + PHN_CONTROL);
+	if (!(hw_status & PHN_CTL_IRQ))
+		return IRQ_NONE;
+
+	iowrite32(0, dev->iaddr);
+	wmb();
+	iowrite32(0xc0, dev->iaddr);
+
+	if (unlikely(idev == NULL))
+		return IRQ_HANDLED;
+
+	input_report_abs(idev, ABS_X, PCI_ENCODER(dev, 0));
+	input_report_abs(idev, ABS_Y, PCI_ENCODER(dev, 1));
+	input_report_abs(idev, ABS_Z, PCI_ENCODER(dev, 2));
+	input_report_abs(idev, ABS_RX, PCI_ENCODER(dev, 3));
+	input_report_abs(idev, ABS_RY, PCI_ENCODER(dev, 4));
+	input_report_abs(idev, ABS_RZ, PCI_ENCODER(dev, 5));
+	input_report_key(idev, BTN_0, !!(hw_status & PHN_CTL_BUT));
+	input_sync(idev);
+	input_event(idev, EV_SYN, SYN_CONFIG, 0);
+
+	for (a = 0; a < PHM_MAX_TORQUES; a++)
+		iowrite32(dev->torques[a] + PHN_ZERO_FORCE, dev->oaddr + a);
+	iowrite32(dev->amp_bit, dev->iaddr + PHN_CONTROL);
+	dev->amp_bit ^= PHN_CTL_AMP;
+
+	return IRQ_HANDLED;
+}
+
+/******************************************************************************
+ * Init and deinit driver
+ */
+static void __devinit phantom_init_idev(const struct pci_dev *pdev,
+		struct phantom_device *dev)
+{
+	struct input_dev *idev = dev->idev;
+
+	idev->private = dev;
+	idev->name = "Sensable Phantom";
+	idev->id.bustype = BUS_PCI;
+	idev->id.vendor = pdev->vendor;
+	idev->id.product = pdev->device;
+	idev->id.version = 1;
+	idev->close = phantom_close;
+
+	set_bit(BTN_0, idev->keybit);
+	input_set_abs_params(idev, ABS_X, -0x8000, 0x7fff, 4, 0);
+	input_set_abs_params(idev, ABS_Y, -0x8000, 0x7fff, 4, 0);
+	input_set_abs_params(idev, ABS_Z, -0x8000, 0x7fff, 4, 0);
+	input_set_abs_params(idev, ABS_RX, -0x8000, 0x7fff, 4, 0);
+	input_set_abs_params(idev, ABS_RY, -0x8000, 0x7fff, 4, 0);
+	input_set_abs_params(idev, ABS_RZ, -0x8000, 0x7fff, 4, 0);
+	set_bit(FF_RAW, idev->ffbit);
+
+	set_bit(EV_KEY, idev->evbit);
+	set_bit(EV_ABS, idev->evbit);
+	set_bit(EV_FF, idev->evbit);
+	set_bit(FF_AUTOCENTER, idev->ffbit); /* reset phantom */
+}
+
+static int __devinit phantom_probe(struct pci_dev *pdev,
+	const struct pci_device_id *pci_id)
+{
+	struct phantom_device *pht;
+	int retval;
+
+	retval = pci_enable_device(pdev);
+	if (retval)
+		goto err;
+
+	retval = pci_request_regions(pdev, "phantom");
+	if (retval)
+		goto err;
+
+	retval = -ENOMEM;
+	pht = kzalloc(sizeof(*pht), GFP_KERNEL);
+	if (pht == NULL) {
+		dev_err(&pdev->dev, "unable to allocate device\n");
+		goto err_reg;
+	}
+
+	pht->caddr = pci_iomap(pdev, 0, 0);
+	if (pht->caddr == NULL) {
+		dev_err(&pdev->dev, "can't remap conf space\n");
+		goto err_fr;
+	}
+	pht->iaddr = pci_iomap(pdev, 2, 0);
+	if (pht->iaddr == NULL) {
+		dev_err(&pdev->dev, "can't remap input space\n");
+		goto err_unmc;
+	}
+	pht->oaddr = pci_iomap(pdev, 3, 0);
+	if (pht->oaddr == NULL) {
+		dev_err(&pdev->dev, "can't remap output space\n");
+		goto err_unmi;
+	}
+
+	iowrite32(0, pht->caddr + PHN_IRQCTL);
+	retval = request_irq(pdev->irq, phantom_isr,
+			IRQF_SHARED | IRQF_DISABLED, "phantom", pht);
+	if (retval) {
+		dev_err(&pdev->dev, "can't establish ISR\n");
+		goto err_unmo;
+	}
+
+	pht->idev = input_allocate_device();
+	if (pht->idev == NULL) {
+		dev_err(&pdev->dev, "can't create input device\n");
+		retval = -ENOMEM;
+		goto err_irq;
+	}
+
+	phantom_init_idev(pdev, pht);
+
+	retval = input_ff_create_memless(pht->idev, NULL, phantom_play);
+	if (retval) {
+		dev_err(&pdev->dev, "can't create FF device\n");
+		goto err_idev;
+	}
+
+	pht->idev->ff->set_autocenter = phantom_autocenter;
+
+	retval = input_register_device(pht->idev);
+	if (retval) {
+		dev_err(&pdev->dev, "can't register input device\n");
+		goto err_idev;
+	}
+
+	pci_set_drvdata(pdev, pht);
+
+	return 0;
+err_idev:
+	input_free_device(pht->idev);
+err_irq:
+	free_irq(pdev->irq, pht);
+err_unmo:
+	pci_iounmap(pdev, pht->oaddr);
+err_unmi:
+	pci_iounmap(pdev, pht->iaddr);
+err_unmc:
+	pci_iounmap(pdev, pht->caddr);
+err_fr:
+	kfree(pht);
+err_reg:
+	pci_release_regions(pdev);
+err:
+	return retval;
+}
+
+static void __devexit phantom_remove(struct pci_dev *pdev)
+{
+	struct phantom_device *pht = pci_get_drvdata(pdev);
+
+	iowrite32(0, pht->caddr + PHN_IRQCTL);
+
+	input_unregister_device(pht->idev);
+
+	free_irq(pdev->irq, pht);
+
+	pci_iounmap(pdev, pht->oaddr);
+	pci_iounmap(pdev, pht->iaddr);
+	pci_iounmap(pdev, pht->caddr);
+
+	kfree(pht);
+
+	pci_release_regions(pdev);
+}
+
+static int phantom_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+	struct phantom_device *dev = pci_get_drvdata(pdev);
+
+	iowrite32(0, dev->caddr + PHN_IRQCTL);
+
+	return 0;
+}
+
+static int phantom_resume(struct pci_dev *pdev)
+{
+	struct phantom_device *dev = pci_get_drvdata(pdev);
+
+	iowrite32(0, dev->caddr + PHN_IRQCTL);
+
+	return 0;
+}
+
+static struct pci_device_id phantom_pci_tbl[] __devinitdata = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050),
+		.class = PCI_CLASS_BRIDGE_OTHER << 8, .class_mask = 0xffff00 },
+	{ 0, }
+};
+MODULE_DEVICE_TABLE(pci, phantom_pci_tbl);
+
+static struct pci_driver phantom_pci_driver = {
+	.name = "phantom",
+	.id_table = phantom_pci_tbl,
+	.probe = phantom_probe,
+	.remove = __devexit_p(phantom_remove),
+	.suspend = phantom_suspend,
+	.resume = phantom_resume
+};
+
+static ssize_t phantom_show_version(struct class *cls, char *buf)
+{
+	return sprintf(buf, PHANTOM_VERSION "\n");
+}
+
+static CLASS_ATTR(version, 0444, phantom_show_version, NULL);
+
+static int __init phantom_init(void)
+{
+	int retval;
+
+	phantom_class = class_create(THIS_MODULE, "phantom");
+	if (IS_ERR(phantom_class)) {
+		retval = PTR_ERR(phantom_class);
+		printk(KERN_ERR "phantom: can't register phantom class\n");
+		goto err;
+	}
+	retval = class_create_file(phantom_class, &class_attr_version);
+	if (retval) {
+		printk(KERN_ERR "phantom: can't create sysfs version file\n");
+		goto err_class;
+	}
+
+	retval = pci_register_driver(&phantom_pci_driver);
+	if (retval) {
+		printk(KERN_ERR "phantom: can't register pci driver\n");
+		goto err_attr;
+	}
+
+	printk(KERN_INFO "Phantom Linux Driver, version " PHANTOM_VERSION ", "
+			"init OK\n");
+
+	return 0;
+err_attr:
+	class_remove_file(phantom_class, &class_attr_version);
+err_class:
+	class_destroy(phantom_class);
+err:
+	return retval;
+}
+
+static void __exit phantom_exit(void)
+{
+	pci_unregister_driver(&phantom_pci_driver);
+
+	class_remove_file(phantom_class, &class_attr_version);
+	class_destroy(phantom_class);
+
+	pr_debug("phantom: module successfully removed\n");
+}
+
+module_init(phantom_init);
+module_exit(phantom_exit);
+
+MODULE_AUTHOR("Jiri Slaby <jirislaby@gmail.com>");
+MODULE_DESCRIPTION("Sensable Phantom driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(PHANTOM_VERSION);

Re: [RFC 1/2] Input: ff, add FF_RAW effect

[johann deneux]

Jiri,

Which solution did you chose to implement? From what I remember, we
last discussed Dmitry's idea of specifying an axis for an effect, then
combine several effects to achieve complex effects.
The implementation would specify the axis using the upper bits of the
effect type.

The patches you have sent seem to implement an easier, but in my
opinion inferior solution. If this FF_RAW effect does not have
semantics which are meaningful for other kinds of devices, I think
using the FF API is not a good idea.


On 4/17/07, Jiri Slaby <jirislaby@gmail.com> wrote:
> So fellows, what about these ones?
>
> --
>
> ff, add FF_RAW effect
>
> Add new FF_RAW effect for devices such Phantom. The new model has up to 6DOF
> torque force feedback independent on any 3d-or-so value.
>

Re: [RFC 1/2] Input: ff, add FF_RAW effect

[Jiri Slaby]

johann deneux napsal(a):
> Jiri,
> 
> Which solution did you chose to implement? From what I remember, we
> last discussed Dmitry's idea of specifying an axis for an effect, then
> combine several effects to achieve complex effects.

I think you mean motor instead of axis, because I don't push real axes to
the devices, but motor's torques...

> The implementation would specify the axis using the upper bits of the
> effect type.

Ok, if this is preferred, I'll post it with the const of having more context
switches for a single effect.

This was just a realization of the idea how I though it with the
quick'n'dirty FF_RAW.

thanks,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: [RFC 1/2] Input: ff, add FF_RAW effect

[johann deneux]

On 4/18/07, Jiri Slaby <jirislaby@gmail.com> wrote:
> johann deneux napsal(a):
> > Jiri,
> >
> > Which solution did you chose to implement? From what I remember, we
> > last discussed Dmitry's idea of specifying an axis for an effect, then
> > combine several effects to achieve complex effects.
>
> I think you mean motor instead of axis, because I don't push real axes to
> the devices, but motor's torques...
>

Yes, sorry, I meant motor.

-- 
Johann

Re: [RFC 1/2] Input: ff, add FF_RAW effect

[Dmitry Torokhov]

Hi,

On Thursday 19 April 2007 00:25, johann deneux wrote:
> On 4/18/07, Jiri Slaby <jirislaby@gmail.com> wrote:
> > johann deneux napsal(a):
> > > Jiri,
> > >
> > > Which solution did you chose to implement? From what I remember, we
> > > last discussed Dmitry's idea of specifying an axis for an effect, then
> > > combine several effects to achieve complex effects.
> >
> > I think you mean motor instead of axis, because I don't push real axes to
> > the devices, but motor's torques...
> >
> 
> Yes, sorry, I meant motor.
> 

I have been thinking about this and I don't think that exporting motor
data is a good idea, at least not in case of Phantom driver. The fact
that there are 3 motors is a hardware implementation detail and it
is not interesting for general application.

My understanding that the end result of controlling these 3 motors
is a force vector (I don't know if there is such english term, this
is a literal translation from russian) applied to user's hand.
If we are interested in using FF API we need to come up with a way
to express this effect without exposing implementation details of
one particular device.

-- 
Dmitry

Re: [RFC 1/2] Input: ff, add FF_RAW effect

[Jiri Slaby]

Dmitry Torokhov napsal(a):
> I have been thinking about this and I don't think that exporting motor
> data is a good idea, at least not in case of Phantom driver. The fact
> that there are 3 motors is a hardware implementation detail and it
> is not interesting for general application.

Ok, so what about torques (despite it's still something like FF_RAW or motor
descriptor)? It seems not to be so bad called effect name -- not only
phantom uses torques on motors as an unit of force, for example I get this
by quick googling:
http://www.caip.rutgers.edu/~bouzit/lrp/glove.html
They use there some motors with 14 torque values too, so the phantom isn't
the only device which uses this approach.

> My understanding that the end result of controlling these 3 motors

Actually there is also a version with 6 motors :).

> is a force vector (I don't know if there is such english term, this
> is a literal translation from russian) applied to user's hand.

Better say torques vector. You must compute a torque for each place from the
3d (or bigger) vector of forces in different way for each device that exists
-- this means forces are not independent unit, torques are in the meaning of
layer which doesn't care about what is connected above and below it.

> If we are interested in using FF API we need to come up with a way
> to express this effect without exposing implementation details of
> one particular device.

Still, torques are better named raw/motor values, which goes to the device
and I'm sceptic about inventing something class-better than this.

regards,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: [RFC 1/2] Input: ff, add FF_RAW effect

[Dmitry Torokhov]

On 4/19/07, Jiri Slaby <jirislaby@gmail.com> wrote:
> Dmitry Torokhov napsal(a):
> > I have been thinking about this and I don't think that exporting motor
> > data is a good idea, at least not in case of Phantom driver. The fact
> > that there are 3 motors is a hardware implementation detail and it
> > is not interesting for general application.
>
> Ok, so what about torques (despite it's still something like FF_RAW or motor
> descriptor)? It seems not to be so bad called effect name -- not only
> phantom uses torques on motors as an unit of force, for example I get this
> by quick googling:
> http://www.caip.rutgers.edu/~bouzit/lrp/glove.html
> They use there some motors with 14 torque values too, so the phantom isn't
> the only device which uses this approach.
>
> > My understanding that the end result of controlling these 3 motors
>
> Actually there is also a version with 6 motors :).
>
> > is a force vector (I don't know if there is such english term, this
> > is a literal translation from russian) applied to user's hand.
>
> Better say torques vector. You must compute a torque for each place from the
> 3d (or bigger) vector of forces in different way for each device that exists
> -- this means forces are not independent unit, torques are in the meaning of
> layer which doesn't care about what is connected above and below it.
>
> > If we are interested in using FF API we need to come up with a way
> > to express this effect without exposing implementation details of
> > one particular device.
>
> Still, torques are better named raw/motor values, which goes to the device
> and I'm sceptic about inventing something class-better than this.
>

Well, I guess we need to make a decision whether moving this kind of
devices into a force feedback layer is possible or whether every
device needs to have an application specifically tailored to that
particular device.

If we say that it is feasible to plug a device into FF layer then we
must not expose hardware implementation details. That means that
device-sepcific translation between 3d vector of forces into motor
torques must be done by the driver itself.

For devices that require tailored application (for example that glove
- I am not sure how a generic application could control it) old
phantom way of controlling via ioctl will suffice. The device may
still use input layer to report back coordinates.

-- 
Dmitry

Re: [RFC 2/2] Input: phantom, add a new driver

[Andrew Morton]

On Tue, 17 Apr 2007 22:02:10 +0200 (CEST) Jiri Slaby <jirislaby@gmail.com> wrote:

> phantom, add a new driver
> 
> Sensable Phantom is a up to 7DOF force feedback (up to 6DOF FF) device. It's
> atypical, so it's based on the new added FF_RAW effect.
> 
> diff --git a/drivers/input/misc/phantom.c b/drivers/input/misc/phantom.c
> new file mode 100644
> index 0000000..58f55cd
> --- /dev/null
> +++ b/drivers/input/misc/phantom.c
> @@ -0,0 +1,387 @@
> +/*
> + *  Copyright (C) 2005-2007 Jiri Slaby <jirislaby@gmail.com>
> + *
> + *  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.
> + *
> + *  You need an userspace library to cooperate with this driver. It (and other
> + *  info) may be obtained here:
> + *  http://www.fi.muni.cz/~xslaby/phantom.html
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/pci.h>
> +#include <linux/fs.h>
> +#include <linux/poll.h>
> +#include <linux/interrupt.h>
> +#include <linux/input.h>
> +
> +#include <asm/io.h>
> +
> +#define PHANTOM_VERSION		"n0.9.4"

That's an impressive version number ;)

> +#define PHM_MAX_TORQUES		3
> +
> +#define PHN_CONTROL		0x6
> +#define PHN_CTL_AMP		0x1
> +#define PHN_CTL_BUT		0x2
> +#define PHN_CTL_IRQ		0x10
> +
> +#define PHN_IRQCTL		0x4c
> +
> +#define PHN_ZERO_FORCE		2048

<wonders what all those do>

> +#define PCI_ENCODER(dev, axis) ((0 - (int)ioread32((dev)->iaddr + (axis))) & \
> +									0xffff)

Is there any reason why this cannot be a lower-cased inline C function? 
Nicer to read, typesafe, etc.

> +#define PHB_RUNNING		1
> +#define PHB_RESET		2
> +
> +static struct PH_CLASSTYPE *phantom_class;

I guess that PH_CLASSTYPE is some protect-me-from-gregkh compatibility
thing.  But there isn't such a macro in the tree.  I switched this to plain
old `class'.

> +struct phantom_device {
> +	void __iomem *caddr;
> +	u32 __iomem *iaddr;
> +	u32 __iomem *oaddr;
> +	u32 amp_bit;
> +	s16 torques[PHM_MAX_TORQUES];
> +	unsigned long status;
> +
> +	struct input_dev *idev;
> +};
> +
> +static int phantom_status(struct phantom_device *dev, unsigned long newstat)
> +{
> +	pr_debug("phantom_status %lx %lx\n", dev->status, newstat);
> +
> +	if (!(dev->status & PHB_RUNNING) && (newstat & PHB_RUNNING)) {
> +		iowrite32(PHN_CTL_IRQ | PHN_CTL_AMP, dev->iaddr + PHN_CONTROL);
> +		dev->amp_bit = PHN_CTL_IRQ;
> +		iowrite32(0x43, dev->caddr + PHN_IRQCTL);
> +	} else if ((dev->status & PHB_RUNNING) && !(newstat & PHB_RUNNING))
> +		iowrite32(0, dev->caddr + PHN_IRQCTL);
> +
> +	dev->status = newstat;
> +
> +	return 0;
> +}
> +
> +static void phantom_close(struct input_dev *idev)
> +{
> +	struct phantom_device *dev = idev->private;
> +
> +	phantom_status(dev, dev->status & ~PHB_RUNNING);
> +}
> +
> +static void phantom_reset(struct phantom_device *dev)
> +{
> +	pr_debug("resetting\n");
> +
> +	iowrite32(~0x1f, dev->iaddr);
> +	wmb();
> +	iowrite32(0x1f, dev->iaddr);
> +	dev->status |= PHB_RESET;
> +}

Does the wmb here actually do anything useful?  If so, a comment is needed
because it isn't possible to work out why that statement is there by
reading the code.

> ...
>
> +
> +static irqreturn_t phantom_isr(int irq, void *data)
> +{
> +	struct phantom_device *dev = data;
> +	struct input_dev *idev = dev->idev;
> +	unsigned int a, hw_status;
> +
> +	hw_status = ioread32(dev->iaddr + PHN_CONTROL);
> +	if (!(hw_status & PHN_CTL_IRQ))
> +		return IRQ_NONE;
> +
> +	iowrite32(0, dev->iaddr);
> +	wmb();
> +	iowrite32(0xc0, dev->iaddr);

there too.

> +	if (unlikely(idev == NULL))
> +		return IRQ_HANDLED;

Can this happen?  If so, a comment explaining why would be nice.

> +	input_report_abs(idev, ABS_X, PCI_ENCODER(dev, 0));
> +	input_report_abs(idev, ABS_Y, PCI_ENCODER(dev, 1));
> +	input_report_abs(idev, ABS_Z, PCI_ENCODER(dev, 2));
> +	input_report_abs(idev, ABS_RX, PCI_ENCODER(dev, 3));
> +	input_report_abs(idev, ABS_RY, PCI_ENCODER(dev, 4));
> +	input_report_abs(idev, ABS_RZ, PCI_ENCODER(dev, 5));
> +	input_report_key(idev, BTN_0, !!(hw_status & PHN_CTL_BUT));
> +	input_sync(idev);
> +	input_event(idev, EV_SYN, SYN_CONFIG, 0);
> +
> +	for (a = 0; a < PHM_MAX_TORQUES; a++)
> +		iowrite32(dev->torques[a] + PHN_ZERO_FORCE, dev->oaddr + a);
> +	iowrite32(dev->amp_bit, dev->iaddr + PHN_CONTROL);
> +	dev->amp_bit ^= PHN_CTL_AMP;
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/******************************************************************************

A plain old

/*
 * Init and deinit driver
 */

is sufficient and typical.

But this is not actually the most useful of comments anyway ;)

> + * Init and deinit driver
> + */
> +static void __devinit phantom_init_idev(const struct pci_dev *pdev,
> +		struct phantom_device *dev)
> +{
> +	struct input_dev *idev = dev->idev;
> +
> +	idev->private = dev;
> +	idev->name = "Sensable Phantom";
> +	idev->id.bustype = BUS_PCI;

<looks>

<wonders why BUS_PCI got defined in input.h>

<and in drivers/isdn/hardware/eicon/cardtype.h>

<and in drivers/char/sxboards.h>

<hurriedly stops looking>

> +	idev->id.vendor = pdev->vendor;
> +	idev->id.product = pdev->device;
> +	idev->id.version = 1;
> +	idev->close = phantom_close;
> +
> +	set_bit(BTN_0, idev->keybit);
> +	input_set_abs_params(idev, ABS_X, -0x8000, 0x7fff, 4, 0);
> +	input_set_abs_params(idev, ABS_Y, -0x8000, 0x7fff, 4, 0);
> +	input_set_abs_params(idev, ABS_Z, -0x8000, 0x7fff, 4, 0);
> +	input_set_abs_params(idev, ABS_RX, -0x8000, 0x7fff, 4, 0);
> +	input_set_abs_params(idev, ABS_RY, -0x8000, 0x7fff, 4, 0);
> +	input_set_abs_params(idev, ABS_RZ, -0x8000, 0x7fff, 4, 0);
> +	set_bit(FF_RAW, idev->ffbit);
> +
> +	set_bit(EV_KEY, idev->evbit);
> +	set_bit(EV_ABS, idev->evbit);
> +	set_bit(EV_FF, idev->evbit);
> +	set_bit(FF_AUTOCENTER, idev->ffbit); /* reset phantom */
> +}
> +
> +static int __devinit phantom_probe(struct pci_dev *pdev,
> +	const struct pci_device_id *pci_id)
> +{
> +	struct phantom_device *pht;
> +	int retval;
> +
> +	retval = pci_enable_device(pdev);
> +	if (retval)
> +		goto err;
> +
> +	retval = pci_request_regions(pdev, "phantom");
> +	if (retval)
> +		goto err;

Missed a pci_disable_device() here?

> +	retval = -ENOMEM;
> +	pht = kzalloc(sizeof(*pht), GFP_KERNEL);
> +	if (pht == NULL) {
> +		dev_err(&pdev->dev, "unable to allocate device\n");
> +		goto err_reg;
> +	}
> +
> +	pht->caddr = pci_iomap(pdev, 0, 0);
> +	if (pht->caddr == NULL) {
> +		dev_err(&pdev->dev, "can't remap conf space\n");
> +		goto err_fr;
> +	}
> +	pht->iaddr = pci_iomap(pdev, 2, 0);
> +	if (pht->iaddr == NULL) {
> +		dev_err(&pdev->dev, "can't remap input space\n");
> +		goto err_unmc;
> +	}
> +	pht->oaddr = pci_iomap(pdev, 3, 0);
> +	if (pht->oaddr == NULL) {
> +		dev_err(&pdev->dev, "can't remap output space\n");
> +		goto err_unmi;
> +	}
> +
> +	iowrite32(0, pht->caddr + PHN_IRQCTL);
> +	retval = request_irq(pdev->irq, phantom_isr,
> +			IRQF_SHARED | IRQF_DISABLED, "phantom", pht);
> +	if (retval) {
> +		dev_err(&pdev->dev, "can't establish ISR\n");
> +		goto err_unmo;
> +	}
> +
> +	pht->idev = input_allocate_device();
> +	if (pht->idev == NULL) {
> +		dev_err(&pdev->dev, "can't create input device\n");
> +		retval = -ENOMEM;
> +		goto err_irq;
> +	}
> +
> +	phantom_init_idev(pdev, pht);
> +
> +	retval = input_ff_create_memless(pht->idev, NULL, phantom_play);
> +	if (retval) {
> +		dev_err(&pdev->dev, "can't create FF device\n");
> +		goto err_idev;
> +	}
> +
> +	pht->idev->ff->set_autocenter = phantom_autocenter;
> +
> +	retval = input_register_device(pht->idev);
> +	if (retval) {
> +		dev_err(&pdev->dev, "can't register input device\n");
> +		goto err_idev;
> +	}
> +
> +	pci_set_drvdata(pdev, pht);
> +
> +	return 0;
> +err_idev:
> +	input_free_device(pht->idev);
> +err_irq:
> +	free_irq(pdev->irq, pht);
> +err_unmo:
> +	pci_iounmap(pdev, pht->oaddr);
> +err_unmi:
> +	pci_iounmap(pdev, pht->iaddr);
> +err_unmc:
> +	pci_iounmap(pdev, pht->caddr);
> +err_fr:
> +	kfree(pht);
> +err_reg:
> +	pci_release_regions(pdev);

err_disable:
	pci_disable_device(pdev);

> +err:
> +	return retval;
> +}
> +
> +static void __devexit phantom_remove(struct pci_dev *pdev)
> +{
> +	struct phantom_device *pht = pci_get_drvdata(pdev);
> +
> +	iowrite32(0, pht->caddr + PHN_IRQCTL);
> +
> +	input_unregister_device(pht->idev);
> +
> +	free_irq(pdev->irq, pht);
> +
> +	pci_iounmap(pdev, pht->oaddr);
> +	pci_iounmap(pdev, pht->iaddr);
> +	pci_iounmap(pdev, pht->caddr);
> +
> +	kfree(pht);
> +
> +	pci_release_regions(pdev);
> +}

#ifdef CONFIG_PM

> +static int phantom_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> +	struct phantom_device *dev = pci_get_drvdata(pdev);
> +
> +	iowrite32(0, dev->caddr + PHN_IRQCTL);
> +
> +	return 0;
> +}
> +
> +static int phantom_resume(struct pci_dev *pdev)
> +{
> +	struct phantom_device *dev = pci_get_drvdata(pdev);
> +
> +	iowrite32(0, dev->caddr + PHN_IRQCTL);
> +
> +	return 0;
> +}

#else
#define phantom_suspend NULL
#define phantom_resume NULL
#endif

(then test it with CONFIG_PM=n!)

> +static struct pci_device_id phantom_pci_tbl[] __devinitdata = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050),
> +		.class = PCI_CLASS_BRIDGE_OTHER << 8, .class_mask = 0xffff00 },
> +	{ 0, }
> +};
> +MODULE_DEVICE_TABLE(pci, phantom_pci_tbl);
> +
> +static struct pci_driver phantom_pci_driver = {
> +	.name = "phantom",
> +	.id_table = phantom_pci_tbl,
> +	.probe = phantom_probe,
> +	.remove = __devexit_p(phantom_remove),
> +	.suspend = phantom_suspend,
> +	.resume = phantom_resume
> +};

This goes into the read/write section.  Make it const?

> +static ssize_t phantom_show_version(struct class *cls, char *buf)
> +{
> +	return sprintf(buf, PHANTOM_VERSION "\n");
> +}
> +
> +static CLASS_ATTR(version, 0444, phantom_show_version, NULL);
> +
> +static int __init phantom_init(void)
> +{
> +	int retval;
> +
> +	phantom_class = class_create(THIS_MODULE, "phantom");
> +	if (IS_ERR(phantom_class)) {
> +		retval = PTR_ERR(phantom_class);
> +		printk(KERN_ERR "phantom: can't register phantom class\n");
> +		goto err;
> +	}
> +	retval = class_create_file(phantom_class, &class_attr_version);
> +	if (retval) {
> +		printk(KERN_ERR "phantom: can't create sysfs version file\n");
> +		goto err_class;
> +	}
> +
> +	retval = pci_register_driver(&phantom_pci_driver);
> +	if (retval) {
> +		printk(KERN_ERR "phantom: can't register pci driver\n");
> +		goto err_attr;
> +	}
> +
> +	printk(KERN_INFO "Phantom Linux Driver, version " PHANTOM_VERSION ", "
> +			"init OK\n");
> +
> +	return 0;
> +err_attr:
> +	class_remove_file(phantom_class, &class_attr_version);
> +err_class:
> +	class_destroy(phantom_class);
> +err:
> +	return retval;
> +}
> +
> +static void __exit phantom_exit(void)
> +{
> +	pci_unregister_driver(&phantom_pci_driver);
> +
> +	class_remove_file(phantom_class, &class_attr_version);
> +	class_destroy(phantom_class);
> +
> +	pr_debug("phantom: module successfully removed\n");
> +}
> +
> +module_init(phantom_init);
> +module_exit(phantom_exit);
> +
> +MODULE_AUTHOR("Jiri Slaby <jirislaby@gmail.com>");
> +MODULE_DESCRIPTION("Sensable Phantom driver");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(PHANTOM_VERSION);

Re: [RFC 2/2] Input: phantom, add a new driver

[Andrew Morton]

On Tue, 17 Apr 2007 22:02:10 +0200 (CEST) Jiri Slaby <jirislaby@gmail.com> wrote:

> phantom, add a new driver
> 
> Sensable Phantom is a up to 7DOF force feedback (up to 6DOF FF) device. It's
> atypical, so it's based on the new added FF_RAW effect.
> 
> diff --git a/drivers/input/misc/phantom.c b/drivers/input/misc/phantom.c
> new file mode 100644
> index 0000000..58f55cd
> --- /dev/null
> +++ b/drivers/input/misc/phantom.c
> @@ -0,0 +1,387 @@
> +/*
> + *  Copyright (C) 2005-2007 Jiri Slaby <jirislaby@gmail.com>
> + *
> + *  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.
> + *
> + *  You need an userspace library to cooperate with this driver. It (and other
> + *  info) may be obtained here:
> + *  http://www.fi.muni.cz/~xslaby/phantom.html
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/pci.h>
> +#include <linux/fs.h>
> +#include <linux/poll.h>
> +#include <linux/interrupt.h>
> +#include <linux/input.h>
> +
> +#include <asm/io.h>
> +
> +#define PHANTOM_VERSION		"n0.9.4"

That's an impressive version number ;)

> +#define PHM_MAX_TORQUES		3
> +
> +#define PHN_CONTROL		0x6
> +#define PHN_CTL_AMP		0x1
> +#define PHN_CTL_BUT		0x2
> +#define PHN_CTL_IRQ		0x10
> +
> +#define PHN_IRQCTL		0x4c
> +
> +#define PHN_ZERO_FORCE		2048

<wonders what all those do>

> +#define PCI_ENCODER(dev, axis) ((0 - (int)ioread32((dev)->iaddr + (axis))) & \
> +									0xffff)

Is there any reason why this cannot be a lower-cased inline C function? 
Nicer to read, typesafe, etc.

> +#define PHB_RUNNING		1
> +#define PHB_RESET		2
> +
> +static struct PH_CLASSTYPE *phantom_class;

I guess that PH_CLASSTYPE is some protect-me-from-gregkh compatibility
thing.  But there isn't such a macro in the tree.  I switched this to plain
old `class'.

> +struct phantom_device {
> +	void __iomem *caddr;
> +	u32 __iomem *iaddr;
> +	u32 __iomem *oaddr;
> +	u32 amp_bit;
> +	s16 torques[PHM_MAX_TORQUES];
> +	unsigned long status;
> +
> +	struct input_dev *idev;
> +};
> +
> +static int phantom_status(struct phantom_device *dev, unsigned long newstat)
> +{
> +	pr_debug("phantom_status %lx %lx\n", dev->status, newstat);
> +
> +	if (!(dev->status & PHB_RUNNING) && (newstat & PHB_RUNNING)) {
> +		iowrite32(PHN_CTL_IRQ | PHN_CTL_AMP, dev->iaddr + PHN_CONTROL);
> +		dev->amp_bit = PHN_CTL_IRQ;
> +		iowrite32(0x43, dev->caddr + PHN_IRQCTL);
> +	} else if ((dev->status & PHB_RUNNING) && !(newstat & PHB_RUNNING))
> +		iowrite32(0, dev->caddr + PHN_IRQCTL);
> +
> +	dev->status = newstat;
> +
> +	return 0;
> +}
> +
> +static void phantom_close(struct input_dev *idev)
> +{
> +	struct phantom_device *dev = idev->private;
> +
> +	phantom_status(dev, dev->status & ~PHB_RUNNING);
> +}
> +
> +static void phantom_reset(struct phantom_device *dev)
> +{
> +	pr_debug("resetting\n");
> +
> +	iowrite32(~0x1f, dev->iaddr);
> +	wmb();
> +	iowrite32(0x1f, dev->iaddr);
> +	dev->status |= PHB_RESET;
> +}

Does the wmb here actually do anything useful?  If so, a comment is needed
because it isn't possible to work out why that statement is there by
reading the code.

> ...
>
> +
> +static irqreturn_t phantom_isr(int irq, void *data)
> +{
> +	struct phantom_device *dev = data;
> +	struct input_dev *idev = dev->idev;
> +	unsigned int a, hw_status;
> +
> +	hw_status = ioread32(dev->iaddr + PHN_CONTROL);
> +	if (!(hw_status & PHN_CTL_IRQ))
> +		return IRQ_NONE;
> +
> +	iowrite32(0, dev->iaddr);
> +	wmb();
> +	iowrite32(0xc0, dev->iaddr);

there too.

> +	if (unlikely(idev == NULL))
> +		return IRQ_HANDLED;

Can this happen?  If so, a comment explaining why would be nice.

> +	input_report_abs(idev, ABS_X, PCI_ENCODER(dev, 0));
> +	input_report_abs(idev, ABS_Y, PCI_ENCODER(dev, 1));
> +	input_report_abs(idev, ABS_Z, PCI_ENCODER(dev, 2));
> +	input_report_abs(idev, ABS_RX, PCI_ENCODER(dev, 3));
> +	input_report_abs(idev, ABS_RY, PCI_ENCODER(dev, 4));
> +	input_report_abs(idev, ABS_RZ, PCI_ENCODER(dev, 5));
> +	input_report_key(idev, BTN_0, !!(hw_status & PHN_CTL_BUT));
> +	input_sync(idev);
> +	input_event(idev, EV_SYN, SYN_CONFIG, 0);
> +
> +	for (a = 0; a < PHM_MAX_TORQUES; a++)
> +		iowrite32(dev->torques[a] + PHN_ZERO_FORCE, dev->oaddr + a);
> +	iowrite32(dev->amp_bit, dev->iaddr + PHN_CONTROL);
> +	dev->amp_bit ^= PHN_CTL_AMP;
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/******************************************************************************

A plain old

/*
 * Init and deinit driver
 */

is sufficient and typical.

But this is not actually the most useful of comments anyway ;)

> + * Init and deinit driver
> + */
> +static void __devinit phantom_init_idev(const struct pci_dev *pdev,
> +		struct phantom_device *dev)
> +{
> +	struct input_dev *idev = dev->idev;
> +
> +	idev->private = dev;
> +	idev->name = "Sensable Phantom";
> +	idev->id.bustype = BUS_PCI;

<looks>

<wonders why BUS_PCI got defined in input.h>

<and in drivers/isdn/hardware/eicon/cardtype.h>

<and in drivers/char/sxboards.h>

<hurriedly stops looking>

> +	idev->id.vendor = pdev->vendor;
> +	idev->id.product = pdev->device;
> +	idev->id.version = 1;
> +	idev->close = phantom_close;
> +
> +	set_bit(BTN_0, idev->keybit);
> +	input_set_abs_params(idev, ABS_X, -0x8000, 0x7fff, 4, 0);
> +	input_set_abs_params(idev, ABS_Y, -0x8000, 0x7fff, 4, 0);
> +	input_set_abs_params(idev, ABS_Z, -0x8000, 0x7fff, 4, 0);
> +	input_set_abs_params(idev, ABS_RX, -0x8000, 0x7fff, 4, 0);
> +	input_set_abs_params(idev, ABS_RY, -0x8000, 0x7fff, 4, 0);
> +	input_set_abs_params(idev, ABS_RZ, -0x8000, 0x7fff, 4, 0);
> +	set_bit(FF_RAW, idev->ffbit);
> +
> +	set_bit(EV_KEY, idev->evbit);
> +	set_bit(EV_ABS, idev->evbit);
> +	set_bit(EV_FF, idev->evbit);
> +	set_bit(FF_AUTOCENTER, idev->ffbit); /* reset phantom */
> +}
> +
> +static int __devinit phantom_probe(struct pci_dev *pdev,
> +	const struct pci_device_id *pci_id)
> +{
> +	struct phantom_device *pht;
> +	int retval;
> +
> +	retval = pci_enable_device(pdev);
> +	if (retval)
> +		goto err;
> +
> +	retval = pci_request_regions(pdev, "phantom");
> +	if (retval)
> +		goto err;

Missed a pci_disable_device() here?

> +	retval = -ENOMEM;
> +	pht = kzalloc(sizeof(*pht), GFP_KERNEL);
> +	if (pht == NULL) {
> +		dev_err(&pdev->dev, "unable to allocate device\n");
> +		goto err_reg;
> +	}
> +
> +	pht->caddr = pci_iomap(pdev, 0, 0);
> +	if (pht->caddr == NULL) {
> +		dev_err(&pdev->dev, "can't remap conf space\n");
> +		goto err_fr;
> +	}
> +	pht->iaddr = pci_iomap(pdev, 2, 0);
> +	if (pht->iaddr == NULL) {
> +		dev_err(&pdev->dev, "can't remap input space\n");
> +		goto err_unmc;
> +	}
> +	pht->oaddr = pci_iomap(pdev, 3, 0);
> +	if (pht->oaddr == NULL) {
> +		dev_err(&pdev->dev, "can't remap output space\n");
> +		goto err_unmi;
> +	}
> +
> +	iowrite32(0, pht->caddr + PHN_IRQCTL);
> +	retval = request_irq(pdev->irq, phantom_isr,
> +			IRQF_SHARED | IRQF_DISABLED, "phantom", pht);
> +	if (retval) {
> +		dev_err(&pdev->dev, "can't establish ISR\n");
> +		goto err_unmo;
> +	}
> +
> +	pht->idev = input_allocate_device();
> +	if (pht->idev == NULL) {
> +		dev_err(&pdev->dev, "can't create input device\n");
> +		retval = -ENOMEM;
> +		goto err_irq;
> +	}
> +
> +	phantom_init_idev(pdev, pht);
> +
> +	retval = input_ff_create_memless(pht->idev, NULL, phantom_play);
> +	if (retval) {
> +		dev_err(&pdev->dev, "can't create FF device\n");
> +		goto err_idev;
> +	}
> +
> +	pht->idev->ff->set_autocenter = phantom_autocenter;
> +
> +	retval = input_register_device(pht->idev);
> +	if (retval) {
> +		dev_err(&pdev->dev, "can't register input device\n");
> +		goto err_idev;
> +	}
> +
> +	pci_set_drvdata(pdev, pht);
> +
> +	return 0;
> +err_idev:
> +	input_free_device(pht->idev);
> +err_irq:
> +	free_irq(pdev->irq, pht);
> +err_unmo:
> +	pci_iounmap(pdev, pht->oaddr);
> +err_unmi:
> +	pci_iounmap(pdev, pht->iaddr);
> +err_unmc:
> +	pci_iounmap(pdev, pht->caddr);
> +err_fr:
> +	kfree(pht);
> +err_reg:
> +	pci_release_regions(pdev);

err_disable:
	pci_disable_device(pdev);

> +err:
> +	return retval;
> +}
> +
> +static void __devexit phantom_remove(struct pci_dev *pdev)
> +{
> +	struct phantom_device *pht = pci_get_drvdata(pdev);
> +
> +	iowrite32(0, pht->caddr + PHN_IRQCTL);
> +
> +	input_unregister_device(pht->idev);
> +
> +	free_irq(pdev->irq, pht);
> +
> +	pci_iounmap(pdev, pht->oaddr);
> +	pci_iounmap(pdev, pht->iaddr);
> +	pci_iounmap(pdev, pht->caddr);
> +
> +	kfree(pht);
> +
> +	pci_release_regions(pdev);
> +}

#ifdef CONFIG_PM

> +static int phantom_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> +	struct phantom_device *dev = pci_get_drvdata(pdev);
> +
> +	iowrite32(0, dev->caddr + PHN_IRQCTL);
> +
> +	return 0;
> +}
> +
> +static int phantom_resume(struct pci_dev *pdev)
> +{
> +	struct phantom_device *dev = pci_get_drvdata(pdev);
> +
> +	iowrite32(0, dev->caddr + PHN_IRQCTL);
> +
> +	return 0;
> +}

#else
#define phantom_suspend NULL
#define phantom_resume NULL
#endif

(then test it with CONFIG_PM=n!)

> +static struct pci_device_id phantom_pci_tbl[] __devinitdata = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_PLX, PCI_DEVICE_ID_PLX_9050),
> +		.class = PCI_CLASS_BRIDGE_OTHER << 8, .class_mask = 0xffff00 },
> +	{ 0, }
> +};
> +MODULE_DEVICE_TABLE(pci, phantom_pci_tbl);
> +
> +static struct pci_driver phantom_pci_driver = {
> +	.name = "phantom",
> +	.id_table = phantom_pci_tbl,
> +	.probe = phantom_probe,
> +	.remove = __devexit_p(phantom_remove),
> +	.suspend = phantom_suspend,
> +	.resume = phantom_resume
> +};

This goes into the read/write section.  Make it const?

> +static ssize_t phantom_show_version(struct class *cls, char *buf)
> +{
> +	return sprintf(buf, PHANTOM_VERSION "\n");
> +}
> +
> +static CLASS_ATTR(version, 0444, phantom_show_version, NULL);
> +
> +static int __init phantom_init(void)
> +{
> +	int retval;
> +
> +	phantom_class = class_create(THIS_MODULE, "phantom");
> +	if (IS_ERR(phantom_class)) {
> +		retval = PTR_ERR(phantom_class);
> +		printk(KERN_ERR "phantom: can't register phantom class\n");
> +		goto err;
> +	}
> +	retval = class_create_file(phantom_class, &class_attr_version);
> +	if (retval) {
> +		printk(KERN_ERR "phantom: can't create sysfs version file\n");
> +		goto err_class;
> +	}
> +
> +	retval = pci_register_driver(&phantom_pci_driver);
> +	if (retval) {
> +		printk(KERN_ERR "phantom: can't register pci driver\n");
> +		goto err_attr;
> +	}
> +
> +	printk(KERN_INFO "Phantom Linux Driver, version " PHANTOM_VERSION ", "
> +			"init OK\n");
> +
> +	return 0;
> +err_attr:
> +	class_remove_file(phantom_class, &class_attr_version);
> +err_class:
> +	class_destroy(phantom_class);
> +err:
> +	return retval;
> +}
> +
> +static void __exit phantom_exit(void)
> +{
> +	pci_unregister_driver(&phantom_pci_driver);
> +
> +	class_remove_file(phantom_class, &class_attr_version);
> +	class_destroy(phantom_class);
> +
> +	pr_debug("phantom: module successfully removed\n");
> +}
> +
> +module_init(phantom_init);
> +module_exit(phantom_exit);
> +
> +MODULE_AUTHOR("Jiri Slaby <jirislaby@gmail.com>");
> +MODULE_DESCRIPTION("Sensable Phantom driver");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(PHANTOM_VERSION);

const struct pci_driver [Was: [RFC 2/2] Input: phantom, add a new driver]

[Jiri Slaby]

Andrew Morton napsal(a):
> On Tue, 17 Apr 2007 22:02:10 +0200 (CEST) Jiri Slaby <jirislaby@gmail.com> wrote:
> 
>> phantom, add a new driver
[...]
>> +static struct pci_driver phantom_pci_driver = {
>> +	.name = "phantom",
>> +	.id_table = phantom_pci_tbl,
>> +	.probe = phantom_probe,
>> +	.remove = __devexit_p(phantom_remove),
>> +	.suspend = phantom_suspend,
>> +	.resume = phantom_resume
>> +};
> 
> This goes into the read/write section.  Make it const?

Hmm, good question, I don't know the answer. Why are not pci_driver structs
const, Greg?

thanks,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: [RFC 2/2] Input: phantom, add a new driver

[Jiri Slaby]

Andrew Morton napsal(a):
> On Tue, 17 Apr 2007 22:02:10 +0200 (CEST) Jiri Slaby <jirislaby@gmail.com> wrote:
> 
>> phantom, add a new driver
[...]
>> +#define PHANTOM_VERSION		"n0.9.4"
> 
> That's an impressive version number ;)

fork of 0.8 or so 2.4 linux driver -> the n in the meaning of new :)

>> +#define PHM_MAX_TORQUES		3
>> +
>> +#define PHN_CONTROL		0x6
>> +#define PHN_CTL_AMP		0x1
>> +#define PHN_CTL_BUT		0x2
>> +#define PHN_CTL_IRQ		0x10
>> +
>> +#define PHN_IRQCTL		0x4c
>> +
>> +#define PHN_ZERO_FORCE		2048
> 
> <wonders what all those do>

I have no clue too, cut&paste from sensable 2.4 driver. But I'll document as
much as possible.

>> +#define PCI_ENCODER(dev, axis) ((0 - (int)ioread32((dev)->iaddr + (axis))) & \
>> +									0xffff)
> 
> Is there any reason why this cannot be a lower-cased inline C function? 
> Nicer to read, typesafe, etc.

yes, I'll switch it.

>> +#define PHB_RUNNING		1
>> +#define PHB_RESET		2
>> +
>> +static struct PH_CLASSTYPE *phantom_class;
> 
> I guess that PH_CLASSTYPE is some protect-me-from-gregkh compatibility
> thing.  But there isn't such a macro in the tree.  I switched this to plain
> old `class'.

Yes, my bad.

>> +
>> +static irqreturn_t phantom_isr(int irq, void *data)
>> +{
>> +	struct phantom_device *dev = data;
>> +	struct input_dev *idev = dev->idev;
>> +	unsigned int a, hw_status;
>> +
>> +	hw_status = ioread32(dev->iaddr + PHN_CONTROL);
>> +	if (!(hw_status & PHN_CTL_IRQ))
>> +		return IRQ_NONE;
>> +
>> +	iowrite32(0, dev->iaddr);
>> +	wmb();
>> +	iowrite32(0xc0, dev->iaddr);
> 
> there too.

Seems reasonable, it can't be reordered. (I hope this holds on all archs.)

>> +	if (unlikely(idev == NULL))
>> +		return IRQ_HANDLED;
> 
> Can this happen?  If so, a comment explaining why would be nice.

In the case of DEBUG_SHIRQ=y. Comment will be added or better -- devinit code
reordered.

thanks for notes,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: const struct pci_driver [Was: [RFC 2/2] Input: phantom, add a new driver]

[Greg KH]

On Fri, Apr 20, 2007 at 10:28:46AM +0200, Jiri Slaby wrote:
> Andrew Morton napsal(a):
> > On Tue, 17 Apr 2007 22:02:10 +0200 (CEST) Jiri Slaby <jirislaby@gmail.com> wrote:
> > 
> >> phantom, add a new driver
> [...]
> >> +static struct pci_driver phantom_pci_driver = {
> >> +	.name = "phantom",
> >> +	.id_table = phantom_pci_tbl,
> >> +	.probe = phantom_probe,
> >> +	.remove = __devexit_p(phantom_remove),
> >> +	.suspend = phantom_suspend,
> >> +	.resume = phantom_resume
> >> +};
> > 
> > This goes into the read/write section.  Make it const?
> 
> Hmm, good question, I don't know the answer. Why are not pci_driver structs
> const, Greg?

Because the pci core sets the needed pci_driver.driver structure fields
that the driver core needs in order to properly hook up things.

I'm considering a change to this that would allow pci (and all other)
driver structures be const, and then dynamically create the needed
driver core structure based on this "template" which would also let us
do some other things that the driver core wants, but that work is a few
months away.  When/if I get to that, I'll go through and mark all of the
driver structures const to move them to the read-only kernel section.

thanks,

greg k-h

Re: [RFC 1/2] Input: ff, add FF_RAW effect

[Jiri Slaby]

On 4/19/07, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On 4/19/07, Jiri Slaby <jirislaby@gmail.com> wrote:
> > Dmitry Torokhov napsal(a):
> > > If we are interested in using FF API we need to come up with a way
> > > to express this effect without exposing implementation details of
> > > one particular device.
> >
> > Still, torques are better named raw/motor values, which goes to the device
> > and I'm sceptic about inventing something class-better than this.
> >
>
> Well, I guess we need to make a decision whether moving this kind of
> devices into a force feedback layer is possible or whether every
> device needs to have an application specifically tailored to that
> particular device.
>
> If we say that it is feasible to plug a device into FF layer then we
> must not expose hardware implementation details. That means that
> device-sepcific translation between 3d vector of forces into motor
> torques must be done by the driver itself.

Hmm, it's 3d at minimum (the glove has 14 torque points, phantom up to
6), I think we won't be able to make the transition in independent way
due to unspecified meaning of the torques.

> For devices that require tailored application (for example that glove
> - I am not sure how a generic application could control it) old
> phantom way of controlling via ioctl will suffice. The device may
> still use input layer to report back coordinates.

Ok, nobody seems to have an idea, I'm going to avoid FF layer in the
code and hope this is the final change ;).

thanks,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: [RFC 1/2] Input: ff, add FF_RAW effect

[Jiri Slaby]

Dmitry Torokhov napsal(a):
> For devices that require tailored application (for example that glove
> - I am not sure how a generic application could control it) old
> phantom way of controlling via ioctl will suffice. The device may
> still use input layer to report back coordinates.

And how about the individual FF ioctl? Did you mean registering another
chardev, which is totally ugly in my eyes or augment evdev.c to support
driver specific ioctl? i.e. either add another 'E' ioctl with pointer to
struct { code, value } as arg param or changing
if (_IOC_TYPE(cmd) != 'E'))
	return -EINVAL;
to sth. like
if (_IOC_TYPE(cmd) != 'E'))
	return dev->ioctl ? dev->ioctl(file, cmd, p) : -EINVAL;
in evdev_ioctl_handler, which is acceptable?

thanks,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
 B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E

Re: [RFC 1/2] Input: ff, add FF_RAW effect

[Dmitry Torokhov]

Hi Jiri,

On 4/23/07, Jiri Slaby <jirislaby@gmail.com> wrote:
> Dmitry Torokhov napsal(a):
> > For devices that require tailored application (for example that glove
> > - I am not sure how a generic application could control it) old
> > phantom way of controlling via ioctl will suffice. The device may
> > still use input layer to report back coordinates.
>
> And how about the individual FF ioctl? Did you mean registering another
> chardev, which is totally ugly in my eyes or augment evdev.c to support
> driver specific ioctl? i.e. either add another 'E' ioctl with pointer to
> struct { code, value } as arg param or changing
> if (_IOC_TYPE(cmd) != 'E'))
>        return -EINVAL;
> to sth. like
> if (_IOC_TYPE(cmd) != 'E'))
>        return dev->ioctl ? dev->ioctl(file, cmd, p) : -EINVAL;
> in evdev_ioctl_handler, which is acceptable?
>

I really do not want to have driver-specific ioctls attaching to
evdev. What is wrong with a separate device to control phantom? You
won't even have to use ioctl but regial write on it.

-- 
Dmitry

Re: [RFC 1/2] Input: ff, add FF_RAW effect

[Dmitry Torokhov]

On 4/22/07, Jiri Slaby <jirislaby@gmail.com> wrote:
> On 4/19/07, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > On 4/19/07, Jiri Slaby <jirislaby@gmail.com> wrote:
> > > Dmitry Torokhov napsal(a):
> > > > If we are interested in using FF API we need to come up with a way
> > > > to express this effect without exposing implementation details of
> > > > one particular device.
> > >
> > > Still, torques are better named raw/motor values, which goes to the device
> > > and I'm sceptic about inventing something class-better than this.
> > >
> >
> > Well, I guess we need to make a decision whether moving this kind of
> > devices into a force feedback layer is possible or whether every
> > device needs to have an application specifically tailored to that
> > particular device.
> >
> > If we say that it is feasible to plug a device into FF layer then we
> > must not expose hardware implementation details. That means that
> > device-sepcific translation between 3d vector of forces into motor
> > torques must be done by the driver itself.
>
> Hmm, it's 3d at minimum (the glove has 14 torque points, phantom up to
> 6),

The vector of forces is always 3d in our world.

> I think we won't be able to make the transition in independent way
> due to unspecified meaning of the torques.
>

With phantom it still seems doable - if I understand correctly ther is
one point of containct and one resulting vector of forces applied to
user's hand. With the glove there are multiple points of contact and
multiple feedback effects applied to different part of hand
simultaneously and that is for sure requires special application.

-- 
Dmitry

Re: [RFC 1/2] Input: ff, add FF_RAW effect

[Jiri Slaby]

Dmitry Torokhov napsal(a):
> On 4/22/07, Jiri Slaby <jirislaby@gmail.com> wrote:
>> I think we won't be able to make the transition in independent way
>> due to unspecified meaning of the torques.
>>
> 
> With phantom it still seems doable - if I understand correctly ther is
> one point of containct and one resulting vector of forces applied to
> user's hand. With the glove there are multiple points of contact and
> multiple feedback effects applied to different part of hand
> simultaneously and that is for sure requires special application.

Hi.

Good news are, that I've got openhaptics (OS independent layer for sensable
devices) under academic license (I don't know, what the "open" word in that name
means -- no sourcecodes; licensed), the bad ones are, that they use ioctl
approach for both reporting and FF.

Furthermore I tried phantom with input layer in X and realized, that it's needed
to compute real axies from that numbers and that's what I'm doing in userspace
with doubke precision.

Ok, so the question is if the old ioctl approach (something which I posted as a
first patch before we ever starrted to talk about FF and input layer without
mmaping anything to US) is acceptable alone (this is what is needed for
OHaptics) or at least some particular computing to convert the numbers and
reporting to input layer should be implemented (which will have no users anyway,
who will ever want to have 60000$ pointer ;))?

thanks a lot,
-- 
http://www.fi.muni.cz/~xslaby/            Jiri Slaby
faculty of informatics, masaryk university, brno, cz
e-mail: jirislaby gmail com, gpg pubkey fingerprint:
B674 9967 0407 CE62 ACC8  22A0 32CC 55C3 39D4 7A7E