Hi Dmitry, Thanks for your review and suggestions first!
於 一，2013-04-22 於 21:12 -0700，Dmitry Torokhov 提到： > On Mon, Apr 22, 2013 at 08:39:15PM +0800, Chun-Yi Lee wrote: > > From: "Lee, Chun-Yi" <[email protected]> > > +static acpi_status > > +find_video_unregister_backlight(acpi_handle handle, u32 lvl, void *context, > > + void **rv) > > +{ > > + struct acpi_device *acpi_dev; > > + struct acpi_video_bus *video = NULL; > > Gratuitous initialization of local variables prevents compiler from > warning when you using variable uninitialized. Yes, I agreed should not put gratuitous initialization, I will remove it in v2 patch. > > > + struct acpi_video_device *dev, *next; > > + > > + if (acpi_bus_get_device(handle, &acpi_dev)) > > + return AE_OK; > > + > > + if (!acpi_match_device_ids(acpi_dev, video_device_ids)) { > > + video = acpi_driver_data(acpi_dev); > > + acpi_video_bus_stop_devices(video); > > + mutex_lock(&video->device_list_lock); > > + list_for_each_entry_safe(dev, next, &video->video_device_list, > > + entry) { > > + if (dev->backlight) { > > + backlight_device_unregister(dev->backlight); > > + dev->backlight = NULL; > > + kfree(dev->brightness->levels); > > + kfree(dev->brightness); > > + } > > + } > > + mutex_unlock(&video->device_list_lock); > > + acpi_video_bus_start_devices(video); > > + } > > + return AE_OK; > > +} > > + > > +void acpi_video_backlight_unregister(void) > > +{ > > + if (!register_count) { > > Locking? It looks like the rest of the driver ignores locking too... Did you mean locking video_device_list? The acpi/video locks video_device_list when add, remove and notify acpi video bus driver. It always do the mutex lock before control video_device_list, so I also add lock when unregister all backlight of devices. Thanks a lot! Joey Lee -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
