Re: SPI redux ... driver model support

From: Leeds United Fan
Date: Fri Sep 02 2005 - 03:00:31 EST


Hi Mark,
you've mentioned the code that you're working on several times, but no one in LKML has ever seen a single line of code from you. Will you please be so kind to share a piece of you SPI subsystem?

TIA!

Mark Underwood wrote:

--- David Brownell <david-b@xxxxxxxxxxx> wrote:



Date: Wed, 31 Aug 2005 08:59:44 +0100 (BST)
From: Mark Underwood <basicmark@xxxxxxxxx>

--- David Brownell wrote:



The last couple times SPI frameworks came up


here, some of the feedback


included "make it use the driver model properly;


don't be like I2C".


In hopes that it'll be useful, here's a small


SPI core with driver model


support driven from board-specific tables


listing devices. I expect the


I/O call(s) could stand to change; but at least


this one starts out right,


based on async I/O. (There's a synchronous


call; it's a trivial wrapper.)


...


Well I guess great minds think alike ;-). After
looking though my SPI core layer I released that


it in


no way reflected the new driver model (not


surprising


as it was a copy of i2c-core.c) and I would


probably


get laughed off the kernel mailing list if I sent


it


as was ;-).

That usually doesn't happen. You'd just be told
"make it use the driver
model properly; don't be like I2C." Though maybe
there'd be a fiew
other criticisms mixed in. :)




I am now writing a new spi-core.c which uses the


new


driver model.


How about just merging the code I sent? It's not
large, and it solves
that problem. I don't much care about the I/O model
issues quite yet,
though requirements for quick sensor captures
(RPC-ish) would seem
different from ones like reading bulk SPI flash
data.




For registering an adapter:
1) Register an adapter that has a cs table showing
where devices sit on the adapter.


But how is the adapter driver itself supposed to
know that?



It gets passed the cs table as part of its platform
data.



That's what I addressed with my patch: the need for
the config tables
to be **independent** of controller (and protocol)
code. It decouples
all the board-specific tables from the drivers.

(Example shown below.)

The nightmare to avoid is this: EVERY time someone
adds a new
SPI-equipped board, working/debugged/stable drivers
need to change,
because the board-specific config data was never
separated from the
drivers. (And we know it can be, as shown in the
patch I posted...)



Now I've fixed my version I'll have a more detailed
look.



Ideally adding a new board means adding a source
file for just that one
board, with the relevent implementation parameters. Only when hardware
guys do something funky should any driver need to
change.




That's what happens in my SPI subsystem. The adapter
driver only knows how the driver the adapter. When a
adapter gets probed it has platform data passed to it
which contains a pointer to the cs table, the number
of entry▓s in the cs table and the pointer to a
function to control some GPIO(s) as cs for adapters
that don▓t have any built in.



2) This causes spi-core to enumerate the devices


on


the cs table and register them.

For un-registering an adapter:
1) Unregister an adapter
2) This causes spi-core to remove all the children


of


the adapter


Right, that's all exactly as in the patch I posted,
though I punted
on the "unregister" path -- an exercise for the
reader! -- because I
wanted to focus on (a) the driver model structure,
like where things
land in sysfs, and (b) how to keep board-specific
initialization code
out of controller and protocol drivers.



OK. If you want I could do the same, that is send the
un/registration and sysfs code before I put the
transfer methods in. I have some dummy devices so you
can see what happens in sysfs.



- Dave


--- o26.orig/arch/arm/mach-omap1/board-osk.c
2005-08-27 02:11:45.000000000 -0700
+++ o26/arch/arm/mach-omap1/board-osk.c 2005-08-27
18:44:20.000000000 -0700
@@ -193,6 +193,34 @@ static struct
omap_board_config_kernel o

#ifdef CONFIG_OMAP_OSK_MISTRAL

+#include <linux/spi.h>
+
+struct ads7864_info { /* FIXME put in standard
header */
+ u16 pen_irq, busy; /* GPIO lines */
+ u16 x_ohms, y_ohms;
+};
+
+static struct ads7864_info mistral_ts_info = {
+ .pen_irq = OMAP_GPIO_IRQ(4),
+ .busy = /* GPIO */ 6,
+ .x_ohms = 419,
+ .y_ohms = 486,
+};
+
+static const struct spi_board_info
mistral_boardinfo[] = {
+{
+ /* MicroWire CS0 has an ads7846e with touchscreen
and
+ * other sensors. It's currently glued into some
OMAP
+ * touchscreen support that ignores the driver
model.
+ */
+ .driver_name = "ads7846",
+ .platform_data = &mistral_ts_info,
+ .max_speed_hz = 2000000,
+ .bus_num = 2, /* spi2 == microwire */



I did think about doing this but the problem is how do
you know bus 2 is the bus you think it is? This would
work for SPI adapters that are platform devices, but
what about hot-plug devices like PCI and USB (we are
thinking of actually making a USB to SPI converter so
customers can try out some of our SPI devices on a PC
:).

Mark



+ .chip_select = 0,
+},
+};
+
#ifdef CONFIG_PM
static irqreturn_t
osk_mistral_wake_interrupt(int irq, void *ignored,
struct pt_regs *regs)
@@ -211,6 +239,9 @@ static void __init
osk_mistral_init(void
* But this is too early for that...
*/

+ spi_register_board_info(mistral_boardinfo,
+ ARRAY_SIZE(mistral_boardinfo));
+
/* the sideways button (SW1) is for use as a
"wakeup" button */
omap_cfg_reg(N15_1610_MPUIO2);
if (omap_request_gpio(OMAP_MPUIO(2)) == 0) {
-
To unsubscribe from this list: send the line
"unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/









___________________________________________________________ Yahoo! Messenger - NEW crystal clear PC to PC calling worldwide with voicemail http://uk.messenger.yahoo.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/





-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/