Re: SPI redux ... driver model support

From: Leeds United Fan (kewell_at_rbcmail.ru)
Date: 09/02/05

  • Next message: Rajat Jain: "Re: ACPI problem with PCI Express Native Hot-plug driver" 
    Date:	Fri, 02 Sep 2005 12:00:29 +0400
To: Mark Underwood <basicmark@yahoo.com>

    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@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 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@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/
    >
    >
    >
    >

    -
    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/

  • Next message: Rajat Jain: "Re: ACPI problem with PCI Express Native Hot-plug driver"