Re: [RFC 2/2] Input: phantom, add a new driver
On Tue, 17 Apr 2007 22:02:10 +0200 (CEST) Jiri Slaby [EMAIL PROTECTED] 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 000..58f55cd
--- /dev/null
+++ b/drivers/input/misc/phantom.c
@@ -0,0 +1,387 @@
+/*
+ * Copyright (C) 2005-2007 Jiri Slaby [EMAIL PROTECTED]
+ *
+ * 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)))
\
+ 0x)
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_RESET2
+
+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
Re: [RFC 2/2] Input: phantom, add a new driver
Andrew Morton napsal(a):
On Tue, 17 Apr 2007 22:02:10 +0200 (CEST) Jiri Slaby [EMAIL PROTECTED]
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, cutpaste from sensable 2.4 driver. But I'll document as
much as possible.
+#define PCI_ENCODER(dev, axis) ((0 - (int)ioread32((dev)-iaddr + (axis)))
\
+0x)
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
[RFC 2/2] Input: phantom, add a new driver
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 [EMAIL PROTECTED]
---
commit 73621a789a0482299242ea61c971af6b5f8b828a
tree 365ba8113af9aaf468415d67238378767efad092
parent 759e7f172031959f49e5d3a7282379e7d73621b3
author Jiri Slaby [EMAIL PROTECTED] Tue, 17 Apr 2007 21:58:27 +0200
committer Jiri Slaby [EMAIL PROTECTED] 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: [EMAIL PROTECTED] (subscribers-only, general
discussion)
W: http://www.nsa.gov/selinux
S: Supported
+SENSABLE PHANTOM
+P: Jiri Slaby
+M: [EMAIL PROTECTED]
+S: Maintained
+
SERIAL ATA (SATA) SUBSYSTEM:
P: Jeff Garzik
M: [EMAIL PROTECTED]
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 000..58f55cd
--- /dev/null
+++ b/drivers/input/misc/phantom.c
@@ -0,0 +1,387 @@
+/*
+ * Copyright (C) 2005-2007 Jiri Slaby [EMAIL PROTECTED]
+ *
+ * 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_VERSIONn0.9.4
+
+#define PHM_MAX_TORQUES3
+
+#define PHN_CONTROL0x6
+#define PHN_CTL_AMP0x1
+#define PHN_CTL_BUT0x2
+#define PHN_CTL_IRQ0x10
+
+#define PHN_IRQCTL 0x4c
+
+#define PHN_ZERO_FORCE 2048
+
+#define PCI_ENCODER(dev, axis) ((0 - (int)ioread32((dev)-iaddr + (axis))) \
+ 0x)
+
+#define PHB_RUNNING1
+#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)
+{
+
