On Wed, Jun 27, 2012 at 02:56:43PM -0700, H Hartley Sweeten wrote: > Add defines for the register map of the device. These will be > used to clarify the code. > > Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx> > Cc: Ian Abbott <abbotti@xxxxxxxxx> > Cc: Frank Mori Hess <fmhess@xxxxxxxxxxxxxxxxxxxxx> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > --- > drivers/staging/comedi/drivers/adl_pci6208.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/staging/comedi/drivers/adl_pci6208.c b/drivers/staging/comedi/drivers/adl_pci6208.c > index f949d20..b6a8439 100644 > --- a/drivers/staging/comedi/drivers/adl_pci6208.c > +++ b/drivers/staging/comedi/drivers/adl_pci6208.c > @@ -53,6 +53,18 @@ References: > */ > #include "../comedidev.h" > > +/* > + * PCI-6208/6216-GL register map > + */ > +#define PCI6208_AO_CONTROL(x) (0x00 + (2 * (x))) > +#define PCI6208_AO_STATUS 0x00 > +#define PCI6208_AO_STATUS_DATA_SEND (1 << 0) > +#define PCI6208_DIO 0x40 > +#define PCI6208_DIO_DO_MASK (0x0f) > +#define PCI6208_DIO_DO_SHIFT (0) > +#define PCI6208_DIO_DI_MASK (0xf0) > +#define PCI6208_DIO_DI_SHIFT (4) This series is nice and I'm not nacking anything, but really is it that useful to say: status = inw(dev->iobase + PCI6208_AO_STATUS); instead of just?: status = inw(dev->iobase); I'm not sure what the 0x00 in PCI6208_AO_CONTROL represents. Some of these are not used like PCI6208_DIO_DI_SHIFT. Does checkpatch.pl complain if you leave off these parenthesis? If so I will complain again to the checkpatch.pl people. Extra parenthesis are silly and there not used consistently. Only PCI6208_AO_CONTROL() and PCI6208_AO_STATUS_DATA_SEND() need paranthesis. Again, I'm fine with this patch and the whole series. These are just comments. regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
