Re: [PATCH] comedi: pcmmio.c: Fix coding style - use BIT macro
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
`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
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
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
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
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
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
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
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
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
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
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
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
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
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()
`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
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
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
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
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
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
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()
`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()
`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
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
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 '*'
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
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
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
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()
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()
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"
`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
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()
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
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
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()
`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()
`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
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()
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"
`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()
`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
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
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()
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
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
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()
`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
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()
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
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
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 '*'
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
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
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
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
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
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()
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
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
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
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()
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
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
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
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
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
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
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
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/