On Fri, Jun 08, 2012 at 01:09:10PM +0300, Jani Nikula wrote: > On Thu, 07 Jun 2012, Daniel Vetter <daniel.vetter at ffwll.ch> wrote: > > #define INTEL_GTT_GEN intel_private.driver->gen > > @@ -1522,14 +1523,32 @@ static int find_gmch(u16 device) > > return 1; > > } > > > > -int intel_gmch_probe(struct pci_dev *pdev, > > - struct agp_bridge_data *bridge) > > +int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev, > > + struct agp_bridge_data *bridge) > > { > > int i, mask; > > - intel_private.driver = NULL; > > A possibly clueless question: should intel_gmch_remove() do this now, > since intel_private.driver and intel_private.refcount are linked > together below? > > > + > > + /* > > + * Can be called from the fake agp driver but also directly from > > + * drm/i915.ko. Hence we need to check whether everything is set up > > + * already. > > + */ > > + if (intel_private.driver) { > > + intel_private.refcount++; > > + return 1; > > + } > > Judging by the commit message, is it intentional that the first call to > probe does not increase the refcount? The cleanup will now happen only > if probe and remove are called twice or more. Should this perhaps be > clarified in a code comment in addition to the commit message? Yeah, things went a bit wrong here. I'll also see whether I can make the cleanup path a bit more robust. Thanks for taking a look at this patch. -Daniel -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48