Re: [PATCH] spi/pl022: Add high priority message pump support

2012-01-27 Thread Grant Likely
On Thu, Jan 26, 2012 at 7:48 AM, Linus Walleij linus.wall...@linaro.org wrote:
 On Wed, Jan 25, 2012 at 3:02 PM, Mark Brown broo...@sirena.org.uk wrote:
 On Tue, Jan 24, 2012 at 10:14:32PM +0100, Linus Walleij wrote:

 This switches the PL022 worker to a kthread in order to get
 hold of a mechanism to control the message pump priority. On
 low-latency systems elevating the message kthread to realtime
 priority give a real sleek response curve. This has been
 confirmed by measurements. Realtime priority elevation for
 a certain PL022 port can be requested from platform data.

 It really feels like we should be pulling this into the core - lots of
 drivers use a workqueue to drive data through the system and they're all
 going to have exactly the same issue.

 That reads to me like the entire message queue and transfer pump
 mechanism from the PL022 driver should be made into generic
 code. That is the key ingredient from the PL022 driver that has
 allowed us to get real nice throughput on it.

 And that observation is correct, but a bit of upfront code.

 If Grant is in on it I might give it a try.

Go for it and see what it looks like.

g.

--
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] spi/pl022: Add high priority message pump support

2012-01-26 Thread Linus Walleij
On Wed, Jan 25, 2012 at 3:02 PM, Mark Brown broo...@sirena.org.uk wrote:
 On Tue, Jan 24, 2012 at 10:14:32PM +0100, Linus Walleij wrote:

 This switches the PL022 worker to a kthread in order to get
 hold of a mechanism to control the message pump priority. On
 low-latency systems elevating the message kthread to realtime
 priority give a real sleek response curve. This has been
 confirmed by measurements. Realtime priority elevation for
 a certain PL022 port can be requested from platform data.

 It really feels like we should be pulling this into the core - lots of
 drivers use a workqueue to drive data through the system and they're all
 going to have exactly the same issue.

That reads to me like the entire message queue and transfer pump
mechanism from the PL022 driver should be made into generic
code. That is the key ingredient from the PL022 driver that has
allowed us to get real nice throughput on it.

And that observation is correct, but a bit of upfront code.

If Grant is in on it I might give it a try.

I would make it an opt-in for SPI drivers to have a generic
message queue instead of treating messages in a one-by-one
manner.

So say I add to spi/Kconfig that SPI_MASTER:s can define
SPI_MASTER_QUEUE then add a
spi_alloc_queued_master() call with some new
#ifdef SPI_MASTER_QUEUE functions and fields
added to in the struct spi_master to handle this stuff,
and compile in the message pump so that these drivers
does not implement the .transfer() call, but instead
some more fine-grained ones would that fit the bill?

Yours,
Linus Walleij

--
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] spi/pl022: Add high priority message pump support

2012-01-26 Thread Mark Brown
On Thu, Jan 26, 2012 at 03:48:59PM +0100, Linus Walleij wrote:
 On Wed, Jan 25, 2012 at 3:02 PM, Mark Brown broo...@sirena.org.uk wrote:

  It really feels like we should be pulling this into the core - lots of
  drivers use a workqueue to drive data through the system and they're all
  going to have exactly the same issue.

 That reads to me like the entire message queue and transfer pump
 mechanism from the PL022 driver should be made into generic
 code. That is the key ingredient from the PL022 driver that has
 allowed us to get real nice throughput on it.

 And that observation is correct, but a bit of upfront code.

Probably, yes - lots of drivers seem to have a workqueue of some kind
that drives the actual transfers and I strongly suspect that there's a
lot of generality there.  I have to confess that I had just thought that
transfer pump was an obscure bit of jargon in the changelog so it's
possible it's doing something device specific but it'd seem surprising
TBH.

 I would make it an opt-in for SPI drivers to have a generic
 message queue instead of treating messages in a one-by-one
 manner.

That'd certainly be nice - where things use a workqueue they do often
just pass the entire request from the caller off to it in one fell swoop
so that sounds compatible.

--
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] spi/pl022: Add high priority message pump support

2012-01-25 Thread Mark Brown
On Tue, Jan 24, 2012 at 10:14:32PM +0100, Linus Walleij wrote:
 From: Chris Blair chris.bl...@stericsson.com
 
 This switches the PL022 worker to a kthread in order to get
 hold of a mechanism to control the message pump priority. On
 low-latency systems elevating the message kthread to realtime
 priority give a real sleek response curve. This has been
 confirmed by measurements. Realtime priority elevation for
 a certain PL022 port can be requested from platform data.

It really feels like we should be pulling this into the core - lots of
drivers use a workqueue to drive data through the system and they're all
going to have exactly the same issue.

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


[PATCH] spi/pl022: Add high priority message pump support

2012-01-24 Thread Linus Walleij
From: Chris Blair chris.bl...@stericsson.com

