Re: [PATCH 00/32] staging: comedi: continue async command cleanup

2014-05-03 Thread gre...@linuxfoundation.org
On Tue, Apr 29, 2014 at 08:07:55PM +, Hartley Sweeten wrote:
 On Tuesday, April 29, 2014 12:59 PM, H Hartley Sweeten wrote:
  Remove some unnecessary pacer divisor calculations. The divisors are 
  calculated
  as part of the (*do_cmdtest) and don't need done in the (*do_cmd).
 
  Remove the older, unused, divisor calc functions in 8253.h to avoid any
  confusion.
 
  Remove some unnecessary private data members in a couple drivers and the
  addi_common.h header.
 
  Tidy up hwdrv_apci3120 a bit.
 
  Fix a couple 8254 timer programming issues. As Ian Abbott pointed out,
  the i8254_load() function does not use the I8254_MODE* defines. Convert
  all drivers to use the i8254_set_mode()/i8254_write() sequence instead.
 
  v2: Fix some i8254_load() issued pointed out by Ian Abbott in patches
  01, 02, 03, and 05.
  Add a couple new patchs (24 thru 32) to fix/clarify the remaining
  i8254_load() issues.
 
 Ugh... Forgot to tag these as v2. Sorry about that.

No worries, I figured it out...

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 00/32] staging: comedi: continue async command cleanup

2014-05-01 Thread Ian Abbott

On 2014-04-30 17:46, Hartley Sweeten wrote:

On Wednesday, April 30, 2014 2:13 AM, Ian Abbott wrote:

