Re: [PATCH 3/3] spi: create an optional message queueing infrastructure
On Thu, Feb 2, 2012 at 5:05 PM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: [Me] + * This function checks if there is any spi message in the queue that + * needs processing and delegate control to appropriate function + * do_polling_transfer()/do_interrupt_dma_transfer() + * based on the kind of the transfer This comment is bitrotted - the do_X_transfer functions are pl022 specific. I had initially thought that this was another opportunity for code sharing but it turns out that unlike the s3c64xx driver pl022 is not switching between the two modes based on transfer size so we do need different logic. Yes atleast for now, I looked at the per-transfer pump we have but I cannot right now figure out if that part can also be genericizied so lets do it piecemal. Fixing the comment in v2. Thanks, Linus Walleij -- Try before you buy = See our experts in action! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-dev2 ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 3/3] spi: create an optional message queueing infrastructure
On Thu, Feb 2, 2012 at 7:52 PM, Grant Likely grant.lik...@secretlab.ca wrote: This looks really good. In fact, can you think of any reason why we wouldn't force all spi drivers to use central queueing? I don't see any advantage in leaving the old mechanism in place other than to allow driver to transition from the old API. Not really... except that it'll be hard to convert all drivers. There is also the spi_bitbang which also implements its own message queue and workqueue. Several drivers make use of even if they don't do bitbanging. Sound like they're abusing it then :-/ Can you investigate dropping patch 2 and making the normal register function set up queued transfers if the transfer hook is not set. If it is set, then make the kernel complain to the console, quietly at first, but we'll get louder before ultimately removing the non-queued interface. Thoughts? OK I'll post v2 soon with these changes. Thanks, Linus Walleij -- Try before you buy = See our experts in action! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-dev2 ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
[PATCH 3/3] spi: create an optional message queueing infrastructure
From: Linus Walleij linus.wall...@linaro.org This rips the message queue in the PL022 driver out and pushes it into (optional) common infrastructure. Drivers that want to use the message pumping thread will need to select the symbol SPI_MASTER_QUEUE and implement three methods in place for the current transfer() method. Most of the design is described in the documentation changes that are included in this patch. Cc: Mark Brown broo...@opensource.wolfsonmicro.com Signed-off-by: Linus Walleij linus.wall...@linaro.org --- Documentation/spi/spi-summary | 56 +- drivers/spi/Kconfig |5 + drivers/spi/spi-pl022.c | 305 ++ drivers/spi/spi.c | 370 + include/linux/spi/spi.h | 53 ++ 5 files changed, 528 insertions(+), 261 deletions(-) diff --git a/Documentation/spi/spi-summary b/Documentation/spi/spi-summary index 4884cb3..6cc950e 100644 --- a/Documentation/spi/spi-summary +++ b/Documentation/spi/spi-summary @@ -1,7 +1,7 @@ Overview of Linux kernel SPI support -21-May-2007 +02-Feb-2012 What is SPI? @@ -482,13 +482,17 @@ used to interact with the SPI core and SPI protocol drivers. It will also initialize its own internal state. (See below about bus numbering and those methods.) -After you initialize the spi_master, then use spi_register_master() to -publish it to the rest of the system. At that time, device nodes for -the controller and any predeclared spi devices will be made available, -and the driver model core will take care of binding them to drivers. +After you initialize the spi_master, then use spi_register_master() or +spi_register_master_queued() to publish it to the rest of the system. At +that time, device nodes for the controller and any predeclared spi devices +will be made available, and the driver model core will take care of binding +them to drivers. spi_register_master_queued() will only be available if +your driver has selected the config option SPI_MASTER_QUEUE in its Kconfig +entry. If you need to remove your SPI controller driver, spi_unregister_master() -will reverse the effect of spi_register_master(). +will reverse the effect of spi_register_master() or +spi_register_master_queued(). BUS NUMBERING @@ -525,17 +529,49 @@ SPI MASTER METHODS This must not sleep. Its responsibility is arrange that the transfer happens and its complete() callback is issued. The two will normally happen later, after other transfers complete, and - if the controller is idle it will need to be kickstarted. + if the controller is idle it will need to be kickstarted. This + method is not used on queued controllers and must be NULL if you + issue spi_register_master_queued(). master-cleanup(struct spi_device *spi) Your controller driver may use spi_device.controller_state to hold state it dynamically associates with that device. If you do that, be sure to provide the cleanup() method to free that state. +OPTIONAL QUEUEING METHODS + +master-prepare_transfer_hardware(struct spi_master *master) + This will be called by the queue mechanism to signal to the driver + that a message is coming in soon, so the subsystem requests the + driver to prepare the transfer hardware by issuing this call. + This may sleep. + +master-unprepare_transfer_hardware(struct spi_master *master) + This will be called by the queue mechanism to signal to the driver + that there are no more messages pending in the queue and it may + relax the hardware (e.g. by power management calls). This may sleep. + +master-transfer_one_message(struct spi_master *master, +struct spi_message *mesg) + The subsystem calls the driver to transfer a single message while + queuing transfers that arrive in the meantime. When the driver is + finished with this message, it must call + spi_finalize_current_message() so the subsystem can issue the next + transfer. This may sleep. + + SPI MESSAGE QUEUE -The bulk of the driver will be managing the I/O queue fed by transfer(). +If you are happy with the standard queueing mechanism provided by the +SPI subsystem, just have your Kconfig entry select SPI_MASTER_QUEUE and +implement the queued methods specified above. Using the message queue +has the upside of centralizing a lot of code and providing pure +process-context execution of methods. The message queue can also be +elevated to realtime priority on high-priority SPI traffic. + +Unless the queueing mechanism in the SPI subsystem is selected, the bulk +of the driver will be managing the I/O queue fed by transfer(). That queue could be purely conceptual. For example, a driver used only for low-frequency sensor access might be fine using synchronous PIO. @@ -561,4
Re: [PATCH 3/3] spi: create an optional message queueing infrastructure
On Thu, Feb 02, 2012 at 01:59:59PM +0100, Linus Walleij wrote: From: Linus Walleij linus.wall...@linaro.org This rips the message queue in the PL022 driver out and pushes it into (optional) common infrastructure. Drivers that want to use the message pumping thread will need to select the symbol SPI_MASTER_QUEUE and implement three methods in place for the current transfer() method. Most of the design is described in the documentation changes that are included in this patch. Cc: Mark Brown broo...@opensource.wolfsonmicro.com Signed-off-by: Linus Walleij linus.wall...@linaro.org This looks really good. In fact, can you think of any reason why we wouldn't force all spi drivers to use central queueing? I don't see any advantage in leaving the old mechanism in place other than to allow driver to transition from the old API. There is also the spi_bitbang which also implements its own message queue and workqueue. Several drivers make use of even if they don't do bitbanging. Can you investigate dropping patch 2 and making the normal register function set up queued transfers if the transfer hook is not set. If it is set, then make the kernel complain to the console, quietly at first, but we'll get louder before ultimately removing the non-queued interface. Thoughts? g. -- Keep Your Developer Skills Current with LearnDevNow! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-d2d ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
Re: [PATCH 3/3] spi: create an optional message queueing infrastructure
On Thu, Feb 2, 2012 at 3:47 PM, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Thu, Feb 02, 2012 at 11:52:47AM -0700, Grant Likely wrote: This looks really good. In fact, can you think of any reason why we wouldn't force all spi drivers to use central queueing? I don't see any advantage in leaving the old mechanism in place other than to allow driver to transition from the old API. Are any of the drivers capable of running from interrupt and which therefore don't need to use a thread at all? There may be. That should be easy to handle though. The queue definitely looks like something that should be shared by all drivers, and maybe that can be the first patch in the series. I can't think of a single reason why any spi driver wouldn't want to have queuing handled by the core. As for the thread, perhaps it can be made optional simply by not populating the pump_messages hook... but even then when a new message is enqueued there needs to be a way for the core to kick the spi bus driver so that it wakes up and processes the messages. If pump_messages isn't populated, then some other hook is needed. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. -- Try before you buy = See our experts in action! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-dev2 ___ spi-devel-general mailing list spi-devel-general@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/spi-devel-general
