Re: [BK PATCH] PCI and PCI Hotplug changes and fixes for 2.5.70

Dave Jones (davej@codemonkey.org.uk)
Thu, 5 Jun 2003 20:10:13 +0100


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/