Messages in this thread | | | Date | Thu, 5 Jun 2003 20:10:13 +0100 | From | Dave Jones <> | Subject | Re: [BK PATCH] PCI and PCI Hotplug changes and fixes for 2.5.70 |
| |
On Thu, Jun 05, 2003 at 10:18:35AM -0700, Greg KH wrote:
> > The fact that a tree-wide 'cleanup' like this goes in just a few hours > > after its posted before chance to comment is another argument, but > > concentrating on the technical point here, I still think this is a > > step backwards. > > Why? I just got rid of a function (well macro) that isn't even needed > (as proven by replacing it with an existing function.) That's > technically a good thing :)
Not when that replacement reduces readability, which in the case of agpgart is all I care about wrt these changes.
> Now I can agree that some of those replacements could be done with a > different function call, as almost none of the replacements in the > driver/* tree really want to walk all of the pci devices in the tree. > They usually just want to walk all devices of a type of pci device (be > it capability, or other trait.) I'd be glad to take changes of this > sort in the future.
Ok, for the ones I'm interested in, (agpgart), I don't see how things can get much cleaner than they used to be.
07 - hammer. Needs to walk the whole list, matching pci devices on bus 0, func 3, slots >23 & <32 This set of rules is so specialised, I see it hard to concieve how a generic helper function for the pci layer could be written. 08 - generic. Needs to know about every AGP device on the bus. ok, a for_each_agp_dev may actually make life easier here, but as its the only place this happens, consider it inlined, using pci_for_each_dev 09 - isoch. Could also use a 'for_each_agp_dev', but to be honest, it shouldn't be scanning at all, but being passed what it needs.
For the time being, I'm actually tempted to hack up a 'for_each_agp_dev' for the latter two, but 07 still bugs me.
Dave
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |