Messages in this thread | | | Date | Wed, 20 Feb 2008 12:05:01 -0500 | From | Jeff Garzik <> | Subject | Re: [PATCH] Add PCI_DEVICE_TABLE macro |
| |
Greg KH wrote: > On Mon, Feb 18, 2008 at 08:34:42AM +0100, Jonas Bonn wrote: >>> And again, what does this buy us? >> Clarity and simplicity, I hope... there are a bunch of definitions >> scattered about the kernel that omit the __devinitdata modifier despite the >> documentation stating that it should always be there. The definition >> really should have been const, which wasn't possible before but has become >> so with the addition of the __devinitconst attribute. >> >> Furthermore, there are definitions that use "const" and __devinitdata, >> which is explicitly wrong but the compiler doesn't catch section mismatches >> if there's only one such one case in the module (which is often the case). >> >> Adding the __devinitconst modifier where there was nothing before buys us >> memory. Adding the const modifier gives the compiler a chance to do its >> thing. Changing __devinitdata to __devinitconst where it was wrong >> actually fixes some compiler errors in older (mid-release) kernels that >> were patched over by "removing" the section attribute altogether (which >> wastes memory). >> >> Adding the macro (Olof's idea, not mine) makes it pretty difficult to get >> this definition wrong... I'll do the rest of the cleanup, but I need to >> know whether it's better to use a macro like this, or to open code the >> definitions. I prefer the macro approach... >> >> Hope this makes some sense... > > Ok, yes it does, thanks for the explaination. > > Please, can you add this very good text to the changelog entry for the > addition of the macro, and to the documentation somewhere? I'd be glad > to take the patch if that was done.
I would suggest having a DECLARE_ prefix in there, like other subsystems do...
Jeff
| |