This switches the PL022 worker to a kthread in order to get
hold of a mechanism to control the message pump priority. On
low-latency systems elevating the message kthread to realtime
priority give a real sleek response curve. This has been
confirmed by measurements. Realtime priority elevation for
a certain PL022 port can be requested from platform data.

Signed-off-by: Chris Blair chris.bl...@stericsson.com
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
 drivers/spi/spi-pl022.c|   77 +--
 include/linux/amba/pl022.h |3 ++
 2 files changed, 55 insertions(+), 25 deletions(-)

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index 2f9cb43..81847c9 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -29,7 +29,7 @@
 #include linux/errno.h
 #include linux/interrupt.h
 #include linux/spi/spi.h
-#include linux/workqueue.h
+#include linux/kthread.h
 #include linux/delay.h
 #include linux/clk.h
 #include linux/err.h
@@ -41,6 +41,7 @@
 #include linux/dma-mapping.h
 #include linux/scatterlist.h
 #include linux/pm_runtime.h
+#include linux/sched.h
 
 /*
  * This macro is used to define some register default values.
@@ -330,12 +331,13 @@ struct vendor_data {
  * @clk: outgoing clock SPICLK for the SPI bus
  * @master: SPI framework hookup
  * @master_info: controller-specific data from machine setup
- * @workqueue: a workqueue on which any spi_message request is queued
- * @pump_messages: work struct for scheduling work to the workqueue
+ * @kworker: thread struct for message pump
+ * @kworker_task: pointer to task for message pump kworker thread
+ * @pump_messages: work struct for scheduling work to the message pump
  * @queue_lock: spinlock to syncronise access to message queue
  * @queue: message queue
- * @busy: workqueue is busy
- * @running: workqueue is running
+ * @busy: message pump is busy
+ * @running: message pump is running
  * @pump_transfers: Tasklet used in Interrupt Transfer mode
  * @cur_msg: Pointer to current spi_message being processed
  * @cur_transfer: Pointer to current spi_transfer
@@ -365,9 +367,10 @@ struct pl022 {
struct clk  *clk;
struct spi_master   *master;
struct pl022_ssp_controller *master_info;
-   /* Driver message queue */
-   struct workqueue_struct *workqueue;
-   struct work_struct  pump_messages;
+   /* Driver message pump */
+   struct kthread_worker   kworker;
+   struct task_struct  *kworker_task;
+   struct kthread_work pump_messages;
spinlock_t  queue_lock;
struct list_headqueue;
boolbusy;
@@ -504,7 +507,7 @@ static void giveback(struct pl022 *pl022)
pl022-cur_msg = NULL;
pl022-cur_transfer = NULL;
pl022-cur_chip = NULL;
-   queue_work(pl022-workqueue, pl022-pump_messages);
+   queue_kthread_work(pl022-kworker, pl022-pump_messages);
spin_unlock_irqrestore(pl022-queue_lock, flags);
 
msg-state = NULL;
@@ -1494,8 +1497,8 @@ out:
 }
 
 /**
- * pump_messages - Workqueue function which processes spi message queue
- * @data: pointer to private data of SSP driver
+ * pump_messages - kthread work function which processes spi message queue
+ * @work: pointer to kthread work struct contained in the pl022 private struct
  *
  * This function checks if there is any spi message in the queue that
  * needs processing and delegate control to appropriate function
@@ -1503,7 +1506,7 @@ out:
  * based on the kind of the transfer
  *
  */
