Re: [PATCH 2/3] staging: comedi: amplc_dio200: add helper macros to check bus type

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

 



On Wed, Aug 15, 2012 at 01:40:01PM +0100, Ian Abbott wrote:
> On 2012-08-15 11:48, Dan Carpenter wrote:
> >On Wed, Aug 15, 2012 at 11:16:20AM +0100, Ian Abbott wrote:
> >>On 2012-08-15 08:14, Dan Carpenter wrote:
> >>>On Tue, Aug 14, 2012 at 04:31:28PM +0100, Ian Abbott wrote:
> >>>>+#define IS_ISA_BOARD(board)	(DO_ISA && (board)->bustype == isa_bustype)
> >>>>+#define IS_PCI_BOARD(board)	(DO_PCI && (board)->bustype == pci_bustype)
> >>>
> >>>It would be better to make this an inline function.
> >>>
> >>>#if IS_ENABLED(CONFIG_COMEDI_AMPLC_DIO200_ISA)
> >>>
> >>>static inline bool is_isa_board(const struct dio200_board *board)
> >>>{
> >>>	return board->bustype == isa_bustype;
> >>>}
> >>>
> >>>#else
> >>>
> >>>static inline bool is_isa_board(const struct dio200_board *board)
> >>>{
> >>>	return 0;
> >>>}
> >>>
> >>>#endif
> 
> >>I prefer the macro as it seems "easier" for the compiler to
> >>recognize it as a constant in the case where DO_ISA expands to 0.
> 
> >If we were just discussing this one function then, sure, we're not
> >going to complain about small style issues, but it's the whole file.
> >It's just riddled with #ifdefs.  It can't be moved out of staging
> >until it is written according the same coding style as the rest of
> >the kernel.
> 
> The remaining #ifdefs are only used to fill in the list of supported
> boards, the static "layout" data referenced by the supported boards
> (the #ifdefs could be removed from the layout array with a moderate
> expansion of module size), and to optionally make it a PCI driver.

Hm...  You're right.  I guess that's not so bad then.

Except I really hate the DO_ISA macro.  It's clearer (more readable)
to just use IS_ENABLED().  The cool thing about IS_ENABLED() is that
it looks like a function and it shows you which CONFIG option has
to be enabled.  DO_ISA is a horrible name and a horrible macro.

regards,
dan carpenter


_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/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