Messages in this thread Patch in this message | | | Date | Fri, 2 Sep 2005 08:21:25 +0100 (BST) | From | Mark Underwood <> | Subject | Re: SPI redux ... driver model support |
| |
--- David Brownell <david-b@pacbell.net> wrote:
> > Date: Wed, 31 Aug 2005 08:59:44 +0100 (BST) > > From: Mark Underwood <basicmark@yahoo.com> > > > > --- 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 entrys in the cs table and the pointer to a function to control some GPIO(s) as cs for adapters that dont 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@vger.kernel.org > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |