RE: [PATCH 00/11] staging: comedi: s626: more cleanups

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Friday, October 18, 2013 6:41 AM, Ian Abbott wrote:
> Tidy up the "s626" driver a bit more.  In particular, the handling of
> encoder setup values is a bit hard to follow with lots of bit-shifting
> and masking, particularly when converting between the "standardized"
> setup values and the 'CRA' and 'CRB' register values.  (There are 6
> encoders in all, split between 3 pairs of encoders, 'A' and 'B', with
> each pair controlled by a pair of 'CRA' and 'CRB' registers.  The 'A'
> encoder is mostly set-up by the 'CRA' register and the 'B' encoder is
> mostly set-up by the 'CRB' register, but both registers are involved in
> setting up each encoder.)
>
> Also, avoid some confusion between the overall mode of the encoder
> specified in the "standardized" setup value and the "counter source"
> value in the hardware, expand the "index source" values in the
> standardized setup value to cover all the hardware values to keep things
> simple, and remove some macros that duplicate the values of other
> macros.
>
> 01) staging: comedi: s626: clock polarity and direction are the same
> 02) staging: comedi: s626: specify bitshift for encoder A clock source
> 03) staging: comedi: s626: correct a comment in s626_get_mode_b()
> 04) staging: comedi: s626: distinguish counter src from encoder mode
> 05) staging: comedi: s626: correct S626_CRAMSK_CLKPOL_A macro (unused)
> 06) staging: comedi: s626: add missing bits for 'CRB' register
> 07) staging: comedi: s626: bitfield manipulation macros for CRA, CRB and
>     setup
> 08) staging: comedi: s626: make CRA and CRB setup conversions more
>     readable
> 09) staging: comedi: s626: expand standardized IndxSrc values
> 10) staging: comedi: s626: remove S626_BF_* macros
> 11) staging: comedi: s626: replace S626_MULT_X? values
>
>  drivers/staging/comedi/drivers/s626.c | 379 ++++++++++++++++------------------
>  drivers/staging/comedi/drivers/s626.h | 306 ++++++++++++++++++++-------
>  2 files changed, 416 insertions(+), 269 deletions(-)

Hmm... I'm a bit indifferent on this series.

It does cleanup the driver somewhat but the header is even more confusing.

Regardless everything looks correct so:

Reviewed-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux