Re: [PATCH] staging: comedi: clarify/unify macros for NI macro-defined terminals

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

 



On Wed, Oct 24, 2018 at 10:39 AM Ian Abbott <abbotti@xxxxxxxxx> wrote:
>
> On 24/10/18 15:33, Spencer E. Olson wrote:
> > Uses a single macro to define multiple macros that represent a series of
> > terminals for NI devices.  This patch also redefines NI_MAX_COUNTERS as the
> > maximum number of counters possible on NI devices (instead of the maximum
> > index of the counters).  This was a little confusing and caused a bug in
> > commit 347e244884c3b ("staging: comedi: tio: implement global tio/ctr routing")
> > when setting/reading registers for counter terminals.
> >
> > Fixes: 347e244884c3b ("staging: comedi: tio: implement global tio/ctr routing")
> > Signed-off-by: Spencer E. Olson <olsonse@xxxxxxxxx>
> > ---
> >   drivers/staging/comedi/comedi.h | 39 ++++++++++++++++++---------------
> >   1 file changed, 21 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/staging/comedi/comedi.h b/drivers/staging/comedi/comedi.h
> > index e90b17775284..09a940066c0e 100644
> > --- a/drivers/staging/comedi/comedi.h
> > +++ b/drivers/staging/comedi/comedi.h
> > @@ -1005,35 +1005,38 @@ enum i8254_mode {
> >    * and INSN_DEVICE_CONFIG_GET_ROUTES.
> >    */
> >   #define NI_NAMES_BASE       0x8000u
> > +
> > +#define _TERM_N(base, n, x)  ((base) + ((x) & ((n) - 1)))
> > +
> >   /*
> >    * not necessarily all allowed 64 PFIs are valid--certainly not for all devices
> >    */
> > -#define NI_PFI(x)    (NI_NAMES_BASE        + ((x) & 0x3f))
> > +#define NI_PFI(x)            _TERM_N(NI_NAMES_BASE, 64, x)
> >   /* 8 trigger lines by standard, Some devices cannot talk to all eight. */
> > -#define TRIGGER_LINE(x)      (NI_PFI(-1)       + 1 + ((x) & 0x7))
> > +#define TRIGGER_LINE(x)              _TERM_N(NI_PFI(-1) + 1, 8, x)
> >   /* 4 RTSI shared MUXes to route signals to/from TRIGGER_LINES on NI hardware */
> > -#define NI_RTSI_BRD(x)       (TRIGGER_LINE(-1) + 1 + ((x) & 0x3))
> > +#define NI_RTSI_BRD(x)               _TERM_N(TRIGGER_LINE(-1) + 1, 4, x)
> >
> >   /* *** Counter/timer names : 8 counters max *** */
> > -#define NI_COUNTER_NAMES_BASE  (NI_RTSI_BRD(-1)  + 1)
> > -#define NI_MAX_COUNTERS             7
> > -#define NI_CtrSource(x)             (NI_COUNTER_NAMES_BASE + ((x) & NI_MAX_COUNTERS))
> > +#define NI_MAX_COUNTERS              8
> > +#define NI_COUNTER_NAMES_BASE        (NI_RTSI_BRD(-1)  + 1)
> > +#define NI_CtrSource(x)            _TERM_N(NI_COUNTER_NAMES_BASE, NI_MAX_COUNTERS, x)
> >   /* Gate, Aux, A,B,Z are all treated, at times as gates */
> > -#define NI_GATES_NAMES_BASE    (NI_CtrSource(-1) + 1)
> > -#define NI_CtrGate(x)               (NI_GATES_NAMES_BASE   + ((x) & NI_MAX_COUNTERS))
> > -#define NI_CtrAux(x)        (NI_CtrGate(-1)   + 1  + ((x) & NI_MAX_COUNTERS))
> > -#define NI_CtrA(x)          (NI_CtrAux(-1)    + 1  + ((x) & NI_MAX_COUNTERS))
> > -#define NI_CtrB(x)          (NI_CtrA(-1)      + 1  + ((x) & NI_MAX_COUNTERS))
> > -#define NI_CtrZ(x)          (NI_CtrB(-1)      + 1  + ((x) & NI_MAX_COUNTERS))
> > -#define NI_GATES_NAMES_MAX     NI_CtrZ(-1)
> > -#define NI_CtrArmStartTrigger(x) (NI_CtrZ(-1)    + 1  + ((x) & NI_MAX_COUNTERS))
> > +#define NI_GATES_NAMES_BASE  (NI_CtrSource(-1) + 1)
> > +#define NI_CtrGate(x)                _TERM_N(NI_GATES_NAMES_BASE, NI_MAX_COUNTERS, x)
> > +#define NI_CtrAux(x)         _TERM_N(NI_CtrGate(-1)  + 1, NI_MAX_COUNTERS, x)
> > +#define NI_CtrA(x)           _TERM_N(NI_CtrAux(-1)   + 1, NI_MAX_COUNTERS, x)
> > +#define NI_CtrB(x)           _TERM_N(NI_CtrA(-1)     + 1, NI_MAX_COUNTERS, x)
> > +#define NI_CtrZ(x)           _TERM_N(NI_CtrB(-1)     + 1, NI_MAX_COUNTERS, x)
> > +#define NI_GATES_NAMES_MAX   NI_CtrZ(-1)
> > +#define NI_CtrArmStartTrigger(x) _TERM_N(NI_CtrZ(-1)    + 1, NI_MAX_COUNTERS, x)
> >   #define NI_CtrInternalOutput(x) \
> > -                  (NI_CtrArmStartTrigger(-1)  + 1  + ((x) & NI_MAX_COUNTERS))
> > +                   _TERM_N(NI_CtrArmStartTrigger(-1) + 1, NI_MAX_COUNTERS, x)
> >   /** external pin(s) labeled conveniently as Ctr<i>Out. */
> > -#define NI_CtrOut(x)  (NI_CtrInternalOutput(-1)  + 1  + ((x) & NI_MAX_COUNTERS))
> > +#define NI_CtrOut(x)   _TERM_N(NI_CtrInternalOutput(-1) + 1, NI_MAX_COUNTERS, x)
> >   /** For Buffered sampling of ctr -- x series capability. */
> > -#define NI_CtrSampleClock(x) (NI_CtrOut(-1)   + 1  + ((x) & NI_MAX_COUNTERS))
> > -#define NI_COUNTER_NAMES_MAX   NI_CtrSampleClock(-1)
> > +#define NI_CtrSampleClock(x) _TERM_N(NI_CtrOut(-1)   + 1, NI_MAX_COUNTERS, x)
> > +#define NI_COUNTER_NAMES_MAX NI_CtrSampleClock(-1)
> >
> >   enum ni_common_signal_names {
> >       /* PXI_Star: this is a non-NI-specific signal */
> >
>
> It's no more ugly than the original, although I pity the fool who has to
> make sense of the preprocessor output!

Unfortunately, I do agree.  Oh well.

>
> Reviewed-by: Ian Abbott <abbotti@xxxxxxxxx>
>
> --
> -=( Ian Abbott <abbotti@xxxxxxxxx> || Web: www.mev.co.uk )=-
> -=( MEV Ltd. is a company registered in England & Wales. )=-
> -=( Registered number: 02862268.  Registered address:    )=-
> -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
_______________________________________________
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