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
