RE: [PATCH] input: spi: Driver for SPI data stream driven vibrator
Hi,
From: ext Alan Cox [mailto:a...@lxorguk.ukuu.org.uk]
Sent: 08 November, 2010 01:52
+datalen = p-custom_len * sizeof(p-custom_data[0]);
signed
+if (datalen MAX_EFFECT_SIZE) {
unsigned
It should be unsigned. I'll fix it.
+memcpy(einfo-buf, p-custom_data, datalen);
ungood
Yep, that's clearly wrong too. Should be copy_from_user() I suppose.
Thanks, Ilkka
--
The Next 800 Companies to Lead America's Growth: New Video Whitepaper
David G. Thomson, author of the best-selling book Blueprint to a
Billion shares his insights and actions to help propel your
business during the next growth cycle. Listen Now!
http://p.sf.net/sfu/SAP-dev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] input: spi: Driver for SPI data stream driven vibrator
On Mon, 8 Nov 2010 12:08:07 +0100
ilkka.koski...@nokia.com wrote:
Hi,
From: ext Alan Cox [mailto:a...@lxorguk.ukuu.org.uk]
Sent: 08 November, 2010 01:52
+ datalen = p-custom_len * sizeof(p-custom_data[0]);
signed
+ if (datalen MAX_EFFECT_SIZE) {
unsigned
It should be unsigned. I'll fix it.
+ memcpy(einfo-buf, p-custom_data, datalen);
ungood
Yep, that's clearly wrong too. Should be copy_from_user() I suppose.
That I hadn't considered - and I'm not sure whether the caller is passed
a kernel copy or not. The problem I was looking at was just the signed
case
datalen 0
if (datalen MAX ..)
Nope
memcpy(kernel, mysource, vastly more than intended (unsigned))
--
The Next 800 Companies to Lead America's Growth: New Video Whitepaper
David G. Thomson, author of the best-selling book Blueprint to a
Billion shares his insights and actions to help propel your
business during the next growth cycle. Listen Now!
http://p.sf.net/sfu/SAP-dev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
RE: [PATCH] input: spi: Driver for SPI data stream driven vibrator
From: ext Alan Cox [mailto:a...@lxorguk.ukuu.org.uk]
Sent: 08 November, 2010 13:39
On Mon, 8 Nov 2010 12:08:07 +0100
ilkka.koski...@nokia.com wrote:
Hi,
From: ext Alan Cox [mailto:a...@lxorguk.ukuu.org.uk]
Sent: 08 November, 2010 01:52
+ datalen = p-custom_len * sizeof(p-custom_data[0]);
signed
+ if (datalen MAX_EFFECT_SIZE) {
unsigned
It should be unsigned. I'll fix it.
+ memcpy(einfo-buf, p-custom_data, datalen);
ungood
Yep, that's clearly wrong too. Should be copy_from_user() I suppose.
That I hadn't considered - and I'm not sure whether the caller is passed
a kernel copy or not. The problem I was looking at was just the signed
case
datalen 0
if (datalen MAX ..)
Nope
memcpy(kernel, mysource, vastly more than intended (unsigned))
Ah, I got it now. Thanks for clarification :)
Cheers, Ilkka
--
The Next 800 Companies to Lead America's Growth: New Video Whitepaper
David G. Thomson, author of the best-selling book Blueprint to a
Billion shares his insights and actions to help propel your
business during the next growth cycle. Listen Now!
http://p.sf.net/sfu/SAP-dev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] input: spi: Driver for SPI data stream driven vibrator
+ datalen = p-custom_len * sizeof(p-custom_data[0]);
signed
+ if (datalen MAX_EFFECT_SIZE) {
unsigned
+ memcpy(einfo-buf, p-custom_data, datalen);
ungood
--
The Next 800 Companies to Lead America's Growth: New Video Whitepaper
David G. Thomson, author of the best-selling book Blueprint to a
Billion shares his insights and actions to help propel your
business during the next growth cycle. Listen Now!
http://p.sf.net/sfu/SAP-dev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] input: spi: Driver for SPI data stream driven vibrator
On Tue, Oct 26, 2010 at 06:50:33PM +0200, ilkka.koski...@nokia.com wrote:
Hi Grant and thanks for comments,
[...]
+static int vibra_spi_playback(struct input_dev *input, int effect_id,
int value)
+{
+struct vibra_data *vibra = input_get_drvdata(input);
+struct effect_info *einfo = vibra-effects[effect_id];
+struct ff_effect *ff_effect = input-ff-effects[effect_id];
+
+if (!vibra-workqueue)
+return -ENODEV;
+
+if (test_bit(FF_EFFECT_UPLOADING, einfo-flags))
+return -EBUSY;
+
+if (value == 0) {
+/* Abort the given effect */
+if (test_bit(FF_EFFECT_PLAYING, einfo-flags))
+__set_bit(FF_EFFECT_ABORTING, einfo-flags);
+
+__clear_bit(FF_EFFECT_QUEUED, einfo-flags);
+} else {
+/* Move the given effect as the next one */
+__clear_bit(FF_EFFECT_QUEUED,
+vibra-effects[vibra-next_effect].flags);
+
+vibra-next_effect = effect_id;
+__set_bit(FF_EFFECT_QUEUED, einfo-flags);
+__clear_bit(FF_EFFECT_ABORTING, einfo-flags);
+einfo-stop_at = jiffies +
+msecs_to_jiffies(ff_effect-replay.length);
+
+if (vibra-status == IDLE) {
+vibra-status = STARTED;
+queue_work(vibra-workqueue, vibra-play_work);
+}
+}
I can't speak anything about the input event handling because I'm not
very familiar with it. However, it looks like the shared effect data
(vibra-effects) is getting modified outside of a critical region. Is
this safe?
Hmmm, I don't know why the force feedback layer is using a spin lock,
but it looks like overkill. Since you're already deferring work, I
would look at queueing the request and pushing down the spin lock
exposure as much as possible, but I'm really not the expert on the
input layer.
--
Nokia and ATT present the 2010 Calling All Innovators-North America contest
Create new apps games for the Nokia N8 for consumers in U.S. and Canada
$10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing
Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store
http://p.sf.net/sfu/nokia-dev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] input: spi: Driver for SPI data stream driven vibrator
On Wed, Oct 27, 2010 at 09:47:30AM +0100, Grant Likely wrote:
On Tue, Oct 26, 2010 at 06:50:33PM +0200, ilkka.koski...@nokia.com wrote:
Hi Grant and thanks for comments,
[...]
+static int vibra_spi_playback(struct input_dev *input, int effect_id,
int value)
+{
+struct vibra_data *vibra = input_get_drvdata(input);
+struct effect_info *einfo = vibra-effects[effect_id];
+struct ff_effect *ff_effect = input-ff-effects[effect_id];
+
+if (!vibra-workqueue)
+return -ENODEV;
+
+if (test_bit(FF_EFFECT_UPLOADING, einfo-flags))
+return -EBUSY;
+
+if (value == 0) {
+/* Abort the given effect */
+if (test_bit(FF_EFFECT_PLAYING, einfo-flags))
+__set_bit(FF_EFFECT_ABORTING, einfo-flags);
+
+__clear_bit(FF_EFFECT_QUEUED, einfo-flags);
+} else {
+/* Move the given effect as the next one */
+__clear_bit(FF_EFFECT_QUEUED,
+vibra-effects[vibra-next_effect].flags);
+
+vibra-next_effect = effect_id;
+__set_bit(FF_EFFECT_QUEUED, einfo-flags);
+__clear_bit(FF_EFFECT_ABORTING, einfo-flags);
+einfo-stop_at = jiffies +
+msecs_to_jiffies(ff_effect-replay.length);
+
+if (vibra-status == IDLE) {
+vibra-status = STARTED;
+queue_work(vibra-workqueue, vibra-play_work);
+}
+}
I can't speak anything about the input event handling because I'm not
very familiar with it. However, it looks like the shared effect data
(vibra-effects) is getting modified outside of a critical region. Is
this safe?
Hmmm, I don't know why the force feedback layer is using a spin lock,
but it looks like overkill. Since you're already deferring work, I
would look at queueing the request and pushing down the spin lock
exposure as much as possible, but I'm really not the expert on the
input layer.
Events are often sent from IRQ context so input core has to use spinlocks to
protect its internal data structures. Since output events travel the
same data path the event handlers are also called in atomic context.
--
Dmitry
--
Nokia and ATT present the 2010 Calling All Innovators-North America contest
Create new apps games for the Nokia N8 for consumers in U.S. and Canada
$10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing
Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store
http://p.sf.net/sfu/nokia-dev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] input: spi: Driver for SPI data stream driven vibrator
The driver could, of course, calculate the bit streams with help of board specific functions. It was just thought that tuning vibration device would be a lot easier, if we could just modify the bit streams provided by user space application. In fact, the bit stream basically consists of series of PWM pulses that are fed to the vibra. Perhaps, calculating the waveforms in the driver at probe time would be the most natural choice :\ Or assuming its a trivial conversion accepting them in a standard format and doing a quick pass across them ? (and failing that adding and documenting the format..) -- Nokia and ATT present the 2010 Calling All Innovators-North America contest Create new apps games for the Nokia N8 for consumers in U.S. and Canada $10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store http://p.sf.net/sfu/nokia-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
RE: [PATCH] input: spi: Driver for SPI data stream driven vibrator
Hi Dmitry, From: ext Dmitry Torokhov [mailto:dmitry.torok...@gmail.com] Sent: 26 October, 2010 19:10 To: Koskinen Ilkka (Nokia-MS/Tampere) Cc: linux-in...@vger.kernel.org; linux-ker...@vger.kernel.org; spi- devel-gene...@lists.sourceforge.net Subject: Re: [PATCH] input: spi: Driver for SPI data stream driven vibrator Hi Ilkka, On Mon, Oct 25, 2010 at 04:31:02PM +0300, Ilkka Koskinen wrote: This driver provides access to drive a vibrator connected to SPI data line via Input layer's Force Feedback interface. Client application provides samples (data streams) to be played as CUSTOM_DATA. The samples are stored in driver's internal buffers. If device is able to do custom waveform can't it also do regular effects (constant, periodic, etc. Or is custom is actually random and you are doing something like rumble effect? How about if I told about the vibration device a little? :) The device can only accept analog signal with relatively narrow frequency band. The bit stream consists of n-bit wide words that are interpreted as PWM pulses. PWM pulses are used to generate the desired waveform (close to sine wave). One problem is that wave period needs to be symmetric in order to avoid DC-offset. Therefore, we always have to play complete periods. Another issue is that due to narrow frequency band, we'd like to minimize the gap (or at least keep it quite stable) between waves as that would affect the overall frequency once again. Narrow band also means that the overall mechanics plays a bigger role and probably needs more careful tuning. Due to tuning issues, I'd prefer custom data provided by user space. By that we would avoid compiling kernel unnecessarily. Calculating the waveforms in the driver with help from board specific functions sounds tempting as well, but would it be better approach after all? The driver is not able to mix the given samples. Instead, it remembers the currently played sample and next one to be played. Why is this driver not using the memoryless FF library (and extends it to handle FF_CUSTOM effects) but rather reimplements it in the driver itself? I don't know how I missed this one :( So if FF_CUSTOM is the option to take, it makes sense to move the buffers to the memoryless FF library. Signed-off-by: Ilkka Koskinen ilkka.koski...@nokia.com --- drivers/input/misc/Kconfig |5 + drivers/input/misc/Makefile|2 +- drivers/input/misc/vibra_spi.c | 429 include/linux/spi/vibra.h | 34 4 files changed, 469 insertions(+), 1 deletions(-) create mode 100644 drivers/input/misc/vibra_spi.c create mode 100644 include/linux/spi/vibra.h diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig index b49e233..3441832 100644 --- a/drivers/input/misc/Kconfig +++ b/drivers/input/misc/Kconfig @@ -438,4 +438,9 @@ config INPUT_ADXL34X_SPI To compile this driver as a module, choose M here: the module will be called adxl34x-spi. +config INPUT_SPI_VIBRA +tristate Support for SPI driven Vibra module +help + Support for Vibra module that is connected to OMAP SPI bus. + To compile this driver as a module. Also please keep Kconfig and Makefile sorted alphabetically. I'll fix this. Cheers, Ilkka -- Nokia and ATT present the 2010 Calling All Innovators-North America contest Create new apps games for the Nokia N8 for consumers in U.S. and Canada $10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store http://p.sf.net/sfu/nokia-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] input: spi: Driver for SPI data stream driven vibrator
+ if (!einfo-buf) {
+ einfo-buf = kzalloc(datalen, GFP_KERNEL | GFP_DMA);
+ if (!einfo-buf) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+ }
+
+ memcpy(einfo-buf, p-custom_data, datalen);
It looks like raw data from userspace is being passed on to the
device. Is this sane? Is there already a data format used by other
vibration/feedback devices that should be used here instead and
translated into the form expected by the hardware?
It also seems to be using GFP_DMA not dma_alloc functions which looks a
bit odd and certainly isn't portable.
--
Nokia and ATT present the 2010 Calling All Innovators-North America contest
Create new apps games for the Nokia N8 for consumers in U.S. and Canada
$10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing
Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store
http://p.sf.net/sfu/nokia-dev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH] input: spi: Driver for SPI data stream driven vibrator
On Mon, Oct 25, 2010 at 04:31:02PM +0300, Ilkka Koskinen wrote:
This driver provides access to drive a vibrator connected
to SPI data line via Input layer's Force Feedback interface.
Client application provides samples (data streams) to be
played as CUSTOM_DATA. The samples are stored in driver's
internal buffers.
The driver is not able to mix the given samples. Instead, it
remembers the currently played sample and next one to be played.
Signed-off-by: Ilkka Koskinen ilkka.koski...@nokia.com
Hi Ilkka,
comments below...
---
drivers/input/misc/Kconfig |5 +
drivers/input/misc/Makefile|2 +-
drivers/input/misc/vibra_spi.c | 429
include/linux/spi/vibra.h | 34
4 files changed, 469 insertions(+), 1 deletions(-)
create mode 100644 drivers/input/misc/vibra_spi.c
create mode 100644 include/linux/spi/vibra.h
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index b49e233..3441832 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -438,4 +438,9 @@ config INPUT_ADXL34X_SPI
To compile this driver as a module, choose M here: the
module will be called adxl34x-spi.
+config INPUT_SPI_VIBRA
To match convention already used in this file: config INPUT_VIBRA_SPI
+ tristate Support for SPI driven Vibra module
+ help
+ Support for Vibra module that is connected to OMAP SPI bus.
Is this an OMAP specific device?
+
endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 19ccca7..cde272f 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -41,4 +41,4 @@ obj-$(CONFIG_INPUT_WINBOND_CIR) += winbond-cir.o
obj-$(CONFIG_INPUT_WISTRON_BTNS) += wistron_btns.o
obj-$(CONFIG_INPUT_WM831X_ON)+= wm831x-on.o
obj-$(CONFIG_INPUT_YEALINK) += yealink.o
-
+obj-$(CONFIG_INPUT_SPI_VIBRA) += vibra_spi.o
This file is nominally alphabetical sorted. Put this line between
CONFIG_INPUT_UINPUT and CONFIG_INPUT_WINBOND_CIR. Also, whitespace
inconsistency. Use tabs for indent.
diff --git a/drivers/input/misc/vibra_spi.c b/drivers/input/misc/vibra_spi.c
new file mode 100644
index 000..551a3b8
--- /dev/null
+++ b/drivers/input/misc/vibra_spi.c
@@ -0,0 +1,429 @@
+/*
+ * This file implements a driver for SPI data driven vibrator.
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * Contact: Ilkka Koskinen ilkka.koski...@nokia.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include linux/irq.h
+#include linux/module.h
+#include linux/workqueue.h
+#include linux/spinlock.h
+#include linux/delay.h
+#include linux/slab.h
+#include linux/jiffies.h
+#include linux/spi/spi.h
+#include linux/input.h
+#include linux/spi/vibra.h
+#include linux/io.h
+
+/* Number of effects handled with memoryless devices */
+#define VIBRA_EFFECTS36
+#define MAX_EFFECT_SIZE 1024 /* In bytes */
+
+#define FF_EFFECT_QUEUED 0
+#define FF_EFFECT_PLAYING1
+#define FF_EFFECT_ABORTING 2
+#define FF_EFFECT_UPLOADING 3
These are only ever used in a bitfield. The code will be simpler if
you change this to:
#define FF_EFFECT_QUEUED(10)
#define FF_EFFECT_PLAYING (11)
#define FF_EFFECT_ABORTING (12)
#define FF_EFFECT_UPLOADING (13)
That way the test_bit() and __set_bit() calls can be replaced with
regular bitwise operators, and the code will be easier to read.
+
+enum vibra_status {
+ IDLE = 0,
+ STARTED,
+ PLAYING,
+ CLOSING,
+};
+
+struct effect_info {
struct vibra_effect_info
+ char*buf;
+ int buflen;
+ unsigned long flags; /* effect state (STARTED, PLAYING, etc) */
+ unsigned long stop_at;
+};
+
+struct vibra_data {
+ struct device *dev;
+ struct input_dev*input_dev;
+
+ struct workqueue_struct *workqueue;
+ struct work_struct play_work;
+
+ struct spi_device *spi_dev;
+ struct spi_transfer t;
+ struct spi_message msg;
+ u32 spi_max_speed_hz;
+
+ void (*set_power)(bool enable);
+
+ enum vibra_status status;
+
+ struct effect_info
Re: [PATCH] input: spi: Driver for SPI data stream driven vibrator
Hi Ilkka, On Mon, Oct 25, 2010 at 04:31:02PM +0300, Ilkka Koskinen wrote: This driver provides access to drive a vibrator connected to SPI data line via Input layer's Force Feedback interface. Client application provides samples (data streams) to be played as CUSTOM_DATA. The samples are stored in driver's internal buffers. If device is able to do custom waveform can't it also do regular effects (constant, periodic, etc. Or is custom is actually random and you are doing something like rumble effect? The driver is not able to mix the given samples. Instead, it remembers the currently played sample and next one to be played. Why is this driver not using the memoryless FF library (and extends it to handle FF_CUSTOM effects) but rather reimplements it in the driver itself? Signed-off-by: Ilkka Koskinen ilkka.koski...@nokia.com --- drivers/input/misc/Kconfig |5 + drivers/input/misc/Makefile|2 +- drivers/input/misc/vibra_spi.c | 429 include/linux/spi/vibra.h | 34 4 files changed, 469 insertions(+), 1 deletions(-) create mode 100644 drivers/input/misc/vibra_spi.c create mode 100644 include/linux/spi/vibra.h diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig index b49e233..3441832 100644 --- a/drivers/input/misc/Kconfig +++ b/drivers/input/misc/Kconfig @@ -438,4 +438,9 @@ config INPUT_ADXL34X_SPI To compile this driver as a module, choose M here: the module will be called adxl34x-spi. +config INPUT_SPI_VIBRA + tristate Support for SPI driven Vibra module + help + Support for Vibra module that is connected to OMAP SPI bus. + To compile this driver as a module. Also please keep Kconfig and Makefile sorted alphabetically. Thanks. -- Dmitry -- Nokia and ATT present the 2010 Calling All Innovators-North America contest Create new apps games for the Nokia N8 for consumers in U.S. and Canada $10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store http://p.sf.net/sfu/nokia-dev2dev ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
RE: [PATCH] input: spi: Driver for SPI data stream driven vibrator
Hi Grant and thanks for comments,
From: Grant Likely [mailto:glik...@secretlab.ca] On Behalf Of ext Grant
Likely
Sent: 26 October, 2010 14:14
Subject: Re: [PATCH] input: spi: Driver for SPI data stream driven
vibrator
On Mon, Oct 25, 2010 at 04:31:02PM +0300, Ilkka Koskinen wrote:
This driver provides access to drive a vibrator connected
to SPI data line via Input layer's Force Feedback interface.
Client application provides samples (data streams) to be
played as CUSTOM_DATA. The samples are stored in driver's
internal buffers.
The driver is not able to mix the given samples. Instead, it
remembers the currently played sample and next one to be played.
Signed-off-by: Ilkka Koskinen ilkka.koski...@nokia.com
Hi Ilkka,
comments below...
---
drivers/input/misc/Kconfig |5 +
drivers/input/misc/Makefile|2 +-
drivers/input/misc/vibra_spi.c | 429
include/linux/spi/vibra.h | 34
4 files changed, 469 insertions(+), 1 deletions(-)
create mode 100644 drivers/input/misc/vibra_spi.c
create mode 100644 include/linux/spi/vibra.h
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index b49e233..3441832 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -438,4 +438,9 @@ config INPUT_ADXL34X_SPI
To compile this driver as a module, choose M here: the
module will be called adxl34x-spi.
+config INPUT_SPI_VIBRA
To match convention already used in this file: config INPUT_VIBRA_SPI
I'll fix it.
+tristate Support for SPI driven Vibra module
+help
+ Support for Vibra module that is connected to OMAP SPI bus.
Is this an OMAP specific device?
Obviously not, I'll change the text.
+
endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 19ccca7..cde272f 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -41,4 +41,4 @@ obj-$(CONFIG_INPUT_WINBOND_CIR)+= winbond-
cir.o
obj-$(CONFIG_INPUT_WISTRON_BTNS)+= wistron_btns.o
obj-$(CONFIG_INPUT_WM831X_ON) += wm831x-on.o
obj-$(CONFIG_INPUT_YEALINK) += yealink.o
-
+obj-$(CONFIG_INPUT_SPI_VIBRA) += vibra_spi.o
This file is nominally alphabetical sorted. Put this line between
CONFIG_INPUT_UINPUT and CONFIG_INPUT_WINBOND_CIR. Also, whitespace
inconsistency. Use tabs for indent.
My bad, sorry about that :(
diff --git a/drivers/input/misc/vibra_spi.c
b/drivers/input/misc/vibra_spi.c
new file mode 100644
index 000..551a3b8
--- /dev/null
+++ b/drivers/input/misc/vibra_spi.c
@@ -0,0 +1,429 @@
+/*
+ * This file implements a driver for SPI data driven vibrator.
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * Contact: Ilkka Koskinen ilkka.koski...@nokia.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include linux/irq.h
+#include linux/module.h
+#include linux/workqueue.h
+#include linux/spinlock.h
+#include linux/delay.h
+#include linux/slab.h
+#include linux/jiffies.h
+#include linux/spi/spi.h
+#include linux/input.h
+#include linux/spi/vibra.h
+#include linux/io.h
+
+/* Number of effects handled with memoryless devices */
+#define VIBRA_EFFECTS 36
+#define MAX_EFFECT_SIZE 1024 /* In bytes */
+
+#define FF_EFFECT_QUEUED0
+#define FF_EFFECT_PLAYING 1
+#define FF_EFFECT_ABORTING 2
+#define FF_EFFECT_UPLOADING 3
These are only ever used in a bitfield. The code will be simpler if
you change this to:
#define FF_EFFECT_QUEUED (10)
#define FF_EFFECT_PLAYING (11)
#define FF_EFFECT_ABORTING (12)
#define FF_EFFECT_UPLOADING(13)
That way the test_bit() and __set_bit() calls can be replaced with
regular bitwise operators, and the code will be easier to read.
Makes sense. I'll change them
+
+enum vibra_status {
+IDLE = 0,
+STARTED,
+PLAYING,
+CLOSING,
+};
+
+struct effect_info {
struct vibra_effect_info
I'll change it
+char*buf;
+int buflen;
+unsigned long flags; /* effect state (STARTED, PLAYING,
etc) */
+unsigned long stop_at;
+};
+
+struct vibra_data {
+struct device *dev;
+struct input_dev*input_dev;
+
+struct workqueue_struct *workqueue;
+struct work_struct
RE: [PATCH] input: spi: Driver for SPI data stream driven vibrator
Hi,
From: ext Alan Cox [mailto:a...@lxorguk.ukuu.org.uk]
Sent: 26 October, 2010 14:18
To: Grant Likely
Cc: Koskinen Ilkka (Nokia-MS/Tampere); linux-in...@vger.kernel.org;
dmitry.torok...@gmail.com; spi-devel-general@lists.sourceforge.net;
linux-ker...@vger.kernel.org
Subject: Re: [PATCH] input: spi: Driver for SPI data stream driven
vibrator
+ if (!einfo-buf) {
+ einfo-buf = kzalloc(datalen, GFP_KERNEL | GFP_DMA);
+ if (!einfo-buf) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+ }
+
+ memcpy(einfo-buf, p-custom_data, datalen);
It looks like raw data from userspace is being passed on to the
device. Is this sane? Is there already a data format used by other
vibration/feedback devices that should be used here instead and
translated into the form expected by the hardware?
It also seems to be using GFP_DMA not dma_alloc functions which looks a
bit odd and certainly isn't portable.
Right, I'll change it to the appropriate ones.
Cheers, Ilkka
--
Nokia and ATT present the 2010 Calling All Innovators-North America contest
Create new apps games for the Nokia N8 for consumers in U.S. and Canada
$10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing
Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store
http://p.sf.net/sfu/nokia-dev2dev
___
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[PATCH] input: spi: Driver for SPI data stream driven vibrator
This driver provides access to drive a vibrator connected
to SPI data line via Input layer's Force Feedback interface.
Client application provides samples (data streams) to be
played as CUSTOM_DATA. The samples are stored in driver's
internal buffers.
The driver is not able to mix the given samples. Instead, it
remembers the currently played sample and next one to be played.
Signed-off-by: Ilkka Koskinen ilkka.koski...@nokia.com
---
drivers/input/misc/Kconfig |5 +
drivers/input/misc/Makefile|2 +-
drivers/input/misc/vibra_spi.c | 429
include/linux/spi/vibra.h | 34
4 files changed, 469 insertions(+), 1 deletions(-)
create mode 100644 drivers/input/misc/vibra_spi.c
create mode 100644 include/linux/spi/vibra.h
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index b49e233..3441832 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -438,4 +438,9 @@ config INPUT_ADXL34X_SPI
To compile this driver as a module, choose M here: the
module will be called adxl34x-spi.
+config INPUT_SPI_VIBRA
+ tristate Support for SPI driven Vibra module
+ help
+ Support for Vibra module that is connected to OMAP SPI bus.
+
endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 19ccca7..cde272f 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -41,4 +41,4 @@ obj-$(CONFIG_INPUT_WINBOND_CIR) += winbond-cir.o
obj-$(CONFIG_INPUT_WISTRON_BTNS) += wistron_btns.o
obj-$(CONFIG_INPUT_WM831X_ON) += wm831x-on.o
obj-$(CONFIG_INPUT_YEALINK)+= yealink.o
-
+obj-$(CONFIG_INPUT_SPI_VIBRA) += vibra_spi.o
diff --git a/drivers/input/misc/vibra_spi.c b/drivers/input/misc/vibra_spi.c
new file mode 100644
index 000..551a3b8
--- /dev/null
+++ b/drivers/input/misc/vibra_spi.c
@@ -0,0 +1,429 @@
+/*
+ * This file implements a driver for SPI data driven vibrator.
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ *
+ * Contact: Ilkka Koskinen ilkka.koski...@nokia.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * 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., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ *
+ */
+
+#include linux/irq.h
+#include linux/module.h
+#include linux/workqueue.h
+#include linux/spinlock.h
+#include linux/delay.h
+#include linux/slab.h
+#include linux/jiffies.h
+#include linux/spi/spi.h
+#include linux/input.h
+#include linux/spi/vibra.h
+#include linux/io.h
+
+/* Number of effects handled with memoryless devices */
+#define VIBRA_EFFECTS 36
+#define MAX_EFFECT_SIZE1024 /* In bytes */
+
+#define FF_EFFECT_QUEUED 0
+#define FF_EFFECT_PLAYING 1
+#define FF_EFFECT_ABORTING 2
+#define FF_EFFECT_UPLOADING3
+
+enum vibra_status {
+ IDLE = 0,
+ STARTED,
+ PLAYING,
+ CLOSING,
+};
+
+struct effect_info {
+ char*buf;
+ int buflen;
+ unsigned long flags; /* effect state (STARTED, PLAYING, etc) */
+ unsigned long stop_at;
+};
+
+struct vibra_data {
+ struct device *dev;
+ struct input_dev*input_dev;
+
+ struct workqueue_struct *workqueue;
+ struct work_struct play_work;
+
+ struct spi_device *spi_dev;
+ struct spi_transfer t;
+ struct spi_message msg;
+ u32 spi_max_speed_hz;
+
+ void (*set_power)(bool enable);
+
+ enum vibra_status status;
+
+ struct effect_info effects[VIBRA_EFFECTS];
+ int next_effect;
+ int current_effect;
+ unsigned long stop_at;
+};
+
+static int vibra_spi_raw_write_effect(struct vibra_data *vibra)
+{
+ spi_message_init(vibra-msg);
+ memset(vibra-t, 0, sizeof(vibra-t));
+
+ vibra-t.tx_buf = vibra-effects[vibra-current_effect].buf;
+ vibra-t.len= vibra-effects[vibra-current_effect].buflen;
+ spi_message_add_tail(vibra-t, vibra-msg);
+
+ return spi_sync(vibra-spi_dev, vibra-msg);
+}
+
+static void vibra_play_work(struct work_struct *work)
+{
+ struct vibra_data *vibra = container_of(work,
+ struct vibra_data, play_work);
+ struct effect_info *curr, *next;
+ unsigned long flags;
+
+ while (1) {
+
