On Wed, May 30, 2012 at 02:14:43PM +0100, Ian Abbott wrote: > On 2012-05-30 12:51, 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. > > It seems most of this increase in size was due to one of my patches > (PATCH 6/8) accidentally removing the static declarator from the > pc236_find_pci function. If I add the static declarator back on > that function, there is no increase in code size unless the > .attach_pci hook is set to pc236_attach_pci(), and if I start > pc236_attach_pci() with the code > > if (!IS_ENABLED(CONFIG_COMEDI_AMPLC_PC236_PCI)) > return -EINVAL; > > the increase in size of the amplc_pc236.ko file is only 145 bytes > and a fair chunk of that increase is only metadata. > > I propose to remove most of the #if code apart from the PCI device > table, struct pci_driver and its probe/remove functions, and parts > of the the pc236_boards[] data array. This should make the code a > lot neater for a modest increase in code size. That sounds like a very good tradeoff. thanks, greg k-h _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
