Greetings,
<snip>
drivers/spi/Kconfig | 33 +++
drivers/spi/Makefile | 14 +
include/linux/spi.h | 232 ++++++++++++++++++++++
Looks familiar. :) But another notion for the headers would be
<linux/spi/spi.h> ... main header
<linux/spi/CHIP.h> ... platform_data, for CHIP.c driver
Not all chips would need them, but it might be nice to have some place
other than <linux/CHIP.h> for such things. The platform_data would have
various important data that can't be ... chip variants, initialization
data, and similar stuff that differs between boards is knowable only by
board-specific init code, yet is needed by board-agnostic driver code.
What about SPI busses that are common for different boards?
<snip>
+static int spi_thread(void *context);
You're imposing the same implementation strategy Mark Underwood was.
I believe I persuaded him not to want that, pointing out three other
implementation strategies that can be just as reasonable:
http://marc.theaimsgroup.com/?l=linux-kernel&m=112684135722116&w=2
It'd be fine if for example your PNX controller driver worked that way
internally. But other drivers shouldn't be forced to allocate kernel
threads when they don't need them.
Hm, so does that imply that the whole -rt patches from
Ingo/Sven/Daniel/etc. are implementing wrong strategy (interrupts in
threads)?
How will your strategy work with that BTW?
<snip>
+ /*
+ * all messages for current selected_device
+ * are processed.
+ * let's switch to another device
+ */
Why are you hard-wiring such an unfair scheduling policy ... and
preventing use of better ones? I'd use FIFO rather than something
as unfair as that; and FIFO is much simpler to code, too.
Sounds reasonable to me.
--- /dev/null
+++ linux-2.6.10/drivers/spi/spi-dev.c
@@ -0,0 +1,219 @@
+/*
+ spi-dev.c - spi driver, char device interface
+
Do you have userspace drivers that work with this? I can see how to use
it with read-only sensors (each read generates a 12bit sample, etc) and
certain write-only devices, I guess.
But it doesn't look like this character device can handle RPC-ish things
like "give me an ADC conversion for line 3" (which commonly maps to a
"write 8 bits, then start reading 12 data bits one half clock after
the last bit is written" ... hard to make that work with separate
read and write transactions, given the half clock rule).
Hm. I thought half-clock cases were to be programmed kernel-wise.
<snip>
+ +--------------+ +---------+
+ | platform_bus | | spi_bus |
+ +--------------+ +---------+
+ |..| |
+ |..|--------+ +---------------+
+ +------------+| is parent to | SPI devices |
+ | SPI busses |+-------------> | |
+ +------------+ +---------------+
+ | |
+ +----------------+ +----------------------+
+ | SPI bus driver | | SPI device driver |
+ +----------------+ +----------------------+
That seems wierd even if I assume "platform_bus" is just an example.
For example there are two rather different "spi bus" notions there,
and it looks like neither one is the physical parent of any SPI device ...
Not sure if I understand you :(
+3.2 How do the SPI devices gets discovered and probed ?
Better IMO to have tables that get consulted when the SPI master controller
drivers register the parent ... tables that are initialized by the board
specific __init section code, early on. (Or maybe by __setup commandline
parameters.)
Doing it the way you are prevents you from declaring all the SPI devices in
a single out-of-the-way location like the arch/.../board-specific.c file,
which is normally responsible for declaring devices that are hard-wired on
a given board and can't be probed.
By what means does it prevent that?
+static inline struct spi_msg *spimsg_alloc(struct spi_device *device,
+ unsigned flags,
+ unsigned short len,
+ void (*status) (struct spi_msg *,
+ int code))
+{
+ ... 30+ lines including...
+
+ msg = kmalloc(sizeof(struct spi_msg), GFP_KERNEL);
+ memset(msg, 0, sizeof(struct spi_msg));
If these things aren't going to be refcounted, then it'd be easier
to just let them be stack-allocated; they ARE that small. And if
they've _got_ to be on the heap, then there's a new "kzalloc()" call
you should be looking at ...
+ msg->devbuf_rd = drv->alloc ?
+ drv->alloc(len, GFP_KERNEL) : kmalloc(len, GFP_KERNEL);
+ msg->databuf_rd = drv->get_buffer ?
+ drv->get_buffer(device, msg->devbuf_rd) : msg->devbuf_rd;
Oy. More dynamic allocation. (Repeated for write buffers too ...)
See above; don't force such costs on all drivers, few will ever need it.
I guess that won't necessarily be actual allocation, it's a matter of
drv callbacks.
<snip>
+#define SPI_MAJOR 153
+
+...
+
+#define SPI_DEV_CHAR "spi-char"
I thought 153 was the official SPI device number.
Best regards,
Vitaly
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
| |
 |