On Friday, October 18, 2013 6:41 AM, Ian Abbott wrote: > Use the new macros defined in "s626.h" for constructing and decomposing > 'CRA', 'CRB' and standardized encoder setup values to make the > conversions between standardized encoder setup values, and CRA/CRB > register values easier to follow. > > There is some messing about with the 'IndxSrc' values which are 1-bit > wide in the standardized encoder setup, and 2-bit wide in the 'CRA' and > 'CRB' register values. This will be addressed by a later patch. > > Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx> > --- > drivers/staging/comedi/drivers/s626.c | 317 ++++++++++++++++------------------ > 1 file changed, 151 insertions(+), 166 deletions(-) > > diff --git a/drivers/staging/comedi/drivers/s626.c b/drivers/staging/comedi/drivers/s626.c > index 92062ed..ce320b3 100644 > --- a/drivers/staging/comedi/drivers/s626.c > +++ b/drivers/staging/comedi/drivers/s626.c [snip] > @@ -765,50 +766,46 @@ static uint16_t s626_get_mode_b(struct comedi_device *dev, > * Populate the standardized counter setup bit fields. > * Note: IndexSrc is restricted to ENC_X or IndxPol. > */ > - setup = ((crb << (S626_STDBIT_INTSRC - S626_CRBBIT_INTSRC_B)) & > - S626_STDMSK_INTSRC) | /* IntSrc = IntSrcB. */ > - ((crb << (S626_STDBIT_LATCHSRC - S626_CRBBIT_LATCHSRC)) & > - S626_STDMSK_LATCHSRC) | /* LatchSrc = LatchSrcB. */ > - ((crb << (S626_STDBIT_LOADSRC - S626_CRBBIT_LOADSRC_B)) & > - S626_STDMSK_LOADSRC) | /* LoadSrc = LoadSrcB. */ > - ((crb << (S626_STDBIT_INDXPOL - S626_CRBBIT_INDXPOL_B)) & > - S626_STDMSK_INDXPOL) | /* IndxPol = IndxPolB. */ > - ((crb >> (S626_CRBBIT_CLKENAB_B - S626_STDBIT_CLKENAB)) & > - S626_STDMSK_CLKENAB) | /* ClkEnab = ClkEnabB. */ > - ((cra >> ((S626_CRABIT_INDXSRC_B + 1) - S626_STDBIT_INDXSRC)) & > - S626_STDMSK_INDXSRC); /* IndxSrc = IndxSrcB<1>. */ > + setup = > + /* IntSrc = IntSrcB. */ > + S626_SET_STD_INTSRC(S626_GET_CRB_INTSRC_B(crb)) | > + /* LatchSrc = LatchSrcB. */ > + S626_SET_STD_LATCHSRC(S626_GET_CRB_LATCHSRC(crb)) | > + /* LoadSrc = LoadSrcB. */ > + S626_SET_STD_LOADSRC(S626_GET_CRB_LOADSRC_B(crb)) | > + /* IndxPol = IndxPolB. */ > + S626_SET_STD_INDXPOL(S626_GET_CRB_INDXPOL_B(crb)) | > + /* ClkEnab = ClkEnabB. */ > + S626_SET_STD_CLKENAB(S626_GET_CRB_CLKENAB_B(crb)) | > + /* IndxSrc = IndxSrcB<1>. */ > + S626_SET_STD_INDXSRC(S626_GET_CRA_INDXSRC_B(cra) >> 1); > > /* Adjust mode-dependent parameters. */ > - if ((crb & S626_CRBMSK_CLKMULT_B) == > - (S626_MULT_X0 << S626_CRBBIT_CLKMULT_B)) { > + cntsrc = S626_GET_CRA_CNTSRC_B(cra); > + clkmult = S626_GET_CRB_CLKMULT_B(crb); > + if (clkmult == S626_MULT_X0) { > /* Extender mode (ClkMultB == S626_MULT_X0): */ > - /* Indicate Extender mode. */ > - setup |= S626_ENCMODE_EXTENDER << S626_STDBIT_ENCMODE; > + encmode = S626_ENCMODE_EXTENDER; > /* Indicate multiplier is 1x. */ > - setup |= S626_MULT_X1 << S626_STDBIT_CLKMULT; > + clkmult = S626_MULT_X1; > /* Set ClkPol equal to Timer count direction (CntSrcB<0>). */ > - setup |= (cra >> (S626_CRABIT_CNTSRC_B - S626_STDBIT_CLKPOL)) & > - S626_STDMSK_CLKPOL; > - } else if (cra & (S626_CNTSRC_SYSCLK << S626_CRABIT_CNTSRC_B)) { > + clkpol = cntsrc & 1; > + } else if (cntsrc & S626_CNTSRC_SYSCLK) { > /* Timer mode (CntSrcB<1> == 1): */ > - /* Indicate Timer mode. */ > - setup |= S626_ENCMODE_TIMER << S626_STDBIT_ENCMODE; > + encmode = S626_ENCMODE_TIMER; > /* Indicate multiplier is 1x. */ > - setup |= S626_MULT_X1 << S626_STDBIT_CLKMULT; > + clkmult = S626_MULT_X1; > /* Set ClkPol equal to Timer count direction (CntSrcB<0>). */ > - setup |= (cra >> (S626_CRABIT_CNTSRC_B - S626_STDBIT_CLKPOL)) & > - S626_STDMSK_CLKPOL; > + setup |= S626_SET_STD_CLKPOL(cntsrc & 1); Oops missed this. Based on the other changes I think you want this one to be: clkpol = cntsrc & 1; [snip] > + setup |= S626_SET_STD_ENCMODE(encmode) | S626_SET_STD_CLKMULT(clkmult) | > + S626_SET_STD_CLKPOL(clkpol); Otherwise the code above produces this warning: drivers/staging/comedi/drivers/s626.c: In function 's626_get_mode_b': drivers/staging/comedi/drivers/s626.c:805:8: warning: 'clkpol' may be used uninitialized in this function [-Wmaybe-uninitialized] Regards, Hartley _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel