Re: [PATCH 8/8] staging: comedi: amplc_pc236: Add attach_pci() hook

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

 



On 2012-05-30 13:43, Dan Carpenter wrote:
On Wed, May 30, 2012 at 12:51:31PM +0100, Ian Abbott wrote:
On 2012-05-29 15:45, Dan Carpenter wrote:
On Tue, May 29, 2012 at 02:49:35PM +0100, Ian Abbott wrote:
Implement the attach_pci() hook as function pc236_attach_pci().  This is
called bu comedi_pci_auto_config() in preference to the old attach()
hook (implemented by pc236_attach()) and avoids searching for the probed
PCI device.

Factor out code common to pc236_find_pci() and pc236_attach_pci() into
new function pc236_find_pci_board().  Factor out most code common to
pc236_attach() and pc236_attach_pci() into new functions
pc236_pci_common_attach() and pc236_common_attach().

Also #if out member 'devid' from struct pc236_board unless PCI boards
are supported as it is not used for ISA boards.


I can't tell you how much I hate these ifdefs.  :(  They're every
where...  Who is it that cares about cares so much about 3 extra
unsigned shorts?

The rules are that #ifdefs are only ok in .h files and not in .c
files.  We should be removing them and not adding them if we want
to move this code out of staging.

As an experiment, with the following patch (sorry for the
line-wrapping which prevents it being applied) and with
CONFIG_COMEDI_AMPLC_PC236_ISA defined and
CONFIG_COMEDI_AMPLC_PC236_PCI not defined, the size of the
amplc_pc236.ko increased in size from 5785 bytes to 6215 bytes and
there was a compiler warning about pc236_attach_pci being defined
but not used (this is what the .attach_pci hook in struct
comedi_driver amplc_pc236_driver would get set to, but that is still
conditionally compiled out).  If I set the .attach_pci hook to
pc236_attach_pci regardless, it gets rid of the warning, but the
size of the amplc_pc236.ko file increased to 6776 bytes as more code
is reachable in theory although not in practice as the PCI probe
code that calls this function indirectly is still conditionally
compiled out.


I was talking about the devid.  And yeah.  It shaves of 40 bytes to
put the #ifdef there.  I can only see three structs that are
affected so I thought it would be less.  But you you never answered
my question about who cares???

Actually, if CONFIG_COMEDI_AMPLC_PC236_PCI isn't enabled, pc236_boards[] is only one element long, so disregarding padding it saves 2 bytes of data. As to your question, I guess no-one cares, but as all the code that used it was #if'ed out I thought I might as well #if out that structure member at the same time.

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
_______________________________________________
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