On Tuesday, April 29, 2014 7:52 AM, Ian Abbott wrote: > On 2014-04-28 19:53, H Hartley Sweeten wrote: >> The analog input async command can use the pacer for the scan_bagin_src >> or the convert_src. The (*do_cmdtest) validates that only one of these >> sources has the TRIG_TIMER selected and calculates the divisors when >> validating the cmd argument. >> >> There is no reason to recalc the divisors in the (*do_cmd). Just use >> the values from the private data. >> >> Also, tidy up cb_pcidas_load_counters() a bit. >> >> Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx> >> Cc: Ian Abbott <abbotti@xxxxxxxxx> >> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> >> --- >> drivers/staging/comedi/drivers/cb_pcidas.c | 25 ++++++++----------------- >> 1 file changed, 8 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/staging/comedi/drivers/cb_pcidas.c b/drivers/staging/comedi/drivers/cb_pcidas.c >> index 26fc00e..ec51186 100644 >> --- a/drivers/staging/comedi/drivers/cb_pcidas.c >> +++ b/drivers/staging/comedi/drivers/cb_pcidas.c >> @@ -943,20 +943,15 @@ static int cb_pcidas_ai_cmdtest(struct comedi_device *dev, >> return 0; >> } >> >> -static void cb_pcidas_load_counters(struct comedi_device *dev, unsigned int *ns, >> - int rounding_flags) >> +static void cb_pcidas_load_counters(struct comedi_device *dev) >> { >> struct cb_pcidas_private *devpriv = dev->private; >> + unsigned long timer_base = devpriv->pacer_counter_dio + ADC8254; >> >> - i8253_cascade_ns_to_timer(I8254_OSC_BASE_10MHZ, >> - &devpriv->divisor1, &devpriv->divisor2, >> - ns, rounding_flags); >> - >> - /* Write the values of ctr1 and ctr2 into counters 1 and 2 */ >> - i8254_load(devpriv->pacer_counter_dio + ADC8254, 0, 1, >> - devpriv->divisor1, 2); >> - i8254_load(devpriv->pacer_counter_dio + ADC8254, 0, 2, >> - devpriv->divisor2, 2); >> + i8254_load(timer_base, 0, >> + 1, devpriv->divisor1, I8254_MODE2 | I8254_BINARY); >> + i8254_load(timer_base, 0, >> + 2, devpriv->divisor2, I8254_MODE2 | I8254_BINARY); >> } > > i8254_load() doesn't take that sort of mode value; it expects a mode > value in the range 0 to 5 and shifts it left by one. The mode values > you are passing are pre-shifted, so it the 8254 channels will actually > be set to mode 4 with those values. Hmm.. I don't think I ever noticed that before. Seems kind of odd. i8254_load() "mode" is a raw number, I8254_BINARY is enforced i8254_set_mode() "mode" is the enum i8254_mode from comedi.h To me it seems they should both use the enum i8254_mode to provide clearer documentation since the i8254_load() is basically a i8254_set_mode() followed by a i8254_write(). Currently i8254_set_mode() is used a lot more in the comedi drivers than the i8254_load(). For now I'll rebase this series to use i8254_set_mode()/ i8254_write(). Thanks for the review, Hartley _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel