Menu
▾
▴
spi-devel-general
|
From: Rui S. <rui...@la...> - 2005-12-13 17:04:13
|
On Tue, 2005-12-13 at 15:09 +0300, dmitry pervushin wrote:
> On Mon, 2005-12-12 at 19:01 +0100, Rui Sousa wrote:
> > How do you handle IRQ's generated by a SPI device (e.g ack the
> > interrupt, check if it was the SPI device that generated the
> > interrupt, ...) if you can't read/write on the SPI bus from interrupt
> > context?
> Hmm... what do you mean by "cannot read/write" ? Normally you can
> write/read registers in interrupt context
The registers I want to read are from a SPI device (a SPI slave attached
to a SPI bus). I need to use SPI bus transfers to access them.
> , and then set the
> flag/complete the completion/what else ?
If I read the API correctly reading/writing a byte from the SPI bus
(synchronously) always implies putting the task doing the read to sleep:
int spi_transfer(struct spi_msg *msg, void (*callback) (struct spi_msg
*, int))
{
...
err = TO_SPI_BUS_DRIVER(bus->driver)->queue(msg);
wait_for_completion(&msg->sync);
...
}
So, how can I, from an interrupt handler, read/write a couple of bytes
from my SPI device using this API?
> In other words, could you please share the code that causes problems ?
>
Rui
|
|
From: dmitry p. <dpe...@gm...> - 2005-12-13 17:15:44
|
On Tue, 2005-12-13 at 16:11 +0100, Rui Sousa wrote:
> On Tue, 2005-12-13 at 15:09 +0300, dmitry pervushin wrote:
> > On Mon, 2005-12-12 at 19:01 +0100, Rui Sousa wrote:
> > > How do you handle IRQ's generated by a SPI device (e.g ack the
> > > interrupt, check if it was the SPI device that generated the
> > > interrupt, ...) if you can't read/write on the SPI bus from interrupt
> > > context?
> > Hmm... what do you mean by "cannot read/write" ? Normally you can
> > write/read registers in interrupt context
>
I expect the answer from David Brownell too; and, because both discussed
frameworks are going to be the only (and the best), please also pay
attention to his (possible) answer. But I'd comment your code :)
> If I read the API correctly reading/writing a byte from the SPI bus
> (synchronously) always implies putting the task doing the read to sleep:
>
> int spi_transfer(struct spi_msg *msg, void (*callback) (struct spi_msg
> *, int))
> {
>
> ...
> err = TO_SPI_BUS_DRIVER(bus->driver)->queue(msg);
> wait_for_completion(&msg->sync);
> ...
> }
>
> So, how can I, from an interrupt handler, read/write a couple of bytes
> from my SPI device using this API?
You can issue the request (in terms of core that you are using, queue
the message) in interrupt context, and perform the rest of processing in
`status' callback. It will be called when message is processed. You can
ack the irq here, and continue your processing.
|
|
From: Vitaly W. <vw...@ru...> - 2005-12-13 14:06:27
|
Greetings,
as a part of the convergence process between two SPI cores (one is developed by David Brownell and the other one -- by Dmitry Pervushin and me) I'd like to post here one more result of mindwork :). Despite our previous activities of that kind which were more related to porting drivers from one core to the other, this is a port of the library for handling async SPI messages using kernel threads, one per each SPI controller, from our core to David's.
This thingie hasn't been thoroughly tested yet, but it's lightweight and easy to understand so I don't think solving the problems that may arise will take long. Though I haven't actually done that yet, I'm sure that Stephen's PXA SSP driver will become easier to understand and less in footprint and will work faster when it's rewritten using this library. (Yes, I do expect performance improvement here as the current implementation schedules multiple tasklets, so scheduling penalty is high.)
This patch performes just minimal intrusion to the SPI core itself, adding a 'void *context' field to the spi_master structure which seems to be a good thing anyway.
drivers/spi/Kconfig | 9 +
drivers/spi/Makefile | 3
drivers/spi/spi-thread.c | 198 +++++++++++++++++++++++++++++++++++++++++
include/linux/spi/spi-thread.h | 27 +++++
include/linux/spi/spi.h | 3
5 files changed, 240 insertions(+)
Signed-off-by: Vitaly Wool <vw...@ru...>
Index: linux-2.6.orig/drivers/spi/Kconfig
===================================================================
--- linux-2.6.orig.orig/drivers/spi/Kconfig
+++ linux-2.6.orig/drivers/spi/Kconfig
@@ -13,6 +13,7 @@ config SPI_ARCH_HAS_MASTER
default y if ARCH_AT91
default y if ARCH_OMAP
default y if ARCH_PXA
+ default y if ARCH_PNX4008
default y if X86 # devel hack only!! (ICH7 can...)
config SPI_ARCH_HAS_SLAVE
@@ -63,6 +64,14 @@ config SPI_MASTER
controller and the protocol drivers for the SPI slave chips
that are connected.
+config SPI_THREAD
+ boolean "Library for threaded asynchronous message processing"
+ depends on SPI_MASTER
+ help
+ Choose this if you want to use thread-based asynchronous
+ message processing library. Using kernel threads for
+ asynchronous data processing is the most common option.
+
comment "SPI Master Controller Drivers"
depends on SPI_MASTER
Index: linux-2.6.orig/drivers/spi/Makefile
===================================================================
--- linux-2.6.orig.orig/drivers/spi/Makefile
+++ linux-2.6.orig/drivers/spi/Makefile
@@ -10,6 +10,9 @@ endif
# config declarations into driver model code
obj-$(CONFIG_SPI_MASTER) += spi.o
+# thread-based async message handling library
+obj-$(CONFIG_SPI_THREAD) += spi-thread.o
+
# SPI master controller drivers (bus)
obj-$(CONFIG_SPI_BITBANG) += spi_bitbang.o
# ... add above this line ...
Index: linux-2.6.orig/drivers/spi/spi-thread.c
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spi-thread.c
@@ -0,0 +1,198 @@
+/*
+ * drivers/spi/spi-thread.c
+ *
+ * Authors:
+ * Vitaly Wool <vw...@ru...>
+ * Dmitry Pervushin <dpe...@gm...>
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <so...@mv...>
+ *
+ * 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.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/config.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/proc_fs.h>
+#include <linux/kmod.h>
+#include <linux/init.h>
+#include <linux/wait.h>
+#include <linux/kthread.h>
+#include <linux/spi/spi.h>
+#include <asm/atomic.h>
+
+static int spi_thread(void *);
+static int spi_queue(struct spi_device *, struct spi_message *);
+
+struct threaded_async_data {
+ atomic_t exiting;
+ struct spi_master *master;
+ struct task_struct *thread;
+ wait_queue_head_t wq;
+ struct list_head msgs;
+ struct semaphore lock;
+ int (*xfer) (struct spi_master *, struct spi_message *);
+};
+
+/**
+ * spi_start_async - start the thread
+ * @master: SPI controller structure which the thread is related to
+ * @return: abstract pointer to the thread context
+ */
+int spi_start_async (struct spi_master *master, int (*xfer)(struct spi_master *, struct spi_message *))
+{
+ struct threaded_async_data *td = kzalloc (sizeof (struct threaded_async_data), GFP_KERNEL);
+ int rc = 0;
+
+ if (!td) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ if (master->context) {
+ rc = -EEXIST;
+ goto out;
+ }
+
+ td->master = master;
+ atomic_set(&td->exiting, 0);
+ td->thread = kthread_run(spi_thread, td, "%s-work", master->cdev.dev->bus_id);
+ init_waitqueue_head(&td->wq);
+ init_MUTEX(&td->lock);
+ INIT_LIST_HEAD(&td->msgs);
+ master->transfer = spi_queue;
+ td->xfer = xfer;
+ master->context = td;
+
+out:
+ return rc;
+}
+EXPORT_SYMBOL_GPL(spi_start_async);
+
+/**
+ * spi_stop_async - stop the thread
+ * @master: SPI controller structure which the thread is related to
+ */
+void spi_stop_async (struct spi_master *master)
+{
+ struct threaded_async_data *td = master->context;
+
+ if (td) {
+ /* TODO: free messages, if any */
+ atomic_inc (&td->exiting);
+ kthread_stop(td->thread);
+ kfree(td);
+ master->context = NULL;
+ }
+}
+EXPORT_SYMBOL_GPL(spi_stop_async);
+
+/**
+ * spi_thread_awake - function that called to determine if thread needs to process any messages
+ * @td: pointer to struct threaded_async_data
+ * Description:
+ * Thread wakes up if there is signal to exit (bd->exiting is set)
+ * or there are any messages in bus' queue.
+ */
+static int spi_thread_awake(struct threaded_async_data *td)
+{
+ int ret = -EINVAL;
+
+ if (!td || atomic_read(&td->exiting)) {
+ goto out;
+ }
+
+ down(&td->lock);
+ ret = !list_empty(&td->msgs);
+ up(&td->lock);
+out:
+ return ret;
+}
+
+/**
+ * spi_get_next_msg - retrieve the next message
+ * @data: pointer to threaded_async_data structure associated with
+ * SPI controller thread
+ */
+static inline struct spi_message *spi_get_next_msg(struct threaded_async_data *data)
+{
+ return list_entry(data->msgs.next, struct spi_message, queue);
+}
+
+/**
+ * spi_thread - the thread that calls bus functions to perform actual transfers
+ * @context: pointer to struct spi_bus_data with bus-specific data
+ * Description:
+ * This function is started as separate thread to perform actual
+ * transfers on SPI bus
+ */
+static int spi_thread(void *context)
+{
+ struct threaded_async_data *td = context;
+ struct spi_message *message = NULL;
+
+ while (!kthread_should_stop()) {
+
+ wait_event_interruptible(td->wq, spi_thread_awake(td));
+
+ if (atomic_read(&td->exiting))
+ goto thr_exit;
+
+ down(&td->lock);
+ while (!list_empty(&td->msgs)) {
+ /*
+ * this part is locked by td->lock,
+ * to protect spi_message extraction
+ */
+ message = spi_get_next_msg(td);
+ list_del (&message->queue);
+
+ up(&td->lock);
+
+ message->status = td->xfer(td->master, message);
+ if (message->complete)
+ message->complete(context);
+
+ /* lock the data again... */
+ down(&td->lock);
+ }
+ up(&td->lock);
+ }
+
+thr_exit:
+ return 0;
+}
+
+/**
+ * spi_queue - (internal) queue the message to be processed asynchronously
+ * @spi: SPI device to perform transfer to/from
+ * @msg: message to be sent
+ * Description:
+ * This function queues the message to SPI controller's queue.
+ */
+static int spi_queue(struct spi_device *spi, struct spi_message *msg)
+{
+ struct threaded_async_data *td = spi->master->context;
+ int rc = 0;
+
+ if (!td) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ msg->spi = spi;
+ down(&td->lock);
+ list_add_tail(&msg->queue, &td->msgs);
+ dev_dbg(spi->dev.parent, "message has been queued\n");
+ up(&td->lock);
+ wake_up_interruptible(&td->wq);
+
+out:
+ return rc;
+}
+
Index: linux-2.6.orig/include/linux/spi/spi-thread.h
===================================================================
--- /dev/null
+++ linux-2.6.orig/include/linux/spi/spi-thread.h
@@ -0,0 +1,27 @@
+/*
+ * linux/drivers/spi/spi-thread.h
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <so...@mv...>
+ *
+ * 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.
+ *
+ */
+#ifndef __SPI_THREAD_H
+#define __SPI_THREAD_H
+
+
+#if defined (CONFIG_SPI_THREAD)
+extern int spi_start_async (struct spi_master *, int (*)(struct spi_master *, struct spi_message *));
+extern void spi_stop_async (struct spi_master *);
+#else
+static inline int spi_start_async (struct spi_master *master, int (*xfer)(struct spi_master *, struct spi_message *))
+{
+ return -EINVAL;
+}
+static inline void spi_stop_async (struct spi_master *master)
+{
+}
+#endif /* CONFIG_SPI_THREAD */
+#endif /* __SPI_THREAD_H */
Index: linux-2.6.orig/include/linux/spi/spi.h
===================================================================
--- linux-2.6.orig.orig/include/linux/spi/spi.h
+++ linux-2.6.orig/include/linux/spi/spi.h
@@ -152,6 +152,7 @@ static inline void spi_unregister_driver
* device's SPI controller; protocol code may call this.
* @transfer: adds a message to the controller's transfer queue.
* @cleanup: frees controller-specific state
+ * @context: controller-specific data
*
* Each SPI master controller can communicate with one or more spi_device
* children. These make a small bus, sharing MOSI, MISO and SCK signals
@@ -207,6 +208,8 @@ struct spi_master {
/* called on release() to free memory provided by spi_master */
void (*cleanup)(const struct spi_device *spi);
+
+ void *context;
};
/* the spi driver core manages memory for the spi_master classdev */
|
|
From: Vitaly W. <vw...@ru...> - 2005-12-13 16:53:12
|
Greetings,
this patch removes nasty buffer allocation in spi core for sync message transfers. (And thus solves the problem of possible priority inversion I've pointed out before).
The write_then_read function sets the flags singalling for buffers not being DMA-safe and it's absolutely up to the controller whatta do with that.
I. e. it will prolly allocate/copy if it performs DMA transfers or just don't do anything special if it's working in PIO. It can even use a single DMA-safe buffer as David's core did (although I wouldn't encourage anyone to do that! :))
This is of coure more flexible and also more optimal as it will *not* require _redundant_ memcpy's and semaphores protection in case of PIO transfers.
Please note that this is a key patch for us in some aspects, although it's rather small! :)
Signed-off-by: Vitaly Wool <vw...@ru...>
Signed-off-by: Dmitry Pervushin <dpe...@gm...>
drivers/spi/spi.c | 35 ++++++-----------------------------
include/linux/spi/spi.h | 7 +++++++
2 files changed, 13 insertions(+), 29 deletions(-)
Index: linux-2.6.orig/drivers/spi/spi.c
===================================================================
--- linux-2.6.orig.orig/drivers/spi/spi.c
+++ linux-2.6.orig/drivers/spi/spi.c
@@ -507,10 +507,6 @@ int spi_sync(struct spi_device *spi, str
}
EXPORT_SYMBOL_GPL(spi_sync);
-#define SPI_BUFSIZ (SMP_CACHE_BYTES)
-
-static u8 *buf;
-
/**
* spi_write_then_read - SPI synchronous write followed by read
* @spi: device with which data will be exchanged
@@ -532,39 +528,28 @@ int spi_write_then_read(struct spi_devic
const u8 *txbuf, unsigned n_tx,
u8 *rxbuf, unsigned n_rx)
{
- static DECLARE_MUTEX(lock);
-
int status;
struct spi_message message;
struct spi_transfer x[2];
- /* Use preallocated DMA-safe buffer. We can't avoid copying here,
- * (as a pure convenience thing), but we can keep heap costs
- * out of the hot path.
- */
- if ((n_tx + n_rx) > SPI_BUFSIZ)
- return -EINVAL;
-
- down(&lock);
memset(x, 0, sizeof x);
+ memset(&message, 0, sizeof message);
- memcpy(buf, txbuf, n_tx);
- x[0].tx_buf = buf;
+ x[0].tx_buf = tx_buf;
x[0].len = n_tx;
+ x[0].tx_dma_unsafe = 1;
- x[1].rx_buf = buf + n_tx;
+ x[1].rx_buf = rx_buf;
x[1].len = n_rx;
+ x[1].rx_dma_unsafe = 1;
/* do the i/o */
message.transfers = x;
message.n_transfer = ARRAY_SIZE(x);
status = spi_sync(spi, &message);
- if (status == 0) {
- memcpy(rxbuf, x[1].rx_buf, n_rx);
+ if (status == 0)
status = message.status;
- }
- up(&lock);
return status;
}
EXPORT_SYMBOL_GPL(spi_write_then_read);
@@ -575,12 +560,6 @@ static int __init spi_init(void)
{
int status;
- buf = kmalloc(SPI_BUFSIZ, SLAB_KERNEL);
- if (!buf) {
- status = -ENOMEM;
- goto err0;
- }
-
status = bus_register(&spi_bus_type);
if (status < 0)
goto err1;
@@ -593,9 +572,7 @@ static int __init spi_init(void)
err2:
bus_unregister(&spi_bus_type);
err1:
- kfree(buf);
buf = NULL;
-err0:
return status;
}
Index: linux-2.6.orig/include/linux/spi/spi.h
===================================================================
--- linux-2.6.orig.orig/include/linux/spi/spi.h
+++ linux-2.6.orig/include/linux/spi/spi.h
@@ -248,6 +248,10 @@ extern struct spi_master *spi_busnum_to_
* @rx_dma: DMA address of buffer, if spi_message.is_dma_mapped
* @len: size of rx and tx buffers (in bytes)
* @cs_change: affects chipselect after this transfer completes
+ * @tx_dma_unsafe: signals to controller driver that the buffer
+ * is not DMA-safe
+ * @rx_dma_unsafe: signals to controller driver that the buffer
+ * is not DMA-safe
* @delay_usecs: microseconds to delay after this transfer before
* (optionally) changing the chipselect status, then starting
* the next transfer or completing this spi_message.
@@ -288,6 +292,9 @@ struct spi_transfer {
dma_addr_t rx_dma;
unsigned cs_change:1;
+ unsigned rx_dma_unsafe:1;
+ unsigned tx_dma_unsafe:1;
+
u16 delay_usecs;
};
|
|
From: David B. <da...@pa...> - 2005-12-13 19:10:01
|
On Tuesday 13 December 2005 8:53 am, Vitaly Wool wrote: > > this patch removes nasty buffer allocation in spi core for sync message transfers. That's not "core" in the normal "everyone must use this" sense. Even the folk that do use synchronous transfers aren't always going to use that particular codepath. Neither is it "remove" in the normal sense either. The hot path never had an allocation before ... but it could easily have one now, because that sort of bounce-buffer semantic is what a DMA_UNSAFE flag demands from the lower levels. (That's a key part part of the proposed change that's not included in this patch... making the chage look much smaller.) How much of the reason you're arguing for this is because you have that WLAN stack that does some static allocation for I/O buffers? Changing that to use dynamic allocation -- the more usual style -- should be easy. But instead, you want all code in the SPI stack to need to worry about two different kinds of I/O memory: the normal stuff, and the DMA-unsafe kind. Not just this WLAN code which for some reason started out using a nonportable scheme for allocating I/O buffers. It'd take a lot more persuasion to make me think that's a good idea. That's the kind of subtle confusion that really promotes hard-to-find bugs in drivers, and lots of developer confusion. And all that to support a new less-portable I/O buffer model... It's way better to just insist that all I/O buffers (in all generic APIs) be DMA-safe. AFAICT that's a pretty standard rule everywhere in Linux. - Dave |
|
From: Greg KH <gr...@kr...> - 2005-12-13 19:18:17
|
On Tue, Dec 13, 2005 at 11:01:01AM -0800, David Brownell wrote: > It's way better to just insist that all I/O buffers (in all > generic APIs) be DMA-safe. AFAICT that's a pretty standard > rule everywhere in Linux. I agree. greg k-h |
|
From: Vitaly W. <vw...@ru...> - 2005-12-14 14:01:37
|
Greg KH wrote: >On Tue, Dec 13, 2005 at 11:01:01AM -0800, David Brownell wrote: > > >>It's way better to just insist that all I/O buffers (in all >>generic APIs) be DMA-safe. AFAICT that's a pretty standard >>rule everywhere in Linux. >> >> > >I agree. > > Well, why then David doesn't insist on that in his own code? His synchronous transfer functions are allocating transfer buffers on stack which is not DMA-safe. Then he starts messing with allocate-or-use-preallocated stuff etc. etc. Why isn't he just kmalloc'ing/kfree'ing buffers each time these functions are called (as he proposes for upper layer drivers to do)? That's a significant inconsistency. Is it also the thing you agree with? And they way he does it implies redundant memcpy's and kmalloc's: suppose we have two controller drivers working in two threads and calling write_then_read in such a way that the one called later has to allocate a new buffer. Suppose also that both controller drivers are working in *PIO* mode. In this situation you have one redundant kmalloc and two redundant memcpy's, not speaking about overhead brought up by mutexes. The thing is that only controller driver is aware whether DMA is needed or not, so it's controller driver that should work it out. Requesting all the buffers to be DMA-safe will make a significant performance drop on all small transfers! Vitaly |
|
From: Vitaly W. <vw...@ru...> - 2005-12-13 21:48:46
|
David Brownell wrote: >Neither is it "remove" in the normal sense either. The hot >path never had an allocation before ... but it could easily >have one now, because that sort of bounce-buffer semantic is >what a DMA_UNSAFE flag demands from the lower levels. (That's >a key part part of the proposed change that's not included in >this patch... making the chage look much smaller.) > > Makes no sense to me, sorry. What 'not included' changes are you talking about? > >How much of the reason you're arguing for this is because you >have that WLAN stack that does some static allocation for I/O >buffers? Changing that to use dynamic allocation -- the more >usual style -- should be easy. But instead, you want all code >in the SPI stack to need to worry about two different kinds of >I/O memory: the normal stuff, and the DMA-unsafe kind. Not > > Err, this change also allows to get rid of ugly stuff in write_then_read. It also allows to keep track of memory allocation in SPI controller driver withough hacky tricks. And... it's not *all* the code; if it doesn't provide such means, it's also fine. >just this WLAN code which for some reason started out using >a nonportable scheme for allocating I/O buffers. > > I'm afraid I just can't understand what you mean by 'portable' here. >It'd take a lot more persuasion to make me think that's a good >idea. That's the kind of subtle confusion that really promotes >hard-to-find bugs in drivers, and lots of developer confusion. > > What hard-to-find bugs, I wonder? And... I'm afraid that your core is unacceptable for us w/o the changes proposed. Vitaly |
|
From: Vitaly W. <vw...@ru...> - 2005-12-13 22:17:13
|
David Brownell wrote: >That's not "core" in the normal "everyone must use this" sense. >Even the folk that do use synchronous transfers aren't always >going to use that particular codepath. > > Lemme explain once more why what you have is a lot worse than what I suggest. Take for instance spi_w8r8 function used in lotsa places in the drivers you and Stephen have posted. This function has a) *implicit memcpy* inherited from spi_write_then_read b) *implicit priority inversion* inherited from the same place. On the other hand, any smart enough SPI controller driver wouldn't ever want to send and receive one byte via DMA, especially chaining messages because the overhead will be great; memcpy's also adding overhead. It's gonna use PIO instead. So this is a serious reason to let controller driver decide things related to memory allocation. It doesn't concern only static-allocated buffers but also, say, stack-allocated (used in your core in many places). And controller driver needs something to track whether it has allocated a buffer for transfer for DMA safety or not. Don't you want a linked list of allocated buffers in the controller driver?! And the flag I'm adding within the patch does that with minimal intrusion to your core. Hope that helps understand my reasons. Vitaly |
|
From: David B. <da...@pa...> - 2005-12-14 17:04:20
|
On Tuesday 13 December 2005 2:15 pm, Vitaly Wool wrote: > Take for instance spi_w8r8 function used in lotsa places in the drivers > you and Stephen have posted. > This function has a) *implicit memcpy* inherited from > spi_write_then_read b) *implicit priority inversion* inherited from the > same place. No, (a) is explicit, along with comments not to use that family of calls when such things matter more than the convenience of those RPC-ish calls. And (b) was fixed a small patch, now merged. - Dave |
|
From: Vitaly W. <vw...@ru...> - 2005-12-14 17:27:22
|
David Brownell wrote: >On Tuesday 13 December 2005 2:15 pm, Vitaly Wool wrote: > > > >>Take for instance spi_w8r8 function used in lotsa places in the drivers >>you and Stephen have posted. >>This function has a) *implicit memcpy* inherited from >>spi_write_then_read b) *implicit priority inversion* inherited from the >>same place. >> >> > >No, (a) is explicit, along with comments not to use that family of >calls when such things matter more than the convenience of those >RPC-ish calls. And (b) was fixed a small patch, now merged. > > Hmm. Why not use a) when it's convenient? You're placing an artificial requirement then. Yep, saw patch for b). One more kmalloc introduces which can be avoided easily with my patch ;) Vitaly |
|
From: Stephen S. <st...@st...> - 2005-12-14 18:49:06
|
On Tue, 2005-12-13 at 17:06 +0300, Vitaly Wool wrote:
> Greetings,
>
> This thingie hasn't been thoroughly tested yet, but it's lightweight
> and easy to understand so I don't think solving the problems that
> may arise will take long. Though I haven't actually done that yet,
> I'm sure that Stephen's PXA SSP driver will become easier to understand
> and less in footprint and will work faster when it's rewritten using
> this library. (Yes, I do expect performance improvement here as the
> current implementation schedules multiple tasklets, so
> scheduling penalty is high.)
Is this really true? Is tasklet scheduling "harder" than kernal thread
scheduling? A close look at my PXA SSP SPI implementation will reveal
that my design is nearly lock-less and callable from any execution
context (i.e. interrupt context).
In general, the "pump_transfer" and "pump_message" tasklets should
mutually exclusive (anything else is a programming
mistake). I certainly could collapse the tasklets into a single one if
you think this would be better?
> + * spi_queue - (internal) queue the message to be processed asynchronously
> + * @spi: SPI device to perform transfer to/from
> + * @msg: message to be sent
> + * Description:
> + * This function queues the message to SPI controller's queue.
> + */
> +static int spi_queue(struct spi_device *spi, struct spi_message *msg)
> +{
> + struct threaded_async_data *td = spi->master->context;
> + int rc = 0;
> +
> + if (!td) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + msg->spi = spi;
> + down(&td->lock);
> + list_add_tail(&msg->queue, &td->msgs);
> + dev_dbg(spi->dev.parent, "message has been queued\n");
> + up(&td->lock);
> + wake_up_interruptible(&td->wq);
> +
> +out:
> + return rc;
> +}
> +
This can not be invoke this from "interrupt context" which is a
requirement for my SPI devices (CS8415A, CS8405A, CS4341).
|
|
From: David B. <da...@pa...> - 2005-12-14 20:19:02
|
On Tuesday 13 December 2005 6:06 am, Vitaly Wool wrote: > Greetings, > > as a part of the convergence process between two SPI cores ... > I'd like to post here one more result of mindwork :). ... > this is a port of the library > for handling async SPI messages using kernel threads, one per each > SPI controller, from our core to David's. I'll have to take some time to look at this; I had planned to evolve the spi_bitbang framework a bit more. It's already working (parport/pc), but needs some improvements. Just wanted you to not expect a response Real Soon Now, since there are other things I have to do too. :) - Dave |
|
From: Vitaly W. <vw...@ru...> - 2005-12-15 12:19:20
|
This is just an update of the library for handling async SPI messages using kernel threads, one per each SPI controller. The only change is that thread's mutex has been changed to spinlock, in order to make it possible to call transfer function from the interrupt context.
Signed-off-by: Vitaly Wool <vw...@ru...>
drivers/spi/Kconfig | 9 +
drivers/spi/Makefile | 3
drivers/spi/spi-thread.c | 198 +++++++++++++++++++++++++++++++++++++++++
include/linux/spi/spi-thread.h | 27 +++++
include/linux/spi/spi.h | 3
5 files changed, 240 insertions(+)
Index: linux-2.6.orig/drivers/spi/Kconfig
===================================================================
--- linux-2.6.orig.orig/drivers/spi/Kconfig
+++ linux-2.6.orig/drivers/spi/Kconfig
@@ -13,6 +13,7 @@ config SPI_ARCH_HAS_MASTER
default y if ARCH_AT91
default y if ARCH_OMAP
default y if ARCH_PXA
+ default y if ARCH_PNX4008
default y if X86 # devel hack only!! (ICH7 can...)
config SPI_ARCH_HAS_SLAVE
@@ -63,6 +64,14 @@ config SPI_MASTER
controller and the protocol drivers for the SPI slave chips
that are connected.
+config SPI_THREAD
+ boolean "Library for threaded asynchronous message processing"
+ depends on SPI_MASTER
+ help
+ Choose this if you want to use thread-based asynchronous
+ message processing library. Using kernel threads for
+ asynchronous data processing is the most common option.
+
comment "SPI Master Controller Drivers"
depends on SPI_MASTER
Index: linux-2.6.orig/drivers/spi/Makefile
===================================================================
--- linux-2.6.orig.orig/drivers/spi/Makefile
+++ linux-2.6.orig/drivers/spi/Makefile
@@ -10,6 +10,9 @@ endif
# config declarations into driver model code
obj-$(CONFIG_SPI_MASTER) += spi.o
+# thread-based async message handling library
+obj-$(CONFIG_SPI_THREAD) += spi-thread.o
+
# SPI master controller drivers (bus)
obj-$(CONFIG_SPI_BITBANG) += spi_bitbang.o
# ... add above this line ...
Index: linux-2.6.orig/drivers/spi/spi-thread.c
===================================================================
--- /dev/null
+++ linux-2.6.orig/drivers/spi/spi-thread.c
@@ -0,0 +1,198 @@
+/*
+ * drivers/spi/spi-thread.c
+ *
+ * Authors:
+ * Vitaly Wool <vw...@ru...>
+ * Dmitry Pervushin <dpe...@gm...>
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <so...@mv...>
+ *
+ * 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.
+ *
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/config.h>
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/proc_fs.h>
+#include <linux/kmod.h>
+#include <linux/init.h>
+#include <linux/wait.h>
+#include <linux/kthread.h>
+#include <linux/spi/spi.h>
+#include <asm/atomic.h>
+
+static int spi_thread(void *);
+static int spi_queue(struct spi_device *, struct spi_message *);
+
+struct threaded_async_data {
+ atomic_t exiting;
+ struct spi_master *master;
+ struct task_struct *thread;
+ wait_queue_head_t wq;
+ struct list_head msgs;
+ spinlock_t lock;
+ int (*xfer) (struct spi_master *, struct spi_message *);
+};
+
+/**
+ * spi_start_async - start the thread
+ * @master: SPI controller structure which the thread is related to
+ * @return: abstract pointer to the thread context
+ */
+int spi_start_async (struct spi_master *master, int (*xfer)(struct spi_master *, struct spi_message *))
+{
+ struct threaded_async_data *td = kzalloc (sizeof (struct threaded_async_data), GFP_KERNEL);
+ int rc = 0;
+
+ if (!td) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ if (master->context) {
+ rc = -EEXIST;
+ goto out;
+ }
+
+ td->master = master;
+ atomic_set(&td->exiting, 0);
+ td->thread = kthread_run(spi_thread, td, "%s-work", master->cdev.dev->bus_id);
+ init_waitqueue_head(&td->wq);
+ spi_lock_init(&td->lock);
+ INIT_LIST_HEAD(&td->msgs);
+ master->transfer = spi_queue;
+ td->xfer = xfer;
+ master->context = td;
+
+out:
+ return rc;
+}
+EXPORT_SYMBOL_GPL(spi_start_async);
+
+/**
+ * spi_stop_async - stop the thread
+ * @master: SPI controller structure which the thread is related to
+ */
+void spi_stop_async (struct spi_master *master)
+{
+ struct threaded_async_data *td = master->context;
+
+ if (td) {
+ /* TODO: free messages, if any */
+ atomic_inc (&td->exiting);
+ kthread_stop(td->thread);
+ kfree(td);
+ master->context = NULL;
+ }
+}
+EXPORT_SYMBOL_GPL(spi_stop_async);
+
+/**
+ * spi_thread_awake - function that called to determine if thread needs to process any messages
+ * @td: pointer to struct threaded_async_data
+ * Description:
+ * Thread wakes up if there is signal to exit (bd->exiting is set)
+ * or there are any messages in bus' queue.
+ */
+static int spi_thread_awake(struct threaded_async_data *td)
+{
+ int ret = -EINVAL;
+
+ if (!td || atomic_read(&td->exiting)) {
+ goto out;
+ }
+
+ spin_lock_irq(&td->lock);
+ ret = !list_empty(&td->msgs);
+ spin_unlock_irq(&td->lock);
+out:
+ return ret;
+}
+
+/**
+ * spi_get_next_msg - retrieve the next message
+ * @data: pointer to threaded_async_data structure associated with
+ * SPI controller thread
+ */
+static inline struct spi_message *spi_get_next_msg(struct threaded_async_data *data)
+{
+ return list_entry(data->msgs.next, struct spi_message, queue);
+}
+
+/**
+ * spi_thread - the thread that calls bus functions to perform actual transfers
+ * @context: pointer to struct spi_bus_data with bus-specific data
+ * Description:
+ * This function is started as separate thread to perform actual
+ * transfers on SPI bus
+ */
+static int spi_thread(void *context)
+{
+ struct threaded_async_data *td = context;
+ struct spi_message *message = NULL;
+
+ while (!kthread_should_stop()) {
+
+ wait_event_interruptible(td->wq, spi_thread_awake(td));
+
+ if (atomic_read(&td->exiting))
+ goto thr_exit;
+
+ spin_lock_irq(&td->lock);
+ while (!list_empty(&td->msgs)) {
+ /*
+ * this part is locked by td->lock,
+ * to protect spi_message extraction
+ */
+ message = spi_get_next_msg(td);
+ list_del (&message->queue);
+
+ spin_unlock_irq(&td->lock);
+
+ message->status = td->xfer(td->master, message);
+ if (message->complete)
+ message->complete(context);
+
+ /* lock the data again... */
+ spin_lock_irq(&td->lock);
+ }
+ spin_unlock_irq(&td->lock);
+ }
+
+thr_exit:
+ return 0;
+}
+
+/**
+ * spi_queue - (internal) queue the message to be processed asynchronously
+ * @spi: SPI device to perform transfer to/from
+ * @msg: message to be sent
+ * Description:
+ * This function queues the message to SPI controller's queue.
+ */
+static int spi_queue(struct spi_device *spi, struct spi_message *msg)
+{
+ struct threaded_async_data *td = spi->master->context;
+ int rc = 0;
+
+ if (!td) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ msg->spi = spi;
+ spin_lock_irq(&td->lock);
+ list_add_tail(&msg->queue, &td->msgs);
+ spin_unlock_irq(&td->lock);
+ dev_dbg(spi->dev.parent, "message has been queued\n");
+ wake_up_interruptible(&td->wq);
+
+out:
+ return rc;
+}
+
Index: linux-2.6.orig/include/linux/spi/spi-thread.h
===================================================================
--- /dev/null
+++ linux-2.6.orig/include/linux/spi/spi-thread.h
@@ -0,0 +1,27 @@
+/*
+ * linux/drivers/spi/spi-thread.h
+ *
+ * Copyright (C) 2005 MontaVista Software, Inc <so...@mv...>
+ *
+ * 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.
+ *
+ */
+#ifndef __SPI_THREAD_H
+#define __SPI_THREAD_H
+
+
+#if defined (CONFIG_SPI_THREAD)
+extern int spi_start_async (struct spi_master *, int (*)(struct spi_master *, struct spi_message *));
+extern void spi_stop_async (struct spi_master *);
+#else
+static inline int spi_start_async (struct spi_master *master, int (*xfer)(struct spi_master *, struct spi_message *))
+{
+ return -EINVAL;
+}
+static inline void spi_stop_async (struct spi_master *master)
+{
+}
+#endif /* CONFIG_SPI_THREAD */
+#endif /* __SPI_THREAD_H */
Index: linux-2.6.orig/include/linux/spi/spi.h
===================================================================
--- linux-2.6.orig.orig/include/linux/spi/spi.h
+++ linux-2.6.orig/include/linux/spi/spi.h
@@ -152,6 +152,7 @@ static inline void spi_unregister_driver
* device's SPI controller; protocol code may call this.
* @transfer: adds a message to the controller's transfer queue.
* @cleanup: frees controller-specific state
+ * @context: controller-specific data
*
* Each SPI master controller can communicate with one or more spi_device
* children. These make a small bus, sharing MOSI, MISO and SCK signals
@@ -207,6 +208,8 @@ struct spi_master {
/* called on release() to free memory provided by spi_master */
void (*cleanup)(const struct spi_device *spi);
+
+ void *context;
};
/* the spi driver core manages memory for the spi_master classdev */
|
|
From: David B. <da...@pa...> - 2005-12-13 16:38:10
|
> On Mon, 2005-12-12 at 19:01 +0100, Rui Sousa wrote: > > How do you handle IRQ's generated by a SPI device (e.g ack the > > interrupt, check if it was the SPI device that generated the > > interrupt, ...) if you can't read/write on the SPI bus from interrupt > > context? I don't understand the question. The SPI controller may be busy handling transactions for a different device, so the best you could do from _any_ IRQ handler is to queue some messages that will get to the IRQ-issuing device sometime later. It's not possible to make the other transactions finish any faster, or abort them safely mid-transfer. The way the ads7846 driver handles it, for example, is to issue a transaction collecting a bunch of touchscreen sensor readings. And disable its (nonsharable) irq, since the interrupt will be raised for some time and there's no portable way for Linux to force the IRQ to trigger only on the relevant edge. See the 2.6.15-rc5-mm1 patches... - Dave |
|
From: Rui S. <rui...@la...> - 2005-12-13 18:14:55
|
On Tue, 2005-12-13 at 08:35 -0800, David Brownell wrote:
> > On Mon, 2005-12-12 at 19:01 +0100, Rui Sousa wrote:
> > > How do you handle IRQ's generated by a SPI device (e.g ack the
> > > interrupt, check if it was the SPI device that generated the
> > > interrupt, ...) if you can't read/write on the SPI bus from interrupt
> > > context?
>
> I don't understand the question. The SPI controller may be busy
> handling transactions for a different device,
If you provide an async API to start with... if everything is sync
then this problem goes away[1]. When I started writing the SPI layer for
my platform (before any SPI patches started circulating) I was going
to use an async API and then decided against it. For the type of device
I'm using, a SLIC device, where only control data is exchanged on the
SPI bus (read register, check some bit, write register, ...) and very
small amounts of data are transferred at a time (4 bytes maximum) there
is really no point to an async API.
Once you give the possibility to transfer data asynchronously on the SPI
bus, and even if you add a pseudo sync interface, you loose the
possibility to call the transfer function (synchronously) from hard/soft
interrupt context. One way or the other you will eventually need to
sleep.
This forces a simple code like:
u8 device_read_reg(int reg)
{
u8 buf;
buf = reg;
spi_write(dev, &buf, 1);
spi_read(dev, &buf, 1);
return buf[0];
}
to be moved to user context or to become much more complex (state
machine + async callbacks).
I guess what I'm trying to say is that for a simple device like the one
I'm using, having an async API adds more trouble that what it's worth
it. Unfortunately I don't have a better solution to put forward.
> so the best you
> could do from _any_ IRQ handler is to queue some messages that
> will get to the IRQ-issuing device sometime later.
> It's not
> possible to make the other transactions finish any faster, or
> abort them safely mid-transfer.
>
> The way the ads7846 driver handles it, for example, is to issue
> a transaction collecting a bunch of touchscreen sensor readings.
> And disable its (nonsharable) irq,
nonsharable is the magic word. I thought of this solution, leaving the
interrupt handler with the interrupt masked and do all the handling in
a workqueue... but my GPIO pin used to generate the interrupt is being
shared with another device.
> since the interrupt will be
> raised for some time and there's no portable way for Linux to
> force the IRQ to trigger only on the relevant edge.
>
> See the 2.6.15-rc5-mm1 patches...
>
> - Dave
>
>
Rui
[1] I know this is not a reasonable solution for a number of cases.
|
|
From: Vitaly W. <vw...@ru...> - 2005-12-14 06:58:01
|
Looks like we'll have to remove explicit wait_for_completion from
spi_trasnfer to make it (potentially) work.
Rui, your turn will then be
a) not to use thread async message handling which is the default but
b) use some kind of PIO method for reading from the SPI bus.
The other solution which looks preferable to me is to have a pretty
simple interrupt handler which just wakes up a thread which in turn
reads what it wants not caring much about sleeps (as it's not gonna be
interrupt context any more).
Vitaly
Rui Sousa wrote:
>On Tue, 2005-12-13 at 15:09 +0300, dmitry pervushin wrote:
>
>
>>On Mon, 2005-12-12 at 19:01 +0100, Rui Sousa wrote:
>>
>>
>>>How do you handle IRQ's generated by a SPI device (e.g ack the
>>>interrupt, check if it was the SPI device that generated the
>>>interrupt, ...) if you can't read/write on the SPI bus from interrupt
>>>context?
>>>
>>>
>>Hmm... what do you mean by "cannot read/write" ? Normally you can
>>write/read registers in interrupt context
>>
>>
>
>The registers I want to read are from a SPI device (a SPI slave attached
>to a SPI bus). I need to use SPI bus transfers to access them.
>
>
>
>>, and then set the
>>flag/complete the completion/what else ?
>>
>>
>
>If I read the API correctly reading/writing a byte from the SPI bus
>(synchronously) always implies putting the task doing the read to sleep:
>
>int spi_transfer(struct spi_msg *msg, void (*callback) (struct spi_msg
>*, int))
>{
>
>...
> err = TO_SPI_BUS_DRIVER(bus->driver)->queue(msg);
> wait_for_completion(&msg->sync);
>...
>}
>
>So, how can I, from an interrupt handler, read/write a couple of bytes
>from my SPI device using this API?
>
>
>
>>In other words, could you please share the code that causes problems ?
>>
>>
>>
>
>Rui
>
>
>
>
|
|
From: Rui S. <rui...@la...> - 2005-12-14 14:52:25
|
On Wed, 2005-12-14 at 09:57 +0300, Vitaly Wool wrote:
> Looks like we'll have to remove explicit wait_for_completion from
> spi_trasnfer to make it (potentially) work.
> Rui, your turn will then be
> a) not to use thread async message handling which is the default but
> b) use some kind of PIO method for reading from the SPI bus.
This is what I'm doing now (sync + PIO). In any case the SPI bus
controller I'm using only supports PIO.
> The other solution which looks preferable to me is to have a pretty
> simple interrupt handler which just wakes up a thread which in turn
> reads what it wants not caring much about sleeps (as it's not gonna be
> interrupt context any more).
The main problem is that just to be able to ack the interrupt I need to
write on the SPI bus. So now I need to (1) modify my GPIO interrupt
handler so that it let's me exit before acking the interrupt on the
device (2) no longer share this GPIO with other devices interrupts...
It's possible to use the proposed API, it's just that in my case it will
increase the SPI slave driver complexity.
> Vitaly
Rui
> Rui Sousa wrote:
>
> >On Tue, 2005-12-13 at 15:09 +0300, dmitry pervushin wrote:
> >
> >
> >>On Mon, 2005-12-12 at 19:01 +0100, Rui Sousa wrote:
> >>
> >>
> >>>How do you handle IRQ's generated by a SPI device (e.g ack the
> >>>interrupt, check if it was the SPI device that generated the
> >>>interrupt, ...) if you can't read/write on the SPI bus from interrupt
> >>>context?
> >>>
> >>>
> >>Hmm... what do you mean by "cannot read/write" ? Normally you can
> >>write/read registers in interrupt context
> >>
> >>
> >
> >The registers I want to read are from a SPI device (a SPI slave attached
> >to a SPI bus). I need to use SPI bus transfers to access them.
> >
> >
> >
> >>, and then set the
> >>flag/complete the completion/what else ?
> >>
> >>
> >
> >If I read the API correctly reading/writing a byte from the SPI bus
> >(synchronously) always implies putting the task doing the read to sleep:
> >
> >int spi_transfer(struct spi_msg *msg, void (*callback) (struct spi_msg
> >*, int))
> >{
> >
> >...
> > err = TO_SPI_BUS_DRIVER(bus->driver)->queue(msg);
> > wait_for_completion(&msg->sync);
> >...
> >}
> >
> >So, how can I, from an interrupt handler, read/write a couple of bytes
> >from my SPI device using this API?
> >
> >
> >
> >>In other words, could you please share the code that causes problems ?
> >>
> >>
> >>
> >
> >Rui
> >
> >
> >
> >
>
>
|
|
From: Greg KH <gr...@kr...> - 2005-12-14 17:24:21
|
On Wed, Dec 14, 2005 at 04:50:03PM +0300, Vitaly Wool wrote: > Greg KH wrote: > > >On Tue, Dec 13, 2005 at 11:01:01AM -0800, David Brownell wrote: > > > > > >>It's way better to just insist that all I/O buffers (in all > >>generic APIs) be DMA-safe. AFAICT that's a pretty standard > >>rule everywhere in Linux. > >> > >> > > > >I agree. > > > > > Well, why then David doesn't insist on that in his own code? Heh, I don't know, only David can answer that :) > His synchronous transfer functions are allocating transfer buffers on > stack which is not DMA-safe. > Then he starts messing with allocate-or-use-preallocated stuff etc. etc. > Why isn't he just kmalloc'ing/kfree'ing buffers each time these > functions are called (as he proposes for upper layer drivers to do)? > That's a significant inconsistency. Is it also the thing you agree with? > > And they way he does it implies redundant memcpy's and kmalloc's: > suppose we have two controller drivers working in two threads and > calling write_then_read in such a way that the one called later has to > allocate a new buffer. Suppose also that both controller drivers are > working in *PIO* mode. In this situation you have one redundant kmalloc > and two redundant memcpy's, not speaking about overhead brought up by > mutexes. > > The thing is that only controller driver is aware whether DMA is > needed or not, so it's controller driver that should work it out. > Requesting all the buffers to be DMA-safe will make a significant > performance drop on all small transfers! What is the speed of your SPI bus? And what are your preformance requirements? thanks, greg k-h |
|
From: Vitaly W. <vw...@ru...> - 2005-12-14 17:54:42
|
Greg KH wrote: >What is the speed of your SPI bus? > >And what are your preformance requirements? > > The maximum frequency for the SPI bus is 26 MHz, WLAN driver is to work at true 10 Mbit/sec. Vitaly P. S. I'm speaking not about this particular case during most part of this conversation. Sound cards behind the SPI bus will suffer a lot more since it's their path to use wXrY functions (lotsa small transfers) rather than WLAN's. |
|
From: David B. <da...@pa...> - 2005-12-14 19:03:30
|
On Wednesday 14 December 2005 9:53 am, Vitaly Wool wrote: > Sound cards behind the SPI bus will suffer a lot more > since it's their path to use wXrY functions (lotsa small transfers) > rather than WLAN's. No, "stupid drivers will suffer"; nothing new. Just observe how the ads7846 touchscreen driver does small async transfers. Remember too that sending audio data over SPI (rather than say I2S, McBSP etc) is a different case than using it for the mixer controls. - Dave |
|
From: Vitaly W. <vw...@ru...> - 2005-12-14 19:20:37
|
David Brownell wrote: >On Wednesday 14 December 2005 9:53 am, Vitaly Wool wrote: > > > >> Sound cards behind the SPI bus will suffer a lot more >>since it's their path to use wXrY functions (lotsa small transfers) >>rather than WLAN's. >> >> > >No, "stupid drivers will suffer"; nothing new. Just observe >how the ads7846 touchscreen driver does small async transfers. > > So just answer please yes or no: are your spi_wXrY functions intended for usage at all or not? Maybe you should remove them completely? Vitaly |
|
From: David B. <da...@pa...> - 2005-12-14 20:21:26
|
> So just answer please yes or no: are your spi_wXrY functions intended > for usage at all or not? Certainly. Not _mis_used though. |
|
From: Vitaly W. <vw...@ru...> - 2005-12-14 19:36:35
|
David Brownell wrote: >On Wednesday 14 December 2005 9:53 am, Vitaly Wool wrote: > > > >> Sound cards behind the SPI bus will suffer a lot more >>since it's their path to use wXrY functions (lotsa small transfers) >>rather than WLAN's. >> >> > >No, "stupid drivers will suffer"; nothing new. Just observe >how the ads7846 touchscreen driver does small async transfers. > > Two more cents: ads7846 has a bunch of code just related to the need to do something from the interrupt context. Without this constraint, I don't see why w8r8 can not be used in most cases (of course, with the changes I propose). Vitaly |
|
From: Vitaly W. <vw...@ru...> - 2005-12-15 06:48:39
|
David Brownell wrote: >No, "stupid drivers will suffer"; nothing new. Just observe >how the ads7846 touchscreen driver does small async transfers. > > One cannot allocate memory in interrupt context, so the way to go is allocating it on stack, thus the buffer is not DMA-safe. Making it DMA-safe in thread that does the very message processing is a good way of overcoming this. Using preallocated buffer is not a good way, since it may well be already used by another interrupt or not yet processed by the worker thread (or tasklet, or whatever). The way it's done in this ads7846 driver is not quite acceptable. Losing the transfer if the previous one is still processed is *not* the way to go in some cases. One can not predict how many transfers are gonna be dropped due to "previous trransfer is being processed" problem, it depends on the system load. And though it's not a problem for touchscreen, it *will* be a problem if it were MMC, for instance. Vitaly |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X