Hi Yakui, * yakui_zhao <yakui.zhao@xxxxxxxxx>: > From: Zhao Yakui <yakui.zhao@xxxxxxxxx> > > Sometimes both acpi video and i915 driver are compiled as modules. > And there exists the strict dependency between the two drivers. > The acpi video bus will be unloaded in course of unloading the i915 driver. > If we unload the acpi video driver, then the kernel oops will be triggered. > > Add the reference count to avoid unloading the ACPI video bus twice. > The reference count should be checked before unregistering the acpi video bus. > If the reference count is already zero, it won't unregister it again. > And after the acpi video bus is already unregistered, the reference count > will be set to zero. Your implementation below isn't really a reference count, so I don't think you should call it that in your changelog. Since you are talking about refcounts, have you tried using try_module_get()? Thanks. /ac > > http://bugzilla.kernel.org/show_bug.cgi?id=13396 > > Signed-off-by: Zhao Yakui <yakui.zhao@xxxxxxxxx> > --- > drivers/acpi/video.c | 41 +++++++++++++++++++++++++++++------ > drivers/gpu/drm/i915/i915_opregion.c | 2 - > include/acpi/video.h | 4 +-- > 3 files changed, 38 insertions(+), 9 deletions(-) > > Index: linux-2.6/drivers/acpi/video.c > =================================================================== > --- linux-2.6.orig/drivers/acpi/video.c 2009-06-16 10:25:12.000000000 +0800 > +++ linux-2.6/drivers/acpi/video.c 2009-06-16 11:09:57.000000000 +0800 > @@ -76,6 +76,7 @@ > static int brightness_switch_enabled = 1; > module_param(brightness_switch_enabled, bool, 0644); > > +static int register_count = 0; > static int acpi_video_bus_add(struct acpi_device *device); > static int acpi_video_bus_remove(struct acpi_device *device, int type); > static int acpi_video_resume(struct acpi_device *device); > @@ -2318,6 +2319,13 @@ > int acpi_video_register(void) > { > int result = 0; > + if (register_count) { > + /* > + * if the function of acpi_video_register is already called, > + * don't register the acpi_vide_bus again and return no error. > + */ > + return 0; > + } > > acpi_video_dir = proc_mkdir(ACPI_VIDEO_CLASS, acpi_root_dir); > if (!acpi_video_dir) > @@ -2329,10 +2337,35 @@ > return -ENODEV; > } > > + /* > + * When the acpi_video_bus is loaded successfully, increase > + * the counter reference. > + */ > + register_count = 1; > + > return 0; > } > EXPORT_SYMBOL(acpi_video_register); > > +void acpi_video_unregister(void) > +{ > + if (!register_count) { > + /* > + * If the acpi video bus is already unloaded, don't > + * unload it again and return directly. > + */ > + return; > + } > + acpi_bus_unregister_driver(&acpi_video_bus); > + > + remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir); > + > + register_count = 0; > + > + return; > +} > +EXPORT_SYMBOL(acpi_video_unregister); > + > /* > * This is kind of nasty. Hardware using Intel chipsets may require > * the video opregion code to be run first in order to initialise > @@ -2350,16 +2383,12 @@ > return acpi_video_register(); > } > > -void acpi_video_exit(void) > +static void __exit acpi_video_exit(void) > { > - > - acpi_bus_unregister_driver(&acpi_video_bus); > - > - remove_proc_entry(ACPI_VIDEO_CLASS, acpi_root_dir); > + acpi_video_unregister(); > > return; > } > -EXPORT_SYMBOL(acpi_video_exit); > > module_init(acpi_video_init); > module_exit(acpi_video_exit); > Index: linux-2.6/drivers/gpu/drm/i915/i915_opregion.c > =================================================================== > --- linux-2.6.orig/drivers/gpu/drm/i915/i915_opregion.c 2009-06-16 10:25:12.000000000 +0800 > +++ linux-2.6/drivers/gpu/drm/i915/i915_opregion.c 2009-06-16 10:29:50.000000000 +0800 > @@ -419,7 +419,7 @@ > return; > > if (!suspend) > - acpi_video_exit(); > + acpi_video_unregister(); > > opregion->acpi->drdy = 0; > > Index: linux-2.6/include/acpi/video.h > =================================================================== > --- linux-2.6.orig/include/acpi/video.h 2009-06-16 10:25:12.000000000 +0800 > +++ linux-2.6/include/acpi/video.h 2009-06-16 11:07:43.000000000 +0800 > @@ -3,10 +3,10 @@ > > #if (defined CONFIG_ACPI_VIDEO || defined CONFIG_ACPI_VIDEO_MODULE) > extern int acpi_video_register(void); > -extern int acpi_video_exit(void); > +extern void acpi_video_unregister(void); > #else > static inline int acpi_video_register(void) { return 0; } > -static inline void acpi_video_exit(void) { return; } > +static inline void acpi_video_unregister(void) { return; } > #endif > > #endif > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
