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 <abbo...@mev.co.uk>
> ---
>  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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to