Re: [PATCH] comedi: pcmmio.c: Fix coding style - use BIT macro

2015-11-12 Thread Ian Abbott

On 11/11/15 15:48, Ranjith Thangavel wrote:

BIT macro is used for defining BIT location instead of
shifting operator - coding style issue

Signed-off-by: Ranjith Thangavel <ranjithec...@gmail.com>
---
  drivers/staging/comedi/drivers/pcmmio.c |   44 +++
  1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/comedi/drivers/pcmmio.c 
b/drivers/staging/comedi/drivers/pcmmio.c
index 10472e6..f7ec224 100644
--- a/drivers/staging/comedi/drivers/pcmmio.c
+++ b/drivers/staging/comedi/drivers/pcmmio.c
@@ -84,25 +84,25 @@
  #define PCMMIO_AI_LSB_REG 0x00
  #define PCMMIO_AI_MSB_REG 0x01
  #define PCMMIO_AI_CMD_REG 0x02
-#define PCMMIO_AI_CMD_SE   (1 << 7)
-#define PCMMIO_AI_CMD_ODD_CHAN (1 << 6)
+#define PCMMIO_AI_CMD_SE   BIT(7)
+#define PCMMIO_AI_CMD_ODD_CHAN BIT(6)
  #define PCMMIO_AI_CMD_CHAN_SEL(x) (((x) & 0x3) << 4)
  #define PCMMIO_AI_CMD_RANGE(x)(((x) & 0x3) << 2)
-#define PCMMIO_RESOURCE_REG0x02
+#define PCMMIO_RESOURCE_REG0x02
  #define PCMMIO_RESOURCE_IRQ(x)(((x) & 0xf) << 0)
  #define PCMMIO_AI_STATUS_REG  0x03
-#define PCMMIO_AI_STATUS_DATA_READY(1 << 7)
-#define PCMMIO_AI_STATUS_DATA_DMA_PEND (1 << 6)
-#define PCMMIO_AI_STATUS_CMD_DMA_PEND  (1 << 5)
-#define PCMMIO_AI_STATUS_IRQ_PEND  (1 << 4)
-#define PCMMIO_AI_STATUS_DATA_DRQ_ENA  (1 << 2)
-#define PCMMIO_AI_STATUS_REG_SEL   (1 << 3)
-#define PCMMIO_AI_STATUS_CMD_DRQ_ENA   (1 << 1)
-#define PCMMIO_AI_STATUS_IRQ_ENA   (1 << 0)
+#define PCMMIO_AI_STATUS_DATA_READYBIT(7)
+#define PCMMIO_AI_STATUS_DATA_DMA_PEND BIT(6)
+#define PCMMIO_AI_STATUS_CMD_DMA_PEND  BIT(5)
+#define PCMMIO_AI_STATUS_IRQ_PEND  BIT(4)
+#define PCMMIO_AI_STATUS_DATA_DRQ_ENA  BIT(2)
+#define PCMMIO_AI_STATUS_REG_SEL   BIT(3)
+#define PCMMIO_AI_STATUS_CMD_DRQ_ENA   BIT(1)
+#define PCMMIO_AI_STATUS_IRQ_ENA   BIT(0)
  #define PCMMIO_AI_RES_ENA_REG 0x03
-#define PCMMIO_AI_RES_ENA_CMD_REG_ACCESS   (0 << 3)
-#define PCMMIO_AI_RES_ENA_AI_RES_ACCESS(1 << 3)
-#define PCMMIO_AI_RES_ENA_DIO_RES_ACCESS   (1 << 4)
+#define PCMMIO_AI_RES_ENA_CMD_REG_ACCESS   0
+#define PCMMIO_AI_RES_ENA_AI_RES_ACCESSBIT(3)
+#define PCMMIO_AI_RES_ENA_DIO_RES_ACCESS   BIT(4)
  #define PCMMIO_AI_2ND_ADC_OFFSET  0x04

  #define PCMMIO_AO_LSB_REG 0x08
@@ -125,14 +125,14 @@
  #define PCMMIO_AO_CMD_CHAN_SEL(x) (((x) & 0x03) << 1)
  #define PCMMIO_AO_CMD_CHAN_SEL_ALL(0x0f << 0)
  #define PCMMIO_AO_STATUS_REG  0x0b
-#define PCMMIO_AO_STATUS_DATA_READY(1 << 7)
-#define PCMMIO_AO_STATUS_DATA_DMA_PEND (1 << 6)
-#define PCMMIO_AO_STATUS_CMD_DMA_PEND  (1 << 5)
-#define PCMMIO_AO_STATUS_IRQ_PEND  (1 << 4)
-#define PCMMIO_AO_STATUS_DATA_DRQ_ENA  (1 << 2)
-#define PCMMIO_AO_STATUS_REG_SEL   (1 << 3)
-#define PCMMIO_AO_STATUS_CMD_DRQ_ENA   (1 << 1)
-#define PCMMIO_AO_STATUS_IRQ_ENA   (1 << 0)
+#define PCMMIO_AO_STATUS_DATA_READYBIT(7)
+#define PCMMIO_AO_STATUS_DATA_DMA_PEND BIT(6)
+#define PCMMIO_AO_STATUS_CMD_DMA_PEND  BIT(5)
+#define PCMMIO_AO_STATUS_IRQ_PEND  BIT(4)
+#define PCMMIO_AO_STATUS_DATA_DRQ_ENA  BIT(2)
+#define PCMMIO_AO_STATUS_REG_SEL   BIT(3)
+#define PCMMIO_AO_STATUS_CMD_DRQ_ENA   BIT(1)
+#define PCMMIO_AO_STATUS_IRQ_ENA   BIT(0)
  #define PCMMIO_AO_RESOURCE_ENA_REG0x0b
  #define PCMMIO_AO_2ND_DAC_OFFSET  0x04




The macro values used to be more-or-less nicely aligned in a column, but 
now they are not so nicely aligned.  Could you add or delete TABs as 
needed to line them up?  Remember, tab stops are every 8 spaces.


--
-=( Ian Abbott @ MEV Ltd.E-mail: <abbo...@mev.co.uk> )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] comedi: ni_65xx: Fix coding style - use BIT macro

2015-11-12 Thread Ian Abbott

On 11/11/15 16:22, Ranjith Thangavel wrote:

BIT macro is used for defining BIT location instead of
shifting operator - coding style issue

Signed-off-by: Ranjith Thangavel <ranjithec...@gmail.com>
---
  drivers/staging/comedi/drivers/ni_65xx.c |   54 +++---
  1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_65xx.c 
b/drivers/staging/comedi/drivers/ni_65xx.c
index 800d574..251117b 100644
--- a/drivers/staging/comedi/drivers/ni_65xx.c
+++ b/drivers/staging/comedi/drivers/ni_65xx.c
@@ -68,25 +68,25 @@
  /* Non-recurring Registers (8-bit except where noted) */
  #define NI_65XX_ID_REG0x00
  #define NI_65XX_CLR_REG   0x01
-#define NI_65XX_CLR_WDOG_INT   (1 << 6)
-#define NI_65XX_CLR_WDOG_PING  (1 << 5)
-#define NI_65XX_CLR_WDOG_EXP   (1 << 4)
-#define NI_65XX_CLR_EDGE_INT   (1 << 3)
-#define NI_65XX_CLR_OVERFLOW_INT   (1 << 2)
+#define NI_65XX_CLR_WDOG_INT   BIT(6)
+#define NI_65XX_CLR_WDOG_PING  BIT(5)
+#define NI_65XX_CLR_WDOG_EXP   BIT(4)
+#define NI_65XX_CLR_EDGE_INT   BIT(3)
+#define NI_65XX_CLR_OVERFLOW_INT   BIT(2)
  #define NI_65XX_STATUS_REG0x02
-#define NI_65XX_STATUS_WDOG_INT(1 << 5)
-#define NI_65XX_STATUS_FALL_EDGE   (1 << 4)
-#define NI_65XX_STATUS_RISE_EDGE   (1 << 3)
-#define NI_65XX_STATUS_INT (1 << 2)
-#define NI_65XX_STATUS_OVERFLOW_INT(1 << 1)
-#define NI_65XX_STATUS_EDGE_INT(1 << 0)
+#define NI_65XX_STATUS_WDOG_INTBIT(5)
+#define NI_65XX_STATUS_FALL_EDGE   BIT(4)
+#define NI_65XX_STATUS_RISE_EDGE   BIT(3)
+#define NI_65XX_STATUS_INT BIT(2)
+#define NI_65XX_STATUS_OVERFLOW_INTBIT(1)
+#define NI_65XX_STATUS_EDGE_INTBIT(0)
  #define NI_65XX_CTRL_REG  0x03
-#define NI_65XX_CTRL_WDOG_ENA  (1 << 5)
-#define NI_65XX_CTRL_FALL_EDGE_ENA (1 << 4)
-#define NI_65XX_CTRL_RISE_EDGE_ENA (1 << 3)
-#define NI_65XX_CTRL_INT_ENA   (1 << 2)
-#define NI_65XX_CTRL_OVERFLOW_ENA  (1 << 1)
-#define NI_65XX_CTRL_EDGE_ENA  (1 << 0)
+#define NI_65XX_CTRL_WDOG_ENA  BIT(5)
+#define NI_65XX_CTRL_FALL_EDGE_ENA BIT(4)
+#define NI_65XX_CTRL_RISE_EDGE_ENA BIT(3)
+#define NI_65XX_CTRL_INT_ENA   BIT(2)
+#define NI_65XX_CTRL_OVERFLOW_ENA  BIT(1)
+#define NI_65XX_CTRL_EDGE_ENA  BIT(0)
  #define NI_65XX_REV_REG   0x04 /* 32-bit */
  #define NI_65XX_FILTER_REG0x08 /* 32-bit */
  #define NI_65XX_RTSI_ROUTE_REG0x0c /* 16-bit */
@@ -94,24 +94,24 @@
  #define NI_65XX_RTSI_WDOG_REG 0x10 /* 16-bit */
  #define NI_65XX_RTSI_TRIG_REG 0x12 /* 16-bit */
  #define NI_65XX_AUTO_CLK_SEL_REG  0x14 /* PXI-6528 only */
-#define NI_65XX_AUTO_CLK_SEL_STATUS(1 << 1)
-#define NI_65XX_AUTO_CLK_SEL_DISABLE   (1 << 0)
+#define NI_65XX_AUTO_CLK_SEL_STATUSBIT(1)
+#define NI_65XX_AUTO_CLK_SEL_DISABLE   BIT(0)
  #define NI_65XX_WDOG_CTRL_REG 0x15
-#define NI_65XX_WDOG_CTRL_ENA  (1 << 0)
+#define NI_65XX_WDOG_CTRL_ENA  BIT(0)
  #define NI_65XX_RTSI_CFG_REG  0x16
-#define NI_65XX_RTSI_CFG_RISE_SENSE(1 << 2)
-#define NI_65XX_RTSI_CFG_FALL_SENSE(1 << 1)
-#define NI_65XX_RTSI_CFG_SYNC_DETECT   (1 << 0)
+#define NI_65XX_RTSI_CFG_RISE_SENSEBIT(2)
+#define NI_65XX_RTSI_CFG_FALL_SENSEBIT(1)
+#define NI_65XX_RTSI_CFG_SYNC_DETECT   BIT(0)
  #define NI_65XX_WDOG_STATUS_REG   0x17
-#define NI_65XX_WDOG_STATUS_EXP(1 << 0)
+#define NI_65XX_WDOG_STATUS_EXPBIT(0)
  #define NI_65XX_WDOG_INTERVAL_REG 0x18 /* 32-bit */

  /* Recurring port registers (8-bit) */
  #define NI_65XX_PORT(x)   ((x) * 0x10)
  #define NI_65XX_IO_DATA_REG(x)(0x40 + NI_65XX_PORT(x))
  #define NI_65XX_IO_SEL_REG(x) (0x41 + NI_65XX_PORT(x))
-#define NI_65XX_IO_SEL_OUTPUT  (0 << 0)
-#define NI_65XX_IO_SEL_INPUT   (1 << 0)
+#define NI_65XX_IO_SEL_OUTPUT  0
+#define NI_65XX_IO_SEL_INPUT   BIT(0)
  #define NI_65XX_RISE_EDGE_ENA_REG(x)  (0x42 + NI_65XX_PORT(x))
  #define NI_65XX_FALL_EDGE_ENA_REG(x)  (0x43 + NI_65XX_PORT(x))
  #define NI_65XX_FILTER_ENA(x) (0x44 + NI_65XX_PORT(x))
@@ -613,7 +613,7 @@ static int ni_65xx_intr_insn_config(struct comedi_device 
*dev,

  /* ripped from mite.h and mite_setup2() to avoid mite dependency */
  #define MITE_IODWBSR  0xc0 /* IO Device Window Base Size Register */
-#define WENAB  (1 << 7) /* window enable */
+#define WENAB          BIT(7) /* window enable */

  static int ni_65xx_mite_init(struct pci_dev *pcidev)
  {



Than

Re: [PATCH] comedi: comedi_parport: Fix coding style - use BIT macro

2015-11-12 Thread Ian Abbott

On 11/11/15 15:54, Ranjith Thangavel wrote:

BIT macro is used for defining BIT location instead of
shifting operator - coding style issue

Signed-off-by: Ranjith Thangavel <ranjithec...@gmail.com>
---
  drivers/staging/comedi/drivers/comedi_parport.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_parport.c 
b/drivers/staging/comedi/drivers/comedi_parport.c
index 15a4093..1bf8ddc 100644
--- a/drivers/staging/comedi/drivers/comedi_parport.c
+++ b/drivers/staging/comedi/drivers/comedi_parport.c
@@ -75,8 +75,8 @@
  #define PARPORT_DATA_REG  0x00
  #define PARPORT_STATUS_REG0x01
  #define PARPORT_CTRL_REG  0x02
-#define PARPORT_CTRL_IRQ_ENA   (1 << 4)
-#define PARPORT_CTRL_BIDIR_ENA (1 << 5)
+#define PARPORT_CTRL_IRQ_ENA   BIT(4)
+#define PARPORT_CTRL_BIDIR_ENA BIT(5)

  static int parport_data_reg_insn_bits(struct comedi_device *dev,
  struct comedi_subdevice *s,



Thanks!

Reviewed-by: Ian Abbott <abbo...@mev.co.uk>

--
-=( Ian Abbott @ MEV Ltd.E-mail: <abbo...@mev.co.uk> )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] comedi: comedi_parport: Fix coding style - use BIT macro

2015-11-11 Thread Ian Abbott

On 11/11/15 14:18, Ranjith T wrote:

Actually my name is Ranjith and father name is Thangavel. So I used to
mention my name as Ranjith T.


How do you write your name on legal documents?  That is what you should 
use on the patch's "Signed-off-by:" line, and on the email's "From:" line.


Thanks.

--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] comedi: comedi_parport: Fix coding style - use BIT macro

2015-11-11 Thread Ian Abbott

On 11/11/2015 13:44, Ranjith T wrote:

Is this patch is fine?.

Thanks,
Ranjith T.


Fine apart from the "Signed-off-by" line.  I'm guessing your last name 
isn't really "T".



On Mon, Nov 9, 2015 at 11:18 PM, Ranjith T  wrote:

BIT macro is used for defining BIT location instead of
shifting operator - coding style issue

Signed-off-by: Ranjith T 
---
  drivers/staging/comedi/drivers/comedi_parport.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_parport.c 
b/drivers/staging/comedi/drivers/comedi_parport.c
index 15a4093..1bf8ddc 100644
--- a/drivers/staging/comedi/drivers/comedi_parport.c
+++ b/drivers/staging/comedi/drivers/comedi_parport.c
@@ -75,8 +75,8 @@
  #define PARPORT_DATA_REG   0x00
  #define PARPORT_STATUS_REG 0x01
  #define PARPORT_CTRL_REG   0x02
-#define PARPORT_CTRL_IRQ_ENA   (1 << 4)
-#define PARPORT_CTRL_BIDIR_ENA (1 << 5)
+#define PARPORT_CTRL_IRQ_ENA   BIT(4)
+#define PARPORT_CTRL_BIDIR_ENA BIT(5)

  static int parport_data_reg_insn_bits(struct comedi_device *dev,
   struct comedi_subdevice *s,
--
1.7.10.4


--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] staging: comedi: use kmalloc_array instead of kmalloc

2015-11-11 Thread Ian Abbott

On 10/11/2015 14:41, Geliang Tang wrote:

Use kmalloc_array instead of kmalloc to allocate memory for an array.

Signed-off-by: Geliang Tang 

---
Changes in v2:
  - preserve the existing whitespace style.
---
  drivers/staging/comedi/drivers/amplc_pci224.c | 11 +++
  drivers/staging/comedi/drivers/ni_670x.c  |  5 +++--
  2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c 
b/drivers/staging/comedi/drivers/amplc_pci224.c
index b2f7679..cac011f 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -1022,14 +1022,17 @@ pci224_auto_attach(struct comedi_device *dev, unsigned 
long context_model)
irq = pci_dev->irq;

/* Allocate buffer to hold values for AO channel scan. */
-   devpriv->ao_scan_vals = kmalloc(sizeof(devpriv->ao_scan_vals[0]) *
-   board->ao_chans, GFP_KERNEL);
+   devpriv->ao_scan_vals = kmalloc_array(board->ao_chans,
+ sizeof(devpriv->ao_scan_vals[0]),
+ GFP_KERNEL);
if (!devpriv->ao_scan_vals)
return -ENOMEM;

/* Allocate buffer to hold AO channel scan order. */
-   devpriv->ao_scan_order = kmalloc(sizeof(devpriv->ao_scan_order[0]) *
-board->ao_chans, GFP_KERNEL);
+   devpriv->ao_scan_order =
+   kmalloc_array(board->ao_chans,
+ sizeof(devpriv->ao_scan_order[0]),
+ GFP_KERNEL);
if (!devpriv->ao_scan_order)
return -ENOMEM;

diff --git a/drivers/staging/comedi/drivers/ni_670x.c 
b/drivers/staging/comedi/drivers/ni_670x.c
index f4c580f..3e7271880 100644
--- a/drivers/staging/comedi/drivers/ni_670x.c
+++ b/drivers/staging/comedi/drivers/ni_670x.c
@@ -214,8 +214,9 @@ static int ni_670x_auto_attach(struct comedi_device *dev,
if (s->n_chan == 32) {
const struct comedi_lrange **range_table_list;

-   range_table_list = kmalloc(sizeof(struct comedi_lrange *) * 32,
-  GFP_KERNEL);
+   range_table_list = kmalloc_array(32,
+sizeof(struct comedi_lrange *),
+GFP_KERNEL);
if (!range_table_list)
return -ENOMEM;
s->range_table_list = range_table_list;



Seems fine!

Reviewed-by: Ian Abbott 

--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v2] staging: comedi: use kmalloc_array instead of kmalloc

2015-11-11 Thread Ian Abbott

On 10/11/2015 14:41, Geliang Tang wrote:

Use kmalloc_array instead of kmalloc to allocate memory for an array.

Signed-off-by: Geliang Tang <geliangt...@163.com>

---
Changes in v2:
  - preserve the existing whitespace style.
---
  drivers/staging/comedi/drivers/amplc_pci224.c | 11 +++
  drivers/staging/comedi/drivers/ni_670x.c  |  5 +++--
  2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c 
b/drivers/staging/comedi/drivers/amplc_pci224.c
index b2f7679..cac011f 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -1022,14 +1022,17 @@ pci224_auto_attach(struct comedi_device *dev, unsigned 
long context_model)
irq = pci_dev->irq;

/* Allocate buffer to hold values for AO channel scan. */
-   devpriv->ao_scan_vals = kmalloc(sizeof(devpriv->ao_scan_vals[0]) *
-   board->ao_chans, GFP_KERNEL);
+   devpriv->ao_scan_vals = kmalloc_array(board->ao_chans,
+ sizeof(devpriv->ao_scan_vals[0]),
+ GFP_KERNEL);
if (!devpriv->ao_scan_vals)
return -ENOMEM;

/* Allocate buffer to hold AO channel scan order. */
-   devpriv->ao_scan_order = kmalloc(sizeof(devpriv->ao_scan_order[0]) *
-board->ao_chans, GFP_KERNEL);
+   devpriv->ao_scan_order =
+   kmalloc_array(board->ao_chans,
+ sizeof(devpriv->ao_scan_order[0]),
+ GFP_KERNEL);
if (!devpriv->ao_scan_order)
return -ENOMEM;

diff --git a/drivers/staging/comedi/drivers/ni_670x.c 
b/drivers/staging/comedi/drivers/ni_670x.c
index f4c580f..3e7271880 100644
--- a/drivers/staging/comedi/drivers/ni_670x.c
+++ b/drivers/staging/comedi/drivers/ni_670x.c
@@ -214,8 +214,9 @@ static int ni_670x_auto_attach(struct comedi_device *dev,
if (s->n_chan == 32) {
const struct comedi_lrange **range_table_list;

-   range_table_list = kmalloc(sizeof(struct comedi_lrange *) * 32,
-  GFP_KERNEL);
+   range_table_list = kmalloc_array(32,
+sizeof(struct comedi_lrange *),
+GFP_KERNEL);
if (!range_table_list)
return -ENOMEM;
s->range_table_list = range_table_list;



Seems fine!

Reviewed-by: Ian Abbott <abbo...@mev.co.uk>

--
-=( Ian Abbott @ MEV Ltd.E-mail: <abbo...@mev.co.uk> )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] comedi: comedi_parport: Fix coding style - use BIT macro

2015-11-11 Thread Ian Abbott

On 11/11/15 14:18, Ranjith T wrote:

Actually my name is Ranjith and father name is Thangavel. So I used to
mention my name as Ranjith T.


How do you write your name on legal documents?  That is what you should 
use on the patch's "Signed-off-by:" line, and on the email's "From:" line.


Thanks.

--
-=( Ian Abbott @ MEV Ltd.E-mail: <abbo...@mev.co.uk> )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] comedi: comedi_parport: Fix coding style - use BIT macro

2015-11-11 Thread Ian Abbott

On 11/11/2015 13:44, Ranjith T wrote:

Is this patch is fine?.

Thanks,
Ranjith T.


Fine apart from the "Signed-off-by" line.  I'm guessing your last name 
isn't really "T".



On Mon, Nov 9, 2015 at 11:18 PM, Ranjith T <ranjithec...@gmail.com> wrote:

BIT macro is used for defining BIT location instead of
shifting operator - coding style issue

Signed-off-by: Ranjith T <ranjithec...@gmail.com>
---
  drivers/staging/comedi/drivers/comedi_parport.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_parport.c 
b/drivers/staging/comedi/drivers/comedi_parport.c
index 15a4093..1bf8ddc 100644
--- a/drivers/staging/comedi/drivers/comedi_parport.c
+++ b/drivers/staging/comedi/drivers/comedi_parport.c
@@ -75,8 +75,8 @@
  #define PARPORT_DATA_REG   0x00
  #define PARPORT_STATUS_REG 0x01
  #define PARPORT_CTRL_REG   0x02
-#define PARPORT_CTRL_IRQ_ENA   (1 << 4)
-#define PARPORT_CTRL_BIDIR_ENA (1 << 5)
+#define PARPORT_CTRL_IRQ_ENA   BIT(4)
+#define PARPORT_CTRL_BIDIR_ENA BIT(5)

  static int parport_data_reg_insn_bits(struct comedi_device *dev,
   struct comedi_subdevice *s,
--
1.7.10.4


--
-=( Ian Abbott @ MEV Ltd.E-mail: <abbo...@mev.co.uk> )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/4] staging: comedi: use kmalloc_array instead of kmalloc

2015-11-09 Thread Ian Abbott

On 08/11/15 14:17, Geliang Tang wrote:

Use kmalloc_array instead of kmalloc to allocate memory for an array.

Signed-off-by: Geliang Tang 
---
  drivers/staging/comedi/drivers/amplc_pci224.c | 8 
  drivers/staging/comedi/drivers/ni_670x.c  | 5 +++--
  2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c 
b/drivers/staging/comedi/drivers/amplc_pci224.c
index b2f7679..d39f02d 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -1022,14 +1022,14 @@ pci224_auto_attach(struct comedi_device *dev, unsigned 
long context_model)
irq = pci_dev->irq;

/* Allocate buffer to hold values for AO channel scan. */
-   devpriv->ao_scan_vals = kmalloc(sizeof(devpriv->ao_scan_vals[0]) *
-   board->ao_chans, GFP_KERNEL);
+   devpriv->ao_scan_vals = kmalloc_array(board->ao_chans,
+   sizeof(devpriv->ao_scan_vals[0]), GFP_KERNEL);
if (!devpriv->ao_scan_vals)
return -ENOMEM;

/* Allocate buffer to hold AO channel scan order. */
-   devpriv->ao_scan_order = kmalloc(sizeof(devpriv->ao_scan_order[0]) *
-board->ao_chans, GFP_KERNEL);
+   devpriv->ao_scan_order = kmalloc_array(board->ao_chans,
+   sizeof(devpriv->ao_scan_order[0]), GFP_KERNEL);
if (!devpriv->ao_scan_order)
return -ENOMEM;



It would be better if the patch preserved the existing whitespace style 
here, even though that increases the number of lines.



diff --git a/drivers/staging/comedi/drivers/ni_670x.c 
b/drivers/staging/comedi/drivers/ni_670x.c
index f4c580f..3e7271880 100644
--- a/drivers/staging/comedi/drivers/ni_670x.c
+++ b/drivers/staging/comedi/drivers/ni_670x.c
@@ -214,8 +214,9 @@ static int ni_670x_auto_attach(struct comedi_device *dev,
if (s->n_chan == 32) {
const struct comedi_lrange **range_table_list;

-   range_table_list = kmalloc(sizeof(struct comedi_lrange *) * 32,
-  GFP_KERNEL);
+   range_table_list = kmalloc_array(32,
+sizeof(struct comedi_lrange *),
+GFP_KERNEL);
if (!range_table_list)
return -ENOMEM;
s->range_table_list = range_table_list;




--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/4] staging: comedi: use kmalloc_array instead of kmalloc

2015-11-09 Thread Ian Abbott

On 08/11/15 14:17, Geliang Tang wrote:

Use kmalloc_array instead of kmalloc to allocate memory for an array.

Signed-off-by: Geliang Tang <geliangt...@163.com>
---
  drivers/staging/comedi/drivers/amplc_pci224.c | 8 
  drivers/staging/comedi/drivers/ni_670x.c  | 5 +++--
  2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c 
b/drivers/staging/comedi/drivers/amplc_pci224.c
index b2f7679..d39f02d 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -1022,14 +1022,14 @@ pci224_auto_attach(struct comedi_device *dev, unsigned 
long context_model)
irq = pci_dev->irq;

/* Allocate buffer to hold values for AO channel scan. */
-   devpriv->ao_scan_vals = kmalloc(sizeof(devpriv->ao_scan_vals[0]) *
-   board->ao_chans, GFP_KERNEL);
+   devpriv->ao_scan_vals = kmalloc_array(board->ao_chans,
+   sizeof(devpriv->ao_scan_vals[0]), GFP_KERNEL);
if (!devpriv->ao_scan_vals)
return -ENOMEM;

/* Allocate buffer to hold AO channel scan order. */
-   devpriv->ao_scan_order = kmalloc(sizeof(devpriv->ao_scan_order[0]) *
-board->ao_chans, GFP_KERNEL);
+   devpriv->ao_scan_order = kmalloc_array(board->ao_chans,
+   sizeof(devpriv->ao_scan_order[0]), GFP_KERNEL);
if (!devpriv->ao_scan_order)
return -ENOMEM;



It would be better if the patch preserved the existing whitespace style 
here, even though that increases the number of lines.



diff --git a/drivers/staging/comedi/drivers/ni_670x.c 
b/drivers/staging/comedi/drivers/ni_670x.c
index f4c580f..3e7271880 100644
--- a/drivers/staging/comedi/drivers/ni_670x.c
+++ b/drivers/staging/comedi/drivers/ni_670x.c
@@ -214,8 +214,9 @@ static int ni_670x_auto_attach(struct comedi_device *dev,
if (s->n_chan == 32) {
const struct comedi_lrange **range_table_list;

-   range_table_list = kmalloc(sizeof(struct comedi_lrange *) * 32,
-  GFP_KERNEL);
+   range_table_list = kmalloc_array(32,
+sizeof(struct comedi_lrange *),
+GFP_KERNEL);
if (!range_table_list)
return -ENOMEM;
    s->range_table_list = range_table_list;




--
-=( Ian Abbott @ MEV Ltd.E-mail: <abbo...@mev.co.uk> )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/3] comedi: driver: Fix - BIT macro used coding style issue

2015-11-06 Thread Ian Abbott

On 06/11/15 13:54, Ranjith T wrote:

Is this patch is fine?..Because I didn't get any reply for this patch


Your patch numbering sucks.  Isolated patches should just use [PATCH], 
not [PATCH 3/3], [PATCH 4/4], etc.


Also, could you mention the name of the COMEDI driver in the patch 
title, e.g.:


[PATCH] comedi: pcmmio: Fix coding style - use BIT macro


Thanks,
Ranjith.T.

On Thu, Nov 5, 2015 at 9:27 PM, Ranjith  wrote:

BIT macro is used for defining bit location instead of shifting
operator - coding style issue

Signed-off-by: Ranjith T 
---
  drivers/staging/comedi/drivers/pcmmio.c |   42 +++
  1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/comedi/drivers/pcmmio.c 
b/drivers/staging/comedi/drivers/pcmmio.c
index 10472e6..807795c 100644
--- a/drivers/staging/comedi/drivers/pcmmio.c
+++ b/drivers/staging/comedi/drivers/pcmmio.c
@@ -84,25 +84,25 @@
  #define PCMMIO_AI_LSB_REG  0x00
  #define PCMMIO_AI_MSB_REG  0x01
  #define PCMMIO_AI_CMD_REG  0x02
-#define PCMMIO_AI_CMD_SE   (1 << 7)
-#define PCMMIO_AI_CMD_ODD_CHAN (1 << 6)
+#define PCMMIO_AI_CMD_SE   BIT(7)


The whitespace is a bit strange here as 'BIT(7)' no longer lines up with 
the surrounding lines.



+#define PCMMIO_AI_CMD_ODD_CHAN BIT(6)
  #define PCMMIO_AI_CMD_CHAN_SEL(x)  (((x) & 0x3) << 4)
  #define PCMMIO_AI_CMD_RANGE(x) (((x) & 0x3) << 2)
  #define PCMMIO_RESOURCE_REG0x02
  #define PCMMIO_RESOURCE_IRQ(x) (((x) & 0xf) << 0)
  #define PCMMIO_AI_STATUS_REG   0x03
-#define PCMMIO_AI_STATUS_DATA_READY(1 << 7)
-#define PCMMIO_AI_STATUS_DATA_DMA_PEND (1 << 6)
-#define PCMMIO_AI_STATUS_CMD_DMA_PEND  (1 << 5)
-#define PCMMIO_AI_STATUS_IRQ_PEND  (1 << 4)
-#define PCMMIO_AI_STATUS_DATA_DRQ_ENA  (1 << 2)
-#define PCMMIO_AI_STATUS_REG_SEL   (1 << 3)
-#define PCMMIO_AI_STATUS_CMD_DRQ_ENA   (1 << 1)
-#define PCMMIO_AI_STATUS_IRQ_ENA   (1 << 0)
+#define PCMMIO_AI_STATUS_DATA_READYBIT(7)
+#define PCMMIO_AI_STATUS_DATA_DMA_PEND BIT(6)
+#define PCMMIO_AI_STATUS_CMD_DMA_PEND  BIT(5)
+#define PCMMIO_AI_STATUS_IRQ_PEND  BIT(4)
+#define PCMMIO_AI_STATUS_DATA_DRQ_ENA  BIT(2)


The 'BIT(2)' also does not line up with the surrounding lines.


+#define PCMMIO_AI_STATUS_REG_SEL   BIT(3)
+#define PCMMIO_AI_STATUS_CMD_DRQ_ENA   BIT(1)
+#define PCMMIO_AI_STATUS_IRQ_ENA   BIT(0)
  #define PCMMIO_AI_RES_ENA_REG  0x03
-#define PCMMIO_AI_RES_ENA_CMD_REG_ACCESS   (0 << 3)
-#define PCMMIO_AI_RES_ENA_AI_RES_ACCESS(1 << 3)
-#define PCMMIO_AI_RES_ENA_DIO_RES_ACCESS   (1 << 4)
+#define PCMMIO_AI_RES_ENA_CMD_REG_ACCESS   0
+#define PCMMIO_AI_RES_ENA_AI_RES_ACCESSBIT(3)
+#define PCMMIO_AI_RES_ENA_DIO_RES_ACCESS   BIT(4)
  #define PCMMIO_AI_2ND_ADC_OFFSET   0x04

  #define PCMMIO_AO_LSB_REG  0x08
@@ -125,14 +125,14 @@
  #define PCMMIO_AO_CMD_CHAN_SEL(x)  (((x) & 0x03) << 1)
  #define PCMMIO_AO_CMD_CHAN_SEL_ALL (0x0f << 0)
  #define PCMMIO_AO_STATUS_REG   0x0b
-#define PCMMIO_AO_STATUS_DATA_READY(1 << 7)
-#define PCMMIO_AO_STATUS_DATA_DMA_PEND (1 << 6)
-#define PCMMIO_AO_STATUS_CMD_DMA_PEND  (1 << 5)
-#define PCMMIO_AO_STATUS_IRQ_PEND  (1 << 4)
-#define PCMMIO_AO_STATUS_DATA_DRQ_ENA  (1 << 2)
-#define PCMMIO_AO_STATUS_REG_SEL   (1 << 3)
-#define PCMMIO_AO_STATUS_CMD_DRQ_ENA   (1 << 1)
-#define PCMMIO_AO_STATUS_IRQ_ENA   (1 << 0)
+#define PCMMIO_AO_STATUS_DATA_READYBIT(7)
+#define PCMMIO_AO_STATUS_DATA_DMA_PEND BIT(6)
+#define PCMMIO_AO_STATUS_CMD_DMA_PEND  BIT(5)
+#define PCMMIO_AO_STATUS_IRQ_PEND  BIT(4)
+#define PCMMIO_AO_STATUS_DATA_DRQ_ENA  BIT(2)
+#define PCMMIO_AO_STATUS_REG_SEL   BIT(3)
+#define PCMMIO_AO_STATUS_CMD_DRQ_ENA   BIT(1)
+#define PCMMIO_AO_STATUS_IRQ_ENA   BIT(0)
  #define PCMMIO_AO_RESOURCE_ENA_REG 0x0b
  #define PCMMIO_AO_2ND_DAC_OFFSET   0x04

--
1.7.10.4




--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/3] comedi: driver: Fix - BIT macro used coding style issue

2015-11-06 Thread Ian Abbott

On 06/11/15 13:54, Ranjith T wrote:

Is this patch is fine?..Because I didn't get any reply for this patch


Your patch numbering sucks.  Isolated patches should just use [PATCH], 
not [PATCH 3/3], [PATCH 4/4], etc.


Also, could you mention the name of the COMEDI driver in the patch 
title, e.g.:


[PATCH] comedi: pcmmio: Fix coding style - use BIT macro


Thanks,
Ranjith.T.

On Thu, Nov 5, 2015 at 9:27 PM, Ranjith <ranjithec...@gmail.com> wrote:

BIT macro is used for defining bit location instead of shifting
operator - coding style issue

Signed-off-by: Ranjith T <ranjithec...@gmail.com>
---
  drivers/staging/comedi/drivers/pcmmio.c |   42 +++
  1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/comedi/drivers/pcmmio.c 
b/drivers/staging/comedi/drivers/pcmmio.c
index 10472e6..807795c 100644
--- a/drivers/staging/comedi/drivers/pcmmio.c
+++ b/drivers/staging/comedi/drivers/pcmmio.c
@@ -84,25 +84,25 @@
  #define PCMMIO_AI_LSB_REG  0x00
  #define PCMMIO_AI_MSB_REG  0x01
  #define PCMMIO_AI_CMD_REG  0x02
-#define PCMMIO_AI_CMD_SE   (1 << 7)
-#define PCMMIO_AI_CMD_ODD_CHAN (1 << 6)
+#define PCMMIO_AI_CMD_SE   BIT(7)


The whitespace is a bit strange here as 'BIT(7)' no longer lines up with 
the surrounding lines.



+#define PCMMIO_AI_CMD_ODD_CHAN BIT(6)
  #define PCMMIO_AI_CMD_CHAN_SEL(x)  (((x) & 0x3) << 4)
  #define PCMMIO_AI_CMD_RANGE(x) (((x) & 0x3) << 2)
  #define PCMMIO_RESOURCE_REG0x02
  #define PCMMIO_RESOURCE_IRQ(x) (((x) & 0xf) << 0)
  #define PCMMIO_AI_STATUS_REG   0x03
-#define PCMMIO_AI_STATUS_DATA_READY(1 << 7)
-#define PCMMIO_AI_STATUS_DATA_DMA_PEND (1 << 6)
-#define PCMMIO_AI_STATUS_CMD_DMA_PEND  (1 << 5)
-#define PCMMIO_AI_STATUS_IRQ_PEND  (1 << 4)
-#define PCMMIO_AI_STATUS_DATA_DRQ_ENA  (1 << 2)
-#define PCMMIO_AI_STATUS_REG_SEL   (1 << 3)
-#define PCMMIO_AI_STATUS_CMD_DRQ_ENA   (1 << 1)
-#define PCMMIO_AI_STATUS_IRQ_ENA   (1 << 0)
+#define PCMMIO_AI_STATUS_DATA_READYBIT(7)
+#define PCMMIO_AI_STATUS_DATA_DMA_PEND BIT(6)
+#define PCMMIO_AI_STATUS_CMD_DMA_PEND  BIT(5)
+#define PCMMIO_AI_STATUS_IRQ_PEND  BIT(4)
+#define PCMMIO_AI_STATUS_DATA_DRQ_ENA  BIT(2)


The 'BIT(2)' also does not line up with the surrounding lines.


+#define PCMMIO_AI_STATUS_REG_SEL   BIT(3)
+#define PCMMIO_AI_STATUS_CMD_DRQ_ENA   BIT(1)
+#define PCMMIO_AI_STATUS_IRQ_ENA   BIT(0)
  #define PCMMIO_AI_RES_ENA_REG  0x03
-#define PCMMIO_AI_RES_ENA_CMD_REG_ACCESS   (0 << 3)
-#define PCMMIO_AI_RES_ENA_AI_RES_ACCESS(1 << 3)
-#define PCMMIO_AI_RES_ENA_DIO_RES_ACCESS   (1 << 4)
+#define PCMMIO_AI_RES_ENA_CMD_REG_ACCESS   0
+#define PCMMIO_AI_RES_ENA_AI_RES_ACCESSBIT(3)
+#define PCMMIO_AI_RES_ENA_DIO_RES_ACCESS   BIT(4)
  #define PCMMIO_AI_2ND_ADC_OFFSET   0x04

  #define PCMMIO_AO_LSB_REG  0x08
@@ -125,14 +125,14 @@
  #define PCMMIO_AO_CMD_CHAN_SEL(x)  (((x) & 0x03) << 1)
  #define PCMMIO_AO_CMD_CHAN_SEL_ALL (0x0f << 0)
  #define PCMMIO_AO_STATUS_REG   0x0b
-#define PCMMIO_AO_STATUS_DATA_READY(1 << 7)
-#define PCMMIO_AO_STATUS_DATA_DMA_PEND (1 << 6)
-#define PCMMIO_AO_STATUS_CMD_DMA_PEND  (1 << 5)
-#define PCMMIO_AO_STATUS_IRQ_PEND  (1 << 4)
-#define PCMMIO_AO_STATUS_DATA_DRQ_ENA  (1 << 2)
-#define PCMMIO_AO_STATUS_REG_SEL   (1 << 3)
-#define PCMMIO_AO_STATUS_CMD_DRQ_ENA   (1 << 1)
-#define PCMMIO_AO_STATUS_IRQ_ENA   (1 << 0)
+#define PCMMIO_AO_STATUS_DATA_READYBIT(7)
+#define PCMMIO_AO_STATUS_DATA_DMA_PEND BIT(6)
+#define PCMMIO_AO_STATUS_CMD_DMA_PEND  BIT(5)
+#define PCMMIO_AO_STATUS_IRQ_PEND  BIT(4)
+#define PCMMIO_AO_STATUS_DATA_DRQ_ENA  BIT(2)
+#define PCMMIO_AO_STATUS_REG_SEL   BIT(3)
+#define PCMMIO_AO_STATUS_CMD_DRQ_ENA   BIT(1)
+#define PCMMIO_AO_STATUS_IRQ_ENA   BIT(0)
  #define PCMMIO_AO_RESOURCE_ENA_REG 0x0b
  #define PCMMIO_AO_2ND_DAC_OFFSET   0x04

--
1.7.10.4




--
-=( Ian Abbott @ MEV Ltd.E-mail: <abbo...@mev.co.uk> )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] comedi: drivers: Fix - BIT macro used coding style issue

2015-11-02 Thread Ian Abbott

On 02/11/15 14:25, ranjithec...@gmail.com wrote:

From: Ranjith 

BIT macro is used for defining BIT location instead of shifting
operator - coding style issue

Signed-off-by: Ranjith 
---
  drivers/staging/comedi/drivers/addi_apci_1032.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1032.c 
b/drivers/staging/comedi/drivers/addi_apci_1032.c
index fd5ce21..168602b 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1032.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1032.c
@@ -84,7 +84,7 @@
  #define APCI1032_MODE2_REG0x08
  #define APCI1032_STATUS_REG   0x0c
  #define APCI1032_CTRL_REG 0x10
-#define APCI1032_CTRL_INT_OR   (0 << 1)
+#define APCI1032_CTRL_INT_OR   BIT(0)
  #define APCI1032_CTRL_INT_AND BIT(1)
  #define APCI1032_CTRL_INT_ENA BIT(2)


No, that's wrong.  (0 << 1) is 0, but BIT(0) is 1.

Hartley already fixed the coding style issue.  It's in linux-next.

--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] comedi: drivers: Fix - BIT macro used coding style issue

2015-11-02 Thread Ian Abbott

On 02/11/15 14:25, ranjithec...@gmail.com wrote:

From: Ranjith 

BIT macro is used for defining bit location instead of shifting
operator -  coding style issue

Signed-off-by: Ranjith T 
---
  drivers/staging/comedi/drivers/addi_apci_1032.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1032.c 
b/drivers/staging/comedi/drivers/addi_apci_1032.c
index b37166d..fd5ce21 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1032.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1032.c
@@ -85,8 +85,8 @@
  #define APCI1032_STATUS_REG   0x0c
  #define APCI1032_CTRL_REG 0x10
  #define APCI1032_CTRL_INT_OR  (0 << 1)
-#define APCI1032_CTRL_INT_AND  (1 << 1)
-#define APCI1032_CTRL_INT_ENA  (1 << 2)
+#define APCI1032_CTRL_INT_AND  BIT(1)
+#define APCI1032_CTRL_INT_ENA  BIT(2)

  struct apci1032_private {
unsigned long amcc_iobase;  /* base of AMCC I/O registers */



Hartley already fixed this coding style issue.  It's in linux-next.

--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] comedi: drivers: Fix - BIT macro used coding style issue

2015-11-02 Thread Ian Abbott

On 02/11/15 14:25, ranjithec...@gmail.com wrote:

From: Ranjith <ranjithec...@gmail.com>

BIT macro is used for defining bit location instead of shifting
operator -  coding style issue

Signed-off-by: Ranjith T <ranjithec...@gmail.com>
---
  drivers/staging/comedi/drivers/addi_apci_1032.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1032.c 
b/drivers/staging/comedi/drivers/addi_apci_1032.c
index b37166d..fd5ce21 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1032.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1032.c
@@ -85,8 +85,8 @@
  #define APCI1032_STATUS_REG   0x0c
  #define APCI1032_CTRL_REG 0x10
  #define APCI1032_CTRL_INT_OR  (0 << 1)
-#define APCI1032_CTRL_INT_AND  (1 << 1)
-#define APCI1032_CTRL_INT_ENA  (1 << 2)
+#define APCI1032_CTRL_INT_AND  BIT(1)
+#define APCI1032_CTRL_INT_ENA  BIT(2)

  struct apci1032_private {
unsigned long amcc_iobase;  /* base of AMCC I/O registers */



Hartley already fixed this coding style issue.  It's in linux-next.

--
-=( Ian Abbott @ MEV Ltd.E-mail: <abbo...@mev.co.uk> )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] comedi: drivers: Fix - BIT macro used coding style issue

2015-11-02 Thread Ian Abbott

On 02/11/15 14:25, ranjithec...@gmail.com wrote:

From: Ranjith <ranjithec...@gmail.com>

BIT macro is used for defining BIT location instead of shifting
operator - coding style issue

Signed-off-by: Ranjith <ranjithec...@gmail.com>
---
  drivers/staging/comedi/drivers/addi_apci_1032.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1032.c 
b/drivers/staging/comedi/drivers/addi_apci_1032.c
index fd5ce21..168602b 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1032.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1032.c
@@ -84,7 +84,7 @@
  #define APCI1032_MODE2_REG0x08
  #define APCI1032_STATUS_REG   0x0c
  #define APCI1032_CTRL_REG 0x10
-#define APCI1032_CTRL_INT_OR   (0 << 1)
+#define APCI1032_CTRL_INT_OR   BIT(0)
  #define APCI1032_CTRL_INT_AND BIT(1)
  #define APCI1032_CTRL_INT_ENA BIT(2)


No, that's wrong.  (0 << 1) is 0, but BIT(0) is 1.

Hartley already fixed the coding style issue.  It's in linux-next.

--
-=( Ian Abbott @ MEV Ltd.E-mail: <abbo...@mev.co.uk> )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 01/16] staging: comedi: comedi_test: reformat multi-line comments

2015-10-27 Thread Ian Abbott
Use the preferred style for multi-line comments.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 94 ++--
 1 file changed, 47 insertions(+), 47 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 80d613c..899faf7 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -1,49 +1,49 @@
 /*
-comedi/drivers/comedi_test.c
+ * comedi/drivers/comedi_test.c
+ *
+ * Generates fake waveform signals that can be read through
+ * the command interface.  It does _not_ read from any board;
+ * it just generates deterministic waveforms.
+ * Useful for various testing purposes.
+ *
+ * Copyright (C) 2002 Joachim Wuttke 
+ * Copyright (C) 2002 Frank Mori Hess 
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 2000 David A. Schleef 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
 
-Generates fake waveform signals that can be read through
-the command interface.  It does _not_ read from any board;
-it just generates deterministic waveforms.
-Useful for various testing purposes.
-
-Copyright (C) 2002 Joachim Wuttke 
-Copyright (C) 2002 Frank Mori Hess 
-
-COMEDI - Linux Control and Measurement Device Interface
-Copyright (C) 2000 David A. Schleef 
-
-This program is free software; you can redistribute it and/or modify
-it under the terms of the GNU General Public License as published by
-the Free Software Foundation; either version 2 of the License, or
-(at your option) any later version.
-
-This program is distributed in the hope that it will be useful,
-but WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
-*/
 /*
-Driver: comedi_test
-Description: generates fake waveforms
-Author: Joachim Wuttke , Frank Mori Hess
-  , ds
-Devices:
-Status: works
-Updated: Sat, 16 Mar 2002 17:34:48 -0800
-
-This driver is mainly for testing purposes, but can also be used to
-generate sample waveforms on systems that don't have data acquisition
-hardware.
-
-Configuration options:
-  [0] - Amplitude in microvolts for fake waveforms (default 1 volt)
-  [1] - Period in microseconds for fake waveforms (default 0.1 sec)
-
-Generates a sawtooth wave on channel 0, square wave on channel 1, additional
-waveforms could be added to other channels (currently they return flatline
-zero volts).
-
-*/
+ * Driver: comedi_test
+ * Description: generates fake waveforms
+ * Author: Joachim Wuttke , Frank Mori Hess
+ *   , ds
+ * Devices:
+ * Status: works
+ * Updated: Sat, 16 Mar 2002 17:34:48 -0800
+ *
+ * This driver is mainly for testing purposes, but can also be used to
+ * generate sample waveforms on systems that don't have data acquisition
+ * hardware.
+ *
+ * Configuration options:
+ *   [0] - Amplitude in microvolts for fake waveforms (default 1 volt)
+ *   [1] - Period in microseconds for fake waveforms (default 0.1 sec)
+ *
+ * Generates a sawtooth wave on channel 0, square wave on channel 1, additional
+ * waveforms could be added to other channels (currently they return flatline
+ * zero volts).
+ */
 
 #include 
 #include "../comedidev.h"
@@ -160,10 +160,10 @@ static unsigned short fake_waveform(struct comedi_device 
*dev,
 }
 
 /*
-   This is the background routine used to generate arbitrary data.
-   It should run in the background; therefore it is scheduled by
-   a timer mechanism.
-*/
+ * This is the background routine used to generate arbitrary data.
+ * It should run in the background; therefore it is scheduled by
+ * a timer mechanism.
+ */
 static void waveform_ai_interrupt(unsigned long arg)
 {
struct comedi_device *dev = (struct comedi_device *)arg;
-- 
2.6.1

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


[PATCH 07/16] staging: comedi: comedi_test: use unsigned int for waveform timing

2015-10-27 Thread Ian Abbott
Use `unsigned int` instead of `unsigned long` to hold the period of the
fake waveform generator and the current time within each waveform.  The
waveform period will be no more than `INT_MAX` and the current time
within the waveform (prior to the modulo operation to bring it actually
within the waveform period) will be no more than `INT_MAX + UINT_MAX /
1000`.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 0215228..158e090 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -64,8 +64,8 @@ struct waveform_private {
struct timer_list timer;
ktime_t last;   /* time last timer interrupt occurred */
unsigned int uvolt_amplitude;   /* waveform amplitude in microvolts */
-   unsigned long usec_period;  /* waveform period in microseconds */
-   unsigned long usec_current; /* current time (mod waveform period) */
+   unsigned int usec_period;   /* waveform period in microseconds */
+   unsigned int usec_current;  /* current time (mod waveform period) */
unsigned long usec_remainder;   /* usec since last scan */
unsigned long state_bits;
unsigned int scan_period;   /* scan period in usec */
@@ -83,7 +83,7 @@ static const struct comedi_lrange waveform_ai_ranges = {
 
 static unsigned short fake_sawtooth(struct comedi_device *dev,
unsigned int range_index,
-   unsigned long current_time)
+   unsigned int current_time)
 {
struct waveform_private *devpriv = dev->private;
struct comedi_subdevice *s = dev->read_subdev;
@@ -115,7 +115,7 @@ static unsigned short fake_sawtooth(struct comedi_device 
*dev,
 
 static unsigned short fake_squarewave(struct comedi_device *dev,
  unsigned int range_index,
- unsigned long current_time)
+ unsigned int current_time)
 {
struct waveform_private *devpriv = dev->private;
struct comedi_subdevice *s = dev->read_subdev;
@@ -145,7 +145,7 @@ static unsigned short fake_squarewave(struct comedi_device 
*dev,
 
 static unsigned short fake_flatline(struct comedi_device *dev,
unsigned int range_index,
-   unsigned long current_time)
+   unsigned int current_time)
 {
return dev->read_subdev->maxdata / 2;
 }
@@ -153,7 +153,7 @@ static unsigned short fake_flatline(struct comedi_device 
*dev,
 /* generates a different waveform depending on what channel is read */
 static unsigned short fake_waveform(struct comedi_device *dev,
unsigned int channel, unsigned int range,
-   unsigned long current_time)
+   unsigned int current_time)
 {
enum {
SAWTOOTH_CHAN,
@@ -468,7 +468,7 @@ static int waveform_attach(struct comedi_device *dev,
(unsigned long)dev);
 
dev_info(dev->class_dev,
-"%s: %i microvolt, %li microsecond waveform attached\n",
+"%s: %u microvolt, %u microsecond waveform attached\n",
 dev->board_name,
 devpriv->uvolt_amplitude, devpriv->usec_period);
 
-- 
2.6.1

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


[PATCH 06/16] staging: comedi: comedi_test: move modulo operations for waveform

2015-10-27 Thread Ian Abbott
The fake waveform generator functions, `fake_sawtooth()` and
`fake_squarewave()`, called from `fake_waveform()`, have a
`current_time` parameter which is the time since the start of a waveform
period.  The parameter value may be greater than the waveform period so
they do a modulo operation to bring it into range.  Do the modulo
operations outside the functions in `waveform_ai_interrupt()` so that
the waveform generator functions always get a `current_time` parameter
less than the waveform period and do not have to do the modulo operation
themselves.  Also, only do the modulo operations when the time since the
start of a waveform exceeds the waveform period.  Usually, several
samples are produced in each waveform period and modulo operations are
typically more expensive than a simple comparison.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index cc35bd6..0215228 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -97,7 +97,6 @@ static unsigned short fake_sawtooth(struct comedi_device *dev,
binary_amplitude *= devpriv->uvolt_amplitude;
do_div(binary_amplitude, krange->max - krange->min);
 
-   current_time %= devpriv->usec_period;
value = current_time;
value *= binary_amplitude * 2;
do_div(value, devpriv->usec_period);
@@ -125,7 +124,6 @@ static unsigned short fake_squarewave(struct comedi_device 
*dev,
const struct comedi_krange *krange =
>range_table->range[range_index];
 
-   current_time %= devpriv->usec_period;
value = s->maxdata;
value *= devpriv->uvolt_amplitude;
do_div(value, krange->max - krange->min);
@@ -206,20 +204,24 @@ static void waveform_ai_interrupt(unsigned long arg)
 
num_scans = comedi_nscans_left(s, num_scans);
for (i = 0; i < num_scans; i++) {
+   unsigned long scan_remain_period = devpriv->scan_period;
+
for (j = 0; j < cmd->chanlist_len; j++) {
unsigned short sample;
 
+   if (devpriv->usec_current >= devpriv->usec_period)
+   devpriv->usec_current %= devpriv->usec_period;
sample = fake_waveform(dev, CR_CHAN(cmd->chanlist[j]),
   CR_RANGE(cmd->chanlist[j]),
-  devpriv->usec_current +
-  i * devpriv->scan_period +
-  j * devpriv->convert_period);
+  devpriv->usec_current);
comedi_buf_write_samples(s, , 1);
+   devpriv->usec_current += devpriv->convert_period;
+   scan_remain_period -= devpriv->convert_period;
}
+   devpriv->usec_current += scan_remain_period;
}
-
-   devpriv->usec_current += elapsed_time;
-   devpriv->usec_current %= devpriv->usec_period;
+   if (devpriv->usec_current >= devpriv->usec_period)
+   devpriv->usec_current %= devpriv->usec_period;
 
if (cmd->stop_src == TRIG_COUNT && async->scans_done >= cmd->stop_arg)
async->events |= COMEDI_CB_EOA;
-- 
2.6.1

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


[PATCH 05/16] staging: comedi: comedi_test: support scan_begin_src == TRIG_FOLLOW

2015-10-27 Thread Ian Abbott
It is quite common for COMEDI subdevices that support commands to
support setting `scan_begin_src` to `TRIG_FOLLOW`.  This means the next
scan begins once all conversions in the current scan are complete.
Support the following timing combinations for the AI subdevice:

  scan_begin_src == TRIG_TIMER && convert_src == TRIG_TIMER
  scan_begin_src == TRIG_TIMER && convert_src == TRIG_NOW
  scan_begin_src == TRIG_FOLLOW && convert_src == TRIG_TIMER

The actual scan period in microseconds is stored in the `scan_period`
member of the private data structure `struct waveform_private`.  An
`unsigned int` is still wide enough, because the conversion period is no
more than `UINT_MAX / 1000` microseconds and the number of conversions
is no more than 16 (`N_CHANS * 2`).

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 64 +++-
 1 file changed, 44 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index f011fbd..cc35bd6 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -239,7 +239,8 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
/* Step 1 : check if triggers are trivially valid */
 
err |= comedi_check_trigger_src(>start_src, TRIG_NOW);
-   err |= comedi_check_trigger_src(>scan_begin_src, TRIG_TIMER);
+   err |= comedi_check_trigger_src(>scan_begin_src,
+   TRIG_FOLLOW | TRIG_TIMER);
err |= comedi_check_trigger_src(>convert_src,
TRIG_NOW | TRIG_TIMER);
err |= comedi_check_trigger_src(>scan_end_src, TRIG_COUNT);
@@ -255,6 +256,9 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
 
/* Step 2b : and mutually compatible */
 
+   if (cmd->scan_begin_src == TRIG_FOLLOW && cmd->convert_src == TRIG_NOW)
+   err |= -EINVAL; /* scan period would be 0 */
+
if (err)
return 2;
 
@@ -262,11 +266,21 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
 
err |= comedi_check_trigger_arg_is(>start_arg, 0);
 
-   if (cmd->convert_src == TRIG_NOW)
+   if (cmd->convert_src == TRIG_NOW) {
err |= comedi_check_trigger_arg_is(>convert_arg, 0);
+   } else {/* cmd->convert_src == TRIG_TIMER */
+   if (cmd->scan_begin_src == TRIG_FOLLOW) {
+   err |= comedi_check_trigger_arg_min(>convert_arg,
+   NSEC_PER_USEC);
+   }
+   }
 
-   err |= comedi_check_trigger_arg_min(>scan_begin_arg,
-   NSEC_PER_USEC);
+   if (cmd->scan_begin_src == TRIG_FOLLOW) {
+   err |= comedi_check_trigger_arg_is(>scan_begin_arg, 0);
+   } else {/* cmd->scan_begin_src == TRIG_TIMER */
+   err |= comedi_check_trigger_arg_min(>scan_begin_arg,
+   NSEC_PER_USEC);
+   }
 
err |= comedi_check_trigger_arg_min(>chanlist_len, 1);
err |= comedi_check_trigger_arg_is(>scan_end_arg,
@@ -274,7 +288,7 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
 
if (cmd->stop_src == TRIG_COUNT)
err |= comedi_check_trigger_arg_min(>stop_arg, 1);
-   else/* TRIG_NONE */
+   else/* cmd->stop_src == TRIG_NONE */
err |= comedi_check_trigger_arg_is(>stop_arg, 0);
 
if (err)
@@ -288,22 +302,27 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
arg = min(arg,
  rounddown(UINT_MAX, (unsigned int)NSEC_PER_USEC));
arg = NSEC_PER_USEC * DIV_ROUND_CLOSEST(arg, NSEC_PER_USEC);
-   /* limit convert_arg to keep scan_begin_arg in range */
-   limit = UINT_MAX / cmd->scan_end_arg;
-   limit = rounddown(limit, (unsigned int)NSEC_PER_SEC);
-   arg = min(arg, limit);
+   if (cmd->scan_begin_arg == TRIG_TIMER) {
+   /* limit convert_arg to keep scan_begin_arg in range */
+   limit = UINT_MAX / cmd->scan_end_arg;
+   limit = rounddown(limit, (unsigned int)NSEC_PER_SEC);
+   arg = min(arg, limit);
+   }
err |= comedi_check_trigger_arg_is(>convert_arg, arg);
}
 
-   /* round scan_begin_arg to nearest microsecond */
-   arg = cmd->scan_begin_arg;
-   arg = min(arg, rounddown(UINT_MAX, (unsigned int)NSEC_PER_USEC));
-   arg = NSEC_PER_USEC * DIV_ROUND_CLOSEST(arg, NSEC_PER_USEC);
-   if (cmd->convert_src == TRIG_TIMER) {
-   /* but ensure scan_begin_arg is large 

[PATCH 04/16] staging: comedi: comedi_test: limit maximum convert_arg

2015-10-27 Thread Ian Abbott
When testing the parameters for setting up an asynchronous command on
the AI subdevice, limit the maximum conversion period
(`cmd->convert_arg`) so that the number of conversions in a scan
(`cmd->scan_end_arg`, same as `cmd->chanlist_len`) multiplied by the
conversion period fits within an `unsigned int`, as that is used to
limit the minimum scan period (`cmd->scan_begin_arg`).

Also ensure rounding of the conversion period and scan period to the
nearest microsecond both fit in an `unsigned int`.  Do all this in stage
4 ("fix up any arguments") of the command testing.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 42 ++--
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index d9810ca..f011fbd 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -234,7 +234,7 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
   struct comedi_cmd *cmd)
 {
int err = 0;
-   unsigned int arg;
+   unsigned int arg, limit;
 
/* Step 1 : check if triggers are trivially valid */
 
@@ -265,16 +265,8 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
if (cmd->convert_src == TRIG_NOW)
err |= comedi_check_trigger_arg_is(>convert_arg, 0);
 
-   if (cmd->scan_begin_src == TRIG_TIMER) {
-   err |= comedi_check_trigger_arg_min(>scan_begin_arg,
-   NSEC_PER_USEC);
-   if (cmd->convert_src == TRIG_TIMER) {
-   err |= comedi_check_trigger_arg_min(>
-   scan_begin_arg,
-   cmd->convert_arg *
-   cmd->chanlist_len);
-   }
-   }
+   err |= comedi_check_trigger_arg_min(>scan_begin_arg,
+   NSEC_PER_USEC);
 
err |= comedi_check_trigger_arg_min(>chanlist_len, 1);
err |= comedi_check_trigger_arg_is(>scan_end_arg,
@@ -290,21 +282,29 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
 
/* step 4: fix up any arguments */
 
-   if (cmd->scan_begin_src == TRIG_TIMER) {
-   arg = cmd->scan_begin_arg;
-   /* round to nearest microsec */
-   arg = NSEC_PER_USEC *
- ((arg + (NSEC_PER_USEC / 2)) / NSEC_PER_USEC);
-   err |= comedi_check_trigger_arg_is(>scan_begin_arg, arg);
-   }
if (cmd->convert_src == TRIG_TIMER) {
+   /* round convert_arg to nearest microsecond */
arg = cmd->convert_arg;
-   /* round to nearest microsec */
-   arg = NSEC_PER_USEC *
- ((arg + (NSEC_PER_USEC / 2)) / NSEC_PER_USEC);
+   arg = min(arg,
+ rounddown(UINT_MAX, (unsigned int)NSEC_PER_USEC));
+   arg = NSEC_PER_USEC * DIV_ROUND_CLOSEST(arg, NSEC_PER_USEC);
+   /* limit convert_arg to keep scan_begin_arg in range */
+   limit = UINT_MAX / cmd->scan_end_arg;
+   limit = rounddown(limit, (unsigned int)NSEC_PER_SEC);
+   arg = min(arg, limit);
err |= comedi_check_trigger_arg_is(>convert_arg, arg);
}
 
+   /* round scan_begin_arg to nearest microsecond */
+   arg = cmd->scan_begin_arg;
+   arg = min(arg, rounddown(UINT_MAX, (unsigned int)NSEC_PER_USEC));
+   arg = NSEC_PER_USEC * DIV_ROUND_CLOSEST(arg, NSEC_PER_USEC);
+   if (cmd->convert_src == TRIG_TIMER) {
+   /* but ensure scan_begin_arg is large enough */
+   arg = max(arg, cmd->convert_arg * cmd->scan_end_arg);
+   }
+   err |= comedi_check_trigger_arg_is(>scan_begin_arg, arg);
+
if (err)
return 4;
 
-- 
2.6.1

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


[PATCH 09/16] staging: comedi: comedi_test: rename members for AI commands

2015-10-27 Thread Ian Abbott
Rename the members of `struct waveform_private` that are used to handle
AI commands, apart from those members used to control fake waveform
generation.  The renames are `timer` --> `ai_timer`, `scan_period` -->
`ai_scan_period`, and `convert_period` --> `ai_convert_period`.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 38 ++--
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 78fde3a..8e618ea 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -61,14 +61,14 @@ enum waveform_state_bits {
 
 /* Data unique to this driver */
 struct waveform_private {
-   struct timer_list timer;
+   struct timer_list ai_timer; /* timer for AI commands */
u64 ai_last_scan_time;  /* time of last AI scan in usec */
unsigned int uvolt_amplitude;   /* waveform amplitude in microvolts */
unsigned int usec_period;   /* waveform period in microseconds */
unsigned int usec_current;  /* current time (mod waveform period) */
unsigned long state_bits;
-   unsigned int scan_period;   /* scan period in usec */
-   unsigned int convert_period;/* conversion period in usec */
+   unsigned int ai_scan_period;/* AI scan period in usec */
+   unsigned int ai_convert_period; /* AI conversion period in usec */
unsigned int ao_loopbacks[N_CHANS];
 };
 
@@ -191,11 +191,11 @@ static void waveform_ai_interrupt(unsigned long arg)
return;
 
elapsed_time = ktime_to_us(ktime_get()) - devpriv->ai_last_scan_time;
-   num_scans = elapsed_time / devpriv->scan_period;
+   num_scans = elapsed_time / devpriv->ai_scan_period;
 
num_scans = comedi_nscans_left(s, num_scans);
for (i = 0; i < num_scans; i++) {
-   unsigned int scan_remain_period = devpriv->scan_period;
+   unsigned int scan_remain_period = devpriv->ai_scan_period;
 
for (j = 0; j < cmd->chanlist_len; j++) {
unsigned short sample;
@@ -206,11 +206,11 @@ static void waveform_ai_interrupt(unsigned long arg)
   CR_RANGE(cmd->chanlist[j]),
   devpriv->usec_current);
comedi_buf_write_samples(s, , 1);
-   devpriv->usec_current += devpriv->convert_period;
-   scan_remain_period -= devpriv->convert_period;
+   devpriv->usec_current += devpriv->ai_convert_period;
+   scan_remain_period -= devpriv->ai_convert_period;
}
devpriv->usec_current += scan_remain_period;
-   devpriv->ai_last_scan_time += devpriv->scan_period;
+   devpriv->ai_last_scan_time += devpriv->ai_scan_period;
}
if (devpriv->usec_current >= devpriv->usec_period)
devpriv->usec_current %= devpriv->usec_period;
@@ -218,7 +218,7 @@ static void waveform_ai_interrupt(unsigned long arg)
if (cmd->stop_src == TRIG_COUNT && async->scans_done >= cmd->stop_arg)
async->events |= COMEDI_CB_EOA;
else
-   mod_timer(>timer, jiffies + 1);
+   mod_timer(>ai_timer, jiffies + 1);
 
comedi_handle_events(dev, s);
 }
@@ -338,15 +338,15 @@ static int waveform_ai_cmd(struct comedi_device *dev,
}
 
if (cmd->convert_src == TRIG_NOW)
-   devpriv->convert_period = 0;
+   devpriv->ai_convert_period = 0;
else/* cmd->convert_src == TRIG_TIMER */
-   devpriv->convert_period = cmd->convert_arg / NSEC_PER_USEC;
+   devpriv->ai_convert_period = cmd->convert_arg / NSEC_PER_USEC;
 
if (cmd->scan_begin_src == TRIG_FOLLOW) {
-   devpriv->scan_period = devpriv->convert_period *
-  cmd->scan_end_arg;
+   devpriv->ai_scan_period = devpriv->ai_convert_period *
+ cmd->scan_end_arg;
} else {/* cmd->scan_begin_src == TRIG_TIMER */
-   devpriv->scan_period = cmd->scan_begin_arg / NSEC_PER_USEC;
+   devpriv->ai_scan_period = cmd->scan_begin_arg / NSEC_PER_USEC;
}
 
devpriv->ai_last_scan_time = ktime_to_us(ktime_get());
@@ -354,12 +354,12 @@ static int waveform_ai_cmd(struct comedi_device *dev,
usec_current = devpriv->ai_last_scan_time;
devpriv->usec_current = do_div(usec_current, devpriv->usec_period);
 
-   devpriv->timer.expires = jiffies + 1;
+  

[PATCH 08/16] staging: comedi: comedi_test: simplify time since last AI scan

2015-10-27 Thread Ian Abbott
The private data structure `struct waveform_private` currently uses
member `last` to remember the time of the last timer interrupt, and the
member `usec_remainder` to keep track of how far into a simulated scan
the interrupt occurred.  Replace these with a single new member
`ai_last_scan_time` that records the time of the last scan.  This
simplifies the calculation of the number of scans to simulate in the
timer routine, `waveform_ai_interrupt()`.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 27 ++-
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 158e090..78fde3a 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -62,11 +62,10 @@ enum waveform_state_bits {
 /* Data unique to this driver */
 struct waveform_private {
struct timer_list timer;
-   ktime_t last;   /* time last timer interrupt occurred */
+   u64 ai_last_scan_time;  /* time of last AI scan in usec */
unsigned int uvolt_amplitude;   /* waveform amplitude in microvolts */
unsigned int usec_period;   /* waveform period in microseconds */
unsigned int usec_current;  /* current time (mod waveform period) */
-   unsigned long usec_remainder;   /* usec since last scan */
unsigned long state_bits;
unsigned int scan_period;   /* scan period in usec */
unsigned int convert_period;/* conversion period in usec */
@@ -184,27 +183,19 @@ static void waveform_ai_interrupt(unsigned long arg)
struct comedi_async *async = s->async;
struct comedi_cmd *cmd = >cmd;
unsigned int i, j;
-   /* all times in microsec */
unsigned long elapsed_time;
unsigned int num_scans;
-   ktime_t now;
 
/* check command is still active */
if (!test_bit(WAVEFORM_AI_RUNNING, >state_bits))
return;
 
-   now = ktime_get();
-
-   elapsed_time = ktime_to_us(ktime_sub(now, devpriv->last));
-   devpriv->last = now;
-   num_scans =
-   (devpriv->usec_remainder + elapsed_time) / devpriv->scan_period;
-   devpriv->usec_remainder =
-   (devpriv->usec_remainder + elapsed_time) % devpriv->scan_period;
+   elapsed_time = ktime_to_us(ktime_get()) - devpriv->ai_last_scan_time;
+   num_scans = elapsed_time / devpriv->scan_period;
 
num_scans = comedi_nscans_left(s, num_scans);
for (i = 0; i < num_scans; i++) {
-   unsigned long scan_remain_period = devpriv->scan_period;
+   unsigned int scan_remain_period = devpriv->scan_period;
 
for (j = 0; j < cmd->chanlist_len; j++) {
unsigned short sample;
@@ -219,6 +210,7 @@ static void waveform_ai_interrupt(unsigned long arg)
scan_remain_period -= devpriv->convert_period;
}
devpriv->usec_current += scan_remain_period;
+   devpriv->ai_last_scan_time += devpriv->scan_period;
}
if (devpriv->usec_current >= devpriv->usec_period)
devpriv->usec_current %= devpriv->usec_period;
@@ -337,6 +329,7 @@ static int waveform_ai_cmd(struct comedi_device *dev,
 {
struct waveform_private *devpriv = dev->private;
struct comedi_cmd *cmd = >async->cmd;
+   u64 usec_current;
 
if (cmd->flags & CMDF_PRIORITY) {
dev_err(dev->class_dev,
@@ -356,10 +349,10 @@ static int waveform_ai_cmd(struct comedi_device *dev,
devpriv->scan_period = cmd->scan_begin_arg / NSEC_PER_USEC;
}
 
-   devpriv->last = ktime_get();
-   devpriv->usec_current =
-   ((u32)ktime_to_us(devpriv->last)) % devpriv->usec_period;
-   devpriv->usec_remainder = 0;
+   devpriv->ai_last_scan_time = ktime_to_us(ktime_get());
+   /* Determine time within waveform period. */
+   usec_current = devpriv->ai_last_scan_time;
+   devpriv->usec_current = do_div(usec_current, devpriv->usec_period);
 
devpriv->timer.expires = jiffies + 1;
/* mark command as active */
-- 
2.6.1

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


[PATCH 11/16] staging: comedi: comedi_test: make timer rate similar to scan rate

2015-10-27 Thread Ian Abbott
The asynchronous command handling for the analog input subdevice uses a
kernel timer which expires approximately `HZ` times a second.  However,
it only needs to do anything after each scan period.  Set the timer to
expire just after the next scan period.

Although the timer expiry function `waveform_ai_interrupt()` uses
precise time values to generate the fake waveforms used to generate the
data, those time values are constructed in a precise sequence, and do
not depend on the time the timer expiry function is actually called.  So
the timer expiry rate does not have to be very precise.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 1b3ad7f..9655dc3 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -52,6 +52,7 @@
 
 #include 
 #include 
+#include 
 
 #define N_CHANS 8
 
@@ -215,10 +216,12 @@ static void waveform_ai_interrupt(unsigned long arg)
if (devpriv->wf_current >= devpriv->wf_period)
devpriv->wf_current %= devpriv->wf_period;
 
-   if (cmd->stop_src == TRIG_COUNT && async->scans_done >= cmd->stop_arg)
+   if (cmd->stop_src == TRIG_COUNT && async->scans_done >= cmd->stop_arg) {
async->events |= COMEDI_CB_EOA;
-   else
-   mod_timer(>ai_timer, jiffies + 1);
+   } else {
+   mod_timer(>ai_timer,
+ jiffies + usecs_to_jiffies(devpriv->ai_scan_period));
+   }
 
comedi_handle_events(dev, s);
 }
@@ -354,7 +357,9 @@ static int waveform_ai_cmd(struct comedi_device *dev,
wf_current = devpriv->ai_last_scan_time;
devpriv->wf_current = do_div(wf_current, devpriv->wf_period);
 
-   devpriv->ai_timer.expires = jiffies + 1;
+   devpriv->ai_timer.expires =
+   jiffies + usecs_to_jiffies(devpriv->ai_scan_period);
+
/* mark command as active */
smp_mb__before_atomic();
set_bit(WAVEFORM_AI_RUNNING, >state_bits);
-- 
2.6.1

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


[PATCH 12/16] staging: comedi: comedi_test: use unsigned short for loopback values

2015-10-27 Thread Ian Abbott
The last sample values written to the AO subdevice channels by its
"insn_write" handler `waveform_ao_insn_write()` are stored in the member
array `ao_loopbacks[]` in the device private data `struct
waveform_private`.  They can be read back via the "insn_read" handler of
the AI subdevice `waveform_ai_insn_read()`.  As the stored sample values
are only 16 bits wide, change the type of the `ao_loopbacks[]` member to
`unsigned short` to save some space.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 9655dc3..a750f84 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -70,7 +70,7 @@ struct waveform_private {
unsigned long state_bits;
unsigned int ai_scan_period;/* AI scan period in usec */
unsigned int ai_convert_period; /* AI conversion period in usec */
-   unsigned int ao_loopbacks[N_CHANS];
+   unsigned short ao_loopbacks[N_CHANS];
 };
 
 /* fake analog input ranges */
-- 
2.6.1

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


[PATCH 10/16] staging: comedi: comedi_test: rename waveform members

2015-10-27 Thread Ian Abbott
Rename the members `struct waveform_private` associated with fake
waveform generation.  The affected members are `uvolt_amplitude` -->
`wf_amplitude` (the amplitude of the waveform), `usec_period` -->
`wf_period` (the period of the waveform), and `usec_current` -->
`wf_current` (the current time within a waveform period).

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 40 ++--
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 8e618ea..1b3ad7f 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -63,9 +63,9 @@ enum waveform_state_bits {
 struct waveform_private {
struct timer_list ai_timer; /* timer for AI commands */
u64 ai_last_scan_time;  /* time of last AI scan in usec */
-   unsigned int uvolt_amplitude;   /* waveform amplitude in microvolts */
-   unsigned int usec_period;   /* waveform period in microseconds */
-   unsigned int usec_current;  /* current time (mod waveform period) */
+   unsigned int wf_amplitude;  /* waveform amplitude in microvolts */
+   unsigned int wf_period; /* waveform period in microseconds */
+   unsigned int wf_current;/* current time in waveform period */
unsigned long state_bits;
unsigned int ai_scan_period;/* AI scan period in usec */
unsigned int ai_convert_period; /* AI conversion period in usec */
@@ -93,12 +93,12 @@ static unsigned short fake_sawtooth(struct comedi_device 
*dev,
u64 binary_amplitude;
 
binary_amplitude = s->maxdata;
-   binary_amplitude *= devpriv->uvolt_amplitude;
+   binary_amplitude *= devpriv->wf_amplitude;
do_div(binary_amplitude, krange->max - krange->min);
 
value = current_time;
value *= binary_amplitude * 2;
-   do_div(value, devpriv->usec_period);
+   do_div(value, devpriv->wf_period);
value += offset;
/* get rid of sawtooth's dc offset and clamp value */
if (value < binary_amplitude) {
@@ -124,11 +124,11 @@ static unsigned short fake_squarewave(struct 
comedi_device *dev,
>range_table->range[range_index];
 
value = s->maxdata;
-   value *= devpriv->uvolt_amplitude;
+   value *= devpriv->wf_amplitude;
do_div(value, krange->max - krange->min);
 
/* get one of two values for square-wave and clamp */
-   if (current_time < devpriv->usec_period / 2) {
+   if (current_time < devpriv->wf_period / 2) {
if (offset < value)
value = 0;  /* negative saturation */
else
@@ -200,20 +200,20 @@ static void waveform_ai_interrupt(unsigned long arg)
for (j = 0; j < cmd->chanlist_len; j++) {
unsigned short sample;
 
-   if (devpriv->usec_current >= devpriv->usec_period)
-   devpriv->usec_current %= devpriv->usec_period;
+   if (devpriv->wf_current >= devpriv->wf_period)
+   devpriv->wf_current %= devpriv->wf_period;
sample = fake_waveform(dev, CR_CHAN(cmd->chanlist[j]),
   CR_RANGE(cmd->chanlist[j]),
-  devpriv->usec_current);
+  devpriv->wf_current);
comedi_buf_write_samples(s, , 1);
-   devpriv->usec_current += devpriv->ai_convert_period;
+   devpriv->wf_current += devpriv->ai_convert_period;
scan_remain_period -= devpriv->ai_convert_period;
}
-   devpriv->usec_current += scan_remain_period;
+   devpriv->wf_current += scan_remain_period;
devpriv->ai_last_scan_time += devpriv->ai_scan_period;
}
-   if (devpriv->usec_current >= devpriv->usec_period)
-   devpriv->usec_current %= devpriv->usec_period;
+   if (devpriv->wf_current >= devpriv->wf_period)
+   devpriv->wf_current %= devpriv->wf_period;
 
if (cmd->stop_src == TRIG_COUNT && async->scans_done >= cmd->stop_arg)
async->events |= COMEDI_CB_EOA;
@@ -329,7 +329,7 @@ static int waveform_ai_cmd(struct comedi_device *dev,
 {
struct waveform_private *devpriv = dev->private;
struct comedi_cmd *cmd = >async->cmd;
-   u64 usec_current;
+   u64 wf_current;
 
if (cmd->flags & CMDF_PRIORITY) {
dev_err(dev->class_dev,
@@ -351,8 +351,8 @@ stati

[PATCH 13/16] staging: comedi: comedi_test: allow read-back of AO channels

2015-10-27 Thread Ian Abbott
COMEDI drivers often allow the last value written to a channel on an
analog output subdevice to be read back via the "insn_read" handler.
The "comedi_test" driver does not currently support that.  It is a bit
special because it loops back the last values written to the channel on
the analog output subdevice to be read back via corresponding channels
on the analog input subdevice.  The "insn_read" handler for the analog
input subdevice is `waveform_ai_insn_read()`.  Set that as the
"insn_read" handler for the analog output subdevice as well.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index a750f84..468847a 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -457,6 +457,7 @@ static int waveform_attach(struct comedi_device *dev,
s->maxdata = 0x;
s->range_table = _ai_ranges;
s->insn_write = waveform_ao_insn_write;
+   s->insn_read = waveform_ai_insn_read;   /* do same as AI insn_read */
 
/* Our default loopback value is just a 0V flatline */
for (i = 0; i < s->n_chan; i++)
-- 
2.6.1

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


[PATCH 14/16] staging: comedi: comedi_test: handle partial scans in timer routine

2015-10-27 Thread Ian Abbott
For asynchronous command handling on the analog input subdevice, a
kernel timer routine is used to generate the fake waveform data.  A
"scan" consists of a number of conversions separated in time by a
conversion period.  Successive scans are separated in time by a scan
period, which is at least the conversion period multiplied by the number
of conversions per scan.  Currently, the timer routine does not generate
any data until the end of a scan period, generating whole scans of data
at a time.  Change it to generate data at the end of each conversion
period, with an extra delay after the final conversion in each scan if
necessary.  Use new member `ai_convert_time` in the private data
structure `struct waveform_private` to keep track of when the next
conversion is due.  This replaces the old member `ai_last_scan_time`
which kept track of the time of the previous scan.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 85 ++--
 1 file changed, 54 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 468847a..318340c 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -63,7 +63,7 @@ enum waveform_state_bits {
 /* Data unique to this driver */
 struct waveform_private {
struct timer_list ai_timer; /* timer for AI commands */
-   u64 ai_last_scan_time;  /* time of last AI scan in usec */
+   u64 ai_convert_time;/* time of next AI conversion in usec */
unsigned int wf_amplitude;  /* waveform amplitude in microvolts */
unsigned int wf_period; /* waveform period in microseconds */
unsigned int wf_current;/* current time in waveform period */
@@ -183,46 +183,51 @@ static void waveform_ai_interrupt(unsigned long arg)
struct comedi_subdevice *s = dev->read_subdev;
struct comedi_async *async = s->async;
struct comedi_cmd *cmd = >cmd;
-   unsigned int i, j;
-   unsigned long elapsed_time;
-   unsigned int num_scans;
+   u64 now;
+   unsigned int nsamples;
+   unsigned int time_increment;
 
/* check command is still active */
if (!test_bit(WAVEFORM_AI_RUNNING, >state_bits))
return;
 
-   elapsed_time = ktime_to_us(ktime_get()) - devpriv->ai_last_scan_time;
-   num_scans = elapsed_time / devpriv->ai_scan_period;
-
-   num_scans = comedi_nscans_left(s, num_scans);
-   for (i = 0; i < num_scans; i++) {
-   unsigned int scan_remain_period = devpriv->ai_scan_period;
-
-   for (j = 0; j < cmd->chanlist_len; j++) {
-   unsigned short sample;
-
-   if (devpriv->wf_current >= devpriv->wf_period)
-   devpriv->wf_current %= devpriv->wf_period;
-   sample = fake_waveform(dev, CR_CHAN(cmd->chanlist[j]),
-  CR_RANGE(cmd->chanlist[j]),
-  devpriv->wf_current);
-   comedi_buf_write_samples(s, , 1);
-   devpriv->wf_current += devpriv->ai_convert_period;
-   scan_remain_period -= devpriv->ai_convert_period;
+   now = ktime_to_us(ktime_get());
+   nsamples = comedi_nsamples_left(s, UINT_MAX);
+
+   while (nsamples && devpriv->ai_convert_time < now) {
+   unsigned int chanspec = cmd->chanlist[async->cur_chan];
+   unsigned short sample;
+
+   sample = fake_waveform(dev, CR_CHAN(chanspec),
+  CR_RANGE(chanspec), devpriv->wf_current);
+   if (comedi_buf_write_samples(s, , 1) == 0)
+   goto overrun;
+   time_increment = devpriv->ai_convert_period;
+   if (async->scan_progress == 0) {
+   /* done last conversion in scan, so add dead time */
+   time_increment += devpriv->ai_scan_period -
+ devpriv->ai_convert_period *
+ cmd->scan_end_arg;
}
-   devpriv->wf_current += scan_remain_period;
-   devpriv->ai_last_scan_time += devpriv->ai_scan_period;
+   devpriv->wf_current += time_increment;
+   if (devpriv->wf_current >= devpriv->wf_period)
+   devpriv->wf_current %= devpriv->wf_period;
+   devpriv->ai_convert_time += time_increment;
+   nsamples--;
}
-   if (devpriv->wf_current >= devpriv->wf_period)
-   devpriv->wf_current %= devpriv->wf_period;
 
if (cmd-&

[PATCH 16/16] staging: comedi: comedi_test: implement commands on AO subdevice

2015-10-27 Thread Ian Abbott
Implement COMEDI asynchronous commands on the fake analog output
subdevice.  This is useful for testing asynchronous commands in the
"write" direction when no real hardware is available.

A normal kernel timer is used to drive the command.  The new timer
expiry function `waveform_ao_timer()` handles whole "scans" at a time
according to the number of scan period that have elapsed since the last
scan.  Data for each channel in the scan is written to the internal
loopback array `devpriv->ao_loopbacks[]` and can be read back on the
analog input channels.  However, if several scan periods are outstanding
in the timer expiry function, only the latest available scan data is
written to the loopback array in order to save processing time.  The
expiry function also checks for underrun conditions, and checks for
normal termination of the asynchronous command when a "stop" scan count
is reached.

After the command is tested by `waveform_ao_cmdtest()` and set up by
`waveform_ao_cmd()`, it is not started until an internal trigger
function `waveform_ao_inttrig_start()` is called as a result of the user
performing an `INSN_INTTRIG` instruction on the subdevice.  The command
is stopped when the "cancel" handler `waveform_ao_cancel()` is called.
This may be due to the command terminating due to completion or an
error, or as a result of the user cancelling the command.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 212 ++-
 1 file changed, 209 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 14a0b62..4ab1866 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -57,7 +57,8 @@
 #define N_CHANS 8
 
 enum waveform_state_bits {
-   WAVEFORM_AI_RUNNING = 0
+   WAVEFORM_AI_RUNNING,
+   WAVEFORM_AO_RUNNING
 };
 
 /* Data unique to this driver */
@@ -70,6 +71,9 @@ struct waveform_private {
unsigned long state_bits;
unsigned int ai_scan_period;/* AI scan period in usec */
unsigned int ai_convert_period; /* AI conversion period in usec */
+   struct timer_list ao_timer; /* timer for AO commands */
+   u64 ao_last_scan_time;  /* time of previous AO scan in usec */
+   unsigned int ao_scan_period;/* AO scan period in usec */
unsigned short ao_loopbacks[N_CHANS];
 };
 
@@ -417,6 +421,201 @@ static int waveform_ai_insn_read(struct comedi_device 
*dev,
return insn->n;
 }
 
+/*
+ * This is the background routine to handle AO commands, scheduled by
+ * a timer mechanism.
+ */
+static void waveform_ao_timer(unsigned long arg)
+{
+   struct comedi_device *dev = (struct comedi_device *)arg;
+   struct waveform_private *devpriv = dev->private;
+   struct comedi_subdevice *s = dev->write_subdev;
+   struct comedi_async *async = s->async;
+   struct comedi_cmd *cmd = >cmd;
+   u64 now;
+   u64 scans_since;
+   unsigned int scans_avail = 0;
+
+   /* check command is still active */
+   if (!test_bit(WAVEFORM_AO_RUNNING, >state_bits))
+   return;
+
+   /* determine number of scan periods since last time */
+   now = ktime_to_us(ktime_get());
+   scans_since = now - devpriv->ao_last_scan_time;
+   do_div(scans_since, devpriv->ao_scan_period);
+   if (scans_since) {
+   unsigned int i;
+
+   /* determine scans in buffer, limit to scans to do this time */
+   scans_avail = comedi_nscans_left(s, 0);
+   if (scans_avail > scans_since)
+   scans_avail = scans_since;
+   if (scans_avail) {
+   /* skip all but the last scan to save processing time */
+   if (scans_avail > 1) {
+   unsigned int skip_bytes, nbytes;
+
+   skip_bytes =
+   comedi_samples_to_bytes(s, cmd->scan_end_arg *
+  (scans_avail - 1));
+   nbytes = comedi_buf_read_alloc(s, skip_bytes);
+   comedi_buf_read_free(s, nbytes);
+   comedi_inc_scan_progress(s, nbytes);
+   if (nbytes < skip_bytes) {
+   /* unexpected underrun! (cancelled?) */
+   async->events |= COMEDI_CB_OVERFLOW;
+   goto underrun;
+   }
+   }
+   /* output the last scan */
+   for (i = 0; i < cmd->scan_end_arg; i++) {
+   unsigned int chan = CR

[PATCH 15/16] staging: comedi: comedi_test: rename waveform_ai_interrupt()

2015-10-27 Thread Ian Abbott
`waveform_ai_interrupt()` is a timer expiry function used to generate
fake waveform data for an analog input subdevice.  Rename it to
`waveform_ai_timer()`.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 318340c..14a0b62 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -176,7 +176,7 @@ static unsigned short fake_waveform(struct comedi_device 
*dev,
  * It should run in the background; therefore it is scheduled by
  * a timer mechanism.
  */
-static void waveform_ai_interrupt(unsigned long arg)
+static void waveform_ai_timer(unsigned long arg)
 {
struct comedi_device *dev = (struct comedi_device *)arg;
struct waveform_private *devpriv = dev->private;
@@ -486,8 +486,7 @@ static int waveform_attach(struct comedi_device *dev,
for (i = 0; i < s->n_chan; i++)
devpriv->ao_loopbacks[i] = s->maxdata / 2;
 
-   setup_timer(>ai_timer, waveform_ai_interrupt,
-   (unsigned long)dev);
+   setup_timer(>ai_timer, waveform_ai_timer, (unsigned long)dev);
 
dev_info(dev->class_dev,
 "%s: %u microvolt, %u microsecond waveform attached\n",
-- 
2.6.1

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


[PATCH 00/16] staging: comedi: comedi_test: enhancements

2015-10-27 Thread Ian Abbott
The "comedi_test" module is a driver for a dummy COMEDI device.  It has
an analog input subdevice and an analog output subdevice.  The analog
input subdevice supports COMEDI asynchronous acquisition commands using
waveform generators to generate the input data for each channel.  A
kernel timer is used to driver the acquisition.

This series of patches cleans up the driver, enhances the existing
asynchronous command support on the analog input subdevice, and adds
asynchronous command support on the analog output subdevice.

01) staging: comedi: comedi_test: reformat multi-line comments
02) staging: comedi: comedi_test: saturate fake waveform values
03) staging: comedi: comedi_test: remove nano_per_micro
04) staging: comedi: comedi_test: limit maximum convert_arg
05) staging: comedi: comedi_test: support scan_begin_src == TRIG_FOLLOW
06) staging: comedi: comedi_test: move modulo operations for waveform
07) staging: comedi: comedi_test: use unsigned int for waveform timing
08) staging: comedi: comedi_test: simplify time since last AI scan
09) staging: comedi: comedi_test: rename members for AI commands
10) staging: comedi: comedi_test: rename waveform members
11) staging: comedi: comedi_test: make timer rate similar to scan rate
12) staging: comedi: comedi_test: use unsigned short for loopback values
13) staging: comedi: comedi_test: allow read-back of AO channels
14) staging: comedi: comedi_test: handle partial scans in timer routine
15) staging: comedi: comedi_test: rename waveform_ai_interrupt()
16) staging: comedi: comedi_test: implement commands on AO subdevice

 drivers/staging/comedi/drivers/comedi_test.c | 565 ---
 1 file changed, 416 insertions(+), 149 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 02/16] staging: comedi: comedi_test: saturate fake waveform values

2015-10-27 Thread Ian Abbott
While an asynchronous command is running on the analog input subdevice,
fake waveform generators are connected to each channel to provide the
input voltages that are converted to sample value.  Channel 0 is
connected to a sawtooth generator, channel 1 is connected to a
squarewave generator, and the remaining channels are connected to a
flatline generator.  The non-flatline generators share the same
amplitude (in microvolts) and period (in microseconds) which are
configured when the COMEDI device is attached.  All waveforms are
centered around 0 microvolts and the non-flatline waveforms go between
-amplitude and +amplitude.

It is possible for the waveforms to swing outside the input range of the
channels to which they are connected.  When that happens, the sample
values resulting from simulated A-to-D conversion will wrap around due
to integer overflow.  Prevent that by clamping the sample values that
would go out of range.  This is closer to how a real hardware device
would behave (assuming the input voltage is not high enough to damage
the hardware!).

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 29 ++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 899faf7..53c9a5c 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -104,9 +104,17 @@ static unsigned short fake_sawtooth(struct comedi_device 
*dev,
value = current_time;
value *= binary_amplitude * 2;
do_div(value, devpriv->usec_period);
-   value -= binary_amplitude;  /* get rid of sawtooth's dc offset */
+   value += offset;
+   /* get rid of sawtooth's dc offset and clamp value */
+   if (value < binary_amplitude) {
+   value = 0;  /* negative saturation */
+   } else {
+   value -= binary_amplitude;
+   if (value > s->maxdata)
+   value = s->maxdata; /* positive saturation */
+   }
 
-   return offset + value;
+   return value;
 }
 
 static unsigned short fake_squarewave(struct comedi_device *dev,
@@ -119,16 +127,25 @@ static unsigned short fake_squarewave(struct 
comedi_device *dev,
u64 value;
const struct comedi_krange *krange =
>range_table->range[range_index];
-   current_time %= devpriv->usec_period;
 
+   current_time %= devpriv->usec_period;
value = s->maxdata;
value *= devpriv->uvolt_amplitude;
do_div(value, krange->max - krange->min);
 
-   if (current_time < devpriv->usec_period / 2)
-   value *= -1;
+   /* get one of two values for square-wave and clamp */
+   if (current_time < devpriv->usec_period / 2) {
+   if (offset < value)
+   value = 0;  /* negative saturation */
+   else
+   value = offset - value;
+   } else {
+   value += offset;
+   if (value > s->maxdata)
+   value = s->maxdata; /* positive saturation */
+   }
 
-   return offset + value;
+   return value;
 }
 
 static unsigned short fake_flatline(struct comedi_device *dev,
-- 
2.6.1

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


[PATCH 03/16] staging: comedi: comedi_test: remove nano_per_micro

2015-10-27 Thread Ian Abbott
The `static const int nano_per_micro` variable is set to 1000, the
number of nanoseconds in a microsecond.  Remove it and use the
`NSEC_PER_USEC` macro from  instead.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers/comedi_test.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 53c9a5c..d9810ca 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -73,9 +73,6 @@ struct waveform_private {
unsigned int ao_loopbacks[N_CHANS];
 };
 
-/* 1000 nanosec in a microsec */
-static const int nano_per_micro = 1000;
-
 /* fake analog input ranges */
 static const struct comedi_lrange waveform_ai_ranges = {
2, {
@@ -270,7 +267,7 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
 
if (cmd->scan_begin_src == TRIG_TIMER) {
err |= comedi_check_trigger_arg_min(>scan_begin_arg,
-   nano_per_micro);
+   NSEC_PER_USEC);
if (cmd->convert_src == TRIG_TIMER) {
err |= comedi_check_trigger_arg_min(>
scan_begin_arg,
@@ -296,15 +293,15 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
if (cmd->scan_begin_src == TRIG_TIMER) {
arg = cmd->scan_begin_arg;
/* round to nearest microsec */
-   arg = nano_per_micro *
- ((arg + (nano_per_micro / 2)) / nano_per_micro);
+   arg = NSEC_PER_USEC *
+ ((arg + (NSEC_PER_USEC / 2)) / NSEC_PER_USEC);
err |= comedi_check_trigger_arg_is(>scan_begin_arg, arg);
}
if (cmd->convert_src == TRIG_TIMER) {
arg = cmd->convert_arg;
/* round to nearest microsec */
-   arg = nano_per_micro *
- ((arg + (nano_per_micro / 2)) / nano_per_micro);
+   arg = NSEC_PER_USEC *
+ ((arg + (NSEC_PER_USEC / 2)) / NSEC_PER_USEC);
err |= comedi_check_trigger_arg_is(>convert_arg, arg);
}
 
@@ -326,12 +323,12 @@ static int waveform_ai_cmd(struct comedi_device *dev,
return -1;
}
 
-   devpriv->scan_period = cmd->scan_begin_arg / nano_per_micro;
+   devpriv->scan_period = cmd->scan_begin_arg / NSEC_PER_USEC;
 
if (cmd->convert_src == TRIG_NOW)
devpriv->convert_period = 0;
else/* TRIG_TIMER */
-   devpriv->convert_period = cmd->convert_arg / nano_per_micro;
+   devpriv->convert_period = cmd->convert_arg / NSEC_PER_USEC;
 
devpriv->last = ktime_get();
devpriv->usec_current =
-- 
2.6.1

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


[PATCH 09/16] staging: comedi: comedi_test: rename members for AI commands

2015-10-27 Thread Ian Abbott
Rename the members of `struct waveform_private` that are used to handle
AI commands, apart from those members used to control fake waveform
generation.  The renames are `timer` --> `ai_timer`, `scan_period` -->
`ai_scan_period`, and `convert_period` --> `ai_convert_period`.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 38 ++--
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 78fde3a..8e618ea 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -61,14 +61,14 @@ enum waveform_state_bits {
 
 /* Data unique to this driver */
 struct waveform_private {
-   struct timer_list timer;
+   struct timer_list ai_timer; /* timer for AI commands */
u64 ai_last_scan_time;  /* time of last AI scan in usec */
unsigned int uvolt_amplitude;   /* waveform amplitude in microvolts */
unsigned int usec_period;   /* waveform period in microseconds */
unsigned int usec_current;  /* current time (mod waveform period) */
unsigned long state_bits;
-   unsigned int scan_period;   /* scan period in usec */
-   unsigned int convert_period;/* conversion period in usec */
+   unsigned int ai_scan_period;/* AI scan period in usec */
+   unsigned int ai_convert_period; /* AI conversion period in usec */
unsigned int ao_loopbacks[N_CHANS];
 };
 
@@ -191,11 +191,11 @@ static void waveform_ai_interrupt(unsigned long arg)
return;
 
elapsed_time = ktime_to_us(ktime_get()) - devpriv->ai_last_scan_time;
-   num_scans = elapsed_time / devpriv->scan_period;
+   num_scans = elapsed_time / devpriv->ai_scan_period;
 
num_scans = comedi_nscans_left(s, num_scans);
for (i = 0; i < num_scans; i++) {
-   unsigned int scan_remain_period = devpriv->scan_period;
+   unsigned int scan_remain_period = devpriv->ai_scan_period;
 
for (j = 0; j < cmd->chanlist_len; j++) {
unsigned short sample;
@@ -206,11 +206,11 @@ static void waveform_ai_interrupt(unsigned long arg)
   CR_RANGE(cmd->chanlist[j]),
   devpriv->usec_current);
comedi_buf_write_samples(s, , 1);
-   devpriv->usec_current += devpriv->convert_period;
-   scan_remain_period -= devpriv->convert_period;
+   devpriv->usec_current += devpriv->ai_convert_period;
+   scan_remain_period -= devpriv->ai_convert_period;
}
devpriv->usec_current += scan_remain_period;
-   devpriv->ai_last_scan_time += devpriv->scan_period;
+   devpriv->ai_last_scan_time += devpriv->ai_scan_period;
}
if (devpriv->usec_current >= devpriv->usec_period)
devpriv->usec_current %= devpriv->usec_period;
@@ -218,7 +218,7 @@ static void waveform_ai_interrupt(unsigned long arg)
if (cmd->stop_src == TRIG_COUNT && async->scans_done >= cmd->stop_arg)
async->events |= COMEDI_CB_EOA;
else
-   mod_timer(>timer, jiffies + 1);
+   mod_timer(>ai_timer, jiffies + 1);
 
comedi_handle_events(dev, s);
 }
@@ -338,15 +338,15 @@ static int waveform_ai_cmd(struct comedi_device *dev,
}
 
if (cmd->convert_src == TRIG_NOW)
-   devpriv->convert_period = 0;
+   devpriv->ai_convert_period = 0;
else/* cmd->convert_src == TRIG_TIMER */
-   devpriv->convert_period = cmd->convert_arg / NSEC_PER_USEC;
+   devpriv->ai_convert_period = cmd->convert_arg / NSEC_PER_USEC;
 
if (cmd->scan_begin_src == TRIG_FOLLOW) {
-   devpriv->scan_period = devpriv->convert_period *
-  cmd->scan_end_arg;
+   devpriv->ai_scan_period = devpriv->ai_convert_period *
+ cmd->scan_end_arg;
} else {/* cmd->scan_begin_src == TRIG_TIMER */
-   devpriv->scan_period = cmd->scan_begin_arg / NSEC_PER_USEC;
+   devpriv->ai_scan_period = cmd->scan_begin_arg / NSEC_PER_USEC;
}
 
devpriv->ai_last_scan_time = ktime_to_us(ktime_get());
@@ -354,12 +354,12 @@ static int waveform_ai_cmd(struct comedi_device *dev,
usec_current = devpriv->ai_last_scan_time;
devpriv->usec_current = do_div(usec_current, devpriv->usec_period);
 
-   devpriv->

[PATCH 08/16] staging: comedi: comedi_test: simplify time since last AI scan

2015-10-27 Thread Ian Abbott
The private data structure `struct waveform_private` currently uses
member `last` to remember the time of the last timer interrupt, and the
member `usec_remainder` to keep track of how far into a simulated scan
the interrupt occurred.  Replace these with a single new member
`ai_last_scan_time` that records the time of the last scan.  This
simplifies the calculation of the number of scans to simulate in the
timer routine, `waveform_ai_interrupt()`.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 27 ++-
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 158e090..78fde3a 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -62,11 +62,10 @@ enum waveform_state_bits {
 /* Data unique to this driver */
 struct waveform_private {
struct timer_list timer;
-   ktime_t last;   /* time last timer interrupt occurred */
+   u64 ai_last_scan_time;  /* time of last AI scan in usec */
unsigned int uvolt_amplitude;   /* waveform amplitude in microvolts */
unsigned int usec_period;   /* waveform period in microseconds */
unsigned int usec_current;  /* current time (mod waveform period) */
-   unsigned long usec_remainder;   /* usec since last scan */
unsigned long state_bits;
unsigned int scan_period;   /* scan period in usec */
unsigned int convert_period;/* conversion period in usec */
@@ -184,27 +183,19 @@ static void waveform_ai_interrupt(unsigned long arg)
struct comedi_async *async = s->async;
struct comedi_cmd *cmd = >cmd;
unsigned int i, j;
-   /* all times in microsec */
unsigned long elapsed_time;
unsigned int num_scans;
-   ktime_t now;
 
/* check command is still active */
if (!test_bit(WAVEFORM_AI_RUNNING, >state_bits))
return;
 
-   now = ktime_get();
-
-   elapsed_time = ktime_to_us(ktime_sub(now, devpriv->last));
-   devpriv->last = now;
-   num_scans =
-   (devpriv->usec_remainder + elapsed_time) / devpriv->scan_period;
-   devpriv->usec_remainder =
-   (devpriv->usec_remainder + elapsed_time) % devpriv->scan_period;
+   elapsed_time = ktime_to_us(ktime_get()) - devpriv->ai_last_scan_time;
+   num_scans = elapsed_time / devpriv->scan_period;
 
num_scans = comedi_nscans_left(s, num_scans);
for (i = 0; i < num_scans; i++) {
-   unsigned long scan_remain_period = devpriv->scan_period;
+   unsigned int scan_remain_period = devpriv->scan_period;
 
for (j = 0; j < cmd->chanlist_len; j++) {
unsigned short sample;
@@ -219,6 +210,7 @@ static void waveform_ai_interrupt(unsigned long arg)
scan_remain_period -= devpriv->convert_period;
}
devpriv->usec_current += scan_remain_period;
+   devpriv->ai_last_scan_time += devpriv->scan_period;
}
if (devpriv->usec_current >= devpriv->usec_period)
devpriv->usec_current %= devpriv->usec_period;
@@ -337,6 +329,7 @@ static int waveform_ai_cmd(struct comedi_device *dev,
 {
struct waveform_private *devpriv = dev->private;
struct comedi_cmd *cmd = >async->cmd;
+   u64 usec_current;
 
if (cmd->flags & CMDF_PRIORITY) {
dev_err(dev->class_dev,
@@ -356,10 +349,10 @@ static int waveform_ai_cmd(struct comedi_device *dev,
devpriv->scan_period = cmd->scan_begin_arg / NSEC_PER_USEC;
}
 
-   devpriv->last = ktime_get();
-   devpriv->usec_current =
-   ((u32)ktime_to_us(devpriv->last)) % devpriv->usec_period;
-   devpriv->usec_remainder = 0;
+   devpriv->ai_last_scan_time = ktime_to_us(ktime_get());
+   /* Determine time within waveform period. */
+   usec_current = devpriv->ai_last_scan_time;
+   devpriv->usec_current = do_div(usec_current, devpriv->usec_period);
 
devpriv->timer.expires = jiffies + 1;
/* mark command as active */
-- 
2.6.1

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


[PATCH 11/16] staging: comedi: comedi_test: make timer rate similar to scan rate

2015-10-27 Thread Ian Abbott
The asynchronous command handling for the analog input subdevice uses a
kernel timer which expires approximately `HZ` times a second.  However,
it only needs to do anything after each scan period.  Set the timer to
expire just after the next scan period.

Although the timer expiry function `waveform_ai_interrupt()` uses
precise time values to generate the fake waveforms used to generate the
data, those time values are constructed in a precise sequence, and do
not depend on the time the timer expiry function is actually called.  So
the timer expiry rate does not have to be very precise.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 1b3ad7f..9655dc3 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -52,6 +52,7 @@
 
 #include 
 #include 
+#include 
 
 #define N_CHANS 8
 
@@ -215,10 +216,12 @@ static void waveform_ai_interrupt(unsigned long arg)
if (devpriv->wf_current >= devpriv->wf_period)
devpriv->wf_current %= devpriv->wf_period;
 
-   if (cmd->stop_src == TRIG_COUNT && async->scans_done >= cmd->stop_arg)
+   if (cmd->stop_src == TRIG_COUNT && async->scans_done >= cmd->stop_arg) {
async->events |= COMEDI_CB_EOA;
-   else
-   mod_timer(>ai_timer, jiffies + 1);
+   } else {
+   mod_timer(>ai_timer,
+ jiffies + usecs_to_jiffies(devpriv->ai_scan_period));
+   }
 
comedi_handle_events(dev, s);
 }
@@ -354,7 +357,9 @@ static int waveform_ai_cmd(struct comedi_device *dev,
wf_current = devpriv->ai_last_scan_time;
devpriv->wf_current = do_div(wf_current, devpriv->wf_period);
 
-   devpriv->ai_timer.expires = jiffies + 1;
+   devpriv->ai_timer.expires =
+   jiffies + usecs_to_jiffies(devpriv->ai_scan_period);
+
/* mark command as active */
smp_mb__before_atomic();
set_bit(WAVEFORM_AI_RUNNING, >state_bits);
-- 
2.6.1

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


[PATCH 10/16] staging: comedi: comedi_test: rename waveform members

2015-10-27 Thread Ian Abbott
Rename the members `struct waveform_private` associated with fake
waveform generation.  The affected members are `uvolt_amplitude` -->
`wf_amplitude` (the amplitude of the waveform), `usec_period` -->
`wf_period` (the period of the waveform), and `usec_current` -->
`wf_current` (the current time within a waveform period).

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 40 ++--
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 8e618ea..1b3ad7f 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -63,9 +63,9 @@ enum waveform_state_bits {
 struct waveform_private {
struct timer_list ai_timer; /* timer for AI commands */
u64 ai_last_scan_time;  /* time of last AI scan in usec */
-   unsigned int uvolt_amplitude;   /* waveform amplitude in microvolts */
-   unsigned int usec_period;   /* waveform period in microseconds */
-   unsigned int usec_current;  /* current time (mod waveform period) */
+   unsigned int wf_amplitude;  /* waveform amplitude in microvolts */
+   unsigned int wf_period; /* waveform period in microseconds */
+   unsigned int wf_current;/* current time in waveform period */
unsigned long state_bits;
unsigned int ai_scan_period;/* AI scan period in usec */
unsigned int ai_convert_period; /* AI conversion period in usec */
@@ -93,12 +93,12 @@ static unsigned short fake_sawtooth(struct comedi_device 
*dev,
u64 binary_amplitude;
 
binary_amplitude = s->maxdata;
-   binary_amplitude *= devpriv->uvolt_amplitude;
+   binary_amplitude *= devpriv->wf_amplitude;
do_div(binary_amplitude, krange->max - krange->min);
 
value = current_time;
value *= binary_amplitude * 2;
-   do_div(value, devpriv->usec_period);
+   do_div(value, devpriv->wf_period);
value += offset;
/* get rid of sawtooth's dc offset and clamp value */
if (value < binary_amplitude) {
@@ -124,11 +124,11 @@ static unsigned short fake_squarewave(struct 
comedi_device *dev,
>range_table->range[range_index];
 
value = s->maxdata;
-   value *= devpriv->uvolt_amplitude;
+   value *= devpriv->wf_amplitude;
do_div(value, krange->max - krange->min);
 
/* get one of two values for square-wave and clamp */
-   if (current_time < devpriv->usec_period / 2) {
+   if (current_time < devpriv->wf_period / 2) {
if (offset < value)
value = 0;  /* negative saturation */
else
@@ -200,20 +200,20 @@ static void waveform_ai_interrupt(unsigned long arg)
for (j = 0; j < cmd->chanlist_len; j++) {
unsigned short sample;
 
-   if (devpriv->usec_current >= devpriv->usec_period)
-   devpriv->usec_current %= devpriv->usec_period;
+   if (devpriv->wf_current >= devpriv->wf_period)
+   devpriv->wf_current %= devpriv->wf_period;
sample = fake_waveform(dev, CR_CHAN(cmd->chanlist[j]),
   CR_RANGE(cmd->chanlist[j]),
-  devpriv->usec_current);
+  devpriv->wf_current);
comedi_buf_write_samples(s, , 1);
-   devpriv->usec_current += devpriv->ai_convert_period;
+   devpriv->wf_current += devpriv->ai_convert_period;
scan_remain_period -= devpriv->ai_convert_period;
}
-   devpriv->usec_current += scan_remain_period;
+   devpriv->wf_current += scan_remain_period;
devpriv->ai_last_scan_time += devpriv->ai_scan_period;
}
-   if (devpriv->usec_current >= devpriv->usec_period)
-   devpriv->usec_current %= devpriv->usec_period;
+   if (devpriv->wf_current >= devpriv->wf_period)
+   devpriv->wf_current %= devpriv->wf_period;
 
if (cmd->stop_src == TRIG_COUNT && async->scans_done >= cmd->stop_arg)
async->events |= COMEDI_CB_EOA;
@@ -329,7 +329,7 @@ static int waveform_ai_cmd(struct comedi_device *dev,
 {
struct waveform_private *devpriv = dev->private;
struct comedi_cmd *cmd = >async->cmd;
-   u64 usec_current;
+   u64 wf_current;
 
if (cmd->flags & CMDF_PRIORITY) {
dev_err(dev->class_dev,

[PATCH 12/16] staging: comedi: comedi_test: use unsigned short for loopback values

2015-10-27 Thread Ian Abbott
The last sample values written to the AO subdevice channels by its
"insn_write" handler `waveform_ao_insn_write()` are stored in the member
array `ao_loopbacks[]` in the device private data `struct
waveform_private`.  They can be read back via the "insn_read" handler of
the AI subdevice `waveform_ai_insn_read()`.  As the stored sample values
are only 16 bits wide, change the type of the `ao_loopbacks[]` member to
`unsigned short` to save some space.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 9655dc3..a750f84 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -70,7 +70,7 @@ struct waveform_private {
unsigned long state_bits;
unsigned int ai_scan_period;/* AI scan period in usec */
unsigned int ai_convert_period; /* AI conversion period in usec */
-   unsigned int ao_loopbacks[N_CHANS];
+   unsigned short ao_loopbacks[N_CHANS];
 };
 
 /* fake analog input ranges */
-- 
2.6.1

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


[PATCH 04/16] staging: comedi: comedi_test: limit maximum convert_arg

2015-10-27 Thread Ian Abbott
When testing the parameters for setting up an asynchronous command on
the AI subdevice, limit the maximum conversion period
(`cmd->convert_arg`) so that the number of conversions in a scan
(`cmd->scan_end_arg`, same as `cmd->chanlist_len`) multiplied by the
conversion period fits within an `unsigned int`, as that is used to
limit the minimum scan period (`cmd->scan_begin_arg`).

Also ensure rounding of the conversion period and scan period to the
nearest microsecond both fit in an `unsigned int`.  Do all this in stage
4 ("fix up any arguments") of the command testing.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 42 ++--
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index d9810ca..f011fbd 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -234,7 +234,7 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
   struct comedi_cmd *cmd)
 {
int err = 0;
-   unsigned int arg;
+   unsigned int arg, limit;
 
/* Step 1 : check if triggers are trivially valid */
 
@@ -265,16 +265,8 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
if (cmd->convert_src == TRIG_NOW)
err |= comedi_check_trigger_arg_is(>convert_arg, 0);
 
-   if (cmd->scan_begin_src == TRIG_TIMER) {
-   err |= comedi_check_trigger_arg_min(>scan_begin_arg,
-   NSEC_PER_USEC);
-   if (cmd->convert_src == TRIG_TIMER) {
-   err |= comedi_check_trigger_arg_min(>
-   scan_begin_arg,
-   cmd->convert_arg *
-   cmd->chanlist_len);
-   }
-   }
+   err |= comedi_check_trigger_arg_min(>scan_begin_arg,
+   NSEC_PER_USEC);
 
err |= comedi_check_trigger_arg_min(>chanlist_len, 1);
err |= comedi_check_trigger_arg_is(>scan_end_arg,
@@ -290,21 +282,29 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
 
/* step 4: fix up any arguments */
 
-   if (cmd->scan_begin_src == TRIG_TIMER) {
-   arg = cmd->scan_begin_arg;
-   /* round to nearest microsec */
-   arg = NSEC_PER_USEC *
- ((arg + (NSEC_PER_USEC / 2)) / NSEC_PER_USEC);
-   err |= comedi_check_trigger_arg_is(>scan_begin_arg, arg);
-   }
if (cmd->convert_src == TRIG_TIMER) {
+   /* round convert_arg to nearest microsecond */
arg = cmd->convert_arg;
-   /* round to nearest microsec */
-   arg = NSEC_PER_USEC *
- ((arg + (NSEC_PER_USEC / 2)) / NSEC_PER_USEC);
+   arg = min(arg,
+ rounddown(UINT_MAX, (unsigned int)NSEC_PER_USEC));
+   arg = NSEC_PER_USEC * DIV_ROUND_CLOSEST(arg, NSEC_PER_USEC);
+   /* limit convert_arg to keep scan_begin_arg in range */
+   limit = UINT_MAX / cmd->scan_end_arg;
+   limit = rounddown(limit, (unsigned int)NSEC_PER_SEC);
+   arg = min(arg, limit);
err |= comedi_check_trigger_arg_is(>convert_arg, arg);
}
 
+   /* round scan_begin_arg to nearest microsecond */
+   arg = cmd->scan_begin_arg;
+   arg = min(arg, rounddown(UINT_MAX, (unsigned int)NSEC_PER_USEC));
+   arg = NSEC_PER_USEC * DIV_ROUND_CLOSEST(arg, NSEC_PER_USEC);
+   if (cmd->convert_src == TRIG_TIMER) {
+   /* but ensure scan_begin_arg is large enough */
+   arg = max(arg, cmd->convert_arg * cmd->scan_end_arg);
+   }
+   err |= comedi_check_trigger_arg_is(>scan_begin_arg, arg);
+
if (err)
return 4;
 
-- 
2.6.1

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


[PATCH 05/16] staging: comedi: comedi_test: support scan_begin_src == TRIG_FOLLOW

2015-10-27 Thread Ian Abbott
It is quite common for COMEDI subdevices that support commands to
support setting `scan_begin_src` to `TRIG_FOLLOW`.  This means the next
scan begins once all conversions in the current scan are complete.
Support the following timing combinations for the AI subdevice:

  scan_begin_src == TRIG_TIMER && convert_src == TRIG_TIMER
  scan_begin_src == TRIG_TIMER && convert_src == TRIG_NOW
  scan_begin_src == TRIG_FOLLOW && convert_src == TRIG_TIMER

The actual scan period in microseconds is stored in the `scan_period`
member of the private data structure `struct waveform_private`.  An
`unsigned int` is still wide enough, because the conversion period is no
more than `UINT_MAX / 1000` microseconds and the number of conversions
is no more than 16 (`N_CHANS * 2`).

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 64 +++-
 1 file changed, 44 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index f011fbd..cc35bd6 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -239,7 +239,8 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
/* Step 1 : check if triggers are trivially valid */
 
err |= comedi_check_trigger_src(>start_src, TRIG_NOW);
-   err |= comedi_check_trigger_src(>scan_begin_src, TRIG_TIMER);
+   err |= comedi_check_trigger_src(>scan_begin_src,
+   TRIG_FOLLOW | TRIG_TIMER);
err |= comedi_check_trigger_src(>convert_src,
TRIG_NOW | TRIG_TIMER);
err |= comedi_check_trigger_src(>scan_end_src, TRIG_COUNT);
@@ -255,6 +256,9 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
 
/* Step 2b : and mutually compatible */
 
+   if (cmd->scan_begin_src == TRIG_FOLLOW && cmd->convert_src == TRIG_NOW)
+   err |= -EINVAL; /* scan period would be 0 */
+
if (err)
return 2;
 
@@ -262,11 +266,21 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
 
err |= comedi_check_trigger_arg_is(>start_arg, 0);
 
-   if (cmd->convert_src == TRIG_NOW)
+   if (cmd->convert_src == TRIG_NOW) {
err |= comedi_check_trigger_arg_is(>convert_arg, 0);
+   } else {/* cmd->convert_src == TRIG_TIMER */
+   if (cmd->scan_begin_src == TRIG_FOLLOW) {
+   err |= comedi_check_trigger_arg_min(>convert_arg,
+   NSEC_PER_USEC);
+   }
+   }
 
-   err |= comedi_check_trigger_arg_min(>scan_begin_arg,
-   NSEC_PER_USEC);
+   if (cmd->scan_begin_src == TRIG_FOLLOW) {
+   err |= comedi_check_trigger_arg_is(>scan_begin_arg, 0);
+   } else {/* cmd->scan_begin_src == TRIG_TIMER */
+   err |= comedi_check_trigger_arg_min(>scan_begin_arg,
+   NSEC_PER_USEC);
+   }
 
err |= comedi_check_trigger_arg_min(>chanlist_len, 1);
err |= comedi_check_trigger_arg_is(>scan_end_arg,
@@ -274,7 +288,7 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
 
if (cmd->stop_src == TRIG_COUNT)
err |= comedi_check_trigger_arg_min(>stop_arg, 1);
-   else/* TRIG_NONE */
+   else/* cmd->stop_src == TRIG_NONE */
err |= comedi_check_trigger_arg_is(>stop_arg, 0);
 
if (err)
@@ -288,22 +302,27 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
arg = min(arg,
  rounddown(UINT_MAX, (unsigned int)NSEC_PER_USEC));
arg = NSEC_PER_USEC * DIV_ROUND_CLOSEST(arg, NSEC_PER_USEC);
-   /* limit convert_arg to keep scan_begin_arg in range */
-   limit = UINT_MAX / cmd->scan_end_arg;
-   limit = rounddown(limit, (unsigned int)NSEC_PER_SEC);
-   arg = min(arg, limit);
+   if (cmd->scan_begin_arg == TRIG_TIMER) {
+   /* limit convert_arg to keep scan_begin_arg in range */
+   limit = UINT_MAX / cmd->scan_end_arg;
+   limit = rounddown(limit, (unsigned int)NSEC_PER_SEC);
+   arg = min(arg, limit);
+   }
err |= comedi_check_trigger_arg_is(>convert_arg, arg);
}
 
-   /* round scan_begin_arg to nearest microsecond */
-   arg = cmd->scan_begin_arg;
-   arg = min(arg, rounddown(UINT_MAX, (unsigned int)NSEC_PER_USEC));
-   arg = NSEC_PER_USEC * DIV_ROUND_CLOSEST(arg, NSEC_PER_USEC);
-   if (cmd->convert_src == TRIG_TIMER) {
-   /*

[PATCH 02/16] staging: comedi: comedi_test: saturate fake waveform values

2015-10-27 Thread Ian Abbott
While an asynchronous command is running on the analog input subdevice,
fake waveform generators are connected to each channel to provide the
input voltages that are converted to sample value.  Channel 0 is
connected to a sawtooth generator, channel 1 is connected to a
squarewave generator, and the remaining channels are connected to a
flatline generator.  The non-flatline generators share the same
amplitude (in microvolts) and period (in microseconds) which are
configured when the COMEDI device is attached.  All waveforms are
centered around 0 microvolts and the non-flatline waveforms go between
-amplitude and +amplitude.

It is possible for the waveforms to swing outside the input range of the
channels to which they are connected.  When that happens, the sample
values resulting from simulated A-to-D conversion will wrap around due
to integer overflow.  Prevent that by clamping the sample values that
would go out of range.  This is closer to how a real hardware device
would behave (assuming the input voltage is not high enough to damage
the hardware!).

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 29 ++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 899faf7..53c9a5c 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -104,9 +104,17 @@ static unsigned short fake_sawtooth(struct comedi_device 
*dev,
value = current_time;
value *= binary_amplitude * 2;
do_div(value, devpriv->usec_period);
-   value -= binary_amplitude;  /* get rid of sawtooth's dc offset */
+   value += offset;
+   /* get rid of sawtooth's dc offset and clamp value */
+   if (value < binary_amplitude) {
+   value = 0;  /* negative saturation */
+   } else {
+   value -= binary_amplitude;
+   if (value > s->maxdata)
+   value = s->maxdata; /* positive saturation */
+   }
 
-   return offset + value;
+   return value;
 }
 
 static unsigned short fake_squarewave(struct comedi_device *dev,
@@ -119,16 +127,25 @@ static unsigned short fake_squarewave(struct 
comedi_device *dev,
u64 value;
const struct comedi_krange *krange =
>range_table->range[range_index];
-   current_time %= devpriv->usec_period;
 
+   current_time %= devpriv->usec_period;
value = s->maxdata;
value *= devpriv->uvolt_amplitude;
do_div(value, krange->max - krange->min);
 
-   if (current_time < devpriv->usec_period / 2)
-   value *= -1;
+   /* get one of two values for square-wave and clamp */
+   if (current_time < devpriv->usec_period / 2) {
+   if (offset < value)
+   value = 0;  /* negative saturation */
+   else
+   value = offset - value;
+   } else {
+   value += offset;
+   if (value > s->maxdata)
+   value = s->maxdata; /* positive saturation */
+   }
 
-   return offset + value;
+   return value;
 }
 
 static unsigned short fake_flatline(struct comedi_device *dev,
-- 
2.6.1

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


[PATCH 03/16] staging: comedi: comedi_test: remove nano_per_micro

2015-10-27 Thread Ian Abbott
The `static const int nano_per_micro` variable is set to 1000, the
number of nanoseconds in a microsecond.  Remove it and use the
`NSEC_PER_USEC` macro from  instead.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 53c9a5c..d9810ca 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -73,9 +73,6 @@ struct waveform_private {
unsigned int ao_loopbacks[N_CHANS];
 };
 
-/* 1000 nanosec in a microsec */
-static const int nano_per_micro = 1000;
-
 /* fake analog input ranges */
 static const struct comedi_lrange waveform_ai_ranges = {
2, {
@@ -270,7 +267,7 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
 
if (cmd->scan_begin_src == TRIG_TIMER) {
err |= comedi_check_trigger_arg_min(>scan_begin_arg,
-   nano_per_micro);
+   NSEC_PER_USEC);
if (cmd->convert_src == TRIG_TIMER) {
err |= comedi_check_trigger_arg_min(>
scan_begin_arg,
@@ -296,15 +293,15 @@ static int waveform_ai_cmdtest(struct comedi_device *dev,
if (cmd->scan_begin_src == TRIG_TIMER) {
arg = cmd->scan_begin_arg;
/* round to nearest microsec */
-   arg = nano_per_micro *
- ((arg + (nano_per_micro / 2)) / nano_per_micro);
+   arg = NSEC_PER_USEC *
+ ((arg + (NSEC_PER_USEC / 2)) / NSEC_PER_USEC);
err |= comedi_check_trigger_arg_is(>scan_begin_arg, arg);
}
if (cmd->convert_src == TRIG_TIMER) {
arg = cmd->convert_arg;
/* round to nearest microsec */
-   arg = nano_per_micro *
- ((arg + (nano_per_micro / 2)) / nano_per_micro);
+   arg = NSEC_PER_USEC *
+ ((arg + (NSEC_PER_USEC / 2)) / NSEC_PER_USEC);
err |= comedi_check_trigger_arg_is(>convert_arg, arg);
}
 
@@ -326,12 +323,12 @@ static int waveform_ai_cmd(struct comedi_device *dev,
return -1;
}
 
-   devpriv->scan_period = cmd->scan_begin_arg / nano_per_micro;
+   devpriv->scan_period = cmd->scan_begin_arg / NSEC_PER_USEC;
 
if (cmd->convert_src == TRIG_NOW)
devpriv->convert_period = 0;
else/* TRIG_TIMER */
-   devpriv->convert_period = cmd->convert_arg / nano_per_micro;
+   devpriv->convert_period = cmd->convert_arg / NSEC_PER_USEC;
 
devpriv->last = ktime_get();
devpriv->usec_current =
-- 
2.6.1

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


[PATCH 00/16] staging: comedi: comedi_test: enhancements

2015-10-27 Thread Ian Abbott
The "comedi_test" module is a driver for a dummy COMEDI device.  It has
an analog input subdevice and an analog output subdevice.  The analog
input subdevice supports COMEDI asynchronous acquisition commands using
waveform generators to generate the input data for each channel.  A
kernel timer is used to driver the acquisition.

This series of patches cleans up the driver, enhances the existing
asynchronous command support on the analog input subdevice, and adds
asynchronous command support on the analog output subdevice.

01) staging: comedi: comedi_test: reformat multi-line comments
02) staging: comedi: comedi_test: saturate fake waveform values
03) staging: comedi: comedi_test: remove nano_per_micro
04) staging: comedi: comedi_test: limit maximum convert_arg
05) staging: comedi: comedi_test: support scan_begin_src == TRIG_FOLLOW
06) staging: comedi: comedi_test: move modulo operations for waveform
07) staging: comedi: comedi_test: use unsigned int for waveform timing
08) staging: comedi: comedi_test: simplify time since last AI scan
09) staging: comedi: comedi_test: rename members for AI commands
10) staging: comedi: comedi_test: rename waveform members
11) staging: comedi: comedi_test: make timer rate similar to scan rate
12) staging: comedi: comedi_test: use unsigned short for loopback values
13) staging: comedi: comedi_test: allow read-back of AO channels
14) staging: comedi: comedi_test: handle partial scans in timer routine
15) staging: comedi: comedi_test: rename waveform_ai_interrupt()
16) staging: comedi: comedi_test: implement commands on AO subdevice

 drivers/staging/comedi/drivers/comedi_test.c | 565 ---
 1 file changed, 416 insertions(+), 149 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 15/16] staging: comedi: comedi_test: rename waveform_ai_interrupt()

2015-10-27 Thread Ian Abbott
`waveform_ai_interrupt()` is a timer expiry function used to generate
fake waveform data for an analog input subdevice.  Rename it to
`waveform_ai_timer()`.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 318340c..14a0b62 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -176,7 +176,7 @@ static unsigned short fake_waveform(struct comedi_device 
*dev,
  * It should run in the background; therefore it is scheduled by
  * a timer mechanism.
  */
-static void waveform_ai_interrupt(unsigned long arg)
+static void waveform_ai_timer(unsigned long arg)
 {
struct comedi_device *dev = (struct comedi_device *)arg;
struct waveform_private *devpriv = dev->private;
@@ -486,8 +486,7 @@ static int waveform_attach(struct comedi_device *dev,
for (i = 0; i < s->n_chan; i++)
devpriv->ao_loopbacks[i] = s->maxdata / 2;
 
-   setup_timer(>ai_timer, waveform_ai_interrupt,
-   (unsigned long)dev);
+   setup_timer(>ai_timer, waveform_ai_timer, (unsigned long)dev);
 
dev_info(dev->class_dev,
 "%s: %u microvolt, %u microsecond waveform attached\n",
-- 
2.6.1

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


[PATCH 01/16] staging: comedi: comedi_test: reformat multi-line comments

2015-10-27 Thread Ian Abbott
Use the preferred style for multi-line comments.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 94 ++--
 1 file changed, 47 insertions(+), 47 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 80d613c..899faf7 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -1,49 +1,49 @@
 /*
-comedi/drivers/comedi_test.c
+ * comedi/drivers/comedi_test.c
+ *
+ * Generates fake waveform signals that can be read through
+ * the command interface.  It does _not_ read from any board;
+ * it just generates deterministic waveforms.
+ * Useful for various testing purposes.
+ *
+ * Copyright (C) 2002 Joachim Wuttke <joachim.wut...@icn.siemens.de>
+ * Copyright (C) 2002 Frank Mori Hess <fmh...@users.sourceforge.net>
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 2000 David A. Schleef <d...@schleef.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
 
-Generates fake waveform signals that can be read through
-the command interface.  It does _not_ read from any board;
-it just generates deterministic waveforms.
-Useful for various testing purposes.
-
-Copyright (C) 2002 Joachim Wuttke <joachim.wut...@icn.siemens.de>
-Copyright (C) 2002 Frank Mori Hess <fmh...@users.sourceforge.net>
-
-COMEDI - Linux Control and Measurement Device Interface
-Copyright (C) 2000 David A. Schleef <d...@schleef.org>
-
-This program is free software; you can redistribute it and/or modify
-it under the terms of the GNU General Public License as published by
-the Free Software Foundation; either version 2 of the License, or
-(at your option) any later version.
-
-This program is distributed in the hope that it will be useful,
-but WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
-*/
 /*
-Driver: comedi_test
-Description: generates fake waveforms
-Author: Joachim Wuttke <joachim.wut...@icn.siemens.de>, Frank Mori Hess
-  <fmh...@users.sourceforge.net>, ds
-Devices:
-Status: works
-Updated: Sat, 16 Mar 2002 17:34:48 -0800
-
-This driver is mainly for testing purposes, but can also be used to
-generate sample waveforms on systems that don't have data acquisition
-hardware.
-
-Configuration options:
-  [0] - Amplitude in microvolts for fake waveforms (default 1 volt)
-  [1] - Period in microseconds for fake waveforms (default 0.1 sec)
-
-Generates a sawtooth wave on channel 0, square wave on channel 1, additional
-waveforms could be added to other channels (currently they return flatline
-zero volts).
-
-*/
+ * Driver: comedi_test
+ * Description: generates fake waveforms
+ * Author: Joachim Wuttke <joachim.wut...@icn.siemens.de>, Frank Mori Hess
+ *   <fmh...@users.sourceforge.net>, ds
+ * Devices:
+ * Status: works
+ * Updated: Sat, 16 Mar 2002 17:34:48 -0800
+ *
+ * This driver is mainly for testing purposes, but can also be used to
+ * generate sample waveforms on systems that don't have data acquisition
+ * hardware.
+ *
+ * Configuration options:
+ *   [0] - Amplitude in microvolts for fake waveforms (default 1 volt)
+ *   [1] - Period in microseconds for fake waveforms (default 0.1 sec)
+ *
+ * Generates a sawtooth wave on channel 0, square wave on channel 1, additional
+ * waveforms could be added to other channels (currently they return flatline
+ * zero volts).
+ */
 
 #include 
 #include "../comedidev.h"
@@ -160,10 +160,10 @@ static unsigned short fake_waveform(struct comedi_device 
*dev,
 }
 
 /*
-   This is the background routine used to generate arbitrary data.
-   It should run in the background; therefore it is scheduled by
-   a timer mechanism.
-*/
+ * This is the background routine used to generate arbitrary data.
+ * It should run in the background; therefore it is scheduled by
+ * a timer mechanism.
+ */
 static void waveform_ai_interrupt(unsigned long arg)
 {
struct comedi_device *dev = (struct comedi_device *)arg;
-- 
2.6.1

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


[PATCH 06/16] staging: comedi: comedi_test: move modulo operations for waveform

2015-10-27 Thread Ian Abbott
The fake waveform generator functions, `fake_sawtooth()` and
`fake_squarewave()`, called from `fake_waveform()`, have a
`current_time` parameter which is the time since the start of a waveform
period.  The parameter value may be greater than the waveform period so
they do a modulo operation to bring it into range.  Do the modulo
operations outside the functions in `waveform_ai_interrupt()` so that
the waveform generator functions always get a `current_time` parameter
less than the waveform period and do not have to do the modulo operation
themselves.  Also, only do the modulo operations when the time since the
start of a waveform exceeds the waveform period.  Usually, several
samples are produced in each waveform period and modulo operations are
typically more expensive than a simple comparison.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index cc35bd6..0215228 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -97,7 +97,6 @@ static unsigned short fake_sawtooth(struct comedi_device *dev,
binary_amplitude *= devpriv->uvolt_amplitude;
do_div(binary_amplitude, krange->max - krange->min);
 
-   current_time %= devpriv->usec_period;
value = current_time;
value *= binary_amplitude * 2;
do_div(value, devpriv->usec_period);
@@ -125,7 +124,6 @@ static unsigned short fake_squarewave(struct comedi_device 
*dev,
const struct comedi_krange *krange =
>range_table->range[range_index];
 
-   current_time %= devpriv->usec_period;
value = s->maxdata;
value *= devpriv->uvolt_amplitude;
do_div(value, krange->max - krange->min);
@@ -206,20 +204,24 @@ static void waveform_ai_interrupt(unsigned long arg)
 
num_scans = comedi_nscans_left(s, num_scans);
for (i = 0; i < num_scans; i++) {
+   unsigned long scan_remain_period = devpriv->scan_period;
+
for (j = 0; j < cmd->chanlist_len; j++) {
unsigned short sample;
 
+   if (devpriv->usec_current >= devpriv->usec_period)
+   devpriv->usec_current %= devpriv->usec_period;
sample = fake_waveform(dev, CR_CHAN(cmd->chanlist[j]),
   CR_RANGE(cmd->chanlist[j]),
-  devpriv->usec_current +
-  i * devpriv->scan_period +
-  j * devpriv->convert_period);
+  devpriv->usec_current);
comedi_buf_write_samples(s, , 1);
+   devpriv->usec_current += devpriv->convert_period;
+   scan_remain_period -= devpriv->convert_period;
}
+   devpriv->usec_current += scan_remain_period;
}
-
-   devpriv->usec_current += elapsed_time;
-   devpriv->usec_current %= devpriv->usec_period;
+   if (devpriv->usec_current >= devpriv->usec_period)
+   devpriv->usec_current %= devpriv->usec_period;
 
if (cmd->stop_src == TRIG_COUNT && async->scans_done >= cmd->stop_arg)
async->events |= COMEDI_CB_EOA;
-- 
2.6.1

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


[PATCH 07/16] staging: comedi: comedi_test: use unsigned int for waveform timing

2015-10-27 Thread Ian Abbott
Use `unsigned int` instead of `unsigned long` to hold the period of the
fake waveform generator and the current time within each waveform.  The
waveform period will be no more than `INT_MAX` and the current time
within the waveform (prior to the modulo operation to bring it actually
within the waveform period) will be no more than `INT_MAX + UINT_MAX /
1000`.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 0215228..158e090 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -64,8 +64,8 @@ struct waveform_private {
struct timer_list timer;
ktime_t last;   /* time last timer interrupt occurred */
unsigned int uvolt_amplitude;   /* waveform amplitude in microvolts */
-   unsigned long usec_period;  /* waveform period in microseconds */
-   unsigned long usec_current; /* current time (mod waveform period) */
+   unsigned int usec_period;   /* waveform period in microseconds */
+   unsigned int usec_current;  /* current time (mod waveform period) */
unsigned long usec_remainder;   /* usec since last scan */
unsigned long state_bits;
unsigned int scan_period;   /* scan period in usec */
@@ -83,7 +83,7 @@ static const struct comedi_lrange waveform_ai_ranges = {
 
 static unsigned short fake_sawtooth(struct comedi_device *dev,
unsigned int range_index,
-   unsigned long current_time)
+   unsigned int current_time)
 {
struct waveform_private *devpriv = dev->private;
struct comedi_subdevice *s = dev->read_subdev;
@@ -115,7 +115,7 @@ static unsigned short fake_sawtooth(struct comedi_device 
*dev,
 
 static unsigned short fake_squarewave(struct comedi_device *dev,
  unsigned int range_index,
- unsigned long current_time)
+ unsigned int current_time)
 {
struct waveform_private *devpriv = dev->private;
struct comedi_subdevice *s = dev->read_subdev;
@@ -145,7 +145,7 @@ static unsigned short fake_squarewave(struct comedi_device 
*dev,
 
 static unsigned short fake_flatline(struct comedi_device *dev,
unsigned int range_index,
-   unsigned long current_time)
+   unsigned int current_time)
 {
return dev->read_subdev->maxdata / 2;
 }
@@ -153,7 +153,7 @@ static unsigned short fake_flatline(struct comedi_device 
*dev,
 /* generates a different waveform depending on what channel is read */
 static unsigned short fake_waveform(struct comedi_device *dev,
unsigned int channel, unsigned int range,
-   unsigned long current_time)
+   unsigned int current_time)
 {
enum {
SAWTOOTH_CHAN,
@@ -468,7 +468,7 @@ static int waveform_attach(struct comedi_device *dev,
(unsigned long)dev);
 
dev_info(dev->class_dev,
-"%s: %i microvolt, %li microsecond waveform attached\n",
+"%s: %u microvolt, %u microsecond waveform attached\n",
 dev->board_name,
 devpriv->uvolt_amplitude, devpriv->usec_period);
 
-- 
2.6.1

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


[PATCH 16/16] staging: comedi: comedi_test: implement commands on AO subdevice

2015-10-27 Thread Ian Abbott
Implement COMEDI asynchronous commands on the fake analog output
subdevice.  This is useful for testing asynchronous commands in the
"write" direction when no real hardware is available.

A normal kernel timer is used to drive the command.  The new timer
expiry function `waveform_ao_timer()` handles whole "scans" at a time
according to the number of scan period that have elapsed since the last
scan.  Data for each channel in the scan is written to the internal
loopback array `devpriv->ao_loopbacks[]` and can be read back on the
analog input channels.  However, if several scan periods are outstanding
in the timer expiry function, only the latest available scan data is
written to the loopback array in order to save processing time.  The
expiry function also checks for underrun conditions, and checks for
normal termination of the asynchronous command when a "stop" scan count
is reached.

After the command is tested by `waveform_ao_cmdtest()` and set up by
`waveform_ao_cmd()`, it is not started until an internal trigger
function `waveform_ao_inttrig_start()` is called as a result of the user
performing an `INSN_INTTRIG` instruction on the subdevice.  The command
is stopped when the "cancel" handler `waveform_ao_cancel()` is called.
This may be due to the command terminating due to completion or an
error, or as a result of the user cancelling the command.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 212 ++-
 1 file changed, 209 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 14a0b62..4ab1866 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -57,7 +57,8 @@
 #define N_CHANS 8
 
 enum waveform_state_bits {
-   WAVEFORM_AI_RUNNING = 0
+   WAVEFORM_AI_RUNNING,
+   WAVEFORM_AO_RUNNING
 };
 
 /* Data unique to this driver */
@@ -70,6 +71,9 @@ struct waveform_private {
unsigned long state_bits;
unsigned int ai_scan_period;/* AI scan period in usec */
unsigned int ai_convert_period; /* AI conversion period in usec */
+   struct timer_list ao_timer; /* timer for AO commands */
+   u64 ao_last_scan_time;  /* time of previous AO scan in usec */
+   unsigned int ao_scan_period;/* AO scan period in usec */
unsigned short ao_loopbacks[N_CHANS];
 };
 
@@ -417,6 +421,201 @@ static int waveform_ai_insn_read(struct comedi_device 
*dev,
return insn->n;
 }
 
+/*
+ * This is the background routine to handle AO commands, scheduled by
+ * a timer mechanism.
+ */
+static void waveform_ao_timer(unsigned long arg)
+{
+   struct comedi_device *dev = (struct comedi_device *)arg;
+   struct waveform_private *devpriv = dev->private;
+   struct comedi_subdevice *s = dev->write_subdev;
+   struct comedi_async *async = s->async;
+   struct comedi_cmd *cmd = >cmd;
+   u64 now;
+   u64 scans_since;
+   unsigned int scans_avail = 0;
+
+   /* check command is still active */
+   if (!test_bit(WAVEFORM_AO_RUNNING, >state_bits))
+   return;
+
+   /* determine number of scan periods since last time */
+   now = ktime_to_us(ktime_get());
+   scans_since = now - devpriv->ao_last_scan_time;
+   do_div(scans_since, devpriv->ao_scan_period);
+   if (scans_since) {
+   unsigned int i;
+
+   /* determine scans in buffer, limit to scans to do this time */
+   scans_avail = comedi_nscans_left(s, 0);
+   if (scans_avail > scans_since)
+   scans_avail = scans_since;
+   if (scans_avail) {
+   /* skip all but the last scan to save processing time */
+   if (scans_avail > 1) {
+   unsigned int skip_bytes, nbytes;
+
+   skip_bytes =
+   comedi_samples_to_bytes(s, cmd->scan_end_arg *
+  (scans_avail - 1));
+   nbytes = comedi_buf_read_alloc(s, skip_bytes);
+   comedi_buf_read_free(s, nbytes);
+   comedi_inc_scan_progress(s, nbytes);
+   if (nbytes < skip_bytes) {
+   /* unexpected underrun! (cancelled?) */
+   async->events |= COMEDI_CB_OVERFLOW;
+   goto underrun;
+   }
+   }
+   /* output the last scan */
+   for (i = 0; i < cmd->scan_end_arg; i++) {
+   unsigned int cha

[PATCH 14/16] staging: comedi: comedi_test: handle partial scans in timer routine

2015-10-27 Thread Ian Abbott
For asynchronous command handling on the analog input subdevice, a
kernel timer routine is used to generate the fake waveform data.  A
"scan" consists of a number of conversions separated in time by a
conversion period.  Successive scans are separated in time by a scan
period, which is at least the conversion period multiplied by the number
of conversions per scan.  Currently, the timer routine does not generate
any data until the end of a scan period, generating whole scans of data
at a time.  Change it to generate data at the end of each conversion
period, with an extra delay after the final conversion in each scan if
necessary.  Use new member `ai_convert_time` in the private data
structure `struct waveform_private` to keep track of when the next
conversion is due.  This replaces the old member `ai_last_scan_time`
which kept track of the time of the previous scan.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 85 ++--
 1 file changed, 54 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index 468847a..318340c 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -63,7 +63,7 @@ enum waveform_state_bits {
 /* Data unique to this driver */
 struct waveform_private {
struct timer_list ai_timer; /* timer for AI commands */
-   u64 ai_last_scan_time;  /* time of last AI scan in usec */
+   u64 ai_convert_time;/* time of next AI conversion in usec */
unsigned int wf_amplitude;  /* waveform amplitude in microvolts */
unsigned int wf_period; /* waveform period in microseconds */
unsigned int wf_current;/* current time in waveform period */
@@ -183,46 +183,51 @@ static void waveform_ai_interrupt(unsigned long arg)
struct comedi_subdevice *s = dev->read_subdev;
struct comedi_async *async = s->async;
struct comedi_cmd *cmd = >cmd;
-   unsigned int i, j;
-   unsigned long elapsed_time;
-   unsigned int num_scans;
+   u64 now;
+   unsigned int nsamples;
+   unsigned int time_increment;
 
/* check command is still active */
if (!test_bit(WAVEFORM_AI_RUNNING, >state_bits))
return;
 
-   elapsed_time = ktime_to_us(ktime_get()) - devpriv->ai_last_scan_time;
-   num_scans = elapsed_time / devpriv->ai_scan_period;
-
-   num_scans = comedi_nscans_left(s, num_scans);
-   for (i = 0; i < num_scans; i++) {
-   unsigned int scan_remain_period = devpriv->ai_scan_period;
-
-   for (j = 0; j < cmd->chanlist_len; j++) {
-   unsigned short sample;
-
-   if (devpriv->wf_current >= devpriv->wf_period)
-   devpriv->wf_current %= devpriv->wf_period;
-   sample = fake_waveform(dev, CR_CHAN(cmd->chanlist[j]),
-  CR_RANGE(cmd->chanlist[j]),
-  devpriv->wf_current);
-   comedi_buf_write_samples(s, , 1);
-   devpriv->wf_current += devpriv->ai_convert_period;
-   scan_remain_period -= devpriv->ai_convert_period;
+   now = ktime_to_us(ktime_get());
+   nsamples = comedi_nsamples_left(s, UINT_MAX);
+
+   while (nsamples && devpriv->ai_convert_time < now) {
+   unsigned int chanspec = cmd->chanlist[async->cur_chan];
+   unsigned short sample;
+
+   sample = fake_waveform(dev, CR_CHAN(chanspec),
+  CR_RANGE(chanspec), devpriv->wf_current);
+   if (comedi_buf_write_samples(s, , 1) == 0)
+   goto overrun;
+   time_increment = devpriv->ai_convert_period;
+   if (async->scan_progress == 0) {
+   /* done last conversion in scan, so add dead time */
+   time_increment += devpriv->ai_scan_period -
+ devpriv->ai_convert_period *
+ cmd->scan_end_arg;
}
-   devpriv->wf_current += scan_remain_period;
-   devpriv->ai_last_scan_time += devpriv->ai_scan_period;
+   devpriv->wf_current += time_increment;
+   if (devpriv->wf_current >= devpriv->wf_period)
+   devpriv->wf_current %= devpriv->wf_period;
+   devpriv->ai_convert_time += time_increment;
+   nsamples--;
}
-   if (devpriv->wf_current >= devpriv->wf_period)
-   devpriv->wf_current %= devpri

[PATCH 13/16] staging: comedi: comedi_test: allow read-back of AO channels

2015-10-27 Thread Ian Abbott
COMEDI drivers often allow the last value written to a channel on an
analog output subdevice to be read back via the "insn_read" handler.
The "comedi_test" driver does not currently support that.  It is a bit
special because it loops back the last values written to the channel on
the analog output subdevice to be read back via corresponding channels
on the analog input subdevice.  The "insn_read" handler for the analog
input subdevice is `waveform_ai_insn_read()`.  Set that as the
"insn_read" handler for the analog output subdevice as well.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers/comedi_test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/comedi/drivers/comedi_test.c 
b/drivers/staging/comedi/drivers/comedi_test.c
index a750f84..468847a 100644
--- a/drivers/staging/comedi/drivers/comedi_test.c
+++ b/drivers/staging/comedi/drivers/comedi_test.c
@@ -457,6 +457,7 @@ static int waveform_attach(struct comedi_device *dev,
s->maxdata = 0x;
s->range_table = _ai_ranges;
s->insn_write = waveform_ao_insn_write;
+   s->insn_read = waveform_ai_insn_read;   /* do same as AI insn_read */
 
/* Our default loopback value is just a 0V flatline */
for (i = 0; i < s->n_chan; i++)
-- 
2.6.1

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


[PATCH] staging: comedi: fix extreme case of comedi_nsamples_left()

2015-10-23 Thread Ian Abbott
`comedi_nsamples_left(s, nsamples)` returns the number of samples
remaining to complete an asynchronous command or the passed in
`nsamples`, whichever is lower.  However, it goes wrong in the extreme
case of setting the `nsamples` parameter to `UINT_MAX` when the number
of conversions per "scan" (`s->async->cmd.scan_end_arg`) is 1.  It uses
`comedi_nscans_remaining(s, nscans)` to determine the number of scans
remaining, or the parameter `nscans`, whichever is lower.  To determine
the parameter `nscans`, it divides `nsamples` by the number of
conversions per scan and adds 1.  The addition of 1 is to avoid setting
the parameter `nscans` to 0, as `comedi_nscans_remaining(s, nscans)`
treats that value specially.  However in the extreme case where
`nsamples` is `UINT_MAX` and the number of samples per scan is 1, the
addition of 1 to `nscans` overflows, producing the unwanted 0.

Fix it by refactoring new a function `__comedi_nscans_remaining(s,
nscans)` out of `comedi_nscans_remaining(s, nscans)`.  The new function
does everything except the special handling when `nscans` is 0.  Change
`comedi_nsamples_remaining()` to call the new function without adding 1
to `nscans` to avoid the overflow.

This overflow bug doesn't affect any of the current COMEDI drivers.  I
stumbled across it while changing to one of the drivers.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/drivers.c | 38 +-
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index aae9481..b63dd2e 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -425,6 +425,24 @@ unsigned int comedi_bytes_per_scan(struct comedi_subdevice 
*s)
 }
 EXPORT_SYMBOL_GPL(comedi_bytes_per_scan);
 
+static unsigned int __comedi_nscans_left(struct comedi_subdevice *s,
+unsigned int nscans)
+{
+   struct comedi_async *async = s->async;
+   struct comedi_cmd *cmd = >cmd;
+
+   if (cmd->stop_src == TRIG_COUNT) {
+   unsigned int scans_left = 0;
+
+   if (async->scans_done < cmd->stop_arg)
+   scans_left = cmd->stop_arg - async->scans_done;
+
+   if (nscans > scans_left)
+   nscans = scans_left;
+   }
+   return nscans;
+}
+
 /**
  * comedi_nscans_left() - Return the number of scans left in the command
  * @s: COMEDI subdevice.
@@ -442,25 +460,12 @@ EXPORT_SYMBOL_GPL(comedi_bytes_per_scan);
 unsigned int comedi_nscans_left(struct comedi_subdevice *s,
unsigned int nscans)
 {
-   struct comedi_async *async = s->async;
-   struct comedi_cmd *cmd = >cmd;
-
if (nscans == 0) {
unsigned int nbytes = comedi_buf_read_n_available(s);
 
nscans = nbytes / comedi_bytes_per_scan(s);
}
-
-   if (cmd->stop_src == TRIG_COUNT) {
-   unsigned int scans_left = 0;
-
-   if (async->scans_done < cmd->stop_arg)
-   scans_left = cmd->stop_arg - async->scans_done;
-
-   if (nscans > scans_left)
-   nscans = scans_left;
-   }
-   return nscans;
+   return __comedi_nscans_left(s, nscans);
 }
 EXPORT_SYMBOL_GPL(comedi_nscans_left);
 
@@ -479,9 +484,8 @@ unsigned int comedi_nsamples_left(struct comedi_subdevice 
*s,
struct comedi_cmd *cmd = >cmd;
 
if (cmd->stop_src == TRIG_COUNT) {
-   /* +1 to force comedi_nscans_left() to return the scans left */
-   unsigned int nscans = (nsamples / cmd->scan_end_arg) + 1;
-   unsigned int scans_left = comedi_nscans_left(s, nscans);
+   unsigned int nscans = nsamples / cmd->scan_end_arg;
+   unsigned int scans_left = __comedi_nscans_left(s, nscans);
unsigned int scan_pos =
comedi_bytes_to_samples(s, async->scan_progress);
unsigned long long samples_left = 0;
-- 
2.6.1

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


[PATCH] staging: comedi: fix extreme case of comedi_nsamples_left()

2015-10-23 Thread Ian Abbott
`comedi_nsamples_left(s, nsamples)` returns the number of samples
remaining to complete an asynchronous command or the passed in
`nsamples`, whichever is lower.  However, it goes wrong in the extreme
case of setting the `nsamples` parameter to `UINT_MAX` when the number
of conversions per "scan" (`s->async->cmd.scan_end_arg`) is 1.  It uses
`comedi_nscans_remaining(s, nscans)` to determine the number of scans
remaining, or the parameter `nscans`, whichever is lower.  To determine
the parameter `nscans`, it divides `nsamples` by the number of
conversions per scan and adds 1.  The addition of 1 is to avoid setting
the parameter `nscans` to 0, as `comedi_nscans_remaining(s, nscans)`
treats that value specially.  However in the extreme case where
`nsamples` is `UINT_MAX` and the number of samples per scan is 1, the
addition of 1 to `nscans` overflows, producing the unwanted 0.

Fix it by refactoring new a function `__comedi_nscans_remaining(s,
nscans)` out of `comedi_nscans_remaining(s, nscans)`.  The new function
does everything except the special handling when `nscans` is 0.  Change
`comedi_nsamples_remaining()` to call the new function without adding 1
to `nscans` to avoid the overflow.

This overflow bug doesn't affect any of the current COMEDI drivers.  I
stumbled across it while changing to one of the drivers.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/drivers.c | 38 +-
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index aae9481..b63dd2e 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -425,6 +425,24 @@ unsigned int comedi_bytes_per_scan(struct comedi_subdevice 
*s)
 }
 EXPORT_SYMBOL_GPL(comedi_bytes_per_scan);
 
+static unsigned int __comedi_nscans_left(struct comedi_subdevice *s,
+unsigned int nscans)
+{
+   struct comedi_async *async = s->async;
+   struct comedi_cmd *cmd = >cmd;
+
+   if (cmd->stop_src == TRIG_COUNT) {
+   unsigned int scans_left = 0;
+
+   if (async->scans_done < cmd->stop_arg)
+   scans_left = cmd->stop_arg - async->scans_done;
+
+   if (nscans > scans_left)
+   nscans = scans_left;
+   }
+   return nscans;
+}
+
 /**
  * comedi_nscans_left() - Return the number of scans left in the command
  * @s: COMEDI subdevice.
@@ -442,25 +460,12 @@ EXPORT_SYMBOL_GPL(comedi_bytes_per_scan);
 unsigned int comedi_nscans_left(struct comedi_subdevice *s,
unsigned int nscans)
 {
-   struct comedi_async *async = s->async;
-   struct comedi_cmd *cmd = >cmd;
-
if (nscans == 0) {
unsigned int nbytes = comedi_buf_read_n_available(s);
 
nscans = nbytes / comedi_bytes_per_scan(s);
}
-
-   if (cmd->stop_src == TRIG_COUNT) {
-   unsigned int scans_left = 0;
-
-   if (async->scans_done < cmd->stop_arg)
-   scans_left = cmd->stop_arg - async->scans_done;
-
-   if (nscans > scans_left)
-   nscans = scans_left;
-   }
-   return nscans;
+   return __comedi_nscans_left(s, nscans);
 }
 EXPORT_SYMBOL_GPL(comedi_nscans_left);
 
@@ -479,9 +484,8 @@ unsigned int comedi_nsamples_left(struct comedi_subdevice 
*s,
struct comedi_cmd *cmd = >cmd;
 
if (cmd->stop_src == TRIG_COUNT) {
-   /* +1 to force comedi_nscans_left() to return the scans left */
-   unsigned int nscans = (nsamples / cmd->scan_end_arg) + 1;
-   unsigned int scans_left = comedi_nscans_left(s, nscans);
+   unsigned int nscans = nsamples / cmd->scan_end_arg;
+   unsigned int scans_left = __comedi_nscans_left(s, nscans);
unsigned int scan_pos =
comedi_bytes_to_samples(s, async->scan_progress);
unsigned long long samples_left = 0;
-- 
2.6.1

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


Re: [PATCH 1/3] staging: comedi: make ni_tio_has_gate2_registers return boolean

2015-10-21 Thread Ian Abbott

On 18/10/15 15:35, Geliang Tang wrote:

This patch makes ni_tio_has_gate2_registers return boolean, since
this function only uses either one or zero as its return value.

Signed-off-by: Geliang Tang 
---
  drivers/staging/comedi/drivers/ni_tio.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_tio.c 
b/drivers/staging/comedi/drivers/ni_tio.c
index c20c51b..b74e44e 100644
--- a/drivers/staging/comedi/drivers/ni_tio.c
+++ b/drivers/staging/comedi/drivers/ni_tio.c
@@ -167,15 +167,15 @@ static inline unsigned GI_HW_ARM_SEL_MASK(enum 
ni_gpct_variant variant)
}
  }

-static int ni_tio_has_gate2_registers(const struct ni_gpct_device *counter_dev)
+static bool ni_tio_has_gate2_registers(const struct ni_gpct_device 
*counter_dev)
  {
switch (counter_dev->variant) {
case ni_gpct_variant_e_series:
default:
-   return 0;
+   return false;
case ni_gpct_variant_m_series:
case ni_gpct_variant_660x:
-   return 1;
+   return true;
}
  }




Looks okay!

Reviewed-by: Ian Abbott 

--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/3] staging: comedi: make ni_tio_has_gate2_registers return boolean

2015-10-21 Thread Ian Abbott

On 18/10/15 15:35, Geliang Tang wrote:

This patch makes ni_tio_has_gate2_registers return boolean, since
this function only uses either one or zero as its return value.

Signed-off-by: Geliang Tang <geliangt...@163.com>
---
  drivers/staging/comedi/drivers/ni_tio.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_tio.c 
b/drivers/staging/comedi/drivers/ni_tio.c
index c20c51b..b74e44e 100644
--- a/drivers/staging/comedi/drivers/ni_tio.c
+++ b/drivers/staging/comedi/drivers/ni_tio.c
@@ -167,15 +167,15 @@ static inline unsigned GI_HW_ARM_SEL_MASK(enum 
ni_gpct_variant variant)
}
  }

-static int ni_tio_has_gate2_registers(const struct ni_gpct_device *counter_dev)
+static bool ni_tio_has_gate2_registers(const struct ni_gpct_device 
*counter_dev)
  {
switch (counter_dev->variant) {
case ni_gpct_variant_e_series:
default:
-   return 0;
+   return false;
case ni_gpct_variant_m_series:
case ni_gpct_variant_660x:
-   return 1;
+   return true;
}
  }




Looks okay!

Reviewed-by: Ian Abbott <abbo...@mev.co.uk>

--
-=( Ian Abbott @ MEV Ltd.E-mail: <abbo...@mev.co.uk> )=-
-=(  Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 2/2] staging: comedi: comedidev.h: spaces preferred around that '*'

2015-10-12 Thread Ian Abbott
Fix the checkpatch.pl issues:

CHECK: spaces preferred around that '*' (ctx:VxV)

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedidev.h | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/comedidev.h 
b/drivers/staging/comedi/comedidev.h
index 7a62e97..1158072 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -580,12 +580,12 @@ int comedi_check_chanlist(struct comedi_subdevice *s,
 
 /* range stuff */
 
-#define RANGE(a, b){(a)*1e6, (b)*1e6, 0}
-#define RANGE_ext(a, b){(a)*1e6, (b)*1e6, RF_EXTERNAL}
-#define RANGE_mA(a, b) {(a)*1e6, (b)*1e6, UNIT_mA}
-#define RANGE_unitless(a, b)   {(a)*1e6, (b)*1e6, 0}
-#define BIP_RANGE(a)   {-(a)*1e6, (a)*1e6, 0}
-#define UNI_RANGE(a)   {0, (a)*1e6, 0}
+#define RANGE(a, b){(a) * 1e6, (b) * 1e6, 0}
+#define RANGE_ext(a, b){(a) * 1e6, (b) * 1e6, RF_EXTERNAL}
+#define RANGE_mA(a, b) {(a) * 1e6, (b) * 1e6, UNIT_mA}
+#define RANGE_unitless(a, b)   {(a) * 1e6, (b) * 1e6, 0}
+#define BIP_RANGE(a)   {-(a) * 1e6, (a) * 1e6, 0}
+#define UNI_RANGE(a)   {0, (a) * 1e6, 0}
 
 extern const struct comedi_lrange range_bipolar10;
 extern const struct comedi_lrange range_bipolar5;
-- 
2.6.1

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


[PATCH 0/2] staging: comedi: comedidev.h: fix some checkpatch issues

2015-10-12 Thread Ian Abbott
Fix some checkpatch issues in "comedidev.h".  There are still some
CamelCase warnings, but since those are for the use of "mA" and it
represents "milliamps", I think I'll leave them alone.

1) staging: comedi: comedidev.h: add comments to spin-lock and mutex
2) staging: comedi: comedidev.h: spaces preferred around that '*'

 drivers/staging/comedi/comedidev.h | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 1/2] staging: comedi: comedidev.h: add comments to spin-lock and mutex

2015-10-12 Thread Ian Abbott
Fix the checkpatch.pl issues:

CHECK: spinlock_t definition without comment
CHECK: struct mutes definition withoug comment

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedidev.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/comedidev.h 
b/drivers/staging/comedi/comedidev.h
index 6062493..7a62e97 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -174,7 +174,7 @@ struct comedi_subdevice {
void *lock;
void *busy;
unsigned runflags;
-   spinlock_t spin_lock;
+   spinlock_t spin_lock;   /* generic spin-lock for COMEDI and drivers */
 
unsigned int io_bits;
 
@@ -537,8 +537,8 @@ struct comedi_device {
const void *board_ptr;
bool attached:1;
bool ioenabled:1;
-   spinlock_t spinlock;
-   struct mutex mutex;
+   spinlock_t spinlock;/* generic spin-lock for low-level driver */
+   struct mutex mutex; /* generic mutex for COMEDI core */
struct rw_semaphore attach_lock;
struct kref refcount;
 
-- 
2.6.1

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


[PATCH 01/10] staging: comedi: remain busy until read end-of-file

2015-10-12 Thread Ian Abbott
If a COMEDI subdevice is busy handling an asynchronous command in the
"read" direction, then after the command has terminated itself, the
"read" file operation handler, `comedi_read()` should keep the subdevice
busy until all available data has been read and it has returned 0 to
indicate an "end-of-file" condition.  Currently, it has a bug where it
can mark the subdevice as non-busy even when returning a non-zero count.
The bug is slightly hidden because the next "read" will return 0 because
the subdevice is no longer busy.  Fix it by checking the return count is
0 before deciding to mark the subdevice as non-busy.

The call to `comedi_is_subdevice_idle()` is superfluous as the
`become_nonbusy` variable will have been set to `true` when considering
becoming non-busy.  Strictly speaking, checking the return count is
superfluous too, as `become_nonbusy` doesn't get set to `true` unless
the count is 0, but check the return count anyway to make the intention
clearer.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index ef4b58b..f533113 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2550,7 +2550,7 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
}
remove_wait_queue(>wait_head, );
set_current_state(TASK_RUNNING);
-   if (become_nonbusy || comedi_is_subdevice_idle(s)) {
+   if (become_nonbusy && count == 0) {
struct comedi_subdevice *new_s;
 
/*
-- 
2.6.1

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


[PATCH 04/10] staging: comedi: make some variables unsigned in comedi_read()

2015-10-12 Thread Ian Abbott
In `comedi_read()`, the `n` and `m` variables are of type `int`.  Change
them to `unsigned int` as they are used to measure a positive number of
bytes.  The `count` variable is also of type `int` and holds the
returned number of bytes.  Change it to type `ssize_t` to match the
function's return type.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index cca3fb1..ae9d519 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2449,7 +2449,9 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
 {
struct comedi_subdevice *s;
struct comedi_async *async;
-   int n, m, count = 0, retval = 0;
+   unsigned int n, m;
+   ssize_t count = 0;
+   int retval = 0;
DECLARE_WAITQUEUE(wait, current);
struct comedi_file *cfp = file->private_data;
struct comedi_device *dev = cfp->dev;
-- 
2.6.1

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


[PATCH 05/10] staging: comedi: avoid bad truncation of a size_t in comedi_read()

2015-10-12 Thread Ian Abbott
At one point in `comedi_read()`, the variable `n` gets assigned to the
minimum of the parameter `nbytes` and the amount of readable buffer
space `m`.  The way that is done currently is unsafe in the unlikely
case that `nbytes` exceeds `UINT_MAX`, so fix it.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index ae9d519..d51c94c 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2492,13 +2492,10 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
while (nbytes > 0 && !retval) {
set_current_state(TASK_INTERRUPTIBLE);
 
-   n = nbytes;
-
m = comedi_buf_read_n_available(s);
if (async->buf_read_ptr + m > async->prealloc_bufsz)
m = async->prealloc_bufsz - async->buf_read_ptr;
-   if (m < n)
-   n = m;
+   n = min_t(size_t, m, nbytes);
 
if (n == 0) {
unsigned runflags = comedi_get_subdevice_runflags(s);
-- 
2.6.1

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


[PATCH 03/10] staging: comedi: do extra checks for becoming non-busy for "read"

2015-10-12 Thread Ian Abbott
`comedi_read()` is the handler for the "read" file operation for COMEDI
devices.  It mostly runs without using the main mutex of the COMEDI
device, but uses the `attach_lock` rwsemaphore to protect against the
COMEDI device becoming "detached".  A file object can read data
resulting from a COMEDI asynchonous command if it initiated the command.
The COMEDI subdevice is marked as busy when the command is started.  At
some point, the "read" handler detects that the command has terminated
and all available data has been read and so marks the subdevice as
non-busy.

In order to mark the subdevice as non-busy, the "read" handler needs to
release the `attach_lock` rwsemaphore and `acquire the main `mutex`.
There is a vulnerable point between the two, so it checks that the
device is still attached after acquiring the mutex.  However, it does
not currently check that the conditions for becoming non-busy still
hold.  Add some more checks that the subdevice is still busy with a
command initiated by the same file object, that command is in the correct
direction (in case the subdevice supports both "read" and "write"), that
command has terminated, and has no data available to be read.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 89e8e87..cca3fb1 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2566,14 +2566,17 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
 * sufficient (unless there have been 2**32 detaches in the
 * meantime!), but check the subdevice pointer as well just in
 * case.
+*
+* Also check the subdevice is still in a suitable state to
+* become non-busy in case it changed behind our back.
 */
new_s = comedi_file_read_subdevice(file);
if (dev->attached && old_detach_count == dev->detach_count &&
-   s == new_s && new_s->async == async) {
-   if (become_nonbusy ||
-   comedi_buf_read_n_available(s) == 0)
-   do_become_nonbusy(dev, s);
-   }
+   s == new_s && new_s->async == async && s->busy == file &&
+   !(async->cmd.flags & CMDF_WRITE) &&
+   !comedi_is_subdevice_running(s) &&
+   comedi_buf_read_n_available(s) == 0)
+   do_become_nonbusy(dev, s);
mutex_unlock(>mutex);
}
 out:
-- 
2.6.1

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


[PATCH 08/10] staging: comedi: return error on "read" if no command set up

2015-10-12 Thread Ian Abbott
The "read" file operation handler, `comedi_read()` returns an error for
pretty much any condition that prevents a "read" going ahead.  One of
the conditions that prevents a "read" going ahead is that no
asynchronous command has been set up, but that currently results in a
return value of 0 (unless COMEDI instructions are being processed or an
asynchronous command has been set up by a different file object).
Change it to return `-EINVAL` in this case.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 9505a34..db88fa5 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2477,8 +2477,12 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
}
 
async = s->async;
-   if (!s->busy || !nbytes)
+   if (!nbytes)
+   goto out;
+   if (!s->busy) {
+   retval = -EINVAL;
goto out;
+   }
if (s->busy != file) {
retval = -EACCES;
goto out;
@@ -2516,6 +2520,7 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
break;
}
if (!s->busy) {
+   retval = -EINVAL;
break;
}
if (s->busy != file) {
-- 
2.6.1

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


[PATCH 09/10] staging: comedi: simplify returned errors for comedi_read()

2015-10-12 Thread Ian Abbott
In order to perform a "read" file operation, an asynchronous COMEDI
command in the "read" direction needs to have been set up by the current
file object on the COMEDI "read" subdevice associated with the file
object.  If there is a "read" subdevice, but a command has not been set
up by the file object (or is has been set-up in the wrong direction),
`comedi_read()` currently returns one of two error values `-EINVAL` or
`-EACCES`.  `-EACCES` is returned if the command was set up by a
different subdevice, or somewhat randomly, if a COMEDI "instruction" is
currently being processed.  `-EINVAL` is returned in other cases.
Simplify it by returning `-EINVAL` for all these cases.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 21 +++--
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index db88fa5..6e8806b4 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2479,15 +2479,7 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
async = s->async;
if (!nbytes)
goto out;
-   if (!s->busy) {
-   retval = -EINVAL;
-   goto out;
-   }
-   if (s->busy != file) {
-   retval = -EACCES;
-   goto out;
-   }
-   if (async->cmd.flags & CMDF_WRITE) {
+   if (s->busy != file || (async->cmd.flags & CMDF_WRITE)) {
retval = -EINVAL;
goto out;
}
@@ -2519,15 +2511,8 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
retval = -ERESTARTSYS;
break;
}
-   if (!s->busy) {
-   retval = -EINVAL;
-   break;
-   }
-   if (s->busy != file) {
-   retval = -EACCES;
-   break;
-   }
-   if (async->cmd.flags & CMDF_WRITE) {
+   if (s->busy != file ||
+   (async->cmd.flags & CMDF_WRITE)) {
retval = -EINVAL;
break;
}
-- 
2.6.1

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


[PATCH 10/10] staging: comedi: check for more errors for zero-length read

2015-10-12 Thread Ian Abbott
If the "read" file operation handler, `comedi_read()` is passed 0 for
the amount to read, some error conditions are currently skipped and the
function just returns 0.  Change it to check those error conditions and
return an error value if appropriate.  The trickiest case is the check
for when the previously set up asynchronous command has terminated with
an error.  In that case, `-EPIPE` is returned (as it is for a read of
non-zero length) and the subdevice gets marked as non-busy.

A zero-length read that returns 0 has no other effects, in particular,
it does not cause the subdevice to be marked as non-busy, and the return
value does not indicate an "end-of-file" condition.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 6e8806b4..9fe5f7b 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2477,15 +2477,13 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
}
 
async = s->async;
-   if (!nbytes)
-   goto out;
if (s->busy != file || (async->cmd.flags & CMDF_WRITE)) {
retval = -EINVAL;
goto out;
}
 
add_wait_queue(>wait_head, );
-   while (nbytes > 0 && !retval) {
+   while (count == 0 && !retval) {
unsigned int rp, n1, n2;
 
set_current_state(TASK_INTERRUPTIBLE);
@@ -2499,9 +2497,12 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
if (!comedi_is_runflags_running(runflags)) {
if (comedi_is_runflags_in_error(runflags))
retval = -EPIPE;
-   become_nonbusy = true;
+   if (retval || nbytes)
+   become_nonbusy = true;
break;
}
+   if (nbytes == 0)
+   break;
if (file->f_flags & O_NONBLOCK) {
retval = -EAGAIN;
break;
@@ -2538,7 +2539,6 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
nbytes -= n;
 
buf += n;
-   break;  /* makes device work like a pipe */
}
remove_wait_queue(>wait_head, );
set_current_state(TASK_RUNNING);
-- 
2.6.1

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


[PATCH 02/10] staging: comedi: don't consider "unmunged" data when becoming non-busy

2015-10-12 Thread Ian Abbott
If an asynchronous "read" command is no longer running but the subdevice
is still busy, it becomes non-busy once there is no more data available
in the buffer.  Some or all of the data written to the buffer might not
have been "munged" yet, and it cannot be read until it has been munged
by the writer.  However, since the command is no longer running, we
cannot expect any remaining unmunged data to get munged so we should
ignore it.  Call `comedi_buf_read_n_available()` to check the amount of
munged data available to be read, replacing the call to
`comedi_buf_n_bytes_ready()` which checked the amount of written (but
possibly not yet munged) data available to be read.  This affects both
the "read" file operation (done in `comedi_read()`) and the
`COMEDI_BUFINFO` ioctl handling (done in `do_bufinfo_ioctl()`).  (The
latter is used when data is transferred directly through the mmapped
buffer instead of via the "read" file operation.)

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index f533113..89e8e87 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1146,7 +1146,7 @@ static int do_bufinfo_ioctl(struct comedi_device *dev,
comedi_buf_read_free(s, bi.bytes_read);
 
if (comedi_is_subdevice_idle(s) &&
-   comedi_buf_n_bytes_ready(s) == 0) {
+   comedi_buf_read_n_available(s) == 0) {
do_become_nonbusy(dev, s);
}
}
@@ -2570,7 +2570,8 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
new_s = comedi_file_read_subdevice(file);
if (dev->attached && old_detach_count == dev->detach_count &&
s == new_s && new_s->async == async) {
-   if (become_nonbusy || comedi_buf_n_bytes_ready(s) == 0)
+   if (become_nonbusy ||
+   comedi_buf_read_n_available(s) == 0)
do_become_nonbusy(dev, s);
}
mutex_unlock(>mutex);
-- 
2.6.1

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


[PATCH 06/10] staging: comedi: allow buffer wraparound in comedi_read()

2015-10-12 Thread Ian Abbott
`comedi_read()` copies data from the acquisition data buffer, which is
cyclic, to the user buffer using a single call to `copy_to_user()`.  It
currently avoids having to deal with wraparound of the cyclic buffer by
limiting the amount it copies (and the amount returned to the user).
Change it to deal with the wraparound using two calls to
`copy_to_user()` if necessary.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index d51c94c..b534b49 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2490,11 +2490,11 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
 
add_wait_queue(>wait_head, );
while (nbytes > 0 && !retval) {
+   unsigned int rp, n1, n2;
+
set_current_state(TASK_INTERRUPTIBLE);
 
m = comedi_buf_read_n_available(s);
-   if (async->buf_read_ptr + m > async->prealloc_bufsz)
-   m = async->prealloc_bufsz - async->buf_read_ptr;
n = min_t(size_t, m, nbytes);
 
if (n == 0) {
@@ -2531,8 +2531,14 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
}
continue;
}
-   m = copy_to_user(buf, async->prealloc_buf +
-async->buf_read_ptr, n);
+   rp = async->buf_read_ptr;
+   n1 = min(n, async->prealloc_bufsz - rp);
+   n2 = n - n1;
+   m = copy_to_user(buf, async->prealloc_buf + rp, n1);
+   if (m)
+   m += n2;
+   else if (n2)
+   m = copy_to_user(buf + n1, async->prealloc_buf, n2);
if (m) {
n -= m;
retval = -EFAULT;
-- 
2.6.1

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


[PATCH 07/10] staging: comedi: remove superfluous retval = 0 in comedi_read()

2015-10-12 Thread Ian Abbott
`comedi_read()` initializes `retval` to 0.  The other `retval = 0`
assignments are superfluous, so remove them.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index b534b49..9505a34 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2503,8 +2503,6 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
if (!comedi_is_runflags_running(runflags)) {
if (comedi_is_runflags_in_error(runflags))
retval = -EPIPE;
-   else
-   retval = 0;
become_nonbusy = true;
break;
}
@@ -2518,7 +2516,6 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
break;
}
if (!s->busy) {
-   retval = 0;
break;
}
if (s->busy != file) {
-- 
2.6.1

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


[PATCH 00/10] staging: comedi: some comedi_read() changes

2015-10-12 Thread Ian Abbott
Tidy up the "read" file operation handler, `comedi_read()` a bit and
improve the error handling and the "end-of-file" handling.

There are some other changes I want to make, such as switching to the
newer wait API (prepare_to_wait()/finish_wait()) and preventing several
tasks trying to read or write the same subdevice at the same time (but
without using the COMEDI device's main mutex as it is too coarse).
Those changes can wait until after I've cleaned up and improved the
"write" file operation handler a bit.

01) staging: comedi: remain busy until read end-of-file
02) staging: comedi: don't consider "unmunged" data when becoming
non-busy
03) staging: comedi: do extra checks for becoming non-busy for "read"
04) staging: comedi: make some variables unsigned in comedi_read()
05) staging: comedi: avoid bad truncation of a size_t in comedi_read()
06) staging: comedi: allow buffer wraparound in comedi_read()
07) staging: comedi: remove superfluous retval = 0 in comedi_read()
08) staging: comedi: return error on "read" if no command set up
09) staging: comedi: simplify returned errors for comedi_read()
10) staging: comedi: check for more errors for zero-length read

 drivers/staging/comedi/comedi_fops.c | 68 +---
 1 file changed, 32 insertions(+), 36 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 05/10] staging: comedi: avoid bad truncation of a size_t in comedi_read()

2015-10-12 Thread Ian Abbott
At one point in `comedi_read()`, the variable `n` gets assigned to the
minimum of the parameter `nbytes` and the amount of readable buffer
space `m`.  The way that is done currently is unsafe in the unlikely
case that `nbytes` exceeds `UINT_MAX`, so fix it.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index ae9d519..d51c94c 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2492,13 +2492,10 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
while (nbytes > 0 && !retval) {
set_current_state(TASK_INTERRUPTIBLE);
 
-   n = nbytes;
-
m = comedi_buf_read_n_available(s);
if (async->buf_read_ptr + m > async->prealloc_bufsz)
m = async->prealloc_bufsz - async->buf_read_ptr;
-   if (m < n)
-   n = m;
+   n = min_t(size_t, m, nbytes);
 
if (n == 0) {
unsigned runflags = comedi_get_subdevice_runflags(s);
-- 
2.6.1

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


[PATCH 03/10] staging: comedi: do extra checks for becoming non-busy for "read"

2015-10-12 Thread Ian Abbott
`comedi_read()` is the handler for the "read" file operation for COMEDI
devices.  It mostly runs without using the main mutex of the COMEDI
device, but uses the `attach_lock` rwsemaphore to protect against the
COMEDI device becoming "detached".  A file object can read data
resulting from a COMEDI asynchonous command if it initiated the command.
The COMEDI subdevice is marked as busy when the command is started.  At
some point, the "read" handler detects that the command has terminated
and all available data has been read and so marks the subdevice as
non-busy.

In order to mark the subdevice as non-busy, the "read" handler needs to
release the `attach_lock` rwsemaphore and `acquire the main `mutex`.
There is a vulnerable point between the two, so it checks that the
device is still attached after acquiring the mutex.  However, it does
not currently check that the conditions for becoming non-busy still
hold.  Add some more checks that the subdevice is still busy with a
command initiated by the same file object, that command is in the correct
direction (in case the subdevice supports both "read" and "write"), that
command has terminated, and has no data available to be read.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 89e8e87..cca3fb1 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2566,14 +2566,17 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
 * sufficient (unless there have been 2**32 detaches in the
 * meantime!), but check the subdevice pointer as well just in
 * case.
+*
+* Also check the subdevice is still in a suitable state to
+* become non-busy in case it changed behind our back.
 */
new_s = comedi_file_read_subdevice(file);
if (dev->attached && old_detach_count == dev->detach_count &&
-   s == new_s && new_s->async == async) {
-   if (become_nonbusy ||
-   comedi_buf_read_n_available(s) == 0)
-   do_become_nonbusy(dev, s);
-   }
+   s == new_s && new_s->async == async && s->busy == file &&
+   !(async->cmd.flags & CMDF_WRITE) &&
+   !comedi_is_subdevice_running(s) &&
+   comedi_buf_read_n_available(s) == 0)
+   do_become_nonbusy(dev, s);
mutex_unlock(>mutex);
}
 out:
-- 
2.6.1

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


[PATCH 07/10] staging: comedi: remove superfluous retval = 0 in comedi_read()

2015-10-12 Thread Ian Abbott
`comedi_read()` initializes `retval` to 0.  The other `retval = 0`
assignments are superfluous, so remove them.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index b534b49..9505a34 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2503,8 +2503,6 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
if (!comedi_is_runflags_running(runflags)) {
if (comedi_is_runflags_in_error(runflags))
retval = -EPIPE;
-   else
-   retval = 0;
become_nonbusy = true;
break;
}
@@ -2518,7 +2516,6 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
break;
}
if (!s->busy) {
-   retval = 0;
break;
}
if (s->busy != file) {
-- 
2.6.1

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


[PATCH 00/10] staging: comedi: some comedi_read() changes

2015-10-12 Thread Ian Abbott
Tidy up the "read" file operation handler, `comedi_read()` a bit and
improve the error handling and the "end-of-file" handling.

There are some other changes I want to make, such as switching to the
newer wait API (prepare_to_wait()/finish_wait()) and preventing several
tasks trying to read or write the same subdevice at the same time (but
without using the COMEDI device's main mutex as it is too coarse).
Those changes can wait until after I've cleaned up and improved the
"write" file operation handler a bit.

01) staging: comedi: remain busy until read end-of-file
02) staging: comedi: don't consider "unmunged" data when becoming
non-busy
03) staging: comedi: do extra checks for becoming non-busy for "read"
04) staging: comedi: make some variables unsigned in comedi_read()
05) staging: comedi: avoid bad truncation of a size_t in comedi_read()
06) staging: comedi: allow buffer wraparound in comedi_read()
07) staging: comedi: remove superfluous retval = 0 in comedi_read()
08) staging: comedi: return error on "read" if no command set up
09) staging: comedi: simplify returned errors for comedi_read()
10) staging: comedi: check for more errors for zero-length read

 drivers/staging/comedi/comedi_fops.c | 68 +---
 1 file changed, 32 insertions(+), 36 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 01/10] staging: comedi: remain busy until read end-of-file

2015-10-12 Thread Ian Abbott
If a COMEDI subdevice is busy handling an asynchronous command in the
"read" direction, then after the command has terminated itself, the
"read" file operation handler, `comedi_read()` should keep the subdevice
busy until all available data has been read and it has returned 0 to
indicate an "end-of-file" condition.  Currently, it has a bug where it
can mark the subdevice as non-busy even when returning a non-zero count.
The bug is slightly hidden because the next "read" will return 0 because
the subdevice is no longer busy.  Fix it by checking the return count is
0 before deciding to mark the subdevice as non-busy.

The call to `comedi_is_subdevice_idle()` is superfluous as the
`become_nonbusy` variable will have been set to `true` when considering
becoming non-busy.  Strictly speaking, checking the return count is
superfluous too, as `become_nonbusy` doesn't get set to `true` unless
the count is 0, but check the return count anyway to make the intention
clearer.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index ef4b58b..f533113 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2550,7 +2550,7 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
}
remove_wait_queue(>wait_head, );
set_current_state(TASK_RUNNING);
-   if (become_nonbusy || comedi_is_subdevice_idle(s)) {
+   if (become_nonbusy && count == 0) {
struct comedi_subdevice *new_s;
 
/*
-- 
2.6.1

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


[PATCH 04/10] staging: comedi: make some variables unsigned in comedi_read()

2015-10-12 Thread Ian Abbott
In `comedi_read()`, the `n` and `m` variables are of type `int`.  Change
them to `unsigned int` as they are used to measure a positive number of
bytes.  The `count` variable is also of type `int` and holds the
returned number of bytes.  Change it to type `ssize_t` to match the
function's return type.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index cca3fb1..ae9d519 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2449,7 +2449,9 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
 {
struct comedi_subdevice *s;
struct comedi_async *async;
-   int n, m, count = 0, retval = 0;
+   unsigned int n, m;
+   ssize_t count = 0;
+   int retval = 0;
DECLARE_WAITQUEUE(wait, current);
struct comedi_file *cfp = file->private_data;
struct comedi_device *dev = cfp->dev;
-- 
2.6.1

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


[PATCH 10/10] staging: comedi: check for more errors for zero-length read

2015-10-12 Thread Ian Abbott
If the "read" file operation handler, `comedi_read()` is passed 0 for
the amount to read, some error conditions are currently skipped and the
function just returns 0.  Change it to check those error conditions and
return an error value if appropriate.  The trickiest case is the check
for when the previously set up asynchronous command has terminated with
an error.  In that case, `-EPIPE` is returned (as it is for a read of
non-zero length) and the subdevice gets marked as non-busy.

A zero-length read that returns 0 has no other effects, in particular,
it does not cause the subdevice to be marked as non-busy, and the return
value does not indicate an "end-of-file" condition.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 6e8806b4..9fe5f7b 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2477,15 +2477,13 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
}
 
async = s->async;
-   if (!nbytes)
-   goto out;
if (s->busy != file || (async->cmd.flags & CMDF_WRITE)) {
retval = -EINVAL;
goto out;
}
 
add_wait_queue(>wait_head, );
-   while (nbytes > 0 && !retval) {
+   while (count == 0 && !retval) {
unsigned int rp, n1, n2;
 
set_current_state(TASK_INTERRUPTIBLE);
@@ -2499,9 +2497,12 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
if (!comedi_is_runflags_running(runflags)) {
if (comedi_is_runflags_in_error(runflags))
retval = -EPIPE;
-   become_nonbusy = true;
+   if (retval || nbytes)
+   become_nonbusy = true;
break;
}
+   if (nbytes == 0)
+   break;
if (file->f_flags & O_NONBLOCK) {
retval = -EAGAIN;
break;
@@ -2538,7 +2539,6 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
nbytes -= n;
 
buf += n;
-   break;  /* makes device work like a pipe */
}
remove_wait_queue(>wait_head, );
set_current_state(TASK_RUNNING);
-- 
2.6.1

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


[PATCH 02/10] staging: comedi: don't consider "unmunged" data when becoming non-busy

2015-10-12 Thread Ian Abbott
If an asynchronous "read" command is no longer running but the subdevice
is still busy, it becomes non-busy once there is no more data available
in the buffer.  Some or all of the data written to the buffer might not
have been "munged" yet, and it cannot be read until it has been munged
by the writer.  However, since the command is no longer running, we
cannot expect any remaining unmunged data to get munged so we should
ignore it.  Call `comedi_buf_read_n_available()` to check the amount of
munged data available to be read, replacing the call to
`comedi_buf_n_bytes_ready()` which checked the amount of written (but
possibly not yet munged) data available to be read.  This affects both
the "read" file operation (done in `comedi_read()`) and the
`COMEDI_BUFINFO` ioctl handling (done in `do_bufinfo_ioctl()`).  (The
latter is used when data is transferred directly through the mmapped
buffer instead of via the "read" file operation.)

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index f533113..89e8e87 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1146,7 +1146,7 @@ static int do_bufinfo_ioctl(struct comedi_device *dev,
comedi_buf_read_free(s, bi.bytes_read);
 
if (comedi_is_subdevice_idle(s) &&
-   comedi_buf_n_bytes_ready(s) == 0) {
+   comedi_buf_read_n_available(s) == 0) {
do_become_nonbusy(dev, s);
}
}
@@ -2570,7 +2570,8 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
new_s = comedi_file_read_subdevice(file);
if (dev->attached && old_detach_count == dev->detach_count &&
s == new_s && new_s->async == async) {
-   if (become_nonbusy || comedi_buf_n_bytes_ready(s) == 0)
+   if (become_nonbusy ||
+   comedi_buf_read_n_available(s) == 0)
do_become_nonbusy(dev, s);
}
mutex_unlock(>mutex);
-- 
2.6.1

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


[PATCH 06/10] staging: comedi: allow buffer wraparound in comedi_read()

2015-10-12 Thread Ian Abbott
`comedi_read()` copies data from the acquisition data buffer, which is
cyclic, to the user buffer using a single call to `copy_to_user()`.  It
currently avoids having to deal with wraparound of the cyclic buffer by
limiting the amount it copies (and the amount returned to the user).
Change it to deal with the wraparound using two calls to
`copy_to_user()` if necessary.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index d51c94c..b534b49 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2490,11 +2490,11 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
 
add_wait_queue(>wait_head, );
while (nbytes > 0 && !retval) {
+   unsigned int rp, n1, n2;
+
set_current_state(TASK_INTERRUPTIBLE);
 
m = comedi_buf_read_n_available(s);
-   if (async->buf_read_ptr + m > async->prealloc_bufsz)
-   m = async->prealloc_bufsz - async->buf_read_ptr;
n = min_t(size_t, m, nbytes);
 
if (n == 0) {
@@ -2531,8 +2531,14 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
}
continue;
}
-   m = copy_to_user(buf, async->prealloc_buf +
-async->buf_read_ptr, n);
+   rp = async->buf_read_ptr;
+   n1 = min(n, async->prealloc_bufsz - rp);
+   n2 = n - n1;
+   m = copy_to_user(buf, async->prealloc_buf + rp, n1);
+   if (m)
+   m += n2;
+   else if (n2)
+   m = copy_to_user(buf + n1, async->prealloc_buf, n2);
if (m) {
n -= m;
retval = -EFAULT;
-- 
2.6.1

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


[PATCH 08/10] staging: comedi: return error on "read" if no command set up

2015-10-12 Thread Ian Abbott
The "read" file operation handler, `comedi_read()` returns an error for
pretty much any condition that prevents a "read" going ahead.  One of
the conditions that prevents a "read" going ahead is that no
asynchronous command has been set up, but that currently results in a
return value of 0 (unless COMEDI instructions are being processed or an
asynchronous command has been set up by a different file object).
Change it to return `-EINVAL` in this case.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 9505a34..db88fa5 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2477,8 +2477,12 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
}
 
async = s->async;
-   if (!s->busy || !nbytes)
+   if (!nbytes)
+   goto out;
+   if (!s->busy) {
+   retval = -EINVAL;
goto out;
+   }
if (s->busy != file) {
retval = -EACCES;
goto out;
@@ -2516,6 +2520,7 @@ static ssize_t comedi_read(struct file *file, char __user 
*buf, size_t nbytes,
break;
}
if (!s->busy) {
+   retval = -EINVAL;
break;
}
if (s->busy != file) {
-- 
2.6.1

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


[PATCH 09/10] staging: comedi: simplify returned errors for comedi_read()

2015-10-12 Thread Ian Abbott
In order to perform a "read" file operation, an asynchronous COMEDI
command in the "read" direction needs to have been set up by the current
file object on the COMEDI "read" subdevice associated with the file
object.  If there is a "read" subdevice, but a command has not been set
up by the file object (or is has been set-up in the wrong direction),
`comedi_read()` currently returns one of two error values `-EINVAL` or
`-EACCES`.  `-EACCES` is returned if the command was set up by a
different subdevice, or somewhat randomly, if a COMEDI "instruction" is
currently being processed.  `-EINVAL` is returned in other cases.
Simplify it by returning `-EINVAL` for all these cases.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 21 +++--
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index db88fa5..6e8806b4 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2479,15 +2479,7 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
async = s->async;
if (!nbytes)
goto out;
-   if (!s->busy) {
-   retval = -EINVAL;
-   goto out;
-   }
-   if (s->busy != file) {
-   retval = -EACCES;
-   goto out;
-   }
-   if (async->cmd.flags & CMDF_WRITE) {
+   if (s->busy != file || (async->cmd.flags & CMDF_WRITE)) {
retval = -EINVAL;
goto out;
}
@@ -2519,15 +2511,8 @@ static ssize_t comedi_read(struct file *file, char 
__user *buf, size_t nbytes,
retval = -ERESTARTSYS;
break;
}
-   if (!s->busy) {
-   retval = -EINVAL;
-   break;
-   }
-   if (s->busy != file) {
-   retval = -EACCES;
-   break;
-   }
-   if (async->cmd.flags & CMDF_WRITE) {
+   if (s->busy != file ||
+   (async->cmd.flags & CMDF_WRITE)) {
retval = -EINVAL;
break;
}
-- 
2.6.1

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


[PATCH 1/2] staging: comedi: comedidev.h: add comments to spin-lock and mutex

2015-10-12 Thread Ian Abbott
Fix the checkpatch.pl issues:

CHECK: spinlock_t definition without comment
CHECK: struct mutes definition withoug comment

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedidev.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/comedidev.h 
b/drivers/staging/comedi/comedidev.h
index 6062493..7a62e97 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -174,7 +174,7 @@ struct comedi_subdevice {
void *lock;
void *busy;
unsigned runflags;
-   spinlock_t spin_lock;
+   spinlock_t spin_lock;   /* generic spin-lock for COMEDI and drivers */
 
unsigned int io_bits;
 
@@ -537,8 +537,8 @@ struct comedi_device {
const void *board_ptr;
bool attached:1;
bool ioenabled:1;
-   spinlock_t spinlock;
-   struct mutex mutex;
+   spinlock_t spinlock;/* generic spin-lock for low-level driver */
+   struct mutex mutex; /* generic mutex for COMEDI core */
struct rw_semaphore attach_lock;
struct kref refcount;
 
-- 
2.6.1

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


[PATCH 0/2] staging: comedi: comedidev.h: fix some checkpatch issues

2015-10-12 Thread Ian Abbott
Fix some checkpatch issues in "comedidev.h".  There are still some
CamelCase warnings, but since those are for the use of "mA" and it
represents "milliamps", I think I'll leave them alone.

1) staging: comedi: comedidev.h: add comments to spin-lock and mutex
2) staging: comedi: comedidev.h: spaces preferred around that '*'

 drivers/staging/comedi/comedidev.h | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 2/2] staging: comedi: comedidev.h: spaces preferred around that '*'

2015-10-12 Thread Ian Abbott
Fix the checkpatch.pl issues:

CHECK: spaces preferred around that '*' (ctx:VxV)

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedidev.h | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/comedidev.h 
b/drivers/staging/comedi/comedidev.h
index 7a62e97..1158072 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -580,12 +580,12 @@ int comedi_check_chanlist(struct comedi_subdevice *s,
 
 /* range stuff */
 
-#define RANGE(a, b){(a)*1e6, (b)*1e6, 0}
-#define RANGE_ext(a, b){(a)*1e6, (b)*1e6, RF_EXTERNAL}
-#define RANGE_mA(a, b) {(a)*1e6, (b)*1e6, UNIT_mA}
-#define RANGE_unitless(a, b)   {(a)*1e6, (b)*1e6, 0}
-#define BIP_RANGE(a)   {-(a)*1e6, (a)*1e6, 0}
-#define UNI_RANGE(a)   {0, (a)*1e6, 0}
+#define RANGE(a, b){(a) * 1e6, (b) * 1e6, 0}
+#define RANGE_ext(a, b){(a) * 1e6, (b) * 1e6, RF_EXTERNAL}
+#define RANGE_mA(a, b) {(a) * 1e6, (b) * 1e6, UNIT_mA}
+#define RANGE_unitless(a, b)   {(a) * 1e6, (b) * 1e6, 0}
+#define BIP_RANGE(a)   {-(a) * 1e6, (a) * 1e6, 0}
+#define UNI_RANGE(a)   {0, (a) * 1e6, 0}
 
 extern const struct comedi_lrange range_bipolar10;
 extern const struct comedi_lrange range_bipolar5;
-- 
2.6.1

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


[PATCH 0/6] staging: comedi: fix some minor issues with file poll op

2015-10-09 Thread Ian Abbott
A few changes for the "poll" file operation to avoid poll-waiting on the
same subdevice for both read and write (patch 1), avoid allocating write
buffer space unnecessarily and possibly inappropriately (patch 4),
consider whether any active commands belong to the current file object
(patch 5), and avoid using the main mutex (for performance reasons)
(patch 6).

1) staging: comedi: don't poll_wait on same subdevice twice
2) staging: comedi: rename comedi_buf_write_n_available
3) staging: comedi: add new comedi_buf_write_n_available()
4) staging: comedi: don't allocate buffer space when polling for write
5) staging: comedi: check command started by file being polled
6) staging: comedi: don't use mutex when polling file

 drivers/staging/comedi/comedi_buf.c  | 19 +--
 drivers/staging/comedi/comedi_fops.c | 17 +
 drivers/staging/comedi/comedi_internal.h |  1 +
 3 files changed, 23 insertions(+), 14 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 4/6] staging: comedi: don't allocate buffer space when polling for write

2015-10-09 Thread Ian Abbott
When handling the "poll" file operation and checking for `POLLOUT`,
don't allocate space from the buffer for writing, just check that space
is available for writing.  That check is done after checking that an
asynchronous "write" command is running on the subdevice.  Allocating
the buffer space before checking a "write" command is running can cause
problems if the subdevice supports commands in either direction and
currently has an active "read" command.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 9dcd486..4793e30 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2287,10 +2287,9 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
 
if (s != s_read)
poll_wait(file, >async->wait_head, wait);
-   comedi_buf_write_alloc(s, s->async->prealloc_bufsz);
if (!s->busy || !comedi_is_subdevice_running(s) ||
!(s->async->cmd.flags & CMDF_WRITE) ||
-   comedi_buf_write_n_allocated(s) >= bps)
+   comedi_buf_write_n_available(s) >= bps)
mask |= POLLOUT | POLLWRNORM;
}
 
-- 
2.6.1

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


[PATCH 2/6] staging: comedi: rename comedi_buf_write_n_available

2015-10-09 Thread Ian Abbott
Rename the local function `comedi_buf_write_n_available()` to
`comedi_buf_write_n_unalloc()`.  It is the amount of unallocated space
available in the buffer that is available to be allocated for writing
and does not include the space that has already been allocated for
writing.  This is unlike the exported function
`comedi_buf_read_n_available()` which includes the space available to be
allocated for reading plus the space already allocated for reading.  The
new name breaks the unintentional naming symmetry (and also clears the
way for the old name to be reused for a new function).

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_buf.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/comedi_buf.c 
b/drivers/staging/comedi/comedi_buf.c
index d45a4b6..4837559 100644
--- a/drivers/staging/comedi/comedi_buf.c
+++ b/drivers/staging/comedi/comedi_buf.c
@@ -245,7 +245,7 @@ void comedi_buf_reset(struct comedi_subdevice *s)
async->events = 0;
 }
 
-static unsigned int comedi_buf_write_n_available(struct comedi_subdevice *s)
+static unsigned int comedi_buf_write_n_unalloc(struct comedi_subdevice *s)
 {
struct comedi_async *async = s->async;
unsigned int free_end = async->buf_read_count + async->prealloc_bufsz;
@@ -268,10 +268,10 @@ unsigned int comedi_buf_write_alloc(struct 
comedi_subdevice *s,
unsigned int nbytes)
 {
struct comedi_async *async = s->async;
-   unsigned int available = comedi_buf_write_n_available(s);
+   unsigned int unalloc = comedi_buf_write_n_unalloc(s);
 
-   if (nbytes > available)
-   nbytes = available;
+   if (nbytes > unalloc)
+   nbytes = unalloc;
 
async->buf_write_alloc_count += nbytes;
 
@@ -557,8 +557,7 @@ unsigned int comedi_buf_write_samples(struct 
comedi_subdevice *s,
 * If not, clamp the nsamples to the number that will fit, flag the
 * buffer overrun and add the samples that fit.
 */
-   max_samples = comedi_bytes_to_samples(s,
- comedi_buf_write_n_available(s));
+   max_samples = comedi_bytes_to_samples(s, comedi_buf_write_n_unalloc(s));
if (nsamples > max_samples) {
dev_warn(s->device->class_dev, "buffer overrun\n");
s->async->events |= COMEDI_CB_OVERFLOW;
-- 
2.6.1

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


[PATCH 5/6] staging: comedi: check command started by file being polled

2015-10-09 Thread Ian Abbott
Currently, the "poll" file operation checks if an asynchronous "read"
(or "write" command is active on the "read" (or "write" subdevice, but
does not consider whether the command was started from the file object
being polled.  Since that is the only file object able to read (or
write) data, take it into consideration.

With this change, if no read (or write) command is running on the
subdevice, or it is started by a different file object, the file object
is marked as readable (or writeable) regardless, but the read (or write)
file operation will return an error.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 4793e30..07bb197 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2275,7 +2275,7 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
s_read = s;
if (s && s->async) {
poll_wait(file, >async->wait_head, wait);
-   if (!s->busy || !comedi_is_subdevice_running(s) ||
+   if (s->busy != file || !comedi_is_subdevice_running(s) ||
(s->async->cmd.flags & CMDF_WRITE) ||
comedi_buf_read_n_available(s) > 0)
mask |= POLLIN | POLLRDNORM;
@@ -2287,7 +2287,7 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
 
if (s != s_read)
poll_wait(file, >async->wait_head, wait);
-   if (!s->busy || !comedi_is_subdevice_running(s) ||
+   if (s->busy != file || !comedi_is_subdevice_running(s) ||
!(s->async->cmd.flags & CMDF_WRITE) ||
comedi_buf_write_n_available(s) >= bps)
mask |= POLLOUT | POLLWRNORM;
-- 
2.6.1

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


[PATCH 6/6] staging: comedi: don't use mutex when polling file

2015-10-09 Thread Ian Abbott
The main mutex in a comedi device can get held for quite a while when
processing comedi instructions, so for performance reasons, the "read"
and "write" file operations do not use it; they use use the
`attach_lock` rwsemaphore to protect against the comedi device becoming
detached at an inopportune moment.  Do the same for the "poll" file
operation.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 07bb197..88e9334 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2264,7 +2264,7 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
struct comedi_device *dev = cfp->dev;
struct comedi_subdevice *s, *s_read;
 
-   mutex_lock(>mutex);
+   down_read(>attach_lock);
 
if (!dev->attached) {
dev_dbg(dev->class_dev, "no driver attached\n");
@@ -2294,7 +2294,7 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
}
 
 done:
-   mutex_unlock(>mutex);
+   up_read(>attach_lock);
return mask;
 }
 
-- 
2.6.1

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


[PATCH 3/6] staging: comedi: add new comedi_buf_write_n_available()

2015-10-09 Thread Ian Abbott
Add a new function `comedi_buf_write_n_available()` to return the amount
of buffer space available for writing, including space already allocated
by `comedi_buf_write_alloc()` plus any unallocated space available.
This is currently just for internal use by the comedi core, so is not
exported.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_buf.c  | 8 
 drivers/staging/comedi/comedi_internal.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/staging/comedi/comedi_buf.c 
b/drivers/staging/comedi/comedi_buf.c
index 4837559..90c2801 100644
--- a/drivers/staging/comedi/comedi_buf.c
+++ b/drivers/staging/comedi/comedi_buf.c
@@ -253,6 +253,14 @@ static unsigned int comedi_buf_write_n_unalloc(struct 
comedi_subdevice *s)
return free_end - async->buf_write_alloc_count;
 }
 
+unsigned int comedi_buf_write_n_available(struct comedi_subdevice *s)
+{
+   struct comedi_async *async = s->async;
+   unsigned int free_end = async->buf_read_count + async->prealloc_bufsz;
+
+   return free_end - async->buf_write_count;
+}
+
 /**
  * comedi_buf_write_alloc() - Reserve buffer space for writing
  * @s: COMEDI subdevice.
diff --git a/drivers/staging/comedi/comedi_internal.h 
b/drivers/staging/comedi/comedi_internal.h
index cd9437f..3f2c88a 100644
--- a/drivers/staging/comedi/comedi_internal.h
+++ b/drivers/staging/comedi/comedi_internal.h
@@ -31,6 +31,7 @@ void comedi_buf_map_get(struct comedi_buf_map *bm);
 int comedi_buf_map_put(struct comedi_buf_map *bm);
 struct comedi_buf_map *comedi_buf_map_from_subdev_get(
struct comedi_subdevice *s);
+unsigned int comedi_buf_write_n_available(struct comedi_subdevice *s);
 unsigned int comedi_buf_write_n_allocated(struct comedi_subdevice *s);
 void comedi_device_cancel_all(struct comedi_device *dev);
 bool comedi_can_auto_free_spriv(struct comedi_subdevice *s);
-- 
2.6.1

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


[PATCH 1/6] staging: comedi: don't poll_wait on same subdevice twice

2015-10-09 Thread Ian Abbott
Comedi subdevices that support asynchronous acquisition commands have a
wait queue head used for blocking reads or writes and for the poll file
operation.  The comedi device may have several subdevices that support
"read" and/or "write" commands, but each open file object has at most
one "read" subdevice and one "write" subdevice.  It's possible (though
rare) for those to be the same subdevice if the subdevice supports
commands in either direction.  In that case, the "poll" file operation
doesn't really need to do a `poll_wait()` on the same subdevice twice.
Although harmless, it wastes a poll table entry.  Check for that, and
avoid it.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_fops.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index ef4b58b..9dcd486 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2262,7 +2262,7 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
unsigned int mask = 0;
struct comedi_file *cfp = file->private_data;
struct comedi_device *dev = cfp->dev;
-   struct comedi_subdevice *s;
+   struct comedi_subdevice *s, *s_read;
 
mutex_lock(>mutex);
 
@@ -2272,6 +2272,7 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
}
 
s = comedi_file_read_subdevice(file);
+   s_read = s;
if (s && s->async) {
poll_wait(file, >async->wait_head, wait);
if (!s->busy || !comedi_is_subdevice_running(s) ||
@@ -2284,7 +2285,8 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
if (s && s->async) {
unsigned int bps = comedi_bytes_per_sample(s);
 
-   poll_wait(file, >async->wait_head, wait);
+   if (s != s_read)
+   poll_wait(file, >async->wait_head, wait);
comedi_buf_write_alloc(s, s->async->prealloc_bufsz);
if (!s->busy || !comedi_is_subdevice_running(s) ||
!(s->async->cmd.flags & CMDF_WRITE) ||
-- 
2.6.1

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


[PATCH 1/6] staging: comedi: don't poll_wait on same subdevice twice

2015-10-09 Thread Ian Abbott
Comedi subdevices that support asynchronous acquisition commands have a
wait queue head used for blocking reads or writes and for the poll file
operation.  The comedi device may have several subdevices that support
"read" and/or "write" commands, but each open file object has at most
one "read" subdevice and one "write" subdevice.  It's possible (though
rare) for those to be the same subdevice if the subdevice supports
commands in either direction.  In that case, the "poll" file operation
doesn't really need to do a `poll_wait()` on the same subdevice twice.
Although harmless, it wastes a poll table entry.  Check for that, and
avoid it.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index ef4b58b..9dcd486 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2262,7 +2262,7 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
unsigned int mask = 0;
struct comedi_file *cfp = file->private_data;
struct comedi_device *dev = cfp->dev;
-   struct comedi_subdevice *s;
+   struct comedi_subdevice *s, *s_read;
 
mutex_lock(>mutex);
 
@@ -2272,6 +2272,7 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
}
 
s = comedi_file_read_subdevice(file);
+   s_read = s;
if (s && s->async) {
poll_wait(file, >async->wait_head, wait);
if (!s->busy || !comedi_is_subdevice_running(s) ||
@@ -2284,7 +2285,8 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
if (s && s->async) {
unsigned int bps = comedi_bytes_per_sample(s);
 
-   poll_wait(file, >async->wait_head, wait);
+   if (s != s_read)
+   poll_wait(file, >async->wait_head, wait);
comedi_buf_write_alloc(s, s->async->prealloc_bufsz);
if (!s->busy || !comedi_is_subdevice_running(s) ||
!(s->async->cmd.flags & CMDF_WRITE) ||
-- 
2.6.1

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


[PATCH 6/6] staging: comedi: don't use mutex when polling file

2015-10-09 Thread Ian Abbott
The main mutex in a comedi device can get held for quite a while when
processing comedi instructions, so for performance reasons, the "read"
and "write" file operations do not use it; they use use the
`attach_lock` rwsemaphore to protect against the comedi device becoming
detached at an inopportune moment.  Do the same for the "poll" file
operation.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 07bb197..88e9334 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2264,7 +2264,7 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
struct comedi_device *dev = cfp->dev;
struct comedi_subdevice *s, *s_read;
 
-   mutex_lock(>mutex);
+   down_read(>attach_lock);
 
if (!dev->attached) {
dev_dbg(dev->class_dev, "no driver attached\n");
@@ -2294,7 +2294,7 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
}
 
 done:
-   mutex_unlock(>mutex);
+   up_read(>attach_lock);
return mask;
 }
 
-- 
2.6.1

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


[PATCH 3/6] staging: comedi: add new comedi_buf_write_n_available()

2015-10-09 Thread Ian Abbott
Add a new function `comedi_buf_write_n_available()` to return the amount
of buffer space available for writing, including space already allocated
by `comedi_buf_write_alloc()` plus any unallocated space available.
This is currently just for internal use by the comedi core, so is not
exported.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_buf.c  | 8 
 drivers/staging/comedi/comedi_internal.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/staging/comedi/comedi_buf.c 
b/drivers/staging/comedi/comedi_buf.c
index 4837559..90c2801 100644
--- a/drivers/staging/comedi/comedi_buf.c
+++ b/drivers/staging/comedi/comedi_buf.c
@@ -253,6 +253,14 @@ static unsigned int comedi_buf_write_n_unalloc(struct 
comedi_subdevice *s)
return free_end - async->buf_write_alloc_count;
 }
 
+unsigned int comedi_buf_write_n_available(struct comedi_subdevice *s)
+{
+   struct comedi_async *async = s->async;
+   unsigned int free_end = async->buf_read_count + async->prealloc_bufsz;
+
+   return free_end - async->buf_write_count;
+}
+
 /**
  * comedi_buf_write_alloc() - Reserve buffer space for writing
  * @s: COMEDI subdevice.
diff --git a/drivers/staging/comedi/comedi_internal.h 
b/drivers/staging/comedi/comedi_internal.h
index cd9437f..3f2c88a 100644
--- a/drivers/staging/comedi/comedi_internal.h
+++ b/drivers/staging/comedi/comedi_internal.h
@@ -31,6 +31,7 @@ void comedi_buf_map_get(struct comedi_buf_map *bm);
 int comedi_buf_map_put(struct comedi_buf_map *bm);
 struct comedi_buf_map *comedi_buf_map_from_subdev_get(
struct comedi_subdevice *s);
+unsigned int comedi_buf_write_n_available(struct comedi_subdevice *s);
 unsigned int comedi_buf_write_n_allocated(struct comedi_subdevice *s);
 void comedi_device_cancel_all(struct comedi_device *dev);
 bool comedi_can_auto_free_spriv(struct comedi_subdevice *s);
-- 
2.6.1

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


[PATCH 2/6] staging: comedi: rename comedi_buf_write_n_available

2015-10-09 Thread Ian Abbott
Rename the local function `comedi_buf_write_n_available()` to
`comedi_buf_write_n_unalloc()`.  It is the amount of unallocated space
available in the buffer that is available to be allocated for writing
and does not include the space that has already been allocated for
writing.  This is unlike the exported function
`comedi_buf_read_n_available()` which includes the space available to be
allocated for reading plus the space already allocated for reading.  The
new name breaks the unintentional naming symmetry (and also clears the
way for the old name to be reused for a new function).

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_buf.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/comedi_buf.c 
b/drivers/staging/comedi/comedi_buf.c
index d45a4b6..4837559 100644
--- a/drivers/staging/comedi/comedi_buf.c
+++ b/drivers/staging/comedi/comedi_buf.c
@@ -245,7 +245,7 @@ void comedi_buf_reset(struct comedi_subdevice *s)
async->events = 0;
 }
 
-static unsigned int comedi_buf_write_n_available(struct comedi_subdevice *s)
+static unsigned int comedi_buf_write_n_unalloc(struct comedi_subdevice *s)
 {
struct comedi_async *async = s->async;
unsigned int free_end = async->buf_read_count + async->prealloc_bufsz;
@@ -268,10 +268,10 @@ unsigned int comedi_buf_write_alloc(struct 
comedi_subdevice *s,
unsigned int nbytes)
 {
struct comedi_async *async = s->async;
-   unsigned int available = comedi_buf_write_n_available(s);
+   unsigned int unalloc = comedi_buf_write_n_unalloc(s);
 
-   if (nbytes > available)
-   nbytes = available;
+   if (nbytes > unalloc)
+   nbytes = unalloc;
 
async->buf_write_alloc_count += nbytes;
 
@@ -557,8 +557,7 @@ unsigned int comedi_buf_write_samples(struct 
comedi_subdevice *s,
 * If not, clamp the nsamples to the number that will fit, flag the
 * buffer overrun and add the samples that fit.
 */
-   max_samples = comedi_bytes_to_samples(s,
- comedi_buf_write_n_available(s));
+   max_samples = comedi_bytes_to_samples(s, comedi_buf_write_n_unalloc(s));
if (nsamples > max_samples) {
dev_warn(s->device->class_dev, "buffer overrun\n");
s->async->events |= COMEDI_CB_OVERFLOW;
-- 
2.6.1

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


[PATCH 5/6] staging: comedi: check command started by file being polled

2015-10-09 Thread Ian Abbott
Currently, the "poll" file operation checks if an asynchronous "read"
(or "write" command is active on the "read" (or "write" subdevice, but
does not consider whether the command was started from the file object
being polled.  Since that is the only file object able to read (or
write) data, take it into consideration.

With this change, if no read (or write) command is running on the
subdevice, or it is started by a different file object, the file object
is marked as readable (or writeable) regardless, but the read (or write)
file operation will return an error.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 4793e30..07bb197 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2275,7 +2275,7 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
s_read = s;
if (s && s->async) {
poll_wait(file, >async->wait_head, wait);
-   if (!s->busy || !comedi_is_subdevice_running(s) ||
+   if (s->busy != file || !comedi_is_subdevice_running(s) ||
(s->async->cmd.flags & CMDF_WRITE) ||
comedi_buf_read_n_available(s) > 0)
mask |= POLLIN | POLLRDNORM;
@@ -2287,7 +2287,7 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
 
if (s != s_read)
poll_wait(file, >async->wait_head, wait);
-   if (!s->busy || !comedi_is_subdevice_running(s) ||
+   if (s->busy != file || !comedi_is_subdevice_running(s) ||
!(s->async->cmd.flags & CMDF_WRITE) ||
comedi_buf_write_n_available(s) >= bps)
mask |= POLLOUT | POLLWRNORM;
-- 
2.6.1

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


[PATCH 4/6] staging: comedi: don't allocate buffer space when polling for write

2015-10-09 Thread Ian Abbott
When handling the "poll" file operation and checking for `POLLOUT`,
don't allocate space from the buffer for writing, just check that space
is available for writing.  That check is done after checking that an
asynchronous "write" command is running on the subdevice.  Allocating
the buffer space before checking a "write" command is running can cause
problems if the subdevice supports commands in either direction and
currently has an active "read" command.

Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
---
 drivers/staging/comedi/comedi_fops.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/comedi_fops.c 
b/drivers/staging/comedi/comedi_fops.c
index 9dcd486..4793e30 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2287,10 +2287,9 @@ static unsigned int comedi_poll(struct file *file, 
poll_table *wait)
 
if (s != s_read)
poll_wait(file, >async->wait_head, wait);
-   comedi_buf_write_alloc(s, s->async->prealloc_bufsz);
if (!s->busy || !comedi_is_subdevice_running(s) ||
!(s->async->cmd.flags & CMDF_WRITE) ||
-   comedi_buf_write_n_allocated(s) >= bps)
+   comedi_buf_write_n_available(s) >= bps)
mask |= POLLOUT | POLLWRNORM;
}
 
-- 
2.6.1

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


[PATCH 0/6] staging: comedi: fix some minor issues with file poll op

2015-10-09 Thread Ian Abbott
A few changes for the "poll" file operation to avoid poll-waiting on the
same subdevice for both read and write (patch 1), avoid allocating write
buffer space unnecessarily and possibly inappropriately (patch 4),
consider whether any active commands belong to the current file object
(patch 5), and avoid using the main mutex (for performance reasons)
(patch 6).

1) staging: comedi: don't poll_wait on same subdevice twice
2) staging: comedi: rename comedi_buf_write_n_available
3) staging: comedi: add new comedi_buf_write_n_available()
4) staging: comedi: don't allocate buffer space when polling for write
5) staging: comedi: check command started by file being polled
6) staging: comedi: don't use mutex when polling file

 drivers/staging/comedi/comedi_buf.c  | 19 +--
 drivers/staging/comedi/comedi_fops.c | 17 +
 drivers/staging/comedi/comedi_internal.h |  1 +
 3 files changed, 23 insertions(+), 14 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] staging: comedi: comedi_usb.c: improve function documentation

2015-09-30 Thread Ian Abbott
Expand the descriptions of the functions and document the return values.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_usb.c | 75 +
 1 file changed, 52 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/comedi/comedi_usb.c 
b/drivers/staging/comedi/comedi_usb.c
index 68b75e8..9c946d4 100644
--- a/drivers/staging/comedi/comedi_usb.c
+++ b/drivers/staging/comedi/comedi_usb.c
@@ -21,8 +21,14 @@
 #include "comedi_usb.h"
 
 /**
- * comedi_to_usb_interface() - comedi_device pointer to usb_interface pointer.
- * @dev: comedi_device struct
+ * comedi_to_usb_interface() - Return USB interface attached to COMEDI device
+ * @dev: COMEDI device.
+ *
+ * Assuming @dev->hw_dev is non-%NULL, it is assumed to be pointing to a
+ * a  device embedded in a  usb_interface.
+ *
+ * Return: Attached USB interface if @dev->hw_dev is non-%NULL.
+ * Return %NULL if @dev->hw_dev is %NULL.
  */
 struct usb_interface *comedi_to_usb_interface(struct comedi_device *dev)
 {
@@ -31,8 +37,14 @@ struct usb_interface *comedi_to_usb_interface(struct 
comedi_device *dev)
 EXPORT_SYMBOL_GPL(comedi_to_usb_interface);
 
 /**
- * comedi_to_usb_dev() - comedi_device pointer to usb_device pointer.
- * @dev: comedi_device struct
+ * comedi_to_usb_dev() - Return USB device attached to COMEDI device
+ * @dev: COMEDI device.
+ *
+ * Assuming @dev->hw_dev is non-%NULL, it is assumed to be pointing to a
+ * a  device embedded in a  usb_interface.
+ *
+ * Return: USB device to which the USB interface belongs if @dev->hw_dev is
+ * non-%NULL.  Return %NULL if @dev->hw_dev is %NULL.
  */
 struct usb_device *comedi_to_usb_dev(struct comedi_device *dev)
 {
@@ -43,12 +55,19 @@ struct usb_device *comedi_to_usb_dev(struct comedi_device 
*dev)
 EXPORT_SYMBOL_GPL(comedi_to_usb_dev);
 
 /**
- * comedi_usb_auto_config() - Configure/probe a comedi USB driver.
- * @intf: usb_interface struct
- * @driver: comedi_driver struct
- * @context: driver specific data, passed to comedi_auto_config()
+ * comedi_usb_auto_config() - Configure/probe a USB COMEDI driver
+ * @intf: USB interface.
+ * @driver: Registered COMEDI driver.
+ * @context: Driver specific data, passed to comedi_auto_config().
  *
- * Typically called from the usb_driver (*probe) function.
+ * Typically called from the usb_driver (*probe) function.  Auto-configure a
+ * COMEDI device, using a pointer to the  device embedded in *@intf as
+ * the hardware device.  The @context value gets passed through to @driver's
+ * "auto_attach" handler.  The "auto_attach" handler may call
+ * comedi_to_usb_interface() on the passed in COMEDI device to recover @intf.
+ *
+ * Return: The result of calling comedi_auto_config() (%0 on success, or
+ * a negative error number on failure).
  */
 int comedi_usb_auto_config(struct usb_interface *intf,
   struct comedi_driver *driver,
@@ -59,10 +78,18 @@ int comedi_usb_auto_config(struct usb_interface *intf,
 EXPORT_SYMBOL_GPL(comedi_usb_auto_config);
 
 /**
- * comedi_pci_auto_unconfig() - Unconfigure/disconnect a comedi USB driver.
- * @intf: usb_interface struct
+ * comedi_usb_auto_unconfig() - Unconfigure/disconnect a USB COMEDI device
+ * @intf: USB interface.
  *
  * Typically called from the usb_driver (*disconnect) function.
+ * Auto-unconfigure a COMEDI device attached to this USB interface, using a
+ * pointer to the  device embedded in *@intf as the hardware device.
+ * The COMEDI driver's "detach" handler will be called during unconfiguration
+ * of the COMEDI device.
+ *
+ * Note that the COMEDI device may have already been unconfigured using the
+ * %COMEDI_DEVCONFIG ioctl, in which case this attempt to unconfigure it
+ * again should be ignored.
  */
 void comedi_usb_auto_unconfig(struct usb_interface *intf)
 {
@@ -71,13 +98,15 @@ void comedi_usb_auto_unconfig(struct usb_interface *intf)
 EXPORT_SYMBOL_GPL(comedi_usb_auto_unconfig);
 
 /**
- * comedi_usb_driver_register() - Register a comedi USB driver.
- * @comedi_driver: comedi_driver struct
- * @usb_driver: usb_driver struct
+ * comedi_usb_driver_register() - Register a USB COMEDI driver
+ * @comedi_driver: COMEDI driver to be registered.
+ * @usb_driver: USB driver to be registered.
+ *
+ * This function is called from the module_init() of USB COMEDI driver modules
+ * to register the COMEDI driver and the USB driver.  Do not call it directly,
+ * use the module_comedi_usb_driver() helper macro instead.
  *
- * This function is used for the module_init() of comedi USB drivers.
- * Do not call it directly, use the module_comedi_usb_driver() helper
- * macro instead.
+ * Return: %0 on success, or a negative error number on failure.
  */
 int comedi_usb_driver_register(struct comedi_driver *comedi_driver,
   struct usb_driver *usb_driver)
@@ -99,13 +128,13 @@ int comedi_usb_driver_register(struct comedi_driver 
*co

[PATCH] staging: comedi: comedi_pcmcia.c: improve function documentation

2015-09-30 Thread Ian Abbott
Expand the descriptions of the functions and document the return values.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_pcmcia.c | 104 -
 1 file changed, 76 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/comedi/comedi_pcmcia.c 
b/drivers/staging/comedi/comedi_pcmcia.c
index 7e78439..d7072a5 100644
--- a/drivers/staging/comedi/comedi_pcmcia.c
+++ b/drivers/staging/comedi/comedi_pcmcia.c
@@ -22,8 +22,14 @@
 #include "comedi_pcmcia.h"
 
 /**
- * comedi_to_pcmcia_dev() - comedi_device pointer to pcmcia_device pointer.
- * @dev: comedi_device struct
+ * comedi_to_pcmcia_dev() - Return PCMCIA device attached to COMEDI device
+ * @dev: COMEDI device.
+ *
+ * Assuming @dev->hw_dev is non-%NULL, it is assumed to be pointing to a
+ * a  device embedded in a  pcmcia_device.
+ *
+ * Return: Attached PCMCIA device if @dev->hw_dev is non-%NULL.
+ * Return %NULL if @dev->hw_dev is %NULL.
  */
 struct pcmcia_device *comedi_to_pcmcia_dev(struct comedi_device *dev)
 {
@@ -41,13 +47,35 @@ static int comedi_pcmcia_conf_check(struct pcmcia_device 
*link,
 }
 
 /**
- * comedi_pcmcia_enable() - Request the regions and enable the PCMCIA device.
- * @dev: comedi_device struct
- * @conf_check: optional callback to check the pcmcia_device configuration
+ * comedi_pcmcia_enable() - Request the regions and enable the PCMCIA device
+ * @dev: COMEDI device.
+ * @conf_check: Optional callback to check each configuration option of the
+ * PCMCIA device and request I/O regions.
+ *
+ * Assuming @dev->hw_dev is non-%NULL, it is assumed to be pointing to a a
+ *  device embedded in a  pcmcia_device.  The comedi PCMCIA
+ * driver needs to set the 'config_flags' member in the  pcmcia_device,
+ * as appropriate for that driver, before calling this function in order to
+ * allow pcmcia_loop_config() to do its internal autoconfiguration.
+ *
+ * If @conf_check is %NULL it is set to a default function.  If is
+ * passed to pcmcia_loop_config() and should return %0 if the configuration
+ * is valid and I/O regions requested successfully, otherwise it should return
+ * a negative error value.  The default function returns -%EINVAL if the
+ * 'config_index' member is %0, otherwise it calls pcmcia_request_io() and
+ * returns the result.
+ *
+ * If the above configuration check passes, pcmcia_enable_device() is called
+ * to set up and activate the PCMCIA device.
  *
- * The comedi PCMCIA driver needs to set the link->config_flags, as
- * appropriate for that driver, before calling this function in order
- * to allow pcmcia_loop_config() to do its internal autoconfiguration.
+ * If this function returns an error, comedi_pcmcia_disable() should be called
+ * to release requested resources.
+ *
+ * Return:
+ * 0 on success,
+ * -%ENODEV id @dev->hw_dev is %NULL,
+ * a negative error number from pcmcia_loop_config() if it fails,
+ * or a negative error number from pcmcia_enable_device() if it fails.
  */
 int comedi_pcmcia_enable(struct comedi_device *dev,
 int (*conf_check)(struct pcmcia_device *, void *))
@@ -70,8 +98,12 @@ int comedi_pcmcia_enable(struct comedi_device *dev,
 EXPORT_SYMBOL_GPL(comedi_pcmcia_enable);
 
 /**
- * comedi_pcmcia_disable() - Disable the PCMCIA device and release the regions.
- * @dev: comedi_device struct
+ * comedi_pcmcia_disable() - Disable the PCMCIA device and release the regions
+ * @dev: COMEDI device.
+ *
+ * Assuming @dev->hw_dev is non-%NULL, it is assumed to be pointing to a
+ * a  device embedded in a  pcmcia_device.  Call
+ * pcmcia_disable_device() to disable and clean up the PCMCIA device.
  */
 void comedi_pcmcia_disable(struct comedi_device *dev)
 {
@@ -83,11 +115,17 @@ void comedi_pcmcia_disable(struct comedi_device *dev)
 EXPORT_SYMBOL_GPL(comedi_pcmcia_disable);
 
 /**
- * comedi_pcmcia_auto_config() - Configure/probe a comedi PCMCIA driver.
- * @link: pcmcia_device struct
- * @driver: comedi_driver struct
+ * comedi_pcmcia_auto_config() - Configure/probe a PCMCIA COMEDI device
+ * @link: PCMCIA device.
+ * @driver: Registered COMEDI driver.
+ *
+ * Typically called from the pcmcia_driver (*probe) function.  Auto-configure
+ * a COMEDI device, using a pointer to the  device embedded in *@link
+ * as the hardware device.  The @driver's "auto_attach" handler may call
+ * comedi_to_pcmcia_dev() on the passed in COMEDI device to recover @link.
  *
- * Typically called from the pcmcia_driver (*probe) function.
+ * Return: The result of calling comedi_auto_config() (0 on success, or a
+ * negative error number on failure).
  */
 int comedi_pcmcia_auto_config(struct pcmcia_device *link,
  struct comedi_driver *driver)
@@ -97,10 +135,18 @@ int comedi_pcmcia_auto_config(struct pcmcia_device *link,
 EXPORT_SYMBOL_GPL(comedi_pcmcia_auto_config);
 
 /**
- * comedi_pcmcia_auto_unconfig() - Unconfigure/remove a comedi PCMCI

[PATCH] staging: comedi: comedi_pci.c: Fix kernel-doc Return tags

2015-09-30 Thread Ian Abbott
Fix the 'Return' tags in the kernel-doc comments as they currently say
'Returns', which is not recognized by kernel-doc.

Signed-off-by: Ian Abbott 
---
 drivers/staging/comedi/comedi_pci.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/comedi/comedi_pci.c 
b/drivers/staging/comedi/comedi_pci.c
index 0601049..51e023a 100644
--- a/drivers/staging/comedi/comedi_pci.c
+++ b/drivers/staging/comedi/comedi_pci.c
@@ -28,8 +28,8 @@
  * Assuming @dev->hw_dev is non-%NULL, it is assumed to be pointing to a
  * a  device embedded in a  pci_dev.
  *
- * Returns: Attached PCI device if @dev->hw_dev is non-%NULL.
- * Returns %NULL if @dev->hw_dev is %NULL.
+ * Return: Attached PCI device if @dev->hw_dev is non-%NULL.
+ * Return %NULL if @dev->hw_dev is %NULL.
  */
 struct pci_dev *comedi_to_pci_dev(struct comedi_device *dev)
 {
@@ -48,7 +48,7 @@ EXPORT_SYMBOL_GPL(comedi_to_pci_dev);
  *
  * Calls to comedi_pci_enable() and comedi_pci_disable() cannot be nested.
  *
- * Returns:
+ * Return:
  * 0 on success,
  * -%ENODEV if @dev->hw_dev is %NULL,
  * -%EBUSY if regions busy,
@@ -143,7 +143,7 @@ EXPORT_SYMBOL_GPL(comedi_pci_detach);
  * "auto_attach" handler.  The "auto_attach" handler may call
  * comedi_to_pci_dev() on the passed in COMEDI device to recover @pcidev.
  *
- * Returns: The result of calling comedi_auto_config() (0 on success, or
+ * Return: The result of calling comedi_auto_config() (0 on success, or
  * a negative error number on failure).
  */
 int comedi_pci_auto_config(struct pci_dev *pcidev,
@@ -183,7 +183,7 @@ EXPORT_SYMBOL_GPL(comedi_pci_auto_unconfig);
  * to register the COMEDI driver and the PCI driver.  Do not call it directly,
  * use the module_comedi_pci_driver() helper macro instead.
  *
- * Returns: 0 on success, or a negative error number on failure.
+ * Return: 0 on success, or a negative error number on failure.
  */
 int comedi_pci_driver_register(struct comedi_driver *comedi_driver,
   struct pci_driver *pci_driver)
-- 
2.5.3

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


<    4   5   6   7   8   9   10   11   12   13   >