-static void pump_messages(struct work_struct *work)
+static void pump_messages(struct kthread_work *work)
 {
struct pl022 *pl022 =
container_of(work, struct pl022, pump_messages);
@@ -1556,7 +1559,7 @@ static void pump_messages(struct work_struct *work)
if (!was_busy)
/*
 * We enable the core voltage and clocks here, then the clocks
-* and core will be disabled when this workqueue is run again
+* and core will be disabled when this thread is run again
 * and there is no more work to be done.
 */
pm_runtime_get_sync(pl022-adev-dev);
@@ -1572,6 +1575,8 @@ static void pump_messages(struct work_struct *work)
 
 static int __init init_queue(struct pl022 *pl022)
 {
+   struct sched_param param = { .sched_priority = MAX_RT_PRIO - 1 };
+
INIT_LIST_HEAD(pl022-queue);
spin_lock_init(pl022-queue_lock);
 
@@ -1581,11 +1586,29 @@ static int __init init_queue(struct pl022 *pl022)
tasklet_init(pl022-pump_transfers, pump_transfers,
(unsigned long)pl022);
 
-   INIT_WORK(pl022-pump_messages, pump_messages);
-   pl022-workqueue = create_singlethread_workqueue(
+   

Re: [PATCH] spi/pl022: Add high priority message pump support

2012-01-24 Thread Viresh Kumar
On 1/25/2012 2:44 AM, Linus WALLEIJ wrote:
 From: Chris Blair chris.bl...@stericsson.com
 
 This switches the PL022 worker to a kthread in order to get
 hold of a mechanism to control the message pump priority. On
 low-latency systems elevating the message kthread to realtime
 priority give a real sleek response curve. This has been
 confirmed by measurements. Realtime priority elevation for
 a certain PL022 port can be requested from platform data.
 
 Signed-off-by: Chris Blair chris.bl...@stericsson.com
 Signed-off-by: Linus Walleij linus.wall...@linaro.org
 ---
  drivers/spi/spi-pl022.c|   77 +--
  include/linux/amba/pl022.h |3 ++
  2 files changed, 55 insertions(+), 25 deletions(-)
 
 diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
 index 2f9cb43..81847c9 100644
 --- a/drivers/spi/spi-pl022.c
 +++ b/drivers/spi/spi-pl022.c
 @@ -29,7 +29,7 @@
  #include linux/errno.h
  #include linux/interrupt.h
  #include linux/spi/spi.h
 -#include linux/workqueue.h
 +#include linux/kthread.h
  #include linux/delay.h
  #include linux/clk.h
  #include linux/err.h
 @@ -41,6 +41,7 @@
  #include linux/dma-mapping.h
  #include linux/scatterlist.h
  #include linux/pm_runtime.h
 +#include linux/sched.h
  
  /*
   * This macro is used to define some register default values.
 @@ -330,12 +331,13 @@ struct vendor_data {
   * @clk: outgoing clock SPICLK for the SPI bus
   * @master: SPI framework hookup
   * @master_info: controller-specific data from machine setup
 - * @workqueue: a workqueue on which any spi_message request is queued
 - * @pump_messages: work struct for scheduling work to the workqueue
 + * @kworker: thread struct for message pump
 + * @kworker_task: pointer to task for message pump kworker thread
 + * @pump_messages: work struct for scheduling work to the message pump
   * @queue_lock: spinlock to syncronise access to message queue
   * @queue: message queue
 - * @busy: workqueue is busy
 - * @running: workqueue is running
 + * @busy: message pump is busy
 + * @running: message pump is running
   * @pump_transfers: Tasklet used in Interrupt Transfer mode
   * @cur_msg: Pointer to current spi_message being processed
   * @cur_transfer: Pointer to current spi_transfer
 @@ -365,9 +367,10 @@ struct pl022 {
   struct clk  *clk;
   struct spi_master   *master;
   struct pl022_ssp_controller *master_info;
 - /* Driver message queue */
 - struct workqueue_struct *workqueue;
 - struct work_struct  pump_messages;
 + /* Driver message pump */
 + struct kthread_worker   kworker;
 + struct task_struct  *kworker_task;
 + struct kthread_work pump_messages;
   spinlock_t  queue_lock;
   struct list_headqueue;
   boolbusy;
 @@ -504,7 +507,7 @@ static void giveback(struct pl022 *pl022)
   pl022-cur_msg = NULL;
   pl022-cur_transfer = NULL;
   pl022-cur_chip = NULL;
 - queue_work(pl022-workqueue, pl022-pump_messages);
 + queue_kthread_work(pl022-kworker, pl022-pump_messages);
   spin_unlock_irqrestore(pl022-queue_lock, flags);
  
   msg-state = NULL;
 @@ -1494,8 +1497,8 @@ out:
  }
  
  /**
 - * pump_messages - Workqueue function which processes spi message queue
 - * @data: pointer to private data of SSP driver
 + * pump_messages - kthread work function which processes spi message queue
 + * @work: pointer to kthread work struct contained in the pl022 private 
 struct
   *
   * This function checks if there is any spi message in the queue that
   * needs processing and delegate control to appropriate function
 @@ -1503,7 +1506,7 @@ out:
   * based on the kind of the transfer
   *
   */
 -static void pump_messages(struct work_struct *work)
 +static void pump_messages(struct kthread_work *work)
  {
   struct pl022 *pl022 =
   container_of(work, struct pl022, pump_messages);
 @@ -1556,7 +1559,7 @@ static void pump_messages(struct work_struct *work)
   if (!was_busy)
   /*
* We enable the core voltage and clocks here, then the clocks
 -  * and core will be disabled when this workqueue is run again
 +  * and core will be disabled when this thread is run again
* and there is no more work to be done.
*/
   pm_runtime_get_sync(pl022-adev-dev);
 @@ -1572,6 +1575,8 @@ static void pump_messages(struct work_struct *work)
  
  static int __init init_queue(struct pl022 *pl022)
  {
 + struct sched_param param = { .sched_priority = MAX_RT_PRIO - 1 };
 +
   INIT_LIST_HEAD(pl022-queue);
   spin_lock_init(pl022-queue_lock);
  
 @@ -1581,11 +1586,29 @@ static int __init init_queue(struct pl022 *pl022)
   tasklet_init(pl022-pump_transfers, pump_transfers,
   (unsigned long)pl022);
  
 -