Hi Krzysztof, > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > > > index e51ed959da89..b98e8af6f1c1 100644 > > > --- a/drivers/gpu/drm/drm_drv.c > > > +++ b/drivers/gpu/drm/drm_drv.c > > > @@ -1140,6 +1140,8 @@ int drm_dev_register(struct drm_device *dev, > > > unsigned long flags) > > > drm_minor_unregister(dev, DRM_MINOR_ACCEL); > > > drm_minor_unregister(dev, DRM_MINOR_PRIMARY); > > > drm_minor_unregister(dev, DRM_MINOR_RENDER); > > > + drm_debugfs_dev_fini(dev); > > > > I think sashiko is right here, this should be removed. > > > > > + dev->registered = false; > > > > dev->registered = false should be set at the very beginning, > > maybe something like this: > > Why? Is .unload() supposed to expect dev->registered = false? I've seen > that moving the flag around might(?) make a bit of a mess (e0f32f78e51b), > and before this patch .unload() is being called with registered = true. > So changing it at the beginning of the unwind might mess up drivers that > rely on it, but it will make no difference for drivers that don't, so > I'd rather keep it this way. Unless we make explicit guarantees that > .unload() will be called with ->registered = false, in the middle of the > call to drm_dev_register().
that's what happens in drm_dev_unregister() and that's not a random choice. If you grep for "dev->registered", you will see that it's in the unload path and it can warn if it's true. Of course this does not affect all the drivers, like i915, but others might get upset. Andi