As a side node, I wonder if it's worth stripping out those `|
I8254_BINARY` bits as it's basically 'OR'ing with zero anyway.


I like the I8254_BINARY, it documents the mode that the counter
is used in. But, if you want to strip them out...


Fair enough.


BTW, I noticed that the i8254_set_mode() helpers have a slight bug.

if (mode  (I8254_MODE5 | I8254_BINARY))
return -1;

This test will not allow the mode (I8254_MODE5 | I8254_BCD). Nothing
uses it right now, and it's a bit silly to use BCD counting anyway. But...


It does get exposed to user-space by a few drivers that expose the 8254 
as a counter subdevice.  The adv_pci_dio and das08 drivers expose it 
when handling the INSN_CONFIG_SET_COUNTER_MODE instruction (also, 
me4000 driver handling it as a GPCT_SET_OPERATION instruction whatever 
that is - it should probably be deprecated in favour of 
INSN_CONFIG_SET_COUNTER_MODE).  Also, the amplc_dio200_common module 
has the same bug in its handling of INSN_CONFIG_SET_COUNTER_MODE but 
uses its own function to set the mode instead of calling i8254_set_mode().


I'll post some patches to fix it later.  Probably not worth backporting 
them to stable.


--
-=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH 00/32] staging: comedi: continue async command cleanup

2014-05-01 Thread Hartley Sweeten
On Thursday, May 01, 2014 2:46 AM, Ian Abbott wrote:
 On 2014-04-30 17:46, Hartley Sweeten wrote:
 On Wednesday, April 30, 2014 2:13 AM, Ian Abbott wrote:
 As a side node, I wonder if it's worth stripping out those `|
 I8254_BINARY` bits as it's basically 'OR'ing with zero anyway.

 I like the I8254_BINARY, it documents the mode that the counter
 is used in. But, if you want to strip them out...

 Fair enough.

 BTW, I noticed that the i8254_set_mode() helpers have a slight bug.

  if (mode  (I8254_MODE5 | I8254_BINARY))
  return -1;

 This test will not allow the mode (I8254_MODE5 | I8254_BCD). Nothing
 uses it right now, and it's a bit silly to use BCD counting anyway. But...

 It does get exposed to user-space by a few drivers that expose the 8254 
 as a counter subdevice.  The adv_pci_dio and das08 drivers expose it 
 when handling the INSN_CONFIG_SET_COUNTER_MODE instruction (also, 
 me4000 driver handling it as a GPCT_SET_OPERATION instruction whatever 
 that is - it should probably be deprecated in favour of 
 INSN_CONFIG_SET_COUNTER_MODE).  Also, the amplc_dio200_common module 
 has the same bug in its handling of INSN_CONFIG_SET_COUNTER_MODE but 
 uses its own function to set the mode instead of calling i8254_set_mode().

 I'll post some patches to fix it later.  Probably not worth backporting 
 them to stable.

Great!

BTW, what's with all the NI_GPCT_* stuff in the main comedi.h header?

These all seem pretty driver specific to ni_tio.c and ni_tiocmd.c. I don't
understand why they are exposed in the user space header.

Thanks,
Hartley
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 00/32] staging: comedi: continue async command cleanup

2014-05-01 Thread Ian Abbott

On 2014-05-01 17:31, Hartley Sweeten wrote:

BTW, what's with all the NI_GPCT_* stuff in the main comedi.h header?

These all seem pretty driver specific to ni_tio.c and ni_tiocmd.c. I don't
understand why they are exposed in the user space header.


Basically for the same reason the 8254 mode values are exposed, so 
user-space can use the constants to construct the values for 
configuration instructions.


--
-=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 00/32] staging: comedi: continue async command cleanup

2014-04-30 Thread Ian Abbott

On 2014-04-29 20:59, H Hartley Sweeten wrote:

Remove some unnecessary pacer divisor calculations. The divisors are calculated
as part of the (*do_cmdtest) and don't need done in the (*do_cmd).

Remomove the older, unused, divisor calc functions in 8253.h to avoid any
confusion.

Remove some unnecessary private data members in a couple drivers and the
addi_common.h header.

Tidy up hwdrv_apci3120 a bit.

Fix a couple 8254 timer programming issues. As Ian Abbott pointed out,
the i8254_load() function does not use the I8254_MODE* defines. Convert
all drivers to use the i8254_set_mode()/i8254_write() sequence instead.

v2: Fix some i8254_load() issued pointed out by Ian Abbott in patches
 01, 02, 03, and 05.
 Add a couple new patchs (24 thru 32) to fix/clarify the remaining
 i8254_load() issues.

H Hartley Sweeten (32):
   staging: comedi: cb_pcidas: don't calc ai pacer divisors twice
   staging: comedi: cb_pcidas: don't calc ao pacer divisors twice
   staging: comedi: das16m1: don't calc pacer divisors twice
   staging: comedi: das1800: refactor Step 4 of das1800_ai_do_cmdtest()
   staging: comedi: das1800: don't calc pacer divisors twice
   staging: comedi: 8253.h: rename i8253_cascade_ns_to_timer_2div()
   staging: comedi: 8253.h: remove the unused i8253_cascade_ns_to_timer_*()
   staging: comedi: adl_pci9111: tidy up (*do_cmdtest) Step 4
   staging: comedi: addi_apci_2032: tidy up cmd use in apci2032_interrupt()
   staging: comedi: amplc_pci224: remove 'ai_stop_continuous' from private data
   staging: comedi: amplc_pci230: remove 'ai_continuous' from private data
   staging: comedi: amplc_pci230: remove 'ao_continuous' from private data
   staging: comedi: addi_common.h: remove 'ui_AiFlags' from private data
   staging: comedi: addi_common.h: remove 'ui_AiScanLength' from private data
   staging: comedi: addi_common.h: remove 'pui_AiChannelList' from private data
   staging: comedi: addi_common.h: remove 'ui_AiTimer0' from private data
   staging: comedi: addi_common.h: remove 'ui_AiTimer1' from private data
   staging: comedi: addi_common.h: remove 'ui_AiDataLength' from private data
   staging: comedi: addi_common.h: remove 'ui_AiNbrofScans' from private data
   staging: comedi: addi_common.h: remove 'b_AiContinuous' from private data
   staging: comedi: hwdrv_apci3120: cmd-convert_src is always TRIG_TIMER
   staging: comedi: hwdrv_apci3120: fix 'b_AiCyclicAcquisition' usage
   staging: comedi: hwdrv_apci3120: remove clearing of 'b_OutputMemoryStatus'
   staging: comedi: pcl711: fix 8254 timer programming
   staging: comedi: ni_at_ao: fix 8254 timer programming
   staging: comedi: me4000: fix 8254 timer programming
   staging: comedi: amplc_pci244: clarify 8254 timer programming
   staging: comedi: das800: clarify 8254 timer programming
   staging: comedi: ni_labpc: fix 8254 timer programming
   staging: comedi: ni_at_a2150: clarify 8254 timer programming
   staging: comedi: das16m1: clarify 8254 timer programming
   staging: comedi: das16: clarify 8254 timer programming

  drivers/staging/comedi/drivers/8253.h  | 105 +
  .../staging/comedi/drivers/addi-data/addi_common.h |  10 +-
  .../comedi/drivers/addi-data/hwdrv_apci3120.c  | 163 +++--
  drivers/staging/comedi/drivers/addi_apci_2032.c|  20 +--
  drivers/staging/comedi/drivers/adl_pci9111.c   |  38 ++---
  drivers/staging/comedi/drivers/amplc_pci224.c  |  30 ++--
  drivers/staging/comedi/drivers/amplc_pci230.c  |  47 ++
  drivers/staging/comedi/drivers/cb_pcidas.c |  49 +++
  drivers/staging/comedi/drivers/das16.c |   7 +-
  drivers/staging/comedi/drivers/das16m1.c   |  48 +++---
  drivers/staging/comedi/drivers/das1800.c   | 147 ++-
  drivers/staging/comedi/drivers/das800.c|  21 +--
  drivers/staging/comedi/drivers/me4000.c|   8 +-
  drivers/staging/comedi/drivers/ni_at_a2150.c   |   4 +-
  drivers/staging/comedi/drivers/ni_at_ao.c  |   8 +-
  drivers/staging/comedi/drivers/ni_labpc.c  |  92 +---
  drivers/staging/comedi/drivers/pcl711.c|  19 ++-
  17 files changed, 274 insertions(+), 542 deletions(-)



Looks good!

As a side node, I wonder if it's worth stripping out those `| 
I8254_BINARY` bits as it's basically 'OR'ing with zero anyway.


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

--
-=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH 00/32] staging: comedi: continue async command cleanup

2014-04-30 Thread Hartley Sweeten
On Wednesday, April 30, 2014 2:13 AM, Ian Abbott wrote:
 On 2014-04-29 20:59, H Hartley Sweeten wrote:
 Remove some unnecessary pacer divisor calculations. The divisors are 
 calculated
 as part of the (*do_cmdtest) and don't need done in the (*do_cmd).

 Remomove the older, unused, divisor calc functions in 8253.h to avoid any
 confusion.

 Remove some unnecessary private data members in a couple drivers and the
 addi_common.h header.

 Tidy up hwdrv_apci3120 a bit.

 Fix a couple 8254 timer programming issues. As Ian Abbott pointed out,
 the i8254_load() function does not use the I8254_MODE* defines. Convert
 all drivers to use the i8254_set_mode()/i8254_write() sequence instead.

 v2: Fix some i8254_load() issued pointed out by Ian Abbott in patches
  01, 02, 03, and 05.
  Add a couple new patchs (24 thru 32) to fix/clarify the remaining
  i8254_load() issues.

 H Hartley Sweeten (32):
[snip]

 Looks good!

 As a side node, I wonder if it's worth stripping out those `| 
 I8254_BINARY` bits as it's basically 'OR'ing with zero anyway.

I like the I8254_BINARY, it documents the mode that the counter
is used in. But, if you want to strip them out...

BTW, I noticed that the i8254_set_mode() helpers have a slight bug.

if (mode  (I8254_MODE5 | I8254_BINARY))
return -1;

This test will not allow the mode (I8254_MODE5 | I8254_BCD). Nothing
uses it right now, and it's a bit silly to use BCD counting anyway. But...

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

Thanks,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 00/32] staging: comedi: continue async command cleanup

2014-04-29 Thread H Hartley Sweeten
Remove some unnecessary pacer divisor calculations. The divisors are calculated
as part of the (*do_cmdtest) and don't need done in the (*do_cmd).

Remomove the older, unused, divisor calc functions in 8253.h to avoid any
confusion.

Remove some unnecessary private data members in a couple drivers and the
addi_common.h header.

Tidy up hwdrv_apci3120 a bit.

Fix a couple 8254 timer programming issues. As Ian Abbott pointed out,
the i8254_load() function does not use the I8254_MODE* defines. Convert
all drivers to use the i8254_set_mode()/i8254_write() sequence instead.

v2: Fix some i8254_load() issued pointed out by Ian Abbott in patches
01, 02, 03, and 05.
Add a couple new patchs (24 thru 32) to fix/clarify the remaining
i8254_load() issues.

H Hartley Sweeten (32):
  staging: comedi: cb_pcidas: don't calc ai pacer divisors twice
  staging: comedi: cb_pcidas: don't calc ao pacer divisors twice
  staging: comedi: das16m1: don't calc pacer divisors twice
  staging: comedi: das1800: refactor Step 4 of das1800_ai_do_cmdtest()
  staging: comedi: das1800: don't calc pacer divisors twice
  staging: comedi: 8253.h: rename i8253_cascade_ns_to_timer_2div()
  staging: comedi: 8253.h: remove the unused i8253_cascade_ns_to_timer_*()
  staging: comedi: adl_pci9111: tidy up (*do_cmdtest) Step 4
  staging: comedi: addi_apci_2032: tidy up cmd use in apci2032_interrupt()
  staging: comedi: amplc_pci224: remove 'ai_stop_continuous' from private data
  staging: comedi: amplc_pci230: remove 'ai_continuous' from private data
  staging: comedi: amplc_pci230: remove 'ao_continuous' from private data
  staging: comedi: addi_common.h: remove 'ui_AiFlags' from private data
  staging: comedi: addi_common.h: remove 'ui_AiScanLength' from private data
  staging: comedi: addi_common.h: remove 'pui_AiChannelList' from private data
  staging: comedi: addi_common.h: remove 'ui_AiTimer0' from private data
  staging: comedi: addi_common.h: remove 'ui_AiTimer1' from private data
  staging: comedi: addi_common.h: remove 'ui_AiDataLength' from private data
  staging: comedi: addi_common.h: remove 'ui_AiNbrofScans' from private data
  staging: comedi: addi_common.h: remove 'b_AiContinuous' from private data
  staging: comedi: hwdrv_apci3120: cmd-convert_src is always TRIG_TIMER
  staging: comedi: hwdrv_apci3120: fix 'b_AiCyclicAcquisition' usage
  staging: comedi: hwdrv_apci3120: remove clearing of 'b_OutputMemoryStatus'
  staging: comedi: pcl711: fix 8254 timer programming
  staging: comedi: ni_at_ao: fix 8254 timer programming
  staging: comedi: me4000: fix 8254 timer programming
  staging: comedi: amplc_pci244: clarify 8254 timer programming
  staging: comedi: das800: clarify 8254 timer programming
  staging: comedi: ni_labpc: fix 8254 timer programming
  staging: comedi: ni_at_a2150: clarify 8254 timer programming
  staging: comedi: das16m1: clarify 8254 timer programming
  staging: comedi: das16: clarify 8254 timer programming

 drivers/staging/comedi/drivers/8253.h  | 105 +
 .../staging/comedi/drivers/addi-data/addi_common.h |  10 +-
 .../comedi/drivers/addi-data/hwdrv_apci3120.c  | 163 +++--
 drivers/staging/comedi/drivers/addi_apci_2032.c|  20 +--
 drivers/staging/comedi/drivers/adl_pci9111.c   |  38 ++---
 drivers/staging/comedi/drivers/amplc_pci224.c  |  30 ++--
 drivers/staging/comedi/drivers/amplc_pci230.c  |  47 ++
 drivers/staging/comedi/drivers/cb_pcidas.c |  49 +++
 drivers/staging/comedi/drivers/das16.c |   7 +-
 drivers/staging/comedi/drivers/das16m1.c   |  48 +++---
 drivers/staging/comedi/drivers/das1800.c   | 147 ++-
 drivers/staging/comedi/drivers/das800.c|  21 +--
 drivers/staging/comedi/drivers/me4000.c|   8 +-
 drivers/staging/comedi/drivers/ni_at_a2150.c   |   4 +-
 drivers/staging/comedi/drivers/ni_at_ao.c  |   8 +-
 drivers/staging/comedi/drivers/ni_labpc.c  |  92 +---
 drivers/staging/comedi/drivers/pcl711.c|  19 ++-
 17 files changed, 274 insertions(+), 542 deletions(-)

-- 
1.9.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH 00/32] staging: comedi: continue async command cleanup

2014-04-29 Thread Hartley Sweeten
On Tuesday, April 29, 2014 12:59 PM, H Hartley Sweeten wrote:
 Remove some unnecessary pacer divisor calculations. The divisors are 
 calculated
 as part of the (*do_cmdtest) and don't need done in the (*do_cmd).

 Remove the older, unused, divisor calc functions in 8253.h to avoid any
 confusion.

 Remove some unnecessary private data members in a couple drivers and the
 addi_common.h header.

 Tidy up hwdrv_apci3120 a bit.

 Fix a couple 8254 timer programming issues. As Ian Abbott pointed out,
 the i8254_load() function does not use the I8254_MODE* defines. Convert
 all drivers to use the i8254_set_mode()/i8254_write() sequence instead.

 v2: Fix some i8254_load() issued pointed out by Ian Abbott in patches
 01, 02, 03, and 05.
 Add a couple new patchs (24 thru 32) to fix/clarify the remaining
 i8254_load() issues.

Ugh... Forgot to tag these as v2. Sorry about that.

Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel