[PATCH 00/83] staging: comedi: rtd520: initial cleanup

[PATCH 00/83] staging: comedi: rtd520: initial cleanup

[H Hartley Sweeten]

The 'devpriv' macro usage in this driver is holding up other cleanup
of the comedi drivers.

This patch series removes all the macros used to read/write the
hardware registers. All of them are simple wrappers around standard
{read,write}[rwl] calls with some of them caching the write value
in a shadow variable in the private data.

Each of the macros is removed in a separate patch to ease reviewing.
They can all be squashed into one patch if desired.

After all the macros are gone, the devpriv macro itself can be removed
along with the boardinfo macro.

The "find pci device" code is also refactored to follow the style of
the other comedi pci drivers.

The range tables are also cleaned up for aesthetic reasons. The large
whitespace causes some pretty nasty line breaks in order to keep the
lines < 80 characters.

H Hartley Sweeten (83):
  staging: comedi: rtd520: remove RtdResetBoard macro
  staging: comedi: rtd520: remove RtdResetCGT macro
  staging: comedi: rtd520: remove RtdClearCGT macro
  staging: comedi: rtd520: remove RtdEnableCGT macro
  staging: comedi: rtd520: remove RtdWriteCGTable macro
  staging: comedi: rtd520: remove RtdWriteCGLatch macro
  staging: comedi: rtd520: remove RtdAdcClearFifo macro
  staging: comedi: rtd520: remove RtdAdcConversionSource macro
  staging: comedi: rtd520: remove RtdBurstStartSource macro
  staging: comedi: rtd520: remove RtdPacerStartSource macro
  staging: comedi: rtd520: remove RtdPacerStopSource macro
  staging: comedi: rtd520: remove RtdPacerClockSource macro
  staging: comedi: rtd520: remove RtdAdcSampleCounterSource macro
  staging: comedi: rtd520: remove RtdPacerTriggerMode macro
  staging: comedi: rtd520: remove RtdAboutStopEnable macro
  staging: comedi: rtd520: remove RtdTriggerPolarity macro
  staging: comedi: rtd520: remove RtdAdcStart macro
  staging: comedi: rtd520: remove RtdAdcFifoGet macro
  staging: comedi: rtd520: remove RtdAdcFifoGet2 macro
  staging: comedi: rtd520: remove RtdFifoStatus macro
  staging: comedi: rtd520: remove RtdPacerStart macro
  staging: comedi: rtd520: remove RtdPacerStop macro
  staging: comedi: rtd520: remove RtdInterruptStatus macro
  staging: comedi: rtd520: remove RtdInterruptMask macro
  staging: comedi: rtd520: remove RtdInterruptClear macro
  staging: comedi: rtd520: remove RtdInterruptClearMask macro
  staging: comedi: rtd520: remove RtdInterruptOverrunStatus macro
  staging: comedi: rtd520: remove RtdInterruptOverrunClear macro
  staging: comedi: rtd520: remove RtdPacerCount macro
  staging: comedi: rtd520: remove RtdPacerCounter macro
  staging: comedi: rtd520: remove RtdBurstCount macro
  staging: comedi: rtd520: remove RtdBurstCounter macro
  staging: comedi: rtd520: remove RtdDelayCount macro
  staging: comedi: rtd520: remove RtdDelayCounter macro
  staging: comedi: rtd520: remove RtdAboutCount macro
  staging: comedi: rtd520: remove RtdAboutCounter macro
  staging: comedi: rtd520: remove RtdAdcSampleCount macro
  staging: comedi: rtd520: remove RtdAdcSampleCounter macro
  staging: comedi: rtd520: remove RtdUtcCounterGet macro
  staging: comedi: rtd520: remove RtdUtcCounterPut macro
  staging: comedi: rtd520: remove RtdUtcCtrlPut macro
  staging: comedi: rtd520: remove RtdUtcClockSource macro
  staging: comedi: rtd520: remove RtdUtcGateSource macro
  staging: comedi: rtd520: remove RtdUsrOutSource macro
  staging: comedi: rtd520: remove RtdDio0Read macro
  staging: comedi: rtd520: remove RtdDio0Write macro
  staging: comedi: rtd520: remove RtdDio1Read macro
  staging: comedi: rtd520: remove RtdDio1Write macro
  staging: comedi: rtd520: remove RtdDioStatusRead macro
  staging: comedi: rtd520: remove RtdDioStatusWrite macro
  staging: comedi: rtd520: remove RtdDio0CtrlRead macro
  staging: comedi: rtd520: remove RtdDio0CtrlWrite macro
  staging: comedi: rtd520: remove RtdDacFifoPut macro
  staging: comedi: rtd520: remove RtdDacUpdate macro
  staging: comedi: rtd520: remove RtdDacBothUpdate macro
  staging: comedi: rtd520: remove RtdDacRange macro
  staging: comedi: rtd520: remove RtdDacClearFifo macro
  staging: comedi: rtd520: remove RtdDma0Source macro
  staging: comedi: rtd520: remove RtdDma1Source macro
  staging: comedi: rtd520: remove RtdDma0Reset macro
  staging: comedi: rtd520: remove RtdDma1Reset macro
  staging: comedi: rtd520: remove RtdPlxInterruptRead macro
  staging: comedi: rtd520: remove RtdPlxInterruptWrite macro
  staging: comedi: rtd520: remove RtdDma0Mode macro
  staging: comedi: rtd520: remove RtdDma0PciAddr macro
  staging: comedi: rtd520: remove RtdDma0LocalAddr macro
  staging: comedi: rtd520: remove RtdDma0Count macro
  staging: comedi: rtd520: remove RtdDma0Count macro
  staging: comedi: rtd520: remove RtdDma1Mode macro
  staging: comedi: rtd520: remove RtdDma1PciAddr macro
  staging: comedi: rtd520: remove RtdDma1LocalAddr macro
  staging: comedi: rtd520: remove RtdDma1Count macro
  staging: comedi: rtd520: remove RtdDma1Next macro
  staging: comedi: rtd520: remove RtdDma0Control macro
  staging: comedi: rtd520: remove RtdDma0Status macro
  staging: comedi: rtd520: remove RtdDma1Control macro
  staging: comedi: rtd520: remove RtdDma1Status macro
  staging: comedi: rtd520: remove devpriv macro
  staging: comedi: rtd520: remove thisboard macro
  staging: comedi: rtd520: factor out the "find pci device" code
  staging: comedi: rtd520: cleanup the "find pci device" code
  staging: comedi: rtd520: cleanup the range tables
  staging: comedi: rtd520: cleanup the boardinfo

 drivers/staging/comedi/drivers/rtd520.c | 915 +++++++++++---------------------
 1 file changed, 318 insertions(+), 597 deletions(-)

-- 
1.7.11

Re: [PATCH 00/83] staging: comedi: rtd520: initial cleanup

[Ian Abbott]

On 2012-07-11 00:36, H Hartley Sweeten wrote:
> The 'devpriv' macro usage in this driver is holding up other cleanup
> of the comedi drivers.
>
> This patch series removes all the macros used to read/write the
> hardware registers. All of them are simple wrappers around standard
> {read,write}[rwl] calls with some of them caching the write value
> in a shadow variable in the private data.
>
> Each of the macros is removed in a separate patch to ease reviewing.
> They can all be squashed into one patch if desired.
>
> After all the macros are gone, the devpriv macro itself can be removed
> along with the boardinfo macro.
>
> The "find pci device" code is also refactored to follow the style of
> the other comedi pci drivers.
>
> The range tables are also cleaned up for aesthetic reasons. The large
> whitespace causes some pretty nasty line breaks in order to keep the
> lines < 80 characters.

My main concern with this series of patches is they make it harder to 
determine what the register accesses actually do, mainly due to the 
removal of useful comments.  For example:

-	RtdPacerStart(dev);	/* Start PACER */
+	readl(devpriv->las0 + LAS0_PACER);

And:

-	RtdPacerStop(dev);	/* Stop PACER */
+	writel(0, devpriv->las0 + LAS0_PACER);

The comments in the original code were quite superfluous, but they would 
be useful for the replacement code.  In the above cases you could figure 
it out after looking at the rtd520.h header file where its reasonably 
clear that reading the LAS0_PACER register starts the pacer and writing 
the register (presumably with any value) stops the pacer.  But it's less 
clear in cases such as:

-		RtdAdcConversionSource(dev, 2);	/* BURST triggers ADC */
+		writel(2, devpriv->las0 + LAS0_ADC_CONVERSION);

Since the values for the LAS0_ADC_CONVERSION register are no longer 
documented anywhere.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

Re: [PATCH 00/83] staging: comedi: rtd520: initial cleanup

[Ian Abbott]

On 2012-07-11 11:20, Ian Abbott wrote:
> On 2012-07-11 00:36, H Hartley Sweeten wrote:
> My main concern with this series of patches is they make it harder to
> determine what the register accesses actually do, mainly due to the
> removal of useful comments.  For example:
>
> -	RtdPacerStart(dev);	/* Start PACER */
> +	readl(devpriv->las0 + LAS0_PACER);
>
> And:
>
> -	RtdPacerStop(dev);	/* Stop PACER */
> +	writel(0, devpriv->las0 + LAS0_PACER);

It would be too much work to redo all those patches though.  I'll just 
make a patch to add back in the useful-looking comments.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

[PATCH] staging: comedi: rtd520: add a few comments

[Ian Abbott]

H Hartley Sweeten's recent series of patches to clean up the rtd520
driver made some of the register accesses harder to understand.  Add a
few comments to provide some clues to the reader.

Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
To be applied after Hartley's 83 patches.
---
 drivers/staging/comedi/drivers/rtd520.c |   23 ++++++++++++++++++-----
 1 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/comedi/drivers/rtd520.c b/drivers/staging/comedi/drivers/rtd520.c
index 5a2953e..9998d6b 100644
--- a/drivers/staging/comedi/drivers/rtd520.c
+++ b/drivers/staging/comedi/drivers/rtd520.c
@@ -494,6 +494,7 @@ static int rtd520_probe_fifo_depth(struct comedi_device *dev)
 
 	writel(0, devpriv->las0 + LAS0_ADC_FIFO_CLEAR);
 	rtd_load_channelgain_list(dev, 1, &chanspec);
