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

Reply via email to