+	/* ADC conversion trigger source: SOFTWARE */
 	writel(0, devpriv->las0 + LAS0_ADC_CONVERSION);
 	/* convert  samples */
 	for (i = 0; i < limit; ++i) {
@@ -544,7 +545,7 @@ static int rtd_ai_rinsn(struct comedi_device *dev,
 	/* write channel to multiplexer and clear channel gain table */
 	rtd_load_channelgain_list(dev, 1, &insn->chanspec);
 
-	/* set conversion source */
+	/* ADC conversion trigger source: SOFTWARE */
 	writel(0, devpriv->las0 + LAS0_ADC_CONVERSION);
 
 	/* convert n samples */
@@ -908,8 +909,9 @@ abortTransfer:
 	/* fall into transferDone */
 
 transferDone:
+	/* pacer stop source: SOFTWARE */
 	writel(0, devpriv->las0 + LAS0_PACER_STOP);
-	writel(0, devpriv->las0 + LAS0_PACER);
+	writel(0, devpriv->las0 + LAS0_PACER);	/* stop pacer */
 	writel(0, devpriv->las0 + LAS0_ADC_CONVERSION);
 	devpriv->intMask = 0;
 	writew(devpriv->intMask, devpriv->las0 + LAS0_IT);
@@ -1180,8 +1182,9 @@ static int rtd_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	int timer;
 
 	/* stop anything currently running */
+	/* pacer stop source: SOFTWARE */
 	writel(0, devpriv->las0 + LAS0_PACER_STOP);
-	writel(0, devpriv->las0 + LAS0_PACER);
+	writel(0, devpriv->las0 + LAS0_PACER);	/* stop pacer */
 	writel(0, devpriv->las0 + LAS0_ADC_CONVERSION);
 	devpriv->intMask = 0;
 	writew(devpriv->intMask, devpriv->las0 + LAS0_IT);
@@ -1215,12 +1218,17 @@ static int rtd_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 	/* setup the common case and override if needed */
 	if (cmd->chanlist_len > 1) {
 		/*DPRINTK ("rtd520: Multi channel setup\n"); */
+		/* pacer start source: SOFTWARE */
 		writel(0, devpriv->las0 + LAS0_PACER_START);
+		/* burst trigger source: PACER */
 		writel(1, devpriv->las0 + LAS0_BURST_START);
+		/* ADC conversion trigger source: BURST */
 		writel(2, devpriv->las0 + LAS0_ADC_CONVERSION);
 	} else {		/* single channel */
 		/*DPRINTK ("rtd520: single channel setup\n"); */
+		/* pacer start source: SOFTWARE */
 		writel(0, devpriv->las0 + LAS0_PACER_START);
+		/* ADC conversion trigger source: PACER */
 		writel(1, devpriv->las0 + LAS0_ADC_CONVERSION);
 	}
 	writel((devpriv->fifoLen / 2 - 1) & 0xffff, devpriv->las0 + LAS0_ACNT);
@@ -1269,7 +1277,9 @@ static int rtd_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 		devpriv->transCount = 0;
 		devpriv->flags &= ~SEND_EOS;
 	}
+	/* pacer clock source: INTERNAL 8MHz */
 	writel(1, devpriv->las0 + LAS0_PACER_SELECT);
+	/* just interrupt, don't stop */
 	writel(1, devpriv->las0 + LAS0_ACNT_STOP_ENABLE);
 
 	/* BUG??? these look like enumerated values, but they are bit fields */
@@ -1305,6 +1315,7 @@ static int rtd_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 		break;
 
 	case TRIG_EXT:
+		/* pacer start source: EXTERNAL */
 		writel(1, devpriv->las0 + LAS0_PACER_START);
 		break;
 
@@ -1327,6 +1338,7 @@ static int rtd_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 		break;
 
 	case TRIG_EXT:		/* external */
+		/* burst trigger source: EXTERNAL */
 		writel(2, devpriv->las0 + LAS0_BURST_START);
 		break;
 
@@ -1378,7 +1390,7 @@ static int rtd_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 
 	/* BUG: start_src is ASSUMED to be TRIG_NOW */
 	/* BUG? it seems like things are running before the "start" */
-	readl(devpriv->las0 + LAS0_PACER);
+	readl(devpriv->las0 + LAS0_PACER);	/* start pacer */
 	return 0;
 }
 
@@ -1391,8 +1403,9 @@ static int rtd_ai_cancel(struct comedi_device *dev, struct comedi_subdevice *s)
 	u32 overrun;
 	u16 status;
 
+	/* pacer stop source: SOFTWARE */
 	writel(0, devpriv->las0 + LAS0_PACER_STOP);
-	writel(0, devpriv->las0 + LAS0_PACER);
+	writel(0, devpriv->las0 + LAS0_PACER);	/* stop pacer */
 	writel(0, devpriv->las0 + LAS0_ADC_CONVERSION);
 	devpriv->intMask = 0;
 	writew(devpriv->intMask, devpriv->las0 + LAS0_IT);
-- 
1.7.8.6

RE: [PATCH 00/83] staging: comedi: rtd520: initial cleanup

[H Hartley Sweeten]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2806 bytes --]

On Wednesday, July 11, 2012 3:20 AM, Ian Abbott wrote:
> On 2012-07-11 00:36, H Hartley Sweeten wrote:
>> The 'devpriv' macro usage in this driver is holding up other cleanup
>> of the comedi drivers.
>>
>> This patch series removes all the macros used to read/write the
>> hardware registers. All of them are simple wrappers around standard
>> {read,write}[rwl] calls with some of them caching the write value
>> in a shadow variable in the private data.
>>
>> Each of the macros is removed in a separate patch to ease reviewing.
>> They can all be squashed into one patch if desired.
>>
>> After all the macros are gone, the devpriv macro itself can be removed
>> along with the boardinfo macro.
>>
>> The "find pci device" code is also refactored to follow the style of
>> the other comedi pci drivers.
>>
>> The range tables are also cleaned up for aesthetic reasons. The large
>> whitespace causes some pretty nasty line breaks in order to keep the
>> lines < 80 characters.
>
> My main concern with this series of patches is they make it harder to 
> determine what the register accesses actually do, mainly due to the 
> removal of useful comments.  For example:
>
> -	RtdPacerStart(dev);	/* Start PACER */
> +	readl(devpriv->las0 + LAS0_PACER);
>
> And:
>
> -	RtdPacerStop(dev);	/* Stop PACER */
> +	writel(0, devpriv->las0 + LAS0_PACER);
>
> The comments in the original code were quite superfluous, but they would 
> be useful for the replacement code.  In the above cases you could figure 
> it out after looking at the rtd520.h header file where its reasonably 
> clear that reading the LAS0_PACER register starts the pacer and writing 
> the register (presumably with any value) stops the pacer.  But it's less 
> clear in cases such as:
>
> -		RtdAdcConversionSource(dev, 2);	/* BURST triggers ADC */
> +		writel(2, devpriv->las0 + LAS0_ADC_CONVERSION);
>
> Since the values for the LAS0_ADC_CONVERSION register are no longer 
> documented anywhere.
> 

Sorry about that. I meant to leave some of the comments as appropriate
but overlooked them during the cleanup.

Some of the magic write values are actually documented in the header with
#define'd values. But, the header is a bit of a mess and it's hard to determine
which bit values go with which register.

The header really needs to be cleaned up and then merged directly into the
source. It's only used by this driver so the defines shouldn't be exposed for
external use. I located the manual for the device and plan on doing that cleanup
eventually.

Regardless, thanks for adding back the comments. I'll look them over and
give you a Reviewed-by shortly.

Thanks,
Hartley

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH] staging: comedi: rtd520: add a few comments

[H Hartley Sweeten]

On Wednesday, July 11, 2012 5:28 AM, Ian Abbott wrote:
> H Hartley Sweeten's recent series of patches to clean up the rtd520
> driver made some of the register accesses harder to understand.  Add a
> few comments to provide some clues to the reader.
>
> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
> Cc: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> To be applied after Hartley's 83 patches.
> ---
>  drivers/staging/comedi/drivers/rtd520.c |   23 ++++++++++++++++++-----
>  1 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/rtd520.c b/drivers/staging/comedi/drivers/rtd520.c
> index 5a2953e..9998d6b 100644
> --- a/drivers/staging/comedi/drivers/rtd520.c
> +++ b/drivers/staging/comedi/drivers/rtd520.c
> @@ -494,6 +494,7 @@ static int rtd520_probe_fifo_depth(struct comedi_device *dev)
>  
>  	writel(0, devpriv->las0 + LAS0_ADC_FIFO_CLEAR);
>  	rtd_load_channelgain_list(dev, 1, &chanspec);
> +	/* ADC conversion trigger source: SOFTWARE */
>  	writel(0, devpriv->las0 + LAS0_ADC_CONVERSION);
>  	/* convert  samples */
>  	for (i = 0; i < limit; ++i) {
> @@ -544,7 +545,7 @@ static int rtd_ai_rinsn(struct comedi_device *dev,
>  	/* write channel to multiplexer and clear channel gain table */
>  	rtd_load_channelgain_list(dev, 1, &insn->chanspec);
>  
> -	/* set conversion source */
> +	/* ADC conversion trigger source: SOFTWARE */
>  	writel(0, devpriv->las0 + LAS0_ADC_CONVERSION);
>  
>  	/* convert n samples */
> @@ -908,8 +909,9 @@ abortTransfer:
>  	/* fall into transferDone */
>  
>  transferDone:
> +	/* pacer stop source: SOFTWARE */
>  	writel(0, devpriv->las0 + LAS0_PACER_STOP);
> -	writel(0, devpriv->las0 + LAS0_PACER);
> +	writel(0, devpriv->las0 + LAS0_PACER);	/* stop pacer */
>  	writel(0, devpriv->las0 + LAS0_ADC_CONVERSION);
>  	devpriv->intMask = 0;
>  	writew(devpriv->intMask, devpriv->las0 + LAS0_IT);
> @@ -1180,8 +1182,9 @@ static int rtd_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
>  	int timer;
>  
>  	/* stop anything currently running */
> +	/* pacer stop source: SOFTWARE */
>  	writel(0, devpriv->las0 + LAS0_PACER_STOP);
> -	writel(0, devpriv->las0 + LAS0_PACER);
> +	writel(0, devpriv->las0 + LAS0_PACER);	/* stop pacer */
>  	writel(0, devpriv->las0 + LAS0_ADC_CONVERSION);
>  	devpriv->intMask = 0;
>  	writew(devpriv->intMask, devpriv->las0 + LAS0_IT);
> @@ -1215,12 +1218,17 @@ static int rtd_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
>  	/* setup the common case and override if needed */
>  	if (cmd->chanlist_len > 1) {
>  		/*DPRINTK ("rtd520: Multi channel setup\n"); */
> +		/* pacer start source: SOFTWARE */
>  		writel(0, devpriv->las0 + LAS0_PACER_START);
> +		/* burst trigger source: PACER */
>  		writel(1, devpriv->las0 + LAS0_BURST_START);
> +		/* ADC conversion trigger source: BURST */
>  		writel(2, devpriv->las0 + LAS0_ADC_CONVERSION);
>  	} else {		/* single channel */
>  		/*DPRINTK ("rtd520: single channel setup\n"); */
> +		/* pacer start source: SOFTWARE */
>  		writel(0, devpriv->las0 + LAS0_PACER_START);
> +		/* ADC conversion trigger source: PACER */
>  		writel(1, devpriv->las0 + LAS0_ADC_CONVERSION);
>  	}
>  	writel((devpriv->fifoLen / 2 - 1) & 0xffff, devpriv->las0 + LAS0_ACNT);
> @@ -1269,7 +1277,9 @@ static int rtd_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
>  		devpriv->transCount = 0;
>  		devpriv->flags &= ~SEND_EOS;
>  	}
> +	/* pacer clock source: INTERNAL 8MHz */
>  	writel(1, devpriv->las0 + LAS0_PACER_SELECT);
> +	/* just interrupt, don't stop */
>  	writel(1, devpriv->las0 + LAS0_ACNT_STOP_ENABLE);
>  
>  	/* BUG??? these look like enumerated values, but they are bit fields */
> @@ -1305,6 +1315,7 @@ static int rtd_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
>  		break;
>  
>  	case TRIG_EXT:
> +		/* pacer start source: EXTERNAL */
>  		writel(1, devpriv->las0 + LAS0_PACER_START);
>  		break;
>  
> @@ -1327,6 +1338,7 @@ static int rtd_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
>  		break;
>  
>  	case TRIG_EXT:		/* external */
> +		/* burst trigger source: EXTERNAL */
>  		writel(2, devpriv->las0 + LAS0_BURST_START);
>  		break;
>  
> @@ -1378,7 +1390,7 @@ static int rtd_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
>  
>  	/* BUG: start_src is ASSUMED to be TRIG_NOW */
>  	/* BUG? it seems like things are running before the "start" */
> -	readl(devpriv->las0 + LAS0_PACER);
> +	readl(devpriv->las0 + LAS0_PACER);	/* start pacer */
>  	return 0;
>  }
>  
> @@ -1391,8 +1403,9 @@ static int rtd_ai_cancel(struct comedi_device *dev, struct comedi_subdevice *s)
>  	u32 overrun;
>  	u16 status;
>  
> +	/* pacer stop source: SOFTWARE */
>  	writel(0, devpriv->las0 + LAS0_PACER_STOP);
> -	writel(0, devpriv->las0 + LAS0_PACER);
> +	writel(0, devpriv->las0 + LAS0_PACER);	/* stop pacer */
>  	writel(0, devpriv->las0 + LAS0_ADC_CONVERSION);
>  	devpriv->intMask = 0;
>  	writew(devpriv->intMask, devpriv->las0 + LAS0_IT);

Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>

Thanks!